refactor(editor): use const assertion for editor types with single source of truth (#8604)

This commit is contained in:
CHAEWAN KIM
2025-12-03 15:16:19 +09:00
committed by GitHub
parent b9b3b8050d
commit 0857345945
3 changed files with 58 additions and 44 deletions

View File

@@ -15,6 +15,7 @@ import {
isNodeError,
parseAndFormatApiError,
safeLiteralReplace,
DEFAULT_GUI_EDITOR,
type AnyDeclarativeTool,
type ToolCall,
type ToolConfirmationPayload,
@@ -435,7 +436,7 @@ export class Task {
outputUpdateHandler: this._schedulerOutputUpdate.bind(this),
onAllToolCallsComplete: this._schedulerAllToolCallsComplete.bind(this),
onToolCallsUpdate: this._schedulerToolCallsUpdate.bind(this),
getPreferredEditor: () => 'vscode',
getPreferredEditor: () => DEFAULT_GUI_EDITOR,
config: this.config,
});
return scheduler;

View File

@@ -13,7 +13,7 @@ import {
modifyWithEditor,
isModifiableDeclarativeTool,
} from './modifiable-tool.js';
import type { EditorType } from '../utils/editor.js';
import { DEFAULT_GUI_EDITOR } from '../utils/editor.js';
import fs from 'node:fs';
import fsp from 'node:fs/promises';
import os from 'node:os';
@@ -24,9 +24,13 @@ import { debugLogger } from '../utils/debugLogger.js';
const mockOpenDiff = vi.hoisted(() => vi.fn());
const mockCreatePatch = vi.hoisted(() => vi.fn());
vi.mock('../utils/editor.js', () => ({
openDiff: mockOpenDiff,
}));
vi.mock('../utils/editor.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('../utils/editor.js')>();
return {
...actual,
openDiff: mockOpenDiff,
};
});
vi.mock('diff', () => ({
createPatch: mockCreatePatch,
@@ -103,7 +107,7 @@ describe('modifyWithEditor', () => {
const result = await modifyWithEditor(
mockParams,
mockModifyContext,
'vscode' as EditorType,
DEFAULT_GUI_EDITOR,
abortSignal,
);
@@ -169,9 +173,14 @@ describe('modifyWithEditor', () => {
await modifyWithEditor(
mockParams,
mockModifyContext,
'vscode' as EditorType,
DEFAULT_GUI_EDITOR,
abortSignal,
);
const [oldFilePath] = mockOpenDiff.mock.calls[0];
const diffDir = path.dirname(oldFilePath);
// Temp directory should be cleaned up after modification
await expect(fsp.stat(diffDir)).rejects.toThrow();
});
});
@@ -184,7 +193,7 @@ describe('modifyWithEditor', () => {
const result = await modifyWithEditor(
mockParams,
mockModifyContext,
'vscode' as EditorType,
DEFAULT_GUI_EDITOR,
abortSignal,
);
@@ -212,7 +221,7 @@ describe('modifyWithEditor', () => {
const result = await modifyWithEditor(
mockParams,
mockModifyContext,
'vscode' as EditorType,
DEFAULT_GUI_EDITOR,
abortSignal,
);
@@ -241,7 +250,7 @@ describe('modifyWithEditor', () => {
await modifyWithEditor(
mockParams,
mockModifyContext,
'vscode' as EditorType,
DEFAULT_GUI_EDITOR,
abortSignal,
{
currentContent: overrideCurrent,
@@ -268,7 +277,7 @@ describe('modifyWithEditor', () => {
await modifyWithEditor(
mockParams,
mockModifyContext,
'vscode' as EditorType,
DEFAULT_GUI_EDITOR,
abortSignal,
{
currentContent: null,
@@ -298,7 +307,7 @@ describe('modifyWithEditor', () => {
modifyWithEditor(
mockParams,
mockModifyContext,
'vscode' as EditorType,
DEFAULT_GUI_EDITOR,
abortSignal,
),
).rejects.toThrow('Editor failed to open');
@@ -327,7 +336,7 @@ describe('modifyWithEditor', () => {
await modifyWithEditor(
mockParams,
mockModifyContext,
'vscode' as EditorType,
DEFAULT_GUI_EDITOR,
abortSignal,
);
@@ -353,7 +362,7 @@ describe('modifyWithEditor', () => {
await modifyWithEditor(
mockParams,
mockModifyContext,
'vscode' as EditorType,
DEFAULT_GUI_EDITOR,
abortSignal,
);
@@ -374,7 +383,7 @@ describe('modifyWithEditor', () => {
await modifyWithEditor(
mockParams,
mockModifyContext,
'vscode' as EditorType,
DEFAULT_GUI_EDITOR,
abortSignal,
);

View File

@@ -8,16 +8,36 @@ import { execSync, spawn, spawnSync } from 'node:child_process';
import { debugLogger } from './debugLogger.js';
import { coreEvents, CoreEvent } from './events.js';
export type EditorType =
| 'vscode'
| 'vscodium'
| 'windsurf'
| 'cursor'
| 'vim'
| 'neovim'
| 'zed'
| 'emacs'
| 'antigravity';
const GUI_EDITORS = [
'vscode',
'vscodium',
'windsurf',
'cursor',
'zed',
'antigravity',
] as const;
const TERMINAL_EDITORS = ['vim', 'neovim', 'emacs'] as const;
const EDITORS = [...GUI_EDITORS, ...TERMINAL_EDITORS] as const;
const GUI_EDITORS_SET = new Set<string>(GUI_EDITORS);
const TERMINAL_EDITORS_SET = new Set<string>(TERMINAL_EDITORS);
const EDITORS_SET = new Set<string>(EDITORS);
export const DEFAULT_GUI_EDITOR: GuiEditorType = 'vscode';
export type GuiEditorType = (typeof GUI_EDITORS)[number];
export type TerminalEditorType = (typeof TERMINAL_EDITORS)[number];
export type EditorType = (typeof EDITORS)[number];
export function isGuiEditor(editor: EditorType): editor is GuiEditorType {
return GUI_EDITORS_SET.has(editor);
}
export function isTerminalEditor(
editor: EditorType,
): editor is TerminalEditorType {
return TERMINAL_EDITORS_SET.has(editor);
}
export const EDITOR_DISPLAY_NAMES: Record<EditorType, string> = {
vscode: 'VS Code',
@@ -36,17 +56,7 @@ export function getEditorDisplayName(editor: EditorType): string {
}
function isValidEditorType(editor: string): editor is EditorType {
return [
'vscode',
'vscodium',
'windsurf',
'cursor',
'vim',
'neovim',
'zed',
'emacs',
'antigravity',
].includes(editor);
return EDITORS_SET.has(editor);
}
interface DiffCommand {
@@ -94,11 +104,7 @@ export function checkHasEditorType(editor: EditorType): boolean {
export function allowEditorTypeInSandbox(editor: EditorType): boolean {
const notUsingSandbox = !process.env['SANDBOX'];
if (
['vscode', 'vscodium', 'windsurf', 'cursor', 'zed', 'antigravity'].includes(
editor,
)
) {
if (isGuiEditor(editor)) {
return notUsingSandbox;
}
// For terminal-based editors like vim and emacs, allow in sandbox.
@@ -197,9 +203,7 @@ export async function openDiff(
return;
}
const isTerminalEditor = ['vim', 'emacs', 'neovim'].includes(editor);
if (isTerminalEditor) {
if (isTerminalEditor(editor)) {
try {
const result = spawnSync(diffCommand.command, diffCommand.args, {
stdio: 'inherit',