Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
18 changes: 14 additions & 4 deletions packages/core/src/core/coreToolScheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import {
type ToolConfirmationPayload,
ToolConfirmationOutcome,
} from '../tools/tools.js';
import type { EditorType } from '../utils/editor.js';
import { resolveEditor, type EditorType } from '../utils/editor.js';
import { coreEvents } from '../utils/events.js';
import type { Config } from '../config/config.js';
import { PolicyDecision, ApprovalMode } from '../policy/types.js';
import { logToolCall } from '../telemetry/loggers.js';
Expand Down Expand Up @@ -758,8 +759,17 @@ export class CoreToolScheduler {
} else if (outcome === ToolConfirmationOutcome.ModifyWithEditor) {
const waitingToolCall = toolCall as WaitingToolCall;

const editorType = this.getPreferredEditor();
if (!editorType) {
// Use resolveEditor to check availability and auto-detect if needed
const preferredEditor = this.getPreferredEditor();
const resolution = resolveEditor(preferredEditor);

if (!resolution.editor) {
// No editor available - emit error feedback and cancel the operation
// This fixes the infinite loop issue reported in #7669
if (resolution.error) {
coreEvents.emitFeedback('error', resolution.error);
}
this.cancelAll(signal);
return;
}

Expand All @@ -770,7 +780,7 @@ export class CoreToolScheduler {

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

Expand Down
54 changes: 48 additions & 6 deletions packages/core/src/scheduler/confirmation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ 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 { resolveEditor, type EditorType } from '../utils/editor.js';
import type { DiffUpdateResult } from '../ide/ide-client.js';
import { fireToolNotificationHook } from '../core/coreToolHookTriggers.js';
import { debugLogger } from '../utils/debugLogger.js';
import { coreEvents } from '../utils/events.js';

export interface ConfirmationResult {
outcome: ToolConfirmationOutcome;
Expand Down Expand Up @@ -151,7 +152,21 @@ export async function resolveConfirmation(
outcome = response.outcome;

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 break the loop
// by cancelling the operation to prevent infinite loop
if (modResult.error) {
coreEvents.emitFeedback('error', modResult.error);
}
// Break the loop by changing outcome to Cancel
// This prevents the infinite loop issue reported in #7669
outcome = ToolConfirmationOutcome.Cancel;
}
} else if (response.payload?.newContent) {
await handleInlineModification(deps, toolCall, response.payload, signal);
outcome = ToolConfirmationOutcome.ProceedOnce;
Expand All @@ -178,8 +193,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 @@ -189,14 +215,29 @@ async function handleExternalModification(
},
toolCall: ValidatingToolCall,
signal: AbortSignal,
): Promise<void> {
): Promise<ExternalModificationResult> {
const { state, modifier, getPreferredEditor } = deps;
const editor = getPreferredEditor();
if (!editor) return;

// Use the new resolveEditor function which handles:
// 1. Checking if preferred editor is available
// 2. Auto-detecting an available editor if none is configured
// 3. Providing helpful error messages
const preferredEditor = getPreferredEditor();
const resolution = resolveEditor(preferredEditor);

if (!resolution.editor) {
// No editor available - return failure with error message
return {
success: false,
error:
resolution.error ||
'No external editor is available. Please run /editor to configure one.',
};
}

const result = await modifier.handleModifyWithEditor(
state.firstActiveCall as WaitingToolCall,
editor,
resolution.editor,
signal,
);
if (result) {
Expand All @@ -207,6 +248,7 @@ async function handleExternalModification(
newInvocation,
);
}
return { success: true };
}

/**
Expand Down
128 changes: 128 additions & 0 deletions packages/core/src/utils/editor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
openDiff,
allowEditorTypeInSandbox,
isEditorAvailable,
detectFirstAvailableEditor,
resolveEditor,
type EditorType,
} from './editor.js';
import { execSync, spawn, spawnSync } from 'node:child_process';
Expand Down Expand Up @@ -542,4 +544,130 @@ describe('editor utils', () => {
expect(isEditorAvailable('neovim')).toBe(true);
});
});

describe('detectFirstAvailableEditor', () => {
it('should return undefined when no editors are installed', () => {
(execSync as Mock).mockImplementation(() => {
throw new Error('Command not found');
});
vi.stubEnv('SANDBOX', '');
expect(detectFirstAvailableEditor()).toBeUndefined();
});

it('should prioritize terminal editors over GUI editors', () => {
// Mock vim as available
(execSync as Mock).mockImplementation((cmd: string) => {
if (cmd.includes('vim') && !cmd.includes('nvim')) {
return Buffer.from('/usr/bin/vim');
}
if (cmd.includes('code')) {
return Buffer.from('/usr/bin/code');
}
throw new Error('Command not found');
});
vi.stubEnv('SANDBOX', '');
expect(detectFirstAvailableEditor()).toBe('vim');
});

it('should return vim when vim is the only editor available in sandbox mode', () => {
(execSync as Mock).mockImplementation((cmd: string) => {
if (cmd.includes('vim') && !cmd.includes('nvim')) {
return Buffer.from('/usr/bin/vim');
}
throw new Error('Command not found');
});
vi.stubEnv('SANDBOX', 'sandbox');
expect(detectFirstAvailableEditor()).toBe('vim');
});

it('should skip GUI editors in sandbox mode', () => {
(execSync as Mock).mockImplementation((cmd: string) => {
if (cmd.includes('code')) {
return Buffer.from('/usr/bin/code');
}
throw new Error('Command not found');
});
vi.stubEnv('SANDBOX', 'sandbox');
// vscode is installed but not allowed in sandbox
expect(detectFirstAvailableEditor()).toBeUndefined();
});

it('should return first available terminal editor (neovim)', () => {
(execSync as Mock).mockImplementation((cmd: string) => {
if (cmd.includes('nvim')) {
return Buffer.from('/usr/bin/nvim');
}
throw new Error('Command not found');
});
vi.stubEnv('SANDBOX', '');
expect(detectFirstAvailableEditor()).toBe('neovim');
});
});

describe('resolveEditor', () => {
it('should return the preferred editor when available', () => {
(execSync as Mock).mockReturnValue(Buffer.from('/usr/bin/vim'));
vi.stubEnv('SANDBOX', '');
const result = resolveEditor('vim');
expect(result.editor).toBe('vim');
expect(result.error).toBeUndefined();
});

it('should return error when preferred editor is not installed', () => {
(execSync as Mock).mockImplementation(() => {
throw new Error('Command not found');
});
vi.stubEnv('SANDBOX', '');
const result = resolveEditor('vim');
expect(result.editor).toBeUndefined();
expect(result.error).toContain('Vim');
expect(result.error).toContain('not installed');
});

it('should return error when preferred GUI editor cannot be used in sandbox mode', () => {
(execSync as Mock).mockReturnValue(Buffer.from('/usr/bin/code'));
vi.stubEnv('SANDBOX', 'sandbox');
const result = resolveEditor('vscode');
expect(result.editor).toBeUndefined();
expect(result.error).toContain('VS Code');
expect(result.error).toContain('sandbox mode');
});

it('should auto-detect editor when no preference is set', () => {
(execSync as Mock).mockImplementation((cmd: string) => {
if (cmd.includes('vim') && !cmd.includes('nvim')) {
return Buffer.from('/usr/bin/vim');
}
throw new Error('Command not found');
});
vi.stubEnv('SANDBOX', '');
const result = resolveEditor(undefined);
expect(result.editor).toBe('vim');
expect(result.error).toBeUndefined();
});

it('should return error when no preference is set and no editors are available', () => {
(execSync as Mock).mockImplementation(() => {
throw new Error('Command not found');
});
vi.stubEnv('SANDBOX', '');
const result = resolveEditor(undefined);
expect(result.editor).toBeUndefined();
expect(result.error).toContain('No external editor');
expect(result.error).toContain('/editor');
});

it('should work with terminal editors in sandbox mode when no preference is set', () => {
(execSync as Mock).mockImplementation((cmd: string) => {
if (cmd.includes('vim') && !cmd.includes('nvim')) {
return Buffer.from('/usr/bin/vim');
}
throw new Error('Command not found');
});
vi.stubEnv('SANDBOX', 'sandbox');
const result = resolveEditor(undefined);
expect(result.editor).toBe('vim');
expect(result.error).toBeUndefined();
});
});
});
73 changes: 73 additions & 0 deletions packages/core/src/utils/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,79 @@ export function isEditorAvailable(editor: string | undefined): boolean {
return false;
}

/**
* Detects the first available editor from the supported list.
* Prioritizes terminal editors (vim, neovim, emacs, hx) as they work in all environments
* including sandboxed mode, then falls back to GUI editors.
* Returns undefined if no supported editor is found.
*/
export function detectFirstAvailableEditor(): EditorType | undefined {
// Prioritize terminal editors as they work in sandbox mode
for (const editor of TERMINAL_EDITORS) {
if (isEditorAvailable(editor)) {
return editor;
}
}
// Fall back to GUI editors (won't work in sandbox mode but checked above)
for (const editor of GUI_EDITORS) {
if (isEditorAvailable(editor)) {
return editor;
}
}
return undefined;
}

/**
* Result of attempting to resolve an editor for use.
*/
export interface EditorResolutionResult {
/** The editor to use, if available */
editor?: EditorType;
/** Error message if no editor is available */
error?: string;
}

/**
* Resolves an editor to use for external editing.
* 1. If a preferred editor is set and available, uses it.
* 2. If a preferred editor is set but not available, returns an error.
* 3. If no preferred editor is set, attempts to auto-detect an available editor.
* 4. If no editor can be found, returns an error with instructions.
*/
export function resolveEditor(
preferredEditor: EditorType | undefined,
): EditorResolutionResult {
// Case 1: Preferred editor is set
if (preferredEditor) {
if (isEditorAvailable(preferredEditor)) {
return { editor: preferredEditor };
}
// Preferred editor is set but not available
const displayName = getEditorDisplayName(preferredEditor);
if (!checkHasEditorType(preferredEditor)) {
return {
error: `${displayName} is configured as your preferred editor but is not installed. Please install it or run /editor to choose a different editor.`,
};
}
// If the editor is installed but not available, it must be due to sandbox restrictions.
return {
error: `${displayName} cannot be used in sandbox mode. Please run /editor to choose a terminal-based editor (vim, neovim, emacs, or helix).`,
};
}

// Case 2: No preferred editor set, try to auto-detect
const detectedEditor = detectFirstAvailableEditor();
if (detectedEditor) {
return { editor: detectedEditor };
}

// Case 3: No editor available at all
return {
error:
'No external editor is configured or available. Please run /editor to set your preferred editor, or install one of the supported editors: vim, neovim, emacs, helix, VS Code, Cursor, Zed, or Windsurf.',
};
}

/**
* Get the diff command for a specific editor.
*/
Expand Down