diff --git a/src/core/progress.ts b/src/core/progress.ts index 3a5ebe0a2..8a40490c9 100644 --- a/src/core/progress.ts +++ b/src/core/progress.ts @@ -50,9 +50,10 @@ export interface ProgressReporter { // every live reporter. Per-instance handlers would leak listeners and interfere // with command-level handlers (e.g. shell-handler abort in jobs.ts). // -// We never call process.exit() or swallow the signal — we just emit abort -// events for live phases, then remove ourselves so the user's own handlers -// (or the default Node behavior) run as usual. +// We never call process.exit() — we just emit abort events for live phases. +// Installing a SIGINT listener suppresses the runtime's default terminate +// behavior, so when no command-level handler remains we re-raise SIGINT after +// the abort event has had a tick to flush. interface LivePhase { reporter: PhaseState; @@ -61,10 +62,15 @@ interface LivePhase { const liveReporters = new Set(); let signalHandlerInstalled = false; +let hadSigintHandlersAtInstall = false; function installSignalHandler(): void { if (signalHandlerInstalled) return; signalHandlerInstalled = true; + // Capture command-level SIGINT ownership before our once-wrapper can be + // consumed by the same signal emission. A preceding once('SIGINT') listener + // is already gone by the time our handler runs. + hadSigintHandlersAtInstall = process.listenerCount('SIGINT') > 0; const onSignal = (reason: 'SIGINT' | 'SIGTERM') => { // Copy to array so abort() can mutate liveReporters during iteration. @@ -76,6 +82,11 @@ function installSignalHandler(): void { /* best-effort */ } } + if (reason === 'SIGINT' && !hadSigintHandlersAtInstall && process.listenerCount('SIGINT') === 0) { + setTimeout(() => { + process.kill(process.pid, 'SIGINT'); + }, 0); + } }; // once() so we don't block user handlers or double-fire. diff --git a/test/progress.test.ts b/test/progress.test.ts index 7f266d770..0e73649bb 100644 --- a/test/progress.test.ts +++ b/test/progress.test.ts @@ -1,7 +1,12 @@ import { describe, test, expect } from 'bun:test'; -import { PassThrough } from 'node:stream'; +import { spawn, type ChildProcessByStdio } from 'node:child_process'; +import { join } from 'node:path'; +import { PassThrough, type Readable } from 'node:stream'; import { createProgress, startHeartbeat, __liveReporterCountForTest, __signalHandlerInstalledForTest } from '../src/core/progress.ts'; +const REPO = join(import.meta.dir, '..'); +type SigintHarnessProcess = ChildProcessByStdio; + /** Collect everything a reporter writes into a string. */ function sink(isTTY = false): { stream: PassThrough & { isTTY?: boolean }; read: () => string } { const s = new PassThrough() as PassThrough & { isTTY?: boolean }; @@ -18,6 +23,84 @@ function parseJsonl(raw: string): Record[] { .map((l) => JSON.parse(l)); } +function waitForStdoutMarker(proc: SigintHarnessProcess, marker: string, readStdout: () => string, timeoutMs = 3000): Promise { + return new Promise((resolve, reject) => { + const cleanup = () => { + clearTimeout(timeout); + proc.stdout.off('data', onData); + proc.off('exit', onExit); + proc.off('error', onError); + }; + const onData = () => { + if (!readStdout().includes(marker)) return; + cleanup(); + resolve(); + }; + const onExit = (code: number | null, signal: NodeJS.Signals | null) => { + cleanup(); + reject(new Error(`child exited before ${marker}: code=${code} signal=${signal}`)); + }; + const onError = (err: Error) => { + cleanup(); + reject(err); + }; + const timeout = setTimeout(() => { + cleanup(); + reject(new Error(`timed out waiting for ${marker}; stdout=${JSON.stringify(readStdout())}`)); + }, timeoutMs); + + proc.stdout.on('data', onData); + proc.once('exit', onExit); + proc.once('error', onError); + onData(); + }); +} + +function waitForExit(proc: SigintHarnessProcess, timeoutMs = 1500): Promise<{ exited: boolean; code: number | null; signal: NodeJS.Signals | null }> { + return new Promise((resolve) => { + const cleanup = () => { + clearTimeout(timeout); + proc.off('exit', onExit); + }; + const onExit = (code: number | null, signal: NodeJS.Signals | null) => { + cleanup(); + resolve({ exited: true, code, signal }); + }; + const timeout = setTimeout(() => { + cleanup(); + resolve({ exited: false, code: null, signal: null }); + }, timeoutMs); + proc.once('exit', onExit); + }); +} + +async function runSigintHarness(script: string): Promise<{ + exited: boolean; + code: number | null; + signal: NodeJS.Signals | null; + stdout: string; + stderr: string; +}> { + const proc = spawn(process.execPath, ['-e', script], { + cwd: REPO, + env: { ...process.env, FORCE_COLOR: '0' }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let stdout = ''; + let stderr = ''; + proc.stdout.on('data', (chunk) => { stdout += chunk.toString('utf8'); }); + proc.stderr.on('data', (chunk) => { stderr += chunk.toString('utf8'); }); + + await waitForStdoutMarker(proc, 'READY\n', () => stdout); + proc.kill('SIGINT'); + const result = await waitForExit(proc); + if (!result.exited) { + proc.kill('SIGKILL'); + await waitForExit(proc); + } + return { ...result, stdout, stderr }; +} + describe('progress reporter', () => { test('auto mode: non-TTY → human-plain (NOT JSON)', () => { const { stream, read } = sink(false); @@ -235,6 +318,42 @@ describe('progress reporter', () => { expect(__liveReporterCountForTest()).toBe(liveBefore); }); + test('SIGINT exits a child process when the progress reporter is the only handler', async () => { + const result = await runSigintHarness(` + const { createProgress } = await import('./src/core/progress.ts'); + const progress = createProgress({ mode: 'json' }); + progress.start('sigint_repro', 1); + process.stdout.write('READY\\n'); + setInterval(() => {}, 1000); + `); + + expect(result.exited).toBe(true); + expect(result.signal === 'SIGINT' || result.code === 130).toBe(true); + expect(result.stderr).toContain('"event":"abort"'); + expect(result.stderr).toContain('"reason":"SIGINT"'); + }); + + test('SIGINT still defers to another process handler when one is installed', async () => { + const result = await runSigintHarness(` + const { createProgress } = await import('./src/core/progress.ts'); + process.once('SIGINT', () => { + process.stdout.write('HANDLED\\n'); + setTimeout(() => process.exit(0), 50); + }); + const progress = createProgress({ mode: 'json' }); + progress.start('sigint_repro', 1); + process.stdout.write('READY\\n'); + setInterval(() => {}, 1000); + `); + + expect(result.exited).toBe(true); + expect(result.code).toBe(0); + expect(result.signal).toBeNull(); + expect(result.stdout).toContain('HANDLED'); + expect(result.stderr).toContain('"event":"abort"'); + expect(result.stderr).toContain('"reason":"SIGINT"'); + }); + test('startHeartbeat() fires heartbeats and stop() clears', async () => { const { stream, read } = sink(false); const p = createProgress({ mode: 'json', stream, minIntervalMs: 0, minItems: 1 });