fix: resolve infinite loop when using 'Modify with external editor'

packages/cli/src/ui/AppContainer.tsx

@@ -524,12 +524,22 @@ export const AppContainer = (props: AppContainerProps) => {
     refreshStatic();
   }, [refreshStatic, isAlternateBuffer, app, config]);
 
+  const [editorError, setEditorError] = useState<string | null>(null);
+  const {
+    isEditorDialogOpen,
+    openEditorDialog,
+    handleEditorSelect,
+    exitEditorDialog,
+  } = useEditorSettings(settings, setEditorError, historyManager.addItem);
+
   useEffect(() => {
     coreEvents.on(CoreEvent.ExternalEditorClosed, handleEditorClose);
+    coreEvents.on(CoreEvent.RequestEditorSelection, openEditorDialog);
     return () => {
       coreEvents.off(CoreEvent.ExternalEditorClosed, handleEditorClose);
+      coreEvents.off(CoreEvent.RequestEditorSelection, openEditorDialog);
     };
-  }, [handleEditorClose]);
+  }, [handleEditorClose, openEditorDialog]);
 
   useEffect(() => {
     if (
@@ -543,6 +553,9 @@ export const AppContainer = (props: AppContainerProps) => {
     }
   }, [bannerVisible, bannerText, settings, config, refreshStatic]);
 
+  const { isSettingsDialogOpen, openSettingsDialog, closeSettingsDialog } =
+    useSettingsCommand();
+
   const {
     isThemeDialogOpen,
     openThemeDialog,
@@ -738,17 +751,6 @@ Logging in with Google... Restarting Gemini CLI to continue.
     onAuthError,
   ]);
 
-  const [editorError, setEditorError] = useState<string | null>(null);
-  const {
-    isEditorDialogOpen,
-    openEditorDialog,
-    handleEditorSelect,
-    exitEditorDialog,
-  } = useEditorSettings(settings, setEditorError, historyManager.addItem);
-
-  const { isSettingsDialogOpen, openSettingsDialog, closeSettingsDialog } =
-    useSettingsCommand();
-
   const { isModelDialogOpen, openModelDialog, closeModelDialog } =
     useModelCommand();
 

packages/cli/src/ui/hooks/useEditorSettings.ts

@@ -15,6 +15,8 @@ import {
   allowEditorTypeInSandbox,
   checkHasEditorType,
   getEditorDisplayName,
+  coreEvents,
+  CoreEvent,
 } from '@google/gemini-cli-core';
 import type { UseHistoryManagerReturn } from './useHistoryManager.js';
 
@@ -66,6 +68,7 @@ export const useEditorSettings = (
         );
         setEditorError(null);
         setIsEditorDialogOpen(false);
+        coreEvents.emit(CoreEvent.EditorSelected, { editor: editorType });
       } catch (error) {
         setEditorError(`Failed to set editor preference: ${error}`);
       }
