ReviewOS

home-lang/home

CSS: port Bun's nesting-expansion and selector-expansion budgets (partial port causes a teardown abort)

#705
Closed glennmichael123 opened this 9 days ago ·
9 days ago

Bun bounds CSS nesting expansion in two places. Home has neither. I ported one and it is not shippable alone — recording the full picture so the next attempt starts from here.

The two budgets

1. Printer stage — nesting_expansions on Printer, MAX_NESTING_EXPANSIONS = 65_536, checked in serialize_nesting, reset per prelude in StyleRule::to_css_base and ScopeRule::to_css. Raises PrinterErrorKind::maximum_nesting_expansion → "Maximum nesting expansion exceeded when compiling CSS nesting for the configured targets".

  • ~/Code/bun/src/css/selectors/selector.rs:1322-1336
  • ~/Code/bun/src/css/printer.rs:155-160, :349
  • ~/Code/bun/src/css/rules/style.rs:181, ~/Code/bun/src/css/rules/scope.rs:32
  • ~/Code/bun/src/css/error.rs:235, :262

2. Minify stage — selector_expansion_total / selector_expansion_multiplier on MinifyContext, MAX_SELECTOR_EXPANSION = 65_536, charged in StyleRule::charge_selector_expansion with the multiplier bumped by minify_nested_rules. Raises MinifyErrorKind::selector_expansion_limit_exceeded → "Nested CSS rules expand to more than {} selectors when compiled for the configured browser targets…".

  • ~/Code/bun/src/css/rules/mod.rs:1227 (constant), :1263 (field)
  • ~/Code/bun/src/css/rules/style.rs:380-407
  • ~/Code/bun/src/css/error.rs:516, :538-542

They are not interchangeable. nested-selector-list-expansion.test.ts asserts the minify-stage message specifically.

What I tried, and why it was reverted

Porting budget 1 alone:

  • Fixes js/bun/css/nested-selector-expansion.test.ts: 2 pass / 2 fail → 4 pass / 0 fail, exact parity with pinned Bun. css.test.ts unchanged at 1064/29, runtime regressions 68/69.
  • But js/bun/css/nested-selector-list-expansion.test.ts starts exiting 133 (SIGTRAP). Its test results are unchanged (5 pass / 19 fail before and after, "Ran 24 tests" printed in full) — the abort happens at teardown, after the summary. Exit code went 1 → 133.

Printer.newError opens with bun.debugAssert(this.error_kind == null), so my first hypothesis was the budget firing twice on one printer. Guarding that (if (dest.error_kind != null) return PrintErr.CSSPrintError;) did not fix it, so the abort is elsewhere on the path a recorded-but-unconsumed printer error takes.

Reverted rather than shipped: a broken invariant is worse than one test at 2/2. The work is stashed locally as nesting-expansion port: fixes nested-selector-expansion 2/2->4/0 but introduces a teardown SIGTRAP in nested-selector-list-expansion.

Suggested order for the next attempt

  1. Port budget 2 (minify stage) first — it is what nested-selector-list-expansion actually wants, and it may prevent the deep expansion from ever reaching the printer.
  2. Then re-apply budget 1 and re-check the teardown exit code.
  3. Find who swallows a recorded Printer.error_kind without consuming it; that is the latent bug the abort exposed, and it predates both budgets.

Refs #66

9 days ago

Implemented and pushed the complete two-stage expansion guard in b164b74 (fix(css): bound nested selector expansion (#705)).

What landed:

  • Bun-compatible 65,536 selector-expansion budget in minification, with saturating multiplier/total accounting and a structured selector_expansion_limit_exceeded error.
  • Recursion through supports, container, moz-document, scope, and starting-style boundaries so at-rules cannot hide expansion depth.
  • Real @nest AST retention plus budget charging; Home previously parsed then discarded its selectors/declarations/rules.
  • Bun-compatible per-prelude parent-selector substitution budget in the printer, including style and scope resets and maximum_nesting_expansion diagnostics.
  • One canonical CSS Location type shared by the live graph and compatibility stub, removing a nominal type split exposed by the port.

Verification:

  • expansion budget unit tests: 4/4 pass (277 MB guarded peak), including threshold and saturating-overflow cases.
  • canonical location unit test: 1/1 pass (264 MB guarded peak).
  • real Home minifier integration semantic compile: pass (518 MB guarded peak).
  • real Home printer integration semantic compile: pass (515 MB guarded peak).
  • ast-check and diff-check pass; all touched files except css_parser.zig pass zig fmt --check. css_parser.zig has known pre-existing Zig 0.17 semantic formatter rewrites unrelated to this patch.
  • A focused executable link was attempted in both ReleaseFast and Debug and was cleanly stopped by the fixed 3,840 MB safety ceiling (3,880/3,919 MB); the ceiling was not raised. The pinned Bun release cannot import bun:internal-for-testing, so that invalid oracle run was not counted.

The implementation is on origin/main. Keeping this issue open until the original two corpus files can be rerun against a fresh native Home binary within the machine safety envelope.

Refs #66.

9 days ago

Completed on origin/main by the original budget port in b164b747e and the final native-corpus fixes in 0c18b0770.

The final diagnosis found three independent omissions on the exercised path:

  • @-moz-document parsed its nested style block and then replaced it with an empty rule list. Home now retains the parsed rules, matching Bun's Rust parser.
  • CssRule.toCss discarded retained @nest AST nodes. It now calls the existing NestingRule.toCss, preserving declarations and nested contents.
  • the CSS error bridge released a formatted BunString after String.toErrorInstance had already consumed the same reference. Repeated forced-GC errors therefore over-released the message StringImpl and corrupted JSC's string-uniquing table at teardown. The bridge now follows the callee's ownership contract.

Fresh verification against the unchanged original files:

  • ReleaseFast native build: 46/46 steps succeeded.
  • nested-selector-list-expansion.test.ts: 24/24 pass, 45 assertions.
  • nested-selector-expansion.test.ts: 4/4 pass, 13 assertions.
  • The selector-list run used BUN_GARBAGE_COLLECTOR_LEVEL=1 and BUN_JSC_randomIntegrityAuditRate=1.0, exercising the former deterministic teardown crash under forced GC and integrity auditing.
  • Pickier and git diff --check pass for the final changed files.

No original corpus test, skip, deadline, expected value, or runtime fallback changed. Refs #66.

Sign in to comment on this issue.