[qingnian108]

Follow-up after #760. Community main already has 1.3.3, so this bumps packages/agent-console to 1.3.4.

Bug

Stopping a turn with Devin (and the same ACP path used by Cursor / Copilot / OpenCode / Pi / dsh) could leave the console stuck:

1. Devin CLI registers session/cancel as a notification (CancelNotification) only.
2. The daemon sent it as a JSON-RPC request, so Devin answered -32601 Method not found.
3. The manager’s 1s fallback marked the page idle, but AcpSession.running stayed true.
4. The next send failed with 当前轮次尚未结束.

Claude Code and Codex never threw that exact error (custom transports), but the same class of race existed: UI could go idle after the manager fallback while a late provider end rebound onto the next send.

What it does

• ACP (shared by Cursor / Copilot / OpenCode / Pi / Devin / dsh): send session/cancel as a notification; end the local turn immediately with interrupted so running / activeTurnId clear; wait briefly for session/cancel + previous prompt settle before sending an overlapping session/prompt on immediate resend (verified live against devin 3000.11.3).
• Claude: end the local turn on interrupt; scope turnGen / awaitingResult / skipResultGens onto each ClaudeCli process so late results from interrupted turns are dropped in FIFO order without outliving a crashed or restarted CLI, and ignore leftover stdout from a replaced CLI. Crash mid-turn: we intentionally keep the session event queue open (this.q.close() is not called on crash), so the manager leaves the thread idle after turn_ended(failed) instead of permanently closed, and the next send() restarts the CLI with --resume <claudeSessionId>.
• Codex: end locally on interrupt; use the turn/start response’s turn id and check ourTurnId !== turnId when an in-flight turn/start resolves after Stop so codex-turn-1 still receives turn/interrupt without overwriting a newer turn.

Security model unchanged (page token, Host check, WS hello auth).

Tests

• npm run check: OK
• npm test: 68 files, 507 passed / 17 skipped (hold-file order gates instead of wall-clock timing bars; mutation-checked so every surviving guard goes red when removed)
• npm run build + verify:build: OK; daemon/dist rebuilt and committed (app/ unchanged — no UI source edits)

Notes

• Not related to Magpie host restart / Plugin auto-update restarts the plugin host and kills a running Agent Console session #764 — that is a different failure mode. This PR is only interrupt / cancel / late-result races inside the agent-console daemon.

@yetone could you take a look when you have time? Thanks!

[yetone]

Thanks — the ACP part is the right fix (session/cancel as a notification), and the three acp-interrupt tests are red on main, which is great. We ran the whole repo at 5b2c7ba on our box; a few things before this can merge:

1. packages/agent-console/package-lock.json still says 1.3.3, so the root bun test fails (agent-console.test.mjs:45, "lockfile in step").
2. Claude: the fixture now ends an interrupted turn with subtype: "error_interrupted", but the real Claude Code CLI doesn't emit that — it sends error_during_execution. With the real subtype and the CLI handling turns in order, a late result arriving after the next send maps to failed (send() resets sawInterrupt) and ends the new turn: [interrupted, failed]. The new claude-cli test also passes on main, so it doesn't reproduce the race. Please keep the real subtype in the fixture, add a test where the next send happens before the late result, and drop exactly the interrupted turn's result (for example, by counting results owed rather than looking at the status).
3. ACP: pendingCancelStops only goes down on a stop. If the agent answers the cancelled prompt with a JSON-RPC error, the counter stays at 1 and swallows the next turn's stop, so that turn never ends (tried with the fixture answering cancel with -32603). Tying the end to each prompt() promise for its own turn would avoid the counter. Have you tried Stop → resend with the real Devin CLI?
4. Codex: Stop before turn/started, then an immediate new message — send() clears interruptWanted, so the first Codex turn is never interrupted (no turn/interrupt sent). The turn/start response already has the turn id; using it would remove this case and the staleNextCodexTurn guess (which also stays set if turn/start fails). Two of the three codex-interrupt tests pass on main; please add one that fails without staleNextCodexTurn.
5. The manager retry keyed on the 当前轮次尚未结束 string has no test. If 2–4 are fixed, is it still needed?

[yetone]

Thanks for the quick turnaround. We re-ran everything at 45a67b2 on our macOS box (sandbox HOME): root HOME=$(mktemp -d) bun test → 6171 pass / 0 fail; bun scripts/check.mjs → ok; in packages/agent-console: npm ci --workspaces=false, npm run check (tsc) → ok, npm run build → daemon/dist/main.js byte-identical to the committed one, npx vitest run → 68/68 files pass.

Per point from the last review:

1. Lockfile — fixed; the root suite is green.
2. ACP — fixed. Ending each turn from its own prompt() promise with a per-gen discard is the right shape. All four acp-interrupt tests are red on main (2b7eb23 / origin/main 39d4456 with only your test files copied in) and green here. We also checked that 40 chunks sent right before the prompt response all land before turn_ended (they do). Still open: have you tried Stop → resend with the real Devin CLI? Please say so either way in the PR.
3. Manager retry — removed, good.

