claude adapter: hold working state while async agents/workflows are pending (stop spurious needs-input pushes) #159

New issue

Closed

opened 2026-07-13 19:01:25 +02:00 by dominik.polakovics · 1 comment

dominik.polakovics commented

2026-07-13 19:01:25 +02:00

Owner

Problem

When the model launches an async agent (Agent tool) or a workflow and ends its turn with text like "launched an agent, waiting on it", the last transcript event is assistant:text, so deriveState (internal/provider/claudecode/chat.go:391-417) returns needs_input and the push gate (internal/chat/notify.go) fires a notification — even though nothing is needed from the operator and the CLI will re-invoke the model when the background work completes. Synchronous tools hold working fine (their tool_result is outstanding); async launches return their tool_result immediately, so nothing holds the state.

Design (grilled 2026-07-13)

Truth source = harness pending-work, not message content. No text heuristics. State is working whenever background work the session spawned is still running.
Detection = transcript markers, tracked in foldTranscript (chat.go:228-327). Maintain a pending set:
- Add: tool_result of an async Agent launch (carries an agent/task ID, e.g. "Async agent launched … agentId: …") and Workflow launches (task/run ID). Exact marker shapes to be pinned by the fixture spike below.
- Remove: injected user message containing a <task-notification> block with a matching task-id — regardless of status (completed/failed/killed all mean "will not re-invoke for this id" / "model was re-invoked"). Removal must be idempotent (the same id may notify more than once). A TaskStop tool call for the id also removes it. Session rotation (/clear, /rewind) resets naturally because folding restarts on the new transcript.
- Spike question: does SendMessage to a completed agent re-activate it? If yes, a SendMessage tool_use targeting a known agent id re-adds it to the pending set.
Pending scope = agents + workflows ONLY. Background Bash (run_in_background) is explicitly NOT counted — a dev server never exits and would pin the session at working forever (suppressing pushes, 409-ing /pull-base, making the clear-hook skip its pull). Add a test asserting a background-Bash launch does not hold.
deriveState change: lastKey == "assistant:text" && pending set non-empty → StateWorking instead of StateNeedsInput. lastKey == "error" stays needs_input even with pending work (errors must surface).
Break-through = structured signals only. The existing ReadChat precedence (chat.go:133-165) is untouched and overrides the hold: live dialog spool (AskUserQuestion/ExitPlanMode) → question + push; Notification-hook blocked marker (permission prompt) → needs_input + push. Plain-text questions the model asks while agents are pending stay silent at working — accepted: the turn resumes when the agents finish and the final turn-end pushes as usual.
No notify-gate changes. The gate (notify.go) already edge-triggers on entering needs_input/question; holding working through the pending window means no episode ever arms. The final turn-end (pending set empty) pushes exactly as today.
UI: plain working badge, no new API surface. Enrichment ("working, 2 agents") deferred.
Scope: claude-code adapter only. Codex has native lifecycle events; gemini is spec-stage.

Plan

Fixture spike first (same issue, first task): drive the pinned claude-code CLI in the tmux rig and capture real transcript JSONL for: async Agent launch + <task-notification> completion, Workflow run, background Bash launch/exit, TaskStop, SendMessage resume of a completed agent, duplicate notifications for one id. These become testdata fixtures — none exist today.
Implement pending-set tracking in foldTranscript + the deriveState hold, matching the marker shapes the fixtures establish.
Table-driven tests in chat_test.go against the fixtures, including: launch→text = working; notification→resume→final text = needs_input (one push); dialog while pending = question; permission marker while pending = needs_input; background Bash = no hold; error while pending = needs_input.
Note the marker-text coupling in the claude version-drift watch (fixtures are the conformance canary).

Acceptance

Turn ending with pending agents/workflows: state working, zero push.
Agents finish, model resumes and ends the run's real turn: exactly one needs-input push.
AskUserQuestion or permission prompt while agents pending: push fires (question/needs_input).
Background Bash alone never holds working past turn-end.

