feat: External editor settings (#882)

7 files changed, 153 insertions, 46 deletions

diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts
index 1e8b2b2a..d9d30f94 100644
--- a/packages/core/src/core/coreToolScheduler.test.ts
+++ b/packages/core/src/core/coreToolScheduler.test.ts
@@ -62,7 +62,6 @@ describe('CoreToolScheduler', () => {
       getFunctionDeclarations: () => [],
       tools: new Map(),
       discovery: {} as any,
-      config: {} as any,
       registerTool: () => {},
       getToolByName: () => mockTool,
       getToolByDisplayName: () => mockTool,
@@ -79,6 +78,7 @@ describe('CoreToolScheduler', () => {
       toolRegistry: Promise.resolve(toolRegistry as any),
       onAllToolCallsComplete,
       onToolCallsUpdate,
+      getPreferredEditor: () => 'vscode',
     });
 
     const abortController = new AbortController();
diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts
index fe409fb1..f8688aeb 100644
--- a/packages/core/src/core/coreToolScheduler.ts
+++ b/packages/core/src/core/coreToolScheduler.ts
@@ -15,6 +15,7 @@ import {
   ApprovalMode,
   EditTool,
   EditToolParams,
+  EditorType,
 } from '../index.js';
 import { Part, PartListUnion } from '@google/genai';
 import { getResponseTextFromParts } from '../utils/generateContentResponseUtilities.js';
@@ -203,6 +204,7 @@ interface CoreToolSchedulerOptions {
   onAllToolCallsComplete?: AllToolCallsCompleteHandler;
   onToolCallsUpdate?: ToolCallsUpdateHandler;
   approvalMode?: ApprovalMode;
+  getPreferredEditor: () => EditorType | undefined;
 }
 
 export class CoreToolScheduler {
@@ -212,6 +214,7 @@ export class CoreToolScheduler {
   private onAllToolCallsComplete?: AllToolCallsCompleteHandler;
   private onToolCallsUpdate?: ToolCallsUpdateHandler;
   private approvalMode: ApprovalMode;
+  private getPreferredEditor: () => EditorType | undefined;
 
   constructor(options: CoreToolSchedulerOptions) {
     this.toolRegistry = options.toolRegistry;
@@ -219,6 +222,7 @@ export class CoreToolScheduler {
     this.onAllToolCallsComplete = options.onAllToolCallsComplete;
     this.onToolCallsUpdate = options.onToolCallsUpdate;
     this.approvalMode = options.approvalMode ?? ApprovalMode.DEFAULT;
+    this.getPreferredEditor = options.getPreferredEditor;
   }
 
   private setStatusInternal(
@@ -484,15 +488,15 @@ export class CoreToolScheduler {
         'cancelled',
         'User did not allow tool call',
       );
-    } else if (
-      outcome === ToolConfirmationOutcome.ModifyVSCode ||
-      outcome === ToolConfirmationOutcome.ModifyWindsurf ||
-      outcome === ToolConfirmationOutcome.ModifyCursor ||
-      outcome === ToolConfirmationOutcome.ModifyVim
-    ) {
+    } else if (outcome === ToolConfirmationOutcome.ModifyWithEditor) {
       const waitingToolCall = toolCall as WaitingToolCall;
       if (waitingToolCall?.confirmationDetails?.type === 'edit') {
         const editTool = waitingToolCall.tool as EditTool;
+        const editorType = this.getPreferredEditor();
+        if (!editorType) {
+          return;
+        }
+
         this.setStatusInternal(callId, 'awaiting_approval', {
           ...waitingToolCall.confirmationDetails,
           isModifying: true,
@@ -501,7 +505,7 @@ export class CoreToolScheduler {
         const modifyResults = await editTool.onModify(
           waitingToolCall.request.args as unknown as EditToolParams,
           signal,
-          outcome,
+          editorType,
         );
 
         if (modifyResults) {
diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts
index 8c351929..dd3b481d 100644
--- a/packages/core/src/tools/edit.test.ts
+++ b/packages/core/src/tools/edit.test.ts
@@ -32,7 +32,6 @@ import fs from 'fs';
 import os from 'os';
 import { ApprovalMode, Config } from '../config/config.js';
 import { Content, Part, SchemaUnion } from '@google/genai';
-import { ToolConfirmationOutcome } from './tools.js';
 
 describe('EditTool', () => {
   let tool: EditTool;
@@ -634,7 +633,7 @@ describe('EditTool', () => {
       const result = await tool.onModify(
         params,
         new AbortController().signal,
-        ToolConfirmationOutcome.ModifyVSCode,
+        'vscode',
       );
 
       expect(mockOpenDiff).toHaveBeenCalledTimes(1);
@@ -678,7 +677,7 @@ describe('EditTool', () => {
       const result = await tool.onModify(
         params,
         new AbortController().signal,
-        ToolConfirmationOutcome.ModifyVSCode,
+        'vscode',
       );
 
       expect(mockOpenDiff).toHaveBeenCalledTimes(1);
@@ -711,7 +710,7 @@ describe('EditTool', () => {
       const result1 = await tool.onModify(
         params,
         new AbortController().signal,
-        ToolConfirmationOutcome.ModifyVSCode,
+        'vscode',
       );
       const firstCall = mockOpenDiff.mock.calls[0];
       const firstOldPath = firstCall[0];
@@ -727,7 +726,7 @@ describe('EditTool', () => {
       const result2 = await tool.onModify(
         params,
         new AbortController().signal,
-        ToolConfirmationOutcome.ModifyVSCode,
+        'vscode',
       );
       const secondCall = mockOpenDiff.mock.calls[1];
       const secondOldPath = secondCall[0];
diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts
index a49b8d83..39352121 100644
--- a/packages/core/src/tools/edit.ts
+++ b/packages/core/src/tools/edit.ts
@@ -432,19 +432,6 @@ Expectation for required parameters:
     }
   }
 
-  async getEditor(outcome: ToolConfirmationOutcome): Promise<EditorType> {
-    switch (outcome) {
-      case ToolConfirmationOutcome.ModifyVSCode:
-        return 'vscode';
-      case ToolConfirmationOutcome.ModifyWindsurf:
-        return 'windsurf';
-      case ToolConfirmationOutcome.ModifyCursor:
-        return 'cursor';
-      default:
-        return 'vim';
-    }
-  }
-
   /**
    * Creates temp files for the current and proposed file contents and opens a diff tool.
    * When the diff tool is closed, the tool will check if the file has been modified and provide the updated params.
@@ -453,7 +440,7 @@ Expectation for required parameters:
   async onModify(
     params: EditToolParams,
     _abortSignal: AbortSignal,
-    outcome: ToolConfirmationOutcome,
+    editorType: EditorType,
   ): Promise<
     { updatedParams: EditToolParams; updatedDiff: string } | undefined
   > {
@@ -461,9 +448,7 @@ Expectation for required parameters:
     this.tempOldDiffPath = oldPath;
     this.tempNewDiffPath = newPath;
 
-    const editor = await this.getEditor(outcome);
-
-    await openDiff(this.tempOldDiffPath, this.tempNewDiffPath, editor);
+    await openDiff(this.tempOldDiffPath, this.tempNewDiffPath, editorType);
     return await this.getUpdatedParamsIfModified(params, _abortSignal);
   }
 
diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts
index e80047df..ced53995 100644
--- a/packages/core/src/tools/tools.ts
+++ b/packages/core/src/tools/tools.ts
@@ -232,9 +232,6 @@ export enum ToolConfirmationOutcome {
   ProceedAlways = 'proceed_always',
   ProceedAlwaysServer = 'proceed_always_server',
   ProceedAlwaysTool = 'proceed_always_tool',
-  ModifyVSCode = 'modify_vscode',
-  ModifyWindsurf = 'modify_windsurf',
-  ModifyCursor = 'modify_cursor',
-  ModifyVim = 'modify_vim',
+  ModifyWithEditor = 'modify_with_editor',
   Cancel = 'cancel',
 }
diff --git a/packages/core/src/utils/editor.test.ts b/packages/core/src/utils/editor.test.ts
index e7c2f55a..8bc7a49c 100644
--- a/packages/core/src/utils/editor.test.ts
+++ b/packages/core/src/utils/editor.test.ts
@@ -4,8 +4,22 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { vi, describe, it, expect, beforeEach, type Mock } from 'vitest';
-import { checkHasEditor, getDiffCommand, openDiff } from './editor.js';
+import {
+  vi,
+  describe,
+  it,
+  expect,
+  beforeEach,
+  afterEach,
+  type Mock,
+} from 'vitest';
+import {
+  checkHasEditorType,
+  getDiffCommand,
+  openDiff,
+  allowEditorTypeInSandbox,
+  isEditorAvailable,
+} from './editor.js';
 import { execSync, spawn } from 'child_process';
 
 vi.mock('child_process', () => ({
@@ -13,14 +27,14 @@ vi.mock('child_process', () => ({
   spawn: vi.fn(),
 }));
 
-describe('checkHasEditor', () => {
+describe('checkHasEditorType', () => {
   beforeEach(() => {
     vi.clearAllMocks();
   });
 
   it('should return true for vscode if "code" command exists', () => {
     (execSync as Mock).mockReturnValue(Buffer.from('/usr/bin/code'));
-    expect(checkHasEditor('vscode')).toBe(true);
+    expect(checkHasEditorType('vscode')).toBe(true);
     const expectedCommand =
       process.platform === 'win32' ? 'where.exe code.cmd' : 'command -v code';
     expect(execSync).toHaveBeenCalledWith(expectedCommand, {
@@ -32,12 +46,12 @@ describe('checkHasEditor', () => {
     (execSync as Mock).mockImplementation(() => {
       throw new Error();
     });
-    expect(checkHasEditor('vscode')).toBe(false);
+    expect(checkHasEditorType('vscode')).toBe(false);
   });
 
   it('should return true for windsurf if "windsurf" command exists', () => {
     (execSync as Mock).mockReturnValue(Buffer.from('/usr/bin/windsurf'));
-    expect(checkHasEditor('windsurf')).toBe(true);
+    expect(checkHasEditorType('windsurf')).toBe(true);
     expect(execSync).toHaveBeenCalledWith('command -v windsurf', {
       stdio: 'ignore',
     });
@@ -47,12 +61,12 @@ describe('checkHasEditor', () => {
     (execSync as Mock).mockImplementation(() => {
       throw new Error();
     });
-    expect(checkHasEditor('windsurf')).toBe(false);
+    expect(checkHasEditorType('windsurf')).toBe(false);
   });
 
   it('should return true for cursor if "cursor" command exists', () => {
     (execSync as Mock).mockReturnValue(Buffer.from('/usr/bin/cursor'));
-    expect(checkHasEditor('cursor')).toBe(true);
+    expect(checkHasEditorType('cursor')).toBe(true);
     expect(execSync).toHaveBeenCalledWith('command -v cursor', {
       stdio: 'ignore',
     });
@@ -62,12 +76,12 @@ describe('checkHasEditor', () => {
     (execSync as Mock).mockImplementation(() => {
       throw new Error();
     });
-    expect(checkHasEditor('cursor')).toBe(false);
+    expect(checkHasEditorType('cursor')).toBe(false);
   });
 
   it('should return true for vim if "vim" command exists', () => {
     (execSync as Mock).mockReturnValue(Buffer.from('/usr/bin/vim'));
-    expect(checkHasEditor('vim')).toBe(true);
+    expect(checkHasEditorType('vim')).toBe(true);
     const expectedCommand =
       process.platform === 'win32' ? 'where.exe vim' : 'command -v vim';
     expect(execSync).toHaveBeenCalledWith(expectedCommand, {
@@ -79,7 +93,7 @@ describe('checkHasEditor', () => {
     (execSync as Mock).mockImplementation(() => {
       throw new Error();
     });
-    expect(checkHasEditor('vim')).toBe(false);
+    expect(checkHasEditorType('vim')).toBe(false);
   });
 });
 
@@ -153,3 +167,85 @@ describe('openDiff', () => {
     );
   });
 });
+
+describe('allowEditorTypeInSandbox', () => {
+  afterEach(() => {
+    delete process.env.SANDBOX;
+  });
+
+  it('should allow vim in sandbox mode', () => {
+    process.env.SANDBOX = 'sandbox';
+    expect(allowEditorTypeInSandbox('vim')).toBe(true);
+  });
+
+  it('should allow vim when not in sandbox mode', () => {
+    delete process.env.SANDBOX;
+    expect(allowEditorTypeInSandbox('vim')).toBe(true);
+  });
+
+  it('should not allow vscode in sandbox mode', () => {
+    process.env.SANDBOX = 'sandbox';
+    expect(allowEditorTypeInSandbox('vscode')).toBe(false);
+  });
+
+  it('should allow vscode when not in sandbox mode', () => {
+    delete process.env.SANDBOX;
+    expect(allowEditorTypeInSandbox('vscode')).toBe(true);
+  });
+
+  it('should not allow windsurf in sandbox mode', () => {
+    process.env.SANDBOX = 'sandbox';
+    expect(allowEditorTypeInSandbox('windsurf')).toBe(false);
+  });
+
+  it('should allow windsurf when not in sandbox mode', () => {
+    delete process.env.SANDBOX;
+    expect(allowEditorTypeInSandbox('windsurf')).toBe(true);
+  });
+
+  it('should not allow cursor in sandbox mode', () => {
+    process.env.SANDBOX = 'sandbox';
+    expect(allowEditorTypeInSandbox('cursor')).toBe(false);
+  });
+
+  it('should allow cursor when not in sandbox mode', () => {
+    delete process.env.SANDBOX;
+    expect(allowEditorTypeInSandbox('cursor')).toBe(true);
+  });
+});
+
+describe('isEditorAvailable', () => {
+  afterEach(() => {
+    delete process.env.SANDBOX;
+  });
+
+  it('should return false for undefined editor', () => {
+    expect(isEditorAvailable(undefined)).toBe(false);
+  });
+
+  it('should return false for empty string editor', () => {
+    expect(isEditorAvailable('')).toBe(false);
+  });
+
+  it('should return false for invalid editor type', () => {
+    expect(isEditorAvailable('invalid-editor')).toBe(false);
+  });
+
+  it('should return true for vscode when installed and not in sandbox mode', () => {
+    (execSync as Mock).mockReturnValue(Buffer.from('/usr/bin/code'));
+    expect(isEditorAvailable('vscode')).toBe(true);
+  });
+
+  it('should return false for vscode when not installed and not in sandbox mode', () => {
+    (execSync as Mock).mockImplementation(() => {
+      throw new Error();
+    });
+    expect(isEditorAvailable('vscode')).toBe(false);
+  });
+
+  it('should return false for vscode when installed and in sandbox mode', () => {
+    (execSync as Mock).mockReturnValue(Buffer.from('/usr/bin/code'));
+    process.env.SANDBOX = 'sandbox';
+    expect(isEditorAvailable('vscode')).toBe(false);
+  });
+});
diff --git a/packages/core/src/utils/editor.ts b/packages/core/src/utils/editor.ts
index 4d09e3a9..15c970d0 100644
--- a/packages/core/src/utils/editor.ts
+++ b/packages/core/src/utils/editor.ts
@@ -8,6 +8,10 @@ import { execSync, spawn } from 'child_process';
 
 export type EditorType = 'vscode' | 'windsurf' | 'cursor' | 'vim';
 
+function isValidEditorType(editor: string): editor is EditorType {
+  return ['vscode', 'windsurf', 'cursor', 'vim'].includes(editor);
+}
+
 interface DiffCommand {
   command: string;
   args: string[];
@@ -32,13 +36,35 @@ const editorCommands: Record<EditorType, { win32: string; default: string }> = {
   vim: { win32: 'vim', default: 'vim' },
 };
 
-export function checkHasEditor(editor: EditorType): boolean {
+export function checkHasEditorType(editor: EditorType): boolean {
   const commandConfig = editorCommands[editor];
   const command =
     process.platform === 'win32' ? commandConfig.win32 : commandConfig.default;
   return commandExists(command);
 }
 
+export function allowEditorTypeInSandbox(editor: EditorType): boolean {
+  const notUsingSandbox = !process.env.SANDBOX;
+  if (['vscode', 'windsurf', 'cursor'].includes(editor)) {
+    return notUsingSandbox;
+  }
+  return true;
+}
+
+/**
+ * Check if the editor is valid and can be used.
+ * Returns false if preferred editor is not set / invalid / not available / not allowed in sandbox.
+ */
+export function isEditorAvailable(editor: string | undefined): boolean {
+  if (editor && isValidEditorType(editor)) {
+    return (
+      checkHasEditorType(editor as EditorType) &&
+      allowEditorTypeInSandbox(editor as EditorType)
+    );
+  }
+  return false;
+}
+
 /**
  * Get the diff command for a specific editor.
  */