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

[reyyanxahmed]

Summary

Fixes #13870

GUI editors like Zed, VS Code, Cursor, and others can legitimately exit with non-zero codes under normal circumstances (e.g., window closed while a workspace is loading, or the editor's internal process lifecycle). Previously, this caused an unhandled promise rejection that crashed the entire CLI process.

Changes

packages/core/src/utils/editor.ts

• GUI editor close handler: Resolve the promise and log a debug warning via debugLogger.warn() instead of rejecting when the editor exits with a non-zero code. This prevents the CLI from crashing.
• Emit CoreEvent.ExternalEditorClosed: The GUI editor path now emits this event on both the close and error handlers, consistent with the terminal editor path (which already did this in a finally block). This ensures the UI properly restores its state after the editor closes regardless of exit code.
• Spawn errors still reject: Actual spawn failures (e.g., editor binary not found) continue to reject the promise so the caller can handle them.

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

• Updated existing test for non-zero GUI editor exit to expect resolution with a warning log instead of rejection.
• Added tests for CoreEvent.ExternalEditorClosed emission on: successful exit, non-zero exit, and spawn error for all GUI editors.

Root Cause

The openDiff function had two code paths:

Path	Non-zero exit behavior	Emits ExternalEditorClosed?
Terminal editors (vim, emacs, etc.)	Throws (caught by finally)	Yes (in finally block)
GUI editors (VS Code, Zed, etc.)	Rejected promise (unhandled)	No

After this change, both paths handle non-zero exits gracefully and always emit ExternalEditorClosed.

Testing

All 155 tests in editor.test.ts pass.

[gemini-code-assist]

Summary of Changes

Hello @reyyanxahmed, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the robustness of the CLI's external editor integration by gracefully handling non-zero exit codes from GUI editors, which previously led to unhandled promise rejections and crashes. It also standardizes the emission of the ExternalEditorClosed event across all editor types, ensuring a more consistent and stable user experience when interacting with external tools.

Highlights

• Graceful GUI Editor Exit Handling: Modified the close handler for GUI editors to resolve the promise and log a debug warning instead of rejecting when the editor exits with a non-zero code. This prevents CLI crashes caused by legitimate non-zero exits from GUI editors.
• Consistent Event Emission: Ensured that the CoreEvent.ExternalEditorClosed event is now emitted for GUI editors on both close and error handlers, aligning its behavior with terminal editors and ensuring proper UI state restoration.
• Preserved Spawn Error Rejection: Maintained the existing behavior where actual spawn failures (e.g., editor binary not found) continue to reject the promise, allowing callers to handle critical errors appropriately.
• Updated Test Suite: Updated existing tests to reflect the new non-zero exit behavior for GUI editors (expecting resolution with a warning). New tests were added to verify CoreEvent.ExternalEditorClosed emission across successful exits, non-zero exits, and spawn errors for GUI editors.

Changelog

• packages/core/src/utils/editor.test.ts
  • Updated the test case for non-zero GUI editor exit to assert that the promise resolves and a warning is logged, rather than rejecting.
  • Added new test cases to verify that CoreEvent.ExternalEditorClosed is emitted when a GUI editor exits successfully (code 0).
  • Added new test cases to verify that CoreEvent.ExternalEditorClosed is emitted when a GUI editor exits with a non-zero code.
  • Added new test cases to verify that CoreEvent.ExternalEditorClosed is emitted when a GUI editor encounters a spawn error.
• packages/core/src/utils/editor.ts
  • Modified the childProcess.on('close') handler to log a warning for non-zero exit codes from GUI editors instead of rejecting the promise, and to emit CoreEvent.ExternalEditorClosed.
  • Added coreEvents.emit(CoreEvent.ExternalEditorClosed) to the childProcess.on('error') handler for GUI editors.

Activity

• The pull request was created by reyyanxahmed.
• The pull request addresses and fixes issue Unhandled Promise Rejection when external editor (Zed) exits with code 1 #13870.
• The author provided a detailed summary of changes, root cause analysis, and testing information.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly addresses a crash caused by unhandled promise rejections when a GUI editor exits with a non-zero status code. The approach of resolving the promise and logging a warning is a good solution. The changes also ensure the ExternalEditorClosed event is consistently emitted, which is a great improvement for UI state management. I've added one comment to make the event handling more robust against a potential race condition where both error and close events could fire for a single failure, preventing duplicate event emissions and ensuring the promise is settled only once.

[gemini-code-assist]

There's a potential race condition here. According to Node.js documentation, the close event can be emitted after an error event. In such a scenario, both event handlers would run. This would cause coreEvents.emit(CoreEvent.ExternalEditorClosed) to be called twice and would attempt to settle the promise twice (first rejecting, then resolving). While attempting to resolve an already-rejected promise is a no-op, emitting the ExternalEditorClosed event twice could lead to unintended side effects in the UI if the listener is not idempotent. To make this more robust, you should ensure that the logic for settling the promise and emitting the event runs only once by using a flag. This approach aligns with the principle of using explicit state variables to manage the state of asynchronous operations, as outlined in our repository's code review guidelines.

    let isSettled = false;

    childProcess.on('close', (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);
    });

References

1. When managing the state of asynchronous operations, rely on an explicit state variable (e.g., a state enum) rather than checking for the existence of a promise object. Promise objects may be cleared in finally blocks upon completion, making them unreliable for state checks after the operation has finished.

[reyyanxahmed]

/gemini-code-assist could you check the latest fix I pushed "862db1b"? Here's what changed:

Added an [isSettled] flag in the GUI editor path of [openDiff] so that if Node.js fires both [error] and close for the same child process failure, only the first handler runs. This prevents [CoreEvent.ExternalEditorClosed] from being emitted twice and the promise from being settled twice.
Added 5 new tests (one per GUI editor) covering the race condition where both events fire sequentially, asserting that [ExternalEditorClosed] is emitted exactly once.

All 160 tests pass.

[gemini-code-assist]

Yes, the changes in commit 862db1b correctly address the potential race condition by introducing the isSettled flag. This ensures that the promise is settled and CoreEvent.ExternalEditorClosed is emitted only once, even if both error and close events fire. The addition of dedicated tests for this specific race condition is also excellent and provides robust validation for the fix.

[jacob314]