From bf24fff1f7d497f3e1d160d4ac21ece5ef5cf5a8 Mon Sep 17 00:00:00 2001 From: wenshao Date: Fri, 24 Apr 2026 20:03:39 +0800 Subject: [PATCH] fix(cli): destroy stdout instead of process.exit on EPIPE Routine CLI patterns like `qwen -p ... | head -1` / `| less` / `| grep -m1` close the downstream pipe and trigger EPIPE. The previous handler called process.exit(0), which bypassed the caller's runExitCleanup -> Config .shutdown -> chat-recording flush() chain and silently dropped queued JSONL writes (most recent assistant turn + tool results). Destroying stdout instead lets writes fail fast and the natural function return drive cleanup. We deliberately do not also abortController.abort() here: the abort path runs handleCancellationError which itself calls process.exit(130), re-introducing the same bypass. Reported by zhangxy-zju on #3581. --- packages/cli/src/nonInteractiveCli.test.ts | 32 ++++++++++++++++++++++ packages/cli/src/nonInteractiveCli.ts | 20 +++++++++----- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/nonInteractiveCli.test.ts b/packages/cli/src/nonInteractiveCli.test.ts index 9f3e339eb..c1f8ab9ba 100644 --- a/packages/cli/src/nonInteractiveCli.test.ts +++ b/packages/cli/src/nonInteractiveCli.test.ts @@ -273,6 +273,38 @@ describe('runNonInteractive', () => { expect(mockShutdownTelemetry).toHaveBeenCalled(); }); + it('on EPIPE, destroys stdout and returns normally instead of process.exit', async () => { + // Regression: process.exit(0) on EPIPE bypassed runExitCleanup → flush() + // and dropped queued JSONL writes for `qwen -p ... | head -1` patterns. + // process.exit is mocked to throw in beforeEach, so reaching the + // assertion also proves the bypass route is gone. + setupMetricsMock(); + const stdoutDestroySpy = vi + .spyOn(process.stdout, 'destroy') + .mockReturnValue(process.stdout); + + mockGeminiClient.sendMessageStream.mockImplementation( + async function* mockStream(): AsyncGenerator { + process.stdout.emit( + 'error', + Object.assign(new Error('EPIPE'), { code: 'EPIPE' }), + ); + yield { type: GeminiEventType.Content, value: 'Hello' }; + yield { + type: GeminiEventType.Finished, + value: { + reason: undefined, + usageMetadata: { totalTokenCount: 0 }, + }, + }; + }, + ); + + await runNonInteractive(mockConfig, mockSettings, 'test', 'p1'); + + expect(stdoutDestroySpy).toHaveBeenCalled(); + }); + it('should handle a single tool call and respond', async () => { setupMetricsMock(); const toolCallEvent: ServerGeminiStreamEvent = { diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index dc90c7775..23965c7d9 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -175,16 +175,22 @@ export async function runNonInteractive( let totalApiDurationMs = 0; const startTime = Date.now(); - const stdoutErrorHandler = (err: NodeJS.ErrnoException) => { - if (err.code === 'EPIPE') { - process.stdout.removeListener('error', stdoutErrorHandler); - process.exit(0); - } - }; - const geminiClient = config.getGeminiClient(); const abortController = options.abortController ?? new AbortController(); + // EPIPE: don't process.exit here — that bypasses the caller's + // runExitCleanup → flush() and drops queued JSONL writes. Destroy + // stdout instead and let the natural return drive cleanup. (Aborting + // is also wrong: the abort path runs handleCancellationError → exit + // 130 and re-introduces the same bypass.) + let pipeBroken = false; + const stdoutErrorHandler = (err: NodeJS.ErrnoException) => { + if (err.code === 'EPIPE' && !pipeBroken) { + pipeBroken = true; + process.stdout.destroy(); + } + }; + // Setup signal handlers for graceful shutdown const shutdownHandler = () => { debugLogger.debug('[runNonInteractive] Shutdown signal received');