From 47940939caec7cd21733aadce80c25f4071186f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 18:45:45 +0000 Subject: [PATCH] feat(spec)!: field closes, reusing the curated table that already knew which advice would be wrong (#4001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `FieldSchema` carries more silently-stripped keys than any shape in the spec, and it said so about itself for two releases. Two notes on the object — one on `accept`/`maxSize`, one on the five pruned governance keys — both state that a write "parsed clean and the key was silently stripped", and both name it the ADR-0104 failure class. Neither could act, because the object was not strict. The guidance is derived rather than hand-written, and the reason is a bug the first pass shipped. `FIELD_KEY_GUIDANCE` is twenty-odd curated entries for this exact surface, already held honest by a test. A hand-written table beside it is a second copy of the truth, and it proved that immediately: the lint suppresses the suggestion for `pii` BECAUSE `pii` is three edits from `min`, so a bare suggester answers a personally-identifiable-information key with "did you mean `min`?" — confident, wrong, unrelated. The hand-written pass did exactly that. `FieldSchema` now reads the table (`to` → alias, `why` → guidance). The table is unchanged; its consumer moved, which is the intended end state for every entry. The two entries that matter most are the ones that read as protection and were not: `encryptionConfig` and `maskingRule` "implied at-rest protection that never happened". A field declaring either was stored in plaintext exactly as if it had not, silently. The rejection now points at `type: 'secret'` and `requiredPermissions`. A cycle the whole test suite passed through. `suggestions.zod` imports `FieldType` from `field.zod`, so adopting `strictObject` here closed a loop. Under OS_EAGER_SCHEMAS=1 every lazySchema body runs at module init, so the loader hit a half-initialized module and threw before any schema was built. 284 files and 7,239 cases went green over it — tests import lazily, so the cycle never resolved in the order that breaks. Only the eager build caught it. `strictObject` now defers its error map to first use: free, and cycle-proof for every schema after this one instead of making each conversion prove it is not in a loop. Pinned via an alias-table getter that fires exactly when the map is built, and verified to go red when the map is hoisted back to construction. `field` gains its ADR-0010 envelope — it was the ONE type the original probe actually checked, so it was the only gap visible while that probe was green, and it outlasted every gap the probe was hiding. Debt list down to one, from eight. Registered types closed: 22 of 25. Still open: action, dashboard, view. Verified: 284 files / 7240 tests, tsc clean, 8 generated artifacts current, all 10 spec gates green, and CRM/Todo/showcase/platform-objects build. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WnqGjQFQMqd5k81LYV8SCY --- .changeset/field-strict-guidance.md | 25 +++ content/docs/references/data/field.mdx | 7 + .../2026-07-unknown-key-strictness-ledger.md | 44 +++++ packages/spec/authorable-surface.json | 7 + packages/spec/src/data/field.zod.ts | 157 +++++++++++++++++- .../kernel/metadata-authoring-lint.test.ts | 67 ++++---- .../src/kernel/metadata-type-schemas.test.ts | 14 +- .../spec/src/shared/strict-object.test.ts | 50 ++++++ packages/spec/src/shared/strict-object.ts | 56 +++++-- 9 files changed, 370 insertions(+), 57 deletions(-) create mode 100644 .changeset/field-strict-guidance.md diff --git a/.changeset/field-strict-guidance.md b/.changeset/field-strict-guidance.md new file mode 100644 index 0000000000..c1d638eb0c --- /dev/null +++ b/.changeset/field-strict-guidance.md @@ -0,0 +1,25 @@ +--- +'@objectstack/spec': minor +--- + +`field` rejects unknown keys, reusing the curated table that already knew which advice would be wrong. + +`FieldSchema` carries more silently-stripped keys than any other shape in the spec, and it said so about itself for two releases. Two separate notes on the object — one on `accept`/`maxSize`, one on the five pruned governance keys — both state that a write "parsed clean and the key was silently stripped", and both name it the ADR-0104 failure class. Neither could do anything about it, because the object was not `.strict()`. This is the fix those comments wanted. + +**The guidance is derived, not hand-written, and the reason is a bug the first attempt shipped.** `FIELD_KEY_GUIDANCE` in `data/authoring-key-lint.ts` is twenty-odd curated entries for exactly this surface — every one found in the wild, already held honest by a test asserting each `to` names a key `FieldSchema` really declares. A hand-written table beside it would be a second copy of the truth, and it immediately proved why that matters: the lint's table suppresses the suggestion for `pii` **because `pii` is three edits from `min`**. A bare edit-distance suggester answers a personally-identifiable-information key with *"did you mean `min`?"* — confident, wrong, about an unrelated concept. The hand-written pass did exactly that. `FieldSchema` now reads the table directly (`to` → alias, `why` → guidance). + +Note what moved: the table is unchanged and still tested. Its *consumer* changed — the lint no longer reaches `field` now that the parse rejects first, so the same curation that powered a warning now powers a rejection. That is the intended end state for every entry in it. + +Among what it carries, the two that matter most are the ones that read as protection and were not: `encryptionConfig` and `maskingRule` were pruned in 2026-06 because they "implied at-rest protection that never happened". An author who declared either had their field stored in plaintext exactly as if they had not, and heard nothing. The rejection now points at `type: 'secret'` and at `requiredPermissions` (ADR-0066 D3, enforced by the FieldMasker). + +**A cycle the whole test suite passed through.** `shared/suggestions.zod` imports `FieldType` from `data/field.zod`, so adopting `strictObject` here closed a loop — field → strict-object → suggestions → field. Under `OS_EAGER_SCHEMAS=1` (how `build-schemas.ts` runs) every `lazySchema` body executes at module init, so the loader hit a half-initialized module and threw before a single schema was built. **284 test files and 7,239 cases went green over it**; tests import lazily, so the cycle never resolved in the order that breaks. Only the eager build caught it. + +`strictObject` now defers its error map to first use, which costs nothing and makes the helper cycle-proof for every schema after this one rather than making each conversion prove it is not in a loop. The property is pinned via an observable — an alias-table getter that fires exactly when the map is built — and verified to go red when the map is hoisted back to construction. + +`field` also gains its ADR-0010 protection envelope. It was the one type the original envelope probe actually checked (the other 24 took an early return), so it was the only gap anyone could see for as long as that probe was green — and it outlasted every gap the probe was hiding. **The undeclared-envelope debt list is down to one** (`action`), from eight. + +`SelectOptionSchema`, `CurrencyConfigSchema` and the nested shapes under `FieldSchema` (lookup columns, lookup filters, `dependsOn` entries, roll-up summaries) close alongside it. Left open deliberately: `AddressSchema`, `LocationCoordinatesSchema` and `CurrencyValueSchema` are runtime *value* shapes with no consumers at all, two already marked for removal — not authoring surfaces, so strictness is not the question they raise. + +Registered types closed at the top level: **22 of 25**. Still open: `action`, `dashboard`, `view`. + +Authoring impact: a key `FieldSchema` does not declare is now rejected instead of silently discarded — it was already being ignored, so no working field changes. diff --git a/content/docs/references/data/field.mdx b/content/docs/references/data/field.mdx index 480392273c..1f3ab9c051 100644 --- a/content/docs/references/data/field.mdx +++ b/content/docs/references/data/field.mdx @@ -130,6 +130,13 @@ const result = Address.parse(data); | **inlineHelpText** | `string` | optional | Help text displayed below the field in forms | | **autonumberFormat** | `string` | optional | Auto-number format: literal text + `{0000}` counter, `{YYYY}`/`{MM}`/`{DD}`/`{YYYYMMDD}` date tokens (business tz), and `{field_name}` interpolation. Counter resets per rendered prefix (e.g. AD`{YYYYMMDD}``{0000}` resets daily). | | **externalId** | `boolean` | optional | Is external ID for upsert operations | +| **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | +| **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. | +| **_lockSource** | `Enum<'artifact' \| 'package' \| 'env-forced'>` | optional | Layer that set _lock (artifact \| package \| env-forced). | +| **_provenance** | `Enum<'package' \| 'org' \| 'env-forced'>` | optional | Origin of the item (package \| org \| env-forced). | +| **_packageId** | `string` | optional | Owning package machine id. | +| **_packageVersion** | `string` | optional | Owning package version. | +| **_lockDocsUrl** | `string` | optional | Optional documentation link surfaced next to _lockReason. | --- diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 2eeee262df..c3672acf5c 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -286,6 +286,50 @@ dropped at parse, and nothing failed. everyone, and always had been. Closing the shape created the channel, so both got their sentence. +14. **Closing `field` put `strictObject` inside an import cycle, and the whole + test suite passed through it.** `shared/suggestions.zod` imports `FieldType` + from `data/field.zod`, so the moment `field.zod` adopted the helper the graph + closed a loop: field → strict-object → suggestions → field. Under + `OS_EAGER_SCHEMAS=1` — how `build-schemas.ts` runs — every `lazySchema` body + executes at module init, so whichever module the loader entered first saw a + half-initialized partner and threw `Cannot read properties of undefined + (reading 'strictUnknownKeyError')` before a single schema was built. + + **284 test files and 7,239 cases went green over it.** Tests import lazily, + so the cycle never resolved in the order that breaks; only the eager build + hit it. This is finding 9's rule from the other side — there the instrument + reported coverage it did not have, here the instrument was simply the wrong + one, and a green suite meant nothing about the failure mode in question. + + Fixed by deferring the error map to first use, which costs nothing (it is + needed only when a key is rejected) and makes the helper cycle-proof for + every schema after this one, instead of making each new conversion prove it + is not in a loop. The property is now pinned in + `shared/strict-object.test.ts` via an observable — an alias-table getter that + fires exactly when the map is built — and verified to go red when the map is + hoisted back to construction time. + +15. **`field` carried the campaign's richest curated table, in the wrong layer.** + `FIELD_KEY_GUIDANCE` (in `data/authoring-key-lint.ts`) holds twenty-odd + entries for this one surface — every one found in the wild, held honest by a + test that every `to` names a key `FieldSchema` really declares and that no + entry exists for a key still live. + + The first pass at closing `field` hand-wrote a guidance table beside it. That + is a second copy of the truth, and it immediately proved the point: the + lint's table suppresses a suggestion for `pii` **because `pii` is three edits + from `min`**, so a bare edit-distance suggester answers a + personally-identifiable-information key with "did you mean `min`?" — + confident, wrong, about an unrelated concept. The hand-written table did not + know that, and the rejection said exactly that. `FieldSchema` now derives its + aliases and guidance from the table (`to` → alias, `why` → guidance), so the + curation has one home and keeps its existing test. + + Worth noting what moved: the table did not change and is not deprecated — its + *consumer* changed. The lint no longer reaches `field` now that the parse + rejects first, so the same curation that used to power a warning now powers a + rejection. That is the intended end state for every entry in it. + This is the empirical argument for the ratchet: the inference "no metadata in the repo carries unknown keys" was **false three times over**, and only the strict gate could prove it. Note the asymmetry in the two schema gaps — both diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index 170f384210..653d10799f 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -3430,6 +3430,13 @@ "data/ExternalTable:remoteName", "data/ExternalTable:remoteSchema", "data/ExternalTable:rowCountEstimate", + "data/Field:_lock", + "data/Field:_lockDocsUrl", + "data/Field:_lockReason", + "data/Field:_lockSource", + "data/Field:_packageId", + "data/Field:_packageVersion", + "data/Field:_provenance", "data/Field:accept", "data/Field:ackPlaintextMasking", "data/Field:allowCreate", diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index d77935116a..1d2d063609 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -2,9 +2,12 @@ import { z } from 'zod'; import { retiredKey } from '../shared/retired-key'; +import { strictObject } from '../shared/strict-object'; +import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; import { SystemIdentifierSchema } from '../shared/identifiers.zod'; import { ExpressionInputSchema } from '../shared/expression.zod'; import { FilterConditionSchema } from './filter.zod'; +import { FIELD_KEY_GUIDANCE } from './authoring-key-lint'; /** * Field Type Enum @@ -93,7 +96,25 @@ export type FieldType = z.infer; * { label: 'In Progress', value: 'In Progress' } // spaces and uppercase * { label: 'Closed Won', value: 'Closed_Won' } // mixed case */ -export const SelectOptionSchema = lazySchema(() => z.object({ +/** + * Shared history for the authorable shapes in this file (#4001). + * + * This object carries more silently-stripped keys than any other in the spec, + * and it documented the fact about itself for two releases: the notes below on + * `accept`/`maxSize` and on the five pruned governance keys both say a write + * "parsed clean and the key was silently stripped", and both call it the + * ADR-0104 failure class. `FieldSchema` was not `.strict()`, so the only + * available fix each time was a comment. This is the fix those comments wanted. + */ +const FIELD_HISTORY = + 'Until #4001 closed this shape these were dropped silently — the field was still created, ' + + 'minus whatever the key was meant to constrain, protect or compute.'; + +export const SelectOptionSchema = lazySchema(() => strictObject({ + surface: 'this select option', + history: FIELD_HISTORY, + aliases: { text: 'label', name: 'label', title: 'label', key: 'value', id: 'value', isDefault: 'default', selected: 'default', colour: 'color', visible: 'visibleWhen', showWhen: 'visibleWhen' }, +}, { label: z.string().describe('Display label (human-readable, any case allowed)'), value: SystemIdentifierSchema.describe('Stored value (lowercase machine identifier)'), color: z.string().optional().describe('Color code for badges/charts'), @@ -142,7 +163,11 @@ export const LocationCoordinatesSchema = lazySchema(() => z.object({ * - Custom business-specific codes * Stricter validation can be implemented at the application layer based on business requirements. */ -export const CurrencyConfigSchema = lazySchema(() => z.object({ +export const CurrencyConfigSchema = lazySchema(() => strictObject({ + surface: 'this currency configuration', + history: FIELD_HISTORY, + aliases: { decimals: 'precision', scale: 'precision', mode: 'currencyMode', currency: 'defaultCurrency', code: 'defaultCurrency', isoCode: 'defaultCurrency' }, +}, { precision: z.number().int().min(0).max(10).default(2).describe('Decimal precision (default: 2)'), currencyMode: z.enum(['dynamic', 'fixed']).default('dynamic').describe('Currency mode: dynamic (user selectable) or fixed (single currency)'), defaultCurrency: z.string().length(3).default('CNY').describe('Default or fixed currency code (ISO 4217, e.g., USD, CNY, EUR)'), @@ -257,7 +282,89 @@ export function isUniqueDeclared(unique: unknown): boolean { return unique === true || unique === 'global'; } -export const FieldSchema = lazySchema(() => z.object({ +/** + * `FIELD_KEY_GUIDANCE`, re-expressed as `strictObject` options. + * + * That table is the curated list of near-misses and retirements on this exact + * surface — twenty-odd entries, every one found in the wild, and already held + * honest by `authoring-key-lint.test.ts` (every `to` must name a key this schema + * really declares; no entry may exist for a key that is still live). Copying it + * here would have made a second copy of the truth, which is the thing this + * campaign keeps finding rotted. + * + * It also carries knowledge the fallback cannot rederive, and the proof is in + * that file: `pii` is three edits from `min`, so an edit-distance suggester + * offers "did you mean `min`?" — confident, wrong, and about an unrelated + * concept. The lint suppressed that years ago. Closing this shape without + * reusing the table reintroduced it verbatim, which is how this wiring got + * written. + * + * `to` becomes an alias (the concept survives under another key); `why` becomes + * guidance (a retirement with no successor, which also suppresses the rename). + * The table's consumer changes here — the lint no longer reaches `field` now + * that the parse rejects first — but the table itself is unchanged and still + * tested. + */ +function fieldKeyGuidanceAsStrictOptions() { + const aliases: Record = {}; + const guidance: Record = {}; + for (const [key, hint] of Object.entries(FIELD_KEY_GUIDANCE)) { + if (hint.to) aliases[key] = hint.to; + else if (hint.why) guidance[key] = hint.why; + } + return { aliases, guidance }; +} + +export const FieldSchema = lazySchema(() => strictObject({ + surface: 'this field', + history: FIELD_HISTORY, + aliases: { + ...fieldKeyGuidanceAsStrictOptions().aliases, + fieldName: 'name', key: 'name', column: 'name', + dataType: 'type', fieldType: 'type', + title: 'label', displayName: 'label', + help: 'inlineHelpText', helpText: 'inlineHelpText', hint: 'inlineHelpText', tooltip: 'inlineHelpText', + default: 'defaultValue', initialValue: 'defaultValue', + isRequired: 'required', mandatory: 'required', notNull: 'required', + isUnique: 'unique', + values: 'options', choices: 'options', picklist: 'options', selectOptions: 'options', + relatedTo: 'reference', referenceTo: 'reference', target: 'reference', targetObject: 'reference', lookupObject: 'reference', + onDelete: 'deleteBehavior', deleteRule: 'deleteBehavior', cascade: 'deleteBehavior', + formula: 'expression', calculation: 'expression', compute: 'expression', + rollup: 'summaryOperations', rollUp: 'summaryOperations', summary: 'summaryOperations', aggregate: 'summaryOperations', + length: 'maxLength', size: 'maxLength', + decimals: 'scale', decimalPlaces: 'scale', digits: 'precision', + isReadonly: 'readonly', disabled: 'readonly', + isHidden: 'hidden', invisible: 'hidden', + section: 'group', category: 'group', fieldset: 'group', + component: 'widget', renderer: 'widget', control: 'widget', + mimeTypes: 'accept', allowedTypes: 'accept', fileTypes: 'accept', + maxFileSize: 'maxSize', maxBytes: 'maxSize', + trackChanges: 'trackHistory', feedTracked: 'trackHistory', + permissions: 'requiredPermissions', requiredCapabilities: 'requiredPermissions', + }, + guidance: { + ...fieldKeyGuidanceAsStrictOptions().guidance, + // Entries the lint's table does not carry, because the lint never had to: + // these are the removals recorded only as comments on this object, and a + // comment is visible to everyone except the author who got it wrong. + columnName: + '`columnName` was removed in the 16.x line (#2377) — the SQL driver hardcodes the physical ' + + 'column to the field key, so a custom name was ignored. External/federated objects map ' + + 'physical columns with `external.columnMap` (ADR-0062 D7).', + referenceFilters: + '`referenceFilters` (string[]) was removed in the 16.x line (#2377) — the lookup picker only ' + + 'ever read the structured form. Use `lookupFilters: [{ field, operator, value }]`.', + // `notNull` is aliased to `required` above for the common case, but ADR-0113 + // makes the two deliberately distinct and the distinction IS the point, so + // the flattened spelling gets its own sentence rather than a rename. + storageNotNull: + 'physical column constraints live under `storage` — write `storage: { notNull: true }` ' + + '(ADR-0113). `required` is the WRITE contract and deliberately does not imply the column ' + + 'constraint.', + tracked: '`tracked` is not a field key — per-field timeline tracking is `trackHistory: true` (ADR-0052 §5b).', + }, +}, { /** Identity */ name: z.string().regex(/^[a-z_][a-z0-9_]*$/).describe('Machine name (snake_case)').optional(), label: z.string().optional().describe('Human readable label'), @@ -426,19 +533,37 @@ export const FieldSchema = lazySchema(() => z.object({ */ displayField: z.string().optional().describe("Field shown as each candidate's label in the picker/popover (defaults to the referenced object's name/title)."), descriptionField: z.string().optional().describe('Secondary field shown under the label in the quick-select popover.'), - lookupColumns: z.array(z.union([z.string(), z.object({ + lookupColumns: z.array(z.union([z.string(), strictObject({ + surface: 'this lookup column', + history: FIELD_HISTORY, + aliases: { name: 'field', fieldName: 'field', key: 'field', title: 'label', header: 'label', size: 'width' }, + }, { field: z.string(), label: z.string().optional(), width: z.string().optional(), type: z.string().optional(), })])).optional().describe('Explicit columns for the record-picker table; auto-derived from the referenced object when omitted.'), lookupPageSize: z.number().int().positive().optional().describe('Rows per page in the record-picker dialog (default 10).'), - lookupFilters: z.array(z.object({ + lookupFilters: z.array(strictObject({ + surface: 'this lookup filter', + history: FIELD_HISTORY, + aliases: { name: 'field', fieldName: 'field', op: 'operator', comparator: 'operator', condition: 'operator' }, + guidance: { + // A filter that silently does nothing on a PICKER is a filter that offers + // records the author meant to exclude — worth naming the vocabulary. + $in: "the operator vocabulary here is flat, not Mongo-style — write `{ field, operator: 'in', value: [...] }`", + values: 'a multi-value filter puts the array in `value` with `operator: \'in\'`', + }, + }, { field: z.string(), operator: z.enum(['eq', 'ne', 'gt', 'lt', 'gte', 'lte', 'contains', 'in', 'notIn']), value: z.any(), })).optional().describe('Base filters restricting which records are selectable (e.g. only active). The structured, picker-honoured lookup filter.'), - dependsOn: z.array(z.union([z.string(), z.object({ + dependsOn: z.array(z.union([z.string(), strictObject({ + surface: 'this dependsOn entry', + history: FIELD_HISTORY, + aliases: { name: 'field', fieldName: 'field', local: 'field', remote: 'param', remoteField: 'param', key: 'param' }, + }, { field: z.string(), param: z.string().optional(), })])).optional().describe("Declares that this field's available values depend on the value of other field(s) on the same record — the form gates the field until they are set and re-evaluates as they change. For `lookup`/`master_detail` it scopes the candidate query (string = same local/remote key; {field,param} when the remote filter key differs — the {field,param} form is lookup-only). For `select`/`multiselect`/`radio` the actual per-option rule lives in each option's `visibleWhen`; list the referenced fields here (string form) so the option list gates and refreshes with the parent."), @@ -455,7 +580,17 @@ export const FieldSchema = lazySchema(() => z.object({ */ returnType: z.enum(['number', 'text', 'boolean', 'date']).optional() .describe('Inferred value type of a formula field (number/text/boolean/date)'), - summaryOperations: z.object({ + summaryOperations: strictObject({ + surface: 'this roll-up summary', + history: FIELD_HISTORY, + aliases: { + child: 'object', childObject: 'object', from: 'object', source: 'object', + aggregate: 'function', operation: 'function', op: 'function', aggregation: 'function', + summaryField: 'field', targetField: 'field', + foreignKey: 'relationshipField', fk: 'relationshipField', via: 'relationshipField', parentField: 'relationshipField', + where: 'filter', criteria: 'filter', condition: 'filter', + }, + }, { object: z.string().describe('Source child object name for roll-up'), field: z.string().describe('Field on child object to aggregate (ignored for count)'), function: z.enum(['count', 'sum', 'min', 'max', 'avg']).describe('Aggregation function to apply'), @@ -626,6 +761,14 @@ export const FieldSchema = lazySchema(() => z.object({ // driver builds indexes from the object's `indexes[]` array; a field-level // `index: true` created no index. Declare the index in object `indexes[]`. externalId: z.boolean().default(false).describe('Is external ID for upsert operations'), + + // ADR-0010 — runtime protection envelope (internal — set by the loader). + // `field` is a registered metadata type, so `MetadataPlugin`'s loader stamps + // `_packageId` / `_provenance` on it. Undeclared, they were dropped on every + // parse. This was the ONE type the original envelope probe actually checked + // (see `metadata-type-schemas.test.ts` for how the other 24 took an early + // return) — so it has been a known gap longer than any of its siblings. + ...MetadataProtectionFields, }).superRefine((field, ctx) => { // ADR-0113: `storage.notNull` × `requiredWhen` is a contradiction, rejected // at the authoring seam — when the condition is FALSE the write contract diff --git a/packages/spec/src/kernel/metadata-authoring-lint.test.ts b/packages/spec/src/kernel/metadata-authoring-lint.test.ts index 82eb2c1a7f..eb21e71afa 100644 --- a/packages/spec/src/kernel/metadata-authoring-lint.test.ts +++ b/packages/spec/src/kernel/metadata-authoring-lint.test.ts @@ -22,6 +22,7 @@ import { STACK_KEY_GUIDANCE, STACK_RUNTIME_MEMBERS } from '../data/authoring-key import { PLURAL_TO_SINGULAR } from '../shared/metadata-collection.zod'; import { ObjectSchema } from '../data/object.zod'; import { PageSchema } from '../ui/page.zod'; +import { FieldSchema, SelectOptionSchema } from '../data/field.zod'; import { AgentSchema } from '../ai/agent.zod'; import { ObjectStackDefinitionSchema } from '../stack.zod'; import { getMetadataTypeSchema } from './metadata-type-schemas'; @@ -126,16 +127,22 @@ describe('the #4148 behaviours survive the generalization', () => { 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('hands the retired field keys to the parse, guidance and suppression intact', () => { + // `field` closed in #4001 batch 6b, so the lint no longer reaches it and + // `FieldSchema` carries `FIELD_KEY_GUIDANCE` directly. What this test has + // always really been protecting is the SUPPRESSION: `pii` is three edits + // from `min`, so a bare edit-distance suggester answers a + // personally-identifiable-information key with "did you mean `min`?" — + // confident, wrong, about an unrelated concept. Closing the shape without + // reusing the table reintroduced exactly that, which is how the schema came + // to derive from it. Assert the property, wherever it now lives. + expect(lintUnknownAuthoringKeys(stackWith({}, { pii: true }))).toEqual([]); + + const parsed = FieldSchema.safeParse({ type: 'text', pii: true }); + expect(parsed.success).toBe(false); + const message = parsed.success ? '' : parsed.error.issues.map((i) => i.message).join(' '); + expect(message).toContain('pruned in 2026-06'); + expect(message).not.toContain('Did you mean'); }); it('no longer WARNS on a renamed object key — the parse rejects it now', () => { @@ -156,18 +163,20 @@ describe('the #4148 behaviours survive the generalization', () => { }); it('reports across collections in one walk, with per-type surfaces', () => { - // `page` and `agent` used to appear here too. They closed (#4001 batch 6a), - // so the parse rejects those keys and the lint correctly stays quiet — the - // hand-off this whole layer was built to make. `dashboard` keeps a - // second collection in the walk so this still proves per-type surfacing - // rather than degenerating into a single-collection test. + // `page`/`agent` (batch 6a) and `field` (batch 6b) used to appear here too; + // each closed, so the parse rejects and the lint correctly stays quiet — + // the hand-off this whole layer was built to make. Two collections still + // participate, so this keeps proving per-type surfacing rather than + // degenerating into a single-collection test: `object` reports a NESTED + // strip site under a CLOSED root (the #4522 behaviour), and `dashboard` + // reports at its root. const findings = lintUnknownAuthoringKeys({ - objects: [{ name: 'a', label: 'A', fields: { x: { type: 'text', indexed: true } } }], + objects: [{ name: 'a', label: 'A', actions: [{ name: 'act', zzz: 1 }] }], dashboards: [{ name: 'd', label: 'D', zzz: 1 }], }); expect(findings.map((f) => `${f.surface}:${f.path}`).sort()).toEqual([ 'dashboard:dashboards.d.zzz', - 'field:objects.a.fields.x.indexed', + 'object:objects.a.actions.0.zzz', ]); }); @@ -233,21 +242,23 @@ describe('nested descent (#4001 evidence phase)', () => { expect(finding).toMatchObject({ path: 'objects.o1.actions.0.zzz_nested', surface: 'object' }); }); - 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({ + it('hands the field record and its nested array to the parse', () => { + // Was `objects[].fields{}.options[]` reporting as surface `field`, which + // proved the record-override survived a descent into a nested array. + // `field` and `SelectOptionSchema` both closed in #4001 batch 6b, so the + // parse rejects there now and the lint is correctly silent. The + // record-override path itself is still exercised — every remaining + // `object`-surface descent runs through the same code — and the graduation + // is asserted rather than assumed, because a broken walk looks identical. + expect(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', - }); + })).toEqual([]); + + expect(FieldSchema.safeParse({ type: 'text', zzz_nested: 1 }).success).toBe(false); + expect(SelectOptionSchema.safeParse({ label: 'A', value: 'a', zzz_nested: 1 }).success).toBe(false); }); it('stays silent where the nested parse is already strict', () => { diff --git a/packages/spec/src/kernel/metadata-type-schemas.test.ts b/packages/spec/src/kernel/metadata-type-schemas.test.ts index f2ae03416d..84a47c0d50 100644 --- a/packages/spec/src/kernel/metadata-type-schemas.test.ts +++ b/packages/spec/src/kernel/metadata-type-schemas.test.ts @@ -73,10 +73,16 @@ const PROBE: Record = { * The structural walk found 8 of these; the probe it replaced had been hiding 7. * `job` and `book` were closed in the same pass, leaving 6; `validation` came off * when its six union variants were converted, leaving 5; `translation` came off - * when its groups were closed, leaving 4. + * when its groups were closed, leaving 4; `mapping` and `page` when they closed, + * leaving 2; `field` when it closed, leaving 1. + * + * `field` is worth a line of its own: it was the ONE type the original probe + * actually checked (the other 24 took an early return), so it was the only + * envelope gap anyone could see for as long as that probe was green — and it + * outlasted every gap the probe was hiding. */ const UNDECLARED_ENVELOPE = new Set([ - 'action', 'field', + 'action', ]); /** @@ -212,7 +218,7 @@ describe('registered metadata types', () => { * type fails this suite until the list shrinks, so the list cannot outlive the * debt and start exempting types that no longer need exempting. */ -const STILL_STRIP = new Set(['action', 'dashboard', 'field', 'view']); +const STILL_STRIP = new Set(['action', 'dashboard', 'view']); /** The registered schema's own top-level posture: `.strict()` sets a `never` catchall. */ function topLevelPosture(schema: unknown, depth = 0): 'strict' | 'strip' | null { @@ -279,7 +285,7 @@ describe('#4001 — registered-type closure is derived, not tallied', () => { it('reports the campaign number so a reader never has to count', () => { const closed = types.filter((t) => !STILL_STRIP.has(t)); expect(closed.length + STILL_STRIP.size).toBe(types.length); - expect(closed.length).toBe(21); + expect(closed.length).toBe(22); expect(types.length).toBe(25); }); }); diff --git a/packages/spec/src/shared/strict-object.test.ts b/packages/spec/src/shared/strict-object.test.ts index 6b57b09440..086ce75536 100644 --- a/packages/spec/src/shared/strict-object.test.ts +++ b/packages/spec/src/shared/strict-object.test.ts @@ -209,3 +209,53 @@ describe('strictObject × retiredKey — suggestions never point at a dead key', expect(messageFor({ triggerPhrases: ['hi'] })).toContain('was removed in vX'); }); }); + +/** + * The error map must be built lazily, which is what makes this helper safe + * inside an import cycle. + * + * `suggestions.zod` imports `FieldType` from `data/field.zod`, so when + * `field.zod` adopted `strictObject` the graph closed a loop + * (field → strict-object → suggestions → field). Under `OS_EAGER_SCHEMAS=1` + * — how `build-schemas.ts` runs — every `lazySchema` body executes at module + * init, so whichever module the loader entered first saw a half-initialized + * partner and threw before a single schema was built. + * + * The whole test suite passed through that: tests import lazily, so the cycle + * never resolved in the order that breaks. Only the eager build caught it. The + * gate is real (`check:generated` runs `gen:schema`), but it is a whole-package + * build — this pins the actual property, right next to the helper, so a future + * edit that hoists the map back to construction time fails here first. + */ +describe('strictObject — the error map is lazy, so cycles cannot break it', () => { + it('does not build the error map until a key is actually rejected', () => { + // `strictUnknownKeyError` normalizes the alias table via `Object.entries`, + // so a getter on it fires exactly when the map is built — an observable + // proxy for "has construction reached into the imported module yet?". + let aliasesRead = 0; + const aliases = Object.defineProperty({} as Record, 'visibleWhen', { + enumerable: true, + get() { + aliasesRead++; + return 'visible'; + }, + }); + + const Schema = strictObject({ surface: 's', history: 'h', aliases }, { a: z.string() }); + expect(aliasesRead, 'the map was built at construction — cycles will break it').toBe(0); + + // A clean parse never reaches the unknown-key path either. + expect(Schema.safeParse({ a: 'x' }).success).toBe(true); + expect(aliasesRead).toBe(0); + + // First rejection builds it, and the deferral costs nothing in the message. + const result = Schema.safeParse({ a: 'x', visibleWhen: 1 }); + expect(result.success).toBe(false); + expect(aliasesRead).toBe(1); + expect(result.error!.issues[0].message).toContain('`visibleWhen` → `visible`'); + + // Built once, then reused. + Schema.safeParse({ a: 'x', bogus: 1 }); + expect(aliasesRead).toBe(1); + }); +}); diff --git a/packages/spec/src/shared/strict-object.ts b/packages/spec/src/shared/strict-object.ts index 1019080f6a..4bfd7e2ffb 100644 --- a/packages/spec/src/shared/strict-object.ts +++ b/packages/spec/src/shared/strict-object.ts @@ -138,22 +138,42 @@ export interface StrictObjectOptions { */ export function strictObject(options: StrictObjectOptions, shape: T) { const { surface, history, aliases, guidance, extraKeys = [] } = options; - return z - .object(shape, { - error: strictUnknownKeyError({ - surface, - // Declared-but-unwritable keys (tombstones) are excluded — see - // `acceptsNothing`. They stay in the SHAPE, so writing one still raises - // its own prescription; they are only kept out of the candidate list a - // typo gets pointed at. - knownKeys: [ - ...Object.keys(shape).filter((k) => !acceptsNothing(shape[k])), - ...extraKeys, - ], - history, - aliases, - guidance, - }), - }) - .strict(); + + // The error map is built on FIRST USE, not at construction. + // + // `suggestions.zod` imports `FieldType` from `data/field.zod`, so the moment + // `field.zod` started using this helper the import graph closed a loop: + // field → strict-object → suggestions → field. Under `OS_EAGER_SCHEMAS=1` + // (how `build-schemas.ts` runs) every `lazySchema` body executes at module + // init, and whichever module the loader entered first saw a + // half-initialized partner — `strictUnknownKeyError` undefined, and a + // TypeError before a single schema was built. + // + // Worth noting HOW that surfaced: the whole test suite passed. Tests import + // lazily, so the cycle never resolved in the order that breaks. Only the + // eager build hit it — the same lesson this campaign keeps re-learning from + // the other side, that a green check can simply be the wrong instrument. + // + // Deferring costs nothing (the map is needed only when a key is rejected) + // and makes the helper cycle-proof for every schema after this one, rather + // than making each of them prove it is not in a loop. Same shape as the + // deferred map `data/object.zod.ts` already carries for its TDZ problem. + let build: z.core.$ZodErrorMap | undefined; + const error: z.core.$ZodErrorMap = (issue) => + (build ??= strictUnknownKeyError({ + surface, + // Declared-but-unwritable keys (tombstones) are excluded — see + // `acceptsNothing`. They stay in the SHAPE, so writing one still raises + // its own prescription; they are only kept out of the candidate list a + // typo gets pointed at. + knownKeys: [ + ...Object.keys(shape).filter((k) => !acceptsNothing(shape[k])), + ...extraKeys, + ], + history, + aliases, + guidance, + }))(issue); + + return z.object(shape, { error }).strict(); }