fix(progress): preserve Ctrl-C termination for shared reporter (#3614) (#3825)

Wave-assembled from PR #3825 by @javieraldape.

Co-Authored-By: Sofía González <sofiagonzalez@Sofias-MacBook-Air.local>
This commit is contained in:
Garry Tan
2026-08-12 14:38:36 -07:00
committed by Sina Matian
co-authored by Sofía González
parent b0b9af042f
commit 2dbaebbe16
2 changed files with 134 additions and 4 deletions
+14 -3
View File
@@ -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<LivePhase>();
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.
+120 -1
View File
@@ -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<null, Readable, Readable>;
/** 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<string, unknown>[] {
.map((l) => JSON.parse(l));
}
function waitForStdoutMarker(proc: SigintHarnessProcess, marker: string, readStdout: () => string, timeoutMs = 3000): Promise<void> {
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 });