From 5e6290fc68c272d3fba2d2d8465a7d28e8115d23 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 16:18:41 +0000 Subject: [PATCH] feat(spec): unknown-key lint descends into nested metadata, not just item roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ledger listed "build the @objectstack/lint unknown-key WARNING layer" as the next step. It had already been built — #3786's lintUnknownAuthoringKeys plus #4167's lintUnknownStackKeys, exported from @objectstack/spec and wired into defineStack(), `os validate` and `os compile` as non-blocking warnings, releases ago. Anyone reading that file for what to do next was being sent to build something that shipped. The ledger now says so. What was actually missing was DEPTH. The walk covered each metadata item's top level plus one hard-coded hop into `object.fields`, leaving 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 dropping keys AND contributing nothing to the evidence base the v18 close-out is supposed 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 it still never double-reports what the parse handles: strict and passthrough stay silent, only strip reports and is descended through. 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 — guessing a branch would invent findings against a shape nobody wrote. `object.fields` keeps reporting as `field` with its curated guidance, now via an explicit override table instead of a special case, so its own nested sites (`fields.*.options[]`) are covered too. The override keys on the path RELATIVE to the item root, and a record's author-chosen keys deliberately do not join that path — otherwise only the first field would resolve as `field`. Audited the result rather than assuming it, since "our own assets are clean" has been wrong before in this campaign: 43 platform objects + 3 apps + 1 dashboard + 3 pages, plus both example apps, report zero unknown keys. No finding this time. Two silences were checked rather than trusted: dashboard widgets are .strict() (the parse owns that), and a component whose `type` matches no discriminator is deliberately not descended into. Verified: spec suite 7172 green; tsc clean; check:docs, liveness, variant-docs and authorable-surface gates green; content/docs/references unchanged (the hook-body lesson from #4207 — a JSDoc in the wrong place rewrites a page). Refs #4001 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0147tNF4Snk7Ry1KGt4a5PY4 --- .changeset/unknown-key-lint-nested-descent.md | 35 ++++ .../2026-07-unknown-key-strictness-ledger.md | 40 ++++- .../kernel/metadata-authoring-lint.test.ts | 76 ++++++++ .../src/kernel/metadata-authoring-lint.ts | 164 +++++++++++++++--- 4 files changed, 290 insertions(+), 25 deletions(-) create mode 100644 .changeset/unknown-key-lint-nested-descent.md 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;