Merge branch 'main' into rename-directory-to-workspace

packages/core/src/config/config.ts

@@ -361,6 +361,10 @@ export interface GeminiCLIExtension {
      */
     directory?: string;
   };
+  /**
+   * Used to migrate an extension to a new repository source.
+   */
+  migratedTo?: string;
 }
 
 export interface ExtensionInstallMetadata {

packages/core/src/services/shellExecutionService.test.ts

@@ -870,6 +870,77 @@ describe('ShellExecutionService', () => {
 
       expect(ShellExecutionService['activePtys'].size).toBe(0);
     });
+
+    it('should destroy the PTY when kill() is called', async () => {
+      // Execute a command to populate activePtys
+      const abortController = new AbortController();
+      await ShellExecutionService.execute(
+        'long-running',
+        '/test/dir',
+        onOutputEventMock,
+        abortController.signal,
+        true,
+        shellExecutionConfig,
+      );
+      await new Promise((resolve) => process.nextTick(resolve));
+
+      const pid = mockPtyProcess.pid;
+      const activePty = ShellExecutionService['activePtys'].get(pid);
+      expect(activePty).toBeTruthy();
+
+      // Spy on the actual stored object's destroy
+      const storedDestroySpy = vi.spyOn(
+        activePty!.ptyProcess as never as { destroy: () => void },
+        'destroy',
+      );
+
+      ShellExecutionService.kill(pid);
+
+      expect(storedDestroySpy).toHaveBeenCalled();
+      expect(ShellExecutionService['activePtys'].has(pid)).toBe(false);
+    });
+
+    it('should destroy the PTY when an exception occurs after spawn in executeWithPty', async () => {
+      // Simulate: spawn succeeds, Promise executor runs fine (pid accesses 1-2),
+      // but the return statement `{ pid: ptyProcess.pid }` (access 3) throws.
+      // The catch block should call spawnedPty.destroy() to release the fd.
+      const destroySpy = vi.fn();
+      let pidAccessCount = 0;
+      const faultyPty = {
+        onData: vi.fn(),
+        onExit: vi.fn(),
+        write: vi.fn(),
+        kill: vi.fn(),
+        resize: vi.fn(),
+        destroy: destroySpy,
+        get pid() {
+          pidAccessCount++;
+          // Accesses 1-2 are inside the Promise executor (setup).
+          // Access 3 is at `return { pid: ptyProcess.pid, result }`,
+          // outside the Promise — caught by the outer try/catch.
+          if (pidAccessCount > 2) {
+            throw new Error('Simulated post-spawn failure on pid access');
+          }
+          return 77777;
+        },
+      };
+      mockPtySpawn.mockReturnValueOnce(faultyPty);
+
+      const handle = await ShellExecutionService.execute(
+        'will-fail-after-spawn',
+        '/test/dir',
+        onOutputEventMock,
+        new AbortController().signal,
+        true,
+        shellExecutionConfig,
+      );
+
+      const result = await handle.result;
+      expect(result.exitCode).toBe(1);
+      expect(result.error).toBeTruthy();
+      // The catch block must call destroy() on spawnedPty to prevent fd leak
+      expect(destroySpy).toHaveBeenCalled();
+    });
   });
 });
 

packages/core/src/services/shellExecutionService.ts

@@ -552,6 +552,8 @@ export class ShellExecutionService {
       // This should not happen, but as a safeguard...
       throw new Error('PTY implementation not found');
     }
+    let spawnedPty: IPty | undefined;
+
     try {
       const cols = shellExecutionConfig.terminalWidth ?? 80;
       const rows = shellExecutionConfig.terminalHeight ?? 30;
@@ -585,6 +587,8 @@ export class ShellExecutionService {
         },
         handleFlowControl: true,
       });
