diff --git a/src/main/external-editor-launch.test.ts b/src/main/external-editor-launch.test.ts index b9d4b2011..507e9b570 100644 --- a/src/main/external-editor-launch.test.ts +++ b/src/main/external-editor-launch.test.ts @@ -28,6 +28,35 @@ describe('resolveExternalEditorLaunchSpec', () => { }) }) + it('treats an existing POSIX executable path with spaces as an executable launcher', () => { + const ideaPath = '/Users/me/Library/Application Support/JetBrains/Toolbox/scripts/idea' + expect( + resolveExternalEditorLaunchSpec(ideaPath, '/tmp/workspace', { + platform: 'darwin', + fileExists: (candidate) => candidate === ideaPath + }) + ).toEqual({ + kind: 'executable', + hideWindowsConsole: true, + spawnCmd: ideaPath, + spawnArgs: ['/tmp/workspace'] + }) + }) + + it('keeps absolute POSIX commands with arguments on the shell launch path', () => { + expect( + resolveExternalEditorLaunchSpec('/usr/local/bin/code --reuse-window', '/tmp/workspace', { + platform: 'darwin', + fileExists: () => false + }) + ).toEqual({ + kind: 'shell', + hideWindowsConsole: true, + spawnCmd: '/bin/sh', + spawnArgs: ['-c', '/usr/local/bin/code --reuse-window /tmp/workspace'] + }) + }) + it('runs compound Windows commands through cmd.exe', () => { expect( resolveExternalEditorLaunchSpec('start "" notepad', 'C:\\note.md', { platform: 'win32' }) diff --git a/src/main/external-editor-launch.ts b/src/main/external-editor-launch.ts index b6c9e9779..8e5c4a0b3 100644 --- a/src/main/external-editor-launch.ts +++ b/src/main/external-editor-launch.ts @@ -1,3 +1,4 @@ +import { existsSync } from 'node:fs' import { basename, posix, win32 } from 'node:path' import { resolveCliCommand } from './codex-cli/command' import { getCmdExePath } from './win32-utils' @@ -65,12 +66,36 @@ function stripMatchingQuotes(value: string): string { return trimmed } -function isDirectExecutablePath(command: string, platform: NodeJS.Platform): boolean { +function hasMatchingOuterQuotes(value: string): boolean { + const trimmed = value.trim() + const quote = trimmed[0] + return (quote === '"' || quote === "'") && trimmed.endsWith(quote) +} + +function isWindowsExecutablePath(command: string): boolean { + return win32.isAbsolute(command) && /\.(?:cmd|exe|bat|com)$/i.test(command) +} + +function isDirectExecutablePath( + command: string, + platform: NodeJS.Platform, + fileExists: (path: string) => boolean +): boolean { const unquoted = stripMatchingQuotes(command) if (!/[\\/]/.test(unquoted)) { return false } - return platform === 'win32' ? win32.isAbsolute(unquoted) : posix.isAbsolute(unquoted) + const isAbsolutePath = + platform === 'win32' ? win32.isAbsolute(unquoted) : posix.isAbsolute(unquoted) + if (!isAbsolutePath) { + return false + } + if (!/\s/.test(unquoted) || hasMatchingOuterQuotes(command)) { + return true + } + // Why: unquoted POSIX paths can contain spaces, but so can shell commands + // with arguments. Only an existing path is safe to treat as one executable. + return platform === 'win32' ? isWindowsExecutablePath(unquoted) : fileExists(unquoted) } function shouldShowWindowsConsole( @@ -119,12 +144,13 @@ function buildShellLaunchSpec( export function resolveExternalEditorLaunchSpec( command: string | undefined, pathValue: string, - options: { platform?: NodeJS.Platform } = {} + options: { platform?: NodeJS.Platform; fileExists?: (path: string) => boolean } = {} ): ExternalEditorLaunchSpec { const platform = options.platform ?? process.platform + const fileExists = options.fileExists ?? existsSync const trimmed = command?.trim() || EXTERNAL_EDITOR_CLI_COMMAND - if (isDirectExecutablePath(trimmed, platform)) { + if (isDirectExecutablePath(trimmed, platform, fileExists)) { const editorCommand = stripMatchingQuotes(trimmed) return { kind: 'executable',