From ce1f1d42754f7676a29f2dd814e2a93abb788bd3 Mon Sep 17 00:00:00 2001 From: ss-dev-01 <11939edb7df583f855dbef923f2358f1184538f88ca452e19e7e35e42ad6d796@buzz.block.builderlab.xyz> Date: Mon, 24 Aug 2026 16:25:01 -0700 Subject: [PATCH 1/2] fix(scripts): govern .mjs in the desktop and web file-size ratchets AGENTS.md states the 1000-line ceiling is "enforced across Desktop, Web, and Mobile by the repository-level `just file-size-check` gate", but the desktop and web rule tables listed only `.ts`/`.tsx` for their script roots, and `runFileSizeCheck` skips any file whose extension is not in its rule's allowlist. Desktop's suite is `*.test.mjs` by convention and its shared test rigs are plain `.mjs` modules, so 545 files inside roots the ratchet already governs were outside the ceiling -- silently, since an uncovered file makes the check exit 0 just like a clean one. Found when review caught a 1253-line test file on #6720 that every local gate and CI had passed. Refs #6726. The gap and the fix, on a clean main checkout: a new 1501-line `.mjs` under `src/features` exited 0 before this change, and byte-identical content named `.ts` exited 1. Adding the extension needs no splits, because the gate is a ratchet rather than an absolute bound -- `allowedLineCount` returns `baseLines` when the base file already exceeds the max. Verified against all twelve oversize files at once: rewriting each without changing its line count exits 0, and adding a single line to each exits 1 naming all twelve with their inherited limits. New files are still held to 1000 from birth. The rule tables move into `file-size-rules.mjs` so the tests can assert the configuration the runners actually execute rather than a restatement of it. The runners stay unconditional: a module that both exports its rules and self-guards its execution can silently stop gating, which is the same failure class this change exists to close. Three tests, each proven non-vacuous by reverting the thing it covers in isolation: mutation fails desktop table back to .ts/.tsx all 3 web table back to .ts/.tsx the allowlist test alone grandfathering -> always maxLines the 2 inherited-size tests The two behavioural tests drive the real rule table against throwaway repositories in a child process, so a leaked `process.exitCode` cannot mark the suite failed. Node exits 1 for an uncaught exception as well as for a violation, so a failing child must also print the violation report to count as one -- otherwise a broken harness would read as a passing gate, which is the same trap again. That guard is itself covered: a bad rules import fails with "crashed rather than gated" instead of passing. Signed-off-by: ss-dev-01 <11939edb7df583f855dbef923f2358f1184538f88ca452e19e7e35e42ad6d796@buzz.block.builderlab.xyz> Co-authored-by: Bradley Axen Signed-off-by: Bradley Axen --- desktop/scripts/check-file-sizes.mjs | 50 +------- desktop/scripts/file-size-rules.mjs | 60 +++++++++ scripts/check-file-sizes-core.test.mjs | 163 ++++++++++++++++++++++++- web/scripts/check-file-sizes.mjs | 21 +--- web/scripts/file-size-rules.mjs | 26 ++++ 5 files changed, 249 insertions(+), 71 deletions(-) create mode 100644 desktop/scripts/file-size-rules.mjs create mode 100644 web/scripts/file-size-rules.mjs diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index bfe4fcc8570..216f6d8a6fc 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -1,59 +1,11 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { runFileSizeCheck } from "../../scripts/check-file-sizes-core.mjs"; +import { rules } from "./file-size-rules.mjs"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const projectRoot = path.resolve(__dirname, ".."); -const MAX_LINES = 1000; - -const rules = [ - { root: "src-tauri/src", extensions: new Set([".rs"]), maxLines: MAX_LINES }, - // Workspace member crates. Without this the ratchet's only Rust root is - // `src-tauri/src`, and a crate under `src-tauri/crates/` is born outside the - // repo's one size discipline -- silently, since the check still exits 0. - { - root: "src-tauri/crates", - extensions: new Set([".rs"]), - maxLines: MAX_LINES, - }, - { - root: "src/app", - extensions: new Set([".ts", ".tsx"]), - maxLines: MAX_LINES, - }, - { - root: "src/features", - extensions: new Set([".ts", ".tsx"]), - maxLines: MAX_LINES, - }, - { - root: "src/shared/api", - extensions: new Set([".ts", ".tsx"]), - maxLines: MAX_LINES, - }, - { - root: "src/shared/context", - extensions: new Set([".ts", ".tsx"]), - maxLines: MAX_LINES, - }, - { - root: "src/shared/lib", - extensions: new Set([".ts", ".tsx"]), - maxLines: MAX_LINES, - }, - { - root: "src/shared/ui", - extensions: new Set([".ts", ".tsx"]), - maxLines: MAX_LINES, - }, - { - root: "src/shared/styles", - extensions: new Set([".css"]), - maxLines: MAX_LINES, - }, -]; - await runFileSizeCheck({ projectRoot, rules, diff --git a/desktop/scripts/file-size-rules.mjs b/desktop/scripts/file-size-rules.mjs new file mode 100644 index 00000000000..f4ae17ce6ca --- /dev/null +++ b/desktop/scripts/file-size-rules.mjs @@ -0,0 +1,60 @@ +// Rule table for the Desktop file-size ratchet, kept in its own module so tests +// can assert the real configuration instead of a restatement of it. The runner +// (`check-file-sizes.mjs`) stays unconditional: a module that both exports its +// rules and self-guards its execution can silently stop gating, which is the +// same class of failure this table's `.mjs` coverage exists to prevent. + +export const MAX_LINES = 1000; + +// Desktop's test suite is `*.test.mjs` by convention and its shared test rigs +// are plain `.mjs` modules, so listing only `.ts`/`.tsx` here left all of them +// outside the ceiling AGENTS.md documents as enforced -- inside roots this +// ratchet already governs, and silently, since the check still exits 0. +export const SCRIPT_EXTENSIONS = new Set([".ts", ".tsx", ".mjs"]); + +export const rules = [ + { root: "src-tauri/src", extensions: new Set([".rs"]), maxLines: MAX_LINES }, + // Workspace member crates. Without this the ratchet's only Rust root is + // `src-tauri/src`, and a crate under `src-tauri/crates/` is born outside the + // repo's one size discipline -- silently, since the check still exits 0. + { + root: "src-tauri/crates", + extensions: new Set([".rs"]), + maxLines: MAX_LINES, + }, + { + root: "src/app", + extensions: SCRIPT_EXTENSIONS, + maxLines: MAX_LINES, + }, + { + root: "src/features", + extensions: SCRIPT_EXTENSIONS, + maxLines: MAX_LINES, + }, + { + root: "src/shared/api", + extensions: SCRIPT_EXTENSIONS, + maxLines: MAX_LINES, + }, + { + root: "src/shared/context", + extensions: SCRIPT_EXTENSIONS, + maxLines: MAX_LINES, + }, + { + root: "src/shared/lib", + extensions: SCRIPT_EXTENSIONS, + maxLines: MAX_LINES, + }, + { + root: "src/shared/ui", + extensions: SCRIPT_EXTENSIONS, + maxLines: MAX_LINES, + }, + { + root: "src/shared/styles", + extensions: new Set([".css"]), + maxLines: MAX_LINES, + }, +]; diff --git a/scripts/check-file-sizes-core.test.mjs b/scripts/check-file-sizes-core.test.mjs index 9b4b910404d..4d200722c6f 100644 --- a/scripts/check-file-sizes-core.test.mjs +++ b/scripts/check-file-sizes-core.test.mjs @@ -1,9 +1,12 @@ import assert from "node:assert/strict"; -import { execFileSync } from "node:child_process"; -import { mkdtempSync } from "node:fs"; +import { execFileSync, spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; +import { fileURLToPath } from "node:url"; import test from "node:test"; +import { rules as desktopRules } from "../desktop/scripts/file-size-rules.mjs"; +import { rules as webRules } from "../web/scripts/file-size-rules.mjs"; import { allowedLineCount, countLines, @@ -117,3 +120,159 @@ test("an inherited oversized file may hold or shrink but not grow", () => { true, ); }); + +// --- The gate's own coverage --------------------------------------------- +// +// An omitted extension does not fail loudly: `runFileSizeCheck` skips the file +// and the check still exits 0, so an uncovered root looks exactly like a clean +// one. That makes the allowlist something to assert rather than assume. These +// read the real rule tables the runners execute, not a restatement of them. + +const scriptsDir = path.dirname(fileURLToPath(import.meta.url)); + +test("every script root governs .mjs alongside .ts and .tsx", () => { + for (const [label, rules] of [ + ["desktop", desktopRules], + ["web", webRules], + ]) { + const scriptRoots = rules.filter((rule) => rule.extensions.has(".ts")); + assert.ok( + scriptRoots.length > 0, + `${label} declares no TypeScript roots, so this assertion cannot fail for the reason it exists; the rule table shape changed`, + ); + for (const rule of scriptRoots) { + assert.ok( + rule.extensions.has(".mjs"), + `${label} root ${rule.root} governs .ts but not .mjs, so test modules and shared rigs there sit outside the ${rule.maxLines}-line ceiling`, + ); + } + } +}); + +// Runs the real desktop rule table against a throwaway repository in a child +// process. A child keeps `process.exitCode` and `console.error` out of this +// test's own process, where a leaked exit code would mark the whole file failed. +function runDesktopGate({ fixtureRoot, baseRef }) { + const result = spawnSync( + process.execPath, + [ + "--input-type=module", + "-e", + ` + import { runFileSizeCheck } from ${JSON.stringify(path.join(scriptsDir, "check-file-sizes-core.mjs"))}; + import { rules } from ${JSON.stringify(path.join(scriptsDir, "..", "desktop", "scripts", "file-size-rules.mjs"))}; + await runFileSizeCheck({ + projectRoot: ${JSON.stringify(path.join(fixtureRoot, "desktop"))}, + rules, + label: "Desktop", + }); + `, + ], + { + encoding: "utf8", + env: { + ...Object.fromEntries( + Object.entries(process.env).filter( + ([key]) => !key.startsWith("GIT_"), + ), + ), + CHECK_FILE_SIZES_BASE: baseRef, + }, + }, + ); + // Node exits 1 for an uncaught exception too, so status alone cannot tell a + // ratchet violation from a crash. The runner's only output is its violation + // report, so a failing status must carry that report to count as a real + // violation; anything else is the harness breaking, not the gate deciding. + const failed = result.status === 1; + if (failed) { + assert.match( + result.stderr, + /file size ratchet failed \(base /, + `the ratchet child exited 1 without reporting a violation, so it crashed rather than gated:\n${result.stderr}`, + ); + } else { + assert.equal( + result.status, + 0, + `the ratchet child exited ${result.status} (signal ${result.signal}):\n${result.stderr}`, + ); + } + return { failed, output: result.stderr }; +} + +function fixtureRepo(prefix) { + const repo = mkdtempSync(path.join(tmpdir(), prefix)); + mkdirSync(path.join(repo, "desktop/src/features/agents"), { + recursive: true, + }); + git(repo, "init", "-b", "main"); + git(repo, "config", "user.name", "Test"); + git(repo, "config", "user.email", "test@example.com"); + return repo; +} + +test("the desktop rules hold a new .mjs file in a governed root to the ceiling", () => { + const repo = fixtureRepo("file-size-mjs-new-"); + git(repo, "commit", "--allow-empty", "-m", "base"); + const target = "desktop/src/features/agents/oversize.test.mjs"; + + // `countLines` counts a trailing newline as a final empty line, so N repeats + // of a newline-terminated line is N + 1 lines. Land exactly on the ceiling. + writeFileSync(path.join(repo, target), `${"// line\n".repeat(999)}// line`); + const atCeiling = runDesktopGate({ fixtureRoot: repo, baseRef: "HEAD" }); + assert.equal( + atCeiling.failed, + false, + `a new .mjs at exactly the ceiling must pass: ${atCeiling.output}`, + ); + + writeFileSync(path.join(repo, target), "// line\n".repeat(1000)); + const overCeiling = runDesktopGate({ fixtureRoot: repo, baseRef: "HEAD" }); + assert.equal( + overCeiling.failed, + true, + "a new 1001-line .mjs under src/features must violate the ceiling", + ); + assert.match( + overCeiling.output, + /src\/features\/agents\/oversize\.test\.mjs: new -> 1001 lines \(allowed 1000\)/, + ); +}); + +test("an inherited oversize .mjs holds or shrinks but may not grow", () => { + const repo = fixtureRepo("file-size-mjs-inherited-"); + const target = "desktop/src/features/agents/inherited.test.mjs"; + + // Commit it already over the ceiling, as the existing oversize suites are. + writeFileSync(path.join(repo, target), "// line\n".repeat(1199)); + git(repo, "add", "-A"); + git(repo, "commit", "-m", "inherited oversize test module"); + const baseRef = git(repo, "rev-parse", "HEAD"); + + // Same line count, different content: an edit that does not grow the file. + writeFileSync(path.join(repo, target), "// edit\n".repeat(1199)); + const held = runDesktopGate({ fixtureRoot: repo, baseRef }); + assert.equal( + held.failed, + false, + `an inherited oversize .mjs must be grandfathered, not failed on sight: ${held.output}`, + ); + + writeFileSync(path.join(repo, target), "// line\n".repeat(1100)); + const shrunk = runDesktopGate({ fixtureRoot: repo, baseRef }); + assert.equal( + shrunk.failed, + false, + `shrinking an inherited oversize .mjs must pass: ${shrunk.output}`, + ); + + writeFileSync(path.join(repo, target), "// line\n".repeat(1200)); + const grown = runDesktopGate({ fixtureRoot: repo, baseRef }); + assert.equal( + grown.failed, + true, + "growing past the inherited size must fail the ratchet", + ); + assert.match(grown.output, /1200 -> 1201 \(\+1\) lines \(allowed 1200\)/); +}); diff --git a/web/scripts/check-file-sizes.mjs b/web/scripts/check-file-sizes.mjs index 810a2b7ae72..2fa3e801532 100644 --- a/web/scripts/check-file-sizes.mjs +++ b/web/scripts/check-file-sizes.mjs @@ -1,30 +1,11 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { runFileSizeCheck } from "../../scripts/check-file-sizes-core.mjs"; +import { rules } from "./file-size-rules.mjs"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const projectRoot = path.resolve(__dirname, ".."); -const MAX_LINES = 1000; - -const rules = [ - { - root: "src/app", - extensions: new Set([".ts", ".tsx"]), - maxLines: MAX_LINES, - }, - { - root: "src/features", - extensions: new Set([".ts", ".tsx"]), - maxLines: MAX_LINES, - }, - { - root: "src/shared/api", - extensions: new Set([".ts", ".tsx"]), - maxLines: MAX_LINES, - }, -]; - await runFileSizeCheck({ projectRoot, rules, diff --git a/web/scripts/file-size-rules.mjs b/web/scripts/file-size-rules.mjs new file mode 100644 index 00000000000..134f50c3508 --- /dev/null +++ b/web/scripts/file-size-rules.mjs @@ -0,0 +1,26 @@ +// Rule table for the Web file-size ratchet. See the sibling Desktop table for +// why the rules live apart from the runner. + +export const MAX_LINES = 1000; + +// `.mjs` is listed alongside `.ts`/`.tsx` so a future test rig or script module +// under these roots is governed from birth rather than discovered later. +export const SCRIPT_EXTENSIONS = new Set([".ts", ".tsx", ".mjs"]); + +export const rules = [ + { + root: "src/app", + extensions: SCRIPT_EXTENSIONS, + maxLines: MAX_LINES, + }, + { + root: "src/features", + extensions: SCRIPT_EXTENSIONS, + maxLines: MAX_LINES, + }, + { + root: "src/shared/api", + extensions: SCRIPT_EXTENSIONS, + maxLines: MAX_LINES, + }, +]; From b9d77258d67ffc6687b84ee687f049d8abd16a35 Mon Sep 17 00:00:00 2001 From: ss-dev-01 <11939edb7df583f855dbef923f2358f1184538f88ca452e19e7e35e42ad6d796@buzz.block.builderlab.xyz> Date: Mon, 24 Aug 2026 16:59:11 -0700 Subject: [PATCH 2/2] test(scripts): require the ratchet child to run to completion The child-process guard proved the gate *started* reporting, not that it finished. Status 1 plus the report heading was accepted as a completed policy decision, so a child that printed the heading -- or the whole report -- and then crashed still read as the gate having decided. The eval script now writes a fixed sentinel to stdout after the awaited `runFileSizeCheck` returns, and the harness requires it on both the exit-0 and exit-1 paths. The two questions are separated: the sentinel proves normal completion, and only then does the exit status carry the policy result. Proven against the case the old guard missed. Injecting a throw after the full violation report is emitted but before the gate returns: old guard (heading only) 9 passing / 0 failing <- crash read as a pass new guard (sentinel) 7 passing / 2 failing <- "did not run to completion ... crashed rather than gated" A throw injected immediately after the heading also fails with the same assertion rather than being misattributed to a missing violation line. This is the same defect the PR closes, one level up: a check that cannot distinguish the outcome it reports from a failure to reach it. Signed-off-by: ss-dev-01 <11939edb7df583f855dbef923f2358f1184538f88ca452e19e7e35e42ad6d796@buzz.block.builderlab.xyz> Co-authored-by: Bradley Axen Signed-off-by: Bradley Axen --- scripts/check-file-sizes-core.test.mjs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/scripts/check-file-sizes-core.test.mjs b/scripts/check-file-sizes-core.test.mjs index 4d200722c6f..bd240e14339 100644 --- a/scripts/check-file-sizes-core.test.mjs +++ b/scripts/check-file-sizes-core.test.mjs @@ -152,6 +152,8 @@ test("every script root governs .mjs alongside .ts and .tsx", () => { // Runs the real desktop rule table against a throwaway repository in a child // process. A child keeps `process.exitCode` and `console.error` out of this // test's own process, where a leaked exit code would mark the whole file failed. +const COMPLETION_SENTINEL = "__file_size_gate_completed__"; + function runDesktopGate({ fixtureRoot, baseRef }) { const result = spawnSync( process.execPath, @@ -166,6 +168,9 @@ function runDesktopGate({ fixtureRoot, baseRef }) { rules, label: "Desktop", }); + // Printed only if the awaited call returned normally. On stdout so it + // cannot be confused with any part of the violation report. + process.stdout.write(${JSON.stringify(COMPLETION_SENTINEL)}); `, ], { @@ -181,15 +186,20 @@ function runDesktopGate({ fixtureRoot, baseRef }) { }, ); // Node exits 1 for an uncaught exception too, so status alone cannot tell a - // ratchet violation from a crash. The runner's only output is its violation - // report, so a failing status must carry that report to count as a real - // violation; anything else is the harness breaking, not the gate deciding. + // ratchet violation from a crash. Split the two questions: the sentinel proves + // the gate ran to completion, and only then does the exit status mean a policy + // decision. Asserting on the report heading alone would accept a child that + // started printing violations and then crashed part-way through. + assert.ok( + result.stdout.includes(COMPLETION_SENTINEL), + `the ratchet child did not run to completion (exit ${result.status}, signal ${result.signal}), so it crashed rather than gated:\n${result.stderr}`, + ); const failed = result.status === 1; if (failed) { assert.match( result.stderr, /file size ratchet failed \(base /, - `the ratchet child exited 1 without reporting a violation, so it crashed rather than gated:\n${result.stderr}`, + `the ratchet child exited 1 without reporting a violation:\n${result.stderr}`, ); } else { assert.equal(