-
Notifications
You must be signed in to change notification settings - Fork 10.9k
fix: resolve infinite loop when using 'Modify with external editor' (… #17453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: resolve infinite loop when using 'Modify with external editor' (… #17453
Conversation
…oogle-gemini#7669) This fix addresses the infinite loop issue reported in google-gemini#7669 where selecting "Modify with external editor" would loop indefinitely when no editor was configured or available. Root cause: When getPreferredEditor() returned undefined, the code silently returned without changing the outcome, causing the while loop to repeat. Changes: - Add detectFirstAvailableEditor() to auto-detect available editors - Add resolveEditor() to handle editor resolution with proper error messages - Update confirmation.ts to break the loop and show error when editor unavailable - Update coreToolScheduler.ts to cancel operation with feedback when editor unavailable - Add 11 new tests for the new editor resolution functions The fix: 1. Properly validates editor availability before attempting to use it 2. Auto-detects an available editor if none is configured 3. Provides clear error messages explaining why the editor cannot be used 4. Breaks the loop gracefully instead of looping infinitely
Summary of ChangesHello @ppgranger, I'm Gemini Code Assist1! 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 addresses a critical user experience issue where attempting to modify content with an external editor, without a valid editor configured or available, would lead to an infinite loop. The changes introduce a comprehensive editor detection and resolution system, ensuring that users receive clear feedback and the operation is gracefully cancelled instead of getting stuck. Highlights
Using Gemini Code AssistThe 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
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 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively resolves a critical infinite loop bug that occurred when attempting to use an external editor without one being configured or available. The introduction of the resolveEditor utility is a robust solution that properly detects available editors, provides clear user feedback on failure, and gracefully cancels the operation. The changes are well-implemented across coreToolScheduler.ts and confirmation.ts, and the new logic is thoroughly covered by unit tests.
I have one suggestion regarding the use of synchronous process execution, which could be refactored to be asynchronous to better align with project conventions for non-blocking operations.
Replace synchronous execSync calls with async alternatives in editor detection functions to prevent blocking the Node.js event loop. Changes: - Add commandExistsAsync using promisified exec - Add checkHasEditorTypeAsync, isEditorAvailableAsync, detectFirstAvailableEditorAsync, and resolveEditorAsync - Update confirmation.ts and coreToolScheduler.ts to use resolveEditorAsync - Mark synchronous resolveEditor as deprecated - Add comprehensive tests for all async functions The synchronous versions are kept for UI components that require synchronous execution (useEditorSettings, editorSettingsManager).
Extract the platform-specific command construction into getCommandExistsCmd() to avoid duplication between commandExists and commandExistsAsync.
|
Small commit : extracted the platform-specific command construction (where.exe on Windows, command -v elsewhere) into a shared getCommandExistsCmd() helper function to remove duplication between commandExists and commandExistsAsync. |
|
Hi there! Thank you for your contribution to Gemini CLI. We really appreciate the time and effort you've put into this pull request. To keep our backlog manageable and ensure we're focusing on current priorities, we are closing pull requests that haven't seen maintainer activity for 30 days. Currently, the team is prioritizing work associated with 🔒 maintainer only or help wanted issues. If you believe this change is still critical, please feel free to comment with updated details. Otherwise, we encourage contributors to focus on open issues labeled as help wanted. Thank you for your understanding! |
Summary
Fix infinite loop when selecting "Modify with external editor" without a configured or available editor. Users were stuck in an
endless loop with no feedback when attempting to use the external editor feature. This fix adds proper editor detection,
auto-fallback, and clear error messages.
Details
Root cause: In both
confirmation.tsandcoreToolScheduler.ts, whengetPreferredEditor()returnedundefined, the codesilently returned without changing the loop outcome variable, causing the confirmation loop to repeat indefinitely.
Solution:
detectFirstAvailableEditor()- auto-detects available editors, prioritizing terminal editors (vim, neovim, emacs, helix)that work in sandbox mode
resolveEditor()- comprehensive editor resolution that validates availability, auto-detects fallbacks, and providesspecific error messages
confirmation.tsto check editor resolution result and break the loop with user feedback when unavailablecoreToolScheduler.tsto cancel the operation cleanly with error feedback when no editor is availableError messages now explain exactly what went wrong:
Related Issues
Fixes #7669
How to Validate
No editor configured:
/editorand select "None"appear and the operation should cancel (no infinite loop)
Editor configured but not installed:
GUI editor in sandbox mode:
Working editor:
Run tests:
Pre-Merge Checklist