From 73b6149e9b81eed71568163b27fe9223291e1e13 Mon Sep 17 00:00:00 2001 From: Andy00L <89641810+Andy00L@users.noreply.github.com> Date: Sun, 5 Jul 2026 20:38:45 -0400 Subject: [PATCH 1/2] feat(cli): graceful termination signals + broken-pipe guard Install handlers for SIGINT/SIGTERM/SIGHUP that print a one-line explanation (any started run keeps executing server-side; resume with `testsprite test list` or `testsprite test wait `) and exit with the conventional 128+signum code (SIGINT -> 130). Also guard EPIPE on stdout/stderr so piping to a reader that closes early (`... | head`) exits cleanly instead of dumping a raw `write EPIPE` stack. process and streams are injectable, so both are unit-tested without spawning a subprocess or sending a real signal. Fixes #75 --- src/index.ts | 9 +++ src/lib/interrupt.test.ts | 93 +++++++++++++++++++++++++++++++ src/lib/interrupt.ts | 113 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 215 insertions(+) create mode 100644 src/lib/interrupt.test.ts create mode 100644 src/lib/interrupt.ts diff --git a/src/index.ts b/src/index.ts index fb935c8..54fbd30 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,6 +12,7 @@ import { createProjectCommand } from './commands/project.js'; import { createTestCommand } from './commands/test.js'; import { createUsageCommand } from './commands/usage.js'; import { ApiError, CLIError, RequestTimeoutError } from './lib/errors.js'; +import { installBrokenPipeGuard, installSignalHandlers } from './lib/interrupt.js'; import { Output, isOutputMode } from './lib/output.js'; import { maybeInstallProxyAgent } from './lib/proxy.js'; import { renderCommanderError, rephraseUnknownOption } from './lib/render-error.js'; @@ -161,6 +162,14 @@ program.hook('preAction', (_thisCommand, actionCommand) => { } }); +// Clean process lifecycle: a clear message + conventional exit code on SIGINT / +// SIGTERM / SIGHUP (instead of Node's silent abrupt kill) so an interrupted +// `test run --wait` explains the run continues server-side; plus an EPIPE guard +// so piping to a reader that closes early (`| head`) exits cleanly instead of +// dumping a raw `write EPIPE` stack. +installSignalHandlers(); +installBrokenPipeGuard(); + // Corporate/CI proxies: honor HTTPS_PROXY/HTTP_PROXY/NO_PROXY (Node's fetch // ignores them by default). No-op when no proxy variable is set. maybeInstallProxyAgent(); diff --git a/src/lib/interrupt.test.ts b/src/lib/interrupt.test.ts new file mode 100644 index 0000000..5df5198 --- /dev/null +++ b/src/lib/interrupt.test.ts @@ -0,0 +1,93 @@ +import { EventEmitter } from 'node:events'; +import { describe, expect, it, vi } from 'vitest'; +import { + SIGINT_EXIT_CODE, + TERMINATION_EXIT_CODES, + formatInterruptMessage, + installBrokenPipeGuard, + installSignalHandlers, +} from './interrupt.js'; + +describe('formatInterruptMessage', () => { + it('defaults to SIGINT and explains the run continues server-side', () => { + const message = formatInterruptMessage(); + expect(message).toContain('Interrupted (SIGINT)'); + expect(message).toContain('test wait'); + expect(message).toContain('test list'); + }); + + it('names the specific signal when given one', () => { + expect(formatInterruptMessage('SIGTERM')).toContain('Interrupted (SIGTERM)'); + expect(formatInterruptMessage('SIGHUP')).toContain('Interrupted (SIGHUP)'); + }); +}); + +describe('installSignalHandlers', () => { + it('registers SIGINT, SIGTERM and SIGHUP with the conventional 128+signum exit codes', () => { + const handlers = new Map void>(); + const stderr: string[] = []; + const exit = vi.fn(); + + installSignalHandlers({ + on: (signal, handler) => handlers.set(signal, handler), + stderr: line => stderr.push(line), + exit, + }); + + expect([...handlers.keys()].sort()).toEqual(['SIGHUP', 'SIGINT', 'SIGTERM']); + + handlers.get('SIGINT')!(); + expect(exit).toHaveBeenLastCalledWith(130); + handlers.get('SIGTERM')!(); + expect(exit).toHaveBeenLastCalledWith(143); + handlers.get('SIGHUP')!(); + expect(exit).toHaveBeenLastCalledWith(129); + + // Each handler emits a leading blank line then the explanation. + expect(stderr[0]).toBe(''); + expect(stderr.join('\n')).toContain('Interrupted (SIGINT)'); + expect(stderr.join('\n')).toContain('Interrupted (SIGTERM)'); + expect(stderr.join('\n')).toContain('Interrupted (SIGHUP)'); + expect(SIGINT_EXIT_CODE).toBe(130); + expect(TERMINATION_EXIT_CODES.SIGTERM).toBe(143); + expect(TERMINATION_EXIT_CODES.SIGHUP).toBe(129); + }); +}); + +describe('installBrokenPipeGuard', () => { + function makeEpipe(): NodeJS.ErrnoException { + return Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }); + } + + it('exits 0 on stdout EPIPE (clean SIGPIPE-equivalent for `| head`)', () => { + const stdout = new EventEmitter(); + const stderr = new EventEmitter(); + const exit = vi.fn(); + installBrokenPipeGuard({ stdout, stderr, exit }); + + stdout.emit('error', makeEpipe()); + expect(exit).toHaveBeenCalledWith(0); + }); + + it('re-throws a non-EPIPE stdout error instead of silently swallowing it', () => { + const stdout = new EventEmitter(); + const stderr = new EventEmitter(); + const exit = vi.fn(); + installBrokenPipeGuard({ stdout, stderr, exit }); + + expect(() => + stdout.emit('error', Object.assign(new Error('boom'), { code: 'ENOSPC' })), + ).toThrow('boom'); + expect(exit).not.toHaveBeenCalled(); + }); + + it('swallows stderr EPIPE without exiting or throwing', () => { + const stdout = new EventEmitter(); + const stderr = new EventEmitter(); + const exit = vi.fn(); + installBrokenPipeGuard({ stdout, stderr, exit }); + + expect(() => stderr.emit('error', makeEpipe())).not.toThrow(); + expect(exit).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/interrupt.ts b/src/lib/interrupt.ts new file mode 100644 index 0000000..20bd01a --- /dev/null +++ b/src/lib/interrupt.ts @@ -0,0 +1,113 @@ +/** + * Process lifecycle hardening: graceful termination signals and broken-pipe. + * + * Termination signals: without a handler, Node terminates the process abruptly + * with no output, so a user (Ctrl+C), a CI runner or `docker stop` (SIGTERM), or + * a closed terminal/SSH session (SIGHUP) that interrupts a long + * `test run --wait` is left unsure whether the run was cancelled or is still + * executing server-side (it is: the CLI only polls; the run lives on the + * backend). The handler prints a one-line explanation plus how to resume, then + * exits with the conventional `128 + signal` code. + * + * Broken pipe: when output is piped to a reader that closes early + * (`testsprite ... | head`), the kernel raises `EPIPE` on the next stdout write. + * Node turns an `'error'` with no listener into an uncaughtException and dumps a + * raw `write EPIPE` stack (exit 1). The guard swallows it and exits 0, the + * conventional SIGPIPE-equivalent result for "the reader went away". + * + * `process` and the streams are injectable so the wiring is unit-testable + * without spawning a subprocess or sending a real signal. + */ + +/** + * Termination signals handled, mapped to their conventional `128 + signum` + * exit code. sourceRef: POSIX signal numbers (SIGHUP=1, SIGINT=2, SIGTERM=15). + */ +export const TERMINATION_EXIT_CODES = { + SIGINT: 130, // 128 + 2 + SIGTERM: 143, // 128 + 15 + SIGHUP: 129, // 128 + 1 +} as const; + +export type TerminationSignal = keyof typeof TERMINATION_EXIT_CODES; + +/** Back-compat alias: SIGINT's conventional exit code. */ +export const SIGINT_EXIT_CODE = TERMINATION_EXIT_CODES.SIGINT; + +export function formatInterruptMessage(signal: TerminationSignal = 'SIGINT'): string { + return ( + `Interrupted (${signal}). Any run already started keeps executing on the server; ` + + 'check it with `testsprite test list` or `testsprite test wait `.' + ); +} + +export interface InterruptDeps { + /** Signal registrar. Defaults to `process.on`. */ + on?: (signal: TerminationSignal, handler: () => void) => void; + /** Line-oriented stderr writer (appends a newline). */ + stderr?: (line: string) => void; + /** Process exit. Defaults to `process.exit`. */ + exit?: (code: number) => void; +} + +/** + * Register handlers for SIGINT, SIGTERM and SIGHUP. Idempotent enough for a + * single top-level call in `index.ts`; not designed to be installed twice. + */ +export function installSignalHandlers(deps: InterruptDeps = {}): void { + const on = + deps.on ?? + ((signal: TerminationSignal, handler: () => void) => { + process.on(signal, handler); + }); + const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); + const exit = deps.exit ?? ((code: number) => process.exit(code)); + + for (const signal of Object.keys(TERMINATION_EXIT_CODES) as TerminationSignal[]) { + on(signal, () => { + // Blank line first so the message starts on its own row rather than + // trailing the progress ticker's in-place line. + stderr(''); + stderr(formatInterruptMessage(signal)); + exit(TERMINATION_EXIT_CODES[signal]); + }); + } +} + +export interface BrokenPipeDeps { + /** stdout stream. Defaults to `process.stdout`. */ + stdout?: NodeJS.EventEmitter; + /** stderr stream. Defaults to `process.stderr`. */ + stderr?: NodeJS.EventEmitter; + /** Process exit. Defaults to `process.exit`. */ + exit?: (code: number) => void; +} + +/** + * Guard against `EPIPE` on stdout/stderr so piping to a reader that closes + * early (`testsprite ... | head`) exits cleanly instead of crashing with an + * unhandled `write EPIPE` stack. Only `EPIPE` is swallowed; any other stream + * error is left to surface normally. + */ +export function installBrokenPipeGuard(deps: BrokenPipeDeps = {}): void { + const stdout = deps.stdout ?? process.stdout; + const stderr = deps.stderr ?? process.stderr; + const exit = deps.exit ?? ((code: number) => process.exit(code)); + + stdout.on('error', (error: NodeJS.ErrnoException) => { + // Reader went away (`| head`, `| less` then q): exit cleanly like SIGPIPE + // rather than dumping an unhandled `write EPIPE` stack. Any other stdout + // error is a genuine, actionable failure, so re-throw it (Node's default). + if (error.code === 'EPIPE') { + exit(0); + return; + } + throw error; + }); + stderr.on('error', (error: NodeJS.ErrnoException) => { + // stderr closed: nothing can be reported over it, so swallow EPIPE. Any + // other error re-throws so a genuine failure is not silently hidden. + if (error.code === 'EPIPE') return; + throw error; + }); +} From d5341d6cf96c5df5ece3f6b1bbe0ee6a4b11e3c4 Mon Sep 17 00:00:00 2001 From: Andy00L <89641810+Andy00L@users.noreply.github.com> Date: Sun, 5 Jul 2026 21:13:48 -0400 Subject: [PATCH 2/2] fix(interrupt): flush the signal message synchronously before exit A signal handler calls process.exit() immediately after writing the interrupt hint. When stderr is a pipe, an async process.stderr.write() may not flush before the process terminates, so the hint could be lost. The default stderr writer now uses fs.writeSync (best-effort, guarded against EPIPE) so the hint is reliably emitted. Added a test mocking fs.writeSync to assert the synchronous write on the default path. --- src/lib/interrupt.test.ts | 26 ++++++++++++++++++++++++++ src/lib/interrupt.ts | 15 ++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/lib/interrupt.test.ts b/src/lib/interrupt.test.ts index 5df5198..1fcf8eb 100644 --- a/src/lib/interrupt.test.ts +++ b/src/lib/interrupt.test.ts @@ -1,4 +1,5 @@ import { EventEmitter } from 'node:events'; +import { writeSync } from 'node:fs'; import { describe, expect, it, vi } from 'vitest'; import { SIGINT_EXIT_CODE, @@ -8,6 +9,13 @@ import { installSignalHandlers, } from './interrupt.js'; +// installSignalHandlers' default stderr writes via fs.writeSync (synchronous, so +// the hint survives a piped stderr before exit); mock it to assert on that path. +vi.mock('node:fs', async importOriginal => { + const actual = (await importOriginal()) as Record; + return { ...actual, writeSync: vi.fn() }; +}); + describe('formatInterruptMessage', () => { it('defaults to SIGINT and explains the run continues server-side', () => { const message = formatInterruptMessage(); @@ -52,6 +60,24 @@ describe('installSignalHandlers', () => { expect(TERMINATION_EXIT_CODES.SIGTERM).toBe(143); expect(TERMINATION_EXIT_CODES.SIGHUP).toBe(129); }); + + it('writes the hint synchronously via writeSync before exit (survives a piped stderr)', () => { + vi.mocked(writeSync).mockClear(); + const handlers = new Map void>(); + const exit = vi.fn(); + // No stderr dep: exercise the synchronous default path. + installSignalHandlers({ + on: (signal, handler) => handlers.set(signal, handler), + exit, + }); + handlers.get('SIGINT')!(); + expect(exit).toHaveBeenCalledWith(130); + const written = vi + .mocked(writeSync) + .mock.calls.map(call => String(call[1])) + .join(''); + expect(written).toContain('Interrupted (SIGINT)'); + }); }); describe('installBrokenPipeGuard', () => { diff --git a/src/lib/interrupt.ts b/src/lib/interrupt.ts index 20bd01a..cc5b4d5 100644 --- a/src/lib/interrupt.ts +++ b/src/lib/interrupt.ts @@ -19,6 +19,8 @@ * without spawning a subprocess or sending a real signal. */ +import { writeSync } from 'node:fs'; + /** * Termination signals handled, mapped to their conventional `128 + signum` * exit code. sourceRef: POSIX signal numbers (SIGHUP=1, SIGINT=2, SIGTERM=15). @@ -60,7 +62,18 @@ export function installSignalHandlers(deps: InterruptDeps = {}): void { ((signal: TerminationSignal, handler: () => void) => { process.on(signal, handler); }); - const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); + const stderr = + deps.stderr ?? + ((line: string) => { + // A signal handler calls process.exit() right after writing, which can + // truncate an async process.stderr.write() when stderr is a pipe. Write + // synchronously so the interrupt hint is flushed before the process exits. + try { + writeSync(process.stderr.fd, `${line}\n`); + } catch { + // Best-effort: if stderr is already gone (EPIPE), still exit cleanly. + } + }); const exit = deps.exit ?? ((code: number) => process.exit(code)); for (const signal of Object.keys(TERMINATION_EXIT_CODES) as TerminationSignal[]) {