diff --git a/.changeset/object-subblock-strictness-batch20.md b/.changeset/object-subblock-strictness-batch20.md new file mode 100644 index 0000000000..4313eaa506 --- /dev/null +++ b/.changeset/object-subblock-strictness-batch20.md @@ -0,0 +1,70 @@ +--- +'@objectstack/spec': major +--- + +**Object inner blocks now reject unknown keys instead of dropping them (#4001 批 20).** + +Thirteen object shapes nested inside `data/object.zod.ts` were still zod's default +`.strip`: a key the schema did not declare was discarded and the parse still +succeeded. The object's TOP level has rejected unknown keys since #1535/#4519/#4522, +and that asymmetry is what made this the batch worth doing — an author who has *seen* +the root reject a typo has every reason to read a clean parse of +`lifecycle: { maxAge: '30d' }` as acceptance. `object` carries the highest author +volume in the repo. + +Closed, each reached through its real carrier key and probed there (strictness does +not recurse, so a closed parent proves nothing about a nested block): + +- `access` — the ADR-0066 D2 exposure posture. +- `lifecycle` **and all four sub-blocks** — `retention`, `ttl`, `storage`, `archive`. +- `fieldGroups[]` — the ADR-0085 group entry. +- `external` — the ADR-0015 federated binding. +- `userActions`, `systemFields`, `activityMilestones[]`, `publicSharing`. +- `objectExtensions[]` — the extension entry (`defineObjectExtension`). + +**Migration.** Any key now rejected was previously stripped and had no runtime +effect — the error carries the fix. The dominant real-world mistake on this file is +**flattening**, so `lifecycle` points DOWN into the sub-block that owns each key: +`maxAge` → `retention`, `expireAfter`/`field` → `ttl`, `strategy`/`shards`/`unit` → +`storage`, `after`/`to`/`keep` → `archive`. That one matters beyond tidiness: a +flattened `maxAge` leaves `retention` absent, so ADR-0057 §3.5 then rejected the +object as *unbounded* — an error naming the wrong key entirely. + +Other wrong-layer pointers, each anchored to a named sibling contract: +`userActions.sort`/`search`/`filter`/`editInline` point at `ui/view.zod.ts`'s +identically-named block, whose vocabulary is completely disjoint from the object's; +`userActions.clone` points at `enable` (ObjectCapabilities); `systemFields.owner` +points at `ownership` — a key the block's own field doc names but the shape never +declared; `external.allowWrites` names the ADR-0015 double opt-in and mirrors +`datasource.zod.ts`'s own `writable → allowWrites` alias in the opposite direction; +`access.sharingModel` and `publicSharing.sharingModel` point up at the top level and +distinguish link sharing from principal sharing; `fieldGroups[].fields` states the +direction of the membership edge (declared on the FIELD, as `group:`); +`objectExtensions[].actions`/`hooks`/`listViews` say plainly that the merge has no +slot for them and name the route that does. Aliases cover the near-misses distance +cannot reach (`export` → `exportCsv`, `audiences` → `allowedAudiences`, `table` → +`remoteName`, `object` → `extend`, …). `fieldGroups[]`'s three DEPRECATED collapse +aliases stay **accepted** — closing a shape must not turn a documented deprecation +into a rejection. + +**`IndexSchema` is deliberately NOT closed, and that hold is the batch's finding.** +The console ships its own hand-copied JSON-Schema for this shape (objectui +`metadata-admin/EmbeddedItemEditor.tsx`), because `index` is an embedded-only +sub-type the framework publishes no schema for — and that copy has drifted: it +offers **`where`** for the partial-index predicate where the spec declares +**`partial`**. The editor splices its output into `object.indexes[]` and PUTs the +whole object, and `saveMetaItem` keeps the body verbatim while validating it, so +closing this one shape would 422 a control the console itself renders (the #5114 +class, caught this time *before* shipping rather than after). The capability is +already dead in both directions — `driver-sql`'s `syncDeclaredIndexes` reads +`name`/`fields`/`unique` only, so neither spelling reaches any DDL — which is +exactly why the close is gated on both the producer rename and an ADR-0049 answer +for `type`/`partial`: pointing an author at `partial` today would be a guidance +entry claiming more than the platform delivers. + +One caveat shipped knowingly: `systemFields` is a `false | {…}` union, so its +rejection is an `invalid_union` whose own message is the bare *"Invalid input"* — +the #5014 flattening. 批 18's `discriminatedUnion` fix is unavailable here (one arm +is a literal, so there is no discriminant to key on), so the behaviour is pinned +honestly rather than papered over. Every other site in the file is a plain object +and surfaces its prescription directly. diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md b/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md index 118b195815..b9de033020 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md @@ -22,14 +22,14 @@ regenerate. |---|---| | Triaged directories | 5 | | Object sites in them | 484 | -| Still-open (strip) sites | 234 | +| Still-open (strip) sites | 221 | | Files carrying at least one | 36 | Remaining strip sites by class: | Bucket | Sites | |---|---| -| authorable — the ruling's forced scope | 28 | +| authorable — the ruling's forced scope | 15 | | unresolved — needs a per-schema verdict | 33 | | wire / open — out of forced scope | 104 | | no door — no carrier, ADR-0049 territory | 38 | @@ -44,11 +44,11 @@ The `strict` column is the one the campaign schedules against; it counts both th | Dir | Sites | strict | passthrough | catchall | strip | |---|---|---|---|---|---| | `ui/` | 200 | 120 | 5 | 0 | 75 | -| `data/` | 162 | 41 | 1 | 0 | 120 | +| `data/` | 162 | 54 | 1 | 0 | 107 | | `automation/` | 75 | 49 | 0 | 0 | 26 | | `security/` | 20 | 7 | 0 | 0 | 13 | | `studio/` | 27 | 27 | 0 | 0 | 0 | -| **total** | **484** | **244** | **6** | **0** | **234** | +| **total** | **484** | **257** | **6** | **0** | **221** | ## File-level triage — site counts @@ -190,7 +190,7 @@ over it is here. ### `data/` — open -**120 strip of 162**, in 16 file(s). +**107 strip of 162**, in 16 file(s). | File | Strip | Sites | |---|---|---| @@ -207,14 +207,14 @@ over it is here. | `field.zod.ts` | 3 | 11 | | `filter.zod.ts` | 11 | 11 | | `hook.zod.ts` | 4 | 6 | -| `object.zod.ts` | 14 | 20 | +| `object.zod.ts` | 1 | 20 | | `query.zod.ts` | 4 | 5 | | `seed-loader.zod.ts` | 12 | 12 | -| **total** | **120** | **162** | +| **total** | **107** | **162** | | Bucket | Sites | |---|---| -| authorable — the ruling's forced scope | 22 | +| authorable — the ruling's forced scope | 9 | | unresolved — needs a per-schema verdict | 33 | | wire / open — out of forced scope | 65 | | no door — no carrier, ADR-0049 territory | 0 | diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index ae45f34f89..dffbdbdac1 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -640,7 +640,7 @@ sites left to be a verdict about. | File | Class | Note | |---|---|---| -| `object.zod.ts` | authorable | top-level already guarded (#1535); inner blocks partially strict | +| `object.zod.ts` | authorable | top-level already guarded (#1535); inner blocks strict as of 批 20 — except `IndexSchema`, held on a measured console drift (see the `data/` remaining-strip row) | | `data-engine.zod.ts` | wire (p) | engine contract shapes (was 14 — `DataEngineBatchRequestSchema` retired with `IDataEngine.batch?`, #4618) | | `external-lookup.zod.ts` | mixed (p) | authored config + wire results | | `seed-loader.zod.ts` | mixed (p) | seed file shapes are authored; loader state is runtime | @@ -827,7 +827,7 @@ next person to open that file will look. | `keyboard.zod.ts` | **no door** | ⛔ same as `touch` — #4988 | | `offline.zod.ts` | **no door** | ⛔ same as `touch` — #4988 | | `sharing.zod.ts` | **no door** | 批 14: `SharingConfig` was a live door and is **closed**; the 1 left is `EmbedConfigSchema`, which no module in the repo even names (BFS-unreachable, zero parse). **This row shrinks without disappearing** — the first `no door` floor, the same read the `Class` column already has to carry for `flow`'s and `etl`'s wire floors. ADR-0049 verdict: #5015 | -| `app.zod.ts` | verify | `BaseNavItemSchema` — the base the strict discriminated-union members extend. Closing a base that is `.extend()`ed is the #4001 trap that bit `view` (finding 16); confirm the members' strictness is not already covering it before touching | +| `app.zod.ts` | verify | **批 19 ran the check and it came back NEGATIVE — no posture change, and the row's `Class` is held at `verify` deliberately (see below).** `BaseNavItemSchema`. The instruction here was to confirm the members' strictness was not already covering it before touching; it is, and the premise this row carried was wrong twice. (1) **The members do not `.extend()` the base — they spread `...BaseNavItemSchema.shape`.** That is a different mechanism, and the difference is the whole of finding 16: `.extend()` clones INHERIT the base's posture (which is how closing two `view` authoring schemas silently closed the Studio round-trip overlay), while a `...shape` spread copies the per-key schemas into a FRESH `z.object` whose posture is its own. Measured in both directions rather than read off the source, because *"closing the base closes the members"* and *"closing the base is a no-op"* are opposite claims: `strictBase.extend({…})` rejects an unknown key, `z.object({...strictBase.shape})` accepts it, `z.object({...openBase.shape}).strict()` rejects it. (2) **All nine branches already apply their own `.strict()`** with the curated `navItemUnknownKeyError` — asserted per branch through the real door (`AppSchema.navigation`, a `discriminatedUnion` on `type`), with a positive control (every base-contributed key, incl. `requiresService` which no branch declares itself, is ACCEPTED) and a negative control (an undeclared key is REJECTED) in the same run. The base is also module-private and has zero `.parse()` anywhere, so `.strict()` here would be a property of a parse that does not exist. Closing it is therefore a guaranteed no-op, and #4583 is explicit that a no-op closure is not neutral. ⚠️ **The open question is the VOCABULARY, not the measurement** — which is why the `Class` cell was not changed, since it is machine-read and a guess here would be published as a confident subtotal. The two-axis table above resolves carrier-absent + parse-absent to `no door`, whose prescribed follow-up is ADR-0049 retirement — and that prescription is *destructive* here: the vocabulary is fully ALIVE and fully GATED at nine consumers, so retiring the base would delete nine branches' shared keys. `no gate` is wrong for the mirror reason (the gate exists, at the members). `authorable` is the `FormFieldBaseSchema` precedent one row over in `view.zod.ts` — but that base really is `.extend()`ed, so closing it WOULD change behaviour, and calling this one `authorable` invites exactly the later sweep that "finishes the job" on a shape nothing parses. None of the eight enumerated verdicts is honest for a shape that is neither a door nor dead, and adding a ninth changes a machine-read contract — so the decision is the maintainer's (**#5249**). Recorded in three places (the `BaseNavItemSchema` JSDoc + `app-strictness-batch19.test.ts` + this row); the pin includes a guard that fails if any branch ever stops rejecting unknown keys, which is the one change that would make this verdict need re-taking | | `notification.zod.ts` | **no door** | 批 14: `NotificationActionSchema` reclassified, not tightened — no carrier key, BFS-unreachable, zero parse; objectui reads its `.shape` as a vocabulary. ADR-0049 verdict: #5015 | `responsive.zod.ts` left this table at **批 13** (#4001) on reverse-pin evidence @@ -910,6 +910,24 @@ discipline. `app.zod.ts`'s single site is held pending the finding-16 `.extend() check rather than counted as ready; it is counted as authorable all the same, which is what `verify` means in the Class grammar. +**批 19 has now RUN that check, and it came back negative — but the row keeps +`verify`, and the reason is worth stating because it is not the usual one.** The +measurement is finished and unambiguous (the branches spread `...shape` rather +than `.extend()`, so nothing inherits from the base; all nine already close their +own surface; the base is module-private and never parsed — closing it is a +guaranteed no-op, and #4583 says a no-op closure is not neutral). What is NOT +settled is which `Class` word is honest for the result, and that cell is +machine-read: `no door` is what the two-axis table mechanically returns and its +prescribed follow-up — ADR-0049 retirement — would delete a vocabulary that nine +live branches carry; `no gate` inverts the same error; `authorable` publishes it +as forced scope and invites the sweep that closes it. The ledger has been here +once before, at 批 15, and the answer that time was to ADD a verdict rather than +round to the nearest wrong one. Adding a ninth changes a machine-read contract, so +it is the maintainer's call — **#5249** — and `verify` — "held pending a check" — is the one +existing value that publishes no claim about the outcome while the question is +open. It keeps the subtotal exactly where it was, which is the honest number +either way: the site is neither closed nor newly reclassified. + **The overwhelming majority of what is left in this directory is the two no-parse classes** — the exact split is in the counts file, and the direction of travel is what matters here: 批 18 closed 15 real doors, 批 17 measured 29 sites @@ -1003,7 +1021,7 @@ triage row record which one was taken. | File | Class | Batch | |---|---|---| -| `object.zod.ts` | authorable | The registered type's top level is closed (#1535/#4519/#4522); these are inner blocks — `Index`, `ObjectAccessConfig`, `Lifecycle` (+4 sub-blocks), `ObjectFieldGroup`, `ObjectExternalBinding`, `userActions`, `systemFields`, `activityMilestones`, `publicSharing`, `ObjectExtension`. Highest author volume in the repo | +| `object.zod.ts` | authorable | **13 of 14 closed at #4001 批 20; the 14th is HELD, and that hold is the batch's finding.** The registered type's top level was already closed (#1535/#4519/#4522) — these were the inner blocks under it, and the asymmetry is why the file mattered: an author who has SEEN the root reject a typo reads a clean parse of `lifecycle: { maxAge: '30d' }` as acceptance. Closed: `ObjectAccessConfig`, `Lifecycle` + all four sub-blocks (`retention`/`ttl`/`storage`/`archive`), `ObjectFieldGroup`, `ObjectExternalBinding`, `userActions`, `systemFields`, `activityMilestones`, `publicSharing`, `ObjectExtension`. Reachability was measured, not assumed: a BFS from all 24 metadata-type roots plus `ObjectStackSchema` (4810 nodes, 25 roots) resolves every one `direct` **by identity** — none rests on the `derived-clone` bridge #5056 found can mark a dead shape reachable — with `ObjectSchema`/`PageSchema` as positive controls and 批 14's `EmbedConfigSchema` UNREACHABLE (overlap 0.00) **in the same run**. The parse door was probed separately and at each path: `ObjectSchema.safeParse` REJECTS at the top level today and SILENTLY STRIPPED at all thirteen nested paths, so each closure converts a measured silent strip, not a hypothesis. Doors: `saveMetaItem`'s 422 (`metadata-protocol/protocol.ts` — `getMetadataTypeSchema('object')`), `ObjectSchema.create()`, `defineObjectExtension()`, and `registry.validate()` (diagnostic-only by #3903 design). ⚠️ **`IndexSchema` is deliberately NOT closed — the #5114 class, caught before it shipped rather than after.** objectui's console ships its own hand-copied JSON-Schema for this shape (`metadata-admin/EmbeddedItemEditor.tsx` → `FALLBACK_SCHEMAS.index`), because `index` is an embedded-only sub-type the framework publishes no schema for; that copy has drifted and offers **`where`** for the partial-index predicate where this schema declares **`partial`** (and `brin` in an enum that has no `brin`). The editor splices its form output into `object.indexes[]` and PUTs the WHOLE object, and `saveMetaItem` keeps the body verbatim while validating it — so closing this one shape would 422 a control the console itself renders. Unlike #5073's `allowAddTab` the capability is not merely un-gated but already DEAD in both directions: `driver-sql`'s `syncDeclaredIndexes` consumes `name`/`fields`/`unique` only, so neither `where` NOR `partial` reaches any DDL. That is why the hold is not just "fix the producer first": pointing an author at `partial` today would be a guidance entry claiming more than the platform delivers (finding 18), so the close is gated on BOTH the objectui rename (**#5247**) and an ADR-0049 answer for `type`/`partial` (**#5248**). Recorded in three places (the `IndexSchema` JSDoc + `object-strictness-batch20.test.ts` §4 + this row). One caveat shipped knowingly: `systemFields` is a `false | {…}` union, so its rejection is an `invalid_union` whose own message is the bare *"Invalid input"* — the #5014 flattening. 批 18's `discriminatedUnion` fix is unavailable (one arm is a literal, so there is no discriminant), so the behaviour is pinned honestly rather than papered over; every other site in the file is a plain object and surfaces its prescription directly. Curation is anchored to named siblings and each claim is asserted: the dominant failure here is FLATTENING (`maxAge`/`expireAfter`/`shards` written one level too high, where §3.5's own refine then rejects the object for the WRONG key), so `lifecycle` carries wrong-layer pointers DOWN into its four sub-blocks; `userActions` points at `ui/view.zod.ts`'s identically-named block, whose vocabulary is completely disjoint; `systemFields.owner` points at `ownership`, a key the block's own field doc names but the shape never declared; `external.allowWrites` names the ADR-0015 double opt-in and mirrors `datasource.zod.ts`'s own `writable → allowWrites` alias in the opposite direction; `fieldGroups[].fields` states the direction of the membership edge (declared on the FIELD), and its three DEPRECATED collapse aliases stay ACCEPTED — closing a shape must not turn a documented deprecation into a rejection. Highest author volume in the repo | | `data-engine.zod.ts` | wire | **out of scope** — engine request/response contracts | | `external-lookup.zod.ts` | mixed (p) | `ExternalDataSource` + `.authentication` and the `ExternalLookup` tree are authored config; needs the per-schema read the ledger never did | | `seed-loader.zod.ts` | mixed (p) | Split is real: `SeedLoaderConfig` / `SeedIdentity` (+`.user`/`.org`) / `ReferenceResolution` are authored; `SeedLoadResult` / `SeedLoaderResult` (+`.summary`) / `ReferenceResolutionError` / `ObjectDependencyNode` / `ObjectDependencyGraph` / `SeedLoaderRequest` are loader runtime | @@ -1030,6 +1048,44 @@ and out of the ruling's forced scope; that count fell by one when #4721 closed `query.zod.ts`'s `SortNodeSchema`, the one row in this directory where the per-schema read moved a site OUT of `open` rather than confirming it. +**批 20 closed 13 of `object.zod.ts`'s 14 and parked the row at 1**, which makes it +the fourth row in this ledger to shrink without disappearing — after `flow` (批 11), +`etl` (批 12) and `sharing`/`i18n` (批 14/16) — and the first to do so on a +`no gate`-shaped reason inside a directory whose other floors are all wire. The +reverse pin cannot see the difference: it fires when a file reaches ZERO, so it +proves a row's work is *done* and is completely silent about a row whose work is +*deliberately partial*. To the gate, "finished, the last site is held on measured +evidence" and "nobody got to it" are the same row, and only the `Class` column and +this prose separate them. The held site is `IndexSchema`; the evidence is in its +row above, the schema's own JSDoc, and §4 of `object-strictness-batch20.test.ts`, +which pins the strip so the day it changes, it changes deliberately. + +**The #5107 split got its first merge-QUEUE test here, and passed silently**, which +is the outcome worth recording precisely because there is nothing to see. 批 20 was +evicted from the merge queue with `MERGE_CONFLICT` after #5237 (#5073's +`allowAddTab`) landed — two batches touching this ledger from two different +directories. Under the old single-file layout that is exactly the shape that merged +**clean and wrong** eleven times: 批 20's branch had written `authorable = 16`, +#5237's had written its own decrement, the prose rows do not overlap so git merges +them cleanly, and the subtotal — conflicting with nothing — would have landed as one +side's number. What actually happened: the prose merged with no conflict at all, +`merge=os-regen` recorded `counts.md` as pending instead of text-merging it, and +`pre-commit` refused to commit until it was regenerated from the merged tree. The +recomputed answer is **15** (`ui/` 7 → 6 from #5237, `data/` 22 → 9 from 批 20) — a +number neither branch ever wrote down, arrived at without anyone having to notice +that it should be recomputed. That is the whole design: the twelfth instance is the +first that cost nobody anything. + +Worth naming for whoever schedules the next `data/` batch: 批 20 is the first +batch in this campaign to catch a #5114-class defect **before** shipping it rather +than after. #5114 was found on `main`, live, because a wave closed +`ViewFilterRuleSchema` without measuring who else writes that shape. The +difference in method was small and entirely mechanical — grep the sibling repos +for producers of every block in scope, then follow the ones that PUT back through +`saveMetaItem` — and it is cheap enough that it should simply be part of the +per-site discipline rather than a lesson. The producer it found is not even in +this repo, which is the part a spec-only reading cannot reach. + #### `security/` — remaining strip sites | File | Class | Batch | diff --git a/packages/spec/src/data/object-strictness-batch20.test.ts b/packages/spec/src/data/object-strictness-batch20.test.ts new file mode 100644 index 0000000000..76536d91c6 --- /dev/null +++ b/packages/spec/src/data/object-strictness-batch20.test.ts @@ -0,0 +1,370 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #4001 批 20 — `data/object.zod.ts` inner-block strictness. + * + * The object's TOP level has been closed since #1535 (`create()`'s bespoke + * guard) and #4519/#4522 (the schema itself). Its inner blocks were not, and + * strictness does not recurse — so until this batch every one of them silently + * discarded whatever the author wrote that it did not declare, behind a root + * that rejects the same mistake one level up. That asymmetry is the reason this + * file mattered: `object` carries the highest author volume in the repo, and an + * author who has SEEN the top level reject a typo has every reason to read a + * clean parse of `lifecycle: { maxAge: '30d' }` as acceptance. + * + * 13 of the 14 open sites are closed here. The fourteenth — `IndexSchema` — is + * deliberately still open, and section 4 pins that decision with the evidence, + * because a deliberately-open shape explained only in prose is indistinguishable + * from one nobody got to, which is how the next sweep "finishes the job" and + * breaks something. + * + * This file is the third of the three places each verdict is recorded (the + * others: the JSDoc on the shape itself, and the `data/` row in + * `docs/audits/2026-07-unknown-key-strictness-ledger.md`). + * + * What is pinned, and why each needs its own assertion: + * + * 1. THE DOOR. `.strict()` is a property of a PARSE — a strict schema nobody + * parses gates nothing (#4583). Asserted directly, not inferred from posture. + * 2. EVERY closed site at its OWN path, reached through the real carrier key + * rather than standalone: strictness does not recurse, so a closed parent + * proves nothing about a nested block. + * 3. The CURATION. Every alias/guidance entry is a CLAIM about the schema + * (ledger finding 18 — this campaign shipped four false ones), so each is + * anchored here to the sibling contract that makes it true. + * 4. The one shape left OPEN, with the measurement that says why. + */ + +import { describe, it, expect } from 'vitest'; + +import { + ObjectSchema, + ObjectAccessConfigSchema, + LifecycleSchema, + ObjectFieldGroupSchema, + ObjectExternalBindingSchema, + ObjectExtensionSchema, + IndexSchema, + defineObjectExtension, +} from './object.zod'; +import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas'; + +/** Reject `value` through `schema` and return its issues as a searchable string. */ +function reject( + schema: { safeParse: (v: unknown) => { success: boolean; error?: unknown } }, + value: unknown, +): string { + const r = schema.safeParse(value); + expect(r.success, `expected REJECTION, got a successful parse of ${JSON.stringify(value)}`).toBe(false); + return JSON.stringify((r.error as { issues?: unknown })?.issues ?? r.error ?? []); +} + +/** Parse `value` and fail loudly (with the issues) if it does not succeed. */ +function accept( + schema: { safeParse: (v: unknown) => { success: boolean; error?: unknown; data?: unknown } }, + value: unknown, +): unknown { + const r = schema.safeParse(value); + expect(r.success, `expected ACCEPTANCE, got ${JSON.stringify((r.error as { issues?: unknown })?.issues ?? '')}`).toBe(true); + return r.data; +} + +/** A minimal object body that parses — every nested probe is layered onto this. */ +const OBJ = { + name: 'batch20_probe', + label: 'Batch 20 Probe', + fields: { name: { type: 'text', label: 'Name' } }, +} as const; + +/** Reject `patch` merged onto a valid object, THROUGH the object root. */ +const rejectOnObject = (patch: Record): string => reject(ObjectSchema, { ...OBJ, ...patch }); + +// =========================================================================== +// 1. The doors — a parse must exist, or none of the rest means anything +// =========================================================================== +describe('#4001 批 20 — the doors these shapes are reachable through', () => { + it('the `object` metadata type resolves to a registered schema (the save-time 422 door)', () => { + expect(getMetadataTypeSchema('object')).toBeDefined(); + }); + + it('`ObjectSchema.create()` is a real parse door — it throws on a malformed config', () => { + expect(() => + ObjectSchema.create({ name: 'batch20_probe', fields: { name: { type: 'text', label: 'N' } }, notAnObjectKey: 1 } as never), + ).toThrow(); + }); + + it('`defineObjectExtension()` is a real parse door — it throws on a malformed config', () => { + expect(() => defineObjectExtension({ extend: 'contact', notAnExtensionKey: 1 } as never)).toThrow(); + }); + + it('controls parse — these tests fail closed, they do not reject everything', () => { + accept(ObjectSchema, OBJ); + accept(ObjectSchema, { + ...OBJ, + access: { default: 'private' }, + lifecycle: { class: 'audit', retention: { maxAge: '7y' } }, + fieldGroups: [{ key: 'contact_info', label: 'Contact' }], + external: { remoteName: 'remote_contacts', writable: true }, + userActions: { create: false, exportCsv: true }, + systemFields: { tenant: false, audit: true }, + activityMilestones: [{ field: 'stage', value: 'won', summary: 'Deal won: {name}' }], + publicSharing: { enabled: true, allowedAudiences: ['link_only'] }, + }); + accept(ObjectExtensionSchema, { extend: 'contact', fields: {} }); + }); + + it('the root itself was already closed — this batch is the level BELOW it (#1535/#4519/#4522)', () => { + expect(rejectOnObject({ notAnObjectKey: 1 })).toContain('notAnObjectKey'); + }); +}); + +// =========================================================================== +// 2. Every closed site, at its own path, through its real carrier +// =========================================================================== +describe('#4001 批 20 — closed sites reject unknown keys where they live', () => { + it('`access` — the ADR-0066 D2 exposure posture', () => { + accept(ObjectAccessConfigSchema, { default: 'private' }); + expect(reject(ObjectAccessConfigSchema, { default: 'private', notAnAccessKey: 1 })).toContain('notAnAccessKey'); + // Through the carrier, because strictness does not recurse. + expect(rejectOnObject({ access: { default: 'private', notAnAccessKey: 1 } })).toContain('notAnAccessKey'); + }); + + it('`lifecycle` — the ADR-0057 block itself', () => { + expect(rejectOnObject({ lifecycle: { class: 'record', notALifecycleKey: 1 } })).toContain('notALifecycleKey'); + }); + + it.each([ + ['retention', { class: 'audit', retention: { maxAge: '7y', notARetentionKey: 1 } }, 'notARetentionKey'], + ['ttl', { class: 'transient', ttl: { field: 'expires_at', expireAfter: '1d', notATtlKey: 1 } }, 'notATtlKey'], + ['storage', { class: 'telemetry', storage: { strategy: 'rotation', shards: 7, unit: 'day', notAStorageKey: 1 } }, 'notAStorageKey'], + [ + 'archive', + { class: 'audit', retention: { maxAge: '7y' }, archive: { after: '7y', to: 'cold', notAnArchiveKey: 1 } }, + 'notAnArchiveKey', + ], + ])('`lifecycle.%s` — the sub-block, one level below an already-closed parent', (_name, lifecycle, key) => { + expect(rejectOnObject({ lifecycle })).toContain(key); + }); + + it('`fieldGroups[]` — the ADR-0085 group entry, inside an array', () => { + accept(ObjectFieldGroupSchema, { key: 'billing', label: 'Billing', collapse: 'collapsed' }); + expect(rejectOnObject({ fieldGroups: [{ key: 'billing', label: 'Billing', notAGroupKey: 1 }] })).toContain('notAGroupKey'); + }); + + it('`fieldGroups[]` still accepts its three DEPRECATED collapse aliases — they are declared, not tombstoned', () => { + // ADR-0085 kept `defaultExpanded`/`collapsible`/`collapsed` as parse-time + // aliases. Closing the shape must not turn a documented deprecation into a + // rejection; that would be finding 18 (a schema making a false claim). + accept(ObjectFieldGroupSchema, { key: 'g', label: 'G', defaultExpanded: false }); + accept(ObjectFieldGroupSchema, { key: 'g', label: 'G', collapsible: true, collapsed: true }); + }); + + it('`external` — the ADR-0015 federated binding', () => { + accept(ObjectExternalBindingSchema, { remoteName: 'remote_contacts', writable: true }); + expect(rejectOnObject({ external: { remoteName: 'r', notABindingKey: 1 } })).toContain('notABindingKey'); + }); + + it('`userActions` — the CRUD affordance overrides', () => { + expect(rejectOnObject({ userActions: { create: true, notAUserActionKey: 1 } })).toContain('notAUserActionKey'); + }); + + it('`systemFields` — the OBJECT arm of the `false | {…}` union', () => { + // The literal arm must keep working: closing the object arm must not make + // the documented opt-out spelling unparseable. + accept(ObjectSchema, { ...OBJ, systemFields: false }); + accept(ObjectSchema, { ...OBJ, systemFields: { tenant: false } }); + expect(rejectOnObject({ systemFields: { tenant: true, notASystemFieldKey: 1 } })).toContain('notASystemFieldKey'); + }); + + it('⚠️ `systemFields` is the batch\'s ONE #5014 flattening — pinned honestly, including the part that does not reach the author', () => { + // This site is a union (`z.literal(false) | {…}`), so its rejection is an + // `invalid_union` whose OWN message is the bare "Invalid input" — the + // curated prescription is real, but it sits one level down in + // `issue.errors[1][0].message`, and #5014 measured renderers flattening + // exactly that away. Every other site in this batch is a plain object and + // surfaces its message directly (see the sibling assertions above). + // + // 批 18 fixed the equivalent on `submitBehavior` by converting to + // `z.discriminatedUnion`, which is NOT available here: one arm is the + // literal `false`, so there is no discriminant property to key on. Rather + // than invent a mechanism inside a strictness batch, the behaviour is + // recorded — a caveat written down is worth more than one silently carried. + const r = ObjectSchema.safeParse({ ...OBJ, systemFields: { tenant: true, owner: false } }); + expect(r.success).toBe(false); + const issues = (r as { error: { issues: Array> } }).error.issues; + expect(issues[0].code).toBe('invalid_union'); + expect(issues[0].message, 'the top-level message the author is most likely to be shown').toBe('Invalid input'); + // The prescription IS there, nested — so the fix is a rendering/union + // concern (#5014), not a missing curation. + expect(JSON.stringify(issues)).toContain('ownership'); + }); + + it('`activityMilestones[]` — the ADR-0052 §5b.2 milestone entry', () => { + expect( + rejectOnObject({ activityMilestones: [{ field: 'stage', value: 'won', summary: 'Won', notAMilestoneKey: 1 }] }), + ).toContain('notAMilestoneKey'); + }); + + it('`publicSharing` — the share-link policy, where a dropped key fails OPEN', () => { + expect(rejectOnObject({ publicSharing: { enabled: true, notASharingKey: 1 } })).toContain('notASharingKey'); + }); + + it('`ObjectExtensionSchema` — the `objectExtensions[]` entry', () => { + accept(ObjectExtensionSchema, { extend: 'contact', priority: 300 }); + expect(reject(ObjectExtensionSchema, { extend: 'contact', notAnExtensionKey: 1 })).toContain('notAnExtensionKey'); + }); +}); + +// =========================================================================== +// 3. The curation — every alias and guidance entry is a CLAIM (finding 18) +// =========================================================================== +describe('#4001 批 20 — curation is anchored to the sibling contract that makes it true', () => { + it('the bare rejection already names the surface and echoes the key — curation is an upgrade, not a precondition', () => { + const msg = rejectOnObject({ publicSharing: { enabled: true, notASharingKey: 1 } }); + expect(msg).toContain('publicSharing'); + expect(msg).toContain('notASharingKey'); + }); + + describe('wrong-LAYER pointers — the key is spelled correctly, just written one level off', () => { + it('`lifecycle.maxAge` points DOWN into `retention`, where the key really lives', () => { + const msg = rejectOnObject({ lifecycle: { class: 'audit', maxAge: '7y' } }); + expect(msg).toContain('retention'); + // And the pointed-at spelling really does parse — a prescription that + // does not work is worse than none (finding 18). + accept(ObjectSchema, { ...OBJ, lifecycle: { class: 'audit', retention: { maxAge: '7y' } } }); + }); + + it('`lifecycle.expireAfter` / `.field` point DOWN into `ttl`', () => { + expect(rejectOnObject({ lifecycle: { class: 'transient', expireAfter: '1d' } })).toContain('ttl'); + expect(rejectOnObject({ lifecycle: { class: 'transient', field: 'expires_at' } })).toContain('ttl'); + accept(ObjectSchema, { ...OBJ, lifecycle: { class: 'transient', ttl: { field: 'expires_at', expireAfter: '1d' } } }); + }); + + it('`lifecycle.shards`/`unit`/`strategy` point DOWN into `storage`', () => { + for (const patch of [{ shards: 7 }, { unit: 'day' }, { strategy: 'rotation' }]) { + expect(rejectOnObject({ lifecycle: { class: 'telemetry', ...patch } })).toContain('storage'); + } + }); + + it('`lifecycle.ttl.maxAge` points SIDEWAYS at `retention`, and says which axis each measures', () => { + const msg = rejectOnObject({ lifecycle: { class: 'transient', ttl: { field: 'f', expireAfter: '1d', maxAge: '7d' } } }); + expect(msg).toContain('retention'); + }); + + it('`access.sharingModel` points UP — it is a real TOP-LEVEL key, so distance would never find it', () => { + const msg = rejectOnObject({ access: { default: 'private', sharingModel: 'private' } }); + expect(msg).toContain('TOP-LEVEL'); + // The claim: `sharingModel` really is accepted one level up. + accept(ObjectSchema, { ...OBJ, sharingModel: 'private' }); + }); + + it('`publicSharing.sharingModel` distinguishes LINK sharing from PRINCIPAL sharing', () => { + const msg = rejectOnObject({ publicSharing: { enabled: true, sharingModel: 'private' } }); + expect(msg).toContain('PRINCIPAL'); + }); + + it('`systemFields.owner` points at `ownership` — the field doc above the block names a key the shape never had', () => { + const msg = rejectOnObject({ systemFields: { tenant: true, owner: false } }); + expect(msg).toContain('ownership'); + // The claim: `ownership` is the real, enforced lever, one level up. + accept(ObjectSchema, { ...OBJ, ownership: 'none' }); + }); + + it('`external.allowWrites` names the DOUBLE opt-in — the datasource half and the object half', () => { + const msg = rejectOnObject({ external: { remoteName: 'r', allowWrites: true } }); + expect(msg).toContain('writable'); + expect(msg).toContain('datasource'); + accept(ObjectExternalBindingSchema, { remoteName: 'r', writable: true }); + }); + + it('`userActions.sort` names the VIEW block it belongs to — same key NAME, disjoint vocabulary', () => { + // `ui/view.zod.ts`'s UserActionsConfigSchema declares sort/search/filter/ + // refresh/rowHeight/addRecordForm/editInline/buttons; the object block + // declares create/import/edit/delete/exportCsv. Nothing overlaps, which + // is exactly why an author who learned one writes it on the other. + const msg = rejectOnObject({ userActions: { sort: false } }); + expect(msg).toContain('VIEW'); + }); + + it('`userActions.clone` points at the `enable` capability block, which really does declare it', () => { + const msg = rejectOnObject({ userActions: { clone: false } }); + expect(msg).toContain('enable'); + accept(ObjectSchema, { ...OBJ, enable: { clone: false } }); + }); + + it('`fieldGroups[].fields` states the direction of the membership edge', () => { + const msg = rejectOnObject({ fieldGroups: [{ key: 'g', label: 'G', fields: ['name'] }] }); + expect(msg).toContain('group'); + // The claim: membership really is declared on the FIELD. + accept(ObjectSchema, { + ...OBJ, + fields: { name: { type: 'text', label: 'Name', group: 'g' } }, + fieldGroups: [{ key: 'g', label: 'G' }], + }); + }); + + it('`objectExtensions[].actions` says the merge has no slot for it, and names the route that does', () => { + // `objectql/src/engine.ts` copies exactly extend/fields/label/pluralLabel/ + // description/validations/indexes/priority onto the extension def, so + // `actions` is not "unsupported yet" — there is nowhere for it to arrive. + const msg = reject(ObjectExtensionSchema, { extend: 'contact', actions: [] }); + expect(msg).toContain('objectName'); + }); + }); + + describe('tombstones — a retired key\'s rejection carries its upgrade', () => { + it('`fieldGroups[].visibleWhen` says it was REMOVED, not misspelled', () => { + const msg = rejectOnObject({ fieldGroups: [{ key: 'g', label: 'G', visibleWhen: 'true' }] }); + expect(msg).toContain('ADR-0085'); + }); + }); + + describe('aliases — semantic near-misses edit distance cannot reach', () => { + it.each([ + ['userActions.export → exportCsv', { userActions: { export: true } }, 'exportCsv'], + ['publicSharing.audiences → allowedAudiences', { publicSharing: { enabled: true, audiences: [] } }, 'allowedAudiences'], + ['publicSharing.redact → redactFields', { publicSharing: { enabled: true, redact: [] } }, 'redactFields'], + ['external.table → remoteName', { external: { table: 't' } }, 'remoteName'], + ['fieldGroups[].title → label', { fieldGroups: [{ key: 'g', label: 'G', title: 'T' }] }, 'label'], + ])('%s', (_name, patch, expected) => { + expect(rejectOnObject(patch)).toContain(expected); + }); + + it('`objectExtensions[].object → extend` — `object` is real on a NEIGHBOURING surface, which is what makes it an alias', () => { + expect(reject(ObjectExtensionSchema, { extend: 'contact', object: 'contact' })).toContain('extend'); + }); + }); +}); + +// =========================================================================== +// 4. The one shape left OPEN — and the measurement that says why +// =========================================================================== +describe('#4001 批 20 — `IndexSchema` is deliberately NOT closed', () => { + it('still strips, on purpose — do not "finish the job" without reading the JSDoc', () => { + const parsed = accept(ObjectSchema, { ...OBJ, indexes: [{ fields: ['name'], where: "status = 'open'" }] }) as { + indexes: Array>; + }; + // The key rides in and is dropped. That IS the defect — it is pinned here + // so the day it changes, it changes deliberately. + expect(parsed.indexes[0].where).toBeUndefined(); + expect(parsed.indexes[0].fields).toEqual(['name']); + }); + + it('the drift is real: the console offers `where`, this schema declares `partial`', () => { + // objectui `metadata-admin/EmbeddedItemEditor.tsx` → FALLBACK_SCHEMAS.index + // publishes `where` for the partial-index predicate. The spec's key is + // `partial`. The editor splices its output into `object.indexes[]` and PUTs + // the whole object, and `saveMetaItem` keeps the body verbatim — so closing + // this shape 422s a control the console itself renders (the #5114 class). + accept(IndexSchema, { fields: ['name'], partial: "status = 'open'" }); + const parsedWhere = accept(IndexSchema, { fields: ['name'], where: "status = 'open'" }) as Record; + expect(parsedWhere.where, 'the console spelling is silently discarded today').toBeUndefined(); + }); + + it('its SIBLINGS in the same file are closed — this is a held site, not an unwalked file', () => { + // The distinction the ledger's Class column exists to carry: a row parked + // at a deliberate floor looks exactly like a row nobody finished. + expect(rejectOnObject({ lifecycle: { class: 'record', notALifecycleKey: 1 } })).toContain('notALifecycleKey'); + expect(rejectOnObject({ fieldGroups: [{ key: 'g', label: 'G', notAGroupKey: 1 }] })).toContain('notAGroupKey'); + }); +}); diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index 4171ff9ba8..5465b6cc2c 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -13,6 +13,7 @@ import { ExpressionInputSchema, TemplateExpressionInputSchema, type Expression, import { lazySchema } from '../shared/lazy-schema'; import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; import { strictUnknownKeyError } from '../shared/suggestions.zod'; +import { strictObject } from '../shared/strict-object'; import { ProtectionSchema } from '../shared/protection.zod'; export const ApiMethod = z.enum([ 'get', 'list', // Read @@ -274,6 +275,36 @@ export const ObjectCapabilities = z.object({ * type: "btree", * unique: true * } + * + * ## ⛔ Deliberately still `.strip()` — #4001 批 20 held this one site + * + * Every other inner block of this file was closed in that batch. This one was + * NOT, and the reason is a measured live defect rather than an unfinished + * to-do, so please do not "finish the job" by adding `strictObject` here. + * + * The console's embedded index editor + * (`objectui` → `metadata-admin/EmbeddedItemEditor.tsx`, `FALLBACK_SCHEMAS.index`) + * ships its own hand-copied JSON-Schema for this shape — the framework does not + * publish one, because `index` is an embedded-only sub-type with no metadata + * type of its own. That copy has drifted: + * + * - it offers **`where`** for the partial-index predicate; this schema + * declares **`partial`**; + * - it offers **`brin`** in the algorithm enum, which this schema does not. + * + * The editor splices its form output into `object.indexes[]` and PUTs the whole + * object, and `saveMetaItem` keeps the body verbatim while validating it. So + * today an admin who fills in "Partial-index predicate" gets a clean save and a + * key that this schema silently drops on every later parse — and closing this + * shape would turn that same click into a 422 on a control the console itself + * renders (the #5114 class). + * + * Closing it is still the right end state, but it is gated on the producer + * being fixed first (contract-first: the drift is in the copy, not here), and + * on an ADR-0049 answer for `type`/`partial` (#5247 / #5248) — neither is read by any driver + * (`syncDeclaredIndexes` in `driver-sql` consumes `name`/`fields`/`unique` + * only), so pointing an author at `partial` today would be a guidance entry + * that claims more than the platform delivers (ledger finding 18). */ export const IndexSchema = lazySchema(() => z.object({ name: z.string().optional().describe('Index name (auto-generated if not provided)'), @@ -394,7 +425,33 @@ export function isTenancyDisabled(schema: unknown): boolean { * Pair with the object's `requiredPermissions` (D3) to additionally gate access * on holding a capability. */ -export const ObjectAccessConfigSchema = lazySchema(() => z.object({ +export const ObjectAccessConfigSchema = lazySchema(() => strictObject({ + surface: "this object's `access` block", + history: + 'Until #4001 these were dropped silently — the block still parsed, so an object the ' + + 'author declared `private` shipped `public`: covered by every `\'*\'` wildcard grant, ' + + 'with no signal that the posture had been discarded.', + aliases: { + visibility: 'default', + posture: 'default', + defaultAccess: 'default', + }, + guidance: { + // Wrong-layer, not typos: both are real TOP-LEVEL object keys, and both + // are the neighbouring half of the same access story — so edit distance + // would never reach them and a bare rejection would read as "no such + // concept" when the concept exists one level up. + sharingModel: + '`sharingModel` is the object-wide default record visibility (OWD) and is a ' + + 'TOP-LEVEL object key, not an `access` key — write it beside `access`, not inside ' + + 'it. `access.default` decides wildcard-GRANT coverage; `sharingModel` decides ' + + 'record visibility between users (ADR-0090).', + requiredPermissions: + '`requiredPermissions` is a TOP-LEVEL object key (ADR-0066 D3) — it gates access on ' + + 'the caller HOLDING a capability, which is a different axis from `access.default` ' + + '(whether a wildcard grant covers this object at all). Pair them, side by side.', + }, +}, { default: z.enum(['public', 'private']).default('public') .describe('Default exposure posture: public (covered by wildcard grants) | private (needs explicit grant; exempt from wildcard RLS).'), })); @@ -465,11 +522,70 @@ export const LIFECYCLE_DURATION_REGEX = /^\d+(h|d|w|y)$/; const lifecycleDuration = (what: string) => z.string().regex(LIFECYCLE_DURATION_REGEX, `${what} must be a duration literal like '6h', '14d', '12w' or '7y'`); -export const LifecycleSchema = lazySchema(() => z.object({ +export const LifecycleSchema = lazySchema(() => strictObject({ + surface: "this object's `lifecycle` block", + history: + 'Until #4001 these were dropped silently — the block still parsed, so a bounding ' + + 'policy written one level too high left the object with NO policy at all. ADR-0057 ' + + "§3.5's own refine then passed, because the key it looks for was never there.", + aliases: { rotation: 'storage' }, + guidance: { + // The dominant failure on this block is FLATTENING: every one of these is a + // real key of a real sub-block, written one level too high. Edit distance + // cannot help — the key is spelled correctly, it is just in the wrong + // object — and the §3.5 refine makes the mistake worse than inert: a + // flattened `maxAge` leaves `retention` absent, so a non-`record` class is + // then rejected as unbounded and the author is told about the wrong key. + maxAge: + '`maxAge` belongs to the retention block, one level down: ' + + "`retention: { maxAge: '30d' }`. Written here it is not read, and a non-`record` " + + 'class with no `retention`/`ttl`/`storage` is rejected as unbounded (ADR-0057 §3.5).', + expireAfter: + '`expireAfter` belongs to the TTL block, one level down: ' + + "`ttl: { field: 'expires_at', expireAfter: '1d' }`.", + field: + '`field` belongs to the TTL block, one level down — it names the timestamp the TTL ' + + "is measured from: `ttl: { field: 'expires_at', expireAfter: '1d' }`.", + after: + '`after` belongs to the archive block, one level down: ' + + "`archive: { after: '7y', to: 'cold_store' }` — and ADR-0057 requires it to EQUAL " + + '`retention.maxAge`.', + to: + '`to` belongs to the archive block, one level down — it names the cold-storage ' + + "datasource: `archive: { after: '7y', to: 'cold_store' }`.", + keep: + '`keep` belongs to the archive block, one level down — it is how long COLD rows are ' + + 'kept. The HOT window is `retention.maxAge`.', + strategy: + '`strategy` belongs to the storage block, one level down: ' + + "`storage: { strategy: 'rotation', shards: 7, unit: 'day' }`.", + shards: + '`shards` belongs to the storage block, one level down: ' + + "`storage: { strategy: 'rotation', shards: 7, unit: 'day' }`.", + unit: + '`unit` belongs to the storage block, one level down: ' + + "`storage: { strategy: 'rotation', shards: 7, unit: 'day' }`.", + }, +}, { class: LifecycleClassSchema.describe( 'Persistence contract: record (business truth, permanent) | audit (compliance ledger) | telemetry (high-freq log) | transient (ephemeral state) | event (bus messages).', ), - retention: z.object({ + retention: strictObject({ + surface: "this object's `lifecycle.retention` block", + history: + 'Until #4001 these were dropped silently — the retention window still parsed, so a ' + + 'row filter written under the wrong key reaped rows the author had meant to exempt.', + aliases: { filter: 'onlyWhen', where: 'onlyWhen', when: 'onlyWhen', age: 'maxAge' }, + guidance: { + expireAfter: + '`expireAfter` is a `ttl` key, not a retention key. Retention reaps by AGE from ' + + '`created_at` (`maxAge`); TTL expires each row relative to a timestamp field you ' + + 'name (`ttl.field`). Pick the one that matches how the rows die.', + keep: + '`keep` is an `archive` key — how long COLD rows survive. The hot window is ' + + '`retention.maxAge`.', + }, + }, { maxAge: lifecycleDuration('retention.maxAge').describe('Rows older than this (by created_at) are deleted by the Reaper — or archived first when `archive` is set.'), onlyWhen: z.record( z.string(), @@ -483,16 +599,52 @@ export const LifecycleSchema = lazySchema(() => z.object({ 'Row filter the retention applies to — per-field equality or {$in: [...]} (e.g. { status: { $in: ["completed", "failed"] } }). Rows OUTSIDE the filter are retained regardless of age: for tables that interleave live workflow state with terminal history (sys_automation_run). Incompatible with rotation storage and archive, which act on whole shards / age alone.', ), }).optional().describe('Age-based retention window enforced by the LifecycleService Reaper.'), - ttl: z.object({ + ttl: strictObject({ + surface: "this object's `lifecycle.ttl` block", + history: + 'Until #4001 these were dropped silently — the TTL block still parsed, so rows the ' + + 'author expected to auto-expire lived forever.', + aliases: { expiresAfter: 'expireAfter', after: 'expireAfter', timestampField: 'field', on: 'field' }, + guidance: { + maxAge: + '`maxAge` is a `retention` key, not a TTL key. TTL measures from the timestamp ' + + 'field named in `ttl.field`; retention measures AGE from `created_at`. For ' + + "age-based reaping write `retention: { maxAge: '30d' }` instead.", + }, + }, { field: z.string().describe('Timestamp field the TTL is measured from (e.g. created_at, expires_at).'), expireAfter: lifecycleDuration('ttl.expireAfter').describe('Rows expire this long after `field` and are deleted by the Reaper.'), }).optional().describe('Per-row TTL auto-expiry (transient/event classes).'), - storage: z.object({ + storage: strictObject({ + surface: "this object's `lifecycle.storage` block", + history: + 'Until #4001 these were dropped silently — the rotation block still parsed, so a ' + + 'telemetry table declared as rotating kept every shard it ever cut.', + aliases: { count: 'shards', interval: 'unit', period: 'unit', granularity: 'unit' }, + guidance: { + maxAge: + '`maxAge` is a `retention` key. Rotation does not reap by age — it retains ' + + '`shards` × `unit` of history and DROPs the oldest shard whole. Set the window ' + + 'with `shards`/`unit`, or use `retention` instead of rotation.', + }, + }, { strategy: z.literal('rotation').describe('Time-shard the table; rotate by DROPping the oldest shard (O(1) reclaim).'), shards: z.number().int().min(2).describe('Number of shards retained; total window = shards × unit.'), unit: z.enum(['day', 'week', 'month']).describe('Time width of one shard.'), }).optional().describe('Physical storage strategy for high-frequency telemetry (LifecycleService Rotator).'), - archive: z.object({ + archive: strictObject({ + surface: "this object's `lifecycle.archive` block", + history: + 'Until #4001 these were dropped silently — the archive block still parsed, so audit ' + + 'rows were reaped hot with no cold copy ever written.', + aliases: { datasource: 'to', target: 'to', destination: 'to', retain: 'keep' }, + guidance: { + maxAge: + '`maxAge` is a `retention` key. The archive boundary is `archive.after`, and ' + + 'ADR-0057 requires the two to be EQUAL — the hot window ends exactly where the ' + + 'archive begins, so declare `retention.maxAge` and `archive.after` with the same value.', + }, + }, { after: lifecycleDuration('archive.after').describe('Rows older than this are copied to the archive datasource before hot deletion.'), to: z.string().describe('Target datasource name for cold storage. When it is not registered, the Archiver skips (audit rows are then retained, never dropped unarchived).'), keep: lifecycleDuration('archive.keep').optional().describe('How long archived rows are kept in cold storage (undefined = forever).'), @@ -579,7 +731,46 @@ export const LifecycleSchema = lazySchema(() => z.object({ * ] * ``` */ -export const ObjectFieldGroupSchema = lazySchema(() => z.object({ +export const ObjectFieldGroupSchema = lazySchema(() => strictObject({ + surface: 'this field group', + history: + 'Until #4001 these were dropped silently — the group still parsed AND still rendered, ' + + 'which is the worst version of the failure: the section appeared, so the author had ' + + 'every reason to believe the setting they wrote had been applied.', + aliases: { + title: 'label', + name: 'key', + id: 'key', + help: 'description', + helpText: 'description', + }, + guidance: { + // Membership is declared the OTHER WAY ROUND, and this is the single most + // likely thing an author reaches for here — a group that lists its fields + // is what every other layout system in this space looks like. + fields: + '`fields` does not live on a group — membership is declared on the FIELD, pointing ' + + "back: `fields: { email: { type: 'email', group: 'contact_info' } }`. The group " + + 'declares only its `key`, `label` and presentation; `deriveFieldGroupLayout` ' + + '(ADR-0085 §5) does the assembly.', + order: + '`order` is not a group key — ARRAY ORDER is display order. `fieldGroups: [...]` ' + + 'renders top to bottom, so move the entry rather than numbering it.', + expanded: + '`expanded` is not a group key — use the `collapse` enum: ' + + "`collapse: 'expanded'` (collapsible, starts open), `'collapsed'` (starts closed), " + + "or `'none'` (always open, no toggle).", + // Tombstone: this key really did exist here, briefly, reading nothing. + visibleWhen: + '`visibleWhen` was REMOVED from field groups (ADR-0085 / ADR-0049 enforce-or-remove) ' + + '— it existed here briefly with no consumer on any surface, so it never gated ' + + 'anything. Gate the individual fields, or assign a Page for per-surface control.', + visibleOn: + '`visibleOn` is not a field-group key — group-level visibility predicates were ' + + 'removed under ADR-0085 / ADR-0049 (nothing evaluated them). Gate the individual ' + + 'fields, or assign a Page.', + }, +}, { /** Group key — referenced by `Field.group` to assign a field to this group. Must be snake_case. */ key: z.string().regex(/^[a-z_][a-z0-9_]*$/, { message: 'Field group key must be lowercase snake_case (e.g., "contact_info", "billing", "system")', @@ -657,7 +848,38 @@ export type ObjectFieldGroupInput = z.input; * has `schemaMode !== 'managed'`") is enforced at metadata-load time, not * in this schema, because the datasource may live in another artefact. */ -export const ObjectExternalBindingSchema = z.object({ +export const ObjectExternalBindingSchema = strictObject({ + surface: "this object's `external` binding (ADR-0015)", + history: + 'Until #4001 these were dropped silently — the binding still parsed, so a federated ' + + 'object bound to the wrong remote table, or shipped read-only after the author had ' + + 'explicitly asked for writes.', + aliases: { + table: 'remoteName', + tableName: 'remoteName', + remoteTable: 'remoteName', + schema: 'remoteSchema', + columns: 'columnMap', + skipColumns: 'ignoreColumns', + excludeColumns: 'ignoreColumns', + }, + guidance: { + // The mirror image of `datasource.zod.ts`'s own `writable → allowWrites` + // alias. ADR-0015 makes writes a DOUBLE opt-in, so the two spellings are + // both correct — each on the other layer — and an author who learned one + // will write it here. Getting this wrong fails open-looking: the object + // stays read-only and nothing says why. + allowWrites: + '`allowWrites` is the DATASOURCE-level gate (`datasource.external.allowWrites`). ' + + 'The per-object opt-in is spelled `writable: true`. ADR-0015 requires BOTH — set ' + + '`writable` here and `allowWrites` on the datasource; either one alone leaves the ' + + 'object read-only.', + schemaMode: + '`schemaMode` is a DATASOURCE key, not an object key — an object becomes external ' + + "by being routed to a datasource whose `schemaMode !== 'managed'`. This block only " + + 'describes the remote binding once that is true.', + }, +}, { remoteName: z.string().optional() .describe('Remote table/view name. Defaults to object.name.'), remoteSchema: z.string().optional() @@ -932,7 +1154,55 @@ const ObjectSchemaBase = z.object({ * Omitting the block (or leaving individual flags `undefined`) keeps * the {@link managedBy}-derived default. */ - userActions: z.object({ + userActions: strictObject({ + surface: "this object's `userActions` block", + history: + 'Until #4001 these were dropped silently — the block still parsed, so an affordance ' + + 'the author meant to hide stayed on the toolbar, and the `managedBy`-derived ' + + 'default silently won.', + aliases: { + export: 'exportCsv', + csvExport: 'exportCsv', + exportcsv: 'exportCsv', + new: 'create', + add: 'create', + insert: 'create', + update: 'edit', + remove: 'delete', + destroy: 'delete', + }, + guidance: { + // Wrong-LAYER, and the trap is that the key name is right somewhere else. + // `ui/view.zod.ts` declares its own `userActions` with a completely + // disjoint vocabulary (sort/search/filter/refresh/rowHeight/ + // addRecordForm/editInline/buttons), so an author who learned that block + // writes these here and gets a shape that has never heard of them. + sort: + '`sort` is a VIEW `userActions` key, not an object one — the two blocks share a ' + + 'name and nothing else. The object block governs CRUD affordances ' + + '(create/import/edit/delete/exportCsv); toolbar controls ' + + '(sort/search/filter/refresh/rowHeight/editInline) live on the view.', + search: + '`search` is a VIEW `userActions` key. This object block governs CRUD affordances ' + + 'only — put toolbar controls on the view that renders the records.', + filter: + '`filter` is a VIEW `userActions` key. This object block governs CRUD affordances ' + + 'only — put toolbar controls on the view that renders the records.', + editInline: + '`editInline` is a VIEW `userActions` key. The object-level `edit` flag decides ' + + 'whether editing is offered AT ALL; how it is offered (inline vs form) is the ' + + "view's call.", + clone: + '`clone` is a CAPABILITY, not a user action — write `enable: { clone: false }`. ' + + 'The `enable` block (ObjectCapabilities) is where record deep-cloning is governed.', + read: + '`userActions` toggles WRITE affordances only; there is no read toggle. Read ' + + 'access is governed by permissions (`requiredPermissions`) and `access.default`.', + view: + '`userActions` toggles WRITE affordances only; there is no view toggle. ' + + 'Visibility is governed by permissions (`requiredPermissions`) and `access.default`.', + }, + }, { create: z.boolean().optional().describe('Show generic "New" button.'), import: z.boolean().optional().describe('Show CSV import wizard entry.'), edit: z.union([z.boolean(), RowCrudActionOverrideSchema]).optional().describe( @@ -978,7 +1248,34 @@ const ObjectSchemaBase = z.object({ systemFields: z .union([ z.literal(false), - z.object({ + strictObject({ + surface: "this object's `systemFields` block", + history: + 'Until #4001 these were dropped silently — the block still parsed, so an ' + + 'opt-out the author wrote had no effect and the registry injected the column ' + + 'anyway.', + aliases: { + organization: 'tenant', + org: 'tenant', + tenancy: 'tenant', + organizationId: 'tenant', + auditFields: 'audit', + timestamps: 'audit', + }, + guidance: { + // The field doc above this block names `owner` while the shape has + // never declared it — so an author following the prose lands exactly + // here. `ownership` is the real, enforced lever. + owner: + '`owner` is not a `systemFields` key — `owner_id` injection is governed by the ' + + "object-level `ownership` property (`ownership: 'none'` skips it; " + + "`'user'`/`'org'` choose the principal). `systemFields` controls only `tenant` " + + '(organization_id) and `audit` (created_at/created_by/updated_at/updated_by).', + ownership: + '`ownership` is a TOP-LEVEL object key, not a `systemFields` key — write it ' + + 'beside `systemFields`. It, not this block, decides whether `owner_id` is injected.', + }, + }, { tenant: z.boolean().optional().describe('Inject the organization_id column. Default true (the column is always provisioned; the multi-tenant flag governs only its index).'), audit: z.boolean().optional().describe('Inject the audit columns (created_at/created_by/updated_at/updated_by). Default true.'), }), @@ -1102,7 +1399,33 @@ const ObjectSchemaBase = z.object({ * precedence over the field-change summary for the same update. Consumed by * `@objectstack/plugin-audit` audit-writers (enforce-or-remove, ADR-0049). */ - activityMilestones: z.array(z.object({ + activityMilestones: z.array(strictObject({ + surface: 'this activity milestone', + history: + 'Until #4001 these were dropped silently — the milestone still parsed, so a ' + + 'mis-keyed template shipped a timeline row with the wrong text, or the milestone ' + + 'never fired at all.', + aliases: { + message: 'summary', + template: 'summary', + text: 'summary', + title: 'summary', + to: 'value', + watch: 'field', + activityType: 'type', + }, + guidance: { + from: + '`from` is not a milestone key — a milestone fires on transition INTO `value`, ' + + 'whatever the previous value was. There is no from-state filter here; when the ' + + 'TRANSITION itself must be constrained, declare a `state_machine` rule in ' + + '`validations` (ADR-0020), which is where the legal transition table lives.', + when: + '`when` is not a milestone key — the trigger is structural: `field` transitions ' + + 'INTO `value`. For a conditional timeline row, gate it with a `state_machine` ' + + 'rule in `validations` or a hook.', + }, + }, { field: z.string().describe('Field to watch (typically a status/stage select).'), value: z.string().describe('The value the field must transition INTO to fire the milestone.'), summary: z.string().describe('Activity summary template; {field} tokens interpolate the record value. e.g. "Deal won: {name}".'), @@ -1244,7 +1567,34 @@ const ObjectSchemaBase = z.object({ * * @see packages/plugins/plugin-sharing/src/share-link-service.ts */ - publicSharing: z.object({ + publicSharing: strictObject({ + surface: "this object's `publicSharing` policy", + history: + 'Until #4001 these were dropped silently — the policy still parsed, so a redaction ' + + 'list or an expiry cap the author wrote was never applied to the links the platform ' + + 'went on to issue. On a policy whose whole job is to be restrictive, a silently ' + + 'dropped key fails OPEN.', + aliases: { + audiences: 'allowedAudiences', + permissions: 'allowedPermissions', + redact: 'redactFields', + redacted: 'redactFields', + maxExpiry: 'maxExpiryDays', + expiryDays: 'maxExpiryDays', + condition: 'eligibility', + }, + guidance: { + sharingModel: + '`sharingModel` governs PRINCIPAL-based sharing (specific users / teams / roles) ' + + 'and is a TOP-LEVEL object key, not a `publicSharing` key. `publicSharing` is the ' + + 'separate opt-in for opaque share LINKS — an object may declare both, and they ' + + 'do not constrain each other.', + externalSharingModel: + '`externalSharingModel` is a TOP-LEVEL object key (ADR-0090 D11) — the OWD for ' + + 'external portal/partner principals. Link sharing is this block; principal-based ' + + 'external access is that key, one level up.', + }, + }, { /** Master switch. When false (default), no share links can be issued for this object. */ enabled: z.boolean().default(false).describe('Allow records of this object to be published via share link'), /** @@ -1899,7 +2249,43 @@ export type ObjectOwnership = z.infer; * }] * ``` */ -export const ObjectExtensionSchema = lazySchema(() => z.object({ +export const ObjectExtensionSchema = lazySchema(() => strictObject({ + surface: 'this object extension', + history: + 'Until #4001 these were dropped silently — the extension still parsed and still ' + + 'registered, so fields or rules an author meant to merge into someone else\'s object ' + + 'simply never arrived, on a surface where the target is owned by another package and ' + + 'the absence is easy to blame on precedence.', + aliases: { + object: 'extend', + objectName: 'extend', + target: 'extend', + name: 'extend', + extends: 'extend', + order: 'priority', + }, + guidance: { + // The merge in `objectql/src/engine.ts` copies EXACTLY the declared keys + // (extend / fields / label / pluralLabel / description / validations / + // indexes / priority) onto the extension def. Anything else is not + // "unsupported yet" — there is no slot for it to arrive through. + actions: + '`actions` cannot be contributed through an object extension — the merge carries ' + + '`fields`, `label`, `pluralLabel`, `description`, `validations` and `indexes` only. ' + + "Declare a top-level action with `objectName: ''`; `defineStack()` attaches " + + 'it to the object.', + hooks: + '`hooks` cannot be contributed through an object extension — declare a top-level ' + + 'hook bound to the target object instead.', + listViews: + '`listViews` cannot be contributed through an object extension — declare a ' + + 'top-level `view` bound to the target object instead.', + fieldGroups: + '`fieldGroups` cannot be contributed through an object extension — the merge does ' + + 'not carry them. Add the fields here and declare the groups on the owning object, ' + + 'or assign a Page for the layout.', + }, +}, { /** The target object name (FQN) to extend */ extend: z.string().describe('Target object name (FQN) to extend'), diff --git a/packages/spec/src/kernel/metadata-authoring-lint.test.ts b/packages/spec/src/kernel/metadata-authoring-lint.test.ts index b36745e52d..cea05b7250 100644 --- a/packages/spec/src/kernel/metadata-authoring-lint.test.ts +++ b/packages/spec/src/kernel/metadata-authoring-lint.test.ts @@ -179,8 +179,14 @@ describe('the #4148 behaviours survive the generalization', () => { // degenerating into a single-collection test: `object` reports a NESTED // strip site under a CLOSED root (the #4522 behaviour), and `view` — the // last open root, and the union case — reports at its own. + // + // The object-side fixture was `userActions` until #4001 批 20 closed it + // (with lifecycle/{retention,ttl,storage,archive}, fieldGroups, external, + // systemFields, activityMilestones, publicSharing and the extension entry). + // `indexes[]` is what that batch deliberately left open — see the + // IndexSchema JSDoc for why — so it is now the object's nested strip site. const findings = lintUnknownAuthoringKeys({ - objects: [{ name: 'a', label: 'A', userActions: { zzz: 1 } }], + objects: [{ name: 'a', label: 'A', indexes: [{ fields: ['a'], zzz: 1 }] }], views: [{ name: 'v', object: 'a', zzz: 1 }], }); // Deduped deliberately: `view` is a union (container | ViewItem | overlay) @@ -190,7 +196,7 @@ describe('the #4148 behaviours survive the generalization', () => { // and it becomes moot when `view` closes. Left recorded rather than papered // over by picking a non-union collection. expect([...new Set(findings.map((f) => `${f.surface}:${f.path}`))].sort()).toEqual([ - 'object:objects.a.userActions.zzz', + 'object:objects.a.indexes.0.zzz', 'view:views.v.zzz', ]); }); @@ -232,41 +238,70 @@ describe('nested descent (#4001 evidence phase)', () => { // and — just as importantly — the two cases where it must stay quiet. it('reports inside a nested object', () => { - const [finding, ...rest] = lintUnknownAuthoringKeys({ + // This one has now run out of subject too, and — unlike the array case + // below — it has not got it back. `userActions` was the last strip-mode + // nested object-valued PROPERTY reachable under any registered root; + // #4001 批 20 closed it together with `lifecycle` (+ its four sub-blocks), + // `fieldGroups`, `external`, `access`, `systemFields`, `activityMilestones`, + // `publicSharing` and the object-extension entry. Measured, not assumed: + // every one of those fixtures now lints CLEAN and PARSES FALSE. + // + // So assert the hand-off, which is the outcome this layer exists to reach — + // the key is not unreported, it is REJECTED — and keep the descent itself + // under test one shape over. + expect(lintUnknownAuthoringKeys({ objects: [{ name: 'o1', userActions: { zzz_nested: 1 } }], + })).toEqual([]); + expect(ObjectSchema.safeParse({ + name: 'o1', + label: 'O', + fields: { a: { type: 'text', label: 'A' } }, + userActions: { zzz_nested: 1 }, + }).success).toBe(false); + + // The per-node descent under a CLOSED root — the #4522 behaviour, and the + // reason the walk no longer gates a whole collection on its root's posture + // — is still exercised, on the one nested shape 批 20 deliberately left + // open (`indexes[]`; the IndexSchema JSDoc says why). When that closes, + // this describe block is genuinely finished. + const [finding, ...rest] = lintUnknownAuthoringKeys({ + objects: [{ name: 'o1', indexes: [{ fields: ['a'], zzz_nested: 1 }] }], }); expect(rest).toEqual([]); expect(finding).toMatchObject({ - path: 'objects.o1.userActions.zzz_nested', + path: 'objects.o1.indexes.0.zzz_nested', surface: 'object', key: 'zzz_nested', }); }); it('reports inside an array element, indexed by position', () => { - // This test has now run out of subject, and that is worth saying plainly - // rather than deleting it or inventing a fixture. + // RESTORED at #4001 批 20, under the standing instruction this test left + // for itself when it ran out of subject at 6d ("if a new strip surface with + // a nested array ever appears, restore the indexed assertion here; do not + // let it go untested a second time"). // - // It was `pages[].regions[]`, then `objects[].actions[]` when `page` closed - // (6a), and with `action` closed (6d) there is no declared ARRAY OF OBJECTS - // left anywhere in the registered surface that is still strip-mode. The - // walker's array-index handling is unchanged and still correct; what is gone - // is any metadata type that exercises it. That is the ratchet finishing, not - // the walk regressing. + // The subject is `object.indexes[]` — a declared ARRAY OF OBJECTS that is + // still strip-mode, and deliberately so: 批 20 closed every other inner + // block of `data/object.zod.ts` and held this one, because the console + // ships a drifted hand-copy of the shape (`where` for the spec's `partial`) + // and closing it would 422 a control the console itself renders. See the + // IndexSchema JSDoc. It did not become open again — it never closed. // - // So: assert the hand-off, and assert what still holds — the per-node - // descent under a CLOSED root, which is the #4522 fix and the reason the - // walk no longer gates a whole collection on its root's posture. If a new - // strip surface with a nested array ever appears, restore the indexed - // assertion here; do not let it go untested a second time. + // Element 1, not 0, so this proves POSITION is carried rather than that a + // path happens to end in an index. + const [indexed, ...rest] = lintUnknownAuthoringKeys({ + objects: [{ name: 'o1', indexes: [{ fields: ['a'] }, { fields: ['b'], zzz_nested: 1 }] }], + }); + expect(rest).toEqual([]); + expect(indexed).toMatchObject({ path: 'objects.o1.indexes.1.zzz_nested', surface: 'object' }); + + // The hand-off half: `actions[]` closed at 6d, so the walk stays quiet + // there. A broken walk would produce the same empty result, which is why + // the positive assertion above has to exist alongside it. expect(lintUnknownAuthoringKeys({ objects: [{ name: 'o1', actions: [{ name: 'a', zzz_nested: 1 }] }], })).toEqual([]); - - const [nested] = lintUnknownAuthoringKeys({ - objects: [{ name: 'o1', userActions: { zzz_nested: 1 } }], - }); - expect(nested).toMatchObject({ path: 'objects.o1.userActions.zzz_nested', surface: 'object' }); }); it('hands the field record and its nested array to the parse', () => { diff --git a/packages/spec/src/ui/app-strictness-batch19.test.ts b/packages/spec/src/ui/app-strictness-batch19.test.ts new file mode 100644 index 0000000000..773936e622 --- /dev/null +++ b/packages/spec/src/ui/app-strictness-batch19.test.ts @@ -0,0 +1,170 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #4001 批 19 — `ui/app.zod.ts`'s last strip site, measured. + * + * The file has ONE site left: `BaseNavItemSchema`. The ledger carried it as + * `verify` — held pending a check, not scheduled work — with this instruction: + * + * > `BaseNavItemSchema` — the base the strict discriminated-union members + * > extend. Closing a base that is `.extend()`ed is the #4001 trap that bit + * > `view` (finding 16); confirm the members' strictness is not already + * > covering it before touching. + * + * ## The check came back NEGATIVE, and the premise was wrong twice over + * + * 1. **The members do not `.extend()` the base — they spread `...Base.shape`.** + * That is a materially different mechanism. `.extend()` produces a clone + * that INHERITS the base's unknown-key posture, which is exactly what made + * finding 16 dangerous on `view` (closing two authoring schemas silently + * closed the Studio round-trip overlay and turned a shape the platform + * itself writes into a 422). A `...shape` spread copies the per-key schemas + * into a FRESH `z.object`, and posture is a property of that new object, not + * of the record of keys it was built from. So the base's posture is not + * inherited by anything, in either direction. + * + * 2. **The members' own strictness already covers every key the base + * contributes.** All nine branches apply their own `.strict()` with a + * curated `navItemUnknownKeyError`, so there is no surface the base's + * posture could protect that is not already protected. + * + * Together those make closing this site a guaranteed no-op — and #4583 is + * explicit that a no-op closure is not neutral: *"a precisely-validated dead + * slot is the more convincing lie"*. The base is also module-private and is + * never parsed; `.strict()` is a property of a PARSE. + * + * So 批 19 changes no posture. This file is what makes that a VERDICT rather + * than an omission — the third place it is recorded, beside the schema comment + * and the `ui/` ledger row. If a future member is ever written as + * `BaseNavItemSchema.extend({...})` WITHOUT its own `.strict()`, the last test + * here is the one that should start failing. + */ + +import { describe, it, expect } from 'vitest'; +import { z } from 'zod'; + +import { + ObjectNavItemSchema, + DashboardNavItemSchema, + PageNavItemSchema, + UrlNavItemSchema, + ReportNavItemSchema, + ActionNavItemSchema, + ComponentNavItemSchema, + GroupNavItemSchema, + AppSchema, +} from './app.zod'; + +/** + * Every key `BaseNavItemSchema` contributes, with a legal value. + * + * Transcribed deliberately: the point of the positive control is to prove the + * spread DELIVERS these to each member, so reading them back off the member's + * own shape would assert nothing. + */ +const BASE_KEYS = { + id: 'nav_probe', + label: 'Probe', + icon: 'circle', + order: 3, + badge: 'New', + badgeVariant: 'secondary', + visible: "'admin' in current_user.positions", + requiredPermissions: ['read_probe'], + requiresObject: 'sys_app', + requiresService: 'tenant', +} as const; + +/** The eight exported branches, each with the payload its own `type` requires. */ +const MEMBERS: Array<[string, { safeParse: (v: unknown) => { success: boolean; error?: unknown } }, Record]> = [ + ['object', ObjectNavItemSchema, { type: 'object', objectName: 'crm_lead' }], + ['dashboard', DashboardNavItemSchema, { type: 'dashboard', dashboardName: 'sales' }], + ['page', PageNavItemSchema, { type: 'page', pageName: 'home' }], + ['url', UrlNavItemSchema, { type: 'url', url: 'https://example.com' }], + ['report', ReportNavItemSchema, { type: 'report', reportName: 'pipeline' }], + ['action', ActionNavItemSchema, { type: 'action', actionDef: { actionName: 'run_it' } }], + ['component', ComponentNavItemSchema, { type: 'component', componentRef: 'metadata:resource' }], + ['group', GroupNavItemSchema, { type: 'group' }], +]; + +describe('#4001 批 19 — the `verify` check on `BaseNavItemSchema`', () => { + describe('POSITIVE control — the spread really does deliver the base keys', () => { + it.each(MEMBERS)('the `%s` branch accepts every key the base contributes', (_name, schema, payload) => { + const r = schema.safeParse({ ...BASE_KEYS, ...payload }); + expect(r.success, JSON.stringify((r.error as { issues?: unknown })?.issues ?? '')).toBe(true); + }); + }); + + describe('NEGATIVE control — the members\' own strictness already covers that surface', () => { + it.each(MEMBERS)('the `%s` branch rejects an undeclared key, with the curated nav error', (_name, schema, payload) => { + const r = schema.safeParse({ ...BASE_KEYS, ...payload, notANavKey: 1 }); + expect(r.success).toBe(false); + const issues = JSON.stringify((r.error as { issues?: unknown })?.issues ?? []); + expect(issues).toContain('notANavKey'); + // The curation is the members', not the base's — more evidence that the + // base's posture has no job here. + expect(issues).toContain('navigation item'); + }); + }); + + it('the base contributes a key that is NOT declared by any member individually — so the spread is load-bearing', () => { + // `requiresService` exists only on the base. If the spread were not + // delivering it, the positive control above would pass vacuously on + // members that happened to declare the same key themselves. + const r = DashboardNavItemSchema.safeParse({ + id: 'nav_x', label: 'X', type: 'dashboard', dashboardName: 'd', requiresService: 'tenant', + }); + expect(r.success).toBe(true); + }); + + it('reaches the same verdict through the real door — `AppSchema.navigation`, not just standalone members', () => { + // `.strict()` is a property of a PARSE, so the door is asserted rather + // than inferred. `navigation` is a `discriminatedUnion` on `type`, so the + // rejection lands on the branch actually written. + const app = { name: 'probe_app', label: 'Probe' }; + expect(AppSchema.safeParse({ + ...app, + navigation: [{ ...BASE_KEYS, type: 'dashboard', dashboardName: 'd' }], + }).success).toBe(true); + + const r = AppSchema.safeParse({ + ...app, + navigation: [{ ...BASE_KEYS, type: 'dashboard', dashboardName: 'd', notANavKey: 1 }], + }); + expect(r.success).toBe(false); + expect(JSON.stringify((r.error as { issues?: unknown })?.issues ?? [])).toContain('notANavKey'); + }); + + it('THE MECHANISM — a `...shape` spread does not carry the source object\'s posture, in either direction', () => { + // This is the whole of 批 19's finding, isolated from `app.zod.ts` so it + // cannot be read as an accident of that file. Finding 16's trap is real, + // and it is a property of `.extend()`, NOT of `.shape`. + const openBase = z.object({ a: z.string() }); + const strictBase = z.object({ a: z.string() }).strict(); + + // `.extend()` INHERITS — this is the finding-16 mechanism. + expect(strictBase.extend({ b: z.string().optional() }).safeParse({ a: 'x', zzz: 1 }).success).toBe(false); + expect(openBase.extend({ b: z.string().optional() }).safeParse({ a: 'x', zzz: 1 }).success).toBe(true); + + // `...shape` does NOT — the new object's posture is its own, whatever the + // source's was. Both directions, because "closing the base closes the + // members" and "closing the base is a no-op" are opposite claims and only + // one of them can be true. + expect(z.object({ ...strictBase.shape, b: z.string().optional() }).safeParse({ a: 'x', zzz: 1 }).success).toBe(true); + expect(z.object({ ...openBase.shape, b: z.string().optional() }).strict().safeParse({ a: 'x', zzz: 1 }).success).toBe(false); + }); + + it('every member applies its OWN `.strict()` — the invariant that makes the base\'s posture irrelevant', () => { + // The guard for the future. If someone adds a tenth branch as + // `BaseNavItemSchema.extend({...})` without `.strict()`, or drops a + // `.strict()` from an existing one, the base stops being a no-op and this + // batch's verdict has to be re-taken. That is what this asserts — not the + // spelling in the source, but the behaviour the spelling is there for. + for (const [name, schema, payload] of MEMBERS) { + expect( + schema.safeParse({ ...BASE_KEYS, ...payload, aKeyNoBranchDeclares: 1 }).success, + `the \`${name}\` branch stopped rejecting unknown keys — re-take the 批 19 verdict`, + ).toBe(false); + } + }); +}); diff --git a/packages/spec/src/ui/app.zod.ts b/packages/spec/src/ui/app.zod.ts index 17f28c0566..4095617917 100644 --- a/packages/spec/src/ui/app.zod.ts +++ b/packages/spec/src/ui/app.zod.ts @@ -148,6 +148,34 @@ const actionDefUnknownKeyError = strictUnknownKeyError({ 'the entry dispatched a different action than the author declared.', }); +/** + * Shared shape of every navigation item — spread into the nine branches below. + * + * ## ⛔ Deliberately still `.strip()` — #4001 批 19 measured it and left it alone + * + * This is `ui/app.zod.ts`'s last open site, and it is NOT unfinished work. + * The ledger held it as `verify` on the assumption that the branches + * `.extend()` this base, which would make closing it inherit down into all + * nine (finding 16 — the trap that turned `view`'s Studio round-trip overlay + * into a 422). They do not: they spread `...BaseNavItemSchema.shape`, and a + * spread copies the per-key schemas into a FRESH `z.object` whose posture is + * its own. Nothing inherits from here, in either direction. + * + * And every branch already applies its own `.strict()` with the curated + * `navItemUnknownKeyError`, so every key this base contributes is ALREADY + * gated at all nine doors. This schema is module-private and is never parsed — + * `.strict()` is a property of a PARSE, so closing it would enforce exactly + * nothing while making a shape fragment look load-bearing (#4583: *"a + * precisely-validated dead slot is the more convincing lie"*). + * + * The `Class` cell it should carry is an OPEN question (#5249): the ledger's + * enumerated vocabulary has no word for a shape that is neither a door nor dead. + * + * Pinned in `app-strictness-batch19.test.ts`, including the mechanism itself + * (`.extend()` inherits posture, `...shape` does not) and a guard that fails if + * any branch ever stops rejecting unknown keys — which is the one change that + * would make this verdict need re-taking. + */ const BaseNavItemSchema = z.object({ /** Unique identifier for the item */ id: SnakeCaseIdentifierSchema.describe('Unique identifier for this navigation item (lowercase snake_case)'),