From 83f49818d31af46ba8375030b57d5bea405eb4cd Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 23 Jun 2026 11:31:41 -0700 Subject: [PATCH] Detect remote agents from shell PATH (#6157) Co-authored-by: Orca --- src/main/ipc/agent-detection-shell-path.ts | 16 ++ src/main/ipc/preflight.test.ts | 59 ++++++ src/main/ipc/preflight.ts | 15 +- src/main/runtime/orca-runtime.test.ts | 28 +-- src/main/runtime/orca-runtime.ts | 5 +- .../runtime/rpc/methods/preflight.test.ts | 10 +- src/main/runtime/rpc/methods/preflight.ts | 4 +- src/relay/preflight-handler.test.ts | 199 +++++++++++++++++- src/relay/preflight-handler.ts | 176 ++++++++++++++-- 9 files changed, 458 insertions(+), 54 deletions(-) create mode 100644 src/main/ipc/agent-detection-shell-path.ts diff --git a/src/main/ipc/agent-detection-shell-path.ts b/src/main/ipc/agent-detection-shell-path.ts new file mode 100644 index 000000000..425697550 --- /dev/null +++ b/src/main/ipc/agent-detection-shell-path.ts @@ -0,0 +1,16 @@ +import { hydrateShellPath, mergePathSegments } from '../startup/hydrate-shell-path' +import { getPreflightWslTarget, type PreflightRuntimeContext } from './preflight-runtime-target' + +export async function hydrateShellPathForAgentDetection( + context?: PreflightRuntimeContext +): Promise { + if (getPreflightWslTarget(context)) { + return + } + // Why: remote runtime servers may inherit a sparse daemon/SSH PATH even + // though the user's shell can run the agents. + const hydration = await hydrateShellPath() + if (hydration.ok) { + mergePathSegments(hydration.segments) + } +} diff --git a/src/main/ipc/preflight.test.ts b/src/main/ipc/preflight.test.ts index 94b969848..d3ffef854 100644 --- a/src/main/ipc/preflight.test.ts +++ b/src/main/ipc/preflight.test.ts @@ -76,6 +76,7 @@ vi.mock('../gitea/client', () => ({ import { _resetPreflightCache, detectInstalledAgents, + detectInstalledAgentsWithShellPathHydration, registerPreflightHandlers, runPreflightCheck } from './preflight' @@ -105,6 +106,7 @@ describe('preflight', () => { handleMock.mockReset() execFileAsyncMock.mockReset() hydrateShellPathMock.mockReset() + hydrateShellPathMock.mockResolvedValue({ segments: [], ok: false, failureReason: 'no_shell' }) mergePathSegmentsMock.mockReset() getActiveMultiplexerMock.mockReset() getBitbucketAuthStatusMock.mockReset() @@ -575,6 +577,63 @@ describe('preflight', () => { await expect(handlers['preflight:detectAgents']()).resolves.toEqual(['openclaude', 'cursor']) }) + it('hydrates shell PATH before user-facing agent detection', async () => { + const originalPath = process.env.PATH + process.env.PATH = '/usr/bin' + hydrateShellPathMock.mockResolvedValueOnce({ + segments: ['/home/test/.local/bin'], + ok: true, + failureReason: 'none' + }) + mergePathSegmentsMock.mockImplementationOnce((segments: string[]) => { + process.env.PATH = [...segments, '/usr/bin'].join(':') + return segments + }) + execFileAsyncMock.mockImplementation(async (command, args) => { + if (command !== 'which') { + throw new Error(`unexpected command ${String(command)}`) + } + if (String(args[0]) === 'codex' && process.env.PATH?.startsWith('/home/test/.local/bin')) { + return { stdout: '/home/test/.local/bin/codex\n' } + } + throw new Error('not found') + }) + + try { + await expect(detectInstalledAgentsWithShellPathHydration()).resolves.toEqual(['codex']) + } finally { + if (originalPath === undefined) { + delete process.env.PATH + } else { + process.env.PATH = originalPath + } + } + expect(hydrateShellPathMock).toHaveBeenCalledWith() + expect(mergePathSegmentsMock).toHaveBeenCalledWith(['/home/test/.local/bin']) + }) + + it('does not run host shell hydration for WSL agent detection', async () => { + Object.defineProperty(process, 'platform', { + configurable: true, + value: 'win32' + }) + execFileAsyncMock.mockImplementation(async (command, args) => { + if (command !== 'wsl.exe') { + throw new Error(`unexpected command ${String(command)}`) + } + const script = String(args[5]) + if (script.includes("'claude'")) { + return { stdout: '__ORCA_AGENT_PATH__claude\t/home/test/.local/bin/claude\n' } + } + throw new Error('not found') + }) + + await expect( + detectInstalledAgentsWithShellPathHydration({ wslDistro: 'Ubuntu' }) + ).resolves.toEqual(['claude']) + expect(hydrateShellPathMock).not.toHaveBeenCalled() + }) + it('detects Mistral Vibe from the installed vibe executable', async () => { execFileAsyncMock.mockImplementation(async (command, args) => { if (command !== 'which') { diff --git a/src/main/ipc/preflight.ts b/src/main/ipc/preflight.ts index 8d65e0acc..fbe00db46 100644 --- a/src/main/ipc/preflight.ts +++ b/src/main/ipc/preflight.ts @@ -15,6 +15,7 @@ import { runPreflightCommandInWsl } from './preflight-wsl-command' import { detectCommandsInInstallDirs } from './local-agent-install-dir-detection' import { buildLocalPreflightEnv } from './preflight-local-env' import { getPreflightWslTarget, type PreflightRuntimeContext } from './preflight-runtime-target' +import { hydrateShellPathForAgentDetection } from './agent-detection-shell-path' const execFileAsync = promisify(execFile) const PREFLIGHT_COMMAND_TIMEOUT_MS = 5000 @@ -195,6 +196,13 @@ export async function detectInstalledAgents(context?: PreflightRuntimeContext): return uniqueAgentIds(checks.filter((c) => c.installed).map((c) => c.id)) } +export async function detectInstalledAgentsWithShellPathHydration( + context?: PreflightRuntimeContext +): Promise { + await hydrateShellPathForAgentDetection(context) + return detectInstalledAgents(context) +} + export type RefreshAgentsResult = { /** Agents detected after hydrating PATH from the user's login shell. */ agents: string[] @@ -352,11 +360,8 @@ export function registerPreflightHandlers(): void { } ) - ipcMain.handle( - 'preflight:detectAgents', - async (_event, args?: PreflightRuntimeContext): Promise => { - return detectInstalledAgents(args) - } + ipcMain.handle('preflight:detectAgents', async (_event, args?: PreflightRuntimeContext) => + detectInstalledAgentsWithShellPathHydration(args) ) ipcMain.handle('preflight:refreshAgents', async (_event, args?: PreflightRuntimeContext) => { diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 096c3ecf7..25dfa727a 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -164,7 +164,7 @@ const { addGitHubIssueCommentMock, listGitHubLabelsMock, listGitHubAssignableUsersMock, - detectInstalledAgentsMock, + detectInstalledAgentsWithShellPathHydrationMock, detectRemoteAgentsMock, listGitLabMergeRequestsMock, listGitLabWorkItemsMock, @@ -255,7 +255,7 @@ const { addGitHubIssueCommentMock: vi.fn(), listGitHubLabelsMock: vi.fn(), listGitHubAssignableUsersMock: vi.fn(), - detectInstalledAgentsMock: vi.fn(), + detectInstalledAgentsWithShellPathHydrationMock: vi.fn(), detectRemoteAgentsMock: vi.fn(), listGitLabMergeRequestsMock: vi.fn(), listGitLabWorkItemsMock: vi.fn(), @@ -319,7 +319,7 @@ vi.mock('../ipc/ssh', () => ({ })) vi.mock('../ipc/preflight', () => ({ - detectInstalledAgents: detectInstalledAgentsMock, + detectInstalledAgentsWithShellPathHydration: detectInstalledAgentsWithShellPathHydrationMock, detectRemoteAgents: detectRemoteAgentsMock })) @@ -620,8 +620,8 @@ function resetRuntimeTestMocks(): void { listGitHubLabelsMock.mockResolvedValue([]) listGitHubAssignableUsersMock.mockReset() listGitHubAssignableUsersMock.mockResolvedValue([]) - detectInstalledAgentsMock.mockReset() - detectInstalledAgentsMock.mockResolvedValue([]) + detectInstalledAgentsWithShellPathHydrationMock.mockReset() + detectInstalledAgentsWithShellPathHydrationMock.mockResolvedValue([]) detectRemoteAgentsMock.mockReset() detectRemoteAgentsMock.mockResolvedValue([]) listGitLabMergeRequestsMock.mockReset() @@ -17898,7 +17898,7 @@ describe('OrcaRuntimeService', () => { }) it('uses desktop task agent selection and bracketed-pastes startup drafts for local worktrees', async () => { - detectInstalledAgentsMock.mockResolvedValue(['claude']) + detectInstalledAgentsWithShellPathHydrationMock.mockResolvedValue(['claude']) const metaById: Record = {} const runtimeStore = { ...store, @@ -17959,7 +17959,7 @@ describe('OrcaRuntimeService', () => { activate: true }) - expect(detectInstalledAgentsMock).not.toHaveBeenCalled() + expect(detectInstalledAgentsWithShellPathHydrationMock).not.toHaveBeenCalled() expect(detectRemoteAgentsMock).not.toHaveBeenCalled() expect(spawn).toHaveBeenCalledWith( expect.objectContaining({ @@ -18215,7 +18215,7 @@ describe('OrcaRuntimeService', () => { }) it('records the resolved fallback agent when the requested startup draft agent is disabled', async () => { - detectInstalledAgentsMock.mockResolvedValue(['claude']) + detectInstalledAgentsWithShellPathHydrationMock.mockResolvedValue(['claude']) const metaById: Record = {} const runtimeStore = { ...store, @@ -18276,7 +18276,7 @@ describe('OrcaRuntimeService', () => { activate: true }) - expect(detectInstalledAgentsMock).toHaveBeenCalled() + expect(detectInstalledAgentsWithShellPathHydrationMock).toHaveBeenCalled() expect(spawn).toHaveBeenCalledWith( expect.objectContaining({ cwd: '/tmp/workspaces/runtime-fallback-draft', @@ -18405,7 +18405,7 @@ describe('OrcaRuntimeService', () => { }) it('lets explicit startup draft agents override the desktop default', async () => { - detectInstalledAgentsMock.mockResolvedValue([]) + detectInstalledAgentsWithShellPathHydrationMock.mockResolvedValue([]) const metaById: Record = {} const runtimeStore = { ...store, @@ -18467,7 +18467,7 @@ describe('OrcaRuntimeService', () => { activate: true }) - expect(detectInstalledAgentsMock).not.toHaveBeenCalled() + expect(detectInstalledAgentsWithShellPathHydrationMock).not.toHaveBeenCalled() expect(detectRemoteAgentsMock).not.toHaveBeenCalled() expect(spawn).toHaveBeenCalledWith( expect.objectContaining({ @@ -18485,7 +18485,7 @@ describe('OrcaRuntimeService', () => { }) it('does not auto-launch an agent for startup drafts when the default is blank', async () => { - detectInstalledAgentsMock.mockResolvedValue(['claude', 'codex']) + detectInstalledAgentsWithShellPathHydrationMock.mockResolvedValue(['claude', 'codex']) const metaById: Record = {} const runtimeStore = { ...store, @@ -18545,7 +18545,7 @@ describe('OrcaRuntimeService', () => { activate: true }) - expect(detectInstalledAgentsMock).not.toHaveBeenCalled() + expect(detectInstalledAgentsWithShellPathHydrationMock).not.toHaveBeenCalled() expect(detectRemoteAgentsMock).not.toHaveBeenCalled() expect(spawn).not.toHaveBeenCalled() expect(metaById[result.worktree.id]?.createdWithAgent).toBeUndefined() @@ -18638,7 +18638,7 @@ describe('OrcaRuntimeService', () => { }) expect(detectRemoteAgentsMock).toHaveBeenCalledWith({ connectionId: 'ssh-1' }) - expect(detectInstalledAgentsMock).not.toHaveBeenCalled() + expect(detectInstalledAgentsWithShellPathHydrationMock).not.toHaveBeenCalled() expect(spawn).toHaveBeenCalledWith( expect.objectContaining({ cwd: '/remote/mobile-startup-draft', diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index a0ca7efca..bd81e59fc 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -172,7 +172,7 @@ import { resolveTuiAgentLaunchEnv } from '../../shared/tui-agent-launch-defaults' import { isTuiAgent, TUI_AGENT_CONFIG } from '../../shared/tui-agent-config' -import { detectInstalledAgents, detectRemoteAgents } from '../ipc/preflight' +import { detectInstalledAgentsWithShellPathHydration, detectRemoteAgents } from '../ipc/preflight' import { markCodexProjectTrusted, markCopilotFolderTrusted, @@ -11436,9 +11436,10 @@ export class OrcaRuntimeService { if (!agent) { let detected: string[] = [] try { + // Why: startup-draft fallback can run from sparse runtime launch envs too. detected = repo.connectionId ? await detectRemoteAgents({ connectionId: repo.connectionId }) - : await detectInstalledAgents() + : await detectInstalledAgentsWithShellPathHydration() } catch { detected = [] } diff --git a/src/main/runtime/rpc/methods/preflight.test.ts b/src/main/runtime/rpc/methods/preflight.test.ts index 51a1231ca..d28b97b8e 100644 --- a/src/main/runtime/rpc/methods/preflight.test.ts +++ b/src/main/runtime/rpc/methods/preflight.test.ts @@ -5,19 +5,19 @@ import type { OrcaRuntimeService } from '../../orca-runtime' import { PREFLIGHT_METHODS } from './preflight' const { - detectInstalledAgentsMock, + detectInstalledAgentsWithShellPathHydrationMock, detectRemoteAgentsMock, refreshShellPathAndDetectAgentsMock, runPreflightCheckMock } = vi.hoisted(() => ({ - detectInstalledAgentsMock: vi.fn(), + detectInstalledAgentsWithShellPathHydrationMock: vi.fn(), detectRemoteAgentsMock: vi.fn(), refreshShellPathAndDetectAgentsMock: vi.fn(), runPreflightCheckMock: vi.fn() })) vi.mock('../../../ipc/preflight', () => ({ - detectInstalledAgents: detectInstalledAgentsMock, + detectInstalledAgentsWithShellPathHydration: detectInstalledAgentsWithShellPathHydrationMock, detectRemoteAgents: detectRemoteAgentsMock, refreshShellPathAndDetectAgents: refreshShellPathAndDetectAgentsMock, runPreflightCheck: runPreflightCheckMock @@ -46,7 +46,7 @@ describe('preflight RPC methods', () => { }) it('detects agents and refreshes PATH on the server through runtime RPC', async () => { - detectInstalledAgentsMock.mockResolvedValueOnce(['codex']) + detectInstalledAgentsWithShellPathHydrationMock.mockResolvedValueOnce(['codex']) refreshShellPathAndDetectAgentsMock.mockResolvedValueOnce({ agents: ['codex', 'claude'], addedPathSegments: ['/opt/bin'], @@ -60,7 +60,7 @@ describe('preflight RPC methods', () => { const detected = await dispatcher.dispatch(makeRequest('preflight.detectAgents')) const refreshed = await dispatcher.dispatch(makeRequest('preflight.refreshAgents')) - expect(detectInstalledAgentsMock).toHaveBeenCalled() + expect(detectInstalledAgentsWithShellPathHydrationMock).toHaveBeenCalled() expect(refreshShellPathAndDetectAgentsMock).toHaveBeenCalled() expect(detected).toMatchObject({ ok: true, result: ['codex'] }) expect(refreshed).toMatchObject({ diff --git a/src/main/runtime/rpc/methods/preflight.ts b/src/main/runtime/rpc/methods/preflight.ts index 720a823cc..4e0f06201 100644 --- a/src/main/runtime/rpc/methods/preflight.ts +++ b/src/main/runtime/rpc/methods/preflight.ts @@ -2,7 +2,7 @@ import { z } from 'zod' import { defineMethod, type RpcMethod } from '../core' import { detectRemoteAgents, - detectInstalledAgents, + detectInstalledAgentsWithShellPathHydration, refreshShellPathAndDetectAgents, runPreflightCheck } from '../../../ipc/preflight' @@ -23,7 +23,7 @@ export const PREFLIGHT_METHODS: RpcMethod[] = [ defineMethod({ name: 'preflight.detectAgents', params: null, - handler: async () => detectInstalledAgents() + handler: async () => detectInstalledAgentsWithShellPathHydration() }), defineMethod({ name: 'preflight.detectRemoteAgents', diff --git a/src/relay/preflight-handler.test.ts b/src/relay/preflight-handler.test.ts index b2dfffe8e..6d2f9339b 100644 --- a/src/relay/preflight-handler.test.ts +++ b/src/relay/preflight-handler.test.ts @@ -1,5 +1,49 @@ -import { describe, expect, it } from 'vitest' -import { buildCommandLookupSpec, hasAbsoluteCommandPath } from './preflight-handler' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { execFileAsyncMock } = vi.hoisted(() => ({ + execFileAsyncMock: vi.fn() +})) + +vi.mock('child_process', () => { + const execFileWithPromisify = Object.assign(vi.fn(), { + [Symbol.for('nodejs.util.promisify.custom')]: execFileAsyncMock + }) + return { execFile: execFileWithPromisify } +}) + +import { + buildCommandLookupSpec, + buildCommandLookupSpecs, + hasAbsoluteCommandPath, + isCommandOnPathForRelay +} from './preflight-handler' + +function lookupArgs(command: string, mode: '-lc' | '-ilc' = '-lc'): string[] { + return [ + mode, + [ + `if resolved=$(command -v ${command} 2>/dev/null); then`, + 'printf \'__ORCA_AGENT_PATH__%s\\n\' "$resolved"', + 'fi' + ].join('\n') + ] +} + +function fishLookupArgs(command: string): string[] { + return [ + '-ilc', + [ + `set -l resolved (command -v ${command} 2>/dev/null)`, + 'if test -n "$resolved"', + 'printf \'__ORCA_AGENT_PATH__%s\\n\' "$resolved"', + 'end' + ].join('\n') + ] +} + +beforeEach(() => { + execFileAsyncMock.mockReset() +}) describe('buildCommandLookupSpec', () => { it('uses where.exe on native Windows SSH hosts', () => { @@ -10,15 +54,160 @@ describe('buildCommandLookupSpec', () => { }) }) - it('passes the command as an argument to the POSIX login-shell probe', () => { - expect(buildCommandLookupSpec('codex', 'linux')).toEqual({ + it('falls back to sh for POSIX probes without a configured shell', () => { + expect(buildCommandLookupSpec('codex', 'linux', {}, null)).toEqual({ file: '/bin/sh', - args: ['-lc', 'command -v "$1"', 'sh', 'codex'] + args: lookupArgs("'codex'") + }) + }) + + it('uses the configured remote shell for POSIX probes', () => { + expect(buildCommandLookupSpec('codex', 'linux', { SHELL: '/bin/zsh' }, '/bin/zsh')).toEqual({ + file: '/bin/zsh', + args: lookupArgs("'codex'", '-ilc') + }) + }) + + it('quotes command names in shell probes', () => { + expect( + buildCommandLookupSpec("agent'cli", 'linux', { SHELL: '/bin/bash' }, '/bin/bash') + ).toEqual({ + file: '/bin/bash', + args: lookupArgs("'agent'\\''cli'", '-ilc') + }) + }) +}) + +describe('buildCommandLookupSpecs', () => { + it('falls back to inherited PATH after a trusted configured POSIX shell', () => { + expect(buildCommandLookupSpecs('codex', 'linux', { SHELL: '/bin/zsh' }, '/bin/zsh')).toEqual([ + { file: '/bin/zsh', args: lookupArgs("'codex'", '-ilc') }, + { file: '/bin/sh', args: lookupArgs("'codex'") } + ]) + }) + + it('allows a custom shell path only when the account login shell matches', () => { + expect( + buildCommandLookupSpecs( + 'codex', + 'darwin', + { SHELL: '/opt/homebrew/bin/zsh' }, + '/opt/homebrew/bin/zsh' + ) + ).toEqual([ + { file: '/opt/homebrew/bin/zsh', args: lookupArgs("'codex'", '-ilc') }, + { file: '/bin/sh', args: lookupArgs("'codex'") } + ]) + }) + + it('allows conservative system shell paths when account lookup is unavailable', () => { + expect(buildCommandLookupSpecs('codex', 'linux', { SHELL: '/usr/bin/bash' }, null)[0]).toEqual({ + file: '/usr/bin/bash', + args: lookupArgs("'codex'", '-ilc') + }) + }) + + it('uses fish syntax for trusted fish shells', () => { + expect(buildCommandLookupSpecs('codex', 'linux', { SHELL: '/usr/bin/fish' }, null)[0]).toEqual({ + file: '/usr/bin/fish', + args: fishLookupArgs("'codex'") + }) + }) + + it('ignores untrusted temp shell paths even when the basename is supported', () => { + expect(buildCommandLookupSpecs('codex', 'linux', { SHELL: '/tmp/zsh' }, '/bin/bash')).toEqual([ + { file: '/bin/sh', args: lookupArgs("'codex'") } + ]) + }) + + it('ignores untrusted home-bin shell paths even when the basename is supported', () => { + expect( + buildCommandLookupSpecs('codex', 'linux', { SHELL: '/home/test/bin/bash' }, '/bin/bash') + ).toEqual([{ file: '/bin/sh', args: lookupArgs("'codex'") }]) + }) +}) + +describe('isCommandOnPathForRelay', () => { + it('falls back to inherited PATH when shell startup returns no absolute command path', async () => { + execFileAsyncMock + .mockResolvedValueOnce({ stdout: 'welcome\ncodex is a function\n' }) + .mockResolvedValueOnce({ stdout: '__ORCA_AGENT_PATH__/relay/path/codex\n' }) + + await expect( + isCommandOnPathForRelay('codex', { + platform: 'linux', + env: { SHELL: '/bin/zsh', PATH: '/usr/bin' }, + accountLoginShell: '/bin/zsh' + }) + ).resolves.toBe(true) + expect(execFileAsyncMock).toHaveBeenNthCalledWith( + 1, + '/bin/zsh', + lookupArgs("'codex'", '-ilc'), + { + encoding: 'utf-8', + env: expect.objectContaining({ SHELL: '/bin/zsh' }), + timeout: 5000 + } + ) + expect(execFileAsyncMock).toHaveBeenNthCalledWith(2, '/bin/sh', lookupArgs("'codex'"), { + encoding: 'utf-8', + env: expect.objectContaining({ SHELL: '/bin/zsh' }), + timeout: 5000 + }) + }) + + it('falls back to inherited PATH when shell startup fails', async () => { + execFileAsyncMock + .mockRejectedValueOnce(new Error('startup failed')) + .mockResolvedValueOnce({ stdout: '__ORCA_AGENT_PATH__/relay/path/codex\n' }) + + await expect( + isCommandOnPathForRelay('codex', { + platform: 'linux', + env: { SHELL: '/bin/bash', PATH: '/usr/bin' }, + accountLoginShell: '/bin/bash' + }) + ).resolves.toBe(true) + expect(execFileAsyncMock).toHaveBeenCalledTimes(2) + }) + + it('does not execute an untrusted configured shell before inherited PATH lookup', async () => { + execFileAsyncMock.mockResolvedValueOnce({ stdout: '__ORCA_AGENT_PATH__/relay/path/codex\n' }) + + await expect( + isCommandOnPathForRelay('codex', { + platform: 'linux', + env: { SHELL: '/tmp/zsh', PATH: '/usr/bin' }, + accountLoginShell: '/bin/bash' + }) + ).resolves.toBe(true) + expect(execFileAsyncMock).toHaveBeenCalledTimes(1) + expect(execFileAsyncMock).toHaveBeenCalledWith('/bin/sh', lookupArgs("'codex'"), { + encoding: 'utf-8', + env: expect.objectContaining({ SHELL: '/tmp/zsh' }), + timeout: 5000 }) }) }) describe('hasAbsoluteCommandPath', () => { + it('ignores banners and shell function output', () => { + expect(hasAbsoluteCommandPath('/tmp/not-the-agent\ncodex is a shell function\n', 'linux')).toBe( + false + ) + }) + + it('ignores unmarked POSIX absolute paths from shell startup output', () => { + expect(hasAbsoluteCommandPath('/tmp/not-the-agent\n', 'linux')).toBe(false) + }) + + it('recognizes a sentinel-marked command path amid shell startup and exit output', () => { + expect( + hasAbsoluteCommandPath('welcome\n__ORCA_AGENT_PATH__/opt/bin/codex\nlogout-banner\n', 'linux') + ).toBe(true) + }) + it('recognizes Windows absolute command paths', () => { expect( hasAbsoluteCommandPath('C:\\Users\\alice\\AppData\\Roaming\\npm\\codex.cmd\r\n', 'win32') diff --git a/src/relay/preflight-handler.ts b/src/relay/preflight-handler.ts index fb4724832..a0382d772 100644 --- a/src/relay/preflight-handler.ts +++ b/src/relay/preflight-handler.ts @@ -1,4 +1,5 @@ import { execFile } from 'child_process' +import { userInfo } from 'os' import { promisify } from 'util' import path, { win32 } from 'path' import type { RelayDispatcher } from './dispatcher' @@ -12,6 +13,16 @@ type CommandLookupSpec = { windowsHide?: true } +type RelayCommandLookupOptions = { + platform?: NodeJS.Platform + env?: NodeJS.ProcessEnv + accountLoginShell?: string | null +} + +const SUPPORTED_POSIX_SHELLS = new Set(['sh', 'dash', 'bash', 'zsh', 'fish']) +const CONSERVATIVE_SYSTEM_SHELL_DIRS = new Set(['/bin', '/usr/bin']) +const AGENT_PATH_PREFIX = '__ORCA_AGENT_PATH__' + export class PreflightHandler { private dispatcher: RelayDispatcher @@ -43,35 +54,77 @@ export class PreflightHandler { return { agents: [...new Set(results.filter((r) => r.installed).map((r) => r.id))] } } - // Why: SSH exec channels give the relay a minimal environment without - // .zprofile/.bash_profile sourced. Running `which` directly would miss - // agents installed via Homebrew, nvm, cargo, pipx, etc. Spawning a login - // shell (`-lc`) ensures PATH matches what the user's PTY sessions see. - // Windows has no /bin/sh on native OpenSSH hosts, so use where.exe there. + // Why: SSH exec channels give the relay a minimal environment without shell + // startup files sourced. Ask the user's configured shell so agent dirs added + // by zsh/bash/fish startup hooks match the remote terminal experience. + // Windows has no POSIX shell on native OpenSSH hosts, so use where.exe there. private async isCommandOnPath(command: string): Promise { - try { - const spec = buildCommandLookupSpec(command, process.platform) - const { stdout } = await execFileAsync(spec.file, spec.args, { - encoding: 'utf-8', - env: buildRelayCommandEnv(), - timeout: 5000, - ...(spec.windowsHide ? { windowsHide: true } : {}) - }) - return hasAbsoluteCommandPath(stdout, process.platform) - } catch { - return false - } + return isCommandOnPathForRelay(command) } } export function buildCommandLookupSpec( command: string, - platform: NodeJS.Platform + platform: NodeJS.Platform, + env: NodeJS.ProcessEnv = process.env, + accountLoginShell?: string | null ): CommandLookupSpec { + const [spec] = buildCommandLookupSpecs(command, platform, env, accountLoginShell) + return spec ?? buildPosixCommandLookupSpec(command, '/bin/sh') +} + +export function buildCommandLookupSpecs( + command: string, + platform: NodeJS.Platform, + env: NodeJS.ProcessEnv = process.env, + accountLoginShell?: string | null +): CommandLookupSpec[] { if (platform === 'win32') { - return { file: 'where.exe', args: [command], windowsHide: true } + return [{ file: 'where.exe', args: [command], windowsHide: true }] } - return { file: '/bin/sh', args: ['-lc', 'command -v "$1"', 'sh', command] } + const trustedShell = pickTrustedPosixShell( + env, + resolveAccountLoginShell(platform, accountLoginShell) + ) + const specs: CommandLookupSpec[] = [] + + if (trustedShell) { + specs.push(buildPosixCommandLookupSpec(command, trustedShell)) + } + + const inheritedPathSpec = buildPosixCommandLookupSpec(command, '/bin/sh') + if (!trustedShell || trustedShell !== inheritedPathSpec.file) { + specs.push(inheritedPathSpec) + } + + return specs +} + +export async function isCommandOnPathForRelay( + command: string, + options: RelayCommandLookupOptions = {} +): Promise { + const platform = options.platform ?? process.platform + const env = options.env ?? process.env + const specs = buildCommandLookupSpecs(command, platform, env, options.accountLoginShell) + + for (const spec of specs) { + try { + const { stdout } = await execFileAsync(spec.file, spec.args, { + encoding: 'utf-8', + env: buildRelayCommandEnv(env, platform), + timeout: 5000, + ...(spec.windowsHide ? { windowsHide: true } : {}) + }) + if (hasAbsoluteCommandPath(stdout, platform)) { + return true + } + } catch { + // Try the inherited-PATH fallback before reporting the agent missing. + } + } + + return false } export function hasAbsoluteCommandPath(output: string, platform: NodeJS.Platform): boolean { @@ -79,5 +132,86 @@ export function hasAbsoluteCommandPath(output: string, platform: NodeJS.Platform return output .split(/\r?\n/) .map((line) => line.trim()) - .some((line) => pathOps.isAbsolute(line)) + .some((line) => { + const resolvedPath = + platform === 'win32' + ? line + : line.startsWith(AGENT_PATH_PREFIX) + ? line.slice(AGENT_PATH_PREFIX.length) + : '' + return pathOps.isAbsolute(resolvedPath) + }) +} + +function buildPosixCommandLookupSpec(command: string, shell: string): CommandLookupSpec { + const shellName = path.posix.basename(shell).toLowerCase() + if (shellName === 'fish') { + return { file: shell, args: ['-ilc', buildFishCommandLookupScript(command)] } + } + return { file: shell, args: [getShellCommandMode(shell), buildShCommandLookupScript(command)] } +} + +function buildShCommandLookupScript(command: string): string { + const quotedCommand = shellQuote(command) + return [ + `if resolved=$(command -v ${quotedCommand} 2>/dev/null); then`, + `printf '${AGENT_PATH_PREFIX}%s\\n' "$resolved"`, + 'fi' + ].join('\n') +} + +function buildFishCommandLookupScript(command: string): string { + const quotedCommand = shellQuote(command) + return [ + `set -l resolved (command -v ${quotedCommand} 2>/dev/null)`, + 'if test -n "$resolved"', + `printf '${AGENT_PATH_PREFIX}%s\\n' "$resolved"`, + 'end' + ].join('\n') +} + +function resolveAccountLoginShell( + platform: NodeJS.Platform, + accountLoginShell?: string | null +): string | null { + if (accountLoginShell !== undefined) { + return accountLoginShell + } + if (platform === 'win32') { + return null + } + try { + return userInfo().shell ?? null + } catch { + return null + } +} + +function pickTrustedPosixShell( + env: NodeJS.ProcessEnv, + accountLoginShell: string | null +): string | null { + const shell = env.SHELL + if (!shell || !path.posix.isAbsolute(shell)) { + return null + } + const shellName = path.posix.basename(shell).toLowerCase() + if (!SUPPORTED_POSIX_SHELLS.has(shellName)) { + return null + } + if (accountLoginShell) { + return shell === accountLoginShell ? shell : null + } + return CONSERVATIVE_SYSTEM_SHELL_DIRS.has(path.posix.dirname(shell)) ? shell : null +} + +function getShellCommandMode(shell: string): '-lc' | '-ilc' { + const shellName = path.posix.basename(shell).toLowerCase() + // Why: bash/zsh/fish users commonly add package-manager bins from interactive + // startup files. POSIX sh/dash may not support interactive login flags. + return shellName === 'sh' || shellName === 'dash' ? '-lc' : '-ilc' +} + +function shellQuote(value: string): string { + return `'${value.replace(/'/g, "'\\''")}'` }