diff --git a/.changeset/ci-broken-pipe-worker-crash.md b/.changeset/ci-broken-pipe-worker-crash.md new file mode 100644 index 0000000..1b3e533 --- /dev/null +++ b/.changeset/ci-broken-pipe-worker-crash.md @@ -0,0 +1,8 @@ +--- +'@offckb/cli': patch +--- + +Fix two CI failures on the macOS/Windows test matrix: + +- The broken-pipe regression test no longer crashes the jest worker ("jest worker process crashed for an unknown reason: exitCode=0"). It captures the EPIPE handlers installed by `installBrokenPipeHandlers` through a spy on `stream.on` instead of binding them to the live process stdout/stderr streams, so a real stream error can no longer reach a handler with the real (unmocked) `process.exit` and terminate the worker. The covered policy is unchanged: EPIPE exits 0 during normal operation, is swallowed during a graceful shutdown, and non-EPIPE errors are rethrown. +- The Windows daemon-identity probe no longer times out during the `verifyDaemonIdentity` test. The PowerShell + CIM query needs a cold start of a second or two, which routinely exceeded the old 5s probe bound on a loaded runner, making the identity check fail closed against a live process it should have accepted. The probe timeout is now 15s — generous enough for a slow machine, still bounded so a genuinely hung probe fails closed. diff --git a/src/util/daemon.ts b/src/util/daemon.ts index d454177..bfaa55f 100644 --- a/src/util/daemon.ts +++ b/src/util/daemon.ts @@ -296,9 +296,14 @@ function parsePsLstart(text: string): number | null { return Number.isFinite(ms) ? ms : null; } +// Generous so the Windows daemon-identity probe (PowerShell + CIM cold start) +// does not fail closed against a legitimate daemon on a slow machine; it only +// needs to bound a genuinely hung probe. +const PROCESS_PROBE_TIMEOUT_MS = 15_000; + function execFileText(command: string, args: string[]): Promise { return new Promise((resolve) => { - execFile(command, args, { timeout: 5000 }, (error, stdout) => { + execFile(command, args, { timeout: PROCESS_PROBE_TIMEOUT_MS }, (error, stdout) => { if (error) { resolve(null); return; diff --git a/tests/broken-pipe.test.ts b/tests/broken-pipe.test.ts index 22be314..3ddfdac 100644 --- a/tests/broken-pipe.test.ts +++ b/tests/broken-pipe.test.ts @@ -25,66 +25,86 @@ function epipe(): NodeJS.ErrnoException { describe('util/shutdown broken-pipe policy', () => { let shutdown: ShutdownModule; let exitSpy: jest.SpyInstance; - let stdoutListeners: unknown[]; - let stderrListeners: unknown[]; beforeEach(() => { jest.resetModules(); shutdown = require('../src/util/shutdown') as ShutdownModule; - stdoutListeners = process.stdout.rawListeners('error'); - stderrListeners = process.stderr.rawListeners('error'); - exitSpy = jest - .spyOn(process, 'exit') - .mockImplementation(((code?: number) => { - throw new ProcessExit(code); - }) as (code?: number) => never); + exitSpy = jest.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new ProcessExit(code); + }) as (code?: number) => never); }); afterEach(() => { exitSpy.mockRestore(); - for (const [stream, original] of [ - [process.stdout, stdoutListeners], - [process.stderr, stderrListeners], - ] as const) { - for (const listener of stream.rawListeners('error')) { - if (!original.includes(listener)) { - stream.removeListener('error', listener as (error: Error) => void); + }); + + /** + * Run installBrokenPipeHandlers without binding anything to the live + * process.stdout/stderr streams. Installing error handlers on the real + * streams inside a shared jest worker is unsafe: a genuine stream error + * (EPIPE on a closed pipe while running with piped output on macOS/Windows) + * can fire asynchronously and reach the handler after the mocked + * process.exit has been restored, causing the worker itself to exit — the + * "jest worker process crashed for an unknown reason: exitCode=0" CI + * failure. Capturing the handlers through a spy on `stream.on` tests the + * exact same policy logic (including that both streams get a handler) with + * no global side effects. + */ + function captureHandlers(): Array<(error: NodeJS.ErrnoException) => void> { + const handlers: Array<(error: NodeJS.ErrnoException) => void> = []; + // jest.spyOn calls through to the original method by default, so a plain + // spy would still register the handlers on the live streams (and + // mockRestore does not remove them). mockReturnThis makes `.on()` a no-op + // that only records the call, keeping the streams untouched. + const stdoutOn = jest.spyOn(process.stdout, 'on').mockReturnThis(); + const stderrOn = jest.spyOn(process.stderr, 'on').mockReturnThis(); + shutdown.installBrokenPipeHandlers(); + for (const streamOn of [stdoutOn, stderrOn]) { + for (const [event, handler] of streamOn.mock.calls) { + if (event === 'error') { + handlers.push(handler as (error: NodeJS.ErrnoException) => void); } } + streamOn.mockRestore(); } - }); + return handlers; + } it('starts outside a graceful shutdown', () => { expect(shutdown.isGracefulShutdownInProgress()).toBe(false); }); + it('installs an error handler on both stdout and stderr', () => { + expect(captureHandlers()).toHaveLength(2); + }); + it('exits 0 on EPIPE during normal operation (the `| head` case)', () => { - shutdown.installBrokenPipeHandlers(); - expect(() => process.stdout.emit('error', epipe())).toThrow(ProcessExit); + const [stdoutHandler] = captureHandlers(); + expect(() => stdoutHandler(epipe())).toThrow(ProcessExit); expect(exitSpy).toHaveBeenCalledWith(0); }); it('handles stderr the same way as stdout', () => { - shutdown.installBrokenPipeHandlers(); - expect(() => process.stderr.emit('error', epipe())).toThrow(ProcessExit); + const [, stderrHandler] = captureHandlers(); + expect(() => stderrHandler(epipe())).toThrow(ProcessExit); expect(exitSpy).toHaveBeenCalledWith(0); }); it('swallows EPIPE once a graceful shutdown is in progress', () => { - shutdown.installBrokenPipeHandlers(); + const [stdoutHandler, stderrHandler] = captureHandlers(); shutdown.enterGracefulShutdown(); expect(shutdown.isGracefulShutdownInProgress()).toBe(true); // Repeated writes to the dead pipe keep erroring; none may exit. - expect(() => process.stdout.emit('error', epipe())).not.toThrow(); - expect(() => process.stderr.emit('error', epipe())).not.toThrow(); + expect(() => stdoutHandler(epipe())).not.toThrow(); + expect(() => stderrHandler(epipe())).not.toThrow(); expect(exitSpy).not.toHaveBeenCalled(); }); it('still rethrows non-EPIPE stream errors during a shutdown', () => { - shutdown.installBrokenPipeHandlers(); + const [stdoutHandler] = captureHandlers(); shutdown.enterGracefulShutdown(); const error = new Error('some other stream failure'); - expect(() => process.stdout.emit('error', error)).toThrow(error); + expect(() => stdoutHandler(error)).toThrow(error); expect(exitSpy).not.toHaveBeenCalled(); }); });