fix(cli): drain runExitCleanup before process.exit in error handlers (#3602)

handleError / handleCancellationError / handleMaxTurnsExceededError all
called process.exit synchronously, bypassing the caller's runExitCleanup
-> Config.shutdown -> chat-recording flush() chain on SIGINT, max-turn,
and fatal-error paths. Same family as the EPIPE bypass fixed in
bf24fff1f, just on a different code path.

Makes the three handlers async and routes the exit through a shared
exitAfterCleanup() helper that awaits runExitCleanup() before the actual
process.exit. The helper carries an exit-once latch so a SIGINT racing a
stream rejection (handleCancellationError + handleError fired
concurrently) doesn't end up running cleanup twice or interleaving exit
calls — only the first caller drains and exits, the second parks on a
never-resolving promise that's killed when process.exit fires.

Text-mode handleError still throws to the caller (unchanged behavior),
but now drains the queue first so the unhandled-rejection path doesn't
lose chat-recording records.

Five call sites in nonInteractiveCli.ts updated to await. Existing 11
errors.test.ts cases adapted to async + rejects.toThrow; added 5 new
regression guards covering cleanup-before-exit ordering for each
handler plus the concurrent-handler race.

Co-authored-by: wenshao <wenshao@U-K7F6PQY3-2157.local>

packages/cli/src/nonInteractiveCli.test.ts

@@ -28,6 +28,8 @@ import { vi, type Mock, type MockInstance } from 'vitest';
 import type { LoadedSettings } from './config/settings.js';
 import { CommandKind, type ExecutionMode } from './ui/commands/types.js';
 import { filterCommandsForMode } from './services/commandUtils.js';
+import { _resetCleanupFunctionsForTest } from './utils/cleanup.js';
+import { _resetExitLatchForTest } from './utils/errors.js';
 
 // Mock core modules
 vi.mock('./ui/hooks/atCommandProcessor.js');
@@ -79,6 +81,12 @@ describe('runNonInteractive', () => {
   let mockGetDebugResponses: Mock;
 
   beforeEach(async () => {
+    // Reset module-level state from any prior test in this file. Without
+    // these resets the once-set exit latch parks subsequent JSON-mode
+    // handleError tests in the never-resolving promise (5s vitest timeout).
+    _resetCleanupFunctionsForTest();
+    _resetExitLatchForTest();
+
     mockCoreExecuteToolCall = vi.mocked(executeToolCall);
     mockShutdownTelemetry = vi.mocked(shutdownTelemetry);
     mockGetCommandsForMode.mockImplementation((mode: ExecutionMode) =>

packages/cli/src/nonInteractiveCli.ts

@@ -349,7 +349,7 @@ export async function runNonInteractive(
           config.getMaxSessionTurns() >= 0 &&
           turnCount > config.getMaxSessionTurns()
         ) {
-          handleMaxTurnsExceededError(config);
+          await handleMaxTurnsExceededError(config);
         }
 
         const toolCallRequests: ToolCallRequestInfo[] = [];
@@ -372,7 +372,7 @@ export async function runNonInteractive(
 
         for await (const event of responseStream) {
           if (abortController.signal.aborted) {
-            handleCancellationError(config);
+            await handleCancellationError(config);
           }
           // Use adapter for all event processing
           adapter.processEvent(event);
@@ -498,7 +498,7 @@ export async function runNonInteractive(
               config.getMaxSessionTurns() >= 0 &&
               turnCount > config.getMaxSessionTurns()
             ) {
-              handleMaxTurnsExceededError(config);
+              await handleMaxTurnsExceededError(config);
             }
 
             const inputFormat =
@@ -711,7 +711,7 @@ export async function runNonInteractive(
               while (localQueue.length > 0) {
                 emitNotificationToSdk(localQueue.shift()!);
               }
-              handleCancellationError(config);
+              await handleCancellationError(config);
             }
             await drainLocalQueue();
             const running = registry.getRunning();
@@ -766,7 +766,7 @@ export async function runNonInteractive(
         usage,
         stats,
       });
-      handleError(error, config);
+      await handleError(error, config);
     } finally {
       const reg = config.getBackgroundTaskRegistry();
       reg.setNotificationCallback(undefined);

packages/cli/src/utils/errors.test.ts

@@ -12,12 +12,14 @@ import {
   ToolErrorType,
 } from '@qwen-code/qwen-code-core';
 import {
+  _resetExitLatchForTest,
   getErrorMessage,
   handleError,
   handleToolError,
   handleCancellationError,
   handleMaxTurnsExceededError,
 } from './errors.js';
+import { _resetCleanupFunctionsForTest, registerCleanup } from './cleanup.js';
 
 const mockWriteStderrLine = vi.hoisted(() => vi.fn());
 const debugLoggerSpy = vi.hoisted(() => ({
@@ -99,6 +101,8 @@ describe('errors', () => {
     debugLoggerSpy.info.mockClear();
     debugLoggerSpy.warn.mockClear();
     debugLoggerSpy.error.mockClear();
+    _resetCleanupFunctionsForTest();
+    _resetExitLatchForTest();
 
     // Mock process.stderr.write
     processStderrWriteSpy = vi
@@ -176,24 +180,24 @@ describe('errors', () => {
         ).mockReturnValue(OutputFormat.TEXT);
       });
 
-      it('should log error message and re-throw', () => {
+      it('should log error message and re-throw', async () => {
         const testError = new Error('Test error');
 
-        expect(() => {
-          handleError(testError, mockConfig);
-        }).toThrow(testError);
+        await expect(handleError(testError, mockConfig)).rejects.toThrow(
+          testError,
+        );
 
         expect(mockWriteStderrLine).toHaveBeenCalledWith(
           'API Error: Test error',
         );
       });
 
-      it('should handle non-Error objects', () => {
+      it('should handle non-Error objects', async () => {
         const testError = 'String error';
 
-        expect(() => {
-          handleError(testError, mockConfig);
-        }).toThrow(testError);
+        await expect(handleError(testError, mockConfig)).rejects.toThrow(
+          testError,
+        );
 
         expect(mockWriteStderrLine).toHaveBeenCalledWith(
           'API Error: String error',
@@ -208,12 +212,12 @@ describe('errors', () => {
         ).mockReturnValue(OutputFormat.JSON);
       });
 
-      it('should format error as JSON and exit with default code', () => {
+      it('should format error as JSON and exit with default code', async () => {
         const testError = new Error('Test error');
 
-        expect(() => {
-          handleError(testError, mockConfig);
-        }).toThrow('process.exit called with code: 1');
+        await expect(handleError(testError, mockConfig)).rejects.toThrow(
+          'process.exit called with code: 1',
+        );
 
         expect(mockWriteStderrLine).toHaveBeenCalledWith(
           JSON.stringify(
@@ -230,12 +234,12 @@ describe('errors', () => {
         );
       });
 
-      it('should use custom error code when provided', () => {
+      it('should use custom error code when provided', async () => {
         const testError = new Error('Test error');
 
-        expect(() => {
-          handleError(testError, mockConfig, 42);
-        }).toThrow('process.exit called with code: 42');
+        await expect(handleError(testError, mockConfig, 42)).rejects.toThrow(
+          'process.exit called with code: 42',
+        );
 
         expect(mockWriteStderrLine).toHaveBeenCalledWith(
           JSON.stringify(
@@ -252,12 +256,12 @@ describe('errors', () => {
         );
       });
 
-      it('should extract exitCode from FatalError instances', () => {
+      it('should extract exitCode from FatalError instances', async () => {
         const fatalError = new FatalInputError('Fatal error');
 
-        expect(() => {
-          handleError(fatalError, mockConfig);
-        }).toThrow('process.exit called with code: 42');
+        await expect(handleError(fatalError, mockConfig)).rejects.toThrow(
+          'process.exit called with code: 42',
+        );
 
         expect(mockWriteStderrLine).toHaveBeenCalledWith(
           JSON.stringify(
@@ -274,26 +278,26 @@ describe('errors', () => {
         );
       });
 
-      it('should handle error with code property', () => {
+      it('should handle error with code property', async () => {
         const errorWithCode = new Error('Error with code') as Error & {
           code: number;
         };
         errorWithCode.code = 404;
 
-        expect(() => {
-          handleError(errorWithCode, mockConfig);
-        }).toThrow('process.exit called with code: 404');
+        await expect(handleError(errorWithCode, mockConfig)).rejects.toThrow(
+          'process.exit called with code: 404',
+        );
       });
 
-      it('should handle error with status property', () => {
+      it('should handle error with status property', async () => {
         const errorWithStatus = new Error('Error with status') as Error & {
           status: string;
         };
         errorWithStatus.status = 'TIMEOUT';
 
-        expect(() => {
-          handleError(errorWithStatus, mockConfig);
-        }).toThrow('process.exit called with code: 1'); // string codes become 1
+        await expect(handleError(errorWithStatus, mockConfig)).rejects.toThrow(
+          'process.exit called with code: 1', // string codes become 1
+        );
 
         expect(mockWriteStderrLine).toHaveBeenCalledWith(
           JSON.stringify(
@@ -590,10 +594,10 @@ describe('errors', () => {
         ).mockReturnValue(OutputFormat.TEXT);
       });
 
-      it('should log cancellation message and exit with 130', () => {
-        expect(() => {
-          handleCancellationError(mockConfig);
-        }).toThrow('process.exit called with code: 130');
+      it('should log cancellation message and exit with 130', async () => {
+        await expect(handleCancellationError(mockConfig)).rejects.toThrow(
+          'process.exit called with code: 130',
+        );
 
         expect(mockWriteStderrLine).toHaveBeenCalledWith(
           'Operation cancelled.',
@@ -608,10 +612,10 @@ describe('errors', () => {
         ).mockReturnValue(OutputFormat.JSON);
       });
 
-      it('should format cancellation as JSON and exit with 130', () => {
-        expect(() => {
-          handleCancellationError(mockConfig);
-        }).toThrow('process.exit called with code: 130');
+      it('should format cancellation as JSON and exit with 130', async () => {
+        await expect(handleCancellationError(mockConfig)).rejects.toThrow(
+          'process.exit called with code: 130',
+        );
 
         expect(mockWriteStderrLine).toHaveBeenCalledWith(
           JSON.stringify(
@@ -638,10 +642,10 @@ describe('errors', () => {
         ).mockReturnValue(OutputFormat.TEXT);
       });
 
-      it('should log max turns message and exit with 53', () => {
-        expect(() => {
-          handleMaxTurnsExceededError(mockConfig);
-        }).toThrow('process.exit called with code: 53');
+      it('should log max turns message and exit with 53', async () => {
+        await expect(handleMaxTurnsExceededError(mockConfig)).rejects.toThrow(
+          'process.exit called with code: 53',
+        );
 
         expect(mockWriteStderrLine).toHaveBeenCalledWith(
           'Reached max session turns for this session. Increase the number of turns by specifying maxSessionTurns in settings.json.',
@@ -656,10 +660,10 @@ describe('errors', () => {
         ).mockReturnValue(OutputFormat.JSON);
       });
 
-      it('should format max turns error as JSON and exit with 53', () => {
-        expect(() => {
-          handleMaxTurnsExceededError(mockConfig);
-        }).toThrow('process.exit called with code: 53');
+      it('should format max turns error as JSON and exit with 53', async () => {
+        await expect(handleMaxTurnsExceededError(mockConfig)).rejects.toThrow(
+          'process.exit called with code: 53',
+        );
 
         expect(mockWriteStderrLine).toHaveBeenCalledWith(
           JSON.stringify(
@@ -678,4 +682,119 @@ describe('errors', () => {
       });
     });
   });
+
+  describe('cleanup-before-exit invariant', () => {
+    // Regression: previously these handlers called process.exit synchronously,
+    // bypassing the caller's runExitCleanup → flush() chain on SIGINT, max-
+    // turn, and fatal-error paths. Same family as the EPIPE/process.exit
+    // bug fixed for stdout in nonInteractiveCli.
+    it('handleCancellationError drains registered cleanups before exit', async () => {
+      const cleanupOrder: string[] = [];
+      registerCleanup(() => {
+        cleanupOrder.push('cleanup');
+      });
+      processExitSpy.mockImplementation((code) => {
+        cleanupOrder.push(`exit:${code}`);
+        throw new Error(`process.exit called with code: ${code}`);
+      });
+
+      await expect(handleCancellationError(mockConfig)).rejects.toThrow(
+        'process.exit called with code: 130',
+      );
+
+      expect(cleanupOrder).toEqual(['cleanup', 'exit:130']);
+    });
+
+    it('handleMaxTurnsExceededError drains registered cleanups before exit', async () => {
+      const cleanupOrder: string[] = [];
+      registerCleanup(() => {
+        cleanupOrder.push('cleanup');
+      });
+      processExitSpy.mockImplementation((code) => {
+        cleanupOrder.push(`exit:${code}`);
+        throw new Error(`process.exit called with code: ${code}`);
+      });
+
+      await expect(handleMaxTurnsExceededError(mockConfig)).rejects.toThrow(
+        'process.exit called with code: 53',
+      );
+
+      expect(cleanupOrder).toEqual(['cleanup', 'exit:53']);
+    });
+
+    it('handleError drains registered cleanups before exit (JSON mode)', async () => {
+      (mockConfig.getOutputFormat as ReturnType<typeof vi.fn>).mockReturnValue(
+        OutputFormat.JSON,
+      );
+      const cleanupOrder: string[] = [];
+      registerCleanup(() => {
+        cleanupOrder.push('cleanup');
+      });
+      processExitSpy.mockImplementation((code) => {
+        cleanupOrder.push(`exit:${code}`);
+        throw new Error(`process.exit called with code: ${code}`);
+      });
+
+      await expect(handleError(new Error('boom'), mockConfig)).rejects.toThrow(
+        'process.exit called with code: 1',
+      );
+
+      expect(cleanupOrder).toEqual(['cleanup', 'exit:1']);
+    });
+
+    it('a second terminating handler does not race the first into double-exit', async () => {
+      // Models the real concurrency: SIGINT → handleCancellationError fires
+      // while a stream rejection lands in the catch → handleError(JSON).
+      // Without the exit-once latch we'd get duplicate cleanup runs +
+      // duplicate process.exit calls + interleaved stderr writes.
+      // (Text-mode handleError throws instead of exiting, so it isn't part
+      // of the race — the latch lives on the exit path.)
+      (mockConfig.getOutputFormat as ReturnType<typeof vi.fn>).mockReturnValue(
+        OutputFormat.JSON,
+      );
+
+      let exitCalls = 0;
+      processExitSpy.mockImplementation((code) => {
+        exitCalls += 1;
+        throw new Error(`process.exit called with code: ${code}`);
+      });
+
+      const first = handleCancellationError(mockConfig);
+      const second = handleError(new Error('boom'), mockConfig);
+
+      await expect(first).rejects.toThrow('process.exit called with code: 130');
+
+      // The second handler is parked in the latch's unresolved promise.
+      let secondSettled = false;
+      void second.then(
+        () => {
+          secondSettled = true;
+        },
+        () => {
+          secondSettled = true;
+        },
+      );
+      await new Promise((r) => setTimeout(r, 20));
+
+      expect(exitCalls).toBe(1);
+      expect(secondSettled).toBe(false);
+    });
+
+    it('handleError drains registered cleanups before re-throw (text mode)', async () => {
+      // Text mode re-throws to the caller; we still want the queue drained
+      // first so the unhandled-rejection path doesn't lose records.
+      (mockConfig.getOutputFormat as ReturnType<typeof vi.fn>).mockReturnValue(
+        OutputFormat.TEXT,
+      );
+      const events: string[] = [];
+      registerCleanup(() => {
+        events.push('cleanup');
+      });
+
+      const original = new Error('boom');
+      await expect(handleError(original, mockConfig)).rejects.toBe(original);
+
+      expect(events).toEqual(['cleanup']);
+    });
+  });
 });

packages/cli/src/utils/errors.ts

@@ -14,6 +14,7 @@ import {
   ToolErrorType,
   createDebugLogger,
 } from '@qwen-code/qwen-code-core';
+import { runExitCleanup } from './cleanup.js';
 import { writeStderrLine } from './stdioHelpers.js';
 
 const debugLogger = createDebugLogger('CLI_ERRORS');
@@ -81,16 +82,45 @@ function getNumericExitCode(errorCode: string | number): number {
   return typeof errorCode === 'number' ? errorCode : 1;
 }
 
+/**
+ * Drains pending cleanup before terminating. Routing every "we're about
+ * to die" path through here keeps async exit-side I/O (chat-recording
+ * flush, telemetry shutdown, MCP disconnect) from being skipped — the
+ * earlier sync writes were inherently bounded so a bare `process.exit`
+ * was safe; with the async-jsonl change it is not.
+ */
+// Guards against double-entry when two terminating paths race (e.g. SIGINT
+// fires `handleCancellationError` while a stream rejection routes through
+// `handleError`): only the first caller drains cleanup + exits; the second
+// suspends forever in the unresolved promise and gets killed when the first
+// caller's process.exit fires.
+let exiting = false;
+
+async function exitAfterCleanup(code: number): Promise<never> {
+  if (exiting) return new Promise<never>(() => {});
+  exiting = true;
+  await runExitCleanup();
+  // `return` so process.exit's `never` narrows the function's terminating
+  // statement — without it TS reports "function returning 'never' cannot
+  // have a reachable end point" because await doesn't propagate `never`.
+  return process.exit(code);
+}
+
+/** Test-only — reset the exit-once latch between cases. */
+export function _resetExitLatchForTest(): void {
+  exiting = false;
+}
+
 /**
  * Handles errors consistently for both JSON and text output formats.
  * In JSON mode, outputs formatted JSON error and exits.
  * In text mode, outputs error message and re-throws.
  */
-export function handleError(
+export async function handleError(
   error: unknown,
   config: Config,
   customErrorCode?: string | number,
-): never {
+): Promise<never> {
   const errorMessage = parseAndFormatApiError(
     error,
     config.getContentGeneratorConfig()?.authType,
@@ -106,9 +136,12 @@ export function handleError(
     );
 
     writeStderrLine(formattedError);
-    process.exit(getNumericExitCode(errorCode));
+    return exitAfterCleanup(getNumericExitCode(errorCode));
   } else {
     writeStderrLine(errorMessage);
+    // Drain queued writes before re-throwing so the unhandled rejection
+    // path doesn't lose chat-recording records that are still in the queue.
+    await runExitCleanup();
     throw error;
   }
 }
@@ -155,7 +188,7 @@ export function handleToolError(
 /**
  * Handles cancellation/abort signals consistently.
  */
-export function handleCancellationError(config: Config): never {
+export async function handleCancellationError(config: Config): Promise<never> {
   const cancellationError = new FatalCancellationError('Operation cancelled.');
 
   if (config.getOutputFormat() === OutputFormat.JSON) {
@@ -166,17 +199,18 @@ export function handleCancellationError(config: Config): never {
     );
 
     writeStderrLine(formattedError);
-    process.exit(cancellationError.exitCode);
   } else {
     writeStderrLine(cancellationError.message);
-    process.exit(cancellationError.exitCode);
   }
+  return exitAfterCleanup(cancellationError.exitCode);
 }
 
 /**
  * Handles max session turns exceeded consistently.
  */
-export function handleMaxTurnsExceededError(config: Config): never {
+export async function handleMaxTurnsExceededError(
+  config: Config,
+): Promise<never> {
   const maxTurnsError = new FatalTurnLimitedError(
     'Reached max session turns for this session. Increase the number of turns by specifying maxSessionTurns in settings.json.',
   );
@@ -189,9 +223,8 @@ export function handleMaxTurnsExceededError(config: Config): never {
     );
 
     writeStderrLine(formattedError);
-    process.exit(maxTurnsError.exitCode);
   } else {
     writeStderrLine(maxTurnsError.message);
-    process.exit(maxTurnsError.exitCode);
   }
+  return exitAfterCleanup(maxTurnsError.exitCode);
 }