From a39461718c46869e7db746845f22e66194fcf763 Mon Sep 17 00:00:00 2001 From: Emily Hedlund Date: Wed, 8 Apr 2026 12:03:36 -0700 Subject: [PATCH] fix(core): ensure robust sandbox cleanup in all process execution paths (#24763) Co-authored-by: Spencer --- .../services/sandboxedFileSystemService.ts | 160 +++++----- .../src/services/shellExecutionService.ts | 38 ++- packages/core/src/tools/grep.ts | 36 ++- packages/core/src/tools/tool-registry.ts | 300 +++++++++--------- packages/core/src/utils/shell-utils.ts | 240 +++++++------- 5 files changed, 412 insertions(+), 362 deletions(-) diff --git a/packages/core/src/services/sandboxedFileSystemService.ts b/packages/core/src/services/sandboxedFileSystemService.ts index 03907657f3..d5e6dd4b4a 100644 --- a/packages/core/src/services/sandboxedFileSystemService.ts +++ b/packages/core/src/services/sandboxedFileSystemService.ts @@ -59,52 +59,56 @@ export class SandboxedFileSystemService implements FileSystemService { }, }); - return new Promise((resolve, reject) => { - // Direct spawn is necessary here for streaming large file contents. + try { + return await new Promise((resolve, reject) => { + // Direct spawn is necessary here for streaming large file contents. - const child = spawn(prepared.program, prepared.args, { - cwd: this.cwd, - env: prepared.env, - }); + const child = spawn(prepared.program, prepared.args, { + cwd: this.cwd, + env: prepared.env, + }); - let output = ''; - let error = ''; + let output = ''; + let error = ''; - child.stdout?.on('data', (data) => { - output += data.toString(); - }); + child.stdout?.on('data', (data) => { + output += data.toString(); + }); - child.stderr?.on('data', (data) => { - error += data.toString(); - }); + child.stderr?.on('data', (data) => { + error += data.toString(); + }); - child.on('close', (code) => { - if (code === 0) { - resolve(output); - } else { - const isEnoent = - error.toLowerCase().includes('no such file or directory') || - error.toLowerCase().includes('enoent') || - error.toLowerCase().includes('could not find file') || - error.toLowerCase().includes('could not find a part of the path'); - const err = new Error( - `Sandbox Error: read_file failed for '${filePath}'. Exit code ${code}. ${error ? 'Details: ' + error : ''}`, - ); - if (isEnoent) { - Object.assign(err, { code: 'ENOENT' }); + child.on('close', (code) => { + if (code === 0) { + resolve(output); + } else { + const isEnoent = + error.toLowerCase().includes('no such file or directory') || + error.toLowerCase().includes('enoent') || + error.toLowerCase().includes('could not find file') || + error.toLowerCase().includes('could not find a part of the path'); + const err = new Error( + `Sandbox Error: read_file failed for '${filePath}'. Exit code ${code}. ${error ? 'Details: ' + error : ''}`, + ); + if (isEnoent) { + Object.assign(err, { code: 'ENOENT' }); + } + reject(err); } - reject(err); - } - }); + }); - child.on('error', (err) => { - reject( - new Error( - `Sandbox Error: Failed to spawn read_file for '${filePath}': ${err.message}`, - ), - ); + child.on('error', (err) => { + reject( + new Error( + `Sandbox Error: Failed to spawn read_file for '${filePath}': ${err.message}`, + ), + ); + }); }); - }); + } finally { + prepared.cleanup?.(); + } } async writeTextFile(filePath: string, content: string): Promise { @@ -124,53 +128,57 @@ export class SandboxedFileSystemService implements FileSystemService { }, }); - return new Promise((resolve, reject) => { - // Direct spawn is necessary here for streaming large file contents. + try { + return await new Promise((resolve, reject) => { + // Direct spawn is necessary here for streaming large file contents. - const child = spawn(prepared.program, prepared.args, { - cwd: this.cwd, - env: prepared.env, - }); + const child = spawn(prepared.program, prepared.args, { + cwd: this.cwd, + env: prepared.env, + }); - child.stdin?.on('error', (err) => { - // Silently ignore EPIPE errors on stdin, they will be caught by the process error/close listeners - if (isNodeError(err) && err.code === 'EPIPE') { - return; - } - debugLogger.error( - `Sandbox Error: stdin error for '${filePath}': ${ - err instanceof Error ? err.message : String(err) - }`, - ); - }); + child.stdin?.on('error', (err) => { + // Silently ignore EPIPE errors on stdin, they will be caught by the process error/close listeners + if (isNodeError(err) && err.code === 'EPIPE') { + return; + } + debugLogger.error( + `Sandbox Error: stdin error for '${filePath}': ${ + err instanceof Error ? err.message : String(err) + }`, + ); + }); - child.stdin?.write(content); - child.stdin?.end(); + child.stdin?.write(content); + child.stdin?.end(); - let error = ''; - child.stderr?.on('data', (data) => { - error += data.toString(); - }); + let error = ''; + child.stderr?.on('data', (data) => { + error += data.toString(); + }); - child.on('close', (code) => { - if (code === 0) { - resolve(); - } else { + child.on('close', (code) => { + if (code === 0) { + resolve(); + } else { + reject( + new Error( + `Sandbox Error: write_file failed for '${filePath}'. Exit code ${code}. ${error ? 'Details: ' + error : ''}`, + ), + ); + } + }); + + child.on('error', (err) => { reject( new Error( - `Sandbox Error: write_file failed for '${filePath}'. Exit code ${code}. ${error ? 'Details: ' + error : ''}`, + `Sandbox Error: Failed to spawn write_file for '${filePath}': ${err.message}`, ), ); - } + }); }); - - child.on('error', (err) => { - reject( - new Error( - `Sandbox Error: Failed to spawn write_file for '${filePath}': ${err.message}`, - ), - ); - }); - }); + } finally { + prepared.cleanup?.(); + } } } diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index dfbb3a5033..46b894426f 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -510,21 +510,24 @@ export class ShellExecutionService { shellExecutionConfig: ShellExecutionConfig, isInteractive: boolean, ): Promise { + let cmdCleanup: (() => void) | undefined; try { const isWindows = os.platform() === 'win32'; + const prepared = await this.prepareExecution( + commandToExecute, + cwd, + shellExecutionConfig, + isInteractive, + ); + cmdCleanup = prepared.cleanup; + const { program: finalExecutable, args: finalArgs, env: finalEnv, cwd: finalCwd, - cleanup: cmdCleanup, - } = await this.prepareExecution( - commandToExecute, - cwd, - shellExecutionConfig, - isInteractive, - ); + } = prepared; const child = cpSpawn(finalExecutable, finalArgs, { cwd: finalCwd, @@ -811,6 +814,7 @@ export class ShellExecutionService { } catch (e) { // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion const error = e as Error; + cmdCleanup?.(); return { pid: undefined, result: Promise.resolve({ @@ -826,7 +830,6 @@ export class ShellExecutionService { }; } } - private static async executeWithPty( commandToExecute: string, cwd: string, @@ -840,23 +843,26 @@ export class ShellExecutionService { throw new Error('PTY implementation not found'); } let spawnedPty: IPty | undefined; + let cmdCleanup: (() => void) | undefined; try { const cols = shellExecutionConfig.terminalWidth ?? 80; const rows = shellExecutionConfig.terminalHeight ?? 30; + const prepared = await this.prepareExecution( + commandToExecute, + cwd, + shellExecutionConfig, + true, + ); + cmdCleanup = prepared.cleanup; + const { program: finalExecutable, args: finalArgs, env: finalEnv, cwd: finalCwd, - cleanup: cmdCleanup, - } = await this.prepareExecution( - commandToExecute, - cwd, - shellExecutionConfig, - true, - ); + } = prepared; // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const ptyProcess = ptyInfo.module.spawn(finalExecutable, finalArgs, { @@ -1237,6 +1243,7 @@ export class ShellExecutionService { } catch (e) { // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion const error = e as Error; + cmdCleanup?.(); if (spawnedPty) { try { @@ -1270,7 +1277,6 @@ export class ShellExecutionService { } } } - /** * Writes a string to the pseudo-terminal (PTY) of a running process. * diff --git a/packages/core/src/tools/grep.ts b/packages/core/src/tools/grep.ts index ac7dc6cf02..3f6fd08ff3 100644 --- a/packages/core/src/tools/grep.ts +++ b/packages/core/src/tools/grep.ts @@ -326,6 +326,7 @@ class GrepToolInvocation extends BaseToolInvocation< let finalCommand = checkCommand; let finalArgs = checkArgs; let finalEnv = process.env; + let cleanup: (() => void) | undefined; if (sandboxManager) { try { @@ -338,6 +339,7 @@ class GrepToolInvocation extends BaseToolInvocation< finalCommand = prepared.program; finalArgs = prepared.args; finalEnv = prepared.env; + cleanup = prepared.cleanup; } catch (err) { debugLogger.debug( `[GrepTool] Sandbox preparation failed for '${command}':`, @@ -346,21 +348,27 @@ class GrepToolInvocation extends BaseToolInvocation< } } - return await new Promise((resolve) => { - const child = spawn(finalCommand, finalArgs, { - stdio: 'ignore', - shell: true, - env: finalEnv, + try { + return await new Promise((resolve) => { + const child = spawn(finalCommand, finalArgs, { + stdio: 'ignore', + shell: true, + env: finalEnv, + }); + child.on('close', (code) => { + resolve(code === 0); + }); + child.on('error', (err) => { + debugLogger.debug( + `[GrepTool] Failed to start process for '${command}':`, + err.message, + ); + resolve(false); + }); }); - child.on('close', (code) => resolve(code === 0)); - child.on('error', (err) => { - debugLogger.debug( - `[GrepTool] Failed to start process for '${command}':`, - err.message, - ); - resolve(false); - }); - }); + } finally { + cleanup?.(); + } } catch { return false; } diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index f9551d75da..5b174a97d7 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -65,6 +65,7 @@ class DiscoveredToolInvocation extends BaseToolInvocation< let finalCommand = callCommand; let finalArgs = args; let finalEnv = process.env; + let cleanupFunc: (() => void) | undefined; const sandboxManager = this.config.sandboxManager; if (sandboxManager) { @@ -77,58 +78,63 @@ class DiscoveredToolInvocation extends BaseToolInvocation< finalCommand = prepared.program; finalArgs = prepared.args; finalEnv = prepared.env; + cleanupFunc = prepared.cleanup; } - const child = spawn(finalCommand, finalArgs, { - env: finalEnv, - }); - child.stdin.write(JSON.stringify(this.params)); - child.stdin.end(); - let stdout = ''; let stderr = ''; let error: Error | null = null; let code: number | null = null; let signal: NodeJS.Signals | null = null; - await new Promise((resolve) => { - const onStdout = (data: Buffer) => { - stdout += data?.toString(); - }; + try { + const child = spawn(finalCommand, finalArgs, { + env: finalEnv, + }); + child.stdin.write(JSON.stringify(this.params)); + child.stdin.end(); - const onStderr = (data: Buffer) => { - stderr += data?.toString(); - }; + await new Promise((resolve) => { + const onStdout = (data: Buffer) => { + stdout += data?.toString(); + }; - const onError = (err: Error) => { - error = err; - }; + const onStderr = (data: Buffer) => { + stderr += data?.toString(); + }; - const onClose = ( - _code: number | null, - _signal: NodeJS.Signals | null, - ) => { - code = _code; - signal = _signal; - cleanup(); - resolve(); - }; + const onError = (err: Error) => { + error = err; + }; - const cleanup = () => { - child.stdout.removeListener('data', onStdout); - child.stderr.removeListener('data', onStderr); - child.removeListener('error', onError); - child.removeListener('close', onClose); - if (child.connected) { - child.disconnect(); - } - }; + const onClose = ( + _code: number | null, + _signal: NodeJS.Signals | null, + ) => { + code = _code; + signal = _signal; + cleanup(); + resolve(); + }; - child.stdout.on('data', onStdout); - child.stderr.on('data', onStderr); - child.on('error', onError); - child.on('close', onClose); - }); + const cleanup = () => { + child.stdout.removeListener('data', onStdout); + child.stderr.removeListener('data', onStderr); + child.removeListener('error', onError); + child.removeListener('close', onClose); + if (child.connected) { + child.disconnect(); + } + }; + + child.stdout.on('data', onStdout); + child.stderr.on('data', onStderr); + child.on('error', onError); + child.on('close', onClose); + }); + } finally { + cleanupFunc?.(); + } // if there is any error, non-zero exit code, signal, or stderr, return error details instead of stdout if (error || code !== 0 || signal || stderr) { @@ -374,6 +380,7 @@ export class ToolRegistry { .slice(1) .filter((p): p is string => typeof p === 'string'); let finalEnv = process.env; + let cleanupFunc: (() => void) | undefined; const sandboxManager = this.config.sandboxManager; if (sandboxManager) { @@ -386,118 +393,127 @@ export class ToolRegistry { finalCommand = prepared.program; finalArgs = prepared.args; finalEnv = prepared.env; + cleanupFunc = prepared.cleanup; } - const proc = spawn(finalCommand, finalArgs, { - env: finalEnv, - }); - let stdout = ''; - const stdoutDecoder = new StringDecoder('utf8'); - let stderr = ''; - const stderrDecoder = new StringDecoder('utf8'); - let sizeLimitExceeded = false; - const MAX_STDOUT_SIZE = 10 * 1024 * 1024; // 10MB limit - const MAX_STDERR_SIZE = 10 * 1024 * 1024; // 10MB limit - - let stdoutByteLength = 0; - let stderrByteLength = 0; - - proc.stdout.on('data', (data) => { - if (sizeLimitExceeded) return; - if (stdoutByteLength + data.length > MAX_STDOUT_SIZE) { - sizeLimitExceeded = true; - proc.kill(); - return; - } - stdoutByteLength += data.length; - stdout += stdoutDecoder.write(data); - }); - - proc.stderr.on('data', (data) => { - if (sizeLimitExceeded) return; - if (stderrByteLength + data.length > MAX_STDERR_SIZE) { - sizeLimitExceeded = true; - proc.kill(); - return; - } - stderrByteLength += data.length; - stderr += stderrDecoder.write(data); - }); - - await new Promise((resolve, reject) => { - proc.on('error', reject); - proc.on('close', (code) => { - stdout += stdoutDecoder.end(); - stderr += stderrDecoder.end(); - - if (sizeLimitExceeded) { - return reject( - new Error( - `Tool discovery command output exceeded size limit of ${MAX_STDOUT_SIZE} bytes.`, - ), - ); - } - - if (code !== 0) { - coreEvents.emitFeedback( - 'error', - `Tool discovery command failed with code ${code}.`, - stderr, - ); - return reject( - new Error(`Tool discovery command failed with exit code ${code}`), - ); - } - resolve(); + try { + const proc = spawn(finalCommand, finalArgs, { + env: finalEnv, }); - }); + let stdout = ''; + const stdoutDecoder = new StringDecoder('utf8'); + let stderr = ''; + const stderrDecoder = new StringDecoder('utf8'); + let sizeLimitExceeded = false; + const MAX_STDOUT_SIZE = 10 * 1024 * 1024; // 10MB limit + const MAX_STDERR_SIZE = 10 * 1024 * 1024; // 10MB limit - // execute discovery command and extract function declarations (w/ or w/o "tool" wrappers) - const functions: FunctionDeclaration[] = []; - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const discoveredItems = JSON.parse(stdout.trim()); + let stdoutByteLength = 0; + let stderrByteLength = 0; - if (!discoveredItems || !Array.isArray(discoveredItems)) { - throw new Error( - 'Tool discovery command did not return a JSON array of tools.', - ); - } + proc.stdout.on('data', (data) => { + if (sizeLimitExceeded) return; + if (stdoutByteLength + data.length > MAX_STDOUT_SIZE) { + sizeLimitExceeded = true; + proc.kill(); + return; + } + stdoutByteLength += data.length; + stdout += stdoutDecoder.write(data); + }); - for (const tool of discoveredItems) { - if (tool && typeof tool === 'object') { - if (Array.isArray(tool['function_declarations'])) { - functions.push(...tool['function_declarations']); - } else if (Array.isArray(tool['functionDeclarations'])) { - functions.push(...tool['functionDeclarations']); - } else if (tool['name']) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - functions.push(tool as FunctionDeclaration); + proc.stderr.on('data', (data) => { + if (sizeLimitExceeded) return; + if (stderrByteLength + data.length > MAX_STDERR_SIZE) { + sizeLimitExceeded = true; + proc.kill(); + return; + } + stderrByteLength += data.length; + stderr += stderrDecoder.write(data); + }); + + await new Promise((resolve, reject) => { + proc.on('error', (err) => { + reject(err); + }); + proc.on('close', (code) => { + stdout += stdoutDecoder.end(); + stderr += stderrDecoder.end(); + + if (sizeLimitExceeded) { + return reject( + new Error( + `Tool discovery command output exceeded size limit of ${MAX_STDOUT_SIZE} bytes.`, + ), + ); + } + + if (code !== 0) { + coreEvents.emitFeedback( + 'error', + `Tool discovery command failed with code ${code}.`, + stderr, + ); + return reject( + new Error( + `Tool discovery command failed with exit code ${code}`, + ), + ); + } + resolve(); + }); + }); + + // execute discovery command and extract function declarations (w/ or w/o "tool" wrappers) + const functions: FunctionDeclaration[] = []; + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const discoveredItems = JSON.parse(stdout.trim()); + + if (!discoveredItems || !Array.isArray(discoveredItems)) { + throw new Error( + 'Tool discovery command did not return a JSON array of tools.', + ); + } + + for (const tool of discoveredItems) { + if (tool && typeof tool === 'object') { + if (Array.isArray(tool['function_declarations'])) { + functions.push(...tool['function_declarations']); + } else if (Array.isArray(tool['functionDeclarations'])) { + functions.push(...tool['functionDeclarations']); + } else if (tool['name']) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + functions.push(tool as FunctionDeclaration); + } } } - } - // register each function as a tool - for (const func of functions) { - if (!func.name) { - debugLogger.warn('Discovered a tool with no name. Skipping.'); - continue; + // register each function as a tool + for (const func of functions) { + if (!func.name) { + debugLogger.warn('Discovered a tool with no name. Skipping.'); + continue; + } + const parameters = + func.parametersJsonSchema && + typeof func.parametersJsonSchema === 'object' && + !Array.isArray(func.parametersJsonSchema) + ? func.parametersJsonSchema + : {}; + this.registerTool( + new DiscoveredTool( + this.config, + func.name, + DISCOVERED_TOOL_PREFIX + func.name, + func.description ?? '', + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + parameters as Record, + this.messageBus, + ), + ); } - const parameters = - func.parametersJsonSchema && - typeof func.parametersJsonSchema === 'object' && - !Array.isArray(func.parametersJsonSchema) - ? func.parametersJsonSchema - : {}; - this.registerTool( - new DiscoveredTool( - this.config, - func.name, - DISCOVERED_TOOL_PREFIX + func.name, - func.description ?? '', - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - parameters as Record, - this.messageBus, - ), - ); + } finally { + cleanupFunc?.(); } } catch (e) { debugLogger.error(`Tool discovery command "${discoveryCmd}" failed:`, e); diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 8486be0de9..46cffa1d35 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -847,34 +847,40 @@ export const spawnAsync = async ( const { program: finalCommand, args: finalArgs, env: finalEnv } = prepared; - return new Promise((resolve, reject) => { - const child = spawn(finalCommand, finalArgs, { - ...options, - env: finalEnv, - }); - let stdout = ''; - let stderr = ''; + try { + return await new Promise((resolve, reject) => { + const child = spawn(finalCommand, finalArgs, { + ...options, + env: finalEnv, + }); + let stdout = ''; + let stderr = ''; - child.stdout.on('data', (data) => { - stdout += data.toString(); - }); + child.stdout.on('data', (data) => { + stdout += data.toString(); + }); - child.stderr.on('data', (data) => { - stderr += data.toString(); - }); + child.stderr.on('data', (data) => { + stderr += data.toString(); + }); - child.on('close', (code) => { - if (code === 0) { - resolve({ stdout, stderr }); - } else { - reject(new Error(`Command failed with exit code ${code}:\n${stderr}`)); - } - }); + child.on('close', (code) => { + if (code === 0) { + resolve({ stdout, stderr }); + } else { + reject( + new Error(`Command failed with exit code ${code}:\n${stderr}`), + ); + } + }); - child.on('error', (err) => { - reject(err); + child.on('error', (err) => { + reject(err); + }); }); - }); + } finally { + prepared.cleanup?.(); + } }; /** @@ -902,109 +908,115 @@ export async function* execStreaming( env: options?.env ?? process.env, }); - const { program: finalCommand, args: finalArgs, env: finalEnv } = prepared; - - const child = spawn(finalCommand, finalArgs, { - ...options, - env: finalEnv, - // ensure we don't open a window on windows if possible/relevant - windowsHide: true, - }); - - const rl = readline.createInterface({ - input: child.stdout, - terminal: false, - }); - - const errorChunks: Buffer[] = []; - let stderrTotalBytes = 0; - const MAX_STDERR_BYTES = 20 * 1024; // 20KB limit - - child.stderr.on('data', (chunk) => { - if (stderrTotalBytes < MAX_STDERR_BYTES) { - errorChunks.push(chunk); - stderrTotalBytes += chunk.length; - } - }); - - let error: Error | null = null; - child.on('error', (err) => { - error = err; - }); - - const onAbort = () => { - // If manually aborted by signal, we kill immediately. - if (!child.killed) child.kill(); - }; - - if (options?.signal?.aborted) { - onAbort(); - } else { - options?.signal?.addEventListener('abort', onAbort); - } - - let finished = false; try { - for await (const line of rl) { - if (options?.signal?.aborted) break; - yield line; - } - finished = true; - } finally { - rl.close(); - options?.signal?.removeEventListener('abort', onAbort); + const { program: finalCommand, args: finalArgs, env: finalEnv } = prepared; - // Ensure process is killed when the generator is closed (consumer breaks loop) - let killedByGenerator = false; - if (!finished && child.exitCode === null && !child.killed) { - try { - child.kill(); - } catch { - // ignore error if process is already dead + const child = spawn(finalCommand, finalArgs, { + ...options, + env: finalEnv, + // ensure we don't open a window on windows if possible/relevant + windowsHide: true, + }); + + const rl = readline.createInterface({ + input: child.stdout, + terminal: false, + }); + + const errorChunks: Buffer[] = []; + let stderrTotalBytes = 0; + const MAX_STDERR_BYTES = 20 * 1024; // 20KB limit + + child.stderr.on('data', (chunk) => { + if (stderrTotalBytes < MAX_STDERR_BYTES) { + errorChunks.push(chunk); + stderrTotalBytes += chunk.length; } - killedByGenerator = true; + }); + + let error: Error | null = null; + child.on('error', (err) => { + error = err; + }); + + const onAbort = () => { + // If manually aborted by signal, we kill immediately. + if (!child.killed) child.kill(); + }; + + if (options?.signal?.aborted) { + onAbort(); + } else { + options?.signal?.addEventListener('abort', onAbort); } - // Ensure we wait for the process to exit to check codes - await new Promise((resolve, reject) => { - // If an error occurred before we got here (e.g. spawn failure), reject immediately. - if (error) { - reject(error); - return; + let finished = false; + try { + for await (const line of rl) { + if (options?.signal?.aborted) break; + yield line; + } + finished = true; + } finally { + rl.close(); + options?.signal?.removeEventListener('abort', onAbort); + + // Ensure process is killed when the generator is closed (consumer breaks loop) + let killedByGenerator = false; + if (!finished && child.exitCode === null && !child.killed) { + try { + child.kill(); + } catch { + // ignore error if process is already dead + } + killedByGenerator = true; } - function checkExit(code: number | null) { - // If we aborted or killed it manually, we treat it as success (stop waiting) - if (options?.signal?.aborted || killedByGenerator) { - resolve(); + // Ensure we wait for the process to exit to check codes + await new Promise((resolve, reject) => { + // If an error occurred before we got here (e.g. spawn failure), reject immediately. + if (error) { + reject(error); return; } - const allowed = options?.allowedExitCodes ?? [0]; - if (code !== null && allowed.includes(code)) { - resolve(); - } else { - // If we have an accumulated error or explicit error event - if (error) reject(error); - else { - const stderr = Buffer.concat(errorChunks).toString('utf8'); - const truncatedMsg = - stderrTotalBytes >= MAX_STDERR_BYTES ? '...[truncated]' : ''; - reject( - new Error( - `Process exited with code ${code}: ${stderr}${truncatedMsg}`, - ), - ); + function checkExit(code: number | null) { + // If we aborted or killed it manually, we treat it as success (stop waiting) + if (options?.signal?.aborted || killedByGenerator) { + resolve(); + return; + } + + const allowed = options?.allowedExitCodes ?? [0]; + if (code !== null && allowed.includes(code)) { + resolve(); + } else { + // If we have an accumulated error or explicit error event + if (error) reject(error); + else { + const stderr = Buffer.concat(errorChunks).toString('utf8'); + const truncatedMsg = + stderrTotalBytes >= MAX_STDERR_BYTES ? '...[truncated]' : ''; + reject( + new Error( + `Process exited with code ${code}: ${stderr}${truncatedMsg}`, + ), + ); + } } } - } - if (child.exitCode !== null) { - checkExit(child.exitCode); - } else { - child.on('close', (code) => checkExit(code)); - child.on('error', (err) => reject(err)); - } - }); + if (child.exitCode !== null) { + checkExit(child.exitCode); + } else { + child.on('close', (code) => checkExit(code)); + child.on('error', (err) => { + reject(err); + }); + } + }); + } + } finally { + prepared.cleanup?.(); } }