diff --git a/src/main/claude/claude-structured-child-env.ts b/src/main/claude/claude-structured-child-env.ts index 19605b467fc..6056dd39c6f 100644 --- a/src/main/claude/claude-structured-child-env.ts +++ b/src/main/claude/claude-structured-child-env.ts @@ -69,7 +69,7 @@ async function readClaudeChildEnvSources(deps: ClaudeEnvDeps): Promise) => Record = (env) => env, - /** False when the session's seal pairs the CLI runtime itself, after the session's identity. */ + /** False when the session's seal pairs the CLI runtime itself, behind this app's CLI. */ { runtimeOnPath = true }: { runtimeOnPath?: boolean } = {} ): Record { const env = decorateEnv({ diff --git a/src/main/claude/claude-structured-launch-resolution.test.ts b/src/main/claude/claude-structured-launch-resolution.test.ts index 63c6ac8ba12..afc72aa9109 100644 --- a/src/main/claude/claude-structured-launch-resolution.test.ts +++ b/src/main/claude/claude-structured-launch-resolution.test.ts @@ -530,7 +530,11 @@ describe('claude structured launch resolution', () => { }) })({ identity: IDENTITY }) - expect((launch.env?.PATH ?? launch.env?.Path)?.split(delimiter)[0]).toBe(binDir) + // Right behind this app's CLI, so a runtime dir holding another `orca` cannot shadow it. + expect((launch.env?.PATH ?? launch.env?.Path)?.split(delimiter).slice(1)).toEqual([ + binDir, + '/usr/bin' + ]) }) it('refuses other hosts, WSL, providers, and account-home variables', async () => { diff --git a/src/main/codex-cli/command.test.ts b/src/main/codex-cli/command.test.ts index b7e4c6e87f1..5fd69be62b4 100644 --- a/src/main/codex-cli/command.test.ts +++ b/src/main/codex-cli/command.test.ts @@ -349,6 +349,33 @@ describe('withCliRuntimeOnPath', () => { expect(withCliRuntimeOnPath(join(brew, 'codex'), env, { platform: 'darwin' })).toBe(env) }) + it('leaves PATH untouched for a compiled CLI beside a node (Homebrew, /usr/bin)', () => { + const root = makeCommandFixture('orca-pair-') + const brew = join(root, 'opt', 'homebrew', 'bin') + makeExecutable(join(brew, 'node')) + makeExecutable(join(brew, 'codex')) + writeFileSync(join(brew, 'codex'), Buffer.from([0xcf, 0xfa, 0xed, 0xfe, 0x07])) + const env = { PATH: '/usr/bin' } + + expect(withCliRuntimeOnPath(join(brew, 'codex'), env, { platform: 'darwin' })).toBe(env) + }) + + it('pairs over the inherited PATH when the env names none, instead of replacing it', () => { + const root = makeCommandFixture('orca-pair-') + const v20 = join(root, '.nvm', 'versions', 'node', 'v20.11.0', 'bin') + makeExecutable(join(v20, 'node')) + makeExecutable(join(v20, 'codex')) + const inherited = process.env.PATH + onTestFinished(() => { + process.env.PATH = inherited + }) + process.env.PATH = ['/opt/inherited/bin', '/usr/bin'].join(delimiter) + + const noPath: NodeJS.ProcessEnv = {} + const paired = withCliRuntimeOnPath(join(v20, 'codex'), noPath, { platform: 'darwin' }) + expect(paired.PATH).toBe([v20, '/opt/inherited/bin', '/usr/bin'].join(delimiter)) + }) + it('leaves PATH untouched for a bare command name', () => { const env = { PATH: '/usr/bin' } expect(withCliRuntimeOnPath('codex', env, { platform: 'darwin' })).toBe(env) diff --git a/src/main/codex/codex-structured-child-environment.test.ts b/src/main/codex/codex-structured-child-environment.test.ts index acb2bc7330d..67adfb369e4 100644 --- a/src/main/codex/codex-structured-child-environment.test.ts +++ b/src/main/codex/codex-structured-child-environment.test.ts @@ -1,4 +1,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { delimiter, join } from 'node:path' import { openCodexAppServerConnection } from './codex-app-server-connection' import { ORCA_AGENT_SESSION_SPAWN_TOKEN_ENV } from '../../shared/agent-session-caller-env' import { sealStructuredSessionChild } from '../runtime/structured-session-child-env' @@ -20,7 +23,8 @@ const DEV_CLI_LAUNCHER = /^[^:;]*[\\/]cli[\\/]bin[\\/]orca-dev$/ /** The sealed env a Codex launch of `sessionId` spawns with, over a shell env of `env`. */ async function codexChildEnv( env: Record, - sessionId: string + sessionId: string, + command = 'codex' ): Promise> { const record = agentSessionRecordFixture({ sessionId, @@ -30,7 +34,7 @@ async function codexChildEnv( const launch = await createCodexStructuredLaunchResolver({ store: { getRecord: () => record, pinLaunchDirectory: vi.fn() }, resolveEnvironment: async () => env, - resolveCommand: () => 'codex', + resolveCommand: () => command, resolveLaunchArgs: () => [] })({ // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a launch reads only the session id of its identity here. @@ -151,6 +155,42 @@ describe('the spawned Codex child', () => { vi.unstubAllEnvs() }) + it.skipIf(process.platform === 'win32')( + 'starts the persistent shebang CLI with its sibling Node over a competing PATH runtime', + async () => { + const root = mkdtempSync(join(tmpdir(), 'orca-codex-shebang-')) + try { + const selectedBin = join(root, 'selected', 'bin') + const competingBin = join(root, 'other', 'bin') + mkdirSync(selectedBin, { recursive: true }) + mkdirSync(competingBin, { recursive: true }) + symlinkSync(process.execPath, join(selectedBin, 'node')) + writeFileSync(join(competingBin, 'node'), '#!/bin/sh\nexit 79\n', { mode: 0o755 }) + const command = join(selectedBin, 'codex') + writeFileSync(command, `#!/usr/bin/env node\n${ENV_REPORTING_APP_SERVER}`, { mode: 0o755 }) + const env = await codexChildEnv({ PATH: competingBin }, 'shebang-session', command) + const connection = await openCodexAppServerConnection({ + command, + args: ['app-server'], + cwd: root, + env + }) + try { + await expect(connection.request('test/env')).resolves.toMatchObject({ + sessionId: 'shebang-session', + path: expect.stringMatching(DEV_CLI_BIN_FIRST), + cliCommand: expect.stringMatching(DEV_CLI_LAUNCHER) + }) + expect(env.PATH.split(delimiter).slice(1)).toEqual([selectedBin, competingBin]) + } finally { + await connection.close() + } + } finally { + rmSync(root, { recursive: true, force: true }) + } + } + ) + it("runs with its own session id and this app's CLI, over an id inherited by Orca itself", async () => { // The builder's output is an overlay on process.env, so only the spawned child proves the id // survives the merge — an Orca launched inside another session inherits that session's id. diff --git a/src/main/codex/codex-structured-launch-resolution.ts b/src/main/codex/codex-structured-launch-resolution.ts index 151f3bc7be9..e2361cbda33 100644 --- a/src/main/codex/codex-structured-launch-resolution.ts +++ b/src/main/codex/codex-structured-launch-resolution.ts @@ -119,7 +119,8 @@ export function codexStructuredLaunchPart( inheritedEnvToDelete: [ ...(visuals ? [] : [NATIVE_CHAT_SCRATCH_DIR_ENV]), LEGACY_NATIVE_CHAT_VISUALS_DIR_ENV - ] + ], + cliRuntimeCommand: command } } } diff --git a/src/main/runtime/__snapshots__/structured-agent-launch-parity.test.ts.snap b/src/main/runtime/__snapshots__/structured-agent-launch-parity.test.ts.snap index 819e1b55e84..88fcd2639dd 100644 --- a/src/main/runtime/__snapshots__/structured-agent-launch-parity.test.ts.snap +++ b/src/main/runtime/__snapshots__/structured-agent-launch-parity.test.ts.snap @@ -564,7 +564,7 @@ exports[`native-chat launch parity > claude floating fresh 1`] = ` "ORCA_USER_DATA_PATH": "/user-data", "ORCA_WORKTREE_ID": "parent-worktree", "PARITY_OVERLAY": "claude", - "PATH": "/bin:/orca-cli/bin:/opt/login-shell/bin:/usr/bin:/bin", + "PATH": "/orca-cli/bin:/bin:/opt/login-shell/bin:/usr/bin:/bin", "SHELL_ONLY": "shell", "SHELL_UNLISTED": "shell", }, @@ -652,7 +652,7 @@ exports[`native-chat launch parity > claude floating resumed 1`] = ` "ORCA_USER_DATA_PATH": "/user-data", "ORCA_WORKTREE_ID": "parent-worktree", "PARITY_OVERLAY": "claude", - "PATH": "/bin:/orca-cli/bin:/opt/login-shell/bin:/usr/bin:/bin", + "PATH": "/orca-cli/bin:/bin:/opt/login-shell/bin:/usr/bin:/bin", "SHELL_ONLY": "shell", "SHELL_UNLISTED": "shell", }, @@ -738,7 +738,7 @@ exports[`native-chat launch parity > claude worktree fresh 1`] = ` "ORCA_USER_DATA_PATH": "/user-data", "ORCA_WORKTREE_ID": "parent-worktree", "PARITY_OVERLAY": "claude", - "PATH": "/bin:/orca-cli/bin:/opt/login-shell/bin:/usr/bin:/bin", + "PATH": "/orca-cli/bin:/bin:/opt/login-shell/bin:/usr/bin:/bin", "SHELL_ONLY": "shell", "SHELL_UNLISTED": "shell", }, @@ -904,7 +904,7 @@ exports[`native-chat launch parity > claude worktree fresh worker 1`] = ` "ORCA_USER_DATA_PATH": "/user-data", "ORCA_WORKTREE_ID": "parent-worktree", "PARITY_OVERLAY": "claude", - "PATH": "/bin:/orca-cli/bin:/opt/login-shell/bin:/usr/bin:/bin", + "PATH": "/orca-cli/bin:/bin:/opt/login-shell/bin:/usr/bin:/bin", "SHELL_ONLY": "shell", "SHELL_UNLISTED": "shell", }, @@ -990,7 +990,7 @@ exports[`native-chat launch parity > claude worktree resumed 1`] = ` "ORCA_USER_DATA_PATH": "/user-data", "ORCA_WORKTREE_ID": "parent-worktree", "PARITY_OVERLAY": "claude", - "PATH": "/bin:/orca-cli/bin:/opt/login-shell/bin:/usr/bin:/bin", + "PATH": "/orca-cli/bin:/bin:/opt/login-shell/bin:/usr/bin:/bin", "SHELL_ONLY": "shell", "SHELL_UNLISTED": "shell", }, diff --git a/src/main/runtime/structured-session-child-env.ts b/src/main/runtime/structured-session-child-env.ts index 7ba6ebd5a71..87f652bc666 100644 --- a/src/main/runtime/structured-session-child-env.ts +++ b/src/main/runtime/structured-session-child-env.ts @@ -97,7 +97,7 @@ export type SealedStructuredSessionChild = { * `env` named as this session's child. `inheritedEnvToDelete` is required so every agent states * what it strips, none included; a key the seal itself names the child by is never stripped (a * registered worker keeps its handle). `cliRuntimeCommand`: a Node CLI whose own directory goes - * first on PATH, after the identity's, so it runs on the runtime beside it. + * on PATH right behind this app's CLI, so it runs on the runtime beside it. */ export function sealStructuredSessionChild(input: { sessionId: string @@ -106,13 +106,14 @@ export function sealStructuredSessionChild(input: { inheritedEnvToDelete: readonly string[] cliRuntimeCommand?: string }): SealedStructuredSessionChild { - const identity = childIdentity(input.sessionId, input.env) + // Why: pair before naming the child so this app's CLI dir stays ahead of a runtime dir holding another `orca`. + const paired = input.cliRuntimeCommand + ? withCliRuntimeOnPath(input.cliRuntimeCommand, input.env, { platform: process.platform }) + : input.env + const identity = childIdentity(input.sessionId, paired) const named = new Set([...Object.keys(identity.commandEnv), ORCA_AGENT_SESSION_SPAWN_TOKEN_ENV]) - const env = input.cliRuntimeCommand - ? withCliRuntimeOnPath(input.cliRuntimeCommand, identity.env, { platform: process.platform }) - : identity.env return { - env: { ...env, [ORCA_AGENT_SESSION_SPAWN_TOKEN_ENV]: input.spawnToken }, + env: { ...identity.env, [ORCA_AGENT_SESSION_SPAWN_TOKEN_ENV]: input.spawnToken }, envToDelete: input.inheritedEnvToDelete.filter((key) => !named.has(key)), commandEnv: identity.commandEnv } diff --git a/src/shared/cli-runtime-pairing-inputs.ts b/src/shared/cli-runtime-pairing-inputs.ts new file mode 100644 index 00000000000..ecabad5fae4 --- /dev/null +++ b/src/shared/cli-runtime-pairing-inputs.ts @@ -0,0 +1,43 @@ +import { closeSync, openSync, readSync } from 'node:fs' + +const NATIVE_EXECUTABLE_MAGIC = [ + [0x7f, 0x45, 0x4c, 0x46], // ELF + [0xcf, 0xfa, 0xed, 0xfe], // Mach-O 64-bit + [0xce, 0xfa, 0xed, 0xfe], // Mach-O 32-bit + [0xca, 0xfe, 0xba, 0xbe], // Mach-O universal + [0x4d, 0x5a] // Windows PE +] + +/** A compiled binary runs on no `node`, so pairing would only reorder PATH (Homebrew, /usr/bin). */ +export function isNativeExecutable(commandPath: string): boolean { + let fd: number | null = null + try { + fd = openSync(commandPath, 'r') + const head = Buffer.alloc(4) + const read = readSync(fd, head, 0, head.length, 0) + return NATIVE_EXECUTABLE_MAGIC.some( + (magic) => read >= magic.length && magic.every((byte, index) => head[index] === byte) + ) + } catch { + return false + } finally { + if (fd !== null) { + closeSync(fd) + } + } +} + +// Why: Win32 resolves env names case-insensitively and object order preserves +// the block order, so the entry the child will actually read is the FIRST +// case-insensitive match — not necessarily `Path` or `PATH`. Reading a narrower +// set than withCliRuntimeOnPath's twin dedupe deletes would destroy a third spelling unread. +// Mirrors resolvePathEnvKey in src/main/pty/windows-path-segment-merge.ts, which +// src/shared must not import. +export function firstWindowsPathEnvKey(env: NodeJS.ProcessEnv): string { + for (const key of Object.keys(env)) { + if (key.toLowerCase() === 'path' && env[key] !== undefined) { + return key + } + } + return 'Path' +} diff --git a/src/shared/node-cli-command-resolution.ts b/src/shared/node-cli-command-resolution.ts index 040894bfae8..a7446c0a1fe 100644 --- a/src/shared/node-cli-command-resolution.ts +++ b/src/shared/node-cli-command-resolution.ts @@ -1,6 +1,7 @@ import { accessSync, constants, existsSync, readFileSync, readdirSync, statSync } from 'node:fs' import { homedir } from 'node:os' import { delimiter, dirname, isAbsolute, join } from 'node:path' +import { firstWindowsPathEnvKey, isNativeExecutable } from './cli-runtime-pairing-inputs' import { getSystemCliInstallDirectories } from './system-cli-install-dirs' type ResolveCommandOptions = { @@ -329,21 +330,6 @@ export function resolveClaudeCommand(options: ResolveCommandOptions = {}): strin return resolveCliCommand('claude', options) } -// Why: Win32 resolves env names case-insensitively and object order preserves -// the block order, so the entry the child will actually read is the FIRST -// case-insensitive match — not necessarily `Path` or `PATH`. Reading a narrower -// set than the dedupe below deletes would destroy a third spelling unread. -// Mirrors resolvePathEnvKey in src/main/pty/windows-path-segment-merge.ts, which -// src/shared must not import. -function firstWindowsPathEnvKey(env: NodeJS.ProcessEnv): string { - for (const key of Object.keys(env)) { - if (key.toLowerCase() === 'path' && env[key] !== undefined) { - return key - } - } - return 'Path' -} - /** * Put a resolved CLI's own directory ahead of PATH when that directory ships a * sibling `node`. @@ -355,8 +341,8 @@ function firstWindowsPathEnvKey(env: NodeJS.ProcessEnv): string { * require (stablyai/orca#10932). Pair the binary with the runtime it was * installed against instead. * - * Only prepends when the sibling `node` really exists, so a CLI resolved from a - * directory that ships no node is left alone. + * Only prepends when the sibling `node` really exists and the CLI is not a + * compiled binary, so a native CLI or one beside no node is left alone. */ export function withCliRuntimeOnPath( commandPath: string, @@ -368,12 +354,16 @@ export function withCliRuntimeOnPath( return env } const commandDirectory = dirname(commandPath) - if (!findFirstExecutable(platform, [commandDirectory], getExecutableNames(platform, 'node'))) { + if ( + !findFirstExecutable(platform, [commandDirectory], getExecutableNames(platform, 'node')) || + isNativeExecutable(commandPath) + ) { return env } const pathKey = platform === 'win32' ? firstWindowsPathEnvKey(env) : 'PATH' const pathDelimiter = platform === 'win32' ? ';' : delimiter - const segments = splitPath(env[pathKey], pathDelimiter) + // Why: the child inherits Orca's PATH when `env` names none, so pair over that, not over nothing. + const segments = splitPath(env[pathKey] ?? process.env[pathKey], pathDelimiter) if (segments[0] === commandDirectory) { return env }