From 813dc631446a91e2898ca2b60b90bb4394da1f7f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 4 Sep 2026 00:08:23 -0700 Subject: [PATCH] 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. --- .../agent-cli-install-dir-fallback.test.ts | 69 ++++++++++++++++--- src/shared/node-cli-command-resolution.ts | 33 ++++----- 2 files changed, 74 insertions(+), 28 deletions(-) diff --git a/src/shared/agent-cli-install-dir-fallback.test.ts b/src/shared/agent-cli-install-dir-fallback.test.ts index 641677551a6..793c0861453 100644 --- a/src/shared/agent-cli-install-dir-fallback.test.ts +++ b/src/shared/agent-cli-install-dir-fallback.test.ts @@ -1,8 +1,13 @@ import { delimiter, join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { detectCommandsInInstallDirs } from './local-agent-install-dir-detection' -import { getVersionManagerBinPaths, resolveCliCommands } from './node-cli-command-resolution' +import { + getVersionManagerBinPaths, + resolveCliCommand, + resolveCliCommands +} from './node-cli-command-resolution' import { buildPosixFallbackPathPrelude } from './posix-version-manager-bin-dirs' +import { getSystemCliInstallDirectories } from './system-cli-install-dirs' /** * The install-dir fallback answers "is this agent CLI installed?" whenever the @@ -148,15 +153,40 @@ describe('agent CLI install-dir fallback', () => { } }) - it('still lets a version-manager install outrank a system one', () => { - const home = '/Users/tester' - stage( - join(home, '.volta', 'bin', 'codex'), - join('/opt/homebrew/bin', 'codex'), - join('/usr/local/bin', 'codex') - ) - expect(resolveAll(['codex'], { platform: 'darwin', homePath: home })).toEqual({ - codex: join(home, '.volta', 'bin', 'codex') + // Why both resolvers and both platforms: resolveCliCommand is what every + // spawn site (codex login, app-server, session-index heal) calls, and its + // list was once spelled separately from resolveCliCommands'. A same-named + // binary in /usr/local/bin must never shadow the one a version manager owns. + describe.each([ + { platform: 'darwin' as const, home: '/Users/tester', systemDir: '/opt/homebrew/bin' }, + { + platform: 'linux' as const, + home: '/home/tester', + systemDir: '/home/linuxbrew/.linuxbrew/bin' + } + ])('$platform: system dirs stay last', ({ platform, home, systemDir }) => { + it('lets a version-manager install outrank a system one', () => { + const managed = join(home, '.volta', 'bin', 'codex') + stage(managed, join(systemDir, 'codex'), join('/usr/local/bin', 'codex')) + expect(resolveCliCommand('codex', { platform, homePath: home })).toBe(managed) + expect(resolveAll(['codex'], { platform, homePath: home })).toEqual({ codex: managed }) + }) + + it('lets an npm --user (~/.local/bin) install outrank a system one', () => { + const managed = join(home, '.local', 'bin', 'codex') + stage(managed, join(systemDir, 'codex')) + expect(resolveCliCommand('codex', { platform, homePath: home })).toBe(managed) + expect(resolveAll(['codex'], { platform, homePath: home })).toEqual({ codex: managed }) + }) + + it('lets a copy already on PATH outrank every install dir', () => { + const onPath = join('/custom/bin', 'codex') + const pathEnv = [GUI_LAUNCH_PATH, '/custom/bin'].join(delimiter) + stage(onPath, join(home, '.volta', 'bin', 'codex'), join(systemDir, 'codex')) + expect(resolveCliCommand('codex', { platform, homePath: home, pathEnv })).toBe(onPath) + expect(resolveCliCommands(['codex'], { platform, homePath: home, pathEnv })).toEqual( + new Map([['codex', onPath]]) + ) }) }) @@ -224,4 +254,23 @@ describe('agent CLI install-dir fallback', () => { // Why absent: a WSL guest is Linux, so /opt/homebrew is never its brew prefix. expect(prelude).not.toContain('/opt/homebrew') }) + + // Why derived: the native and guest lists drifted apart once by hand. Every + // version-manager dir the native resolver knows must precede the guest's + // first system dir, and the guest's system block must be the native one. + it('keeps the WSL guest prelude in step with the native Linux lists', () => { + const prelude = buildPosixFallbackPathPrelude() + const asGuest = (dir: string): string => `"${dir.split('\\').join('/')}"` + const systemDirs = getSystemCliInstallDirectories('linux', '$HOME').map(asGuest) + const firstSystemOffset = prelude.indexOf(systemDirs[0]) + expect(firstSystemOffset).toBeGreaterThan(0) + for (const dir of getVersionManagerBinPaths({ platform: 'linux', homePath: '$HOME' })) { + const offset = prelude.indexOf(asGuest(dir)) + expect(offset, dir).toBeGreaterThanOrEqual(0) + expect(offset, dir).toBeLessThan(firstSystemOffset) + } + const systemOffsets = systemDirs.map((dir) => prelude.indexOf(dir)) + expect(systemOffsets.every((offset) => offset >= firstSystemOffset)).toBe(true) + expect([...systemOffsets].sort((a, b) => a - b)).toEqual(systemOffsets) + }) }) diff --git a/src/shared/node-cli-command-resolution.ts b/src/shared/node-cli-command-resolution.ts index 0e8753f04ca..c7407ac5eb1 100644 --- a/src/shared/node-cli-command-resolution.ts +++ b/src/shared/node-cli-command-resolution.ts @@ -246,6 +246,17 @@ function getVersionManagerDirectories( 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 = {} @@ -259,22 +270,12 @@ export function resolveCliCommand( } const homePath = options.homePath ?? homedir() - const nvmCandidate = findFirstExecutable( + const installCandidate = findFirstExecutable( platform, - getNvmVersionDirectories(homePath), + getCliInstallDirectories(platform, homePath), executableNames ) - const versionManagerCandidate = - nvmCandidate ?? - findFirstExecutable( - platform, - [ - ...getBaseVersionManagerDirectories(platform, homePath), - ...getSystemCliInstallDirectories(platform, homePath) - ], - executableNames - ) - return versionManagerCandidate ?? commandName + return installCandidate ?? commandName } export function resolveCliCommands( @@ -285,11 +286,7 @@ export function resolveCliCommands( const pathEnv = options.pathEnv ?? process.env.PATH ?? process.env.Path ?? null const pathDirectories = splitPath(pathEnv) const homePath = options.homePath ?? homedir() - const installDirectories = [ - ...getNvmVersionDirectories(homePath), - ...getBaseVersionManagerDirectories(platform, homePath), - ...getSystemCliInstallDirectories(platform, homePath) - ] + const installDirectories = getCliInstallDirectories(platform, homePath) const resolved = new Map() for (const commandName of new Set(commandNames)) {