Skip to content

ci: relax file-size ceilings by surface - #6485

Merged
wesbillman merged 9 commits into
mainfrom
carl/relax-file-size-ceilings
Sep 1, 2026
Merged

ci: relax file-size ceilings by surface#6485
wesbillman merged 9 commits into
mainfrom
carl/relax-file-size-ceilings

Conversation

@wesbillman

Copy link
Copy Markdown
Collaborator

Summary

  • raise Desktop Rust's differential file-size ceiling from 1,000 to 1,500 lines
  • raise Desktop frontend and Mobile ceilings from 1,000 to 1,200 lines
  • keep Web at 1,000 lines and preserve the existing no-growth ratchet above each ceiling

Why

The flat 1,000-line limit is forcing mechanical trimming in Desktop and Mobile even for small cohesive changes. Surface-specific ceilings relieve that pressure without granting Web or every authored component a blanket 2,000-line budget.

Testing

  • just file-size-check
  • cd desktop && pnpm exec biome check scripts/check-file-sizes.mjs
  • node --check mobile/scripts/check-file-sizes.mjs
  • git diff HEAD^ --check

@wesbillman
wesbillman requested a review from a team as a code owner August 21, 2026 15:45
wpfleger96
wpfleger96 previously approved these changes Aug 21, 2026
@wesbillman
wesbillman enabled auto-merge (squash) August 21, 2026 15:48

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: 4e3c9e619c93dd26677b392ad1f8cf0d12c8f855..e9074a7f22e75219f789091ea152fff5a0fcf226 (exact head e9074a7f22e75219f789091ea152fff5a0fcf226)
Risk: low runtime risk, but this PR changes an always-on CI policy whose numeric surface contract currently has no automated regression coverage.

Behavior/contracts traced: Desktop frontend/CSS → 1,200 lines; Desktop Rust including src-tauri/crates → 1,500; Mobile → 1,200; Web remains 1,000; files already above a configured ceiling retain the no-growth ratchet through max(configured ceiling, base line count).

Blocking finding

  • The changed surface ceilings are not asserted by any checked-in test. The production values live in desktop/scripts/check-file-sizes.mjs:8-59 and mobile/scripts/check-file-sizes.mjs:8-15, while scripts/check-file-sizes-core.test.mjs:56-83,102-118 only exercises the generic core with caller-supplied maxLines: 1000. Mutation proof changed Desktop Rust to 1,200, Desktop frontend to 1,500, and Mobile to 2,000; the complete just file-size-check still exited 0. Consequently, CI cannot detect a wrong ceiling in the policy this PR introduces. Add a checked-in contract/integration test that proves ceiling passes and ceiling+1 fails for Desktop Rust, Desktop frontend, Mobile, and Web, plus inherited over-ceiling growth rejection.

Validation at matching clean HEAD

  • CHECK_FILE_SIZES_BASE=4e3c9e619c93dd26677b392ad1f8cf0d12c8f855 just file-size-check — PASS (6/6 generic tests and all surface scripts).
  • Boundary probes — PASS: Desktop Rust 1500/1501, Desktop frontend 1200/1201, Mobile 1200/1201, governed Web 1000/1001, and inherited over-ceiling Rust growth.
  • cd desktop && pnpm exec biome check scripts/check-file-sizes.mjs — PASS.
  • node --check mobile/scripts/check-file-sizes.mjs and git diff ... --check — PASS.
  • Wrong-ceiling mutation plus full just file-size-check — unexpectedly PASS, establishing the coverage gap; mutations were restored and the review tree was clean.

Manual/native evidence: not warranted; no runtime or user-visible surface changed.

Residual risk: Desktop Core was still pending at the last lane refresh. No completed check was failing, but green completion would not exercise the missing policy assertions above.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: 4e3c9e619c93dd26677b392ad1f8cf0d12c8f855..e9074a7f22e75219f789091ea152fff5a0fcf226 (exact head e9074a7f22e75219f789091ea152fff5a0fcf226)
Risk: low — this changes CI policy only; no runtime, UI, accessibility, persistence, auth, packaging, or release-artifact behavior changes.

Behavior/contracts traced: Desktop frontend/CSS now permits 1,200 lines and Desktop Rust (including src-tauri/crates) 1,500 (desktop/scripts/check-file-sizes.mjs:8-59); Mobile now permits 1,200 (mobile/scripts/check-file-sizes.mjs:8-15); Web remains at 1,000 (web/scripts/check-file-sizes.mjs:8-25). The shared differential rule still uses max(configured ceiling, base line count) and rejects candidate growth above that limit (scripts/check-file-sizes-core.mjs:31-40,147-159), so inherited over-ceiling files remain no-growth ratcheted. Justfile:100-104 runs the core tests plus each surface script, and .github/workflows/ci.yml:76-91 invokes that gate unconditionally.

Blocking finding: The changed surface policy has no automated contract test capable of detecting an incorrect surface ceiling. The existing shared tests exercise only generic caller-supplied maxLines: 1000 (scripts/check-file-sizes-core.test.mjs:56-83,102-118); they never assert Desktop Rust=1,500, Desktop frontend=1,200, Mobile=1,200, or Web=1,000. Mutation proof changed Desktop Rust to 1,200, Desktop frontend to 1,500, and Mobile to 2,000, yet the full just file-size-check still exited 0. Because policy is the entire behavior introduced here, add a surface-level contract test proving ceiling passes and ceiling+1 fails for each surface, Web stays at 1,000, and growth of an inherited over-ceiling file fails.

