diff --git a/cli/engine/design-system.mjs b/cli/engine/design-system.mjs index b9d9f3f69..87320dbe5 100644 --- a/cli/engine/design-system.mjs +++ b/cli/engine/design-system.mjs @@ -14,6 +14,10 @@ const FALLBACK_DIRS = ['.agents/context', 'docs']; // boundaries; `.impeccable` is our own project marker. const PROJECT_ROOT_MARKERS = ['.git', 'package.json', '.impeccable']; const COLOR_CHANNEL_TOLERANCE = 6; +// Shadow blacks at different alphas are different tokens (0.28 vs 0.55 is the +// difference between a documented shadow and drift), so shadow matching cannot +// reuse the r/g/b-only channel tolerance. +const SHADOW_ALPHA_TOLERANCE = 0.02; const RADIUS_TOLERANCE_PX = 0.5; const FONT_SIZE_TOLERANCE_PX = 0.5; const FONT_SIZE_LITERAL_RE = /^-?[\d.]+(?:px|rem)$/; @@ -474,6 +478,25 @@ function addSidecarRadii(out, sidecar) { } } +// Sidecar `extensions.shadows` entries ({ name, value, purpose }) carry the +// documented shadow vocabulary that Stitch's frontmatter schema can't hold. +// Their colors go into a separate allowlist — NOT allowedColorKeys — because a +// shadow black is only documented *as a shadow*: feeding it into the general +// color allowlist would legalize #000 as a page ground (alpha is dropped from +// colorKey), which is the hole issue #547 warns against. +function addSidecarShadows(out, sidecar) { + const shadows = sidecar?.extensions?.shadows; + if (!Array.isArray(shadows)) return; + + for (const entry of shadows) { + if (typeof entry?.value !== 'string') continue; + for (const match of entry.value.matchAll(CSS_COLOR_RE)) { + const parsed = parseDesignColor(match[0]); + if (parsed) out.allowedShadowColors.push({ color: parsed }); + } + } +} + function normalizeDesignSystem(input = {}) { const frontmatter = input.frontmatter || {}; const sidecar = input.sidecar || null; @@ -486,6 +509,7 @@ function normalizeDesignSystem(input = {}) { allowedColorKeys: new Map(), allowedRadii: [], allowedFontSizes: [], + allowedShadowColors: [], hasPillRadius: false, }; @@ -495,6 +519,7 @@ function normalizeDesignSystem(input = {}) { addSidecarColors(out, sidecar); addRoundedScale(out, frontmatter.rounded); addSidecarRadii(out, sidecar); + addSidecarShadows(out, sidecar); out.hasFonts = out.allowedFonts.size > 0; out.hasColors = out.allowedColorKeys.size > 0; @@ -614,6 +639,20 @@ function isAllowedColorRaw(raw, designSystem) { return false; } +// A color is a documented shadow color only when both the r/g/b channels AND +// the alpha match a sidecar shadow token's color. Alpha has to be compared +// here because colorKey()/colorsClose() drop it, and a match on r/g/b alone +// would let every black at every alpha through. +function isAllowedShadowColorRaw(raw, designSystem) { + if (!designSystem?.allowedShadowColors?.length) return false; + const parsed = parseDesignColor(String(raw || '').trim().toLowerCase()); + if (!parsed) return false; + return designSystem.allowedShadowColors.some(entry => + colorsClose(parsed, entry.color) && + Math.abs((parsed.a ?? 1) - (entry.color.a ?? 1)) <= SHADOW_ALPHA_TOLERANCE, + ); +} + function isAllowedRadiusRaw(raw, designSystem) { if (!designSystem?.hasRadii) return true; const text = String(raw || '').trim().toLowerCase(); @@ -691,6 +730,23 @@ function isProbablyColorLiteral(line, match) { return styleContext || cssFunctionContext || jsColorKeyContext; } +// True when the color literal sits inside a box-shadow / text-shadow value — +// the only contexts where a documented shadow color is legal. Anchored to the +// end of `before` (no ; } { or quote in between) so a shadow property earlier +// on the line can't leak the allowance into a later declaration. Kept separate +// from isProbablyColorLiteral(), which stays a boolean for its existing call +// sites and deliberately discards which property matched. +function isShadowPropertyContext(line, match) { + const index = match.index ?? -1; + if (index < 0) return false; + const before = line.slice(0, index); + // Unlike jsColorKeyContext, the JS tail admits commas: a multi-layer shadow + // string is comma-separated, and a later property on the same line is still + // blocked because it sits past the string's closing quote. + return /(?:^|[{\s;"'`(,])(?:box-shadow|text-shadow)\s*:\s*[^;{}"'`]*$/i.test(before) + || /(?:^|[,{]\s*)(?:boxShadow|textShadow)\s*[:=]\s*["'`]?[^"'`}]*$/i.test(before); +} + function isInsideCssAttributeSelector(line, index) { if (index < 0) return false; const before = line.slice(0, index); @@ -824,6 +880,7 @@ function checkSourceDesignSystem(content, filePath, options = {}) { if (!isProbablyColorLiteral(line, match)) continue; const raw = cssColorLabel(match[0]); if (isAllowedColorRaw(raw, designSystem)) continue; + if (isShadowPropertyContext(line, match) && isAllowedShadowColorRaw(raw, designSystem)) continue; findings.push(makeDesignFinding( 'design-system-color', filePath, @@ -1038,6 +1095,7 @@ export { loadDesignSystemForCwd, isAllowedFont, isAllowedColorRaw, + isAllowedShadowColorRaw, isAllowedRadiusRaw, isAllowedFontSizeRaw, checkSourceDesignSystem, diff --git a/tests/design-system.test.mjs b/tests/design-system.test.mjs index 7999df940..6dc3bcffa 100644 --- a/tests/design-system.test.mjs +++ b/tests/design-system.test.mjs @@ -13,6 +13,7 @@ import { checkSourceDesignSystem, collectStaticDesignSystemFindings, isAllowedColorRaw, + isAllowedShadowColorRaw, isAllowedFont, isAllowedRadiusRaw, isAllowedFontSizeRaw, @@ -282,6 +283,9 @@ rounded: roundedMeta: { lg: { canonical: '24px' }, }, + shadows: [ + { name: 'ambient-low', value: '0 4px 24px rgba(0,0,0,0.12)', purpose: 'Diffuse hover glow.' }, + ], }, })); @@ -296,6 +300,8 @@ rounded: assert.equal(isAllowedColorRaw('#d55a42', loaded), true); assert.equal(isAllowedRadiusRaw('80px', loaded), true); assert.equal(isAllowedRadiusRaw('24px', loaded), true); + assert.equal(isAllowedShadowColorRaw('rgba(0, 0, 0, 0.12)', loaded), true); + assert.equal(isAllowedShadowColorRaw('rgba(0, 0, 0, 0.5)', loaded), false); }); it('unescapes YAML-escaped quotes around multi-word font families (issue #428)', () => { @@ -474,6 +480,106 @@ const badge = { className: "text-[10px]" }; }); }); +describe('sidecar shadow tokens (issue #547)', () => { + // Mirrors the sidecar `extensions.shadows` schema from document.md Step 4b. + function shadowDesignSystem() { + return normalizeDesignSystem({ + frontmatter: { + colors: { ink: '#241f1a', paper: '#f7f4ee' }, + }, + sidecar: { + extensions: { + shadows: [ + { + name: 'outset', + value: 'inset 0 1px 0 oklch(1 0 0 / 0.07), 0 1px 2px oklch(0 0 0 / 0.28), 0 4px 12px oklch(0 0 0 / 0.22)', + purpose: 'Default card shadow.', + }, + ], + }, + }, + }); + } + + it('matches documented shadow colors on alpha, not just r/g/b', () => { + const designSystem = shadowDesignSystem(); + assert.equal(isAllowedShadowColorRaw('oklch(0 0 0 / 0.28)', designSystem), true); + assert.equal(isAllowedShadowColorRaw('rgba(0, 0, 0, 0.28)', designSystem), true); + assert.equal(isAllowedShadowColorRaw('oklch(1 0 0 / 0.07)', designSystem), true); + // Same black, undocumented alpha: the r/g/b channels alone must not match. + assert.equal(isAllowedShadowColorRaw('oklch(0 0 0 / 55%)', designSystem), false); + assert.equal(isAllowedShadowColorRaw('#000', designSystem), false); + // Shadow tokens must not switch the general color rule's allowlist on. + assert.equal(isAllowedColorRaw('oklch(0 0 0 / 0.28)', designSystem), false); + }); + + it('allows documented shadow colors in shadow contexts only', () => { + const designSystem = shadowDesignSystem(); + const findings = checkSourceDesignSystem(` +.a { box-shadow: 0 1px 2px oklch(0 0 0 / 0.28); } +.b { box-shadow: 0 20px 50px oklch(0 0 0 / 55%); } +.c { background: #000; } +.d { background: oklch(0 0 0 / 0.28); } +.e { text-shadow: 0 1px 2px oklch(0 0 0 / 0.28); } +.f { box-shadow: inset 0 1px 0 oklch(1 0 0 / 0.07), 0 4px 12px oklch(0 0 0 / 0.22); } +const card = { boxShadow: "0 1px 2px rgba(0, 0, 0, 0.28)" }; +const layered = { boxShadow: "0 1px 2px rgba(0, 0, 0, 0.28), 0 4px 12px rgba(0, 0, 0, 0.22)" }; +const bad = { color: "rgba(0, 0, 0, 0.28)" }; +const leak = { boxShadow: "0 1px 2px rgba(0, 0, 0, 0.28)", color: "rgba(0, 0, 0, 0.28)" }; +`, '/tmp/shadows.css', { designSystem }); + const colors = findings.filter((item) => item.antipattern === 'design-system-color'); + + // .a, .e, .f, and both JS boxShadow strings (including the second layer + // past the comma) pass; .b (undocumented alpha), .c (forbidden ground), + // .d (documented alpha outside a shadow), and both JS color keys still + // fire — the `leak` line proves the closing quote stops the shadow + // context from reaching a later property. A fix that silences .d has + // stopped discriminating between shadow usage and page grounds. + assert.deepEqual( + colors.map((item) => [item.line, item.ignoreValue]), + [ + [3, 'oklch(0 0 0 / 55%)'], + [4, '#000'], + [5, 'oklch(0 0 0 / 0.28)'], + [10, 'rgba(0, 0, 0, 0.28)'], + [11, 'rgba(0, 0, 0, 0.28)'], + ], + ); + }); + + it('abstains from the color rule entirely when only shadows are documented', () => { + // A shadows-only sidecar must not switch hasColors on: with no palette to + // measure against, the engine abstains rather than guesses, same as every + // other design-system rule. + const designSystem = normalizeDesignSystem({ + sidecar: { + extensions: { + shadows: [{ name: 'outset', value: '0 1px 2px oklch(0 0 0 / 0.28)' }], + }, + }, + }); + assert.equal(designSystem.hasColors, false); + const findings = checkSourceDesignSystem( + '.c { background: #000; }', + '/tmp/shadows-only.css', + { designSystem }, + ); + assert.equal(findings.some((item) => item.antipattern === 'design-system-color'), false); + }); + + it('does not allow a later declaration to inherit shadow context from earlier on the line', () => { + const designSystem = shadowDesignSystem(); + const findings = checkSourceDesignSystem( + '.x { box-shadow: 0 1px 2px oklch(0 0 0 / 0.28); background: oklch(0 0 0 / 0.28); }', + '/tmp/one-line.css', + { designSystem }, + ); + const colors = findings.filter((item) => item.antipattern === 'design-system-color'); + assert.equal(colors.length, 1); + assert.equal(colors[0].ignoreValue, 'oklch(0 0 0 / 0.28)'); + }); +}); + describe('collectStaticDesignSystemFindings()', () => { function makeElement(tagName, { text = '', attrs = {}, style = {}, parentElement = null } = {}) { return {