@@ -75,6 +78,7 @@ export const useEditorSettings = (
 
   const exitEditorDialog = useCallback(() => {
     setIsEditorDialogOpen(false);
+    coreEvents.emit(CoreEvent.EditorSelected, { editor: undefined });
   }, []);
 
   return {

packages/core/src/scheduler/confirmation.ts

@@ -21,9 +21,14 @@ import type { ValidatingToolCall, WaitingToolCall } from './types.js';
 import type { Config } from '../config/config.js';
 import type { SchedulerStateManager } from './state-manager.js';
 import type { ToolModificationHandler } from './tool-modifier.js';
-import type { EditorType } from '../utils/editor.js';
+import {
+  resolveEditorAsync,
+  type EditorType,
+  NO_EDITOR_AVAILABLE_ERROR,
+} from '../utils/editor.js';
 import type { DiffUpdateResult } from '../ide/ide-client.js';
 import { debugLogger } from '../utils/debugLogger.js';
+import { coreEvents } from '../utils/events.js';
 
 export interface ConfirmationResult {
   outcome: ToolConfirmationOutcome;
@@ -155,7 +160,18 @@ export async function resolveConfirmation(
     }
 
     if (outcome === ToolConfirmationOutcome.ModifyWithEditor) {
-      await handleExternalModification(deps, toolCall, signal);
+      const modResult = await handleExternalModification(
+        deps,
+        toolCall,
+        signal,
+      );
+      if (!modResult.success) {
+        // Editor is not available - emit error feedback and stay in the loop
+        // to return to previous confirmation screen.
+        if (modResult.error) {
+          coreEvents.emitFeedback('error', modResult.error);
+        }
+      }
     } else if (response.payload && 'newContent' in response.payload) {
       await handleInlineModification(deps, toolCall, response.payload, signal);
       outcome = ToolConfirmationOutcome.ProceedOnce;
@@ -182,8 +198,19 @@ async function notifyHooks(
   }
 }
 
+/**
+ * Result of attempting external modification.
+ */
+interface ExternalModificationResult {
+  /** Whether the modification was successful (editor was opened) */
+  success: boolean;
+  /** Error message if the modification failed */
+  error?: string;
+}
+
 /**
  * Handles modification via an external editor (e.g. Vim).
+ * Returns a result indicating success or failure with an error message.
  */
 async function handleExternalModification(
   deps: {
@@ -193,10 +220,19 @@ async function handleExternalModification(
   },
   toolCall: ValidatingToolCall,
   signal: AbortSignal,
-): Promise<void> {
+): Promise<ExternalModificationResult> {
   const { state, modifier, getPreferredEditor } = deps;
-  const editor = getPreferredEditor();
-  if (!editor) return;
+
+  const preferredEditor = getPreferredEditor();
+  const editor = await resolveEditorAsync(preferredEditor, signal);
+
+  if (!editor) {
+    // No editor available - return failure with error message
+    return {
+      success: false,
+      error: NO_EDITOR_AVAILABLE_ERROR,
+    };
+  }
 
   const result = await modifier.handleModifyWithEditor(
     state.firstActiveCall as WaitingToolCall,
@@ -211,6 +247,7 @@ async function handleExternalModification(
       newInvocation,
     );
   }
+  return { success: true };
 }
 
 /**

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

@@ -15,16 +15,21 @@ import {
 } from 'vitest';
 import {
   checkHasEditorType,
+  checkHasEditorTypeAsync,
   getDiffCommand,
   openDiff,
   allowEditorTypeInSandbox,
   isEditorAvailable,
+  isEditorAvailableAsync,
+  resolveEditorAsync,
   type EditorType,
 } from './editor.js';
-import { execSync, spawn, spawnSync } from 'node:child_process';
+import { coreEvents, CoreEvent } from './events.js';
+import { exec, execSync, spawn, spawnSync } from 'node:child_process';
 import { debugLogger } from './debugLogger.js';
 
 vi.mock('child_process', () => ({
+  exec: vi.fn(),
   execSync: vi.fn(),
   spawn: vi.fn(),
   spawnSync: vi.fn(() => ({ error: null, status: 0 })),
@@ -542,4 +547,167 @@ describe('editor utils', () => {
       expect(isEditorAvailable('neovim')).toBe(true);
     });
   });
+
+  // Helper to create a mock exec that simulates async behavior
+  const mockExecAsync = (implementation: (cmd: string) => boolean): void => {
+    (exec as unknown as Mock).mockImplementation(
+      (
+        cmd: string,
+        callback: (error: Error | null, stdout: string, stderr: string) => void,
+      ) => {
+        if (implementation(cmd)) {
+          callback(null, '/usr/bin/cmd', '');
+        } else {
+          callback(new Error('Command not found'), '', '');
+        }
+      },
+    );
+  };
+
+  describe('checkHasEditorTypeAsync', () => {
+    it('should return true if vim command exists', async () => {
+      Object.defineProperty(process, 'platform', { value: 'linux' });
+      mockExecAsync((cmd) => cmd.includes('vim'));
+      expect(await checkHasEditorTypeAsync('vim')).toBe(true);
+    });
+
+    it('should return false if vim command does not exist', async () => {
+      Object.defineProperty(process, 'platform', { value: 'linux' });
+      mockExecAsync(() => false);
+      expect(await checkHasEditorTypeAsync('vim')).toBe(false);
+    });
+
+    it('should check zed and zeditor commands in order', async () => {
+      Object.defineProperty(process, 'platform', { value: 'linux' });
+      mockExecAsync((cmd) => cmd.includes('zeditor'));
+      expect(await checkHasEditorTypeAsync('zed')).toBe(true);
+    });
+  });
+
+  describe('isEditorAvailableAsync', () => {
+    it('should return false for undefined editor', async () => {
+      expect(await isEditorAvailableAsync(undefined)).toBe(false);
+    });
+
+    it('should return false for empty string editor', async () => {
+      expect(await isEditorAvailableAsync('')).toBe(false);
+    });
+
+    it('should return false for invalid editor type', async () => {
+      expect(await isEditorAvailableAsync('invalid-editor')).toBe(false);
+    });
+
+    it('should return true for vscode when installed and not in sandbox mode', async () => {
+      mockExecAsync((cmd) => cmd.includes('code'));
+      vi.stubEnv('SANDBOX', '');
+      expect(await isEditorAvailableAsync('vscode')).toBe(true);
+    });
+
+    it('should return false for vscode when not installed', async () => {
+      mockExecAsync(() => false);
+      expect(await isEditorAvailableAsync('vscode')).toBe(false);
+    });
+
+    it('should return false for vscode in sandbox mode', async () => {
+      mockExecAsync((cmd) => cmd.includes('code'));
+      vi.stubEnv('SANDBOX', 'sandbox');
+      expect(await isEditorAvailableAsync('vscode')).toBe(false);
+    });
+
+    it('should return true for vim in sandbox mode', async () => {
+      mockExecAsync((cmd) => cmd.includes('vim'));
+      vi.stubEnv('SANDBOX', 'sandbox');
+      expect(await isEditorAvailableAsync('vim')).toBe(true);
+    });
+  });
+
+  describe('resolveEditorAsync', () => {
+    it('should return the preferred editor when available', async () => {
+      mockExecAsync((cmd) => cmd.includes('vim'));
+      vi.stubEnv('SANDBOX', '');
+      const result = await resolveEditorAsync('vim');
+      expect(result).toBe('vim');
+    });
+
+    it('should request editor selection when preferred editor is not installed', async () => {
+      mockExecAsync(() => false);
+      vi.stubEnv('SANDBOX', '');
+      const resolvePromise = resolveEditorAsync('vim');
+      setTimeout(
+        () => coreEvents.emit(CoreEvent.EditorSelected, { editor: 'neovim' }),
+        0,
+      );
+      const result = await resolvePromise;
+      expect(result).toBe('neovim');
+    });
+
+    it('should request editor selection when preferred GUI editor cannot be used in sandbox mode', async () => {
+      mockExecAsync((cmd) => cmd.includes('code'));
+      vi.stubEnv('SANDBOX', 'sandbox');
+      const resolvePromise = resolveEditorAsync('vscode');
+      setTimeout(
+        () => coreEvents.emit(CoreEvent.EditorSelected, { editor: 'vim' }),
+        0,
+      );
+      const result = await resolvePromise;
+      expect(result).toBe('vim');
+    });
+
+    it('should request editor selection when no preference is set', async () => {
+      const emitSpy = vi.spyOn(coreEvents, 'emit');
+      vi.stubEnv('SANDBOX', '');
+
+      const resolvePromise = resolveEditorAsync(undefined);
+
+      // Simulate UI selection
+      setTimeout(
+        () => coreEvents.emit(CoreEvent.EditorSelected, { editor: 'vim' }),
+        0,
+      );
+
+      const result = await resolvePromise;
+      expect(result).toBe('vim');
+      expect(emitSpy).toHaveBeenCalledWith(CoreEvent.RequestEditorSelection);
+    });
+
+    it('should return undefined when editor selection is cancelled', async () => {
+      const resolvePromise = resolveEditorAsync(undefined);
+
+      // Simulate UI cancellation (exit dialog)
+      setTimeout(
+        () => coreEvents.emit(CoreEvent.EditorSelected, { editor: undefined }),
+        0,
+      );
+
+      const result = await resolvePromise;
+      expect(result).toBeUndefined();
+    });
+
+    it('should return undefined when abort signal is triggered', async () => {
+      const controller = new AbortController();
+      const resolvePromise = resolveEditorAsync(undefined, controller.signal);
+
+      setTimeout(() => controller.abort(), 0);
+
+      const result = await resolvePromise;
+      expect(result).toBeUndefined();
+    });
+
+    it('should request editor selection in sandbox mode when no preference is set', async () => {
+      const emitSpy = vi.spyOn(coreEvents, 'emit');
+      vi.stubEnv('SANDBOX', 'sandbox');
+
+      const resolvePromise = resolveEditorAsync(undefined);
+
+      // Simulate UI selection
+      setTimeout(
+        () => coreEvents.emit(CoreEvent.EditorSelected, { editor: 'vim' }),
+        0,
+      );
+
+      const result = await resolvePromise;
+      expect(result).toBe('vim');
+      expect(emitSpy).toHaveBeenCalledWith(CoreEvent.RequestEditorSelection);
+    });
+  });
 });