diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index a9ad273b..e501e6ec 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -108,47 +108,38 @@ export class ShellExecutionService { private static activePtys = new Map(); private static activeChildProcesses = new Set(); - static { - const cleanup = () => { - // Cleanup PTYs - for (const [pid, pty] of this.activePtys) { - try { - if (os.platform() === 'win32') { - pty.ptyProcess.kill(); - } else { - process.kill(-pid, 'SIGKILL'); - } - } catch { - // ignore + static cleanup() { + // Cleanup PTYs + for (const [pid, pty] of this.activePtys) { + try { + if (os.platform() === 'win32') { + pty.ptyProcess.kill(); + } else { + process.kill(-pid, 'SIGKILL'); } + } catch { + // ignore } - - // Cleanup child processes - for (const pid of this.activeChildProcesses) { - try { - if (os.platform() === 'win32') { - spawnSync('taskkill', ['/pid', pid.toString(), '/f', '/t']); - } else { - process.kill(-pid, 'SIGKILL'); - } - } catch { - // ignore - } - } - }; - - process.on('exit', cleanup); - - // Ensure cleanup happens on SIGINT/SIGTERM - const signalHandler = () => { - process.exit(); - }; - - // We only attach these if we are in a node environment where we can control the process - if (typeof process !== 'undefined' && process.on) { - process.on('SIGINT', signalHandler); - process.on('SIGTERM', signalHandler); } + + // Cleanup child processes + for (const pid of this.activeChildProcesses) { + try { + if (os.platform() === 'win32') { + spawnSync('taskkill', ['/pid', pid.toString(), '/f', '/t']); + } else { + process.kill(-pid, 'SIGKILL'); + } + } catch { + // ignore + } + } + } + + static { + process.on('exit', () => { + ShellExecutionService.cleanup(); + }); } /** diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index bde50837..b431f494 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -831,4 +831,33 @@ describe('ShellTool', () => { expect(shellTool.description).toMatchSnapshot(); }); }); + + describe('Windows background execution', () => { + it('should detect immediate failure in Windows background task', async () => { + vi.mocked(os.platform).mockReturnValue('win32'); + const mockAbortSignal = new AbortController().signal; + + const invocation = shellTool.build({ + command: 'invalid_command', + is_background: true, + }); + + const promise = invocation.execute(mockAbortSignal); + + // Wait a tick to ensure mockShellOutputCallback is assigned + await new Promise((resolve) => setTimeout(resolve, 0)); + + if (mockShellOutputCallback) { + mockShellOutputCallback({ + type: 'data', + chunk: + "'invalid_command' is not recognized as an internal or external command,\r\noperable program or batch file.\r\n", + }); + } + + const result = await promise; + expect(result.error).toBeDefined(); + expect(result.llmContent).toContain('Command failed to start'); + }); + }); }); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index b303b0fa..4ee7e79c 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -194,16 +194,16 @@ export class ShellToolInvocation extends BaseToolInvocation< commandToExecute, cwd, (event: ShellOutputEvent) => { - if (!updateOutput) { - return; - } - let shouldUpdate = false; switch (event.type) { case 'data': if (isBinaryStream) break; - cumulativeOutput = event.chunk; + if (typeof cumulativeOutput === 'string') { + cumulativeOutput += event.chunk; + } else { + cumulativeOutput = event.chunk; + } shouldUpdate = true; break; case 'binary_detected': @@ -226,7 +226,7 @@ export class ShellToolInvocation extends BaseToolInvocation< } } - if (shouldUpdate) { + if (shouldUpdate && updateOutput) { updateOutput( typeof cumulativeOutput === 'string' ? cumulativeOutput @@ -258,6 +258,32 @@ export class ShellToolInvocation extends BaseToolInvocation< if (raceResult === null) { // Timeout reached, process is still running. + // throw new Error(`DEBUG: raceResult is null. Output: ${JSON.stringify(cumulativeOutput)}`); + + // Check for common Windows error messages in the output + const outputStr = + typeof cumulativeOutput === 'string' + ? cumulativeOutput + : JSON.stringify(cumulativeOutput); + console.log('DEBUG: outputStr:', outputStr); + const errorPatterns = [ + 'is not recognized as an internal or external command', + 'The system cannot find the path specified', + 'Access is denied', + ]; + + if (errorPatterns.some((pattern) => outputStr.includes(pattern))) { + abortController.abort(); + return { + llmContent: `Command failed to start: ${outputStr}`, + returnDisplay: `Command failed to start: ${outputStr}`, + error: { + type: ToolErrorType.EXECUTION_FAILED, + message: `Command failed to start: ${outputStr}`, + }, + }; + } + const pidMsg = pid ? ` PID: ${pid}` : ''; const winHint = isWindows ? ' (Note: Use taskkill /F /T /PID to stop)'