fix: A2A task still running at the deadline is reported as failed,…

[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 task still running at the deadline is reported as failed,…

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-pending-task-not-failed.md
• Or: explicitly marked this PR as docs / tests / evals-only (no changeset needed — see .changeset/README.md)

Related issues

Closes #395

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 client fix works, but the node still logs a task that is still running as a transport failure, which is one of the false reports #395 complains about.

The adapter side resolves #395. A task still submitted/working when the deadline passes or the caller aborts now rejects with A2ATaskPendingError. That error carries the task id and the reason, and it sets retryable = false. The driver checks retryable === false at node-execution-driver.ts:323 and stops, so the failure policy can't start a second remote task. The test that pinned failed has been updated. There are new tests for the timeout and the caller abort, and they assert exact values: taskId, reason, retryable and the full message. The abort test takes a real path: the abort fires during the poll, settle returns the last task it saw, and the error reports 'aborted'. The A2AClient contract JSDoc and the changeset are updated to match.

1 finding is posted as a comment on the diff.

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

_{Reviewed at 6de61aa · with claude-opus-5-5 · workflow run}

[wmcmahan]

Addressed the review feedback in the latest commit.

When the remote task is still running at the deadline, the a2a node now logs a separate a2a_task_pending event with the task_id. Before, it logged this case as a2a_transport_failed. The orchestrator can't import A2ATaskPendingError, so a small private helper pendingA2ATaskId in a2a.ts picks out this case by checking name === 'A2ATaskPendingError' and that taskId is a string, the same duck-typing the driver uses for retryable.

A new test file, packages/orchestrator/test/a2a-node-logging.test.ts, covers both cases: the pending log and the unchanged transport-failure log. It passes, as do the existing a2a-node.test.ts suite and npm run lint --workspace=packages/orchestrator. I also added .changeset/a2a-pending-task-log-event.md, a patch for cycgraph/orchestrator, because the log output changes.

The failing CI job, GitHub Advanced Security, doesn't point at anything in this PR: its log ends with CAPIError: 400 The requested model is not supported. from the scanning tool itself. No code change can fix that, so it needs a re-run or a fix to the action's setup.

_{Revised in ee34530 · 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 fix resolves #395: a task still running when the wait bound fires, or when the caller aborts, now rejects with a non-retryable A2ATaskPendingError that carries the task id. The revision logs that case as its own event.

What I checked in the tree:

• In deliver(), the pending check runs only after settle(). abortSignal?.aborted correctly tells a caller abort apart from a timeout, because the internal timeout uses its own controller. The catch rethrows A2ATaskPendingError before its "no task observed" branches, so the old unreachable abort message is now reachable only when it's actually true.
• isPending takes unknown and checks it against the pending spellings, so pendingTaskId is safe to call on whatever settle() returns.
• The driver stops retrying only when retryable === false. retryable = false on the new error is exactly what stops the node's failure policy from starting a duplicate remote task.
• The test that pinned failed at the deadline now asserts the error type, taskId and reason. The new tests cover the timeout path (including retryable) and the abort-after-task path, each with exact message assertions.

The issue suggested resuming the same task id as one option, and this PR doesn't do that. It takes the issue's fallback of not starting a fresh task, which ends the duplicate remote spend. That's a reasonable scope for this fix.

Both changesets are present, and the public-contract change is written into the A2AClient JSDoc. The revision is clean: it's typed carefully, and the logging test sits in its own file because it has to mock the logger module.

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

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