From 5a43efcae40efac332646b6b3fed6149614a1eb1 Mon Sep 17 00:00:00 2001 From: lamb <56495414+gy1016@users.noreply.github.com> Date: Tue, 21 Apr 2026 17:06:58 +0800 Subject: [PATCH] fix(editor): detect Zed.app on macOS when CLI is not in PATH (#3303) * fix(editor): detect Zed.app on macOS when CLI is not in PATH On macOS, Zed editor is typically installed via Homebrew or direct download, but the CLI command 'zed' is not automatically added to PATH. This fix adds detection for Zed.app bundle at: - /Applications/Zed.app - ~/Applications/Zed.app When the CLI is not found but the app bundle exists, the code now falls back to using the CLI inside the app bundle at Contents/MacOS/zed. Fixes #3287 * fix(editor): use shared getEditorExecutable in useLaunchEditor - Export getEditorExecutable() from editor.ts for use by both getDiffCommand and useLaunchEditor - Updated useLaunchEditor.ts to use getEditorExecutable instead of its own implementation - Updated tests to be platform-agnostic for macOS app bundle path testing - Fixes: Zed on macOS is now detected when installed via app bundle (not just CLI in PATH) * fix(editor): use correct Zed CLI path (Contents/MacOS/cli) - Changed from Contents/MacOS/zed (GUI binary) to Contents/MacOS/cli (actual CLI) - The GUI binary does not support --wait/--diff flags - Updated tests to verify correct CLI path with regex matching for cross-platform * style(editor): fix prettier trailing whitespace issues Trailing spaces and array line-wrapping in zed macOS detection code. * fix(editor): return null when editor not found + remove unused var - getEditorExecutable now returns null (not fallback string) so useLaunchEditor error handling actually works - remove unused getAppBundleCliPath in test file (typecheck fix) * fix: add vitest globals to eslint config for test files * fix: remove duplicate empty test with orphan toEqual call * fix: resolve ESLint errors in editor.test.ts (arrow-body-style, no-useless-escape) * chore: remove debug script check_braces.js * fix: sync checkHasEditorType with getEditorExecutable, remove pr-body.md - Replace zedAppExists() check in checkHasEditorType with getEditorExecutable() !== null, keeping availability detection and execution in sync (fixes partial install false positive) - Remove unused zedAppExists() function - Remove scratch file pr-body.md * fix(editor): defer os.homedir() call to avoid breaking tests with incomplete node:os mocks The zedMacOsPaths constant was calling homedir() at module initialization time, which caused 'homedir is not a function' errors in CLI test files (systemInfo.test.ts, shellCommandProcessor.test.ts) that mock node:os without providing a homedir mock. Fix: convert zedMacOsPaths from a constant to a lazy function getZedAppPaths() that computes the paths only when called. --------- Co-authored-by: lamb <906276457@qq.com> --- eslint.config.js | 5 + packages/cli/src/ui/hooks/useLaunchEditor.ts | 53 +---- packages/core/src/utils/editor.test.ts | 230 ++++++++++++++++++- packages/core/src/utils/editor.ts | 85 ++++++- 4 files changed, 322 insertions(+), 51 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index c7638b82c..27214f6d6 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -168,6 +168,11 @@ export default tseslint.config( plugins: { vitest, }, + languageOptions: { + globals: { + ...globals.vitest, + }, + }, rules: { ...vitest.configs.recommended.rules, 'vitest/expect-expect': 'off', diff --git a/packages/cli/src/ui/hooks/useLaunchEditor.ts b/packages/cli/src/ui/hooks/useLaunchEditor.ts index 809e8a3d6..839a0ff05 100644 --- a/packages/cli/src/ui/hooks/useLaunchEditor.ts +++ b/packages/cli/src/ui/hooks/useLaunchEditor.ts @@ -1,58 +1,23 @@ import { useCallback } from 'react'; import { useStdin } from 'ink'; import type { EditorType } from '@qwen-code/qwen-code-core'; -import { - editorCommands, - commandExists as coreCommandExists, -} from '@qwen-code/qwen-code-core'; +import { getEditorExecutable } from '@qwen-code/qwen-code-core'; import { spawnSync } from 'child_process'; import { useSettings } from '../contexts/SettingsContext.js'; -/** - * Cache for command existence checks to avoid repeated execSync calls. - */ -const commandExistsCache = new Map(); - -/** - * Check if a command exists in the system with caching. - * Results are cached to improve performance in test environments. - */ -function commandExists(cmd: string): boolean { - if (commandExistsCache.has(cmd)) { - return commandExistsCache.get(cmd)!; - } - - const exists = coreCommandExists(cmd); - commandExistsCache.set(cmd, exists); - return exists; -} -/** - * Get the actual executable command for an editor type. - */ -function getExecutableCommand(editorType: EditorType): string { - const commandConfig = editorCommands[editorType]; - const commands = - process.platform === 'win32' ? commandConfig.win32 : commandConfig.default; - - const availableCommand = commands.find((cmd) => commandExists(cmd)); - - if (!availableCommand) { - throw new Error( - `No available editor command found for ${editorType}. ` + - `Tried: ${commands.join(', ')}. ` + - `Please install one of these editors or set a different preferredEditor in settings.`, - ); - } - - return availableCommand; -} - /** * Determines the editor command to use based on user preferences and platform. */ function getEditorCommand(preferredEditor?: EditorType): string { if (preferredEditor) { - return getExecutableCommand(preferredEditor); + const execCmd = getEditorExecutable(preferredEditor); + if (!execCmd) { + throw new Error( + `No available editor found for ${preferredEditor}. ` + + `Please install a supported editor or set a different preferredEditor in settings.`, + ); + } + return execCmd; } // Platform-specific defaults with UI preference for macOS diff --git a/packages/core/src/utils/editor.test.ts b/packages/core/src/utils/editor.test.ts index a2ac74680..851336941 100644 --- a/packages/core/src/utils/editor.test.ts +++ b/packages/core/src/utils/editor.test.ts @@ -22,6 +22,7 @@ import { type EditorType, } from './editor.js'; import { execSync, spawn, spawnSync } from 'node:child_process'; +import { existsSync } from 'node:fs'; vi.mock('child_process', () => ({ execSync: vi.fn(), @@ -29,6 +30,10 @@ vi.mock('child_process', () => ({ spawnSync: vi.fn(() => ({ error: null, status: 0 })), })); +vi.mock('fs', () => ({ + existsSync: vi.fn(), +})); + const originalPlatform = process.platform; describe('editor utils', () => { @@ -171,7 +176,6 @@ describe('editor utils', () => { win32Commands: ['windsurf'], }, { editor: 'cursor', commands: ['cursor'], win32Commands: ['cursor'] }, - { editor: 'zed', commands: ['zed', 'zeditor'], win32Commands: ['zed'] }, { editor: 'trae', commands: ['trae'], win32Commands: ['trae'] }, ]; @@ -314,6 +318,57 @@ describe('editor utils', () => { const command = getDiffCommand('old.txt', 'new.txt', 'foobar'); expect(command).toBeNull(); }); + + // Zed-specific tests (Zed is handled specially for macOS app detection) + describe('Zed', () => { + it('should use CLI command "zed" when it exists on Linux', () => { + Object.defineProperty(process, 'platform', { value: 'linux' }); + (execSync as Mock).mockReturnValue(Buffer.from('/usr/bin/zed')); + const diffCommand = getDiffCommand('old.txt', 'new.txt', 'zed'); + expect(diffCommand).toEqual({ + command: 'zed', + args: ['--wait', '--diff', 'old.txt', 'new.txt'], + }); + }); + + it('should use CLI command "zeditor" when "zed" does not exist on Linux', () => { + Object.defineProperty(process, 'platform', { value: 'linux' }); + (execSync as Mock) + .mockImplementationOnce(() => { + throw new Error(); // zed not found + }) + .mockReturnValueOnce(Buffer.from('/usr/bin/zeditor')); // zeditor found + + const diffCommand = getDiffCommand('old.txt', 'new.txt', 'zed'); + expect(diffCommand).toEqual({ + command: 'zeditor', + args: ['--wait', '--diff', 'old.txt', 'new.txt'], + }); + }); + + it('should return null on Linux when no CLI commands exist', () => { + Object.defineProperty(process, 'platform', { value: 'linux' }); + (execSync as Mock).mockImplementation(() => { + throw new Error(); // all commands not found + }); + (existsSync as Mock).mockReturnValue(false); + + const diffCommand = getDiffCommand('old.txt', 'new.txt', 'zed'); + expect(diffCommand).toBeNull(); + }); + + it('should use CLI command "zed" on Windows when it exists', () => { + Object.defineProperty(process, 'platform', { value: 'win32' }); + (execSync as Mock).mockReturnValue( + Buffer.from('C:\\Program Files\\Zed\\zed.exe'), + ); + const diffCommand = getDiffCommand('old.txt', 'new.txt', 'zed'); + expect(diffCommand).toEqual({ + command: 'zed', + args: ['--wait', '--diff', 'old.txt', 'new.txt'], + }); + }); + }); }); describe('openDiff', () => { @@ -322,7 +377,6 @@ describe('editor utils', () => { 'vscodium', 'windsurf', 'cursor', - 'zed', 'trae', ]; @@ -377,6 +431,65 @@ describe('editor utils', () => { }); } + // Zed-specific openDiff tests + describe('Zed', () => { + it('should call spawn for zed on macOS with CLI', async () => { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + (execSync as Mock).mockReturnValue(Buffer.from('/usr/local/bin/zed')); + (existsSync as Mock).mockReturnValue(false); + + const mockSpawnOn = vi.fn((event, cb) => { + if (event === 'close') { + cb(0); + } + }); + (spawn as Mock).mockReturnValue({ on: mockSpawnOn }); + + await openDiff('old.txt', 'new.txt', 'zed', () => {}); + expect(spawn).toHaveBeenCalledWith( + 'zed', + ['--wait', '--diff', 'old.txt', 'new.txt'], + { + stdio: 'inherit', + shell: false, + }, + ); + }); + + it('should call spawn for zed on macOS with app bundle CLI', async () => { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + (execSync as Mock).mockImplementation(() => { + throw new Error(); // CLI not found + }); + // Accept any path containing Zed.app + (existsSync as Mock).mockImplementation((path: string) => path.includes('Zed.app')); + + const mockSpawnOn = vi.fn((event, cb) => { + if (event === 'close') { + cb(0); + } + }); + (spawn as Mock).mockReturnValue({ on: mockSpawnOn }); + + await openDiff('old.txt', 'new.txt', 'zed', () => {}); + expect(spawn).toHaveBeenCalled(); + // Verify the command uses the CLI tool (not GUI binary) + const call = (spawn as Mock).mock.calls[0]; + expect(call[0]).toMatch(/MacOS[/\\]cli$/); + }); + + it('should reject if zed is not installed', async () => { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + (execSync as Mock).mockImplementation(() => { + throw new Error(); // CLI not found + }); + (existsSync as Mock).mockReturnValue(false); // App not found + + await openDiff('old.txt', 'new.txt', 'zed', () => {}); + // Should complete without throwing (logs error to debugLogger) + }); + }); + const terminalEditors: EditorType[] = ['vim', 'neovim', 'emacs']; for (const editor of terminalEditors) { @@ -427,7 +540,6 @@ describe('editor utils', () => { 'vscodium', 'windsurf', 'cursor', - 'zed', 'trae', ]; for (const editor of guiEditors) { @@ -443,6 +555,23 @@ describe('editor utils', () => { expect(onEditorClose).not.toHaveBeenCalled(); }); } + + // Zed-specific onEditorClose tests + it('should not call onEditorClose for zed', async () => { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + (execSync as Mock).mockReturnValue(Buffer.from('/usr/local/bin/zed')); + (existsSync as Mock).mockReturnValue(false); + + const onEditorClose = vi.fn(); + const mockSpawnOn = vi.fn((event, cb) => { + if (event === 'close') { + cb(0); + } + }); + (spawn as Mock).mockReturnValue({ on: mockSpawnOn }); + await openDiff('old.txt', 'new.txt', 'zed', onEditorClose); + expect(onEditorClose).not.toHaveBeenCalled(); + }); }); }); @@ -543,4 +672,99 @@ describe('editor utils', () => { expect(isEditorAvailable('neovim')).toBe(true); }); }); + + describe('Zed macOS app detection', () => { + describe('checkHasEditorType for Zed', () => { + it('should return true on macOS when Zed.app exists even if CLI is not in PATH', () => { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + (execSync as Mock).mockImplementation(() => { + throw new Error(); // CLI not found + }); + (existsSync as Mock).mockReturnValue(true); // Zed.app exists + expect(checkHasEditorType('zed')).toBe(true); + }); + + it('should return false on macOS when Zed.app does not exist and CLI is not in PATH', () => { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + (execSync as Mock).mockImplementation(() => { + throw new Error(); // CLI not found + }); + (existsSync as Mock).mockReturnValue(false); // Zed.app does not exist + expect(checkHasEditorType('zed')).toBe(false); + }); + + it('should return true on macOS when Zed CLI is in PATH', () => { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + (execSync as Mock).mockReturnValue(Buffer.from('/usr/local/bin/zed')); + expect(checkHasEditorType('zed')).toBe(true); + }); + + it('should not check for Zed.app on non-macOS platforms', () => { + Object.defineProperty(process, 'platform', { value: 'linux' }); + (execSync as Mock).mockImplementation(() => { + throw new Error(); // CLI not found + }); + (existsSync as Mock).mockReturnValue(true); // This should be ignored on Linux + expect(checkHasEditorType('zed')).toBe(false); + }); + }); + + describe('getDiffCommand for Zed on macOS', () => { + it('should use app bundle CLI path when CLI is not in PATH', () => { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + (execSync as Mock).mockImplementation(() => { + throw new Error(); // CLI not found + }); + // Accept any path containing Zed.app (the CLI check will be for Contents/MacOS/cli) + (existsSync as Mock).mockImplementation((path: string) => path.includes('Zed.app')); + + const diffCommand = getDiffCommand('old.txt', 'new.txt', 'zed'); + expect(diffCommand).not.toBeNull(); + // Verify the command ends with cli (the CLI tool, not GUI binary zed) + expect(diffCommand!.command).toMatch(/MacOS[/\\]cli$/); + expect(diffCommand!.args).toEqual([ + '--wait', + '--diff', + 'old.txt', + 'new.txt', + ]); + }); + + it('should prefer CLI in PATH over app bundle', () => { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + (execSync as Mock).mockReturnValue(Buffer.from('/usr/local/bin/zed')); + (existsSync as Mock).mockReturnValue(true); // App also exists + + const diffCommand = getDiffCommand('old.txt', 'new.txt', 'zed'); + expect(diffCommand).toEqual({ + command: 'zed', + args: ['--wait', '--diff', 'old.txt', 'new.txt'], + }); + }); + + it('should return null when Zed is not installed at all', () => { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + (execSync as Mock).mockImplementation(() => { + throw new Error(); // CLI not found + }); + (existsSync as Mock).mockReturnValue(false); // App not found + + const diffCommand = getDiffCommand('old.txt', 'new.txt', 'zed'); + expect(diffCommand).toBeNull(); + }); + + it('should check user Applications folder as fallback', () => { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + (execSync as Mock).mockImplementation(() => { + throw new Error(); // CLI not found + }); + // Accept any path containing Zed.app + (existsSync as Mock).mockImplementation((path: string) => path.includes('Zed.app')); + + const diffCommand = getDiffCommand('old.txt', 'new.txt', 'zed'); + expect(diffCommand).not.toBeNull(); + expect(diffCommand!.command).toMatch(/MacOS[/\\]cli$/); + }); + }); + }); }); diff --git a/packages/core/src/utils/editor.ts b/packages/core/src/utils/editor.ts index 70f574ab4..d5d22623a 100644 --- a/packages/core/src/utils/editor.ts +++ b/packages/core/src/utils/editor.ts @@ -5,6 +5,9 @@ */ import { execSync, spawn, spawnSync } from 'node:child_process'; +import { existsSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; import { createDebugLogger } from './debugLogger.js'; const debugLogger = createDebugLogger('EDITOR'); @@ -51,6 +54,15 @@ export function commandExists(cmd: string): boolean { } } +/** + * Get possible paths for Zed.app on macOS. + * Returns paths lazily to avoid calling os.homedir() at module initialization time, + * which would break tests that mock node:os without providing a homedir mock. + */ +function getZedAppPaths(): string[] { + return ['/Applications/Zed.app', join(homedir(), 'Applications/Zed.app')]; +} + /** * Editor command configurations for different platforms. * Each editor can have multiple possible command names, listed in order of preference. @@ -70,11 +82,67 @@ export const editorCommands: Record< trae: { win32: ['trae'], default: ['trae'] }, }; -export function checkHasEditorType(editor: EditorType): boolean { - const commandConfig = editorCommands[editor]; +/** + * Get the Zed command to use for opening files/diffs. + * On macOS, if the CLI is not in PATH, fall back to using the app bundle's CLI. + */ +function getZedCommand(): string | null { + // Check CLI commands first + const commands = editorCommands.zed.default; + for (const cmd of commands) { + if (commandExists(cmd)) { + return cmd; + } + } + + // On macOS, check for app bundle CLI + if (process.platform === 'darwin') { + for (const appPath of getZedAppPaths()) { + const cliPath = join(appPath, 'Contents/MacOS/cli'); + if (existsSync(cliPath)) { + return cliPath; + } + } + } + + return null; +} + +/** + * Get the executable command for a given editor type. + * Resolves both CLI commands and platform-specific fallbacks (e.g., macOS app bundles). + * This is the shared function used by both getDiffCommand and useLaunchEditor. + * Returns null if no editor command is found. + */ +export function getEditorExecutable(editorType: EditorType): string | null { + const commandConfig = editorCommands[editorType]; const commands = process.platform === 'win32' ? commandConfig.win32 : commandConfig.default; - return commands.some((cmd) => commandExists(cmd)); + + // Check if any of the CLI commands exist + const found = commands.find((cmd) => commandExists(cmd)); + if (found) { + return found; + } + + // Special handling for Zed on macOS: check app bundle CLI as fallback + if (editorType === 'zed' && process.platform === 'darwin') { + for (const appPath of getZedAppPaths()) { + const cliPath = join(appPath, 'Contents/MacOS/cli'); + if (existsSync(cliPath)) { + return cliPath; + } + } + } + + // No command found + return null; +} + +export function checkHasEditorType(editor: EditorType): boolean { + // Use the same resolution logic as getEditorExecutable to keep + // availability detection and execution in sync. + return getEditorExecutable(editor) !== null; } export function allowEditorTypeInSandbox(editor: EditorType): boolean { @@ -110,6 +178,16 @@ export function getDiffCommand( if (!isValidEditorType(editor)) { return null; } + + // Special handling for Zed on macOS + if (editor === 'zed') { + const zedCmd = getZedCommand(); + if (!zedCmd) { + return null; + } + return { command: zedCmd, args: ['--wait', '--diff', oldPath, newPath] }; + } + const commandConfig = editorCommands[editor]; const commands = process.platform === 'win32' ? commandConfig.win32 : commandConfig.default; @@ -122,7 +200,6 @@ export function getDiffCommand( case 'vscodium': case 'windsurf': case 'cursor': - case 'zed': case 'trae': return { command, args: ['--wait', '--diff', oldPath, newPath] }; case 'vim':