mirror of
https://github.com/stablyai/orca.git
synced 2026-10-09 16:02:46 +00:00
Neil
901c6771ff
* 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.
66 lines
2.7 KiB
TypeScript
66 lines
2.7 KiB
TypeScript
/**
|
|
* Where a version manager puts binaries inside a POSIX guest.
|
|
*
|
|
* The guest-side twin of `detectCommandsInInstallDirs`, which the native
|
|
* preflight branch already consults for the same reason -- "PATH may still be
|
|
* unhydrated on a cold GUI launch". Without it, a WSL probe that cannot
|
|
* establish the login PATH reports an nvm-installed claude/codex as not
|
|
* installed, which is #9725.
|
|
*
|
|
* Kept in step with `getBaseVersionManagerDirectories` in
|
|
* node-cli-command-resolution.ts and `getSystemCliInstallDirectories` in
|
|
* system-cli-install-dirs.ts: a WSL user on asdf, mise, volta or fnm -- or on
|
|
* Linuxbrew, snap or nix -- would otherwise still hit #9725 while the same user
|
|
* on native does not. `/opt/homebrew` stays out because a WSL guest is Linux,
|
|
* where Homebrew installs to the Linuxbrew prefix below.
|
|
*
|
|
* Version-manager dirs lead and the system block trails, which took the one
|
|
* behavior change here: `/usr/local/bin` moved from before the nvm glob to
|
|
* after it, so the guest ranks a version manager over a system install the way
|
|
* native does. Bounded, not free: every entry is APPENDED behind a resolved
|
|
* login PATH, so this can only re-rank a command that BOTH consumers would
|
|
* otherwise miss, and both only test presence. Not full parity either -- the
|
|
* glob expands lexicographically, while native orders nvm dirs
|
|
* default-alias-first (#10932).
|
|
*
|
|
* Each entry is quoted so a `$HOME` containing a space cannot word-split into
|
|
* a relative path -- except the nvm glob, where only the prefix is quoted so
|
|
* the `*` still expands.
|
|
*/
|
|
const POSIX_VERSION_MANAGER_BIN_DIRS = [
|
|
'"$HOME/.local/bin"',
|
|
'"$HOME/.local/share/pnpm"',
|
|
'"$HOME/.yarn/bin"',
|
|
'"$HOME/.bun/bin"',
|
|
'"$HOME/.volta/bin"',
|
|
'"$HOME/.asdf/shims"',
|
|
'"$HOME/.fnm/aliases/default/bin"',
|
|
'"$HOME/.local/share/mise/shims"',
|
|
'"$HOME"/.nvm/versions/node/*/bin',
|
|
'"/usr/local/bin"',
|
|
'"/snap/bin"',
|
|
'"/home/linuxbrew/.linuxbrew/bin"',
|
|
'"/nix/var/nix/profiles/default/bin"',
|
|
'"$HOME/.nix-profile/bin"',
|
|
// Why both: the opencode and Pi installers' own defaults, which no version manager owns (#829).
|
|
'"$HOME/.opencode/bin"',
|
|
'"$HOME/.vite-plus/bin"'
|
|
].join(' ')
|
|
|
|
/**
|
|
* A prelude that APPENDS those directories to PATH.
|
|
*
|
|
* Append, never prepend: when the login PATH did resolve it is authoritative,
|
|
* and a prepended fallback could shadow the binary the user actually runs with
|
|
* an older one from a stale nvm version directory.
|
|
*/
|
|
export function buildPosixFallbackPathPrelude(): string {
|
|
return [
|
|
`for _orca_dir in ${POSIX_VERSION_MANAGER_BIN_DIRS}; do`,
|
|
' if [ -d "$_orca_dir" ]; then PATH="$PATH:$_orca_dir"; fi',
|
|
'done',
|
|
'export PATH',
|
|
'unset _orca_dir'
|
|
].join('\n')
|
|
}
|