From 2100c327aab755e76e0800bd3907d88a190df758 Mon Sep 17 00:00:00 2001 From: zodyp Date: Sat, 19 Sep 2026 22:51:17 -0300 Subject: [PATCH] fix(gates): the new-code gates are unrunnable on Windows, so nobody runs them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `newCodeMode.withBaseWorktree` lints the merge-base in a throwaway git worktree and links the repo's node_modules into it. The link was a "dir" symlink, which on Windows needs SeCreateSymbolicLinkPrivilege — an elevated shell or Developer Mode. Without it `fs.symlinkSync` throws EPERM and the gate dies *before comparing anything*: Error: EPERM: operation not permitted, symlink '...\node_modules' -> '...' at withBaseWorktree (scripts/check/newCodeMode.mjs:79:31) So check:complexity-ratchets, check:dead-code and check:file-size cannot be run locally at all on a Windows checkout, and a contributor there learns about a regression only when CI says so. That is not hypothetical: three separate agents shipped new-code regressions in this release line, each having run the gates locally and seen them "pass". A junction is the same thing for this purpose — a directory reparse point — and needs no privilege. Measured on this machine: symlink dir: FALHA -> EPERM junction : OK The link now goes through `linkNodeModules`, which picks "junction" on win32 and "dir" elsewhere, and turns a failure into a message that says what broke and why it matters rather than a bare errno. Proof it now runs end to end on Windows, against a base with source files in scope (the earlier short-circuit on "0 changed files" proves nothing): $ node scripts/check/check-complexity-ratchets.mjs --base-ref b581c39cfdcd [complexity] OK (código novo) — 7 violações nos arquivos tocados (base 7) [cognitive-complexity] OK (código novo) — 4 violações nos arquivos tocados (base 4) exit=0 tests/unit/newcode-gate-node-modules-link.test.ts covers both halves: the link resolves on whatever platform runs it (reading a file *through* it, since a link that exists but does not resolve leaves the linter blind), the failure message is actionable, and — statically — that win32 still picks "junction". The static assertion earns its place because CI runs on Linux and could never catch that regression behaviourally. Verification (isolated DATA_DIR/HOME/USERPROFILE/APPDATA): tests/unit/newcode-gate-node-modules-link.test.ts 3 pass / 0 fail eslint --max-warnings=0 exit 0 tsc -p tsconfig.typecheck-core.json exit 0, 0 errors Co-Authored-By: Claude Opus 5 --- scripts/check/newCodeMode.mjs | 37 ++++++++- .../newcode-gate-node-modules-link.test.ts | 75 +++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 tests/unit/newcode-gate-node-modules-link.test.ts diff --git a/scripts/check/newCodeMode.mjs b/scripts/check/newCodeMode.mjs index ab118a632d0..17b9ea50df6 100644 --- a/scripts/check/newCodeMode.mjs +++ b/scripts/check/newCodeMode.mjs @@ -70,13 +70,48 @@ export function filterScope(paths, { dirs, exts, excludePrefixes = [] }) { * Materialize `sha` in a throwaway worktree with node_modules linked from ROOT, run `fn(dir)`, * always tear it down. Never touches the caller's tree or index (no stash, no checkout). */ +/** + + * Link the repo's node_modules into the throwaway base worktree. + + * + + * A "dir" symlink needs SeCreateSymbolicLinkPrivilege on Windows — that is, an elevated + + * shell or Developer Mode. Without it `fs.symlinkSync` throws EPERM and every new-code + + * gate dies before it compares anything, so a contributor on Windows cannot run + + * check:complexity-ratchets, check:dead-code or check:file-size locally at all and only + + * learns about a regression from CI. A **junction** is the same thing for our purpose — + + * a directory reparse point — and needs no privilege. Junctions are Windows-only and + + * require an absolute target, which `nm` already is. + + */ + +export function linkNodeModules(nm, target) { + const type = process.platform === "win32" ? "junction" : "dir"; + + try { + fs.symlinkSync(nm, target, type); + } catch (err) { + throw new Error( + `could not link node_modules into the base worktree (${type}): ${err.message} +` + "The new-code gates need it to lint the base revision." + ); + } +} + export function withBaseWorktree(sha, fn) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-newcode-base-")); fs.rmdirSync(dir); // git worktree add wants a non-existent path git(["worktree", "add", "--detach", "--quiet", dir, sha]); try { const nm = path.join(ROOT, "node_modules"); - if (fs.existsSync(nm)) fs.symlinkSync(nm, path.join(dir, "node_modules"), "dir"); + if (fs.existsSync(nm)) linkNodeModules(nm, path.join(dir, "node_modules")); return fn(dir); } finally { try { diff --git a/tests/unit/newcode-gate-node-modules-link.test.ts b/tests/unit/newcode-gate-node-modules-link.test.ts new file mode 100644 index 00000000000..d7956ceba17 --- /dev/null +++ b/tests/unit/newcode-gate-node-modules-link.test.ts @@ -0,0 +1,75 @@ +/** + * The new-code gates (complexity-ratchets, dead-code, file-size) lint the merge-base in a + * throwaway git worktree, and link the repo's `node_modules` into it so the linter can + * resolve imports. + * + * That link used to be a `"dir"` symlink, which on Windows needs + * SeCreateSymbolicLinkPrivilege — an elevated shell or Developer Mode. Without it + * `fs.symlinkSync` throws `EPERM` and the gate dies *before comparing anything*, so a + * contributor on Windows cannot run those gates locally at all and only finds out about a + * regression when CI says so. Three separate agents shipped new-code regressions in this + * release line for exactly that reason, each believing their local run had passed. + * + * A **junction** is the same thing for this purpose and needs no privilege. So: + * - the behavioural test proves the link works on whatever platform is running it; + * - the static test guards the Windows branch, because CI runs on Linux and would + * otherwise never notice if `"junction"` were dropped again. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { linkNodeModules } from "../../scripts/check/newCodeMode.mjs"; + +test("links node_modules into a base worktree on this platform", () => { + const src = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-link-src-")); + const box = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-link-dst-")); + const target = path.join(box, "node_modules"); + fs.writeFileSync(path.join(src, "marker.txt"), "resolved", "utf8"); + + try { + linkNodeModules(src, target); + // Reading through the link is the real assertion: a link that exists but does not + // resolve would leave the linter unable to find any dependency. + assert.equal(fs.readFileSync(path.join(target, "marker.txt"), "utf8"), "resolved"); + } finally { + fs.rmSync(box, { recursive: true, force: true }); + fs.rmSync(src, { recursive: true, force: true }); + } +}); + +test("reports an actionable error instead of a bare EPERM", () => { + const src = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-link-src-")); + try { + // Target inside a directory that does not exist — the link cannot be created. + assert.throws( + () => linkNodeModules(src, path.join(src, "missing-parent", "node_modules")), + (err: unknown) => { + const message = err instanceof Error ? err.message : String(err); + assert.match(message, /could not link node_modules/); + assert.match(message, /new-code gates/); + return true; + } + ); + } finally { + fs.rmSync(src, { recursive: true, force: true }); + } +}); + +test("Windows uses a junction, which needs no elevation", () => { + const source = readFileSync( + fileURLToPath(new URL("../../scripts/check/newCodeMode.mjs", import.meta.url)), + "utf8" + ); + + assert.match( + source, + /process\.platform === "win32" \? "junction" : "dir"/, + 'newCodeMode must pick "junction" on win32: a "dir" symlink throws EPERM without ' + + "Developer Mode, which silently disables every new-code gate for Windows contributors. " + + "CI runs on Linux and cannot catch this regression behaviourally." + ); +});