Validation at matching clean HEAD:

  • CHECK_FILE_SIZES_BASE=4e3c9e619c93dd26677b392ad1f8cf0d12c8f855 just file-size-check — PASS (6/6 generic tests plus Desktop/Web/Mobile scripts).
  • Temporary governed-file boundary probes — expected pass/fail at Desktop frontend 1,200/1,201, Desktop Rust 1,500/1,501, Mobile 1,200/1,201, and Web 1,000/1,001; inherited over-ceiling Rust file plus one line rejected.
  • cd desktop && pnpm exec biome check scripts/check-file-sizes.mjs — PASS.
  • node --check mobile/scripts/check-file-sizes.mjs and git diff 4e3c9e619c93dd26677b392ad1f8cf0d12c8f855..HEAD --check — PASS.
  • Wrong-ceiling mutation plus full just file-size-check — unexpected PASS, establishing the coverage gap; mutations were restored and the tree was clean.

Manual/native evidence: Not warranted for a CI-policy-only change.

Residual risk: Desktop Core remained in progress at final review refresh; no checks were failed. The missing policy regression test is independently blocking.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: 4e3c9e619c93dd26677b392ad1f8cf0d12c8f855..07624ace3fea4c6dc5d6f189634c2fdb71f02c7a (delta from prior reviewed head e9074a7f22e75219f789091ea152fff5a0fcf226)
Risk: low runtime risk, but the new test still does not guard the effective always-on CI policy it claims to lock.

Blocking finding

  • scripts/check-file-sizes-core.test.mjs:56-75 reads each entrypoint as text and matches only the ceiling constant declarations. Production enforcement consumes the separately wired rules[*].maxLines values (desktop/scripts/check-file-sizes.mjs:11-59, mobile/scripts/check-file-sizes.mjs:10-16, web/scripts/check-file-sizes.mjs:10-26; consumed by scripts/check-file-sizes-core.mjs:138-151). Two independent mutations proved the gap: (1) leaving all declarations unchanged while rewiring every production maxLines to incorrect numeric values still left the new test green, and (2) rewiring both Desktop Rust roots from the 1,500 constant to the 1,200 frontend constant left the complete just file-size-check green. Thus the effective surface ceiling can regress while CI reports that the intended ceilings are locked.

Use behavior-level coverage through the actual production policy: an import-safe exported rule/policy module asserted against evaluateFileSize, or fixture-repository entrypoint tests proving ceiling passes and ceiling+1 fails for Desktop Rust, Desktop frontend, Mobile, and unchanged Web. The inherited over-ceiling hold/shrink/growth test at scripts/check-file-sizes-core.test.mjs:123-140 is effective at the shared-policy layer; mutation of allowedLineCount made it fail, so that portion need not be duplicated gratuitously unless using an end-to-end fixture approach.

Validation at matching clean HEAD

  • CHECK_FILE_SIZES_BASE=4e3c9e619c93dd26677b392ad1f8cf0d12c8f855 just file-size-check — PASS (7/7 tests and all surface entrypoints).
  • node --check scripts/check-file-sizes-core.test.mjs — PASS.
  • git diff e9074a7f22e75219f789091ea152fff5a0fcf226..HEAD --check — PASS.
  • Wrong production-wiring mutations with declarations unchanged — unexpectedly PASS, preserving the original blind spot; mutations restored and trees clean.
  • Live PR head remained 07624ace3fea4c6dc5d6f189634c2fdb71f02c7a; all reported GitHub checks were complete and green/skipped as expected.

Manual/native evidence: not warranted; this delta changes CI tests only and no shipped runtime/UI/native behavior.

Residual risk: no runtime surface was exercised. The unresolved risk is specifically that future production rule wiring can diverge from the declarations while this source-regex test remains green.

@wesbillman
wesbillman force-pushed the carl/relax-file-size-ceilings branch from 07624ac to b4d955f Compare August 24, 2026 22:30
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Addressed in b4d955f45d241ce512f0e4d7e77db6321ac32cb9.

The entrypoints now consume import-safe production policy modules, and the policy test imports those same rule arrays. It asserts the complete root-to-ceiling map for Desktop, Mobile, and Web, then proves each rule allows its ceiling and rejects ceiling + 1. The existing inherited-over-ceiling test remains unchanged.

Verification on the pushed head:

  • focused policy suite: 7/7 pass
  • just file-size-check: pass
  • reviewer mutation reproduced against the fix: changing Desktop Rust’s production ceiling from 1,500 to 1,200 makes the policy suite fail on both Rust roots
  • pre-push hooks: pass, including file-size check, Desktop check/typecheck/tests (5,432/5,432), and Mobile checks

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — request changes at head b4d955f45d241ce512f0e4d7e77db6321ac32cb9 (base 30d2fc52f96138311f2006627ffc1a6d5ff1865b).

The numeric boundary coverage is materially improved: all four surface ceilings are now mutation-sensitive, Web remains at 1,000, and inherited over-ceiling hold/shrink/grow behavior is covered. However, the production-wiring blind spot remains.

P2 — Test the policy that each production entrypoint actually consumes

