Files
orca/src/shared/nvm-default-alias.test.ts
Neil 4218d5068e fix(cli): seed nvm's default version, not the newest install (#16420)
* fix(cli): seed nvm's default version, not the newest install

#16314 stopped the login-shell probe inheriting the seeded PATH, but left the
seed itself picking the newest installed nvm version. That ordering decides
which node a CLI runs under whenever the probe does not land — a timeout, or a
login shell whose rc never initializes nvm — and newest is precisely the wrong
guess: it is usually the version the user just added and has installed nothing
into. That is the root cause reported in #10932.

Resolve `alias/default` instead, mirroring nvm: follow the alias chain
(`default` -> `lts/*` -> `lts/krypton` -> a version), resolve a partial version
like `24` to the highest matching install, and treat `system`/`node`/`stable`
as no preference. The chain is bounded and cycle-guarded because nvm's own
resolver tracks seen aliases and hand-edited files can point at each other.

Ordering is a preference, not a restriction: the remaining versions stay behind
the default, so a CLI installed outside it is still reachable.

Measured on a real machine with nvm default=24 and a bare v26.7.0 installed:
the old resolver seeds v26.7.0/bin (no CLIs), the new one seeds v24.18.0/bin
(every CLI). Tests were written first and verified to fail on the three bug
cases against main before the fix existed.

Also raise the probe budget from 5s to 10s. The old value was never measured
against a real profile: a bash -ilc loading nvm, rvm, conda and gcloud takes
~1s idle but 6-7s on a loaded machine, so a cold start under load silently
fell back to the seed. Startup does not block on the probe, and the one
awaited consumer is agent detection, which is better served by a probe that
finishes late than one that gives up early.

* fix(cli): reject non-version alias tokens instead of matching v0.x

Review finding, and a real bug I introduced. parseVersionSegment coerces
every unparseable segment to 0, so an unresolvable default alias — `garbage`,
`iojs`, `lts/nonexistent`, any hand-named alias — became [0] and prefix-matched
a `v0.12.x` install, or any stray non-version directory. Orca would then seed a
decade-old node as the preferred runtime. Real nvm answers N/A for all of them.

The `wanted.length === 0` bail could never have caught this: ''.split('.') is
[''], never empty. Replaced with a shape check that still admits legitimate
numeric prefixes — verified against nvm itself, which resolves `24` to
v24.18.0 and `0` to an installed v0.x while answering N/A for the rest.

Also corrects two comments that no longer described the code: the seed is no
longer "newest install", and the probe budget note claimed startup never blocks
on hydration, which is false on packaged Windows where it gates terminal
services and git. The traversal-guard comment claimed a containment join()
already normalizes away; the real guarantee is that matchNvmVersion can only
return an entry of the versions directory.

* fix(cli): match nvm's version-token grammar, not just its first character

Round-2 review finding, and the same bug one layer down. The previous guard
anchored only the first character, but parseInt stops at the first non-digit,
so `0x18`, `00` and `0abc` still parsed to [0] and prefix-matched a v0.12.x
install — the decade-old-node seed the earlier fix was supposed to close.

Reachable: `nvm alias default 0x18` warns that the version does not exist and
writes the alias anyway, then resolves it to N/A.

Use nvm's actual grammar, leading zeros included — nvm calls `00` and `024`
N/A while parseInt reads them as 0 and 24. Verified by executing 17 tokens
against a five-version fixture: every one now agrees with nvm, including the
legitimate prefixes `0`, `0.12`, `24` and `v24.18.0`.

Also drops a dead disjunct (the hop bound already caps the loop, so seen.size
can never exceed it) and corrects the log comment in index.ts, which still
told the reader a failed probe leaves the newest install in front. It leaves
the default version in front now, which is usually survivable but still not
what the shell would have resolved.

