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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Mohamed Boudra <boudra.moha@gmail.com>
This commit is contained in:
co-authored by
Claude
Mohamed Boudra
parent
5ef118ea33
commit
f275275074
@@ -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<string>;
|
||||
private query: Query | null = null;
|
||||
private childProcess: ChildProcess | null = null;
|
||||
private input: AsyncMessageInput<SDKUserMessage> | 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();
|
||||
|
||||
@@ -21,6 +21,8 @@ export interface ClaudeQueryContext {
|
||||
runtimeSettings?: ProviderRuntimeSettings;
|
||||
launchEnv?: Record<string, string>;
|
||||
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<string, string>,
|
||||
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),
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user