From a7505fd9110ac30587c8c526dbaadba282148a92 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 24 Aug 2026 22:30:04 -0700 Subject: [PATCH] fix(cli): spawn a version-manager CLI with its own node runtime (#16365) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cli): spawn a version-manager CLI with its own node runtime resolveCliCommand falls back to scanning every version-manager install when PATH misses, so it can hand back ~/.nvm/versions/node/v20.x/bin/codex while PATH still leads with v22. Nothing paired the binary with the runtime it was installed against, so its `#!/usr/bin/env node` shebang loaded a v20-built native module under a v22 ABI and the agent died on first require (#10932). Reproduced with a real addon rather than asserted: a CLI requiring a cpu-features build for NODE_MODULE_VERSION 115, spawned with v24 leading PATH, fails with ERR_DLOPEN_FAILED and exit 1. With the CLI's own bin directory prepended it runs clean. withCliRuntimeOnPath prepends the resolved command's directory when that directory ships a sibling node, and is a no-op otherwise — so a Homebrew or /usr/local CLI is untouched, and the WSL paths pass a bare `codex`/`claude` that is not absolute and so never matches. Host CLI resolution in the Claude login path is now lazy, keeping the WSL branch from resolving a host binary it never spawns. * fix(cli): split PATH on the delimiter we join with, pair app-server too Readiness review findings, all four addressed. withCliRuntimeOnPath chose its join delimiter from the platform option but split with the host's. Passing platform:'win32' from a posix host turned `C:\Windows;C:\Windows\System32` into `C;\Windows;C;\Windows\System32` — every drive letter torn off at its colon. Latent, since no shipped caller passes platform, but the sole win32 test was written against the corrupted value and asserted one split segment, so it green-lit the shredding. That test's other assertion was vacuous: it seeded only `Path`, so the `PATH` key it asserted absent could never exist. Deleting the whole case-dedupe block left the suite green. It now seeds both keys and asserts the full joined string; removing the block fails it. Nothing covered the wiring, and the argument choice is the easy thing to get silently wrong. Note it only diverges on win32 — on posix getSpawnArgsForWindows returns the CLI itself, so pairing the spawn command is indistinguishable there. The new test drives the win32 branch with a .cmd fixture; pairing spawnCmd or dropping the wrapper both fail it now. codex-trust-grant-host and codex-session-index-heal spawn the same `codex app-server` subcommand through runCodexAppServerSession and were left unpaired. Pair centrally there via a new optional cliPath, since invocation.command may be a cmd.exe wrapper. Pairing tests live in their own file: adding them inline pushed codex-fetcher.test.ts past the 800-line ratchet. * fix(cli): read the Windows path key the child will actually use Round-2 review finding. The read was narrower than the delete: the key was picked from exactly two spellings (`Path`, else `PATH`), while the twin dedupe removed every key whose lowercase form is `path`. A block spelling it `path` or `pATh` therefore had its value deleted without ever being read, handing the child a PATH containing only the CLI's own directory — a strictly worse outcome than not pairing at all. Win32 resolves env names case-insensitively and object order preserves block order, so the entry the child reads is the first case-insensitive match. The repo already encodes that rule in resolvePathEnvKey (src/main/pty/windows-path-segment-merge.ts); src/shared cannot import from src/main, so mirror it locally. Verified by execution across six env shapes: lowercase, mixed-case, Path-only, PATH-only, both twins, and a PATHEXT control that must not be touched. All preserve the original PATH; before the fix the first two lost it entirely. Reverting the selector fails the new test and nothing else. --- src/main/claude-accounts/service.ts | 23 +-- src/main/codex-accounts/service.ts | 5 +- src/main/codex-cli/command.test.ts | 77 ++++++++++ src/main/codex/codex-app-server-session.ts | 11 +- src/main/codex/codex-session-index-heal.ts | 1 + src/main/codex/codex-trust-grant-host.ts | 1 + src/main/rate-limits/claude-pty.ts | 5 +- .../codex-fetcher-runtime-pairing.test.ts | 133 ++++++++++++++++++ src/main/rate-limits/codex-fetcher.ts | 11 +- src/shared/node-cli-command-resolution.ts | 73 +++++++++- 10 files changed, 320 insertions(+), 20 deletions(-) create mode 100644 src/main/rate-limits/codex-fetcher-runtime-pairing.test.ts diff --git a/src/main/claude-accounts/service.ts b/src/main/claude-accounts/service.ts index 88e3712fde8..b5c4bc220f5 100644 --- a/src/main/claude-accounts/service.ts +++ b/src/main/claude-accounts/service.ts @@ -13,6 +13,7 @@ import type { import type { Store } from '../persistence' import type { RateLimitService } from '../rate-limits/service' import { resolveClaudeCommand } from '../codex-cli/command' +import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' import type { ClaudeRuntimeAuthService } from './runtime-auth-service' import { getClaudeManagedAccountsRoot, @@ -1050,17 +1051,21 @@ export class ClaudeAccountService { configDir.wslDistro === null && args[0] === 'auth' && args[1] === 'login' + // Why lazy: the WSL branch runs `claude` inside the distro, so resolving a + // host binary there would be wasted filesystem probing for a path never used. + let cachedHostClaudeCommand: string | null = null + const hostClaudeCommand = (): string => (cachedHostClaudeCommand ??= resolveClaudeCommand()) const interactiveLogin = isWindowsHostInteractiveLogin - ? buildWindowsHostInteractiveLoginSpawn(resolveClaudeCommand(), args) + ? buildWindowsHostInteractiveLoginSpawn(hostClaudeCommand(), args) : null const spawnConfig = interactiveLogin ? { command: interactiveLogin.command, args: interactiveLogin.args, - env: { + env: withCliRuntimeOnPath(hostClaudeCommand(), { ...process.env, CLAUDE_CONFIG_DIR: configDir.windowsPath - }, + }), shell: false, windowsVerbatimArguments: false } @@ -1081,20 +1086,20 @@ export class ClaudeAccountService { } : process.platform === 'win32' ? { - ...buildWindowsCommandInvocation(resolveClaudeCommand(), args), - env: { + ...buildWindowsCommandInvocation(hostClaudeCommand(), args), + env: withCliRuntimeOnPath(hostClaudeCommand(), { ...process.env, CLAUDE_CONFIG_DIR: configDir.windowsPath - }, + }), shell: false } : { - command: resolveClaudeCommand(), + command: hostClaudeCommand(), args, - env: { + env: withCliRuntimeOnPath(hostClaudeCommand(), { ...process.env, CLAUDE_CONFIG_DIR: configDir.windowsPath - }, + }), shell: false, windowsVerbatimArguments: false } diff --git a/src/main/codex-accounts/service.ts b/src/main/codex-accounts/service.ts index 5fd0a5ef649..56cfd630f2d 100644 --- a/src/main/codex-accounts/service.ts +++ b/src/main/codex-accounts/service.ts @@ -40,6 +40,7 @@ import { getSystemCodexHomePath } from '../codex/codex-home-paths' import { MANAGED_HOOK_TIMEOUT_SECONDS } from '../agent-hooks/installer-utils' import { readCodexTopLevelModelProvider } from '../codex/codex-model-provider-config' import { resolveCodexCommand } from '../codex-cli/command' +import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' import type { Store } from '../persistence' import type { RateLimitService } from '../rate-limits/service' import { parseWslUncPath } from '../../shared/wsl-paths' @@ -1736,10 +1737,10 @@ export class CodexAccountService { return { command: spawnCmd, args: spawnArgs, - env: { + env: withCliRuntimeOnPath(codexCommand, { ...process.env, CODEX_HOME: managedHomePath - }, + }), codexCommand, interactiveLogin } diff --git a/src/main/codex-cli/command.test.ts b/src/main/codex-cli/command.test.ts index 045048f2919..dbe1c358418 100644 --- a/src/main/codex-cli/command.test.ts +++ b/src/main/codex-cli/command.test.ts @@ -4,6 +4,7 @@ import { delimiter, dirname, join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import { getVersionManagerBinPaths, + withCliRuntimeOnPath, resolveClaudeCommand, resolveCliCommands, resolveCodexCommand @@ -313,3 +314,79 @@ describe('getVersionManagerBinPaths', () => { expect(paths).toContain(join(root, 'AppData', 'Roaming', 'npm')) }) }) + +describe('withCliRuntimeOnPath', () => { + it('pairs a version-manager CLI with its sibling node (stablyai/orca#10932)', () => { + const root = mkdtempSync(join(tmpdir(), 'orca-pair-')) + const v20 = join(root, '.nvm', 'versions', 'node', 'v20.11.0', 'bin') + const v22 = join(root, '.nvm', 'versions', 'node', 'v22.9.0', 'bin') + makeExecutable(join(v20, 'node')) + makeExecutable(join(v20, 'codex')) + makeExecutable(join(v22, 'node')) + + // default is v22, but codex only exists under v20 + const env = { PATH: [v22, '/usr/bin'].join(delimiter) } + const codex = resolveCodexCommand({ platform: 'darwin', pathEnv: env.PATH, homePath: root }) + expect(codex).toBe(join(v20, 'codex')) + + const paired = withCliRuntimeOnPath(codex, env, { platform: 'darwin' }) + // the shebang's `node` must now come from v20, not v22 + expect(paired.PATH.split(delimiter)[0]).toBe(v20) + }) + + it('leaves PATH untouched for a CLI whose directory ships no node', () => { + const root = mkdtempSync(join(tmpdir(), 'orca-pair-')) + const brew = join(root, 'opt', 'homebrew', 'bin') + makeExecutable(join(brew, 'codex')) + const env = { PATH: '/usr/bin' } + + expect(withCliRuntimeOnPath(join(brew, 'codex'), env, { platform: 'darwin' })).toBe(env) + }) + + it('leaves PATH untouched for a bare command name', () => { + const env = { PATH: '/usr/bin' } + expect(withCliRuntimeOnPath('codex', env, { platform: 'darwin' })).toBe(env) + }) + + it('is a no-op when the runtime directory already leads PATH', () => { + const root = mkdtempSync(join(tmpdir(), 'orca-pair-')) + const v20 = join(root, '.nvm', 'versions', 'node', 'v20.11.0', 'bin') + makeExecutable(join(v20, 'node')) + makeExecutable(join(v20, 'codex')) + const env = { PATH: [v20, '/usr/bin'].join(delimiter) } + + expect(withCliRuntimeOnPath(join(v20, 'codex'), env, { platform: 'darwin' })).toBe(env) + }) + + it('reads the Windows path key the child will actually use, whatever its casing', () => { + const root = mkdtempSync(join(tmpdir(), 'orca-pair-')) + const v20 = join(root, '.nvm', 'versions', 'node', 'v20.11.0', 'bin') + makeExecutable(join(v20, 'node.exe')) + makeExecutable(join(v20, 'codex.cmd')) + // Why: win32 resolves env names case-insensitively, so a block may spell it + // any way. Reading a narrower set than the twin-dedupe deletes would drop + // this entry unread and hand the child a PATH containing only our directory. + const env = { path: 'C:\\Windows;C:\\Windows\\System32', HOME: 'x' } + + const paired = withCliRuntimeOnPath(join(v20, 'codex.cmd'), env, { platform: 'win32' }) + expect(paired.path).toBe([v20, 'C:\\Windows', 'C:\\Windows\\System32'].join(';')) + expect(Object.keys(paired).filter((key) => key.toLowerCase() === 'path')).toEqual(['path']) + }) + + it('writes the Windows Path key without leaving a differently-cased twin', () => { + const root = mkdtempSync(join(tmpdir(), 'orca-pair-')) + const v20 = join(root, '.nvm', 'versions', 'node', 'v20.11.0', 'bin') + makeExecutable(join(v20, 'node.exe')) + makeExecutable(join(v20, 'codex.cmd')) + // Why both keys: with only `Path` seeded the assertion is vacuous — the + // helper cannot invent a `PATH` key, so the dedupe loop could be deleted + // wholesale and this test would still pass. + const env = { Path: 'C:\\Windows;C:\\Windows\\System32', PATH: 'C:\\Stale' } + + const paired = withCliRuntimeOnPath(join(v20, 'codex.cmd'), env, { platform: 'win32' }) + expect(Object.keys(paired)).toEqual(['Path']) + // Why the whole string: splitting on the host delimiter while joining on ';' + // shredded every drive letter into `C;\\Windows`. + expect(paired.Path).toBe([v20, 'C:\\Windows', 'C:\\Windows\\System32'].join(';')) + }) +}) diff --git a/src/main/codex/codex-app-server-session.ts b/src/main/codex/codex-app-server-session.ts index 04748e6eaa2..2f82242532b 100644 --- a/src/main/codex/codex-app-server-session.ts +++ b/src/main/codex/codex-app-server-session.ts @@ -1,6 +1,7 @@ import { spawn, type ChildProcess, type ChildProcessWithoutNullStreams } from 'node:child_process' import { waitForProcessExitUntil } from './codex-process-exit-deadline' import { stderrIndicatesMissingAppServer } from './codex-app-server-capability-signal' +import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' // Why: `codex app-server` is Orca's sanctioned RPC surface into Codex-owned // state (hook trust hashes, the sqlite thread index). This module owns the @@ -10,6 +11,13 @@ import { stderrIndicatesMissingAppServer } from './codex-app-server-capability-s export type CodexAppServerInvocation = { command: string args: string[] + /** + * The resolved CLI path, when `command` is a wrapper such as cmd.exe. Used to + * pair the CLI with the `node` it was installed against; without it a CLI + * resolved out of a version-manager directory runs under whatever node leads + * PATH and dies on a NODE_MODULE_VERSION mismatch (stablyai/orca#10932). + */ + cliPath?: string /** Overlay applied on top of the inherited environment (e.g. CODEX_HOME). */ env?: Record /** Env keys stripped from the inherited environment before spawn (e.g. an @@ -111,8 +119,9 @@ export async function runCodexAppServerSession( for (const key of invocation.envToDelete ?? []) { delete childEnv[key] } + const pairedEnv = withCliRuntimeOnPath(invocation.cliPath ?? invocation.command, childEnv) const child = spawnImpl(invocation.command, invocation.args, { - env: childEnv, + env: pairedEnv, stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true }) as ChildProcessWithoutNullStreams diff --git a/src/main/codex/codex-session-index-heal.ts b/src/main/codex/codex-session-index-heal.ts index 49e575f4f12..baaba13358b 100644 --- a/src/main/codex/codex-session-index-heal.ts +++ b/src/main/codex/codex-session-index-heal.ts @@ -267,6 +267,7 @@ function buildNativeHealInvocation( return { command: spawnCmd, args: spawnArgs, + cliPath: command, // Why: pin the real home explicitly — nested Orca launches can inherit a // managed CODEX_HOME from the daemon environment, which would index the // wrong sqlite DB. diff --git a/src/main/codex/codex-trust-grant-host.ts b/src/main/codex/codex-trust-grant-host.ts index b769445ef0a..ce36909cbd4 100644 --- a/src/main/codex/codex-trust-grant-host.ts +++ b/src/main/codex/codex-trust-grant-host.ts @@ -65,6 +65,7 @@ export function resolveCodexTrustGrantHost(host: CodexTrustGrantHost): ResolvedC invocation: { command: spawnCmd, args: spawnArgs, + cliPath: command, ...(useDefaultCodexHome ? { envToDelete: ['CODEX_HOME'] } : { env: { CODEX_HOME: input.runtimeHomePath } }), diff --git a/src/main/rate-limits/claude-pty.ts b/src/main/rate-limits/claude-pty.ts index 10feb7e90b2..210f4778ac9 100644 --- a/src/main/rate-limits/claude-pty.ts +++ b/src/main/rate-limits/claude-pty.ts @@ -4,6 +4,9 @@ make the lifecycle harder to audit. */ import type { ProviderRateLimits, RateLimitWindow } from '../../shared/rate-limit-types' import { buildConfiguredProxyEnv, type NetworkProxySettings } from '../../shared/network-proxy' import { resolveClaudeCommand } from '../codex-cli/command' +// Why: import from the shared module, not the codex-cli re-export, so a test that +// mocks '../codex-cli/command' does not have to restate this pure helper. +import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' import type { ClaudeRuntimeAuthPreparation } from '../claude-accounts/runtime-auth-service' import { applyClaudeEnvPatch } from '../claude-accounts/environment' import { withMacTailscaleDnsHint } from '../network/macos-tailscale-dns-diagnostic' @@ -292,7 +295,7 @@ export async function fetchViaPty(options?: { // Why: hidden usage PTYs must not inherit the process cwd (e.g. / or a // drive root), which can trigger unbounded file discovery. cwd: resolveHiddenRateLimitPtyCwd(), - env: spawnEnv + env: withCliRuntimeOnPath(claudeCommand, spawnEnv) }) const termDisposables: { dispose: () => void }[] = [registerHiddenRateLimitPty(term)] let enterInterval: ReturnType | null = null diff --git a/src/main/rate-limits/codex-fetcher-runtime-pairing.test.ts b/src/main/rate-limits/codex-fetcher-runtime-pairing.test.ts new file mode 100644 index 00000000000..0353bd4c31d --- /dev/null +++ b/src/main/rate-limits/codex-fetcher-runtime-pairing.test.ts @@ -0,0 +1,133 @@ +import { chmodSync, mkdirSync, mkdtempSync, writeFileSync } from 'node:fs' +import { EventEmitter } from 'node:events' +import { tmpdir } from 'node:os' +import { delimiter, join } from 'node:path' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { childSpawnMock, readFileMock, resolveCodexCommandMock, ptySpawnMock } = vi.hoisted(() => ({ + childSpawnMock: vi.fn(), + readFileMock: vi.fn(), + resolveCodexCommandMock: vi.fn(), + ptySpawnMock: vi.fn() +})) + +vi.mock('node:child_process', () => ({ spawn: childSpawnMock })) +vi.mock('node:fs/promises', () => ({ readFile: readFileMock })) +vi.mock('../codex-cli/command', () => ({ resolveCodexCommand: resolveCodexCommandMock })) +vi.mock('node-pty', () => ({ spawn: ptySpawnMock })) +vi.mock('../codex/codex-state-db', () => ({ isCodexStateDbBackfillPending: vi.fn(() => false) })) +vi.mock('../codex/codex-state-db-backfill-recovery', () => ({ + startCodexStateDbBackfillRecoveryInBackground: vi.fn(() => Promise.resolve(null)) +})) +vi.mock('./codex-auth-presence', () => ({ probeCodexAuthPresence: vi.fn(() => 'present') })) + +import { fetchCodexRateLimits } from './codex-fetcher' + +function makeRpcChild() { + const child = new EventEmitter() as EventEmitter & { + stdout: EventEmitter + stderr: EventEmitter + stdin: EventEmitter & { write: ReturnType; end: ReturnType } + kill: ReturnType + exitCode: number | null + } + child.stdout = new EventEmitter() + child.stderr = new EventEmitter() + const exitNow = (): void => { + child.exitCode = 0 + child.emit('exit', 0, null) + child.emit('close', 0, null) + } + child.stdin = Object.assign(new EventEmitter(), { write: vi.fn(), end: vi.fn(exitNow) }) + child.exitCode = null + child.kill = vi.fn(() => { + exitNow() + return true + }) + return child +} + +/** A CLI installed under one version manager entry, with its sibling node. */ +function makeVersionManagerCli(cliName = 'codex'): { bin: string; cli: string } { + const root = mkdtempSync(join(tmpdir(), 'orca-fetch-pair-')) + const bin = join(root, '.nvm', 'versions', 'node', 'v20.11.0', 'bin') + mkdirSync(bin, { recursive: true }) + for (const name of ['node', 'node.exe', cliName]) { + writeFileSync(join(bin, name), '') + chmodSync(join(bin, name), 0o755) + } + return { bin, cli: join(bin, cliName) } +} + +describe('codex rate-limit spawn runtime pairing', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.clearAllMocks() + readFileMock.mockRejectedValue(new Error('no auth fixture')) + vi.stubGlobal('fetch', vi.fn()) + }) + + it("spawns the RPC reader with the resolved CLI's own node ahead of PATH", async () => { + // Why a real fixture: the helper short-circuits on a non-absolute command, + // so a bare 'codex' mock would make this pass vacuously. + const { bin, cli } = makeVersionManagerCli() + resolveCodexCommandMock.mockReturnValue(cli) + const rpcChild = makeRpcChild() + childSpawnMock.mockReturnValue(rpcChild) + + const resultPromise = fetchCodexRateLimits({ allowPtyFallback: false }) + await vi.advanceTimersByTimeAsync(0) + + const spawnEnv = childSpawnMock.mock.calls[0]?.[2]?.env as NodeJS.ProcessEnv + // Guards the argument choice: pairing spawnCmd (cmd.exe on win32) rather than + // the resolved CLI silently reverts the ABI fix (stablyai/orca#10932). + expect(spawnEnv.PATH?.split(delimiter)[0]).toBe(bin) + + rpcChild.emit('close') + await resultPromise + }) + + it('pairs the resolved CLI on win32, where the spawn command is cmd.exe', async () => { + // Why win32 specifically: on posix getSpawnArgsForWindows returns the CLI + // itself, so pairing the spawn command instead of the resolved CLI is + // indistinguishable. Only here does the wrong argument become cmd.exe. + const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + try { + const { bin, cli } = makeVersionManagerCli('codex.cmd') + resolveCodexCommandMock.mockReturnValue(cli) + const rpcChild = makeRpcChild() + childSpawnMock.mockReturnValue(rpcChild) + + const resultPromise = fetchCodexRateLimits({ allowPtyFallback: false }) + await vi.advanceTimersByTimeAsync(0) + + const spawnCommand = childSpawnMock.mock.calls[0]?.[0] as string + expect(spawnCommand.toLowerCase()).toContain('cmd.exe') + const spawnEnv = childSpawnMock.mock.calls[0]?.[2]?.env as NodeJS.ProcessEnv + expect((spawnEnv.Path ?? spawnEnv.PATH)?.split(';')[0]).toBe(bin) + + rpcChild.emit('close') + await resultPromise + } finally { + if (originalPlatform) { + Object.defineProperty(process, 'platform', originalPlatform) + } + } + }) + + it('leaves PATH alone when the CLI resolves to a bare command name', async () => { + resolveCodexCommandMock.mockReturnValue('codex') + const rpcChild = makeRpcChild() + childSpawnMock.mockReturnValue(rpcChild) + + const resultPromise = fetchCodexRateLimits({ allowPtyFallback: false }) + await vi.advanceTimersByTimeAsync(0) + + const spawnEnv = childSpawnMock.mock.calls[0]?.[2]?.env as NodeJS.ProcessEnv + expect(spawnEnv.PATH).toBe(process.env.PATH) + + rpcChild.emit('close') + await resultPromise + }) +}) diff --git a/src/main/rate-limits/codex-fetcher.ts b/src/main/rate-limits/codex-fetcher.ts index dc18e715fdd..00c78aaa5f8 100644 --- a/src/main/rate-limits/codex-fetcher.ts +++ b/src/main/rate-limits/codex-fetcher.ts @@ -19,6 +19,9 @@ import { type CodexRateWindowSnapshot } from './codex-rate-limit-window-classification' import { resolveCodexCommand } from '../codex-cli/command' +// Why: import from the shared module, not the codex-cli re-export, so a test that +// mocks '../codex-cli/command' does not have to restate this pure helper. +import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' import { CODEX_READ_ONLY_APP_SERVER_ARGS } from '../codex-cli/codex-read-only-app-server-args' import { withMacTailscaleDnsHint } from '../network/macos-tailscale-dns-diagnostic' import { getCmdExePath, getSpawnArgsForWindows } from '../win32-utils' @@ -648,10 +651,10 @@ async function fetchViaRpc(options?: FetchCodexRateLimitsOptions): Promise | null = null @@ -1005,11 +1008,11 @@ async function fetchViaPty(options?: FetchCodexRateLimitsOptions): Promise void }[] = [registerHiddenRateLimitPty(term)] diff --git a/src/shared/node-cli-command-resolution.ts b/src/shared/node-cli-command-resolution.ts index 44e85c4dd08..aa336e7f190 100644 --- a/src/shared/node-cli-command-resolution.ts +++ b/src/shared/node-cli-command-resolution.ts @@ -1,6 +1,6 @@ import { accessSync, constants, existsSync, readdirSync, statSync } from 'node:fs' import { homedir } from 'node:os' -import { delimiter, dirname, join } from 'node:path' +import { delimiter, dirname, isAbsolute, join } from 'node:path' type ResolveCommandOptions = { pathEnv?: string | null @@ -16,13 +16,16 @@ function getExecutableNames(platform: NodeJS.Platform, commandName: string): str return [commandName] } -function splitPath(pathEnv: string | null | undefined): string[] { +function splitPath( + pathEnv: string | null | undefined, + pathDelimiter: string = delimiter +): string[] { if (!pathEnv) { return [] } return pathEnv - .split(delimiter) + .split(pathDelimiter) .map((entry) => entry.trim()) .filter(Boolean) } @@ -209,6 +212,70 @@ 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`. + * + * Why: `resolveCliCommand` falls back to scanning every version-manager install + * when PATH misses, so it can hand back `~/.nvm/versions/node/v20.x/bin/codex` + * while PATH still leads with v22. The CLI's `#!/usr/bin/env node` shebang then + * loads a v20-built native module under a v22 ABI and the agent dies on first + * 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. + */ +export function withCliRuntimeOnPath( + commandPath: string, + env: T, + options: Pick = {} +): T { + const platform = options.platform ?? process.platform + if (!isAbsolute(commandPath)) { + return env + } + const commandDirectory = dirname(commandPath) + if (!findFirstExecutable(platform, [commandDirectory], getExecutableNames(platform, 'node'))) { + return env + } + const pathKey = platform === 'win32' ? firstWindowsPathEnvKey(env) : 'PATH' + const pathDelimiter = platform === 'win32' ? ';' : delimiter + const segments = splitPath(env[pathKey], pathDelimiter) + if (segments[0] === commandDirectory) { + return env + } + const next = [commandDirectory, ...segments.filter((entry) => entry !== commandDirectory)].join( + pathDelimiter + ) + const paired = { ...env, [pathKey]: next } + if (platform === 'win32') { + // Why: the spread is case-sensitive while Windows env lookup is not, so a + // differently-cased twin would keep shadowing the value we just wrote. + for (const name of Object.keys(paired)) { + if (name !== pathKey && name.toLowerCase() === pathKey.toLowerCase()) { + delete (paired as NodeJS.ProcessEnv)[name] + } + } + } + return paired as T +} + // Why: Node-script CLIs need their version-manager sibling `node` on PATH. export function getVersionManagerBinPaths(options: ResolveCommandOptions = {}): string[] { const platform = options.platform ?? process.platform