fix(design): report inspection gaps and harden source tracing

Author: Bill Leoutsakos

.agents/skills/emcn-design-review/SKILL.md

@@ -6,7 +6,9 @@ argument-hint: "[scope] [fix=true|false]"
 
 # EMCN design review
 
-Review the requested product UI scope (default: current changes). When `fix=false`, explain proposed changes without applying them.
+User arguments: $ARGUMENTS
+
+Interpret the arguments as the product UI scope (default: current changes) and an optional `fix=true|false` mode (default: `false`). When `fix=false`, explain proposed changes without applying them.
 
 1. When EMCN, global styles, recipes or design ownership metadata change, run `bun run design:generate` and commit `scripts/design-conformance/contracts.generated.json` with the source. `bun run check:design-generated` checks freshness without writing. Regeneration does not hide the originating design-system finding.
 2. During UI work, run `bun run check:design --base origin/staging --working-tree` from the repo root, substituting the actual PR target for `origin/staging`. After committing, use `--head HEAD` for the immutable PR comparison. Exit 1 means findings to review; exit 2 means the check failed and must be repaired or reported. CI is warning-only for findings and fails on incomplete analysis.

bun.lock

@@ -18,8 +18,8 @@
     "test:setup": "bun run --cwd packages/sim-setup test",
     "test:integration": "bun --no-env-file scripts/test-integration.ts",
     "test:search-performance": "KNOWLEDGE_SEARCH_PERFORMANCE_TEST=true bun --no-env-file scripts/test-integration.ts search-latency",
-    "format": "biome format --write scripts/design-conformance scripts/check-design-conformance*.ts && turbo run format",
-    "format:check": "biome format scripts/design-conformance scripts/check-design-conformance*.ts && turbo run format:check",
+    "format": "biome format --write scripts/design-conformance scripts/design-scan scripts/design-studio tools/design-studio scripts/check-design-conformance*.ts scripts/generate-design-contracts*.ts && turbo run format",
+    "format:check": "biome format scripts/design-conformance scripts/design-scan scripts/design-studio tools/design-studio scripts/check-design-conformance*.ts scripts/generate-design-contracts*.ts && turbo run format:check",
     "lint": "biome check --write scripts/design-conformance scripts/design-scan scripts/design-studio tools/design-studio scripts/check-design-conformance*.ts scripts/generate-design-contracts*.ts && turbo run lint",
     "lint:check": "biome check scripts/design-conformance scripts/design-scan scripts/design-studio tools/design-studio scripts/check-design-conformance*.ts scripts/generate-design-contracts*.ts && turbo run lint:check",
     "lint:helm": "helm lint helm/sim --strict --values helm/sim/ci/default-values.yaml",
@@ -173,13 +173,16 @@
     "@octokit/rest": "^21.0.0",
     "@sim/utils": "workspace:*",
     "@types/babel__traverse": "7.28.0",
+    "@types/aria-query": "5.0.4",
     "@types/bun": "1.4.1",
     "@types/node": "24.2.1",
     "@types/opentype.js": "1.3.10",
     "@typescript/native": "npm:typescript@^7.0.2",
     "@typescript/typescript6": "^6.0.2",
     "@vercel/og": "0.6.8",
     "ajv": "8.18.0",
+    "aria-query": "5.3.2",
+    "color-name": "1.1.4",
     "commander": "^11.1.0",
     "concurrently": "10.0.5",
     "drizzle-orm": "^0.45.2",

package.json

@@ -774,7 +774,7 @@ export type ChipModalFieldProps =
  * static permission list). For a multi-email chip-list input, prefer
  * `type='emails'` over a `type='custom'` `TagInput` wrapper — it internalizes
  * chip rendering, dedupe, format validation, paste, and Backspace handling.
