From b2ad8d955b3cbd276771cd9f564047e971a93eef Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 14:05:01 +0000 Subject: [PATCH 1/2] feat(spec): the unknown-authoring-key lint covers every metadata collection (#3786) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4148's lint covered object and field — the two surfaces #4120 caught real drift on. A measurement showed those two were a sample, not the population: of the authorable metadata types, only four are .strict() (flow / permission / position / tool, the #4001 Tier-A set); the other eighteen strip an undeclared key exactly the way field did. A misspelled key on a page, an agent or a dashboard got the same parse-clean-value-gone silence, with no lint watching. The walker now covers every metadata collection — 17 today — with its coverage DERIVED, not listed: collections from PLURAL_TO_SINGULAR (the same boundary map the normalizer uses), the judging schema from the canonical type→Zod registry, and lintability read off each schema's own unknown-key posture (strip → lint; strict → skip, the parse is already loud; passthrough → skip, nothing is dropped; unions → member-key union, lintable only with a stripping member and no passthrough member). A third hand-written coverage list would have been the #3786 shape all over again, inside the tool built to end it. Structure: the walker moves to kernel/metadata-authoring-lint.ts, beside the schema registry it reads — covering every type means importing every schema, and the /data subpath it used to live in is consumed by frontend bundles (objectui reads REFERENCE_VALUE_TYPES from it); the comparator, guidance tables and finding shape stay in data/authoring-key-lint.ts, frontend-safe. The root export is unchanged, so defineStack, os validate and os build pick the wider coverage up with no code change of their own. listLintableAuthoringCollections is exported so the coverage test can assert the derivation has not quietly shrunk — `view` is pinned by name there, because it is the union representative: a regression dropping unions would shrink coverage without failing the count. Verified clean against app-todo, app-crm and app-showcase (zero false positives; the first showcase run failed on a stale dist missing #4160's FormSection.pane — the documented check:api-surface caveat, gone on rebuild). Verified by mutation: dropping union handling, inverting the strict filter, and skipping a collection each turn the tests red. spec 7108 tests green, all six generated-artifact gates PASS. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb --- .../authoring-key-lint-full-coverage.md | 51 ++++ .../docs/deployment/validating-metadata.mdx | 27 ++- packages/spec/api-surface.json | 8 +- .../spec/src/data/authoring-key-lint.test.ts | 127 ++++------ packages/spec/src/data/authoring-key-lint.ts | 89 +++---- packages/spec/src/data/index.ts | 7 +- packages/spec/src/index.ts | 12 +- packages/spec/src/kernel/index.ts | 3 + .../kernel/metadata-authoring-lint.test.ts | 132 +++++++++++ .../src/kernel/metadata-authoring-lint.ts | 220 ++++++++++++++++++ packages/spec/src/stack.zod.ts | 6 +- 11 files changed, 513 insertions(+), 169 deletions(-) create mode 100644 .changeset/authoring-key-lint-full-coverage.md create mode 100644 packages/spec/src/kernel/metadata-authoring-lint.test.ts create mode 100644 packages/spec/src/kernel/metadata-authoring-lint.ts diff --git a/.changeset/authoring-key-lint-full-coverage.md b/.changeset/authoring-key-lint-full-coverage.md new file mode 100644 index 0000000000..9036317033 --- /dev/null +++ b/.changeset/authoring-key-lint-full-coverage.md @@ -0,0 +1,51 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec): the unknown-authoring-key lint covers every metadata collection, not just objects (#3786) + +#4148 introduced the lint for `object` and `field` — the two surfaces #4120 +caught real drift on. Those two were a sample, not the population: of the +authorable metadata types, only four are `.strict()` (`flow` / `permission` / +`position` / `tool`, the #4001 Tier-A set). Every other type strips an +undeclared key exactly the way `field` did — an author who misspells a key on a +`page`, an `agent` or a `dashboard` got the same parse-clean-value-gone silence, +with no lint watching. + +`lintUnknownAuthoringKeys` now walks **every metadata collection** — 17 of them +today: object, app, page, dashboard, report, dataset, action, job, agent, skill, +hook, mapping, datasource, view, email_template, doc, book — and its coverage is +**derived, not listed**: which collections exist comes from `PLURAL_TO_SINGULAR` +(the same boundary map the normalizer uses), which schema judges each comes from +the canonical type→Zod registry, and whether linting is even meaningful is read +off each schema's own unknown-key posture. A third hand-written "types the lint +covers" list would have been the #3786 shape all over again, inside the tool +built to end it. + +The posture rules keep the lint from ever disagreeing with the parse: + +- **strip** (zod default) → lint: the parse drops unknown keys silently, and + that silence is what gets reported. +- **strict** → skip: the parse already rejects loudly with the schema's own + tombstone guidance; a second, possibly disagreeing voice helps nobody. +- **passthrough** → skip: unknown keys survive the parse, nothing is dropped. +- **unions** (`view`) → the union of member keys; lintable only when a member + strips and none passes unknowns through. + +`defineStack`, `os validate` and `os build` pick the wider coverage up with no +code change of their own. Verified against the three first-party example apps +(28 pages, 29 flows, 11 actions and friends in the showcase): all clean, zero +false positives. Verified by mutation: dropping union handling, inverting the +strict filter, and skipping a collection each turn the tests red. + +New root/kernel exports: `listLintableAuthoringCollections` (+ +`LintableAuthoringCollection`) — the derived coverage as data, so tooling can +report what the evidence base for the #4001 strict tiers actually spans. + +One import-site change: `lintUnknownAuthoringKeys` moved from the `/data` +subpath to the package root and `/kernel` (`@objectstack/spec` root import is +unchanged and remains the canonical site). Covering every type means importing +every schema, and `/data` is consumed by frontend bundles — the walker moving +out keeps that chunk from inheriting the whole schema universe. If you imported +it from `@objectstack/spec/data`, import from `@objectstack/spec` instead. The +comparator, guidance tables and finding types stay in `/data`, unchanged. diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index 49dceef4d6..36baca1ef7 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -209,11 +209,12 @@ literal (it comes from React state or a variable), a usage carrying a `{...sprea a chart given static `data` (its columns are the author's own), and objects another package defines. -### 9. Object and field keys the schema never declared +### 9. Authoring keys the schema never declared -`ObjectSchema` and `FieldSchema` are deliberately not strict, so a key they do -not declare **parses clean and is dropped** on the way to storage. Nothing fails; -the setting simply is not there. +Most metadata schemas are deliberately not strict — of the authorable types, +only `flow`, `permission`, `position` and `tool` reject unknown keys. On every +other type a key the schema does not declare **parses clean and is dropped** on +the way to storage. Nothing fails; the setting simply is not there. ```ts fields: { @@ -238,11 +239,19 @@ Plain typos get a "did you mean" (`requred` → `required`); a retired key does not, because the nearest declared key by spelling would be noise rather than advice. +The check covers **every metadata collection** — pages, apps, agents, +dashboards, views, actions, and the rest — with its coverage derived from the +same collection map the loader uses, so a newly registered collection is +covered the moment it exists. Types that are already strict are skipped: there +the parse itself rejects loudly, with the schema's own guidance. Because the +lint reads each schema's real unknown-key posture, it can never disagree with +the parse. + This is **advisory** — the stack still loads. Strict rejection is where these -schemas are headed (ADR-0049 enforce-or-remove), but they are the two -most-authored surfaces in the protocol, so the tightening is scheduled on what -this check finds rather than assumed. `defineStack` reports the same findings at -config-load time, so an author sees them without running the CLI at all. +schemas are headed (ADR-0049 enforce-or-remove), but these are the protocol's +most-authored surfaces, so the tightening is scheduled on what this check finds +rather than assumed. `defineStack` reports the same findings at config-load +time, so an author sees them without running the CLI at all. ## The one gate, two entry points @@ -263,7 +272,7 @@ config-load time, so an author sees them without running the CLI at all. | View references — form targets, view-key collisions (#2554) | ✓ | ✓ | | Flow authoring anti-patterns (#1874) | ✓ | ✓ | | Liveness author-warnings | ✓ | ✓ | -| Undeclared object/field keys (#3786) | ✓ | ✓ | +| Undeclared authoring keys, every metadata collection (#3786) | ✓ | ✓ | | Emits `dist/objectstack.json` | — | ✓ | So `os validate` is the fast inner-loop check (no artifact); `os build` is what diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 862cb109c3..46a786dc84 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -52,6 +52,7 @@ "F (const)", "FIELD_KEY_GUIDANCE (const)", "GUEST_POSITION (const)", + "LintableAuthoringCollection (interface)", "MAP_SUPPORTED_FIELDS (const)", "MEMBERSHIP_ROLE_ADMIN (const)", "MEMBERSHIP_ROLE_DELEGATED_ADMIN (const)", @@ -156,6 +157,7 @@ "isAggregatedViewContainer (function)", "isKnownPlatformCapability (function)", "lintUnknownAuthoringKeys (function)", + "listLintableAuthoringCollections (function)", "mapMembershipRole (function)", "normalizeMetadataCollection (function)", "normalizePluginMetadata (function)", @@ -601,10 +603,11 @@ "isKnownFilterToken (function)", "isLegacyApiMethod (function)", "isMultiValueField (function)", + "isPlainRecord (function)", "isTenancyDisabled (function)", "isTitleEligible (function)", "isUniqueDeclared (function)", - "lintUnknownAuthoringKeys (function)", + "lintAuthoredRecordKeys (function)", "missingFieldValues (function)", "nextUtcCalendarDay (function)", "objectForm (const)", @@ -1517,6 +1520,7 @@ "KernelSecurityVulnerabilitySchema (const)", "KernelShutdownEvent (type)", "KernelShutdownEventSchema (const)", + "LintableAuthoringCollection (interface)", "ListPackagesRequest (type)", "ListPackagesRequestSchema (const)", "ListPackagesResponse (type)", @@ -1857,6 +1861,8 @@ "getMetadataTypeSchema (function)", "isConsumerInstallable (function)", "isKnownPlatformCapability (function)", + "lintUnknownAuthoringKeys (function)", + "listLintableAuthoringCollections (function)", "listMetadataCreateSeedTypes (function)", "listMetadataTypeSchemaTypes (function)", "lowerRequiresFeature (function)", diff --git a/packages/spec/src/data/authoring-key-lint.test.ts b/packages/spec/src/data/authoring-key-lint.test.ts index ce486416a7..159ce6edfa 100644 --- a/packages/spec/src/data/authoring-key-lint.test.ts +++ b/packages/spec/src/data/authoring-key-lint.test.ts @@ -1,118 +1,71 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * Tests for the unknown-authoring-key lint (#3786). + * Tests for the unknown-authoring-key CORE (#3786): the comparator and the + * curated guidance tables. The stack walker that applies them across every + * metadata collection is tested in `kernel/metadata-authoring-lint.test.ts`. * - * Two jobs: prove the rule actually fires on the drifts that motivated it, and - * hold the guidance tables to the same non-rotting discipline the #4045 / #4040 - * ledgers use — a `to` that names a key the schema no longer declares is advice - * pointing into a void, which is worse than no advice. + * The guidance tables are held to the #4045 / #4040 non-rotting discipline — a + * `to` that names a key the schema no longer declares is advice pointing into a + * void, which is worse than no advice. */ import { describe, it, expect } from 'vitest'; import { - lintUnknownAuthoringKeys, + lintAuthoredRecordKeys, formatUnknownAuthoringKey, FIELD_KEY_GUIDANCE, OBJECT_KEY_GUIDANCE, + type UnknownAuthoringKeyFinding, } from './authoring-key-lint'; import { ObjectSchema } from './object.zod'; import { FieldSchema } from './field.zod'; const shapeKeys = (s: unknown) => Object.keys((s as { shape: Record }).shape); -/** A minimal well-formed stack with one object and one field. */ -const stackWith = (obj: Record, field: Record) => ({ - objects: [ - { name: 'crm_case', label: 'Case', fields: { owner: { label: 'Owner', type: 'text', ...field } }, ...obj }, - ], -}); - -describe('lintUnknownAuthoringKeys (#3786)', () => { - it('is silent on a clean stack', () => { - expect(lintUnknownAuthoringKeys(stackWith({}, {}))).toEqual([]); - }); - - it('reports a field key the schema does not declare', () => { - const [finding, ...rest] = lintUnknownAuthoringKeys(stackWith({}, { pii: true })); - expect(rest).toEqual([]); - expect(finding).toMatchObject({ - path: 'objects.crm_case.fields.owner.pii', - surface: 'field', - key: 'pii', - }); - // A retired key gets a prescription, not a "did you mean" into a void. - expect(finding.guidance).toBeTruthy(); - expect(finding.suggestion).toBeUndefined(); +function runComparator( + record: Record, + declared: readonly string[], + guidance: Readonly> = {}, +): UnknownAuthoringKeyFinding[] { + const out: UnknownAuthoringKeyFinding[] = []; + lintAuthoredRecordKeys(record, new Set(declared), guidance, 'field', 'p', out); + return out; +} + +describe('lintAuthoredRecordKeys (#3786)', () => { + it('is silent when every key is declared', () => { + expect(runComparator({ a: 1, b: 2 }, ['a', 'b', 'c'])).toEqual([]); }); - it('reports an object key the schema does not declare, with the rename', () => { - const [finding] = lintUnknownAuthoringKeys(stackWith({ capabilities: { trackHistory: true } }, {})); - expect(finding).toMatchObject({ - path: 'objects.crm_case.capabilities', - surface: 'object', - key: 'capabilities', - suggestion: 'enable', - }); + it('reports an undeclared key with an edit-distance suggestion for a typo', () => { + const [f] = runComparator({ requred: true }, ['required', 'label']); + expect(f).toMatchObject({ path: 'p.requred', key: 'requred', suggestion: 'required' }); }); - /** - * The exact set #4120 found rendering in `object.form.ts` while saving - * nothing. If the lint had existed, each of these would have been one warning - * rather than releases of a dead toggle. - */ - const FORM_DRIFT_KEYS = [ - 'indexed', 'immutable', 'filterable', 'placeholder', 'validation', 'errorMessage', - 'audit', 'pii', 'encrypted', 'startingNumber', - 'referenceFilter', 'cascadeDelete', 'formula', 'displayFormat', 'summaryType', 'summaryField', - ] as const; - - it.each(FORM_DRIFT_KEYS)('catches the #4120 drift key %s and says something actionable', (key) => { - const [finding, ...rest] = lintUnknownAuthoringKeys(stackWith({}, { [key]: 'x' })); - expect(finding, `${key} should be reported`).toBeDefined(); - expect(rest).toEqual([]); - expect(finding.key).toBe(key); - // Either a rename target or a retirement reason — never a bare "unknown". - expect( - finding.suggestion ?? finding.guidance, - `${key} needs a rename target or a retirement reason`, - ).toBeTruthy(); - expect(formatUnknownAuthoringKey(finding)).toContain(key); - }); - - it('falls back to edit distance for a plain typo', () => { - const [finding] = lintUnknownAuthoringKeys(stackWith({}, { requred: true })); - expect(finding.suggestion).toBe('required'); + it('a retirement suppresses the edit-distance fallback', () => { + // `pii` is 3 edits from `min` — "did you mean min?" reads as advice while + // being nonsense. A `why` with no `to` must yield guidance and NO suggestion. + const [f] = runComparator({ pii: true }, ['min', 'max'], FIELD_KEY_GUIDANCE); + expect(f.guidance).toBeTruthy(); + expect(f.suggestion).toBeUndefined(); }); - it('ignores the underscore-prefixed packaging channel', () => { - const findings = lintUnknownAuthoringKeys( - stackWith({ _packageId: 'p', _lock: true }, { _provenance: 'x' }), - ); - expect(findings).toEqual([]); + it('a rename wins over edit distance', () => { + const [f] = runComparator({ capabilities: {} }, ['enable', 'label'], OBJECT_KEY_GUIDANCE); + expect(f.suggestion).toBe('enable'); }); - it('reports every offending key, across objects and fields', () => { - const findings = lintUnknownAuthoringKeys({ - objects: [ - { name: 'a', label: 'A', capabilities: {}, fields: { x: { type: 'text', pii: true } } }, - { name: 'b', label: 'B', fields: { y: { type: 'text', indexed: true } } }, - ], - }); - expect(findings.map((f) => f.path).sort()).toEqual([ - 'objects.a.capabilities', - 'objects.a.fields.x.pii', - 'objects.b.fields.y.indexed', - ]); + it('skips the underscore-prefixed packaging channel', () => { + expect(runComparator({ _packageId: 'p', _lock: true, _provenance: 'x' }, ['label'])).toEqual([]); }); - it('survives malformed input rather than throwing', () => { - // The lint runs before the parse, so it is handed whatever the author wrote. - for (const junk of [undefined, null, 42, 'x', {}, { objects: 'nope' }, { objects: [null, 7] }]) { - expect(() => lintUnknownAuthoringKeys(junk)).not.toThrow(); - } - expect(lintUnknownAuthoringKeys({ objects: [{ name: 'a', fields: 'nope' }] })).toEqual([]); + it('formats guidance over suggestion over bare', () => { + const base = { path: 'p.k', surface: 'field', key: 'k' } as UnknownAuthoringKeyFinding; + expect(formatUnknownAuthoringKey({ ...base, guidance: 'gone.' })).toContain('gone.'); + expect(formatUnknownAuthoringKey({ ...base, suggestion: 's' })).toContain("did you mean 's'"); + expect(formatUnknownAuthoringKey(base)).toMatch(/dropped at load\.$/); }); }); diff --git a/packages/spec/src/data/authoring-key-lint.ts b/packages/spec/src/data/authoring-key-lint.ts index f002f80baa..e8676bc455 100644 --- a/packages/spec/src/data/authoring-key-lint.ts +++ b/packages/spec/src/data/authoring-key-lint.ts @@ -29,15 +29,28 @@ * has to run **pre-parse**, on the authored input, which is the same seam the * ADR-0087 D2 conversion notices already use in `defineStack`. * + * ## Why this file is only the CORE + * + * This module carries the comparator, the finding shape and the curated guidance + * tables — deliberately nothing heavier. The stack WALKER + * (`lintUnknownAuthoringKeys`) lives in `kernel/metadata-authoring-lint.ts`, + * because covering every metadata type means importing every schema, and this + * file is re-exported from the `/data` subpath that frontend bundles consume + * (objectui reads `REFERENCE_VALUE_TYPES` and friends from it). Pulling the + * whole schema universe into that chunk to run a build-time lint would be its + * own regression. The kernel already is the everything-imports zone. + * * @see ADR-0049 enforce-or-remove · ADR-0104 · #4001 (the strict tiers) */ import { findClosestMatches } from '../shared/suggestions.zod'; -import { ObjectSchema } from './object.zod'; -import { FieldSchema } from './field.zod'; -/** Which authoring surface a finding came from. */ -export type AuthoringKeySurface = 'object' | 'field'; +/** + * Which authoring surface a finding came from — a metadata type machine name + * (`'object'`, `'page'`, `'agent'`, …) or `'field'` for the one nested surface + * the walker descends into (an object's `fields` record). + */ +export type AuthoringKeySurface = string; /** One unknown key on one authored object or field. */ export interface UnknownAuthoringKeyFinding { @@ -122,40 +135,37 @@ export const OBJECT_KEY_GUIDANCE: Readonly< tableName: { why: 'removed — the table name always equals the object `name`.' }, }); -/** Declared top-level keys of a Zod object schema, read off `.shape`. */ -function declaredKeys(schema: unknown): readonly string[] { - const shape = (schema as { shape?: Record }).shape; - return shape ? Object.keys(shape) : []; -} - -function isPlainRecord(v: unknown): v is Record { +/** A plain object — the only shape an authored metadata item can take. */ +export function isPlainRecord(v: unknown): v is Record { return !!v && typeof v === 'object' && !Array.isArray(v); } /** - * Compare one authored record's keys against a declared key set. + * Compare one authored record's keys against a declared key set, appending a + * finding per unknown key. The single comparator behind every surface the + * walker covers. * * Keys starting with `_` are skipped: they are the packaging/provenance channel * (`_packageId`, `_lock`, `_provenance`) that tooling stamps onto artifacts, and * a stray one is a tooling concern, not an authoring mistake. */ -function lintRecord( +export function lintAuthoredRecordKeys( record: Record, - declared: readonly string[], + declared: ReadonlySet, guidance: Readonly>, surface: AuthoringKeySurface, basePath: string, out: UnknownAuthoringKeyFinding[], ): void { - const known = new Set(declared); + const candidates = [...declared]; for (const key of Object.keys(record)) { - if (known.has(key) || key.startsWith('_')) continue; + if (declared.has(key) || key.startsWith('_')) continue; const hint = guidance[key]; // A retirement (`why`, no `to`) deliberately suppresses the edit-distance // fallback: there IS no successor, and the nearest declared key by spelling // is noise — `pii` is 3 edits from `min`, which would read as real advice. - const suggestion = hint?.to ?? (hint?.why ? undefined : findClosestMatches(key, declared, 3, 1)[0]); + const suggestion = hint?.to ?? (hint?.why ? undefined : findClosestMatches(key, candidates, 3, 1)[0]); out.push({ path: `${basePath}.${key}`, surface, @@ -166,51 +176,6 @@ function lintRecord( } } -/** - * Report every key an authored stack sets on an object or field that the schema - * does not declare — i.e. every value the parse is about to discard silently. - * - * Pure and side-effect free. Runs on the **raw** (normalized but unparsed) - * stack: see the module note for why a parsed stack is too late. - * - * @param rawStack The authored stack, after `normalizeStackInput` and before - * `ObjectStackDefinitionSchema.parse`. - */ -export function lintUnknownAuthoringKeys(rawStack: unknown): UnknownAuthoringKeyFinding[] { - if (!isPlainRecord(rawStack)) return []; - const objects = rawStack.objects; - if (!Array.isArray(objects)) return []; - - const objectKeys = declaredKeys(ObjectSchema); - const fieldKeys = declaredKeys(FieldSchema); - // A schema that failed to expose a shape would make this rule silently pass on - // everything — the failure mode a lint must not have. - if (objectKeys.length === 0 || fieldKeys.length === 0) return []; - - const out: UnknownAuthoringKeyFinding[] = []; - for (const obj of objects) { - if (!isPlainRecord(obj)) continue; - const objName = typeof obj.name === 'string' && obj.name ? obj.name : ''; - const objPath = `objects.${objName}`; - lintRecord(obj, objectKeys, OBJECT_KEY_GUIDANCE, 'object', objPath, out); - - const fields = obj.fields; - if (!isPlainRecord(fields)) continue; - for (const [fieldName, field] of Object.entries(fields)) { - if (!isPlainRecord(field)) continue; - lintRecord( - field, - fieldKeys, - FIELD_KEY_GUIDANCE, - 'field', - `${objPath}.fields.${fieldName}`, - out, - ); - } - } - return out; -} - /** One human-readable line for a finding — shared by `defineStack` and the CLI. */ export function formatUnknownAuthoringKey(f: UnknownAuthoringKeyFinding): string { const head = `${f.path}: '${f.key}' is not a declared ${f.surface} key, so its value is dropped at load`; diff --git a/packages/spec/src/data/index.ts b/packages/spec/src/data/index.ts index 299d03fa28..a62c27a047 100644 --- a/packages/spec/src/data/index.ts +++ b/packages/spec/src/data/index.ts @@ -18,9 +18,10 @@ export * from './object.zod'; // `enable.apiMethods` whitelist into its effective operation set (#3391). export * from './api-derivation'; export * from './field.zod'; -// Pre-parse report of authored keys ObjectSchema/FieldSchema would strip (#3786). -// Neither schema is `.strict()`, so an undeclared key parses clean and is dropped; -// this is what turns that silence into a warning the author can act on. +// The unknown-authoring-key lint's CORE — comparator, finding shape, curated +// guidance tables (#3786). Kept frontend-safe: the stack WALKER that imports +// every schema lives in kernel/metadata-authoring-lint.ts, so this subpath's +// bundles don't inherit the whole schema universe. export * from './authoring-key-lint'; // Field runtime value-shape contract (ADR-0104 D1) export * from './field-value.zod'; diff --git a/packages/spec/src/index.ts b/packages/spec/src/index.ts index 0ca5dd0347..35765bd56d 100644 --- a/packages/spec/src/index.ts +++ b/packages/spec/src/index.ts @@ -91,11 +91,17 @@ export { defineEmailTemplateDefinition } from './system/email-template.zod'; export { defineReport } from './ui/report.zod'; export { defineWebhook } from './automation/webhook.zod'; export { defineObjectExtension } from './data/object.zod'; -// Pre-parse report of authored object/field keys the schemas would strip (#3786). -// Exported from the ROOT, not only from `/data`: `defineStack` (this package) and -// the CLI's `os validate` are the two callers, and both import from the root. +// Pre-parse report of authored keys the metadata schemas would strip (#3786). +// Exported from the ROOT: `defineStack` (this package) and the CLI's +// `os validate`/`os build` are the callers, and all import from the root. The +// walker lives in kernel/ (it imports every schema); the comparator, guidance +// tables and finding shape stay in data/ (frontend-safe). export { lintUnknownAuthoringKeys, + listLintableAuthoringCollections, +} from './kernel/metadata-authoring-lint'; +export type { LintableAuthoringCollection } from './kernel/metadata-authoring-lint'; +export { formatUnknownAuthoringKey, FIELD_KEY_GUIDANCE, OBJECT_KEY_GUIDANCE, diff --git a/packages/spec/src/kernel/index.ts b/packages/spec/src/kernel/index.ts index b5bc8eb401..ac59265dff 100644 --- a/packages/spec/src/kernel/index.ts +++ b/packages/spec/src/kernel/index.ts @@ -28,6 +28,9 @@ export * from './metadata-loader.zod'; export * from './metadata-plugin.zod'; export * from './metadata-protection.zod'; export * from './metadata-type-schemas'; +// Pre-parse unknown-key walker over EVERY metadata collection (#3786). Lives +// here, not in data/, because covering every type means importing every schema. +export * from './metadata-authoring-lint'; export * from './package-artifact.zod'; export * from './package-registry.zod'; export * from './package-upgrade.zod'; diff --git a/packages/spec/src/kernel/metadata-authoring-lint.test.ts b/packages/spec/src/kernel/metadata-authoring-lint.test.ts new file mode 100644 index 0000000000..3387857d3b --- /dev/null +++ b/packages/spec/src/kernel/metadata-authoring-lint.test.ts @@ -0,0 +1,132 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Tests for the generalized unknown-authoring-key walker (#3786). + * + * Three jobs: prove the walk still fires on the drifts that motivated #4148 + * (object/field), prove the NEW coverage really spans the non-strict metadata + * population instead of sampling it, and prove the lint's posture agrees with + * each schema's own — silent where the parse is loud (strict) or lenient + * (passthrough), loud only where the parse is silent (strip). + */ + +import { describe, it, expect } from 'vitest'; + +import { + lintUnknownAuthoringKeys, + listLintableAuthoringCollections, +} from './metadata-authoring-lint'; +import { PLURAL_TO_SINGULAR } from '../shared/metadata-collection.zod'; +import { getMetadataTypeSchema } from './metadata-type-schemas'; + +const lintables = listLintableAuthoringCollections(); +const lintableTypes = new Set(lintables.map((l) => l.type)); + +describe('coverage derivation (#3786 — no third hand-written list)', () => { + it('spans the non-strict population, not a sample', () => { + // #4148 covered object+field: 2 surfaces. The point of this walker is the + // rest. If the derivation regresses to a handful, the "evidence base" for + // the #4001 strict tiers quietly becomes a sample again. + expect(lintables.length).toBeGreaterThanOrEqual(15); + // `view` matters doubly: it is a UNION (container | ViewItem | overlay), so + // its presence pins the union half of the posture logic — a regression that + // silently dropped unions would shrink coverage without failing the count. + for (const expected of ['object', 'page', 'app', 'agent', 'dashboard', 'action', 'report', 'hook', 'view']) { + expect(lintableTypes, `expected '${expected}' to be lint-covered`).toContain(expected); + } + }); + + it('excludes the strict Tier-A types — the parse is already loud there', () => { + for (const strict of ['flow', 'permission', 'position', 'tool']) { + expect(lintableTypes, `'${strict}' is .strict(); the lint must not double-report`).not.toContain(strict); + } + }); + + it('every lintable entry is a real collection with a real schema', () => { + for (const { collection, type } of lintables) { + expect(PLURAL_TO_SINGULAR[collection]).toBe(type); + expect(getMetadataTypeSchema(type), `no schema for '${type}'`).toBeDefined(); + } + }); + + it('every lintable collection actually produces a finding for a bogus key', () => { + // The derivation says these are covered; this proves the walk agrees, per + // collection — the assertion that keeps "covered" from becoming a claim. + for (const { collection, type } of lintables) { + const stack = { [collection]: [{ name: 'probe_item', zzz_bogus_key: 1 }] }; + const findings = lintUnknownAuthoringKeys(stack); + const hit = findings.find((f) => f.key === 'zzz_bogus_key'); + expect(hit, `${collection} (${type}) swallowed an unknown key`).toBeDefined(); + expect(hit!.surface).toBe(type); + expect(hit!.path).toBe(`${collection}.probe_item.zzz_bogus_key`); + } + }); + + it('a strict collection stays silent — the parse owns that failure', () => { + const findings = lintUnknownAuthoringKeys({ + flows: [{ name: 'f1', zzz_bogus_key: 1 }], + }); + expect(findings).toEqual([]); + }); +}); + +describe('the #4148 behaviours survive the generalization', () => { + const stackWith = (obj: Record, field: Record) => ({ + objects: [ + { name: 'crm_case', label: 'Case', fields: { owner: { label: 'Owner', type: 'text', ...field } }, ...obj }, + ], + }); + + it('is silent on a clean stack', () => { + expect(lintUnknownAuthoringKeys(stackWith({}, {}))).toEqual([]); + }); + + it('reports a retired field key with guidance and no suggestion', () => { + const [finding, ...rest] = lintUnknownAuthoringKeys(stackWith({}, { pii: true })); + expect(rest).toEqual([]); + expect(finding).toMatchObject({ + path: 'objects.crm_case.fields.owner.pii', + surface: 'field', + key: 'pii', + }); + expect(finding.guidance).toBeTruthy(); + expect(finding.suggestion).toBeUndefined(); + }); + + it('reports the renamed object key with its rename', () => { + const [finding] = lintUnknownAuthoringKeys(stackWith({ capabilities: { trackHistory: true } }, {})); + expect(finding).toMatchObject({ + path: 'objects.crm_case.capabilities', + surface: 'object', + suggestion: 'enable', + }); + }); + + it('reports across collections in one walk, with per-type surfaces', () => { + const findings = lintUnknownAuthoringKeys({ + objects: [{ name: 'a', label: 'A', fields: { x: { type: 'text', indexed: true } } }], + pages: [{ name: 'p', label: 'P', zzz: 1 }], + agents: [{ name: 'ag', zzz: 1 }], + }); + expect(findings.map((f) => `${f.surface}:${f.path}`).sort()).toEqual([ + 'agent:agents.ag.zzz', + 'field:objects.a.fields.x.indexed', + 'page:pages.p.zzz', + ]); + }); + + it('survives malformed input rather than throwing', () => { + for (const junk of [undefined, null, 42, 'x', {}, { objects: 'nope' }, { pages: [null, 7] }]) { + expect(() => lintUnknownAuthoringKeys(junk)).not.toThrow(); + } + expect(lintUnknownAuthoringKeys({ objects: [{ name: 'a', fields: 'nope' }] })).toEqual([]); + }); + + it('ignores the underscore-prefixed packaging channel everywhere', () => { + const findings = lintUnknownAuthoringKeys({ + objects: [{ name: 'a', _packageId: 'p', fields: { x: { type: 'text', _provenance: 'x' } } }], + pages: [{ name: 'p', _lock: true }], + }); + expect(findings).toEqual([]); + }); +}); diff --git a/packages/spec/src/kernel/metadata-authoring-lint.ts b/packages/spec/src/kernel/metadata-authoring-lint.ts new file mode 100644 index 0000000000..9d1e7cd8f4 --- /dev/null +++ b/packages/spec/src/kernel/metadata-authoring-lint.ts @@ -0,0 +1,220 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * **Every metadata collection, not just objects** (#3786 follow-up to #4148). + * + * #4148 introduced the unknown-authoring-key lint for `object` and `field` — the + * two surfaces #4120 caught real drift on. But those two were a sample, not the + * population: of the 25 authorable metadata types, only four are `.strict()` + * (`flow` / `permission` / `position` / `tool`, the #4001 Tier-A set). The other + * eighteen strip an undeclared key exactly the way `field` did — an author who + * misspells a key on a `page`, an `agent` or a `dashboard` gets the same + * parse-clean-value-gone silence, with no lint watching. + * + * This walker closes that gap by DERIVING its coverage instead of listing it: + * + * - which collections exist → `PLURAL_TO_SINGULAR`, the same boundary map + * `normalizeStackInput` uses — so the lint sees exactly the collections the + * normalizer recognises, and a new collection registered there is covered + * the moment it exists; + * - which schema judges each → `getMetadataTypeSchema`, the canonical + * type→Zod registry; + * - whether linting is even meaningful → read off the schema itself (below). + * + * A third hand-written list of "types the lint covers" would have been the + * #3786 shape all over again, inside the tool built to end it. + * + * ## Why it lives in `kernel/` and not beside its core in `data/` + * + * Covering every type means importing every schema. The comparator and guidance + * tables stay in `data/authoring-key-lint.ts` (light, frontend-safe — the + * `/data` subpath is consumed by browser bundles); this walker sits beside + * `metadata-type-schemas.ts`, which already imports the world. + * + * ## What "lintable" means, per schema + * + * Read from the schema's own unknown-key posture, so the lint can never + * disagree with the parse: + * + * - **strip** (no catchall — zod's default) → LINT. The parse will silently + * drop unknown keys; this is the silence being reported. + * - **strict** (catchall `never`) → SKIP. The parse itself rejects loudly, + * with the schema's own tombstone guidance — a lint warning on top would be + * a second, possibly disagreeing voice. + * - **passthrough** (catchall `any`/`unknown`) → SKIP. Unknown keys survive + * the parse; nothing is dropped, so there is nothing to report. + * - **unions** → the union of the members' declared keys (an author may + * legally write any member's key); lintable only if at least one member + * strips and none passes unknowns through. + */ + +import { + lintAuthoredRecordKeys, + isPlainRecord, + FIELD_KEY_GUIDANCE, + OBJECT_KEY_GUIDANCE, + 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'; + +const EMPTY_GUIDANCE: Readonly> = Object.freeze({}); + +/** + * Curated guidance, per surface. Only `object` and `field` carry curated + * tables today — every entry in them was found in the wild (#4120). Other + * surfaces fall back to the edit-distance suggestion; grow a table here the + * day a real drift is found on one, not before. + */ +const GUIDANCE_BY_SURFACE: Readonly< + Record>> +> = Object.freeze({ + object: OBJECT_KEY_GUIDANCE, + field: FIELD_KEY_GUIDANCE, +}); + +/** How a schema treats a key it does not declare. */ +type UnknownKeyMode = 'strip' | 'strict' | 'passthrough'; + +interface KeyPosture { + /** Every key an author may legally write (union members contribute all). */ + keys: ReadonlySet; + mode: UnknownKeyMode; +} + +/** Peel wrapper nodes until an object or union node is reached. */ +function unwrap(schema: unknown, depth = 0): any { + const s = schema as any; + if (!s || depth > 25) return s; + const d = s.def ?? s._def; + if (!d) return s; + switch (d.type) { + case 'optional': + case 'nullable': + case 'default': + case 'prefault': + case 'readonly': + case 'catch': + case 'nonoptional': + return unwrap(d.innerType, depth + 1); + case 'lazy': + return unwrap(d.getter(), depth + 1); + case 'pipe': + return unwrap(d.in, depth + 1); + default: + return s; + } +} + +/** The key set + unknown-key mode of a schema, or `null` if not key-bearing. */ +function keyPosture(schema: unknown, depth = 0): KeyPosture | null { + if (depth > 6) return null; + const u = unwrap(schema); + const d = u?.def ?? u?._def; + + if (d?.type === 'object') { + const keys = new Set(Object.keys(d.shape ?? u.shape ?? {})); + const catchall = d.catchall; + if (!catchall) return { keys, mode: 'strip' }; + const catchallType = (catchall.def ?? catchall._def)?.type; + return { keys, mode: catchallType === 'never' ? 'strict' : 'passthrough' }; + } + + if (d?.type === 'union' || d?.type === 'discriminated_union') { + const members = (d.options ?? []) + .map((o: unknown) => keyPosture(o, depth + 1)) + .filter((p: KeyPosture | null): p is KeyPosture => p !== null); + if (members.length === 0) return null; + const keys = new Set(members.flatMap((m: KeyPosture) => [...m.keys])); + // A passthrough member means an unknown key may legally SURVIVE the parse + // (we cannot know pre-parse which member will match), so reporting it as + // dropped would be a lie. One is enough to disqualify the whole union. + if (members.some((m: KeyPosture) => m.mode === 'passthrough')) return { keys, mode: 'passthrough' }; + // With only strict members the parse is loud on its own. + if (members.every((m: KeyPosture) => m.mode === 'strict')) return { keys, mode: 'strict' }; + return { keys, mode: 'strip' }; + } + + return null; +} + +/** One collection the walker will lint, and why it qualifies. */ +export interface LintableAuthoringCollection { + /** Stack collection key, e.g. `'pages'`. */ + collection: string; + /** Singular metadata type it holds, e.g. `'page'`. */ + type: string; +} + +/** + * The collections the walker covers, computed from the same sources it lints + * with. Exported so the coverage TEST can assert the derivation has not quietly + * shrunk — and so tooling can report what the evidence base actually spans. + */ +export function listLintableAuthoringCollections(): LintableAuthoringCollection[] { + const out: LintableAuthoringCollection[] = []; + for (const [collection, type] of Object.entries(PLURAL_TO_SINGULAR)) { + const schema = getMetadataTypeSchema(type); + if (!schema) continue; + const posture = keyPosture(schema); + if (posture && posture.mode === 'strip' && posture.keys.size > 0) { + out.push({ collection, type }); + } + } + return out; +} + +/** + * 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 + * is about to discard silently. + * + * Pure and side-effect free. Runs on the **raw** (normalized but unparsed) + * stack; after the parse the unknown keys no longer exist to report. + * + * @param rawStack The authored stack, after `normalizeStackInput` and before + * `ObjectStackDefinitionSchema.parse`. + */ +export function lintUnknownAuthoringKeys(rawStack: unknown): UnknownAuthoringKeyFinding[] { + if (!isPlainRecord(rawStack)) return []; + const out: UnknownAuthoringKeyFinding[] = []; + + for (const [collection, type] of Object.entries(PLURAL_TO_SINGULAR)) { + const items = rawStack[collection]; + if (!Array.isArray(items) || items.length === 0) continue; + const schema = getMetadataTypeSchema(type); + if (!schema) continue; + const posture = keyPosture(schema); + if (!posture || posture.mode !== 'strip' || posture.keys.size === 0) continue; + + const guidance = GUIDANCE_BY_SURFACE[type] ?? EMPTY_GUIDANCE; + for (let i = 0; i < items.length; i++) { + const item = items[i]; + if (!isPlainRecord(item)) continue; + 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, + ); + } + } + } + } + } + return out; +} diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index b670673500..89014a1a9a 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -11,10 +11,8 @@ import { hasPlatformObjectPrefix } from './system/constants/platform-object-name import { objectStackErrorMap, formatZodError } from './shared/error-map.zod'; import { normalizeStackInput, type MetadataCollectionInput, type MapSupportedField } from './shared/metadata-collection.zod'; import type { ConversionNotice } from './conversions/types.js'; -import { - lintUnknownAuthoringKeys, - formatUnknownAuthoringKey, -} from './data/authoring-key-lint'; +import { formatUnknownAuthoringKey } from './data/authoring-key-lint'; +import { lintUnknownAuthoringKeys } from './kernel/metadata-authoring-lint'; // Data Protocol import { ObjectSchema, ObjectExtensionSchema } from './data/object.zod'; From 36fe92699a21160f4d1ea91232dafd19aa8f66c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 14:14:40 +0000 Subject: [PATCH 2/2] =?UTF-8?q?test(spec):=20`app`=20graduated=20to=20stri?= =?UTF-8?q?ct=20mid-review=20(#4165)=20=E2=80=94=20the=20coverage=20gate?= =?UTF-8?q?=20did=20its=20job?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merge origin/main and record the first live graduation: #4165 made the app shell strict while this PR was in CI. The derivation adapted on its own — app fell out of the lintable set because its parse is now loud — and the pinned expectation list is what turned that shrink into a red test instead of a silent one. That is the exact scenario the pin exists for: a human confirms the shrink is a graduation, not a regression, and moves the name from the covered list to the strict list. Changeset and docs updated to stop hardcoding the count and the strict roster — both now describe the rule (the strict bucket grows as tiers graduate) rather than a snapshot of it. 7133 tests green on the merged base; all six gates PASS; the three example apps stay clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb --- .changeset/authoring-key-lint-full-coverage.md | 15 +++++++++------ content/docs/deployment/validating-metadata.mdx | 9 +++++---- .../src/kernel/metadata-authoring-lint.test.ts | 11 ++++++++--- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/.changeset/authoring-key-lint-full-coverage.md b/.changeset/authoring-key-lint-full-coverage.md index 9036317033..e611c85feb 100644 --- a/.changeset/authoring-key-lint-full-coverage.md +++ b/.changeset/authoring-key-lint-full-coverage.md @@ -6,15 +6,16 @@ feat(spec): the unknown-authoring-key lint covers every metadata collection, not #4148 introduced the lint for `object` and `field` — the two surfaces #4120 caught real drift on. Those two were a sample, not the population: of the -authorable metadata types, only four are `.strict()` (`flow` / `permission` / -`position` / `tool`, the #4001 Tier-A set). Every other type strips an +authorable metadata types, only a handful are `.strict()` (`flow` / `permission` +/ `position` / `tool` from #4001 Tier-A, joined mid-review by `app` via #4165). +Every other type strips an undeclared key exactly the way `field` did — an author who misspells a key on a `page`, an `agent` or a `dashboard` got the same parse-clean-value-gone silence, with no lint watching. -`lintUnknownAuthoringKeys` now walks **every metadata collection** — 17 of them -today: object, app, page, dashboard, report, dataset, action, job, agent, skill, -hook, mapping, datasource, view, email_template, doc, book — and its coverage is +`lintUnknownAuthoringKeys` now walks **every metadata collection** — 16 today: +object, page, dashboard, report, dataset, action, job, agent, skill, hook, +mapping, datasource, view, email_template, doc, book — and its coverage is **derived, not listed**: which collections exist comes from `PLURAL_TO_SINGULAR` (the same boundary map the normalizer uses), which schema judges each comes from the canonical type→Zod registry, and whether linting is even meaningful is read @@ -27,7 +28,9 @@ The posture rules keep the lint from ever disagreeing with the parse: - **strip** (zod default) → lint: the parse drops unknown keys silently, and that silence is what gets reported. - **strict** → skip: the parse already rejects loudly with the schema's own - tombstone guidance; a second, possibly disagreeing voice helps nobody. + tombstone guidance; a second, possibly disagreeing voice helps nobody. This + bucket GROWS as #4001 tiers graduate schemas — `app` graduated (#4165) while + this change was in review, and the derivation adapted without an edit. - **passthrough** → skip: unknown keys survive the parse, nothing is dropped. - **unions** (`view`) → the union of member keys; lintable only when a member strips and none passes unknowns through. diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index 36baca1ef7..67788fe173 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -211,10 +211,11 @@ another package defines. ### 9. Authoring keys the schema never declared -Most metadata schemas are deliberately not strict — of the authorable types, -only `flow`, `permission`, `position` and `tool` reject unknown keys. On every -other type a key the schema does not declare **parses clean and is dropped** on -the way to storage. Nothing fails; the setting simply is not there. +Most metadata schemas are deliberately not strict — only the types the +ADR-0049 tier programme has hardened (`flow`, `permission`, `position`, `tool`, +`app`, …) reject unknown keys. On every other type a key the schema does not +declare **parses clean and is dropped** on the way to storage. Nothing fails; +the setting simply is not there. ```ts fields: { diff --git a/packages/spec/src/kernel/metadata-authoring-lint.test.ts b/packages/spec/src/kernel/metadata-authoring-lint.test.ts index 3387857d3b..ecadf24256 100644 --- a/packages/spec/src/kernel/metadata-authoring-lint.test.ts +++ b/packages/spec/src/kernel/metadata-authoring-lint.test.ts @@ -31,13 +31,18 @@ describe('coverage derivation (#3786 — no third hand-written list)', () => { // `view` matters doubly: it is a UNION (container | ViewItem | overlay), so // its presence pins the union half of the posture logic — a regression that // silently dropped unions would shrink coverage without failing the count. - for (const expected of ['object', 'page', 'app', 'agent', 'dashboard', 'action', 'report', 'hook', 'view']) { + for (const expected of ['object', 'page', 'agent', 'dashboard', 'action', 'report', 'hook', 'view']) { expect(lintableTypes, `expected '${expected}' to be lint-covered`).toContain(expected); } }); - it('excludes the strict Tier-A types — the parse is already loud there', () => { - for (const strict of ['flow', 'permission', 'position', 'tool']) { + it('excludes the strict types — the parse is already loud there', () => { + // This list GROWS as the #4001 tier programme hardens schemas, and each + // graduation shrinks the lint's coverage by design — the parse takes over. + // `app` graduated mid-flight (#4165) while this very test was in review: + // the derivation adapted on its own, and the pinned expectation above is + // what forced a human to confirm the shrink was a graduation, not a bug. + for (const strict of ['flow', 'permission', 'position', 'tool', 'app']) { expect(lintableTypes, `'${strict}' is .strict(); the lint must not double-report`).not.toContain(strict); } });