Files
arah2/.planning/phases/01-design-tokens-rams-foundation/01-REVIEW-FIX.md
T
2026-10-23 22:37:55 +08:00

8.7 KiB
Raw Blame History

phase, fixed_at, review_path, iteration, findings_in_scope, fixed, skipped, status
phase fixed_at review_path iteration findings_in_scope fixed skipped status
01-design-tokens-rams-foundation 2026-10-22T02:05:00Z .planning/phases/01-design-tokens-rams-foundation/01-REVIEW.md 1 6 6 0 all_fixed

Phase 1: Code Review Fix Report

Fixed at: 2026-10-22T02:05:00Z Source review: .planning/phases/01-design-tokens-rams-foundation/01-REVIEW.md Iteration: 1

Summary:

  • Findings in scope: 6 (BLOCKER CR-01 + WR-01..WR-05; INFO findings out of scope per orchestrator)
  • Fixed: 6
  • Skipped: 0

Note on provenance: a prior fixer run applied all six fixes as atomic commits (61d15c2…43b7d99) but terminated before writing this report. This pass independently re-verified each committed fix (diffs audited against the review's prescribed mechanism, all four verification gates re-run empirically, see Verification section) — no fixes were re-applied; master was already clean.

Fixed Issues

CR-01: Dead --radius-DEFAULT token — bare rounded compiles to 0.25rem (BLOCKER)

Commit: 61d15c2 Files modified: theme/src/tokens.css, theme/scripts/audit.mjs, theme/scripts/audit.test.mjs Applied fix: Replaced the inert v3-style --radius-DEFAULT: 0px with the deprecated bare --radius: 0px — the key Tailwind v4.3.3 actually drives the bare rounded utility from (node_modules/tailwindcss/theme.css "Deprecated" block). Block comment corrected (bare rounded is now var-driven; rounded-none/rounded-full are literal built-ins; the DEFAULT sub-key must not reappear). Audit radius drift guard extended to /--radius(?:-[A-Za-z0-9-]*)?…/ so a future non-zero bare key trips exit 2. Regression tests T9 added: bare-key presence, inert-DEFAULT absence, non-zero bare key → exit 2, and a controlled build asserting .rounded compiles to var(--radius) with --radius: 0px emitted by @theme static.

WR-01: Prose candidate leakage compiles Rams-forbidden .shadow/.blur into the shipped stylesheet

Commit: 689bc9f Files modified: theme/src/theme.css, theme/src/tokens.css, theme/scripts/audit.test.mjs Applied fix: (1) @source narrowed from the whole mockup tree to the four mockup markup files, plus @source not "../scripts" / @source not "../basemap" so toolchain prose can never mint utility classes. (2) Defense in depth at the key: the deprecated bare --shadow, --shadow-inner, --drop-shadow, --blur namespaces are set to initial in @theme static, so bare shadow/blur utilities can never compile from any candidate source (scale keys remain — mockup-worn shadow-* classes still compile and stay under adjudication 1). T5 regression checks added: no .shadow{ / .blur{ in dist/rams.css, no border-radius:.25rem anywhere in dist.

WR-02: familiesFromShorthand logic bug — off-token families in font: shorthands pass unflagged

Commit: 5e95741 Files modified: theme/scripts/audit.mjs, theme/scripts/audit.test.mjs Applied fix: Two-layers, contract-true: (1) any literal (non-var()) font: shorthand is flagged outright with the convention message (font: var(--type-*)), naming the extracted families; var() shorthands pass. (2) familiesFromShorthand itself repaired — iteratively strips the leading style/variant/weight keyword run (any CSS-grammar order, incl. oblique angles), then the size[/line-height] component (incl. calc()), per comma group, so the family is genuinely extracted for the message instead of being silently dropped by the old single-strip + numeric-filter bug. T10 regression tests: literal shorthand → exit 1, Arial extracted, sanctioned-family shorthand still flagged, var() shorthand passes.

WR-03: Color-evasion gaps — named keywords, modern color functions, text-shadow all pass

Commit: ee48e76 Files modified: theme/scripts/audit.mjs, theme/scripts/audit.test.mjs Applied fix: (1) Full CSS Color keyword list (NAMED_COLORS, ~148 entries) flagged on the color-bearing property set (COLORISH_PROPS incl. shorthands, fill/stroke, borders, text-decoration, accent-color); currentcolor/transparent and css-wide keywords stay sanctioned. (2) Modern color functions — oklch()/oklab()/lab()/lch()/hwb()/color()/color-mix() — flagged on any property unless the value is var()-bearing (Tailwind's own color-mix emissions are var-driven and pass). (3) text-shadow added to shadowViolations: any non-none value forbidden (Rams Shapes). T11 regression tests: color: red, oklch(...), color(display-p3 ...), non-hex text-shadow all exit 1 and are listed; sanctioned keywords pass. Audit header (§5) updated to document the extended surface.

WR-04: D-13 contract shortfall — spacing and typography numerics not audited

Commit: 5e254cb Files modified: theme/scripts/audit.mjs, theme/gallery.html Applied fix: Implemented the missing checks rather than amending the claim. (1) Spacing: padding/margin/gap families (logical variants included) — literals must equal a --spacing-* token value, be zero, or auto; dimensional geometry (width/height, offsets, border widths, the 18×18 checkbox) is spec-per-components-header and out of scope (adjudication 6). (2) Typography: font-size/font-weight/line-height/letter-spacing validated against the --type-* scale, parsed from the compound token grammar (weight size/line-height family… + -tracking companions); normal/bold keyword equivalents sanctioned; @font-face blocks exempt from numeric scale as asset metadata (D-16) but still color/family-checked (adjudication 7). npm run audit output now reports "6 spacing values / 10 type sizes / 9 trackings". Gallery D-13 evidence wording updated to "colors, spacing, typography, radius, shadows, blur". T12 regression tests: off-scale numerics (padding: 13px, font-size: 17px, weight 650, tracking) → exit 1 and listed; token-value literals (1rem, 0.25rem, 14px, 700, 20px, -0.04em), zero/auto, and font-face metadata pass.

WR-05: Static server path guard uses prefix match without separator

Commit: 43b7d99 Files modified: theme/scripts/screenshot.mjs Applied fix: sep imported from node:path; guard changed to if (target !== THEME && !target.startsWith(THEME + sep)) — the docroot prefix must end at a path separator, so a decoded ..%2FthemeX (which joins to a themeX-sibling path passing the old bare startsWith(THEME)) now 403s.

Skipped Issues

None — all six in-scope findings fixed and verified.

Verification (all four gates, empirically re-run in this pass)

  1. Build radius gate — npm run build --workspace theme; grep -o 'border-radius:[^;}]*' theme/dist/rams.css | sort -u → border-radius:0, border-radius:var(--radius), border-radius:3.40282e38px (the sanctioned rounded-full ≥1e30px literal clamp). No .25rem / non-token radius forms. .rounded compiles to border-radius:var(--radius) with --radius:0px emitted by @theme static. ✅
  2. Audit gate — npm run audit (repo root) → AUDIT CLEAN — 354 declarations checked … — 0 off-token values. token sheet: 55 colors / 9 radius tokens (all zero) / 7 sanctioned font families / 6 spacing values / 10 type sizes / 9 trackings. ✅
  3. Test suite — node theme/scripts/audit.test.mjs → 38 passed, 0 failed (was 16/16 pre-fix; now includes T9 CR-01, T10 WR-02, T11 WR-03, T12 WR-04, extended T5 WR-01). ✅
  4. Screenshot server guard (live probe) — server started under the real capture run (node theme/scripts/screenshot.mjs, listener located by PID), probed with curl --path-as-is:
    • /..%2FthemeX/secret → 403 forbidden (the exact WR-05 exploit shape: a themeX sibling that the old bare prefix check passed)
    • /..%2FthemeX/gallery.html → 403
    • /..%2Fstitch_minimalist_route_navigator/… → 403
    • /src/sheet.html → 200 (correct content — no over-blocking)
    • /gallery.html → 200 — ✅ (Method note: a first probe port grab had hit Chromium's own devtools listener — PID-matched listener selection fixed that; the guard result above is from the real capture server.) Bonus: full capture run completed CAPTURE OK — 6/6 non-blank captures.

Also verified byte-for-byte: deep diff audit of all six fix commits against the REVIEW.md prescribed fix shapes (git show 61d15c2 689bc9f 5e95741 ee48e76 5e254cb 43b7d99) — mechanisms are real (key-level token disarmament, regex/grammar repair, parser extension, guard change), not cosmetic suppressions; per CR-01 guidance the elimination of the banned utility is the intended goal, so post-fix compiled output genuinely lacks .rounded/.shadow/.blur non-zero/off-token forms.


Fixed: 2026-10-22T02:05:00Z Fixer: OpenCode (gsd-code-fixer) Iteration: 1