Skip to content

Commit 0680866

Browse files
committed
Terminate the process on SIGINT, SIGTERM and SIGHUP
Corbits registered no signal handlers of its own. OpenTUI's vendored renderer restores the terminal on these signals when a TUI is mounted, but never calls process.exit, so the process (and the run's state on disk) was left hanging indefinitely after an external kill. Outside an interactive session (exec mode, or before a host mounts) nothing handled the signal at all, so Bun's default disposition killed the process with no chance to close out run.json. The new handler restores the terminal and finalizes run state itself rather than relying on OpenTUI's own listener to run first, since that would make correctness depend on a vendored listener's registration order and internals this codebase doesn't own; the terminal-restore call is idempotent so a redundant call from OpenTUI's own listener is harmless. A forked-pty regression test pins the empirical finding this design depends on: Bun's raw-mode stdin clears ISIG, so a real Ctrl+C keypress during an interactive session is delivered only as a stdin byte, never as a SIGINT, leaving the existing double-tap-to-quit gesture as the sole owner of in-session Ctrl+C.
1 parent f3ca332 commit 0680866

7 files changed

Lines changed: 275 additions & 0 deletions

File tree

src/index.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,80 @@ export function installCrashHandlers(): void {
206206
});
207207
}
208208

209+
// Mirrors finalizeActiveRunOnCrash but is not itself a crash — a signal is a
210+
// clean, externally-requested termination (operator, shell, orchestrator),
211+
// so the run is left "failed" (interrupted) rather than "crashed", and no
212+
// crash report is written for it.
213+
async function finalizeActiveRunOnSignal(signal: NodeJS.Signals): Promise<void> {
214+
const run = getActiveRun();
215+
if (run === null || !run.active) return;
216+
try {
217+
await saveCrashState(run.cwd, run.sessionId, {
218+
status: "failed",
219+
turnsUsed: 0,
220+
task: run.task,
221+
startedAt: run.startedAt,
222+
finishedAt: Date.now(),
223+
error: `terminated by ${signal}`,
224+
...(run.model !== undefined ? { model: run.model } : {}),
225+
});
226+
} catch (saveErr: unknown) {
227+
process.stderr.write(
228+
`failed to finalize run state after ${signal}: ${saveErr instanceof Error ? saveErr.message : String(saveErr)}\n`,
229+
);
230+
}
231+
}
232+
233+
const SIGNAL_EXIT_NUMBER: Record<"SIGINT" | "SIGTERM" | "SIGHUP", number> = {
234+
SIGHUP: 1,
235+
SIGINT: 2,
236+
SIGTERM: 15,
237+
};
238+
239+
// Bun's tty raw mode (which the TUI runs under for its whole session) clears
240+
// ISIG, so a real terminal's Ctrl+C never reaches this handler while a
241+
// session is interactive — confirmed empirically (see the raw-mode SIGINT
242+
// regression test) rather than assumed. The in-session double-tap-to-quit
243+
// gesture (shell.ts, CTRL_C_EXIT_WINDOW_MS) is therefore untouched by this
244+
// handler; it owns Ctrl+C exclusively for the interactive case. This handler
245+
// exists for the signal actually reaching the process: external
246+
// orchestration (kill, systemd, docker stop), or a terminal that never
247+
// entered raw mode at all (exec mode has no TUI host and no raw stdin, so
248+
// its Ctrl+C is a real SIGINT today with no listener at all — Bun's default
249+
// disposition kills it immediately without a chance to close out run.json).
250+
//
251+
// Terminal restore is done directly here, the same way handleFatal does it,
252+
// rather than left to OpenTUI's own same-signal listener (registered later,
253+
// at host-mount time, once a TUI is actually running): relying on a
254+
// vendored listener's registration order and internal behavior to already
255+
// cover teardown would make correctness depend on undocumented @opentui
256+
// internals that could change on any version bump, with terminal-left-wedged
257+
// as the silent failure mode. disposeHost is idempotent, so calling it here
258+
// even when OpenTUI's own listener also runs is harmless.
259+
// Exported so an integration test can register these process-level handlers
260+
// and send a real signal without spawning the full TUI stack.
261+
export function installSignalHandlers(): void {
262+
for (const signal of ["SIGINT", "SIGTERM", "SIGHUP"] as const) {
263+
process.on(signal, () => {
264+
if (terminating) return;
265+
terminating = true;
266+
try {
267+
getActiveDisposeHost()?.();
268+
} catch (disposeErr: unknown) {
269+
process.stderr.write(
270+
`host dispose failed handling ${signal}: ${disposeErr instanceof Error ? disposeErr.message : String(disposeErr)}\n`,
271+
);
272+
}
273+
void finalizeActiveRunOnSignal(signal).finally(() => {
274+
process.exit(128 + SIGNAL_EXIT_NUMBER[signal]);
275+
});
276+
});
277+
}
278+
}
279+
209280
if (import.meta.main) {
210281
installCrashHandlers();
282+
installSignalHandlers();
211283

212284
let code: number;
213285
try {

src/tui-opentui/product-host.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,10 @@ export async function mountProductHost(
296296
const renderer = config.createRenderer
297297
? await config.createRenderer()
298298
: await createCliRenderer({
299+
// Leaves Ctrl+C entirely to shell.ts's own double-tap-to-quit
300+
// gesture (CTRL_C_EXIT_WINDOW_MS). index.ts's SIGINT handler also
301+
// depends on this staying false: Ctrl+C only reaches it as a real
302+
// OS signal when nothing already consumed it as a keypress.
299303
exitOnCtrlC: false,
300304
targetFps: 30,
301305
// Mouse reporting on by default: without it, wheel/trackpad scroll
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Spawned as a subprocess by tests/integration/signal-finalize.test.ts.
2+
// Mimics what runTUI does at startup (register the active run, write the
3+
// initial "running" run.json) and what index.ts does at process entry
4+
// (install the signal handlers), then waits to receive a real signal sent by
5+
// the test from outside the process.
6+
import { installSignalHandlers } from "../../../src/index.js";
7+
import { setActiveRun } from "../../../src/session/active-run.js";
8+
import { sessionDir } from "../../../src/session/index.js";
9+
import { saveState } from "../../../src/session/state.js";
10+
11+
const cwd = process.cwd();
12+
const sessionId = process.env["SIGNAL_TEST_SESSION_ID"];
13+
if (sessionId === undefined) {
14+
throw new Error("SIGNAL_TEST_SESSION_ID must be set");
15+
}
16+
17+
const startedAt = Date.now();
18+
const task = "simulated signal task";
19+
const model = "test-provider:test-model";
20+
21+
await saveState(cwd, sessionId, {
22+
status: "running",
23+
turnsUsed: 3,
24+
task,
25+
startedAt,
26+
model,
27+
});
28+
29+
setActiveRun({ sessionId, cwd, active: true, task, startedAt, model });
30+
installSignalHandlers();
31+
32+
process.stdout.write(`${sessionDir(cwd, sessionId)}\n`);
33+
process.stdout.write("ready\n");
34+
35+
// Keep the event loop alive until the test sends a signal.
36+
setInterval(() => {}, 60_000);
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Fixture for tests/integration/rawmode-sigint.test.ts. Proves, on the real
2+
// Bun runtime rather than by assumption, whether a Ctrl+C keypress (0x03)
3+
// generates a SIGINT deliverable to process.on("SIGINT") while stdin is in
4+
// raw mode — the empirical claim src/index.ts's installSignalHandlers
5+
// depends on to leave the in-session double-tap-to-quit gesture untouched.
6+
process.stdin.setRawMode(true);
7+
process.on("SIGINT", () => {
8+
process.stdout.write("GOT_SIGINT\n");
9+
process.exit(0);
10+
});
11+
process.stdin.resume();
12+
process.stdin.on("data", (chunk: Buffer) => {
13+
if (chunk.includes(0x03)) process.stdout.write("GOT_CTRL_C_BYTE\n");
14+
});
15+
setTimeout(() => {
16+
process.stdout.write("NO_SIGINT_ON_CTRL_C\n");
17+
process.exit(0);
18+
}, 3000);
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
#!/usr/bin/env python3
2+
# Drives tests/fixtures/rawmode-sigint/probe.ts inside a real forked pty
3+
# (stdlib `pty`/`os`/`select`, no third-party deps) so the fixture's stdin
4+
# is a genuine tty rather than a pipe -- `setRawMode` only has the raw-mode
5+
# vs. cooked-mode distinction this test cares about on a real tty.
6+
import os
7+
import pty
8+
import select
9+
import sys
10+
import time
11+
12+
def main() -> int:
13+
probe_path = sys.argv[1]
14+
pid, fd = pty.fork()
15+
if pid == 0:
16+
os.execvp("bun", ["bun", "run", probe_path])
17+
os._exit(127)
18+
19+
time.sleep(1)
20+
os.write(fd, b"\x03")
21+
22+
out = b""
23+
deadline = time.time() + 4
24+
while time.time() < deadline:
25+
ready, _, _ = select.select([fd], [], [], 0.5)
26+
if fd not in ready:
27+
continue
28+
try:
29+
chunk = os.read(fd, 4096)
30+
except OSError:
31+
break
32+
if not chunk:
33+
break
34+
out += chunk
35+
if b"NO_SIGINT_ON_CTRL_C" in out or b"GOT_SIGINT" in out:
36+
break
37+
38+
try:
39+
os.kill(pid, 9)
40+
except OSError:
41+
pass
42+
os.waitpid(pid, 0)
43+
44+
sys.stdout.write(out.decode(errors="replace"))
45+
return 0
46+
47+
if __name__ == "__main__":
48+
raise SystemExit(main())
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { describe, expect, test } from "bun:test";
2+
3+
// src/index.ts's installSignalHandlers relies on an empirical claim: Bun's
4+
// stdin.setRawMode(true) clears ISIG on this platform, so a real Ctrl+C
5+
// keypress never reaches process.on("SIGINT") during an interactive TUI
6+
// session -- only out-of-band kill(2) signals do. If a future Bun upgrade
7+
// changes that, the in-session double-tap-to-quit gesture (shell.ts,
8+
// CTRL_C_EXIT_WINDOW_MS) would silently start racing a process-level exit
9+
// on the very first Ctrl+C. This test pins the assumption against a real
10+
// forked pty rather than trusting it to hold forever.
11+
describe("integration — raw-mode stdin and SIGINT", () => {
12+
test("Ctrl+C is delivered as a stdin byte, not as SIGINT, while raw mode is active", async () => {
13+
const probe = new URL("../fixtures/rawmode-sigint/probe.ts", import.meta.url).pathname;
14+
const driver = new URL("../fixtures/rawmode-sigint/pty_probe.py", import.meta.url).pathname;
15+
16+
const proc = Bun.spawn(["python3", driver, probe], {
17+
stdout: "pipe",
18+
stderr: "pipe",
19+
});
20+
const [stdout, exitCode] = await Promise.all([
21+
new Response(proc.stdout).text(),
22+
proc.exited,
23+
]);
24+
25+
expect(exitCode).toBe(0);
26+
expect(stdout).toContain("GOT_CTRL_C_BYTE");
27+
expect(stdout).toContain("NO_SIGINT_ON_CTRL_C");
28+
expect(stdout).not.toContain("GOT_SIGINT");
29+
}, 15000);
30+
});
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import { mkdtempSync, readFileSync, rmSync } from "node:fs";
2+
import { tmpdir } from "node:os";
3+
import { join } from "node:path";
4+
5+
import { describe, expect, test } from "bun:test";
6+
7+
import { generateSessionId } from "../../src/session/index.js";
8+
import type { RunState } from "../../src/session/state.js";
9+
10+
const FIXTURE = join(import.meta.dirname, "../fixtures/crash-run/simulate-signal.ts");
11+
12+
async function readLine(stream: ReadableStream<Uint8Array>): Promise<string> {
13+
const reader = stream.getReader();
14+
const decoder = new TextDecoder();
15+
let buffer = "";
16+
while (!buffer.includes("\n")) {
17+
const { value, done } = await reader.read();
18+
if (done) break;
19+
buffer += decoder.decode(value, { stream: true });
20+
}
21+
reader.releaseLock();
22+
return buffer;
23+
}
24+
25+
describe("integration — signal finalizes run.json", () => {
26+
test.each([
27+
["SIGINT", 130],
28+
["SIGTERM", 143],
29+
["SIGHUP", 129],
30+
] as const)("%s writes status: failed and exits with %i", async (signal, expectedExitCode) => {
31+
const cwd = mkdtempSync(join(tmpdir(), "corbits-signal-cwd-"));
32+
const home = mkdtempSync(join(tmpdir(), "corbits-signal-home-"));
33+
const sessionId = generateSessionId();
34+
35+
try {
36+
const proc = Bun.spawn(["bun", "run", FIXTURE], {
37+
cwd,
38+
env: { ...process.env, HOME: home, SIGNAL_TEST_SESSION_ID: sessionId },
39+
stdout: "pipe",
40+
stderr: "pipe",
41+
});
42+
43+
const output = await readLine(proc.stdout);
44+
const [runDir] = output.split("\n");
45+
if (runDir === undefined || runDir.length === 0) {
46+
throw new Error(`fixture did not report a run directory: ${JSON.stringify(output)}`);
47+
}
48+
49+
proc.kill(signal);
50+
const exitCode = await proc.exited;
51+
52+
expect(exitCode).toBe(expectedExitCode);
53+
54+
const runJsonPath = join(runDir, "run.json");
55+
const raw = readFileSync(runJsonPath, "utf8");
56+
const state = JSON.parse(raw) as RunState;
57+
58+
expect(state.status).toBe("failed");
59+
expect(state.finishedAt).toBeGreaterThan(0);
60+
expect(state.error).toBe(`terminated by ${signal}`);
61+
expect(state.task).toBe("simulated signal task");
62+
} finally {
63+
rmSync(cwd, { recursive: true, force: true });
64+
rmSync(home, { recursive: true, force: true });
65+
}
66+
}, 15_000);
67+
});

0 commit comments

Comments
 (0)