fix(core): resolve CSS @import and rebase url() paths during bundling

[miguel-heygen]

Summary

• Resolve CSS @import statements when inlining stylesheets into the bundle. The bundler reads <link> CSS files and concatenates them into a <style> block, but left @import statements unresolved — their paths were relative to the CSS file's directory, causing 404s after inlining.
• Rebase url() paths in inlined CSS so fonts, images, and cursors from subdirectory stylesheets resolve correctly against the project root. Uses a placeholder approach during @import resolution to avoid double-rebasing in nested chains.

Root cause

When styles/canvas.css contains @import url('./tokens.css') and gets inlined into <style>, the @import resolves relative to the HTML document instead of styles/ — fetching /preview/tokens.css (404) instead of /preview/styles/tokens.css. Same issue for url() references inside those imported files: url('assets/fonts/font.woff2') from styles/tokens.css resolves to the project root instead of styles/assets/fonts/.

What changed

• inlineCssFile() — recursively resolves @import, handles url(), bare string, and media-query syntax. Circular import protection via visited set. Absolute URLs preserved.
• rebaseCssUrls() — rewrites relative url() paths from the CSS file's directory context to the project root. Preserves absolute URLs, data URIs, query strings, and hash fragments.

Test plan

• 9 new test cases covering: basic @import, nested chains, media queries, absolute URL preservation, url() rebasing from subdirectories, ../ traversal, absolute/data URI preservation, query string + hash preservation
• All 28 bundler tests pass
• Verified end-to-end with a real composition (trove-logo) — preview now matches rendered video for colors, fonts, shadows, and background
• Verified font URLs return 200 (were 404 before fix)
• Pre-commit hooks pass (oxlint, oxfmt, typecheck, commitlint)

[jrusso1020]

APPROVE — focused fix, real change is +57/-3 in htmlBundler.ts with thorough tests; the -238k deletion count is the packages/producer/dist/ artifact purge (98 files), separately fine since dist/ is in .gitignore at the repo root.

Audited

• packages/core/src/compiler/htmlBundler.ts — new regexes (CSS_IMPORT_RE, REBASE_URL_RE), new functions (rebaseCssUrls, inlineCssFile), callsite change from css → inlineCssFile(css, dirname(cssPath), projectDir) ✓
• packages/core/src/compiler/htmlBundler.test.ts — 8 new tests covering: basic @import, nested chains, media-query wrapping, absolute-URL preservation, url() rebasing across project-root / subdir / ../ traversal, absolute+data url preservation, query+hash preservation ✓
• packages/producer/package.json exports — main: "dist/index.js" + files: ["dist/"] + workspace build script. Producer is consumed via workspace:^ by sibling packages; dist must rebuild before consumers can import via the package entry. CI's full build chain should handle this.
• .gitignore confirms dist/ was always intended to be gitignored ✓

Trusting

