Skip to content

Commit 3e91410

Browse files
dmealingclaude
andcommitted
fix(cli): the advisories reported findings inside build output
`meta verify` named `public/main.js:243 — <EntityFetcherProvider> is mounted with no baseUrl` on an estate whose source, client/src/main.tsx:26, passes baseUrl="/api" correctly. A 711KB bundle: a file nobody edits, a fix that belongs elsewhere, and the FIRST line of the advisory on a project that had already done the right thing. No ignore list would have caught it — they name dist/build/out/.next/.output and this project builds to `public/`. So the rule is a property of the artifact: looksBundled() calls a file with any line of 5,000+ characters build output, an order of magnitude above the longest line a human writes and an order below what minification produces. Applied at BOTH doors, because both had the hole in different shapes: the base-URL advisory had no size guard at all, and the anti-pattern pass's 512KB gate is about scan cost, so a 300KB minified bundle sails through it while being exactly as unfixable. The walkers stay separate — they diverge on purpose — and share only this judgment. Verified against the estate: the false positive is gone, its 26 real findings in client/src are not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTcEKXTQMYt84fAjuw5A2M
1 parent f403354 commit 3e91410

7 files changed

Lines changed: 154 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,27 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
77

88
## [Unreleased]
99

10+
### Fixed — both advisory scanners reported findings inside BUILD OUTPUT
11+
12+
`meta verify` on an adopter estate reported
13+
`public/main.js:243 — <EntityFetcherProvider> is mounted with no baseUrl` while the source
14+
it was built from, `client/src/main.tsx:26`, passes `baseUrl="/api"` correctly. The finding
15+
named a 711KB bundle: a file nobody edits, pointing away from the fix, on a project that had
16+
already done the right thing — and it was the FIRST line of the advisory, spending the
17+
scanner's whole false-positive budget on a project doing nothing wrong.
18+
19+
The ignore lists could not have caught it. They name `dist`, `build`, `out`, `.next`,
20+
`.output` — and this project builds to `public/`. So the rule is a property of the ARTIFACT
21+
instead: `looksBundled()` (new, `lib/authored-source.ts`) treats a file with any line of
22+
5,000+ characters as build output, an order of magnitude above the longest line a human
23+
writes and an order below what minification produces. Both scanners apply it — the base-URL
24+
advisory, which had no size guard at all, and the anti-pattern pass, whose existing 512KB
25+
gate is about scan COST and lets a 300KB minified bundle straight through.
26+
27+
The two scanners keep their own walkers (they diverge deliberately); what they now share is
28+
the one judgment about what counts as authored source.
29+
30+
1031
### Fixed — `meta upgrade` printed one retirement's rationale once per OCCURRENCE
1132

1233
An adopter estate carrying 24 `@forge*` attributes got ~96 lines of output for one

server/typescript/packages/cli/src/lib/anti-patterns.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import { readdirSync, readFileSync, statSync } from "node:fs";
1919
import { join } from "node:path";
2020
import { relPosix } from "./rel-posix.js";
21+
import { looksBundled } from "./authored-source.js";
2122

