fix(tui): show real terminal status for background agents - #197
Conversation
The Agent tool's run_in_background=true call returns a non-error ToolResult whose body just says "status: running". The transcript card derived its done/failed badge from that result, so every terminated background agent — including ones reconcile reclassifies as lost on resume — kept the green "✓ Completed" label even when the actual task failed, was killed, or never came back. Push the real BackgroundTaskInfo.status into the matching Agent card so the badge reflects what happened. The card's resolver prefers subagent agentId (live) and falls back to the description on resume; on resume the apply step also runs after replay finishes so the agent group can reach the borrowed components. Also adds an agent-core regression test that pins live, busy, group, race, and resume scenarios for the bg notification chain.
🦋 Changeset detectedLatest commit: e0e190b The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5337ccdfac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.backgroundTaskTerminalPhase = phase; | ||
| this.backgroundTaskTerminalStatus = status; | ||
| this.subagentEndedAtMs ??= Date.now(); | ||
| this.syncSubagentElapsedTimer(); | ||
| this.notifySnapshotChange(); |
There was a problem hiding this comment.
When the matched Agent tool call is still a standalone ToolCallComponent (for example, a replay with only one background Agent call in the step, so it was never upgraded to AgentGroupComponent), this method only updates the snapshot listener state. The standalone render path uses getDerivedSubagentPhase(), which still derives done from the non-error spawn-success ToolResult, and no header/content rebuild is requested here, so the visible card continues to show Completed for lost/failed/killed tasks. The new status override only affects grouped Agent cards that consume getSubagentSnapshot().
Useful? React with 👍 / 👎.
Standalone Agent cards (only one Agent tool call in a step, never upgraded into an AgentGroupComponent) bypassed the previous `setBackgroundTaskTerminalStatus` path: the standalone header reads `getDerivedSubagentPhase`, which still derived `done` from the non-error spawn-success ToolResult, and the method did not request a header/content rebuild. Lost/failed/killed bg agents in this shape still rendered as `✓ Completed`. Thread the override through `getDerivedSubagentPhase`, populate `subagentError` with the friendly failure message so both render paths share one source of truth, and trigger the same header + content rebuild that `onSubagentFailed` does. Also include the override in `hasSubagentState` / the subagent-block early-return so a replayed solo bg agent (no replayed subagent block, no sub-tool activity) switches to the subagent-aware layout instead of the generic `Used Agent` rendering. Adds two standalone-render regression tests so the path no longer relies on the grouped snapshot to stay correct.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9849cc93f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let descAmbiguous = false; | ||
| const visit = (tc: ToolCallComponent): void => { | ||
| if (agentIdMatch !== undefined) return; | ||
| if (args.agentId !== undefined && tc.getSubagentAgentId() === args.agentId) { |
There was a problem hiding this comment.
When two live background Agent calls use the same description, this agentId branch will not actually disambiguate them: handleSubagentSpawned returns for event.runInBackground before calling tc.onSubagentSpawned (session-event-handler.ts:698-703), so the corresponding ToolCallComponent never stores subagentAgentId. The lookup then falls through to the description match, marks it ambiguous, and skips applying the terminal status, leaving both cards with the spawn-success Completed state that this change is meant to fix.
Useful? React with 👍 / 👎.
| this.host.streamingUI.applyBackgroundTaskTerminalStatus({ | ||
| agentId: info.agentId, | ||
| description: info.description, |
There was a problem hiding this comment.
On resume this loops over every persisted terminal background task, but renderRecords only mounts the last REPLAY_TURN_LIMIT turns and persisted agent tasks often arrive without agentId, forcing the description fallback. If an older terminal background agent outside the replay window shares a description with the single rendered Agent card, this call applies the old task's lost/failed/completed status to the unrelated recent card, so the transcript can show a wrong terminal state.
Useful? React with 👍 / 👎.
A backgrounded subagent that ends as `lost`/`failed`/`killed` is
already a soft-recoverable thing — `subagentHost.resume` will
reanimate the persisted Agent instance — but the LLM had to dig
through the original spawn-success ToolResult to find the right id
and figure out the recovery shape on its own. The two look-alike
identifiers (the BackgroundManager `task_id` aka `source_id`, and
the `subagentHost` `agent_id`) regularly got confused in practice.
Surface what the model needs at the moment of decision:
- Add `agent_id` as a top-level `<notification>` attribute for
agent-* tasks, so the right id is structural, not buried in
prose. Render path keeps backward-compat by omitting the
attribute when no agent_id is known (bash tasks, old sessions).
- On non-success agent terminal states, append a recovery
paragraph to the body: the precise `Agent(resume=...)` call,
the disambiguation between `agent_id` and `source_id`, the
`run_in_background` option, and what state survives the
restart vs. what may need to be redone.
- Tighten the spawn-time `resume_hint` with the same
disambiguation and an explicit pointer at the
`task.lost`/`task.failed`/`task.killed` recovery trigger.
- Persist `agent_id` and `subagent_type` in PersistedTask so the
recovery body still works after a session restart, where
in-memory `BackgroundTaskInfo.agentId` would otherwise be
undefined. Optional fields keep the disk schema
forward/backward compatible — pre-PR records load without
them and silently fall back to the original short body.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a80e8f76e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const child of this.host.state.transcriptContainer.children) { | ||
| if (child instanceof ToolCallComponent) { | ||
| visit(child); |
There was a problem hiding this comment.
When a background Agent terminates before its spawn-success tool.result has cleaned up _pendingToolComponents, the same standalone card is present both in _pendingToolComponents and in transcriptContainer. If there is no agentId match yet, the pending pass records descMatch, then this transcript pass visits the same object again and sets descAmbiguous, so the terminal status is dropped even though only one card exists; the later spawn-success result can then leave a failed/lost fast-starting background agent rendered as completed. Track visited components or skip the already-seen pending card before treating a second description match as ambiguous.
Useful? React with 👍 / 👎.
…ription `tc.subagentAgentId` is left undefined for every backgrounded agent. `handleSubagentSpawned` early-returns for `runInBackground` before calling `tc.onSubagentSpawned`, and the wire replay path drops the `subagent` block entirely (`toolCallFromReplayMessage` returns only id/name/args). So the `agentId` branch in `applyBackgroundTaskTerminalStatus` never matched in practice, every call fell through to the description-based fallback, and the persisted `agent_id` we added in the previous commit was effectively dead. That fallback also has a real failure mode: if a foreground Agent and a backgrounded Agent share the same `args.description`, the only candidate found is the live (unrelated) card, which gets incorrectly relabeled as the lost task's terminal state. Parse `agent_id: agent-N` out of the AgentTool spawn-success ToolResult body inside `getSubagentAgentId` so the id is always recoverable, regardless of whether the in-memory subagent metadata was ever populated. Foreground and backgrounded Agent cards now carry distinct ids and route correctly. Also pipe the real `subagent.failed` error through to the parent card. The background branch of `handleSubagentFailed` previously only appended the dedicated transcript entry; the parent Agent card was left with the generic "Background agent failed" written by the later `background.task.terminated` event. Add an optional `errorText` to `setBackgroundTaskTerminalStatus` / `applyBackgroundTaskTerminalStatus` and pass `event.error` through on the failed branch — the real reason now reaches both the card and the entry.
…vents
Previously `applyBackgroundTaskTerminalStatus` always tried agent_id
first and then fell back to description match on miss. That fallback
caused two cross-card bugs:
1. On resume, `applyTerminalBackgroundAgentStatuses` iterates every
persisted terminal task, including ones whose tool calls fell
outside the `REPLAY_TURN_LIMIT` window and were never mounted.
Description fallback could route an old `lost` status onto an
unrelated recent Agent card sharing the same `args.description`.
2. During the live spawn → terminate window, the same card briefly
lives in both `_pendingToolComponents` and `transcriptContainer`.
A description-only walk visits the same component twice and flags
itself ambiguous, dropping the otherwise unambiguous update.
When `args.agentId` is provided we now match only by id and skip on
miss. With `getSubagentAgentId` already parsing `agent_id: agent-N`
out of the spawn-success ToolResult, the id path is reliable for
both live and resume even though `tc.subagentAgentId` is never
populated for backgrounded agents. Description fallback is preserved
solely for old pre-PR sessions whose persisted records lack
`agent_id` — same best-effort behavior as before.
1 participant
Related Issue
No prior issue. Discovered while debugging "main agent doesn't receive notifications after a group of background agents finish" — the underlying notification chain turned out to be fine, but the TUI was masking real outcomes behind a green ✓ Completed.
Problem
Agenttool calls withrun_in_background=truereturn aToolResultwhose body saysstatus: runningand is not flagged as an error. The transcript card derives its done/failed badge fromthis.result, so every terminated background agent — including ones reconcile reclassifies asloston resume — keeps the green✓ Completedlabel even when the actual task failed, was killed, or never came back. On resume this is the user's primary signal that the spawned work finished, and it lies.Reproduction: spawn 2+ background agents, kill the CLI mid-run, restart with
-C. Disk showsstatus: "lost"for the tasks. The transcript shows✗ Failed · Background agent lost (session restarted before completion). Before this PR, the same scenario showed✓ Completed.What changed
Agenttool call card fromBackgroundTaskInfo.status(newsetBackgroundTaskTerminalStatus), so terminated bg agents render✓ Completedonly when the backing task actually completed."Background agent lost (session restarted before completion)", etc.) instead of leaking the spawn-successToolResultbody as the error line.background.task.terminatedevents throughStreamingUIController.applyBackgroundTaskTerminalStatus, which resolves the target card by subagent id first (live path) and falls back to a unique-description match (resume path). The lookup walks pending tool components, transcript children, and the components borrowed byAgentGroupComponent.SessionReplayRendererre-applies the status of already-terminal bg agents afterrenderRecordsmounts the cards —hydrateBackgroundStateruns too early to reach them, and reconcile's events have already fired before the TUI subscribes.No agent-core semantics changed. The silent-on-resume notification behavior (
restoreBackgroundTaskNotificationusesappendUserMessage, notsteer) is intentional and left as-is.Test plan
tool-call.test.ts: 6 new unit tests pin the legacyresult-onlyderivation and assert everyBackgroundTaskInfo.statusmaps to the correct snapshot phase and error text. Also pins order-independence (status applied before result lands).bg-idle-notification-repro.test.ts(new, agent-core): full chain regression on a realAgentinstance — idle single, busy single, idle × N group, after-turn-end race, and resume silent-inject. Pins both the working behavior and the deliberate resume-no-auto-turn behavior so future changes have to acknowledge them.sleep 60bg agents, killed the CLI, restarted with-C. After fix:✗ Failed · Background agent lost (session restarted before completion). Before fix:✓ Completed.Checklist