Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 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 @@ -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 (
Expand All @@ -543,6 +553,9 @@ export const AppContainer = (props: AppContainerProps) => {
}
}, [bannerVisible, bannerText, settings, config, refreshStatic]);

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

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

Expand Down
4 changes: 4 additions & 0 deletions packages/cli/src/ui/hooks/useEditorSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
allowEditorTypeInSandbox,
checkHasEditorType,
getEditorDisplayName,
coreEvents,
CoreEvent,
} from '@google/gemini-cli-core';
import type { UseHistoryManagerReturn } from './useHistoryManager.js';

Expand Down Expand Up @@ -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
6 changes: 6 additions & 0 deletions packages/core/src/core/coreToolScheduler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
MOCK_TOOL_SHOULD_CONFIRM_EXECUTE,
} from '../test-utils/mock-tool.js';
import * as modifiableToolModule from '../tools/modifiable-tool.js';
import * as editorModule from '../utils/editor.js';
import { DEFAULT_GEMINI_MODEL } from '../config/models.js';
import type { PolicyEngine } from '../policy/policy-engine.js';
import { DiscoveredMCPTool } from '../tools/mcp-tool.js';
Expand Down Expand Up @@ -1762,6 +1763,10 @@ describe('CoreToolScheduler Sequential Execution', () => {
});

it('should pass confirmation diff data into modifyWithEditor overrides', async () => {
const resolveEditorAsyncSpy = vi
.spyOn(editorModule, 'resolveEditorAsync')
.mockResolvedValue('vscode');

const modifyWithEditorSpy = vi
.spyOn(modifiableToolModule, 'modifyWithEditor')
.mockResolvedValue({
Expand Down Expand Up @@ -1842,6 +1847,7 @@ describe('CoreToolScheduler Sequential Execution', () => {
});

modifyWithEditorSpy.mockRestore();
resolveEditorAsyncSpy.mockRestore();
});

it('should handle inline modify with empty new content', async () => {
Expand Down
21 changes: 17 additions & 4 deletions packages/core/src/core/coreToolScheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ import {
type ToolConfirmationPayload,
ToolConfirmationOutcome,
} from '../tools/tools.js';
import type { EditorType } from '../utils/editor.js';
import {
resolveEditorAsync,
type EditorType,
NO_EDITOR_AVAILABLE_ERROR,
} from '../utils/editor.js';
import { coreEvents } from '../utils/events.js';
import type { Config } from '../config/config.js';
import { PolicyDecision } from '../policy/types.js';
import { logToolCall } from '../telemetry/loggers.js';
Expand Down Expand Up @@ -751,8 +756,16 @@ export class CoreToolScheduler {
} else if (outcome === ToolConfirmationOutcome.ModifyWithEditor) {
const waitingToolCall = toolCall as WaitingToolCall;

const editorType = this.getPreferredEditor();
if (!editorType) {
const preferredEditor = this.getPreferredEditor();
const editor = await resolveEditorAsync(preferredEditor, signal);

if (!editor) {
// No editor available - emit error feedback and return to previous confirmation screen
coreEvents.emitFeedback('error', NO_EDITOR_AVAILABLE_ERROR);
this.setStatusInternal(callId, 'awaiting_approval', signal, {
...waitingToolCall.confirmationDetails,
isModifying: false,
} as ToolCallConfirmationDetails);
return;
}

Expand All @@ -763,7 +776,7 @@ export class CoreToolScheduler {

const result = await this.toolModifier.handleModifyWithEditor(
waitingToolCall,
editorType,
editor,
signal,
);

Expand Down
47 changes: 42 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,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;
Expand All @@ -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: {
Expand All @@ -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,
Expand All @@ -211,6 +247,7 @@ async function handleExternalModification(
newInvocation,
);
}
return { success: true };
}

/**
Expand Down
Loading
Loading