test

packages/core/src/tools/exit-plan-mode.test.ts

@@ -240,6 +240,28 @@ Read and follow the plan strictly during implementation.`,
       expect(mockConfig.setApprovedPlanPath).toHaveBeenCalledWith(expectedPath);
     });
 
+    it('should include modified note when planModified is true', async () => {
+      const planRelativePath = createPlanFile('test.md', '# Content');
+      const invocation = tool.build({ plan_path: planRelativePath });
+
+      const confirmDetails = await invocation.shouldConfirmExecute(
+        new AbortController().signal,
+      );
+      expect(confirmDetails).not.toBe(false);
+      if (confirmDetails === false) return;
+
+      await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce, {
+        approved: true,
+        approvalMode: ApprovalMode.DEFAULT,
+        planModified: true,
+      });
+
+      const result = await invocation.execute(new AbortController().signal);
+      expect(result.llmContent).toContain(
+        'Note: The user modified the plan file in an external editor.',
+      );
+    });
+
     it('should return feedback message when plan is rejected with feedback', async () => {
       const planRelativePath = createPlanFile('test.md', '# Content');
       const invocation = tool.build({ plan_path: planRelativePath });

packages/core/src/tools/exit-plan-mode.ts

@@ -228,9 +228,12 @@ export class ExitPlanModeInvocation extends BaseToolInvocation<
       logPlanExecution(this.config, new PlanExecutionEvent(newMode));
 
       const description = getApprovalModeDescription(newMode);
+      const modifiedNote = payload.planModified
+        ? '\n\nNote: The user modified the plan file in an external editor.'
+        : '';
 
       return {
-        llmContent: `Plan approved. Switching to ${description}.
+        llmContent: `Plan approved.${modifiedNote} Switching to ${description}.
 
 The approved implementation plan is stored at: ${resolvedPlanPath}
 Read and follow the plan strictly during implementation.`,
@@ -238,9 +241,12 @@ Read and follow the plan strictly during implementation.`,
       };
     } else {
       const feedback = payload?.feedback?.trim();
+      const modifiedNote = payload?.planModified
+        ? ' (Note: The user also modified the plan file in an external editor)'
+        : '';
       if (feedback) {
         return {
-          llmContent: `Plan rejected. User feedback: ${feedback}
+          llmContent: `Plan rejected. User feedback: ${feedback}${modifiedNote}
 
 The plan is stored at: ${resolvedPlanPath}
 Revise the plan based on the feedback.`,
@@ -248,7 +254,7 @@ Revise the plan based on the feedback.`,
         };
       } else {
         return {
-          llmContent: `Plan rejected. No feedback provided.
+          llmContent: `Plan rejected. No feedback provided.${modifiedNote}
 
 The plan is stored at: ${resolvedPlanPath}
 Ask the user for specific feedback on how to improve the plan.`,

packages/core/src/tools/tools.ts

@@ -734,6 +734,8 @@ export interface ToolExitPlanModeConfirmationPayload {
   approvalMode?: ApprovalMode;
   /** If rejected, the user's feedback */
   feedback?: string;
+  /** Whether the user modified the plan file in an external editor */
+  planModified?: boolean;
 }
 
 export type ToolConfirmationPayload =

packages/core/src/utils/editor.test.ts

@@ -22,6 +22,7 @@ import {
   isEditorAvailable,
   isEditorAvailableAsync,
   resolveEditorAsync,
+  openFileInEditor,
   type EditorType,
 } from './editor.js';
 import { coreEvents, CoreEvent } from './events.js';
@@ -710,4 +711,86 @@ describe('editor utils', () => {
       expect(emitSpy).toHaveBeenCalledWith(CoreEvent.RequestEditorSelection);
     });
   });
