Skip to content

fix: settle an aborted turn on Windows instead of leaving it pending - #217

Open
PerryLink wants to merge 2 commits into
0xsline:mainfrom
PerryLink:fix/windows-abort-settles
Open

PerryLink wants to merge 2 commits into
0xsline:mainfrom
PerryLink:fix/windows-abort-settles

Conversation

@PerryLink

@PerryLink PerryLink commented Oct 17, 2026 •

Copy link
Copy Markdown

What breaks

On Windows, aborting a running Claude Code turn never settles. The Stop button and the turn timeout both leave the turn promise pending: the spinner keeps going and the server-side run never reaches a terminal state. On Windows npm test also reaches this verify and never returns from it.

Two separate things combine in server/claude-code/turn-runner.ts:

  1. server/claude-code/command.ts spawns a .cmd through cmd.exe, so the CLI is a grandchild of the process the runner holds. onAbort only called child.kill(), which terminates cmd.exe; the real CLI keeps running and keeps the stdout write end open, so for await (const rawLine of rl) waits for an EOF that never arrives.
  2. child.once('exit') was registered after that loop, so once the loop could end the exit event had already fired and the following await promise would never settle either.

Measured on Windows with Node 24.21.0, running the project's own verify server/claude-code/claude-code-agent.verify.ts (the file npm run verify:server-runs invokes), against main at 72ddb33:

$ node server/claude-code/claude-code-agent.verify.ts   # under a 60 s wrapper that kills the process tree
[runtest] TIMEOUT after 60000ms -- killing process tree 13768
[runtest] file=server/claude-code/claude-code-agent.verify.ts exit=1 signal=null elapsed=60297ms timedOut=true

No output at all in 60 s. (The wrapper is 20 lines: spawn this file under the local node, kill the tree at the deadline, print the exit code and the elapsed time.)

An instrumented probe of the abort case alone shows where it stops:

[+   214ms] case B aborted
[+  2699ms] case B settled= false elapsed= 2587 ms grandchildAlive= true
[+  2699ms] RESULT: the turn promise is still pending -- the verify would hang here

grandchildAlive is process.kill(pid, 0) on the pid the fake CLI wrote to disk, i.e. the process child.kill() was supposed to remove.

The change

One file, server/claude-code/turn-runner.ts, +21 / -5.

  • onAbort now closes the readline interface and destroys child.stdout / child.stderr before child.kill(). Closing this side of the pipe is what ends the for await, because no EOF is coming from the grandchild.
  • The exit promise and its listener move above the read loop, and the code reads child.exitCode ?? (await exitPromise), so an exit that already happened is not missed.
  • finally destroys both pipes too. Without that a surviving grandchild leaves a readable handle in the event loop and the process never exits: on my first attempt the file printed ok and then hung past 60 s.

Same verify, same machine, patched:

$ node server/claude-code/claude-code-agent.verify.ts   # same 60 s wrapper
claude-code-agent.verify.ts: ok
[runtest] file=server/claude-code/claude-code-agent.verify.ts exit=0 signal=null elapsed=509ms timedOut=false

Five runs: 505, 507, 509, 511 and 518 ms, all ok with exit 0. The abort case on its own now settles in 192 ms (probe, patched) instead of never.

No assertion in the verify file was touched. The abort case still requires the emitted events to be exactly [{ type: 'session', sessionId: 'fake-session-hang' }], and the successful-turn, oversized-system-prompt and failing-turn cases all still pass.

What I deliberately did not change

  • The grandchild still survives the abort on Windows. child.kill() reaches cmd.exe only; I did not add taskkill /T or a job object. The verify file already acknowledges this by skipping its "the CLI process is gone" assertion on win32. Terminating the tree is a different decision about what abort guarantees, so this change only makes the turn settle. Worth a follow-up if you want the process itself gone.
  • No restructuring of the runner, no change to command.ts, no change to any other backend, and no change to the event translation.
  • No new dependency and no new test file.

What remains unproven

  • tsc -b and oxlint were not run: this machine has no checkout with node_modules. The change only uses Promise.withResolvers, child.exitCode and Readable#destroy, which are already available to this file.
  • The verify was run with Node 24 native type-stripping (node server/claude-code/claude-code-agent.verify.ts), not through tsx, because tsx is not installed here. The assertions that ran are the file's own, unmodified.
  • Only that one verify was run. server/agent-runs/claude-code-turn.verify.ts needs the app's dependency tree and could not run here.
  • The full npm test was not run, for the same reason. What is proven is the behaviour of the single file it invokes.
  • No Windows CI job covers server/claude-code/** — ci.yml runs npm test on ubuntu-latest and the windows-*.yml workflows filter on other paths — so CI will not exercise this path on Windows either way.

@0xsline 0xsline left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this fixes the hang, and registering the exit listener before the read loop is the right call.

One gap before we merge: on Windows the real Claude Code CLI still survives Stop. child.kill() ends cmd.exe, and closing our ends of the pipes lets the turn settle, but the grandchild keeps running. Your own probe shows grandchildAlive= true, and this patch doesn't change that. So after the user presses Stop (or the turn times out), the CLI can keep executing tool calls and editing the project in the background.

Could you terminate the whole process tree on win32? For example, run taskkill /pid <child.pid> /T /F (with windowsHide: true) in onAbort instead of relying on child.kill() there, and keep child.kill() for other platforms. Please also extend the abort case in the probe or verify to assert the grandchild is gone after abort, not only that the turn settles.

Happy to merge once the tree is killed.

\child.kill()\ ends the cmd.exe that \claudeCodeCommand()\ spawns for a
.cmd shim, but the CLI itself is that cmd.exe's child and keeps running:
it holds the stdout write end open and can keep editing the project after
the user pressed Stop.

Use \	askkill /pid <pid> /T /F\ on win32 and keep \child.kill()\
everywhere else.

Add turn-runner-abort.verify.ts, which runs the real turn against a
fake .cmd that starts a long-lived grandchild, aborts the turn, and
asserts both that it settles and that the grandchild is gone. It is
wired into verify:server-runs and is a no-op off Windows.
@PerryLink

Copy link
Copy Markdown
Author

Fixed in a598ce3.

  • killTurnTree() runs taskkill /pid <pid> /T /F with windowsHide: true on win32 and falls back to child.kill() when taskkill does not report success (and on every other platform).
  • server/claude-code/turn-runner-abort.verify.ts drives the real runClaudeCodeTurn against a fake claude.cmd that starts a long-lived Node grandchild, aborts once the grandchild's pid is written, and then asserts both that the turn settles and that the grandchild is gone. It is a no-op off Windows and is wired into verify:server-runs.

Against the previous child.kill() the new verify fails with the CLI grandchild <pid> survived the abort; with the tree kill it passes.

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.

2 participants