diff --git a/src/main/codex-accounts/runtime-home-status-hook-independence.test.ts b/src/main/codex-accounts/runtime-home-status-hook-independence.test.ts index ca3866b3436..9670d1676f5 100644 --- a/src/main/codex-accounts/runtime-home-status-hook-independence.test.ts +++ b/src/main/codex-accounts/runtime-home-status-hook-independence.test.ts @@ -27,7 +27,8 @@ vi.mock('node:os', async (importOriginal) => ({ })) vi.mock('../codex-cli/command', async (importOriginal) => ({ ...(await importOriginal()), - resolveCodexCommand: () => mocks.codexPath + resolveCodexCommand: () => mocks.codexPath, + resolveCodexCommandForSpawn: async () => mocks.codexPath })) vi.mock('../codex/codex-hook-trust-derivation', async (importOriginal) => ({ ...(await importOriginal()), diff --git a/src/main/codex-cli/command.ts b/src/main/codex-cli/command.ts index 240e4ef405f..22e3890ff0d 100644 --- a/src/main/codex-cli/command.ts +++ b/src/main/codex-cli/command.ts @@ -1,3 +1,4 @@ +import { resolveCliCommandAsync } from '../../shared/node-cli-command-async-resolution' import { resolveCliCommand } from '../../shared/node-cli-command-resolution' import { resolveGuestNativeProgram } from '../ipc/wsl-guest-windows-drive-path' import { WINDOWS_ONLY_AGENT_CLI_DIR } from './windows-only-agent-cli' @@ -33,6 +34,12 @@ export function resolveCodexCommand(options: ResolveCommandOptions = {}): string return resolveGuestNativeAgentCommand('codex', options) } +/** Per-spawn lookup (#35565): only Windows' per-candidate stats are slow enough to move off the event loop. */ +export async function resolveCodexCommandForSpawn(): Promise { + // win32 has no WSL drive mounts, so the guest-native skip would be a no-op there. + return process.platform === 'win32' ? resolveCliCommandAsync('codex') : resolveCodexCommand() +} + export function resolveClaudeCommand(options: ResolveCommandOptions = {}): string { return resolveGuestNativeAgentCommand('claude', options) } diff --git a/src/main/codex/codex-hook-file-entry-binary-contract.test.ts b/src/main/codex/codex-hook-file-entry-binary-contract.test.ts index 0584babdd8d..3bb8b379746 100644 --- a/src/main/codex/codex-hook-file-entry-binary-contract.test.ts +++ b/src/main/codex/codex-hook-file-entry-binary-contract.test.ts @@ -47,7 +47,8 @@ import { vi.mock('../codex-cli/command', async (importOriginal) => ({ ...(await importOriginal()), - resolveCodexCommand: () => process.env.ORCA_CODEX_HOOK_CONTRACT_BINARY ?? 'codex' + resolveCodexCommand: () => process.env.ORCA_CODEX_HOOK_CONTRACT_BINARY ?? 'codex', + resolveCodexCommandForSpawn: async () => process.env.ORCA_CODEX_HOOK_CONTRACT_BINARY ?? 'codex' })) vi.mock('electron', () => ({ app: { diff --git a/src/main/codex/codex-hook-hash-lookup.test.ts b/src/main/codex/codex-hook-hash-lookup.test.ts index 85b53324803..942147d7b60 100644 --- a/src/main/codex/codex-hook-hash-lookup.test.ts +++ b/src/main/codex/codex-hook-hash-lookup.test.ts @@ -16,7 +16,8 @@ const mocks = vi.hoisted(() => ({ vi.mock('../codex-cli/command', async (importOriginal) => ({ ...(await importOriginal()), - resolveCodexCommand: () => mocks.codexPath + resolveCodexCommand: () => mocks.codexPath, + resolveCodexCommandForSpawn: async () => mocks.codexPath })) vi.mock('./codex-hook-trust-derivation', async (importOriginal) => ({ ...(await importOriginal()), diff --git a/src/main/codex/codex-hook-hash-lookup.ts b/src/main/codex/codex-hook-hash-lookup.ts index 915f69c24e0..e5b9e168966 100644 --- a/src/main/codex/codex-hook-hash-lookup.ts +++ b/src/main/codex/codex-hook-hash-lookup.ts @@ -1,6 +1,6 @@ import { withTimeout } from '../../shared/promise-timeout-fallback' import { dedupeInFlightRun } from '../in-flight-run-dedupe' -import { resolveCodexCommand } from '../codex-cli/command' +import { resolveCodexCommand, resolveCodexCommandForSpawn } from '../codex-cli/command' import { getManagedCommand, getManagedScriptPath } from './codex-hook-definition' import { deriveCodexHookHashes, @@ -107,16 +107,26 @@ function remember(codexPath: string, known: KnownAnswer): CodexHookAnswer { return known.answer } -/** Query the Codex on PATH only in app main. */ -export async function resolveCodexHookAnswer(): Promise { +/** The Codex on the same hydrated PATH that panes use; off the event loop on Windows. */ +export async function resolveAppCodexPath(): Promise { + await appPathReady + return resolveCodexCommandForSpawn() +} + +/** Query the Codex on PATH only in app main; pass a path from resolveAppCodexPath to skip re-resolving it. */ +export async function resolveCodexHookAnswer(codexPath?: string): Promise { // Resolve the same hydrated PATH that panes use. await appPathReady - return lookupCodexHookAnswer(resolveCodexCommand(), getManagedCommand(getManagedScriptPath())) + return lookupCodexHookAnswer( + codexPath ?? resolveCodexCommand(), + getManagedCommand(getManagedScriptPath()) + ) } /** Read status from memory or disk without spawning Codex. */ -export function readKnownCodexHookAnswer(): CodexHookAnswer | null { - const codexPath = resolveCodexCommand() +export function readKnownCodexHookAnswer( + codexPath: string = resolveCodexCommand() +): CodexHookAnswer | null { const fingerprint = fingerprintCodex(codexPath) if (fingerprint === null) { return codexNotFound(codexPath) diff --git a/src/main/codex/codex-hook-real-home-status.test.ts b/src/main/codex/codex-hook-real-home-status.test.ts index 9f8deef9a71..ddfb0c49951 100644 --- a/src/main/codex/codex-hook-real-home-status.test.ts +++ b/src/main/codex/codex-hook-real-home-status.test.ts @@ -19,7 +19,8 @@ vi.mock('node:os', async (importOriginal) => ({ })) vi.mock('../codex-cli/command', async (importOriginal) => ({ ...(await importOriginal()), - resolveCodexCommand: () => mocks.codexPath + resolveCodexCommand: () => mocks.codexPath, + resolveCodexCommandForSpawn: async () => mocks.codexPath })) vi.mock('./codex-hook-trust-derivation', async (importOriginal) => ({ ...(await importOriginal()), diff --git a/src/main/codex/codex-hook-reconcile.test.ts b/src/main/codex/codex-hook-reconcile.test.ts index 89ba8064100..95a7235d707 100644 --- a/src/main/codex/codex-hook-reconcile.test.ts +++ b/src/main/codex/codex-hook-reconcile.test.ts @@ -20,7 +20,10 @@ import type * as TrustDerivation from './codex-hook-trust-derivation' import type * as RealHomeInstall from './codex-real-home-hook-install' const mocks = vi.hoisted(() => { - const held: { holdRealHome: Promise | null } = { holdRealHome: null } + const held: { holdRealHome: Promise | null; holdCodexScan: Promise | null } = { + holdRealHome: null, + holdCodexScan: null + } return { ...held, homedir: vi.fn<() => string>(), @@ -37,7 +40,13 @@ vi.mock('node:os', async (importOriginal) => ({ })) vi.mock('../codex-cli/command', async (importOriginal) => ({ ...(await importOriginal()), - resolveCodexCommand: () => mocks.codexPath + resolveCodexCommand: () => mocks.codexPath, + // Answers what PATH held when the scan began, as a scan that walked past a later install would. + resolveCodexCommandForSpawn: async () => { + const codexPath = mocks.codexPath + await mocks.holdCodexScan + return codexPath + } })) vi.mock('./codex-hook-trust-derivation', async (importOriginal) => ({ ...(await importOriginal()), @@ -197,6 +206,7 @@ beforeEach(() => { }) mocks.realHomeRuns = 0 mocks.holdRealHome = null + mocks.holdCodexScan = null enabled = true usesRealHome = true _internals.resetForTesting() @@ -372,6 +382,48 @@ describe('reconcileCodexHooks', () => { expect(mocks.realHomeRuns).toBe(1) }) + it('rechecks a spawn that lands while a run is still finding Codex, which may miss a fresh install', async () => { + const installed = mocks.codexPath + mocks.codexPath = join(userData, 'missing-codex') + await start() + const scan = Promise.withResolvers() + mocks.holdCodexScan = scan.promise + + scheduleCodexHookReconcile() + await new Promise((resolve) => setImmediate(resolve)) + mocks.codexPath = installed + mocks.holdCodexScan = null + scheduleCodexHookReconcile() + scan.resolve() + await settleSpawn() + + expect(readHooks().Stop).toEqual([{ hooks: [buildCodexManagedHook(command(), 'Stop')] }]) + }) + + it('rechecks a spawn that lands while Codex is still answering, so an upgrade that moved it is not missed', async () => { + await start() + rmSync(hooksPath()) + const old = mocks.codexPath + // New bytes make the lookup ask this binary again. + writeFileSync(old, 'codex 0.161.0') + const probe = Promise.withResolvers() + mocks.probeCodexVersion.mockReturnValueOnce(probe.promise) + const probesBefore = mocks.probeCodexVersion.mock.calls.length + + scheduleCodexHookReconcile() + await vi.waitFor(() => expect(mocks.probeCodexVersion.mock.calls.length).toBe(probesBefore + 1)) + rmSync(old) + const moved = join(userData, 'moved', 'codex') + mkdirSync(join(moved, '..')) + writeFileSync(moved, 'codex 0.161.0') + mocks.codexPath = moved + scheduleCodexHookReconcile() + probe.resolve(null) + await settleSpawn() + + expect(readHooks().Stop).toEqual([{ hooks: [buildCodexManagedHook(command(), 'Stop')] }]) + }) + it("leaves an older build's entry alone on a pane spawn, and converts it at app start", async () => { await start() writeHooks({ Stop: [olderBuildStop()] }) diff --git a/src/main/codex/codex-hook-reconcile.ts b/src/main/codex/codex-hook-reconcile.ts index fec1414bd13..b0b02ed23c1 100644 --- a/src/main/codex/codex-hook-reconcile.ts +++ b/src/main/codex/codex-hook-reconcile.ts @@ -2,6 +2,7 @@ import { withTimeout } from '../../shared/promise-timeout-fallback' import { CODEX_HOOK_LAUNCH_WAIT_MS, readKnownCodexHookAnswer, + resolveAppCodexPath, resolveCodexHookAnswer, startCodexHookHashLookup } from './codex-hook-hash-lookup' @@ -31,6 +32,8 @@ let rerun = false let convertRequested = false // Consume launch requests on the next run; their launch proceeds even if reconciliation cannot. let realHomeLaunchRequested = false +// Until its file pass, a run may still settle on a Codex an install or upgrade has since moved. +let findingCodexAnswer = false // Leave room in the launch's 3s budget to write stopgap approvals. const ANSWER_WAIT_MS = 500 @@ -79,7 +82,7 @@ function reconcileAfter(ready: Promise, request: ReconcileRequest): Pro export function scheduleCodexHookReconcile(): void { // One run already reads after the spawn; coalesce calls from its multiple env builders. - if (config && !running) { + if (config && (!running || findingCodexAnswer)) { void reconcileCodexHooks() } } @@ -139,14 +142,13 @@ async function reconcileOnce(realHomeLaunch: boolean): Promise { if (!current?.isEnabled() || !(realHomeLaunch || resolveHome(current).kind === 'real')) { return } - const lookup = resolveCodexHookAnswer() - const live = await withTimeout(lookup, ANSWER_WAIT_MS, null) - if (!live) { - // Replace stopgap approvals once Codex's answer arrives. - void lookup.then(() => reconcileCodexHooks({ realHomeLaunch })) + findingCodexAnswer = true + let answer: CodexHookAnswer | null + try { + answer = await findCodexAnswer(realHomeLaunch) + } finally { + findingCodexAnswer = false } - // Reuse Codex's saved answer so a slow startup probe does not disturb existing hooks. - const answer = live && live.kind !== 'pending' ? live : readKnownCodexHookAnswer() if (answer?.kind === 'refused' || (answer?.kind === 'pending' && answer.codexMissing)) { return } @@ -159,6 +161,19 @@ async function reconcileOnce(realHomeLaunch: boolean): Promise { }) } +async function findCodexAnswer(realHomeLaunch: boolean): Promise { + // Every native spawn runs this; resolve once (#35565). + const codexPath = await resolveAppCodexPath() + const lookup = resolveCodexHookAnswer(codexPath) + const live = await withTimeout(lookup, ANSWER_WAIT_MS, null) + if (!live) { + // Replace stopgap approvals once Codex's answer arrives. + void lookup.then(() => reconcileCodexHooks({ realHomeLaunch })) + } + // Reuse Codex's saved answer so a slow startup probe does not disturb existing hooks. + return live && live.kind !== 'pending' ? live : readKnownCodexHookAnswer(codexPath) +} + export const _internals = { resetForTesting(): void { config = null @@ -167,6 +182,7 @@ export const _internals = { rerun = false convertRequested = false realHomeLaunchRequested = false + findingCodexAnswer = false }, async settledForTesting(): Promise { while (running) { diff --git a/src/main/codex/codex-hook-toggle-off-wins.test.ts b/src/main/codex/codex-hook-toggle-off-wins.test.ts index 9f57f64f143..e6e083ed0f8 100644 --- a/src/main/codex/codex-hook-toggle-off-wins.test.ts +++ b/src/main/codex/codex-hook-toggle-off-wins.test.ts @@ -27,7 +27,8 @@ vi.mock('node:os', async (importOriginal) => ({ })) vi.mock('../codex-cli/command', async (importOriginal) => ({ ...(await importOriginal()), - resolveCodexCommand: () => mocks.codexPath + resolveCodexCommand: () => mocks.codexPath, + resolveCodexCommandForSpawn: async () => mocks.codexPath })) vi.mock('./codex-hook-trust-derivation', async (importOriginal) => ({ ...(await importOriginal()), diff --git a/src/shared/cli-fs-reads.ts b/src/shared/cli-fs-reads.ts new file mode 100644 index 00000000000..3e34a56ab15 --- /dev/null +++ b/src/shared/cli-fs-reads.ts @@ -0,0 +1,95 @@ +import { accessSync, constants, readFileSync, readdirSync, statSync } from 'node:fs' +import { access, readdir, readFile, stat } from 'node:fs/promises' + +/** One filesystem read a CLI lookup needs; lookups yield these so sync and async callers share one walk. */ +export type CliFsRead = + | { kind: 'runnable'; platform: NodeJS.Platform; path: string } + /** A regular file's contents, else null. */ + | { kind: 'file'; path: string } + /** Subdirectory names, else null. */ + | { kind: 'dirs'; path: string } +export type CliFsReadResult = boolean | string | string[] | null +export type CliFsReads = Generator + +export function isRunnableCommand(platform: NodeJS.Platform, candidate: string): boolean { + try { + if (!statSync(candidate).isFile()) { + return false + } + if (platform === 'win32') { + return true + } + // Skip nonexecutables/placeholders now so GUI probing reaches a runnable CLI instead of later EACCES/EISDIR. + accessSync(candidate, constants.X_OK) + return true + } catch { + return false + } +} + +async function isRunnableCommandAsync( + platform: NodeJS.Platform, + candidate: string +): Promise { + try { + if (!(await stat(candidate)).isFile()) { + return false + } + if (platform !== 'win32') { + await access(candidate, constants.X_OK) + } + return true + } catch { + return false + } +} + +function readSync(read: CliFsRead): CliFsReadResult { + try { + switch (read.kind) { + case 'runnable': + return isRunnableCommand(read.platform, read.path) + case 'file': + return statSync(read.path).isFile() ? readFileSync(read.path, 'utf8') : null + case 'dirs': + return readdirSync(read.path, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + } + } catch { + return null + } +} + +async function readAsync(read: CliFsRead): Promise { + try { + switch (read.kind) { + case 'runnable': + return await isRunnableCommandAsync(read.platform, read.path) + case 'file': + return (await stat(read.path)).isFile() ? await readFile(read.path, 'utf8') : null + case 'dirs': + return (await readdir(read.path, { withFileTypes: true })) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + } + } catch { + return null + } +} + +export function runCliFsReadsSync(reads: CliFsReads): T { + let step = reads.next() + while (!step.done) { + step = reads.next(readSync(step.value)) + } + return step.value +} + +export async function runCliFsReadsAsync(reads: CliFsReads): Promise { + let step = reads.next() + while (!step.done) { + step = reads.next(await readAsync(step.value)) + } + return step.value +} diff --git a/src/shared/node-cli-command-async-resolution.test.ts b/src/shared/node-cli-command-async-resolution.test.ts new file mode 100644 index 00000000000..334de773d10 --- /dev/null +++ b/src/shared/node-cli-command-async-resolution.test.ts @@ -0,0 +1,124 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import type * as NodeFs from 'node:fs' +import { tmpdir } from 'node:os' +import { delimiter, join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { resolveCliCommandAsync } from './node-cli-command-async-resolution' +import { resolveCliCommand } from './node-cli-command-resolution' + +const syncFs = vi.hoisted(() => ({ forbidden: false, calls: new Array() })) + +// Throws from every sync read a CLI lookup could make while forbidden, proving the async lookup makes none. +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal() + const guard = + (name: string, read: (...args: A) => R) => + (...args: A): R => { + if (syncFs.forbidden) { + syncFs.calls.push(name) + throw new Error(`sync fs call: ${name}`) + } + return read(...args) + } + return { + ...actual, + accessSync: guard('accessSync', actual.accessSync), + existsSync: guard('existsSync', actual.existsSync), + lstatSync: guard('lstatSync', actual.lstatSync), + readdirSync: guard('readdirSync', actual.readdirSync), + readFileSync: guard('readFileSync', actual.readFileSync), + realpathSync: guard('realpathSync', actual.realpathSync), + statSync: guard('statSync', actual.statSync) + } +}) + +let root: string + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'orca-cli-resolve-async-')) +}) + +afterEach(() => { + syncFs.forbidden = false + syncFs.calls = [] + rmSync(root, { recursive: true, force: true }) +}) + +function stage(...relative: string[]): void { + for (const path of relative) { + mkdirSync(join(root, path, '..'), { recursive: true }) + writeFileSync(join(root, path), '') + } +} + +// Windows' spawn recheck uses the async resolver; it must pick what panes' sync resolver picks. +// platform: 'win32' skips the X_OK check, so these run on any host. +describe('resolveCliCommandAsync agrees with resolveCliCommand on Windows', () => { + const options = (...dirs: string[]) => ({ + pathEnv: dirs.map((dir) => join(root, dir)).join(delimiter), + homePath: join(root, 'home'), + platform: 'win32' as const + }) + + it.each([ + { + name: 'the .cmd shim beats .exe and .bat in one dir', + files: ['a/codex.exe', 'a/codex.bat', 'a/codex.cmd'], + expected: 'a/codex.cmd' + }, + { name: '.exe beats .bat', files: ['a/codex.bat', 'a/codex.exe'], expected: 'a/codex.exe' }, + { + name: 'an earlier PATH dir beats a better extension later', + files: ['a/codex.bat', 'b/codex.cmd'], + expected: 'a/codex.bat' + }, + { + name: 'a PATH miss falls back to the npm global dir', + files: ['home/AppData/Roaming/npm/codex.cmd'], + expected: 'home/AppData/Roaming/npm/codex.cmd' + }, + { + name: 'a PATH copy beats an install-dir copy', + files: ['home/AppData/Roaming/npm/codex.cmd', 'b/codex.exe'], + expected: 'b/codex.exe' + }, + { + name: "nvm's default alias outranks a newer nvm install", + files: [ + 'home/.nvm/alias/default', + 'home/.nvm/versions/node/v22.0.0/bin/codex.cmd', + 'home/.nvm/versions/node/v20.1.0/bin/codex.cmd' + ], + alias: '20', + expected: 'home/.nvm/versions/node/v20.1.0/bin/codex.cmd' + }, + { name: 'a miss everywhere returns the bare name', files: [], expected: null } + ])('$name', async ({ files, alias, expected }) => { + stage(...files) + if (alias) { + writeFileSync(join(root, 'home/.nvm/alias/default'), alias) + } + const sync = resolveCliCommand('codex', options('a', 'b')) + expect(sync).toBe(expected === null ? 'codex' : join(root, expected)) + expect(await resolveCliCommandAsync('codex', options('a', 'b'))).toBe(sync) + }) +}) + +it('reads nothing synchronously, even when a PATH miss walks nvm and install dirs', async () => { + stage('home/.nvm/alias/default', 'home/.nvm/versions/node/v20.1.0/bin/codex.cmd') + writeFileSync(join(root, 'home/.nvm/alias/default'), '20') + const options = { + pathEnv: join(root, 'a'), + homePath: join(root, 'home'), + platform: 'win32' as const + } + syncFs.forbidden = true + + await expect(resolveCliCommandAsync('codex', options)).resolves.toBe( + join(root, 'home/.nvm/versions/node/v20.1.0/bin/codex.cmd') + ) + expect(syncFs.calls).toEqual([]) + // The guard is live: the sync resolver hits it and cannot find the same install. + expect(resolveCliCommand('codex', options)).toBe('codex') + expect(syncFs.calls).not.toEqual([]) +}) diff --git a/src/shared/node-cli-command-async-resolution.ts b/src/shared/node-cli-command-async-resolution.ts new file mode 100644 index 00000000000..a92c7416bcc --- /dev/null +++ b/src/shared/node-cli-command-async-resolution.ts @@ -0,0 +1,10 @@ +import { runCliFsReadsAsync } from './cli-fs-reads' +import { cliCommandLookup, type ResolveCommandOptions } from './node-cli-command-resolution' + +/** resolveCliCommand with every filesystem read off the event loop. */ +export function resolveCliCommandAsync( + commandName: string, + options: ResolveCommandOptions = {} +): Promise { + return runCliFsReadsAsync(cliCommandLookup(commandName, options)) +} diff --git a/src/shared/node-cli-command-resolution.ts b/src/shared/node-cli-command-resolution.ts index 38140fa35a1..b14ec331aa3 100644 --- a/src/shared/node-cli-command-resolution.ts +++ b/src/shared/node-cli-command-resolution.ts @@ -1,11 +1,12 @@ -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 { getFnmDefaultAliasDirectories } from './fnm-default-alias-dirs' +import { getNvmVersionDirectories, nvmVersionDirectories } from './nvm-version-directories' +import { isRunnableCommand, runCliFsReadsSync, type CliFsReads } from './cli-fs-reads' import { getSystemCliInstallDirectories } from './system-cli-install-dirs' -type ResolveCommandOptions = { +export type ResolveCommandOptions = { pathEnv?: string | null platform?: NodeJS.Platform homePath?: string @@ -35,27 +36,20 @@ function splitPath( .filter(Boolean) } -function parseVersionSegment(raw: string): number[] { - return raw - .replace(/^v/i, '') - .split('.') - .map((segment) => Number.parseInt(segment, 10)) - .map((segment) => (Number.isFinite(segment) ? segment : 0)) -} - -function compareVersionDesc(left: string, right: string): number { - const leftParts = parseVersionSegment(left) - const rightParts = parseVersionSegment(right) - const length = Math.max(leftParts.length, rightParts.length) - - for (let index = 0; index < length; index += 1) { - const delta = (rightParts[index] ?? 0) - (leftParts[index] ?? 0) - if (delta !== 0) { - return delta +function* firstRunnable( + platform: NodeJS.Platform, + directories: string[], + executableNames: string[] +): CliFsReads { + for (const directory of directories) { + for (const executableName of executableNames) { + const path = join(directory, executableName) + if ((yield { kind: 'runnable', platform, path }) === true) { + return path + } } } - - return right.localeCompare(left) + return null } function findFirstExecutable( @@ -63,33 +57,7 @@ function findFirstExecutable( directories: string[], executableNames: string[] ): string | null { - for (const directory of directories) { - for (const executableName of executableNames) { - const candidate = join(directory, executableName) - if (isRunnableCommand(platform, candidate)) { - return candidate - } - } - } - - return null -} - -function isRunnableCommand(platform: NodeJS.Platform, candidate: string): boolean { - try { - const stats = statSync(candidate) - if (!stats.isFile()) { - return false - } - if (platform === 'win32') { - return true - } - // Skip nonexecutables/placeholders now so GUI probing reaches a runnable CLI instead of later EACCES/EISDIR. - accessSync(candidate, constants.X_OK) - return true - } catch { - return false - } + return runCliFsReadsSync(firstRunnable(platform, directories, executableNames)) } function getBaseVersionManagerDirectories(platform: NodeJS.Platform, homePath: string): string[] { @@ -121,94 +89,6 @@ function getBaseVersionManagerDirectories(platform: NodeJS.Platform, homePath: s return directories } -// Bound/track nvm aliases (default → lts/* → lts/krypton → version). Hand-edited cycles must yield no preference, -// not whichever alias happens to exhaust the hop limit; mirror nvm’s seen-set behavior. -const NVM_ALIAS_CHAIN_LIMIT = 10 - -/** Resolves `alias/default` to an installed version directory name, or null. */ -function resolveNvmDefaultVersion(nvmVersionsDir: string, installed: string[]): string | null { - const aliasDir = join(nvmVersionsDir, '..', '..', 'alias') - let token = readNvmAlias(join(aliasDir, 'default')) - const seen = new Set() - for (let hop = 0; token && hop < NVM_ALIAS_CHAIN_LIMIT; hop += 1) { - if (seen.has(token)) { - return null - } - seen.add(token) - const next = readNvmAlias(join(aliasDir, token)) - if (!next) { - break - } - token = next - } - if (!token) { - return null - } - // Why: `system` selects the OS node, so nvm owns nothing to prefer here. - // `node`/`stable` mean newest, which is the ordering we already produce. - if (token === 'system' || token === 'node' || token === 'stable') { - return null - } - return matchNvmVersion(token, installed) -} - -function readNvmAlias(aliasPath: string): string | null { - // Reject literal traversal cheaply; join normalizes it away. Real containment comes from matchNvmVersion returning only installed readdir entries, - // so arbitrary alias tokens can never add foreign paths to PATH. - if (aliasPath.includes('..')) { - return null - } - try { - if (!statSync(aliasPath).isFile()) { - return null - } - const value = readFileSync(aliasPath, 'utf8').trim() - return value.length > 0 ? value : null - } catch { - return null - } -} - -/** `24` matches the highest installed `v24.x.y`; `v24.18.0` matches exactly. */ -function matchNvmVersion(token: string, installed: string[]): string | null { - // Validate full decimal version shape with no leading zeros: parseInt would coerce garbage/0x18/00/0abc to zero and select old v0.x. - // nvm stores nonexistent aliases but resolves them as N/A; preserve newest-first fallback for those, including 024. - if (!/^v?(0|[1-9]\d*)(\.(0|[1-9]\d*))*$/.test(token)) { - return null - } - const wanted = parseVersionSegment(token) - const matches = installed.filter((entry) => { - const parts = parseVersionSegment(entry) - return wanted.every((segment, index) => parts[index] === segment) - }) - return matches.sort(compareVersionDesc)[0] ?? null -} - -function getNvmVersionDirectories(homePath: string): string[] { - const nvmVersionsDir = join(homePath, '.nvm', 'versions', 'node') - if (!existsSync(nvmVersionsDir)) { - return [] - } - - let installed: string[] - try { - installed = readdirSync(nvmVersionsDir, { withFileTypes: true }) - .filter((entry) => entry.isDirectory()) - .map((entry) => entry.name) - .sort(compareVersionDesc) - } catch { - return [] - } - - // Prefer nvm default when login probing fails. Freshest installs often have no CLIs and mismatched native ABIs (#10932); - // keep other installed versions as fallbacks so their CLIs remain reachable. - const preferred = resolveNvmDefaultVersion(nvmVersionsDir, installed) - const ordered = preferred - ? [preferred, ...installed.filter((entry) => entry !== preferred)] - : installed - return ordered.map((entry) => join(nvmVersionsDir, entry, 'bin')) -} - function getVersionManagerDirectories( platform: NodeJS.Platform, homePath: string, @@ -227,33 +107,42 @@ function getVersionManagerDirectories( } // Share native/WSL ordering: version-manager bins must precede system Homebrew/npm/snap installs. -function getCliInstallDirectories(platform: NodeJS.Platform, homePath: string): string[] { +function* cliInstallDirectories(platform: NodeJS.Platform, homePath: string): CliFsReads { return [ - ...getNvmVersionDirectories(homePath), + ...(yield* nvmVersionDirectories(homePath)), ...getBaseVersionManagerDirectories(platform, homePath), ...getSystemCliInstallDirectories(platform, homePath) ] } +function getCliInstallDirectories(platform: NodeJS.Platform, homePath: string): string[] { + return runCliFsReadsSync(cliInstallDirectories(platform, homePath)) +} + +/** PATH entries first; install dirs are listed only once PATH misses. Run with cli-fs-reads. */ +export function* cliCommandLookup( + commandName: string, + options: ResolveCommandOptions = {} +): CliFsReads { + const platform = options.platform ?? process.platform + const names = getExecutableNames(platform, commandName) + const pathEnv = options.pathEnv ?? process.env.PATH ?? process.env.Path ?? null + return ( + (yield* firstRunnable(platform, splitPath(pathEnv), names)) ?? + (yield* firstRunnable( + platform, + yield* cliInstallDirectories(platform, options.homePath ?? homedir()), + names + )) ?? + commandName + ) +} + export function resolveCliCommand( commandName: string, options: ResolveCommandOptions = {} ): string { - const platform = options.platform ?? process.platform - const executableNames = getExecutableNames(platform, commandName) - const pathEnv = options.pathEnv ?? process.env.PATH ?? process.env.Path ?? null - const pathCandidate = findFirstExecutable(platform, splitPath(pathEnv), executableNames) - if (pathCandidate) { - return pathCandidate - } - - const homePath = options.homePath ?? homedir() - const installCandidate = findFirstExecutable( - platform, - getCliInstallDirectories(platform, homePath), - executableNames - ) - return installCandidate ?? commandName + return runCliFsReadsSync(cliCommandLookup(commandName, options)) } /** Resolve a literal override without falling back to an unverified command name. */ diff --git a/src/shared/nvm-version-directories.ts b/src/shared/nvm-version-directories.ts new file mode 100644 index 00000000000..9cbf63de3a5 --- /dev/null +++ b/src/shared/nvm-version-directories.ts @@ -0,0 +1,107 @@ +import { join } from 'node:path' +import { runCliFsReadsSync, type CliFsReads } from './cli-fs-reads' + +function parseVersionSegment(raw: string): number[] { + return raw + .replace(/^v/i, '') + .split('.') + .map((segment) => Number.parseInt(segment, 10)) + .map((segment) => (Number.isFinite(segment) ? segment : 0)) +} + +function compareVersionDesc(left: string, right: string): number { + const leftParts = parseVersionSegment(left) + const rightParts = parseVersionSegment(right) + const length = Math.max(leftParts.length, rightParts.length) + + for (let index = 0; index < length; index += 1) { + const delta = (rightParts[index] ?? 0) - (leftParts[index] ?? 0) + if (delta !== 0) { + return delta + } + } + + return right.localeCompare(left) +} + +// Bound/track nvm aliases (default → lts/* → lts/krypton → version). Hand-edited cycles must yield no preference, +// not whichever alias happens to exhaust the hop limit; mirror nvm’s seen-set behavior. +const NVM_ALIAS_CHAIN_LIMIT = 10 + +/** Resolves `alias/default` to an installed version directory name, or null. */ +function* resolveNvmDefaultVersion( + nvmVersionsDir: string, + installed: string[] +): CliFsReads { + const aliasDir = join(nvmVersionsDir, '..', '..', 'alias') + let token = yield* readNvmAlias(join(aliasDir, 'default')) + const seen = new Set() + for (let hop = 0; token && hop < NVM_ALIAS_CHAIN_LIMIT; hop += 1) { + if (seen.has(token)) { + return null + } + seen.add(token) + const next = yield* readNvmAlias(join(aliasDir, token)) + if (!next) { + break + } + token = next + } + if (!token) { + return null + } + // Why: `system` selects the OS node, so nvm owns nothing to prefer here. + // `node`/`stable` mean newest, which is the ordering we already produce. + if (token === 'system' || token === 'node' || token === 'stable') { + return null + } + return matchNvmVersion(token, installed) +} + +function* readNvmAlias(aliasPath: string): CliFsReads { + // Reject literal traversal cheaply; join normalizes it away. Real containment comes from matchNvmVersion returning only installed readdir entries, + // so arbitrary alias tokens can never add foreign paths to PATH. + if (aliasPath.includes('..')) { + return null + } + const contents = yield { kind: 'file', path: aliasPath } + const value = typeof contents === 'string' ? contents.trim() : '' + return value.length > 0 ? value : null +} + +/** `24` matches the highest installed `v24.x.y`; `v24.18.0` matches exactly. */ +function matchNvmVersion(token: string, installed: string[]): string | null { + // Validate full decimal version shape with no leading zeros: parseInt would coerce garbage/0x18/00/0abc to zero and select old v0.x. + // nvm stores nonexistent aliases but resolves them as N/A; preserve newest-first fallback for those, including 024. + if (!/^v?(0|[1-9]\d*)(\.(0|[1-9]\d*))*$/.test(token)) { + return null + } + const wanted = parseVersionSegment(token) + const matches = installed.filter((entry) => { + const parts = parseVersionSegment(entry) + return wanted.every((segment, index) => parts[index] === segment) + }) + return matches.sort(compareVersionDesc)[0] ?? null +} + +/** nvm's bin dirs, default alias first; yields its reads so async callers stay off the event loop. */ +export function* nvmVersionDirectories(homePath: string): CliFsReads { + const nvmVersionsDir = join(homePath, '.nvm', 'versions', 'node') + const entries = yield { kind: 'dirs', path: nvmVersionsDir } + if (!Array.isArray(entries)) { + return [] + } + const installed = entries.sort(compareVersionDesc) + + // Prefer nvm default when login probing fails. Freshest installs often have no CLIs and mismatched native ABIs (#10932); + // keep other installed versions as fallbacks so their CLIs remain reachable. + const preferred = yield* resolveNvmDefaultVersion(nvmVersionsDir, installed) + const ordered = preferred + ? [preferred, ...installed.filter((entry) => entry !== preferred)] + : installed + return ordered.map((entry) => join(nvmVersionsDir, entry, 'bin')) +} + +export function getNvmVersionDirectories(homePath: string): string[] { + return runCliFsReadsSync(nvmVersionDirectories(homePath)) +}