2223
export interface AntiPatternFinding {
2324
file: string; // path relative to the scan root (posix-ish, sep-normalized)
@@ -259,6 +260,10 @@ function walk(dir: string, root: string, ignore: readonly RegExp[], acc: AntiPat
259260
}
260261
// Skip MetaObjects-generated output — it legitimately contains AVG/CHECK etc.
261262
if (text.slice(0, 600).includes(GENERATED_MARKER)) continue;
263+
// …and skip a BUNDLE, which is nobody's source. The 512KB gate above is about cost, not
264+
// authorship: a 300KB minified file passes it while being just as unfixable. Same rule
265+
// the base-URL advisory applies, from the same module. See lib/authored-source.ts.
266+
if (looksBundled(text)) continue;
262267
const isSql = e.name.endsWith(".sql");
263268
const lines = text.split("\n");
264269
for (let i = 0; i < lines.length; i++) {
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* Is this file text a HUMAN wrote, or a bundler's output?
3+
*
4+
* Both advisory scanners — the base-URL one and the anti-pattern one — exist to teach the
5+
* author of a file something about the file. A bundle is not authored: nobody can act on a
6+
* finding in it, the fix belongs in the source it was built from, and naming a generated
7+
* artifact spends the scanners' whole false-positive budget in one line.
8+
*
9+
* The rule is a property of the ARTIFACT, not of a directory name. An estate proved why:
10+
* its build output is `public/`, which no ignore list would have guessed (the lists cover
11+
* `dist`, `build`, `.next`, `.output`, `out`), so `meta verify` reported
12+
* `public/main.js:243 — <EntityFetcherProvider> is mounted with no baseUrl` while
13+
* `client/src/main.tsx:26` — the source, the file a person edits — passes `baseUrl="/api"`
14+
* correctly. Adding `public` to a list would have fixed that estate and nobody else.
15+
*
16+
* The two scanners keep their own walkers, deliberately (they diverge on eight axes and
17+
* unifying them needs ~6 options for two callers). What they share is this judgment, so it
18+
* lives here once rather than being decided twice.
19+
*/
20+
21+
/**
22+
* Bundlers emit lines in the thousands-to-hundreds-of-thousands of characters; the estate
23+
* bundle that produced the false positive has a longest line of 266,226. Authored source
24+
* does not reach 5,000 — the threshold sits an order of magnitude above the longest line
25+
* any human writes and an order of magnitude below what minification produces, so it is a
26+
* wide gap rather than a tuned one.
27+
*
28+
* NOT `>= 512KB`, which `anti-patterns.ts` already applies for a different reason (cost):
29+
* that gate is about how much work a scan does, and a 300KB bundle sails through it while
30+
* being exactly as unauthored as a 700KB one.
31+
*/
32+
const BUNDLED_LINE_LENGTH = 5000;
33+
34+
/** True when `text` looks like build output rather than source someone edits. */
35+
export function looksBundled(text: string): boolean {
36+
// Scan for a long line without splitting the whole file: a 700KB bundle would otherwise
37+
// allocate an array of it, and this runs per file on every `meta verify` / `meta gen`.
38+
let lineStart = 0;
39+
for (let i = 0; i < text.length; i++) {
40+
if (text.charCodeAt(i) !== 10 /* \n */) continue;
41+
if (i - lineStart >= BUNDLED_LINE_LENGTH) return true;
42+
lineStart = i + 1;
43+
}
44+
return text.length - lineStart >= BUNDLED_LINE_LENGTH;
45+
}

server/typescript/packages/cli/src/lib/base-url-advisory.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { readdirSync, readFileSync } from "node:fs";
22
import { join } from "node:path";
33
import { relPosix } from "./rel-posix.js";
4+
import { looksBundled } from "./authored-source.js";
45

56
/**
67
* F52 — a provider mounted with no `baseUrl` on a project whose `apiPrefix` is not empty.
@@ -155,6 +156,10 @@ function walk(dir: string, root: string, acc: BaseUrlFinding[], apiPrefix: strin
155156
let src: string;
156157
try { src = readFileSync(abs, "utf8"); } catch { continue; }
157158
if (!src.includes("EntityFetcher")) continue;
159+
// A bundle is not authored source: the fix belongs in the file it was built FROM, and
160+
// an estate whose output directory is `public/` proved that no ignore list guesses the
161+
// name. See lib/authored-source.ts.
162+
if (looksBundled(src)) continue;
158163
acc.push(...scanFile(relPosix(root, abs), src, apiPrefix));
159164
}
160165
}

server/typescript/packages/cli/test/anti-patterns.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,3 +367,30 @@ describe("scanSourceForAntiPatterns — project-declared ignore globs", () => {
367367
}
368368
});
369369
});
370+
371+
describe("build output is not authored source", () => {
372+
const REDUCE = "const avg = ratings.reduce((acc, r) => acc + r.value, 0) / ratings.length;";
373+
374+
test("a bundle is not scanned, however small", () => {
375+
// The existing 512KB gate is about COST — a 300KB minified file passes it and is just
376+
// as unfixable, because the fix belongs in the source it was built from. Same rule the
377+
// base-URL advisory applies, from the same module. Found on an estate whose build
378+
// output is `public/`, a directory name no ignore list guesses.
379+
const root = scaffold({ "public/main.js": `var a=1;${"b".repeat(6000)}\n${REDUCE}\n` });
380+
try {
381+
expect(scanSourceForAntiPatterns(root)).toEqual([]);
382+
} finally {
383+
rmSync(root, { recursive: true, force: true });
384+
}
385+
});
386+
387+
test("…and the same line in real source still IS reported", () => {
388+
// Non-vacuous: identical content, minus the bundling.
389+
const root = scaffold({ "public/main.js": `var a=1;\n${REDUCE}\n` });
390+
try {
391+
expect(scanSourceForAntiPatterns(root).length).toBeGreaterThan(0);
392+
} finally {
393+
rmSync(root, { recursive: true, force: true });
394+
}
395+
});
396+
});

server/typescript/packages/cli/test/base-url-advisory.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,3 +158,25 @@ describe("scanForMissingBaseUrl — what is code and what is not", () => {
158158
expect(scanForMissingBaseUrl(root, "/api")).toHaveLength(1);
159159
});
160160
});
161+
162+
describe("build output is not authored source", () => {
163+
test("a bundle carrying the provider is not reported", async () => {
164+
// Found on an estate whose build output is `public/` — a directory no ignore list
165+
// guesses. `meta verify` named `public/main.js:243` while the SOURCE,
166+
// `client/src/main.tsx`, passes baseUrl correctly: a finding in a file nobody edits,
167+
// pointing away from the fix, on a project that had already done the right thing.
168+
const root = await project({
169+
"client/src/main.tsx": `<EntityFetcherProvider fetcher={fetcher} baseUrl="/api">`,
170+
"public/main.js": `var a=1;${"b".repeat(6000)}\n<EntityFetcherProvider fetcher={f}>\n`,
171+
});
172+
expect(scanForMissingBaseUrl(root, "/api")).toEqual([]);
173+
});
174+
175+
test("…and the same provider in real source still IS reported", async () => {
176+
// The gate has to stay non-vacuous: it is the same file content, minus the bundling.
177+
const root = await project({
178+
"public/main.js": `var a=1;\n<EntityFetcherProvider fetcher={f}>\n`,
179+
});
180+
expect(scanForMissingBaseUrl(root, "/api")).toHaveLength(1);
181+
});
182+
});
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { describe, test, expect } from "bun:test";
2+
import { looksBundled } from "../../src/lib/authored-source.js";
3+
4+
describe("looksBundled", () => {
5+
test("ordinary source is not bundled", () => {
6+
expect(looksBundled(`export function f() {\n return 1;\n}\n`)).toBe(false);
7+
});
8+
9+
test("a long-but-human line is not bundled", () => {
10+
// A 900-char line is unpleasant and entirely possible in hand-written code.
11+
expect(looksBundled(`const x = "${"a".repeat(900)}";\n`)).toBe(false);
12+
});
13+
14+
test("a minified line is bundled", () => {
15+
expect(looksBundled(`var a=1;${"b".repeat(6000)}\n`)).toBe(true);
16+
});
17+
18+
test("the long line is found anywhere in the file, not just first", () => {
19+
expect(looksBundled(`// header\nconst a = 1;\n${"z".repeat(6000)}\nconst b = 2;\n`)).toBe(true);
20+
});
21+
22+
test("a final line with no trailing newline still counts", () => {
23+
expect(looksBundled(`// header\n${"z".repeat(6000)}`)).toBe(true);
24+
});
25+
26+
test("an empty file is not bundled", () => {
27+
expect(looksBundled("")).toBe(false);
28+
});
29+
});

0 commit comments

Comments
 (0)