Skip to content

fix(coding-agent): settle extension command turns before returning - #298

Merged
liuedcson merged 1 commit into
mainfrom
fix/extension-command-turn-v2
Oct 23, 2026
Merged

liuedcson merged 1 commit into
mainfrom
fix/extension-command-turn-v2

Conversation

@liuedcson

Copy link
Copy Markdown
Collaborator

Problem

pi.sendUserMessage and pi.sendMessage({ triggerTurn: true }) are fire-and-forget bindings: an extension command handler returns while the agent turn it started is still running. prompt() treated the command as fully handled and returned at that point.

Print and JSON mode dispose the runtime as soon as prompt() resolves, so a command that hands work to the agent tore the session down mid-turn. Every extension then failed with:

Extension error (<inline:Step>): This extension ctx is stale after session replacement or reload.

…the task never ran, and no session file was written.

Reachable today through the most common headless entry point:

step -p "/plan 把 target.txt 改成 hacked"

Exit status was 0, stdout was empty, and nothing happened. The TUI hid the bug because the UI keeps the session alive.

Change

AgentSession tracks the turns each command handler starts and settles them before prompt() returns.

  • Acceptance stays immediate. preflightResult(true) fires as soon as the handler returns, before the wait. RPC answers the client from that callback, and the deferred turn can open a dialog (plan review) that needs the client — delaying the ack would deadlock it.
  • Open handlers are a stack. Two handlers can overlap when one is parked on a dialog while another is dispatched, and they need not finish in LIFO order. Every open tracker is charged, so a turn can be over-awaited by an unrelated command but never lost. A single save/restore slot was tried first and drops turns in that overlap; the overlap test below fails on it.
  • Failures still surface. A rejected deferred turn reaches the extension error path (and a failed turn still sets the print-mode exit code) instead of failing the command.

Verification

New suite regression command-deferred-turn.test.ts, 7 cases:

case on main
deferred sendUserMessage turn is awaited fails
sendMessage({ triggerTurn }) turn is awaited fails
acceptance fires before the turn finishes fails
two overlapping handlers finish out of order fails on the single-slot variant
nested command's deferred turn is awaited passes
command with no turn is not delayed passes (guards over-waiting)
rejected deferred turn does not fail the command passes (guards rethrowing)
  • tsgo --noEmit clean; biome check --error-on-warnings clean (the earlier preflightResult: () => (accepted = true) tripped noAssignInExpressions and would have failed CI — fixed).
  • check:ts-imports, check:layer-direction, check:public-boundary pass.
  • Full vitest --run in this worktree: 6 failures, all pre-existing test/completion-check-cli.test.ts cases that also fail on an unmodified origin/main worktree.

Docs

  • packages/coding-agent/docs/sdk.md: a slash command is accepted when its handler returns, while prompt() still resolves only after the turn settles.
  • packages/coding-agent/docs/extensions.md: sendUserMessage is fire-and-forget, but a turn it starts inside a command handler is awaited before prompt() resolves.

Not addressed

createReplacedSessionContext() binds sendMessage/sendUserMessage directly, bypassing the tracker. A withSession callback that does not await its send can still outlive the caller; the documented examples all await. Left as-is rather than charging the outgoing session's tracker.

`pi.sendUserMessage` and `pi.sendMessage({ triggerTurn })` are fire-and-forget
bindings: the command handler returns while the turn it started is still
running. `prompt()` reported the command as complete at that point.

Print and JSON mode dispose the runtime as soon as `prompt()` resolves, so a
command that hands work to the agent — `/plan <task>` is the reachable one —
tore the session down mid-turn. Every extension then failed with "This extension
ctx is stale after session replacement or reload", the task never ran, and no
session file was written. Interactive mode hid it because the UI keeps the
session alive.

Track the turns each command handler starts and settle them before `prompt()`
returns. Acceptance stays immediate: `preflightResult(true)` fires once the
handler returns, before the wait, because RPC answers the client from it and a
turn can open a dialog that needs that client. Open handlers are kept on a
stack and every one is charged, so two overlapping commands cannot orphan a
tracker and let a turn escape the wait.

Covered by a suite regression for a deferred turn, a triggered custom message,
acceptance ordering, overlapping handlers, nested commands, the no-turn control,
and a rejected deferred turn that must not fail the command.
@liuedcson
liuedcson merged commit a12325d into main Oct 23, 2026
2 checks passed
@liuedcson
liuedcson deleted the fix/extension-command-turn-v2 branch October 23, 2026 06:23
liuedcson added a commit that referenced this pull request Oct 23, 2026
Brings the seven commits main gained since dff413f:

- feat(coding-agent): make a feedback report investigable (#297)
- fix(coding-agent): settle extension command turns before returning (#298)
- fix(coding-agent): keep the tool block stable across plan mode (#296)
- fix(cli): save model selection as default on Enter (#285)
- feat(cli): add -C/--cwd flag (#289)
- fix(coding-agent): preserve task-state prompt cache prefixes (#293)

Two conflicts, both in the plan extension, where this branch had added
behaviour on top of files main rewrote:

- `plan-mode-tools.ts`: main's comment for `exitPlanMode` ("restore any tools
  plan mode added") with this branch's optional `submittedPlanHash` parameter,
  which records the proposal handed to an external reviewer.
- `step-plan.ts`: main's `version: 2` entry and `carriesLegacyTools` migration
  path, plus this branch's `submittedPlanHash` / `planHashAtEnter` on the same
  entry, and both sides of the restore step (legacy tool baseline and the
  persisted hashes). Main's `resolveToCwd` guard and `planModeAddedTools`
  bookkeeping are kept as-is: plan mode now appends tools instead of replacing
  the list, which is what keeps the request's tool block stable.

The host plan-mode test asserted the old refusal wording; the guard now says
"Only the plan file may be written", so it matches on that.
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.

1 participant