fix: handle windows background execution errors and add tests

This commit is contained in:
xuewenjie
2025-12-09 11:24:30 +08:00
parent 4c69d536ac
commit 28d178b5c1
3 changed files with 90 additions and 44 deletions

View File

@@ -108,8 +108,7 @@ export class ShellExecutionService {
private static activePtys = new Map<number, ActivePty>();
private static activeChildProcesses = new Set<number>();
static {
const cleanup = () => {
static cleanup() {
// Cleanup PTYs
for (const [pid, pty] of this.activePtys) {
try {
@@ -135,20 +134,12 @@ export class ShellExecutionService {
// 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);
}
static {
process.on('exit', () => {
ShellExecutionService.cleanup();
});
}
/**

View File

@@ -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');
});
});
});

View File

@@ -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;
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 <pid> to stop)'