- * @designProtect className *
+ * @designProtect className colours typography padding height gap border-radius borders box-shadow flex-direction align-items justify-content
  */
 function ChipModalField(props: ChipModalFieldProps) {
   const id = React.useId()

packages/emcn/src/components/chip-modal/chip-modal.tsx

@@ -1,6 +1,7 @@
 /** biome-ignore-all lint/suspicious/noTemplateCurlyInString: Fixtures contain literal proposed JavaScript templates. */
 import { existsSync } from 'node:fs'
 import { beforeAll, expect, test } from 'vitest'
+import { inspectControls } from '#control-analysis/inventory'
 import { extract } from '#design-conformance/extract'
 import { loadCatalogue } from '#design-conformance/io'
 import { Linter } from '#design-conformance/lint'
@@ -413,3 +414,37 @@ test('tag differences are not blanket-equated for text-bearing elements', () =>
     diff('<span className="p-2">Label</span>', '<div className="p-2">Label</div>').flagged
   ).toBe(true)
 })
+
+test('changing one of repeated styled surfaces retains its previous appearance', () => {
+  const r = diff(
+    'export const A=()=> <><button className="p-2"/><button className="p-2"/></>',
+    'export const A=()=> <><button className="p-2"/><button className="p-4"/></>'
+  )
+  expect(r.findings.some((f) => f.rule === 'appearance-changed' && f.before !== null)).toBe(true)
+})
+
+test('appearance findings retain direct attribution to the cited consumer slot', () => {
+  const source = 'export const A=()=> <button className="p-4">Save</button>'
+  const report = diff(source.replace('p-4', 'p-2'), source)
+  const findings = report.findings.map((f, index) => ({
+    ...f,
+    id: String(index),
+    observedFrom: [ui],
+  }))
+  const result = inspectControls(
+    {
+      entries: [
+        {
+          path: ui,
+          blob: 'a'.repeat(40),
+          mode: '100644',
+          kind: 'blob',
+          bytes: Buffer.byteLength(source),
+        },
+      ],
+      read: () => source,
+    },
+    findings
+  )
+  expect(result.records[0].findingIds).toEqual(findings.map((f) => f.id))
+})

scripts/check-design-conformance-appearance.test.ts

@@ -510,3 +510,20 @@ test('literal joins and unshadowed String.raw resolve class assignments without
     }).length
   ).toBeGreaterThan(0)
 })
+
+test('a global shadow must contain a complete shadow recipe, not just a named variable', () => {
+  for (const definition of ['8px', 'red', 'var(--text-body)', '0 0 2px var(--missing)']) {
+    const report = inspect({
+      [tokens]: `${globals}:root{--shadow-card:${definition}}`,
+      [ui]: 'export const A=()=> <span style={{boxShadow:"var(--shadow-card)"}}/>',
+    })
+    expect(report.verifiedUsages).toEqual([])
+    expect([...report.findings, ...report.unchecked].length).toBeGreaterThan(0)
+  }
+  expect(
+    inspect({
+      [tokens]: `${globals}:root{--shadow-card:0 1px 2px #000}`,
+      [ui]: 'export const A=()=> <span style={{boxShadow:"var(--shadow-card)"}}/>',
+    }).assignments.some((a) => a.name === 'box-shadow' && a.status === 'verified')
+  ).toBe(true)
+})

scripts/check-design-conformance-colour-assignments.test.ts

@@ -444,3 +444,17 @@ test('public diff checker separates layout allowances and exposes a later intern
   expect(changed.findings.some((f) => f.value === 'p-4')).toBe(true)
   expect(changed.findings.some((f) => f.value === 'flex-row')).toBe(false)
 })
+
+test('the public diff includes local custom-property writes despite legacy token-policy exemptions', async () => {
+  const repo = mkdtempSync(path.join(temp, 'local-writer-'))
+  git(repo, ['init', '-q'])
+  put(repo, 'apps/sim/app/_styles/globals.css', ':root{--text-body:#444}')
+  put(repo, ui, 'export const A=()=> <span>Label</span>')
+  const base = commit(repo)
+  put(repo, ui, 'export const A=()=> <span style={{"--text-body":"red"}}>Label</span>')
+  const head = commit(repo)
+  const report = await checkComparison({ repo, base, head, policy: 'conformance' })
+  expect(
+    report.findings.some((f) => f.rule === 'central-colour-assignment' && f.value.includes('red'))
+  ).toBe(true)
+})

scripts/check-design-conformance-control-comparison.test.ts