* test(cli): skip the lts/* chain fixture on Windows

Round-3 review finding. makeNvmHome materializes each alias as a real file,
and the chain case uses nvm's actual `lts/*` alias — `*` is a reserved Win32
filename character, so writeFileSync fails with EINVAL. PR CI runs a Windows
allowlist that excludes this file, so the breakage only reaches a Windows
developer running the suite locally.

Skipped rather than renamed: `lts/*` is the alias nvm really ships, and the
assertion pins platform: 'darwin' anyway, so the real name costs no coverage.
Matches the skipIf convention already used across src/shared.

Also reflows a comment line that a previous edit ran to 143 characters;
oxfmt does not reflow comments, so nothing would have caught it.
2026-08-25 03:05:24 -07:00

159 lines
6.3 KiB
TypeScript

import { chmodSync, mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { describe, expect, it } from 'vitest'
import { getVersionManagerBinPaths, resolveCliCommand } from './node-cli-command-resolution'
/**
* The seed decides which node a CLI runs under whenever the login-shell probe
* does not land (timeout, or a shell whose rc never initializes nvm). Picking
* the newest install there is what broke #10932: the newest version is usually
* the one the user just added and has installed nothing into.
*/
function makeNvmHome(options: {
versions: string[]
defaultAlias?: string
aliases?: Record<string, string>
cliIn?: string
}): string {
const home = mkdtempSync(join(tmpdir(), 'orca-nvm-'))
for (const version of options.versions) {
const bin = join(home, '.nvm', 'versions', 'node', version, 'bin')
mkdirSync(bin, { recursive: true })
for (const name of ['node', 'node.exe']) {
writeFileSync(join(bin, name), '')
chmodSync(join(bin, name), 0o755)
}
if (options.cliIn === version) {
writeFileSync(join(bin, 'codex'), '')
chmodSync(join(bin, 'codex'), 0o755)
}
}
if (options.defaultAlias !== undefined) {
mkdirSync(join(home, '.nvm', 'alias'), { recursive: true })
writeFileSync(join(home, '.nvm', 'alias', 'default'), `${options.defaultAlias}\n`)
}
for (const [name, value] of Object.entries(options.aliases ?? {})) {
const file = join(home, '.nvm', 'alias', name)
mkdirSync(join(file, '..'), { recursive: true })
writeFileSync(file, `${value}\n`)
}
return home
}
function seededNvmDir(homePath: string): string | undefined {
return getVersionManagerBinPaths({ platform: 'darwin', pathEnv: '', homePath }).find((entry) =>
entry.includes('.nvm')
)
}
describe('nvm default alias decides the seeded runtime', () => {
it('seeds the default version, not the newest install (#10932)', () => {
// The exact shape that broke: `nvm install 26` adds a bare newest version
// while every CLI lives under the default.
const home = makeNvmHome({
versions: ['v24.18.0', 'v26.7.0'],
defaultAlias: '24',
cliIn: 'v24.18.0'
})
expect(seededNvmDir(home)).toBe(join(home, '.nvm', 'versions', 'node', 'v24.18.0', 'bin'))
})
it('resolves a partial default to the highest matching install', () => {
const home = makeNvmHome({
versions: ['v24.9.0', 'v24.18.0', 'v26.7.0'],
defaultAlias: '24',
cliIn: 'v24.18.0'
})
expect(seededNvmDir(home)).toBe(join(home, '.nvm', 'versions', 'node', 'v24.18.0', 'bin'))
})
// Why skipIf rather than renaming the fixture: `lts/*` is the real alias nvm
// ships, and `*` is a reserved Win32 filename character, so the fixture cannot
// be materialized there. Keeping the real name is worth more than the case
// running on a platform where seededNvmDir already pins platform: 'darwin'.
it.skipIf(process.platform === 'win32')(
'follows an alias chain (default -> lts/* -> lts/krypton -> version)',
() => {
const home = makeNvmHome({
versions: ['v22.9.0', 'v26.7.0'],
defaultAlias: 'lts/*',
aliases: { 'lts/*': 'lts/krypton', 'lts/krypton': 'v22.9.0' },
cliIn: 'v22.9.0'
})
expect(seededNvmDir(home)).toBe(join(home, '.nvm', 'versions', 'node', 'v22.9.0', 'bin'))
}
)
it('falls back to newest when there is no default alias', () => {
const home = makeNvmHome({ versions: ['v24.18.0', 'v26.7.0'] })
expect(seededNvmDir(home)).toBe(join(home, '.nvm', 'versions', 'node', 'v26.7.0', 'bin'))
})
it('falls back to newest when the default names an uninstalled version', () => {
const home = makeNvmHome({ versions: ['v24.18.0', 'v26.7.0'], defaultAlias: '18' })
expect(seededNvmDir(home)).toBe(join(home, '.nvm', 'versions', 'node', 'v26.7.0', 'bin'))
})
it('survives a cyclic alias chain instead of hanging', () => {
const home = makeNvmHome({
versions: ['v24.18.0', 'v26.7.0'],
defaultAlias: 'a',
aliases: { a: 'b', b: 'a' }
})
expect(seededNvmDir(home)).toBe(join(home, '.nvm', 'versions', 'node', 'v26.7.0', 'bin'))
})
it('ignores a `system` default, which means no nvm node at all', () => {
const home = makeNvmHome({ versions: ['v24.18.0', 'v26.7.0'], defaultAlias: 'system' })
expect(seededNvmDir(home)).toBe(join(home, '.nvm', 'versions', 'node', 'v26.7.0', 'bin'))
})
// Why the digit-leading entries: parseInt stops at the first non-digit, so
// anchoring only the first character still let `0x18` and `00` parse to 0 and
// prefix-match a decade-old v0.x. nvm writes such a token to the alias file
// even while warning it does not exist, then answers N/A for it.
it.each([
'garbage',
'iojs',
'lts/nonexistent',
'my-custom-alias',
'0x18',
'00',
'0abc',
'024',
'24abc',
'v24.18.0-nightly',
'V24'
])('treats the unresolvable default %s as no preference, like nvm N/A', (token) => {
// Why v0.12.7 is in the fixture: parseVersionSegment coerces unparseable
// segments to 0, so before the shape guard these tokens became [0] and
// prefix-matched the 0.x install — seeding a decade-old node.
const home = makeNvmHome({
versions: ['v0.12.7', 'v24.18.0', 'v26.7.0'],
defaultAlias: token
})
expect(seededNvmDir(home)).toBe(join(home, '.nvm', 'versions', 'node', 'v26.7.0', 'bin'))
})
it('still treats a numeric default as a version prefix, matching nvm', () => {
// The guard must reject non-versions without rejecting legitimate prefixes:
// real nvm resolves `0` to an installed v0.x.
const home = makeNvmHome({ versions: ['v0.12.7', 'v24.18.0'], defaultAlias: '0' })
expect(seededNvmDir(home)).toBe(join(home, '.nvm', 'versions', 'node', 'v0.12.7', 'bin'))
})
it('still finds a CLI that lives outside the default version', () => {
// Ordering must be a preference, not a restriction: the other versions stay
// as fallbacks so a CLI installed elsewhere is still reachable.
const home = makeNvmHome({
versions: ['v24.18.0', 'v26.7.0'],
defaultAlias: '24',
cliIn: 'v26.7.0'
})
expect(resolveCliCommand('codex', { platform: 'darwin', pathEnv: '', homePath: home })).toBe(
join(home, '.nvm', 'versions', 'node', 'v26.7.0', 'bin', 'codex')
)
})
})