From 8f33a618dd78b317b47644ef8fe24f60d7052c6e Mon Sep 17 00:00:00 2001 From: Ryan Dombrowski Date: Thu, 2 Jul 2026 12:31:31 -0400 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20surface=20gates=20S1-S3=20=E2=80=94?= =?UTF-8?q?=20gate=20runner,=20rule=20registry,=20all=20three=20evaluators?= =?UTF-8?q?=20(M1=20PR-4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - src/core/lint/: gate runner (S1 generic surface schema, S2 contract vocabulary, S3 governance) — independently reported, never implicit in generation (the S0 spike found Ollama's mlx engine silently ignoring `format`, which is why S2 is a check on the artifact). - rule-type registry + evaluators per spec/dspack-v0.3.md §5.3 semantics. Findings carry both severity faces (requirement: must|should, level: error|warn); rationales verbatim; locations as $.root… paths. Unknown rule types throw UnknownRuleTypeError (CLI exit 4) — never skip. - DEVIATION FROM THE M1 DIRECTIVE, flagged for review: forbidden-composition is implemented now (not M2/PR-8). Forced by a conflict discovered in implementation: the v0.3 shadcn contract carries a UNIVERSAL forbidden-composition rule (rule.button-no-interactive-descendants) and spec §5.4 forbids skipping unimplemented types — a two-evaluator linter would exit 4 on every lint of the real contract. Fixture F5 activates with it (all five golden fixtures active). - CLI `lint`: JSON report on stdout (golden-comparable), human rendering on stderr; exits 0 clean / 2 any S-gate error / 4 unknown rule type. - fixtures/golden/violating/F1-F5 + clean golden + checked-in expected reports; core-boundary test now walks recursively (lint/ included), ajv allowed as the only non-node bare import. Verify: npm test; npx tsx src/cli.ts lint --dspack fixtures/shadcn.v0_3.dspack.json --surface fixtures/golden/violating/F1-dialog-for-delete.dsurface.json (exit 2, stdout equals F1-dialog-for-delete.expected.json) Co-Authored-By: Claude Fable 5 --- .github/workflows/test.yml | 10 + .../golden/clean/delete-account.dsurface.json | 53 +++++ .../F1-dialog-for-delete.dsurface.json | 52 +++++ .../F1-dialog-for-delete.expected.json | 54 +++++ .../F2-no-confirmation.dsurface.json | 18 ++ .../F2-no-confirmation.expected.json | 39 ++++ .../violating/F3-missing-cancel.dsurface.json | 49 +++++ .../violating/F3-missing-cancel.expected.json | 39 ++++ .../violating/F4-missing-title.dsurface.json | 49 +++++ .../violating/F4-missing-title.expected.json | 39 ++++ .../F5-nested-interactive.dsurface.json | 63 ++++++ .../F5-nested-interactive.expected.json | 38 ++++ src/cli.ts | 45 +++- src/core/core-boundary.test.ts | 15 +- src/core/index.ts | 1 + src/core/lint/findings.ts | 80 +++++++ src/core/lint/index.ts | 80 +++++++ src/core/lint/lint.test.ts | 130 +++++++++++ src/core/lint/rules.ts | 205 ++++++++++++++++++ src/core/lint/vocabulary.ts | 70 ++++++ src/core/lint/walk.ts | 44 ++++ src/core/surface-schema.ts | 86 ++++++++ 22 files changed, 1247 insertions(+), 12 deletions(-) create mode 100644 fixtures/golden/clean/delete-account.dsurface.json create mode 100644 fixtures/golden/violating/F1-dialog-for-delete.dsurface.json create mode 100644 fixtures/golden/violating/F1-dialog-for-delete.expected.json create mode 100644 fixtures/golden/violating/F2-no-confirmation.dsurface.json create mode 100644 fixtures/golden/violating/F2-no-confirmation.expected.json create mode 100644 fixtures/golden/violating/F3-missing-cancel.dsurface.json create mode 100644 fixtures/golden/violating/F3-missing-cancel.expected.json create mode 100644 fixtures/golden/violating/F4-missing-title.dsurface.json create mode 100644 fixtures/golden/violating/F4-missing-title.expected.json create mode 100644 fixtures/golden/violating/F5-nested-interactive.dsurface.json create mode 100644 fixtures/golden/violating/F5-nested-interactive.expected.json create mode 100644 src/core/lint/findings.ts create mode 100644 src/core/lint/index.ts create mode 100644 src/core/lint/lint.test.ts create mode 100644 src/core/lint/rules.ts create mode 100644 src/core/lint/vocabulary.ts create mode 100644 src/core/lint/walk.ts create mode 100644 src/core/surface-schema.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 688797e..9f9bc50 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,5 +18,15 @@ jobs: - run: npm test - name: Build (exports map consumers) run: npm run build + - name: Lint CLI gate (F1 golden, exit 2) + run: | + set +e + npx tsx src/cli.ts lint --dspack fixtures/shadcn.v0_3.dspack.json \ + --surface fixtures/golden/violating/F1-dialog-for-delete.dsurface.json \ + > /tmp/f1.json 2>/dev/null + code=$? + set -e + test "$code" -eq 2 + diff /tmp/f1.json fixtures/golden/violating/F1-dialog-for-delete.expected.json - name: CLI smoke run: npm run context -- --dspack fixtures/shadcn.v0_3.dspack.json --intent destructive-action > /dev/null diff --git a/fixtures/golden/clean/delete-account.dsurface.json b/fixtures/golden/clean/delete-account.dsurface.json new file mode 100644 index 0000000..07db943 --- /dev/null +++ b/fixtures/golden/clean/delete-account.dsurface.json @@ -0,0 +1,53 @@ +{ + "dspackSurface": "0.1", + "system": "shadcn/ui", + "intent": "destructive-action", + "root": { + "component": "card", + "children": [ + { + "component": "alert-dialog", + "children": [ + { + "component": "alert-dialog-trigger", + "children": [ + { + "component": "button", + "props": { + "variant": "destructive" + }, + "text": "Delete account" + } + ] + }, + { + "component": "alert-dialog-content", + "children": [ + { + "component": "alert-dialog-title", + "text": "Delete your account?" + }, + { + "component": "alert-dialog-description", + "text": "This will permanently delete your account and all associated data. This action cannot be undone." + }, + { + "component": "alert-dialog-footer", + "children": [ + { + "component": "alert-dialog-cancel", + "text": "Cancel" + }, + { + "component": "alert-dialog-action", + "text": "Delete account" + } + ] + } + ] + } + ] + } + ] + } +} \ No newline at end of file diff --git a/fixtures/golden/violating/F1-dialog-for-delete.dsurface.json b/fixtures/golden/violating/F1-dialog-for-delete.dsurface.json new file mode 100644 index 0000000..7fc1a72 --- /dev/null +++ b/fixtures/golden/violating/F1-dialog-for-delete.dsurface.json @@ -0,0 +1,52 @@ +{ + "dspackSurface": "0.1", + "system": "shadcn/ui", + "intent": "destructive-action", + "root": { + "component": "card", + "children": [ + { + "component": "dialog", + "children": [ + { + "component": "dialog-trigger", + "children": [ + { + "component": "button", + "props": { + "variant": "destructive" + }, + "text": "Delete account" + } + ] + }, + { + "component": "dialog-content", + "children": [ + { + "component": "dialog-title", + "text": "Delete your account?" + }, + { + "component": "dialog-description", + "text": "This will permanently delete your account." + }, + { + "component": "dialog-footer", + "children": [ + { + "component": "button", + "props": { + "variant": "destructive" + }, + "text": "Delete" + } + ] + } + ] + } + ] + } + ] + } +} \ No newline at end of file diff --git a/fixtures/golden/violating/F1-dialog-for-delete.expected.json b/fixtures/golden/violating/F1-dialog-for-delete.expected.json new file mode 100644 index 0000000..823dd44 --- /dev/null +++ b/fixtures/golden/violating/F1-dialog-for-delete.expected.json @@ -0,0 +1,54 @@ +{ + "gates": [ + { + "gate": "S1", + "name": "surface-schema", + "status": "PASS" + }, + { + "gate": "S2", + "name": "contract-vocabulary", + "status": "PASS" + }, + { + "gate": "S3", + "name": "governance", + "status": "FAIL" + } + ], + "findings": [ + { + "ruleId": "rule.destructive-requires-alertdialog", + "type": "component-choice", + "requirement": "must", + "level": "error", + "message": "Component 'dialog' is forbidden for intent 'destructive-action'.", + "rationale": "Dialog can be dismissed by clicking the overlay or pressing Escape, so a user can bypass a destructive confirmation without making a conscious choice. AlertDialog forces an explicit confirm/cancel decision and is announced with greater urgency by screen readers.", + "location": { + "path": "$.root.children[0]", + "component": "dialog" + }, + "exampleIds": [ + "ex.delete-account-confirmation" + ] + }, + { + "ruleId": "rule.destructive-requires-alertdialog", + "type": "component-choice", + "requirement": "must", + "level": "error", + "message": "Required component 'alert-dialog' does not appear in the surface.", + "rationale": "Dialog can be dismissed by clicking the overlay or pressing Escape, so a user can bypass a destructive confirmation without making a conscious choice. AlertDialog forces an explicit confirm/cancel decision and is announced with greater urgency by screen readers.", + "location": { + "path": "$.root", + "component": "surface" + }, + "exampleIds": [ + "ex.delete-account-confirmation" + ] + } + ], + "errorCount": 2, + "warnCount": 0, + "pass": false +} diff --git a/fixtures/golden/violating/F2-no-confirmation.dsurface.json b/fixtures/golden/violating/F2-no-confirmation.dsurface.json new file mode 100644 index 0000000..d25bfec --- /dev/null +++ b/fixtures/golden/violating/F2-no-confirmation.dsurface.json @@ -0,0 +1,18 @@ +{ + "dspackSurface": "0.1", + "system": "shadcn/ui", + "intent": "destructive-action", + "root": { + "component": "card", + "children": [ + { + "component": "button", + "id": "delete_btn", + "props": { + "variant": "destructive" + }, + "text": "Delete account" + } + ] + } +} \ No newline at end of file diff --git a/fixtures/golden/violating/F2-no-confirmation.expected.json b/fixtures/golden/violating/F2-no-confirmation.expected.json new file mode 100644 index 0000000..e5f820e --- /dev/null +++ b/fixtures/golden/violating/F2-no-confirmation.expected.json @@ -0,0 +1,39 @@ +{ + "gates": [ + { + "gate": "S1", + "name": "surface-schema", + "status": "PASS" + }, + { + "gate": "S2", + "name": "contract-vocabulary", + "status": "PASS" + }, + { + "gate": "S3", + "name": "governance", + "status": "FAIL" + } + ], + "findings": [ + { + "ruleId": "rule.destructive-requires-alertdialog", + "type": "component-choice", + "requirement": "must", + "level": "error", + "message": "Required component 'alert-dialog' does not appear in the surface.", + "rationale": "Dialog can be dismissed by clicking the overlay or pressing Escape, so a user can bypass a destructive confirmation without making a conscious choice. AlertDialog forces an explicit confirm/cancel decision and is announced with greater urgency by screen readers.", + "location": { + "path": "$.root", + "component": "surface" + }, + "exampleIds": [ + "ex.delete-account-confirmation" + ] + } + ], + "errorCount": 1, + "warnCount": 0, + "pass": false +} diff --git a/fixtures/golden/violating/F3-missing-cancel.dsurface.json b/fixtures/golden/violating/F3-missing-cancel.dsurface.json new file mode 100644 index 0000000..61c43cd --- /dev/null +++ b/fixtures/golden/violating/F3-missing-cancel.dsurface.json @@ -0,0 +1,49 @@ +{ + "dspackSurface": "0.1", + "system": "shadcn/ui", + "intent": "destructive-action", + "root": { + "component": "card", + "children": [ + { + "component": "alert-dialog", + "children": [ + { + "component": "alert-dialog-trigger", + "children": [ + { + "component": "button", + "props": { + "variant": "destructive" + }, + "text": "Delete account" + } + ] + }, + { + "component": "alert-dialog-content", + "children": [ + { + "component": "alert-dialog-title", + "text": "Delete your account?" + }, + { + "component": "alert-dialog-description", + "text": "This cannot be undone." + }, + { + "component": "alert-dialog-footer", + "children": [ + { + "component": "alert-dialog-action", + "text": "Delete account" + } + ] + } + ] + } + ] + } + ] + } +} \ No newline at end of file diff --git a/fixtures/golden/violating/F3-missing-cancel.expected.json b/fixtures/golden/violating/F3-missing-cancel.expected.json new file mode 100644 index 0000000..bc6b954 --- /dev/null +++ b/fixtures/golden/violating/F3-missing-cancel.expected.json @@ -0,0 +1,39 @@ +{ + "gates": [ + { + "gate": "S1", + "name": "surface-schema", + "status": "PASS" + }, + { + "gate": "S2", + "name": "contract-vocabulary", + "status": "PASS" + }, + { + "gate": "S3", + "name": "governance", + "status": "FAIL" + } + ], + "findings": [ + { + "ruleId": "rule.alertdialog-requires-cancel", + "type": "required-composition", + "requirement": "must", + "level": "error", + "message": "Required sub-component 'alert-dialog-cancel' (min 1) not found among descendants (found 0).", + "rationale": "A confirmation without an explicit cancel action and a title naming the consequence funnels the user toward the destructive action; the title is also required for aria-labelledby.", + "location": { + "path": "$.root.children[0]", + "component": "alert-dialog" + }, + "exampleIds": [ + "ex.delete-account-confirmation" + ] + } + ], + "errorCount": 1, + "warnCount": 0, + "pass": false +} diff --git a/fixtures/golden/violating/F4-missing-title.dsurface.json b/fixtures/golden/violating/F4-missing-title.dsurface.json new file mode 100644 index 0000000..42fae17 --- /dev/null +++ b/fixtures/golden/violating/F4-missing-title.dsurface.json @@ -0,0 +1,49 @@ +{ + "dspackSurface": "0.1", + "system": "shadcn/ui", + "intent": "destructive-action", + "root": { + "component": "card", + "children": [ + { + "component": "alert-dialog", + "children": [ + { + "component": "alert-dialog-trigger", + "children": [ + { + "component": "button", + "props": { + "variant": "destructive" + }, + "text": "Delete account" + } + ] + }, + { + "component": "alert-dialog-content", + "children": [ + { + "component": "alert-dialog-description", + "text": "This cannot be undone." + }, + { + "component": "alert-dialog-footer", + "children": [ + { + "component": "alert-dialog-cancel", + "text": "Cancel" + }, + { + "component": "alert-dialog-action", + "text": "Delete account" + } + ] + } + ] + } + ] + } + ] + } +} \ No newline at end of file diff --git a/fixtures/golden/violating/F4-missing-title.expected.json b/fixtures/golden/violating/F4-missing-title.expected.json new file mode 100644 index 0000000..55caff6 --- /dev/null +++ b/fixtures/golden/violating/F4-missing-title.expected.json @@ -0,0 +1,39 @@ +{ + "gates": [ + { + "gate": "S1", + "name": "surface-schema", + "status": "PASS" + }, + { + "gate": "S2", + "name": "contract-vocabulary", + "status": "PASS" + }, + { + "gate": "S3", + "name": "governance", + "status": "FAIL" + } + ], + "findings": [ + { + "ruleId": "rule.alertdialog-requires-cancel", + "type": "required-composition", + "requirement": "must", + "level": "error", + "message": "Required sub-component 'alert-dialog-title' (min 1) not found among descendants (found 0).", + "rationale": "A confirmation without an explicit cancel action and a title naming the consequence funnels the user toward the destructive action; the title is also required for aria-labelledby.", + "location": { + "path": "$.root.children[0]", + "component": "alert-dialog" + }, + "exampleIds": [ + "ex.delete-account-confirmation" + ] + } + ], + "errorCount": 1, + "warnCount": 0, + "pass": false +} diff --git a/fixtures/golden/violating/F5-nested-interactive.dsurface.json b/fixtures/golden/violating/F5-nested-interactive.dsurface.json new file mode 100644 index 0000000..b312827 --- /dev/null +++ b/fixtures/golden/violating/F5-nested-interactive.dsurface.json @@ -0,0 +1,63 @@ +{ + "dspackSurface": "0.1", + "system": "shadcn/ui", + "intent": "destructive-action", + "root": { + "component": "card", + "children": [ + { + "component": "alert-dialog", + "children": [ + { + "component": "alert-dialog-trigger", + "children": [ + { + "component": "button", + "props": { + "variant": "destructive" + }, + "text": "Delete account" + } + ] + }, + { + "component": "alert-dialog-content", + "children": [ + { + "component": "alert-dialog-title", + "text": "Delete your account?" + }, + { + "component": "alert-dialog-footer", + "children": [ + { + "component": "alert-dialog-cancel", + "text": "Cancel" + }, + { + "component": "alert-dialog-action", + "text": "Delete" + } + ] + } + ] + } + ] + }, + { + "component": "button", + "id": "subscribe", + "text": "Subscribe", + "children": [ + { + "component": "input", + "props": { + "type": "email" + }, + "text": "Email" + } + ] + } + ] + } +} \ No newline at end of file diff --git a/fixtures/golden/violating/F5-nested-interactive.expected.json b/fixtures/golden/violating/F5-nested-interactive.expected.json new file mode 100644 index 0000000..ea1ae06 --- /dev/null +++ b/fixtures/golden/violating/F5-nested-interactive.expected.json @@ -0,0 +1,38 @@ +{ + "gates": [ + { + "gate": "S1", + "name": "surface-schema", + "status": "PASS" + }, + { + "gate": "S2", + "name": "contract-vocabulary", + "status": "PASS" + }, + { + "gate": "S3", + "name": "governance", + "status": "FAIL" + } + ], + "findings": [ + { + "ruleId": "rule.button-no-interactive-descendants", + "type": "forbidden-composition", + "requirement": "must", + "level": "error", + "message": "Forbidden descendant 'input' found at $.root.children[1].children[0].", + "rationale": "Nested interactive elements create ambiguous click targets and are an accessibility violation: screen readers cannot determine intent and click handling varies across browsers.", + "location": { + "path": "$.root.children[1]", + "component": "button", + "nodeId": "subscribe" + }, + "exampleIds": [] + } + ], + "errorCount": 1, + "warnCount": 0, + "pass": false +} diff --git a/src/cli.ts b/src/cli.ts index bfb90fd..9c8c636 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -3,17 +3,21 @@ * dspack-gen CLI. * * dspack-gen context --dspack --intent [--depth N] [--no-steering] + * dspack-gen lint --dspack --surface * - * Prints the compiled generation context ({ system, schema, fewshot }) as JSON. - * Later PRs add `lint` (surface gates S1–S3) and `run` (the full pipeline). + * `context` prints the compiled generation context ({ system, schema, fewshot }). + * `lint` runs surface gates S1–S3: machine-readable JSON report on stdout, + * human rendering on stderr. Later PRs add `run` (the full pipeline). * * Exit codes (full table in README): 0 clean, 1 internal/usage error, - * 2 governance failure, 3 emitter-gate failure, 4 unknown rule type. + * 2 governance failure (any S-gate error), 3 emitter-gate failure, + * 4 unknown rule type. */ import { readFileSync } from "node:fs"; import { resolve } from "node:path"; import type { Contract } from "./core/contract.js"; import { compileContext } from "./core/compiler.js"; +import { lintSurface, renderText, UnknownRuleTypeError } from "./core/lint/index.js"; function fail(message: string): never { console.error(`error: ${message}`); @@ -41,11 +45,7 @@ function parseFlags(argv: string[]): Map { return flags; } -function main(): void { - const [command, ...rest] = process.argv.slice(2); - if (command !== "context") fail(`unknown command '${command ?? ""}' (available: context)`); - - const flags = parseFlags(rest); +function commandContext(flags: Map): void { const contractPath = flags.get("dspack") ?? fail("--dspack is required"); const intent = flags.get("intent") ?? fail("--intent is required"); @@ -62,4 +62,33 @@ function main(): void { process.stdout.write(JSON.stringify(context, null, 2) + "\n"); } +function commandLint(flags: Map): void { + const contractPath = flags.get("dspack") ?? fail("--dspack is required"); + const surfacePath = flags.get("surface") ?? fail("--surface is required"); + + const contract = JSON.parse(readFileSync(resolve(contractPath), "utf8")) as Contract; + const surface = JSON.parse(readFileSync(resolve(surfacePath), "utf8")) as unknown; + + try { + const report = lintSurface(surface, contract); + process.stdout.write(JSON.stringify(report, null, 2) + "\n"); + process.stderr.write(renderText(report) + "\n"); + process.exit(report.pass ? 0 : 2); + } catch (e) { + if (e instanceof UnknownRuleTypeError) { + console.error(`error: ${e.message}`); + process.exit(4); + } + throw e; + } +} + +function main(): void { + const [command, ...rest] = process.argv.slice(2); + const flags = parseFlags(rest); + if (command === "context") return commandContext(flags); + if (command === "lint") return commandLint(flags); + fail(`unknown command '${command ?? ""}' (available: context, lint)`); +} + main(); diff --git a/src/core/core-boundary.test.ts b/src/core/core-boundary.test.ts index d747b6f..a1e16b5 100644 --- a/src/core/core-boundary.test.ts +++ b/src/core/core-boundary.test.ts @@ -21,7 +21,8 @@ const FORBIDDEN_MODULES = [ "undici", ]; -const files = readdirSync(CORE_DIR) +const files = readdirSync(CORE_DIR, { recursive: true }) + .map(String) .filter((f) => f.endsWith(".ts") && !f.endsWith(".test.ts")) .map((f) => join(CORE_DIR, f)); @@ -38,10 +39,16 @@ describe("core boundary", () => { for (const specifier of imports) { expect(FORBIDDEN_MODULES, `${file} imports ${specifier}`).not.toContain(specifier); if (specifier.startsWith(".")) { - expect(specifier, `${file} escapes core via ${specifier}`).not.toMatch(/^\.\.\//); + // Relative imports may traverse within core (lint/ -> ../contract.js) + // but must never escape src/core itself. + const depth = file.split("/").length - 2; // segments below src/ + const ups = specifier.match(/\.\.\//g)?.length ?? 0; + expect(ups, `${file} escapes core via ${specifier}`).toBeLessThan(depth); } else { - // Bare imports must be dependency-free core tooling only (none today). - expect(specifier.startsWith("node:"), `${file} bare-imports ${specifier}`).toBe(true); + // Bare imports: node built-ins (non-network, checked above) and the + // pure-computation validator ajv only. + const allowed = specifier.startsWith("node:") || specifier === "ajv" || specifier.startsWith("ajv/") || specifier === "ajv-formats"; + expect(allowed, `${file} bare-imports ${specifier}`).toBe(true); } } expect(source, `${file} performs network I/O`).not.toMatch(/\bfetch\s*\(/); diff --git a/src/core/index.ts b/src/core/index.ts index 81a9144..c78e6de 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -9,3 +9,4 @@ export * from "./contract.js"; export * from "./generation-schema.js"; export * from "./compiler.js"; +export * from "./lint/index.js"; diff --git a/src/core/lint/findings.ts b/src/core/lint/findings.ts new file mode 100644 index 0000000..355bd47 --- /dev/null +++ b/src/core/lint/findings.ts @@ -0,0 +1,80 @@ +/** + * Lint findings and the gate report — ONE object, two serializations (ADR-7): + * the JSON here is embedded verbatim in audit reports and rendered + * deterministically into repair feedback; `renderText` is the human CLI view. + * + * Severity carries both faces (ADR-11): `requirement` is the contract's + * RFC 2119 term (must|should), `level` the tool mapping (error|warn). + */ +import type { RuleSeverity } from "../contract.js"; + +export type FindingLevel = "error" | "warn"; + +export const LEVEL_OF: Record = { + must: "error", + should: "warn", +}; + +export interface FindingLocation { + /** Path from the surface root, e.g. `$.root.children[0]`; `$.root` for surface-wide findings. */ + path: string; + /** The component id at the location, or "surface" for surface-wide findings. */ + component: string; + nodeId?: string; +} + +export interface Finding { + ruleId: string; + type: string; + requirement: RuleSeverity; + level: FindingLevel; + message: string; + rationale: string; + location: FindingLocation; + exampleIds: string[]; +} + +export type GateName = "S1" | "S2" | "S3"; +export type GateStatus = "PASS" | "FAIL" | "SKIPPED"; + +export interface GateReport { + gate: GateName; + name: string; + status: GateStatus; + /** S1/S2 error strings; S3 uses `findings` instead. */ + errors?: string[]; +} + +export interface LintReport { + gates: GateReport[]; + findings: Finding[]; + /** Errors only (warn findings never fail the lint in v0.3). */ + errorCount: number; + warnCount: number; + pass: boolean; +} + +export function renderText(report: LintReport): string { + const lines: string[] = []; + for (const gate of report.gates) { + lines.push(`gate ${gate.gate} ${gate.name.padEnd(21)} ${gate.status}`); + for (const error of gate.errors ?? []) lines.push(` ✖ ${error}`); + } + for (const finding of report.findings) { + const mark = finding.level === "error" ? "✖" : "▲"; + lines.push(`${mark} ${finding.level} [${finding.requirement}] ${finding.ruleId} [${finding.type}]`); + const where = finding.location.nodeId + ? `${finding.location.path} (component: ${finding.location.component}, id: "${finding.location.nodeId}")` + : `${finding.location.path} (component: ${finding.location.component})`; + lines.push(` at ${where}`); + lines.push(` ${finding.message}`); + lines.push(` Rationale: ${finding.rationale}`); + if (finding.exampleIds.length) lines.push(` See example: ${finding.exampleIds.join(", ")}`); + } + lines.push( + report.pass + ? `lint: PASS — 0 error(s), ${report.warnCount} warning(s)` + : `lint: FAIL — ${report.errorCount} error(s), ${report.warnCount} warning(s)`, + ); + return lines.join("\n"); +} diff --git a/src/core/lint/index.ts b/src/core/lint/index.ts new file mode 100644 index 0000000..5cd6252 --- /dev/null +++ b/src/core/lint/index.ts @@ -0,0 +1,80 @@ +/** + * The surface gates, run in order and independently reported (never implicit + * steps inside generation): + * S1 — generic surface schema (vendored dspack.surface.v0_1) + * S2 — contract vocabulary + * S3 — governance rules (typed evaluators, registry in rules.ts) + * + * S1 failure skips S2/S3 (the tree shape is not trustworthy); S2 failure + * still evaluates S3 (the walk is safe and more findings help repair). + * `pass` is false iff any gate FAILs; warn-level findings never fail (v0.3). + */ +import Ajv2020 from "ajv/dist/2020.js"; +import type { Contract, Surface } from "../contract.js"; +import { surfaceSchemaV0_1 } from "../surface-schema.js"; +import { type Finding, type GateReport, type LintReport } from "./findings.js"; +import { checkVocabulary } from "./vocabulary.js"; +import { evaluateRules } from "./rules.js"; + +export { renderText, LEVEL_OF } from "./findings.js"; +export type { Finding, FindingLevel, FindingLocation, GateName, GateReport, GateStatus, LintReport } from "./findings.js"; +export { UnknownRuleTypeError } from "./rules.js"; +export { walkSurface } from "./walk.js"; + +const ajv = new Ajv2020({ strict: false, allErrors: true }); +const validateSurfaceSchema = ajv.compile(surfaceSchemaV0_1 as unknown as Record); + +export function lintSurface(surface: unknown, contract: Contract): LintReport { + const gates: GateReport[] = []; + let findings: Finding[] = []; + + // S1 — generic surface schema. + const s1Ok = validateSurfaceSchema(surface) as boolean; + gates.push({ + gate: "S1", + name: "surface-schema", + status: s1Ok ? "PASS" : "FAIL", + errors: s1Ok + ? undefined + : (validateSurfaceSchema.errors ?? []).map((e) => `${e.instancePath || "(root)"} ${e.message ?? ""}`.trim()), + }); + + if (!s1Ok) { + gates.push({ gate: "S2", name: "contract-vocabulary", status: "SKIPPED" }); + gates.push({ gate: "S3", name: "governance", status: "SKIPPED" }); + return summarize(gates, findings); + } + + const typed = surface as Surface; + + // S2 — contract vocabulary (a check on the artifact, whatever produced it). + const s2Errors = checkVocabulary(typed, contract); + gates.push({ + gate: "S2", + name: "contract-vocabulary", + status: s2Errors.length === 0 ? "PASS" : "FAIL", + errors: s2Errors.length ? s2Errors : undefined, + }); + + // S3 — governance. Unknown rule types throw (CLI exit 4), never skip. + findings = evaluateRules(typed, contract); + gates.push({ + gate: "S3", + name: "governance", + status: findings.some((f) => f.level === "error") ? "FAIL" : "PASS", + }); + + return summarize(gates, findings); +} + +function summarize(gates: GateReport[], findings: Finding[]): LintReport { + const errorCount = findings.filter((f) => f.level === "error").length; + const warnCount = findings.filter((f) => f.level === "warn").length; + return { + gates, + findings, + errorCount, + warnCount, + pass: gates.every((g) => g.status !== "FAIL"), + }; +} diff --git a/src/core/lint/lint.test.ts b/src/core/lint/lint.test.ts new file mode 100644 index 0000000..2ad7aff --- /dev/null +++ b/src/core/lint/lint.test.ts @@ -0,0 +1,130 @@ +/** + * PR-4 acceptance gates: surface gates S1–S3 over the golden fixtures. + * + * Violating goldens F1–F5 each reproduce their checked-in *.expected.json + * exactly (regenerate deliberately with UPDATE_GOLDEN=1 npm test — the diff + * is the review artifact). The clean golden (the contract's worked example) + * passes all gates. S1/S2 are independently reported; unknown rule types + * throw (CLI exit 4). All three v0.3 evaluators are active — see rules.ts for + * why forbidden-composition could not defer to M2. + */ +import { readFileSync, readdirSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import type { Contract } from "../contract.js"; +import { lintSurface, UnknownRuleTypeError } from "./index.js"; + +const contract = JSON.parse(readFileSync("fixtures/shadcn.v0_3.dspack.json", "utf8")) as Contract; +const VIOLATING = "fixtures/golden/violating"; + +const load = (path: string) => JSON.parse(readFileSync(path, "utf8")); + +describe("violating goldens reproduce their expected reports exactly", () => { + const fixtures = readdirSync(VIOLATING).filter((f) => f.endsWith(".dsurface.json")); + + it("all five violating fixtures are present", () => { + expect(fixtures.length).toBe(5); + }); + + it.each(fixtures)("%s", (fixture) => { + const report = lintSurface(load(join(VIOLATING, fixture)), contract); + const expectedPath = join(VIOLATING, fixture.replace(".dsurface.json", ".expected.json")); + const rendered = JSON.stringify(report, null, 2) + "\n"; + if (process.env.UPDATE_GOLDEN) writeFileSync(expectedPath, rendered); + expect(rendered).toBe(readFileSync(expectedPath, "utf8")); + expect(report.pass).toBe(false); + expect(report.errorCount).toBeGreaterThan(0); + }); +}); + +describe("expected findings, spot-checked (goldens carry the full detail)", () => { + it("F1: dialog forbidden AND alert-dialog required — two findings of rule.destructive-requires-alertdialog", () => { + const report = lintSurface(load(join(VIOLATING, "F1-dialog-for-delete.dsurface.json")), contract); + const ids = report.findings.map((f) => f.ruleId); + expect(ids.filter((id) => id === "rule.destructive-requires-alertdialog").length).toBe(2); + expect(report.findings[0].location.component).toBe("dialog"); + expect(report.findings[0].level).toBe("error"); + expect(report.findings[0].requirement).toBe("must"); + expect(report.gates.map((g) => `${g.gate}:${g.status}`)).toEqual(["S1:PASS", "S2:PASS", "S3:FAIL"]); + }); + + it("F3: missing cancel — required-composition names the sub-component and count", () => { + const report = lintSurface(load(join(VIOLATING, "F3-missing-cancel.dsurface.json")), contract); + const cancel = report.findings.find((f) => f.ruleId === "rule.alertdialog-requires-cancel")!; + expect(cancel.message).toBe( + "Required sub-component 'alert-dialog-cancel' (min 1) not found among descendants (found 0).", + ); + expect(cancel.location.component).toBe("alert-dialog"); + }); + + it("F5: nested interactive — forbidden descendant located at the offending node", () => { + const report = lintSurface(load(join(VIOLATING, "F5-nested-interactive.dsurface.json")), contract); + const nested = report.findings.find((f) => f.ruleId === "rule.button-no-interactive-descendants")!; + expect(nested.message).toMatch(/Forbidden descendant 'input' found at \$\.root\.children\[1\]\.children\[0\]/); + expect(nested.location.nodeId).toBe("subscribe"); + }); +}); + +describe("clean golden and gate independence", () => { + it("the contract's worked example passes all gates", () => { + const report = lintSurface(load("fixtures/golden/clean/delete-account.dsurface.json"), contract); + expect(report.gates.map((g) => `${g.gate}:${g.status}`)).toEqual(["S1:PASS", "S2:PASS", "S3:PASS"]); + expect(report.pass).toBe(true); + expect(report.findings).toEqual([]); + }); + + it("S1 failure is reported independently and skips S2/S3", () => { + const report = lintSurface({ dspackSurface: "0.1", system: "shadcn/ui", intent: "destructive-action", root: { text: "no component" } }, contract); + expect(report.gates.map((g) => `${g.gate}:${g.status}`)).toEqual(["S1:FAIL", "S2:SKIPPED", "S3:SKIPPED"]); + expect(report.pass).toBe(false); + }); + + it("S2 failure (out-of-vocabulary) is reported independently; S3 still runs", () => { + const report = lintSurface( + { dspackSurface: "0.1", system: "shadcn/ui", intent: "destructive-action", root: { component: "carousel" } }, + contract, + ); + const s2 = report.gates.find((g) => g.gate === "S2")!; + expect(s2.status).toBe("FAIL"); + expect(s2.errors!.join("\n")).toMatch(/'carousel' is not a component/); + expect(report.gates.find((g) => g.gate === "S3")!.status).toBe("FAIL"); // alert-dialog still required + }); + + it("warn-level (should) findings never fail the lint in v0.3", () => { + const softContract: Contract = { + ...contract, + rules: [ + { + id: "rule.soft-preference", + type: "component-choice", + severity: "should", + appliesTo: { intents: ["destructive-action"] }, + require: ["badge"], + rationale: "Soft preference for testing the warn tier.", + }, + ], + }; + const report = lintSurface(load("fixtures/golden/clean/delete-account.dsurface.json"), softContract); + expect(report.warnCount).toBe(1); + expect(report.errorCount).toBe(0); + expect(report.pass).toBe(true); + expect(report.findings[0]).toMatchObject({ requirement: "should", level: "warn" }); + }); + + it("unknown rule types throw UnknownRuleTypeError (CLI exit 4), never skip", () => { + const futureContract: Contract = { + ...contract, + rules: [ + { + id: "rule.from-the-future", + type: "layout-order", + severity: "must", + rationale: "A v0.4 rule type this linter does not know.", + }, + ], + }; + expect(() => lintSurface(load("fixtures/golden/clean/delete-account.dsurface.json"), futureContract)).toThrowError( + UnknownRuleTypeError, + ); + }); +}); diff --git a/src/core/lint/rules.ts b/src/core/lint/rules.ts new file mode 100644 index 0000000..2932f67 --- /dev/null +++ b/src/core/lint/rules.ts @@ -0,0 +1,205 @@ +/** + * Gate S3 — the rule-type registry and evaluators (normative semantics: + * spec/dspack-v0.3.md §5.3). + * + * The registry is the thesis-bearing seam: new rule types land additively in + * v0.4 by adding an entry — never by touching existing evaluators. A rule + * whose type has no registry entry is a HARD error (UnknownRuleTypeError → + * CLI exit 4): silently skipping would misreport a surface as governed. + * + * All three v0.3 evaluators are implemented. (The M1 plan deferred + * `forbidden-composition` to M2, but the v0.3 shadcn contract carries a + * UNIVERSAL forbidden-composition rule and spec §5.4 forbids skipping — a + * two-evaluator linter would hard-error on every lint of the real contract. + * Deviation flagged for maintainer review in the PR; the evaluator is ~40 + * lines and fixture F5 activates with it.) + */ +import type { + ComponentChoiceRule, + Contract, + ForbiddenCompositionRule, + RequiredCompositionRule, + RuleEntry, + Surface, +} from "../contract.js"; +import { LEVEL_OF, type Finding } from "./findings.js"; +import { descendantsOf, walkSurface, type VisitedNode } from "./walk.js"; + +export class UnknownRuleTypeError extends Error { + constructor(readonly ruleId: string, readonly ruleType: string, detail: string) { + super(`rule '${ruleId}' has unknown or unimplemented type '${ruleType}': ${detail}`); + this.name = "UnknownRuleTypeError"; + } +} + +type Evaluator = (rule: RuleEntry, surface: Surface, contract: Contract) => Finding[]; + +/** The rule-type registry. Additive-only across spec versions. */ +const REGISTRY: Record = { + "component-choice": evaluateComponentChoice, + "required-composition": evaluateRequiredComposition, + "forbidden-composition": evaluateForbiddenComposition, +}; + +export function evaluateRules(surface: Surface, contract: Contract): Finding[] { + const findings: Finding[] = []; + for (const rule of contract.rules ?? []) { + if (rule.appliesTo && !rule.appliesTo.intents.includes(surface.intent)) continue; + const evaluator = REGISTRY[rule.type]; + if (evaluator === undefined) { + throw new UnknownRuleTypeError(rule.id, rule.type, "not a v0.3 rule type"); + } + findings.push(...evaluator(rule, surface, contract)); + } + return findings; +} + +function finding(rule: RuleEntry, message: string, location: Finding["location"]): Finding { + return { + ruleId: rule.id, + type: rule.type, + requirement: rule.severity, + level: LEVEL_OF[rule.severity], + message, + rationale: rule.rationale, + location, + exampleIds: rule.examples ?? [], + }; +} + +const SURFACE_LOCATION = { path: "$.root", component: "surface" } as const; + +function locationOf(visited: VisitedNode): Finding["location"] { + return { path: visited.path, component: visited.node.component, nodeId: visited.node.id }; +} + +/** + * component-choice: every id in `require` MUST appear ≥1 time (finding at the + * surface root per missing id); every id in `forbid` MUST appear 0 times + * (finding per matching node). + */ +function evaluateComponentChoice(entry: RuleEntry, surface: Surface): Finding[] { + const rule = entry as ComponentChoiceRule; + const findings: Finding[] = []; + const nodes = walkSurface(surface); + + for (const id of rule.forbid ?? []) { + for (const visited of nodes.filter((v) => v.node.component === id)) { + findings.push( + finding(rule, `Component '${id}' is forbidden for intent '${surface.intent}'.`, locationOf(visited)), + ); + } + } + for (const id of rule.require ?? []) { + if (!nodes.some((v) => v.node.component === id)) { + findings.push(finding(rule, `Required component '${id}' does not appear in the surface.`, SURFACE_LOCATION)); + } + } + return findings; +} + +/** + * required-composition: for EVERY node matching `component`, each + * requiredSubComponents entry MUST have ≥ min matching descendants, and each + * requiredProps entry MUST hold (on the node itself, or on every descendant + * matching `on` — of which at least one must exist). + */ +function evaluateRequiredComposition(entry: RuleEntry, surface: Surface): Finding[] { + const rule = entry as RequiredCompositionRule; + const findings: Finding[] = []; + + for (const visited of walkSurface(surface).filter((v) => v.node.component === rule.component)) { + const descendants = descendantsOf(visited); + + for (const requirement of rule.requiredSubComponents ?? []) { + const min = requirement.min ?? 1; + const found = descendants.filter((d) => d.node.component === requirement.id).length; + if (found < min) { + findings.push( + finding( + rule, + `Required sub-component '${requirement.id}' (min ${min}) not found among descendants (found ${found}).`, + locationOf(visited), + ), + ); + } + } + + for (const requirement of rule.requiredProps ?? []) { + const holds = (candidate: VisitedNode): boolean => + requirement.oneOf.includes(candidate.node.props?.[requirement.prop] as never); + if (requirement.on === undefined) { + if (!holds(visited)) { + findings.push( + finding( + rule, + `Required prop '${requirement.prop}' must be one of ${requirement.oneOf.map((v) => JSON.stringify(v)).join(", ")}.`, + locationOf(visited), + ), + ); + } + } else { + const targets = descendants.filter((d) => d.node.component === requirement.on); + if (targets.length === 0) { + findings.push( + finding( + rule, + `No descendant '${requirement.on}' found to satisfy required prop '${requirement.prop}'.`, + locationOf(visited), + ), + ); + } + for (const target of targets.filter((t) => !holds(t))) { + findings.push( + finding( + rule, + `Required prop '${requirement.prop}' on '${requirement.on}' must be one of ${requirement.oneOf.map((v) => JSON.stringify(v)).join(", ")}.`, + locationOf(target), + ), + ); + } + } + } + } + return findings; +} + +/** + * forbidden-composition: for EVERY node matching `component`, no descendant + * may match any forbiddenDescendants id (finding at the offending descendant), + * and no forbiddenProps entry may hold (on the node itself, or on descendants + * matching `on`). + */ +function evaluateForbiddenComposition(entry: RuleEntry, surface: Surface): Finding[] { + const rule = entry as ForbiddenCompositionRule; + const findings: Finding[] = []; + + for (const visited of walkSurface(surface).filter((v) => v.node.component === rule.component)) { + const descendants = descendantsOf(visited); + + for (const id of rule.forbiddenDescendants ?? []) { + for (const offender of descendants.filter((d) => d.node.component === id)) { + findings.push( + finding(rule, `Forbidden descendant '${id}' found at ${offender.path}.`, locationOf(visited)), + ); + } + } + + for (const constraint of rule.forbiddenProps ?? []) { + const violates = (candidate: VisitedNode): boolean => + constraint.values.includes(candidate.node.props?.[constraint.prop] as never); + const targets = + constraint.on === undefined ? [visited] : descendants.filter((d) => d.node.component === constraint.on); + for (const target of targets.filter(violates)) { + findings.push( + finding( + rule, + `Forbidden value ${JSON.stringify(target.node.props?.[constraint.prop])} for prop '${constraint.prop}'${constraint.on ? ` on '${constraint.on}'` : ""}.`, + locationOf(target), + ), + ); + } + } + } + return findings; +} diff --git a/src/core/lint/vocabulary.ts b/src/core/lint/vocabulary.ts new file mode 100644 index 0000000..5133fa6 --- /dev/null +++ b/src/core/lint/vocabulary.ts @@ -0,0 +1,70 @@ +/** + * Gate S2 — contract vocabulary. A check on ANY produced surface (model + * output, hand-authored, fixture), regardless of how generation was + * constrained: the S0 spike caught Ollama's mlx engine silently ignoring + * `format`, which is why S2 is never assumed from generation. + * + * Scope per spec §8: component/sub-component ids, prop names on components, + * enum prop values, declared slot names, plus surface-level consistency + * (registered intent, matching system name). Deliberately NOT checked: + * acceptsChildren semantics, non-enum prop types, ordering. + */ +import { type Contract, type Surface, enumValues, subComponentIndex } from "../contract.js"; +import { walkSurface } from "./walk.js"; + +export function checkVocabulary(surface: Surface, contract: Contract): string[] { + const errors: string[] = []; + const components = contract.components ?? {}; + const subIndex = subComponentIndex(contract); + + if (surface.system !== contract.name) { + errors.push(`$.system: surface.system '${surface.system}' does not match contract name '${contract.name}'`); + } + if (!(contract.intents ?? []).some((i) => i.id === surface.intent)) { + errors.push(`$.intent: intent '${surface.intent}' is not registered in the contract`); + } + + for (const { node, path } of walkSurface(surface)) { + const isComponent = node.component in components; + const isSub = subIndex.has(node.component); + if (!isComponent && !isSub) { + errors.push(`${path}: component '${node.component}' is not a component or sub-component of the contract`); + continue; + } + if (node.props && Object.keys(node.props).length > 0) { + if (isSub) { + errors.push(`${path}: sub-component '${node.component}' does not declare props in this contract`); + } else { + const declared = components[node.component].props ?? {}; + for (const [name, value] of Object.entries(node.props)) { + const descriptor = declared[name]; + if (!descriptor) { + errors.push(`${path}: prop '${name}' is not declared on component '${node.component}'`); + continue; + } + const allowed = enumValues(descriptor); + if (allowed && !allowed.includes(value)) { + errors.push( + `${path}: prop '${name}' on '${node.component}' has value ${JSON.stringify(value)}; allowed: ${allowed + .map((v) => JSON.stringify(v)) + .join(", ")}`, + ); + } + } + } + } + if (node.slots && isComponent) { + const declaredSlots = new Set( + (components[node.component].composition?.subComponents ?? []) + .map((s) => s.slot) + .filter((s): s is string => Boolean(s)), + ); + for (const slot of Object.keys(node.slots)) { + if (!declaredSlots.has(slot)) { + errors.push(`${path}: slot '${slot}' is not declared on component '${node.component}'`); + } + } + } + } + return errors; +} diff --git a/src/core/lint/walk.ts b/src/core/lint/walk.ts new file mode 100644 index 0000000..3215470 --- /dev/null +++ b/src/core/lint/walk.ts @@ -0,0 +1,44 @@ +/** + * Surface-tree traversal shared by S2 and S3. Paths use the `$.root…` JSONPath + * style that findings, the emitter, and the spec's worked failures all share. + * "Descendants" (spec §5.3) = everything reachable through `children` and + * `slots`, at any depth. + */ +import type { Surface, SurfaceNode } from "../contract.js"; + +export interface VisitedNode { + node: SurfaceNode; + path: string; +} + +export function walkSurface(surface: Surface): VisitedNode[] { + const visited: VisitedNode[] = []; + const visit = (node: SurfaceNode, path: string): void => { + visited.push({ node, path }); + (node.children ?? []).forEach((child, i) => visit(child, `${path}.children[${i}]`)); + for (const slot of Object.keys(node.slots ?? {}).sort()) { + node.slots![slot].forEach((child, i) => visit(child, `${path}.slots.${slot}[${i}]`)); + } + }; + visit(surface.root, "$.root"); + return visited; +} + +/** Descendants of `origin` (excluding itself), with absolute paths. */ +export function descendantsOf(origin: VisitedNode): VisitedNode[] { + const visited: VisitedNode[] = []; + const visit = (node: SurfaceNode, path: string): void => { + (node.children ?? []).forEach((child, i) => { + visited.push({ node: child, path: `${path}.children[${i}]` }); + visit(child, `${path}.children[${i}]`); + }); + for (const slot of Object.keys(node.slots ?? {}).sort()) { + node.slots![slot].forEach((child, i) => { + visited.push({ node: child, path: `${path}.slots.${slot}[${i}]` }); + visit(child, `${path}.slots.${slot}[${i}]`); + }); + } + }; + visit(origin.node, origin.path); + return visited; +} diff --git a/src/core/surface-schema.ts b/src/core/surface-schema.ts new file mode 100644 index 0000000..f3e6c3c --- /dev/null +++ b/src/core/surface-schema.ts @@ -0,0 +1,86 @@ +/** + * Vendored copy of dspack.surface.v0_1.schema.json (dspack repo, schema/). + * Gate S1's ground truth. Re-vendor when the surface schema version bumps; + * the const below is asserted against surface documents at lint time. + */ +export const SURFACE_SCHEMA_VERSION = "0.1"; + +export const surfaceSchemaV0_1 = { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/aestheticfunction/dspack/blob/main/schema/dspack.surface.v0_1.schema.json", + "title": "dspack surface v0.1", + "description": "Schema for a dspack surface document — a protocol-neutral, nested component tree expressed in a dspack contract's vocabulary (component IDs, sub-component IDs, props, slots, text leaves) plus a declared generation intent. A surface is the pipeline's intermediate representation: never rendered, never transported, always compiled to a protocol by an emitter. This generic schema is the portable validation floor (gate S1); checking the tree against a specific contract's vocabulary is a separate gate (S2), and governance rules are a third (S3). The schema answers whether the object can exist; the linter answers whether it is correct; a renderer answers whether its compiled form can render. These layers must never collapse.", + "type": "object", + "required": [ + "dspackSurface", + "system", + "intent", + "root" + ], + "properties": { + "dspackSurface": { + "type": "string", + "const": "0.1", + "description": "Surface format version. MUST be \"0.1\" for documents conforming to this version." + }, + "system": { + "type": "string", + "minLength": 1, + "description": "Name of the design system contract this surface is expressed against (the contract's top-level name)." + }, + "intent": { + "type": "string", + "description": "The declared generation intent (an intent ID registered in the bound contract). Declared by the caller, carried in the artifact, and used by the linter to activate intent-scoped rules." + }, + "root": { + "$ref": "#/$defs/node", + "description": "The root component node of the surface tree." + } + }, + "additionalProperties": false, + "$defs": { + "node": { + "type": "object", + "description": "A component instance in the surface tree. 'component' is a component ID or sub-component ID from the bound contract's vocabulary.", + "required": [ + "component" + ], + "properties": { + "component": { + "type": "string", + "description": "Component ID or sub-component ID from the bound contract." + }, + "id": { + "type": "string", + "description": "Optional stable identifier for this node, used in lint finding locations and emitted artifacts." + }, + "props": { + "type": "object", + "description": "Prop values for this node. Prop names and enum values are validated against the bound contract (gate S2), not by this schema." + }, + "text": { + "type": "string", + "description": "Text content for leaf nodes (components or sub-components that accept text children)." + }, + "children": { + "type": "array", + "description": "Ordered child nodes.", + "items": { + "$ref": "#/$defs/node" + } + }, + "slots": { + "type": "object", + "description": "Named slot contents, keyed by slot name declared on the parent's sub-components in the bound contract.", + "additionalProperties": { + "type": "array", + "items": { + "$ref": "#/$defs/node" + } + } + } + }, + "additionalProperties": false + } + } +} as const; From 533e4dea1c724933b8bbf6260234db4db3cb1553 Mon Sep 17 00:00:00 2001 From: Ryan Dombrowski Date: Thu, 2 Jul 2026 17:01:00 -0400 Subject: [PATCH 2/2] fix: reject ambiguous sub-component ids in gate S2 (mirror of dspack#9 review fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Semantic alignment with the now-normative spec v0.3 §5: S2 and rule resolution work by sub-component id alone, so duplicate ids across components fail loudly — naming the id and every declaring component — before any id-dependent check, instead of resolving by object iteration order. Same error shape as the dspack validate harness; covered by a new lint test. No golden outputs change (the shadcn contract has no duplicates). Co-Authored-By: Claude Fable 5 --- src/core/contract.ts | 16 ++++++++++++++++ src/core/lint/lint.test.ts | 26 ++++++++++++++++++++++++++ src/core/lint/vocabulary.ts | 12 +++++++++++- 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/core/contract.ts b/src/core/contract.ts index 70ce1ee..db43431 100644 --- a/src/core/contract.ts +++ b/src/core/contract.ts @@ -135,6 +135,22 @@ export function subComponentIndex(contract: Contract): Map { return index; } +/** + * Sub-component ids declared by more than one component, with all declaring + * parents. Spec v0.3 §5 makes document-wide uniqueness normative for + * contracts using governance blocks: S2 and rule resolution work by id alone + * and must never depend on object iteration order. + */ +export function duplicateSubComponentIds(contract: Contract): Map { + const parents = new Map(); + for (const [id, component] of Object.entries(contract.components ?? {})) { + for (const sub of component.composition?.subComponents ?? []) { + parents.set(sub.id, [...(parents.get(sub.id) ?? []), id]); + } + } + return new Map([...parents].filter(([, declaredBy]) => declaredBy.length > 1)); +} + export function getIntent(contract: Contract, intentId: string): IntentEntry { const intent = (contract.intents ?? []).find((i) => i.id === intentId); if (!intent) { diff --git a/src/core/lint/lint.test.ts b/src/core/lint/lint.test.ts index 2ad7aff..a45c1f9 100644 --- a/src/core/lint/lint.test.ts +++ b/src/core/lint/lint.test.ts @@ -111,6 +111,32 @@ describe("clean golden and gate independence", () => { expect(report.findings[0]).toMatchObject({ requirement: "should", level: "warn" }); }); + it("duplicate sub-component ids fail S2 loudly, naming the id and every declaring component", () => { + const ambiguous: Contract = { + ...contract, + components: { + ...contract.components, + "widget-a": { + name: "WidgetA", + description: "First parent.", + composition: { subComponents: [{ id: "shared-part", name: "SharedPart" }] }, + }, + "widget-b": { + name: "WidgetB", + description: "Second parent.", + composition: { subComponents: [{ id: "shared-part", name: "SharedPart" }] }, + }, + }, + }; + const report = lintSurface(load("fixtures/golden/clean/delete-account.dsurface.json"), ambiguous); + const s2 = report.gates.find((g) => g.gate === "S2")!; + expect(s2.status).toBe("FAIL"); + expect(s2.errors!.join("\n")).toMatch( + /sub-component id 'shared-part' is declared by multiple components \(widget-a, widget-b\)/, + ); + expect(report.pass).toBe(false); + }); + it("unknown rule types throw UnknownRuleTypeError (CLI exit 4), never skip", () => { const futureContract: Contract = { ...contract, diff --git a/src/core/lint/vocabulary.ts b/src/core/lint/vocabulary.ts index 5133fa6..d22fc0c 100644 --- a/src/core/lint/vocabulary.ts +++ b/src/core/lint/vocabulary.ts @@ -9,7 +9,7 @@ * (registered intent, matching system name). Deliberately NOT checked: * acceptsChildren semantics, non-enum prop types, ordering. */ -import { type Contract, type Surface, enumValues, subComponentIndex } from "../contract.js"; +import { type Contract, type Surface, duplicateSubComponentIds, enumValues, subComponentIndex } from "../contract.js"; import { walkSurface } from "./walk.js"; export function checkVocabulary(surface: Surface, contract: Contract): string[] { @@ -17,6 +17,16 @@ export function checkVocabulary(surface: Surface, contract: Contract): string[] const components = contract.components ?? {}; const subIndex = subComponentIndex(contract); + // Ambiguous vocabulary fails loudly before any id-dependent check — S2 and + // rule resolution must never depend on object iteration order (mirrors the + // dspack validate harness; spec v0.3 §5). + for (const [id, declaredBy] of duplicateSubComponentIds(contract)) { + errors.push( + `contract: sub-component id '${id}' is declared by multiple components (${declaredBy.join(", ")}); ` + + `sub-component ids must be unique document-wide for deterministic S2 and rule resolution`, + ); + } + if (surface.system !== contract.name) { errors.push(`$.system: surface.system '${surface.system}' does not match contract name '${contract.name}'`); }