From bcd59481db14e0c20ef051ca622d903263b8a762 Mon Sep 17 00:00:00 2001
From: Neil <4138956+nwparker@users.noreply.github.com>
Date: Thu, 3 Sep 2026 03:35:10 -0700
Subject: [PATCH] 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.
---
 .../agent-cli-install-dir-fallback.test.ts    | 21 ++++++++++++++++---
 src/shared/posix-version-manager-bin-dirs.ts  | 11 +++++++---
 src/shared/system-cli-install-dirs.ts         | 20 +++++++++++-------
 3 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/src/shared/agent-cli-install-dir-fallback.test.ts b/src/shared/agent-cli-install-dir-fallback.test.ts
index fddaf81958b..68442ebb84e 100644
--- a/src/shared/agent-cli-install-dir-fallback.test.ts
+++ b/src/shared/agent-cli-install-dir-fallback.test.ts
@@ -163,9 +163,24 @@ describe('agent CLI install-dir fallback', () => {
   // patchPackagedProcessPath and the CLI's addAgentNodePaths, so a system dir
   // leaking into it would re-rank binaries the user already has (#18234).
   it('keeps system install dirs out of the PATH seed list', () => {
-    const seeded = getVersionManagerBinPaths({ platform: 'darwin', homePath: '/Users/tester' })
-    expect(seeded).not.toContain('/opt/homebrew/bin')
-    expect(seeded).not.toContain('/usr/local/bin')
+    for (const platform of ['darwin', 'linux'] as const) {
+      const home = platform === 'darwin' ? '/Users/tester' : '/home/tester'
+      const seeded = getVersionManagerBinPaths({ platform, homePath: home })
+      // Spelled out, not derived from the list under test: a guard that iterates
+      // getSystemCliInstallDirectories passes vacuously if that list is emptied
+      // into getBaseVersionManagerDirectories, which is the leak it guards.
+      for (const directory of [
+        '/opt/homebrew/bin',
+        '/usr/local/bin',
+        '/snap/bin',
+        '/home/linuxbrew/.linuxbrew/bin',
+        '/nix/var/nix/profiles/default/bin',
+        join(home, '.nix-profile', 'bin'),
+        join(home, '.opencode', 'bin')
+      ]) {
+        expect(seeded).not.toContain(directory)
+      }
+    }
   })
 
   // Why through this entry point: it is what the `orca` CLI's agent detection
diff --git a/src/shared/posix-version-manager-bin-dirs.ts b/src/shared/posix-version-manager-bin-dirs.ts
index 61f45be0eda..14c43dc3602 100644
--- a/src/shared/posix-version-manager-bin-dirs.ts
+++ b/src/shared/posix-version-manager-bin-dirs.ts
@@ -14,9 +14,14 @@
  * 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, in the same relative
- * order as the other two lists, so a CLI present in two of them resolves to the
- * same binary in the guest as it does natively.
+ * 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
diff --git a/src/shared/system-cli-install-dirs.ts b/src/shared/system-cli-install-dirs.ts
index 2095404fb42..d814ebb07eb 100644
--- a/src/shared/system-cli-install-dirs.ts
+++ b/src/shared/system-cli-install-dirs.ts
@@ -6,13 +6,19 @@ import { join } from 'node:path'
  * (#829 named `~/.opencode/bin` as the motivating case, but only for the
  * login-shell probe; the fallback used when that probe fails never gained it).
  *
- * Ordered to match what `patchPackagedProcessPath` appends to PATH, so a CLI
- * present in two of these dirs resolves to the same binary here, in the packaged
- * PATH scan, and in `POSIX_VERSION_MANAGER_BIN_DIRS`. Three deliberate gaps vs
- * that seed: the `sbin` dirs and the generic `~/bin` / `~/.local/bin` (no agent
- * CLI installer targets those, and `~/.local/bin` is already a version-manager
- * dir here); `~/.vite-plus/bin`, which no probed agent command maps to; and
- * `/opt/homebrew` off darwin, since a Linux box's brew prefix is Linuxbrew's.
+ * Ordered to match the system block `patchPackagedProcessPath` appends to PATH,
+ * so a CLI present in two of *these* dirs resolves to the same binary here, in
+ * the packaged PATH scan, and in `POSIX_VERSION_MANAGER_BIN_DIRS`. That parity
+ * stops at the block boundary and is not claimed across it: the seed appends
+ * `~/.local/bin` after this block, while here it arrives ahead of it from
+ * `getBaseVersionManagerDirectories`, so a `claude` installed in both
+ * `~/.local/bin` and `/opt/homebrew/bin` resolves to the former via this
+ * fallback and the latter via the seeded PATH. Pre-existing, and left alone
+ * because closing it means hoisting a system dir over a version-manager one.
+ *
+ * Deliberate gaps vs that seed: the `sbin` dirs and the generic `~/bin`;
+ * `~/.vite-plus/bin`, which no probed agent command maps to; and `/opt/homebrew`
+ * off darwin, since a Linux box's brew prefix is Linuxbrew's.
  *
  * Lookup-only, deliberately outside `getBaseVersionManagerDirectories`: that
  * list is PREPENDED to PATH by `getVersionManagerBinPaths` callers, and hoisting