feat(a2a-claude): hold the A2A Task open while background work is in flight - #34
Merged
shashikanth-gs merged 14 commits intoAug 25, 2026
Merged
Conversation
Tracks the live set of Claude SDK background tasks from system/background_tasks_changed, which the SDK documents as a level signal with replace (not merge) semantics. Pure and dependency-free; not wired in yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tTTwsL7XNgwm4iXHmxqfb
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tTTwsL7XNgwm4iXHmxqfb
…nts flags Add two new feature flags to control background task lifecycle behavior. Both default to true: holdTaskForBackgroundWork holds the A2A Task open while background work is in flight; emitBackgroundTaskEvents publishes background-task set changes as sideband events. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tTTwsL7XNgwm4iXHmxqfb
…tasks event agent_started and agent_finished must fire exactly once per A2A Task, not once per SDK turn, now that a Task can span several turns while background work is in flight. EventMapper tracks sawInit/emittedFinished instance state to suppress duplicates, and handleResult takes a held flag so the executor can defer agent_finished until the turn that actually ends the Task. Also adds handleBackgroundTasks(), gated by features.emitBackgroundTaskEvents, so an orchestrator can see why a Task is sitting in working. Requires adding "background_tasks" to core's EventType union, following the same precedent as the rate_limit event type. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tTTwsL7XNgwm4iXHmxqfb
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tTTwsL7XNgwm4iXHmxqfb
Follow-up to 723afce: - Post-loop fallback no longer reports `completed` for a turn the prompt timer aborted. Guarded with `!timedOut`, plus an explicit timed-out branch so that path still emits a terminal event. - Explain why the rate-limit `break` need not pre-resolve the input deferred, unlike the other two breaks. No behaviour change. - Extract `publishRoundArtifact()` and `endTurnCompleted()` closures, removing the two byte-identical repeats in `turnFn`. - Add opt-in `hangUntilInputClosed` to the test fake so a query models a real subprocess that only exits once stdin closes, and cover the held-task-never-wakes path with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tTTwsL7XNgwm4iXHmxqfb
The post-loop fallback was guarded only on `timedOut`, so an abort that ended the iterator cleanly rather than throwing fell through to `completed`. After `cancelTask` had already published `canceled`, that handed the client two terminal events that contradict each other. Guard on `abortController.signal.aborted` instead, which covers both abort routes: a timeout publishes `failed` exactly once, and a cancel publishes nothing from `turnFn` because `cancelTask` already did. Both clean-end branches are now covered rather than defensive. Adds an opt-in `endCleanlyOnAbort` script flag to the test fake so the race is reachable at all; without it the fake always rejects on abort and the branches could not be exercised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tTTwsL7XNgwm4iXHmxqfb
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tTTwsL7XNgwm4iXHmxqfb
…d bookend
Six findings from the final review of the held-open task lifecycle.
`background_tasks` had no entry in core's `EVENT_TO_TRACE_KEY`, and
`A2ATransport.send` silently drops any event type it cannot map — so on the
shipped defaults (`events.enabled: true`, `events.transport: "a2a"`) the event
this branch documents was undeliverable. Adds a `trace.background_tasks`
mapping so the README's claim is true. The sideband event is not redundant
with `metadata.backgroundTasks` on the held status update: membership can
change several times within one round, and only the event carries those
transitions. `rate_limit` and `context_window` have the same gap; that is
pre-existing and left alone, but now documented rather than silent.
The post-loop "iterator ended while still held" fallback published `completed`
without ever emitting `agent_finished`: the last round's result had been
consumed with `{ held: true }`, which suppresses the bookend, and nothing
re-emitted it. A consumer pairing bookends leaked a span. The fallback now
closes it via `EventMapper.emitFinishedBookend`, which shares the
`emittedFinished` latch so no path can double-fire.
Test hardening, each proven by mutation:
- Both "closes the input stream" tests were vacuous — the `finally` resolves
`inputClosed` on every exit path, so deleting both in-loop resolves left
every test green. Adds a `returnAwaitsInputClosed` script option whose
`iterator.return()` drains the input pump, which wedges a `break` that did
not pre-close the stream. The success and error breaks are now pinned by it;
the rate-limit break, which deliberately does not pre-close, is pinned by a
mirror test that runs without the option so only the `finally` can satisfy
it. All three mutations now fail a test, and each fails a different one.
- Nothing pinned the catch's `terminalPublished` guard. Adds the teardown-throw
-after-completion case that produced a contradictory `failed` without it.
- Nothing pinned the `else if (finalText)` artifact guard. Adds the
empty-success-result case.
Docs: the prompt-timeout section said it bounds a single turn, which stopped
being true when `holdTaskForBackgroundWork` started defaulting on; the
background-tasks smoke README claimed a `decisions` array the script has never
printed; and `handleMessage`'s `case "result"` now carries a note that it is a
secondary entry point with no hold state, so nobody re-routes results through
it and loses suppression.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018tTTwsL7XNgwm4iXHmxqfb
Three review fixes ahead of the upstream PR. Pin the SDK exactly rather than `^0.3.235`. It is the only exact-pinned dependency in the package and was deliberately so: this is a pre-1.0 SDK whose message shapes change between patch releases — `0.3.202` did not emit `background_tasks_changed` at all, which is the whole reason for the bump. `0.3.235` is the floor, but `0.3.245` is what the feature was actually exercised against, so that is what gets pinned. The lockfile still recorded `0.3.202`, so `npm ci` would have installed an SDK that cannot emit the message this feature reads. Regenerating it also picks up unrelated stale entries the committed lockfile carried from the A2A v1.0 upgrade (workspace versions, core 1.7.0 → 2.0.0, npm `libc` fields). Narrow the `holdTaskForBackgroundWork` wording. The flag governs the Task completion decision only; streaming input is unconditional, so "restore the previous behaviour" over-promised a revert the flag does not perform. Trim the changeset to what a changelog reader needs.
The spikes ran an unrestricted Claude in `process.cwd()` — the repo working tree, per the README's own instructions — while a comment claimed they were sandboxed. Nothing sandboxed them. They need a shell and nothing else, so pre-approve exactly that: `permissionMode: "dontAsk"` with Bash/BashOutput/KillShell allowed and everything else denied, in a fresh mkdtemp directory. File writes are now impossible rather than merely unlikely, and the comment describes what the code does. Deliberately not `auto`, which the SDK also offers: it resolves permissions with a model classifier, which would make an unattended run that spends real quota non-deterministic and add classifier traffic to the very message stream these spikes exist to observe. `dontAsk` is also the mode this repo already endorses for headless operation in VALID_PERMISSION_MODES. Log `system/permission_denied` explicitly in both spikes. It is the new failure mode, and spike-chain's if/else chain drops unmatched messages silently — a denial there would have looked like a stalled run. Not executed as part of this change: these cost real quota.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
An A2A Task reaches a terminal state as soon as Claude's first turn ends — including when that turn had just started a background process and said it was waiting on the result. A2A gives an agent no way to open a new turn against a terminal Task, so the follow-up report has nowhere to land: the client has already been told the work finished while it is still running.
Change
The Task now stays in
workingfor as long as Claude reports background work in flight, and completes only once a turn ends with nothing left running. Each turn publishes its ownresponseartifact plus a non-finalworkingstatus update whosemetadata.backgroundTaskslists what is still running. Chains of any length — start the build, kick off the deploy, report the result — run as rounds of one Task rather than a string of separate ones.Controlled by
features.holdTaskForBackgroundWork(defaulttrue; setfalsefor the previous complete-at-first-result behaviour) andfeatures.emitBackgroundTaskEvents(defaulttrue), which publishes a newbackground_taskssideband event added to@a2a-wrapper/core.Bumps
@anthropic-ai/claude-agent-sdkfrom0.3.202to0.3.245. The feature needs at least0.3.235, the first version to emitbackground_tasks_changed.Worth flagging
Three changes apply even with
holdTaskForBackgroundWorkoff:agent_started/agent_finishedare emitted once per A2A Task rather than once per SDK turn.responseartifact.Other things a reviewer should know:
claude.maxTurnsandtimeouts.promptnow span a held-open Task's rounds. Documented in the README, along with the fact that a held-open Task blocks later messages on the samecontextId.timeouts.prompt: 0, a held-open Task has no automatic release andtasks/cancelis the only escape. Known limitation of this design, documented; a non-timeout release mechanism is a follow-up.agent_finishedis still not emitted on the timeout / cancel / rate-limit paths. That is pre-existing onmain, not introduced here.npm ciwould have installed an SDK that cannot emit the message this feature reads.Running in production for several days.