Skip to content

fix(server): stop actually stops a runaway Claude turn - #3

Merged
xiaogwu merged 1 commit into
integrationfrom
server/stop-turn-hard-stop
Aug 17, 2026
Merged

fix(server): stop actually stops a runaway Claude turn#3
xiaogwu merged 1 commit into
integrationfrom
server/stop-turn-hard-stop

Conversation

@xiaogwu

@xiaogwu xiaogwu commented Aug 17, 2026

Copy link
Copy Markdown
Owner

The problem

Clicking Stop during a Claude turn could freeze every thread in the app for close to two minutes, and then not stop the agent at all.

Two defects combine:

  1. ClaudeAdapter.interruptTurn awaited context.query.interrupt() with no time bound. The Claude Agent SDK's interrupt is a control request that can block until the in-flight tool call returns. Measured twice on 2026-08-17 against claudeAgent: 114s and 111.5s.
  2. ProviderCommandReactor drains every thread command through a single DrainableWorker lane. One hung provider call therefore blocks all threads, on every client. Later Stop clicks were persisted to orchestration_events and never processed.

An acknowledged interrupt is also not proof the turn ended. The event log shows the agent kept working straight through the stall: a burst of identically-timestamped tool calls flushed the instant the queue cleared.

The fix

Stop now escalates instead of trusting the provider.

interruptTurn arms a detached watchdog, then asks the SDK to interrupt with a 5 second bound on the ack. If the same turn is still live 5 seconds later, the session is torn down through the existing stopSessionInternal. That path routes through completeTurn, which updates the resume cursor, so a forced teardown still leaves the thread resumable and the UI honest about the turn being interrupted.

Turn identity (context.turnState?.turnId compared against the id captured at interrupt time) is what distinguishes "same turn still live" from "turn ended" and "a new turn started". No new signal or Deferred was needed.

The watchdog runs on a make-level Effect.runForkWith capture, so under test it inherits TestClock and the escalation is deterministic rather than timing-dependent.

Deliberate non-change

The reactor's queue stays serial. Forking the interrupt off the shared lane would let a following turn-start-requested race ahead of a session teardown and prompt into a dying session. Bounding the adapter call shrinks the global block from ~112s to ~5s, which is the win without the race.

Scope

Only the Claude adapter changes. It is the only provider with observed hangs. Codex, Cursor, Grok, Gemini and OpenCode are not audited yet.

No client change is needed. Web, desktop and mobile all send the same thread.turn.interrupt request through packages/client-runtime/src/operations/commands.ts, so all three surfaces are fixed by the server change alone.

Verification

  • vp test run apps/server/src/provider/Layers/ClaudeAdapter.test.ts — 72 passed, including two new tests.
  • Negative check: with the fix reverted, the new never-acknowledged-interrupt test hangs to the 60s harness timeout. That is the bug reproducing.
  • tsgo --noEmit on apps/server — 0 errors.
  • vp lint on both files — clean (one no-useless-spread warning, pre-existing on main).
  • Not verified at runtime in a real client.

Upstream

Upstream knows about this bug and has not shipped a fix. Open issues #4524 and #4713 describe it. Open PR #5891 fixes it by always closing the session and never calling interrupt() at all, which discards the session even on a Stop that would have worked. This fork keeps the graceful path and escalates only when the agent ignores it. If pingdotgg#5891 merges, expect a conflict in these two files.

Written by Claude Opus 5 in T3 Code.

Clicking Stop during a Claude turn could freeze every thread in the app
for close to two minutes, and then not stop the agent at all.

Two defects combined:

1. `ClaudeAdapter.interruptTurn` awaited `context.query.interrupt()` with
   no time bound. The SDK's interrupt is a control request that can block
   until the in-flight tool call returns. Measured twice on 2026-08-17:
   114s and 111.5s.
2. `ProviderCommandReactor` drains every thread command through a single
   `DrainableWorker` lane, so that one hung call blocked all threads.
   Further Stop clicks were persisted and never processed.

An acknowledged interrupt is also not proof the turn ended. The event log
showed the agent kept working straight through the stall: a burst of
identically-timestamped tool calls flushed the moment the queue cleared.

So Stop now escalates. It arms a detached watchdog, asks the SDK to
interrupt with a 5 second bound on the ack, and if the same turn is still
live 5 seconds later it tears the session down through the existing
`stopSessionInternal`. That path routes through `completeTurn`, which
updates the resume cursor, so a forced teardown still leaves the thread
resumable.

The reactor's queue stays serial on purpose. Forking the interrupt would
let a following `turn-start-requested` race ahead of a session teardown
and prompt into a dying session. Bounding the adapter call shrinks the
global block from ~112s to ~5s, which is the win without the race.

Verified: 72 tests pass in ClaudeAdapter.test.ts. The new
never-acknowledged-interrupt test hangs to the 60s harness timeout
without the fix.

Only the Claude adapter is changed; it is the only provider with
observed hangs. Codex, Cursor, Grok and OpenCode are not audited yet.

Written by Claude Opus 5 in T3 Code.
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M labels Aug 17, 2026
@xiaogwu
xiaogwu marked this pull request as ready for review August 17, 2026 02:53
@xiaogwu
xiaogwu merged commit 5d50f74 into integration Aug 17, 2026
11 of 15 checks passed
xiaogwu pushed a commit that referenced this pull request Aug 17, 2026
….1116

origin/integration carried the two fork PRs (#3 Stop hard-stop, #4 palette
search scroll); local integration carried the nightly 1116 merge. Neither
had both. No conflicts: the sides touch different files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant