mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-18 01:51:20 -07:00
refactor(editor): use async functions to avoid blocking event loop
Replace synchronous execSync calls with async alternatives in editor detection functions to prevent blocking the Node.js event loop. Changes: - Add commandExistsAsync using promisified exec - Add checkHasEditorTypeAsync, isEditorAvailableAsync, detectFirstAvailableEditorAsync, and resolveEditorAsync - Update confirmation.ts and coreToolScheduler.ts to use resolveEditorAsync - Mark synchronous resolveEditor as deprecated - Add comprehensive tests for all async functions The synchronous versions are kept for UI components that require synchronous execution (useEditorSettings, editorSettingsManager).
This commit is contained in:
@@ -12,7 +12,7 @@ import {
|
||||
type ToolConfirmationPayload,
|
||||
ToolConfirmationOutcome,
|
||||
} from '../tools/tools.js';
|
||||
import { resolveEditor, type EditorType } from '../utils/editor.js';
|
||||
import { resolveEditorAsync, 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';
|
||||
@@ -759,9 +759,10 @@ export class CoreToolScheduler {
|
||||
} else if (outcome === ToolConfirmationOutcome.ModifyWithEditor) {
|
||||
const waitingToolCall = toolCall as WaitingToolCall;
|
||||
|
||||
// Use resolveEditor to check availability and auto-detect if needed
|
||||
// Use resolveEditorAsync to check availability and auto-detect if needed
|
||||
// Using async version to avoid blocking the event loop
|
||||
const preferredEditor = this.getPreferredEditor();
|
||||
const resolution = resolveEditor(preferredEditor);
|
||||
const resolution = await resolveEditorAsync(preferredEditor);
|
||||
|
||||
if (!resolution.editor) {
|
||||
// No editor available - emit error feedback and cancel the operation
|
||||
|
||||
@@ -21,7 +21,7 @@ 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 { resolveEditor, type EditorType } from '../utils/editor.js';
|
||||
import { resolveEditorAsync, 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';
|
||||
@@ -218,12 +218,13 @@ async function handleExternalModification(
|
||||
): Promise<ExternalModificationResult> {
|
||||
const { state, modifier, getPreferredEditor } = deps;
|
||||
|
||||
// Use the new resolveEditor function which handles:
|
||||
// Use the new resolveEditorAsync 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
|
||||
// Using async version to avoid blocking the event loop
|
||||
const preferredEditor = getPreferredEditor();
|
||||
const resolution = resolveEditor(preferredEditor);
|
||||
const resolution = await resolveEditorAsync(preferredEditor);
|
||||
|
||||
if (!resolution.editor) {
|
||||
// No editor available - return failure with error message
|
||||
|
||||
@@ -15,18 +15,23 @@ import {
|
||||
} from 'vitest';
|
||||
import {
|
||||
checkHasEditorType,
|
||||
checkHasEditorTypeAsync,
|
||||
getDiffCommand,
|
||||
openDiff,
|
||||
allowEditorTypeInSandbox,
|
||||
isEditorAvailable,
|
||||
isEditorAvailableAsync,
|
||||
detectFirstAvailableEditor,
|
||||
detectFirstAvailableEditorAsync,
|
||||
resolveEditor,
|
||||
resolveEditorAsync,
|
||||
type EditorType,
|
||||
} from './editor.js';
|
||||
import { execSync, spawn, spawnSync } from 'node:child_process';
|
||||
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 })),
|
||||
@@ -670,4 +675,160 @@ describe('editor utils', () => {
|
||||
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(
|
||||
(
|
||||
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('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();
|
||||
});
|
||||
|
||||
it('should return error 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');
|
||||
});
|
||||
|
||||
it('should return error 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');
|
||||
});
|
||||
|
||||
it('should auto-detect editor when no preference is set', async () => {
|
||||
mockExecAsync((cmd) => cmd.includes('vim') && !cmd.includes('nvim'));
|
||||
vi.stubEnv('SANDBOX', '');
|
||||
const result = await resolveEditorAsync(undefined);
|
||||
expect(result.editor).toBe('vim');
|
||||
expect(result.error).toBeUndefined();
|
||||
});
|
||||
|
||||
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 work with terminal editors in sandbox mode when no preference is set', async () => {
|
||||
mockExecAsync((cmd) => cmd.includes('vim') && !cmd.includes('nvim'));
|
||||
vi.stubEnv('SANDBOX', 'sandbox');
|
||||
const result = await resolveEditorAsync(undefined);
|
||||
expect(result.editor).toBe('vim');
|
||||
expect(result.error).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,7 +4,8 @@
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { execSync, spawn, spawnSync } from 'node:child_process';
|
||||
import { exec, execSync, spawn, spawnSync } from 'node:child_process';
|
||||
import { promisify } from 'node:util';
|
||||
import { debugLogger } from './debugLogger.js';
|
||||
import { coreEvents, CoreEvent } from './events.js';
|
||||
|
||||
@@ -73,6 +74,8 @@ interface DiffCommand {
|
||||
args: string[];
|
||||
}
|
||||
|
||||
const execAsync = promisify(exec);
|
||||
|
||||
function commandExists(cmd: string): boolean {
|
||||
try {
|
||||
execSync(
|
||||
@@ -85,6 +88,17 @@ function commandExists(cmd: string): boolean {
|
||||
}
|
||||
}
|
||||
|
||||
async function commandExistsAsync(cmd: string): Promise<boolean> {
|
||||
try {
|
||||
await execAsync(
|
||||
process.platform === 'win32' ? `where.exe ${cmd}` : `command -v ${cmd}`,
|
||||
);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Editor command configurations for different platforms.
|
||||
* Each editor can have multiple possible command names, listed in order of preference.
|
||||
@@ -112,6 +126,20 @@ export function checkHasEditorType(editor: EditorType): boolean {
|
||||
return commands.some((cmd) => commandExists(cmd));
|
||||
}
|
||||
|
||||
export async function checkHasEditorTypeAsync(
|
||||
editor: EditorType,
|
||||
): Promise<boolean> {
|
||||
const commandConfig = editorCommands[editor];
|
||||
const commands =
|
||||
process.platform === 'win32' ? commandConfig.win32 : commandConfig.default;
|
||||
for (const cmd of commands) {
|
||||
if (await commandExistsAsync(cmd)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
export function getEditorCommand(editor: EditorType): string {
|
||||
const commandConfig = editorCommands[editor];
|
||||
const commands =
|
||||
@@ -142,6 +170,23 @@ export function isEditorAvailable(editor: string | undefined): boolean {
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Async version of isEditorAvailable.
|
||||
* Check if the editor is valid and can be used without blocking the event loop.
|
||||
* Returns false if preferred editor is not set / invalid / not available / not allowed in sandbox.
|
||||
*/
|
||||
export async function isEditorAvailableAsync(
|
||||
editor: string | undefined,
|
||||
): Promise<boolean> {
|
||||
if (editor && isValidEditorType(editor)) {
|
||||
return (
|
||||
(await checkHasEditorTypeAsync(editor)) &&
|
||||
allowEditorTypeInSandbox(editor)
|
||||
);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Detects the first available editor from the supported list.
|
||||
* Prioritizes terminal editors (vim, neovim, emacs, hx) as they work in all environments
|
||||
@@ -164,6 +209,31 @@ export function detectFirstAvailableEditor(): EditorType | undefined {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Async version of detectFirstAvailableEditor.
|
||||
* Detects the first available editor from the supported list without blocking the event loop.
|
||||
* 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 async function detectFirstAvailableEditorAsync(): Promise<
|
||||
EditorType | undefined
|
||||
> {
|
||||
// Prioritize terminal editors as they work in sandbox mode
|
||||
for (const editor of TERMINAL_EDITORS) {
|
||||
if (await isEditorAvailableAsync(editor)) {
|
||||
return editor;
|
||||
}
|
||||
}
|
||||
// Fall back to GUI editors (won't work in sandbox mode but checked above)
|
||||
for (const editor of GUI_EDITORS) {
|
||||
if (await isEditorAvailableAsync(editor)) {
|
||||
return editor;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Result of attempting to resolve an editor for use.
|
||||
*/
|
||||
@@ -180,6 +250,8 @@ export interface EditorResolutionResult {
|
||||
* 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.
|
||||
*
|
||||
* @deprecated Use resolveEditorAsync instead to avoid blocking the event loop.
|
||||
*/
|
||||
export function resolveEditor(
|
||||
preferredEditor: EditorType | undefined,
|
||||
@@ -215,6 +287,48 @@ export function resolveEditor(
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Async version of resolveEditor.
|
||||
* Resolves an editor to use for external editing without blocking the event loop.
|
||||
* 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 async function resolveEditorAsync(
|
||||
preferredEditor: EditorType | undefined,
|
||||
): Promise<EditorResolutionResult> {
|
||||
// Case 1: Preferred editor is set
|
||||
if (preferredEditor) {
|
||||
if (await isEditorAvailableAsync(preferredEditor)) {
|
||||
return { editor: preferredEditor };
|
||||
}
|
||||
// Preferred editor is set but not available
|
||||
const displayName = getEditorDisplayName(preferredEditor);
|
||||
if (!(await checkHasEditorTypeAsync(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 = await detectFirstAvailableEditorAsync();
|
||||
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.
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user