From f3ca332c6692f6836f420b36cbfd1190d38efebb Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:16:49 -0700 Subject: [PATCH 1/2] Restore the terminal on a detached throw before exiting A throw that escapes runTUI's own try/catch (e.g. inside a fire-and-forget void call) only reached the top-level uncaughtException/unhandledRejection handler, which had no way to call runTUI's terminal-restore routine since it lives as a closure bound only once the OpenTUI host mounts. The process exited with the terminal left in the alternate screen and raw mode still on. A module-level slot mirroring the existing active-run.ts pattern lets the handler reach it. --- src/index.ts | 22 ++++++++++++++++++++++ src/session/active-host.test.ts | 32 ++++++++++++++++++++++++++++++++ src/session/active-host.ts | 20 ++++++++++++++++++++ src/tui/runner.ts | 4 ++++ 4 files changed, 78 insertions(+) create mode 100644 src/session/active-host.test.ts create mode 100644 src/session/active-host.ts diff --git a/src/index.ts b/src/index.ts index 0ce70559e..8c44d75f0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2,6 +2,7 @@ import { getLogger } from "@intx/log"; import { LOG_NAMESPACE_ROOT } from "./branding.js"; import { primeCrashReporting, writeCrashReport, type CrashKind } from "./crash/report.js"; import { getActiveRun, markCrashed } from "./session/active-run.js"; +import { getActiveDisposeHost } from "./session/active-host.js"; import { saveCrashState } from "./session/state.js"; import { loadConfig } from "./config/index.js"; import { ensureTelemetrySettings, globalSettingsPath } from "./config/settings.js"; @@ -115,9 +116,30 @@ export async function main(argv: readonly string[]): Promise { }); } +// Shared by handleFatal and the signal handlers below so a signal arriving +// mid-crash-unwind (or a crash surfacing while a signal is already tearing +// the process down) can't re-enter either path a second time. +let terminating = false; + // Exported so an integration test can register these process-level handlers // and inject a crash without spawning the full TUI stack. export async function handleFatal(kind: CrashKind, error: unknown): Promise { + if (terminating) return; + terminating = true; + // OpenTUI's own uncaughtException/unhandledRejection listener only logs + // (see installCrashHandlers' comment below) — it never tears down the + // terminal the way its signal listener does. Without this, a throw that + // escapes runTUI's own try/catch (e.g. inside a fire-and-forget `void` + // call) leaves the alternate screen and raw mode stuck. disposeHost is + // idempotent, so this is safe even if runTUI's own catch block already + // ran it moments earlier. + try { + getActiveDisposeHost()?.(); + } catch (disposeErr: unknown) { + process.stderr.write( + `host dispose failed during fatal handling: ${disposeErr instanceof Error ? disposeErr.message : String(disposeErr)}\n`, + ); + } // Flip this before any awaits below so any snapshot write still queued // behind another one in state.ts's per-session chain sees it and steps // aside the moment it's next in line, rather than racing saveCrashState's diff --git a/src/session/active-host.test.ts b/src/session/active-host.test.ts new file mode 100644 index 000000000..39abe7268 --- /dev/null +++ b/src/session/active-host.test.ts @@ -0,0 +1,32 @@ +import { afterEach, describe, expect, test } from "bun:test"; + +import { clearActiveDisposeHost, getActiveDisposeHost, setActiveDisposeHost } from "./active-host.js"; + +describe("active-host", () => { + afterEach(() => { + clearActiveDisposeHost(); + }); + + test("starts with no active dispose handle", () => { + expect(getActiveDisposeHost()).toBeNull(); + }); + + test("returns the handle set by setActiveDisposeHost", () => { + const disposeHost = () => {}; + setActiveDisposeHost(disposeHost); + expect(getActiveDisposeHost()).toBe(disposeHost); + }); + + test("clearActiveDisposeHost removes the handle", () => { + setActiveDisposeHost(() => {}); + clearActiveDisposeHost(); + expect(getActiveDisposeHost()).toBeNull(); + }); + + test("setActiveDisposeHost overwrites a previously set handle", () => { + setActiveDisposeHost(() => {}); + const second = () => {}; + setActiveDisposeHost(second); + expect(getActiveDisposeHost()).toBe(second); + }); +}); diff --git a/src/session/active-host.ts b/src/session/active-host.ts new file mode 100644 index 000000000..046d7f63d --- /dev/null +++ b/src/session/active-host.ts @@ -0,0 +1,20 @@ +// A module-level slot mirroring active-run.ts's pattern: the top-level +// process handlers in src/index.ts (a detached-throw handler today, a signal +// handler alongside it) need to reach runTUI's terminal-restore routine even +// though it is a closure local to runTUI, bound only once the OpenTUI host +// has mounted. Cleared the moment runTUI itself finalizes (normally or via +// its own crash path) so a signal arriving after teardown has nothing left +// to call. +let activeDisposeHost: (() => void) | null = null; + +export function setActiveDisposeHost(disposeHost: () => void): void { + activeDisposeHost = disposeHost; +} + +export function clearActiveDisposeHost(): void { + activeDisposeHost = null; +} + +export function getActiveDisposeHost(): (() => void) | null { + return activeDisposeHost; +} diff --git a/src/tui/runner.ts b/src/tui/runner.ts index 49a5d0ed8..50711d637 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -167,6 +167,7 @@ import { generateSessionId, initSessionDir, renameSession, sessionContextDir, se import { resolveSessionLabel, truncateSessionLabel } from "../session/session-label.js"; import { loadState, saveState, type ConnectedMcpServer, type RunState } from "../session/state.js"; import { setActiveRun, clearActiveRun, type RunStateHandle } from "../session/active-run.js"; +import { setActiveDisposeHost, clearActiveDisposeHost } from "../session/active-host.js"; import { openInBrowser } from "../auth/oauth/browser.js"; import { pickSession } from "./pick-session.js"; import { RESUME_TRANSCRIPT_BLOCK_LIMIT, turnsToContentBlocks } from "./turns-to-blocks.js"; @@ -529,6 +530,7 @@ export async function runTUI(initialConfig: Config): Promise { finalized = true; activeRunHandle.active = false; clearActiveRun(); + clearActiveDisposeHost(); await flushPartialOnCrash().catch((flushErr: unknown) => { // Best-effort only — still attempt saveState below. Log so a flush // failure is not invisible when diagnosing a crash exit. @@ -2192,6 +2194,7 @@ export async function runTUI(initialConfig: Config): Promise { }); disposeHost = host.dispose; + setActiveDisposeHost(disposeHost); setMentionSuggestionSource(host.shell, (prefix) => listPathSuggestions(prefix, config.cwd)); @@ -2296,6 +2299,7 @@ export async function runTUI(initialConfig: Config): Promise { finalized = true; activeRunHandle.active = false; clearActiveRun(); + clearActiveDisposeHost(); await writeRunSnapshot(persistedStatus, { finishedAt, ...(sinkError !== undefined ? { error: sinkError } : {}), From 06808660e49c272464ba9176ccac3e83196a7121 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:19:49 -0700 Subject: [PATCH 2/2] 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. --- src/index.ts | 72 +++++++++++++++++++++ src/tui-opentui/product-host.ts | 4 ++ tests/fixtures/crash-run/simulate-signal.ts | 36 +++++++++++ tests/fixtures/rawmode-sigint/probe.ts | 18 ++++++ tests/fixtures/rawmode-sigint/pty_probe.py | 48 ++++++++++++++ tests/integration/rawmode-sigint.test.ts | 30 +++++++++ tests/integration/signal-finalize.test.ts | 67 +++++++++++++++++++ 7 files changed, 275 insertions(+) create mode 100644 tests/fixtures/crash-run/simulate-signal.ts create mode 100644 tests/fixtures/rawmode-sigint/probe.ts create mode 100644 tests/fixtures/rawmode-sigint/pty_probe.py create mode 100644 tests/integration/rawmode-sigint.test.ts create mode 100644 tests/integration/signal-finalize.test.ts diff --git a/src/index.ts b/src/index.ts index 8c44d75f0..35106d3d6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -206,8 +206,80 @@ export function installCrashHandlers(): void { }); } +// Mirrors finalizeActiveRunOnCrash but is not itself a crash — a signal is a +// clean, externally-requested termination (operator, shell, orchestrator), +// so the run is left "failed" (interrupted) rather than "crashed", and no +// crash report is written for it. +async function finalizeActiveRunOnSignal(signal: NodeJS.Signals): Promise { + const run = getActiveRun(); + if (run === null || !run.active) return; + try { + await saveCrashState(run.cwd, run.sessionId, { + status: "failed", + turnsUsed: 0, + task: run.task, + startedAt: run.startedAt, + finishedAt: Date.now(), + error: `terminated by ${signal}`, + ...(run.model !== undefined ? { model: run.model } : {}), + }); + } catch (saveErr: unknown) { + process.stderr.write( + `failed to finalize run state after ${signal}: ${saveErr instanceof Error ? saveErr.message : String(saveErr)}\n`, + ); + } +} + +const SIGNAL_EXIT_NUMBER: Record<"SIGINT" | "SIGTERM" | "SIGHUP", number> = { + SIGHUP: 1, + SIGINT: 2, + SIGTERM: 15, +}; + +// Bun's tty raw mode (which the TUI runs under for its whole session) clears +// ISIG, so a real terminal's Ctrl+C never reaches this handler while a +// session is interactive — confirmed empirically (see the raw-mode SIGINT +// regression test) rather than assumed. The in-session double-tap-to-quit +// gesture (shell.ts, CTRL_C_EXIT_WINDOW_MS) is therefore untouched by this +// handler; it owns Ctrl+C exclusively for the interactive case. This handler +// exists for the signal actually reaching the process: external +// orchestration (kill, systemd, docker stop), or a terminal that never +// entered raw mode at all (exec mode has no TUI host and no raw stdin, so +// its Ctrl+C is a real SIGINT today with no listener at all — Bun's default +// disposition kills it immediately without a chance to close out run.json). +// +// Terminal restore is done directly here, the same way handleFatal does it, +// rather than left to OpenTUI's own same-signal listener (registered later, +// at host-mount time, once a TUI is actually running): relying on a +// vendored listener's registration order and internal behavior to already +// cover teardown would make correctness depend on undocumented @opentui +// internals that could change on any version bump, with terminal-left-wedged +// as the silent failure mode. disposeHost is idempotent, so calling it here +// even when OpenTUI's own listener also runs is harmless. +// Exported so an integration test can register these process-level handlers +// and send a real signal without spawning the full TUI stack. +export function installSignalHandlers(): void { + for (const signal of ["SIGINT", "SIGTERM", "SIGHUP"] as const) { + process.on(signal, () => { + if (terminating) return; + terminating = true; + try { + getActiveDisposeHost()?.(); + } catch (disposeErr: unknown) { + process.stderr.write( + `host dispose failed handling ${signal}: ${disposeErr instanceof Error ? disposeErr.message : String(disposeErr)}\n`, + ); + } + void finalizeActiveRunOnSignal(signal).finally(() => { + process.exit(128 + SIGNAL_EXIT_NUMBER[signal]); + }); + }); + } +} + if (import.meta.main) { installCrashHandlers(); + installSignalHandlers(); let code: number; try { diff --git a/src/tui-opentui/product-host.ts b/src/tui-opentui/product-host.ts index 376ff255b..fb07d70aa 100644 --- a/src/tui-opentui/product-host.ts +++ b/src/tui-opentui/product-host.ts @@ -296,6 +296,10 @@ export async function mountProductHost( const renderer = config.createRenderer ? await config.createRenderer() : await createCliRenderer({ + // Leaves Ctrl+C entirely to shell.ts's own double-tap-to-quit + // gesture (CTRL_C_EXIT_WINDOW_MS). index.ts's SIGINT handler also + // depends on this staying false: Ctrl+C only reaches it as a real + // OS signal when nothing already consumed it as a keypress. exitOnCtrlC: false, targetFps: 30, // Mouse reporting on by default: without it, wheel/trackpad scroll diff --git a/tests/fixtures/crash-run/simulate-signal.ts b/tests/fixtures/crash-run/simulate-signal.ts new file mode 100644 index 000000000..ce660292e --- /dev/null +++ b/tests/fixtures/crash-run/simulate-signal.ts @@ -0,0 +1,36 @@ +// Spawned as a subprocess by tests/integration/signal-finalize.test.ts. +// Mimics what runTUI does at startup (register the active run, write the +// initial "running" run.json) and what index.ts does at process entry +// (install the signal handlers), then waits to receive a real signal sent by +// the test from outside the process. +import { installSignalHandlers } from "../../../src/index.js"; +import { setActiveRun } from "../../../src/session/active-run.js"; +import { sessionDir } from "../../../src/session/index.js"; +import { saveState } from "../../../src/session/state.js"; + +const cwd = process.cwd(); +const sessionId = process.env["SIGNAL_TEST_SESSION_ID"]; +if (sessionId === undefined) { + throw new Error("SIGNAL_TEST_SESSION_ID must be set"); +} + +const startedAt = Date.now(); +const task = "simulated signal task"; +const model = "test-provider:test-model"; + +await saveState(cwd, sessionId, { + status: "running", + turnsUsed: 3, + task, + startedAt, + model, +}); + +setActiveRun({ sessionId, cwd, active: true, task, startedAt, model }); +installSignalHandlers(); + +process.stdout.write(`${sessionDir(cwd, sessionId)}\n`); +process.stdout.write("ready\n"); + +// Keep the event loop alive until the test sends a signal. +setInterval(() => {}, 60_000); diff --git a/tests/fixtures/rawmode-sigint/probe.ts b/tests/fixtures/rawmode-sigint/probe.ts new file mode 100644 index 000000000..609cf4d99 --- /dev/null +++ b/tests/fixtures/rawmode-sigint/probe.ts @@ -0,0 +1,18 @@ +// Fixture for tests/integration/rawmode-sigint.test.ts. Proves, on the real +// Bun runtime rather than by assumption, whether a Ctrl+C keypress (0x03) +// generates a SIGINT deliverable to process.on("SIGINT") while stdin is in +// raw mode — the empirical claim src/index.ts's installSignalHandlers +// depends on to leave the in-session double-tap-to-quit gesture untouched. +process.stdin.setRawMode(true); +process.on("SIGINT", () => { + process.stdout.write("GOT_SIGINT\n"); + process.exit(0); +}); +process.stdin.resume(); +process.stdin.on("data", (chunk: Buffer) => { + if (chunk.includes(0x03)) process.stdout.write("GOT_CTRL_C_BYTE\n"); +}); +setTimeout(() => { + process.stdout.write("NO_SIGINT_ON_CTRL_C\n"); + process.exit(0); +}, 3000); diff --git a/tests/fixtures/rawmode-sigint/pty_probe.py b/tests/fixtures/rawmode-sigint/pty_probe.py new file mode 100644 index 000000000..3c3912003 --- /dev/null +++ b/tests/fixtures/rawmode-sigint/pty_probe.py @@ -0,0 +1,48 @@ +#!/usr/bin/env python3 +# Drives tests/fixtures/rawmode-sigint/probe.ts inside a real forked pty +# (stdlib `pty`/`os`/`select`, no third-party deps) so the fixture's stdin +# is a genuine tty rather than a pipe -- `setRawMode` only has the raw-mode +# vs. cooked-mode distinction this test cares about on a real tty. +import os +import pty +import select +import sys +import time + +def main() -> int: + probe_path = sys.argv[1] + pid, fd = pty.fork() + if pid == 0: + os.execvp("bun", ["bun", "run", probe_path]) + os._exit(127) + + time.sleep(1) + os.write(fd, b"\x03") + + out = b"" + deadline = time.time() + 4 + while time.time() < deadline: + ready, _, _ = select.select([fd], [], [], 0.5) + if fd not in ready: + continue + try: + chunk = os.read(fd, 4096) + except OSError: + break + if not chunk: + break + out += chunk + if b"NO_SIGINT_ON_CTRL_C" in out or b"GOT_SIGINT" in out: + break + + try: + os.kill(pid, 9) + except OSError: + pass + os.waitpid(pid, 0) + + sys.stdout.write(out.decode(errors="replace")) + return 0 + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/integration/rawmode-sigint.test.ts b/tests/integration/rawmode-sigint.test.ts new file mode 100644 index 000000000..59085f1b2 --- /dev/null +++ b/tests/integration/rawmode-sigint.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, test } from "bun:test"; + +// src/index.ts's installSignalHandlers relies on an empirical claim: Bun's +// stdin.setRawMode(true) clears ISIG on this platform, so a real Ctrl+C +// keypress never reaches process.on("SIGINT") during an interactive TUI +// session -- only out-of-band kill(2) signals do. If a future Bun upgrade +// changes that, the in-session double-tap-to-quit gesture (shell.ts, +// CTRL_C_EXIT_WINDOW_MS) would silently start racing a process-level exit +// on the very first Ctrl+C. This test pins the assumption against a real +// forked pty rather than trusting it to hold forever. +describe("integration — raw-mode stdin and SIGINT", () => { + test("Ctrl+C is delivered as a stdin byte, not as SIGINT, while raw mode is active", async () => { + const probe = new URL("../fixtures/rawmode-sigint/probe.ts", import.meta.url).pathname; + const driver = new URL("../fixtures/rawmode-sigint/pty_probe.py", import.meta.url).pathname; + + const proc = Bun.spawn(["python3", driver, probe], { + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + proc.exited, + ]); + + expect(exitCode).toBe(0); + expect(stdout).toContain("GOT_CTRL_C_BYTE"); + expect(stdout).toContain("NO_SIGINT_ON_CTRL_C"); + expect(stdout).not.toContain("GOT_SIGINT"); + }, 15000); +}); diff --git a/tests/integration/signal-finalize.test.ts b/tests/integration/signal-finalize.test.ts new file mode 100644 index 000000000..270707c19 --- /dev/null +++ b/tests/integration/signal-finalize.test.ts @@ -0,0 +1,67 @@ +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { describe, expect, test } from "bun:test"; + +import { generateSessionId } from "../../src/session/index.js"; +import type { RunState } from "../../src/session/state.js"; + +const FIXTURE = join(import.meta.dirname, "../fixtures/crash-run/simulate-signal.ts"); + +async function readLine(stream: ReadableStream): Promise { + const reader = stream.getReader(); + const decoder = new TextDecoder(); + let buffer = ""; + while (!buffer.includes("\n")) { + const { value, done } = await reader.read(); + if (done) break; + buffer += decoder.decode(value, { stream: true }); + } + reader.releaseLock(); + return buffer; +} + +describe("integration — signal finalizes run.json", () => { + test.each([ + ["SIGINT", 130], + ["SIGTERM", 143], + ["SIGHUP", 129], + ] as const)("%s writes status: failed and exits with %i", async (signal, expectedExitCode) => { + const cwd = mkdtempSync(join(tmpdir(), "corbits-signal-cwd-")); + const home = mkdtempSync(join(tmpdir(), "corbits-signal-home-")); + const sessionId = generateSessionId(); + + try { + const proc = Bun.spawn(["bun", "run", FIXTURE], { + cwd, + env: { ...process.env, HOME: home, SIGNAL_TEST_SESSION_ID: sessionId }, + stdout: "pipe", + stderr: "pipe", + }); + + const output = await readLine(proc.stdout); + const [runDir] = output.split("\n"); + if (runDir === undefined || runDir.length === 0) { + throw new Error(`fixture did not report a run directory: ${JSON.stringify(output)}`); + } + + proc.kill(signal); + const exitCode = await proc.exited; + + expect(exitCode).toBe(expectedExitCode); + + const runJsonPath = join(runDir, "run.json"); + const raw = readFileSync(runJsonPath, "utf8"); + const state = JSON.parse(raw) as RunState; + + expect(state.status).toBe("failed"); + expect(state.finishedAt).toBeGreaterThan(0); + expect(state.error).toBe(`terminated by ${signal}`); + expect(state.task).toBe("simulated signal task"); + } finally { + rmSync(cwd, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }, 15_000); +});