diff --git a/.changeset/alias-integrity-direct-call-sites.md b/.changeset/alias-integrity-direct-call-sites.md new file mode 100644 index 0000000000..1e5f337e1c --- /dev/null +++ b/.changeset/alias-integrity-direct-call-sites.md @@ -0,0 +1,47 @@ +--- +"@objectstack/spec": patch +--- + +Put the 44 pre-helper alias tables under the alias-integrity gate — including the +collision claim they were never measured on. + +`alias-integrity.test.ts` (#5013) judges alias tables built by `strictObject`, +which registers each one alongside the `.shape` it makes claims about. The 44 +call sites that predate that helper call `strictUnknownKeyError` directly and +hand it a transcribed `knownKeys` array; with no shape to register, they sat +outside every claim the gate makes. + +`strictUnknownKeyError` now records its own `{ surface, knownKeys, aliases, +guidance }` in an internal registry, and the gate judges that batch too. The +registration lives in the factory, so no schema module changed. + +What each claim is worth on this batch differs, and the gate says so rather than +implying uniform coverage: + +- **alias key is not a known key** and **alias target is a known key** are + answered against the transcribed array. If that array has drifted from its + schema, both answers inherit the drift — the gap `strictObject` exists to + abolish, and only migrating a call site closes it (#5593). +- **no two alias keys share an `aliasProbe`** (#5481) loses nothing: a collision + is a property of the table alone. These tables were **unmeasured** on it, not + clean — #5481 postdates the measurement recorded in #5483 — and the sweep they + had never had comes back clean at 52 tables. + +The target claim found a real one on its first run: `ui/app.zod.ts` shares four +"start expanded" aliases across all nine navigation-item variants, but `expanded` +is declared on the `group` variant alone, so on the other eight the suggestion +names a key that variant also rejects — the second rejection this campaign +exists to end. Filed as #5555 (the fix rewrites author-facing message text) and +pinned here shrink-only, structurally, so nothing else can enter the tolerance. + +Two supporting pieces, both deliberately visible rather than implicit: the walk +now forces error maps that build themselves on first use (`data/object.zod.ts` +defers its map around a temporal dead zone, and would otherwise never register), +and the six prose alias targets on `ui/app.zod.ts`'s navigation items — +`type: 'url' (with url)` and friends, which name a variant rather than a key — +are an enumerated allowlist scoped to that surface, with a staleness check that +fails if an entry stops being used. + +The shrink-only ratchet at 44 is unchanged. What it discourages — a new call +site minting a fresh second copy of a key list — is exactly as undesirable as it +was before these tables gained a guard. diff --git a/packages/spec/src/shared/alias-integrity.test.ts b/packages/spec/src/shared/alias-integrity.test.ts index e04339a89a..18a325c2c9 100644 --- a/packages/spec/src/shared/alias-integrity.test.ts +++ b/packages/spec/src/shared/alias-integrity.test.ts @@ -49,12 +49,6 @@ * That is the half that makes absence loud: a table the walk cannot reach is a * table this gate is not judging, and it fails rather than passing quietly. * - * ## What it deliberately does not judge - * - * - Tables reaching `strictUnknownKeyError` directly, which carry a transcribed - * `knownKeys` array instead of a shape — measured clean, pinned shrink-only - * below, extension tracked as #5483. - * * ## The third claim (#5481) * * The two claims above are about a table and its *schema*. The third is about a @@ -72,6 +66,38 @@ * general lesson rather than an exemption: since the probe already folds case * and separators, a second spelling of one probe is **never** reachable. It is * dead either way; it is only sometimes also a defect. + * + * ## The second batch: tables that never had a shape (#5483) + * + * `strictObject` is not the only way to get an alias table. The 44 call sites + * that predate the helper call `strictUnknownKeyError` directly and hand it a + * **hand-transcribed `knownKeys` array**, so there is no `.shape` to judge them + * against and no `strictObject` construction to register them. They were + * outside all three claims above. + * + * They are judged now, from a second registry the factory itself fills + * (`alias-table-registry.ts`) — no call site was touched, which is what keeps + * the migration in #5593 a separable change rather than one this guard has + * already half-done. What that buys differs sharply by claim: + * + * - claims 1 and 2 are answered against the **transcription**. If the array has + * drifted from the schema it describes, both answers inherit the drift. That + * gap is exactly what `strictObject` abolishes and only migration closes. + * - claim 3 loses **nothing**: an `aliasProbe` collision is a property of the + * table alone. These 44 were not "measured clean" on it — #5481 postdates the + * measurement recorded in #5483, so they were *unmeasured*. Now they are + * measured, and the sweep comes back clean at 52 tables. + * + * Claim 2 did not come back clean, and the finding is filed rather than fixed + * here: `ui/app.zod.ts` shares four "start expanded" aliases across all nine + * navigation-item variants while `expanded` is declared on `group` alone, so on + * the other eight the suggestion names a key that variant also rejects (#5555). + * Pinned shrink-only below, structurally, because the fix rewrites author-facing + * message text in a call site this transitional guard may not touch. + * + * The shrink-only ratchet also survives unchanged, because what it discourages + * — a NEW direct call site, carrying a fresh second copy of a key list — is + * exactly as undesirable as it was before these tables gained a guard. */ import fs from 'node:fs'; @@ -82,11 +108,23 @@ import { describe, it, expect, beforeAll } from 'vitest'; import ts from 'typescript'; import { aliasProbe } from './alias-probe'; +import { + directAliasTableOverflow, + directAliasTables, + type DirectAliasTableDeclaration, +} from './alias-table-registry'; import { acceptsNothing, strictObjectDeclarations, type StrictObjectDeclaration } from './strict-object'; const HERE = path.dirname(fileURLToPath(import.meta.url)); const SPEC_SRC = path.resolve(HERE, '..'); +/** + * The two modules that *define* the helpers rather than using them. Shared by + * the direct-call AST scan and the shrink-only ratchet so the two can never + * disagree about what counts as a call site. + */ +const HELPER_MODULES = new Set(['shared/suggestions.zod.ts', 'shared/strict-object.ts']); + // --------------------------------------------------------------------------- // The file set — one list, shared by the runtime walk and the AST coverage scan // --------------------------------------------------------------------------- @@ -119,6 +157,22 @@ const isSchema = (v: unknown): boolean => && (typeof v === 'object' || typeof v === 'function') && typeof (v as { _zod?: { def?: unknown } })._zod?.def === 'object'; +/** + * A synthetic `unrecognized_keys` issue, used to force an error map that builds + * itself on first use (see {@link force}). + * + * Shaped as the real thing rather than an empty object because the maps it + * drives read `issue.code` and `issue.keys`. The key is deliberately absurd: if + * a map ever does something more interesting than build a closure, it does it + * over a string no schema declares. + */ +const PROBE_ISSUE = { + code: 'unrecognized_keys', + keys: ['__alias_integrity_probe__'], + path: [], + input: {}, +} as never; + /** * Touch every node in the zod graph under `root`. * @@ -128,6 +182,22 @@ const isSchema = (v: unknown): boolean => * lazy proxy, and descending the graph reaches nested shapes that only build * when their parent does. * + * **Calling the error map is part of forcing** (#5483). Two schemas defer the + * map itself, not just the schema: `strictObject` builds it on first rejection + * (to break the `field.zod` ↔ `suggestions.zod` import cycle) and + * `data/object.zod.ts` does the same to step around a temporal dead zone. For a + * direct `strictUnknownKeyError` call that deferral is the difference between a + * registered table and an invisible one, so the map is invoked here with a + * synthetic issue — the same code path a rejected key takes, minus the parse. + * Building a map is pure, and the `strictObject` maps that also get built this + * way decline to register a second time. + * + * Failures are swallowed on purpose: an error map is arbitrary user code and + * this is a probe, not an assertion. Absence is caught where it means + * something — the coverage checks below fail if a table the source declares + * never reached a registry, which is a far more precise complaint than + * "somebody's error map threw on a synthetic issue". + * * Deliberately generic over `_zod.def` rather than switch-per-zod-type: a node * kind this file does not know about (a future wrapper, a pipe, a discriminated * union arm) must not silently drop the subtree beneath it. The cost is @@ -143,7 +213,13 @@ function force(root: unknown, seen: Set): void { const def = (node as { _zod: { def: Record } })._zod.def; // Reading `.shape` is what forces an object schema's own lazy members. - if ((def as { type?: string }).type === 'object') void (node as { shape?: unknown }).shape; + if ((def as { type?: string }).type === 'object') { + void (node as { shape?: unknown }).shape; + const map = (def as { error?: unknown }).error; + if (typeof map === 'function') { + try { (map as (issue: never) => unknown)(PROBE_ISSUE); } catch { /* see docblock */ } + } + } for (const value of Object.values(def)) { if (typeof value === 'function') { @@ -173,6 +249,9 @@ function force(root: unknown, seen: Set): void { /** Every declaration built by the forcing walk, de-duplicated by content. */ let SURFACES: StrictObjectDeclaration[] = []; +/** The same, for tables that reached `strictUnknownKeyError` directly (#5483). */ +let DIRECT: DirectAliasTableDeclaration[] = []; + beforeAll(async () => { const seen = new Set(); for (const file of MODULES) { @@ -195,6 +274,25 @@ beforeAll(async () => { ); } SURFACES = [...unique.values()]; + + // Same collapse for the direct batch, and for the same reason: the nav-item + // factory in `ui/app.zod.ts` is ONE call site that runs nine times (once per + // `type` variant), and each variant is a genuinely different table — same + // surface template, different `knownKeys`, different cross-variant aliases — + // so the key has to include the key list, not just the surface and aliases. + const uniqueDirect = new Map(); + for (const d of directAliasTables()) { + uniqueDirect.set( + JSON.stringify([ + d.options.surface, + d.options.aliases ?? {}, + d.options.guidance ? Object.keys(d.options.guidance).sort() : [], + [...d.options.knownKeys].sort(), + ]), + d, + ); + } + DIRECT = [...uniqueDirect.values()]; }, 180_000); // --------------------------------------------------------------------------- @@ -212,7 +310,17 @@ interface CallSite { assembled: boolean; } -function callSites(file: string): CallSite[] { +/** + * The two helper spellings, and where each keeps its options literal. + * `strictObject(options, shape)` vs `strictUnknownKeyError(options)`. + */ +const CALLEES = { + strictObject: 2, + strictUnknownKeyError: 1, +} as const; + +function callSites(file: string, callee: keyof typeof CALLEES): CallSite[] { + const arity = CALLEES[callee]; const source = ts.createSourceFile(file, fs.readFileSync(file, 'utf8'), ts.ScriptTarget.Latest, true); const literal = (n: ts.Node): string | null => ts.isStringLiteral(n) || ts.isNoSubstitutionTemplateLiteral(n) ? n.text : null; @@ -227,8 +335,8 @@ function callSites(file: string): CallSite[] { if ( ts.isCallExpression(node) && ts.isIdentifier(node.expression) - && node.expression.text === 'strictObject' - && node.arguments.length === 2 + && node.expression.text === callee + && node.arguments.length === arity && ts.isObjectLiteralExpression(node.arguments[0]) ) { const opts = node.arguments[0]; @@ -262,7 +370,15 @@ function callSites(file: string): CallSite[] { return out; } -const CALL_SITES = MODULES.flatMap(callSites); +const CALL_SITES = MODULES.flatMap((f) => callSites(f, 'strictObject')); + +/** + * The pre-helper wiring's call sites (#5483) — the helper modules excluded, so + * `strictObject`'s own internal call is not mistaken for one of them. + */ +const DIRECT_CALL_SITES = MODULES + .filter((f) => !HELPER_MODULES.has(path.relative(SPEC_SRC, f))) + .flatMap((f) => callSites(f, 'strictUnknownKeyError')); // --------------------------------------------------------------------------- // 1. Coverage — the walk reached every table the source declares @@ -301,23 +417,22 @@ describe('alias integrity — coverage', () => { expect(unreached, 'these alias tables are not reachable from any module export, so nothing judges them').toEqual([]); }); - it('the surface this gate does NOT cover only ever shrinks', () => { + it('the surface this gate can only judge by transcription only ever shrinks', () => { // `strictObject` is not the only way to get an alias table: the pre-helper // wiring calls `strictUnknownKeyError` directly with a hand-transcribed - // `knownKeys` array, and those tables never reach the registry this gate - // reads. Measured at 44 call sites when the gate was written, and measured - // CLEAN on both criteria at the same time — so this is a coverage boundary, - // not hidden debt. It is pinned shrink-only rather than left implicit - // because an uncovered table that nobody can see growing is precisely the - // "green check over source nothing read" failure the campaign keeps paying - // for: migrating one to `strictObject` is free, adding a NEW one fails here - // and forces the choice to be deliberate. Extending the judgement over them - // is #5483 — they carry a transcribed key list rather than a shape, so it - // is a different measurement, not more of this one. - const uncovered = MODULES.filter((f) => { - const rel = path.relative(SPEC_SRC, f); - return rel !== 'shared/suggestions.zod.ts' && rel !== 'shared/strict-object.ts'; - }).flatMap((f) => { + // `knownKeys` array. Since #5483 those tables ARE judged — the factory + // registers them and the block below reads them — so this number is no + // longer a measure of what nothing watches. It measures what is watched + // with the WEAKER instrument: three claims, two of them answered against + // the transcription rather than the shape, so a drifted array drags both + // answers with it and this file cannot tell. + // + // Which leaves the ratchet meaning exactly what it always meant. Migrating + // one call site to `strictObject` is free and moves it to the shape-backed + // half; adding a NEW one mints a fresh second copy of a key list and fails + // here, forcing the choice to be deliberate. The batched migration that + // takes this to 0 is #5593. + const uncovered = MODULES.filter((f) => !HELPER_MODULES.has(path.relative(SPEC_SRC, f))).flatMap((f) => { const source = fs.readFileSync(f, 'utf8'); return [...source.matchAll(/\bstrictUnknownKeyError\s*\(/g)].map(() => path.relative(SPEC_SRC, f)); }); @@ -439,3 +554,270 @@ describe('alias integrity — every table is a true claim about its schema', () expect(dead.sort()).toEqual([]); }); }); + +// --------------------------------------------------------------------------- +// 3. The same claims over the tables that never had a shape (#5483) +// --------------------------------------------------------------------------- + +/** + * Alias targets that are deliberately **prose, not a key name** — the only + * place in `packages/spec` where that is true, and an explicit allowlist rather + * than a relaxed criterion because the two are not the same promise. + * + * `ui/app.zod.ts` builds one table per navigation-item `type`, and the + * commonest nav mistake is not a typo: it is `dashboardName` written on a `url` + * item — a real key, on the wrong variant. Naming a key there would be wrong + * twice over (the key IS spelled correctly, and writing it is still not enough + * without the matching `type`), so the target is a sentence: + * + * Did you mean `dashboardname` → `type: 'dashboard' (with dashboardName)`? + * + * Enumerated, not pattern-matched, and paired with the surface family that owns + * them, so a new prose target anywhere — including a second one on this very + * surface — fails the target criterion and has to be argued for here. The + * staleness test below is the other half: an entry nothing uses is deleted, so + * this list cannot quietly outlive the tables it excuses. + */ +const PROSE_ALIAS_TARGETS: ReadonlySet = new Set([ + "type: 'object' (with objectName)", + "type: 'page' (with pageName)", + "type: 'url' (with url)", + "type: 'dashboard' (with dashboardName)", + "type: 'report' (with reportName)", + "type: 'component' (with componentRef)", +]); + +/** The surface family the prose targets are allowed on, and nowhere else. */ +const PROSE_TARGET_SURFACE = /^this `[a-z]+` navigation item$/; + +const isProseTarget = (surface: string, target: string): boolean => + PROSE_TARGET_SURFACE.test(surface) && PROSE_ALIAS_TARGETS.has(target); + +/** + * A **defect this gate found**, pinned shrink-only rather than exempted: the + * four spellings of "start expanded" that `NAV_ITEM_ALIASES` redirects to + * `expanded` (#5555). + * + * `expanded` is declared on the `group` variant alone, but the shared alias + * table is stamped into all nine, so on the other eight the redirect names a + * key that variant also rejects — ledger finding 7's second rejection, from the + * campaign built to end it. The mechanism is visible in the file: the six + * CROSS-VARIANT payload keys immediately below get prose targets + * (`type: 'object' (with objectName)`) precisely because a bare key name would + * mislead; these four are cross-variant too, and were written in the shared + * table where that was not apparent. + * + * Distinguished from {@link PROSE_ALIAS_TARGETS} on purpose. That list says + * "this is right"; this one says "this is wrong, it is 32 occurrences of one + * mistake, and the fix belongs to the schema rather than the gate" — the fix + * rewrites author-facing message text in `ui/app.zod.ts`, which #5483's + * transitional guard is explicitly not allowed to touch. + * + * Structural rather than a list of 32 strings, so the tolerance cannot widen by + * accident: only this alias key, only this target, only this surface family. + * The count below is the ratchet. + */ +const EXPANDED_CROSS_VARIANT_KEYS: ReadonlySet = new Set([ + 'defaultopen', 'open', 'collapsed', 'isopen', +]); + +const isPinnedExpandedDefect = (surface: string, written: string, target: string): boolean => + PROSE_TARGET_SURFACE.test(surface) + && target === 'expanded' + && EXPANDED_CROSS_VARIANT_KEYS.has(written); + +/** + * Guidance filed once for a table stamped nine times, legal on two of them. + * + * `children` really is declared on the `object` and `group` nav variants, so + * the prescription "`children` is only meaningful on a `group` item…" correctly + * never fires there — it is written for the other seven, where it does. That is + * a criterion meeting a variant family, not a dead entry: the shape-backed half + * of this gate judges one authored table against one shape, and here one + * authored table is stamped against nine. + * + * Exempted rather than pinned as debt because there is nothing to fix — no + * author is misinformed and no prescription is lost. Enumerated by + * `(surface, key)` so it cannot cover a second guidance entry that IS dead. + */ +const VARIANT_LEGAL_GUIDANCE: ReadonlySet = new Set([ + 'this `object` navigation item::children', + 'this `group` navigation item::children', +]); + +/** `file:line — "surface": \`written\` -> \`target\``, for the direct batch. */ +const directEntry = (d: DirectAliasTableDeclaration, written: string, target: string): string => { + const site = DIRECT_CALL_SITES.find( + (c) => c.surface === d.options.surface + && Object.entries(c.aliases).every(([k, v]) => d.options.aliases?.[k] === v), + ); + const where = site ? `${site.file}:${site.line}` : '(location unresolved)'; + return `${where} — "${d.options.surface}": \`${written}\` -> \`${target}\``; +}; + +describe('alias integrity — direct `strictUnknownKeyError` tables (#5483)', () => { + it('the direct call sites really registered (self-test before the verdict)', () => { + // Same guard as the shape-backed half: state the scale before the verdict, + // so a registration that silently stopped working reads as a failure and + // not as forty-four clean tables. + expect(DIRECT_CALL_SITES.length).toBeGreaterThanOrEqual(40); + // One call site (the nav-item factory) runs nine times, so the registry is + // legitimately LARGER than the source count. It can never be smaller + // without a table having gone unjudged. + expect(DIRECT.length).toBeGreaterThanOrEqual(DIRECT_CALL_SITES.length); + expect(DIRECT.some((d) => Object.keys(d.options.aliases ?? {}).length > 0)).toBe(true); + // The registry is capped (it is filled by a PUBLISHED factory — see + // `alias-table-registry.ts`). Overflow would mean judging a prefix. + expect(directAliasTableOverflow(), 'the direct registry overflowed; raise CAPACITY').toBe(0); + }); + + it('every direct call site with an alias table was reached at runtime', () => { + const bySurface = new Map(); + for (const d of DIRECT) { + const list = bySurface.get(d.options.surface) ?? []; + list.push(d); + bySurface.set(d.options.surface, list); + } + const unreached: string[] = []; + for (const site of DIRECT_CALL_SITES) { + if (!site.hasAliases) continue; + const candidates = site.surface ? (bySurface.get(site.surface) ?? []) : DIRECT; + const matched = candidates.some((d) => + Object.entries(site.aliases).every(([k, v]) => d.options.aliases?.[k] === v)); + if (!matched) unreached.push(`${site.file}:${site.line} (${site.surface ?? 'assembled surface'})`); + } + expect(unreached, 'these alias tables never reached the registry, so nothing judges them').toEqual([]); + }); + + it('the deferred error maps were forced too', () => { + // `data/object.zod.ts` builds its map on FIRST USE, to step around a + // temporal dead zone. Nothing in this file parses anything, so without the + // synthetic-issue poke in `force()` that table never registers. + // + // Measured, not assumed: deleting the poke turns this red AND the coverage + // check above (that site's `surface` and alias entries are both literals, + // so the AST can see it and report it unreached). Two failures for one + // cause — so this assertion is not what makes the poke's absence *visible*, + // it is what makes it legible. "`this object` is missing" names the + // mechanism; "some site at object.zod.ts:962 is unreached" sends the next + // reader looking for a walk bug that is not there. + expect( + DIRECT.map((d) => d.options.surface), + "`data/object.zod.ts`'s deferred map did not register — did the forcing poke stop working?", + ).toContain('this object'); + }); + + it('the nav-item factory registered one table per variant', () => { + // The one direct site the AST reads as an empty assembled table: both its + // surface (a template) and its aliases (spreads) are computed. Nine + // variants in, nine tables out — the count is the coverage. + const navTables = DIRECT.filter((d) => PROSE_TARGET_SURFACE.test(d.options.surface)); + expect(navTables.length).toBe(9); + }); + + it('no alias key is itself a known key (a dead entry that can never fire)', () => { + // Claim 1, answered against the transcribed `knownKeys` rather than a + // shape. Weaker — a key list that has drifted from its schema drags the + // answer with it — but it is the list the SUGGESTER reads, so a hit here is + // a real dead entry either way. + const dead: string[] = []; + for (const d of DIRECT) { + const known = new Set(d.options.knownKeys); + for (const [written, target] of Object.entries(d.options.aliases ?? {})) { + if (known.has(written)) { + dead.push(`${directEntry(d, written, target)} — \`${written}\` is a known key here`); + } + } + } + expect(dead.sort()).toEqual([]); + }); + + it('every alias target is a key the table claims to accept', () => { + // Claim 2. The tombstone half of the shape-backed version has no analogue + // here: `knownKeys` is a flat array with no schemas in it, so "this target + // is declared but accepts nothing" is invisible until the call site + // migrates (#5593). + const broken: string[] = []; + const pinned: string[] = []; + for (const d of DIRECT) { + const known = new Set(d.options.knownKeys); + for (const [written, target] of Object.entries(d.options.aliases ?? {})) { + if (known.has(target) || isProseTarget(d.options.surface, target)) continue; + const report = `${directEntry(d, written, target)} — \`${target}\` is not a known key here`; + if (isPinnedExpandedDefect(d.options.surface, written, target)) pinned.push(report); + else broken.push(report); + } + } + expect(broken.sort()).toEqual([]); + // The known defect, ratcheted (#5555). Shrink-only: fixing a variant drops + // the number, and nothing else can raise it — `isPinnedExpandedDefect` is + // structural, so a new broken target anywhere (including a fifth spelling + // of "expanded" on this very family) lands in `broken` above instead. + expect(pinned.length, 'the pinned #5555 tolerance grew; it may only shrink').toBeLessThanOrEqual(32); + }); + + it('every prose-target exemption is still load-bearing', () => { + // An allowlist nobody reaches is the silent pass-through this exemption was + // written to avoid, one release later. If a nav variant is reworded, or the + // family migrates to `strictObject`, the stale entries surface here instead + // of quietly widening what claim 2 will forgive. + const used = new Set(); + for (const d of DIRECT) { + if (!PROSE_TARGET_SURFACE.test(d.options.surface)) continue; + for (const target of Object.values(d.options.aliases ?? {})) { + if (PROSE_ALIAS_TARGETS.has(target)) used.add(target); + } + } + expect([...PROSE_ALIAS_TARGETS].filter((t) => !used.has(t)).sort()).toEqual([]); + }); + + it('no two alias keys in one table collapse onto the same probe (#5481)', () => { + // Claim 3, and the only one that loses NOTHING for lack of a shape: the + // probe reads the alias table alone. This dimension was never measured on + // these 44 tables — #5481 postdates #5483's "measured clean" note, which + // covered the other two claims — so this assertion is the measurement, not + // a re-statement of one. + const collisions: string[] = []; + for (const d of DIRECT) { + const byProbe = new Map(); + for (const key of Object.keys(d.options.aliases ?? {})) { + byProbe.set(aliasProbe(key), [...(byProbe.get(aliasProbe(key)) ?? []), key]); + } + for (const [probe, keys] of byProbe) { + if (keys.length < 2) continue; + const written = keys.map((k) => `\`${k}\` -> \`${d.options.aliases?.[k]}\``).join(', '); + collisions.push( + `${directEntry(d, keys[0], d.options.aliases?.[keys[0]] ?? '?')} — ${keys.length} keys share the probe \`${probe}\`: ${written}` + + ` — only \`${d.options.aliases?.[keys[keys.length - 1]]}\` survives`, + ); + } + } + expect(collisions.sort()).toEqual([]); + }); + + it('no guidance key is itself a known key (the same dead entry, other channel)', () => { + const dead: string[] = []; + for (const d of DIRECT) { + const known = new Set(d.options.knownKeys); + for (const written of Object.keys(d.options.guidance ?? {})) { + if (!known.has(written)) continue; + if (VARIANT_LEGAL_GUIDANCE.has(`${d.options.surface}::${written}`)) continue; + dead.push(`"${d.options.surface}": guidance for \`${written}\`, which is a known key here`); + } + } + expect(dead.sort()).toEqual([]); + }); + + it('every variant-legal guidance exemption is still load-bearing', () => { + // Same staleness rule the prose targets get: an exemption nobody reaches is + // deleted, not left widening what the criterion above forgives. + const reached = new Set(); + for (const d of DIRECT) { + const known = new Set(d.options.knownKeys); + for (const written of Object.keys(d.options.guidance ?? {})) { + if (known.has(written)) reached.add(`${d.options.surface}::${written}`); + } + } + expect([...VARIANT_LEGAL_GUIDANCE].filter((e) => !reached.has(e)).sort()).toEqual([]); + }); +}); diff --git a/packages/spec/src/shared/alias-table-registry.ts b/packages/spec/src/shared/alias-table-registry.ts new file mode 100644 index 0000000000..c1b472ff3b --- /dev/null +++ b/packages/spec/src/shared/alias-table-registry.ts @@ -0,0 +1,118 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The registry for alias tables that reach {@link strictUnknownKeyError} + * **directly** — the pre-`strictObject` wiring (#5483). + * + * ## Why a second registry + * + * `strictObject` records `{ options, shape }` at construction, so + * `alias-integrity.test.ts` can judge a table against the **shape** it makes + * claims about. The 44 call sites that predate the helper have no shape to + * offer: they hand `strictUnknownKeyError` a hand-transcribed `knownKeys` + * array. That is a weaker instrument — the array can drift from the schema it + * describes and nothing here can see it — and mixing the two into one registry + * would quietly relabel that weakness as shape-backed coverage. + * + * So they are kept apart, and the difference is the point: + * + * | claim | `strictObject` registry | this registry | + * |:--|:--|:--| + * | alias key is not a declared key | judged vs `.shape` | judged vs `knownKeys` | + * | alias target is a declared key | judged vs `.shape` (+ tombstone check) | judged vs `knownKeys` | + * | no two alias keys share a probe | judged | judged — **identically** | + * + * The third claim (#5481) is the one that loses nothing: `aliasProbe` + * collisions are a property of the table *alone*, so judging them here is the + * same measurement the covered surfaces get, not an approximation of it. That + * is what made the transitional guard worth shipping ahead of the migration — + * before it, those 44 tables were **unmeasured** on collisions, not clean. + * + * Retiring this registry is the migration in #5593: every call site that moves + * to `strictObject` leaves here and arrives there, shape-backed, and the last + * one takes this file with it. + * + * ## Not part of the package contract + * + * Deliberately **not** re-exported from `shared/index.ts`, like + * `strict-object.ts` and `alias-probe.ts`: it is an internal seam the gate + * reaches by relative path. Adding it to the barrel would publish a mutable + * process-global as API. + */ + +import type { StrictUnknownKeyErrorOptions } from './suggestions.zod'; + +/** One alias table recorded as its owning error map was built. */ +export interface DirectAliasTableDeclaration { + /** The authoring metadata the error map was built from. */ + readonly options: StrictUnknownKeyErrorOptions; +} + +/** + * Upper bound on recorded tables. + * + * `strictObject` is internal, so its registry can only ever grow with the + * schemas this repo declares. `strictUnknownKeyError` is **published** (it is + * in `api-surface.json` under `./shared`), and a consumer is free to build one + * error map per tenant, per request, in a loop — a registry that recorded every + * one of those would be an unbounded retainer in someone else's process for the + * sole benefit of a test in ours. + * + * The cap makes the failure mode a *measurement* problem rather than a memory + * one, and `overflowed()` makes even that loud: the gate asserts it is zero, so + * if this repo ever declares more tables than fit, the check fails instead of + * silently judging a prefix. In-repo occupancy is ~52 of 512. + */ +const CAPACITY = 512; + +const DECLARATIONS: DirectAliasTableDeclaration[] = []; +let overflow = 0; +let suppression = 0; + +/** + * Record a table built by a direct {@link strictUnknownKeyError} call. + * + * Called from the factory itself rather than from the 44 call sites, which is + * the whole reason the transitional guard costs no call-site edits. + */ +export function registerDirectAliasTable(options: StrictUnknownKeyErrorOptions): void { + if (suppression > 0) return; + if (DECLARATIONS.length >= CAPACITY) { + overflow++; + return; + } + DECLARATIONS.push({ options }); +} + +/** + * Run `build` with registration turned off — for `strictObject`, whose tables + * are already recorded **with their shape** in the richer registry. + * + * Without this, a `strictObject` surface would land in both registries the + * moment its deferred error map got built (which the gate's forcing pass does + * deliberately, and any parse failure does incidentally), and this registry's + * population would depend on whether some earlier test had rejected a key. + * A count that moves with unrelated test ordering is not a measurement. + * + * Depth-counted rather than a boolean because the suppressed callback is free + * to build another map; synchronous throughout, so there is no interleaving to + * lose track of. + */ +export function withoutDirectAliasTableRegistration(build: () => T): T { + suppression++; + try { + return build(); + } finally { + suppression--; + } +} + +/** Every direct table built **so far in this process** (#5483). */ +export function directAliasTables(): readonly DirectAliasTableDeclaration[] { + return DECLARATIONS; +} + +/** How many tables were dropped for want of {@link CAPACITY}. Asserted zero. */ +export function directAliasTableOverflow(): number { + return overflow; +} diff --git a/packages/spec/src/shared/strict-object.ts b/packages/spec/src/shared/strict-object.ts index 7f78f651bb..4ef334cd99 100644 --- a/packages/spec/src/shared/strict-object.ts +++ b/packages/spec/src/shared/strict-object.ts @@ -60,6 +60,7 @@ import { z } from 'zod'; +import { withoutDirectAliasTableRegistration } from './alias-table-registry'; import { strictUnknownKeyError } from './suggestions.zod'; /** @@ -228,7 +229,14 @@ export function strictObject(options: StrictObjectOptio typeof issue.input === 'string' ? retiredForms[issue.input] : undefined; if (prescription) return prescription; } - return (build ??= strictUnknownKeyError({ + // Built WITHOUT registering in the direct-call registry (#5483). This table + // is already recorded below with its `shape`, which is the stronger record: + // letting it land in both would judge it twice — the second time against + // the transcription-shaped view (`knownKeys`, tombstones filtered out) + // rather than the shape — and would make that registry's population depend + // on whether anything happened to reject a key first, since this build is + // deferred to the first rejection. + return (build ??= withoutDirectAliasTableRegistration(() => strictUnknownKeyError({ surface, // Declared-but-unwritable keys (tombstones) are excluded — see // `acceptsNothing`. They stay in the SHAPE, so writing one still raises @@ -241,7 +249,7 @@ export function strictObject(options: StrictObjectOptio history, aliases, guidance, - }))(issue); + })))(issue); }; DECLARATIONS.push({ options, shape }); diff --git a/packages/spec/src/shared/suggestions.zod.ts b/packages/spec/src/shared/suggestions.zod.ts index 740ebbffce..f35824a60c 100644 --- a/packages/spec/src/shared/suggestions.zod.ts +++ b/packages/spec/src/shared/suggestions.zod.ts @@ -4,6 +4,7 @@ import type { z } from 'zod'; import { FieldType } from '../data/field.zod'; import { aliasProbe } from './alias-probe'; +import { registerDirectAliasTable } from './alias-table-registry'; /** * "Did you mean?" Suggestion Utilities @@ -267,8 +268,26 @@ export interface StrictUnknownKeyErrorOptions { * * First consumers: `ui/action.zod.ts` (#3746, the template this generalizes), * `security/permission.zod.ts`, `automation/flow.zod.ts`. + * + * ## The table is recorded as it is built (#5483) + * + * Every call registers its `{ surface, knownKeys, aliases, guidance }` with + * `alias-table-registry`, which is what puts the 44 remaining direct call sites + * under `alias-integrity.test.ts`. Registering **here** rather than at the call + * sites is the entire trick: the gate gains 44 tables and the schemas gain no + * edit, so the migration to `strictObject` (#5593) stays a clean, separable + * change rather than something this guard has already half-done. + * + * What the gate can judge from a registration is bounded by what a direct call + * offers. `knownKeys` is a hand-transcribed array, so "is this alias target a + * real key?" is answered against the transcription, not the shape — the + * array-vs-shape drift `strictObject` exists to abolish stays unguarded until + * the call site migrates. The `aliasProbe` collision claim has no such caveat: + * it reads the alias table alone, so it is judged here exactly as it is on a + * `strictObject` surface. */ export function strictUnknownKeyError(options: StrictUnknownKeyErrorOptions): z.core.$ZodErrorMap { + registerDirectAliasTable(options); const { surface, knownKeys, guidance = {}, history } = options; const aliases: Record = {}; for (const [key, canonical] of Object.entries(options.aliases ?? {})) {