## Problem When the model launches an async agent (Agent tool) or a workflow and ends its turn with text like "launched an agent, waiting on it", the last transcript event is `assistant:text`, so `deriveState` (internal/provider/claudecode/chat.go:391-417) returns `needs_input` and the push gate (internal/chat/notify.go) fires a notification — even though nothing is needed from the operator and the CLI will re-invoke the model when the background work completes. Synchronous tools hold `working` fine (their tool_result is outstanding); async launches return their tool_result immediately, so nothing holds the state. ## Design (grilled 2026-07-13) 1. **Truth source = harness pending-work, not message content.** No text heuristics. State is `working` whenever background work the session spawned is still running. 2. **Detection = transcript markers, tracked in `foldTranscript`** (chat.go:228-327). Maintain a pending set: - **Add**: tool_result of an async Agent launch (carries an agent/task ID, e.g. "Async agent launched … agentId: …") and Workflow launches (task/run ID). Exact marker shapes to be pinned by the fixture spike below. - **Remove**: injected user message containing a `<task-notification>` block with a matching task-id — regardless of status (completed/failed/killed all mean "will not re-invoke for this id" / "model was re-invoked"). Removal must be idempotent (the same id may notify more than once). A `TaskStop` tool call for the id also removes it. Session rotation (/clear, /rewind) resets naturally because folding restarts on the new transcript. - **Spike question**: does `SendMessage` to a completed agent re-activate it? If yes, a SendMessage tool_use targeting a known agent id re-adds it to the pending set. 3. **Pending scope = agents + workflows ONLY.** Background Bash (`run_in_background`) is explicitly NOT counted — a dev server never exits and would pin the session at `working` forever (suppressing pushes, 409-ing /pull-base, making the clear-hook skip its pull). Add a test asserting a background-Bash launch does not hold. 4. **`deriveState` change**: `lastKey == "assistant:text"` && pending set non-empty → `StateWorking` instead of `StateNeedsInput`. `lastKey == "error"` stays `needs_input` even with pending work (errors must surface). 5. **Break-through = structured signals only.** The existing ReadChat precedence (chat.go:133-165) is untouched and overrides the hold: live dialog spool (AskUserQuestion/ExitPlanMode) → `question` + push; Notification-hook blocked marker (permission prompt) → `needs_input` + push. Plain-text questions the model asks while agents are pending stay silent at `working` — accepted: the turn resumes when the agents finish and the final turn-end pushes as usual. 6. **No notify-gate changes.** The gate (notify.go) already edge-triggers on entering needs_input/question; holding `working` through the pending window means no episode ever arms. The final turn-end (pending set empty) pushes exactly as today. 7. **UI: plain `working` badge, no new API surface.** Enrichment ("working, 2 agents") deferred. 8. **Scope: claude-code adapter only.** Codex has native lifecycle events; gemini is spec-stage. ## Plan 1. **Fixture spike first** (same issue, first task): drive the pinned claude-code CLI in the tmux rig and capture real transcript JSONL for: async Agent launch + `<task-notification>` completion, Workflow run, background Bash launch/exit, TaskStop, SendMessage resume of a completed agent, duplicate notifications for one id. These become testdata fixtures — none exist today. 2. Implement pending-set tracking in `foldTranscript` + the `deriveState` hold, matching the marker shapes the fixtures establish. 3. Table-driven tests in chat_test.go against the fixtures, including: launch→text = working; notification→resume→final text = needs_input (one push); dialog while pending = question; permission marker while pending = needs_input; background Bash = no hold; error while pending = needs_input. 4. Note the marker-text coupling in the claude version-drift watch (fixtures are the conformance canary). ## Acceptance - Turn ending with pending agents/workflows: state `working`, zero push. - Agents finish, model resumes and ends the run's real turn: exactly one needs-input push. - AskUserQuestion or permission prompt while agents pending: push fires (question/needs_input). - Background Bash alone never holds `working` past turn-end.

dominik.polakovics added the

ready-for-agent

label

2026-07-13 19:01:25 +02:00

dominik.polakovics referenced this issue from a commit

2026-07-13 19:57:25 +02:00

feat(claudecode): hold working while async agents/workflows are pending

dominik.polakovics referenced this issue from a pull request that will close it,

2026-07-13 19:57:52 +02:00

feat(claudecode): hold working while async agents/workflows are pending #161

dominik.polakovics commented

