From e5ba5975df248bb8baeced6aa0519647b49b7e51 Mon Sep 17 00:00:00 2001 From: Aashish Mahato <145881415+aashish254@users.noreply.github.com> Date: Sun, 4 Oct 2026 09:30:52 +0545 Subject: [PATCH] Recover working local forge CLIs behind broken PATH launchers Recover a working local forge CLI when an earlier PATH launcher is broken. Bound executable probes and reuse the verified selection for native operations without replaying authentication or user requests. Fixes #22975 Co-authored-by: Aashish <145881415+aashish254@users.noreply.github.com> --- .github/workflows/pr.yml | 2 + config/scripts/pr-code-change-scope.mjs | 2 + .../git/command-runner/exec-file-capture.ts | 6 +- src/main/ipc/command-path-resolver.test.ts | 129 ++++++- src/main/ipc/command-path-resolver.ts | 143 +++++++- .../ipc/preflight-agent-detection.test.ts | 10 +- src/main/ipc/preflight-agent-refresh.test.ts | 10 +- src/main/ipc/preflight-command-exec.test.ts | 254 +++++++++++++- src/main/ipc/preflight-command-exec.ts | 142 +++++++- .../ipc/preflight-host-cli-status.test.ts | 120 ++++++- ...eflight-provider-command-selection.test.ts | 321 ++++++++++++++++++ src/main/ipc/preflight-remote-ssh.test.ts | 10 +- .../ipc/preflight-runnable-local-cli.test.ts | 249 ++++++++++++++ src/main/ipc/preflight-test-harness.ts | 7 + src/main/preflight/agent-detection.ts | 39 ++- 15 files changed, 1391 insertions(+), 53 deletions(-) create mode 100644 src/main/ipc/preflight-provider-command-selection.test.ts create mode 100644 src/main/ipc/preflight-runnable-local-cli.test.ts diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index a90bd45d91e..9558255e482 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -1168,6 +1168,8 @@ jobs: src/main/runtime/unreadable-secret-store-preservation.win32.test.ts src/main/ipc/pty-codex-account-attribution.test.ts src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts + src/main/ipc/preflight-provider-command-selection.test.ts + src/main/ipc/preflight-runnable-local-cli.test.ts src/relay/windows-port-scan.win32.test.ts src/main/ssh/ssh-relay-upload-stage-windows-identity.test.ts src/main/ssh/remote-node-runtime-store-windows.test.ts diff --git a/config/scripts/pr-code-change-scope.mjs b/config/scripts/pr-code-change-scope.mjs index 800854c8008..9f2f672dc84 100644 --- a/config/scripts/pr-code-change-scope.mjs +++ b/config/scripts/pr-code-change-scope.mjs @@ -357,6 +357,8 @@ const WINDOWS_PACKAGE_TESTS = [ 'src/main/runtime/unreadable-secret-store-preservation.win32.test.ts', 'src/main/ipc/pty-codex-account-attribution.test.ts', 'src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts', + 'src/main/ipc/preflight-provider-command-selection.test.ts', + 'src/main/ipc/preflight-runnable-local-cli.test.ts', 'src/relay/windows-port-scan.win32.test.ts', 'src/main/ssh/ssh-relay-upload-stage-windows-identity.test.ts', 'src/main/ssh/remote-node-runtime-store-windows.test.ts' diff --git a/src/main/git/command-runner/exec-file-capture.ts b/src/main/git/command-runner/exec-file-capture.ts index b94d2f59509..267fc7e20a5 100644 --- a/src/main/git/command-runner/exec-file-capture.ts +++ b/src/main/git/command-runner/exec-file-capture.ts @@ -2,6 +2,7 @@ import { execFile, type ChildProcess, type ExecFileOptions } from 'node:child_pr import { recordSubprocessSpawn } from '../../diagnostics/main-thread-churn-probe' import { endSubprocessStdin } from '../../../shared/subprocess-stdin-write' import { runProcess } from '../../../shared/child-process/run-process' +import { resolveSelectedLocalCommand } from '../../ipc/command-path-resolver' import type { WslProcessGroupTermination } from '../wsl-process-group-termination' import { createAbortError } from './abort-error' import { killSpawnedCommandTree } from './spawned-command-tree-kill' @@ -30,7 +31,10 @@ export async function execFileCaptureToTermination( // Spawn cost is reported by spawnProcess's observer, which runProcess goes // through; recording it again here would double-count every capture. const pending = runProcess({ - program: command, + program: resolveSelectedLocalCommand(command, { + env: options.env, + cwd: typeof options.cwd === 'string' ? options.cwd : undefined + }), args, cwd: typeof options.cwd === 'string' ? options.cwd : undefined, env: options.env, diff --git a/src/main/ipc/command-path-resolver.test.ts b/src/main/ipc/command-path-resolver.test.ts index 9172d1c2465..9e1c9cd7d5c 100644 --- a/src/main/ipc/command-path-resolver.test.ts +++ b/src/main/ipc/command-path-resolver.test.ts @@ -1,8 +1,8 @@ -import { chmod, mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises' +import { access, chmod, mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import path from 'node:path' import { afterAll, beforeAll, describe, expect, it } from 'vitest' -import { isCommandOnLocalPath } from './command-path-resolver' +import { isCommandOnLocalPath, listLocalCommandPaths } from './command-path-resolver' describe('isCommandOnLocalPath', () => { it('returns false for an empty command', async () => { @@ -112,4 +112,129 @@ describe('isCommandOnLocalPath', () => { ).resolves.toBe(true) }) }) + + // Why the full list exists (#22975): a dead version-manager shim passes the + // same executable check as the binary it shadows, so a winner-only lookup can + // only ever hand the caller the shim. The copies behind it are the answer. + describe.skipIf(process.platform === 'win32')('listLocalCommandPaths', () => { + let front = '' + let middle = '' + let back = '' + + async function executable(dir: string, name: string, body: string): Promise { + await writeFile(path.join(dir, name), body) + await chmod(path.join(dir, name), 0o755) + } + + beforeAll(async () => { + front = await mkdtemp(path.join(tmpdir(), 'cmd-list-front-')) + middle = await mkdtemp(path.join(tmpdir(), 'cmd-list-middle-')) + back = await mkdtemp(path.join(tmpdir(), 'cmd-list-back-')) + await executable(front, 'gh', '#!/usr/bin/env bash\nexec /nope/asdf exec "gh" "$@"\n') + await executable(back, 'gh', "#!/bin/sh\nprintf 'gh version 2.98.0\\n'\n") + // Both a non-executable file and a directory in the middle: neither is a + // match, and neither may end the scan. + await writeFile(path.join(middle, 'gh'), 'not executable\n') + await chmod(path.join(middle, 'gh'), 0o644) + await mkdir(path.join(middle, 'glab')) + await executable(back, 'glab', '#!/bin/sh\n') + }) + + afterAll(async () => { + await rm(front, { recursive: true, force: true }) + await rm(middle, { recursive: true, force: true }) + await rm(back, { recursive: true, force: true }) + }) + + it('returns every match in PATH order, shim first', async () => { + await expect( + listLocalCommandPaths('gh', { + platform: 'linux', + env: { PATH: `${front}:${middle}:${back}` } + }) + ).resolves.toEqual([path.posix.join(front, 'gh'), path.posix.join(back, 'gh')]) + }) + + it('follows PATH order rather than which copy runs', async () => { + await expect( + listLocalCommandPaths('gh', { platform: 'linux', env: { PATH: `${back}:${front}` } }) + ).resolves.toEqual([path.posix.join(back, 'gh'), path.posix.join(front, 'gh')]) + }) + + it('skips a directory that matches the command name and keeps scanning', async () => { + await expect( + listLocalCommandPaths('glab', { + platform: 'linux', + env: { PATH: `${middle}:${front}:${back}` } + }) + ).resolves.toEqual([path.posix.join(back, 'glab')]) + }) + + it('returns an empty list when PATH holds no absolute match', async () => { + await expect( + listLocalCommandPaths('gh', { platform: 'linux', env: { PATH: '' } }) + ).resolves.toEqual([]) + }) + + it('applies the absolute-only gate to the whole list, not just the winner', async () => { + await expect( + listLocalCommandPaths('gh', { + platform: 'linux', + env: { PATH: `.${path.delimiter}${front}` } + }) + ).resolves.toEqual([path.posix.join(front, 'gh')]) + }) + + it('drops a relative match the filesystem would otherwise find', async () => { + // Why a second relative case: `path.posix.join('.', 'gh')` collapses to + // `gh`, so the case above is unaffected by deleting the gate. `fs.access` + // resolves a relative candidate against the process cwd, so only a PATH + // entry that really reaches a fixture proves the filter — and the `access` + // proves that reach first, making this fail loudly instead of going + // vacuous if the suite is ever run from a directory that hides it. + const relative = path.relative(process.cwd(), back) + await expect(access(path.join(relative, 'gh'))).resolves.toBeUndefined() + await expect( + listLocalCommandPaths('gh', { platform: 'linux', env: { PATH: relative } }) + ).resolves.toEqual([]) + }) + + it('lists a PATH entry that repeats only once', async () => { + // Why it matters: callers spawn each entry, and `/usr/local/bin` appearing + // twice in a real PATH is normal — a doomed shim must not be probed twice. + await expect( + listLocalCommandPaths('gh', { + platform: 'linux', + env: { PATH: `${back}:${front}:${back}` } + }) + ).resolves.toEqual([path.posix.join(back, 'gh'), path.posix.join(front, 'gh')]) + }) + + it('returns an empty list for an empty command', async () => { + await expect(listLocalCommandPaths('')).resolves.toEqual([]) + }) + + it('resolves an absolute command path directly', async () => { + await expect( + listLocalCommandPaths(path.join(front, 'gh'), { platform: 'linux', env: { PATH: '' } }) + ).resolves.toEqual([`${front}/gh`]) + }) + + it('lists every PATHEXT permutation on win32, in resolver order', async () => { + const dir = await mkdtemp(path.join(tmpdir(), 'cmd-list-win32-')) + try { + await writeFile(path.join(dir, 'tool.CMD'), '@echo off\n') + await writeFile(path.join(dir, 'tool.EXE'), '') + await expect( + listLocalCommandPaths('tool', { + platform: 'win32', + env: { Path: dir, PATHEXT: '.CMD;.EXE' }, + cwd: dir + }) + ).resolves.toEqual([`${dir}/tool.CMD`, `${dir}/tool.EXE`]) + } finally { + await rm(dir, { recursive: true, force: true }) + } + }) + }) }) diff --git a/src/main/ipc/command-path-resolver.ts b/src/main/ipc/command-path-resolver.ts index 1f2934ba8b2..27808b6aded 100644 --- a/src/main/ipc/command-path-resolver.ts +++ b/src/main/ipc/command-path-resolver.ts @@ -1,4 +1,6 @@ import { access, constants as fsConstants, stat } from 'node:fs/promises' +import { statSync, type Stats } from 'node:fs' +import { homedir } from 'node:os' import path from 'node:path' export type ResolveCommandOptions = { @@ -8,6 +10,8 @@ export type ResolveCommandOptions = { env?: NodeJS.ProcessEnv /** CWD used only for the win32 "search current directory first" rule. */ cwd?: string + /** Stop after this many matches; defaults to the complete list. */ + maxResults?: number } // Why: Windows env keys are case-insensitive (PATH is usually stored as `Path`, @@ -38,6 +42,109 @@ function getWindowsExtensions(env: NodeJS.ProcessEnv, command: string): string[] return extensions } +type LocalCommandSelection = { + scope: string + selected?: { binary: string; stamp: string; cwd?: string } +} + +const localCommandSelections = new Map() + +function selectionScope(options: ResolveCommandOptions): string { + const platform = options.platform ?? process.platform + const env = options.env ?? process.env + const isWin = platform === 'win32' + const pathValue = readEnvCaseInsensitive(env, 'PATH') ?? '' + const pathApi = isWin ? path.win32 : path.posix + const needsCwd = pathValue.split(isWin ? ';' : ':').some((dir) => !pathApi.isAbsolute(dir)) + return JSON.stringify([ + platform, + pathValue, + isWin ? readEnvCaseInsensitive(env, 'PATHEXT') : null, + env.HOME, + env.USERPROFILE, + homedir(), + needsCwd ? (options.cwd ?? process.cwd()) : null + ]) +} + +function commandFileStamp(stats: Stats): string { + return [stats.dev, stats.ino, stats.size, stats.mtimeMs, stats.ctimeMs, stats.mode].join(':') +} + +/** Publish only a successful version probe; a newer probe supersedes an older one. */ +export function beginLocalCommandSelection( + command: string +): (binary: string | null) => Promise { + if (command !== 'gh' && command !== 'glab') { + return async () => {} + } + const scope = selectionScope({}) + const probeCwd = process.cwd() + const previous = localCommandSelections.get(command) + const selection: LocalCommandSelection = { + scope, + selected: previous?.scope === scope ? previous.selected : undefined + } + localCommandSelections.set(command, selection) + return async (binary) => { + if (localCommandSelections.get(command) !== selection) { + return + } + if (binary === null || !path.isAbsolute(binary)) { + delete selection.selected + return + } + try { + const stats = await stat(binary) + if (localCommandSelections.get(command) === selection) { + const cwd = + process.platform === 'win32' && + path.win32.resolve(path.win32.dirname(binary)).toLowerCase() === + path.win32.resolve(probeCwd).toLowerCase() + ? probeCwd + : undefined + // Only a current-directory CLI needs to stay tied to the probe's folder. + selection.selected = stats.isFile() + ? { binary, stamp: commandFileStamp(stats), cwd } + : undefined + } + } catch { + // A binary removed during the probe must not become the runtime selection. + if (localCommandSelections.get(command) === selection) { + delete selection.selected + } + } + } +} + +/** Native execution reuses preflight's selection without probing or replaying the operation. */ +export function resolveSelectedLocalCommand( + command: string, + options: ResolveCommandOptions = {} +): string { + const selection = localCommandSelections.get(command) + if (!selection?.selected || selection.scope !== selectionScope(options)) { + return command + } + if ( + selection.selected.cwd && + path.win32.resolve(options.cwd ?? process.cwd()).toLowerCase() !== + path.win32.resolve(selection.selected.cwd).toLowerCase() + ) { + return command + } + try { + const stats = statSync(selection.selected.binary) + if (commandFileStamp(stats) === selection.selected.stamp) { + return selection.selected.binary + } + } catch { + // Missing or replaced binaries require a fresh version probe. + } + delete selection.selected + return command +} + async function isExecutableFile(candidate: string, isWin: boolean): Promise { try { // Why: stat (not lstat) so symlinked CLIs resolve to their real target. @@ -63,12 +170,15 @@ async function isExecutableFile(candidate: string, isWin: boolean): Promise { - return (await resolveCommandOnLocalPath(command, options)) !== null + return (await findLocalCommandPaths(command, options, true)).length > 0 } /** The absolute path `isCommandOnLocalPath` found, or null. */ @@ -76,8 +186,24 @@ export async function resolveCommandOnLocalPath( command: string, options: ResolveCommandOptions = {} ): Promise { + return (await findLocalCommandPaths(command, options, true))[0] ?? null +} + +/** Ordered, deduplicated candidates, including executable shims that may fail to run. */ +export async function listLocalCommandPaths( + command: string, + options: ResolveCommandOptions = {} +): Promise { + return findLocalCommandPaths(command, options, false) +} + +async function findLocalCommandPaths( + command: string, + options: ResolveCommandOptions, + stopAtFirst: boolean +): Promise { if (!command) { - return null + return [] } const platform = options.platform ?? process.platform const env = options.env ?? process.env @@ -95,6 +221,8 @@ export async function resolveCommandOnLocalPath( const searchDirs = hasPathSeparator ? [''] : isWin ? [cwd, ...pathDirs] : pathDirs const extensions = isWin ? getWindowsExtensions(env, command) : [''] + const found: string[] = [] + const seen = new Set() for (const dir of searchDirs) { for (const ext of extensions) { // Why: forward-slash joins so candidates are statable on every platform @@ -102,13 +230,18 @@ export async function resolveCommandOnLocalPath( const candidate = path.posix.join(dir, command) + ext // Why: preserve the prior `.some(line => path.isAbsolute(line))` filter // over where/which stdout — only absolute resolutions count. - if (!isAbsolute(candidate)) { + const candidateKey = isWin ? candidate.toLowerCase() : candidate + if (!isAbsolute(candidate) || seen.has(candidateKey)) { continue } + seen.add(candidateKey) if (await isExecutableFile(candidate, isWin)) { - return candidate + found.push(candidate) + if (stopAtFirst || found.length >= (options.maxResults ?? Infinity)) { + return found + } } } } - return null + return found } diff --git a/src/main/ipc/preflight-agent-detection.test.ts b/src/main/ipc/preflight-agent-detection.test.ts index 37a061e0541..ec13423fcb6 100644 --- a/src/main/ipc/preflight-agent-detection.test.ts +++ b/src/main/ipc/preflight-agent-detection.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as LocalCommandResolver from './command-path-resolver' const { handleMock, @@ -12,6 +13,7 @@ const { getGiteaAuthStatusMock, resolveCliCommandsMock, isCommandOnLocalPathMock, + listLocalCommandPathsMock, mergePersistedWindowsPathAsyncMock, mergePersistedWindowsPathMock } = vi.hoisted(() => ({ @@ -26,6 +28,7 @@ const { getGiteaAuthStatusMock: vi.fn(), resolveCliCommandsMock: vi.fn(), isCommandOnLocalPathMock: vi.fn(), + listLocalCommandPathsMock: vi.fn(), mergePersistedWindowsPathAsyncMock: vi.fn(), mergePersistedWindowsPathMock: vi.fn() })) @@ -63,8 +66,10 @@ vi.mock('../../shared/node-cli-command-resolution', () => ({ // Why (#9297): local PATH resolution is now fs-based (no where/which spawn). // These tests express "which commands are on PATH" via the where/which mock, // so route the resolver through that same mock to preserve their intent. -vi.mock('./command-path-resolver', () => ({ - isCommandOnLocalPath: isCommandOnLocalPathMock +vi.mock('./command-path-resolver', async (importOriginal) => ({ + ...(await importOriginal()), + isCommandOnLocalPath: isCommandOnLocalPathMock, + listLocalCommandPaths: listLocalCommandPathsMock })) vi.mock('../pty/windows-environment-path', () => ({ @@ -113,6 +118,7 @@ describe('preflight', () => { getGiteaAuthStatusMock, resolveCliCommandsMock, isCommandOnLocalPathMock, + listLocalCommandPathsMock, mergePersistedWindowsPathAsyncMock, mergePersistedWindowsPathMock }, diff --git a/src/main/ipc/preflight-agent-refresh.test.ts b/src/main/ipc/preflight-agent-refresh.test.ts index cc64e2beeca..d4159c3f7b2 100644 --- a/src/main/ipc/preflight-agent-refresh.test.ts +++ b/src/main/ipc/preflight-agent-refresh.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as LocalCommandResolver from './command-path-resolver' const { handleMock, @@ -12,6 +13,7 @@ const { getGiteaAuthStatusMock, resolveCliCommandsMock, isCommandOnLocalPathMock, + listLocalCommandPathsMock, mergePersistedWindowsPathAsyncMock, mergePersistedWindowsPathMock } = vi.hoisted(() => ({ @@ -26,6 +28,7 @@ const { getGiteaAuthStatusMock: vi.fn(), resolveCliCommandsMock: vi.fn(), isCommandOnLocalPathMock: vi.fn(), + listLocalCommandPathsMock: vi.fn(), mergePersistedWindowsPathAsyncMock: vi.fn(), mergePersistedWindowsPathMock: vi.fn() })) @@ -63,8 +66,10 @@ vi.mock('../../shared/node-cli-command-resolution', () => ({ // Why (#9297): local PATH resolution is now fs-based (no where/which spawn). // These tests express "which commands are on PATH" via the where/which mock, // so route the resolver through that same mock to preserve their intent. -vi.mock('./command-path-resolver', () => ({ - isCommandOnLocalPath: isCommandOnLocalPathMock +vi.mock('./command-path-resolver', async (importOriginal) => ({ + ...(await importOriginal()), + isCommandOnLocalPath: isCommandOnLocalPathMock, + listLocalCommandPaths: listLocalCommandPathsMock })) vi.mock('../pty/windows-environment-path', () => ({ @@ -109,6 +114,7 @@ describe('preflight', () => { getGiteaAuthStatusMock, resolveCliCommandsMock, isCommandOnLocalPathMock, + listLocalCommandPathsMock, mergePersistedWindowsPathAsyncMock, mergePersistedWindowsPathMock }, diff --git a/src/main/ipc/preflight-command-exec.test.ts b/src/main/ipc/preflight-command-exec.test.ts index 63c2b35aceb..5e571b18613 100644 --- a/src/main/ipc/preflight-command-exec.test.ts +++ b/src/main/ipc/preflight-command-exec.test.ts @@ -1,15 +1,44 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import path from 'node:path' +import type * as LocalCommandResolver from './command-path-resolver' import { buildPosixCommandPathLookupScript } from '../../shared/posix-command-path-lookup' -const { runPreflightCommandInWslMock } = vi.hoisted(() => ({ - runPreflightCommandInWslMock: vi.fn() +const { + runPreflightCommandInWslMock, + execFileAsyncMock, + listLocalCommandPathsMock, + runProcessMock +} = vi.hoisted(() => ({ + runPreflightCommandInWslMock: vi.fn(), + execFileAsyncMock: vi.fn(), + listLocalCommandPathsMock: vi.fn(), + runProcessMock: vi.fn() })) +vi.mock('../../shared/child-process/run-process', () => ({ runProcess: runProcessMock })) + +vi.mock('./preflight-local-env', () => ({ buildLocalPreflightEnv: () => undefined })) + vi.mock('./preflight-wsl-command', () => ({ runPreflightCommandInWsl: runPreflightCommandInWslMock })) -import { isCommandOnPath } from './preflight-command-exec' +vi.mock('./command-path-resolver', async (importOriginal) => ({ + ...(await importOriginal()), + isCommandOnLocalPath: vi.fn(async () => false), + listLocalCommandPaths: listLocalCommandPathsMock +})) + +// Why: `findRunnableLocalCommand` decides what to spawn next from the error +// `execFile` puts on a failed probe, so the shapes below have to be its own. +vi.mock('child_process', () => { + const execFileWithPromisify = Object.assign(vi.fn(), { + [Symbol.for('nodejs.util.promisify.custom')]: execFileAsyncMock + }) + return { execFile: execFileWithPromisify, spawn: vi.fn() } +}) + +import { findRunnableLocalCommand, isCommandOnPath } from './preflight-command-exec' describe('isCommandOnPath', () => { const sentinel = '__ORCA_PREFLIGHT_COMMAND_PATH__' @@ -59,3 +88,222 @@ describe('isCommandOnPath', () => { await expect(isCommandOnPath('codex', { distro: 'Ubuntu' })).resolves.toBe(expected) }) }) + +describe('findRunnableLocalCommand', () => { + const shim = '/Users/tester/.asdf/shims/gh' + const second = '/Users/tester/.volta/bin/gh' + const third = '/usr/local/bin/gh' + + const spawnedCommands = () => execFileAsyncMock.mock.calls.map(([command]) => command) + + let pathBefore = '' + + beforeEach(() => { + pathBefore = process.env.PATH ?? '' + execFileAsyncMock.mockReset() + runProcessMock.mockReset() + listLocalCommandPathsMock.mockReset() + listLocalCommandPathsMock.mockResolvedValue([shim, second, third]) + }) + + afterEach(() => { + vi.restoreAllMocks() + process.env.PATH = pathBefore + }) + + it('returns the first copy that runs and leaves the rest alone', async () => { + execFileAsyncMock.mockImplementation(async (command: string) => { + if (command === shim) { + throw Object.assign(new Error('cannot execute'), { code: 126 }) + } + return { stdout: 'gh version 2.98.0\n', stderr: '' } + }) + + await expect(findRunnableLocalCommand('gh')).resolves.toEqual({ + status: 'available', + binary: second + }) + expect(spawnedCommands()).toEqual([shim, second]) + }) + + it('keeps looking when a copy exits non-zero', async () => { + execFileAsyncMock.mockImplementation(async (command: string) => { + if (command === third) { + return { stdout: 'gh version 2.98.0\n', stderr: '' } + } + throw Object.assign(new Error('Command failed'), { code: 1 }) + }) + + await expect(findRunnableLocalCommand('gh')).resolves.toEqual({ + status: 'available', + binary: third + }) + }) + + it('stops at a timed-out copy instead of paying the timeout once per copy', async () => { + execFileAsyncMock.mockImplementation(async (command: string) => { + if (command === second) { + throw Object.assign(new Error('Timed out'), { killed: true, code: null }) + } + throw Object.assign(new Error('cannot execute'), { code: 126 }) + }) + + await expect(findRunnableLocalCommand('gh')).resolves.toEqual({ + status: 'timeout', + binary: second + }) + expect(spawnedCommands()).toEqual([shim, second]) + }) + + it('stops on the ETIMEDOUT shape as well as the killed shape', async () => { + execFileAsyncMock.mockImplementation((command: string) => + command === second + ? Promise.reject(Object.assign(new Error('Timed out'), { code: 'ETIMEDOUT' })) + : Promise.reject(Object.assign(new Error('cannot execute'), { code: 126 })) + ) + + await expect(findRunnableLocalCommand('gh')).resolves.toEqual({ + status: 'timeout', + binary: second + }) + expect(spawnedCommands()).toEqual([shim, second]) + }) + + it('reads a rejection that is not an object as an ordinary failure', async () => { + execFileAsyncMock.mockImplementation((command: string) => + command === third + ? Promise.resolve({ stdout: 'gh version 2.98.0\n', stderr: '' }) + : Promise.reject('not an error object') + ) + + await expect(findRunnableLocalCommand('gh')).resolves.toEqual({ + status: 'available', + binary: third + }) + expect(spawnedCommands()).toEqual([shim, second, third]) + }) + + it('probes the bare command name when fs finds no candidate', async () => { + listLocalCommandPathsMock.mockResolvedValue([]) + execFileAsyncMock.mockResolvedValue({ stdout: 'gh version 2.98.0\n', stderr: '' }) + + await expect(findRunnableLocalCommand('gh')).resolves.toEqual({ + status: 'available', + binary: 'gh' + }) + expect(spawnedCommands()).toEqual(['gh']) + }) + + it('probes a relative PATH entry as the absolute directory cwd gives it', async () => { + const relativeDir = path.join('.', 'tools') + process.env.PATH = [shim.replace('/gh', ''), relativeDir].join(path.delimiter) + const absoluteDir = path.resolve(relativeDir) + const hidden = `${absoluteDir}/gh` + const absolutePath = [shim.replace('/gh', ''), absoluteDir].join(path.delimiter) + listLocalCommandPathsMock.mockImplementation( + async (_command: string, options?: { env?: NodeJS.ProcessEnv }) => + options?.env?.PATH === absolutePath ? [shim, hidden] : [] + ) + execFileAsyncMock.mockImplementation(async (command: string) => { + if (command === hidden) { + return { stdout: 'gh version 2.98.0\n', stderr: '' } + } + throw Object.assign(new Error('cannot execute'), { code: 126 }) + }) + + await expect(findRunnableLocalCommand('gh')).resolves.toEqual({ + status: 'available', + binary: hidden + }) + expect(spawnedCommands()).toEqual([shim, hidden]) + }) + + it('does not pay for the bare name when every PATH entry is absolute', async () => { + process.env.PATH = ['/Users/tester/.asdf/shims', '/usr/local/bin'].join(path.delimiter) + execFileAsyncMock.mockRejectedValue(Object.assign(new Error('cannot execute'), { code: 126 })) + + await expect(findRunnableLocalCommand('gh')).resolves.toEqual({ + status: 'exec_failed', + binary: third + }) + expect(spawnedCommands()).not.toContain('gh') + }) + + it('runs an explicitly selected Windows cmd shim through the shared runner', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + const command = path.win32.join('C:\\tools', 'gh.cmd') + runProcessMock.mockResolvedValue({ + code: 0, + stdout: 'gh version fixture', + stderr: '', + timedOut: false + }) + + await expect(findRunnableLocalCommand(command)).resolves.toEqual({ + status: 'available', + binary: command + }) + expect(runProcessMock).toHaveBeenCalledWith({ + program: command, + args: ['--version'], + env: undefined, + timeoutMs: expect.any(Number) + }) + expect(execFileAsyncMock).not.toHaveBeenCalled() + expect(listLocalCommandPathsMock).not.toHaveBeenCalled() + }) + + it('keeps searching Windows PATH after a broken cmd shim', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + const command = path.win32.join('C:\\tools', 'gh.cmd') + const working = path.win32.join('C:\\working', 'gh.exe') + listLocalCommandPathsMock.mockResolvedValue([command, working]) + runProcessMock.mockResolvedValue({ + code: 126, + stdout: '', + stderr: 'broken shim', + timedOut: false + }) + execFileAsyncMock.mockResolvedValue({ stdout: 'gh version fixture', stderr: '' }) + + await expect(findRunnableLocalCommand('gh')).resolves.toEqual({ + status: 'available', + binary: working + }) + expect(runProcessMock).toHaveBeenCalledOnce() + expect(spawnedCommands()).toEqual([working]) + }) + + it('stops on a shared runner timeout', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + const command = path.win32.join('C:\\tools', 'gh.cmd') + listLocalCommandPathsMock.mockResolvedValue([command, third]) + runProcessMock.mockResolvedValue({ code: null, stdout: '', stderr: '', timedOut: true }) + + await expect(findRunnableLocalCommand('gh')).resolves.toEqual({ + status: 'timeout', + binary: command + }) + expect(execFileAsyncMock).not.toHaveBeenCalled() + }) + + it('shares the five second timeout across failed candidates', async () => { + let now = 1000 + vi.spyOn(Date, 'now').mockImplementation(() => now) + execFileAsyncMock.mockImplementation(async (command: string) => { + if (command === shim) { + now += 4000 + throw Object.assign(new Error('cannot execute'), { code: 126 }) + } + throw Object.assign(new Error('timeout'), { code: 'ETIMEDOUT' }) + }) + + await expect(findRunnableLocalCommand('gh')).resolves.toEqual({ + status: 'timeout', + binary: second + }) + expect(execFileAsyncMock.mock.calls.map(([, , options]) => options.timeout)).toEqual([ + 5000, 1000 + ]) + }) +}) diff --git a/src/main/ipc/preflight-command-exec.ts b/src/main/ipc/preflight-command-exec.ts index 9c67045d512..41bc378fdfe 100644 --- a/src/main/ipc/preflight-command-exec.ts +++ b/src/main/ipc/preflight-command-exec.ts @@ -1,8 +1,15 @@ import { execFile } from 'node:child_process' import path from 'node:path' +import { homedir } from 'node:os' import { promisify } from 'node:util' import { buildPosixCommandPathLookupScript } from '../../shared/posix-command-path-lookup' -import { isCommandOnLocalPath } from './command-path-resolver' +import { getSystemCliInstallDirectories } from '../../shared/system-cli-install-dirs' +import { runProcess } from '../../shared/child-process/run-process' +import { + beginLocalCommandSelection, + isCommandOnLocalPath, + listLocalCommandPaths +} from './command-path-resolver' import { buildLocalPreflightEnv } from './preflight-local-env' import { runPreflightCommandInWsl } from './preflight-wsl-command' import type { WslPreflightTarget } from './preflight-wsl-agent-detection' @@ -17,7 +24,11 @@ export function shellQuote(value: string): string { return `'${value.replace(/'/g, "'\\''")}'` } -async function withPreflightTimeout(command: string, commandPromise: Promise): Promise { +async function withPreflightTimeout( + command: string, + commandPromise: Promise, + timeoutMs = PREFLIGHT_COMMAND_TIMEOUT_MS +): Promise { let timeout: ReturnType | null = null try { return await Promise.race([ @@ -28,7 +39,7 @@ async function withPreflightTimeout(command: string, commandPromise: Promise< code: 'ETIMEDOUT' }) reject(error) - }, PREFLIGHT_COMMAND_TIMEOUT_MS) + }, timeoutMs) if (typeof timeout.unref === 'function') { timeout.unref() } @@ -47,19 +58,36 @@ async function withPreflightTimeout(command: string, commandPromise: Promise< * docs/reference/wsl-probe-failure-semantics.md before doing so. */ export async function execLocalPreflightCommandOrThrow( command: string, - args: string[] + args: string[], + options: { env?: NodeJS.ProcessEnv; timeoutMs?: number } = {} ): Promise { - const env = buildLocalPreflightEnv() + const env = options.env ?? buildLocalPreflightEnv() + const timeoutMs = options.timeoutMs ?? PREFLIGHT_COMMAND_TIMEOUT_MS + // Node cannot execFile a batch shim; the shared runner handles its argv safely. + if (process.platform === 'win32' && /\.(cmd|bat)$/i.test(command)) { + const result = await withPreflightTimeout( + command, + runProcess({ program: command, args, env, timeoutMs }), + timeoutMs + ) + if (result.timedOut || result.code !== 0) { + throw Object.assign(new Error(`Failed running ${command}`), { + ...result, + code: result.timedOut ? 'ETIMEDOUT' : result.code + }) + } + return { stdout: result.stdout, stderr: result.stderr } + } const commandPromise = execFileAsync(command, args, { encoding: 'utf-8', - timeout: PREFLIGHT_COMMAND_TIMEOUT_MS, + timeout: timeoutMs, // Preflight probes console-subsystem binaries (git, gh, node); without this // each one flashes a console and steals foreground on Windows (#10488). windowsHide: true, ...(env ? { env } : {}) - }) as Promise + }) - return withPreflightTimeout(command, commandPromise) + return withPreflightTimeout(command, commandPromise, timeoutMs) } // Throws on any failure — a distro that is booting/unreachable throws the @@ -76,14 +104,106 @@ export async function execCommandInWslOrThrow( return withPreflightTimeout('wsl command', commandPromise) } +const PREFLIGHT_LOCAL_PROBE_LIMIT = 4 + +export type LocalCommandProbe = + | { status: 'available'; binary: string } + | { status: 'absent' } + | { status: 'exec_failed' | 'timeout' | 'limit_reached'; binary: string } + +function probeTimedOut(error: unknown): boolean { + if (typeof error !== 'object' || error === null) { + return false + } + return ( + ('killed' in error && error.killed === true) || ('code' in error && error.code === 'ETIMEDOUT') + ) +} + +async function localProbeCandidates( + command: string, + env: NodeJS.ProcessEnv | undefined +): Promise { + const isWin = process.platform === 'win32' + const probeEnv = env ?? process.env + // Keep relative PATH entries in their original position, as execFile does. + const absoluteEnv = isWin + ? probeEnv + : { + ...probeEnv, + PATH: (probeEnv.PATH ?? '') + .split(path.delimiter) + .map((dir) => path.resolve(dir)) + .join(path.delimiter) + } + const maxResults = PREFLIGHT_LOCAL_PROBE_LIMIT + 1 + const paths = await listLocalCommandPaths(command, { env: absoluteEnv, maxResults }) + const installPaths = + isWin || paths.length >= maxResults + ? [] + : await listLocalCommandPaths(command, { + env: { + PATH: getSystemCliInstallDirectories(process.platform, homedir()).join(path.delimiter) + }, + maxResults + }) + return [...new Set([...paths, ...installPaths])] +} + +/** Try only version probes; authentication must stay on the selected binary. */ +export async function findRunnableLocalCommand(command: string): Promise { + const publishSelection = beginLocalCommandSelection(command) + const result = await probeRunnableLocalCommand(command) + await publishSelection(result.status === 'available' ? result.binary : null) + return result +} + +async function probeRunnableLocalCommand(command: string): Promise { + const env = buildLocalPreflightEnv() + const explicit = command.includes('/') || (process.platform === 'win32' && command.includes('\\')) + // An explicit path is the user's selection, even when it cannot run. + const candidates = explicit ? [] : await localProbeCandidates(command, env) + const probes = candidates.length ? candidates.slice(0, PREFLIGHT_LOCAL_PROBE_LIMIT) : [command] + const deadline = Date.now() + PREFLIGHT_COMMAND_TIMEOUT_MS + for (const binary of probes) { + const timeoutMs = deadline - Date.now() + if (timeoutMs <= 0) { + return { status: 'timeout', binary } + } + try { + await execLocalPreflightCommandOrThrow(binary, ['--version'], { env, timeoutMs }) + return { status: 'available', binary } + } catch (error) { + if (probeTimedOut(error)) { + return { status: 'timeout', binary } + } + if ( + candidates.length === 0 && + typeof error === 'object' && + error !== null && + 'code' in error && + error.code === 'ENOENT' && + !(await isCommandOnLocalPath(explicit ? path.resolve(command) : command, { env })) + ) { + return { status: 'absent' } + } + } + } + return { + status: candidates.length > PREFLIGHT_LOCAL_PROBE_LIMIT ? 'limit_reached' : 'exec_failed', + binary: probes.at(-1) ?? command + } +} + export async function isCommandAvailable( command: string, wslTarget?: WslPreflightTarget ): Promise { + if (!wslTarget) { + return (await findRunnableLocalCommand(command)).status === 'available' + } try { - await (wslTarget - ? execCommandInWslOrThrow(wslTarget, `${shellQuote(command)} --version`) - : execLocalPreflightCommandOrThrow(command, ['--version'])) + await execCommandInWslOrThrow(wslTarget, `${shellQuote(command)} --version`) return true } catch { return false diff --git a/src/main/ipc/preflight-host-cli-status.test.ts b/src/main/ipc/preflight-host-cli-status.test.ts index cc005da88e6..b68c3e8afb5 100644 --- a/src/main/ipc/preflight-host-cli-status.test.ts +++ b/src/main/ipc/preflight-host-cli-status.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as LocalCommandResolver from './command-path-resolver' const { handleMock, @@ -13,6 +14,7 @@ const { getGiteaAuthStatusMock, resolveCliCommandsMock, isCommandOnLocalPathMock, + listLocalCommandPathsMock, mergePersistedWindowsPathAsyncMock, mergePersistedWindowsPathMock } = vi.hoisted(() => ({ @@ -28,6 +30,7 @@ const { getGiteaAuthStatusMock: vi.fn(), resolveCliCommandsMock: vi.fn(), isCommandOnLocalPathMock: vi.fn(), + listLocalCommandPathsMock: vi.fn(), mergePersistedWindowsPathAsyncMock: vi.fn(), mergePersistedWindowsPathMock: vi.fn() })) @@ -63,8 +66,10 @@ vi.mock('../../shared/node-cli-command-resolution', () => ({ // Why (#9297): local PATH resolution is now fs-based (no where/which spawn). // These tests express "which commands are on PATH" via the where/which mock, // so route the resolver through that same mock to preserve their intent. -vi.mock('./command-path-resolver', () => ({ - isCommandOnLocalPath: isCommandOnLocalPathMock +vi.mock('./command-path-resolver', async (importOriginal) => ({ + ...(await importOriginal()), + isCommandOnLocalPath: isCommandOnLocalPathMock, + listLocalCommandPaths: listLocalCommandPathsMock })) vi.mock('../pty/windows-environment-path', () => ({ @@ -115,6 +120,7 @@ describe('preflight', () => { getGiteaAuthStatusMock, resolveCliCommandsMock, isCommandOnLocalPathMock, + listLocalCommandPathsMock, mergePersistedWindowsPathAsyncMock, mergePersistedWindowsPathMock }, @@ -162,17 +168,29 @@ describe('preflight', () => { }) }) - it('treats gh as unauthenticated when gh auth status fails without auth markers', async () => { - execFileAsyncMock - .mockResolvedValueOnce({ stdout: 'git version 2.0.0\n' }) - .mockResolvedValueOnce({ stdout: 'gh version 2.0.0\n' }) - .mockResolvedValueOnce({ stdout: 'glab version 1.92.1\n' }) - .mockRejectedValueOnce({ stderr: 'You are not logged into any GitHub hosts.\n' }) - .mockResolvedValueOnce({ stdout: 'Logged in to gitlab.com\n' }) + it.each(['gh', 'glab'])('does not switch %s copies after authentication fails', async (cli) => { + const first = `/test/first/${cli}` + const second = `/test/second/${cli}` + listLocalCommandPathsMock.mockImplementation(async (command: string) => + command === cli ? [first, second] : [] + ) + execFileAsyncMock.mockImplementation(async (command: string, args: string[]) => { + if (command === first && args[0] === 'auth') { + throw Object.assign(new Error('not authenticated'), { code: 1, stderr: 'not logged in' }) + } + return { stdout: 'fixture success', stderr: '' } + }) const status = await runPreflightCheck() - expect(status.gh).toEqual({ installed: true, authenticated: false }) + expect(cli === 'gh' ? status.gh : status.glab).toEqual({ + installed: true, + authenticated: false + }) + expect( + execFileAsyncMock.mock.calls.filter(([command]) => command === first).map(([, args]) => args) + ).toEqual([['--version'], ['auth', 'status']]) + expect(execFileAsyncMock.mock.calls.some(([command]) => command === second)).toBe(false) }) it('keeps older gh stderr success output from showing a false auth warning', async () => { @@ -188,6 +206,88 @@ describe('preflight', () => { expect(status.gh).toEqual({ installed: true, authenticated: true }) }) + // Why (#22975): these two cases key the spawn mock on (command, args) instead + // of call order, because the fallback probes exactly which copy runs is the + // behaviour under test — and `gh auth status` runs in parallel with + // `glab auth status`, so a fixed sequence would not be the real one. + it('authenticates the gh copy that actually ran, not the shim PATH chose', async () => { + const shim = '/Users/octocat/.asdf/shims/gh' + const working = '/opt/homebrew/bin/gh' + const glabShim = '/Users/octocat/.asdf/shims/glab' + const glabWorking = '/usr/local/bin/glab' + const shims = new Set([shim, glabShim]) + listLocalCommandPathsMock.mockImplementation(async (command: string) => { + if (command === 'gh') { + return [shim, working] + } + return command === 'glab' ? [glabShim, glabWorking] : [] + }) + execFileAsyncMock.mockImplementation(async (command: string, args: string[]) => { + if (shims.has(command)) { + throw Object.assign(new Error('spawn failed'), { code: 126 }) + } + const auth = args[0] === 'auth' + if (command === working) { + return { stdout: auth ? 'github.com\n - Active account: true\n' : 'gh version 2.6.0\n' } + } + if (command === glabWorking) { + return { stdout: auth ? 'Logged in to gitlab.com\n' : 'glab version 1.92.1\n' } + } + if (command === 'git') { + return { stdout: 'git version 2.0.0\n' } + } + throw new Error(`unexpected command ${command}`) + }) + + const status = await runPreflightCheck() + + expect(status).toMatchObject({ + gh: { installed: true, authenticated: true }, + glab: { installed: true, authenticated: true } + }) + for (const [cli, copy] of [ + ['gh', working], + ['glab', glabWorking] + ]) { + expect(execFileAsyncMock).toHaveBeenCalledWith(copy, ['auth', 'status'], { + encoding: 'utf-8', + timeout: 5000, + windowsHide: true + }) + expect(execFileAsyncMock).not.toHaveBeenCalledWith(cli, ['auth', 'status'], { + encoding: 'utf-8', + timeout: 5000, + windowsHide: true + }) + } + }) + + it('marks gh not installed when no copy PATH offers can run', async () => { + const doomed = ['/Users/octocat/.asdf/shims/gh', '/Users/octocat/.local/bin/gh'] + listLocalCommandPathsMock.mockImplementation(async (command: string) => + command === 'gh' ? doomed : [] + ) + execFileAsyncMock.mockImplementation(async (command: string, args: string[]) => { + if (doomed.includes(command)) { + throw Object.assign(new Error('spawn failed'), { code: 126 }) + } + if (command === 'git') { + return { stdout: 'git version 2.0.0\n' } + } + if (command === 'glab') { + return { + stdout: args[0] === 'auth' ? 'Logged in to gitlab.com\n' : 'glab version 1.92.1\n' + } + } + throw new Error(`unexpected command ${command}`) + }) + + const status = await runPreflightCheck() + + expect(status.gh).toEqual({ installed: false, authenticated: false }) + expect(execFileAsyncMock).toHaveBeenCalledTimes(5) + }) + it('marks glab as not installed when `glab --version` fails', async () => { execFileAsyncMock .mockResolvedValueOnce({ stdout: 'git version 2.0.0\n' }) diff --git a/src/main/ipc/preflight-provider-command-selection.test.ts b/src/main/ipc/preflight-provider-command-selection.test.ts new file mode 100644 index 00000000000..a51ac4a886c --- /dev/null +++ b/src/main/ipc/preflight-provider-command-selection.test.ts @@ -0,0 +1,321 @@ +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { removeTree } from '../../shared/windows-transient-lock-removal' +import { ghExecFileAsync } from '../git/command-runner/gh-exec-file' +import { glabExecFileAsync } from '../git/command-runner/glab-exec-file' +import { execFileCaptureToTermination } from '../git/command-runner/exec-file-capture' +import { beginLocalCommandSelection, resolveSelectedLocalCommand } from './command-path-resolver' +import { + execLocalPreflightCommandOrThrow, + findRunnableLocalCommand, + shellQuote +} from './preflight-command-exec' + +vi.mock('../../shared/system-cli-install-dirs', () => ({ + getSystemCliInstallDirectories: (_platform: NodeJS.Platform, home: string) => [ + path.join(home, '.nix-profile', 'bin') + ] +})) + +let root = '' +const CLIS = ['gh', 'glab'] as const + +async function fixtureCli( + cli: string, + label: string, + body = `case "$1" in\n--version) echo fixture-version;;\nauth) echo 'Logged in fixture';;\napi) echo '${label}-api';;\nesac\n` +): Promise { + const dir = path.join(root, label) + await mkdir(dir, { recursive: true }) + const binary = path.join(dir, cli) + await writeFile( + binary, + `#!/bin/sh\nprintf '%s\\n' "$*" >> ${shellQuote(`${binary}-calls`)}\n${body}`, + { mode: 0o755 } + ) + return binary +} + +async function calls(binary: string): Promise { + try { + return (await readFile(`${binary}-calls`, 'utf8')).trim().split('\n') + } catch (error) { + if (typeof error === 'object' && error !== null && 'code' in error && error.code === 'ENOENT') { + return [] + } + throw error + } +} + +function providerCommand(cli: 'gh' | 'glab', env?: NodeJS.ProcessEnv, cwd?: string) { + const execute = cli === 'gh' ? ghExecFileAsync : glabExecFileAsync + return execute(['api', 'user'], { timeout: 1000, idempotent: false, env, cwd }) +} + +describe.skipIf(process.platform === 'win32')( + 'preflight selection in native provider commands', + () => { + beforeEach(async () => { + root = await mkdtemp(path.join(tmpdir(), 'orca-22975-provider-')) + vi.stubEnv('HOME', root) + }) + + afterEach(async () => { + vi.unstubAllEnvs() + await removeTree(root) + }) + + it.each(CLIS)( + 'uses the same runnable %s for version, auth and provider operations', + async (cli) => { + const shim = await fixtureCli(cli, 'shim', 'echo broken-shim >&2\nexit 126\n') + const good = await fixtureCli(cli, 'good') + vi.stubEnv('PATH', [path.dirname(shim), path.dirname(good)].join(path.delimiter)) + + const selected = await findRunnableLocalCommand(cli) + expect(selected).toEqual({ status: 'available', binary: good }) + if (selected.status !== 'available') { + throw new Error('Expected the working fixture to be selected') + } + await expect( + execLocalPreflightCommandOrThrow(selected.binary, ['auth', 'status']) + ).resolves.toMatchObject({ stdout: 'Logged in fixture\n' }) + await expect(providerCommand(cli, undefined, root)).resolves.toMatchObject({ + stdout: 'good-api\n' + }) + expect(await calls(shim)).toEqual(['--version']) + expect(await calls(good)).toEqual(['--version', 'auth status', 'api user']) + } + ) + + it.each(CLIS)( + 'keeps the proven %s usable while its version selection refreshes', + async (cli) => { + const shim = await fixtureCli(cli, 'shim', 'echo broken-shim >&2\nexit 126\n') + const good = await fixtureCli(cli, 'good') + vi.stubEnv('PATH', [path.dirname(shim), path.dirname(good)].join(path.delimiter)) + await findRunnableLocalCommand(cli) + + const refreshing = findRunnableLocalCommand(cli) + await expect(providerCommand(cli)).resolves.toMatchObject({ stdout: 'good-api\n' }) + await expect(refreshing).resolves.toEqual({ status: 'available', binary: good }) + expect(await calls(shim)).toEqual(['--version', '--version']) + } + ) + + it('clears the previous selection only after the newest version probe fails', async () => { + const shim = await fixtureCli('gh', 'shim', 'exit 126\n') + const good = await fixtureCli( + 'gh', + 'good', + 'if [ "$1" = --version ] && [ "$ORCA_22975_VERSION_DISABLED" = 1 ]; then exit 126; fi\necho good-api\n' + ) + vi.stubEnv('PATH', [path.dirname(shim), path.dirname(good)].join(path.delimiter)) + await findRunnableLocalCommand('gh') + vi.stubEnv('ORCA_22975_VERSION_DISABLED', '1') + + const refreshing = findRunnableLocalCommand('gh') + await expect(providerCommand('gh')).resolves.toMatchObject({ stdout: 'good-api\n' }) + await expect(refreshing).resolves.toMatchObject({ status: 'exec_failed' }) + expect(resolveSelectedLocalCommand('gh')).toBe('gh') + }) + + it.each(CLIS)('also uses the recovered %s from a known install directory', async (cli) => { + const shim = await fixtureCli(cli, 'shim', 'exit 126\n') + const good = await fixtureCli(cli, path.join('.nix-profile', 'bin')) + vi.stubEnv('PATH', path.dirname(shim)) + + await expect(findRunnableLocalCommand(cli)).resolves.toEqual({ + status: 'available', + binary: good + }) + await expect(providerCommand(cli)).resolves.toMatchObject({ + stdout: `${path.join('.nix-profile', 'bin')}-api\n` + }) + expect(await calls(shim)).toEqual(['--version']) + }) + + it.each(CLIS)('keeps %s selected when authentication fails', async (cli) => { + const first = await fixtureCli( + cli, + 'first', + 'if [ "$1" = auth ]; then echo unauthenticated >&2; exit 1; fi\necho first-result\n' + ) + const other = await fixtureCli(cli, 'other') + vi.stubEnv('PATH', [path.dirname(first), path.dirname(other)].join(path.delimiter)) + + await findRunnableLocalCommand(cli) + await expect( + execLocalPreflightCommandOrThrow(first, ['auth', 'status']) + ).rejects.toMatchObject({ + code: 1 + }) + await expect(providerCommand(cli)).resolves.toMatchObject({ stdout: 'first-result\n' }) + expect(await calls(other)).toEqual([]) + }) + + it.each(CLIS)('never retries a failed %s operation on another binary', async (cli) => { + const first = await fixtureCli( + cli, + 'first', + 'if [ "$1" = api ]; then echo operation-failed >&2; exit 126; fi\necho fixture-version\n' + ) + const other = await fixtureCli(cli, 'other') + vi.stubEnv('PATH', [path.dirname(first), path.dirname(other)].join(path.delimiter)) + + await findRunnableLocalCommand(cli) + await expect(providerCommand(cli)).rejects.toMatchObject({ + code: 126, + stderr: 'operation-failed\n' + }) + expect(await calls(first)).toEqual(['--version', 'api user']) + expect(await calls(other)).toEqual([]) + }) + + it('respects a caller PATH and explicit binary instead of the global selection', async () => { + const shim = await fixtureCli('gh', 'shim', 'exit 126\n') + const good = await fixtureCli('gh', 'good') + const custom = await fixtureCli('gh', 'custom') + vi.stubEnv('PATH', [path.dirname(shim), path.dirname(good)].join(path.delimiter)) + await findRunnableLocalCommand('gh') + const refreshing = findRunnableLocalCommand('gh') + + await expect( + providerCommand('gh', { ...process.env, PATH: path.dirname(custom) }) + ).resolves.toMatchObject({ stdout: 'custom-api\n' }) + await expect( + execFileCaptureToTermination(custom, ['api', 'user'], { encoding: 'utf8', timeout: 1000 }) + ).resolves.toMatchObject({ stdout: 'custom-api\n' }) + await expect(providerCommand('gh')).resolves.toMatchObject({ stdout: 'good-api\n' }) + await expect(refreshing).resolves.toEqual({ status: 'available', binary: good }) + }) + + it('bypasses a selection after PATH or relative-PATH cwd changes', async () => { + const good = await fixtureCli('gh', 'good') + const custom = await fixtureCli('gh', 'custom') + vi.stubEnv('PATH', path.dirname(good)) + await findRunnableLocalCommand('gh') + const refreshing = findRunnableLocalCommand('gh') + vi.stubEnv('PATH', path.dirname(custom)) + await expect(providerCommand('gh')).resolves.toMatchObject({ stdout: 'custom-api\n' }) + await refreshing + await expect(providerCommand('gh')).resolves.toMatchObject({ stdout: 'custom-api\n' }) + + vi.stubEnv('PATH', path.relative(process.cwd(), path.dirname(good))) + await findRunnableLocalCommand('gh') + expect(resolveSelectedLocalCommand('gh', { cwd: root })).toBe('gh') + }) + + it('discards a replaced binary and clears a selection when the next probe fails', async () => { + const shim = await fixtureCli('gh', 'shim', 'exit 126\n') + const good = await fixtureCli('gh', 'good') + vi.stubEnv('PATH', [path.dirname(shim), path.dirname(good)].join(path.delimiter)) + await findRunnableLocalCommand('gh') + await writeFile(good, '#!/bin/sh\necho changed >&2\nexit 126\n') + + expect(resolveSelectedLocalCommand('gh')).toBe('gh') + await expect(findRunnableLocalCommand('gh')).resolves.toMatchObject({ status: 'exec_failed' }) + expect(resolveSelectedLocalCommand('gh')).toBe('gh') + await fixtureCli('gh', 'good') + await findRunnableLocalCommand('gh') + expect(resolveSelectedLocalCommand('gh')).toBe(good) + await rm(good) + expect(resolveSelectedLocalCommand('gh')).toBe('gh') + }) + + it('keeps an older concurrent probe from replacing the newer selection', async () => { + const first = await fixtureCli('gh', 'first') + const second = await fixtureCli('gh', 'second') + const older = beginLocalCommandSelection('gh') + const newer = beginLocalCommandSelection('gh') + await newer(second) + await older(first) + await older(null) + + expect(resolveSelectedLocalCommand('gh')).toBe(second) + }) + } +) + +describe('Windows selection across working folders', () => { + it('keeps an absolute PATH selection in a different provider cwd', async () => { + const directory = await mkdtemp(path.join(tmpdir(), 'orca-22975-cwd-')) + const binary = path.join(directory, 'gh.CMD') + const descriptor = Object.getOwnPropertyDescriptor(process, 'platform') + await writeFile(binary, '@echo off\r\nexit /b 0\r\n') + try { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + vi.stubEnv('PATH', directory) + const publish = beginLocalCommandSelection('gh') + await publish(binary) + expect(resolveSelectedLocalCommand('gh', { cwd: path.join(directory, 'project') })).toBe( + binary + ) + } finally { + vi.unstubAllEnvs() + if (descriptor) { + Object.defineProperty(process, 'platform', descriptor) + } + await removeTree(directory) + } + }) + + it('does not carry a current-directory CLI into another folder', async () => { + const directory = await mkdtemp(path.join(tmpdir(), 'orca-22975-local-cwd-')) + const binary = path.join(directory, 'gh.CMD') + const descriptor = Object.getOwnPropertyDescriptor(process, 'platform') + await writeFile(binary, '@echo off\r\nexit /b 0\r\n') + const cwd = vi.spyOn(process, 'cwd').mockReturnValue(directory) + try { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + vi.stubEnv('PATH', path.join(directory, 'other')) + const publish = beginLocalCommandSelection('gh') + await publish(binary) + expect(resolveSelectedLocalCommand('gh')).toBe(binary) + expect(resolveSelectedLocalCommand('gh', { cwd: path.join(directory, 'project') })).toBe('gh') + } finally { + cwd.mockRestore() + vi.unstubAllEnvs() + if (descriptor) { + Object.defineProperty(process, 'platform', descriptor) + } + await removeTree(directory) + } + }) +}) + +describe.runIf(process.platform === 'win32')('native provider batch selection', () => { + beforeEach(async () => { + root = await mkdtemp(path.join(tmpdir(), 'orca-22975-provider-cmd-')) + }) + + afterEach(async () => { + vi.unstubAllEnvs() + await removeTree(root) + }) + + it.each(CLIS)('passes the selected %s.cmd through the native runner', async (cli) => { + const shim = path.join(root, 'shim') + const good = path.join(root, 'good') + await mkdir(shim) + await mkdir(good) + await writeFile(path.join(shim, `${cli}.CMD`), '@echo off\r\nexit /b 126\r\n') + await writeFile( + path.join(good, `${cli}.CMD`), + '@echo off\r\nif "%~1"=="api" (echo good-api) else (echo fixture-version)\r\nexit /b 0\r\n' + ) + vi.stubEnv('PATH', [shim, good].join(path.delimiter)) + vi.stubEnv('Path', [shim, good].join(path.delimiter)) + vi.stubEnv('PATHEXT', '.CMD') + + await expect(findRunnableLocalCommand(cli)).resolves.toEqual({ + status: 'available', + binary: path.posix.join(good, `${cli}.CMD`) + }) + await expect(providerCommand(cli, undefined, root)).resolves.toMatchObject({ + stdout: expect.stringContaining('good-api') + }) + }) +}) diff --git a/src/main/ipc/preflight-remote-ssh.test.ts b/src/main/ipc/preflight-remote-ssh.test.ts index 14d79e953e8..4ad02ec5d04 100644 --- a/src/main/ipc/preflight-remote-ssh.test.ts +++ b/src/main/ipc/preflight-remote-ssh.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as LocalCommandResolver from './command-path-resolver' const { handleMock, @@ -12,6 +13,7 @@ const { getGiteaAuthStatusMock, resolveCliCommandsMock, isCommandOnLocalPathMock, + listLocalCommandPathsMock, mergePersistedWindowsPathAsyncMock, mergePersistedWindowsPathMock } = vi.hoisted(() => ({ @@ -26,6 +28,7 @@ const { getGiteaAuthStatusMock: vi.fn(), resolveCliCommandsMock: vi.fn(), isCommandOnLocalPathMock: vi.fn(), + listLocalCommandPathsMock: vi.fn(), mergePersistedWindowsPathAsyncMock: vi.fn(), mergePersistedWindowsPathMock: vi.fn() })) @@ -58,8 +61,10 @@ vi.mock('../../shared/node-cli-command-resolution', () => ({ // Why (#9297): local PATH resolution is now fs-based (no where/which spawn). // These tests express "which commands are on PATH" via the where/which mock, // so route the resolver through that same mock to preserve their intent. -vi.mock('./command-path-resolver', () => ({ - isCommandOnLocalPath: isCommandOnLocalPathMock +vi.mock('./command-path-resolver', async (importOriginal) => ({ + ...(await importOriginal()), + isCommandOnLocalPath: isCommandOnLocalPathMock, + listLocalCommandPaths: listLocalCommandPathsMock })) vi.mock('../pty/windows-environment-path', () => ({ @@ -106,6 +111,7 @@ describe('preflight', () => { getGiteaAuthStatusMock, resolveCliCommandsMock, isCommandOnLocalPathMock, + listLocalCommandPathsMock, mergePersistedWindowsPathAsyncMock, mergePersistedWindowsPathMock }, diff --git a/src/main/ipc/preflight-runnable-local-cli.test.ts b/src/main/ipc/preflight-runnable-local-cli.test.ts new file mode 100644 index 00000000000..2f41efd7b78 --- /dev/null +++ b/src/main/ipc/preflight-runnable-local-cli.test.ts @@ -0,0 +1,249 @@ +import { chmod, mkdir, mkdtemp, readFile, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { removeTree } from '../../shared/windows-transient-lock-removal' +import { + execLocalPreflightCommandOrThrow, + findRunnableLocalCommand, + isCommandAvailable, + isCommandOnPath, + shellQuote +} from './preflight-command-exec' + +const COMMAND = 'orca-22975-gh' +const BROKEN_SHIM = 'echo broken-shim >&2\nexit 126\n' +let root = '' + +async function cliInDirectory(label: string, body = 'echo gh-version-fixture\n'): Promise { + const dir = path.join(root, label) + await mkdir(dir, { recursive: true }) + await writeFile( + path.join(dir, COMMAND), + `#!/bin/sh\nprintf '%s\\n' "$@" >> ${shellQuote(path.join(dir, 'argv.txt'))}\n${body}`, + { mode: 0o755 } + ) + return dir +} + +async function probedArgs(dir: string): Promise { + try { + return (await readFile(path.join(dir, 'argv.txt'), 'utf8')).split('\n').filter(Boolean) + } catch (error) { + if (typeof error === 'object' && error !== null && 'code' in error && error.code === 'ENOENT') { + return [] + } + throw error + } +} + +describe.skipIf(process.platform === 'win32')( + 'local CLI version probes with real processes', + () => { + beforeEach(async () => { + root = await mkdtemp(path.join(tmpdir(), 'orca-22975-case-')) + vi.stubEnv('HOME', root) + }) + + afterEach(async () => { + vi.unstubAllEnvs() + await removeTree(root) + }) + + it('detects a runnable copy behind an executable shim that exits 126', async () => { + const shim = await cliInDirectory('shim', BROKEN_SHIM) + const good = await cliInDirectory('good') + vi.stubEnv('PATH', [shim, good].join(path.delimiter)) + + await expect(isCommandOnPath(COMMAND)).resolves.toBe(true) + await expect( + execLocalPreflightCommandOrThrow(path.join(shim, COMMAND), ['--version']) + ).rejects.toMatchObject({ code: 126, stderr: 'broken-shim\n' }) + await expect( + execLocalPreflightCommandOrThrow(path.join(good, COMMAND), ['--version']) + ).resolves.toMatchObject({ stdout: 'gh-version-fixture\n' }) + await expect(isCommandAvailable(COMMAND)).resolves.toBe(true) + await expect(findRunnableLocalCommand(COMMAND)).resolves.toEqual({ + status: 'available', + binary: path.join(good, COMMAND) + }) + }) + + it('keeps PATH order and leaves later copies unprobed when the first works', async () => { + const first = await cliInDirectory('first') + const second = await cliInDirectory('second') + vi.stubEnv('PATH', [first, second].join(path.delimiter)) + + await expect(findRunnableLocalCommand(COMMAND)).resolves.toEqual({ + status: 'available', + binary: path.join(first, COMMAND) + }) + expect(await probedArgs(first)).toEqual(['--version']) + expect(await probedArgs(second)).toEqual([]) + }) + + it('distinguishes exhausted failing copies from an absent command', async () => { + const first = await cliInDirectory('first', BROKEN_SHIM) + const second = await cliInDirectory('second', BROKEN_SHIM) + vi.stubEnv('PATH', [first, second].join(path.delimiter)) + + await expect(findRunnableLocalCommand(COMMAND)).resolves.toEqual({ + status: 'exec_failed', + binary: path.join(second, COMMAND) + }) + await expect(findRunnableLocalCommand('orca-22975-absent-cli')).resolves.toEqual({ + status: 'absent' + }) + expect(await probedArgs(first)).toEqual(['--version']) + expect(await probedArgs(second)).toEqual(['--version']) + }) + + it('skips directories and non-executable files before a working copy', async () => { + const directory = path.join(root, 'directory') + await mkdir(path.join(directory, COMMAND), { recursive: true }) + const plain = await cliInDirectory('plain') + await chmod(path.join(plain, COMMAND), 0o644) + const good = await cliInDirectory('good') + vi.stubEnv('PATH', [directory, plain, good].join(path.delimiter)) + + await expect(findRunnableLocalCommand(COMMAND)).resolves.toEqual({ + status: 'available', + binary: path.join(good, COMMAND) + }) + expect(await probedArgs(plain)).toEqual([]) + }) + + it('never substitutes another binary for an explicit failing path', async () => { + const shim = await cliInDirectory('shim', BROKEN_SHIM) + const good = await cliInDirectory('good') + vi.stubEnv('PATH', good) + const selected = path.join(shim, COMMAND) + + await expect(findRunnableLocalCommand(selected)).resolves.toEqual({ + status: 'exec_failed', + binary: selected + }) + expect(await probedArgs(good)).toEqual([]) + }) + + it('executes relative explicit paths and reports an explicit missing path', async () => { + const good = await cliInDirectory('good') + const selected = path.relative(process.cwd(), path.join(good, COMMAND)) + vi.stubEnv('PATH', '') + + await expect(findRunnableLocalCommand(selected)).resolves.toEqual({ + status: 'available', + binary: selected + }) + await expect(findRunnableLocalCommand(path.join(root, 'missing', COMMAND))).resolves.toEqual({ + status: 'absent' + }) + }) + + it.each([true, false])( + 'preserves relative PATH order with shim first: %s', + async (shimFirst) => { + const good = await cliInDirectory('good') + const shim = await cliInDirectory('shim', BROKEN_SHIM) + const relative = path.relative(process.cwd(), good) + vi.stubEnv('PATH', (shimFirst ? [shim, relative] : [relative, shim]).join(path.delimiter)) + + await expect(findRunnableLocalCommand(COMMAND)).resolves.toEqual({ + status: 'available', + binary: path.join(good, COMMAND) + }) + expect(await probedArgs(good)).toEqual(['--version']) + expect(await probedArgs(shim)).toEqual(shimFirst ? ['--version'] : []) + } + ) + + it('deduplicates PATH entries before spending the probe limit', async () => { + const shim = await cliInDirectory('shim', BROKEN_SHIM) + const good = await cliInDirectory('good') + vi.stubEnv('PATH', [shim, shim, shim, shim, good].join(path.delimiter)) + + await expect(findRunnableLocalCommand(COMMAND)).resolves.toEqual({ + status: 'available', + binary: path.join(good, COMMAND) + }) + expect(await probedArgs(shim)).toEqual(['--version']) + }) + + it('caps execution at four distinct candidates and reports the limit', async () => { + const doomed: string[] = [] + for (const index of [0, 1, 2, 3]) { + doomed.push(await cliInDirectory(`doomed-${index}`, BROKEN_SHIM)) + } + const good = await cliInDirectory('past-cap') + vi.stubEnv('PATH', [...doomed, good].join(path.delimiter)) + + await expect(findRunnableLocalCommand(COMMAND)).resolves.toEqual({ + status: 'limit_reached', + binary: path.join(doomed[3], COMMAND) + }) + for (const dir of doomed) { + expect(await probedArgs(dir)).toEqual(['--version']) + } + expect(await probedArgs(good)).toEqual([]) + }) + + it('stops at a real timeout and leaves later copies unprobed', async () => { + const hung = await cliInDirectory('hung', 'exec /bin/sleep 30\n') + const good = await cliInDirectory('good') + vi.stubEnv('PATH', [hung, good].join(path.delimiter)) + + await expect(findRunnableLocalCommand(COMMAND)).resolves.toEqual({ + status: 'timeout', + binary: path.join(hung, COMMAND) + }) + expect(await probedArgs(good)).toEqual([]) + }, 10_000) + + it('uses the known Nix install directory after PATH copies fail', async () => { + const shim = await cliInDirectory('shim', BROKEN_SHIM) + const profile = await cliInDirectory(path.join('.nix-profile', 'bin')) + vi.stubEnv('PATH', shim) + + await expect(findRunnableLocalCommand(COMMAND)).resolves.toEqual({ + status: 'available', + binary: path.join(profile, COMMAND) + }) + }) + } +) + +describe.runIf(process.platform === 'win32')('Windows preflight batch shims', () => { + beforeEach(async () => { + root = await mkdtemp(path.join(tmpdir(), 'orca-22975-cmd-')) + }) + + afterEach(async () => { + vi.unstubAllEnvs() + await removeTree(root) + }) + + it('runs the next cmd shim after an earlier copy exits 126', async () => { + const first = path.join(root, 'first') + const second = path.join(root, 'second') + for (const dir of [first, second]) { + await mkdir(dir) + } + const name = `${COMMAND}.CMD` + await writeFile(path.join(first, name), '@echo off\r\nexit /b 126\r\n') + await writeFile( + path.join(second, name), + '@echo off\r\necho gh-version-fixture\r\nexit /b 0\r\n' + ) + vi.stubEnv('PATH', [first, second].join(path.delimiter)) + vi.stubEnv('Path', [first, second].join(path.delimiter)) + + const result = await findRunnableLocalCommand(COMMAND) + + expect(result).toEqual({ status: 'available', binary: path.posix.join(second, name) }) + if (result.status === 'available') { + await expect( + execLocalPreflightCommandOrThrow(result.binary, ['auth', 'status']) + ).resolves.toMatchObject({ stdout: expect.stringContaining('gh-version-fixture') }) + } + }) +}) diff --git a/src/main/ipc/preflight-test-harness.ts b/src/main/ipc/preflight-test-harness.ts index abdee69985c..e38848ad5eb 100644 --- a/src/main/ipc/preflight-test-harness.ts +++ b/src/main/ipc/preflight-test-harness.ts @@ -16,6 +16,7 @@ export type PreflightMocks = { getGiteaAuthStatusMock: Mock resolveCliCommandsMock: Mock isCommandOnLocalPathMock: Mock + listLocalCommandPathsMock: Mock mergePersistedWindowsPathAsyncMock: Mock mergePersistedWindowsPathMock: Mock } @@ -52,6 +53,7 @@ export function resetPreflightMocks(mocks: PreflightMocks, handlers: HandlerMap) getGiteaAuthStatusMock, resolveCliCommandsMock, isCommandOnLocalPathMock, + listLocalCommandPathsMock, mergePersistedWindowsPathAsyncMock, mergePersistedWindowsPathMock } = mocks @@ -63,6 +65,11 @@ export function resetPreflightMocks(mocks: PreflightMocks, handlers: HandlerMap) hydrateShellPathMock.mockResolvedValue({ segments: [], ok: false, failureReason: 'no_shell' }) mergePathSegmentsMock.mockReset() getActiveMultiplexerMock.mockReset() + // Why empty by default: with no fs candidates the local probe keeps its + // historical bare-name spawn, so cases that stub only where/which and execFile + // still assert on the command names they were written against. + listLocalCommandPathsMock.mockReset() + listLocalCommandPathsMock.mockResolvedValue([]) getBitbucketAuthStatusMock.mockReset() getAzureDevOpsAuthStatusMock.mockReset() getGiteaAuthStatusMock.mockReset() diff --git a/src/main/preflight/agent-detection.ts b/src/main/preflight/agent-detection.ts index 5abac7cd342..c06f9ee7f12 100644 --- a/src/main/preflight/agent-detection.ts +++ b/src/main/preflight/agent-detection.ts @@ -31,6 +31,7 @@ import { hydrateShellPathForAgentDetection } from '../ipc/agent-detection-shell- import { execCommandInWslOrThrow, execLocalPreflightCommandOrThrow, + findRunnableLocalCommand, isCommandAvailable, isCommandOnPath, shellQuote @@ -126,20 +127,24 @@ function uniqueAgentIds(ids: Iterable): string[] { return [...new Set(ids)] } +/** A CLI verdict and, on the local path, the exact copy that produced it. */ +type CommandRuntime = { installed: boolean; wslTarget?: WslPreflightTarget; binary?: string } + async function detectCommandRuntime( command: string, context?: PreflightRuntimeContext -): Promise<{ installed: boolean; wslTarget?: WslPreflightTarget }> { +): Promise { const wslTarget = getPreflightWslTarget(context) if (wslTarget) { return (await isCommandAvailable(command, wslTarget)) ? { installed: true, wslTarget } : { installed: false } } - if (await isCommandAvailable(command)) { - return { installed: true } - } - return { installed: false } + // Pin auth to the copy that passed --version, so PATH cannot select the dead shim again. + const probe = await findRunnableLocalCommand(command) + return probe.status === 'available' + ? { installed: true, binary: probe.binary } + : { installed: false } } export async function detectInstalledAgents(context?: PreflightRuntimeContext): Promise { @@ -253,11 +258,15 @@ export async function detectRemoteAgents(args: { connectionId: string }): Promis return uniqueAgentIds(result.agents) } -async function isGhAuthenticated(wslTarget?: WslPreflightTarget): Promise { +// Why the probe object rather than the bare command name: on the local path +// `binary` is the copy that just passed `--version`, which on a shim-shadowed +// host is not what PATH would resolve (#22975). WSL has no `binary` — the guest +// resolves the name inside the distro, where Orca's PATH ordering cannot apply. +async function isGhAuthenticated(probe: CommandRuntime): Promise { try { - await (wslTarget - ? execCommandInWslOrThrow(wslTarget, `${shellQuote('gh')} auth status`) - : execLocalPreflightCommandOrThrow('gh', ['auth', 'status'])) + await (probe.wslTarget + ? execCommandInWslOrThrow(probe.wslTarget, `${shellQuote('gh')} auth status`) + : execLocalPreflightCommandOrThrow(probe.binary ?? 'gh', ['auth', 'status'])) // Why: for plain-text `gh auth status`, exit 0 means gh did not detect any // authentication issues for the checked hosts/accounts. return true @@ -274,11 +283,11 @@ async function isGhAuthenticated(wslTarget?: WslPreflightTarget): Promise { +async function isGlabAuthenticated(probe: CommandRuntime): Promise { try { - await (wslTarget - ? execCommandInWslOrThrow(wslTarget, `${shellQuote('glab')} auth status`) - : execLocalPreflightCommandOrThrow('glab', ['auth', 'status'])) + await (probe.wslTarget + ? execCommandInWslOrThrow(probe.wslTarget, `${shellQuote('glab')} auth status`) + : execLocalPreflightCommandOrThrow(probe.binary ?? 'glab', ['auth', 'status'])) return true } catch (error) { const stdout = (error as { stdout?: string }).stdout ?? '' @@ -380,8 +389,8 @@ async function executePreflightCheck( ]) const [ghAuthenticated, glabAuthenticated, bitbucket, azureDevOps, gitea] = await Promise.all([ - ghProbe.installed ? isGhAuthenticated(ghProbe.wslTarget) : Promise.resolve(false), - glabProbe.installed ? isGlabAuthenticated(glabProbe.wslTarget) : Promise.resolve(false), + ghProbe.installed ? isGhAuthenticated(ghProbe) : Promise.resolve(false), + glabProbe.installed ? isGlabAuthenticated(glabProbe) : Promise.resolve(false), getBitbucketAuthStatus(), getAzureDevOpsAuthStatus(), getGiteaAuthStatus()