• Existing 17 tests pass (didn't re-run locally; relying on CI)
• coalesceHeadStylesAndBodyScripts still correctly handles preserved @imports (absolute URLs like Google Fonts) after this stage, since inlineCssFile runs earlier in the pipeline
• The prepublishOnly: "echo skip" script is intentional and the actual publish flow runs build (not verified from this diff)

Non-blocking flags

1. rebaseCssUrls doesn't bound resolved paths to projectDir.

Compare:

// inlineCssFile (line 119–122): path-traversal protection
const resolved = resolve(cssFileDir, importPath);
const normalizedBase = resolve(projectDir) + sep;
if (!resolved.startsWith(normalizedBase) || visited.has(resolved)) return full;

// rebaseCssUrls (line 103–113): no bound check
const absolutePath = resolve(resolvedDir, basePath);
const rebased = relative(resolvedRoot, absolutePath);

If a CSS file at subdir/foo.css references url('../../../etc/passwd'), the rebased path becomes ../../etc/passwd (or whatever the relative climbs to). That gets written into the bundled HTML's url(...) and would resolve relative to the bundle's serve location at render time. Realistically: the bundler runs on trusted CSS at build time, not user input, so this is defense-in-depth, not a current exploit path. But the inconsistency with inlineCssFile's traversal check is worth resolving — either both bound to projectDir or neither does (and the rationale documented).

2. Diamond-import edge case in the visited set.

if (!resolved.startsWith(normalizedBase) || visited.has(resolved)) return full;

If files form a diamond — A imports B and A imports C, both B and C import D — the second occurrence of D hits visited.has(resolved) and returns the original @import url('./D.css'); unchanged. That string then sits inside the inlined output and would try to HTTP-fetch ./D.css relative to the bundle's URL at render time, which won't resolve (the bundle is a single HTML file, not a server). The first occurrence is correctly inlined; only the redundant second occurrence leaves a stale reference.

Two fixes possible:

• Return empty string "" on visited → just drop the redundant @import (since the rules were already inlined upstream)
• Or move the visited check to track on a per-recursion-chain basis (not global), accepting one rule-duplication for diamonds

Empty-string drop is the simpler call.

3. dist/ purge consumer-side verification.

packages/producer is imported by sibling workspaces as workspace:^. The main and exports resolve through dist/, so any workflow that consumed the committed dist/ directly — not via the build chain — needs to rebuild before it works. Worth a quick bun run build from a fresh checkout to confirm nothing downstream breaks. If your local dev loop already does bun run build on producer at startup or via filesystem watches, no action needed.

Nit

CSS_IMPORT_RE requires a trailing ;. CSS at EOF without a final ; after the last @import (legal but rare) wouldn't match. Not worth fixing unless real CSS in your project hits this.

— Rames

[vanceingalls]

Solid bug fix on a real issue (composition preview parity for @import-using stylesheets), with a clever placeholder approach to avoid double-rebasing in nested chains. One platform-correctness blocker and one diamond-import edge case worth fixing before merge.

Strengths

• Placeholder dance in inlineCssFile (htmlBundler.ts:224-247) cleanly prevents the parent rebaseCssUrls from re-rewriting URLs that the child recursion has already rebased. Easy to get wrong, and this version reads correctly.
• splitUrlSuffix reuse for ?query#hash preservation matches the convention maybeInlineRelativeAssetUrl already uses — consistent.
• 9 new tests cover the headline cases (basic/nested @import, media query, absolute-URL preservation, url() rebasing in nested files, ../ traversal, data URI / absolute / suffix preservation). Good coverage of the obvious paths.
• Circular-import protection via visited set is present.

---

blocker — Path emission uses platform-native path; will emit backslashes (and break) on Windows

htmlBundler.ts:2 adds relative, dirname to the from \"path\" import, and rebaseCssUrls calls relative(resolvedRoot, absolutePath) whose return value is then spliced directly into a url(...) string. On Windows, path.relative() returns \\-separated paths, so the bundled CSS becomes url('styles\\assets\\fonts\\brand.woff2') — which browsers cannot resolve.

The codebase already has explicit precedent for exactly this: packages/core/src/compiler/rewriteSubCompPaths.ts:15-19:

// URL paths in HTML output are POSIX regardless of host OS — use the `posix`
// submodule so Windows builds don't emit backslash-separated paths (or worse,
// drive-letter-prefixed artifacts from `resolve(\"/\", ...)`).
import { posix } from \"path\";
const { join, resolve, dirname } = posix;

And rewriteSubCompPaths.test.ts:29 has a \"never emits backslashes on any platform\" assertion guarding it. The new code path violates this established invariant.

The new tests at htmlBundler.test.ts:101, :120, :139, :163, :180 all assert paths like url('styles/assets/fonts/brand.woff2') with forward slashes. The Windows CI job (Tests on windows-latest, currently pending) will fail on these once it runs. Windows CI is a required check on this repo.

Fix: switch the path math in rebaseCssUrls and the recursive dirname(resolved) call in inlineCssFile to posix. Either import { posix } from \"path\"; const { relative, dirname, resolve, sep } = posix;, or import from \"node:path/posix\". Add a regression-style test (analogous to rewriteSubCompPaths.test.ts:29) that asserts the output never contains a backslash.

important — Diamond imports leave a stray, unresolvable @import in the output

When the same CSS file is @import-ed via two different paths (e.g., canvas.css imports both tokens.css and theme.css, and theme.css also imports tokens.css), the second visit to tokens.css hits the visited.has(resolved) branch (htmlBundler.ts:232) and returns full — the original @import url('./tokens.css') text. That text then survives into withPlaceholders, gets rebased by rebaseCssUrls to a project-relative URL like url('styles/tokens.css'), and is hoisted to the top of the merged <style> by coalesceHeadStylesAndBodyScripts (htmlBundler.ts:258-283).

But the bundled output is a single self-contained HTML — there is no separate styles/tokens.css to fetch. The browser will 404 on it, and depending on whether the @import resolves synchronously (it doesn't, but FOUC behavior varies) you can lose styles in the second-import branch entirely.

Fix: on the visited branch, return \"\" instead of full. That dedups the content (which is what CSS-correct semantics want anyway) and removes the broken import. Add a test for the diamond case.

No registry composition currently triggers this (all repo @import use is for https://fonts.googleapis.com/... absolute URLs, which are correctly preserved), so this is important rather than blocker — but it's the kind of latent issue users hit the moment they build a tokens.css design system.

nit — Drive-by deletion of 106 packages/producer/dist/ files

The diff is 230 additions / 238,248 deletions / 108 changed files, but the actual source change is only htmlBundler.ts + htmlBundler.test.ts. The other 106 files are packages/producer/dist/* artifacts that were committed by mistake at some point (last touched in #748) and are correctly gitignored. Cleaning them up is fine and harmless, but it makes the diff much harder to review at a glance and conflates two unrelated changes. Future change of this size: separate cleanup PR.

nit — @import regex matches inside CSS comments

CSS_IMPORT_RE is run against the raw CSS without first stripping comments. A construct like /* @import url('./old.css'); */ will be matched, splice the imported content out of the comment, and leave malformed /* ... */ fragments around the placeholder. Esoteric — commented-out imports are rare — but trivial to defend against (strip block comments before matching, or anchor the regex to not match inside /* */).

nit / scope clarification — Sub-composition inline <style> blocks still don't get @import resolution

htmlBundler.ts:505-510 and :519-525 run rewriteCssAssetUrls (which only rebases url(...)) on <style> text extracted from sub-composition HTML files. They do not call inlineCssFile, so a sub-composition that embeds <style>@import url('./tokens.css');</style> still produces an unresolvable import in the bundle. This is pre-existing and not a regression — but the PR title ("resolve CSS @import statements when inlining stylesheets") reads as if it covers the general case. Worth either (a) extending the fix to those code paths in this PR, or (b) explicitly scoping the PR description to <link>-sourced CSS and filing a follow-up for sub-comp <style> blocks.

---

Verdict: REQUEST CHANGES
Reasoning: Windows-path emission is a concrete platform bug that breaks the bundler on Windows and will also fail the required Tests on windows-latest check once it runs; this needs to land as posix paths. Diamond-import behavior is a correctness bug that's latent today but trivial to fix while you're in this code.

Review by Vai

[jrusso1020]

Retracting my APPROVE — aligning with @vai's REQUEST_CHANGES. Two substantive misses on my end:

1. Windows path separator regression (blocker). I read the imports as from "path" and didn't connect that to the Tests on windows-latest required check. path.relative() / dirname() from node:path produce \\ on Windows; the new tests assert forward-slash paths like url('styles/fonts/brand.woff2'), which would fail on win-latest. The codebase has an explicit posix convention at packages/core/src/compiler/rewriteSubCompPaths.ts:15-19 (which I didn't grep for) precisely to avoid this. The fix Vai named — import from node:path/posix instead of node:path — matches the existing convention. I should have audited path-rewriting code against the existing posix-path convention before approving; my review didn't do that.

2. Diamond-import severity miscalibration. I flagged this as a nit ("easy fix"). Vai correctly elevated it to "important" because the result is a 404'd network fetch in the rendered bundle, not just code smell. When the same CSS file is reached via two parent stylesheets, the second visited.has(resolved) return full leaves a literal @import url("..."); in the bundled output that fetches at render time. Same severity-calibration mistake I made on hf#921's caption-texture-lava install gap — naming a UX harm but undervaluing it.

I'm going to update my reviewer memory on both lessons. @miguel-heygen please address Vai's REQUEST_CHANGES before re-review.

— Rames