Production selects the first matching rule and filters by its extensions (scripts/check-file-sizes-core.mjs:40-42,138-150). Each surface entrypoint independently imports its policy (for example, desktop/scripts/check-file-sizes.mjs:4,9-12). The new test instead imports policy modules directly and reduces them to root→ceiling maps (scripts/check-file-sizes-core.test.mjs:7-9,59-116). That oracle does not preserve entrypoint imports, extensions, or first-match precedence.

We mutation-proved the resulting false negatives against the complete checked-in gate:

  1. Repointing Desktop's production entrypoint to Mobile/Web policy left CHECK_FILE_SIZES_BASE=30d2fc52f96138311f2006627ffc1a6d5ff1865b just file-size-check green.
  2. Changing Desktop Rust extension sets from .rs to .ts left the same gate green.
  3. Prepending a shadowing duplicate src-tauri/src rule with a different ceiling left the gate green; production uses the first rule while the test's Map collapses duplicate roots.

In each case production silently governs the wrong files or applies the wrong ceiling while the policy test still claims the contract is locked. This is the same failure class the behavior-level replacement is intended to eliminate.

Author action: add checked-in coverage through each actual surface entrypoint, or export and test the exact production rule-selection/configuration path. The coverage must preserve and assert entrypoint policy identity, roots, extensions, first-match behavior, and ceilings for Desktop Rust (both roots), Desktop frontend/CSS, Mobile Dart, and Web TS/TSX. Please mutation-prove that wrong entrypoint import, wrong extension, and a shadowing duplicate rule fail.

Verification owner: reviewer will rerun those three mutations plus the clean full gate on the replacement head.

Verified at this head

  • Clean full file-size gate: 7/7 policy tests plus Desktop/Web/Mobile entrypoints pass.
  • Mutating Desktop Rust 1,500, Desktop frontend 1,200, Mobile 1,200, or Web 1,000 makes the policy test fail.
  • Removing inherited over-ceiling behavior makes its regression test fail.
  • Changed .mjs syntax checks, Desktop Biome checks, and diff whitespace checks pass.
  • Hosted checks are complete and green/skipped as expected; no PR-caused red gate was found.
  • No native/UI run is warranted for this CI-policy-only change.

Non-blocking: AGENTS.md:586-593 accurately describes the surface-specific policy; two historical Desktop comments still mention a 1,000-line ceiling, but they do not define current enforcement.

Any new head invalidates this verdict.

wesbillman and others added 4 commits August 25, 2026 10:53
Give Desktop and Mobile enough headroom to avoid mechanical line trimming
while preserving the tighter Web limit and the existing differential ratchet.

Co-authored-by: Wes <wesbillman@users.noreply.github.com>
Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Keep the chosen Desktop, Mobile, and Web limits explicit in the lightweight
policy suite without duplicating the ratchet boundary matrix.

Co-authored-by: Wes <wesbillman@users.noreply.github.com>
Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Move each surface rule set into an import-safe policy module and assert the exact production rules at their pass/fail boundaries.

Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Export the exact policy each entrypoint executes and assert ordered roots, extensions, ceilings, and boundaries so import and precedence mistakes cannot pass.

Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
@wesbillman
wesbillman force-pushed the carl/relax-file-size-ceilings branch from b4d955f to 8b36ffc Compare August 25, 2026 16:56
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Addressed in 8b36ffcaf4f238cfd1400b29a0340cad7f172c03.

Each production entrypoint now exports the exact policy object it passes to runFileSizeCheck, and only executes when invoked as the main script. The test imports those entrypoints, not their policy modules, and asserts every rule as an ordered (root, extensions, maxLines) tuple before exercising ceiling and ceiling+1 behavior.

Mutation verification on this head:

  • repointing Desktop's entrypoint to Mobile policy fails
  • changing Desktop Rust extensions from .rs to .ts fails
  • prepending a shadowing duplicate Desktop Rust rule fails

Clean verification:

  • focused policy suite: 7/7 pass
  • just file-size-check: pass
  • pre-push hooks: pass, including file-size check, Desktop check/typecheck/tests (5,476/5,476), and Mobile checks

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: base 12f3fea26e4c638a5fae20dce1ec0876e3bbca41, exact head 8b36ffcaf4f238cfd1400b29a0340cad7f172c03
Risk: low runtime risk, but the changed always-on CI policy can still report success while a production surface performs no file-size enforcement.

The previous production-policy defects are closed: mutation of Desktop's policy import, Rust extension, or first-match rule precedence now fails the checked-in suite. Boundary probes also behaved correctly across Desktop Rust (1,500), Desktop frontend/CSS (1,200), Mobile Dart (1,200), unchanged Web TS/TSX (1,000), and inherited over-ceiling shrink/growth behavior.

P2 — Test that each production entrypoint actually executes its exported policy

The policy test imports and inspects each exported policy (scripts/check-file-sizes-core.test.mjs:7-9,59-119), but production execution remains separate guarded code (desktop/scripts/check-file-sizes.mjs:15-16, with the same Mobile/Web shape). The suite therefore proves the object exposed by an entrypoint, not that the entrypoint executes that object.

Two mutations independently demonstrated the bypass against the complete CHECK_FILE_SIZES_BASE=12f3fea26e4c638a5fae20dce1ec0876e3bbca41 just file-size-check gate:

  1. Changing Desktop's direct-execution guard to if (false) left the gate green (rc 0): 7/7 tests passed and the Desktop command silently did nothing.
  2. Changing Desktop's invocation to runFileSizeCheck({ ...policy, rules: [] }) also left the gate green (rc 0): the exported policy stayed correct while production consumed no rules.

