-
Notifications
You must be signed in to change notification settings - Fork 0
feat(ci): fail-loud client-bundle leak gate + PR-comment size report (#3670) #3676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
c3ebad6
fix(client): source getEnv from its leaf so the client barrel drops t…
mattboon 63e2824
test(server): fail-loud client-bundle leak gate + PR-comment size rep…
mattboon a386c62
fix(lint): correct import-map resolution + track the sandbox worker leak
mattboon f4d148b
fix(lint): drop erased type-only clauses from the client graph + scop…
kojiwakayama 7d3b895
Merge remote-tracking branch 'origin/main' into feat/client-bundle-le…
kojiwakayama c5e9285
fix(lint): keep value edges that a type-shaped clause still ships
kojiwakayama File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| name: Client bundle report | ||
|
|
||
| # Posts (and keeps updated) a sticky PR comment with the client-bundle boundary | ||
| # report — module count, source size, and any server-module leaks per browser | ||
| # entrypoint. This is the informational half of the #3670 gate; the hard | ||
| # fail-fast check runs in the `ci (lint)` job via `deno task lint:client-bundle`. | ||
| on: | ||
| pull_request: | ||
| branches: [main] | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
||
| concurrency: | ||
| group: client-bundle-report-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| report: | ||
| # Same-repo PRs only: forks get a read-only token that cannot comment, and | ||
| # the repo already scopes CI to same-repo PRs. | ||
| if: ${{ github.event.pull_request.head.repo.full_name == github.repository }} | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| persist-credentials: false | ||
| - uses: ./.github/actions/setup-deno | ||
| timeout-minutes: 5 | ||
| - name: Compute client bundle report | ||
| run: deno task client-bundle:report > client-bundle-report.md | ||
| - name: Post sticky PR comment | ||
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 | ||
| with: | ||
| script: | | ||
| const fs = require("fs"); | ||
| const marker = "<!-- client-bundle-report -->"; | ||
| const body = fs.readFileSync("client-bundle-report.md", "utf8"); | ||
|
|
||
| const comments = await github.rest.issues.listComments({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: context.payload.pull_request.number, | ||
| }); | ||
| // Only ever update our own comment: another bot that quotes the | ||
| // marker (a review bot echoing this report, say) must not be | ||
| // overwritten. github-script runs on GITHUB_TOKEN, so the sticky | ||
| // comment is always authored by github-actions[bot]. | ||
| const existing = comments.data.find( | ||
| (c) => | ||
| c.user?.login === "github-actions[bot]" && | ||
| c.body.includes(marker), | ||
| ); | ||
|
|
||
| if (existing) { | ||
| await github.rest.issues.updateComment({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| comment_id: existing.id, | ||
| body, | ||
| }); | ||
| } else { | ||
| await github.rest.issues.createComment({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: context.payload.pull_request.number, | ||
| body, | ||
| }); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,263 @@ | ||
| /** | ||
| * Fail-loud client-bundle boundary lint (#3670). | ||
| * | ||
| * Walks the static import graph of each browser entrypoint and fails CI when a | ||
| * server module reaches the client bundle. Two tiers: | ||
| * | ||
| * - CRITICAL (the #3661 crash class — runtime filesystem adapters): must be | ||
| * zero, always. Never baselineable. | ||
| * - SERVER-ONLY (the broader leak surface — `adapters/fs/veryfront/*`, | ||
| * `veryfront-api-client`, `compat/process/command`, …): ratcheted against a | ||
| * baseline so a *new* leak fails fast, while the pre-existing debt stays | ||
| * visible until it is burned down. Regenerate with `--update`. | ||
| * | ||
| * It also reports the client-graph size (modules + source bytes) so a size | ||
| * regression — the tell-tale of a server barrel sneaking in — surfaces on the | ||
| * PR as an annotation and a sticky comment for a human or agent to pick up. | ||
| * | ||
| * Usage: | ||
| * deno run --allow-read scripts/lint/audit-client-bundle.ts # check (fail-fast) | ||
| * deno run --allow-read --allow-write scripts/lint/audit-client-bundle.ts --update | ||
| * deno run --allow-read scripts/lint/audit-client-bundle.ts --markdown # PR-comment body | ||
| */ | ||
|
|
||
| import { | ||
| type ClientGraph, | ||
| collectClientGraph, | ||
| createRealReader, | ||
| findServerOnlyLeaks, | ||
| loadImportMap, | ||
| summarizeGraph, | ||
| traceLeak, | ||
| } from "./client-bundle-graph.ts"; | ||
|
|
||
| const ROOT = new URL("../../", import.meta.url); | ||
| const BASELINE_URL = new URL("client-bundle-baseline.json", import.meta.url); | ||
|
|
||
| /** Browser entrypoints whose client graph must stay server-free. */ | ||
| const ENTRYPOINTS: ReadonlyArray<{ label: string; entry: string }> = [ | ||
| { label: "veryfront (browser/SSR barrel)", entry: "src/index.client.ts" }, | ||
| ]; | ||
|
|
||
| /** The #3661 crash class: reaching any of these in a browser aborts hydration. Never baselineable. */ | ||
| const CRITICAL_PATTERNS: readonly RegExp[] = [ | ||
| /\/platform\/adapters\/runtime\/[^/]+\/(adapter|filesystem-adapter)\.ts$/, | ||
| /\/platform\/adapters\/runtime\/shared\/node-filesystem-adapter\.ts$/, | ||
| ]; | ||
|
|
||
| /** | ||
| * Warn (not fail) once the client graph exceeds this many modules. The graph is | ||
| * 451 today; a broad server barrel leak jumps it into four figures. A soft | ||
| * ceiling flags that jump for review without inventing a hard byte budget. | ||
| * | ||
| * Deliberately a warning rather than a gate. The leak check above already fails | ||
| * the build on a server module reaching the client, which is the condition worth | ||
| * blocking on; this count is a canary for the shape of a regression the | ||
| * pattern list has not learned yet. Turning it into a hard budget needs a | ||
| * ratchet that records the count per entrypoint and only lets it fall — a | ||
| * fixed ceiling either sits so close to today's graph that ordinary growth | ||
| * trips it, or so far above that it never fires. | ||
| */ | ||
| const SIZE_WARN_MODULES = 550; | ||
|
kojiwakayama marked this conversation as resolved.
|
||
|
|
||
| interface Baseline { | ||
| readonly note: string; | ||
| readonly entrypoints: Record<string, string[]>; | ||
| } | ||
|
|
||
| interface EntryReport { | ||
| label: string; | ||
| entry: string; | ||
| moduleCount: number; | ||
| byteCount: number; | ||
| critical: string[]; | ||
| newLeaks: string[]; | ||
| knownLeaks: string[]; | ||
| fixedLeaks: string[]; | ||
| graph: ClientGraph; | ||
| } | ||
|
|
||
| function isCritical(path: string): boolean { | ||
| return CRITICAL_PATTERNS.some((pattern) => pattern.test("/" + path)); | ||
| } | ||
|
|
||
| async function readBaseline(): Promise<Baseline> { | ||
| try { | ||
| return JSON.parse(await Deno.readTextFile(BASELINE_URL)) as Baseline; | ||
| } catch { | ||
| return { note: "", entrypoints: {} }; | ||
| } | ||
| } | ||
|
|
||
| async function analyze(baseline: Baseline): Promise<EntryReport[]> { | ||
| const importMap = await loadImportMap(ROOT); | ||
| const reader = createRealReader(ROOT); | ||
| const reports: EntryReport[] = []; | ||
|
|
||
| for (const { label, entry } of ENTRYPOINTS) { | ||
| const graph = await collectClientGraph(entry, importMap, reader); | ||
| const { moduleCount, byteCount } = summarizeGraph(graph); | ||
| const leaks = findServerOnlyLeaks(graph); | ||
| const allowed = new Set(baseline.entrypoints[entry] ?? []); | ||
|
|
||
| const critical = leaks.filter(isCritical); | ||
| const newLeaks = leaks.filter((leak) => | ||
| !isCritical(leak) && !allowed.has(leak) | ||
| ); | ||
| const knownLeaks = leaks.filter((leak) => | ||
| !isCritical(leak) && allowed.has(leak) | ||
| ); | ||
| const present = new Set(leaks); | ||
| const fixedLeaks = [...allowed].filter((leak) => !present.has(leak)); | ||
|
|
||
| reports.push({ | ||
| label, | ||
| entry, | ||
| moduleCount, | ||
| byteCount, | ||
| critical, | ||
| newLeaks, | ||
| knownLeaks, | ||
| fixedLeaks, | ||
| graph, | ||
| }); | ||
| } | ||
| return reports; | ||
| } | ||
|
|
||
| function kib(bytes: number): string { | ||
| return `${(bytes / 1024).toFixed(0)} KiB`; | ||
| } | ||
|
|
||
| function renderMarkdown(reports: EntryReport[]): string { | ||
| const rows = reports.map((r) => { | ||
| const leakCell = r.critical.length > 0 || r.newLeaks.length > 0 | ||
| ? `❌ ${r.critical.length + r.newLeaks.length} new` | ||
| : r.knownLeaks.length > 0 | ||
| ? `⚠️ ${r.knownLeaks.length} known` | ||
| : "✅ 0"; | ||
| const sizeFlag = r.moduleCount > SIZE_WARN_MODULES ? " ⚠️" : ""; | ||
| return `| \`${r.entry}\` | ${r.moduleCount}${sizeFlag} | ${ | ||
| kib(r.byteCount) | ||
| } | ${leakCell} |`; | ||
| }); | ||
| return [ | ||
| "<!-- client-bundle-report -->", | ||
| "### 📦 Client bundle boundary", | ||
| "", | ||
| "| Entrypoint | Modules | Source size | Server leaks |", | ||
| "| --- | ---: | ---: | ---: |", | ||
| ...rows, | ||
| "", | ||
| "_A server module in a client graph aborts hydration in the browser. New leaks fail CI;" + | ||
| " known leaks are tracked in `scripts/lint/client-bundle-baseline.json` to burn down._", | ||
| ].join("\n"); | ||
| } | ||
|
|
||
| function reportToJson(reports: EntryReport[]) { | ||
| return reports.map((r) => ({ | ||
| entry: r.entry, | ||
| moduleCount: r.moduleCount, | ||
| byteCount: r.byteCount, | ||
| critical: r.critical, | ||
| newLeaks: r.newLeaks, | ||
| knownLeaks: r.knownLeaks.length, | ||
| })); | ||
| } | ||
|
|
||
| async function main(): Promise<void> { | ||
| const args = new Set(Deno.args); | ||
|
|
||
| if (args.has("--update")) { | ||
| const reports = await analyze({ note: "", entrypoints: {} }); | ||
| const entrypoints: Record<string, string[]> = {}; | ||
| for (const r of reports) { | ||
| entrypoints[r.entry] = [...r.knownLeaks, ...r.newLeaks].filter((l) => | ||
| !isCritical(l) | ||
| ).toSorted(); | ||
| } | ||
| const baseline: Baseline = { | ||
| note: | ||
| "Known server modules reachable from a browser entrypoint (#3670). Burn down, never grow. " + | ||
| "Regenerate with: deno run --allow-read --allow-write scripts/lint/audit-client-bundle.ts --update", | ||
| entrypoints, | ||
| }; | ||
| await Deno.writeTextFile( | ||
| BASELINE_URL, | ||
| JSON.stringify(baseline, null, 2) + "\n", | ||
| ); | ||
| console.log(`Wrote baseline for ${reports.length} entrypoint(s).`); | ||
| return; | ||
| } | ||
|
|
||
| const baseline = await readBaseline(); | ||
| const reports = await analyze(baseline); | ||
|
|
||
| if (args.has("--markdown")) { | ||
| console.log(renderMarkdown(reports)); | ||
| return; | ||
| } | ||
|
|
||
| // Machine-readable summary line for agents / downstream tooling. | ||
| console.log("CLIENT_BUNDLE_REPORT " + JSON.stringify(reportToJson(reports))); | ||
|
|
||
| let failed = false; | ||
| for (const r of reports) { | ||
| console.log( | ||
| `${r.entry}: ${r.moduleCount} modules, ${kib(r.byteCount)} source` + | ||
| (r.knownLeaks.length | ||
| ? `, ${r.knownLeaks.length} known server leak(s)` | ||
| : ""), | ||
| ); | ||
|
|
||
| for (const leak of r.critical) { | ||
| failed = true; | ||
| console.error( | ||
| `::error file=${r.entry}::CRITICAL: the client graph reaches the server runtime adapter ` + | ||
| `${leak} (the #3661 hydration crash). Import chain: ${ | ||
| traceLeak(r.graph, leak) | ||
| }`, | ||
| ); | ||
| } | ||
| for (const leak of r.newLeaks) { | ||
| failed = true; | ||
| console.error( | ||
| `::error file=${r.entry}::New server module in the client bundle: ${leak}. ` + | ||
| `Import chain: ${ | ||
| traceLeak(r.graph, leak) | ||
| }. Break the import, or (only if intentional) ` + | ||
| `re-baseline with \`deno task lint:client-bundle:update\`.`, | ||
| ); | ||
| } | ||
| if (r.fixedLeaks.length > 0) { | ||
| console.log( | ||
| `::warning file=${r.entry}::${r.fixedLeaks.length} baselined leak(s) are gone — ` + | ||
| `run \`deno task lint:client-bundle:update\` to lock in the improvement: ${ | ||
| r.fixedLeaks.join(", ") | ||
| }`, | ||
| ); | ||
| } | ||
| if (r.moduleCount > SIZE_WARN_MODULES) { | ||
| console.log( | ||
| `::warning file=${r.entry}::Client graph grew to ${r.moduleCount} modules ` + | ||
| `(${ | ||
| kib(r.byteCount) | ||
| }) — a server barrel may have leaked in; investigate before it ships.`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| if (failed) { | ||
| console.error( | ||
| "\nServer module(s) reached a client bundle. See the annotations above.", | ||
| ); | ||
| Deno.exit(1); | ||
| } | ||
| console.log( | ||
| "\nClient bundle boundary verified: no critical or new server leaks.", | ||
| ); | ||
| } | ||
|
|
||
| if (import.meta.main) { | ||
| await main(); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.