From 385e3c59a105e655698c5600a32a1c24cb8228e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Wed, 3 Jun 2026 03:19:49 +0100 Subject: [PATCH] fix(hooks): resolve gitnexus on PATH with a pure-Node scan, all-OS (#1938) (#1980) --- .../hooks/resolve-analyze-cmd.cjs | 112 +++++---- gitnexus/hooks/claude/resolve-analyze-cmd.cjs | 112 +++++---- .../integration/antigravity-hook-e2e.test.ts | 40 +++- gitnexus/test/integration/hooks-e2e.test.ts | 72 +++++- gitnexus/test/unit/resolve-invocation.test.ts | 220 +++++++++++++----- gitnexus/test/utils/hook-test-helpers.ts | 83 ++++++- 6 files changed, 496 insertions(+), 143 deletions(-) diff --git a/gitnexus-claude-plugin/hooks/resolve-analyze-cmd.cjs b/gitnexus-claude-plugin/hooks/resolve-analyze-cmd.cjs index cf87c3bd2b..f6f6c2cc24 100644 --- a/gitnexus-claude-plugin/hooks/resolve-analyze-cmd.cjs +++ b/gitnexus-claude-plugin/hooks/resolve-analyze-cmd.cjs @@ -27,6 +27,8 @@ */ const { execFileSync } = require('child_process'); +const fs = require('fs'); +const path = require('path'); const NPX_REF = 'gitnexus@latest'; @@ -34,46 +36,72 @@ const NPX_REF = 'gitnexus@latest'; const PNPM_ALLOW_BUILD_BASE = ['@ladybugdb/core', 'gitnexus', 'tree-sitter']; const PNPM_ALLOW_BUILD_EMBEDDINGS = ['onnxruntime-node']; -// Probe timeout, kept under Claude Code's 10s hook budget. In a linked worktree -// the stale-index hook first runs `git rev-parse --git-common-dir` (~2s) and -// `git rev-parse HEAD` (~3s); the pnpm path then adds up to four 1s probes -// (which gitnexus, npm --version, which pnpm, pnpm --version), so the worst case -// is ~9s — within budget but tight. A healthy `which`/`where`/`--version` -// returns in well under a second, so the realistic cost is far lower. +// Version-probe timeout, kept under Claude Code's 10s hook budget. PATH presence +// detection is now spawn-free (resolveOnPath scans PATH directly), so the only +// subprocesses left are the version probes: in a linked worktree the stale-index +// hook first runs `git rev-parse --git-common-dir` (~2s) and `git rev-parse HEAD` +// (~3s); the pnpm path then adds up to two 1s `--version` probes (npm, pnpm), so +// the worst case is ~7s — within budget. A healthy `--version` returns in well +// under a second, so the realistic cost is far lower. const PROBE_TIMEOUT_MS = 1000; /** - * Pick the best match from `where`/`which` output. A global `gitnexus` may be a - * `.cmd`/`.bat` (npm), a `.exe`, or an extensionless shim (Volta, scoop), so on - * Windows we prefer a recognized executable extension but accept any hit — the - * emitted hint is `gitnexus analyze` regardless of which shim resolves it. Pure - * and exported so the shim-matching can be unit-tested without spawning. + * Absolute path to `command` on PATH, or null — a pure-Node, spawn-free lookup + * that mirrors how a shell resolves a bare command name: each PATH dir × the + * platform's executable extensions (PATHEXT on Windows; the bare name + X_OK on + * POSIX). This replaces the former `where`/`which` subprocess (#1938 "Option A"): + * it is byte-for-byte identical on every OS, with no dependency on the probe + * binary being reachable (a sanitized PATH that drops System32 / `/usr/bin` no + * longer defeats detection), no shell-spawn surface (CVE-2024-27980), and no + * spawn timeout to tune. On Windows it matches PATHEXT extensions ONLY — exactly + * what `where`/cmd.exe resolve — so neither an un-spawnable `.ps1`-only shim (not + * in default PATHEXT) nor a bare extensionless file (which the shell cannot launch + * as `command`) is a false positive. `preferExecExt` returns a recognized + * `.cmd`/`.bat`/`.exe` shim ahead of an exotic PATHEXT hit (e.g. `.COM`) when both + * match, matching what a user would actually launch. Pure (platform/env injectable) + * so it is unit-testable without touching the host PATH. */ -function pickPathMatch(output, { isWin, gitnexusWrapper } = {}) { - const lines = output - .split('\n') - .map((l) => l.trim()) - .filter(Boolean); - if (isWin && gitnexusWrapper) { - return lines.find((l) => /\.(cmd|bat|exe)$/i.test(l)) || lines[0] || null; - } - return lines[0] || null; -} - -/** Absolute path to `command` on PATH, or null. `gitnexusWrapper` enables the Windows shim match. */ -function resolveOnPath(command, gitnexusWrapper = false) { - const isWin = process.platform === 'win32'; - try { - const output = execFileSync(isWin ? 'where' : 'which', [command], { - encoding: 'utf-8', - timeout: PROBE_TIMEOUT_MS, - stdio: ['ignore', 'pipe', 'ignore'], - windowsHide: true, - }); - return pickPathMatch(output, { isWin, gitnexusWrapper }); - } catch { - return null; +function resolveOnPath( + command, + preferExecExt = false, + { platform = process.platform, env = process.env } = {}, +) { + const pathValue = env.PATH || env.Path || env.path || ''; + if (!pathValue) return null; + const isWin = platform === 'win32'; + const exts = isWin + ? (env.PATHEXT || '.COM;.EXE;.BAT;.CMD') + .split(';') + .map((e) => e.trim()) + .filter(Boolean) + .map((e) => (e.startsWith('.') ? e : `.${e}`)) + : ['']; + let weakHit = null; + // Split on the host's PATH delimiter. `platform` is injected only to choose the + // extension/exec-bit rules; the PATH string is always host-format, so it must + // split on the host delimiter (`path.delimiter`) — in production `platform` IS + // the host, so they coincide. (Deriving the delimiter from an injected platform + // would split a Windows drive-letter path `C:\…` at its colon under a POSIX + // injection.) + for (const dir of pathValue.split(path.delimiter).filter(Boolean)) { + for (const ext of exts) { + const candidate = path.join(dir, `${command}${ext}`); + try { + if (!fs.statSync(candidate).isFile()) continue; + if (!isWin) fs.accessSync(candidate, fs.constants.X_OK); + // Prefer a runnable .cmd/.bat/.exe shim; remember an exotic PATHEXT hit + // (e.g. .COM) only as a last resort if nothing better turns up. + if (isWin && preferExecExt && !/\.(cmd|bat|exe)$/i.test(ext)) { + weakHit = weakHit || candidate; + continue; + } + return candidate; + } catch { + /* not a runnable file here — try the next candidate */ + } + } } + return weakHit; } // One spawn of ` --version` → { major, minor } (each null when @@ -193,12 +221,12 @@ function formatPnpmDlxCommand(gitnexusArgs, options = {}, deps = {}) { function formatAnalyzeCommand(options = {}, deps = {}) { const suffix = options.embeddings ? ' --embeddings' : ''; // Keep the stale-index hook budget tight by querying each tool at most once. - // A memoized PATH probe is shared with resolveInvocationMode (so `gitnexus` - // isn't probed twice), and pnpm's version is captured by a single - // `pnpm --version` that proves both presence (for mode resolution) and - // version (for the allow-build gate) — replacing the former `which pnpm` + - // `pnpm --version` double spawn. Injected deps (tests) and forced/global - // modes skip the pnpm probe. + // The memoized `probe` is a spawn-free PATH scan (resolveOnPath) shared with + // resolveInvocationMode, so `gitnexus` is scanned only once and no subprocess + // is spawned for presence. pnpm's *version* is still captured by a single + // `pnpm --version` (the allow-build gate needs the number), which also proves + // presence; the memoized scan only re-checks pnpm when that version is + // unreadable. Injected deps (tests) and forced/global modes skip the pnpm probe. const cache = new Map(); const probe = (command, gitnexusWrapper) => { const key = `${command}:${gitnexusWrapper ? 1 : 0}`; @@ -256,7 +284,7 @@ module.exports = { formatPnpmDlxCommand, resolveInvocationMode, buildRunnerArgv, - pickPathMatch, + resolveOnPath, getNpmMajorVersion, NPX_REF, PNPM_ALLOW_BUILD_BASE, diff --git a/gitnexus/hooks/claude/resolve-analyze-cmd.cjs b/gitnexus/hooks/claude/resolve-analyze-cmd.cjs index cf87c3bd2b..f6f6c2cc24 100644 --- a/gitnexus/hooks/claude/resolve-analyze-cmd.cjs +++ b/gitnexus/hooks/claude/resolve-analyze-cmd.cjs @@ -27,6 +27,8 @@ */ const { execFileSync } = require('child_process'); +const fs = require('fs'); +const path = require('path'); const NPX_REF = 'gitnexus@latest'; @@ -34,46 +36,72 @@ const NPX_REF = 'gitnexus@latest'; const PNPM_ALLOW_BUILD_BASE = ['@ladybugdb/core', 'gitnexus', 'tree-sitter']; const PNPM_ALLOW_BUILD_EMBEDDINGS = ['onnxruntime-node']; -// Probe timeout, kept under Claude Code's 10s hook budget. In a linked worktree -// the stale-index hook first runs `git rev-parse --git-common-dir` (~2s) and -// `git rev-parse HEAD` (~3s); the pnpm path then adds up to four 1s probes -// (which gitnexus, npm --version, which pnpm, pnpm --version), so the worst case -// is ~9s — within budget but tight. A healthy `which`/`where`/`--version` -// returns in well under a second, so the realistic cost is far lower. +// Version-probe timeout, kept under Claude Code's 10s hook budget. PATH presence +// detection is now spawn-free (resolveOnPath scans PATH directly), so the only +// subprocesses left are the version probes: in a linked worktree the stale-index +// hook first runs `git rev-parse --git-common-dir` (~2s) and `git rev-parse HEAD` +// (~3s); the pnpm path then adds up to two 1s `--version` probes (npm, pnpm), so +// the worst case is ~7s — within budget. A healthy `--version` returns in well +// under a second, so the realistic cost is far lower. const PROBE_TIMEOUT_MS = 1000; /** - * Pick the best match from `where`/`which` output. A global `gitnexus` may be a - * `.cmd`/`.bat` (npm), a `.exe`, or an extensionless shim (Volta, scoop), so on - * Windows we prefer a recognized executable extension but accept any hit — the - * emitted hint is `gitnexus analyze` regardless of which shim resolves it. Pure - * and exported so the shim-matching can be unit-tested without spawning. + * Absolute path to `command` on PATH, or null — a pure-Node, spawn-free lookup + * that mirrors how a shell resolves a bare command name: each PATH dir × the + * platform's executable extensions (PATHEXT on Windows; the bare name + X_OK on + * POSIX). This replaces the former `where`/`which` subprocess (#1938 "Option A"): + * it is byte-for-byte identical on every OS, with no dependency on the probe + * binary being reachable (a sanitized PATH that drops System32 / `/usr/bin` no + * longer defeats detection), no shell-spawn surface (CVE-2024-27980), and no + * spawn timeout to tune. On Windows it matches PATHEXT extensions ONLY — exactly + * what `where`/cmd.exe resolve — so neither an un-spawnable `.ps1`-only shim (not + * in default PATHEXT) nor a bare extensionless file (which the shell cannot launch + * as `command`) is a false positive. `preferExecExt` returns a recognized + * `.cmd`/`.bat`/`.exe` shim ahead of an exotic PATHEXT hit (e.g. `.COM`) when both + * match, matching what a user would actually launch. Pure (platform/env injectable) + * so it is unit-testable without touching the host PATH. */ -function pickPathMatch(output, { isWin, gitnexusWrapper } = {}) { - const lines = output - .split('\n') - .map((l) => l.trim()) - .filter(Boolean); - if (isWin && gitnexusWrapper) { - return lines.find((l) => /\.(cmd|bat|exe)$/i.test(l)) || lines[0] || null; - } - return lines[0] || null; -} - -/** Absolute path to `command` on PATH, or null. `gitnexusWrapper` enables the Windows shim match. */ -function resolveOnPath(command, gitnexusWrapper = false) { - const isWin = process.platform === 'win32'; - try { - const output = execFileSync(isWin ? 'where' : 'which', [command], { - encoding: 'utf-8', - timeout: PROBE_TIMEOUT_MS, - stdio: ['ignore', 'pipe', 'ignore'], - windowsHide: true, - }); - return pickPathMatch(output, { isWin, gitnexusWrapper }); - } catch { - return null; +function resolveOnPath( + command, + preferExecExt = false, + { platform = process.platform, env = process.env } = {}, +) { + const pathValue = env.PATH || env.Path || env.path || ''; + if (!pathValue) return null; + const isWin = platform === 'win32'; + const exts = isWin + ? (env.PATHEXT || '.COM;.EXE;.BAT;.CMD') + .split(';') + .map((e) => e.trim()) + .filter(Boolean) + .map((e) => (e.startsWith('.') ? e : `.${e}`)) + : ['']; + let weakHit = null; + // Split on the host's PATH delimiter. `platform` is injected only to choose the + // extension/exec-bit rules; the PATH string is always host-format, so it must + // split on the host delimiter (`path.delimiter`) — in production `platform` IS + // the host, so they coincide. (Deriving the delimiter from an injected platform + // would split a Windows drive-letter path `C:\…` at its colon under a POSIX + // injection.) + for (const dir of pathValue.split(path.delimiter).filter(Boolean)) { + for (const ext of exts) { + const candidate = path.join(dir, `${command}${ext}`); + try { + if (!fs.statSync(candidate).isFile()) continue; + if (!isWin) fs.accessSync(candidate, fs.constants.X_OK); + // Prefer a runnable .cmd/.bat/.exe shim; remember an exotic PATHEXT hit + // (e.g. .COM) only as a last resort if nothing better turns up. + if (isWin && preferExecExt && !/\.(cmd|bat|exe)$/i.test(ext)) { + weakHit = weakHit || candidate; + continue; + } + return candidate; + } catch { + /* not a runnable file here — try the next candidate */ + } + } } + return weakHit; } // One spawn of ` --version` → { major, minor } (each null when @@ -193,12 +221,12 @@ function formatPnpmDlxCommand(gitnexusArgs, options = {}, deps = {}) { function formatAnalyzeCommand(options = {}, deps = {}) { const suffix = options.embeddings ? ' --embeddings' : ''; // Keep the stale-index hook budget tight by querying each tool at most once. - // A memoized PATH probe is shared with resolveInvocationMode (so `gitnexus` - // isn't probed twice), and pnpm's version is captured by a single - // `pnpm --version` that proves both presence (for mode resolution) and - // version (for the allow-build gate) — replacing the former `which pnpm` + - // `pnpm --version` double spawn. Injected deps (tests) and forced/global - // modes skip the pnpm probe. + // The memoized `probe` is a spawn-free PATH scan (resolveOnPath) shared with + // resolveInvocationMode, so `gitnexus` is scanned only once and no subprocess + // is spawned for presence. pnpm's *version* is still captured by a single + // `pnpm --version` (the allow-build gate needs the number), which also proves + // presence; the memoized scan only re-checks pnpm when that version is + // unreadable. Injected deps (tests) and forced/global modes skip the pnpm probe. const cache = new Map(); const probe = (command, gitnexusWrapper) => { const key = `${command}:${gitnexusWrapper ? 1 : 0}`; @@ -256,7 +284,7 @@ module.exports = { formatPnpmDlxCommand, resolveInvocationMode, buildRunnerArgv, - pickPathMatch, + resolveOnPath, getNpmMajorVersion, NPX_REF, PNPM_ALLOW_BUILD_BASE, diff --git a/gitnexus/test/integration/antigravity-hook-e2e.test.ts b/gitnexus/test/integration/antigravity-hook-e2e.test.ts index 0150d1f065..33b2ca56fb 100644 --- a/gitnexus/test/integration/antigravity-hook-e2e.test.ts +++ b/gitnexus/test/integration/antigravity-hook-e2e.test.ts @@ -22,7 +22,12 @@ import fsp from 'fs/promises'; import path from 'path'; import { cleanupTempDir, cleanupTempDirSync } from '../helpers/test-db.js'; import os from 'os'; -import { runHook, parseHookOutput } from '../utils/hook-test-helpers.js'; +import { + runHook, + parseHookOutput, + createGitNexusPathEntry, + envWithPath, +} from '../utils/hook-test-helpers.js'; import { setupCommand } from '../../src/cli/setup.js'; let tempHome: string; @@ -126,6 +131,39 @@ describe('antigravity hook adapter e2e', () => { expect(result.stderr).toContain('[GitNexus] index is stale'); }); + it('auto-detects a PATH-installed gitnexus and suggests `gitnexus analyze` (no npx)', () => { + // No GITNEXUS_INVOCATION forcing — exercises the installed hook's real PATH + // probe (#1938). The installed adapter resolves the analyze command through + // the copied resolve-analyze-cmd.cjs, so a launcher on PATH yields + // `gitnexus analyze` rather than the npm-11 npx crash path. + fs.writeFileSync( + path.join(gitNexusDir, 'meta.json'), + JSON.stringify({ lastCommit: 'a'.repeat(39) + 'b', stats: {} }), + ); + const gn = createGitNexusPathEntry(); + try { + const result = runHook( + installedHook, + { + hook_event_name: 'AfterTool', + tool_name: 'run_shell_command', + tool_input: { command: 'git commit -m "test"' }, + tool_response: { llmContent: '[committed]' }, + cwd: tmpDir, + }, + tmpDir, + { env: envWithPath(gn.pathValue) }, + ); + + const output = parseHookOutput(result.stdout); + expect(output).not.toBeNull(); + expect(output!.additionalContext).toContain('Run `gitnexus analyze`'); + expect(output!.additionalContext).not.toContain('npx gitnexus'); + } finally { + gn.cleanup(); + } + }); + it('stays silent when meta.json lastCommit matches HEAD', () => { const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: tmpDir, diff --git a/gitnexus/test/integration/hooks-e2e.test.ts b/gitnexus/test/integration/hooks-e2e.test.ts index ea802ed3a8..3402d23abe 100644 --- a/gitnexus/test/integration/hooks-e2e.test.ts +++ b/gitnexus/test/integration/hooks-e2e.test.ts @@ -10,7 +10,12 @@ import { spawnSync } from 'child_process'; import fs from 'fs'; import path from 'path'; import os from 'os'; -import { runHook, parseHookOutput } from '../utils/hook-test-helpers.js'; +import { + runHook, + parseHookOutput, + createGitNexusPathEntry, + envWithPath, +} from '../utils/hook-test-helpers.js'; // ─── Paths to both hook variants ──────────────────────────────────── @@ -110,6 +115,71 @@ describe.each(HOOKS)('hooks e2e ($name)', ({ name, path: hookPath }) => { expect(output!.additionalContext).toContain('gitnexus@latest analyze'); }); + it('auto-detects a PATH-installed gitnexus and suggests `gitnexus analyze` (no npx)', () => { + // No GITNEXUS_INVOCATION forcing — this exercises the hook's real PATH probe + // (#1938): a launcher on PATH must yield `gitnexus analyze`, never the + // npm-11 npx crash path. createGitNexusPathEntry scrubs any ambient gitnexus + // first, so the result cannot pass for the wrong reason. + fs.writeFileSync( + path.join(gitNexusDir, 'meta.json'), + JSON.stringify({ lastCommit: 'abababababababababababababababababababab', stats: {} }), + ); + const gn = createGitNexusPathEntry(); + try { + const result = runHook( + hookPath, + { + hook_event_name: 'PostToolUse', + tool_name: 'Bash', + tool_input: { command: 'git commit -m "test"' }, + tool_output: { exit_code: 0 }, + cwd: tmpDir, + }, + tmpDir, + { env: envWithPath(gn.pathValue) }, + ); + + const output = parseHookOutput(result.stdout); + expect(output).not.toBeNull(); + expect(output!.additionalContext).toContain('Run `gitnexus analyze`'); + expect(output!.additionalContext).not.toContain('npx gitnexus'); + } finally { + gn.cleanup(); + } + }); + + it('appends --embeddings to the auto-detected `gitnexus analyze` when the index had embeddings', () => { + fs.writeFileSync( + path.join(gitNexusDir, 'meta.json'), + JSON.stringify({ + lastCommit: 'cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd', + stats: { embeddings: 42 }, + }), + ); + const gn = createGitNexusPathEntry(); + try { + const result = runHook( + hookPath, + { + hook_event_name: 'PostToolUse', + tool_name: 'Bash', + tool_input: { command: 'git commit -m "test"' }, + tool_output: { exit_code: 0 }, + cwd: tmpDir, + }, + tmpDir, + { env: envWithPath(gn.pathValue) }, + ); + + const output = parseHookOutput(result.stdout); + expect(output).not.toBeNull(); + expect(output!.additionalContext).toContain('Run `gitnexus analyze --embeddings`'); + expect(output!.additionalContext).not.toContain('npx gitnexus'); + } finally { + gn.cleanup(); + } + }); + it('stays silent when meta.json lastCommit matches HEAD', () => { // Get current HEAD const headResult = spawnSync('git', ['rev-parse', 'HEAD'], { diff --git a/gitnexus/test/unit/resolve-invocation.test.ts b/gitnexus/test/unit/resolve-invocation.test.ts index 4511611b90..56946ad370 100644 --- a/gitnexus/test/unit/resolve-invocation.test.ts +++ b/gitnexus/test/unit/resolve-invocation.test.ts @@ -10,9 +10,10 @@ import { warnIfNpm11NpxRisk, NPX_REF, } from '../../src/cli/resolve-invocation.js'; -import { readFileSync } from 'node:fs'; +import { readFileSync, mkdtempSync, writeFileSync, chmodSync, rmSync } from 'node:fs'; import { createRequire } from 'node:module'; import path from 'node:path'; +import os from 'node:os'; const mockedExec = vi.mocked(execFileSync); @@ -49,9 +50,10 @@ interface CjsModule { probe?: (command: string, gitnexusWrapper?: boolean) => string | null, deps?: { npmMajor?: number | null; pnpmMajor?: number | null; pnpmPresent?: boolean }, ) => 'gitnexus' | 'pnpm' | 'npx'; - pickPathMatch: ( - output: string, - opts?: { isWin?: boolean; gitnexusWrapper?: boolean }, + resolveOnPath: ( + command: string, + preferExecExt?: boolean, + opts?: { platform?: NodeJS.Platform; env?: NodeJS.ProcessEnv }, ) => string | null; buildRunnerArgv: ( mode: 'gitnexus' | 'pnpm' | 'npx', @@ -65,10 +67,11 @@ interface CjsModule { // the tests exercise production code, not a TypeScript mirror of it. // // Determinism invariant: createRequire bypasses vitest's node:child_process mock, -// so this module's resolveOnPath() would spawn a real `which`/`where`. Every test -// below avoids the live probe — by forcing GITNEXUS_INVOCATION, injecting a fake -// `probe`, or calling the pure pickPathMatch() — so results never depend on the -// host PATH. Keep new tests on one of those three paths. +// so the only live subprocess this module can run is probeVersion (`npm`/`pnpm +// --version`). resolveOnPath is now spawn-free — a pure PATH scan — so tests pin +// it by passing an injected `{ platform, env }` (never the host PATH). Mode tests +// inject a fake `probe` or force GITNEXUS_INVOCATION; version tests inject `deps`. +// Keep new tests on one of those paths so results never depend on the host. const cjs = cjsRequire(CANONICAL_CJS) as CjsModule; describe('resolve-analyze-cmd.cjs (canonical invocation resolver)', () => { @@ -204,54 +207,6 @@ describe('resolve-analyze-cmd.cjs (canonical invocation resolver)', () => { }); }); -describe('pickPathMatch — Windows global-shim detection', () => { - it('detects a .exe-only shim (Volta/scoop)', () => { - expect( - cjs.pickPathMatch('C:\\Users\\me\\AppData\\Local\\Volta\\bin\\gitnexus.exe\r\n', { - isWin: true, - gitnexusWrapper: true, - }), - ).toBe('C:\\Users\\me\\AppData\\Local\\Volta\\bin\\gitnexus.exe'); - }); - - it('detects an extensionless shim', () => { - expect( - cjs.pickPathMatch('C:\\tools\\gitnexus\r\n', { isWin: true, gitnexusWrapper: true }), - ).toBe('C:\\tools\\gitnexus'); - }); - - it('prefers a .cmd over an extensionless sibling', () => { - expect( - cjs.pickPathMatch('C:\\npm\\gitnexus\r\nC:\\npm\\gitnexus.cmd\r\n', { - isWin: true, - gitnexusWrapper: true, - }), - ).toBe('C:\\npm\\gitnexus.cmd'); - }); - - it('strips the CRLF carriage return from the chosen path', () => { - const bin = cjs.pickPathMatch('C:\\npm\\gitnexus.cmd\r\n', { - isWin: true, - gitnexusWrapper: true, - }); - expect(bin).not.toMatch(/\r/); - expect(bin).toBe('C:\\npm\\gitnexus.cmd'); - }); - - it('returns the first hit on non-Windows / non-wrapper lookups, null on empty', () => { - expect(cjs.pickPathMatch('/usr/local/bin/pnpm\n', { isWin: false })).toBe( - '/usr/local/bin/pnpm', - ); - expect(cjs.pickPathMatch('', { isWin: true, gitnexusWrapper: true })).toBeNull(); - }); - - it('returns the first hit for a Windows non-wrapper lookup (pnpm probe)', () => { - expect( - cjs.pickPathMatch('C:\\npm\\pnpm.cmd\r\n', { isWin: true, gitnexusWrapper: false }), - ).toBe('C:\\npm\\pnpm.cmd'); - }); -}); - describe('warnIfNpm11NpxRisk (#1939 npm-11 nudge)', () => { afterEach(() => { vi.clearAllMocks(); @@ -426,6 +381,159 @@ describe('buildRunnerArgv (project-local runner exec, #1945)', () => { }); }); +describe('resolveOnPath — pure-Node PATH scan (#1938, all-OS, spawn-free)', () => { + const tmpDirs: string[] = []; + const mkBinDir = (): string => { + const dir = mkdtempSync(path.join(os.tmpdir(), 'resolve-path-')); + tmpDirs.push(dir); + return dir; + }; + afterEach(() => { + while (tmpDirs.length) rmSync(tmpDirs.pop() as string, { recursive: true, force: true }); + }); + + it('finds an executable launcher on a POSIX PATH', () => { + const dir = mkBinDir(); + const bin = path.join(dir, 'gitnexus'); + writeFileSync(bin, '#!/bin/sh\nexit 0\n'); + chmodSync(bin, 0o755); + expect(cjs.resolveOnPath('gitnexus', true, { platform: 'linux', env: { PATH: dir } })).toBe( + bin, + ); + }); + + // X_OK is meaningless on Windows (every file reads as accessible), so this + // POSIX-only guarantee can only be asserted on a POSIX host. + it.skipIf(process.platform === 'win32')( + 'skips a non-executable file on POSIX (requires X_OK)', + () => { + const dir = mkBinDir(); + writeFileSync(path.join(dir, 'gitnexus'), 'not executable'); // intentionally no chmod +x + expect( + cjs.resolveOnPath('gitnexus', true, { platform: 'linux', env: { PATH: dir } }), + ).toBeNull(); + }, + ); + + it('returns null when the launcher is absent or PATH is empty', () => { + const dir = mkBinDir(); + expect( + cjs.resolveOnPath('gitnexus', true, { platform: 'linux', env: { PATH: dir } }), + ).toBeNull(); + expect(cjs.resolveOnPath('gitnexus', true, { platform: 'linux', env: {} })).toBeNull(); + }); + + it('honors PATHEXT on Windows (a .cmd shim is detected)', () => { + const dir = mkBinDir(); + const bin = path.join(dir, 'gitnexus.cmd'); + writeFileSync(bin, '@echo off\r\n'); + // The PATHEXT entry case matches the fixture so the assertion is deterministic + // on case-sensitive CI filesystems; real Windows is case-insensitive, so the + // casing of PATHEXT vs the on-disk shim never matters there. + expect( + cjs.resolveOnPath('gitnexus', true, { + platform: 'win32', + env: { PATH: dir, PATHEXT: '.COM;.EXE;.BAT;.cmd' }, + }), + ).toBe(bin); + }); + + it('does not treat a .ps1-only shim as on PATH when PATHEXT excludes .PS1', () => { + // A .ps1 is not launchable as `gitnexus` without a shell and is absent from + // default PATHEXT, so mirroring `where`/cmd.exe (PATHEXT-driven) avoids a hint + // that would fail when run. + const dir = mkBinDir(); + writeFileSync(path.join(dir, 'gitnexus.ps1'), 'exit 0'); + expect( + cjs.resolveOnPath('gitnexus', true, { + platform: 'win32', + env: { PATH: dir, PATHEXT: '.COM;.EXE;.BAT;.CMD' }, + }), + ).toBeNull(); + }); + + it('on Windows ignores a bare extensionless file and returns the PATHEXT shim', () => { + // Windows matches PATHEXT extensions only — an extensionless `gitnexus` is not + // launchable as `gitnexus` from a shell, so when both exist the .cmd shim wins + // and the bare file is never the result (it would be an un-spawnable hint). + const dir = mkBinDir(); + writeFileSync(path.join(dir, 'gitnexus'), 'not a shim'); + const cmd = path.join(dir, 'gitnexus.cmd'); + writeFileSync(cmd, '@echo off\r\n'); + expect( + cjs.resolveOnPath('gitnexus', true, { + platform: 'win32', + env: { PATH: dir, PATHEXT: '.COM;.EXE;.BAT;.cmd' }, + }), + ).toBe(cmd); + }); + + it('on Windows returns null for an extensionless-only file (not in PATHEXT)', () => { + const dir = mkBinDir(); + writeFileSync(path.join(dir, 'gitnexus'), 'not a shim'); + expect( + cjs.resolveOnPath('gitnexus', true, { + platform: 'win32', + env: { PATH: dir, PATHEXT: '.COM;.EXE;.BAT;.CMD' }, + }), + ).toBeNull(); + }); + + it('with preferExecExt, prefers a .cmd/.exe shim over an exotic .COM hit, but accepts .COM alone', () => { + // preferExecExt mirrors the old `where` wrapper preference: a recognized + // .cmd/.bat/.exe wins over a .COM, yet a lone .COM is still detected (better a + // resolvable hint than none). Fixture/PATHEXT cases match for CI determinism. + const both = mkBinDir(); + writeFileSync(path.join(both, 'gitnexus.com'), 'x'); + const cmd = path.join(both, 'gitnexus.cmd'); + writeFileSync(cmd, '@echo off\r\n'); + expect( + cjs.resolveOnPath('gitnexus', true, { + platform: 'win32', + env: { PATH: both, PATHEXT: '.com;.cmd' }, + }), + ).toBe(cmd); + + const comOnly = mkBinDir(); + const com = path.join(comOnly, 'gitnexus.com'); + writeFileSync(com, 'x'); + expect( + cjs.resolveOnPath('gitnexus', true, { + platform: 'win32', + env: { PATH: comOnly, PATHEXT: '.com;.cmd' }, + }), + ).toBe(com); + }); +}); + +describe('formatAnalyzeCommand end-to-end via the pure scan (#1938)', () => { + // Exercises the public entry through resolveOnPath against a real PATH (no + // mocks, no GITNEXUS_INVOCATION): with a launcher on PATH the hint resolves to + // `gitnexus analyze`. Because resolveOnPath is now spawn-free, this works + // identically on every OS — there is no `where`/`which` reachability caveat. + const savedPath = process.env.PATH; + let binDir: string | undefined; + afterEach(() => { + if (savedPath === undefined) delete process.env.PATH; + else process.env.PATH = savedPath; + if (binDir) rmSync(binDir, { recursive: true, force: true }); + binDir = undefined; + delete process.env.GITNEXUS_INVOCATION; + }); + + it('resolves `gitnexus analyze` when a launcher is the only thing on PATH', () => { + binDir = mkdtempSync(path.join(os.tmpdir(), 'gn-e2e-')); + const isWin = process.platform === 'win32'; + const launcher = path.join(binDir, isWin ? 'gitnexus.cmd' : 'gitnexus'); + writeFileSync(launcher, isWin ? '@echo off\r\nexit /b 0\r\n' : '#!/bin/sh\nexit 0\n'); + if (!isWin) chmodSync(launcher, 0o755); + // PATH reduced to just the launcher dir — the former `where`/`which` resolver + // would have ENOENT'd here; the pure scan finds the launcher directly. + process.env.PATH = binDir; + expect(cjs.formatAnalyzeCommand()).toBe('gitnexus analyze'); + }); +}); + describe('resolve-analyze-cmd.cjs parity', () => { it('keeps the two CJS hook copies byte-identical', () => { expect(readFileSync(CANONICAL_CJS, 'utf-8')).toBe(readFileSync(PLUGIN_CJS, 'utf-8')); diff --git a/gitnexus/test/utils/hook-test-helpers.ts b/gitnexus/test/utils/hook-test-helpers.ts index 0898252556..f659a58348 100644 --- a/gitnexus/test/utils/hook-test-helpers.ts +++ b/gitnexus/test/utils/hook-test-helpers.ts @@ -2,6 +2,9 @@ * Shared helpers for hook test files (unit + integration). */ import { spawnSync } from 'child_process'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; export function runHook( hookPath: string, @@ -14,7 +17,12 @@ export function runHook( encoding: 'utf-8', timeout: 10000, cwd, - env: options.env ? { ...process.env, ...options.env } : process.env, + // Used as-is when provided: every caller passes a full env (a spread of + // process.env plus overrides), so re-merging process.env here is redundant + // and, worse, on Windows it re-adds the original `Path` key alongside a + // replaced `PATH` — defeating envWithPath(), which deletes path variants so a + // scrubbed PATH is honored deterministically. + env: options.env ?? process.env, stdio: ['pipe', 'pipe', 'pipe'], }); return { @@ -35,3 +43,76 @@ export function parseHookOutput( return null; } } + +// ─── Stale-index hint PATH-detection helpers (#1938) ──────────────── +// +// The hooks emit `gitnexus analyze` (no npx) when a launcher is on PATH. These +// helpers let an e2e test fabricate that condition deterministically: scrub any +// ambient `gitnexus` off PATH, then prepend a synthetic launcher — so the test +// asserts the hook's real PATH auto-detection rather than env-var forcing. + +/** Names a global `gitnexus` may take on each platform (for scrub + fabricate). */ +function gitNexusLauncherNames(): string[] { + return process.platform === 'win32' + ? ['gitnexus', 'gitnexus.cmd', 'gitnexus.bat', 'gitnexus.exe', 'gitnexus.ps1'] + : ['gitnexus']; +} + +/** True if `dir` holds a runnable `gitnexus` launcher (isFile + X_OK on POSIX). */ +function hasGitNexusLauncher(dir: string): boolean { + return gitNexusLauncherNames().some((name) => { + const candidate = path.join(dir, name); + try { + if (!fs.statSync(candidate).isFile()) return false; + if (process.platform !== 'win32') fs.accessSync(candidate, fs.constants.X_OK); + return true; + } catch { + return false; + } + }); +} + +/** + * The current PATH with every dir that contains a `gitnexus` launcher removed, so + * a test box that already has gitnexus installed cannot make the assertion pass + * (or fail) for the wrong reason. Mirrors the hook's own detection — isFile() + + * X_OK — rather than a bare existsSync. + */ +export function pathWithoutGitNexus( + pathValue: string = process.env.PATH || process.env.Path || process.env.path || '', +): string { + return pathValue + .split(path.delimiter) + .filter((dir) => dir && !hasGitNexusLauncher(dir)) + .join(path.delimiter); +} + +/** A full env copy with PATH replaced by `pathValue` and all case variants of the key removed. */ +export function envWithPath(pathValue: string): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = { ...process.env }; + for (const key of Object.keys(env)) { + if (key.toLowerCase() === 'path') delete env[key]; + } + env.PATH = pathValue; + return env; +} + +/** + * Create a temp dir holding a runnable `gitnexus` launcher and return a PATH that + * puts it first (with all other gitnexus launchers scrubbed). Caller must invoke + * cleanup() to remove the temp dir. + */ +export function createGitNexusPathEntry(): { pathValue: string; cleanup: () => void } { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-path-')); + const launcher = path.join(binDir, process.platform === 'win32' ? 'gitnexus.cmd' : 'gitnexus'); + fs.writeFileSync( + launcher, + process.platform === 'win32' ? '@echo off\r\nexit /b 0\r\n' : '#!/bin/sh\nexit 0\n', + ); + if (process.platform !== 'win32') fs.chmodSync(launcher, 0o755); + + return { + pathValue: [binDir, pathWithoutGitNexus()].filter(Boolean).join(path.delimiter), + cleanup: () => fs.rmSync(binDir, { recursive: true, force: true }), + }; +}