fix(core): handle GUI editor non-zero exit codes gracefully (#20376)

Co-authored-by: Jacob Richman <jacob314@gmail.com>

Author: reyyanxahmed

packages/core/src/utils/editor.test.ts

@@ -392,17 +392,84 @@ describe('editor utils', () => {
         );
       });
 
-      it(`should reject if ${editor} exits with non-zero code`, async () => {
+      it(`should resolve and log warning if ${editor} exits with non-zero code`, async () => {
+        const warnSpy = vi
+          .spyOn(debugLogger, 'warn')
+          .mockImplementation(() => {});
         const mockSpawnOn = vi.fn((event, cb) => {
           if (event === 'close') {
             cb(1);
           }
         });
         (spawn as Mock).mockReturnValue({ on: mockSpawnOn });
 
+        await openDiff('old.txt', 'new.txt', editor);
+        expect(warnSpy).toHaveBeenCalledWith(`${editor} exited with code 1`);
+      });
+
+      it(`should emit ExternalEditorClosed when ${editor} exits successfully`, async () => {
+        const emitSpy = vi.spyOn(coreEvents, 'emit');
+        const mockSpawnOn = vi.fn((event, cb) => {
+          if (event === 'close') {
+            cb(0);
+          }
+        });
+        (spawn as Mock).mockReturnValue({ on: mockSpawnOn });
+
+        await openDiff('old.txt', 'new.txt', editor);
+        expect(emitSpy).toHaveBeenCalledWith(CoreEvent.ExternalEditorClosed);
+      });
+
+      it(`should emit ExternalEditorClosed when ${editor} exits with non-zero code`, async () => {
+        vi.spyOn(debugLogger, 'warn').mockImplementation(() => {});
+        const emitSpy = vi.spyOn(coreEvents, 'emit');
+        const mockSpawnOn = vi.fn((event, cb) => {
+          if (event === 'close') {
+            cb(1);
+          }
+        });
+        (spawn as Mock).mockReturnValue({ on: mockSpawnOn });
+
+        await openDiff('old.txt', 'new.txt', editor);
+        expect(emitSpy).toHaveBeenCalledWith(CoreEvent.ExternalEditorClosed);
+      });
+
+      it(`should emit ExternalEditorClosed when ${editor} spawn errors`, async () => {
+        const emitSpy = vi.spyOn(coreEvents, 'emit');
+        const mockError = new Error('spawn error');
+        const mockSpawnOn = vi.fn((event, cb) => {
+          if (event === 'error') {
+            cb(mockError);
+          }
+        });
+        (spawn as Mock).mockReturnValue({ on: mockSpawnOn });
+
         await expect(openDiff('old.txt', 'new.txt', editor)).rejects.toThrow(
-          `${editor} exited with code 1`,
+          'spawn error',
+        );
+        expect(emitSpy).toHaveBeenCalledWith(CoreEvent.ExternalEditorClosed);
+      });
+
+      it(`should only emit ExternalEditorClosed once when ${editor} fires both error and close`, async () => {
+        const emitSpy = vi.spyOn(coreEvents, 'emit');
+        const callbacks: Record<string, (arg: unknown) => void> = {};
+        const mockSpawnOn = vi.fn(
+          (event: string, cb: (arg: unknown) => void) => {
+            callbacks[event] = cb;
+          },
+        );
+        (spawn as Mock).mockReturnValue({ on: mockSpawnOn });
+
+        const promise = openDiff('old.txt', 'new.txt', editor);
+        // Simulate Node.js behavior: error fires first, then close.
+        callbacks['error'](new Error('spawn error'));
+        callbacks['close'](1);
+
+        await expect(promise).rejects.toThrow('spawn error');
+        const editorClosedEmissions = emitSpy.mock.calls.filter(
+          (call) => call[0] === CoreEvent.ExternalEditorClosed,
         );
+        expect(editorClosedEmissions).toHaveLength(1);
       });
     }
 

packages/core/src/utils/editor.ts

@@ -323,15 +323,30 @@ export async function openDiff(
       shell: process.platform === 'win32',
     });
 
+    // Guard against both 'error' and 'close' firing for a single failure,
+    // which would emit ExternalEditorClosed twice and attempt to settle
+    // the promise twice.
+    let isSettled = false;
+
     childProcess.on('close', (code) => {
-      if (code === 0) {
-        resolve();
-      } else {
-        reject(new Error(`${editor} exited with code ${code}`));
+      if (isSettled) return;
+      isSettled = true;
+
+      if (code !== 0) {
+        // GUI editors (VS Code, Zed, etc.) can exit with non-zero codes
+        // under normal circumstances (e.g., window closed while loading).
+        // Log a warning instead of crashing the CLI process.
+        debugLogger.warn(`${editor} exited with code ${code}`);
       }
+      coreEvents.emit(CoreEvent.ExternalEditorClosed);
+      resolve();
     });
 
     childProcess.on('error', (error) => {
+      if (isSettled) return;
+      isSettled = true;
+
+      coreEvents.emit(CoreEvent.ExternalEditorClosed);
       reject(error);
     });
   });