@@ -1,3 +1,4 @@
+/** biome-ignore-all lint/suspicious/noTemplateCurlyInString: Fixtures contain proposed source text. */
 import { readFileSync } from 'node:fs'
 import { expect, test } from 'vitest'
 import type { ControlSource } from '#control-analysis/model'
@@ -409,3 +410,24 @@ test('syntax and extraction errors remain distinguishable', () => {
     false
   )
 })
+
+test('rendered CSS template substitutions remain visible as unchecked analysis', () => {
+  const report = inspect({
+    [ui]: 'const html=`<style>body{color:${color}}</style>`;new Blob([html],{type:"text/html"})',
+  })
+  expect(report.review.unchecked.some((n) => /CSS|substitut/i.test(n.reason))).toBe(true)
+})
+
+test('hidden false and runtime visibility cannot hide local control styling', () => {
+  for (const value of ['false', 'visible']) {
+    const report = inspect({
+      [ui]: `export const A=()=> <button hidden={${value}} className="rounded-xl bg-red-500">Go</button>`,
+    })
+    expect(report.review.findings.some((f) => f.rule === 'local-control')).toBe(true)
+  }
+  expect(
+    inspect({
+      [ui]: 'export const A=()=> <button hidden className="rounded-xl bg-red-500">Go</button>',
+    }).review.findings.some((f) => f.rule === 'local-control')
+  ).toBe(false)
+})

scripts/check-design-conformance-coverage.test.ts

@@ -1,4 +1,4 @@
-import { execFileSync, spawnSync } from 'node:child_process'
+import { spawnSync } from 'node:child_process'
 import { createHash } from 'node:crypto'
 import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
 import os from 'node:os'
@@ -170,9 +170,9 @@ test('single and multi-process evaluation match on synthetic stored data', () =>
     )
     expect(storedChanges(dataset, c)).toHaveLength(1)
   }
-  const run = (workers: number) => {
-    const out = path.join(repo, `evaluation-${workers}`)
-    execFileSync(
+  const run = (workers: number, expectedExit = 0) => {
+    const out = path.join(repo, `evaluation-${workers}-${expectedExit}`)
+    const result = spawnSync(
       'bun',
       [
         '--no-env-file',
@@ -188,6 +188,7 @@ test('single and multi-process evaluation match on synthetic stored data', () =>
       ],
       { stdio: ['ignore', 'pipe', 'pipe'] }
     )
+    expect(result.status).toBe(expectedExit)
     return out
   }
   const one = run(1)
@@ -196,4 +197,33 @@ test('single and multi-process evaluation match on synthetic stored data', () =>
     expect(readFileSync(path.join(one, `${pr}.json`), 'utf8')).toBe(
       readFileSync(path.join(two, `${pr}.json`), 'utf8')
     )
+  writeFileSync(path.join(dataset, 'objects', `${blob}.gz`), gzipSync('tampered input'))
+  for (const workers of [1, 2]) {
+    const output = run(workers, 2)
+    const summary = JSON.parse(readFileSync(path.join(output, 'evaluation.json'), 'utf8'))
+    expect(summary.failed).toEqual([1, 2])
+    for (const pr of [1, 2])
+      expect(JSON.parse(readFileSync(path.join(output, `${pr}.json`), 'utf8')).status).toBe(
+        'failed'
+      )
+  }
+})
+
+test('unwritable CLI output remains an operational exit, not an uncaught retry', () => {
+  const result = spawnSync('bun', [
+    '--no-env-file',
+    cli,
+    '--policy',
+    'appearance',
+    '--repo',
+    repo,
+    '--base',
+    base,
+    '--head',
+    head,
+    '--output',
+    repo,
+  ])
+  expect(result.status).toBe(2)
+  expect(result.stderr.toString()).toContain('failed')
 })

scripts/check-design-conformance-io.test.ts

@@ -86,3 +86,9 @@ test('registered source definitions integrate with snapshots and system-change r
   expect(unresolved.flagged).toBe(false)
   expect(unresolved.unchecked.some((n) => n.reason.includes(name))).toBe(true)
 })
+
+test('removing an unresolved registered recipe still reports the source deletion', () => {
+  expect(recipeDiff(file, parse(source('unknown()')), parse(''))).toMatchObject([
+    { value: '(registered export removed)', before: 'unknown()' },
+  ])
+})