mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
fix(codex): pair structured launches with the CLI runtime (#32061)
* fix(codex): pair structured launches with the CLI runtime * test(codex): align launch snapshots with runtime pairing * fix(agent-session): keep this app's CLI ahead of the paired runtime dir Pairing ran after the Orca CLI prepend, so a runtime dir that also holds an `orca` (e.g. /usr/local/bin with node+codex+orca) shadowed this app's launcher for the child. Pair first; the CLI dir stays first on PATH. Drops a PATH-shape test the parity snapshots already cover. Co-Authored-By: Claude <noreply@anthropic.com> * fix(cli-runtime): skip compiled CLIs and keep inherited PATH when pairing a sibling node A native codex/claude beside a node (Homebrew, /usr/bin) never runs on it, so pairing only reordered PATH. An env naming no PATH now pairs over the PATH the child inherits instead of replacing it with the CLI dir alone. Co-Authored-By: Claude <noreply@anthropic.com> * test(cli-runtime): type the PATH-less env fixture Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -69,7 +69,7 @@ async function readClaudeChildEnvSources(deps: ClaudeEnvDeps): Promise<ClaudeChi
|
||||
export function claudeChildEnv(
|
||||
sources: ClaudeChildEnvSources,
|
||||
decorateEnv: (env: Record<string, string>) => Record<string, string> = (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<string, string> {
|
||||
const env = decorateEnv({
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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<string, string>,
|
||||
sessionId: string
|
||||
sessionId: string,
|
||||
command = 'codex'
|
||||
): Promise<Record<string, string>> {
|
||||
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.
|
||||
|
||||
@@ -119,7 +119,8 @@ export function codexStructuredLaunchPart(
|
||||
inheritedEnvToDelete: [
|
||||
...(visuals ? [] : [NATIVE_CHAT_SCRATCH_DIR_ENV]),
|
||||
LEGACY_NATIVE_CHAT_VISUALS_DIR_ENV
|
||||
]
|
||||
],
|
||||
cliRuntimeCommand: command
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -564,7 +564,7 @@ exports[`native-chat launch parity > claude floating fresh 1`] = `
|
||||
"ORCA_USER_DATA_PATH": "<root>/user-data",
|
||||
"ORCA_WORKTREE_ID": "parent-worktree",
|
||||
"PARITY_OVERLAY": "claude",
|
||||
"PATH": "<root>/bin:<root>/orca-cli/bin:/opt/login-shell/bin:/usr/bin:/bin",
|
||||
"PATH": "<root>/orca-cli/bin:<root>/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": "<root>/user-data",
|
||||
"ORCA_WORKTREE_ID": "parent-worktree",
|
||||
"PARITY_OVERLAY": "claude",
|
||||
"PATH": "<root>/bin:<root>/orca-cli/bin:/opt/login-shell/bin:/usr/bin:/bin",
|
||||
"PATH": "<root>/orca-cli/bin:<root>/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": "<root>/user-data",
|
||||
"ORCA_WORKTREE_ID": "parent-worktree",
|
||||
"PARITY_OVERLAY": "claude",
|
||||
"PATH": "<root>/bin:<root>/orca-cli/bin:/opt/login-shell/bin:/usr/bin:/bin",
|
||||
"PATH": "<root>/orca-cli/bin:<root>/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": "<root>/user-data",
|
||||
"ORCA_WORKTREE_ID": "parent-worktree",
|
||||
"PARITY_OVERLAY": "claude",
|
||||
"PATH": "<root>/bin:<root>/orca-cli/bin:/opt/login-shell/bin:/usr/bin:/bin",
|
||||
"PATH": "<root>/orca-cli/bin:<root>/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": "<root>/user-data",
|
||||
"ORCA_WORKTREE_ID": "parent-worktree",
|
||||
"PARITY_OVERLAY": "claude",
|
||||
"PATH": "<root>/bin:<root>/orca-cli/bin:/opt/login-shell/bin:/usr/bin:/bin",
|
||||
"PATH": "<root>/orca-cli/bin:<root>/bin:/opt/login-shell/bin:/usr/bin:/bin",
|
||||
"SHELL_ONLY": "shell",
|
||||
"SHELL_UNLISTED": "shell",
|
||||
},
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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'
|
||||
}
|
||||
@@ -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<T extends NodeJS.ProcessEnv>(
|
||||
commandPath: string,
|
||||
@@ -368,12 +354,16 @@ export function withCliRuntimeOnPath<T extends NodeJS.ProcessEnv>(
|
||||
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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user