Skip to content

fix(steering): retain prompt ownership after injection - #937

Closed
ran411285752 wants to merge 1 commit into
agentclientprotocol:mainfrom
ran411285752:fix/steering-injected-ownership
Closed

fix(steering): retain prompt ownership after injection#937
ran411285752 wants to merge 1 commit into
agentclientprotocol:mainfrom
ran411285752:fix/steering-injected-ownership

Conversation

@ran411285752

@ran411285752 ran411285752 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #934

Problem

Active steering (_session/steering with outcome injected) previously let the interrupted command's terminal result settle (or reject) the original ACP v1 session/prompt before 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/steering wire contract remains unchanged:

  • The request still supports the optional _meta.steering.idleBehavior: "promptRequired" field.
  • The response outcomes remain injected, startedNewTurn, and promptRequired.
  • No activeBehavior field or new capability is introduced.

Why injected keeps the original prompt pending

An injected outcome means the steering message joined the current prompt turn, not a new one. The SDK cancels the running generation to process the priority: "now" message, emits an interrupted boundary result, then replays the injected user echo and runs the steered generation. The original session/prompt is 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 injected SDKUserMessage keeps its freshly generated UUID;
  • userMessage.priority = "now" (existing STEER_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

  1. Early resolve — the interrupted result (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.
  2. Diagnostic reject — the same superseded classification also prevents the interrupted result's [ede_diagnostic] reject payload from failing the prompt.
  3. Held turn — a turn already held open behind a live subagent (Turn.deferredSettle) previously got handoff-settled by ensureActiveTurn on the injected echo's next user-turn result, resolving the original prompt before the steered terminal result. ensureActiveTurn now keeps a retained-steering held turn held while the injection is in flight, and the steered terminal result clears the marker before settleOrDefer, so a subsequent still-live subagent followup treats it as an ordinary held turn again.

Additional retained-turn lifecycle handling

  1. Held turn awaiting its echo goes unowed-idle (High) — if the injected echo never replays, an unowed idle (phase: "awaiting_echo" with owedTrailingIdles === 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 standard no_result error 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).
  2. Steered refusal leaves a stale active marker (Medium) — the retainedSteering marker 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 stale phase: "active" marker that made ensureActiveTurn skip 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.
  3. Autonomous followup settles a held prompt before steering completes (High) — a drained subagent's autonomous followup result reached 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 any retainedSteering state exists: the phase: "active" marker is claimed only by a user-driven result; the autonomous branch skips settleDeferredIfDrained(); 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.
  4. Post-echo missing steered result leaves the prompt pending forever (High) — the held no-result idle branch only matched 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 skips settleDeferredIfDrained() while a retained marker exists and never fails the prompt — permanent pending. The no-result condition is now any retainedSteering state with owedTrailingIdles === 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 standard no_result error. 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

session_state_changed: running
user echo (original prompt)
result (stop_reason null)  -> superseded boundary, prompt still pending
user echo (injected UUID)  -> retained steering phase becomes active
steered assistant output   -> e.g. STEERED-REAL-OK
result (end_turn)          -> settles the original prompt with summed usage
session_state_changed: idle -> single trailing idle, absorbed by owed debt

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 startedNewTurn and promptRequired behaviors are untouched. This fix changes only the active injected path, which now retains prompt ownership by default.

Automated verification

  • RED: the new Injected steering can outlive its owning session/prompt #934 tests fail on the base commit (premature resolve, diagnostic reject, pre-activation, held-turn settle, missing echo, stale marker, autonomous followup before/after echo, post-echo missing result) and pass after the fix.
  • Focused: 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, and prettier --check on 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 in acp-agent.test.ts and do not touch the Steering implementation.
  • npm run format:check -> fails only on files that are already failing on origin/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: injected outcome, interrupted result, prompt still pending, injected echo, STEERED-REAL-OK output, terminal result, one final idle, and the original session/prompt resolving end_turn with 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.

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.
@ran411285752

Copy link
Copy Markdown
Contributor Author

Closing this PR because #958 has since landed and supersedes its core steering state machine.

#958 now owns injected steering settlement through steeredEchoes / steeredSettle and settles the turn at SDK idle. Keeping this PR's separate retainedSteering state machine would duplicate ownership and create conflicting settlement paths.

I verified the #937 regression cases against current main: 10 pass and 7 fail. Most failures reflect the intentionally different missing-echo semantics adopted by #958 or tests coupled to the old private state. Two focused correctness gaps remain and will be addressed in a small follow-up based directly on #958:

  • an interrupted diagnostic result before the injected echo can incorrectly reject the owning prompt;
  • after the injected echo, an idle without a steered final result can incorrectly reuse the pre-echo result and report success.

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.

@ran411285752

Copy link
Copy Markdown
Contributor Author

The focused follow-up is now available as #969.

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.

Injected steering can outlive its owning session/prompt

1 participant