mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-22 20:14:58 -07:00
fix(core): resolve infinite loop and improve editor selection flow
- Fixes an infinite loop when using 'Modify with Editor' without a configured editor. - Implements interactive editor selection via a UI dialog. - Returns to the previous confirmation prompt if selection is cancelled or fails. - Simplifies editor availability logic and removes deprecated sync functions. Fixes #7669
This commit is contained in:
ehedlund
@@ -21,12 +21,10 @@ import {
|
||||
allowEditorTypeInSandbox,
|
||||
isEditorAvailable,
|
||||
isEditorAvailableAsync,
|
||||
detectFirstAvailableEditor,
|
||||
detectFirstAvailableEditorAsync,
|
||||
resolveEditor,
|
||||
resolveEditorAsync,
|
||||
type EditorType,
|
||||
} from './editor.js';
|
||||
import { coreEvents, CoreEvent } from './events.js';
|
||||
import { exec, execSync, spawn, spawnSync } from 'node:child_process';
|
||||
import { debugLogger } from './debugLogger.js';
|
||||
|
||||
@@ -550,132 +548,6 @@ describe('editor utils', () => {
|
||||
});
|
||||
});
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
// Helper to create a mock exec that simulates async behavior
|
||||
const mockExecAsync = (implementation: (cmd: string) => boolean): void => {
|
||||
(exec as unknown as Mock).mockImplementation(
|
||||
@@ -749,86 +621,93 @@ describe('editor utils', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('detectFirstAvailableEditorAsync', () => {
|
||||
it('should return undefined when no editors are installed', async () => {
|
||||
mockExecAsync(() => false);
|
||||
vi.stubEnv('SANDBOX', '');
|
||||
expect(await detectFirstAvailableEditorAsync()).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should prioritize terminal editors over GUI editors', async () => {
|
||||
mockExecAsync(
|
||||
(cmd) =>
|
||||
(cmd.includes('vim') && !cmd.includes('nvim')) ||
|
||||
cmd.includes('code'),
|
||||
);
|
||||
vi.stubEnv('SANDBOX', '');
|
||||
expect(await detectFirstAvailableEditorAsync()).toBe('vim');
|
||||
});
|
||||
|
||||
it('should return vim in sandbox mode', async () => {
|
||||
mockExecAsync((cmd) => cmd.includes('vim') && !cmd.includes('nvim'));
|
||||
vi.stubEnv('SANDBOX', 'sandbox');
|
||||
expect(await detectFirstAvailableEditorAsync()).toBe('vim');
|
||||
});
|
||||
|
||||
it('should skip GUI editors in sandbox mode', async () => {
|
||||
mockExecAsync((cmd) => cmd.includes('code'));
|
||||
vi.stubEnv('SANDBOX', 'sandbox');
|
||||
expect(await detectFirstAvailableEditorAsync()).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
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.editor).toBe('vim');
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(result).toBe('vim');
|
||||
});
|
||||
|
||||
it('should return error when preferred editor is not installed', async () => {
|
||||
it('should request editor selection when preferred editor is not installed', async () => {
|
||||
mockExecAsync(() => false);
|
||||
vi.stubEnv('SANDBOX', '');
|
||||
const result = await resolveEditorAsync('vim');
|
||||
expect(result.editor).toBeUndefined();
|
||||
expect(result.error).toContain('Vim');
|
||||
expect(result.error).toContain('not installed');
|
||||
const resolvePromise = resolveEditorAsync('vim');
|
||||
setTimeout(
|
||||
() => coreEvents.emit(CoreEvent.EditorSelected, { editor: 'neovim' }),
|
||||
0,
|
||||
);
|
||||
const result = await resolvePromise;
|
||||
expect(result).toBe('neovim');
|
||||
});
|
||||
|
||||
it('should return error when preferred GUI editor cannot be used in sandbox mode', async () => {
|
||||
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 result = await resolveEditorAsync('vscode');
|
||||
expect(result.editor).toBeUndefined();
|
||||
expect(result.error).toContain('VS Code');
|
||||
expect(result.error).toContain('sandbox mode');
|
||||
const resolvePromise = resolveEditorAsync('vscode');
|
||||
setTimeout(
|
||||
() => coreEvents.emit(CoreEvent.EditorSelected, { editor: 'vim' }),
|
||||
0,
|
||||
);
|
||||
const result = await resolvePromise;
|
||||
expect(result).toBe('vim');
|
||||
});
|
||||
|
||||
it('should auto-detect editor when no preference is set', async () => {
|
||||
mockExecAsync((cmd) => cmd.includes('vim') && !cmd.includes('nvim'));
|
||||
it('should request editor selection when no preference is set', async () => {
|
||||
const emitSpy = vi.spyOn(coreEvents, 'emit');
|
||||
vi.stubEnv('SANDBOX', '');
|
||||
const result = await resolveEditorAsync(undefined);
|
||||
expect(result.editor).toBe('vim');
|
||||
expect(result.error).toBeUndefined();
|
||||
|
||||
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 error when no preference is set and no editors are available', async () => {
|
||||
mockExecAsync(() => false);
|
||||
vi.stubEnv('SANDBOX', '');
|
||||
const result = await resolveEditorAsync(undefined);
|
||||
expect(result.editor).toBeUndefined();
|
||||
expect(result.error).toContain('No external editor');
|
||||
expect(result.error).toContain('/editor');
|
||||
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 work with terminal editors in sandbox mode when no preference is set', async () => {
|
||||
mockExecAsync((cmd) => cmd.includes('vim') && !cmd.includes('nvim'));
|
||||
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 result = await resolveEditorAsync(undefined);
|
||||
expect(result.editor).toBe('vim');
|
||||
expect(result.error).toBeUndefined();
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user