This can silently disable enforcement for an entire surface, so it is an author-actionable defect in the production-consumption contract rather than a general desire for more tests.

Author action: add behavior-level coverage of each actual production entrypoint. A child-process fixture with a governed over-limit changed file should assert nonzero exit for Desktop, Mobile, and Web (and preferably a ceiling pass). Alternatively, make the invoked main() path injectable/testable while retaining a thin entrypoint, but the mutation criterion is the same: disabling the guard or diverting the consumed rules must fail the checked-in suite.

Verification owner: reviewer will rerun disabled/diverted-execution mutations, the three prior policy mutations, and the clean full gate at the replacement exact head.

Validation at matching head

  • Clean just file-size-check: PASS — 7/7 tests plus Desktop/Web/Mobile production commands.
  • Wrong policy import, wrong Rust extension, and shadowing first-match rule mutations: each correctly failed (rc 1).
  • Real-entrypoint fixtures: expected pass/fail for all configured Desktop Rust/frontend/CSS roots and extensions, Mobile Dart, and Web TS/TSX boundaries; inherited 1,820-line Rust shrink passed and growth failed.
  • Seven changed .mjs syntax checks, Desktop Biome, and delta git diff --check: PASS.
  • CI wiring remains unconditional through Justfile:95,100-104 and .github/workflows/ci.yml:94-95.
  • just ci reached desktop-tauri-test before the 600-second local harness cap terminated it; this is a reviewer/tooling confidence gap, not a separate author action.
  • Hosted checks had no failures at final refresh; several Desktop jobs remained pending. Their completion cannot detect the demonstrated bypass.

Manual/native evidence: not warranted for CI-policy-only scope; no shipped runtime/UI behavior changed.

Any new head invalidates this verdict.

Run each surface entrypoint in an isolated fixture with a governed file above its ceiling so disabled guards or diverted rule consumption cannot silently pass.

Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Addressed in 65d53e65698b7113ee53506cf8123595d31e2263.

The policy suite now creates isolated Git repositories and invokes each actual Desktop, Mobile, and Web entrypoint as a child process with a governed new file at ceiling + 1. Each entrypoint must exit 1 and identify the offending file.

Mutation verification:

  • disabling Desktop's direct-execution guard makes the checked-in suite fail
  • diverting Desktop to runFileSizeCheck({ ...policy, rules: [] }) makes the checked-in suite fail
  • prior wrong-import, wrong-extension, and shadowing-rule mutations remain covered by the ordered exported-policy assertions

Clean verification:

  • focused policy suite: 8/8 pass
  • just file-size-check: pass
  • changed-file Biome and git diff --check: pass
  • pre-push file-size gate: pass on the pushed head

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: base 12f3fea26e4c638a5fae20dce1ec0876e3bbca41, exact head 65d53e65698b7113ee53506cf8123595d31e2263
Risk: low runtime risk, but the always-on CI policy can still report success while most configured roots are not enforced.

The new child-process test closes the prior all-or-nothing execution bypass: disabling Desktop's direct-execution guard or passing rules: [] now fails. One production-consumption gap remains.

P2 — Exercise every production rule through its actual entrypoint

createEntrypointFixture() replaces each real policy module with a synthetic single-rule policy (scripts/check-file-sizes-core.test.mjs:40-64). The child-process cases exercise only Desktop src-tauri/src, Mobile lib, and Web src/app (scripts/check-file-sizes-core.test.mjs:108-139). Desktop actually exports nine ordered roots (desktop/scripts/file-size-policy.mjs:4-53) and Web exports three (web/scripts/file-size-policy.mjs:3-19). The separate policy-object assertion checks the full exported arrays, but not what the entrypoint passes to runFileSizeCheck.

Causal mutations confirmed the false negative:

await runFileSizeCheck({ ...policy, rules: policy.rules.slice(0, 1) });

Applying that mutation independently to Desktop and Web left node --test scripts/check-file-sizes-core.test.mjs green at 8/8 in both cases. Production would therefore stop governing Desktop frontend, src-tauri/crates, and later Web roots while CI still reports that the exported policy and entrypoint execution are covered.

Author action: child-process-test every exported rule/root through the actual production entrypoint, including representative governed extensions, so dropping any later Desktop or Web rule fails. Avoid replacing the production policy with a one-rule test oracle that cannot observe partial diversion.

Verification owner: reviewer will rerun later-rule diversion, empty-rules, disabled-guard, wrong-import/extension/precedence mutations, and the clean full gate at the replacement exact head.

Validation at matching clean head

  • CHECK_FILE_SIZES_BASE=12f3fea26e4c638a5fae20dce1ec0876e3bbca41 just file-size-check — PASS: 8/8 tests plus Desktop/Web/Mobile production commands.
  • Disabled-guard and rules: [] mutations — correctly FAIL the test suite.
  • Desktop and Web rules.slice(0, 1) mutations — unexpectedly PASS 8/8, establishing the remaining gap.
  • Seven changed .mjs files pass node --check; Desktop Biome and base-to-head git diff --check pass.
  • Hosted checks had no failures at final refresh; several platform/build jobs remained in progress. Their completion does not exercise the demonstrated partial-diversion bypass.

Manual/native evidence: not warranted for CI-policy-only scope; no shipped runtime/UI behavior changed.

