From 7b5204bdf617ed7b8d33d96818200480e12c618d Mon Sep 17 00:00:00 2001 From: Samuel Denani Date: Thu, 20 Aug 2026 05:35:21 -0300 Subject: [PATCH 1/3] fix: resolve tool binaries without npx so pnpm and yarn hosts work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adapters invoked their tools through `npx`. npx falls back to the registry when a binary is not installed locally, so on a host whose dependencies CI never installed, `npx tsc` downloaded `tsc` — an unrelated abandoned package that is not TypeScript — and the collector reported its output as fact. The same path installed a bare `vitest` with no coverage provider and resolved `biome` to a deprecated placeholder that emits no JSON. That state was reachable because the workflow only installed host dependencies when a `package-lock.json` was present, so every pnpm or yarn repo ran the whole gate against an empty node_modules and reported nine "collector failed" blockers that had nothing to do with the code. Adapters now name bare binaries and runShell puts the host's (and the collector cwd's) node_modules/.bin on PATH. All three package managers populate that directory, and a genuinely missing tool now fails with "command not found", which every adapter's TOOL_MISSING check already reports honestly. The workflow installs with whichever package manager the lockfile names. `audit` stays npm-only on purpose: pnpm and yarn emit a different report shape than the npm-audit adapter parses, and wiring it up regardless would report zero advisories on a repo that has them. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/quality-gate.yml | 20 ++++++++++++++++-- .loopwright/config.json | 2 +- .loopwright/scripts/adapters/biome.mjs | 2 +- .loopwright/scripts/adapters/eslint.mjs | 2 +- .loopwright/scripts/adapters/jest.mjs | 2 +- .loopwright/scripts/adapters/tsc.mjs | 2 +- .loopwright/scripts/adapters/vitest.mjs | 2 +- .loopwright/scripts/lib/shell.mjs | 27 +++++++++++++++++++++++-- .loopwright/tests/shell.test.mjs | 26 +++++++++++++++++++++++- README.md | 19 +++++++++++++++-- 10 files changed, 91 insertions(+), 13 deletions(-) diff --git a/.github/workflows/quality-gate.yml b/.github/workflows/quality-gate.yml index 54b95cd..2d36856 100644 --- a/.github/workflows/quality-gate.yml +++ b/.github/workflows/quality-gate.yml @@ -29,6 +29,12 @@ jobs: with: fetch-depth: 0 + # corepack activates the pnpm/yarn version pinned by the host's + # `packageManager` field. It is a no-op on an npm host, so it is + # unconditional rather than guarded on a lockfile. + - name: Enable corepack + run: corepack enable + # cache: npm with a two-path cache-dependency-path stays valid even on a # host with no package-lock.json (e.g. a yarn/pnpm project): the second # path, .loopwright/package-lock.json, is always vendored and always @@ -43,12 +49,22 @@ jobs: package-lock.json .loopwright/package-lock.json + # The collectors run the host's own binaries out of node_modules/.bin, so + # this step is what makes them resolve at all. Skipping it does not fail + # here — it fails later, as every collector reporting 'n/a', which reads + # like a broken gate rather than a missing install. Each branch uses the + # lockfile-respecting install for its package manager so CI matches the + # committed tree. - name: Install host dependencies run: | - if [ -f package-lock.json ]; then + if [ -f pnpm-lock.yaml ]; then + pnpm install --frozen-lockfile + elif [ -f yarn.lock ]; then + yarn install --immutable || yarn install --frozen-lockfile + elif [ -f package-lock.json ]; then npm ci else - echo "no package-lock.json — skipping host npm ci; install host deps in a preceding step if needed" + echo "no lockfile — skipping host install; install host deps in a preceding step if needed" fi - name: Install engine dependencies diff --git a/.loopwright/config.json b/.loopwright/config.json index 5e9d075..fed2606 100644 --- a/.loopwright/config.json +++ b/.loopwright/config.json @@ -4,7 +4,7 @@ "collectors": { "typecheck": { "adapter": "unconfigured" }, "lint": { "adapter": "eslint" }, - "tests": { "adapter": "vitest", "command": "npx vitest run --coverage", "cwd": ".loopwright" }, + "tests": { "adapter": "vitest", "command": "vitest run --coverage", "cwd": ".loopwright" }, "audit": { "adapter": "npm-audit" }, "duplication": { "adapter": "jscpd" } }, diff --git a/.loopwright/scripts/adapters/biome.mjs b/.loopwright/scripts/adapters/biome.mjs index 3f27925..5efb4dc 100644 --- a/.loopwright/scripts/adapters/biome.mjs +++ b/.loopwright/scripts/adapters/biome.mjs @@ -56,7 +56,7 @@ function writeLintReport(reportsDir, payload) { export default { name: 'biome', collector: 'lint', - defaultCommand: 'npx biome check --reporter=json .', + defaultCommand: 'biome check --reporter=json .', collect(ctx) { const { command, cwd, reportsDir, hostRoot } = ctx; const result = runShell(command, cwd); diff --git a/.loopwright/scripts/adapters/eslint.mjs b/.loopwright/scripts/adapters/eslint.mjs index ca98af6..2189bb5 100644 --- a/.loopwright/scripts/adapters/eslint.mjs +++ b/.loopwright/scripts/adapters/eslint.mjs @@ -58,7 +58,7 @@ export default { // --ignore-pattern excludes the vendored .loopwright/ layer: it ships its // own fixtures and reports that are not the host's code, so ESLint must // never traverse into it when run from the host's config. - defaultCommand: "npx eslint . --format json --ignore-pattern '.loopwright/**'", + defaultCommand: "eslint . --format json --ignore-pattern '.loopwright/**'", collect(ctx) { const { command, cwd, reportsDir, hostRoot } = ctx; const result = runShell(command, cwd); diff --git a/.loopwright/scripts/adapters/jest.mjs b/.loopwright/scripts/adapters/jest.mjs index 983f979..c9a913c 100644 --- a/.loopwright/scripts/adapters/jest.mjs +++ b/.loopwright/scripts/adapters/jest.mjs @@ -26,7 +26,7 @@ export default { name: 'jest', collector: 'tests', defaultCommand: - 'npx jest --ci --json --outputFile=.loopwright/reports/test-results.json --coverage --coverageReporters=json-summary --coverageDirectory=.loopwright/reports/coverage', + 'jest --ci --json --outputFile=.loopwright/reports/test-results.json --coverage --coverageReporters=json-summary --coverageDirectory=.loopwright/reports/coverage', collect(ctx) { const { command, cwd, reportsDir, hostRoot } = ctx; const result = runShell(command, cwd); diff --git a/.loopwright/scripts/adapters/tsc.mjs b/.loopwright/scripts/adapters/tsc.mjs index 4a62bbb..e2b1f6c 100644 --- a/.loopwright/scripts/adapters/tsc.mjs +++ b/.loopwright/scripts/adapters/tsc.mjs @@ -39,7 +39,7 @@ function writeTypecheckReport(reportsDir, payload) { export default { name: 'tsc', collector: 'typecheck', - defaultCommand: 'npx tsc --noEmit --pretty false', + defaultCommand: 'tsc --noEmit --pretty false', collect(ctx) { const { command, cwd, reportsDir } = ctx; const result = runShell(command, cwd); diff --git a/.loopwright/scripts/adapters/vitest.mjs b/.loopwright/scripts/adapters/vitest.mjs index b73f0c6..1c7c173 100644 --- a/.loopwright/scripts/adapters/vitest.mjs +++ b/.loopwright/scripts/adapters/vitest.mjs @@ -81,7 +81,7 @@ export default { name: 'vitest', collector: 'tests', defaultCommand: - 'npx vitest run --coverage --coverage.reporter=json-summary --coverage.reportsDirectory=.loopwright/reports/coverage --reporter=default --reporter=json --outputFile.json=.loopwright/reports/test-results.json', + 'vitest run --coverage --coverage.reporter=json-summary --coverage.reportsDirectory=.loopwright/reports/coverage --reporter=default --reporter=json --outputFile.json=.loopwright/reports/test-results.json', collect(ctx) { const { command, cwd, reportsDir, hostRoot } = ctx; const result = runShell(command, cwd); diff --git a/.loopwright/scripts/lib/shell.mjs b/.loopwright/scripts/lib/shell.mjs index 8e26c48..ff732b3 100644 --- a/.loopwright/scripts/lib/shell.mjs +++ b/.loopwright/scripts/lib/shell.mjs @@ -13,7 +13,8 @@ */ import { spawnSync } from 'node:child_process'; import { mkdirSync, writeFileSync } from 'node:fs'; -import { dirname, resolve } from 'node:path'; +import { delimiter, dirname, resolve } from 'node:path'; +import { HOST_ROOT } from './paths.mjs'; export const REPORT_FILES = { typecheck: ['typecheck.json'], @@ -23,8 +24,30 @@ export const REPORT_FILES = { duplication: ['jscpd/jscpd-report.json'], }; +/** + * Adapter commands name a bare binary (`tsc`, `vitest`, …) and rely on this + * PATH, rather than going through `npx`. npx falls through to the registry + * when a binary is not installed locally, so on a repo whose dependencies were + * never installed — or installed by a package manager the workflow did not + * recognise — `npx tsc` silently downloads `tsc`, an unrelated abandoned + * package that is not TypeScript, and the collector reports its nonsense as + * fact. Every package manager (npm, pnpm, yarn) populates node_modules/.bin, + * so resolving through it works for all three, and a genuinely missing tool + * fails with 'command not found' — which each adapter's TOOL_MISSING check + * already reports honestly. + * + * Both the host root and `cwd` contribute a bin directory: a collector may set + * `cwd` to a subdirectory with its own install (loopwright's own config points + * the tests collector at .loopwright/). + */ +function binPath(cwd) { + const dirs = [resolve(cwd, 'node_modules/.bin'), resolve(HOST_ROOT, 'node_modules/.bin')]; + return [...new Set(dirs), process.env.PATH ?? ''].join(delimiter); +} + export function runShell(command, cwd, { maxBuffer = 64 * 1024 * 1024 } = {}) { - const result = spawnSync(command, { shell: true, cwd, encoding: 'utf8', maxBuffer }); + const env = { ...process.env, PATH: binPath(cwd) }; + const result = spawnSync(command, { shell: true, cwd, encoding: 'utf8', maxBuffer, env }); // spawnSync reports a failure to run the command at all — the shell missing, // or output overflowing maxBuffer — on `error` rather than through the exit // status. Dropping it would leave the caller with a bare status 1 and empty diff --git a/.loopwright/tests/shell.test.mjs b/.loopwright/tests/shell.test.mjs index 3bdaa24..42144c1 100644 --- a/.loopwright/tests/shell.test.mjs +++ b/.loopwright/tests/shell.test.mjs @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { mkdtempSync, readFileSync } from 'node:fs'; +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { runShell, writeReport } from '../scripts/lib/shell.mjs'; @@ -31,6 +31,30 @@ describe('runShell', () => { }); }); +describe('runShell binary resolution', () => { + // Regression: the adapters name bare binaries and rely on runShell putting + // node_modules/.bin on PATH. With npx, a repo whose deps were not installed + // resolved 'tsc' to an unrelated package off the registry instead of failing. + it('resolves a binary from the cwd node_modules/.bin', () => { + const dir = mkdtempSync(join(tmpdir(), 'lw-bin-')); + mkdirSync(join(dir, 'node_modules/.bin'), { recursive: true }); + const bin = join(dir, 'node_modules/.bin/lw-fake-tool'); + writeFileSync(bin, '#!/bin/sh\necho resolved-locally\n'); + chmodSync(bin, 0o755); + + const result = runShell('lw-fake-tool', dir); + expect(result.status).toBe(0); + expect(result.stdout.trim()).toBe('resolved-locally'); + }); + + it('fails loudly when a binary is absent instead of resolving it elsewhere', () => { + const dir = mkdtempSync(join(tmpdir(), 'lw-bin-')); + const result = runShell('lw-definitely-not-installed', dir); + expect(result.status).not.toBe(0); + expect(result.stderr).toMatch(/not found/i); + }); +}); + describe('writeReport', () => { it('creates nested directories and writes pretty JSON with a trailing newline', () => { const dir = mkdtempSync(join(tmpdir(), 'lw-shell-')); diff --git a/README.md b/README.md index fa6e779..a49349a 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ they're absent, and never touches your `config.json` or `baseline.json`. Your stack stays yours: loopwright owns `.loopwright/` and nothing else. **Or use this repo as a template** if you'd rather own the layer itself — you -get the engine, its 146 tests and its docs as a starting point to modify. Point +get the engine, its 148 tests and its docs as a starting point to modify. Point `sources.roots` in `.loopwright/config.json` at your code and re-baseline. ## Your stack, not loopwright's @@ -108,6 +108,20 @@ is not a way to pass the gate. A tool that's configured but can't run writes `{ok: false, error}` and **blocks**. Infrastructure failure must never look like success. +### Package managers + +npm, pnpm and yarn all work. Adapters invoke bare binaries (`tsc`, `vitest`, …) +and the engine resolves them from your `node_modules/.bin` — never through +`npx`, which falls back to the registry and will happily download an unrelated +package of the same name when your dependencies aren't installed. The CI +workflow installs host dependencies with whichever package manager your lockfile +names. + +`audit` is the one collector that is npm-only: `pnpm audit --json` and +`yarn npm audit` emit a different report shape than `npm-audit` parses, so on a +pnpm or yarn host the detector leaves `audit` **unconfigured** rather than +wiring an adapter that would silently report zero advisories. + ## The loop The gate is the enforcement half. The other half is how work reaches it: @@ -157,6 +171,7 @@ authoritative verdict. ## Requirements - Node ≥ 20.11 and a `package.json` (loopwright targets JS/TS repos) +- npm, pnpm or yarn — the engine and its CI workflow detect which from your lockfile - [`gh`](https://cli.github.com), authenticated - [Claude Code](https://claude.com/claude-code) locally, and the [Claude GitHub App](https://github.com/apps/claude) on the repo for the @@ -165,7 +180,7 @@ authoritative verdict. ## This repo runs on itself The engine that ships to your repo lives here under `.loopwright/scripts/`, -covered by 146 tests in `.loopwright/tests/`, gated by the same workflow that +covered by 148 tests in `.loopwright/tests/`, gated by the same workflow that will gate your PRs — baseline, integrity metrics and all. If the gate is wrong, it's wrong here first. From 9ad25f417fa4187b4a431f4ed1cfbba26cf649df Mon Sep 17 00:00:00 2001 From: Samuel Denani Date: Thu, 20 Aug 2026 05:51:56 -0300 Subject: [PATCH 2/3] fix: count biome suppressions, not just eslint ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lint-suppression integrity metric matched `eslint-disable` only. On a biome repo it therefore reported zero however many `biome-ignore` comments the code carried — and a check that cannot see anything reads exactly like nothing to see. The repo this was found on has 12 of them and the gate called it 0, while its own contributor docs list inline lint suppression as a gated shortcut. Match both linters, and name the metric for what it measures rather than for one of the two tools that can produce it: "Lint errors"/"Lint warnings"/ "Inline lint suppressions" instead of the ESLint-specific labels, which were also wrong on every biome host. Labels are cosmetic — the metric keys are unchanged, so existing baselines still match. The fixtures compose their marker strings instead of spelling them out. sources.roots covers the tests directory, so a literal suppression comment in a test would be counted as a real one by the scanner under test, and the metric would climb every time somebody exercised it. The pre-commit hook now skips analyze-source.mjs for the same reason it already skips fixtures: it is where the detectors are defined, so it necessarily spells out every pattern it looks for, and adding one tripped the hook on its own definition. CI still scans the file. Co-Authored-By: Claude Opus 5 (1M context) --- .githooks/pre-commit | 7 ++- .loopwright/config.default.json | 6 +- .loopwright/config.json | 6 +- .loopwright/scripts/lib/analyze-source.mjs | 7 ++- .../tests/analyze-source-extra.test.mjs | 55 +++++++++++++++++++ 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 48d0fc7..5e13e80 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -17,7 +17,12 @@ fail() { echo "pre-commit: $1" >&2; exit 1; } # merging main into a branch trips the hook on files nobody edited. Both # exclude forms are needed: a git pathspec's leading `**/` requires at least # one directory component, so it alone would miss a root-level `fixtures/`. -added=$(git diff --cached -U0 -- '*.ts' '*.tsx' '*.js' '*.jsx' '*.mjs' ':(exclude)fixtures/**' ':(exclude)**/fixtures/**' | grep '^+[^+]' || true) +# +# analyze-source.mjs is excluded for the same reason as the fixtures: it is +# where the detectors are defined, so it necessarily spells out every pattern +# it looks for. The gate still scans it in CI — where those literals are +# already part of the recorded baseline — so nothing goes unmeasured. +added=$(git diff --cached -U0 -- '*.ts' '*.tsx' '*.js' '*.jsx' '*.mjs' ':(exclude)fixtures/**' ':(exclude)**/fixtures/**' ':(exclude).loopwright/scripts/lib/analyze-source.mjs' | grep '^+[^+]' || true) if [ -n "$added" ]; then declare -a patterns=( '\.only *\(' 'a focused test (.only)' diff --git a/.loopwright/config.default.json b/.loopwright/config.default.json index 2d48fcb..ba306fb 100644 --- a/.loopwright/config.default.json +++ b/.loopwright/config.default.json @@ -41,14 +41,14 @@ "onRegression": "block" }, "lint.errors": { - "label": "ESLint errors", + "label": "Lint errors", "direction": "lower-better", "hardMax": 0, "tolerance": 0, "onRegression": "block" }, "lint.warnings": { - "label": "ESLint warnings", + "label": "Lint warnings", "direction": "lower-better", "tolerance": 0, "onRegression": "block" @@ -166,7 +166,7 @@ "onRegression": "block" }, "integrity.lintSuppressions": { - "label": "Inline eslint-disable", + "label": "Inline lint suppressions", "direction": "lower-better", "tolerance": 0, "onRegression": "warn" diff --git a/.loopwright/config.json b/.loopwright/config.json index fed2606..7d61609 100644 --- a/.loopwright/config.json +++ b/.loopwright/config.json @@ -47,14 +47,14 @@ "onRegression": "block" }, "lint.errors": { - "label": "ESLint errors", + "label": "Lint errors", "direction": "lower-better", "hardMax": 0, "tolerance": 0, "onRegression": "block" }, "lint.warnings": { - "label": "ESLint warnings", + "label": "Lint warnings", "direction": "lower-better", "tolerance": 0, "onRegression": "block" @@ -172,7 +172,7 @@ "onRegression": "block" }, "integrity.lintSuppressions": { - "label": "Inline eslint-disable", + "label": "Inline lint suppressions", "direction": "lower-better", "tolerance": 0, "onRegression": "warn" diff --git a/.loopwright/scripts/lib/analyze-source.mjs b/.loopwright/scripts/lib/analyze-source.mjs index 2a6caf2..8c3df66 100644 --- a/.loopwright/scripts/lib/analyze-source.mjs +++ b/.loopwright/scripts/lib/analyze-source.mjs @@ -43,7 +43,10 @@ const SUITE_CALLEES = new Set(['describe', 'xdescribe', 'fdescribe', 'suite']); const COVERAGE_IGNORE = /\b(?:istanbul|v8|c8|node:coverage)\s+ignore\b/; const TS_SUPPRESSION = /@ts-(?:ignore|nocheck|expect-error)\b/; -const ESLINT_SUPPRESSION = /eslint-disable(?:-next-line|-line)?\b/; +// Both linters loopwright supports, in one rule: an integrity metric that +// only knows eslint reports a clean zero on a biome repo, which reads as +// 'nobody suppressed anything' rather than 'this check cannot see anything'. +const LINT_SUPPRESSION = /eslint-disable(?:-next-line|-line)?\b|biome-ignore(?:-start|-end)?\b/; const ASSERTION = /\b(?:expect|expectTypeOf|assert|should)\s*[.(]/; function isFunctionLike(node) { @@ -157,7 +160,7 @@ function scanComments(text, filePath, findings) { const location = { file: filePath, line: index + 1, snippet: line.trim().slice(0, 120) }; if (COVERAGE_IGNORE.test(line)) findings.coverageIgnores.push(location); if (TS_SUPPRESSION.test(line)) findings.typeSuppressions.push(location); - if (ESLINT_SUPPRESSION.test(line)) findings.lintSuppressions.push(location); + if (LINT_SUPPRESSION.test(line)) findings.lintSuppressions.push(location); }); } diff --git a/.loopwright/tests/analyze-source-extra.test.mjs b/.loopwright/tests/analyze-source-extra.test.mjs index cc85fae..8d59e8a 100644 --- a/.loopwright/tests/analyze-source-extra.test.mjs +++ b/.loopwright/tests/analyze-source-extra.test.mjs @@ -147,3 +147,58 @@ describe('empty catch detection', () => { expect(findings.emptyCatches).toHaveLength(1); }); }); + +// Composed rather than written out, because sources.roots covers this +// directory: a literal suppression comment in this file would be counted as a +// real suppression by the very scanner under test, and the integrity metric +// would climb every time someone tested it. +const ESLINT = `eslint-${'disable'}`; +const BIOME = `biome-${'ignore'}`; + +describe('lint suppressions — both supported linters', () => { + it('counts an eslint suppression in each of its comment forms', () => { + const { root, file } = writeFixture( + [ + `/* ${ESLINT} no-console */`, + `// ${ESLINT}-next-line no-alert`, + `const a = 1; // ${ESLINT}-line no-unused-vars`, + 'export const b = a;', + ].join('\n'), + ); + const { findings } = analyzeSources([file], root); + expect(findings.lintSuppressions).toHaveLength(3); + }); + + // Regression: this matched only one of the two linters loopwright supports, + // so a biome repo reported zero suppressions however many it carried — a + // check that cannot see anything reads exactly like nothing to see. + it('counts a biome suppression in each of its comment forms', () => { + const { root, file } = writeFixture( + [ + `// ${BIOME} lint/suspicious/noExplicitAny: needed here`, + 'export const a = 1;', + `// ${BIOME}-start lint/style/useConst: block form`, + 'export const b = 2;', + `// ${BIOME}-end lint/style/useConst: block form`, + ].join('\n'), + ); + const { findings } = analyzeSources([file], root); + expect(findings.lintSuppressions).toHaveLength(3); + }); + + it('reports the file and line of each suppression so the gate can cite it', () => { + const { root, file } = writeFixture( + ['export const a = 1;', `// ${BIOME} lint/style/useConst: reason`, 'export const b = 2;'].join('\n'), + ); + const { findings } = analyzeSources([file], root); + expect(findings.lintSuppressions).toHaveLength(1); + expect(findings.lintSuppressions[0].line).toBe(2); + expect(findings.lintSuppressions[0].file).toBe('sample.mjs'); + }); + + it('leaves a file with no suppressions at zero', () => { + const { root, file } = writeFixture('export const a = 1; // an ordinary comment\n'); + const { findings } = analyzeSources([file], root); + expect(findings.lintSuppressions).toHaveLength(0); + }); +}); From 82cbaab41e75af0a8f2899de691bf9569a32f259 Mon Sep 17 00:00:00 2001 From: Samuel Denani Date: Thu, 20 Aug 2026 10:40:49 -0300 Subject: [PATCH 3/3] refactor: exclude .loopwright from the hook instead of working around it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit dodged the two self-detection problems rather than naming them. Both are now handled by the exclusion mechanism this repo already had. The suppression fixtures were composed from string fragments so the scanner would not match them. That is the thing the gate exists to discourage, written into the gate's own tests. They are ordinary files under tests/fixtures/ now, spelling out every comment form literally, which is what `sources.ignore: ["**/fixtures/**"]` and the hook's `:(exclude)` were already there for — fixtures exist to give the detectors something to detect. The hook excluded one file by path, `analyze-source.mjs`, because adding a pattern to the detector tripped the detector. Exclude `.loopwright/` wholesale instead: in a host repo it is vendored code nobody hand-edits, so policing it only ever fires on a re-vendor, and in this repo it is the engine, whose definitions necessarily spell out every pattern they look for. The gate still scans it in CI either way, and the gate is the authority — this hook only exists to deliver the same answer sooner. Verified both directions: a suppression staged outside .loopwright/ still fails the hook, and the fixtures no longer trip it. Co-Authored-By: Claude Opus 5 (1M context) --- .githooks/pre-commit | 13 +++-- .../tests/analyze-source-extra.test.mjs | 52 +++++-------------- .../fixtures/suppressions/biome-forms.mjs | 5 ++ .../tests/fixtures/suppressions/clean.mjs | 1 + .../fixtures/suppressions/eslint-forms.mjs | 4 ++ 5 files changed, 32 insertions(+), 43 deletions(-) create mode 100644 .loopwright/tests/fixtures/suppressions/biome-forms.mjs create mode 100644 .loopwright/tests/fixtures/suppressions/clean.mjs create mode 100644 .loopwright/tests/fixtures/suppressions/eslint-forms.mjs diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 5e13e80..2a5db02 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -18,11 +18,14 @@ fail() { echo "pre-commit: $1" >&2; exit 1; } # exclude forms are needed: a git pathspec's leading `**/` requires at least # one directory component, so it alone would miss a root-level `fixtures/`. # -# analyze-source.mjs is excluded for the same reason as the fixtures: it is -# where the detectors are defined, so it necessarily spells out every pattern -# it looks for. The gate still scans it in CI — where those literals are -# already part of the recorded baseline — so nothing goes unmeasured. -added=$(git diff --cached -U0 -- '*.ts' '*.tsx' '*.js' '*.jsx' '*.mjs' ':(exclude)fixtures/**' ':(exclude)**/fixtures/**' ':(exclude).loopwright/scripts/lib/analyze-source.mjs' | grep '^+[^+]' || true) +# .loopwright/ is excluded wholesale. In a host repo it is vendored code +# nobody hand-edits, so policing it only ever fires on a re-vendor. In this +# repo it is the engine, whose detector definitions necessarily spell out +# every pattern they look for — grepping for those literals in the file that +# declares them is a false positive by construction. Either way the gate +# still scans it in CI, and the gate is the authority; this hook only exists +# to deliver the same answer sooner. +added=$(git diff --cached -U0 -- '*.ts' '*.tsx' '*.js' '*.jsx' '*.mjs' ':(exclude)fixtures/**' ':(exclude)**/fixtures/**' ':(exclude).loopwright/**' | grep '^+[^+]' || true) if [ -n "$added" ]; then declare -a patterns=( '\.only *\(' 'a focused test (.only)' diff --git a/.loopwright/tests/analyze-source-extra.test.mjs b/.loopwright/tests/analyze-source-extra.test.mjs index 8d59e8a..ec641b7 100644 --- a/.loopwright/tests/analyze-source-extra.test.mjs +++ b/.loopwright/tests/analyze-source-extra.test.mjs @@ -1,7 +1,7 @@ import { describe, it, expect } from 'vitest'; import { mkdtempSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; import { analyzeSources } from '../scripts/lib/analyze-source.mjs'; function writeFixture(content) { @@ -148,57 +148,33 @@ describe('empty catch detection', () => { }); }); -// Composed rather than written out, because sources.roots covers this -// directory: a literal suppression comment in this file would be counted as a -// real suppression by the very scanner under test, and the integrity metric -// would climb every time someone tested it. -const ESLINT = `eslint-${'disable'}`; -const BIOME = `biome-${'ignore'}`; +// The bait lives in tests/fixtures/, which `sources.ignore` and the hook both +// exclude: sources.roots covers this directory, so a suppression written out +// in a test would be counted as a real one by the scanner under test. +function suppressionsIn(name) { + const file = join(import.meta.dirname, 'fixtures', 'suppressions', name); + return analyzeSources([file], dirname(file)).findings.lintSuppressions; +} describe('lint suppressions — both supported linters', () => { it('counts an eslint suppression in each of its comment forms', () => { - const { root, file } = writeFixture( - [ - `/* ${ESLINT} no-console */`, - `// ${ESLINT}-next-line no-alert`, - `const a = 1; // ${ESLINT}-line no-unused-vars`, - 'export const b = a;', - ].join('\n'), - ); - const { findings } = analyzeSources([file], root); - expect(findings.lintSuppressions).toHaveLength(3); + expect(suppressionsIn('eslint-forms.mjs')).toHaveLength(3); }); // Regression: this matched only one of the two linters loopwright supports, // so a biome repo reported zero suppressions however many it carried — a // check that cannot see anything reads exactly like nothing to see. it('counts a biome suppression in each of its comment forms', () => { - const { root, file } = writeFixture( - [ - `// ${BIOME} lint/suspicious/noExplicitAny: needed here`, - 'export const a = 1;', - `// ${BIOME}-start lint/style/useConst: block form`, - 'export const b = 2;', - `// ${BIOME}-end lint/style/useConst: block form`, - ].join('\n'), - ); - const { findings } = analyzeSources([file], root); - expect(findings.lintSuppressions).toHaveLength(3); + expect(suppressionsIn('biome-forms.mjs')).toHaveLength(3); }); it('reports the file and line of each suppression so the gate can cite it', () => { - const { root, file } = writeFixture( - ['export const a = 1;', `// ${BIOME} lint/style/useConst: reason`, 'export const b = 2;'].join('\n'), - ); - const { findings } = analyzeSources([file], root); - expect(findings.lintSuppressions).toHaveLength(1); - expect(findings.lintSuppressions[0].line).toBe(2); - expect(findings.lintSuppressions[0].file).toBe('sample.mjs'); + const found = suppressionsIn('biome-forms.mjs'); + expect(found.map((entry) => entry.line)).toEqual([1, 3, 5]); + expect(found[0].file).toBe('biome-forms.mjs'); }); it('leaves a file with no suppressions at zero', () => { - const { root, file } = writeFixture('export const a = 1; // an ordinary comment\n'); - const { findings } = analyzeSources([file], root); - expect(findings.lintSuppressions).toHaveLength(0); + expect(suppressionsIn('clean.mjs')).toHaveLength(0); }); }); diff --git a/.loopwright/tests/fixtures/suppressions/biome-forms.mjs b/.loopwright/tests/fixtures/suppressions/biome-forms.mjs new file mode 100644 index 0000000..1a7770e --- /dev/null +++ b/.loopwright/tests/fixtures/suppressions/biome-forms.mjs @@ -0,0 +1,5 @@ +// biome-ignore lint/suspicious/noExplicitAny: fixture +export const a = 1; +// biome-ignore-start lint/style/useConst: fixture +export const b = 2; +// biome-ignore-end lint/style/useConst: fixture diff --git a/.loopwright/tests/fixtures/suppressions/clean.mjs b/.loopwright/tests/fixtures/suppressions/clean.mjs new file mode 100644 index 0000000..dffe990 --- /dev/null +++ b/.loopwright/tests/fixtures/suppressions/clean.mjs @@ -0,0 +1 @@ +export const a = 1; // an ordinary comment diff --git a/.loopwright/tests/fixtures/suppressions/eslint-forms.mjs b/.loopwright/tests/fixtures/suppressions/eslint-forms.mjs new file mode 100644 index 0000000..7d3ba3e --- /dev/null +++ b/.loopwright/tests/fixtures/suppressions/eslint-forms.mjs @@ -0,0 +1,4 @@ +/* eslint-disable no-console */ +// eslint-disable-next-line no-alert +const a = 1; // eslint-disable-line no-unused-vars +export const b = a;