+
+  describe('openFileInEditor', () => {
+    it('should return modified: true when no readTextFile is provided (legacy behavior)', async () => {
+      (spawnSync as Mock).mockReturnValue({ status: 0 });
+
+      const result = await openFileInEditor('test.txt');
+
+      expect(result).toEqual({ modified: true });
+    });
+
+    it('should return modified: true when content changed', async () => {
+      (spawnSync as Mock).mockReturnValue({ status: 0 });
+      const readTextFile = vi
+        .fn()
+        .mockResolvedValueOnce('initial content')
+        .mockResolvedValueOnce('changed content');
+
+      const result = await openFileInEditor('test.txt', { readTextFile });
+
+      expect(result).toEqual({ modified: true });
+      expect(readTextFile).toHaveBeenCalledTimes(2);
+    });
+
+    it('should return modified: false when content is same', async () => {
+      (spawnSync as Mock).mockReturnValue({ status: 0 });
+      const readTextFile = vi
+        .fn()
+        .mockResolvedValueOnce('initial content')
+        .mockResolvedValueOnce('initial content');
+
+      const result = await openFileInEditor('test.txt', { readTextFile });
+
+      expect(result).toEqual({ modified: false });
+      expect(readTextFile).toHaveBeenCalledTimes(2);
+    });
+
+    it("should return modified: true if file was created (didn't exist before)", async () => {
+      (spawnSync as Mock).mockReturnValue({ status: 0 });
+      const readTextFile = vi
+        .fn()
+        .mockRejectedValueOnce(new Error('ENOENT'))
+        .mockResolvedValueOnce('new content');
+
+      const result = await openFileInEditor('test.txt', { readTextFile });
+
+      expect(result).toEqual({ modified: true });
+      expect(readTextFile).toHaveBeenCalledTimes(2);
+    });
+
+    it('should return modified: true when using GUI spawn and content changed', async () => {
+      const mockChild = {
+        on: vi.fn((event, cb) => {
+          if (event === 'close') {
+            setTimeout(() => cb(0), 0);
+          }
+        }),
+      };
+      (spawn as Mock).mockReturnValue(mockChild);
+
+      const readTextFile = vi
+        .fn()
+        .mockResolvedValueOnce('initial content')
+        .mockResolvedValueOnce('changed content');
+
+      const result = await openFileInEditor('test.txt', {
+        readTextFile,
+        preferredEditor: 'vscode',
+      });
+
+      expect(result).toEqual({ modified: true });
+      expect(readTextFile).toHaveBeenCalledTimes(2);
+    });
+
+    it('should emit ExternalEditorClosed event', async () => {
+      (spawnSync as Mock).mockReturnValue({ status: 0 });
+      const emitSpy = vi.spyOn(coreEvents, 'emit');
+
+      await openFileInEditor('test.txt');
+
+      expect(emitSpy).toHaveBeenCalledWith(CoreEvent.ExternalEditorClosed);
+    });
+  });
 });

packages/core/src/utils/editor.ts

@@ -4,11 +4,30 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { exec, execSync, spawn, spawnSync } from 'node:child_process';
+import { exec, spawn, spawnSync, execSync } from 'node:child_process';
 import { promisify } from 'node:util';
 import { once } from 'node:events';
 import { debugLogger } from './debugLogger.js';
 import { coreEvents, CoreEvent, type EditorSelectedPayload } from './events.js';