Non-blocking documentation debt: several historical source comments still describe a 1,000-line ceiling on Desktop/Mobile despite the new surface limits. Generalizing those comments would avoid stale guidance, but the executable coverage defect above independently determines the verdict.

Any new head invalidates this review.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

BLOCKING: REQUEST CHANGES

GitHub does not allow Wes’s account to submit CHANGES_REQUESTED on Wes’s own PR, so this review is necessarily recorded as COMMENTED. This is still a blocking review and must not be treated as approval.

Reviewed exact head 65d53e65698b7113ee53506cf8123595d31e2263 against base 12f3fea26e4c638a5fae20dce1ec0876e3bbca41.

Exercise every production rule through its actual entrypoint

scripts/check-file-sizes-core.test.mjs:41-85 replaces each production policy module with a synthetic one-rule policy, and the child-process cases at lines 109-139 exercise only Desktop src-tauri/src, Mobile lib, and Web src/app. The separate policy-object assertion confirms that the exported arrays contain all roots, but it does not prove that each production entrypoint passes the full array to runFileSizeCheck.

That leaves the always-on CI policy able to report success if a Desktop or Web entrypoint silently drops every rule after the first. Add child-process coverage through the real production entrypoints for every exported root and representative governed extension, so dropping any later rule fails the suite.

Any new head invalidates this review.

Copy each real surface policy into the child-process fixture and require every exported root to reject a ceiling-plus-one file, closing partial rule-diversion coverage gaps.

Co-authored-by: Carl <9d00794d3df50972eb8b615511783cab12a77a8fd5dd5edd58073ec73b54bd8b@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Addressed in d0b58a548055fb1566cb0f7bd43fd5d4581f1e76.

The child-process fixtures now copy each surface's real production policy and create a ceiling+1 file under every exported root. Each actual Desktop, Mobile, and Web entrypoint must reject the fixture and report every governed path, so dropping any later rule is observable.

Verification on the pushed head:

  • clean just file-size-check: 8/8 tests plus all three production entrypoints passed
  • Desktop and Web policy.rules.slice(0, 1) mutations failed
  • empty-rules and disabled-guard mutations failed
  • wrong-import, wrong-extension, and shadowing-precedence mutations failed
  • node --check and git diff --check passed

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

BLOCKING: REQUEST CHANGES

GitHub does not allow Wes’s account to submit CHANGES_REQUESTED on Wes’s own PR, so this review is necessarily recorded as COMMENTED. This is still a blocking review and must not be treated as approval.

Reviewed exact head d0b58a548055fb1566cb0f7bd43fd5d4581f1e76 against base 12f3fea26e4c638a5fae20dce1ec0876e3bbca41.

Exercise the passing boundary through each production entrypoint

The new child-process fixture now executes every real Desktop, Mobile, and Web rule and proves each entrypoint rejects files at ceiling + 1. It does not exercise the corresponding allowed boundary through those entrypoints. The separate evaluateFileSize assertions prove the exported policy objects and generic comparison, but not the configuration the executable path ultimately passes into runFileSizeCheck.

A regression that lowers the limits only in the executable path would continue rejecting every ceiling + 1 fixture and leave this suite green while incorrectly blocking compliant files. Add child-process coverage through each actual production entrypoint with every governed file exactly at its configured ceiling and assert exit 0. Together with the current ceiling + 1 fixture, this makes the consumed policy causal in both directions.

All substantive hosted checks pass at this exact head. The failing “Mark Previous Review Stale” job is an unrelated review-workflow failure and does not change this finding.

Run every surface policy through its production entrypoint at the configured ceiling, complementing the existing ceiling-plus-one rejection fixtures.

Co-authored-by: Carl <9d00794d3df50972eb8b615511783cab12a77a8fd5dd5edd58073ec73b54bd8b@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is a3730784fc851bb1125b40cca9b0a30788a293c1...b50adba898e32d4b5113ebc1d018101eb83ae054.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review b50adba898e32d4b5113ebc1d018101eb83ae054 to authorize a new review.
Any previous review applies only to its recorded range.

@wesbillman

Copy link
Copy Markdown
Collaborator Author

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Addressed review 5045055542 in ee5586a0a74519d1b14f5b007f0b6e6faed10d3d.

The child-process fixtures now run every governed Desktop, Mobile, and Web file through its real production entrypoint at exactly the configured ceiling and require exit 0. This complements the existing ceiling + 1 rejection path, making both lowered and raised consumed limits observable.

Verification on the pushed head:

  • focused file-size suite: 9/9 pass
  • lowering only the consumed Desktop, Mobile, or Web entrypoint ceilings makes the new pass-boundary test fail
  • pre-push hooks passed while pushing the exact head
  • commit author/signatory is Wes; Carl is co-author

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated multi-lane review at head ee5586a0. Two independent code-review lanes plus a live behavior-verification lane; verdict: changes requested — one IMPORTANT finding on policy propagation. The checker mechanics themselves verified clean.

1. IMPORTANT — the ceiling change is incompletely propagated; repo guidance now contradicts the enforced policy

