From 79753667549775993a488cfc7d416d2d7008bfca Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Fri, 24 Jul 2026 06:22:53 +0200 Subject: [PATCH] fix: actually kill the Claude process tree on stop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking Stop showed 'Claude code was stopped.' (plus a misleading red 'Error running Claude: The operation was aborted' box) but the work kept running until VS Code was closed (#178, #115). Root cause is the kill ordering in _killClaudeProcess. abort() ran first: on Windows the tracked PID is the cmd.exe shell from spawn(..., shell: true), so abort() synchronously killed only that shell and set child.killed. The taskkill /pid X /t /f that followed — the correct tree kill — hit an already-dead root, could not enumerate the process tree anymore and failed silently (errors are swallowed), so the real claude worker (a grandchild) and everything it spawned orphaned and kept running. The pre-set killed flag also turned the exit-wait/SIGKILL escalation into dead code, and the synchronous AbortError surfaced as the red error box. Fix: invert the order. 1. Clear _currentClaudeProcess first (before any await) so close/error events firing mid-kill hit the handlers' no-current-process guards and no-op — no false error box. 2. Tree-kill (SIGTERM) while the root is still alive, wait up to 2s for the exit — 'exitCode !== null' also covers a death Node observed during the wait, since taskkill never sets killed — then escalate to SIGKILL only if the process is really still running. 3. abort() last, purely as cleanup of the AbortController; its late AbortError is dropped by the error handler's guard. All kill paths benefit: the Stop button, New Chat and panel disposal. Unix/WSL branches of _killProcessGroup are untouched. Co-Authored-By: Claude Fable 5 --- src/extension.ts | 65 ++++++++++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index 8fa37fb..b234b83 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -3168,41 +3168,52 @@ class ClaudeChatProvider { const processToKill = this._currentClaudeProcess; const pid = processToKill?.pid; - // 1. Abort via controller (clean API) - this._abortController?.abort(); - this._abortController = undefined; - - // 2. Clear reference immediately + // Ordering invariant: clear the reference FIRST (before any await and + // before abort), tree-kill SECOND while the process is still alive, and call + // abort() LAST. Previously abort() ran first — on Windows that synchronously + // kills the tracked root (the cmd.exe shell from shell:true) and sets + // `killed`, so the taskkill /t that followed hit an already-dead root, + // couldn't enumerate the tree, and failed silently (try/catch) — the real + // claude worker and its children orphaned and kept running. Clearing the + // reference before the kill sequence also means a close/error firing + // mid-kill hits the handlers' "no current process" guards and no-ops + // (no false error box). + + // 1. Clear reference immediately so late close/error handlers no-op. this._currentClaudeProcess = undefined; - if (!pid) { - return; - } + if (pid) { + // 2. Kill process group (handles children) while the tree is still alive. + await this._killProcessGroup(pid, 'SIGTERM'); + + // 3. Wait for process to exit, with timeout. `exitCode !== null` also + // covers a death Node observed during the await above (taskkill kills + // externally and never sets `killed`, which only child.kill() would). + const exitPromise = new Promise((resolve) => { + if (!processToKill || processToKill.killed || processToKill.exitCode !== null) { + resolve(); + return; + } + processToKill.once('exit', () => resolve()); + }); + const timeoutPromise = new Promise((resolve) => { + setTimeout(() => resolve(), 2000); + }); - // 3. Kill process group (handles children) - await this._killProcessGroup(pid, 'SIGTERM'); + await Promise.race([exitPromise, timeoutPromise]); - // 4. Wait for process to exit, with timeout - const exitPromise = new Promise((resolve) => { - if (processToKill?.killed) { - resolve(); - return; + // 4. Force kill if still running + if (processToKill && !processToKill.killed && processToKill.exitCode === null) { + await this._killProcessGroup(pid, 'SIGKILL'); } - processToKill?.once('exit', () => resolve()); - }); - - const timeoutPromise = new Promise((resolve) => { - setTimeout(() => resolve(), 2000); - }); - - await Promise.race([exitPromise, timeoutPromise]); - - // 5. Force kill if still running - if (processToKill && !processToKill.killed) { - await this._killProcessGroup(pid, 'SIGKILL'); } + // 5. Abort via controller LAST. The tree is already dead, and because the + // _currentClaudeProcess reference is cleared, the error handler's guard + // drops any late AbortError — no red "operation was aborted" box. + this._abortController?.abort(); + this._abortController = undefined; } private async _stopClaudeProcess(): Promise {