Skip to content
5 changes: 5 additions & 0 deletions .changeset/quiet-terminals-exit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"hunkdiff": patch
---

Exit cleanly when the terminal hosting a review disconnects instead of leaving an unreachable Hunk process behind.
97 changes: 97 additions & 0 deletions src/core/terminal.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,44 @@
import { describe, expect, test } from "bun:test";
import type { CliInput } from "./types";
import {
installTerminalDisconnectSupport,
openControllingTerminal,
resolveRuntimeCliInput,
shouldUseMouseForApp,
shouldUsePagerMode,
usesPipedPatchInput,
} from "./terminal";

function createTestTerminalInputEvents(
state: { isTTY?: boolean; destroyed?: boolean; readableEnded?: boolean } = {},
) {
const listeners = new Map<string, Set<(...args: unknown[]) => void>>();

return {
isTTY: true,
...state,
emit(event: "close" | "end" | "error") {
for (const listener of listeners.get(event) ?? []) {
listener();
}
},
listenerCount(event: "close" | "end" | "error") {
return listeners.get(event)?.size ?? 0;
},
on(event: "close" | "end" | "error", listener: (...args: unknown[]) => void) {
let eventListeners = listeners.get(event);
if (!eventListeners) {
eventListeners = new Set();
listeners.set(event, eventListeners);
}
eventListeners.add(listener);
},
off(event: "close" | "end" | "error", listener: (...args: unknown[]) => void) {
listeners.get(event)?.delete(listener);
},
};
}

function createPatchInput(file?: string, pager = false): CliInput {
return {
kind: "patch",
Expand Down Expand Up @@ -114,3 +145,69 @@ describe("controlling terminal attachment", () => {
expect(controllingTerminal).toBeNull();
});
});

describe("terminal disconnect support", () => {
test.each(["close", "end", "error"] as const)("shuts down once on %s", (event) => {
const input = createTestTerminalInputEvents();
let disconnectCalls = 0;
installTerminalDisconnectSupport(input, () => {
disconnectCalls += 1;
});

input.emit(event);
input.emit(event);

expect(disconnectCalls).toBe(1);
});

test("dispose removes every input listener", () => {
const input = createTestTerminalInputEvents();
let disconnectCalls = 0;
const support = installTerminalDisconnectSupport(input, () => {
disconnectCalls += 1;
});

support.dispose();

expect(input.listenerCount("close")).toBe(0);
expect(input.listenerCount("end")).toBe(0);
expect(input.listenerCount("error")).toBe(0);
input.emit("close");
expect(disconnectCalls).toBe(0);
});

test.each(["destroyed", "readableEnded"] as const)(
"shuts down when input is already %s",
async (state) => {
const input = createTestTerminalInputEvents({ [state]: true });
let disconnectCalls = 0;
installTerminalDisconnectSupport(input, () => {
disconnectCalls += 1;
});

await Promise.resolve();

expect(disconnectCalls).toBe(1);
},
);

// Non-interactive input ends the moment the renderer resumes it.
test.each([{ isTTY: false }, { isTTY: undefined }])(
"ignores non-terminal input (%o)",
async (state) => {
const input = createTestTerminalInputEvents({ ...state, readableEnded: true });
let disconnectCalls = 0;
installTerminalDisconnectSupport(input, () => {
disconnectCalls += 1;
});

await Promise.resolve();
input.emit("end");
input.emit("close");
input.emit("error");

expect(input.listenerCount("end")).toBe(0);
expect(disconnectCalls).toBe(0);
},
);
});
53 changes: 53 additions & 0 deletions src/core/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,59 @@ export interface ControllingTerminal {
close: () => void;
}

type TerminalDisconnectEvent = "close" | "end" | "error";

export interface TerminalInputEvents {
isTTY?: boolean;
destroyed?: boolean;
readableEnded?: boolean;
on: (event: TerminalDisconnectEvent, listener: (...args: unknown[]) => void) => unknown;
off: (event: TerminalDisconnectEvent, listener: (...args: unknown[]) => void) => unknown;
}

export interface TerminalDisconnectSupport {
dispose: () => void;
}

/** Shut the app down when its renderer input is closed or revoked by the terminal host. */
export function installTerminalDisconnectSupport(
input: TerminalInputEvents,
onDisconnect: () => void,
): TerminalDisconnectSupport {
if (input.isTTY !== true) {
return { dispose: () => undefined };
}

const events: TerminalDisconnectEvent[] = ["close", "end", "error"];
let disposed = false;

const disconnect = () => {
if (disposed) {
return;
}
disposed = true;
onDisconnect();
};

for (const event of events) {
input.on(event, disconnect);
}

// Stream ended before listeners handle disconnect
if (input.destroyed || input.readableEnded) {
queueMicrotask(disconnect);
}

return {
dispose: () => {
disposed = true;
for (const event of events) {
input.off(event, disconnect);
}
},
};
}

