Consolidate @supports gating checks and fix ungated color-mix() in components

[jackgranatowski]

Summary

Consolidates the fragmented @supports-gating validation logic into a shared, robust scanner (tests/supports-helpers.js), expands P2 coverage from tokens.css alone to all source CSS files, and fixes an ungated color-mix() declaration in optional/components.css that violated the framework's modern-expression gating policy.

Key changes

• New shared scanner (tests/supports-helpers.js): Implements stripSupports() and findUngatedModernExpressions() helpers that are character-based, parenthesis-aware, and comment-stripped. Replaces the line-based scanner in P7 that historically miscounted braces inside @supports prelude expressions (e.g., @property feature queries).

• Expanded P2 coverage (tests/tier1-p2-coverage.test.js): Now scans all core/*.css and optional/*.css files (not just tokens.css), catching modern expressions used directly in component/utility rules, not only in colour tokens. Uses the shared scanner so P2 and P7 can never drift.

• New P7 whole-bundle guard (tests/tier1-p7-oldengine.test.js): Adds a second check that scans every declaration in the built bundle (custom properties AND plain properties), complementing the existing custom-property-only checks.

• Fixed ungated color-mix() (optional/components.css): Wrapped the .sf-btn--secondary:hover and .sf-btn--ghost:hover color-mix() background declarations in an @supports block, with explanatory comment linking to the framework-wide gating policy.

• New contract test (tests/configurator-data-contract.test.js): Validates that generated configurator data files (api-index.generated.json, classes.generated.json, bundles.generated.json, token-registry.generated.json) maintain their required schema — required keys present on every row, non-empty catalogues. Catches silent shape changes that would break the vendored plugin.

• Improved error messages: Added metaFor() helper in scripts/lib/api-index/extract.js that fails loudly when a source file is added to registry-sources.js but lacks a FILE_META entry, instead of the opaque Cannot read properties of undefined crash. Same pattern applied to scripts/gen-class-reference.js with FILE_TITLES.

• Enhanced string literal handling (scripts/lib/parse.js): Fixed stripStrings() and maskStrings() regexes to honour backslash escapes (e.g., content: "\"") so escaped quotes inside strings don't end the match early.

• Artifact tracking safeguard (scripts/check-artifacts.js): Added validation that declared artifact outputs are git-tracked; untracked/ignored files now fail loudly instead of silently bypassing the freshness check.

• Updated artifact registry (scripts/artifacts.json): Added classes.generated.json and the full-api demos (demos/generate.mjs outputs) to the tracked artifacts list.

• Documentation updates: Clarified version-sync ownership in CLAUDE.md, added status headers to reports/full-api-audit/REPORT.md and analysis/gap-analysis.md explaining their manual/historical nature, and improved comments in scripts/gen-class-reference.js and scripts/lib/parse.js.

• Removed unused dependency: Removed lightningcss from package.json devDependencies (no longer used in the build).

Implementation notes

• The new stripSupports() scanner is character-based and tracks parenthesis depth to correctly identify the body-opening { at depth 0, avoiding the historical P7 bug where braces inside @supports prelude expressions (like @property feature queries) were miscounted.
• Comments are stripped before scanning so commented-out braces or @supports text cannot steer the scan.
• P2 and P7 now share the same scanner, enfor

https://claude.ai/code/session_01Mxc1F7deC8GbWntXZ4gXkG

[coderabbitai]

Warning

Review limit reached

@jackgranatowski, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 48 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6617fa99-a6da-49e9-8907-1c7bdbb86d81

📥 Commits

Reviewing files that changed from the base of the PR and between feda6d0 and 2f825fe.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (18)

• CLAUDE.md
• analysis/gap-analysis.md
• demos/full-api-demo-with-overrides.html
• demos/full-api-demo.html
• demos/ultimate-override.css
• optional/components.css
• package.json
• reports/full-api-audit/REPORT.md
• scripts/artifacts.json
• scripts/check-artifacts.js
• scripts/gen-class-reference.js
• scripts/lib/api-index/extract.js
• scripts/lib/parse.js
• tests/configurator-data-contract.test.js
• tests/supports-helpers.js
• tests/supports-helpers.test.js
• tests/tier1-p2-coverage.test.js
• tests/tier1-p7-oldengine.test.js

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/repo-security-audit-dlbw5f

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Consolidate @supports gating scanner; fix ungated color-mix() in components.css

🐞 Bug fix 🧪 Tests ✨ Enhancement 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Fixes an ungated color-mix() hover background in optional/components.css by wrapping it in
 @supports, matching the framework's modern-expression gating policy.
• Introduces shared tests/supports-helpers.js scanner (char/parenthesis-based) replacing the buggy
 line-based P7 scanner and eliminating drift between P2 and P7 tests.
• Expands P2 coverage to scan all core/*.css/optional/*.css files and adds a P7 whole-bundle
 check for plain (non-custom-property) declarations.
• Adds tests/configurator-data-contract.test.js to pin the schema of generated configurator data
 consumed by the vendored plugin.
• Hardens tooling: fail-loud metaFor()/FILE_TITLES lookups, artifact-tracking validation in
 check-artifacts.js, and escaped-quote handling in parse.js string strippers.
• Updates artifacts.json to track new generated files, bumps demo files to v0.7.3, removes unused
 lightningcss dependency, and clarifies documentation.

Diagram

graph TD
  A["optional/components.css"] -->|ungated color-mix fixed| B["@supports gate"]
  C["tests/supports-helpers.js"] -->|stripSupports + scanner| D["tests/tier1-p2-coverage.test.js"]
  C -->|shared scanner| E["tests/tier1-p7-oldengine.test.js"]
  D -->|scans| F["core/*.css and optional/*.css"]
  E -->|scans| G["dist/slashed.full.css"]
  H["tests/configurator-data-contract.test.js"] -->|validates schema| I["configurator/src/data/*.generated.json"]
  J["scripts/check-artifacts.js"] -->|validates tracked outputs| K["scripts/artifacts.json"]
  L["scripts/lib/parse.js"] -->|shared string/comment strip| M["scripts/gen-class-reference.js"]
  L --> N["scripts/lib/api-index/extract.js"]

Loading

High-Level Assessment

Consolidating the duplicated @supports scanning logic into a single character-based, parenthesis-aware helper is the correct fix — it directly addresses the root cause (line-based brace counting failing on @Property feature-query preludes) and eliminates the risk of P2/P7 drifting apart. No alternative architecture (e.g., a full CSS parser/AST library) is warranted for this narrow, well-understood scanning need.

Files changed (17) +330 / -101

Enhancement (2) +52 / -26

gen-class-reference.jsFail loudly on missing FILE_TITLES entry; share parse helpers +32/-24

Fail loudly on missing FILE_TITLES entry; share parse helpers

• Replaces the inline FILE_META object with FILE_TITLES plus an explicit throw when a CLASS_FILES entry lacks a title, and switches to the shared stripComments/stripStrings/requireFile helpers from scripts/lib/parse.js.

scripts/gen-class-reference.js

extract.jsAdd metaFor() helper to fail loudly on missing FILE_META entries +20/-2

Add metaFor() helper to fail loudly on missing FILE_META entries

• Introduces metaFor(rel) which throws an actionable error instead of returning undefined when a registry-sources.js file lacks a FILE_META entry, and replaces direct FILE_META lookups with it.

scripts/lib/api-index/extract.js

Bug fix (3) +35 / -8

components.cssWrap ungated color-mix() hover backgrounds in @supports +13/-3

Wrap ungated color-mix() hover backgrounds in @supports

• Fixes an ungated color-mix() declaration on .sf-btn--secondary/--ghost hover backgrounds by wrapping both in an @supports (background: color-mix(...)) block, with an explanatory comment referencing the gating policy and its enforcing tests.

optional/components.css

check-artifacts.jsFail loudly when a declared artifact output is not git-tracked +13/-0

Fail loudly when a declared artifact output is not git-tracked

• Adds an isTracked() check before the git diff freshness check so untracked or gitignored declared outputs cause an explicit error instead of silently bypassing the guard.

scripts/check-artifacts.js

parse.jsHonour backslash escapes in stripStrings()/maskStrings() regexes +9/-5

Honour backslash escapes in stripStrings()/maskStrings() regexes

• Updates the string-literal regexes to correctly handle escaped quotes (e.g. content: "\"") so they no longer terminate the match early, and clarifies documentation comments.

scripts/lib/parse.js

Tests (4) +199 / -54

configurator-data-contract.test.jsAdd contract test pinning generated configurator data schema +66/-0

Add contract test pinning generated configurator data schema

• New test file validating required keys and non-empty catalogues for api-index, classes, bundles, and token-registry generated JSON files to catch silent shape changes breaking the vendored plugin.

tests/configurator-data-contract.test.js

supports-helpers.jsAdd shared character-based @supports scanner helpers +85/-0

Add shared character-based @supports scanner helpers

• New module implementing stripSupports() and findUngatedModernExpressions(), a parenthesis-aware, comment-stripped scanner shared by P2 and P7 tests to prevent drift and fix prior brace-miscounting bugs.

tests/supports-helpers.js

tier1-p2-coverage.test.jsExpand P2 coverage to all core/optional CSS files using shared scanner +32/-27

Expand P2 coverage to all core/optional CSS files using shared scanner

• Replaces the tokens.css-only, line-based checks with a loop over every core/*.css and optional/*.css file using the shared findUngatedModernExpressions() scanner.

tests/tier1-p2-coverage.test.js

tier1-p7-oldengine.test.jsReplace inline line-based stripSupports with shared scanner; add whole-bundle check +16/-27

Replace inline line-based stripSupports with shared scanner; add whole-bundle check

• Removes the local buggy line-based stripSupports() implementation in favor of the shared tests/supports-helpers.js version, and adds a new test scanning every declaration (not just custom properties) in the built bundle for ungated modern expressions.

tests/tier1-p7-oldengine.test.js

Documentation (3) +29 / -9

CLAUDE.mdClarify version-sync ownership table with 'How it's kept correct' column +13/-9

Clarify version-sync ownership table with 'How it's kept correct' column

• Adds a third column to the version-sync table distinguishing manually-synced fields from build-derived ones, and explains why build-derived artifacts can't drift.

CLAUDE.md

gap-analysis.mdAdd STATUS header marking file as a historical snapshot +5/-0

Add STATUS header marking file as a historical snapshot

• Adds a comment header clarifying the document is a one-off historical analysis not wired into CI, to be superseded rather than edited in place.

analysis/gap-analysis.md

REPORT.mdAdd ownership/status header to full-api-audit report +11/-0

Add ownership/status header to full-api-audit report

• Documents that the audit harness is manually run, not part of CI, and clarifies how committed result snapshots relate to docs-artifacts-sync tests.

reports/full-api-audit/REPORT.md

Other (5) +15 / -4

full-api-demo-with-overrides.htmlBump demo version string to v0.7.3 +1/-1

Bump demo version string to v0.7.3

• Updates the displayed framework version from v0.7.0 to v0.7.3 to match the current release.

demos/full-api-demo-with-overrides.html

full-api-demo.htmlBump demo version string to v0.7.3 +1/-1

Bump demo version string to v0.7.3

• Updates the displayed framework version from v0.7.0 to v0.7.3 to match the current release.

demos/full-api-demo.html

ultimate-override.cssBump generated override CSS version header to v0.7.3 +1/-1

Bump generated override CSS version header to v0.7.3

• Updates the version comment in the generated override stylesheet to match the current release.

demos/ultimate-override.css

package.jsonRemove unused lightningcss devDependency +0/-1

Remove unused lightningcss devDependency

• Removes the lightningcss package from devDependencies since it is no longer used in the build.

package.json

artifacts.jsonRegister classes.generated.json and demo outputs as tracked artifacts +12/-0

Register classes.generated.json and demo outputs as tracked artifacts

• Adds configurator/src/data/classes.generated.json to the guarded outputs list and registers a new 'full-api demos' artifact entry covering demos/generate.mjs outputs.

scripts/artifacts.json

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 12 rules

1. @supports stripper ignores strings ✓ Resolved 🐞 Bug ☼ Reliability

Description

tests/supports-helpers.js stripSupports() counts {/} while walking an @supports body but never
skips quoted strings, so braces (or parentheses) inside string literals can prematurely end (or
extend) the removed region and leave gated declarations behind or remove ungated ones. This can make
tier1-p2-coverage and tier1-p7-oldengine intermittently incorrect as soon as a supported block
contains delimiter-like characters inside strings (e.g., content strings or data URIs).

Code

tests/supports-helpers.js[R41-47]

+      let depth = 1;
+      j++;
+      while (j < n && depth > 0) {
+        const ch = src[j];
+        if (ch === '{') depth++;
+        else if (ch === '}') depth--;
+        j++;

Relevance

⭐⭐⭐ High

Team frequently hardens scanners to skip strings/comments (parse.js string-escape fixes accepted in
PR #474; related scanner tweaks in #196).

PR-#474
PR-#196
PR-#254

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

stripSupports() increments/decrements brace depth for every {/} character and has no handling
for quoted strings, so delimiters inside strings affect block matching. The repo already contains a
brace-walker that explicitly skips quoted strings during brace matching, demonstrating this is a
known correctness requirement for this kind of scanner.

tests/supports-helpers.js[19-55]
scripts/bundle.js[81-115]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`stripSupports()` removes `@supports` blocks by brace-counting, but it does not skip over quoted strings while scanning the prelude/body. Any `{`, `}`, `(`, or `)` inside a string literal can perturb `paren`/`depth` and cause incorrect stripping.

### Issue Context
This helper is now shared by P2 and P7, so a stripping bug impacts both source-level and bundle-level gating tests.

### Fix Focus Areas
- tests/supports-helpers.js[19-56]

### Implementation notes
- Update both scanners in `stripSupports()` (prelude scan and body scan) to detect `"` / `'` and advance `j` past the closing quote (handling backslash escapes) without interpreting braces/parentheses within the string.
- Preserve output text unchanged outside removed `@supports` blocks (i.e., do not globally replace strings before stripping, since P7 also uses the stripped output for declaration extraction).
- You can mirror the string-skip logic already used elsewhere in the repo for brace-walking (see `findMatchingBrace()` in `scripts/bundle.js`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[jackgranatowski]

This PR was already merged into main via merge commit 40d352f ("Merge pull request #565 from codeslash-dev/claude/repo-security-audit-dlbw5f"). Its second parent is exactly this PR's head commit 2f825fe, so all of its changes are in main.

The web merge succeeded at the git level but the request errored before GitHub could flip the PR status, leaving it stuck as "Open". Because the code is already in main, re-merging isn't possible (and would only create a duplicate/empty merge), so closing this as complete. The merge is permanently recorded in main's history via 40d352f.

---

Generated by Claude Code