diff --git a/.changeset/authoring-key-lint-full-coverage.md b/.changeset/authoring-key-lint-full-coverage.md new file mode 100644 index 0000000000..e611c85feb --- /dev/null +++ b/.changeset/authoring-key-lint-full-coverage.md @@ -0,0 +1,54 @@ +--- +"@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 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** — 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 +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. 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. + +`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..67788fe173 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -209,10 +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; +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 @@ -238,11 +240,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 +273,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..ecadf24256 --- /dev/null +++ b/packages/spec/src/kernel/metadata-authoring-lint.test.ts @@ -0,0 +1,137 @@ +// 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', 'agent', 'dashboard', 'action', 'report', 'hook', 'view']) { + expect(lintableTypes, `expected '${expected}' to be lint-covered`).toContain(expected); + } + }); + + 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); + } + }); + + 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';