From 1941caf7bc5d56134c54361452b596e708536dcc Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 30 Oct 2026 07:27:10 +0000 Subject: [PATCH] fix(add-lang): the extraction check finds codegraph's npm shim on Windows, and takes CODEGRAPH_BIN (#3047) scripts/add-lang/verify-extraction.mjs ran execFileSync('codegraph', ...). On Windows, npm installs codegraph as a .cmd shim (so do npm link and local-install.sh), and Node neither finds a .cmd by bare name nor runs one without a shell. The check always died with "spawnSync codegraph ENOENT" (exit 2), and /add-lang's bench.sh read that as broken extraction and skipped its A/B. The script now asks, in order: - CODEGRAPH_BIN, when set. A .js, .mjs or .cjs path runs with the current node; anything else runs directly (a .cmd or .bat through cmd.exe). - On Windows, the codegraph Windows itself would pick: the first PATH folder holding codegraph under one of PATHEXT's runnable extensions. A .cmd or .bat runs through cmd.exe (shell: true) as one command string. Each token is quoted for the C runtime and caret-escaped for cmd, so spaces, &, parentheses, a %NAME% and a trailing backslash in the repo path arrive intact. An .exe or .com needs no shell. - Otherwise codegraph by bare name, exactly as before. On macOS and Linux without CODEGRAPH_BIN, the call and the output are unchanged. bench.sh takes CODEGRAPH_BIN too. It makes a path absolute (indexing runs from inside the repo), exports it so verify-extraction.mjs reads the index with the same build, and runs its own --version and init calls with it. Without it, bench.sh behaves exactly as before. The A/B's MCP server stays run-all.sh's own CG_BIN. Validation (Windows 11, Node 22.22.0): - New __tests__/add-lang-verify-extraction.test.ts covers CODEGRAPH_BIN with a .js stand-in (all platforms), a .cmd shim on PATH with an awkward repo path (Windows only), and the bare-name call (macOS and Linux only). With main's script both tests that run here fail ("could not read codegraph status"); with the fix, 2 pass and 1 is skipped. - The fixture from PR 1246's review (a codegraph.cmd on PATH): main exits 2 with ENOENT; the fix reads the status. - 13 scenarios with npm's own generated shims, and a real index in a folder named "lua repo & co (x86) 100%": main fails every one that needs the shim or CODEGRAPH_BIN, and the fix passes them with the exact argv reaching codegraph. With no codegraph anywhere, both exit 2 the same way. - bench.sh with the A/B stubbed: with npm link's shims on PATH, main's copy skips the A/B (verify exit 2) and the fix reaches it (PASS). CODEGRAPH_BIN as an absolute or a relative path reaches it too, and a missing one stops with a clear message. - With process.platform reported as linux or darwin, main's script and the new one make the same execFileSync call and print the same output. Raised by PR 1246 (@bslatner). Co-Authored-By: Claude Opus 5.5 --- __tests__/add-lang-verify-extraction.test.ts | 97 ++++++++++++++++++++ scripts/add-lang/bench.sh | 23 ++++- scripts/add-lang/verify-extraction.mjs | 51 +++++++++- 3 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 __tests__/add-lang-verify-extraction.test.ts diff --git a/__tests__/add-lang-verify-extraction.test.ts b/__tests__/add-lang-verify-extraction.test.ts new file mode 100644 index 000000000..2e3f38a3c --- /dev/null +++ b/__tests__/add-lang-verify-extraction.test.ts @@ -0,0 +1,97 @@ +/** + * scripts/add-lang/verify-extraction.mjs reads `codegraph status --json` from + * the codegraph that built an index. It used to call `codegraph` by bare name, + * which never works on Windows: npm installs codegraph there as a .cmd shim, + * and Node can neither find a .cmd by bare name nor run one without a shell. + * /add-lang's bench.sh then skipped its A/B as if extraction had failed. + * + * A stand-in for codegraph prints a healthy status and records the arguments + * it received, so each test can check what reached it. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +const SCRIPT = path.resolve('scripts/add-lang/verify-extraction.mjs'); +const roots: string[] = []; + +const FAKE_CODEGRAPH = ` +const fs = require('fs'); +fs.writeFileSync(process.env.CG_TEST_ARGV, JSON.stringify(process.argv.slice(2))); +console.log(JSON.stringify({ + initialized: true, backend: 'node-sqlite', fileCount: 2, nodeCount: 9, edgeCount: 7, + languages: ['lua'], nodesByKind: { file: 2, function: 5, variable: 2 }, +})); +`; + +function kit() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-verify-extraction-')); + roots.push(root); + const fake = path.join(root, 'fake-codegraph.cjs'); + fs.writeFileSync(fake, FAKE_CODEGRAPH); + // A folder name like npm's prefix under "Program Files (x86)". + const bin = path.join(root, 'bin (x86)'); + fs.mkdirSync(bin); + return { root, fake, bin, argv: path.join(root, 'argv.json') }; +} + +// The script's environment, with PATH limited to `dirs` plus what node needs, +// so no codegraph installed on the machine running the tests can answer. +function envWith(k: ReturnType, dirs: string[], extra: NodeJS.ProcessEnv = {}) { + const env: NodeJS.ProcessEnv = {}; + for (const [key, value] of Object.entries(process.env)) { + if (key.toUpperCase() !== 'PATH' && key !== 'CODEGRAPH_BIN') env[key] = value; + } + const system = process.platform === 'win32' ? [path.join(process.env.SystemRoot ?? 'C:\\Windows', 'System32')] : ['/usr/bin', '/bin']; + env.PATH = [...dirs, path.dirname(process.execPath), ...system].join(path.delimiter); + env.CG_TEST_ARGV = k.argv; + return { ...env, ...extra }; +} + +function verify(k: ReturnType, repo: string, env: NodeJS.ProcessEnv) { + const r = spawnSync(process.execPath, [SCRIPT, repo, 'lua'], { encoding: 'utf8', timeout: 60_000, env }); + const received = fs.existsSync(k.argv) ? JSON.parse(fs.readFileSync(k.argv, 'utf8')) : null; + return { status: r.status, output: `${r.stdout}${r.stderr}`, received }; +} + +afterEach(() => { + for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true }); +}); + +describe('add-lang verify-extraction: which codegraph it asks', () => { + it('reads the status through CODEGRAPH_BIN, running a .js path with node', () => { + const k = kit(); + const repo = path.join(k.root, 'some repo'); + const r = verify(k, repo, envWith(k, [], { CODEGRAPH_BIN: k.fake })); + expect(r.output).toContain('RESULT: PASS'); + expect(r.status).toBe(0); + expect(r.received).toEqual(['status', repo, '--json']); + }); + + it.runIf(process.platform === 'win32')('finds the npm .cmd shim on PATH and passes an awkward repo path through cmd.exe intact', () => { + const k = kit(); + // The shape of npm's shim: cmd.exe runs it and %* hands the arguments on. + fs.writeFileSync(path.join(k.bin, 'codegraph.cmd'), '@ECHO off\r\nSETLOCAL\r\nSET "_prog=node"\r\n"%_prog%" "%~dp0..\\fake-codegraph.cjs" %*\r\n'); + // Spaces, & and parentheses, a defined %NAME%, a lone %, and a trailing + // backslash that must not escape the closing quote. + const repo = path.join(k.root, 'repo & co (x86) %CG_TEST_PCT% 50%') + '\\'; + const r = verify(k, repo, envWith(k, [k.bin], { CG_TEST_PCT: 'expanded' })); + expect(r.output).toContain('RESULT: PASS'); + expect(r.status).toBe(0); + expect(r.received).toEqual(['status', repo, '--json']); + }); + + it.runIf(process.platform !== 'win32')('runs the codegraph on PATH directly, as before', () => { + const k = kit(); + const shim = path.join(k.bin, 'codegraph'); + fs.writeFileSync(shim, `#!/bin/sh\nexec "${process.execPath}" "${k.fake}" "$@"\n`); + fs.chmodSync(shim, 0o755); + const repo = path.join(k.root, 'some repo'); + const r = verify(k, repo, envWith(k, [k.bin])); + expect(r.output).toContain('RESULT: PASS'); + expect(r.status).toBe(0); + expect(r.received).toEqual(['status', repo, '--json']); + }); +}); diff --git a/scripts/add-lang/bench.sh b/scripts/add-lang/bench.sh index 172fe4064..08dcd2ade 100755 --- a/scripts/add-lang/bench.sh +++ b/scripts/add-lang/bench.sh @@ -10,6 +10,10 @@ # # Usage: bench.sh "" [headless|tmux|all] # Env: CORPUS corpus dir (default /tmp/codegraph-corpus, shared with agent-eval) +# CODEGRAPH_BIN index and verify with this codegraph instead of the one on +# PATH: a binary, or a build's dist/bin/codegraph.js (run with +# node). Exported, so verify-extraction.mjs uses it too. The A/B's +# MCP server is run-all.sh's own CG_BIN (default: codegraph on PATH). set -uo pipefail LANG_TOKEN="${1:?usage: bench.sh \"\" [mode]}" @@ -23,10 +27,25 @@ AGENT_EVAL="$(cd "$HARNESS/../agent-eval" && pwd)" CORPUS="${CORPUS:-/tmp/codegraph-corpus}" REPO="$CORPUS/$NAME" -command -v codegraph >/dev/null || { echo "no codegraph on PATH (build + ./scripts/local-install.sh first)"; exit 1; } +if [ -n "${CODEGRAPH_BIN:-}" ]; then + # A path is made absolute, since indexing runs from inside the repo. + if [ -e "$CODEGRAPH_BIN" ]; then + CODEGRAPH_BIN="$(cd "$(dirname "$CODEGRAPH_BIN")" && pwd)/$(basename "$CODEGRAPH_BIN")" + else + command -v "$CODEGRAPH_BIN" >/dev/null || { echo "CODEGRAPH_BIN not found: $CODEGRAPH_BIN"; exit 1; } + fi + export CODEGRAPH_BIN + # Stands in for the PATH codegraph in this script's own calls. + case "$CODEGRAPH_BIN" in + *.js|*.mjs|*.cjs) codegraph() { node "$CODEGRAPH_BIN" "$@"; } ;; + *) codegraph() { "$CODEGRAPH_BIN" "$@"; } ;; + esac +else + command -v codegraph >/dev/null || { echo "no codegraph on PATH (build + ./scripts/local-install.sh first)"; exit 1; } +fi echo "==================== add-lang bench: $NAME ($LANG_TOKEN) ====================" -echo "codegraph: $(command -v codegraph) -> $(codegraph --version 2>/dev/null || echo '?')" +echo "codegraph: ${CODEGRAPH_BIN:-$(command -v codegraph)} -> $(codegraph --version 2>/dev/null || echo '?')" # 1. Ensure the repo (shallow clone, reuse if present). mkdir -p "$CORPUS" diff --git a/scripts/add-lang/verify-extraction.mjs b/scripts/add-lang/verify-extraction.mjs index bdb443e25..b7e68ef98 100755 --- a/scripts/add-lang/verify-extraction.mjs +++ b/scripts/add-lang/verify-extraction.mjs @@ -5,11 +5,15 @@ // // Usage: node scripts/add-lang/verify-extraction.mjs // Reads `codegraph status --json` using whatever codegraph is on PATH, -// so it reflects the binary that built the index. +// so it reflects the binary that built the index. Set CODEGRAPH_BIN to read it +// with another one: a binary, or a build's dist/bin/codegraph.js, which runs +// with this node. bench.sh exports CODEGRAPH_BIN when it is given one. // // Exit codes: 0 = pass or soft-warn, 1 = critical fail, 2 = could not run. import { execFileSync } from 'node:child_process'; +import { existsSync } from 'node:fs'; +import path from 'node:path'; const [repo, lang] = process.argv.slice(2); if (!repo || !lang) { @@ -17,9 +21,52 @@ if (!repo || !lang) { process.exit(2); } +// On Windows, npm installs codegraph as a .cmd shim (`npm link` too), and +// execFileSync can neither find a .cmd by bare name nor run one without a +// shell. So look where Windows would: the first PATH folder holding codegraph +// under one of PATHEXT's runnable extensions. Only a .cmd or .bat needs the +// shell; an .exe or .com is found by the plain call below on its own. +function findWindowsShim() { + const runnable = ['.com', '.exe', '.bat', '.cmd']; + const exts = (process.env.PATHEXT || runnable.join(';')).toLowerCase().split(';').filter((e) => runnable.includes(e)); + for (const dir of (process.env.PATH || '').split(path.delimiter)) { + if (!dir) continue; + for (const ext of exts) { + const file = path.join(dir, `codegraph${ext}`); + if (existsSync(file)) return ext === '.bat' || ext === '.cmd' ? file : null; + } + } + return null; +} + +// One token of a command line for cmd.exe, which hands it on to node.exe +// through the shim's %*. Double quotes keep it one argument for the C runtime +// that splits node.exe's command line, so a backslash run before the closing +// quote is doubled (or it would escape that quote). Then every cmd +// metacharacter, the quotes included, gets a caret, so cmd neither splits the +// token nor expands a %NAME% inside it. A Windows path can't contain a double +// quote, so a token with one is refused rather than mangled. +function quoteForCmd(token) { + if (/["\r\n]/.test(token)) throw new Error(`cannot pass ${JSON.stringify(token)} through cmd.exe`); + return `"${token.replace(/(\\+)$/, '$1$1')}"`.replace(/[()[\]%!^"`<>&|;, *?]/g, '^$&'); +} + +function runCodegraph(args) { + const options = { encoding: 'utf8' }; + const bin = process.env.CODEGRAPH_BIN || (process.platform === 'win32' ? findWindowsShim() : null); + if (!bin) return execFileSync('codegraph', args, options); + if (/\.[cm]?js$/i.test(bin)) return execFileSync(process.execPath, [bin, ...args], options); + if (process.platform === 'win32' && /\.(bat|cmd)$/i.test(bin)) { + // A single command string with no args array: Node hands it to cmd.exe + // unchanged, so the quoting above is all there is. + return execFileSync([bin, ...args].map(quoteForCmd).join(' '), { ...options, shell: true }); + } + return execFileSync(bin, args, options); +} + let status; try { - const out = execFileSync('codegraph', ['status', repo, '--json'], { encoding: 'utf8' }); + const out = runCodegraph(['status', repo, '--json']); status = JSON.parse(out); } catch (e) { console.error(`[verify] could not read codegraph status for ${repo}: ${e.message}`);