From 004f61989d637b3976b4461870eae1eb2be313eb Mon Sep 17 00:00:00 2001 From: norvalbv Date: Mon, 10 Aug 2026 01:04:39 +0100 Subject: [PATCH 1/2] fix(guard-size): scope pull-request CI to the PR diff (sc-1539) ## Summary - make pull-request size checks evaluate only files changed from the exact GitHub base commit - keep push-main, manual, and legacy-baseline audits whole-tree - resolve pull-request scope only for the gate command - share Git prefix resolution across PR diff selection and size preflight - add regressions for inherited line debt, changed growth, inherited/legacy disable debt, stale bases, and freeze isolation ## Root cause A pull-request checkout has a clean index, so guard-size fell into its whole-tree audit path. Pre-existing oversized files inherited from main were therefore blamed on unrelated PRs. ## Impact Required PR checks now fail only when the PR adds or worsens per-file size debt. Main-branch pushes still report inherited repository drift, legacy disable baselines still require migration, and non-gate commands are isolated from PR-only environment state. ## Validation - focused ratchet suites: 58 passed - Biome lint: passed - structure lint: passed - TypeScript typecheck: passed - build: passed - exact PR #376 base/head reproduction: whole-tree audit failed as before; PR-base scope passed - GitNexus staged audit: LOW risk, zero affected flows - duplication matcher and commit-guard: passed - full suite reached a pre-existing childless Vitest-parent hang; recurrence report 4c791856-dea7-4883-b460-caba479c90b3 Story: sc-1539 Autonomous report: 6f6fb84b-8fc5-484f-9d7e-50038de4b6e1 --- .github/workflows/gate.yml | 2 + .../ratchets/__tests__/size-disable.test.mts | 130 ++++++++++++++++++ gate-engine/ratchets/git-index.mts | 49 +++++++ gate-engine/ratchets/size-disable.mts | 37 ++--- gate-engine/ratchets/size-preflight.mts | 7 +- 5 files changed, 202 insertions(+), 23 deletions(-) diff --git a/.github/workflows/gate.yml b/.github/workflows/gate.yml index 2b0ca466..72589bd9 100644 --- a/.github/workflows/gate.yml +++ b/.github/workflows/gate.yml @@ -33,6 +33,8 @@ jobs: run: node gate-engine/ratchets/folder-fanout.mts gate - name: Ratchet — size-disable + env: + GUARD_RATCHET_BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || '' }} run: node gate-engine/ratchets/size-disable.mts gate - name: Typecheck diff --git a/gate-engine/ratchets/__tests__/size-disable.test.mts b/gate-engine/ratchets/__tests__/size-disable.test.mts index 8d2892c3..937205bd 100644 --- a/gate-engine/ratchets/__tests__/size-disable.test.mts +++ b/gate-engine/ratchets/__tests__/size-disable.test.mts @@ -128,6 +128,19 @@ describe('CLI freeze/gate contract (what a pre-commit hook relies on)', () => { expect(run(root, 'gate').status).toBe(0); }); + it('freeze ignores a stale pull-request base because only gate consumes PR scope', () => { + const root = makeRoot(); + write(root, 'src/a.ts', '/* eslint-disable max-lines */\nexport {};\n'); + const r = spawnSync(process.execPath, [SCRIPT, 'freeze'], { + cwd: root, + encoding: 'utf8', + env: { ...process.env, GUARD_RATCHET_BASE: 'missing-base' }, + }); + expect(r.status, r.stderr).toBe(0); + const frozen = JSON.parse(readFileSync(join(root, 'eslint/baselines/size.json'), 'utf8')); + expect(frozen).toEqual({ files: { 'src/a.ts': { file: 1, fn: 0 } } }); + }); + it('writes the baseline under the CONSUMER cwd, not the package dir (W-3)', () => { const root = makeRoot(); write(root, 'src/a.ts', '/* eslint-disable max-lines */\nexport {};\n'); @@ -470,6 +483,76 @@ describe('raw-line cap (the maxLines gate — size owned by the ratchet, not esl expect(baseline.files['src/legacy.ts']).toBe(80); // unchanged — no mutation without a commit }); + it('pull-request CI ignores inherited line drift outside the diff', () => { + const root = makeRoot(); + gitInit(root); + writeConfig(root, { scanRoots: ['src'], sourceExtensions: ['ts'], maxLines: 50 }); + write(root, 'src/inherited.ts', big(80)); + write(root, 'src/clean.ts', big(10)); + write( + root, + 'eslint/baselines/size-lines.json', + JSON.stringify({ maxLines: 50, files: { 'src/inherited.ts': 60 } }), + ); + gitAdd(root, '-A'); + execFileSync('git', ['commit', '-qm', 'base with inherited drift'], { cwd: root }); + const base = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: root, encoding: 'utf8' }).trim(); + write(root, 'src/clean.ts', big(11)); + gitAdd(root, 'src/clean.ts'); + execFileSync('git', ['commit', '-qm', 'unrelated PR change'], { cwd: root }); + + expect(run(root, 'gate').status).toBe(1); // push/manual audit still sees the inherited drift + const pr = spawnSync(process.execPath, [SCRIPT, 'gate'], { + cwd: root, + encoding: 'utf8', + env: { ...process.env, GUARD_RATCHET_BASE: base }, + }); + expect(pr.status, pr.stderr).toBe(0); + expect(pr.stderr).not.toContain('src/inherited.ts'); + }); + + it('pull-request CI still blocks a changed file that exceeds its ceiling', () => { + const root = makeRoot(); + gitInit(root); + writeConfig(root, { scanRoots: ['src'], sourceExtensions: ['ts'], maxLines: 50 }); + write(root, 'src/changed.ts', big(80)); + write( + root, + 'eslint/baselines/size-lines.json', + JSON.stringify({ maxLines: 50, files: { 'src/changed.ts': 80 } }), + ); + gitAdd(root, '-A'); + execFileSync('git', ['commit', '-qm', 'base'], { cwd: root }); + const base = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: root, encoding: 'utf8' }).trim(); + write(root, 'src/changed.ts', big(90)); + gitAdd(root, 'src/changed.ts'); + execFileSync('git', ['commit', '-qm', 'grow changed file'], { cwd: root }); + + const pr = spawnSync(process.execPath, [SCRIPT, 'gate'], { + cwd: root, + encoding: 'utf8', + env: { ...process.env, GUARD_RATCHET_BASE: base }, + }); + expect(pr.status).toBe(1); + expect(pr.stderr).toContain('src/changed.ts: 90 lines (max 80)'); + }); + + it('pull-request CI fails unavailable when its supplied base cannot be resolved', () => { + const root = makeRoot(); + gitInit(root); + writeConfig(root, { scanRoots: ['src'], sourceExtensions: ['ts'], maxLines: 50 }); + write(root, 'src/clean.ts', big(10)); + gitAdd(root, '-A'); + execFileSync('git', ['commit', '-qm', 'base'], { cwd: root }); + const pr = spawnSync(process.execPath, [SCRIPT, 'gate'], { + cwd: root, + encoding: 'utf8', + env: { ...process.env, GUARD_RATCHET_BASE: 'missing-base' }, + }); + expect(pr.status).toBe(2); + expect(pr.stderr).toContain('pull-request base is unavailable'); + }); + it('freeze is monotone-down: never raises a recorded ceiling (anti-laundering)', () => { const root = makeRoot(); writeConfig(root, { scanRoots: ['src'], sourceExtensions: ['ts'], maxLines: 50 }); @@ -584,6 +667,29 @@ describe('per-file disable ratchet (auto-lower, migration, net-zero)', () => { expect(r.stderr).toContain('src/b.ts'); }); + it('pull-request CI ignores inherited disable debt outside the diff', () => { + const root = makeRoot(); + gitInit(root); + writeConfig(root, { scanRoots: ['src'] }); + write(root, 'src/inherited.ts', dis(1)); + write(root, 'src/clean.ts', 'export {};\n'); + gitAdd(root, '-A'); + execFileSync('git', ['commit', '-qm', 'base with inherited debt'], { cwd: root }); + const base = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: root, encoding: 'utf8' }).trim(); + write(root, 'src/clean.ts', 'export const clean = true;\n'); + gitAdd(root, 'src/clean.ts'); + execFileSync('git', ['commit', '-qm', 'unrelated PR change'], { cwd: root }); + + expect(run(root, 'gate').status).toBe(1); // whole-tree audit retains the migration block + const pr = spawnSync(process.execPath, [SCRIPT, 'gate'], { + cwd: root, + encoding: 'utf8', + env: { ...process.env, GUARD_RATCHET_BASE: base }, + }); + expect(pr.status, pr.stderr).toBe(0); + expect(pr.stderr).not.toContain('src/inherited.ts'); + }); + it('a stale {0,0} legacy baseline self-deletes + stages in a commit (the qavis case)', () => { const root = makeRoot(); gitInit(root); @@ -633,6 +739,30 @@ describe('per-file disable ratchet (auto-lower, migration, net-zero)', () => { expect(() => readBaseline(root)).not.toThrow(); // size.json is NOT deleted }); + it('pull-request CI still blocks a legacy baseline until its disables are migrated', () => { + const root = makeRoot(); + gitInit(root); + writeConfig(root, { scanRoots: ['src'] }); + write(root, 'src/inherited.ts', dis(1)); + write(root, 'src/clean.ts', 'export {};\n'); + write(root, 'eslint/baselines/size.json', JSON.stringify({ fileDisables: 1, fnDisables: 0 })); + gitAdd(root, '-A'); + execFileSync('git', ['commit', '-qm', 'legacy baseline'], { cwd: root }); + const base = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: root, encoding: 'utf8' }).trim(); + write(root, 'src/clean.ts', 'export const clean = true;\n'); + gitAdd(root, 'src/clean.ts'); + execFileSync('git', ['commit', '-qm', 'unrelated PR change'], { cwd: root }); + + const r = spawnSync(process.execPath, [SCRIPT, 'gate'], { + cwd: root, + encoding: 'utf8', + env: { ...process.env, GUARD_RATCHET_BASE: base }, + }); + expect(r.status).toBe(1); + expect(r.stderr).toContain('pre-per-file baseline'); + expect(r.stderr).toContain('guard-size freeze'); + }); + it('a legacy baseline migrates to per-file shape on `guard-size freeze`', () => { const root = makeRoot(); writeConfig(root, { scanRoots: ['src'] }); diff --git a/gate-engine/ratchets/git-index.mts b/gate-engine/ratchets/git-index.mts index d5befed6..677394b4 100644 --- a/gate-engine/ratchets/git-index.mts +++ b/gate-engine/ratchets/git-index.mts @@ -80,3 +80,52 @@ export function stagedSet(root: string): Set | null { return null; } } + +// The CWD path inside its repository, slash-terminated (empty at the repository root). +export function gitPrefix(root: string): string { + try { + return execFileSync('git', ['rev-parse', '--show-prefix'], { + cwd: root, + encoding: 'utf8', + }).trim(); + } catch { + return ''; + } +} + +// The CWD-relative paths changed between an exact base commit and HEAD. This is the clean-index +// analogue of stagedSet for pull-request CI: GitHub supplies the base SHA, so inherited base debt +// is not attributed to the PR. NUL delimiters preserve every valid path byte except NUL itself. +export function changedSetSince(root: string, baseRef: string): Set | null { + try { + execFileSync('git', ['rev-parse', '--verify', `${baseRef}^{commit}`], { + cwd: root, + stdio: ['ignore', 'pipe', 'ignore'], + }); + const prefix = gitPrefix(root); + const out = execFileSync( + 'git', + ['diff', '--name-only', '-z', '--diff-filter=ACMR', `${baseRef}...HEAD`], + { cwd: root, encoding: 'utf8' }, + ); + const paths = out.split('\0').filter(Boolean); + return new Set( + prefix + ? paths.filter((file) => file.startsWith(prefix)).map((file) => file.slice(prefix.length)) + : paths, + ); + } catch { + return null; + } +} + +// GitHub PR checks opt into change attribution with the event's exact base SHA. No environment +// value means the ordinary local/push audit; an invalid supplied ref is an unavailable hard failure. +export function pullRequestScope(root: string): Set | null { + const baseRef = process.env.GUARD_RATCHET_BASE; + if (!baseRef) return null; + const scope = changedSetSince(root, baseRef); + if (scope) return scope; + console.error(`guard-size: pull-request base is unavailable: ${baseRef}`); + process.exit(2); +} diff --git a/gate-engine/ratchets/size-disable.mts b/gate-engine/ratchets/size-disable.mts index 400d33f6..e59cfbab 100755 --- a/gate-engine/ratchets/size-disable.mts +++ b/gate-engine/ratchets/size-disable.mts @@ -23,7 +23,7 @@ import { import { dirname, join } from 'node:path'; import { pathToFileURL } from 'node:url'; import { CONFIG_FILENAME, resolveGuardConfig, sourceMatchers } from '../config.mts'; -import { hasStagedFiles, stageBaseline, stagedSet } from './git-index.mts'; +import { hasStagedFiles, pullRequestScope, stageBaseline, stagedSet } from './git-index.mts'; import { LINES_BASELINE, SIZE_SKIP_DIRS } from './size-policy.mts'; import { runPreflightCli } from './size-preflight.mts'; @@ -262,6 +262,7 @@ function runLinesGate( root: string, cfg: ReturnType, linesBaselineFile: string, + ciScope: Set | null, ): void { const over = countOversized(root); const grandfathered: Record = existsSync(linesBaselineFile) @@ -271,9 +272,9 @@ function runLinesGate( const inCommit = staged !== null && hasStagedFiles(root); const match = sourceMatchers(cfg.sourceExtensions); const cap = (f: string) => (match.isTest(f) ? cfg.maxTestLines : cfg.maxLines); - // Scope to the committing files; with nothing staged, fall back to the whole tree (CI). - const scoped = inCommit ? over.filter((o) => staged?.has(o.file)) : over; - + // A PR supplies an exact base scope; local commits use the index; audits use the whole tree. + const selected = ciScope ?? (inCommit ? staged : null); + const scoped = selected ? over.filter((o) => selected.has(o.file)) : over; // A file fails when it exceeds its own recorded ceiling (grandfathered) or the cap (new file). const grew = scoped.filter((o) => o.lines > Math.max(cap(o.file), grandfathered[o.file] ?? 0)); if (grew.length) { @@ -285,7 +286,7 @@ function runLinesGate( } process.exit(1); } - if (!inCommit || !staged) return; // no commit in progress → never tighten/stage + if (ciScope || !inCommit || !staged) return; // CI never tightens/stages // Tighten only the committing files' ceilings; every other recorded count is preserved as-is, // so a concurrent agent's uncommitted shrink is never locked in. @@ -319,7 +320,6 @@ function runLinesGate( } } } - // Read the disable baseline. A pre-per-file `{ fileDisables, fnDisables }` shape (no `files` key) is // reported empty + `legacy: true` so the gate blocks with a migrate hint (real disables) or // self-cleans it (a stale {0,0}); a re-freeze rewrites it to the per-file shape. @@ -344,6 +344,7 @@ function runDisableGate( root: string, baselineFile: string, current: ReturnType, + ciScope: Set | null, ): void { const { grandfathered, legacy } = readDisableBaseline(baselineFile); const cur = current.perFile; @@ -351,15 +352,15 @@ function runDisableGate( const inCommit = staged !== null && hasStagedFiles(root); const ceil = (f: string): DisableCount => grandfathered[f] ?? { file: 0, fn: 0 }; - // A file fails when its disables exceed its recorded ceiling (0 for an unlisted/new file). Scope to - // the committing files; with nothing staged, the whole tree (CI). A LEGACY baseline is always - // whole-tree: it has no per-file grandfathering, so any disable ANYWHERE is unrecognised and must - // block the migrate — else an unstaged disable slips past and the commit path below deletes + // A file fails when its disables exceed its recorded ceiling (0 for an unlisted/new file). A PR + // scopes to its diff. Otherwise a LEGACY baseline stays whole-tree: it has no per-file + // grandfathering, so an unstaged disable must block rather than let the commit path below delete // size.json wholesale (changed=legacy, empty map), silently un-grandfathering it. - const scoped = legacy - ? Object.keys(cur) - : inCommit - ? [...(staged as Set)] + const selected = legacy ? null : (ciScope ?? (inCommit ? staged : null)); + const scoped = selected + ? [...selected] + : legacy + ? Object.keys(cur) : Object.keys({ ...cur, ...grandfathered }); const grew = scoped.filter( (f) => cur[f] && (cur[f].file > ceil(f).file || cur[f].fn > ceil(f).fn), @@ -381,7 +382,7 @@ function runDisableGate( process.exit(1); } - if (!inCommit || !staged) { + if (ciScope || !inCommit || !staged) { // No commit in progress → never mutate. Nudge a re-freeze if anything shrank or a legacy file lingers. if (legacy) { console.log( @@ -471,6 +472,7 @@ function runCli(cmd: string): void { // Reason: the two ratchets (folder-fanout / size-disable) are parallel-by-design independent guard bins (+ tests); each self-contained with the same freeze/gate CLI shell // fallow-ignore-next-line code-duplication if (cmd === 'gate') { + const ciScope = pullRequestScope(root); const hasBaseline = existsSync(baselineFile); // A missing baseline means "no grandfathered debt". Enforce from config (empty baseline = 0/0) // whenever the repo is governed (guard.config.json present — true in devkit's own repo, CI, and @@ -481,9 +483,8 @@ function runCli(cmd: string): void { process.exit(2); // ungoverned + un-frozen → fail open } // Disable ratchet: per-file, per-commit shrink-only (auto-lowers as disables are removed). - runDisableGate(root, baselineFile, current); - // Raw-line caps: a per-file, per-COMMIT shrink-only ratchet. - if (cfg.maxLines || cfg.maxTestLines) runLinesGate(root, cfg, linesBaselineFile); + runDisableGate(root, baselineFile, current, ciScope); + if (cfg.maxLines || cfg.maxTestLines) runLinesGate(root, cfg, linesBaselineFile, ciScope); process.exit(0); } diff --git a/gate-engine/ratchets/size-preflight.mts b/gate-engine/ratchets/size-preflight.mts index a1b4dbac..151f26fb 100644 --- a/gate-engine/ratchets/size-preflight.mts +++ b/gate-engine/ratchets/size-preflight.mts @@ -2,7 +2,7 @@ import { execFileSync } from 'node:child_process'; import { existsSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; import { resolveGuardConfig, sourceMatchers } from '../config.mts'; -import { stagedSet } from './git-index.mts'; +import { gitPrefix, stagedSet } from './git-index.mts'; import { LINES_BASELINE, SIZE_SKIP_DIRS } from './size-policy.mts'; interface LinesBaseline { @@ -28,10 +28,7 @@ function readLinesBaselineAtRef(root: string, ref: string): LinesBaseline | null cwd: root, stdio: ['ignore', 'pipe', 'ignore'], }); - const prefix = execFileSync('git', ['rev-parse', '--show-prefix'], { - cwd: root, - encoding: 'utf8', - }).trim(); + const prefix = gitPrefix(root); let text: string; try { text = execFileSync('git', ['show', `${ref}:${prefix}${LINES_BASELINE}`], { From 64ecfff608c8e2002bab01e1aa4e72fc98b4f250 Mon Sep 17 00:00:00 2001 From: norvalbv Date: Mon, 10 Aug 2026 08:55:06 +0100 Subject: [PATCH 2/2] fix(guard-size): preserve leading whitespace in git prefixes --- .../ratchets/__tests__/size-disable.test.mts | 16 +++++++++++++++- gate-engine/ratchets/git-index.mts | 2 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/gate-engine/ratchets/__tests__/size-disable.test.mts b/gate-engine/ratchets/__tests__/size-disable.test.mts index 937205bd..7fd14f46 100644 --- a/gate-engine/ratchets/__tests__/size-disable.test.mts +++ b/gate-engine/ratchets/__tests__/size-disable.test.mts @@ -4,7 +4,7 @@ import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { afterEach, describe, expect, it } from 'vitest'; -import { stagedSet } from '../git-index.mts'; +import { changedSetSince, stagedSet } from '../git-index.mts'; import { countDisables, countOversized, freezeLines } from '../size-disable.mts'; const SCRIPT = join(dirname(fileURLToPath(import.meta.url)), '..', 'size-disable.mts'); @@ -468,6 +468,20 @@ describe('raw-line cap (the maxLines gate — size owned by the ratchet, not esl expect(resolved.stderr).not.toContain('src/upstream.ts'); }); + it('preserves leading whitespace when scoping a PR from a nested directory', () => { + const root = makeRoot(); + gitInit(root); + write(root, ' leading/base.ts', 'export {};\n'); + gitAdd(root, '-A'); + execFileSync('git', ['commit', '-qm', 'base'], { cwd: root }); + const base = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: root, encoding: 'utf8' }).trim(); + write(root, ' leading/changed.ts', 'export const changed = true;\n'); + gitAdd(root, '-A'); + execFileSync('git', ['commit', '-qm', 'change nested file'], { cwd: root }); + + expect(changedSetSince(join(root, ' leading'), base)).toEqual(new Set(['changed.ts'])); + }); + it('with nothing staged (CI / audit) the whole tree is enforced and the baseline is not mutated', () => { const root = makeRoot(); gitInit(root); diff --git a/gate-engine/ratchets/git-index.mts b/gate-engine/ratchets/git-index.mts index 677394b4..3104c4db 100644 --- a/gate-engine/ratchets/git-index.mts +++ b/gate-engine/ratchets/git-index.mts @@ -87,7 +87,7 @@ export function gitPrefix(root: string): string { return execFileSync('git', ['rev-parse', '--show-prefix'], { cwd: root, encoding: 'utf8', - }).trim(); + }).trimEnd(); } catch { return ''; }