agentHost: keep the parent output after a background subagent settles - #332075
agentHost: keep the parent output after a background subagent settles#332075Ryan Ewen (RyanEwen) wants to merge 1 commit into
Conversation
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: TylerLeonhardtMatched files:
|
There was a problem hiding this comment.
Pull request overview
Fixes dropped Claude parent output after background subagents by preserving and reusing the spawning turn ID.
Changes:
- Records turn IDs for subagent spawns.
- Adds fallback turn resolution for turn-less SDK messages.
- Adds registry and agent-level coverage.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
claudeSubagentSignals.test.ts |
Tests spawn turn tracking. |
claudeAgent.test.ts |
Tests resumed parent output. |
claudeSubagentSignals.ts |
Records the spawning turn. |
claudeSubagentRegistry.ts |
Exposes outstanding spawn turn IDs. |
claudeSdkMessageRouter.ts |
Uses spawn turns for turn-less messages. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * resumes producing output once those settle. | ||
| */ | ||
| async handle(message: SDKMessage, turnId: string | undefined, context?: IClaudeSdkMessageContext): Promise<void> { | ||
| const resolvedTurnId = turnId ?? this._subagents.outstandingSpawnTurnId(); |
There was a problem hiding this comment.
Correct, and it is the case that actually matters. The parent resumes after the notification, so removeSpawn clears the anchor at exactly the moment it is needed. My end-to-end test omitted task_notification entirely, so it was guarding the window before completion rather than after it, which is why it passed.
Reworked to anchor on the spawn event rather than on live spawns. task_started records the spawning turn on the registry as a resume anchor, which outlives removeSpawn; the router falls back to it for a message with no turn, and clears it once a message arrives with a real turn. The test now sends task_notification before the resumed stream_event output, so it covers the boundary you named.
AI disclosure: this comment and the related code were written with the assistance of AI.
| outstandingSpawnTurnId(): string | undefined { | ||
| for (const spawn of this._spawns.values()) { | ||
| if (!spawn.completed && spawn.turnId !== undefined) { | ||
| return spawn.turnId; | ||
| } |
There was a problem hiding this comment.
Correct. outstandingSpawnTurnId is gone, so nothing scans the spawn map for a turn any more. The anchor is written by the concrete task_started event for that spawn's own tool_use_id, so when several background spawns are alive the most recently started one wins deterministically instead of whichever entry the map happened to yield first.
AI disclosure: this comment and the related code were written with the assistance of AI.
The pipeline takes the turn from the prompt queue head, so a parent that spawns background subagents ends its own turn and every message after it arrives with no turn id. `ClaudeSdkMessageRouter.handle` dropped those outright, silently at every log level, so once the subagents settled and the parent resumed the chat showed nothing further. The transcript kept all of it, which is why restarting the agent host made the whole response appear. Anchor a message that arrives with no active turn to the turn a background subagent was spawned in. The spawn records that turn first-writer-wins like its other fields, `task_started` writes it onto the registry as a resume anchor, and the router falls back to it before deciding to drop. The anchor lives on the registry rather than the spawn so it outlives the `removeSpawn` that completion performs, which is the moment the parent actually resumes. It is written by the concrete `task_started` event for that spawn's own tool use id, so several live background spawns resolve to the most recently started one rather than to whichever the spawn map happened to yield first. Fixes microsoft#332073
ef70f91 to
6171f7c
Compare
Mohammad javad Dianat (dianatofficial)
left a comment
Mohammad javad Dianat (dianatofficial)
left a comment
There was a problem hiding this comment.
Great catch on this edge case. Fallback logic is robust.
|
Correction to this PR's description, which offers to close #331872. It does not fully supersede it. There is a case #331872 covers that this one does not: a background subagent spawned in turn A, where turn B starts and ends before the subagent settles. The first turn-B message clears the spawn anchor, so by the time the subagent completes there is no active turn and no anchor, and neither the spawn-turn nor the resume-anchor path closes the chip. #331872's idle-completion branch is what closes it. Keeping both is the smaller change, so I would rather correct the claim here than close #331872 and leave that hole open. AI disclosure: this comment and the related code were written with the assistance of AI. |
Fixes #332073
Problem
ClaudeSdkPipeline._processMessagesderives the turn from the prompt queue head:A parent that spawns background subagents ends its own turn, so the queue drains and everything that follows arrives with
turnId === undefined.ClaudeSdkMessageRouter.handlethen drops it:When the subagents settle and the parent resumes producing output, every one of those messages is discarded. The chat stops at whatever the parent said before spawning, and the early return is silent at every log level, so nothing is reported. The transcript keeps all of it, which is why restarting the agent host makes the full response appear.
Change
Anchor a message that arrives with no active turn to the turn a background subagent was spawned in:
SubagentSpawnrecords the turn that spawned it, first-writer-wins like its other fields.task_startedwrites that turn onto the registry as a resume anchor. It lives on the registry rather than the spawn so it outlivesremoveSpawn, whichtask_notificationperforms on completion, and completion is exactly when the parent resumes.ClaudeSdkMessageRouter.handlefalls back to the anchor for a message with no turn, and drops the anchor once a message arrives for a different turn.The anchor is written by the concrete
task_startedevent for that spawn's owntool_use_id, so when several background spawns are alive the most recently started one wins deterministically. Nothing scans the spawn map for a turn.Nothing changes while a turn is active, or when no background subagent has been spawned: the early return still applies, so an idle session drops stray messages exactly as before.
How to test
The parent's output keeps appearing as the subagents report back. On
mainthe response stops at the line before the spawn and never resumes, and restarting the agent host is the only way to see the rest.Unit coverage adds two tests: one that the resume anchor outlives the background spawn that set it, and one end to end that output the parent produces after the subagent's completion notification still reaches the chat.
Note on #331872
That PR hooks the same early return for
system/task_notificationso a background subagent's chip completes. This is the general case: once a turn is resolved the mapper handlestask_notificationnormally, so this supersedes it and I am happy to close #331872 if this lands.AI disclosure: this pull request and the related code were written with the assistance of AI.