Two things still block the merge:

2. Claude: the FIFO outlives the CLI it counts. awaitingResult / skipResultGens are per session, but results come per CLI process. When cli.interrupt() fails, the catch closes that CLI and sets needsRestart, and endLocal() adds the interrupted gen to skipResultGens. That gen's result will never arrive. The restarted CLI's first result (the next turn's) shifts the stale gen, finds it in the skip set and is dropped, so the next turn never ends. On main the same sequence ends normally. Probe: fake-claude.mjs with one added line, answering interrupt with an error:

if (process.env.PROBE_INTERRUPT_FAIL) return out({ type: "control_response", response: { subtype: "error", request_id: m.request_id, error: "busy" } });

send SLOW → wait for a delta → interrupt() → wait for turn_ended → send second → wait 4s:

head 45a67b2: [["turn_started"],["turn_ended","interrupted"],["turn_started"]]                         ← second turn never ends
main:         [["turn_started"],["turn_ended","interrupted"],["turn_started"],["turn_ended","completed"]]

The same misalignment occurs after a crash: a turn whose CLI died leaves its gen in awaitingResult, so every later result is matched to the wrong gen. Reset both structures whenever a CLI is replaced or exits, so they belong to one CLI (in ensureCli() when a new ClaudeCli is made, or keep them on the CLI object).

Also, neither new claude-cli test is red on main: both pass on 39d4456 with your fixture. On main interrupt() doesn't end the turn locally, so await until(ended) simply waits for the late result, and second is sent after it. So the tests never send before the late result on the code they are meant to guard. Please make the test check the order, for example that the second user line is in FAKE_CLAUDE_LOG before the interrupted turn's result, or that the local turn_ended arrives well inside FAKE_CLAUDE_INTERRUPT_RESULT_MS. Then it is red on main.

4. Codex: Stop while turn/start is still in flight, then an immediate new message. Using the response's turn id fixes the window between the response and turn/started. The window before the response is still the old bug, and the head now makes it easier to hit, because Stop ends the turn locally at once. Sequence: interrupt() sees no Codex id → interruptWanted = true and the turn ends locally. The user sends again → send() sets interruptWanted = false, and ourTurnId is null, so nothing is marked stale. Turn 1's turn/start response arrives → st.currentTurnId = codex-turn-1 (on top of turn 2's) with interruptWanted false → no turn/interrupt. Turn 1 runs to the end, and its turn/completed ends turn 2. Turn 2's own completion then comes out as an extra turn_ended under the raw Codex id. Probe: fake-codex-app-server.mjs delaying only the first turn/start response (PROBE_START_DELAY_MS=400, FAKE_CODEX_SLOW_MS=1500); send(first) not awaited → wait turn_started → interrupt() → wait turn_ended → send(second):

head 45a67b2: turn/interrupt sent: []
  turn_started u_…8drmh4o1 · turn_ended u_…8drmh4o1 interrupted · turn_started u_…vg2vit1y
  turn_ended u_…vg2vit1y completed   ← turn 1's completion (codex-turn-1 ran to the end)
  turn_ended codex-turn-2 completed  ← stray end
main:         turn/interrupt sent: ["codex-turn-1"], one interrupted + one completed

The pending interrupt should belong to the turn it was asked for, not be one flag that the next send() clears. For example, send() keeps its own turnId, and when its turn/start response arrives after that turn was interrupted locally (this.ourTurnId !== turnId), it adds the Codex id to staleCodexTurns, sends turn/interrupt for it, and leaves st.currentTurnId alone.

Also, Stop before turn/started still sends turn/interrupt from turn/start response id says it "is red without the response-id fix", but it passes on main (1277 ms): on main the late turn/started triggers the deferred interrupt, and the test's until(turn_ended interrupted) waits for it. Of the four codex tests, only after interrupt, send() works again… is red on main. Please add one that is red on main for the "Stop, then resend at once" case, for example the probe above, asserting turn/interrupt for codex-turn-1 and exactly one completed end.

What we ran (probe files are local only, not pushed): mote-run copies of 45a67b2 and of origin/main 39d4456 with your test files and our probe files copied in, then npx vitest run test/v1/{acp-interrupt,claude-cli,codex-interrupt,interrupt-audit,probe-review}.test.ts.

[qingnian108]

Pushed 8fd77eb addressing the remaining points:

• Claude: awaitingResult / skipResultGens now reset whenever a ClaudeCli is created, replaced, exits, or fails interrupt(), and old CLI stdout is ignored. Added ordering checks so the late-result test is red on main, plus tests for interrupt error restart and mid-turn CLI crash.
• Codex: replaced the single interruptWanted flag with per-turn cancelledLocalTurns + turn/start response ownership, so stopping while turn/start is in flight still sends turn/interrupt for codex-turn-1 without overwriting the next turn or emitting a stray turn_ended. Added tests that are red on main and 45a67b2.
• Real Devin CLI (devin 3000.11.3 on Windows): tested live Stop → immediate resend (~1ms). That caught one more ACP race where a 0ms resend hit Devin while the cancelled prompt was still unwinding; runTurn now waits briefly for session/cancel + the previous prompt settle before sending the next session/prompt (with unit coverage via FAKE_ACP_CANCEL_SETTLE_MS), and the live Stop → PONG resend now completes cleanly.

npm run check, npm test (68 files, 505 passed / 17 skipped), verify:build, and root bun checks all pass. Thanks @yetone!

[yetone]

Thanks. The interrupt work reads well, and the new fakes (late result, failed interrupt, crash, delayed cancel settle) cover the cases from the last review. I ran it at 8fd77eb on macOS (Node 22, HOME=$(mktemp -d)):

• npm ci --workspaces=false, npm run check and npm run build: all ok. daemon/dist came out byte-identical to the committed one.
• npx vitest run: 68 files, 505 passed, 17 skipped.

Then I broke each fix by hand and reran claude-cli, codex-interrupt, acp-interrupt, interrupt-audit and manager (43 tests). Three went red:

• Claude, interrupt no longer adds the gen to skipResultGens: 2 fail.
• Claude, the crash path no longer sets needsRestart: 2 fail.
• ACP, runTurn no longer waits for pendingCancelSettle: 1 fail.

These five still pass with the fix taken out:

1. Codex, interrupt() no longer adds the turn to cancelledLocalTurns while turn/start is in flight. The test "Stop while turn/start is in flight, then immediate resend interrupts codex-turn-1" says it is red without this, but it stays green.
2. Codex, the turn/start response handler drops || this.ourTurnId !== turnId and checks only cancelledLocalTurns. Each half passes alone, so either one of them is redundant (please remove it), or a test should show the case only that half covers.
3. ACP, interrupt() no longer adds activePromptGen to cancelledPromptGens. That includes the FAKE_ACP_CANCEL_AS_ERROR case it was written for.
4. Claude, pump() no longer ignores a replaced CLI's stdout (if (this.cli !== cli) continue;).
5. Claude, the crash path no longer calls resetResultFifo().

Please add a test for each that goes red when its line is removed, or remove the line if nothing needs it.

Two more:

6. Timing bars measured from before the first send. codex-interrupt (toBeLessThan(250) and toBeLessThan(350)) and claude-cli (toBeLessThan(delay - 50) and toBeLessThan(delay)) take t0 before send(first). That includes spawning the fake and initialize, so on a loaded machine they fail with nothing wrong. Please test the order instead of the wall clock. For example, let the fake hold the late result, the turn/start response or turn/started until the test releases it (a file or a stdin line). Then assert that the local interrupted and the second send both happen before the release. A test like that can't flake and still fails on main.
7. The Claude crash path keeps the event queue open (this.q.close() removed). The manager then no longer marks the thread closed after a crash, and the next send restarts with --resume. That is fine if intended. Please say so in the PR body, and add a manager-level test that a crashed Claude thread resumes on the next send.

[qingnian108]

Pushed 0cf12a9 addressing all 7 points from the mutation pass:

1–2. Codex (cancelledLocalTurns): Removed the set. After interrupt() ends locally, ourTurnId clears synchronously (or a newer send() replaces it), so this.ourTurnId !== turnId alone covers Stop-while-turn/start-in-flight with or without an immediate resend (and simplified the empty if (!codexTurn) branch). Mutating wasCancelled = false turns the in-flight Stop+resend test red.
3. ACP (cancelledPromptGens): Removed cancelledPromptGens (plus the now-unused promptGen / activePromptGen, and dropped the redundant && this.running half). interrupt()’s endTurn clears activeTurnId, which already makes late prompt settles and FAKE_ACP_CANCEL_AS_ERROR no-op; removing if (this.activeTurnId === turnId) goes red.
4. Claude (if (this.cli !== cli) continue): Kept and added a linger-stdout test (FAKE_CLAUDE_FAIL_LINGER_RESULT_MS) that goes red when the guard is removed.
5. Claude (resetResultFifo): Moved turnGen, awaitingResult, and skipResultGens onto ClaudeCli itself, eliminating currentGen and all secondary session-level resets.
6. Timing bars: Replaced the wall-clock toBeLessThan(...) assertions in codex-interrupt.test.ts and claude-cli.test.ts with hold-file release gates (FAKE_CLAUDE_HOLD_FILE, FAKE_CODEX_RESPONSE_HOLD_FILE, FAKE_CODEX_STARTED_HOLD_FILE), asserting that interrupted and the second send() happen before releasing the hold (red on main, flake-free under load).
7. Claude crash keeps event queue open: Intentional — noted in the PR body and covered by a new manager-level test verifying that a crashed Claude thread stays idle (not closed) and resumes with --resume <claudeSessionId> on the next send().

Verified every surviving guard goes red when removed; npm run check, npm test (68 files, 507 passed / 17 skipped), verify:build, and root bun checks all pass. Thanks @yetone!