From f275275074ecc7a786db15ffab3158de8e9fa40c Mon Sep 17 00:00:00 2001 From: Hu Sheng Date: Fri, 26 Jun 2026 22:02:25 +0800 Subject: [PATCH] fix(server): use terminateWithTreeKill in Claude Code provider close() (#1540) * fix(server): use terminateWithTreeKill in Claude Code provider close() The Claude Code agent's close() method only called SDK-level cleanup (query.close/interrupt/return) which may only kill the direct child process. MCP server processes spawned by the Claude CLI survived as orphans, accumulating over time and consuming significant memory. Fix: - Add onChildProcess callback to ClaudeQueryContext to capture the spawned child process reference - Store the child process in ClaudeAgentSession - In close(), call terminateWithTreeKill() after SDK cleanup to recursively terminate the entire process tree (claude + MCP children) - Use 2s graceful (SIGTERM) / 2s force (SIGKILL) timeouts, consistent with other providers (ACP, Codex, Pi, OpenCode) Co-Authored-By: Claude * fix(server): tree-kill old process during query restart to prevent MCP orphans Greptile review identified a gap: ensureQuery() replaces this.childProcess when spawning a new query without first killing the old process tree. During reconnection events, this can leak MCP children from the previous claude process. Fix: add terminateWithTreeKill() in the queryRestartNeeded block after SDK cleanup, before the replacement query spawns. Co-Authored-By: Claude --------- Co-authored-by: Claude Co-authored-by: Mohamed Boudra --- .../server/agent/providers/claude/agent.ts | 34 +++++++++++++++++++ .../server/agent/providers/claude/query.ts | 13 ++++--- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/packages/server/src/server/agent/providers/claude/agent.ts b/packages/server/src/server/agent/providers/claude/agent.ts index 62813bcec..7761d8ea0 100644 --- a/packages/server/src/server/agent/providers/claude/agent.ts +++ b/packages/server/src/server/agent/providers/claude/agent.ts @@ -1,3 +1,4 @@ +import type { ChildProcess } from "node:child_process"; import { randomUUID } from "node:crypto"; import fs from "node:fs"; import { promises } from "node:fs"; @@ -96,6 +97,7 @@ import { type ResolvedProviderLaunch, } from "../../provider-launch-config.js"; import { withTimeout } from "../../../../utils/promise-timeout.js"; +import { terminateWithTreeKill } from "../../../../utils/tree-kill.js"; import { execCommand } from "../../../../utils/spawn.js"; import { composeSystemPromptParts } from "../../system-prompt.js"; @@ -1880,6 +1882,7 @@ class ClaudeAgentSession implements AgentSession { private readonly queryFactory?: ClaudeQueryFactory; private readonly resolveBinary: () => Promise; private query: Query | null = null; + private childProcess: ChildProcess | null = null; private input: AsyncMessageInput | null = null; private claudeSessionId: string | null; private persistence: AgentPersistenceHandle | null; @@ -2353,6 +2356,22 @@ class ClaudeAgentSession implements AgentSession { await this.awaitWithTimeout(this.query?.return?.(), "close query return"); this.query = null; this.input = null; + // Terminate the entire process tree (claude + MCP children) to prevent + // orphan accumulation. The SDK's internal cleanup may only kill the + // direct child process. + if (this.childProcess) { + const result = await terminateWithTreeKill(this.childProcess, { + gracefulTimeoutMs: 2_000, + forceTimeoutMs: 2_000, + }); + if (result === "kill-timeout") { + this.logger.warn( + { pid: this.childProcess.pid, agentId: this.agentId }, + "Claude process tree did not report exit after SIGKILL", + ); + } + this.childProcess = null; + } if (this.persistSession === false && this.claudeSessionId) { // Claude Code currently ignores --no-session-persistence outside --print mode // (see `claude --help`), so the SDK's persistSession=false is silently dropped @@ -2751,6 +2770,18 @@ class ClaudeAgentSession implements AgentSession { } catch { /* ignore */ } + // Tree-kill the old process tree now that the SDK has cleaned up. + // If we skip this, MCP children of the previous claude process can + // survive as orphans when the session spawns a replacement query. + if (this.childProcess) { + await terminateWithTreeKill(this.childProcess, { + gracefulTimeoutMs: 2_000, + forceTimeoutMs: 2_000, + }).catch(() => { + /* process may already be dead */ + }); + this.childProcess = null; + } } // Preserve claudeSessionId across query recreation so buildOptions() passes @@ -2767,6 +2798,9 @@ class ClaudeAgentSession implements AgentSession { runtimeSettings: this.runtimeSettings, launchEnv: this.launchEnv, queryFactory: this.queryFactory, + onChildProcess: (child) => { + this.childProcess = child; + }, }, ); const fastMode = this.resolveFastModeSetting(); diff --git a/packages/server/src/server/agent/providers/claude/query.ts b/packages/server/src/server/agent/providers/claude/query.ts index a1917e400..ccd511816 100644 --- a/packages/server/src/server/agent/providers/claude/query.ts +++ b/packages/server/src/server/agent/providers/claude/query.ts @@ -21,6 +21,8 @@ export interface ClaudeQueryContext { runtimeSettings?: ProviderRuntimeSettings; launchEnv?: Record; queryFactory?: ClaudeQueryFactory; + /** Called with the spawned child process so the caller can tree-kill it on close. */ + onChildProcess?: (child: ChildProcess) => void; } function isChildProcessWithStreams(child: ChildProcess): child is ChildProcessWithoutNullStreams { @@ -54,9 +56,9 @@ function resolveClaudeSpawnCommand( function applyRuntimeSettingsToClaudeOptions( options: ClaudeOptions, - runtimeSettings?: ProviderRuntimeSettings, - launchEnv?: Record, + context: ClaudeQueryContext, ): ClaudeOptions { + const { runtimeSettings, launchEnv, onChildProcess } = context; return { ...options, spawnClaudeCodeProcess: (spawnOptions) => { @@ -94,6 +96,7 @@ function applyRuntimeSettingsToClaudeOptions( // The command is always a resolved binary path, so shell routing is unnecessary. shell: false, }); + onChildProcess?.(child); if (typeof options.stderr === "function") { child.stderr?.on("data", (chunk: Buffer | string) => { options.stderr?.(chunk.toString()); @@ -111,10 +114,6 @@ export function claudeQuery(input: ClaudeQueryInput, context: ClaudeQueryContext const launchQuery = context.queryFactory ?? query; return launchQuery({ ...input, - options: applyRuntimeSettingsToClaudeOptions( - input.options, - context.runtimeSettings, - context.launchEnv, - ), + options: applyRuntimeSettingsToClaudeOptions(input.options, context), }); }