/** Minimal terminal construction hooks so tests can cover `/dev/tty` attach behavior. */
export interface ControllingTerminalDeps {
openSync: typeof fs.openSync;
Expand Down
27 changes: 22 additions & 5 deletions src/ui/runInteractiveApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ import {
type JobControlSuspendSupport,
} from "../core/jobControl";
import { shutdownSession } from "../core/shutdown";
import { shouldUseMouseForApp, type ControllingTerminal } from "../core/terminal";
import {
installTerminalDisconnectSupport,
shouldUseMouseForApp,
type ControllingTerminal,
type TerminalDisconnectSupport,
} from "../core/terminal";
import type { AppBootstrap } from "../core/types";
import { resolveStartupUpdateNotice } from "../core/updateNotice";
import { ReviewProducer } from "../app/review/producer";
Expand All @@ -29,6 +34,12 @@ export interface InteractiveAppInput {
controllingTerminal: ControllingTerminal | null;
}

// Leave fatal process faults to their default OS disposition.
const APP_SHUTDOWN_SIGNALS: NodeJS.Signals[] =
process.platform === "win32"
? ["SIGINT", "SIGTERM", "SIGBREAK"]
: ["SIGINT", "SIGTERM", "SIGHUP", "SIGQUIT", "SIGPIPE"];

/** Load and run the OpenTUI review app after startup has selected an interactive plan. */
export async function runInteractiveApp({
bootstrap,
Expand All @@ -54,25 +65,28 @@ export async function runInteractiveApp({
hostClient.start();

// Keep OpenTUI's platform-safe threading default (enabled on macOS, disabled on Linux).
const rendererStdin = controllingTerminal?.stdin ?? process.stdin;
const renderer = await createCliRenderer({
stdin: controllingTerminal?.stdin,
stdin: rendererStdin,
stdout: process.stdout,
useMouse: shouldUseMouseForApp({
hasControllingTerminal: Boolean(controllingTerminal),
}),
screenMode: "alternate-screen",
exitOnCtrlC: false,
// OpenTUI's destroy-only handlers can strand sessions with active broker handles.
exitSignals: [],
openConsoleOnError: true,
onDestroy: () => controllingTerminal?.close(),
});

const appRenderer = renderer;
const root = createRoot(appRenderer);
const shutdownSignals: NodeJS.Signals[] = ["SIGINT", "SIGTERM"];
const externalQuitController = new AbortController();
let shuttingDown = false;
let jobControlSuspendSupport: JobControlSuspendSupport = { dispose: () => undefined };
let jobControlInterruptSupport: JobControlInterruptSupport = { dispose: () => undefined };
let terminalDisconnectSupport: TerminalDisconnectSupport = { dispose: () => undefined };

/** Ask AppHost to retire extension authority before tearing down the terminal. */
function requestQuit() {
Expand All @@ -86,18 +100,21 @@ export async function runInteractiveApp({
}

shuttingDown = true;
for (const signal of shutdownSignals) {
for (const signal of APP_SHUTDOWN_SIGNALS) {
process.off(signal, requestQuit);
}
jobControlInterruptSupport.dispose();
jobControlSuspendSupport.dispose();
terminalDisconnectSupport.dispose();
hostClient.stop();
shutdownSession({ root, renderer: appRenderer });
}

for (const signal of shutdownSignals) {
for (const signal of APP_SHUTDOWN_SIGNALS) {
process.once(signal, requestQuit);
}
// Install after the renderer so a disconnect closes the live session instead of racing startup.
terminalDisconnectSupport = installTerminalDisconnectSupport(rendererStdin, requestQuit);
jobControlInterruptSupport = installJobControlInterruptSupport(appRenderer, requestQuit);
jobControlSuspendSupport = installJobControlSuspendSupport(appRenderer);

Expand Down
72 changes: 72 additions & 0 deletions test/cli/non-interactive-stdin.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { describe, expect, test } from "bun:test";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";

const MINIMUM_RENDERED_BYTES = 1_000;

async function readUntilRendered(
stream: ReadableStream<Uint8Array>,
minimumBytes: number,
timeoutMs: number,
) {
const reader = stream.getReader();
const deadline = Date.now() + timeoutMs;
let bytes = 0;

try {
while (bytes < minimumBytes && Date.now() < deadline) {
const next = await Promise.race([
reader.read(),
Bun.sleep(Math.max(0, deadline - Date.now())).then(() => "timeout" as const),
]);
if (next === "timeout" || next.done) {
break;
}
bytes += next.value.length;
}
} finally {
reader.releaseLock();
}

return bytes;
}

describe("non-interactive stdin contracts", () => {
test("renders the review and stays alive when stdin is not a terminal", async () => {
const dir = mkdtempSync(join(tmpdir(), "hunk-non-tty-stdin-"));
const before = join(dir, "before.ts");
const after = join(dir, "after.ts");
writeFileSync(before, "export const value = 1;\n");
writeFileSync(after, "export const value = 2;\n");

const proc = Bun.spawn(["bun", "run", "src/main.tsx", "--", "diff", before, after], {
cwd: process.cwd(),
stdin: "ignore",
stdout: "pipe",
stderr: "pipe",
env: {
...process.env,
TERM: "xterm-256color",
HUNK_MCP_DISABLE: "1",
HUNK_DISABLE_UPDATE_NOTICE: "1",
XDG_CONFIG_HOME: dir,
},
});

try {
const bytes = await readUntilRendered(proc.stdout, MINIMUM_RENDERED_BYTES, 15_000);
expect(bytes).toBeGreaterThanOrEqual(MINIMUM_RENDERED_BYTES);
await expect(
Promise.race([
proc.exited.then((code) => ({ exited: true, code })),
Bun.sleep(250).then(() => ({ exited: false })),
]),
).resolves.toEqual({ exited: false });
} finally {
proc.kill();
await proc.exited;
rmSync(dir, { recursive: true, force: true });
}
}, 30_000);
});
Loading