Skip to content

fix: preserve background Agent terminal lifecycle - #941

Open
ran411285752 wants to merge 2 commits into
agentclientprotocol:mainfrom
ran411285752:codex/fix-background-agent-terminal-status
Open

fix: preserve background Agent terminal lifecycle#941
ran411285752 wants to merge 2 commits into
agentclientprotocol:mainfrom
ran411285752:codex/fix-background-agent-terminal-status

Conversation

@ran411285752

@ran411285752 ran411285752 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Background Agent/Task results with status: "async_launched" acknowledge a launch; they do not mean the subagent has finished. The adapter currently converts that acknowledgement into an ACP completed tool call and later consumes task terminal events only for internal bookkeeping, so hosts cannot determine when the parent Agent card actually settles.

This change keeps those parent calls in_progress until Claude reports a real terminal state, while preserving the launch result content and raw output on the existing card.

Changes

  • Detect async Agent and legacy Task launch acknowledgements from the structured tool_use_result and correlate agentId with the parent tool_use_id.
  • Keep only confirmed async Agent/Task calls open; synchronous Agent calls and background Bash retain their existing behavior.
  • Translate provider terminal states to standard ACP tool statuses:
    • completed -> completed
    • failed, killed, stopped -> failed
  • Accept terminal state from the SDK paths observed across runtime versions:
    • system task_notification
    • terminal system task_updated
    • trusted user messages with origin.kind: "task-notification"
    • structured TaskOutput results as a fallback
  • Preserve the original provider terminal state in _meta.claudeCode.taskStatus when the parent tool metadata is available.
  • Handle lifecycle ordering races, including:
    • async_launched before a partially populated task_started
    • terminal events before the async launch acknowledgement
    • permission-emitted cards before streamed tool_use
    • straggling results whose assistant tool_use was dropped after cancellation
  • Make duplicate terminal signals idempotent and clean the retained tool/cache state when the parent settles.

Scope

This PR intentionally addresses only the background Agent/Task parent-tool lifecycle described in #865. It does not expose the full background_tasks_changed set and does not change background Bash/shell completion reporting, so it uses Addresses rather than Fixes.

Addresses #865

Verification

Real Claude end-to-end smoke test

Ran the built adapter against the vendored Claude Code 2.1.220 executable with @anthropic-ai/claude-agent-sdk 0.3.220 and an authenticated first-party OAuth session. The prompt required one real Agent call with run_in_background=true; the child used Bash to wait five seconds. Raw SDK messages and ACP session updates were recorded independently.

Two consecutive runs passed. The second run produced this parent lifecycle (elapsed from process start):

Event Time
SDK structured async_launched acknowledgement 8,445 ms
Parent ACP tool update: in_progress 8,447 ms
SDK Agent task_updated: completed 38,676 ms
Parent ACP tool update: completed 38,676 ms
session/prompt returned 47,250 ms

The Agent task id and parent tool-use id were correlated from the real SDK payloads. The parent emitted exactly one ACP terminal update. A separate background Bash task inside the Agent also emitted its own task lifecycle and did not settle the parent early. The first run independently showed the same ordering (in_progress at 9,068 ms, Agent/ACP terminal at 38,541 ms, prompt return at 41,949 ms).

Automated checks

  • npx vitest run src/tests/acp-agent.test.ts -t "background Agent tool lifecycle" - 18 passed
  • npm run build - passed
  • npm run lint -- --no-fix - passed
  • npx tsc --noEmit - passed
  • npx prettier --check src/acp-agent.ts src/tests/acp-agent.test.ts - passed
  • git diff --check - passed
  • npm run test:run - 709 passed, 20 skipped, with the same 6 pre-existing Windows path-separator failures as the unmodified baseline (src/main.ts expected, src\\main.ts received)

Regression coverage

The new tests cover completed/failed/killed/stopped mappings, duplicate signals, trusted and untrusted task-notification messages, TaskOutput fallback, optional isAsync, early-terminal races, permission-before-stream ordering, missing-cache stragglers, synchronous Agent cleanup, and unchanged foreground Agent/background Bash behavior.

Why:
- Background Agent launch acknowledgements were treated as terminal tool results, while later task terminal signals only pruned internal state.
What:
- Keep async Agent/Task calls in progress and bridge trusted task notifications, task updates, and TaskOutput results to ACP terminal statuses.
- Cover ordering races, duplicate signals, untrusted text, synchronous Agents, and background Bash regressions.
Impact:
- Only Agent/Task results identified as async launches use the deferred lifecycle; synchronous tools retain existing behavior.
Refs: agentclientprotocol#865
@ran411285752
ran411285752 marked this pull request as draft August 2, 2026 15:52
@ran411285752
ran411285752 marked this pull request as ready for review August 2, 2026 16:09
@ran411285752

Copy link
Copy Markdown
Contributor Author

@benbrandt Could you please review this PR when you have a chance?

@jstrunk

jstrunk commented Aug 13, 2026

Copy link
Copy Markdown

Thanks for tackling this — #941 lines up almost exactly with an independently-developed fix I've been running as a patch on the built adapter (dist/acp-agent.js), driving a client that surfaces background sub-agents as cards. My version converged on the same shape as yours: keep the parent Task card in_progress at launch, emit exactly one terminal tool_call_update on the parent id, map completed → completed and failed / killed / stopped → failed, and verify a background Bash inside the Agent doesn't settle the parent early. So this is a review with two small hardening deltas I found the hard way, not a competing approach.

1. Read tool_use_id off the message on the task_updated path too, not just task_notification. As written, handleBackgroundAgentTerminal is passed message.tool_use_id as a hint on the task_notification paths, but the task_updated call (handleBackgroundAgentTerminal(message.task_id, message.patch.status)) passes no hint and resolves the parent via record?.parentToolUseId out of liveBackgroundTasks. In my captures that registry entry is already deleted by the time the terminal is handled — the background_tasks_changed level-sweep (added in #870) empties it first, and the ordering between the sweep and the terminal handler is unspecified. Result: the task_updated close silently no-ops (guard fails → card never closes). I instrumented this — at the terminal, the registry lookup returned found=false every time for the settling sub-agent. Reading the id off the message on that path too (falling back to the registry only if the message lacks it) removes the dependence on sweep-vs-terminal ordering. This was the single defect that made an earlier version of my own patch pass all shape tests while emitting nothing in production.

2. Attach the completion result body, not just the status. The terminal update flips status and records _meta.claudeCode.taskStatus, but doesn't carry the sub-agent's actual completion output. Clients that render the sub-agent's result in the card's output pane get an empty terminal. I put the completion result on _meta.claudeCode.toolResponse.content (a text part) on the terminal frame; a probe against the patched adapter confirmed the real result arrives there. Worth adding so downstreams don't have to correlate a separate frame.

Happy to open a small follow-up PR against your branch with both, or you can fold them in — whichever you prefer. One caveat for maintainers: @benbrandt noted in #876 that v2 "removes the turn concept a bit," so if the terminal _meta shape is going to churn there, delta 2's field is the only additive surface to worry about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants