fix(steering): retain prompt ownership after injection - #937
Conversation
Why: - An interrupted SDK result could settle the ACP v1 prompt before the injected response started. - Held turns could also settle or remain pending when Steering interleaved with subagent and autonomous lifecycles. What: - Track the latest injected UUID on the existing Turn and retain prompt ownership through its terminal result. - Treat pre-echo results as superseded boundaries without false idle debt. - Preserve usage, cancellation, refusal, autonomous followups, and held-turn no-result handling. - Document the unchanged injected and idle fallback contracts. Impact: - Active Steering keeps the original session/prompt pending until the steered response completes. - Idle startedNewTurn and promptRequired behavior remain unchanged.
|
Closing this PR because #958 has since landed and supersedes its core steering state machine. #958 now owns injected steering settlement through I verified the #937 regression cases against current
The follow-up will preserve #958's state machine and add only the minimal guards and regression tests for those two cases. Thanks to the maintainers for landing the broader fix in #958. |
|
The focused follow-up is now available as #969. |
Fixes #934
Problem
Active steering (
_session/steeringwith outcomeinjected) previously let the interrupted command's terminal result settle (or reject) the original ACP v1session/promptbefore the injected response even started. The steered output then streamed with no owning turn, so its own terminal result leaked a false trailing-idle debt and the client observed the original prompt resolving early (even with zero usage) or rejecting.Scope
This change is limited to ACP v1. The existing
_session/steeringwire contract remains unchanged:_meta.steering.idleBehavior: "promptRequired"field.injected,startedNewTurn, andpromptRequired.activeBehaviorfield or new capability is introduced.Why
injectedkeeps the original prompt pendingAn
injectedoutcome means the steering message joined the current prompt turn, not a new one. The SDK cancels the running generation to process thepriority: "now"message, emits an interrupted boundary result, then replays the injected user echo and runs the steered generation. The originalsession/promptis the request that owns all of that: it stays pending through the interrupted result and the injected echo, and only the steered terminal result settles it.The exact SDK delivery path (unchanged)
userMessage.uuid = randomUUID()— the injectedSDKUserMessagekeeps its freshly generated UUID;userMessage.priority = "now"(existingSTEER_PRIORITY);session.input.push(userMessage)— the same input stream and conversation context as every other message in the turn.No change to UUID generation, priority, the input stream, or context.
Both premature-settlement variants fixed
stop_reason: null) previously settled the prompt before the steered response. It is now classified as a superseded boundary result: its usage is still accumulated into the owning turn, but it never settles the prompt, never counts idle debt, and exits before any terminal handling.[ede_diagnostic]reject payload from failing the prompt.Turn.deferredSettle) previously got handoff-settled byensureActiveTurnon the injected echo's next user-turn result, resolving the original prompt before the steered terminal result.ensureActiveTurnnow keeps a retained-steering held turn held while the injection is in flight, and the steered terminal result clears the marker beforesettleOrDefer, so a subsequent still-live subagent followup treats it as an ordinary held turn again.Additional retained-turn lifecycle handling
phase: "awaiting_echo"withowedTrailingIdles === 0) on a held turn previously left the prompt pending forever behind a still-live subagent, or — once the subagent drained — settled it with the PRE-steering deferred outcome (wrong usage, no steered content). The idle handler now fails it with the standardno_resulterror before the ordinary held-turn branch, covering both live and drained cases (same contract as the Prompt loop never terminates a turn when the model stream ends abnormally; session hangs 'running' until the next prompt drains it (0.52.0, sdk 0.3.191) #825 idle-fail).retainedSteeringmarker was only cleared at the bottom of the result handler's normal path, so a steered refusal (which re-holds the turn behind a still-live subagent) left a stalephase: "active"marker that madeensureActiveTurnskip the next echo-less result's hand-off. The result handler now claims the active marker right after the superseded-result check and before all terminal early-return lanes; a result still awaiting its echo keeps the marker so an autonomous subagent followup landing before the echo stays part of the injected boundary.settleDeferredIfDrained()while the retained marker was still set, and the active marker was cleared even by an autonomous result. Either path resolved the held prompt with the PRE-steering deferred outcome (wrong usage, no steered content) before the injected echo / steered terminal result, so the steered output streamed outside the ended prompt. Three gates now hold while anyretainedSteeringstate exists: thephase: "active"marker is claimed only by a user-driven result; the autonomous branch skipssettleDeferredIfDrained(); and the held-turn idle branch skips it too (the autonomous followup's own trailing idle can't settle the prompt). Autonomous usage,usage_update, and idle debt are unchanged.phase: "awaiting_echo". Once the injected echo replayed (phase: "active"), a missing steered terminal result with an unowed idle fell into the ordinary held branch, which skipssettleDeferredIfDrained()while a retained marker exists and never fails the prompt — permanentpending. The no-result condition is now anyretainedSteeringstate withowedTrailingIdles === 0: an unowed idle while the marker is set means the SDK turned over without the retained echo or the steered terminal result it owes, so it fails with the standardno_resulterror. An idle WITH debt still belongs to a counted result (an autonomous followup's trailer) and is absorbed by the ordinary held branch — the autonomous-result regression tests exercise exactly that.Verified result / echo / final-result / idle order
For a held turn (deferred subagent), the same order holds with the prompt staying held through the steered result; if the subagent is still live at that point the turn re-defers and only settles when the subagent drains, at which point the summed usage of interrupted + steered commands is attributed to the original prompt.
Unchanged idle behavior
The idle
startedNewTurnandpromptRequiredbehaviors are untouched. This fix changes only the activeinjectedpath, which now retains prompt ownership by default.Automated verification
npx vitest run src/tests/acp-agent.test.ts -t "turn steering|session/cancel wedge recovery|session_state_changed|deferred settlement for live background subagents|autonomous followup lands|never emits its final result"-> 51 passed; the whole file -> 324 passed, 6 failed (pre-existing path-separator), 9 skipped.npm run build,npm run lint,git diff --check, andprettier --checkon the committed content of the changed files -> pass.npm run test:run-> 695 passed, 6 failed; the remaining failures are the pre-existing Windows path-separator assertions inacp-agent.test.tsand do not touch the Steering implementation.npm run format:check-> fails only on files that are already failing onorigin/main(repo-wide prettier drift on this Windows host); the changed files are clean on their committed content.Real-model verification
The latest run on the final commit used the issue's prompts and showed the full order above:
injectedoutcome, interrupted result, prompt still pending, injected echo,STEERED-REAL-OKoutput, terminal result, one final idle, and the originalsession/promptresolvingend_turnwith the steered command's usage attributed to it (input=93,output=41,cache_read=27904,total=28038) — versus the pre-fix zero-usage early resolve. Two earlier runs showed the same ordering and ownership behavior.