2026-07-13 20:07:04 +02:00

Author

Owner

This was generated by AI while landing a PR.

Landing audit — PR #161

Verdict: PASS.

Verification signal relied on: Forgejo Actions ci / native — success (5m12s, run 175). Not re-run. The ci-nix gate did not run, and correctly so: it is path-gated to **/*.nix / flake.lock / go.mod / go.sum, and this diff touches none of them (8 files, all Go + testdata + compat docs). The native gate exercised the whole change.

Merge state: afk/159 is up to date with origin/main (no conflict, no resolution commit — the green CI is over the code as it will merge).

Conventions: title is Conventional Commits (feat(claudecode): …); body carries a working Closes #159; head branch afk/159 matches the AFK contract.

Diff review against this issues design: implementation matches the grilled design point-for-point — pending set in foldTranscript keyed on structural toolUseResult markers (never prose), deriveState softening only the assistant:text edge, API errors still unconditionally needs_input, ReadChats live-signal precedence untouched and pinned by a new test. Verified in the captured fixtures that the add/exclude gate is genuinely structural: the async Agent launch result carries status:"async_launched" + agentId, while background Bash carries backgroundTaskId and no status — so decision 3 (Bash never holds) holds by construction, not by name-matching.

One deviation from the issue text, and it is an improvement: decision 2 said remove on a <task-notification> "regardless of status". The implementation requires a non-empty <status> to remove, because the spike found Monitor emits status-less interim <event> payloads through the same carrier — removing on those would release the hold early. Stricter, spike-backed, documented in compat §5.

Residual (accepted by design, not a blocker): a pending id that never receives a terminal notification — an agent whose process dies without notifying — pins the run at working and suppresses its pushes for the rest of the session. That is the inverse failure of the bug being fixed, and it is inherent to decision 5s structured-only break-through: a live dialog or a blocked marker still breaks through, but plain turn-end no longer does while pending is non-empty. Worth watching in the wild; if it bites, the fix is a liveness backstop, not a change to this fold.

> *This was generated by AI while landing a PR.* ## Landing audit — PR #161 **Verdict: PASS.** **Verification signal relied on:** Forgejo Actions `ci / native` — **success** (5m12s, run 175). Not re-run. The `ci-nix` gate did not run, and correctly so: it is path-gated to `**/*.nix` / `flake.lock` / `go.mod` / `go.sum`, and this diff touches none of them (8 files, all Go + testdata + compat docs). The native gate exercised the whole change. **Merge state:** `afk/159` is up to date with `origin/main` (no conflict, no resolution commit — the green CI is over the code as it will merge). **Conventions:** title is Conventional Commits (`feat(claudecode): …`); body carries a working `Closes #159`; head branch `afk/159` matches the AFK contract. **Diff review against this issues design:** implementation matches the grilled design point-for-point — pending set in `foldTranscript` keyed on structural `toolUseResult` markers (never prose), `deriveState` softening only the `assistant:text` edge, API errors still unconditionally `needs_input`, ReadChats live-signal precedence untouched and pinned by a new test. Verified in the captured fixtures that the add/exclude gate is genuinely structural: the async Agent launch result carries `status:"async_launched"` + `agentId`, while background Bash carries `backgroundTaskId` and **no** status — so decision 3 (Bash never holds) holds by construction, not by name-matching. **One deviation from the issue text, and it is an improvement:** decision 2 said remove on a `<task-notification>` "regardless of status". The implementation requires a **non-empty `<status>`** to remove, because the spike found Monitor emits status-less interim `<event>` payloads through the same carrier — removing on those would release the hold early. Stricter, spike-backed, documented in compat §5. **Residual (accepted by design, not a blocker):** a pending id that never receives a terminal notification — an agent whose process dies without notifying — pins the run at `working` and suppresses its pushes for the rest of the session. That is the inverse failure of the bug being fixed, and it is inherent to decision 5s structured-only break-through: a live dialog or a blocked marker still breaks through, but plain turn-end no longer does while pending is non-empty. Worth watching in the wild; if it bites, the fix is a liveness backstop, not a change to this fold.

dominik.polakovics closed this issue

2026-07-13 20:44:54 +02:00