mirror of
https://github.com/block/buzz.git
synced 2026-11-09 10:51:42 +00:00
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 <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
60
desktop/scripts/file-size-rules.mjs
Normal file
60
desktop/scripts/file-size-rules.mjs
Normal file
@@ -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,
|
||||
},
|
||||
];
|
||||
@@ -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\)/);
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
|
||||
26
web/scripts/file-size-rules.mjs
Normal file
26
web/scripts/file-size-rules.mjs
Normal file
@@ -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,
|
||||
},
|
||||
];
|
||||
Reference in New Issue
Block a user