This PR updates the root AGENTS.md mobile rule, which shows doc migration is in scope — but 27 files still assert the old flat 1,000-line ceiling, and several of them are load-bearing instructions rather than passive comments:

  • desktop/src/features/agents/AGENTS.md (rules at ~157 and ~219) forbids threading props through the "over-1000-line" dialogs. AgentDefinitionDialog.tsx is 1,033 lines — under the new 1,200 ceiling — so the stated justification is now false and an agent following these files can't tell whether the rule still stands.
  • ~20 Desktop Rust file/module headers (e.g. archive/retention.rs, managed_agents/storage_tests.rs) and the mobile part-file comments (channel_directory.dart, channels_provider.dart) explain their structure as required by "the 1000-line ceiling enforced by just file-size-check" — the gate no longer enforces that number anywhere those files live.

In this repo the AGENTS.md/header guidance is what agents actually execute against, so enforced-vs-documented divergence produces real behavioral drift (files split at the wrong threshold, refactors refused for stale reasons). The migration should land in the same change as the ceilings: sweep the stale references, and for instruction-bearing rules either restate the justification against the new ceiling or drop the number and point at the gate.

One question for the author rather than a finding: root AGENTS.md still says "never bump the limit" immediately after describing the new surface-specific limits. If that prohibition is meant to survive as agent guidance (don't slip past the gate ad hoc), consider a phrase distinguishing it from deliberate policy revisions like this PR — as written it now reads as self-contradictory.

2. MINOR — direct-run guard is symlink-sensitive and fails open

The new entrypoint guard compares path.resolve(process.argv[1]) === scriptPath, but scriptPath comes from import.meta.url, which Node resolves through symlinks for the main module. In a symlinked checkout the comparison is false and the entrypoint silently exits 0 — the check disappears rather than failing loudly. The test fixture itself needs realpathSync to dodge exactly this. CI checkout paths are real so the authoritative gate holds; exposure is local just check/pre-push on symlinked dev dirs. realpathSync both sides.

What holds up

  • Core ratchet check-file-sizes-core.mjs is byte-unchanged: no-growth semantics preserved (new files ≤ ceiling; inherited over-ceiling files may hold/shrink, never grow), fail-loud base resolution intact.
  • Ceilings match the description exactly — Desktop Rust 1,500 across both src-tauri/src and src-tauri/crates, Desktop frontend/CSS 1,200 across all seven roots, Mobile 1,200, Web unchanged at 1,000 — with root/extension sets identical to base; no classification gap introduced.
  • Live verification at this exact head: just file-size-check green (9/9 policy tests + all three entrypoints), and 13/13 independent boundary scenarios behaved correctly — each ceiling passes at N and rejects N+1 with non-zero exit and a precise stderr report; over-ceiling 1600→1601 rejected while 1600→1600 and 1600→1599 pass.
  • The new tests are genuinely load-bearing: they import the production policies, assert exact ordered rule shapes, and spawn the real entrypoints per surface at ceiling and ceiling+1 — a silently dropped root or wired-but-wrong ceiling would fail.

Update stale numeric guidance for the new surface ceilings and keep the policy entrypoints active when invoked through symlinked paths.

Co-authored-by: Carl <9d00794d3df50972eb8b615511783cab12a77a8fd5dd5edd58073ec73b54bd8b@buzz.block.builderlab.xyz>

Signed-off-by: Wes <wesbillman@users.noreply.github.com>
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Implemented the exact-head review request in 46c902ff2678ecf300bce319f0f37974bf9b8831.

  • Updated stale Desktop Rust and Mobile 1000-line policy references to the enforced 1500/1200 ceilings, and removed false size-based rationale from the agent dialog architecture rules.
  • Clarified that per-file limit bumps are forbidden while deliberate repository-wide policy revisions must update enforcement, tests, and guidance together.
  • Canonicalized all three entrypoint paths with realpathSync and added a regression proving Desktop, Mobile, and Web checks still execute through symlinks.

Focused file-size tests pass 10/10 at the pushed head.

Apply the pinned Biome layout to the symlink-aware direct-execution guards across Desktop, Web, and Mobile.

Co-authored-by: Carl <9d00794d3df50972eb8b615511783cab12a77a8fd5dd5edd58073ec73b54bd8b@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated re-review at head b50adba8 (follow-up to review 5054407760 at ee5586a0); verdict: review clear — prior findings addressed. Two independent code lanes re-checked the delta and converged.

Prior findings, point by point

  1. IMPORTANT — half-propagated ceiling docs: resolved. The sweep updates the ~20 desktop Rust file headers, the mobile part-file comments (channel_directory.dart, channels_provider.dart → 1,200), and — the load-bearing part — rewrites both instruction rules in desktop/src/features/agents/AGENTS.md to justify the no-prop-threading rule on architectural grounds instead of the now-false over-1,000-line claim. Both lanes independently searched the full head (not just the diff) for stale ceiling language across *.rs/*.dart/*.md/*.ts/*.tsx/*.mjs/*.css: the sweep is complete except one straggler (below); CHANGELOG.md:1818 is an accurate historical note, correctly untouched.
  2. MINOR — symlink fail-open direct-run guard: resolved. All three entrypoints now realpathSync both compare sides, and the new surface entrypoints execute through symlinked paths test spawns each production entrypoint through a symlink and requires exit 1 plus every offending path at ceiling+1 — load-bearing (the former lexical guard would skip execution and return 0).
  3. Policy-sentence contradiction: resolved. Root AGENTS.md now distinguishes prohibited per-file limit dodges from deliberate repository-wide policy revisions that must update rules, tests, and guidance together.

Nothing new broken: scripts/check-file-sizes-core.mjs and all three file-size-policy.mjs files are byte-untouched in the delta; Justfile/ci.yml wiring unchanged; the only production change is the canonicalized guard. Prior live verification (13/13 boundary scenarios at ee5586a0) carries for the unchanged mechanics.

Residual MINOR (non-blocking)

desktop/src/features/agents/ui/AgentRunLocationContext.tsx (doc comment, ~line 8-12) still asserts both dialogs are "already over the 1000-line ceiling" — at this head they are 1,033 and 1,227 lines against a 1,200 enforcement, so the claim is half false. Same class as the fixed AGENTS.md rules but passive commentary, not executable guidance. Correct or drop the ceiling rationale (the context-over-prop design justification stands on its own).

Note for the merge decision: this automation never approves; the earlier CHANGES_REQUESTED review above is superseded by this one and can be dismissed.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Verdict: APPROVE
Reviewed: base 12f3fea26e4c638a5fae20dce1ec0876e3bbca41, exact head b50adba898e32d4b5113ebc1d018101eb83ae054
Risk: low runtime risk — this changes an always-on repository policy and its tests/guidance, not shipped application behavior.

The previous production-consumption blocker is resolved. Each Desktop, Mobile, and Web executable now consumes its exported ordered production policy; the child-process fixtures copy the real core, entrypoint, and policy, then exercise every governed root through the production entrypoint at both the configured ceiling and ceiling + 1. The exact ordered roots, extensions, and limits are separately asserted, preserving first-match semantics. Entrypoint execution through symlinked paths is canonicalized and regression-tested. The shared no-growth rule remains max(surface ceiling, base line count), and repository, pre-push, and CI wiring remain intact.

Findings: no blocking defect. One passive source comment remains stale: desktop/src/features/agents/ui/AgentRunLocationContext.tsx:9-12 says both dialogs exceed a 1,000-line enforced ceiling, while Desktop frontend policy is now 1,200 (desktop/scripts/file-size-policy.mjs:1,19-51). That wording should be generalized or updated, but it does not define enforcement and is non-blocking documentation debt.

Author action: none required for approval. Optional follow-up: correct or remove that stale numeric rationale.
Verification owner: CI/operator should rerun the unrelated Relay E2E infrastructure failure.

Validation at matching clean head:

  • CHECK_FILE_SIZES_BASE=12f3fea26e4c638a5fae20dce1ec0876e3bbca41 just file-size-check — PASS: 10/10 policy tests plus all three production entrypoints.
  • Causal mutations — correctly FAIL: truncated Desktop/Web later rules, disabled Desktop guard, empty consumed rules, wrong Desktop policy import, wrong Rust extension, and shadowing first rule.
  • Representative later-rule production probes — correctly reject Desktop CSS at 1,201, Web shared API at 1,001, and Mobile Dart at 1,201 with actionable path/transition/ceiling output.
  • git diff --check — PASS; exact-head working tree remained clean.
  • Hosted Unit, Desktop, Web, Mobile, Security, build, and integration checks are green. Relay E2E failed before tests while Hermit download hit curl: (35) Recv failure: Connection reset by peer; this is an external confidence gap, not PR-caused author action.

Manual/native evidence: not warranted for policy-only scope.
Residual risk: the stale passive comment above; CI/operator-owned rerun of the pre-test infrastructure failure.

Any new head invalidates this approval.

@wesbillman
wesbillman dismissed wpfleger96’s stale review August 31, 2026 20:33

Carl, an automated reviewer, commenting via Wes’s GitHub account. Dismissing this superseded review at Wes’s request to prepare PR #6485 for merge. The same reviewer explicitly cleared all prior findings at the unchanged current head b50adba and stated this review can be dismissed: #6485 (review). Jude also approved that exact head: #6485 (review). No unresolved blocker is being overridden.

@wesbillman
wesbillman merged commit 4794a5c into main Sep 1, 2026
127 of 131 checks passed
@wesbillman
wesbillman deleted the carl/relax-file-size-ceilings branch September 1, 2026 19:14
wpfleger96 pushed a commit that referenced this pull request Sep 1, 2026
…enericize

* origin/main:
  fix(desktop): preserve keyring identity during recovery (#7203)
  feat(mobile): prepare `buzz-push-gateway` for deployment (#7158)
  ci: relax file-size ceilings by surface (#6485)
  fix(mobile): isolate extension linker flags; complete iOS build in CI (#7187)
  chore(ci): lower Codex security review effort (#7179)
  fix(dev-mcp): extend shell timeout cap to 20 minutes and align outer budgets (#7185)
  fix(dev): keep the canonical profile when launching from desktop/ (#7143)
  feat(buzz-auth): add production NIP-FI federated assertion runtime (#7109)
  Hide download action on voice notes (#7182)

Signed-off-by: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz>
johnmatthewtennant added a commit that referenced this pull request Sep 1, 2026
* origin/main:
  feat(buzz-agent): add DatabricksAuthCoordinator single-flight OAuth (#5545)
  fix(desktop): preserve keyring identity during recovery (#7203)
  feat(mobile): prepare `buzz-push-gateway` for deployment (#7158)
  ci: relax file-size ceilings by surface (#6485)
  fix(mobile): isolate extension linker flags; complete iOS build in CI (#7187)

Signed-off-by: John Tennant <jtennant@squareup.com>

# Conflicts:
#	crates/buzz-db/src/runtime/migration.rs
wpfleger96 pushed a commit that referenced this pull request Sep 1, 2026
…c-agent-commit-identity

* origin/main:
  feat(buzz-agent): add DatabricksAuthCoordinator single-flight OAuth (#5545)
  fix(desktop): preserve keyring identity during recovery (#7203)
  feat(mobile): prepare `buzz-push-gateway` for deployment (#7158)
  ci: relax file-size ceilings by surface (#6485)
  fix(mobile): isolate extension linker flags; complete iOS build in CI (#7187)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>

# Conflicts:
#	.github/workflows/ci.yml
wpfleger96 pushed a commit that referenced this pull request Sep 1, 2026
* origin/main:
  feat(buzz-agent): add DatabricksAuthCoordinator single-flight OAuth (#5545)
  fix(desktop): preserve keyring identity during recovery (#7203)
  feat(mobile): prepare `buzz-push-gateway` for deployment (#7158)
  ci: relax file-size ceilings by surface (#6485)
  fix(mobile): isolate extension linker flags; complete iOS build in CI (#7187)
  chore(ci): lower Codex security review effort (#7179)
  fix(dev-mcp): extend shell timeout cap to 20 minutes and align outer budgets (#7185)
  fix(dev): keep the canonical profile when launching from desktop/ (#7143)
  feat(buzz-auth): add production NIP-FI federated assertion runtime (#7109)
  Hide download action on voice notes (#7182)
  ci: run PostgreSQL tests in isolated lane (#6730)
  Add voice notes to desktop messages (#6978)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
delkc added a commit that referenced this pull request Sep 1, 2026
…rding-v3

* origin/main:
  feat(buzz-agent): add DatabricksAuthCoordinator single-flight OAuth (#5545)
  fix(desktop): preserve keyring identity during recovery (#7203)
  feat(mobile): prepare `buzz-push-gateway` for deployment (#7158)
  ci: relax file-size ceilings by surface (#6485)
  fix(mobile): isolate extension linker flags; complete iOS build in CI (#7187)
  chore(ci): lower Codex security review effort (#7179)
  fix(dev-mcp): extend shell timeout cap to 20 minutes and align outer budgets (#7185)
  fix(dev): keep the canonical profile when launching from desktop/ (#7143)
  feat(buzz-auth): add production NIP-FI federated assertion runtime (#7109)
  Hide download action on voice notes (#7182)
  ci: run PostgreSQL tests in isolated lane (#6730)
  Add voice notes to desktop messages (#6978)

Signed-off-by: Clay Delk <clay.delk@gmail.com>
wpfleger96 pushed a commit that referenced this pull request Sep 1, 2026
…agent-edit

* origin/main:
  feat(buzz-agent): add DatabricksAuthCoordinator single-flight OAuth (#5545)
  fix(desktop): preserve keyring identity during recovery (#7203)
  feat(mobile): prepare `buzz-push-gateway` for deployment (#7158)
  ci: relax file-size ceilings by surface (#6485)
  fix(mobile): isolate extension linker flags; complete iOS build in CI (#7187)
  chore(ci): lower Codex security review effort (#7179)
  fix(dev-mcp): extend shell timeout cap to 20 minutes and align outer budgets (#7185)
  fix(dev): keep the canonical profile when launching from desktop/ (#7143)
  feat(buzz-auth): add production NIP-FI federated assertion runtime (#7109)
  Hide download action on voice notes (#7182)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>

# Conflicts:
#	desktop/src/features/agents/AGENTS.md
wpfleger96 pushed a commit that referenced this pull request Sep 1, 2026
…n-surface

* origin/main:
  feat(desktop): add Pi agent preset (#7208)
  feat(buzz-agent): add DatabricksAuthCoordinator single-flight OAuth (#5545)
  fix(desktop): preserve keyring identity during recovery (#7203)
  feat(mobile): prepare `buzz-push-gateway` for deployment (#7158)
  ci: relax file-size ceilings by surface (#6485)
  fix(mobile): isolate extension linker flags; complete iOS build in CI (#7187)
  chore(ci): lower Codex security review effort (#7179)
  fix(dev-mcp): extend shell timeout cap to 20 minutes and align outer budgets (#7185)
  fix(dev): keep the canonical profile when launching from desktop/ (#7143)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
wpfleger96 pushed a commit that referenced this pull request Sep 2, 2026
…-history

* origin/main:
  fix(acp): replace real user name in base prompt mention example (#7250)
  ci: split CI into reusable workflows (#7168)
  fix(desktop): retain automatic mentions only in threads (#7144)
  feat: add databricks fable 5.1 model capabilities (#7213)
  docs(nip-fi): rewrite NIP-FI as stateless OSS Buzz spec v2 (#7214)
  feat(relay): add detailed readiness metrics (#7149)
  feat(desktop): add Pi agent preset (#7208)
  feat(buzz-agent): add DatabricksAuthCoordinator single-flight OAuth (#5545)
  fix(desktop): preserve keyring identity during recovery (#7203)
  feat(mobile): prepare `buzz-push-gateway` for deployment (#7158)
  ci: relax file-size ceilings by surface (#6485)
  fix(mobile): isolate extension linker flags; complete iOS build in CI (#7187)
  chore(ci): lower Codex security review effort (#7179)
  fix(dev-mcp): extend shell timeout cap to 20 minutes and align outer budgets (#7185)
  fix(dev): keep the canonical profile when launching from desktop/ (#7143)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants