216a7021a0
* 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>
77 lines
4.2 KiB
TypeScript
77 lines
4.2 KiB
TypeScript
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);
|
|
});
|
|
}
|
|
});
|