Files
orca/src/shared/node-cli-command-resolution.ts
T
Neil 901c6771ff fix(agents): detect agent CLIs installed outside a version manager (#18336)
* fix(agents): detect agent CLIs installed outside a version manager

The install-dir fallback that answers "is this agent installed?" when the
login-shell PATH probe fails listed only version-manager bin dirs, so
codex/opencode/cursor-agent installed by Homebrew, npm's default global
prefix, snap, nix, or the CLI's own installer read as not installed.

Where this is decisive, corrected from the previous message: the `orca` CLI,
whose detectSkillsCliAgentKeys (src/cli/handlers/skills.ts) calls
detectCommandsInInstallDirs directly and whose entry point seeds no PATH, plus
unpackaged/dev runs. NOT the packaged desktop app: patchPackagedProcessPath
(configure-process.ts:114) appends /opt/homebrew/bin, /usr/local/bin,
~/.opencode/bin and the Linux/nix prefixes onto process.env.PATH at
main-process-preflight.ts:147, before any detection, and mergePathSegments
preserves them, so the PATH scan reaches those dirs first and this fallback
never fires for them. That means this does NOT explain the packaged macOS
v1.4.194 report of codex/cursor-agent/opencode all undetected -- that report
stays open and uninvestigated.

Second correction: the fallback now carries the prefixes Homebrew actually
uses on Linux (/home/linuxbrew/.linuxbrew/bin), plus /snap/bin and the two nix
profile dirs, matching what patchPackagedProcessPath already seeds. The WSL
guest prelude gains the same entries. Leaving them out closed the native/WSL
asymmetry on darwin only, on the platforms where the fallback is decisive.

Appended last so a version-manager install still wins, and kept out of
getVersionManagerBinPaths, whose result is PREPENDED to PATH (#18234). Lives in
its own module so node-cli-command-resolution.ts stays under max-lines.

The test stages every path through `join` and asserts via
detectCommandsInInstallDirs as well as resolveCliCommands, so it holds on a
Windows dev machine and pins the "absolute path means installed" contract.

* fix(agents): align system install-dir order across the three PATH lists

Round-2 review remediation. The blocking finding was about the handoff
artifact, not the code: the summary handed to review described a 2-file/+38
change with 3 new macOS dirs, while HEAD is 4 files/+227 with 6 lookup dirs
plus 5 new WSL-guest prelude entries, and the quoted failing test names never
existed. Restated against HEAD in the handoff; no rebuttal, the reviewer was
right.

Justification for the entries the summary never described: the fallback exists
to close the native/WSL asymmetry for a CLI no version manager installed, and
patchPackagedProcessPath already seeds Linuxbrew, /snap/bin and both nix
profile dirs (configure-process.ts:141-158). Shipping only the darwin subset
would have left a Linux or WSL user with a snap/nix install still reading as
not installed while the packaged macOS user did not -- the asymmetry the change
is for.

Code changes, all from the non-blocking list:

- The three lists disagreed on order while claiming to be kept in step, so a
  CLI in both /usr/local/bin and /snap/bin could resolve to a different binary
  than the seeded PATH scan or the WSL guest probe found. All three now use the
  seed's relative order, pinned by a new duplicate-install test and an offset
  assertion on the prelude. The prelude's system block also moved after the nvm
  glob so a version manager still wins in the guest, as it does natively.
- The parity docstring asserted "the same set patchPackagedProcessPath appends,
  minus the sbin dirs and the generic ~/bin", which was false: it also omits
  ~/.vite-plus/bin (seeded by configure-process.ts:156, but no probed agent
  command maps to it) and /opt/homebrew off darwin. All three gaps are now
  named as deliberate.
- win32 returns [] and stays that way, but the branch now says why:
  %USERPROFILE%\.opencode\bin has never had install-dir coverage in either
  list, and the seed's system block is POSIX-only too. Pre-existing, unchanged.
- The detectCommandsInInstallDirs case read the ambient process.env.PATH, so on
  a box with /usr/local/bin on PATH only the opencode assertion exercised the
  fallback. It now stubs the GUI-launch PATH, so both do.

Unchanged and restated: this does NOT explain the packaged macOS v1.4.194
report of codex/cursor-agent/opencode all undetected, and must not close G6
report #3. patchPackagedProcessPath returns early unless app.isPackaged and
seeds all six dirs before any detection, and mergePathSegments never deletes
them, so the packaged PATH scan reaches them first and this fallback never
fires there. Decisive only for the `orca` CLI's detectSkillsCliAgentKeys and
unpackaged/dev runs.

Verification: the suite is red without the production hunks (5 failed/3 passed)
and green with them (8 passed); 11 related suites pass (197 tests); tc:node,
tc:cli, oxlint, oxfmt and the max-lines ratchet are clean.

* fix(agents): correct the ordering-parity claims and widen the seed-leak guard

Round-3 review found two docstring claims that are false as written and one
guard that only asserted a third of its list.

- system-cli-install-dirs.ts claimed a CLI in two of these dirs resolves the
  same here as in the packaged PATH seed. True inside the system block, false
  across it: `claude` in both ~/.local/bin and /opt/homebrew/bin resolves to
  ~/.local/bin via the fallback (getBaseVersionManagerDirectories leads) and to
  /opt/homebrew/bin via the seed, which appends ~/.local/bin last. Scope the
  claim to the block and name the gap instead of asserting it away. No behavior
  change: closing it would hoist a system dir over a version-manager one
  (#18234).
- posix-version-manager-bin-dirs.ts justified moving "/usr/local/bin" after the
  nvm glob with "a version manager still wins in the guest, as it does
  natively". The glob expands lexicographically; native orders nvm dirs
  default-alias-first (#10932), so that is not parity. Record the move as the
  one behavior change in the file and bound it: entries are appended behind a
  resolved login PATH, and both consumers only test presence.
- The #18234 seed-leak guard asserted only /opt/homebrew/bin and /usr/local/bin
  were absent from getVersionManagerBinPaths, leaving the four other new dirs
  unpinned. It now spells out all seven across darwin and linux -- spelled out
  rather than derived from getSystemCliInstallDirectories, which would pass
  vacuously against exactly the refactor it guards.

Tests 8 passed (8); 5 failed / 3 passed with the production hunks reverted to
origin/main. tsc node + cli clean, oxlint clean.

* fix(agents): find Pi's own installer dir in the CLI install-dir fallback

The fallback added `~/.opencode/bin` but skipped `~/.vite-plus/bin` on the
claim that no probed agent command maps to it. False: `pi` is a probed detect
command on every runtime (`tui-agent-config.ts`, no `detectUnsupportedRuntimes`)
and `~/.vite-plus/bin` is the Pi installer's default — the two dirs #829 named
and `patchPackagedProcessPath` seeds together. Added to both the native
fallback and the WSL guest prelude, so a Pi installed by its own script is
found by the `orca` CLI and in WSL, not just on a seeded packaged PATH.

Also, all narrower:
- `/snap/bin` + Linuxbrew now gate on `linux` like the seed does, instead of
  every non-darwin posix.
- Docstring: `/opt/homebrew` off darwin is the one remaining seed gap and says
  why; the "lookup-only" paragraph names the `withCliRuntimeOnPath` exception.
- New seed-order test derives the expected order from
  `getSystemCliInstallDirectories`, so reordering either list fails.

The PR body's claim that SSH hosts benefit is dropped: they answer
`preflight.detectAgents` from `src/relay/preflight-handler.ts` via
`isCommandOnPathForRelay`, a separate bundle that never reaches this module.

* fix(agents): build the CLI install-dir fallback order once and pin it on both resolvers

resolveCliCommand (every spawn site) and resolveCliCommands (detection) each
spelled the nvm -> version-manager -> system-dir order by hand, which is how
the native and WSL lists drifted apart before. One getCliInstallDirectories
now feeds both, and the test pins system dirs LAST on both resolvers, on
darwin and linux, plus a derived check that the WSL guest prelude keeps every
native version-manager dir ahead of the native system block.
2026-09-04 15:18:07 -07:00

382 lines
14 KiB
TypeScript

import { accessSync, constants, existsSync, readFileSync, readdirSync, statSync } from 'node:fs'
import { homedir } from 'node:os'
import { delimiter, dirname, isAbsolute, join } from 'node:path'
import { getSystemCliInstallDirectories } from './system-cli-install-dirs'
type ResolveCommandOptions = {
pathEnv?: string | null
platform?: NodeJS.Platform
homePath?: string
}
function getExecutableNames(platform: NodeJS.Platform, commandName: string): string[] {
if (platform === 'win32') {
return [`${commandName}.cmd`, `${commandName}.exe`, `${commandName}.bat`, commandName]
}
return [commandName]
}
function splitPath(
pathEnv: string | null | undefined,
pathDelimiter: string = delimiter
): string[] {
if (!pathEnv) {
return []
}
return pathEnv
.split(pathDelimiter)
.map((entry) => entry.trim())
.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
}
}
return right.localeCompare(left)
}
function findFirstExecutable(
platform: NodeJS.Platform,
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
}
// Why: GUI fallback probing should skip placeholders/directories so spawn
// can continue to a runnable CLI instead of failing later with EACCES/EISDIR.
accessSync(candidate, constants.X_OK)
return true
} catch {
return false
}
}
function getBaseVersionManagerDirectories(platform: NodeJS.Platform, homePath: string): string[] {
const directories = [
join(homePath, '.volta', 'bin'),
join(homePath, '.asdf', 'shims'),
join(homePath, '.fnm', 'aliases', 'default', 'bin'),
// Why: mise (formerly rtx) exposes managed tool binaries via a shims
// directory, similar to asdf.
join(homePath, '.local', 'share', 'mise', 'shims')
]
if (platform === 'win32') {
// Why: Anthropic's native Windows installer places claude.exe here, and
// GUI-launched Orca may not inherit the user's PATH entry for it.
directories.push(join(homePath, '.local', 'bin'))
directories.push(join(homePath, 'AppData', 'Roaming', 'npm'))
directories.push(join(homePath, 'AppData', 'Local', 'pnpm'))
directories.push(join(homePath, 'AppData', 'Local', 'Yarn', 'bin'))
} else {
directories.push(join(homePath, '.local', 'bin'))
// Why: pnpm uses platform-specific global bin directories that differ from
// npm's ~/.local/bin.
if (platform === 'darwin') {
directories.push(join(homePath, 'Library', 'pnpm'))
} else {
directories.push(join(homePath, '.local', 'share', 'pnpm'))
}
directories.push(join(homePath, '.yarn', 'bin'))
}
directories.push(join(homePath, '.bun', 'bin'))
return directories
}
// Why bounded and cycle-guarded: an nvm alias may point at another alias
// (`default` -> `lts/*` -> `lts/krypton` -> a version), and a hand-edited pair
// can point at each other. nvm's own resolver tracks seen aliases; mirror that
// rather than trusting the files to be acyclic. Termination comes from the hop
// bound; the seen-set is what turns a cycle into "no preference" instead of
// silently resolving whichever alias the walk happened to stop on.
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<string>()
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 {
// Why this is a cheap check and not the actual containment: join() normalizes
// `..` away before we ever see it, so this only rejects the literal spelling.
// The real guarantee is downstream — matchNvmVersion can only ever return an
// entry of readdirSync(versions/node), so no token can put a foreign path on
// PATH regardless of what the alias file says.
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 {
// Why a full shape check: parseVersionSegment coerces every unparseable
// segment to 0 (parseInt stops at the first non-digit), so an unresolvable
// token prefix-matched `v0.12.x` — or any stray non-version directory —
// instead of matching nothing. Anchoring only the first character was not
// enough: `0x18`, `00` and `0abc` all still parsed to [0]. nvm writes such a
// token to the alias file even while warning it does not exist, then answers
// N/A for it, and so must we — which leaves newest-first ordering untouched.
// Leading zeros are rejected for the same reason: nvm calls `00` and `024`
// N/A, while parseInt happily reads them as 0 and 24.
// (A length check cannot catch any of this: ''.split('.') is [''].)
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 []
}
// Why default-first rather than newest-first: this ordering decides which node
// a CLI runs under whenever the login-shell probe does not land. Newest is
// usually the version the user just installed and has put nothing into, so it
// hid every globally installed CLI and mismatched native module ABIs
// (stablyai/orca#10932). The rest stay behind it as fallbacks, so a CLI
// installed outside the default version is still 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,
executableNames: string[]
): string[] {
const directories = getBaseVersionManagerDirectories(platform, homePath)
const firstNvmMatch = findFirstExecutable(
platform,
getNvmVersionDirectories(homePath),
executableNames
)
if (firstNvmMatch) {
directories.unshift(dirname(firstNvmMatch))
}
return directories
}
// Why one list for both resolvers: the system block must stay LAST so a
// version-manager install always outranks a Homebrew/npm/snap one, and two
// hand-spelled spreads is how the native and WSL lists drifted apart before.
function getCliInstallDirectories(platform: NodeJS.Platform, homePath: string): string[] {
return [
...getNvmVersionDirectories(homePath),
...getBaseVersionManagerDirectories(platform, homePath),
...getSystemCliInstallDirectories(platform, homePath)
]
}
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
}
export function resolveCliCommands(
commandNames: readonly string[],
options: ResolveCommandOptions = {}
): Map<string, string> {
const platform = options.platform ?? process.platform
const pathEnv = options.pathEnv ?? process.env.PATH ?? process.env.Path ?? null
const pathDirectories = splitPath(pathEnv)
const homePath = options.homePath ?? homedir()
const installDirectories = getCliInstallDirectories(platform, homePath)
const resolved = new Map<string, string>()
for (const commandName of new Set(commandNames)) {
const executableNames = getExecutableNames(platform, commandName)
const pathCandidate = findFirstExecutable(platform, pathDirectories, executableNames)
const installCandidate =
pathCandidate ?? findFirstExecutable(platform, installDirectories, executableNames)
resolved.set(commandName, installCandidate ?? commandName)
}
return resolved
}
export function resolveCodexCommand(options: ResolveCommandOptions = {}): string {
return resolveCliCommand('codex', options)
}
export function resolveClaudeCommand(options: ResolveCommandOptions = {}): string {
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<T extends NodeJS.ProcessEnv>(
commandPath: string,
env: T,
options: Pick<ResolveCommandOptions, 'platform'> = {}
): 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
const homePath = options.homePath ?? homedir()
const nodeNames = getExecutableNames(platform, 'node')
return getVersionManagerDirectories(platform, homePath, nodeNames)
}