+      // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
+      spawnedPty = ptyProcess as IPty;
 
       const result = new Promise<ShellExecutionResult>((resolve) => {
         this.activeResolvers.set(ptyProcess.pid, resolve);
@@ -882,6 +886,15 @@ export class ShellExecutionService {
     } catch (e) {
       // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
       const error = e as Error;
+
+      if (spawnedPty) {
+        try {
+          (spawnedPty as IPty & { destroy?: () => void }).destroy?.();
+        } catch {
+          // Ignore errors during cleanup
+        }
+      }
+
       if (error.message.includes('posix_spawnp failed')) {
         onOutputEvent({
           type: 'data',
@@ -1008,6 +1021,11 @@ export class ShellExecutionService {
       this.activeChildProcesses.delete(pid);
     } else if (activePty) {
       killProcessGroup({ pid, pty: activePty.ptyProcess }).catch(() => {});
+      try {
+        (activePty.ptyProcess as IPty & { destroy?: () => void }).destroy?.();
+      } catch {
+        // Ignore errors during cleanup
+      }
       this.activePtys.delete(pid);
     }
 

packages/core/src/tools/read-many-files.test.ts

@@ -776,7 +776,7 @@ Content of file[1]
 
       // Mock to track concurrent vs sequential execution
       detectFileTypeSpy.mockImplementation(async (filePath: string) => {
-        const fileName = filePath.split('/').pop() || '';
+        const fileName = path.basename(filePath);
         executionOrder.push(`start:${fileName}`);
 
         // Add delay to make timing differences visible

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

@@ -392,7 +392,10 @@ describe('editor utils', () => {
         );
       });
 
-      it(`should reject if ${editor} exits with non-zero code`, async () => {
+      it(`should resolve and log warning if ${editor} exits with non-zero code`, async () => {
+        const warnSpy = vi
+          .spyOn(debugLogger, 'warn')
+          .mockImplementation(() => {});
         const mockSpawnOn = vi.fn((event, cb) => {
           if (event === 'close') {
             cb(1);
@@ -400,9 +403,73 @@ describe('editor utils', () => {
         });
         (spawn as Mock).mockReturnValue({ on: mockSpawnOn });
 
+        await openDiff('old.txt', 'new.txt', editor);
+        expect(warnSpy).toHaveBeenCalledWith(`${editor} exited with code 1`);
+      });
+
+      it(`should emit ExternalEditorClosed when ${editor} exits successfully`, async () => {
+        const emitSpy = vi.spyOn(coreEvents, 'emit');
+        const mockSpawnOn = vi.fn((event, cb) => {
+          if (event === 'close') {
+            cb(0);
+          }
+        });
+        (spawn as Mock).mockReturnValue({ on: mockSpawnOn });
+
+        await openDiff('old.txt', 'new.txt', editor);
+        expect(emitSpy).toHaveBeenCalledWith(CoreEvent.ExternalEditorClosed);
+      });
+
+      it(`should emit ExternalEditorClosed when ${editor} exits with non-zero code`, async () => {
+        vi.spyOn(debugLogger, 'warn').mockImplementation(() => {});
+        const emitSpy = vi.spyOn(coreEvents, 'emit');
+        const mockSpawnOn = vi.fn((event, cb) => {
+          if (event === 'close') {
+            cb(1);
+          }
+        });
+        (spawn as Mock).mockReturnValue({ on: mockSpawnOn });
+
+        await openDiff('old.txt', 'new.txt', editor);
+        expect(emitSpy).toHaveBeenCalledWith(CoreEvent.ExternalEditorClosed);
+      });
+
+      it(`should emit ExternalEditorClosed when ${editor} spawn errors`, async () => {
+        const emitSpy = vi.spyOn(coreEvents, 'emit');
+        const mockError = new Error('spawn error');
+        const mockSpawnOn = vi.fn((event, cb) => {
+          if (event === 'error') {
+            cb(mockError);
+          }
+        });
+        (spawn as Mock).mockReturnValue({ on: mockSpawnOn });
+
         await expect(openDiff('old.txt', 'new.txt', editor)).rejects.toThrow(
-          `${editor} exited with code 1`,
+          'spawn error',
         );
+        expect(emitSpy).toHaveBeenCalledWith(CoreEvent.ExternalEditorClosed);
+      });
+
+      it(`should only emit ExternalEditorClosed once when ${editor} fires both error and close`, async () => {
+        const emitSpy = vi.spyOn(coreEvents, 'emit');
+        const callbacks: Record<string, (arg: unknown) => void> = {};
+        const mockSpawnOn = vi.fn(
+          (event: string, cb: (arg: unknown) => void) => {
+            callbacks[event] = cb;
+          },
+        );
+        (spawn as Mock).mockReturnValue({ on: mockSpawnOn });
+
+        const promise = openDiff('old.txt', 'new.txt', editor);
+        // Simulate Node.js behavior: error fires first, then close.
+        callbacks['error'](new Error('spawn error'));
+        callbacks['close'](1);
+
+        await expect(promise).rejects.toThrow('spawn error');
+        const editorClosedEmissions = emitSpy.mock.calls.filter(
+          (call) => call[0] === CoreEvent.ExternalEditorClosed,
+        );
+        expect(editorClosedEmissions).toHaveLength(1);
       });
     }
 

packages/core/src/utils/editor.ts

@@ -323,15 +323,30 @@ export async function openDiff(
       shell: process.platform === 'win32',
     });
 
+    // Guard against both 'error' and 'close' firing for a single failure,
+    // which would emit ExternalEditorClosed twice and attempt to settle
+    // the promise twice.
+    let isSettled = false;
+
     childProcess.on('close', (code) => {
-      if (code === 0) {
-        resolve();
-      } else {
-        reject(new Error(`${editor} exited with code ${code}`));
+      if (isSettled) return;
+      isSettled = true;
+
+      if (code !== 0) {
+        // GUI editors (VS Code, Zed, etc.) can exit with non-zero codes
+        // under normal circumstances (e.g., window closed while loading).
+        // Log a warning instead of crashing the CLI process.
+        debugLogger.warn(`${editor} exited with code ${code}`);
       }
+      coreEvents.emit(CoreEvent.ExternalEditorClosed);
+      resolve();
     });
 
     childProcess.on('error', (error) => {
+      if (isSettled) return;
+      isSettled = true;
+
+      coreEvents.emit(CoreEvent.ExternalEditorClosed);
       reject(error);
     });
   });

packages/core/src/utils/fileUtils.ts

@@ -8,7 +8,6 @@ import fs from 'node:fs';
 import fsPromises from 'node:fs/promises';
 import path from 'node:path';
 import type { PartUnion } from '@google/genai';
-
 import mime from 'mime/lite';
 import type { FileSystemService } from '../services/fileSystemService.js';
 import { ToolErrorType } from '../tools/tool-error.js';
@@ -473,7 +472,7 @@ export async function processSingleFileContent(
       case 'text': {
         // Use BOM-aware reader to avoid leaving a BOM character in content and to support UTF-16/32 transparently
         const content = await readFileWithEncoding(filePath);
-        const lines = content.split('\n');
+        const lines = content.split(/\r?\n/);
         const originalLineCount = lines.length;
 
         let sliceStart = 0;