test(cli): add unit tests for editor fallback and improve env var handling

packages/cli/src/ui/components/shared/text-buffer-external.test.ts

@@ -0,0 +1,126 @@
+/**
+ * @license
+ * Copyright 2026 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { act } from 'react';
+import { renderHook } from '../../../test-utils/render.js';
+import { useTextBuffer } from './text-buffer.js';
+import {
+  openFileInEditor,
+  EditorNotConfiguredError,
+} from '../../utils/editorUtils.js';
+import { coreEvents, CoreEvent } from '@google/gemini-cli-core';
+import fs from 'node:fs';
+
+vi.mock('node:fs', () => ({
+  default: {
+    mkdtempSync: vi.fn().mockReturnValue('/tmp/gemini-edit-123'),
+    writeFileSync: vi.fn(),
+    readFileSync: vi.fn().mockReturnValue('updated text'),
+    unlinkSync: vi.fn(),
+    rmdirSync: vi.fn(),
+  },
+}));
+
+vi.mock('node:os', () => ({
+  default: {
+    tmpdir: vi.fn().mockReturnValue('/tmp'),
+  },
+}));
+
+vi.mock('../../utils/editorUtils.js', () => ({
+  openFileInEditor: vi.fn(),
+  EditorNotConfiguredError: class extends Error {
+    constructor() {
+      super('No external editor configured');
+      this.name = 'EditorNotConfiguredError';
+    }
+  },
+}));
+
+vi.mock('@google/gemini-cli-core', async (importOriginal) => {
+  const original =
+    await importOriginal<typeof import('@google/gemini-cli-core')>();
+  return {
+    ...original,
+    coreEvents: {
+      emit: vi.fn(),
+      emitFeedback: vi.fn(),
+    },
+  };
+});
+
+describe('useTextBuffer external editor', () => {
+  const viewport = { width: 80, height: 24 };
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('should emit RequestEditorSelection when openFileInEditor throws EditorNotConfiguredError', async () => {
+    vi.mocked(openFileInEditor).mockRejectedValue(
+      new EditorNotConfiguredError(),
+    );
+
+    const { result } = await renderHook(() =>
+      useTextBuffer({
+        initialText: 'some text',
+        viewport,
+      }),
+    );
+
+    await act(async () => {
+      await result.current.openInExternalEditor();
+    });
+
+    expect(coreEvents.emit).toHaveBeenCalledWith(
+      CoreEvent.RequestEditorSelection,
+    );
+  });
+
+  it('should update text when openFileInEditor succeeds', async () => {
+    vi.mocked(openFileInEditor).mockResolvedValue(undefined);
+    vi.mocked(fs.readFileSync).mockReturnValue('updated text from editor');
+
+    const { result } = await renderHook(() =>
+      useTextBuffer({
+        initialText: 'initial text',
+        viewport,
+      }),
+    );
+
+    await act(async () => {
+      await result.current.openInExternalEditor();
+    });
+
+    expect(result.current.text).toBe('updated text from editor');
+  });
+
+  it('should log feedback error for other errors', async () => {
+    const unexpectedError = new Error('Some unexpected error');
+    vi.mocked(openFileInEditor).mockRejectedValue(unexpectedError);
+
+    const { result } = await renderHook(() =>
+      useTextBuffer({
+        initialText: 'some text',
+        viewport,
+      }),
+    );
+
+    await act(async () => {
+      await result.current.openInExternalEditor();
+    });
+
+    expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
+      'error',
+      '[useTextBuffer] external editor error',
+      unexpectedError,
+    );
+    expect(coreEvents.emit).not.toHaveBeenCalledWith(
+      CoreEvent.RequestEditorSelection,
+    );
+  });
+});

packages/cli/src/ui/utils/editorUtils.test.ts

@@ -0,0 +1,125 @@
+/**
+ * @license
+ * Copyright 2026 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { openFileInEditor, EditorNotConfiguredError } from './editorUtils.js';
+import {
+  spawnSync,
+  spawn,
+  type SpawnSyncReturns,
+  type ChildProcess,
+} from 'node:child_process';
+import { CoreEvent, coreEvents } from '@google/gemini-cli-core';
+
+vi.mock('node:child_process', () => ({
+  spawnSync: vi.fn(),
+  spawn: vi.fn(),
+}));
+
+vi.mock('@google/gemini-cli-core', async (importOriginal) => {
+  const original =
+    await importOriginal<typeof import('@google/gemini-cli-core')>();
+  return {
+    ...original,
+    coreEvents: {
+      emit: vi.fn(),
+      emitFeedback: vi.fn(),
+    },
+  };
+});
+
+describe('editorUtils', () => {
+  beforeEach(() => {
+    vi.stubEnv('VISUAL', '');
+    vi.stubEnv('EDITOR', '');
+    vi.clearAllMocks();
+  });
+
+  afterEach(() => {
+    vi.unstubAllEnvs();
+  });
+
+  it('should throw EditorNotConfiguredError if no editor is configured and no env vars set', async () => {
+    await expect(openFileInEditor('test.txt', null, undefined)).rejects.toThrow(
+      EditorNotConfiguredError,
+    );
+  });
+
+  it('should use preferredEditorType if provided (terminal editor)', async () => {
+    vi.mocked(spawnSync).mockReturnValue({
+      status: 0,
+    } as SpawnSyncReturns<Buffer>);
+    await openFileInEditor('test.txt', null, undefined, 'vim');
+    expect(spawnSync).toHaveBeenCalledWith(
+      'vim',
+      expect.arrayContaining(['test.txt']),
+      expect.anything(),
+    );
+    expect(coreEvents.emit).toHaveBeenCalledWith(
+      CoreEvent.ExternalEditorClosed,
+    );
+  });
+
+  it('should use preferredEditorType if provided (GUI editor)', async () => {
+    const mockChild = {
+      on: vi.fn((event: string, cb: (code: number) => void) => {
+        if (event === 'close') cb(0);
+        return mockChild;
+      }),
+    };
+    vi.mocked(spawn).mockReturnValue(mockChild as unknown as ChildProcess);
+    await openFileInEditor('test.txt', null, undefined, 'vscode');
+    expect(spawn).toHaveBeenCalledWith(
+      'code',
+      expect.arrayContaining(['--wait', 'test.txt']),
+      expect.anything(),
+    );
+    expect(coreEvents.emit).toHaveBeenCalledWith(
+      CoreEvent.ExternalEditorClosed,
+    );
+  });
+
+  it('should use VISUAL env var if set', async () => {
+    vi.stubEnv('VISUAL', 'nano');
+    vi.mocked(spawnSync).mockReturnValue({
+      status: 0,
+    } as SpawnSyncReturns<Buffer>);
+    await openFileInEditor('test.txt', null, undefined);
+    expect(spawnSync).toHaveBeenCalledWith(
+      'nano',
+      expect.arrayContaining(['test.txt']),
+      expect.anything(),
+    );
+  });
+
+  it('should use EDITOR env var if set', async () => {
+    vi.stubEnv('EDITOR', 'nano');
+    vi.mocked(spawnSync).mockReturnValue({
+      status: 0,
+    } as SpawnSyncReturns<Buffer>);
+    await openFileInEditor('test.txt', null, undefined);
+    expect(spawnSync).toHaveBeenCalledWith(
+      'nano',
+      expect.arrayContaining(['test.txt']),
+      expect.anything(),
+    );
+  });
+
+  it('should handle editor exit with non-zero status', async () => {
+    vi.stubEnv('EDITOR', 'vim');
+    vi.mocked(spawnSync).mockReturnValue({
+      status: 1,
+    } as SpawnSyncReturns<Buffer>);
+    await expect(openFileInEditor('test.txt', null, undefined)).rejects.toThrow(
+      'External editor exited with status 1',
+    );
+    expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
+      'error',
+      expect.any(String),
+      expect.any(Error),
+    );
+  });
+});

packages/cli/src/ui/utils/editorUtils.ts

@@ -48,7 +48,7 @@ export async function openFileInEditor(
   }
 
   if (!command) {
-    command = process.env['VISUAL'] ?? process.env['EDITOR'];
+    command = process.env['VISUAL'] || process.env['EDITOR'];
     if (command) {
       const lowerCommand = command.toLowerCase();
       const isGui = ['code', 'cursor', 'subl', 'zed', 'atom'].some((gui) =>