Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c904f4c
fix: resolve infinite loop when using 'Modify with external editor' (…
ppgranger Jan 22, 2026
a526c76
refactor(editor): use async functions to avoid blocking event loop
ppgranger Jan 24, 2026
4bd33f3
refactor(editor): extract command construction to shared function
ppgranger Jan 24, 2026
0ead53b
Merge branch 'main' into fix/modify-with-external-editor-infinite-loo…
ppgranger Feb 1, 2026
59422b0
Merge branch 'main' into fix/modify-with-external-editor-infinite-loo…
jackwotherspoon Feb 2, 2026
fa46232
Merge branch 'google-gemini:main' into fix/modify-with-external-edito…
ppgranger Feb 4, 2026
37dabd8
fix(confirmation): use type guard for ToolConfirmationPayload union type
ppgranger Feb 4, 2026
02fc2aa
fix(core): resolve infinite loop and improve editor selection flow
ehedlund Feb 4, 2026
8fd65b5
Move error message to constant.
ehedlund Feb 4, 2026
e981f8e
Merge branch 'main' into fix/modify-with-external-editor-infinite-loo…
ehedlund Feb 4, 2026
819a08d
test(core): mock resolveEditorAsync in modifyWithEditor test
ppgranger Feb 4, 2026
3f86193
Merge branch 'main' into fix/modify-with-external-editor-infinite-loo…
ppgranger Feb 4, 2026
2dcc453
Revert changes to legacy file coreToolScheduler.
ehedlund Feb 4, 2026
4db4cf9
refactor(core): address PR review feedback
ppgranger Feb 5, 2026
856b996
Merge branch 'main' into fix/modify-with-external-editor-infinite-loo…
ppgranger Feb 5, 2026
4e648eb
- Rename `mockCheckHasEditorType` -> `mockHasValidEditorCommand`
ehedlund Feb 5, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions packages/cli/src/ui/AppContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -525,12 +525,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 (
Expand All @@ -544,6 +554,9 @@ export const AppContainer = (props: AppContainerProps) => {
}
}, [bannerVisible, bannerText, settings, config, refreshStatic]);

const { isSettingsDialogOpen, openSettingsDialog, closeSettingsDialog } =
useSettingsCommand();

const {
isThemeDialogOpen,
openThemeDialog,
Expand Down Expand Up @@ -739,17 +752,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();

Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/ui/editors/editorSettingsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import {
allowEditorTypeInSandbox,
checkHasEditorType,
hasValidEditorCommand,
type EditorType,
EDITOR_DISPLAY_NAMES,
} from '@google/gemini-cli-core';
Expand All @@ -31,7 +31,7 @@ class EditorSettingsManager {
disabled: false,
},
...editorTypes.map((type) => {
const hasEditor = checkHasEditorType(type);
const hasEditor = hasValidEditorCommand(type);
const isAllowedInSandbox = allowEditorTypeInSandbox(type);

let labelSuffix = !isAllowedInSandbox
Expand Down
10 changes: 5 additions & 5 deletions packages/cli/src/ui/hooks/useEditorSettings.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { SettingScope } from '../../config/settings.js';
import { MessageType } from '../types.js';
import {
type EditorType,
checkHasEditorType,
hasValidEditorCommand,
allowEditorTypeInSandbox,
} from '@google/gemini-cli-core';
import type { UseHistoryManagerReturn } from './useHistoryManager.js';
Expand All @@ -35,12 +35,12 @@ vi.mock('@google/gemini-cli-core', async () => {
const actual = await vi.importActual('@google/gemini-cli-core');
return {
...actual,
checkHasEditorType: vi.fn(() => true),
hasValidEditorCommand: vi.fn(() => true),
allowEditorTypeInSandbox: vi.fn(() => true),
};
});

const mockCheckHasEditorType = vi.mocked(checkHasEditorType);
const mockHasValidEditorCommand = vi.mocked(hasValidEditorCommand);
const mockAllowEditorTypeInSandbox = vi.mocked(allowEditorTypeInSandbox);

describe('useEditorSettings', () => {
Expand Down Expand Up @@ -69,7 +69,7 @@ describe('useEditorSettings', () => {
mockAddItem = vi.fn();

// Reset mock implementations to default
mockCheckHasEditorType.mockReturnValue(true);
mockHasValidEditorCommand.mockReturnValue(true);
mockAllowEditorTypeInSandbox.mockReturnValue(true);
});

Expand Down Expand Up @@ -224,7 +224,7 @@ describe('useEditorSettings', () => {
it('should not set preference for unavailable editors', () => {
render(<TestComponent />);

mockCheckHasEditorType.mockReturnValue(false);
mockHasValidEditorCommand.mockReturnValue(false);

const editorType: EditorType = 'vscode';
const scope = SettingScope.User;
Expand Down
8 changes: 6 additions & 2 deletions packages/cli/src/ui/hooks/useEditorSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ import { MessageType } from '../types.js';
import type { EditorType } from '@google/gemini-cli-core';
import {
allowEditorTypeInSandbox,
checkHasEditorType,
hasValidEditorCommand,
getEditorDisplayName,
coreEvents,
CoreEvent,
} from '@google/gemini-cli-core';
import type { UseHistoryManagerReturn } from './useHistoryManager.js';

Expand Down Expand Up @@ -45,7 +47,7 @@ export const useEditorSettings = (
(editorType: EditorType | undefined, scope: LoadableSettingScope) => {
if (
editorType &&
(!checkHasEditorType(editorType) ||
(!hasValidEditorCommand(editorType) ||
!allowEditorTypeInSandbox(editorType))
) {
return;
Expand All @@ -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}`);
}
Expand All @@ -75,6 +78,7 @@ export const useEditorSettings = (

const exitEditorDialog = useCallback(() => {
setIsEditorDialogOpen(false);
coreEvents.emit(CoreEvent.EditorSelected, { editor: undefined });
}, []);

return {
Expand Down
41 changes: 36 additions & 5 deletions packages/core/src/scheduler/confirmation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -155,7 +160,16 @@ export async function resolveConfirmation(
}

if (outcome === ToolConfirmationOutcome.ModifyWithEditor) {
await handleExternalModification(deps, toolCall, signal);
const modResult = await handleExternalModification(
deps,
toolCall,
signal,
);
// 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;
Expand All @@ -182,8 +196,18 @@ async function notifyHooks(
}
}

/**
* Result of attempting external modification.
* If error is defined, the modification failed.
*/
interface ExternalModificationResult {
/** 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: {
Expand All @@ -193,10 +217,16 @@ 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 { error: NO_EDITOR_AVAILABLE_ERROR };
}

const result = await modifier.handleModifyWithEditor(
state.firstActiveCall as WaitingToolCall,
Expand All @@ -211,6 +241,7 @@ async function handleExternalModification(
newInvocation,
);
}
return {};
}

/**
Expand Down
Loading
Loading