diff --git a/.changeset/unknown-key-lint-nested-descent.md b/.changeset/unknown-key-lint-nested-descent.md new file mode 100644 index 0000000000..8867e6882e --- /dev/null +++ b/.changeset/unknown-key-lint-nested-descent.md @@ -0,0 +1,35 @@ +--- +"@objectstack/spec": minor +--- + +The unknown-authoring-key lint now descends into nested metadata, not just each +item's top level. + +`lintUnknownAuthoringKeys` (#3786) reported unknown keys on each metadata item's +top level plus one hard-coded hop into `object.fields`. That left **227 +strip-mode objects** nested below those roots reporting nothing — and they are +concentrated exactly where authoring volume is: `object` 71, `view` 49, `page` +24, `dashboard` 18, `agent` 16, `mapping` 14. Those sites were both silently +dropping keys and contributing nothing to the evidence base the v18 strict +close-out is meant to be scheduled on. + +The walk now follows the authored value alongside its schema through nested +objects, arrays and records. Posture rules are unchanged, so the lint still never +double-reports what the parse already handles: + +- `strict` → silent (the parse is loud on its own) +- `passthrough` → silent (the key legally survives) +- `strip` → reported, and the descent continues through it + +Unions descend only when the authored value picks a branch unambiguously — a +discriminated union whose discriminator the author actually wrote. Otherwise the +merged posture applies at that level and the walk stops, because guessing a +branch would invent findings against a shape nobody wrote. + +`object.fields` still reports as the `field` surface with its curated guidance, +now via an explicit override table rather than a special case — so its own nested +sites (`fields.*.options[]`, …) are covered too. + +Still non-blocking: these are warnings from `defineStack()`, `os validate` and +`os compile`, exactly as before. Expect existing projects to surface more of +them — each one is a key that was already being dropped, now visible. diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 635b4362a1..a7ff9942e8 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -213,13 +213,12 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). ## Next steps (verify-then-enforce, one shape at a time) -1. The `@objectstack/lint` unknown-key WARNING layer: non-breaking, shippable - in a minor, and it extends AI-detectable coverage to every remaining - authorable site at once while accumulating evidence (which keys real - tenant projects actually carry) for a v18 strict close-out. -2. Promote this ledger to a machine-checked gate (pattern of +1. Promote this ledger to a machine-checked gate (pattern of `packages/spec/liveness/` + `check:liveness`) once enough of the surface is classified that the table above is enforceable rather than descriptive. +2. Let the warning layer run in the wild for a release, then schedule the v18 + strict close-out on what it actually reports — which is the whole point of + having built it. Nothing more to do here until there is field data. Done in step 2: `security/rls.zod.ts` + `security/sharing.zod.ts` strict; `PositionSchema` strict with the protection envelope declared (closing the @@ -262,5 +261,36 @@ the flip carried strictness INTO the JSON schema. It did not; approval's schema would have said `false` regardless. The registration-time rejection it describes is real, but it came from the published schema, not from the flip.) +## The warning layer (was "next step 1" — it already existed) + +The `@objectstack/lint` unknown-key WARNING layer this list carried as a pending +next step **had already been built** by the time the data step landed: #3786's +`lintUnknownAuthoringKeys` plus #4167's `lintUnknownStackKeys`, exported from +`@objectstack/spec` and wired into `defineStack()`, `os validate` and +`os compile` as non-blocking warnings. Anyone reading this file for what to do +next was being sent to build something that shipped releases ago. + +What was genuinely missing was **depth**, not existence. The walk covered each +metadata item's top level plus one hard-coded descent into `object.fields`, +which left 227 strip-mode objects nested below those roots reporting nothing — +concentrated exactly where authoring volume is: `object` 71, `view` 49, +`page` 24, `dashboard` 18, `agent` 16, `mapping` 14. Those sites were both +silently eating keys AND contributing nothing to the evidence base the v18 +close-out is supposed to be scheduled on. + +The walk now descends the authored value alongside its schema through nested +objects, arrays and records, applying the same posture rules (`strict` and +`passthrough` stay silent; only `strip` reports). Unions descend only when the +authored value picks a branch unambiguously — guessing would invent findings +against a shape nobody wrote. `object.fields` keeps reporting as `field` with +its curated guidance, now via an explicit override table rather than a special +case, so its own nested sites (`fields.*.options[]`, …) are covered too. + +Audited after the change, since "our own assets are clean" has been wrong +before: 43 platform objects + 3 apps + 1 dashboard + 3 pages, and both example +apps (23 + 6 objects, 20 + 1 pages, 4 + 1 datasets, 3 + 1 dashboards) report +**zero** unknown keys. No finding this time — worth recording precisely because +the app step's `ACCOUNT_APP.defaultOpen` came from exactly this class of check. + Long tail stays gated on a verification pass per shape — never a one-shot "make all ~453 sites strict" (ADR-0054 ratchet; #4001's own recommendation). diff --git a/packages/spec/src/kernel/metadata-authoring-lint.test.ts b/packages/spec/src/kernel/metadata-authoring-lint.test.ts index b2a48c6f30..4261790987 100644 --- a/packages/spec/src/kernel/metadata-authoring-lint.test.ts +++ b/packages/spec/src/kernel/metadata-authoring-lint.test.ts @@ -144,6 +144,82 @@ describe('the #4148 behaviours survive the generalization', () => { }); }); +describe('nested descent (#4001 evidence phase)', () => { + // Before this the walk stopped at each item's top level plus a hard-coded + // hop into `object.fields`, leaving 227 strip-mode objects below those roots + // reporting nothing. These pin the four structural moves the descent makes + // and — just as importantly — the two cases where it must stay quiet. + + it('reports inside a nested object', () => { + const [finding, ...rest] = lintUnknownAuthoringKeys({ + objects: [{ name: 'o1', userActions: { zzz_nested: 1 } }], + }); + expect(rest).toEqual([]); + expect(finding).toMatchObject({ + path: 'objects.o1.userActions.zzz_nested', + surface: 'object', + key: 'zzz_nested', + }); + }); + + it('reports inside an array element, indexed by position', () => { + const [finding] = lintUnknownAuthoringKeys({ + pages: [{ name: 'p1', regions: [{ name: 'r', zzz_nested: 1 }] }], + }); + expect(finding).toMatchObject({ path: 'pages.p1.regions.0.zzz_nested', surface: 'page' }); + }); + + it('carries the field surface through a record INTO its nested array', () => { + // The override is keyed on the path relative to the item root, so a record's + // author-chosen keys must not join that path — otherwise only the first + // field would resolve as `field`. This also proves the descent continues + // BELOW the override rather than stopping at it. + const [finding] = lintUnknownAuthoringKeys({ + objects: [{ + name: 'o2', + fields: { s: { type: 'select', options: [{ label: 'A', value: 'a', zzz_nested: 1 }] } }, + }], + }); + expect(finding).toMatchObject({ + path: 'objects.o2.fields.s.options.0.zzz_nested', + surface: 'field', + }); + }); + + it('stays silent where the nested parse is already strict', () => { + // A dashboard widget is .strict(); reporting here would double-report what + // the parse rejects loudly on its own. + expect(lintUnknownAuthoringKeys({ + dashboards: [{ name: 'd1', widgets: [{ id: 'w', type: 'chart', zzz_nested: 1 }] }], + })).toEqual([]); + }); + + it('does not guess a union branch the author never picked', () => { + // `type: 'not_a_real_component'` matches no discriminator literal. Descending + // into an arbitrary member would invent findings against a shape nobody wrote. + expect(lintUnknownAuthoringKeys({ + pages: [{ name: 'p1', regions: [{ name: 'r', components: [{ type: 'not_a_real_component', zzz_nested: 1 }] }] }], + })).toEqual([]); + }); + + it('still survives malformed nested input rather than throwing', () => { + for (const junk of [ + { objects: [{ name: 'o', userActions: 'nope' }] }, + { pages: [{ name: 'p', regions: 'nope' }] }, + { pages: [{ name: 'p', regions: [null, 7, { name: 'r' }] }] }, + { objects: [{ name: 'o', fields: { f: { type: 'select', options: 'nope' } } }] }, + ]) { + expect(() => lintUnknownAuthoringKeys(junk)).not.toThrow(); + } + }); + + it('keeps ignoring the underscore channel at depth', () => { + expect(lintUnknownAuthoringKeys({ + pages: [{ name: 'p1', regions: [{ name: 'r', _provenance: 'x' }] }], + })).toEqual([]); + }); +}); + describe('top-level stack keys (#4167)', () => { const lint = (raw: unknown) => lintUnknownStackKeys(raw, ObjectStackDefinitionSchema); diff --git a/packages/spec/src/kernel/metadata-authoring-lint.ts b/packages/spec/src/kernel/metadata-authoring-lint.ts index 136800bdc6..2baa360a61 100644 --- a/packages/spec/src/kernel/metadata-authoring-lint.ts +++ b/packages/spec/src/kernel/metadata-authoring-lint.ts @@ -57,7 +57,6 @@ import { STACK_RUNTIME_MEMBERS, type UnknownAuthoringKeyFinding, } from '../data/authoring-key-lint'; -import { FieldSchema } from '../data/field.zod'; import { PLURAL_TO_SINGULAR } from '../shared/metadata-collection.zod'; import { getMetadataTypeSchema } from './metadata-type-schemas'; @@ -167,6 +166,149 @@ export function listLintableAuthoringCollections(): LintableAuthoringCollection[ return out; } +/** + * How deep below a metadata item the walk goes. The descent is bounded by the + * AUTHORED data, not by the schema, so this only matters for pathologically + * nested input; 12 clears the deepest real shape (a page's nested region tree). + */ +const MAX_DESCENT_DEPTH = 12; + +/** + * Nested surfaces that report under their own name instead of the enclosing + * metadata type, keyed by `:`. + * + * Only `object.fields` qualifies today. It is the surface #4120 mined for the + * curated guidance table, and a finding on it has always said `field` rather + * than `object` — callers and tests depend on that. Everything else nested + * reports under its metadata type, which is the axis authors think in ("this is + * a page problem"), and falls back to the edit-distance suggestion. + */ +const NESTED_SURFACES: Readonly< + Record> }> +> = Object.freeze({ + 'object:fields': { surface: 'field', guidance: FIELD_KEY_GUIDANCE }, +}); + +/** + * Walk the authored value alongside its schema, reporting unknown keys at every + * strip-mode object below the item root (#4001 evidence phase). + * + * Before this, the walk stopped at each metadata item's top level plus one + * hard-coded descent into `object.fields`. That left 227 strip-mode objects + * nested below those roots — `page.regions[].components[]`, `dashboard.widgets[]`, + * `view.config.data`, `action.params[].options[]` — silently eating keys AND + * contributing nothing to the evidence base the v18 strict close-out is meant to + * be scheduled on. The two most-authored types were the worst off: `object` has + * 71 such sites, `view` 49. + * + * Posture rules match {@link keyPosture}, so the lint never double-reports what + * the parse already rejects: + * - `strict` → silent, the parse is loud on its own. + * - `passthrough` → silent, the key legally survives. + * - `strip` → reported, and the descent continues through it. + * + * Unions descend only when the authored value picks a branch unambiguously (a + * discriminated union whose discriminator the author actually wrote). Otherwise + * the merged posture from {@link keyPosture} is applied at this level and the + * walk stops: guessing a branch would invent findings against a shape the author + * never chose. + */ +function descend( + schema: unknown, + raw: unknown, + path: string, + relPath: string, + surface: string, + guidance: Readonly>, + out: UnknownAuthoringKeyFinding[], + depth: number, +): void { + if (depth > MAX_DESCENT_DEPTH || raw == null) return; + const u = unwrap(schema); + const d = u?.def ?? u?._def; + if (!d) return; + + if (d.type === 'union' || d.type === 'discriminated_union') { + const branch = d.discriminator && isPlainRecord(raw) + ? pickUnionBranch(d, raw[d.discriminator]) + : undefined; + if (branch) { + descend(branch, raw, path, relPath, surface, guidance, out, depth + 1); + return; + } + const merged = keyPosture(u); + if (merged?.mode === 'strip' && merged.keys.size > 0 && isPlainRecord(raw)) { + lintAuthoredRecordKeys(raw, merged.keys, guidance, surface, path, out); + } + return; + } + + if (d.type === 'object') { + if (!isPlainRecord(raw)) return; + const shape = (typeof d.shape === 'function' ? d.shape() : d.shape) ?? {}; + // depth 0 is the item root, already reported by the caller. + if (depth > 0) { + const posture = keyPosture(u); + if (posture?.mode === 'strip' && posture.keys.size > 0) { + lintAuthoredRecordKeys(raw, posture.keys, guidance, surface, path, out); + } + } + for (const [key, child] of Object.entries(shape)) { + if (!(key in raw)) continue; + const childRel = relPath ? `${relPath}.${key}` : key; + const override = NESTED_SURFACES[`${surface}:${childRel}`]; + descend( + child, + raw[key], + `${path}.${key}`, + childRel, + override?.surface ?? surface, + override?.guidance ?? guidance, + out, + depth + 1, + ); + } + return; + } + + if (d.element) { + if (!Array.isArray(raw)) return; + for (let i = 0; i < raw.length; i++) { + descend(d.element, raw[i], `${path}.${i}`, relPath, surface, guidance, out, depth + 1); + } + return; + } + + if (d.valueType) { + if (!isPlainRecord(raw)) return; + for (const [key, value] of Object.entries(raw)) { + // A record's KEYS are author-chosen names, so they never join relPath — + // `object.fields` must stay `fields`, not `fields.owner`, or the override + // above would miss every field but one. + descend(d.valueType, value, `${path}.${key}`, relPath, surface, guidance, out, depth + 1); + } + } +} + +/** The union member whose discriminator literal matches `value`, if exactly one does. */ +function pickUnionBranch(unionDef: any, value: unknown): unknown { + if (value === undefined) return undefined; + for (const option of unionDef.options ?? []) { + const od = unwrap(option); + const shape = (() => { + const dd = od?.def ?? od?._def; + return typeof dd?.shape === 'function' ? dd.shape() : dd?.shape; + })(); + const discDef = (() => { + const s = unwrap(shape?.[unionDef.discriminator]); + return s?.def ?? s?._def; + })(); + const literals = discDef?.values ?? (discDef?.value !== undefined ? [discDef.value] : []); + if (literals && [...literals].includes(value as never)) return option; + } + return undefined; +} + /** * Report every key an authored stack sets — on any item of any metadata * collection — that the item's schema does not declare: every value the parse @@ -197,25 +339,7 @@ export function lintUnknownAuthoringKeys(rawStack: unknown): UnknownAuthoringKey const name = typeof item.name === 'string' && item.name ? item.name : String(i); const basePath = `${collection}.${name}`; lintAuthoredRecordKeys(item, posture.keys, guidance, type, basePath, out); - - // The one nested surface: an object's `fields` record, judged by - // FieldSchema — where #4120 found the worst of the drift. - if (type === 'object' && isPlainRecord(item.fields)) { - const fieldPosture = keyPosture(FieldSchema); - if (fieldPosture && fieldPosture.mode === 'strip' && fieldPosture.keys.size > 0) { - for (const [fieldName, field] of Object.entries(item.fields)) { - if (!isPlainRecord(field)) continue; - lintAuthoredRecordKeys( - field, - fieldPosture.keys, - FIELD_KEY_GUIDANCE, - 'field', - `${basePath}.fields.${fieldName}`, - out, - ); - } - } - } + descend(schema, item, basePath, '', type, guidance, out, 0); } } return out;