fix: A2A registry timeout_ms is documented but never read

[wmcmahan]

Summary

Filed by maintenance discovery (code-scan or repo-audit), approved by label, fixed by the issue-fix workflow. the tree was changed; the reviewer judges fidelity to the audited finding

Changes

• fix: A2A registry timeout_ms is documented but never read

Test plan

• All existing tests pass (npm test) — npm test ran clean in the workspace before commit
• New tests added for new functionality (unit + integration as appropriate) — 1 test file(s) changed
• Tested manually (describe how — link a runnable example if you wrote one) — not performed — automated fix, verified by re-scan and repository checks

Quality checklist

• Code follows the coding standards — advisory reviewer approved the diff against CLAUDE.md
• Zod schemas added for any new input/output boundary — not verified — confirm if the diff adds an input/output boundary
• No direct state mutation — all changes flow through reducers
• ES module imports use the .js extension — no extensionless relative imports added
• Cross-workspace imports use @cycgraph/* package names, not relative paths — no relative imports across package boundaries added
• No secrets, API keys, or .env contents in code or tests — no secret-shaped strings in the diff
• No new console.warn / console.error for things that should be observable — emit a stream event or use createLogger — none added in this diff

Database & data integrity

Not applicable — no changes under packages/orchestrator-postgres or the memory schemas.

Security

Not applicable — no changes to permission, MCP, taint, or budget paths.

Changeset

• Ran npx changeset and selected the right semver bump (patch / minor / major) — changeset included: .changeset/a2a-request-timeout.md
• Or: explicitly marked this PR as docs / tests / evals-only (no changeset needed — see .changeset/README.md)

Related issues

Closes #396

Provenance

issue-fix: the finding was re-located mechanically, fixed by an agent in a jailed clone, and verified by re-scan, a class-specific anti-gaming guard, and repository checks before commit.

[wmcmahan]

Advisory review by the pr-review workflow — the human merge decision stands either way.

VERDICT: REVISE
The fix resolves #396, but it also caps every blocking message/send at the 30s timeout_ms default, so remote tasks that take longer than 30s now fail.

The client-side race is clean: the delivery bound wins when both timers fire, a connect attempt that times out is retried within the budget, and the finally releases requests that were abandoned.

Overall

• packages/orchestrator/test/a2a-node.test.ts:187 The bug in #396 was that the node never read server.timeout_ms, and nothing tests that the node now forwards it. Add tests next to the maxRetries forwarding cases: one where the entry sets timeoutMs: 5_000 and capture.request.requestTimeoutMs is 5000, and one where the entry leaves it unset and the value is 30000.
  • The two new tests in packages/a2a/test/client.test.ts only cover the client. The a2a.ts change on lines 153 and 164 has no test.

1 finding is posted as a comment on the diff.

@cycgraph please address the open review threads and any findings above.

_{Reviewed at 171f731 · with claude-opus-5-5 · workflow run}

[wmcmahan]

Addressed the review feedback in the latest commit.

I took the second option from T1: message/send is back under timeoutMs only. The per-request timeout_ms still bounds each connection attempt and each status poll. The two forwarding tests from C1 are added. The a2a and orchestrator builds pass, and the client suite (56) and a2a node suite (52) pass.

• Code: the send goes through raceDeliveryBound again instead of raceRequestBound.

• Docs: I updated the createA2AClient JSDoc, A2ATaskRequest.requestTimeoutMs and the timeout_ms schema comment to say the send is exempt because it can block until the remote task finishes.

• Changeset: .changeset/a2a-request-timeout.md now says timeout_ms covers connection attempts and polls, and that the blocking send is still bounded only by task_timeout_ms.

• CI: the failing GitHub Advanced Security job failed on its own infrastructure (CAPIError: 400 The requested model is not supported), not on this code. No change can fix it; it needs a re-run.

• Re: the review summary The 30s send cap is gone (see T1), and next to the maxRetries cases the a2a node now has two tests: an entry with timeoutMs: 5_000 forwards requestTimeoutMs 5000, and an entry that leaves it unset forwards the 30000 default.

_{Revised in 8c47652 · with claude-opus-5-5 · workflow run}

[wmcmahan]

Advisory review by the pr-review workflow — the human merge decision stands either way.

VERDICT: APPROVE
The node now forwards timeout_ms as requestTimeoutMs, and cycgraph/a2a applies it to each connection attempt and each status poll. The earlier blocking-send problem and the missing node-level test are both fixed.

This mostly does what #396 asked. The one exception is the send, which the issue listed among the calls to bound. Exempting it was a deliberate choice made in response to the earlier review, because message/send blocks until the remote task finishes, and the schema comment, JSDoc and changeset all record it. raceRequestBound follows the existing raceAbort/raceDeliveryBound pattern and still reports a caller abort or an exhausted budget as such. The new client tests use exact assertions and fake timers, like the tests already in that file.

Resolved since the last review

• packages/orchestrator/test/a2a-node.test.ts:187 The bug in #396 was that the node never read server.timeout_ms, and nothing tests that the node now forwards it. Add tests next to the maxRetries forwarding cases: one where the entry sets timeoutMs: 5_000 and capture.request.requestTimeoutMs is 5000, and one where the entry leaves it unset and the value is 30000.
  • a2a-node.test.ts now has two forwarding tests next to the maxRetries cases. One checks that requestTimeoutMs is 5000 when the entry sets timeoutMs: 5_000. The other checks that it falls back to the schema default of 30000.

Earlier threads: 1 addressed, 0 still open. Each judgment is a reply in its thread.

_{Reviewed at 8c47652 · with claude-opus-5-5 · workflow run}