From ff08dfdf00c3c072ce95708a66437a8821698ffd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 09:25:30 +0000 Subject: [PATCH] fix(spec): the liveness gate stops trusting the direction it never checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate walked the schema and looked each property up in the ledger. That direction was well defended — a property with no row is UNCLASSIFIED and fails CI. The reverse had no check at all, and the two removal routes make that gap load-bearing: - a `retiredKey()` tombstone leaves `z.never()` in the walked shape, so the key is still a property and its row must STAY (deleting it fails as UNCLASSIFIED — 14 at once in the #3896 close-out sweep); - a strict removal takes the key OUT of the shape, so its row must GO. Nothing asked. The forward pass stops enquiring about a key it can no longer see, and a stale `dead`/`live` claim about a nonexistent property rots in place — still read by anyone treating the ledger as the capability catalogue it doubles as. Not hypothetical: the report `aria`/`performance` rows outlived their keys by a full release and were deleted by hand in the #3896 sweep, noticed only because someone happened to be reading the file. One direction failed loudly; the other never failed at all — the same silent-no-op shape this whole ledger exists to catch, one layer up. `scripts/liveness/orphans.mts` closes it, as a FAILURE rather than a warning. The tree is orphan-free across all sixteen governed types, so there is no debt to amortise and nothing to soften: a warning here would have re-created the original defect (the README's own verdict is that a permanently-noisy check is a check nobody reads, the same way a stale row is a claim nobody re-tests). The error text names BOTH causes, because which one applies is the author's judgement: delete the row, or — if the property really is still authorable — fix the WALK, since a property the walk cannot see is one the ratchet cannot govern. Deliberately silent in one case: a row declaring `children` on a non-container. The forward pass already reports that as UNCLASSIFIED with a more precise message, and two headings for one fix reads as two problems. Confirmed against `tool.parameters` (a `z.record`, undrillable) — the forward pass claimed it and the orphan scan stayed quiet. Because the tree is clean, a green gate proves nothing about whether the scan can fire, so the proof lives in `orphans.test.ts` (9 cases: both orphan levels, the tombstone that must NOT flag, the deferred non-container, entry fields that are not child props, absent/empty props). Both legs were also driven end-to-end by injecting a real orphan into a real ledger: `tool/category` (top level) and `view/list.ghostListKey` (drilled child) each failed the gate with exit 1 and the full prescription; the ledgers were restored and verified byte-identical. The skill and the ledger README carried the now-false claim that this direction never fails; both corrected, with the orphan leg documented as the fix. Gate-only change under `packages/spec/scripts/`, which is not published — empty changeset. Verification: all ten spec gates pass (`check:api-surface` needed a rebuild first — the documented stale-dist false alarm, on a dist predating 2af1988); liveness script suite 98/98; spec `tsc` clean. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QuViRSR1j6GJjf9qGbnqFX --- .changeset/liveness-orphan-row-gate.md | 4 + .../skills/spec-property-retirement/SKILL.md | 21 ++- packages/spec/liveness/README.md | 15 +++ .../spec/scripts/liveness/check-liveness.mts | 43 +++++- packages/spec/scripts/liveness/orphans.mts | 111 ++++++++++++++++ .../spec/scripts/liveness/orphans.test.ts | 122 ++++++++++++++++++ 6 files changed, 306 insertions(+), 10 deletions(-) create mode 100644 .changeset/liveness-orphan-row-gate.md create mode 100644 packages/spec/scripts/liveness/orphans.mts create mode 100644 packages/spec/scripts/liveness/orphans.test.ts diff --git a/.changeset/liveness-orphan-row-gate.md b/.changeset/liveness-orphan-row-gate.md new file mode 100644 index 0000000000..3f21e177ac --- /dev/null +++ b/.changeset/liveness-orphan-row-gate.md @@ -0,0 +1,4 @@ +--- +--- + +fix(spec): the liveness gate checks the reverse direction — a ledger row that outlived its schema property now fails CI instead of rotting silently. Gate-only (`packages/spec/scripts/`, not published), so this releases nothing. diff --git a/.claude/skills/spec-property-retirement/SKILL.md b/.claude/skills/spec-property-retirement/SKILL.md index 0cb9ee29ce..8c79f9359f 100644 --- a/.claude/skills/spec-property-retirement/SKILL.md +++ b/.claude/skills/spec-property-retirement/SKILL.md @@ -107,13 +107,20 @@ that shape. Therefore: | `retiredKey()` tombstone | **YES** (`z.never()` is a property) | **STAYS** — `status: "dead"`, a `verifiedAt`, and a `note` saying REMOVED + why the entry remains | | strict removal | no | **DELETED**, along with any CLI advisory-lint expectation | -Get this backwards and: deleting a tombstoned key's entry reports it -**UNCLASSIFIED and fails CI** (14 at once, in the #3896 sweep — the mistake this -section exists to prevent). The other direction is worse in the long run: the -gate never walks entries looking for absent schema keys, so a **leftover entry -after a strict removal rots invisibly** — report `aria`/`performance` sat stale -for a release before a human deleted them as hygiene. One direction fails -loudly; the other never fails at all. +Both directions now fail CI, so getting it backwards is loud either way: + +- deleting a **tombstoned** key's row reports it **UNCLASSIFIED** (14 at once in + the #3896 sweep — the mistake this section exists to prevent); +- leaving a **strict-removed** key's row reports it as an **ORPHAN** row. + +The orphan leg is new (`scripts/liveness/orphans.mts`). Until it landed, this +direction never failed at all — the gate walked the schema and looked rows up, so +a row whose key had left the shape was simply never asked about, and rotted in +place. That is how the report `aria`/`performance` rows outlived their keys by a +full release, deleted by hand only because someone happened to read the file. If +you hit the orphan error and the property really is still authorable, the fix is +the **walk**, not the row: a property the walk cannot see is a property the +ratchet cannot govern. Note template for a tombstone entry (verbatim house style, e.g. `liveness/action.json`): diff --git a/packages/spec/liveness/README.md b/packages/spec/liveness/README.md index 55eb900cc5..be7ba0d04c 100644 --- a/packages/spec/liveness/README.md +++ b/packages/spec/liveness/README.md @@ -11,6 +11,18 @@ This ledger makes that classification **explicit and regression-proof**: every p of a governed metadata type must declare a liveness status with evidence, or CI fails (the ratchet — you can't add new undeclared surface). +The gate checks **both directions**, and it took a while to notice the second one was +missing. Schema → ledger catches a property with no row (`UNCLASSIFIED`). Ledger → +schema catches a **row that outlived its property** (`ORPHAN`): when a key is removed +from a `.strict()` schema it leaves the walked shape, so the forward pass simply stops +asking about it and a stale `dead`/`live` claim rots in place — which is exactly what +the report `aria`/`performance` rows did for a full release, deleted by hand only +because someone happened to read the file. Note the asymmetry that makes this +error-prone: a `retiredKey()` tombstone **keeps** the key in the walked shape, so a +tombstoned key's row must **stay**, while a strict-removed key's row must **go**. The +route decides the disposition — see `scripts/liveness/orphans.mts` and +`.claude/skills/spec-property-retirement/SKILL.md` §2. + ## Source of truth = the metadata-type registry The gate reads `BUILTIN_METADATA_TYPE_SCHEMAS` (`packages/spec/src/kernel/metadata-type-schemas.ts`) @@ -415,6 +427,9 @@ over-share. - `.json` — the ledger for a governed metadata type. - `../scripts/liveness/check-liveness.mts` — the gate (tsx; imports the registry). +- `../scripts/liveness/orphans.mts` — the reverse (ledger → schema) scan: rows whose + property is gone. Pure + unit-tested, because the tree was orphan-free when it + landed, so a green gate proves nothing about whether the scan can fire. - `../scripts/liveness/check-empty-state.mts` — the empty-state gate (above); `empty-state-registry.mts` is its source of truth. diff --git a/packages/spec/scripts/liveness/check-liveness.mts b/packages/spec/scripts/liveness/check-liveness.mts index 12c8b633ce..43cf6e8325 100644 --- a/packages/spec/scripts/liveness/check-liveness.mts +++ b/packages/spec/scripts/liveness/check-liveness.mts @@ -19,6 +19,11 @@ // (object / record / array-of-object) may be drilled into via `"children"` so e.g. // `permission.objects.allowCreate` stays distinguishable from a blanket `objects`. // +// BOTH DIRECTIONS. Schema → ledger catches an undeclared property (UNCLASSIFIED). +// Ledger → schema catches the reverse: a row that outlived its property, which +// went unchecked until #4080 mapped the asymmetry (a strict removal takes the key +// out of the walked shape, so the forward pass just stops asking). See orphans.mts. +// // Statuses: live | experimental | planned | dead. Resolution per property: // ledger entry → spec `.describe()` marker ([EXPERIMENTAL — not enforced]) → UNCLASSIFIED // @@ -64,6 +69,7 @@ import { type VerificationReport, } from './verification.mts'; import { checkEvidence } from './evidence.mts'; +import { ORPHAN_GUIDANCE, findOrphanEntries, type Orphan } from './orphans.mts'; const here = dirname(fileURLToPath(import.meta.url)); const specRoot = resolve(here, '../..'); // packages/spec @@ -202,6 +208,7 @@ const report: any = { proofErrors: [] as string[], // a `proof` ref that doesn't resolve (missing file / missing tag / malformed) proofMissing: [] as string[], // a bound high-risk `live` entry with no proof at all orphanProofs: [] as string[], // a dogfood `@proof:` tag not registered in proof-registry.mts + orphanEntries: [] as string[], // a ledger row whose property is gone from the schema (the reverse direction) verification: null as VerificationReport | null, // `verifiedAt` ages — the re-verification worklist evidenceLocal: 0, // repo-rooted evidence paths actually resolved against this checkout evidenceForeign: 0, // evidence paths attributed to objectui / cloud — not resolvable here @@ -279,7 +286,24 @@ for (const type of GOVERNED) { const ledger = loadLedger(type); const props = ledger.props || {}; const cat = { classified: 0, unclassified: 0, byStatus: {} as Record }; - for (const { key, node, description } of topProps(type)) { + const walked = topProps(type); + + // ── reverse direction: a row whose property is gone (see orphans.mts) ── + // Runs off the SAME walk the forward pass classifies against, so the two + // directions can never disagree about what the schema contains. + const nodeOf = new Map(walked.map((p) => [p.key, p.node])); + const orphans: Orphan[] = findOrphanEntries({ + type, + props, + shapeKeys: walked.map((p) => p.key), + childKeysOf: (key) => { + const cs = childShape(nodeOf.get(key)); + return cs ? Object.keys(cs) : null; + }, + }); + for (const o of orphans) report.orphanEntries.push(o.key); + + for (const { key, node, description } of walked) { if (FRAMEWORK_FIELDS.has(key)) { classify(type, key, 'live', null, cat); continue; } const led = props[key]; if (led?.children) { @@ -314,7 +338,11 @@ report.verification = buildVerificationReport(verificationEntries, { staleDays } const totalUnclassified = report.unclassified.length; const totalProofFailures = report.proofErrors.length + report.proofMissing.length; -const failed = totalUnclassified > 0 || totalProofFailures > 0 || report.verification.errors.length > 0; +const failed = + totalUnclassified > 0 || + totalProofFailures > 0 || + report.orphanEntries.length > 0 || + report.verification.errors.length > 0; if (asJson) { process.stdout.write(JSON.stringify(report, null, 2) + '\n'); } else { @@ -349,6 +377,12 @@ if (asJson) { console.log(`\n✗ ${totalUnclassified} UNCLASSIFIED — classify in packages/spec/liveness/.json:`); report.unclassified.forEach((s: string) => console.log(` ${s}`)); } + if (report.orphanEntries.length) { + console.log(`\n✗ ${report.orphanEntries.length} ORPHAN ledger row(s) — the property is gone from the schema:`); + report.orphanEntries.forEach((s: string) => console.log(` ${s}`)); + console.log(''); + ORPHAN_GUIDANCE.forEach((line) => console.log(line ? ` ${line}` : '')); + } // ── re-verification clock ── const v = report.verification!; if (v.errors.length) { @@ -373,7 +407,10 @@ if (asJson) { console.log(' run with --stale-verification[=days] for the worklist.'); } if (!failed) { - console.log('\n✓ all governed-type properties are classified; all bound high-risk proofs resolve.'); + console.log( + '\n✓ all governed-type properties are classified, no ledger row outlives its property, ' + + 'and all bound high-risk proofs resolve.', + ); } } process.exit(failed ? 1 : 0); diff --git a/packages/spec/scripts/liveness/orphans.mts b/packages/spec/scripts/liveness/orphans.mts new file mode 100644 index 0000000000..cd871f58e4 --- /dev/null +++ b/packages/spec/scripts/liveness/orphans.mts @@ -0,0 +1,111 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Orphan ledger entries — the silent half of the liveness gate's asymmetry. +// +// WHY THIS EXISTS. The gate walks the SCHEMA and looks each property up in the +// ledger. That direction is well defended: a property with no row reports +// UNCLASSIFIED and fails CI (the ratchet — no new undeclared surface). The +// reverse direction had no check at all, and the two removal routes make that +// gap load-bearing: +// +// - `retiredKey()` tombstone — `z.never()` is still a property, so the key +// STAYS in the walked shape and its row must stay with it. Deleting the row +// reports UNCLASSIFIED and fails loudly (14 at once in the #3896 close-out +// sweep, which is how this asymmetry got mapped in the first place). +// - strict removal — the key leaves the shape entirely, so its row must go. +// Nothing asked. The forward pass simply stops enquiring about a key it can +// no longer see, and the row rots in place: a `dead`/`live` claim about a +// property that does not exist, still read by anyone treating the ledger as +// the capability catalogue it doubles as. +// +// That is not hypothetical. The report `aria`/`performance` rows outlived their +// schema keys by a full release — the keys had left in the report-liveness +// close-out — and were deleted by hand in the #3896 sweep as hygiene, noticed +// only because a human happened to be reading the file. One direction fails +// loudly; the other never failed at all. +// +// So this closes it, and closes it as a FAILURE rather than a warning. The tree +// was clean when the check landed (zero orphans across all sixteen governed +// types), so there is no debt to amortise and nothing to soften: it is a pure +// ratchet on the remaining direction. A warning here would have re-created the +// original defect one layer up — the ledger README's own verdict is that a +// permanently-noisy check is a check nobody reads, the same way a stale row is a +// claim nobody re-tests. + +/** One orphaned ledger coordinate, with the cause narrowed as far as data allows. */ +export interface Orphan { + /** `/` — the ledger coordinate that has no schema property. */ + key: string; + /** Whether the row sits at the top level or under a parent's `children`. */ + level: 'top' | 'child'; +} + +export interface OrphanScanInput { + /** The governed metadata type being scanned. */ + type: string; + /** The ledger's `props` object (may be empty / absent). */ + props: Record | undefined; + /** Top-level keys the gate's schema walk produced for this type. */ + shapeKeys: readonly string[]; + /** + * Child keys of a container property, or `null` when the property is not a + * container. Injected so every Zod-walking detail stays in the gate and this + * module stays pure and testable. + */ + childKeysOf: (key: string) => readonly string[] | null; +} + +/** + * Find ledger rows with no corresponding schema property. + * + * Deliberately silent in one case: a row that declares `children` on a property + * that is not a container. The FORWARD pass already reports that as + * UNCLASSIFIED with a more specific message, and reporting it twice under two + * different headings would obscure the single fix. + */ +export function findOrphanEntries({ type, props, shapeKeys, childKeysOf }: OrphanScanInput): Orphan[] { + const orphans: Orphan[] = []; + const shape = new Set(shapeKeys); + + for (const key of Object.keys(props ?? {})) { + if (!shape.has(key)) { + orphans.push({ key: `${type}/${key}`, level: 'top' }); + continue; + } + const declaredChildren = props![key]?.children; + if (!declaredChildren) continue; + + const childKeys = childKeysOf(key); + if (!childKeys) continue; // the forward pass owns this one — see the docblock + const childShape = new Set(childKeys); + for (const childKey of Object.keys(declaredChildren)) { + if (!childShape.has(childKey)) orphans.push({ key: `${type}/${key}.${childKey}`, level: 'child' }); + } + } + + return orphans; +} + +/** + * The prescription printed under the orphan list. Both causes are actionable, + * and which one applies is a judgement the author has to make — so name both + * rather than guessing, and state the asymmetry that makes the wrong guess + * tempting. + */ +export const ORPHAN_GUIDANCE = [ + 'A ledger row outlives its property when a key is removed from the schema and', + 'its row is left behind. That is the STRICT-REMOVAL route: the key leaves the', + 'walked shape, so the forward pass stops asking about it and nothing else ever', + 'does. Delete the row (and any CLI advisory-lint expectation keyed on it).', + '', + 'Mind the asymmetry before you reach for the opposite fix: a `retiredKey()`', + 'tombstone KEEPS the key in the walked shape, so a tombstoned key\'s row must', + 'STAY — deleting it reports UNCLASSIFIED instead. Route decides disposition.', + '', + 'If the property IS still authorable, then the ledger is right and the WALK is', + 'wrong — the gate cannot see it (e.g. it lives on a union member `shapeOf`', + 'skips). Fix the walk, not the row: a property the walk cannot see is also a', + 'property the ratchet cannot govern.', + '', + 'See .claude/skills/spec-property-retirement/SKILL.md §2.', +]; diff --git a/packages/spec/scripts/liveness/orphans.test.ts b/packages/spec/scripts/liveness/orphans.test.ts new file mode 100644 index 0000000000..c74a423928 --- /dev/null +++ b/packages/spec/scripts/liveness/orphans.test.ts @@ -0,0 +1,122 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Unit tests for the orphan-ledger-entry scan (the reverse direction of the +// liveness gate — see orphans.mts for why it exists). +// +// These matter more than a typical unit test: the tree was ORPHAN-FREE when the +// check landed, so a green `check:liveness` proves nothing about whether the +// scan can fire at all. That proof has to come from here. Real-ledger coverage +// is the gate's own CI job (`Spec property liveness`), which walks the live Zod +// schemas — this file owns the logic, including every case it must stay quiet on. + +import { describe, it, expect } from 'vitest'; +import { findOrphanEntries, ORPHAN_GUIDANCE } from './orphans.mts'; + +/** No property is a container unless a test says so. */ +const noChildren = () => null; + +describe('findOrphanEntries — rows it must catch', () => { + it('catches a top-level row whose key left the schema (the strict-removal shape)', () => { + const orphans = findOrphanEntries({ + type: 'tool', + props: { name: { status: 'live' }, category: { status: 'dead' } }, + shapeKeys: ['name'], // `category` was deleted from the `.strict()` ToolSchema + childKeysOf: noChildren, + }); + expect(orphans).toEqual([{ key: 'tool/category', level: 'top' }]); + }); + + it('catches a drilled child row whose key left its container', () => { + const orphans = findOrphanEntries({ + type: 'report', + props: { layout: { children: { columns: { status: 'live' }, aria: { status: 'dead' } } } }, + shapeKeys: ['layout'], + childKeysOf: (key) => (key === 'layout' ? ['columns'] : null), + }); + expect(orphans).toEqual([{ key: 'report/layout.aria', level: 'child' }]); + }); + + it('reports every orphan, not just the first', () => { + const orphans = findOrphanEntries({ + type: 'view', + props: { + gone: { status: 'dead' }, + alsoGone: { status: 'live' }, + list: { children: { type: { status: 'live' }, responsive: { status: 'dead' } } }, + }, + shapeKeys: ['list'], // `gone` and `alsoGone` both left the schema + childKeysOf: (key) => (key === 'list' ? ['type'] : null), + }); + expect(orphans.map((o) => o.key)).toEqual(['view/gone', 'view/alsoGone', 'view/list.responsive']); + }); +}); + +describe('findOrphanEntries — cases it must stay quiet on', () => { + it('does NOT flag a `retiredKey()` tombstone — z.never() is still a property', () => { + // The asymmetry this whole check exists to make legible: a tombstoned key + // stays in the walked shape, so its row is correct and must stay. Flagging + // it would tell the author to delete a row whose deletion fails CI as + // UNCLASSIFIED — pushing them into an infinite loop between two gates. + const orphans = findOrphanEntries({ + type: 'action', + props: { shortcut: { status: 'dead', note: 'REMOVED — tombstoned, row stays' } }, + shapeKeys: ['shortcut'], + childKeysOf: noChildren, + }); + expect(orphans).toEqual([]); + }); + + it('defers `children` declared on a non-container to the forward pass', () => { + // The forward walk already reports this as UNCLASSIFIED with a more precise + // message ("declared children but property is not a container"). Two + // headings for one fix reads as two problems. + const orphans = findOrphanEntries({ + type: 'flow', + props: { name: { children: { nope: { status: 'dead' } } } }, + shapeKeys: ['name'], + childKeysOf: noChildren, // `name` is a string, not a container + }); + expect(orphans).toEqual([]); + }); + + it('is quiet on a ledger that matches its schema exactly', () => { + const orphans = findOrphanEntries({ + type: 'skill', + props: { name: { status: 'live' }, tools: { status: 'live' } }, + shapeKeys: ['name', 'tools', 'label'], // an unclassified extra is the FORWARD pass's job + childKeysOf: noChildren, + }); + expect(orphans).toEqual([]); + }); + + it('handles an absent or empty props block without throwing', () => { + const base = { type: 'page', shapeKeys: ['name'], childKeysOf: noChildren }; + expect(findOrphanEntries({ ...base, props: undefined })).toEqual([]); + expect(findOrphanEntries({ ...base, props: {} })).toEqual([]); + }); + + it('does not confuse an entry FIELD with a child prop', () => { + // `status`/`evidence`/`verifiedAt`/`note`/`authorWarn` live on the row, not + // under `children` — only a declared `children` map is drilled. + const orphans = findOrphanEntries({ + type: 'agent', + props: { + role: { status: 'live', evidence: 'x.ts:1', verifiedAt: '2026-07-30', authorWarn: true, note: 'n' }, + }, + shapeKeys: ['role'], + childKeysOf: () => ['definitely', 'not', 'these'], + }); + expect(orphans).toEqual([]); + }); +}); + +describe('ORPHAN_GUIDANCE', () => { + it('names both causes and the asymmetry, so the wrong fix is not the obvious one', () => { + const text = ORPHAN_GUIDANCE.join(' '); + expect(text).toMatch(/STRICT-REMOVAL/); + expect(text).toMatch(/Delete the row/); + expect(text).toMatch(/retiredKey\(\).*KEEPS the key/); + expect(text).toMatch(/UNCLASSIFIED/); + expect(text).toMatch(/Fix the walk, not the row/); + }); +});