From d48d095be81a5f72818e22ee4eaa48eb8e9eb1bb Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:07:42 +0200 Subject: [PATCH 1/5] ci: add base-governed PR size exception registry --- README.md | 8 +- config/pr-size-exceptions.json | 54 +++++ scripts/check-pr-size.d.mts | 43 ++++ scripts/check-pr-size.mjs | 314 +++++++++++++++++++++++- tests/unit/tooling/checkPrSize.test.ts | 317 ++++++++++++++++++++++++- 5 files changed, 717 insertions(+), 19 deletions(-) create mode 100644 config/pr-size-exceptions.json diff --git a/README.md b/README.md index ec4549a0..f98b94d3 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ IndexedDB v8 PWA v3.0 i18n 19 locales — 2925 keys - 7205+ tests / 589 files + 7217+ tests / 589 files Codecov Coverage License MIT CI Status @@ -511,7 +511,7 @@ The Settings → AI panel shows a live GPU status badge with adapter details and | **Document Export** | docx + jszip | Word-compatible `.docx` generation (lazy-loaded) | | **PWA** | Service Worker + Web App Manifest v3 | Offline support, installability, Workbox chunking | | **i18n** | Custom React Context (`I18nContext.tsx`) | 2925 keys × 19 locales (de/en/es/fr/it + ar/he/fa RTL Beta + ja/zh/pt/el/fi/sv/hu/is/eu/ru/ko Beta); EN fallback; `localStorage` persistence | -| **Testing** | Vitest 4.x (7205+ tests / 589 files) + Playwright E2E | Unit/integration + cross-browser E2E; Stryker mutation (manual workflow) | +| **Testing** | Vitest 4.x (7217+ tests / 589 files) + Playwright E2E | Unit/integration + cross-browser E2E; Stryker mutation (manual workflow) | | **Code Quality** | Biome (lint + format) + TypeScript 7 (tsgo) strict | `--error-on-warnings` in CI; zero `any` policy | | **Visualization** | Force-directed graph | Interactive character relationship network | | **Desktop** | Tauri v2 | Cross-platform installer; auto-updater via `latest.json` | @@ -549,7 +549,7 @@ WorldScript-Studio/ │ ├── sw.js # PWA Service Worker │ └── manifest.json # PWA Web App Manifest v3 ├── tests/ -│ ├── unit/ # Vitest unit tests (7205+ tests, 589 files) — count spans tests/, components/, packages/*/tests/, not just this folder +│ ├── unit/ # Vitest unit tests (7217+ tests, 589 files) — count spans tests/, components/, packages/*/tests/, not just this folder │ │ ├── ai/ # aiSmallModules, aiCoreFallbackPaths │ │ └── settings/ # WebLlmPanel, AiSections │ └── e2e/ # Playwright specs + helpers.ts @@ -711,7 +711,7 @@ The main pipeline is [`.github/workflows/ci.yml`](.github/workflows/ci.yml). Opt | `scorecard` | weekly + `main` push | OpenSSF Scorecard — SARIF uploaded to GitHub Code Scanning | **Current test metrics (2026-08-27, source-synchronized; CI remains authoritative for pass/fail):** -- **7205+ unit tests** across **589 test files** — CI is authoritative for pass/fail +- **7217+ unit tests** across **589 test files** — CI is authoritative for pass/fail - Coverage thresholds: lines ≥ 80 · branches ≥ 66 · functions ≥ 72 · statements ≥ 78 — enforced in CI (see Codecov badge for live metrics) - i18n: **2925 keys × 19 locales** (en/de/fr/es/it + ar/he/fa RTL Beta + ja/zh/pt/el/fi/sv/hu/is/eu/ru/ko Beta) diff --git a/config/pr-size-exceptions.json b/config/pr-size-exceptions.json new file mode 100644 index 00000000..8ab7e583 --- /dev/null +++ b/config/pr-size-exceptions.json @@ -0,0 +1,54 @@ +{ + "schemaVersion": 1, + "exceptions": [ + { + "id": "pr-539-dual-graph-report-regeneration", + "repository": "qnbs/WorldScript-Studio", + "prNumber": 539, + "baseRef": "main", + "headRef": "chore/dual-graph-solo-local-optimization", + "maxFiles": 30, + "maxCommits": 15, + "maxNonExemptMeaningfulLines": 3000, + "supplementalLineAllowances": [ + { + "path": "graphify-out/GRAPH_REPORT.md", + "maxMeaningfulLines": 5050 + }, + { + "path": ".codegraph/CODEGRAPH_REPORT.md", + "maxMeaningfulLines": 150 + } + ], + "allowedPaths": [ + ".codegraph/CODEGRAPH_REPORT.md", + ".gitignore", + ".graphifyignore", + ".vscode/tasks.json", + "AUDIT.md", + "CLAUDE.md", + "README.md", + "config/graph-tools-versions.json", + "docs/codegraph.md", + "docs/dual-graph-setup.md", + "docs/graphify.md", + "graphify-out/GRAPH_REPORT.md", + "package.json", + "scripts/codegraph-bootstrap.mjs", + "scripts/codegraph-report.d.mts", + "scripts/codegraph-report.mjs", + "scripts/dual-graph-update.mjs", + "scripts/graphSourceFingerprint.d.mts", + "scripts/graphSourceFingerprint.mjs", + "scripts/graphify-bootstrap.mjs", + "scripts/graphify-report.mjs", + "scripts/graphs-cli.mjs", + "scripts/pre-commit-codegraph.mjs", + "tests/unit/scripts/codegraphReport.test.ts", + "tests/unit/scripts/graphSourceFingerprint.test.ts", + "turbo.json" + ], + "reason": "One-time deterministic replacement of stale committed graph reports; executable tooling remains governed by the ordinary absolute meaningful-line ceiling." + } + ] +} diff --git a/scripts/check-pr-size.d.mts b/scripts/check-pr-size.d.mts index 8d4e19e8..ec999688 100644 --- a/scripts/check-pr-size.d.mts +++ b/scripts/check-pr-size.d.mts @@ -4,6 +4,11 @@ export interface GitDependencies { args: string[], options: { encoding: 'utf8' }, ) => { status: number | null; stdout: string; stderr: string; error?: Error }; + readFileSync?: (path: string, encoding: 'utf8') => string; + existsSync?: (path: string) => boolean; + writeFileSync?: (path: string, content: string) => void; + unlinkSync?: (path: string) => void; + env?: NodeJS.ProcessEnv; } export function getChangedFilesNumstat( @@ -28,6 +33,35 @@ export function parseNumstat(numstatOutput: string): NumstatRow[]; export function computeMeaningfulLines(rows: NumstatRow[]): number; +export interface SupplementalLineAllowance { + path: string; + maxMeaningfulLines: number; +} + +export interface PrSizeException { + id: string; + repository: string; + prNumber: number; + baseRef: string; + headRef: string; + maxFiles: number; + maxCommits: number; + maxNonExemptMeaningfulLines: number; + supplementalLineAllowances: SupplementalLineAllowance[]; + allowedPaths: string[]; + reason: string; +} + +export function computeNonExemptMeaningfulLines( + rows: NumstatRow[], + exception?: PrSizeException, +): number; + +export function computeSupplementalReportLines( + rows: NumstatRow[], + exception: PrSizeException, +): Record; + export function computeGovernedFileCount(rows: NumstatRow[]): number; export function isAllDocs(rows: NumstatRow[]): boolean; @@ -70,6 +104,15 @@ export interface PrSizeEvaluation { lineCount?: number; commitCount?: number; allDocs?: boolean; + nonExemptLineCount?: number; + supplementalReportLines?: Record; + exception?: { + applied: boolean; + id?: string; + identityMatch: boolean; + pathScopeMatch: boolean; + baseGoverned: boolean; + }; severity?: SizeSeverity; report?: string; } diff --git a/scripts/check-pr-size.mjs b/scripts/check-pr-size.mjs index 30668a6f..52eb78b3 100644 --- a/scripts/check-pr-size.mjs +++ b/scripts/check-pr-size.mjs @@ -12,6 +12,9 @@ const TIERS = { absolute: { files: 30, lines: 3000, commits: 15 }, }; +const EXCEPTION_REGISTRY_PATH = 'config/pr-size-exceptions.json'; +const REQUIRED_EXCEPTION_SCHEMA_VERSION = 1; + function runGit(args, dependencies = {}) { const spawn = dependencies.spawnSync ?? spawnSync; const result = spawn('git', args, { encoding: 'utf8' }); @@ -118,6 +121,216 @@ export function computeMeaningfulLines(rows) { return total; } +function allowanceMap(exception) { + return new Map( + exception?.supplementalLineAllowances.map((allowance) => [allowance.path, allowance]) ?? [], + ); +} + +export function computeNonExemptMeaningfulLines(rows, exception) { + const allowances = allowanceMap(exception); + return rows.reduce((total, row) => { + if (isGovernanceExcluded(row.path) || allowances.has(row.path)) return total; + return total + row.added + row.removed; + }, 0); +} + +export function computeSupplementalReportLines(rows, exception) { + return Object.fromEntries( + exception.supplementalLineAllowances.map(({ path }) => [ + path, + rows + .filter((row) => row.path === path && !isGovernanceExcluded(row.path)) + .reduce((total, row) => total + row.added + row.removed, 0), + ]), + ); +} + +function isValidPositiveInteger(value) { + return Number.isInteger(value) && value >= 0; +} + +function validateExceptionRegistry(value) { + if ( + !value || + typeof value !== 'object' || + value.schemaVersion !== REQUIRED_EXCEPTION_SCHEMA_VERSION + ) { + throw new Error( + `invalid ${EXCEPTION_REGISTRY_PATH}: expected schemaVersion ${REQUIRED_EXCEPTION_SCHEMA_VERSION}`, + ); + } + if (!Array.isArray(value.exceptions)) { + throw new Error(`invalid ${EXCEPTION_REGISTRY_PATH}: exceptions must be an array`); + } + + const ids = new Set(); + return value.exceptions.map((entry, index) => { + if (!entry || typeof entry !== 'object') { + throw new Error(`invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} is not an object`); + } + const requiredStrings = ['id', 'repository', 'baseRef', 'headRef', 'reason']; + for (const field of requiredStrings) { + if (typeof entry[field] !== 'string' || entry[field].length === 0) { + throw new Error( + `invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} has invalid ${field}`, + ); + } + } + if (ids.has(entry.id)) { + throw new Error(`invalid ${EXCEPTION_REGISTRY_PATH}: duplicate exception id ${entry.id}`); + } + ids.add(entry.id); + if (!isValidPositiveInteger(entry.prNumber) || entry.prNumber < 1) { + throw new Error( + `invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} has invalid prNumber`, + ); + } + for (const field of ['maxFiles', 'maxCommits', 'maxNonExemptMeaningfulLines']) { + if (!isValidPositiveInteger(entry[field])) { + throw new Error( + `invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} has invalid ${field}`, + ); + } + } + if (!Array.isArray(entry.allowedPaths) || entry.allowedPaths.length === 0) { + throw new Error(`invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} needs allowedPaths`); + } + for (const path of entry.allowedPaths) { + if ( + typeof path !== 'string' || + path.length === 0 || + path.startsWith('/') || + path.includes('\\') || + path.includes('..') || + /[*?[\]]/.test(path) + ) { + throw new Error( + `invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} has invalid allowed path`, + ); + } + } + if (!Array.isArray(entry.supplementalLineAllowances)) { + throw new Error( + `invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} needs supplementalLineAllowances`, + ); + } + const supplementalPaths = new Set(); + for (const allowance of entry.supplementalLineAllowances) { + if ( + !allowance || + typeof allowance.path !== 'string' || + allowance.path.length === 0 || + allowance.path.startsWith('/') || + allowance.path.includes('\\') || + allowance.path.includes('..') || + /[*?[\]]/.test(allowance.path) || + !isValidPositiveInteger(allowance.maxMeaningfulLines) + ) { + throw new Error( + `invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} has invalid supplemental allowance`, + ); + } + if (supplementalPaths.has(allowance.path) || !entry.allowedPaths.includes(allowance.path)) { + throw new Error( + `invalid ${EXCEPTION_REGISTRY_PATH}: supplemental path is not unique and allowed`, + ); + } + supplementalPaths.add(allowance.path); + } + return entry; + }); +} + +function getPullRequestIdentity(dependencies = {}) { + const env = dependencies.env ?? process.env; + if (env.GITHUB_EVENT_NAME !== 'pull_request' || !env.GITHUB_EVENT_PATH) return null; + const readFile = dependencies.readFileSync ?? readFileSync; + try { + const payload = JSON.parse(readFile(env.GITHUB_EVENT_PATH, 'utf8')); + const repository = payload?.repository?.full_name; + const number = payload?.number; + const baseRef = payload?.pull_request?.base?.ref; + const headRef = payload?.pull_request?.head?.ref; + if ( + typeof repository !== 'string' || + !Number.isInteger(number) || + number < 1 || + typeof baseRef !== 'string' || + typeof headRef !== 'string' + ) { + return null; + } + return { repository, prNumber: number, baseRef, headRef }; + } catch { + return null; + } +} + +function readBaseExceptionRegistry(base, dependencies = {}) { + const spawn = dependencies.spawnSync ?? spawnSync; + const baseCheck = spawn('git', ['rev-parse', '--verify', `${base}^{commit}`], { + encoding: 'utf8', + }); + if (baseCheck.error || baseCheck.status !== 0) { + throw new Error('could not verify the base commit while loading PR-size governance'); + } + const exists = spawn('git', ['cat-file', '-e', `${base}:${EXCEPTION_REGISTRY_PATH}`], { + encoding: 'utf8', + }); + if (exists.error) throw new Error(`could not inspect ${EXCEPTION_REGISTRY_PATH} on the base ref`); + if (exists.status !== 0) return null; + const shown = spawn('git', ['show', `${base}:${EXCEPTION_REGISTRY_PATH}`], { encoding: 'utf8' }); + if (shown.error || shown.status !== 0) { + throw new Error(`could not read ${EXCEPTION_REGISTRY_PATH} from the base ref`); + } + try { + return validateExceptionRegistry(JSON.parse(shown.stdout)); + } catch (error) { + throw new Error(error instanceof Error ? error.message : `invalid ${EXCEPTION_REGISTRY_PATH}`); + } +} + +function getChangedPaths(base, head, dependencies = {}) { + const output = runGit( + ['diff', '--name-only', '--no-renames', '-z', `${base}...${head}`], + dependencies, + ); + if (output === null) return null; + return output.split('\0').filter(Boolean); +} + +function resolveException(base, head, dependencies = {}) { + const identity = getPullRequestIdentity(dependencies); + if (!identity) + return { applied: false, identityMatch: false, pathScopeMatch: false, baseGoverned: false }; + const registry = readBaseExceptionRegistry(base, dependencies); + if (!registry) + return { applied: false, identityMatch: false, pathScopeMatch: false, baseGoverned: true }; + const matches = registry.filter( + (entry) => + entry.repository === identity.repository && + entry.prNumber === identity.prNumber && + entry.baseRef === identity.baseRef && + entry.headRef === identity.headRef, + ); + if (matches.length > 1) throw new Error('ambiguous PR-size exception matches on the base ref'); + const entry = matches[0]; + if (!entry) + return { applied: false, identityMatch: false, pathScopeMatch: false, baseGoverned: true }; + const changedPaths = getChangedPaths(base, head, dependencies); + if (changedPaths === null) + throw new Error('could not resolve changed paths for PR-size exception scope'); + const pathScopeMatch = changedPaths.every((path) => entry.allowedPaths.includes(path)); + return { + applied: pathScopeMatch, + identityMatch: true, + pathScopeMatch, + baseGoverned: true, + ...(pathScopeMatch ? { entry } : {}), + }; +} + // QNBS-v3: a locale-parity edit always touches 19 rebuilt bundles — exclude them, mirroring lines. export function computeGovernedFileCount(rows) { return rows.filter((row) => !isGovernanceExcluded(row.path)).length; @@ -151,7 +364,14 @@ export function selectSeverity({ fileCount, lineCount, commitCount, allDocs }) { return { tier: 'ok', blocking: false, limits: TIERS.target }; } -export function formatReport({ fileCount, totalFileCount, lineCount, commitCount, allDocs, severity }) { +export function formatReport({ + fileCount, + totalFileCount, + lineCount, + commitCount, + allDocs, + severity, +}) { const { tier, blocking, limits } = severity; const filesNote = totalFileCount > fileCount ? ` (${totalFileCount} total incl. generated)` : ''; if (tier === 'ok') { @@ -162,6 +382,38 @@ export function formatReport({ fileCount, totalFileCount, lineCount, commitCount return `PR size ${kind} (${profile} profile): ${fileCount} files${filesNote}, ${lineCount} meaningful lines, ${commitCount} commits — limit ≤${limits.files} files / ≤${limits.lines} lines / ≤${limits.commits} commits. ${blocking ? 'Split this PR into smaller, independently reviewable PRs before merge.' : 'Consider splitting into smaller, independently reviewable PRs.'}`; } +function formatExceptionReport({ + fileCount, + totalLineCount, + nonExemptLineCount, + supplementalReportLines, + commitCount, + exception, + blocking, +}) { + const allowanceText = Object.entries(supplementalReportLines) + .map(([path, count]) => { + const allowance = exception.entry.supplementalLineAllowances.find( + (item) => item.path === path, + ); + return `${path}=${count}/${allowance.maxMeaningfulLines}`; + }) + .join(', '); + const status = blocking ? 'BLOCKED' : 'APPLIED'; + return ( + `PR_SIZE_EXCEPTION=${status} id=${exception.entry.id} pr=#${exception.entry.prNumber} ` + + `scope=${fileCount}/${exception.entry.maxFiles} files ` + + `TOTAL_MEANINGFUL_LINES=${totalLineCount} ` + + `NON_EXEMPT_MEANINGFUL_LINES=${nonExemptLineCount}/${exception.entry.maxNonExemptMeaningfulLines} ` + + `SUPPLEMENTAL_REPORT_LINES=${allowanceText} ` + + `commits=${commitCount}/${exception.entry.maxCommits} ` + + `BASE_GOVERNED=${exception.baseGoverned ? 'YES' : 'NO'} ` + + `IDENTITY_MATCH=${exception.identityMatch ? 'YES' : 'NO'} ` + + `PATH_SCOPE_MATCH=${exception.pathScopeMatch ? 'YES' : 'NO'} ` + + `EFFECTIVE_LIMITS=files<=${exception.entry.maxFiles};nonExemptLines<=${exception.entry.maxNonExemptMeaningfulLines};commits<=${exception.entry.maxCommits}` + ); +} + export function evaluatePrSize(base, head, dependencies = {}) { const numstat = getChangedFilesNumstat(base, head, dependencies); const commitCount = getCommitCount(base, head, dependencies); @@ -173,7 +425,44 @@ export function evaluatePrSize(base, head, dependencies = {}) { const fileCount = computeGovernedFileCount(rows); const lineCount = computeMeaningfulLines(rows); const allDocs = isAllDocs(rows); - const severity = selectSeverity({ fileCount, lineCount, commitCount, allDocs }); + let exception; + try { + exception = resolveException(base, head, dependencies); + } catch (error) { + return { + ok: false, + error: error instanceof Error ? error.message : 'invalid PR-size exception governance', + }; + } + const nonExemptLineCount = exception.entry + ? computeNonExemptMeaningfulLines(rows, exception.entry) + : lineCount; + const supplementalReportLines = exception.entry + ? computeSupplementalReportLines(rows, exception.entry) + : {}; + const exceptionWithinLimits = + !exception.entry || + (fileCount <= exception.entry.maxFiles && + commitCount <= exception.entry.maxCommits && + nonExemptLineCount <= exception.entry.maxNonExemptMeaningfulLines && + Object.entries(supplementalReportLines).every(([path, count]) => { + const allowance = exception.entry.supplementalLineAllowances.find( + (item) => item.path === path, + ); + return count <= allowance.maxMeaningfulLines; + })); + const severity = + exception.entry && !exceptionWithinLimits + ? { + tier: 'absolute', + blocking: true, + limits: { + files: exception.entry.maxFiles, + lines: exception.entry.maxNonExemptMeaningfulLines, + commits: exception.entry.maxCommits, + }, + } + : selectSeverity({ fileCount, lineCount: nonExemptLineCount, commitCount, allDocs }); return { ok: true, fileCount, @@ -181,8 +470,27 @@ export function evaluatePrSize(base, head, dependencies = {}) { lineCount, commitCount, allDocs, + nonExemptLineCount, + supplementalReportLines, + exception: { + applied: Boolean(exception.entry), + ...(exception.entry ? { id: exception.entry.id } : {}), + identityMatch: exception.identityMatch, + pathScopeMatch: exception.pathScopeMatch, + baseGoverned: exception.baseGoverned, + }, severity, - report: formatReport({ fileCount, totalFileCount, lineCount, commitCount, allDocs, severity }), + report: exception.entry + ? formatExceptionReport({ + fileCount, + totalLineCount: lineCount, + nonExemptLineCount, + supplementalReportLines, + commitCount, + exception, + blocking: severity.blocking, + }) + : formatReport({ fileCount, totalFileCount, lineCount, commitCount, allDocs, severity }), }; } diff --git a/tests/unit/tooling/checkPrSize.test.ts b/tests/unit/tooling/checkPrSize.test.ts index 35d86499..1db66969 100644 --- a/tests/unit/tooling/checkPrSize.test.ts +++ b/tests/unit/tooling/checkPrSize.test.ts @@ -1,14 +1,101 @@ import { describe, expect, it } from 'vitest'; +import type { NumstatRow } from '../../../scripts/check-pr-size.d.mts'; import { computeGovernedFileCount, computeMeaningfulLines, + computeNonExemptMeaningfulLines, + computeSupplementalReportLines, evaluatePrSize, formatReport, isAllDocs, parseNumstat, selectSeverity, } from '../../../scripts/check-pr-size.mjs'; -import type { NumstatRow } from '../../../scripts/check-pr-size.d.mts'; + +const exception = { + id: 'test-exception', + repository: 'qnbs/WorldScript-Studio', + prNumber: 539, + baseRef: 'main', + headRef: 'feature', + maxFiles: 30, + maxCommits: 15, + maxNonExemptMeaningfulLines: 3000, + supplementalLineAllowances: [ + { path: 'graphify-out/GRAPH_REPORT.md', maxMeaningfulLines: 5050 }, + { path: '.codegraph/CODEGRAPH_REPORT.md', maxMeaningfulLines: 150 }, + ], + allowedPaths: [ + 'scripts/tool.mjs', + 'graphify-out/GRAPH_REPORT.md', + '.codegraph/CODEGRAPH_REPORT.md', + ], + reason: 'test', +}; + +function exceptionEvent() { + return { + GITHUB_EVENT_NAME: 'pull_request', + GITHUB_EVENT_PATH: '/tmp/pr-event.json', + }; +} + +function pullRequestEvent({ + repository = exception.repository, + number = exception.prNumber, + baseRef = exception.baseRef, + headRef = exception.headRef, +} = {}) { + return { + repository: { full_name: repository }, + number, + pull_request: { base: { ref: baseRef }, head: { ref: headRef } }, + }; +} + +function exceptionDependencies({ + rows, + changedPaths = rows.map((row) => row.path), + registry = { schemaVersion: 1, exceptions: [exception] }, + event = { + repository: { full_name: exception.repository }, + number: exception.prNumber, + pull_request: { base: { ref: exception.baseRef }, head: { ref: exception.headRef } }, + }, + commitCount = 4, + registryExists = true, +}: { + rows: NumstatRow[]; + changedPaths?: string[]; + registry?: unknown; + event?: unknown; + commitCount?: number; + registryExists?: boolean; +}) { + const numstat = rows.map((row) => `${row.added}\t${row.removed}\t${row.path}\x00`).join(''); + const spawnSync = (_command: string, args: string[]) => { + if (args[0] === 'rev-parse' && args[1] === '--git-path') + return { status: 1, stdout: '', stderr: '' }; + if (args[0] === 'diff' && args.includes('--numstat')) + return { status: 0, stdout: numstat, stderr: '' }; + if (args[0] === 'diff' && args.includes('--name-only')) { + return { status: 0, stdout: `${changedPaths.join('\x00')}\x00`, stderr: '' }; + } + if (args[0] === 'rev-list') return { status: 0, stdout: `${commitCount}\n`, stderr: '' }; + if (args[0] === 'rev-parse' && args[1] === '--verify') + return { status: 0, stdout: 'base\n', stderr: '' }; + if (args[0] === 'cat-file') { + return { status: registryExists ? 0 : 1, stdout: '', stderr: '' }; + } + if (args[0] === 'show') return { status: 0, stdout: JSON.stringify(registry), stderr: '' }; + return { status: 1, stdout: '', stderr: 'unexpected git call' }; + }; + return { + spawnSync, + env: exceptionEvent(), + readFileSync: () => JSON.stringify(event), + }; +} // QNBS-v3: -z is NUL-delimited (git diff --numstat -z), not newline-delimited — matches real output. describe('parseNumstat', () => { @@ -41,7 +128,7 @@ describe('parseNumstat', () => { expect(rows).toEqual([{ path: 'src/new.ts', added: 3, removed: 1 }]); }); - it('preserves raw UTF-8 paths instead of git\'s octal-quoted representation', () => { + it("preserves raw UTF-8 paths instead of git's octal-quoted representation", () => { // -z output is raw UTF-8; the quoted "docs/\303\251.md" form only appears without -z. const rows = parseNumstat('1\t0\tdocs/spécial.md\x00'); expect(rows).toEqual([{ path: 'docs/spécial.md', added: 1, removed: 0 }]); @@ -94,7 +181,9 @@ describe('computeMeaningfulLines', () => { // QNBS-v3: only index.json mirrors community-templates/ — content-guard.mjs never touches the locale variants. it('zeroes out only the content-guard-mirrored community-templates/index.json', () => { - const rows: NumstatRow[] = [{ path: 'public/community-templates/index.json', added: 300, removed: 300 }]; + const rows: NumstatRow[] = [ + { path: 'public/community-templates/index.json', added: 300, removed: 300 }, + ]; expect(computeMeaningfulLines(rows)).toBe(0); }); @@ -178,19 +267,34 @@ describe('selectSeverity', () => { }); it('returns target when over target but within hard (normal profile)', () => { - const result = selectSeverity({ fileCount: 10, lineCount: 500, commitCount: 7, allDocs: false }); + const result = selectSeverity({ + fileCount: 10, + lineCount: 500, + commitCount: 7, + allDocs: false, + }); expect(result.tier).toBe('target'); expect(result.blocking).toBe(false); }); it('returns hard when over hard but within absolute (normal profile)', () => { - const result = selectSeverity({ fileCount: 25, lineCount: 500, commitCount: 7, allDocs: false }); + const result = selectSeverity({ + fileCount: 25, + lineCount: 500, + commitCount: 7, + allDocs: false, + }); expect(result.tier).toBe('hard'); expect(result.blocking).toBe(false); }); it('returns absolute (blocking) when over the absolute ceiling', () => { - const result = selectSeverity({ fileCount: 35, lineCount: 500, commitCount: 7, allDocs: false }); + const result = selectSeverity({ + fileCount: 35, + lineCount: 500, + commitCount: 7, + allDocs: false, + }); expect(result.tier).toBe('absolute'); expect(result.blocking).toBe(true); }); @@ -218,7 +322,12 @@ describe('selectSeverity', () => { describe('formatReport', () => { it('reports "within target" for an ok result', () => { - const severity = selectSeverity({ fileCount: 3, lineCount: 100, commitCount: 2, allDocs: false }); + const severity = selectSeverity({ + fileCount: 3, + lineCount: 100, + commitCount: 2, + allDocs: false, + }); const report = formatReport({ fileCount: 3, totalFileCount: 3, @@ -231,7 +340,12 @@ describe('formatReport', () => { }); it('reports a blocking message for the absolute tier', () => { - const severity = selectSeverity({ fileCount: 35, lineCount: 500, commitCount: 7, allDocs: false }); + const severity = selectSeverity({ + fileCount: 35, + lineCount: 500, + commitCount: 7, + allDocs: false, + }); const report = formatReport({ fileCount: 35, totalFileCount: 35, @@ -245,7 +359,12 @@ describe('formatReport', () => { }); it('reports a non-blocking suggestion for the target/hard tiers', () => { - const severity = selectSeverity({ fileCount: 25, lineCount: 500, commitCount: 7, allDocs: false }); + const severity = selectSeverity({ + fileCount: 25, + lineCount: 500, + commitCount: 7, + allDocs: false, + }); const report = formatReport({ fileCount: 25, totalFileCount: 25, @@ -260,7 +379,12 @@ describe('formatReport', () => { // QNBS-v3: surfaces the excluded generated count so the report isn't silently smaller than the real diff. it('notes the total file count when it exceeds the governed count', () => { - const severity = selectSeverity({ fileCount: 3, lineCount: 100, commitCount: 2, allDocs: false }); + const severity = selectSeverity({ + fileCount: 3, + lineCount: 100, + commitCount: 2, + allDocs: false, + }); const report = formatReport({ fileCount: 3, totalFileCount: 41, @@ -317,7 +441,12 @@ describe('evaluatePrSize', () => { }); it('fails closed (ok: false) on a spawn error (e.g. git not found)', () => { - const spawnSync = () => ({ status: null, error: new Error('spawn git ENOENT'), stdout: '', stderr: '' }); + const spawnSync = () => ({ + status: null, + error: new Error('spawn git ENOENT'), + stdout: '', + stderr: '', + }); const result = evaluatePrSize('base', 'head', { spawnSync }); expect(result.ok).toBe(false); }); @@ -342,6 +471,166 @@ describe('evaluatePrSize', () => { expect(result.fileCount).toBe(19); expect(result.severity?.tier).not.toBe('absolute'); }); + + describe('base-governed supplemental report budgets', () => { + it('partitions ordinary lines from exact report-path allowances', () => { + const rows: NumstatRow[] = [ + { path: 'scripts/tool.mjs', added: 2900, removed: 0 }, + { path: 'graphify-out/GRAPH_REPORT.md', added: 4569, removed: 0 }, + { path: '.codegraph/CODEGRAPH_REPORT.md', added: 92, removed: 0 }, + ]; + expect(computeNonExemptMeaningfulLines(rows, exception)).toBe(2900); + expect(computeSupplementalReportLines(rows, exception)).toEqual({ + 'graphify-out/GRAPH_REPORT.md': 4569, + '.codegraph/CODEGRAPH_REPORT.md': 92, + }); + }); + + it('passes valid report churn while keeping non-exempt lines under 3000', () => { + const rows: NumstatRow[] = [ + { path: 'scripts/tool.mjs', added: 2900, removed: 0 }, + { path: 'graphify-out/GRAPH_REPORT.md', added: 4569, removed: 0 }, + { path: '.codegraph/CODEGRAPH_REPORT.md', added: 92, removed: 0 }, + ]; + const result = evaluatePrSize('base', 'head', exceptionDependencies({ rows })); + expect(result.ok).toBe(true); + expect(result.severity?.blocking).toBe(false); + expect(result.exception).toMatchObject({ + applied: true, + id: 'test-exception', + identityMatch: true, + pathScopeMatch: true, + baseGoverned: true, + }); + expect(result.report).toContain('PR_SIZE_EXCEPTION=APPLIED'); + expect(result.report).toContain('NON_EXEMPT_MEANINGFUL_LINES=2900/3000'); + }); + + it('blocks when non-exempt lines exceed the ordinary absolute ceiling', () => { + const rows: NumstatRow[] = [{ path: 'scripts/tool.mjs', added: 3001, removed: 0 }]; + const result = evaluatePrSize('base', 'head', exceptionDependencies({ rows })); + expect(result.severity?.blocking).toBe(true); + expect(result.report).toContain('NON_EXEMPT_MEANINGFUL_LINES=3001/3000'); + }); + + it('blocks when a report exceeds its own supplemental allowance', () => { + const rows: NumstatRow[] = [ + { path: 'graphify-out/GRAPH_REPORT.md', added: 5051, removed: 0 }, + ]; + const result = evaluatePrSize('base', 'head', exceptionDependencies({ rows })); + expect(result.severity?.blocking).toBe(true); + expect(result.report).toContain('graphify-out/GRAPH_REPORT.md=5051/5050'); + }); + + it('cannot transfer unused allowance between report paths', () => { + const rows: NumstatRow[] = [ + { path: 'graphify-out/GRAPH_REPORT.md', added: 5050, removed: 0 }, + { path: '.codegraph/CODEGRAPH_REPORT.md', added: 151, removed: 0 }, + ]; + const result = evaluatePrSize('base', 'head', exceptionDependencies({ rows })); + expect(result.severity?.blocking).toBe(true); + }); + + it('rejects an out-of-scope path, including a same-named report elsewhere', () => { + const rows: NumstatRow[] = [{ path: 'other/GRAPH_REPORT.md', added: 4000, removed: 0 }]; + const result = evaluatePrSize('base', 'head', exceptionDependencies({ rows })); + expect(result.exception).toMatchObject({ + applied: false, + identityMatch: true, + pathScopeMatch: false, + }); + expect(result.severity?.blocking).toBe(true); + }); + + it('rejects rename-style scope smuggling through the no-renames path list', () => { + const rows: NumstatRow[] = [{ path: 'graphify-out/GRAPH_REPORT.md', added: 10, removed: 10 }]; + const result = evaluatePrSize( + 'base', + 'head', + exceptionDependencies({ + rows, + changedPaths: ['old/GRAPH_REPORT.md', 'graphify-out/GRAPH_REPORT.md'], + }), + ); + expect(result.exception?.pathScopeMatch).toBe(false); + }); + + it.each([ + ['repository', pullRequestEvent({ repository: 'other/repo' })], + ['PR number', pullRequestEvent({ number: 540 })], + ['base ref', pullRequestEvent({ baseRef: 'develop' })], + ['head ref', pullRequestEvent({ headRef: 'other' })], + ])('does not apply for a mismatched %s', (_label, event) => { + const rows: NumstatRow[] = [{ path: 'scripts/tool.mjs', added: 4000, removed: 0 }]; + const result = evaluatePrSize('base', 'head', exceptionDependencies({ rows, event })); + expect(result.exception?.applied).toBe(false); + expect(result.severity?.blocking).toBe(true); + }); + + it('does not apply without a pull_request event identity', () => { + const rows: NumstatRow[] = [{ path: 'scripts/tool.mjs', added: 4000, removed: 0 }]; + const result = evaluatePrSize('base', 'head', { + ...exceptionDependencies({ rows }), + env: {}, + }); + expect(result.exception?.applied).toBe(false); + expect(result.severity?.blocking).toBe(true); + }); + + it('ignores a registry that exists only on the PR head', () => { + const rows: NumstatRow[] = [{ path: 'scripts/tool.mjs', added: 4000, removed: 0 }]; + const result = evaluatePrSize( + 'base', + 'head', + exceptionDependencies({ rows, registryExists: false }), + ); + expect(result.exception?.applied).toBe(false); + expect(result.severity?.blocking).toBe(true); + }); + + it('fails closed for a malformed base registry', () => { + const rows: NumstatRow[] = [{ path: 'scripts/tool.mjs', added: 10, removed: 0 }]; + const result = evaluatePrSize('base', 'head', exceptionDependencies({ rows, registry: '{' })); + expect(result.ok).toBe(false); + expect(result.error).toContain('invalid config/pr-size-exceptions.json'); + }); + + it('fails closed for duplicate matching identities', () => { + const rows: NumstatRow[] = [{ path: 'scripts/tool.mjs', added: 10, removed: 0 }]; + const duplicate = { ...exception, id: 'second-test-exception' }; + const result = evaluatePrSize( + 'base', + 'head', + exceptionDependencies({ + rows, + registry: { schemaVersion: 1, exceptions: [exception, duplicate] }, + }), + ); + expect(result.ok).toBe(false); + expect(result.error).toContain('ambiguous'); + }); + + it('still enforces the exception file and commit ceilings', () => { + const rows: NumstatRow[] = [ + { path: 'scripts/tool.mjs', added: 10, removed: 0 }, + { path: 'graphify-out/GRAPH_REPORT.md', added: 10, removed: 0 }, + { path: '.codegraph/CODEGRAPH_REPORT.md', added: 10, removed: 0 }, + ]; + const narrow = { ...exception, maxFiles: 2, allowedPaths: [...exception.allowedPaths] }; + const tooManyFiles = evaluatePrSize( + 'base', + 'head', + exceptionDependencies({ rows, registry: { schemaVersion: 1, exceptions: [narrow] } }), + ); + expect(tooManyFiles.severity?.blocking).toBe(true); + const tooManyCommits = evaluatePrSize( + 'base', + 'head', + exceptionDependencies({ rows, commitCount: 16 }), + ); + expect(tooManyCommits.severity?.blocking).toBe(true); + }); + }); }); // QNBS-v3: real git repo test — proves the info/attributes override defeats a PR-controlled -diff. @@ -396,7 +685,11 @@ describe('getChangedFilesNumstat (gitattributes evasion protection, real git rep encoding: 'utf8', }); expect(child.status).toBe(0); - const rows = JSON.parse(child.stdout) as Array<{ path: string; added: number; removed: number }>; + const rows = JSON.parse(child.stdout) as Array<{ + path: string; + added: number; + removed: number; + }>; const revealedRow = rows.find((row) => row.path === 'file.txt'); expect(revealedRow?.added).toBeGreaterThan(0); From f8d0435b97b2aa1ae6819da90234e828104f8c4b Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:30:12 +0200 Subject: [PATCH 2/5] test: isolate local PR-size fixtures from CI event identity --- tests/unit/tooling/checkPrSize.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/tooling/checkPrSize.test.ts b/tests/unit/tooling/checkPrSize.test.ts index 1db66969..1fa66bb6 100644 --- a/tests/unit/tooling/checkPrSize.test.ts +++ b/tests/unit/tooling/checkPrSize.test.ts @@ -412,7 +412,7 @@ describe('evaluatePrSize', () => { if (args[0] === 'diff') return { ...okGit(), stdout: '10\t2\tscripts/foo.mjs\x00' }; return { ...okGit(), stdout: '2\n' }; }; - const result = evaluatePrSize('base', 'head', { spawnSync }); + const result = evaluatePrSize('base', 'head', { spawnSync, env: {} }); expect(result.ok).toBe(true); expect(result.fileCount).toBe(1); expect(result.lineCount).toBe(12); @@ -425,7 +425,7 @@ describe('evaluatePrSize', () => { if (args.includes('--git-path')) return { status: 1, stdout: '', stderr: '' }; return { status: 1, stdout: '', stderr: 'fatal: bad range' }; }; - const result = evaluatePrSize('base', 'head', { spawnSync }); + const result = evaluatePrSize('base', 'head', { spawnSync, env: {} }); expect(result.ok).toBe(false); expect(result.error).toBeTruthy(); }); @@ -436,7 +436,7 @@ describe('evaluatePrSize', () => { if (args[0] === 'diff') return { ...okGit(), stdout: '10\t2\tscripts/foo.mjs\x00' }; return { status: 1, stdout: '', stderr: 'fatal: bad range' }; }; - const result = evaluatePrSize('base', 'head', { spawnSync }); + const result = evaluatePrSize('base', 'head', { spawnSync, env: {} }); expect(result.ok).toBe(false); }); @@ -447,7 +447,7 @@ describe('evaluatePrSize', () => { stdout: '', stderr: '', }); - const result = evaluatePrSize('base', 'head', { spawnSync }); + const result = evaluatePrSize('base', 'head', { spawnSync, env: {} }); expect(result.ok).toBe(false); }); @@ -465,7 +465,7 @@ describe('evaluatePrSize', () => { if (args[0] === 'diff') return { ...okGit(), stdout: numstat }; return { ...okGit(), stdout: '1\n' }; }; - const result = evaluatePrSize('base', 'head', { spawnSync }); + const result = evaluatePrSize('base', 'head', { spawnSync, env: {} }); expect(result.ok).toBe(true); expect(result.totalFileCount).toBe(38); expect(result.fileCount).toBe(19); From b8e3ed933b1031f15b429cab4b58f2510b225dfd Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:38:11 +0200 Subject: [PATCH 3/5] fix: reconcile PR-size exception review findings --- scripts/check-pr-size.mjs | 12 +++++-- tests/unit/tooling/checkPrSize.test.ts | 44 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/scripts/check-pr-size.mjs b/scripts/check-pr-size.mjs index 52eb78b3..f9aef19d 100644 --- a/scripts/check-pr-size.mjs +++ b/scripts/check-pr-size.mjs @@ -231,9 +231,14 @@ function validateExceptionRegistry(value) { `invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} has invalid supplemental allowance`, ); } - if (supplementalPaths.has(allowance.path) || !entry.allowedPaths.includes(allowance.path)) { + if (supplementalPaths.has(allowance.path)) { throw new Error( - `invalid ${EXCEPTION_REGISTRY_PATH}: supplemental path is not unique and allowed`, + `invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} has a duplicate supplemental path ${allowance.path}`, + ); + } + if (!entry.allowedPaths.includes(allowance.path)) { + throw new Error( + `invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} supplemental path ${allowance.path} is not in allowedPaths`, ); } supplementalPaths.add(allowance.path); @@ -401,7 +406,8 @@ function formatExceptionReport({ .join(', '); const status = blocking ? 'BLOCKED' : 'APPLIED'; return ( - `PR_SIZE_EXCEPTION=${status} id=${exception.entry.id} pr=#${exception.entry.prNumber} ` + + `PR_SIZE_EXCEPTION=${status} outcome=${blocking ? 'blocked' : 'within target'} ` + + `id=${exception.entry.id} pr=#${exception.entry.prNumber} ` + `scope=${fileCount}/${exception.entry.maxFiles} files ` + `TOTAL_MEANINGFUL_LINES=${totalLineCount} ` + `NON_EXEMPT_MEANINGFUL_LINES=${nonExemptLineCount}/${exception.entry.maxNonExemptMeaningfulLines} ` + diff --git a/tests/unit/tooling/checkPrSize.test.ts b/tests/unit/tooling/checkPrSize.test.ts index 1fa66bb6..df14b718 100644 --- a/tests/unit/tooling/checkPrSize.test.ts +++ b/tests/unit/tooling/checkPrSize.test.ts @@ -503,6 +503,7 @@ describe('evaluatePrSize', () => { baseGoverned: true, }); expect(result.report).toContain('PR_SIZE_EXCEPTION=APPLIED'); + expect(result.report).toContain('outcome=within target'); expect(result.report).toContain('NON_EXEMPT_MEANINGFUL_LINES=2900/3000'); }); @@ -595,6 +596,49 @@ describe('evaluatePrSize', () => { expect(result.error).toContain('invalid config/pr-size-exceptions.json'); }); + it('reports duplicate supplemental paths separately', () => { + const rows: NumstatRow[] = [{ path: 'scripts/tool.mjs', added: 10, removed: 0 }]; + const duplicateAllowance = { + ...exception, + supplementalLineAllowances: [ + { path: 'scripts/tool.mjs', maxMeaningfulLines: 10 }, + { path: 'scripts/tool.mjs', maxMeaningfulLines: 20 }, + ], + }; + const result = evaluatePrSize( + 'base', + 'head', + exceptionDependencies({ + rows, + registry: { schemaVersion: 1, exceptions: [duplicateAllowance] }, + }), + ); + expect(result.ok).toBe(false); + expect(result.error).toContain('duplicate supplemental path scripts/tool.mjs'); + expect(result.error).not.toContain('is not in allowedPaths'); + }); + + it('reports a supplemental path outside allowedPaths separately', () => { + const rows: NumstatRow[] = [{ path: 'scripts/tool.mjs', added: 10, removed: 0 }]; + const outOfScopeAllowance = { + ...exception, + supplementalLineAllowances: [{ path: 'other/GRAPH_REPORT.md', maxMeaningfulLines: 10 }], + }; + const result = evaluatePrSize( + 'base', + 'head', + exceptionDependencies({ + rows, + registry: { schemaVersion: 1, exceptions: [outOfScopeAllowance] }, + }), + ); + expect(result.ok).toBe(false); + expect(result.error).toContain( + 'supplemental path other/GRAPH_REPORT.md is not in allowedPaths', + ); + expect(result.error).not.toContain('duplicate supplemental path'); + }); + it('fails closed for duplicate matching identities', () => { const rows: NumstatRow[] = [{ path: 'scripts/tool.mjs', added: 10, removed: 0 }]; const duplicate = { ...exception, id: 'second-test-exception' }; From d6504a2a1d0dcc0e03f76519f5f97640e4826d49 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:38:43 +0200 Subject: [PATCH 4/5] docs: refresh test count after governance coverage --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index f98b94d3..aff61b89 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ IndexedDB v8 PWA v3.0 i18n 19 locales — 2925 keys - 7217+ tests / 589 files + 7219+ tests / 589 files Codecov Coverage License MIT CI Status @@ -511,7 +511,7 @@ The Settings → AI panel shows a live GPU status badge with adapter details and | **Document Export** | docx + jszip | Word-compatible `.docx` generation (lazy-loaded) | | **PWA** | Service Worker + Web App Manifest v3 | Offline support, installability, Workbox chunking | | **i18n** | Custom React Context (`I18nContext.tsx`) | 2925 keys × 19 locales (de/en/es/fr/it + ar/he/fa RTL Beta + ja/zh/pt/el/fi/sv/hu/is/eu/ru/ko Beta); EN fallback; `localStorage` persistence | -| **Testing** | Vitest 4.x (7217+ tests / 589 files) + Playwright E2E | Unit/integration + cross-browser E2E; Stryker mutation (manual workflow) | +| **Testing** | Vitest 4.x (7219+ tests / 589 files) + Playwright E2E | Unit/integration + cross-browser E2E; Stryker mutation (manual workflow) | | **Code Quality** | Biome (lint + format) + TypeScript 7 (tsgo) strict | `--error-on-warnings` in CI; zero `any` policy | | **Visualization** | Force-directed graph | Interactive character relationship network | | **Desktop** | Tauri v2 | Cross-platform installer; auto-updater via `latest.json` | @@ -549,7 +549,7 @@ WorldScript-Studio/ │ ├── sw.js # PWA Service Worker │ └── manifest.json # PWA Web App Manifest v3 ├── tests/ -│ ├── unit/ # Vitest unit tests (7217+ tests, 589 files) — count spans tests/, components/, packages/*/tests/, not just this folder +│ ├── unit/ # Vitest unit tests (7219+ tests, 589 files) — count spans tests/, components/, packages/*/tests/, not just this folder │ │ ├── ai/ # aiSmallModules, aiCoreFallbackPaths │ │ └── settings/ # WebLlmPanel, AiSections │ └── e2e/ # Playwright specs + helpers.ts @@ -711,7 +711,7 @@ The main pipeline is [`.github/workflows/ci.yml`](.github/workflows/ci.yml). Opt | `scorecard` | weekly + `main` push | OpenSSF Scorecard — SARIF uploaded to GitHub Code Scanning | **Current test metrics (2026-08-27, source-synchronized; CI remains authoritative for pass/fail):** -- **7217+ unit tests** across **589 test files** — CI is authoritative for pass/fail +- **7219+ unit tests** across **589 test files** — CI is authoritative for pass/fail - Coverage thresholds: lines ≥ 80 · branches ≥ 66 · functions ≥ 72 · statements ≥ 78 — enforced in CI (see Codecov badge for live metrics) - i18n: **2925 keys × 19 locales** (en/de/fr/es/it + ar/he/fa RTL Beta + ja/zh/pt/el/fi/sv/hu/is/eu/ru/ko Beta) From 13d3af83f789f199e7ae040a38a2f79970e3682a Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:53:29 +0200 Subject: [PATCH 5/5] refactor: close governance review nitpicks --- scripts/check-pr-size.mjs | 33 ++++++++++++++------------ tests/unit/tooling/checkPrSize.test.ts | 18 ++++++++------ 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/scripts/check-pr-size.mjs b/scripts/check-pr-size.mjs index f9aef19d..9b8ac3d0 100644 --- a/scripts/check-pr-size.mjs +++ b/scripts/check-pr-size.mjs @@ -150,6 +150,18 @@ function isValidPositiveInteger(value) { return Number.isInteger(value) && value >= 0; } +// QNBS-v3: keep every registry path relative, literal, and free of traversal syntax. +function isSafeRegistryPath(value) { + return ( + typeof value === 'string' && + value.length > 0 && + !value.startsWith('/') && + !value.includes('\\') && + !value.includes('..') && + !/[*?[\]]/.test(value) + ); +} + function validateExceptionRegistry(value) { if ( !value || @@ -197,14 +209,7 @@ function validateExceptionRegistry(value) { throw new Error(`invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} needs allowedPaths`); } for (const path of entry.allowedPaths) { - if ( - typeof path !== 'string' || - path.length === 0 || - path.startsWith('/') || - path.includes('\\') || - path.includes('..') || - /[*?[\]]/.test(path) - ) { + if (!isSafeRegistryPath(path)) { throw new Error( `invalid ${EXCEPTION_REGISTRY_PATH}: exception ${index} has invalid allowed path`, ); @@ -219,12 +224,7 @@ function validateExceptionRegistry(value) { for (const allowance of entry.supplementalLineAllowances) { if ( !allowance || - typeof allowance.path !== 'string' || - allowance.path.length === 0 || - allowance.path.startsWith('/') || - allowance.path.includes('\\') || - allowance.path.includes('..') || - /[*?[\]]/.test(allowance.path) || + !isSafeRegistryPath(allowance.path) || !isValidPositiveInteger(allowance.maxMeaningfulLines) ) { throw new Error( @@ -292,7 +292,10 @@ function readBaseExceptionRegistry(base, dependencies = {}) { try { return validateExceptionRegistry(JSON.parse(shown.stdout)); } catch (error) { - throw new Error(error instanceof Error ? error.message : `invalid ${EXCEPTION_REGISTRY_PATH}`); + if (error instanceof Error && error.message.startsWith(`invalid ${EXCEPTION_REGISTRY_PATH}`)) { + throw error; + } + throw new Error(`invalid ${EXCEPTION_REGISTRY_PATH}: malformed JSON`); } } diff --git a/tests/unit/tooling/checkPrSize.test.ts b/tests/unit/tooling/checkPrSize.test.ts index df14b718..704bb157 100644 --- a/tests/unit/tooling/checkPrSize.test.ts +++ b/tests/unit/tooling/checkPrSize.test.ts @@ -57,13 +57,10 @@ function exceptionDependencies({ rows, changedPaths = rows.map((row) => row.path), registry = { schemaVersion: 1, exceptions: [exception] }, - event = { - repository: { full_name: exception.repository }, - number: exception.prNumber, - pull_request: { base: { ref: exception.baseRef }, head: { ref: exception.headRef } }, - }, + event = pullRequestEvent(), commitCount = 4, registryExists = true, + rawRegistry, }: { rows: NumstatRow[]; changedPaths?: string[]; @@ -71,6 +68,7 @@ function exceptionDependencies({ event?: unknown; commitCount?: number; registryExists?: boolean; + rawRegistry?: string; }) { const numstat = rows.map((row) => `${row.added}\t${row.removed}\t${row.path}\x00`).join(''); const spawnSync = (_command: string, args: string[]) => { @@ -87,7 +85,9 @@ function exceptionDependencies({ if (args[0] === 'cat-file') { return { status: registryExists ? 0 : 1, stdout: '', stderr: '' }; } - if (args[0] === 'show') return { status: 0, stdout: JSON.stringify(registry), stderr: '' }; + if (args[0] === 'show') { + return { status: 0, stdout: rawRegistry ?? JSON.stringify(registry), stderr: '' }; + } return { status: 1, stdout: '', stderr: 'unexpected git call' }; }; return { @@ -591,7 +591,11 @@ describe('evaluatePrSize', () => { it('fails closed for a malformed base registry', () => { const rows: NumstatRow[] = [{ path: 'scripts/tool.mjs', added: 10, removed: 0 }]; - const result = evaluatePrSize('base', 'head', exceptionDependencies({ rows, registry: '{' })); + const result = evaluatePrSize( + 'base', + 'head', + exceptionDependencies({ rows, rawRegistry: '{' }), + ); expect(result.ok).toBe(false); expect(result.error).toContain('invalid config/pr-size-exceptions.json'); });