+import type { DiffUpdateResult } from '../ide/ide-client.js';
+
+const execAsync = promisify(exec);
+
+/**
+ * Interface for an object that can open a diff in an IDE.
+ * Decouples editor utility from IdeClient implementation to avoid circular dependencies.
+ */
+export interface OpenFileIdeClient {
+  isDiffingEnabled(): boolean;
+  openDiff(filePath: string, content: string): Promise<DiffUpdateResult>;
+}
+
+export interface OpenFileInEditorOptions {
+  preferredEditor?: EditorType;
+  ideClient?: OpenFileIdeClient;
+  readTextFile?: (path: string) => Promise<string>;
+  writeTextFile?: (path: string, content: string) => Promise<void>;
+}
 
 const GUI_EDITORS = [
   'vscode',
@@ -61,7 +80,7 @@ export function getEditorDisplayName(editor: EditorType): string {
   return EDITOR_DISPLAY_NAMES[editor] || editor;
 }
 
-function isValidEditorType(editor: string): editor is EditorType {
+export function isValidEditorType(editor: string): editor is EditorType {
   return EDITORS_SET.has(editor);
 }
 
@@ -78,8 +97,6 @@ interface DiffCommand {
   args: string[];
 }
 
-const execAsync = promisify(exec);
-
 function getCommandExistsCmd(cmd: string): string {
   return process.platform === 'win32'
     ? `where.exe ${cmd}`
@@ -141,11 +158,10 @@ export function hasValidEditorCommand(editor: EditorType): boolean {
 export async function hasValidEditorCommandAsync(
   editor: EditorType,
 ): Promise<boolean> {
-  return Promise.any(
-    getEditorCommands(editor).map((cmd) =>
-      commandExistsAsync(cmd).then((exists) => exists || Promise.reject()),
-    ),
-  ).catch(() => false);
+  const results = await Promise.allSettled(
+    getEditorCommands(editor).map((cmd) => commandExistsAsync(cmd)),
+  );
+  return results.some((r) => r.status === 'fulfilled' && r.value);
 }
 
 export function getEditorCommand(editor: EditorType): string {
@@ -336,3 +352,130 @@ export async function openDiff(
     });
   });
 }
+
+/**
+ * Opens a file in an editor.
+ * If an IDE client is provided and connected, it uses openDiff for an integrated experience.
+ * Otherwise, it falls back to external editors (GUI or Terminal).
+ */
+export async function openFileInEditor(
+  filePath: string,
+  options: OpenFileInEditorOptions = {},
+): Promise<{ modified: boolean }> {
+  const { ideClient, preferredEditor, readTextFile, writeTextFile } = options;
+
+  // 1. Try IDE Flow
+  if (ideClient?.isDiffingEnabled() && readTextFile && writeTextFile) {
+    debugLogger.debug(`openFileInEditor: Using IDE flow for ${filePath}`);
+    try {
+      const currentContent = await readTextFile(filePath);
+      const result = await ideClient.openDiff(filePath, currentContent);
+      if (result.status === 'accepted' && result.content !== undefined) {
+        if (result.content !== currentContent) {
+          await writeTextFile(filePath, result.content);
+          return { modified: true };
+        }
+      }
+      return { modified: false };
+    } catch (err) {
+      debugLogger.error(
+        'openFileInEditor: IDE flow failed, falling back:',
+        err,
+      );
+      // Fall through to external editor
+    }
+  }
+
+  // 2. Resolve external editor command
+  let command: string | undefined = undefined;
+  const args = [filePath];
+
+  if (preferredEditor) {
+    command = getEditorCommand(preferredEditor);
+    if (isGuiEditor(preferredEditor)) {
+      args.unshift('--wait');
+    }
+  }
+
+  if (!command) {
+    command =
+      process.env['VISUAL'] ??
+      process.env['EDITOR'] ??
+      (process.platform === 'win32' ? 'notepad' : 'vim');
+  }
+
+  // DEFINITIVE FIX for Vim E138: Always add -i NONE when we detect vim/nvim
+  const commandBase = command.toLowerCase();
+  if (commandBase.includes('vim') || commandBase.includes('nvim')) {
+    args.unshift('-i', 'NONE');
+  }
+
+  const useGuiSpawn = preferredEditor && isGuiEditor(preferredEditor);
+  debugLogger.debug(
+    `openFileInEditor: Using external editor: ${command} ${args.join(' ')} (GUI spawn: ${useGuiSpawn})`,
+  );
+
+  const initialContent = readTextFile
+    ? await readTextFile(filePath).catch(() => undefined)
+    : undefined;
+
+  return new Promise<{ modified: boolean }>((resolve, reject) => {
+    const wasRaw = process.stdin.isRaw;
+    if (!useGuiSpawn && wasRaw) {
+      process.stdin.setRawMode(false);
+    }
+
+    const onExit = async (status: number | null, error?: Error) => {
+      if (!useGuiSpawn && wasRaw) {
+        process.stdin.setRawMode(true);
+      }
+      coreEvents.emit(CoreEvent.ExternalEditorClosed);
+
+      if (error) {
+        reject(error);
+      } else if (status !== null && status !== 0) {
+        reject(new Error(`Editor exited with status ${status}`));
+      } else {
+        if (readTextFile) {
+          try {
+            const finalContent = await readTextFile(filePath);
+            resolve({ modified: initialContent !== finalContent });
+          } catch (err) {
+            debugLogger.error(
+              `openFileInEditor: Failed to read file after editor exit: ${filePath}`,
+              err,
+            );
+            // Fallback to true if we can't read the file but it existed or was supposed to
+            resolve({ modified: true });
+          }
+        } else {
+          // Assume modified if external editor was used and closed successfully
+          resolve({ modified: true });
+        }
+      }
+    };
+
+    if (useGuiSpawn) {
+      const child = spawn(command, args, {
+        stdio: 'inherit',
+        shell: process.platform === 'win32',
+      });
+      child.on('close', (code) => {
+        void onExit(code);
+      });
+      child.on('error', (err) => {
+        void onExit(null, err);
+      });
+    } else {
+      try {
+        const result = spawnSync(command, args, {
+          stdio: 'inherit',
+          shell: process.platform === 'win32',
+        });
+        void onExit(result.status, result.error || undefined);
+      } catch (err: unknown) {
+        void onExit(null, err instanceof Error ? err : new Error(String(err)));
+      }
+    }
+  });
+}