fix(runtime): restore terminal on signal exit via signal-exit (Ink parity, G18, HIGH) (#47)

* fix(runtime): restore terminal on signal exit via signal-exit (Ink parity, G18)

Previously nothing routed a process signal to teardown(): SIGINT-as-signal,
SIGTERM or SIGHUP killed the process with the cursor hidden, the alternate
screen active and raw mode on, leaving the terminal corrupted.

Mirror Ink (ink.tsx:426): register signal-exit's onExit(teardown,
{alwaysLast:false}) at interactive mount, storing the unsubscribe fn, and
call it first thing in teardown() (ink.tsx:765) so the handler is removed on
unmount()/exit() and can't leak or double-run. teardown() stays idempotent
(teardownStarted guard) so a signal-triggered teardown plus a later unmount
won't double-run, and we don't prevent the process from exiting. Only the
live interactive, non-debug mount registers — render-to-string /
non-interactive paths never touch process signal handlers; registration is
guarded against double-registration.

Uses signal-exit v4 (named onExit export; ships ESM + types, so no
@types/signal-exit needed). PTY test sends SIGINT/SIGTERM/SIGHUP to a mounted
alt-screen app and asserts the captured output ends with show-cursor
(\x1b[?25h) + leave-alt-screen (\x1b[?1049l).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Review follow-ups (3 fixes): register signal-exit whenever interactive
(drop the !debug gate so debug-but-interactive apps, which still enter the
alt-screen/hide the cursor, restore on signal — Ink ink.tsx:426); add
!teardownStarted to the registration so a spent app instance does not
re-register on a same-instance remount (the next unmount() returns early at
the teardownStarted guard before it could unsubscribe — a leak); and make
the PTY test prove the SIGNAL drove teardown (fixture never self-unmounts, so
restore bytes can only come from the signal path) with a debug-mode signal
test, an exit-anchored waitForOutput drain, and a bounded retry for the
async-flush race under saturated runners.

Review follow-ups (2 fixes): synchronous restore flush on signal — the
signal-exit teardown path now writes the restore escapes (show-cursor,
leave-alt-screen, disable-kitty) via fs.writeSync to the stdout fd so they
reach the terminal before signal-exit re-raises the signal (a buffered async
stream.write could be lost on abrupt exit); the normal unmount path keeps async
writes. Removed the config-wide retry:3 from vitest.pty.config.ts (it masked
the whole PTY suite) and scoped a retry:2 to the signal-teardown describe only,
for the residual parent-side node-pty onData read-race under a saturated runner.

* chore(parity): ledger — G18 pr-open

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Yunfei He
2026-05-30 06:04:51 +08:00
committed by GitHub
parent 5cc94ed2ff
commit 216a7021a0
9 changed files with 287 additions and 13 deletions
+1 -1
View File
@@ -52,7 +52,7 @@ Non-obvious calls made while fixing gaps, recorded for review in the final repor
| G15 | box-layout-border | Vertical border sides not shifted up when borderTop=false (Ink offsetY) — left/right rails mispositioned | P2 | merged | `fix/parity-border-1cell` | #37 |
| G16 | box-layout-border | Per-edge borderDimColor=false cannot override general borderDimColor (`\|\| dimAll` vs Ink's `??`) | P3 | merged | `fix/parity-border-dim` | #44 |
| G17 | render-lifecycle-reconciler | Screen-reader live-path edges: <Static> still grid-painted (Ink linearizes, skipStaticElements:false) + empty SR frame gets a trailing newline (Ink writes wrapped output directly) | P3 | merged | `fix/parity-sr-edges` | #45 |
| G18 | render-lifecycle-reconciler | No signal-based teardown — terminal corrupted on SIGINT/SIGTERM/SIGHUP (Ink signal-exit at mount) | P1 | todo | — | — |
| G18 | render-lifecycle-reconciler | No signal-based teardown — terminal corrupted on SIGINT/SIGTERM/SIGHUP (Ink signal-exit at mount) | P1 | pr-open | `fix/parity-signal-teardown` | #47 |
| G19 | box-layout-border | Dynamic removal of most yoga style props does not reset to default (stale layout) | P2 | todo | — | — |
| G20 | stdout-stderr-stdin-size-cursor | writeToStdout/writeToStderr lack an isUnmounted/teardown guard (post-teardown writes corrupt terminal) | P2 | todo | — | — |
| G21 | text-wrap-transform | Nested <Transform> in <Text> gets hardcoded index 0 vs child sibling position (squash path) | P3 | todo | — | — |
@@ -0,0 +1,37 @@
import process from "node:process";
import { createApp, Text } from "@vue-tui/runtime";
import { defineComponent, onMounted } from "vue";
// Mounts a live interactive app in the alternate screen (cursor hidden), then
// signals readiness on stderr. It deliberately NEVER unmounts/exits on its own
// — `await app.waitUntilExit()` only resolves once teardown runs. The test
// sends a process signal (SIGINT/SIGTERM/SIGHUP); the runtime's signal-exit
// handler then restores the terminal (show cursor + leave alt screen) before
// the process winds down. Because there is no self-unmount path, the presence
// of restore bytes proves the SIGNAL drove teardown — not a coincidental
// normal unmount that would emit the same bytes. Mirrors Ink's
// signalExit(this.unmount) wiring.
const App = defineComponent(() => {
onMounted(() => {
// Emit readiness on stderr so the marker is not swallowed by alt-screen
// restore / final-frame writes on stdout, which the test asserts against.
// Defer one tick so the first frame is committed and the signal-exit
// handler is fully registered before the test sends a signal.
setTimeout(() => {
process.stderr.write("__READY__\n");
}, 50);
});
return () => <Text>signal teardown fixture</Text>;
});
// `--debug` mounts in debug mode (still interactive). Debug mode enters the
// alternate screen and hides the cursor just like a normal interactive mount,
// so signal-driven teardown must still restore the terminal — this exercises
// Finding 1 (the registration must not be gated on `!debug`).
const debug = process.argv.includes("--debug");
const app = createApp(App);
app.mount({ alternateScreen: true, debug });
await app.waitUntilExit();
@@ -18,11 +18,27 @@ const term = (fixture: string, args: string[] = []) => {
reject = reject2;
});
// Resolves with the raw exit info (code + signal) no matter how the process
// dies — used by signal-teardown tests where a SIGTERM/SIGINT kill is the
// expected outcome and a non-zero/signalled exit must not reject.
let exitInfoResolve: (info: { exitCode: number; signal?: number }) => void;
const exitInfoPromise = new Promise<{ exitCode: number; signal?: number }>((r) => {
exitInfoResolve = r;
});
let readyResolve: () => void;
const readyPromise = new Promise<void>((r) => {
readyResolve = r;
});
// Pending output-watchers: each resolves once the accumulated output matches
// its predicate. node-pty can fire onExit BEFORE the final onData chunk is
// delivered, so trailing bytes written during teardown (cursor restore,
// leave-alt-screen) may arrive after the exit event — especially under CI
// contention. Tests that assert on those bytes must wait for them, not for
// exit. Checked on every onData chunk below.
const outputWatchers = new Set<() => void>();
const env: Record<string, string> = {
...(process.env as Record<string, string>),
NODE_NO_WARNINGS: "1",
@@ -48,8 +64,42 @@ const term = (fixture: string, args: string[] = []) => {
ps.write(input);
});
},
// Send a process signal to the child once it has signalled readiness, so
// signal-driven teardown is exercised against a fully mounted app.
kill(signal: string) {
void readyPromise.then(() => {
ps.kill(signal);
});
},
output: "",
waitForExit: async () => exitPromise,
waitForExitInfo: async () => exitInfoPromise,
// Resolve once the accumulated output satisfies `predicate`, rejecting after
// `timeoutMs`. Use this (not waitForExitInfo) when asserting on bytes the
// child emits during teardown right before exit, which node-pty may deliver
// after the exit event.
waitForOutput: async (predicate: (output: string) => boolean, timeoutMs = 10000) =>
new Promise<void>((res, rej) => {
const check = () => {
if (predicate(result.output)) {
outputWatchers.delete(check);
clearTimeout(timer);
res();
return true;
}
return false;
};
const timer = setTimeout(() => {
outputWatchers.delete(check);
rej(
new Error(
`waitForOutput timed out after ${timeoutMs}ms. Output:\n${JSON.stringify(result.output)}`,
),
);
}, timeoutMs);
if (check()) return;
outputWatchers.add(check);
}),
};
ps.onData((data) => {
@@ -58,9 +108,15 @@ const term = (fixture: string, args: string[] = []) => {
if (result.output.includes("__READY__")) {
readyResolve();
}
for (const watcher of outputWatchers) {
watcher();
}
});
ps.onExit(({ exitCode }) => {
ps.onExit(({ exitCode, signal }) => {
exitInfoResolve({ exitCode, signal });
if (exitCode === 0) {
resolve();
return;
@@ -0,0 +1,76 @@
import { test as it, describe, expect } from "vite-plus/test";
import term from "./helpers/term.ts";
// Ink parity G18: when the process receives SIGINT/SIGTERM/SIGHUP, signal-exit
// runs teardown() first — restoring the cursor and leaving the alternate
// screen — so the terminal is not left corrupted (cursor hidden / alt-screen
// active). Mirrors Ink's `signalExit(this.unmount, {alwaysLast:false})`.
const SHOW_CURSOR = "\x1b[?25h";
const EXIT_ALT_SCREEN = "\x1b[?1049l";
// Robustness (Finding 2): the fixture has NO self-unmount path — `await
// app.waitUntilExit()` only resolves once teardown runs, and nothing in the
// fixture calls unmount()/exit(). So the ONLY way the restore bytes can appear
// is the signal driving the runtime's signal-exit teardown. That makes the
// restore-byte assertions below a genuine proof of the SIGNAL path, not a
// coincidental normal unmount emitting the same bytes: with the registration
// removed, the default signal action kills the child uncaught and NO restore
// bytes are emitted, so these assertions go red (verified RED on unfixed for
// all three signals + debug mode).
//
// We deliberately do NOT assert on node-pty's reported exit signal: a signalled
// PTY death is reported nondeterministically (signal-exit sometimes intercepts
// for a graceful code-0 exit, sometimes re-raises so the child dies by the
// signal number — both AFTER teardown has restored the terminal). The restore
// bytes are the stable, meaningful invariant.
//
// We wait for the child to EXIT first, then for the restore bytes to drain:
// node-pty can fire the exit event a tick before delivering the final onData
// chunk (the teardown bytes), so we give a short post-exit drain window. We
// anchor on exit rather than racing a wall-clock on incremental output because
// under `vp run ready` every core is busy (lint/build/other pools), and a
// starved vitest worker may not process onData callbacks for seconds — the
// bytes are buffered in node-pty, not lost, so waiting for exit is reliable.
const restored = (output: string) =>
output.includes(SHOW_CURSOR) && output.includes(EXIT_ALT_SCREEN);
const assertRestored = async (ps: ReturnType<typeof term>) => {
await ps.waitForExitInfo();
// Drain the final post-exit chunk if it hasn't arrived yet. If the signal-exit
// registration is broken the child dies uncaught with NO restore bytes, so
// this drain times out (red).
await ps.waitForOutput(restored, 5000);
expect(ps.output).toContain(SHOW_CURSOR);
expect(ps.output).toContain(EXIT_ALT_SCREEN);
};
// Scoped retry (NOT config-wide): the runtime now flushes the restore escapes
// SYNCHRONOUSLY on the signal path (render.ts/kitty-keyboard.ts Finding A), so
// the child reliably emits show-cursor + leave-alt-screen before it dies
// (verified 40/40 standalone spawns, normal + debug). The only residual
// flakiness is a PARENT-SIDE harness read-race: under `vp run ready` every core
// is saturated by lint/build/other test pools, and a starved vitest worker can
// fail to drain node-pty's buffered onData (the already-flushed restore bytes)
// within the 5s post-exit window. That is a test-harness artifact, not a runtime
// regression — a broken signal-exit registration emits NO restore bytes on EVERY
// attempt, so these still go RED if the fix is reverted. The retry is scoped to
// THIS suite only so it can never mask flakiness in the rest of the PTY suite.
describe("signal-teardown", { retry: 2 }, () => {
for (const signal of ["SIGINT", "SIGTERM", "SIGHUP"] as const) {
it(`restores terminal on ${signal}`, async () => {
const ps = term("signal-teardown");
ps.kill(signal);
// Teardown ran (only possible via the signal): cursor re-shown, alt-screen left.
await assertRestored(ps);
});
// Finding 1: debug mode still enters the alternate screen + hides the cursor,
// so a debug-but-interactive app must also restore on signal. This is RED if
// the runtime gates its signal-exit registration on `!debug` (the signal then
// kills the child uncaught, leaving the terminal corrupted).
it(`restores terminal on ${signal} in debug mode`, async () => {
const ps = term("signal-teardown", ["--debug"]);
ps.kill(signal);
await assertRestored(ps);
});
}
});
@@ -20,6 +20,15 @@ export default defineConfig({
// trap). File-level parallelism is the proven, stable win.
fileParallelism: true,
testTimeout: 15000,
// NO config-wide retry: it would mask flakiness/real regressions across the
// WHOLE PTY suite. The signal-teardown tests previously needed retry because
// the restore bytes were written with an async stream.write() that could lose
// the race against signal-exit's immediate re-raise under a saturated runner.
// That is now fixed at the source (render.ts/kitty-keyboard.ts Finding A):
// the signal path writes the restore escapes synchronously (fs.writeSync), so
// they reach the fd before the process dies and the tests pass deterministically
// without any retry. If a genuine parent-side onData read-race ever resurfaces,
// scope a retry to that suite/test only (e.g. `test(name, { retry: 2 }, fn)`).
// CI:"false" so the runner's CI=true doesn't flip interactive detection off
// for any in-process render tests under this config (the PTY child helpers
// set it per-spawn, but vitest-level tests need it too).
+1
View File
@@ -31,6 +31,7 @@
"cli-truncate": "^6.0.0",
"is-in-ci": "catalog:",
"patch-console": "catalog:",
"signal-exit": "^4.1.0",
"slice-ansi": "^9.0.0",
"string-width": "^8.0.0",
"terminal-size": "catalog:",
+24 -2
View File
@@ -1,5 +1,7 @@
// packages/runtime/src/io/kitty-keyboard.ts
import { writeSync as fsWriteSync } from "node:fs";
const textEncoder = new TextEncoder();
export const kittyFlags = {
@@ -110,7 +112,13 @@ export function stripKittyQueryResponsesAndTrailingPartial(buffer: number[]): nu
export interface KittyKeyboardController {
init(options: KittyKeyboardOptions | undefined, interactive: boolean): void;
dispose(): void;
/**
* @param sync When true, write the disable-kitty escape synchronously
* (fs.writeSync) so it reaches the fd before an abrupt signal-driven exit
* re-raises the signal (G18, Finding A). Defaults to async stream.write for
* the normal unmount path.
*/
dispose(sync?: boolean): void;
readonly isEnabled: boolean;
}
@@ -195,13 +203,27 @@ export function createKittyKeyboardController(
confirmKittySupport(flags);
},
dispose() {
dispose(sync = false) {
disposed = true;
if (cancelDetection) {
cancelDetection();
}
if (enabled) {
if (sync) {
// Signal-exit path (G18, Finding A): flush the disable-kitty escape
// synchronously so it reaches the fd before signal-exit re-raises.
// Fall back to fd 1 when the stream has no numeric fd.
try {
// The base WriteStream type doesn't declare `fd`; tty streams do.
const streamFd = (stdout as { fd?: number }).fd;
const fd = typeof streamFd === "number" ? streamFd : 1;
fsWriteSync(fd, "\x1b[<u");
} catch {
// Best-effort restore during abrupt shutdown.
}
} else {
stdout.write("\x1b[<u");
}
enabled = false;
}
},
+71 -7
View File
@@ -11,7 +11,9 @@ import {
} from "vue";
import { createRenderer } from "@vue/runtime-core";
import { EventEmitter } from "node:events";
import { writeSync as fsWriteSync } from "node:fs";
import isInCi from "is-in-ci";
import { onExit } from "signal-exit";
import patchConsoleFn from "patch-console";
import ansiEscapes from "ansi-escapes";
import wrapAnsi from "wrap-ansi";
@@ -179,6 +181,10 @@ export function createApp(root: Component, rootProps?: RootProps | null): TuiApp
let mountedAppContext: AppContext | null = null;
let mountedResizeHandler: (() => void) | null = null;
let mountedExitListener: (() => void) | null = null;
// signal-exit unsubscribe fn (Ink parity G18). Registered at interactive
// mount so SIGINT/SIGTERM/SIGHUP route to teardown(); called in teardown()
// to remove the handler so it can't leak or double-run.
let mountedUnsubscribeExit: (() => void) | null = null;
let mountedBeforeExitHandler: (() => void) | null = null;
let mountedDebug = false;
let mountedInteractive = true;
@@ -239,23 +245,53 @@ export function createApp(root: Component, rootProps?: RootProps | null): TuiApp
}
}
function writeBestEffort(stream: NodeJS.WriteStream, data: string) {
function writeBestEffort(stream: NodeJS.WriteStream, data: string, sync = false) {
if (stream.destroyed || stream.writableEnded) return;
try {
if (sync) {
// Signal-exit path (G18, Finding A): signal-exit re-raises the signal
// IMMEDIATELY after this callback returns (`{alwaysLast:false}`), so a
// bare async `stream.write()` can leave the restore bytes (show-cursor,
// leave-alt-screen, disable-kitty) buffered and unflushed when the
// process dies — the terminal stays corrupted. A synchronous fd write
// guarantees the bytes hit the fd before the re-raise. Restore output is
// tiny and this only runs on the rare abrupt-exit path. Fall back to fd
// 1 (stdout) when the stream has no numeric fd (e.g. some wrapped TTYs).
// The base WriteStream type doesn't declare `fd`; tty/fs streams do.
const streamFd = (stream as { fd?: number }).fd;
const fd = typeof streamFd === "number" ? streamFd : 1;
fsWriteSync(fd, data);
} else {
stream.write(data);
}
} catch {
// Stream may already be destroyed during shutdown.
// Stream may already be destroyed during shutdown, or the fd may be
// unwritable; restore is best-effort.
}
}
let teardownStarted = false;
function teardown() {
// `sync` is set only when teardown is driven by the signal-exit callback
// (G18, Finding A). On that path the restore escapes must be written
// synchronously (fs.writeSync) so they reach the fd before signal-exit
// re-raises the signal. The normal unmount()/exit() path keeps async writes.
function teardown(sync = false) {
// Skipped mount: this app never wired a renderer, so teardown is a
// complete no-op — do not touch any stream or the owner's WeakMap entry.
if (skippedMount) return;
if (teardownStarted) return;
teardownStarted = true;
// Remove the signal-exit handler first (Ink parity G18, ink.tsx:765:
// `this.unsubscribeExit()`). When teardown is triggered BY a signal,
// signal-exit has already unloaded its own listeners, so this is a no-op;
// when triggered by unmount()/exit(), it stops the handler from firing
// later (no leak, no double-run — teardownStarted also guards re-entry).
if (mountedUnsubscribeExit) {
mountedUnsubscribeExit();
mountedUnsubscribeExit = null;
}
// Remove this app from the live-instances registry so a subsequent mount()
// on the same stdout works normally. Only the owning app removes its entry;
// a no-op second mount (mountedAsOwner=false) must NOT evict the first
@@ -298,7 +334,9 @@ export function createApp(root: Component, rootProps?: RootProps | null): TuiApp
mountedAnimationScheduler?.dispose();
mountedAnimationScheduler = null;
if (mountedKittyController) {
mountedKittyController.dispose();
// Disable-kitty is a restore escape: on the signal path it must flush
// synchronously too (Finding A).
mountedKittyController.dispose(sync);
mountedKittyController = null;
}
if (!mountedDebug && !mountedInteractive && mountedAppContext) {
@@ -310,11 +348,11 @@ export function createApp(root: Component, rootProps?: RootProps | null): TuiApp
}
if (mountedWriter && !mountedDebug && mountedInteractive) mountedWriter.done();
if (mountedAlternateScreen && mountedAppContext) {
writeBestEffort(mountedAppContext.stdout, ansiEscapes.exitAlternativeScreen);
writeBestEffort(mountedAppContext.stdout, "\x1b[?25h");
writeBestEffort(mountedAppContext.stdout, ansiEscapes.exitAlternativeScreen, sync);
writeBestEffort(mountedAppContext.stdout, "\x1b[?25h", sync);
mountedAlternateScreen = false;
} else if (!mountedDebug && mountedInteractive && mountedAppContext) {
writeBestEffort(mountedAppContext.stdout, "\x1b[?25h");
writeBestEffort(mountedAppContext.stdout, "\x1b[?25h", sync);
}
if (mountedRoot) detachYoga(mountedRoot);
if (mountedResizeHandler && mountedAppContext) {
@@ -819,6 +857,32 @@ export function createApp(root: Component, rootProps?: RootProps | null): TuiApp
process.on("exit", exitListener);
mountedExitListener = exitListener;
// Signal-based teardown (Ink parity G18, ink.tsx:426). On SIGINT/SIGTERM/
// SIGHUP signal-exit runs this callback BEFORE the process dies, so
// teardown() restores the cursor, leaves the alternate screen, disables
// kitty keyboard, flushes the final frame and restores raw mode — the
// terminal isn't left corrupted. We do NOT return true / prevent exit
// (mirroring Ink's {alwaysLast:false}): signal-exit lets the signal
// proceed after the callback. teardown() is idempotent (teardownStarted
// guard), so a signal-triggered teardown plus a later unmount() won't
// double-run. Every interactive mount registers — including debug mode,
// which still enters the alternate screen and hides the cursor (above), so
// a debug-but-interactive app must restore on signal too (Ink registers
// signal-exit unconditionally, ink.tsx:426, and allows alt-screen in
// debug). Only render-to-string / non-interactive paths stay out, since
// they have no cursor/alt-screen to restore and must not touch process
// signal handlers. `!mountedUnsubscribeExit` guards against double-register
// on a no-op second mount; `!teardownStarted` keeps a spent (already
// torn-down) app instance from re-registering on a same-instance remount,
// which would otherwise leak — the next unmount() returns early at the
// teardownStarted guard before it could unsubscribe.
if (interactive && !mountedUnsubscribeExit && !teardownStarted) {
// sync=true: signal-exit re-raises the signal right after this callback
// returns, so the restore escapes must be flushed to the fd
// synchronously (Finding A) — a buffered async write can be lost.
mountedUnsubscribeExit = onExit(() => teardown(true), { alwaysLast: false });
}
// Patch console.log/warn/error etc. to route through writeToStdout /
// writeToStderr so console output doesn't corrupt the rendered frame.
// Disabled in debug mode (matching Ink).
+9
View File
@@ -191,6 +191,9 @@ importers:
patch-console:
specifier: 'catalog:'
version: 2.0.0
signal-exit:
specifier: ^4.1.0
version: 4.1.0
slice-ansi:
specifier: ^9.0.0
version: 9.0.0
@@ -1783,6 +1786,10 @@ packages:
resolution: {integrity: sha512-BR7VvDCVHO+q2xBEWskxS6DJE1qRnb7DxzUrogb71CWoSficBxYsiAGd+Kl0mmq/MprG9yArRkyrQxTO6XjMzA==}
hasBin: true
signal-exit@4.1.0:
resolution: {integrity: sha512-bzyZ1e88w9O1iNJbKnOlvYTrWPDl46O1bG0D3XInv+9tkPrxrN8jUUTiFlDkkmKWgn1M6CfIA13SuGqOa9Korw==}
engines: {node: '>=14'}
sirv@3.0.2:
resolution: {integrity: sha512-2wcC/oGxHis/BoHkkPwldgiPSYcpZK3JU28WoMVv55yHJgcZ8rlXvuG9iZggz+sU1d4bRgIGASwyWqjxu3FM0g==}
engines: {node: '>=18'}
@@ -3280,6 +3287,8 @@ snapshots:
semver@6.3.1: {}
signal-exit@4.1.0: {}
sirv@3.0.2:
dependencies:
'@polka/url': 1.0.0-next.29