Skip to content

Commit d39f405

Browse files
committed
🤖 fix: report correct exit codes for signal-terminated background processes
When a background process is killed by signal (SIGTERM, SIGKILL, etc.), Node's exit event provides code=null and signal name. The spawnBackground handler was defaulting null to 0, making killed processes appear as successful exits. - Add getExitCode() helper to convert signals to Unix-conventional exit codes (128 + signal_number): SIGTERM=143, SIGKILL=137, etc. - Add test verifying terminated processes report exit code >= 128 This restores behavior that was lost when bashExecutionService.ts was removed during the Runtime refactor.
1 parent 8bcee44 commit d39f405

File tree

2 files changed

+44
-2
lines changed

2 files changed

+44
-2
lines changed

‎src/node/runtime/LocalRuntime.ts‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,27 @@ import { getProjectName } from "@/node/utils/runtime/helpers";
3535
import { getErrorMessage } from "@/common/utils/errors";
3636
import { once } from "node:events";
3737
import { log } from "@/node/services/log";
38+
39+
/**
40+
* Convert Node.js exit event args to a single exit code.
41+
* When a process is killed by signal, Node provides code=null and signal name.
42+
* This converts to Unix-conventional exit codes (128 + signal_number).
43+
*/
44+
function getExitCode(code: number | null, signal: NodeJS.Signals | null): number {
45+
if (code !== null) return code;
46+
if (signal) {
47+
const signalNumbers: Record<string, number> = {
48+
SIGHUP: 1,
49+
SIGINT: 2,
50+
SIGQUIT: 3,
51+
SIGKILL: 9,
52+
SIGTERM: 15,
53+
};
54+
return 128 + (signalNumbers[signal] ?? 0);
55+
}
56+
return 0;
57+
}
58+
3859
import { expandTilde } from "./tildeExpansion";
3960

4061
/**
@@ -246,11 +267,11 @@ export class LocalRuntime implements Runtime {
246267
}
247268
});
248269

249-
childProcess.on("exit", (code) => {
270+
childProcess.on("exit", (code, signal) => {
250271
// Flush remaining partial lines
251272
if (stdoutBuffer) handle._emitStdout(stdoutBuffer);
252273
if (stderrBuffer) handle._emitStderr(stderrBuffer);
253-
handle._emitExit(code ?? 0);
274+
handle._emitExit(getExitCode(code, signal));
254275
});
255276

256277
handle = new LocalBackgroundHandle(disposable);

‎src/node/services/backgroundProcessManager.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,5 +224,26 @@ describe("BackgroundProcessManager", () => {
224224
expect(proc?.status).toBe("killed");
225225
}
226226
});
227+
228+
it("should report non-zero exit code for signal-terminated processes", async () => {
229+
// Spawn a long-running process
230+
const result = await manager.spawn(runtime, testWorkspaceId, "sleep 60", {
231+
cwd: process.cwd(),
232+
});
233+
234+
if (result.success) {
235+
// Terminate it (sends SIGTERM, then SIGKILL after 2s)
236+
await manager.terminate(result.processId);
237+
238+
// Wait for exit to be recorded
239+
await new Promise((resolve) => setTimeout(resolve, 100));
240+
241+
const proc = manager.getProcess(result.processId);
242+
expect(proc).not.toBeNull();
243+
// Exit code should be 128 + signal number (SIGTERM=15 → 143, SIGKILL=9 → 137)
244+
// Either is acceptable depending on timing
245+
expect(proc!.exitCode).toBeGreaterThanOrEqual(128);
246+
}
247+
});
227248
});
228249
});

0 commit comments

Comments
 (0)