diff --git a/.changeset/chart-aggregate-groupby-strict.md b/.changeset/chart-aggregate-groupby-strict.md new file mode 100644 index 0000000000..4f3ba36134 --- /dev/null +++ b/.changeset/chart-aggregate-groupby-strict.md @@ -0,0 +1,56 @@ +--- +"@objectstack/spec": minor +"@objectstack/lint": patch +--- + +feat(spec): `ChartAggregateSchema` / `ChartGroupBySchema` reject unknown keys instead of dropping them (#5583, #4001 批 15's last two sites) + +`` is the react tier's object-bound chart binding, +and until now a key it did not declare was **silently stripped by the parse**. +`groupby` for `groupBy` degraded the chart to a single ungrouped point, `fn` for +`function` fell back to the default, `dateGranularty` for `dateGranularity` +turned off date bucketing — each with `os build` / `os validate` fully green. +That is #4001's founding failure mode, on the surface an AI page author is most +likely to write. + +Both object shapes are `strictObject` now, so an undeclared key is a named +rejection carrying the surface, the offending key and a rename: + +``` +Unrecognized key(s) on this chart aggregate: `groupby`. +Did you mean `groupby` → `groupBy`? Until #5583 an undeclared aggregate key was +dropped at parse — … +``` + +Curated beyond edit distance where the near-miss is semantic rather than a typo: +`fn` / `agg` / `aggregation` → `function`, `measure` → `field`, and the ADR-0021 +dataset vocabulary an author carries over from the other binding mode +(`dimension` / `category` → `groupBy`). Wrong-LAYER keys get a prescription +instead of a rename — `dateGranularity` written *beside* `groupBy` did nothing at +all and now says where it belongs; `alias`, `filter`, `objectName` and a +`measures` array are pointed at the surface that owns them. + +**Why this took two issues.** `.strict()` is a property of a PARSE, and until +#5020 nothing parsed these schemas: the react-page publish gate re-derived the +vocabulary by hand. Closing them first would have shipped a precisely-validated +door with nothing behind it (#4583). #5020 wired the parse; this is the posture. + +**The zod-4 union collapse is load-bearing here.** `groupBy` is a union, so the +`unrecognized_keys` its strict arm raises never reaches `error.issues` — zod +reports one `invalid_union` whose own message is the bare string `"Invalid +input"`. What carries the named rejection to the author is `packages/lint`'s +`describeIssue` arm unpacking, pinned end to end on both sides. + +**`groupBy` stays REQUIRED — the product question this pair raised is answered, +and the answer does not move the schema.** An ungrouped single-value chart is +not a supported `` shape: the single-value need is served by the +separate `object-metric` block, the example corpus authors zero ungrouped +`` aggregates, and objectui's `schema.aggregate?.groupBy || +schema.xAxisKey` reads are optional-chained on `aggregate` itself — they serve +charts with **no aggregate at all**, not ungrouped ones. #5020's `warning`-level +tolerance for an absent `groupBy` therefore stays a tolerance rather than +becoming a blessing; its hint now states the ruling. + +**Upgrading:** if a chart aggregate carried a key this schema does not declare, +it was already being ignored — the rejection names it and prescribes the fix. No +legal declaration changes meaning. diff --git a/.changeset/strict-unknown-key-error-call-sites-migrated.md b/.changeset/strict-unknown-key-error-call-sites-migrated.md new file mode 100644 index 0000000000..aa9f60d700 --- /dev/null +++ b/.changeset/strict-unknown-key-error-call-sites-migrated.md @@ -0,0 +1,59 @@ +--- +"@objectstack/spec": patch +--- + +refactor(spec): the last 44 hand-transcribed key lists are gone — every alias table is judged against its schema's real shape (#5593) + +Forty-four authoring schemas predated `strictObject` and wired their unknown-key +error by hand: a `const X_KEYS = [...] as const` transcription of the shape, a +`strictUnknownKeyError({ knownKeys: X_KEYS, … })` call, and a drift-probe test +whose only job was to catch the two copies disagreeing. All 44 now call +`strictObject(options, shape)`, which reads the candidate list from the shape +itself, and the 16 transcriptions plus their probe tests are deleted. + +The point is not the line count — it is what the alias-integrity gate (#5013) +can now assert about them. #5483 had put these tables under the gate through a +transitional registry, but two of its three claims were answered against the +*transcription*: an array that had drifted from its schema dragged both answers +with it, and "this alias target is a tombstone" was invisible because a flat +string array holds no schemas. Migrating closes that half, and the migration +itself found what the transcriptions were hiding: + +- **11 alias/suggestion targets were retired keys.** `app` (8: `apis`, `aria`, + `embed`, `homePageId`, `mobileNavigation`, `objects`, `sharing`, `version`), + `flow` (`active`, `template`) and `flow node` (`outputSchema`) are + `retiredKey()` tombstones the arrays still listed, so a near-miss was steered + onto the one key guaranteed to be rejected next — ledger finding 12, three + files, live. `strictObject` excludes anything the shape cannot accept, so the + author now gets the tombstone's own upgrade prescription instead. +- **A nav `separator` was answering with keys it rejects.** The nine navigation + variants shared one transcription that handed every variant the base nav keys — + but `SeparatorNavItemSchema` spreads nothing and declares `type` / `id` / + `order` alone. Writing `title` on a separator was answered *"did you mean + `label`?"*, and `label` was rejected too: finding 7, from the campaign built to + end it. The separator now carries the alias entries whose target it really has, + and one prescription for the nine base keys it does not. +- **Three ADR-0010 envelopes were missing from their own pools** (`datasource`, + `hook`, `sharing rule`): the protection keys the shapes spread were never + transcribed, so a typo of one got no suggestion at all. + +Author-facing messages are otherwise unchanged — the surface name, the offending +key, the rename and the curated prescriptions all survive verbatim, verified by +comparing every migrated surface's old array against its new derived pool and by +sampling a real rejection from each. + +Two structural consequences: + +- the shrink-only ratchet on direct `strictUnknownKeyError` call sites is a hard + **zero**, and the assertion changed meaning with the number: it no longer + measures how much of the gate runs on the weaker instrument, it forbids the + weaker instrument. `strictUnknownKeyError` stays published for external + callers; inside `packages/spec` the only caller is `strictObject`. +- `shared/alias-table-registry.ts` — #5483's transitional registry — is deleted + with its last call site, along with the suppression hook `strictObject` needed + to stay out of it. + +`data/object.zod.ts`'s error map was built lazily to step around a temporal dead +zone; `strictObject` evaluates its options at construction, so the deferral is +replaced by declaration order (`UNKNOWN_KEY_GUIDANCE` moved above the shape) and +that order is now load-bearing. 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 09d461a238..50678d5b6e 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 | 434 | -| Still-open (strip) sites | 182 | -| Files carrying at least one | 28 | +| Still-open (strip) sites | 180 | +| Files carrying at least one | 27 | Remaining strip sites by class: | Bucket | Sites | |---|---| -| authorable — the ruling's forced scope | 43 | +| authorable — the ruling's forced scope | 41 | | unresolved — needs a per-schema verdict | 33 | | wire / open — out of forced scope | 104 | | no door — no carrier, ADR-0049 territory | 1 | @@ -44,12 +44,12 @@ The `strict` column is the one the campaign schedules against; it counts both th | Dir | Sites | strict | passthrough | catchall | strip | |---|---|---|---|---|---| -| `ui/` | 160 | 116 | 5 | 0 | 39 | +| `ui/` | 160 | 118 | 5 | 0 | 37 | | `data/` | 162 | 54 | 1 | 0 | 107 | | `automation/` | 65 | 42 | 0 | 0 | 23 | | `security/` | 20 | 7 | 0 | 0 | 13 | | `studio/` | 27 | 27 | 0 | 0 | 0 | -| **total** | **434** | **246** | **6** | **0** | **182** | +| **total** | **434** | **248** | **6** | **0** | **180** | ## File-level triage — site counts @@ -156,21 +156,20 @@ over it is here. ### `ui/` — open -**39 strip of 160**, in 6 file(s). +**37 strip of 160**, in 5 file(s). | File | Strip | Sites | |---|---|---| | `action-params.zod.ts` | 1 | 1 | | `app.zod.ts` | 1 | 18 | -| `chart.zod.ts` | 2 | 8 | | `component.zod.ts` | 31 | 31 | | `view.zod.ts` | 3 | 53 | | `widget.zod.ts` | 1 | 1 | -| **total** | **39** | **160** | +| **total** | **37** | **160** | | Bucket | Sites | |---|---| -| authorable — the ruling's forced scope | 34 | +| authorable — the ruling's forced scope | 32 | | unresolved — needs a per-schema verdict | 0 | | wire / open — out of forced scope | 3 | | no door — no carrier, ADR-0049 territory | 1 | diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 48c0b0738e..74758fbb27 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -653,7 +653,7 @@ sites left to be a verdict about. | `dashboard.zod.ts` | authorable | **strict as of #4001 批 14 — 0 strip sites remain.** `DashboardWidgetSchema` has been strict since the ADR-0021 cutover; 批 14 closed the two NESTED holes inside it (`compareTo`'s object arm, `layout`), the same strict-shell-over-strip-children silhouette 批 13 found on `page.components[]`. `DashboardWidgetOptionsSchema` stays `passthrough` **deliberately** (renderer escape hatch) and the `responsive` tombstone (#4876) is untouched. ⚠️ **The `compareTo` union caveat this row carried is RESOLVED, and it is the one entry in this table whose limit was dissolved rather than worked around.** 批 14 recorded that `compareTo` was a UNION, so its curated prescription was produced but never delivered — `zodIssuesToFields` maps only top-level issues and a failed union collapses to a bare `Invalid input` (#5014) — with the rejection itself unaffected. **#5011 removed the union**: the slot converged onto the analytics executor's own contract, `{ kind, dimension? }`, a plain strict object whose message IS top-level. The reason was not the message, it was worse — all three declared arms were broken on the ADR-0021 dataset path (the two strings silently dropped by the renderer, `{ offset }` throwing `compareTo requires a timeDimension "undefined"`), while all three worked on the legacy inline path: same key, two fates, the failing one blessed. The union-free shape is the design benefit, pinned in `dashboard-compareto.test.ts` so it cannot silently return. **#5014 still binds every OTHER curated message this campaign has put inside a union arm** — this row is one slot's correction, not the finding's retraction. ⚠️ **#5010 retired four more widget keys and moved this row's posture by nothing, which is the point.** The `#4956` drill gave `DashboardWidgetSchema`'s 22 widget-level keys their first per-key verdicts and found six dead; `actionUrl`/`actionType`/`actionIcon` (a per-widget action BUTTON no renderer in either repo has ever drawn — all 14 `actionUrl` reads in `DashboardRenderer` are scoped to `header.actions[]`) and `aria` (ARIA attributes that never reached the DOM — the dashboard-level `aria` the #3896 sweep removed, one level down) are now `retiredKey` tombstones beside `responsive`. **Strip sites remain 0 and the strictness verdict is untouched**, because a retirement is ADR-0049 work and this ratchet is not: closing a door makes a *dropped* key loud, it cannot make a *declared* one live — the same boundary `theme.zod.ts` records two rows up, met here from the other side. The removal also settled a second-order cost the strictness campaign could never have reached: `packages/lint`'s dashboard action-ref rule enforced ERROR-severity reference integrity on `widgets[].actionUrl`, its docblock calling the key "the per-widget button" and claiming to mirror a runtime dispatch that does not exist, so an author could FAIL A BUILD because a control that cannot render pointed at an action that also did not — an enforcement gate sustaining the very false affordance ADR-0049 wrote it to delete. That widget branch is gone, pinned. ⚠️ **`colorVariant`, the fifth dead key, is deliberately NOT retired here and this row must not be read as closing it**: the rewrite target the #4956 triage assumed (`options.colorVariant`) measured dead too — `options` only reaches a renderer through `componentSchema` on the INLINE path, and `dataset` is required on this schema, so every spec-authorable widget is dataset-bound and renders through `DatasetWidget`, which has no colour affordance at all. Moving the key there would relocate 16 authored sites from one dead slot to another and mint a second inert key. Returned for adjudication; `chartConfig`'s dashboard-face inertness (11 of 12 keys, #5175) is the same shape on the neighbouring slot | | `widget.zod.ts` | ~~authorable (p)~~ **no door** | **no authoring door (measured, #4001 批 16)** — the `(p)` resolved NEGATIVE for the whole file, the second such run after 批 13's five. Three independent measurements on 2026-08-04: (1) nothing under `packages/spec/src` imports this module except the `ui/index.ts` barrel, so no schema anywhere declares a carrier key for a widget shape — `field.widget` is a `z.string()` naming a registered *component* and has never referenced `WidgetManifest`; (2) a BFS over the in-memory Zod graph from all 24 metadata-type roots plus `defineStack` (4 766 nodes) reaches none of the six shapes, while `PageSchema` / `ObjectListViewSchema` resolve in the same run, a fresh `z.object` and a deliberate look-alike both resolve unreachable, and a synthetic carrier flips all six to reachable; (3) zero `.parse()` / `.safeParse()` in `objectstack`, `objectui` or `cloud` outside this file's own tests — objectui re-exports the inferred TYPES only and under different names (`RuntimeWidgetManifest` / `FieldWidgetComponentProps`, #4115 / #3161), and a `cloud` code search returns 0 for every symbol against a working index (`"@objectstack/spec"` → 345). ADR-0049 enforce-or-remove is **#5055**. ⚠️ **The campaign's own BFS said REACHABLE on the first run** — a false positive in the derived-clone bridge, filed as **#5056**: zod's `.describe()` returns a clone that SHARES the original `_zod.def`, so `WidgetManifestSchema.name` / `.label` (a described `SnakeCaseIdentifierSchema` / `I18nLabelSchema`) are def-identical to the same leaves on live schemas, and a bridge firing on ANY one shared property links two unrelated shapes. 2 shared keys of 20. The error is one-directional — it can only manufacture a door, i.e. it can only make a batch tighten something dead. Corrected to whole-shape overlap in `ui/door-reachability.testkit.ts` and pinned in `widget.test.ts` ✅ **#5055 ANSWERED the ADR-0049 call, and the answer SPLIT 8/1** (maintainer ruling 2026-08-06; window moved v18 → v17 on 2026-08-07). Eight of the nine sites were REMOVED — `WidgetManifestSchema`, `WidgetLifecycleSchema`, `WidgetEventSchema`, `WidgetPropertySchema` and `WidgetSourceSchema` (3 union branches) — after all three measurements above were re-run on `origin/main` with their controls passing in the same run. Route 3 ("nothing parses it → neither"): no carrier key means no shape for a `retiredKey()` tombstone and no source for a D2 conversion, so the declared record is the D3 `SemanticMigration` `ui-widget-i18n-family-retired` plus `RETIRED_DEFS_BY_MAJOR`. `WidgetManifest.performance`'s own tombstone (#3896) was subsumed by the removal of the shape that carried it. ⚠️ **The NINTH site, `FieldWidgetPropsSchema`, was KEPT — do not finish this file.** Its evidence shape differs and the difference arrived one day before 批 16 measured: it is a REACT PROPS CONTRACT, never authorable (absent from `authorable-surface/` and `json-schema.manifest/` — `onChange` is a `z.function()`), so "zero parse" is its design rather than its defect; and objectui PR #3289 (merged 2026-08-03) renamed `@object-ui/fields`' validation slot onto this contract's `error` with no alias, made the form renderer produce it, and pinned it in `packages/fields/src/__tests__/spec-symbol-batch7.test.ts` as a deliberate tripwire — "the day the spec stops exporting `FieldWidgetProps`, this file stops compiling". Re-verified on objectui `origin/main` 2026-08-07. That is a live cross-repo compile-time consumer, and `tsc` is where a props contract is enforced. So this row's remaining site stays `no door` **and stays**: unreachability is not the retirement trigger for a shape that was never authorable. Pinned bidirectionally in `ui/widget-i18n-retirement.test.ts`. ⚠️ The #5056 fixture moved with the schema: `door-reachability.testkit.test.ts` rebuilds the same 2-of-19 shared-leaf shape locally, so the instrument's regression bound is still measured rather than remembered | | `page.zod.ts` | authorable | partially strict (ADR-0089) | -| `chart.zod.ts` | **mixed — 8 authorable** (~~2 no gate~~ **gate wired at #5020**) | **5 strict as of #4001 批 15**, a sixth added at **#5022**; 2 still open, but no longer `no gate` — see the #5020 note at the end of this cell. `ChartConfigSchema` / `ChartAxis` / `ChartSeries` / `ChartAnnotation` / `ChartInteraction` are `root-graph`-reachable from the `dashboard` and `report` metadata roots (`DashboardWidget.chartConfig`, `ReportChartSchema`), so they are judged on the stored-metadata path and are now closed. **`ChartAggregateSchema` and `ChartGroupBySchema`'s object arm are NOT**, and this is the batch's real finding. They are not 批 13's no-door case — their carrier is LIVE: `aggregate` is a real authorable prop on the react tier's `` (ADR-0081), published in the generated react-blocks contract, and objectui's `ObjectChart` reads `schema.aggregate` to run the query. What is missing is the PARSE: neither schema is reachable from any metadata-type root or from `ObjectStackSchema` (both `UNREACHABLE` in the run where the five above come back `root-graph`), nothing in the three repos calls `.parse()` on them outside this file's unit tests, and the gate that DOES judge an authored `aggregate` — the react-page publish lint — re-derives the rules by hand (`CHART_FUNCTIONS`, the count/field requirement, the result-column naming) and never checks unknown keys. `react-blocks.ts` publishes the prop as a hand-written TYPE STRING; the Zod schema beside it is not what the contract is generated from. So `groupby` / `dateGranularty` are silently dropped today and would go on being silently dropped after a `strictObject` here — `.strict()` is a property of a parse. A fourth class, **`no gate`**: carrier live, parse absent. Distinct from `no door` (批 13), where the carrier itself does not exist. The contract-first fix is to make the publish gate PARSE the schema instead of re-deriving it — a `packages/lint` change, filed rather than smuggled into a spec strictness batch. Recorded in three places (schema-adjacent comment, test pin incl. a standing BFS assertion that goes red the day a carrier key appears, this row). ✅ **That fix landed at #5020, and this row's `no gate` verdict is spent — the two sites are now `authorable`** (the second half of the `Class` cell above; the strip row further down carries the same flip). The publish gate calls `ChartAggregateSchema.safeParse()` on a static `aggregate={{…}}` literal, and `CHART_FUNCTIONS` plus the hand-written count/field twin are DELETED, so the vocabulary and the refinement are single-source again. Read the flip precisely, because it is the class's first worked example and the distinction is the whole value of having added `no gate`: what changed is the PARSE, not the posture. Both schemas are still STRIP, so `groupby` / `dateGranularty` are still dropped silently — wiring the parse is the *precondition* for closing them, not the closing, and the closing is **#5583** (a sub-issue of the campaign, where the two `chart.test.ts` STRIP pins invert). #5020 also pinned today's tolerance out loud in `validate-react-page-props.test.ts` so a wired gate cannot be mistaken for a closed door — the #4583 shape, guarded from the other side. One severity note that belongs in this ledger because it is a *declared ≠ enforced* judgement, not a lint detail: an absent `groupBy` reports at **`warning`**, alone among the graded violations, because the schema and the published react-blocks type declare it required while objectui's renderer honours its absence (`schema.aggregate?.groupBy || schema.xAxisKey`) and this protocol's own `chartAggregateCategoryKey` documents the ungrouped single-row result. Gating it would enforce a declaration the platform does not itself keep; which of the two moves is #5583's product question. ⚠️ One correction shipped with the tightening: the `clickAction` migration text #3752 wrote into this file prescribed **`drillDown`, which at the time was not a key this protocol declared anywhere** — it was an untyped `(schema as any).drillDown` read inside objectui's `ObjectChart`. Promoting that sentence into a strict rejection would have handed an author the platform's authority for a key the same gate then rejects: finding 7, third occurrence, this time caught before shipping. The prose and the tombstone now name `onSegmentClick` / `ReportSchema.drilldown` / the widget's `options` bag, all of which exist. Filed separately — and **closed at #5022**, which is the entry worth reading twice, because the fix is not the one the file's own prose implied. The gap was real (a live renderer capability with no declaration), but the two carriers that prose pointed at both measured DEAD on the dashboard metadata path: `widget.chartConfig.drillDown` is read by nothing (`DashboardRenderer` never looks at `chartConfig`; `DatasetWidget` forwards exactly one key out of it, `showLegend`), and `widget.options.drillDown` is read only inside `DashboardRenderer`'s legacy `isObjectProvider` branch, which a spec-legal v17 widget cannot reach — `dataset` is required, so `datasetBound` is always true and that component schema is discarded unrendered. An ADR-0021 dataset-bound widget drills through the semantic layer and reads no drill config at all, which the platform's own docs had already said (`content/docs/ui/dashboards.mdx`: *there is no per-widget drill configuration in the dataset form*) while this ledger row pointed authors at the `options` bag. So `drillDown` was declared as `ChartDrillDownSchema` at the ONE surface measured to read it — the react tier's `` prop, published through `react-blocks.ts`'s interaction overlay rather than through `ChartConfigSchema`, precisely so the dashboard surface does not inherit an inert key. The shape is the honest six (`enabled`/`filter`/`title`/`target`/`columns`/`maxRows`); objectui's wider renderer-side `DrillDownConfig` (`mode`/`report`/`view`/`sort`, and a `navigate` target) was NOT copied — a chart reads none of them and two are read by no widget at all (objectui#3354) — and each absent key is a `guidance` entry saying so rather than a rename. Two second-order findings came out of the same measurement and are filed, not fixed here: **#5175** (`chartConfig` delivers 1 of its 12 keys on the dashboard path, and `liveness/dashboard.json` records evidence that overstates it) and **objectui#3354**. **`chart` 6 → 7 at the re-measurement** — no schema changed: `ChartAggregateSchema` is written `z\n .object({`, and the old counter's `z\.object\(` could not match across the line break | +| `chart.zod.ts` | **authorable — all 8** (~~2 no gate~~ ~~gate wired at #5020~~ **closed at #5583**) | **5 strict as of #4001 批 15**, a sixth added at **#5022**, and the last two at **#5583** — the file is now 0 strip and its row left the remaining-strip map. The cell below is kept as WRITTEN AT THE TIME, in present tense, because the two-step order it argues for is the reusable part; the two ✅ notes at the end record what each step actually moved. `ChartConfigSchema` / `ChartAxis` / `ChartSeries` / `ChartAnnotation` / `ChartInteraction` are `root-graph`-reachable from the `dashboard` and `report` metadata roots (`DashboardWidget.chartConfig`, `ReportChartSchema`), so they are judged on the stored-metadata path and are now closed. **`ChartAggregateSchema` and `ChartGroupBySchema`'s object arm are NOT**, and this is the batch's real finding. They are not 批 13's no-door case — their carrier is LIVE: `aggregate` is a real authorable prop on the react tier's `` (ADR-0081), published in the generated react-blocks contract, and objectui's `ObjectChart` reads `schema.aggregate` to run the query. What is missing is the PARSE: neither schema is reachable from any metadata-type root or from `ObjectStackSchema` (both `UNREACHABLE` in the run where the five above come back `root-graph`), nothing in the three repos calls `.parse()` on them outside this file's unit tests, and the gate that DOES judge an authored `aggregate` — the react-page publish lint — re-derives the rules by hand (`CHART_FUNCTIONS`, the count/field requirement, the result-column naming) and never checks unknown keys. `react-blocks.ts` publishes the prop as a hand-written TYPE STRING; the Zod schema beside it is not what the contract is generated from. So `groupby` / `dateGranularty` are silently dropped today and would go on being silently dropped after a `strictObject` here — `.strict()` is a property of a parse. A fourth class, **`no gate`**: carrier live, parse absent. Distinct from `no door` (批 13), where the carrier itself does not exist. The contract-first fix is to make the publish gate PARSE the schema instead of re-deriving it — a `packages/lint` change, filed rather than smuggled into a spec strictness batch. Recorded in three places (schema-adjacent comment, test pin incl. a standing BFS assertion that goes red the day a carrier key appears, this row). ✅ **That fix landed at #5020, and this row's `no gate` verdict is spent — the two sites are now `authorable`** (the second half of the `Class` cell above; the strip row further down carries the same flip). The publish gate calls `ChartAggregateSchema.safeParse()` on a static `aggregate={{…}}` literal, and `CHART_FUNCTIONS` plus the hand-written count/field twin are DELETED, so the vocabulary and the refinement are single-source again. Read the flip precisely, because it is the class's first worked example and the distinction is the whole value of having added `no gate`: what changed is the PARSE, not the posture. Both schemas are still STRIP, so `groupby` / `dateGranularty` are still dropped silently — wiring the parse is the *precondition* for closing them, not the closing, and the closing is **#5583** (a sub-issue of the campaign, where the two `chart.test.ts` STRIP pins invert). #5020 also pinned today's tolerance out loud in `validate-react-page-props.test.ts` so a wired gate cannot be mistaken for a closed door — the #4583 shape, guarded from the other side. One severity note that belongs in this ledger because it is a *declared ≠ enforced* judgement, not a lint detail: an absent `groupBy` reports at **`warning`**, alone among the graded violations, because the schema and the published react-blocks type declare it required while objectui's renderer honours its absence (`schema.aggregate?.groupBy || schema.xAxisKey`) and this protocol's own `chartAggregateCategoryKey` documents the ungrouped single-row result. Gating it would enforce a declaration the platform does not itself keep; which of the two moves is #5583's product question. ⚠️ One correction shipped with the tightening: the `clickAction` migration text #3752 wrote into this file prescribed **`drillDown`, which at the time was not a key this protocol declared anywhere** — it was an untyped `(schema as any).drillDown` read inside objectui's `ObjectChart`. Promoting that sentence into a strict rejection would have handed an author the platform's authority for a key the same gate then rejects: finding 7, third occurrence, this time caught before shipping. The prose and the tombstone now name `onSegmentClick` / `ReportSchema.drilldown` / the widget's `options` bag, all of which exist. Filed separately — and **closed at #5022**, which is the entry worth reading twice, because the fix is not the one the file's own prose implied. The gap was real (a live renderer capability with no declaration), but the two carriers that prose pointed at both measured DEAD on the dashboard metadata path: `widget.chartConfig.drillDown` is read by nothing (`DashboardRenderer` never looks at `chartConfig`; `DatasetWidget` forwards exactly one key out of it, `showLegend`), and `widget.options.drillDown` is read only inside `DashboardRenderer`'s legacy `isObjectProvider` branch, which a spec-legal v17 widget cannot reach — `dataset` is required, so `datasetBound` is always true and that component schema is discarded unrendered. An ADR-0021 dataset-bound widget drills through the semantic layer and reads no drill config at all, which the platform's own docs had already said (`content/docs/ui/dashboards.mdx`: *there is no per-widget drill configuration in the dataset form*) while this ledger row pointed authors at the `options` bag. So `drillDown` was declared as `ChartDrillDownSchema` at the ONE surface measured to read it — the react tier's `` prop, published through `react-blocks.ts`'s interaction overlay rather than through `ChartConfigSchema`, precisely so the dashboard surface does not inherit an inert key. The shape is the honest six (`enabled`/`filter`/`title`/`target`/`columns`/`maxRows`); objectui's wider renderer-side `DrillDownConfig` (`mode`/`report`/`view`/`sort`, and a `navigate` target) was NOT copied — a chart reads none of them and two are read by no widget at all (objectui#3354) — and each absent key is a `guidance` entry saying so rather than a rename. Two second-order findings came out of the same measurement and are filed, not fixed here: **#5175** (`chartConfig` delivers 1 of its 12 keys on the dashboard path, and `liveness/dashboard.json` records evidence that overstates it) and **objectui#3354**. **`chart` 6 → 7 at the re-measurement** — no schema changed: `ChartAggregateSchema` is written `z\n .object({`, and the old counter's `z\.object\(` could not match across the line break ✅ **#5583 moved the POSTURE, which is the second and last step — the file is closed.** Both object arms are `strictObject` now: `groupby` → `groupBy`, `fn` → `function` and `dateGranularty` → `dateGranularity` are named rejections carrying the surface, the offending key and a rename, and `dateGranularity` written BESIDE `groupBy` gets a curated wrong-layer prescription instead of a rename it cannot use. The two `chart.test.ts` STRIP pins and the two companion tolerance pins in `packages/lint`'s `validate-react-page-props.test.ts` INVERTED in the same change rather than being deleted. ⚠️ **The union collapse is now load-bearing and belongs in this ledger, not only in the lint package:** `groupBy` is a union, so the `unrecognized_keys` its strict arm raises never reaches `error.issues` — zod 4 reports one `invalid_union` whose own message is the bare string *"Invalid input"* (#5014's flattening, met from the strictness side). The named rejection reaches an author only through `packages/lint/src/zod-issue-format.ts`'s arm unpacking, so **a strict object arm inside a union is exactly as loud as its consumer's unpacking** — a general constraint on every remaining union site in this campaign, pinned end-to-end on both sides. ✅ **The product question is ANSWERED and it did not move the schema: `groupBy` stays REQUIRED.** Measured 2026-08-08 rather than argued — the example corpus authors exactly one `` (`renewals-pipeline.page.ts`) and it carries `groupBy`, while the ungrouped single-value need is served by a DIFFERENT registered block, objectui's `object-metric` (`ObjectMetricWidget`), which the showcase authors seven times with `aggregate: { field, function }` and no `groupBy`. The `schema.aggregate?.groupBy || schema.xAxisKey` reads this row cited as evidence that the renderer honours the absence are optional-chained **on `aggregate` itself**, so what they actually serve is a chart with no aggregate at all (a `data=` / `dataset=` binding) — they keep option-colour resolution, the comparison merge and the drill-down filter working there, and none of them makes an ungrouped aggregate draw; the one client-side aggregation path declares `groupBy` required and buckets every record under `String(undefined)` when it is missing. So the evidence was mis-attributed, and declaring the key optional would have advertised a shape the renderer does not deliver (PD#10). #5020's `warning` therefore stays a TOLERANCE rather than becoming a blessing; promoting it to `error` is a separate acceptance surface (every consumer's pages, not just the corpus, which carries zero instances) and is filed, not smuggled in. | | `i18n.zod.ts` | **split** | **`i18n` SPLITS across two classes (measured, #4001 批 16)** and is the file this table's standing warning was about. The warning said "label shapes are wide-open records by design"; measurement says something more useful. `AriaPropsSchema` is a **real door and is closed** — carried as `aria:` on ~30 live shapes under six metadata-type roots (`ListViewSchema`, `PageSchema`, `PageComponentSchema`, `DashboardWidgetSchema`, `ChartConfigSchema`, `ActionSchema`, 20 SDUI component defs) and directly BFS-reachable. It was stripping in the wild: through the `view` root, `aria: { label: 'Accounts', describedBy: 'x' }` parsed CLEAN and returned `aria: {}`, so the accessible name existed in the source file and nowhere else. The other five (`I18nObject`, `PluralRule`, `NumberFormat`, `DateFormat`, `LocaleConfig`) are **no door** — no carrier, unreachable, zero parse in all three repos; ADR-0049 is #5055. Note `NumberFormat` / `DateFormat` DO have a carrier (`LocaleConfig.numberFormat` / `.dateFormat`) but the carrier is itself doorless, so the subtree is `no door`, not `no gate`. And the warning's own subject — the wide-open **record** level — was never one of the six sites: `I18nObject.params` is a `z.record` interpolation bag whose key space is whatever the message template names, so openness there is the contract and there was nothing to close. Pinned in `i18n.zod.ts`'s header, in `i18n.test.ts`, and here ✅ **#5055 ANSWERED the ADR-0049 call: all five are REMOVED** (maintainer ruling 2026-08-06; window moved v18 → v17 on 2026-08-07), after the three measurements were re-run on `origin/main` with controls passing in the same run. `NumberFormat` / `DateFormat` went with their doorless carrier as one subtree rather than surviving as exported schemas nothing references (#3950), and `I18nObject` turned out to be superseded by its own file-neighbour: `I18nLabelSchema`'s documentation already says translation keys are generated at registration time and translations live in translation files, and the live surface is `system/translation.zod.ts`, which uses none of these shapes. Route 3 — no tombstone, no D2 conversion; the declared record is the D3 `SemanticMigration` `ui-widget-i18n-family-retired` plus `RETIRED_DEFS_BY_MAJOR`. **`AriaPropsSchema` and `I18nLabelSchema` are untouched**, and their survival is pinned as the other half of `ui/widget-i18n-retirement.test.ts` — a sweep that emptied this file would satisfy every absence assertion and take the directory's most widely carried live shape with it. The standing warning's own subject, the wide-open `I18nObject.params` record, left with its schema: openness there was the contract, and there is now no shape for it to be the contract of | | `responsive.zod.ts` | authorable | **strict as of #4001 批 13** — all four sites (`ResponsiveConfig`, `ResponsiveStyles`, and the two per-breakpoint maps). This is the one file of batch 13's six whose `(p)` resolved POSITIVE, and it resolved on the graph rather than on the file's face: `page.components[].responsive` / `.responsiveStyles` put both shapes inside the `page` metadata-type root (`dashboard.widgets[].responsive` was the second carrier until #4876 retired it, same day). What the closure bought is the batch's whole argument in one parse — **`PageComponentSchema` has been `.strict()` since ADR-0089 D3a and that never reached these blocks**, so `{ type:'element:text', responsiveStyles: { lg: {…} }, responsive: { colums: {…}, hideOn: [] } }` parsed CLEAN and returned `responsiveStyles: {}, responsive: {}` — every styling and layout instruction the author wrote, gone, reported valid. A strict shell over strip-mode children is a closed surface's silhouette, not a closed surface. The curation is the file's real hazard rather than typos: it carries TWO breakpoint vocabularies sixteen lines apart on the same component (`responsiveStyles`' `large`/`medium`/`small`/`xsmall`, ADR-0065, against `responsive`'s Tailwind `xs`…`2xl`), so the aliases run BOTH ways between them and are anchored to the named sibling, not to edit distance — batch 12's method, and the only thing that can answer `lg` → `large`. Two entries had to be measured rather than reasoned: `{ columns: { large: 4, lg: 3 } }` used to keep HALF the map (the node laid out, at the wrong width, on breakpoints the author never named — worse than a total loss, which is at least visible); and `hideOn` → `hiddenOn` needed a hand-written alias because the distance fallback provably cannot reach it — it lowercases the input but not the candidates, so a capital in a declared key costs an extra edit against a budget of 2, and the all-lowercase `hiddenon` resolves while the correctly-cased `hideOn` does not. That asymmetry is general to camelCase keys, i.e. to most of the spec, and is filed as **#4990**. `StyleMapSchema` stays deliberately OPEN (its key space is every CSS property; objectui's `declarations()` emits whatever it is handed) — recorded in the schema JSDoc, in a test pin, and in this row | | `dataset.zod.ts` | authorable | **strict as of #4001 批 14 — 0 strip sites remain.** `DatasetSchema` was strict from the ADR-0021 cutover while the two shapes carrying the actual semantic contract — `DatasetDimension`, `DatasetMeasure` (+ `.derived`) — were not. Curated against the sibling this module's own header names, `data/analytics.zod.ts`'s Cube layer: a Cube metric's `type` IS its aggregation, so `{ name: 'revenue', type: 'sum', field: 'amount' }` parsed clean and computed a `count`; `sql` gets guidance rather than an alias, because aiming `SUM(amount)` at `field` is finding 7's trap | @@ -888,7 +888,6 @@ next person to open that file will look. | `component.zod.ts` | **authorable** | **was `no gate` until #5068** (one verdict per cell on purpose — it is the machine-readable input to the generated subtotal, so the history lives here in the evidence). ⛔ **was not strictness work** — measured at 批 17 as having no parse at all: BFS-unreachable from every metadata root (all 52 targets, controls green in the same run), zero production `.parse()` sites in the three repos, and an unknown key inside `components[].properties` demonstrably survives the live `definePage()` door. The carrier (`PageComponentSchema.properties`) is live but is `z.record(z.string(), z.unknown())` — ADR-0089 D3a strictness does not recurse into it. Closing these 29 sites would have gated nothing (#4583), so the batch recorded the verdict and filed the wiring as **#5068**. ✅ **#5068 wired it, and this row's `no gate` verdict is spent — the sites are `authorable`.** `packages/lint/src/validate-component-props.ts` dispatches `ComponentPropsMap` by the component's `type` and judges `properties`: undeclared keys through `lintUnknownKeysAgainstSchema` (the same walker `lintUnknownAuthoringKeys` runs on every metadata collection — one implementation of the posture rules, not a second), values through `safeParse`. It runs on all three authoring commands from the shared registry. **Read the flip precisely, exactly as at #5020: what changed is the PARSE, not the posture.** All 31 entries still STRIP; the gate reports an undeclared key because the walker reads a strip-mode object, and converting these sites to `strictObject` moves that same report into the gate's `safeParse` half (`unrecognized_keys`, routed to the same rule id) — which is what makes the ratchet meaningful rather than cosmetic. Three things the flip did NOT do, each of which someone will otherwise assume: (1) **the carrier is unchanged, by decision** — the maintainer's 2026-08-05 ruling took direction A (gate at the authoring door) and DECLINED direction B (a discriminated `properties`) as breaking against an open `type` union, so `PageComponentSchema.properties` is still `z.record(z.string(), z.unknown())` and `component.test.ts`'s three standing assertions stay GREEN — measured against the landed gate, with their prose updated to say which dispatch landed; (2) **unregistered types are SKIPPED**, a required semantic rather than leniency — the example corpus alone authors 87 nodes across ten types this map does not carry (`flex`, `grid`, `object-metric`, `object-chart`, `record:line_items`, …), and judging them against an absent schema would report every one as broken; (3) **the storage path is still open** — a `saveMetaItem` / REST `/meta` write stores an unvalidated props bag (#4463's fourth wall), recorded rather than fixed. ⚠️ The gate is **WARNING-level** in this first step, and the reason is a measurement: on the example corpus + the three published platform pages it reports **52 findings** (44 value verdicts, 8 undeclared keys), of which 34 are inline `{ en, 'zh-CN' }` label maps against an `I18nLabelSchema` that is a plain `z.string()` (**#5728**, undecided) and 8 more are the same shape on `element:text.content`. Gating those would fail the platform's own pages to enforce declarations the platform does not keep — the `groupBy` judgement #5020 had to make, at corpus scale. That inventory is the acceptance baseline for the error upgrade, which is its own step. See the triage row for the full 批 17 measurement | | `view.zod.ts` | mixed · 1 authorable, 2 wire | **15 of 20 closed at #4001 批 18**, a sixteenth (`UserFiltersSchema`) at **#5073** once its protocol blocker was adjudicated, a seventeenth — `ViewFilterRuleSchema`, closed by an EARLIER wave — reopened at **#5114**, and then the file's last authoring debt cleared at **#5074**, which closed `ViewItemSchema` (×2 arms), `ListView.sort` AND `ViewFilterRuleSchema` in one structural change. **The strip count went 5 → 3, and the arithmetic is the finding, not the number: FOUR sites closed and TWO were ADDED** — the two arms of the new `ViewItemWireSchema`, which are strip BY DESIGN. That is why this row's Class cell is now a split (`1 authorable, 2 wire`) rather than a smaller `authorable` count: the wire contract that used to live on "the member nobody closed" now has a name, and this map measures posture, not intent. Closed: `ViewDataSchema`'s four provider arms, `UserFilterField.options`, `GanttQuickFilter.options`, `GanttConfig.tooltipFields`, `ListView.conditionalFormatting` / `.emptyState`, `FormFieldBase.keyField`, `FormView.subforms`, and `submitBehavior`'s four arms. Reachability was measured, not assumed: a BFS from all 24 metadata-type roots plus `ObjectStackSchema` resolves every one `root-graph`, with `ViewSchema`/`FormViewSchema`/`ViewItemSchema`/`PageSchema` as positive controls and 批 13's no-door shapes UNREACHABLE **in the same run** — and the instrument had to be fixed first: `lazySchema` returns a Proxy, but a carrier writes `X.optional()`, which RESOLVES it, so the closure holds the real instance and comparing the Proxy alone false-negatived `ViewDataSchema` (caught by cross-checking its two literal carrier keys, not by trusting the reading). ⚠️ **Re-checked against #5056**: every 批 18 target is `root-graph` by **identity**, so **none** of the fifteen rests on the `derived-clone` bridge that 批 16 found can mark a dead shape reachable. The one `derived-clone` verdict in the run is `ListViewSchema` — a positive CONTROL, not a target, and independently identity-reachable via `ObjectListViewSchema`. Every closed shape also has a literal carrier key in this file and a named parse door (`defineView` / `defineViewItem` / the `view` metadata-type schema / objectui's `GanttConfigSchema.safeParse` at `plugin-gantt/src/ObjectGantt.tsx:408`) — the strong-evidence class #5056 leaves standing. ⚠️ **`ListView.sort` was closed, REVERTED, and closed again at #5074 — the round trip is the file's most useful finding.** It carried `direction → order`, the #4721 alias for the identical tuple (`{field, direction:'desc'}` parsed to `{field, order:'asc'}` — a silently REVERSED sort). The full suite then failed one case: `view-metadata-schema.test.ts` pins `sort: [{ id, field, order }]` as the exact body a console column-sort PUT persists, and objectui stamps that `id` per row (`components/src/custom/sort-builder.tsx:68`/`:94`, `crypto.randomUUID()`). **The mechanism governs every nested block in this file and is the opposite of what the union's own comment implies: `.strip()` does NOT recurse.** `ViewMetadataSchema` rescues Studio's round-trip keys by making its flattened members `.strip()`, but that re-opens the TOP level only — a nested block closed inside `ListViewSchema` is still reached through that member, so a console-stamped key inside it becomes a 422 regardless. `id` was deliberately NOT declared to silence it: it is a React list key, and declaring it would put a UI artifact on the authorable surface and tell an AI author to emit one. **#5074 supplied the missing half and the shape is now CLOSED**: the write door removes the declared decoration vocabulary (`VIEW_CONSOLE_ROW_DECORATIONS` / `stripViewConsoleDecorations`, the mirror of `stripReadDecorations`) BEFORE the union runs, so the opening is recursive-effective where a member-level `.strip()` can never be, and the authoring surface never grew the key. The `direction → order` alias came back with it. Curation on what DID close is anchored to named siblings: an option `count` gets a wrong-layer pointer to `showCount` because objectui COMPUTES it per render; and a bare `name` on the `object` data source is deliberately NOT aliased — it is a real key on the view ITEM, so a rename would be finding 7 again. `submitBehavior` became a `discriminatedUnion` on the `kind` literal it already required: as a plain union of four strict members the rejection is an `invalid_union` whose prescription #5014 measured the renderers flattening away. ⚠️ **`GanttConfigSchema` / `TreeConfigSchema` are `strictObject(…).passthrough()`** — open at the parent by design, and this ledger's own counter used to read them as `strict`, because `postureOf` returned early on the `strictObject` idiom instead of walking the chain. **Fixed at #5072**: the idiom now seeds the initial posture and the chain always runs, so the two read `passthrough` and the directory's strict count drops by 2. The strip count was never affected — neither posture is strip — so this row's numbers do not move. **`UserFiltersSchema` is CLOSED as of #5073, and it is the one site in this file whose blocker was never a strictness question.** Closing it would have 422'd `allowAddTab` — a key objectui's renderer reads (`plugin-list/src/UserFilters.tsx:182`/`:742`) and the spec never declared; because `saveMetaItem` validates but persists the ORIGINAL body, the stripped key still reached the renderer, so the capability WORKED and closing would have removed it rather than making a silent failure loud. 批 18 stopped and filed rather than guessing, and the maintainer adjudicated **promote, then close, in one PR** (2026-08-04): `allowAddTab` is now DECLARED here, so the capability is discoverable from the contract (JSON Schema / Studio SchemaForm / an AI author) instead of living in one React file, and the shape closes behind it with no intermediate state. The rejected option was `SANCTIONED_LOCAL` in objectui, which would have made spec and objectui two sources of truth for one contract — the fork #2231's derive-by-reference exists to prevent (PD#12) — and would have taught authors to delete a working key with a rejection that was itself "correct" (finding 7). Two details the close is worth remembering for. **(a)** The promotion is scoped to what the renderer really does: the add-tab button objectui renders carries no click handler, so `allowAddTab` declares that the affordance RENDERS and deliberately says nothing about creating presets — a `.describe()` promising more would be PD#10's advertise-what-you-don't-deliver, and the renderer gap is filed as **#5236**. **(b)** The 批 6e reliance question resolved exactly as predicted — `ObjectUserFiltersSchema` is `.omit()`ed off this base and `.omit()` inherits posture, so the pin flipped from "drops" to "rejects", which is wanted (the CLI lint `validate-list-view-mode.ts` was already reporting these) — but inheriting the posture also inherits the base's ERROR MAP, whose `knownKeys` were read from the base shape and therefore still listed the omitted keys. Measured on the flip: `tab` was answered *"Did you mean `tab` → `tabs`?"*, steering the author at the one key that surface refuses — finding 7 produced by the fix for finding 7. So the object variant now carries its own map built over the OMITTED shape (the shape still derived by `.omit()`, so #2231 holds), with `guidance` pointing all three page-only keys at `listViews`. **⚠️ #5074 — the authoring/wire SPLIT, and the row's headline.** `ViewItemSchema` wore two contracts: the authoring gate (`defineViewItem`, objectui's view-create form, which validates `createBuildBody`'s output against the real spec schema) and member 1 of `ViewMetadataSchema`, the union `saveMetaItem` validates every persisted `view` body against. The wire role was measured, not inferred — objectui's pin control PUTs `{...storedItem, isPinned}` (`ObjectView.tsx:882` → `data-objectstack/src/index.ts:2801`); a stored ViewItem record carries `viewKind` AND `config`, so the merged body lands on member 1 (the flattened members are excluded by their `config: z.undefined()` guard) and closing the one schema would have 422'd pinning a saved view. The maintainer ruled **split** (2026-08-04), and the two-axis reasoning is worth keeping: `defineViewItem({name, object, viewKind, confg: {…}})` — one letter — used to strip the typo and hand back a ViewItem with **no view configuration at all**, parsed clean, which is #1535's `workflows: [...]` replayed on the file's densest authoring surface. `ViewItemSchema` is now `strictObject` on both arms; `ViewItemWireSchema` is the `.strip()` wire variant, built from the SAME `viewItemArmShape()` (derive-by-reference, #2231 — a `discriminatedUnion` cannot be `.extend()`ed, so sharing the shape factory is what keeps one contract from becoming two transcriptions), and `isPinned`/`sortOrder` are DECLARED on it — an explicit home, instead of surviving because nobody closed the member. **The scope addendum's hard requirement was recursive-effective openness, and that is the part a posture flip could not deliver.** `.strip()` re-opens a member's TOP level only, so the two console-decorated NESTED blocks (`ListView.sort[].id`, `ViewFilterRule.id`) were still reached at full strictness through it. The route taken is the addendum's second sanctioned one: a declared decoration vocabulary stripped before validation, at the wire door, reaching every carrier at every depth — including ones added later, which a hand-maintained parallel wire tree would not. It is deliberately NOT a second schema tree (PD#12's fork) and deliberately NOT a declared `id` (批 18 Q1's two-axis rejection: a React list key on the authoring surface teaches AI authors to emit UUIDs). Two landmines were named in the ruling and both are pinned in `view-authoring-wire-split.test.ts` §5: `z.toJSONSchema()` must still emit a four-member `anyOf` (the `/api/v1/meta/types/view` endpoint feeds Studio's SchemaForm from it — it does; a pipe converts to its output side, asserted in BOTH io directions), and the `lazySchema` Proxy's ADR-0089 D3a crash (`Cannot set properties of undefined (setting 'ref')`) must not recur under a pipe-rooted lazy schema — it does not, and each new schema is converted directly rather than only through its parent. **One real hazard the change surfaced, fixed in the same PR:** a `z.preprocess` at a registered root put TWO gate walkers into the exact blind spot #4488 had already found and fixed in `check-liveness.mts` — `metadata-authoring-lint.ts` and `metadata-form-zod-reconciliation.test.ts` both unwrapped a pipe via `def.in`, which for a preprocess is the TRANSFORM, so each reported `view` as *not key-bearing* and silently stopped covering it. Caught by their own coverage assertions (`lintables.length >= 1`, `root schema is not key-bearing`), which is precisely what those assertions exist for; both now prefer whichever side is not the transform. **A gate going quiet is worse than a gate failing** — and the pattern will recur on the next preprocess-rooted registration, so it is recorded here rather than only in the diff. **Still open, one site, measured:** `FormFieldBaseSchema` — a module-private BASE whose sole consumer already applies `.strict()` plus the ADR-0089 `strictVisibilityError` map; the door is closed, the ledger counts the base. The two remaining strip sites beyond it are `ViewItemWireSchema`'s arms, which are `wire` by design and are not debt. `ViewFilterRuleSchema` — **the same wire contamination, one block over, and it was already LIVE on `main`** (#5114): closed by an earlier wave, while objectui's filter builder stamps `id: crypto.randomUUID()` on every row it writes (`components/src/custom/filter-builder.tsx:228`, re-stamped on read-back at `plugin-view/src/config/view-config-utils.ts:146`/`:160`), and `saveMetaItem` persists the AUTHORED body verbatim — so saving a filter from the console 422'd, on all three paths including the flattened overlay that is the body actually PUT. Reopened as a p1 hotfix; `id` deliberately NOT declared, for the reason given for `sort` above. **That reopen was explicitly PROVISIONAL — "pending #5074" — and #5074 retired it rather than leaving it standing: the shape is CLOSED again, by the same decoration strip that closed `sort`, so the authoring gate rejects `id` by name while the console's own three paths still parse.** Its pin file now asserts the split per door, and the direction is the INVERTED one worth flagging to the next reader: probes 1/3 and 2/3 were GREEN before #5074 and are RED after (that IS the close), while 3/3 — the body the console actually PUTs — is green on BOTH sides and must stay so; a file that only asserted "the console body parses" would have passed unchanged through a change that quietly declared `id` as authorable. Two details worth keeping: the overlay path's rejection surfaces as `invalid_union` / *"Invalid input"* — the #5014 flattening, so the key that caused it is not in the message the author sees, which is why this sat on `main` unnoticed; and the reopening was verified in BOTH directions (re-close it and 7 assertions in `view-filter-rule-wire-id.test.ts` go red, while that file's two mechanism CONTROLS — top-level aux key rides, nested `emptyState` still rejects — stay green either way, which is what makes them controls). #5074's scope addendum named this site; the gate it was waiting on — a wire opening that REACHES a nested block — landed with it. Each verdict is recorded in three places (schema JSDoc + `view-strictness-batch18.test.ts` / `view-filter-rule-wire-id.test.ts` + this row) | | `widget.zod.ts` | **no door** | ⛔ **not strictness work** — the whole file measured unreachable from every authoring root (#4001 批 16), with no carrier key and zero parse in all three repos. ADR-0049 triage was **#5055**, and it is ANSWERED: eight of the nine sites were REMOVED (the whole widget-registration vocabulary). The row does not disappear, because the NINTH — `FieldWidgetPropsSchema` — was deliberately KEPT: it is a React props contract rather than authorable metadata, it never appeared in the authorable surface at all, and objectui PR #3289 gave it a live compile-time consumer. ⛔ **Do not close it and do not finish this file** — this is the fourth row in the ledger parked at a deliberate floor (after `flow` 批 11, `etl` 批 12 and `i18n` above), and the reverse pin fires on ZERO either way, so only this cell separates "parked" from "unfinished". See the triage row above, including why the campaign's own BFS said otherwise first (**#5056**) | -| `chart.zod.ts` | **authorable** | **was `no gate` until #5020** (the cell carries one verdict on purpose — it is the machine-readable input to the generated subtotal, so the history lives here in the evidence). `ChartAggregateSchema` + `ChartGroupBySchema`'s object arm. Config / axis / series / annotation / interaction closed at 批 15; these two were held OUT of the ratchet as `no gate` — carrier live, no parse — because closing them would have gated nothing (#4583). **#5020 wired the parse, so the hold is over and these two are ordinary strictness work again.** `packages/lint/src/validate-react-page-props.ts` now calls `ChartAggregateSchema.safeParse()` on a static `aggregate={{…}}` literal, and the hand-derived `CHART_FUNCTIONS` list + count/field refinement twin are deleted. That is the path **#5022 demonstrated on one key** and this row was blocked on: `ChartDrillDownSchema` arrived with its gate already wired, parsing instead of re-deriving, while `aggregate` beside it did the opposite. ⚠️ **The flip is `no gate` → `authorable`, NOT → closed.** Both sites still STRIP: the parse the gate runs drops `groupby` / `dateGranularty` rather than reporting them, so the ADR-0078 failure mode survives until the posture changes. Converting the two object arms to `strictObject` is **#5583** (Blocked-by resolved; a sub-issue of #4001), which is also where the two `chart.test.ts` "still STRIPS — deliberate" pins invert and where the one product question lands — `groupBy` is declared REQUIRED here and in the published react-blocks type while the renderer honours its absence, so #5020's gate reports that single case at `warning` instead of gating a shape the platform delivers | | `app.zod.ts` | covered | **批 19 ran the check and it came back NEGATIVE — no posture change; the `Class` was held at `verify` pending #5249 and is now `covered`, the verdict that ruling created (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 was the VOCABULARY, not the measurement** — which is why 批 19 left the cell alone, since it is machine-read and a guess here would be published as a confident subtotal. The two-axis table above resolved 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. ✅ **RESOLVED at #5249 (maintainer ruling 2026-08-06, option A): the vocabulary grew a ninth verdict, `covered`, and this row is its first and — as of the sweep below — its ONLY instance.** The ruling took the same route 批 15 took for `no gate` rather than rounding to the nearest wrong answer, on the ground that the cell's readers are later agents and a verdict naming the wrong ACTION is amplified by whoever acts on it. The re-review the ruling required was run over all **197** strip sites in the five triaged directories, not just this file, and it is mechanical rather than a reading: `covered` requires the keys to reach consumers by `...X.shape` SPREAD (a spread lands them in a fresh `z.object` with its own posture, so the base is inert), whereas `.extend()`/`.merge()`/`.omit()` inherit posture and keep the base a real door. Exactly **one** of the 197 sites spreads — this one, into eight of the nine branches (`SeparatorNavItemSchema` declares its own two keys and spreads nothing, and is `.strict()` all the same). The three other module-private strip bases all resolve elsewhere and stay put: `view.zod.ts`'s `FormFieldBaseSchema` is `.extend()`ed at `:1475` → posture inherits → a real door → stays `authorable`; `query.zod.ts`'s `BaseQuerySchema` is `.extend()`ed at `:485` into `QuerySchema` → same → stays `open`; `component.zod.ts`'s `EmptyProps` is used as a VALUE under eleven `ComponentPropsMap` carrier keys → carrier present → not carrier-absent at all. The remaining ~50 sites are inline nested literals under a property, so they carry a carrier by construction and cannot be `covered`. 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 | | `action-params.zod.ts` | wire | **out of scope** — `ActionSessionSchema`, the action-body `ctx.session` the runtime hands a body (#5697). Tolerant on purpose, same disposition as `data/hook.zod.ts`'s `HookContextSchema`. What this surface needed was never a closed door but a gate that RUNS: its consistency with the real producer is pinned in `packages/runtime/src/action-session-shape-contract.test.ts`, which asserts that a non-strict parse of the built object returns it UNCHANGED — so a key the builder starts producing without declaring it here is stripped, and the pin goes red | @@ -902,6 +901,42 @@ was deleted. `action.zod.ts`, `report.zod.ts`, `dataset.zod.ts` and `dashboard.zod.ts` left it the same way at **批 14**, and `theme.zod.ts` at **批 15**. +`chart.zod.ts` left this table at **#5583**, and it is the one departure worth +reading as a METHOD rather than a number. Its last two sites +(`ChartAggregateSchema`, `ChartGroupBySchema`'s object arm) were held open for +five batches on a measured `no gate` verdict — carrier live, no parse — and were +closed in **two separate issues, in order**: #5020 wired the parse +(`packages/lint`'s react-page publish gate stopped re-deriving the vocabulary and +started calling `ChartAggregateSchema.safeParse()`), and only then did #5583 move +the posture. Closing them in one step would have shipped a `.strict()` over a +schema nothing parsed — #4583's "precisely validated dead slot", and this row is +the campaign's worked example of refusing it. The two `chart.test.ts` "still +STRIPS — deliberate" pins were INVERTED rather than deleted, and the companion +tolerance pins in `packages/lint`'s `validate-react-page-props.test.ts` with +them, so both states stay legible to the next reader. + +Two things #5583 recorded that a later batch will need. **(a) The zod-4 union +collapse is now load-bearing on this file.** `groupBy` is a union, so the +`unrecognized_keys` its strict arm raises never reaches `error.issues` — zod +reports one `invalid_union` whose own message is the bare string *"Invalid +input"* (#5014, the same flattening that hid `dashboard`'s `compareTo` +prescription). What carries the named surface and the rename to the author is +`packages/lint/src/zod-issue-format.ts`'s arm unpacking, which #5020 had already +built; **a strict object arm inside a union is only as loud as its consumer's +unpacking**, and that is a general fact about this campaign's remaining union +sites, not a chart detail. **(b) The product question this row carried is +ANSWERED and it did NOT move the schema.** `groupBy` stays REQUIRED: measured on +2026-08-08, the example corpus authors exactly one `` +and it carries `groupBy`, while the ungrouped single-value need is served by a +different registered block (objectui's `object-metric`, seven instances in the +showcase). objectui's three `schema.aggregate?.groupBy || schema.xAxisKey` reads +are optional-chained on `aggregate` itself, so what they serve is a chart with +**no aggregate at all** — they were mis-read as evidence that the renderer +honours an ungrouped aggregate. Declaring `groupBy` optional would have +advertised a shape the renderer does not deliver, so #5020's `warning`-level +tolerance stays a tolerance; promoting it to `error` is a separate acceptance +surface and is filed rather than smuggled in. + **Five more rows left at #4988, and their destination is not "closed" — it is "deleted".** `touch.zod.ts`, `animation.zod.ts`, `dnd.zod.ts`, `keyboard.zod.ts` and `offline.zod.ts` reached 0 strip sites because the FILES @@ -1045,8 +1080,10 @@ remains open is overwhelmingly work for OTHER issues: answers the same way. - **`no gate`** — ~~`chart.zod.ts`'s remaining pair from 批 15~~ **left this class at #5020**, which wired the react-page publish gate to parse - `ChartAggregateSchema` instead of re-deriving it; the pair is `authorable` - again and its strictness half is #5583. ~~Still here: **all of + `ChartAggregateSchema` instead of re-deriving it; the pair went back to + `authorable`, and **its strictness half landed at #5583, so the pair is now + CLOSED** — the class's only complete round trip so far, and the evidence that + the two-step move below is finishable rather than a way of deferring. ~~Still here: **all of `component.zod.ts` from 批 17**~~ — **#5068 wired that gate too, so as of it the class is EMPTY.** `component.zod.ts` was the campaign's largest single reclassification and the reason this subtotal fell by 29 without one site diff --git a/packages/lint/src/validate-react-page-props.test.ts b/packages/lint/src/validate-react-page-props.test.ts index 9ebc1029a6..886dbfcb73 100644 --- a/packages/lint/src/validate-react-page-props.test.ts +++ b/packages/lint/src/validate-react-page-props.test.ts @@ -791,9 +791,9 @@ describe('validateReactPageProps — resolve per child obj // schema is `.strict()`, but `.strict()` is a property of a PARSE — before // this gate nothing on the react surface called one, which was exactly the // `no gate` verdict the strictness ledger recorded for `aggregate` two props -// over until #5020 wired that parse too (its own block below; the difference -// that remains is posture — `aggregate`'s schema still STRIPS, so its -// unknown-key half waits on #5583). The rule parses instead of re-deriving, so the surface name, the +// over until #5020 wired that parse too (its own block below), with the +// posture following at #5583 — so both props are now parsed AND closed, and the +// asymmetry this paragraph used to record is history. The rule parses instead of re-deriving, so the surface name, the // near-key guidance and the `target` union all arrive without being restated // here — which is why #5435's widening needed no edit to the rule itself. // ───────────────────────────────────────────────────────────────────────── @@ -911,12 +911,16 @@ describe('validateReactPageProps — (#5022)', () => { // asserted. // 2. SEVERITY GRADING (#5020 R2) — every violation the schema, the published // react-blocks type and objectui's renderer agree on gates at `error`; -// an absent `groupBy` is a `warning`, because the renderer honours the -// absence and the ledger's rule is "declare before you gate". -// 3. THE GAP, pinned open. Both schemas are STRIP-posture, so wiring the -// parse did NOT close the unknown-key hole `groupby` walks through — -// #5583 is the spec-side half. Asserting the tolerance out loud is the -// only thing that stops this gate reading as a closed door (#4583). +// an absent `groupBy` is a `warning`. #5583 ruled that the ungrouped +// single-value chart is NOT a supported shape, so that warning is an +// un-promoted gate rather than a blessed form; promoting it is its own +// acceptance surface and its own step. +// 3. THE GAP #5020 PINNED OPEN, CLOSED AT #5583. Both schemas were +// STRIP-posture, so wiring the parse did not close the unknown-key hole +// `groupby` walked through. They are `strictObject` now and the two +// tolerance pins INVERTED — including the harder half, where the +// rejection reaches the author only because `describeIssue` unpacks the +// collapsed `invalid_union` (#5014). // ───────────────────────────────────────────────────────────────────────── describe('validateReactPageProps — PARSED (#5020)', () => { @@ -1006,18 +1010,21 @@ describe('validateReactPageProps — PARSED (#5020)', () // ── R2: the one violation that only WARNS ────────────────────────────── - it('WARNS on an absent groupBy and does not block — the renderer honours it', () => { - // `ChartAggregateSchema` and `react-blocks.ts` both declare `groupBy` - // required, but ObjectChart falls back to `xAxisKey` - // (`schema.aggregate?.groupBy || schema.xAxisKey`) and this protocol's own - // `chartAggregateCategoryKey` documents the ungrouped single-row result. - // Gating it would break a working authoring shape to enforce a declaration - // the platform does not keep; #5583 decides which of the two moves. + it('WARNS on an absent groupBy and does not block — a tolerance, not a supported shape (#5583 ruled)', () => { + // The severity is unchanged; what changed at #5583 is what it MEANS. The + // ruling was that an ungrouped single-value chart is NOT a supported + // `` shape (`groupBy` stays required; the single-value need is + // served by the separate `object-metric` block), so this warning is an + // un-promoted gate rather than a blessed authoring form. Promoting it is a + // separate acceptance surface — every consumer's pages, not just the example + // corpus, which carries zero instances — so it is deliberately still a + // warning and the hint now says which way the question was decided. const hit = aggFindings(`{ function: 'count' }`); expect(hit.length).toBe(1); expect(hit[0].severity).toBe('warning'); expect(hit[0].message).toContain('aggregate.groupBy is not set'); expect(hit[0].hint).toContain('5583'); + expect(hit[0].hint, 'the hint must carry the RULING, not an open question').toContain('NOT a supported'); expect( validateReactPageProps(agg(`{ function: 'count' }`)).filter((x) => x.severity === 'error'), 'nothing about this aggregate may gate the build', @@ -1064,64 +1071,61 @@ describe('validateReactPageProps — PARSED (#5020)', () expect(hit[0].message).toContain('"quarter"'); }); - // ── The gap this PR does NOT close, pinned open (#5583) ───────────────── - - it('⚠️ STILL ACCEPTS an unknown key inside aggregate — the parse STRIPS it (#5583)', () => { - // NOT the desired end state. `ChartAggregateSchema` is a STRIP-posture - // `z.object()`, so `groupby` is silently dropped BY THE PARSE and this gate - // has nothing to report — the #4001 failure mode this issue set out to - // close survives one layer down. Wiring the parse was the precondition - // (`.strict()` is a property of a parse, and there was none); #5583 is the - // spec-side tightening, after which this assertion INVERTS and the finding - // arrives carrying the schema's named surface and rename suggestion. - // - // Asserted rather than left implicit so nobody reads this gate as a closed - // door: a rule that looks like it rejects `groupby` and does not is worse - // than one that visibly does not (#4583). - expect(aggFindings(`{ function: 'count', groupBy: 'status', groupby: 'status' }`)).toEqual([]); - - // …and the tolerance is pinned on a PARSED aggregate, not on a rule that - // happens to look at nothing. Without this half the assertion above would - // stay green for the wrong reason — an empty finding list proves nothing on - // its own (the #5046 trap). Same unknown key, plus one violation only the - // parse can report: exactly one finding comes back, and it is not about - // `groupby`. + // ── The gap #5020 pinned OPEN, and #5583 closed ───────────────────────── + // + // ⚠️ These two are the same assertions #5020 wrote, INVERTED. They were + // written as `.toEqual([])` / `.not.toContain('Unrecognized key')` so that a + // gate wired over a STRIP-posture schema could not be mistaken for a closed + // door (#4583); the spec-side tightening landed at #5583 and they now assert + // the rejection instead. Kept as a pair rather than replaced, because the + // before/after is what makes the deliberate two-step order legible. + + it('REJECTS an unknown key inside aggregate — the schema names it (#5583, was a silent strip)', () => { + const hit = aggFindings(`{ function: 'count', groupBy: 'status', groupby: 'status' }`); + expect(hit.length, 'the stripped key is now a finding').toBe(1); + expect(hit[0].severity).toBe('error'); + // The three things the author gets, all of them from `packages/spec` — this + // rule restates none of them. + expect(hit[0].message).toContain('Unrecognized key'); + expect(hit[0].message, "the schema's own surface name").toContain('this chart aggregate'); + expect(hit[0].message, 'the rename').toContain('`groupby` → `groupBy`'); + + // The parse still runs on everything else, and the two findings coexist — + // the #5046 trap in reverse: a green "it rejects" proves nothing unless the + // rule is still reading the rest of the aggregate in the same pass. const alsoBad = aggFindings(`{ function: 'kount', groupBy: 'status', groupby: 'status' }`); - expect(alsoBad.length, 'the parse ran').toBe(1); - expect(alsoBad[0].message).toContain('aggregate.function'); - // Only the schema's own rejection is worded this way — the deleted - // hand-rolled check said "is not an aggregation this chart can run" — so - // this is what makes the tolerance above a statement about a PARSED - // aggregate rather than one the rule never looked at. - expect(alsoBad[0].message, 'and the finding came from the schema').toContain('expected one of'); - // What #5583 adds, and what today's STRIP posture cannot produce. - expect(alsoBad[0].message, 'the stripped key is invisible to the gate — today').not.toContain( - 'Unrecognized key', - ); - }); - - it("⚠️ STILL ACCEPTS an unknown key inside a structured groupBy (#5583)", () => { - // The same hole one level deeper: `dateGranularty` is dropped by - // `ChartGroupBySchema`'s object arm, so the dates are never bucketed and - // the trend line is one flat segment — silently. + expect(alsoBad.length, 'both the unknown key and the bad function are reported').toBe(2); expect( - aggFindings(`{ function: 'count', groupBy: { field: 'closed_at', dateGranularty: 'month' } }`), - ).toEqual([]); + alsoBad.some((f) => f.message.includes('aggregate.function') && f.message.includes('expected one of')), + 'the function rejection still comes from the schema', + ).toBe(true); + expect(alsoBad.some((f) => f.message.includes('Unrecognized key')), 'and the unknown key too').toBe(true); + }); + + it('REJECTS an unknown key inside a structured groupBy — through the UNION collapse (#5583)', () => { + // The same hole one level deeper, and the harder half: `groupBy` is a + // UNION, so the arm's `unrecognized_keys` never reaches `error.issues` — + // zod 4 reports one `invalid_union` whose own message is the bare string + // "Invalid input" (#5014). What carries the named rejection to the author is + // `describeIssue`'s arm unpacking in `zod-issue-format.ts`. This assertion + // is therefore an END-TO-END pin on that path, not a restatement of the + // schema's: delete the unpacking and it goes red while the spec's own tests + // stay green. + const hit = aggFindings(`{ function: 'count', groupBy: { field: 'closed_at', dateGranularty: 'month' } }`); + expect(hit.length).toBe(1); + expect(hit[0].severity).toBe('error'); + expect(hit[0].message, 'the collapsed union was unpacked').toContain('no accepted form matched'); + expect(hit[0].message, "the strict arm's surface").toContain('this chart groupBy'); + expect(hit[0].message, 'and its rename').toContain('`dateGranularty` → `dateGranularity`'); + expect(hit[0].message, 'the bare collapsed message must not be the whole report').not.toBe( + 'aggregate.groupBy: Invalid input', + ); - // The same parse-proof as above, so this tolerance is not green merely - // because nothing reads the structured arm: plant the typo NEXT TO a value - // the arm does judge, and the arm's own rejection comes back while the - // unknown key stays invisible. - const alsoBad = aggFindings( - `{ function: 'count', groupBy: { field: 'closed_at', dateGranularty: 'month', dateGranularity: 'fortnight' } }`, - ); - expect(alsoBad.length, 'the structured arm was parsed').toBe(1); - expect(alsoBad[0].message).toContain('dateGranularity'); - // The typo DOES appear in this message — inside the echo of the author's own - // object — but only as data, never as a rejection. `Unrecognized key` is the - // sentence #5583 makes possible and today's STRIP posture cannot produce, so - // that is what this pin watches for. - expect(alsoBad[0].message, 'the stripped near-key is not REJECTED — today').not.toContain('Unrecognized key'); + // Control in the same run: a structured groupBy with no unknown key still + // parses clean, so this is a statement about the key and not about the arm. + expect( + aggFindings(`{ function: 'count', groupBy: { field: 'closed_at', dateGranularity: 'month' } }`), + ).toEqual([]); }); // ── Unresolvable is not wrong (ADR-0072 D1) ──────────────────────────── diff --git a/packages/lint/src/validate-react-page-props.ts b/packages/lint/src/validate-react-page-props.ts index a93f7b644b..35d575adf7 100644 --- a/packages/lint/src/validate-react-page-props.ts +++ b/packages/lint/src/validate-react-page-props.ts @@ -317,18 +317,24 @@ function checkChartDrillDown( * the schema the single source: the vocabulary, the refinement message and every * key's type arrive from `packages/spec` with nothing restated here. * - * ## What this does NOT yet close, and why the pin test says so out loud + * ## The unknown-key half, closed at #5583 * - * ⚠️ `ChartAggregateSchema` is still a STRIP-posture `z.object()` (and - * `ChartGroupBySchema`'s object arm with it), so an unknown key is *silently - * dropped by the parse* rather than reported. Wiring the parse is a - * precondition for closing that, not the closing itself: `.strict()` is a - * property of a parse, and until this commit there was no parse to make strict. - * The spec-side tightening is **#5583**, and - * `validate-react-page-props.test.ts` pins today's tolerance explicitly so this - * gate cannot be mistaken for one that already rejects `groupby` — a gate that - * READS like it closes a hole while leaving it open is the #4583 shape this - * campaign keeps paying for. + * Until #5583 `ChartAggregateSchema` was a STRIP-posture `z.object()` (and + * `ChartGroupBySchema`'s object arm with it), so an unknown key was *silently + * dropped by the parse* and this gate had nothing to report. Wiring the parse + * was the precondition, not the closing — `.strict()` is a property of a parse, + * and until #5020 there was no parse to make strict. Both are `strictObject` + * now, so `groupby` arrives here as an `unrecognized_keys` issue carrying the + * schema's own surface name and rename suggestion, and this rule forwards it + * verbatim rather than restating anything. The pin that used to record the + * tolerance in `validate-react-page-props.test.ts` inverted with it. + * + * ⚠️ **The rejection reaches the author only because `describeIssue` unpacks + * `invalid_union`.** An `unrecognized_keys` raised inside `groupBy`'s object arm + * never reaches `error.issues`; zod 4 collapses the union into one issue whose + * message is the bare string "Invalid input" (#5014). `zod-issue-format.ts` is + * what recovers the arm messages, which is why the lint side had to exist before + * the spec side closed. * * ## Severity is not uniform, and the split is measured * @@ -336,12 +342,19 @@ function checkChartDrillDown( * renderer agree on gates at `error` (declared = enforced): `function` present * and in the enum, `field` a string, `aggregate` an object, and a non-`count` * function carrying a `field`. **An absent `groupBy` is a `warning`**, alone - * among them: the schema and `react-blocks.ts` both declare it required, but the - * renderer HONOURS its absence (`ObjectChart.tsx`: `schema.aggregate?.groupBy || - * schema.xAxisKey`) and this protocol's own `chartAggregateCategoryKey` documents - * the ungrouped single-row result. Gating on it would break a working authoring - * shape to enforce a declaration the platform does not itself keep; whether the - * schema loosens or the renderer tightens is the product question on #5583. + * among them. + * + * #5583 ANSWERED the product question behind that split, and the answer was that + * an ungrouped single-value chart is **not** a supported `` shape: + * `groupBy` stays declared REQUIRED, because the single-value need is served by + * the separate `object-metric` block and the renderer's + * `schema.aggregate?.groupBy || schema.xAxisKey` reads are optional-chained on + * `aggregate` itself — they serve charts with no aggregate at all, not ungrouped + * ones. So the `warning` here is a TOLERANCE, not a blessing: it is deliberately + * not upgraded to `error` in the same change, because promoting a gate is a + * separate acceptance surface (every consumer's pages, not just the example + * corpus, which carries zero instances). The upgrade is the follow-up; what must + * not happen is this warning being read as "the platform supports this". */ function checkChartAggregate( raw: unknown, @@ -368,7 +381,8 @@ function checkChartAggregate( REACT_CHART_AGGREGATE_INVALID, 'aggregate.groupBy is not set, so the aggregate returns ONE ungrouped row and the chart plots a single point.', 'Add aggregate.groupBy (a field name, or { field, dateGranularity } to bucket dates) to give the chart a category axis. ' + - 'Deliberate single-value charts are tolerated at warning level for now: ChartAggregateSchema declares groupBy required while ObjectChart honours its absence by falling back to xAxisKey — objectstack#5583 decides which of the two moves.', + 'objectstack#5583 ruled that an ungrouped single-value chart is NOT a supported shape — groupBy stays required, and a single number belongs in an object-metric block instead. ' + + 'This stays a warning rather than an error only because promoting it is its own step.', ); } diff --git a/packages/lint/src/zod-issue-format.ts b/packages/lint/src/zod-issue-format.ts index ecb5728d4a..1b6c8808bf 100644 --- a/packages/lint/src/zod-issue-format.ts +++ b/packages/lint/src/zod-issue-format.ts @@ -23,10 +23,13 @@ * bare field name or `{ field, dateGranularity?, alias? }`) and so is * `RecordHighlightsProps.fields[]` (`RecordHighlightsField` — bare field * name or `{ name, label?, icon?, type?, readonly? }`), so this is the - * common path on both surfaces. It matters more after #5583 / a future - * `strictObject` batch: an `unrecognized_keys` raised inside an object arm - * collapses exactly the same way, so the unpacking is what will carry the - * strict rejection's named surface + rename suggestion to the author. + * common path on both surfaces. **As of #5583 it is also the only thing + * carrying a STRICT rejection out of a union arm**: `ChartGroupBySchema`'s + * object arm is a `strictObject` now, and the `unrecognized_keys` it raises + * collapses exactly like any other arm failure — so this unpacking, not the + * schema, is what puts the named surface and the rename in front of the + * author. `validate-react-page-props.test.ts` pins that end to end; deleting + * the unpacking turns it red while `packages/spec`'s own tests stay green. * 2. **The offending value is dropped.** `Invalid option: expected one of * "count"|"sum"|…` never echoes what was actually written, and the * hand-rolled check #5020 replaced did (`aggregate.function "median" is not diff --git a/packages/services/service-automation/src/flow-cold-boot-bind.test.ts b/packages/services/service-automation/src/flow-cold-boot-bind.test.ts index 3f5eba2a25..8eac18c984 100644 --- a/packages/services/service-automation/src/flow-cold-boot-bind.test.ts +++ b/packages/services/service-automation/src/flow-cold-boot-bind.test.ts @@ -83,7 +83,8 @@ function fakeProtocolService(flows: unknown[]) { * spreads `_diagnostics` onto every served item, a preview read badges `_draft`, * and an overlay row carries its `_packageId`. cloud#971 — the first two are * read-time annotations the closed `FlowSchema` (#4001) rejects; the third is - * ADR-0010 envelope state `FLOW_KEYS` allowlists and the bind must PRESERVE. + * ADR-0010 envelope state `FlowSchema` DECLARES (via `MetadataProtectionFields`) + * and the bind must PRESERVE. */ function asServedByProtocol(flow: T) { return { @@ -183,7 +184,7 @@ describe('cold-boot bind survives the read path annotations (cloud#971)', () => it('keeps the ADR-0010 protection envelope — the strip is not a blanket "_" purge', async () => { // `_packageId` shares the underscore spelling but is envelope state - // `FLOW_KEYS` allowlists. Dropping it would erase a packaged flow's + // `FlowSchema` declares. Dropping it would erase a packaged flow's // provenance on every rebind, so the strip must be exactly the read // decorations and nothing more. const rec = recordingRecordChangeTrigger(); diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts index f8c2cdb05d..d26cca586c 100644 --- a/packages/services/service-automation/src/plugin.ts +++ b/packages/services/service-automation/src/plugin.ts @@ -1523,7 +1523,7 @@ export class AutomationServicePlugin implements Plugin { * producer's annotation is the producer's to remove. Widening the schema * would make our own read shape a second, permanent contract. Note the * strip removes only the read decorations, never the ADR-0010 protection - * envelope (`_lock`, `_packageId`, …) — `FLOW_KEYS` allowlists those, and + * envelope (`_lock`, `_packageId`, …) — `FlowSchema` DECLARES those, and * dropping them would strip a packaged flow's provenance on every rebind. */ private async readFlowDefsFromProtocol( diff --git a/packages/spec/scripts/strictness-ledger.test.ts b/packages/spec/scripts/strictness-ledger.test.ts index ac4331c702..4afdab95e1 100644 --- a/packages/spec/scripts/strictness-ledger.test.ts +++ b/packages/spec/scripts/strictness-ledger.test.ts @@ -162,11 +162,30 @@ describe('posture reading, with a red control for each', () => { }); it('reads the OLDER z.object(…).strict() spelling as strict too', () => { - // The reading the `strictObject(`-only count could not make. These four are - // why `security/` is done and `automation/` is not zero. + // The reading the `strictObject(`-only count could not make. + // + // The fixture used to be `security/permission.zod.ts`, whose four sites + // were the campaign's canonical `z.object(shape, { error }).strict()` + // wiring; #5593 migrated all four to `strictObject`, so the file no longer + // exercises the branch under test. `data/object.zod.ts` carries the + // spelling deliberately and is expected to keep carrying it: its two + // remaining `{ error: … }` maps are HAND-WRITTEN `$ZodErrorMap`s + // (`strictCapabilitiesError`, `strictTenancyError`) that emit a standing + // explainer the shared template cannot express, which #6416 recorded as + // out of #5593's reach. If they are ever converted, move this fixture + // rather than deleting the assertion — the AST reader still has to make + // the reading, and `packages/spec` is not the only tree it reads. + const objectSites = analyzeSites(at('data/object.zod.ts')); + const tenancy = objectSites.find((s) => s.name === 'TenancyConfigSchema'); + expect(tenancy?.posture, 'a plain `.strict()` chain is still strict').toBe('strict'); + expect(tenancy?.idiom).toBe('z.object'); + + // The permission file's four are now the helper, and still strict — the + // control that keeps this test a statement about the READER rather than + // about one file. const perm = analyzeSites(at('security/permission.zod.ts')); expect(perm.filter((s) => s.posture === 'strict')).toHaveLength(4); - expect(perm.find((s) => s.name === 'PermissionSetSchema')?.idiom).toBe('z.object'); + expect(perm.find((s) => s.name === 'PermissionSetSchema')?.idiom).toBe('strictObject'); expect(countStripSites(at('security/permission.zod.ts'))).toBe(0); }); diff --git a/packages/spec/src/automation/approval.test.ts b/packages/spec/src/automation/approval.test.ts index a41efdb888..a47affe404 100644 --- a/packages/spec/src/automation/approval.test.ts +++ b/packages/spec/src/automation/approval.test.ts @@ -353,22 +353,6 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { expect(unknownKeyIssue(ApprovalNodeConfigSchema, { ...minimalConfig, quorum: 2 })!.message) .toContain('`quorum` → `minApprovals`'); }); - - it('accepts every key the schema declares (guards APPROVAL_NODE_CONFIG_KEYS drift)', () => { - const probes: Record = { - behavior: 'quorum', minApprovals: 2, lockRecord: false, - approvalStatusField: 'approval_status', onEmptyApprovers: 'fail', - decisionOutputs: ['next_approver', { key: 'picked', type: 'user' }], - escalation: { enabled: true, timeoutHours: 4 }, maxRevisions: 1, - }; - for (const [key, value] of Object.entries(probes)) { - const result = ApprovalNodeConfigSchema.safeParse({ ...minimalConfig, [key]: value }); - const unknown = result.success - ? undefined - : result.error.issues.find((i) => i.code === 'unrecognized_keys'); - expect(unknown, `\`${key}\` should be a declared config key`).toBeUndefined(); - } - }); }); describe('ApprovalNodeApproverSchema', () => { diff --git a/packages/spec/src/automation/approval.zod.ts b/packages/spec/src/automation/approval.zod.ts index 173e35f5fe..d56f24ba9d 100644 --- a/packages/spec/src/automation/approval.zod.ts +++ b/packages/spec/src/automation/approval.zod.ts @@ -2,7 +2,7 @@ import { z } from 'zod'; import { lazySchema } from '../shared/lazy-schema'; -import { strictUnknownKeyError } from '../shared/suggestions.zod'; +import { strictObject } from '../shared/strict-object'; // Why the members sit in THIS order (the generated reference renders the JSDoc // below; this rationale stays in source): @@ -368,26 +368,22 @@ export const APPROVAL_BRANCH_LABELS = { * worst instance of the ADR-0078 trap. */ -/** Keys {@link ApprovalNodeApproverSchema} declares (drift-guarded by approval.test.ts). */ -const APPROVAL_APPROVER_KEYS = ['type', 'value', 'resolveAs', 'group', 'organization'] as const; - -const approvalApproverUnknownKeyError = strictUnknownKeyError({ - surface: 'this approval approver', - knownKeys: APPROVAL_APPROVER_KEYS, - aliases: { - approver: 'value', - userid: 'value', - org: 'organization', - grouplabel: 'group', - expandas: 'resolveAs', - }, - history: - 'Until #4001 these were dropped silently — the approver still parsed, so the ' + - 'request could route to the wrong slate without a diagnostic.', -}); - /** A single approver assignment on an Approval node. */ -export const ApprovalNodeApproverSchema = lazySchema(() => z.object({ +export const ApprovalNodeApproverSchema = lazySchema(() => strictObject( + { + surface: 'this approval approver', + aliases: { + approver: 'value', + userid: 'value', + org: 'organization', + grouplabel: 'group', + expandas: 'resolveAs', + }, + history: + 'Until #4001 these were dropped silently — the approver still parsed, so the ' + + 'request could route to the wrong slate without a diagnostic.', + }, + { // `xEnumDeprecated` lists enum members that still PARSE but must not be // offered for new authoring. Without it the Studio designer derives its // approver-type dropdown straight from this enum and keeps offering `role` @@ -500,26 +496,25 @@ export const ApprovalNodeApproverSchema = lazySchema(() => z.object({ + '`$parent` (one level up), or an organization slug. Omitted = the request\'s own organization.', xRef: { kind: 'organization', symbols: [...APPROVER_ORG_SYMBOLS] }, }), -}, { error: approvalApproverUnknownKeyError }).strict()); +})); export type ApprovalNodeApprover = z.input; -/** - * A TYPED decision-output declaration (#3447 P2 follow-up). The bare-string - * form of a `decisionOutputs` entry renders as free text; this form tells the - * decision UI which picker to render and whether to collect one id or many. - * The runtime treats `key` as the whitelist entry either way — `type` and - * `multiple` only shape the INPUT WIDGET, never the accepted value. - */ -const decisionOutputUnknownKeyError = strictUnknownKeyError({ - surface: 'this decision-output declaration', - knownKeys: ['key', 'label', 'type', 'multiple', 'required'], - aliases: { name: 'key', widget: 'type', many: 'multiple' }, - history: - 'Until #4001 these were dropped silently — the declaration still parsed, so the ' + - 'decision dialog rendered a different input than the author specified.', -}); - -export const DecisionOutputDefSchema = lazySchema(() => z.object({ +export const DecisionOutputDefSchema = lazySchema(() => strictObject( + /** + * A TYPED decision-output declaration (#3447 P2 follow-up). The bare-string + * form of a `decisionOutputs` entry renders as free text; this form tells the + * decision UI which picker to render and whether to collect one id or many. + * The runtime treats `key` as the whitelist entry either way — `type` and + * `multiple` only shape the INPUT WIDGET, never the accepted value. + */ + { + surface: 'this decision-output declaration', + aliases: { name: 'key', widget: 'type', many: 'multiple' }, + history: + 'Until #4001 these were dropped silently — the declaration still parsed, so the ' + + 'decision dialog rendered a different input than the author specified.', + }, + { /** The output key — what the flow receives as `.`. */ key: z.string().min(1).describe('Output key (the flow variable name under the node id)'), /** Display label for the decision-dialog field; defaults to a title-cased key. */ @@ -549,7 +544,7 @@ export const DecisionOutputDefSchema = lazySchema(() => z.object({ * filled them in. */ required: z.boolean().optional().describe('Approver must supply this output to approve'), -}, { error: decisionOutputUnknownKeyError }).strict()); +})); export type DecisionOutputDef = z.input; /** @@ -581,26 +576,25 @@ export function normalizeDecisionOutputs( return out; } -/** - * Per-node SLA escalation — carried on the Approval node itself, so each - * Approval step on the canvas defines its own SLA. - */ -const approvalEscalationUnknownKeyError = strictUnknownKeyError({ - surface: 'this approval escalation', - knownKeys: ['enabled', 'timeoutHours', 'action', 'escalateTo', 'notifySubmitter'], - aliases: { - timeout: 'timeoutHours', - hours: 'timeoutHours', - sla: 'timeoutHours', - to: 'escalateTo', - target: 'escalateTo', +export const ApprovalEscalationSchema = lazySchema(() => strictObject( + /** + * Per-node SLA escalation — carried on the Approval node itself, so each + * Approval step on the canvas defines its own SLA. + */ + { + surface: 'this approval escalation', + aliases: { + timeout: 'timeoutHours', + hours: 'timeoutHours', + sla: 'timeoutHours', + to: 'escalateTo', + target: 'escalateTo', + }, + history: + 'Until #4001 these were dropped silently — the escalation still parsed, so an SLA ' + + 'the author declared never fired the way they intended.', }, - history: - 'Until #4001 these were dropped silently — the escalation still parsed, so an SLA ' + - 'the author declared never fired the way they intended.', -}); - -export const ApprovalEscalationSchema = lazySchema(() => z.object({ + { enabled: z.boolean().default(false).describe('Enable SLA-based escalation for this node'), timeoutHours: z.number().min(1).describe('Hours before escalation triggers'), action: z.enum(['reassign', 'auto_approve', 'auto_reject', 'notify']).default('notify') @@ -616,7 +610,7 @@ export const ApprovalEscalationSchema = lazySchema(() => z.object({ xRef: { kind: 'position' }, }), notifySubmitter: z.boolean().default(true).describe('Notify the original submitter on escalation'), -}, { error: approvalEscalationUnknownKeyError }).strict()); +})); export type ApprovalEscalation = z.input; /** Post-parse shape of {@link ApprovalEscalation} — defaults applied, transforms run (ADR-0122). */ export type ApprovalEscalationParsed = z.infer; @@ -637,50 +631,43 @@ export type ApprovalEscalationParsed = z.infer; * first-class engine-adjacent state owned by `plugin-approvals`; this config * only describes how the node behaves. */ -/** Keys {@link ApprovalNodeConfigSchema} declares (drift-guarded by approval.test.ts). */ -const APPROVAL_NODE_CONFIG_KEYS = [ - 'approvers', 'behavior', 'minApprovals', 'lockRecord', 'approvalStatusField', - 'onEmptyApprovers', 'decisionOutputs', 'escalation', 'maxRevisions', -] as const; - -const approvalNodeConfigUnknownKeyError = strictUnknownKeyError({ - surface: "this approval node's config", - knownKeys: APPROVAL_NODE_CONFIG_KEYS, - aliases: { - approver: 'approvers', - approvalmode: 'behavior', - mode: 'behavior', - statusfield: 'approvalStatusField', - quorum: 'minApprovals', - }, - guidance: { - // The ADR-0019 re-home map: the process-level approval concepts an author - // (or AI) trained on Salesforce-style approval processes reaches for, each - // pointed at where the concept lives on the flow graph now. - steps: - '`steps` is not an approval-node config key — ADR-0019 collapsed the standalone ' + - 'approval process into Flow: successive approval STEPS are successive `approval` ' + - 'NODES on the canvas, each with its own config.', - entryCriteria: - '`entryCriteria` is not an approval-node config key — entry criteria are the ' + - '`condition` on the EDGE entering this node (ADR-0019).', - onApprove: - '`onApprove` is not an approval-node config key — on-approve actions are the ' + - "nodes wired to this node's `approve` out-edge (ADR-0019).", - onReject: - '`onReject` is not an approval-node config key — on-reject actions are the ' + - "nodes wired to this node's `reject` out-edge (ADR-0019).", - rejectionBehavior: - '`rejectionBehavior` is not an approval-node config key — back-to-previous is a ' + - 'declared BACK-EDGE to an earlier node (`type: \'back\'`, ADR-0044), and the ' + - 'revise loop is the `revise` out-edge with `maxRevisions` bounding it.', +export const ApprovalNodeConfigSchema = lazySchema(() => strictObject( + { + surface: "this approval node's config", + aliases: { + approver: 'approvers', + approvalmode: 'behavior', + mode: 'behavior', + statusfield: 'approvalStatusField', + quorum: 'minApprovals', + }, + guidance: { + // The ADR-0019 re-home map: the process-level approval concepts an author + // (or AI) trained on Salesforce-style approval processes reaches for, each + // pointed at where the concept lives on the flow graph now. + steps: + '`steps` is not an approval-node config key — ADR-0019 collapsed the standalone ' + + 'approval process into Flow: successive approval STEPS are successive `approval` ' + + 'NODES on the canvas, each with its own config.', + entryCriteria: + '`entryCriteria` is not an approval-node config key — entry criteria are the ' + + '`condition` on the EDGE entering this node (ADR-0019).', + onApprove: + '`onApprove` is not an approval-node config key — on-approve actions are the ' + + "nodes wired to this node's `approve` out-edge (ADR-0019).", + onReject: + '`onReject` is not an approval-node config key — on-reject actions are the ' + + "nodes wired to this node's `reject` out-edge (ADR-0019).", + rejectionBehavior: + '`rejectionBehavior` is not an approval-node config key — back-to-previous is a ' + + 'declared BACK-EDGE to an earlier node (`type: \'back\'`, ADR-0044), and the ' + + 'revise loop is the `revise` out-edge with `maxRevisions` bounding it.', + }, + history: + 'Until #4001 these were dropped silently — the node still parsed, so an approval ' + + 'gate shipped that quietly ignored part of its declared behavior.', }, - history: - 'Until #4001 these were dropped silently — the node still parsed, so an approval ' + - 'gate shipped that quietly ignored part of its declared behavior.', -}); - -export const ApprovalNodeConfigSchema = lazySchema(() => z.object({ + { /** Who may act on this step. */ approvers: z.array(ApprovalNodeApproverSchema).min(1).describe('Allowed approvers for this node'), @@ -780,7 +767,7 @@ export const ApprovalNodeConfigSchema = lazySchema(() => z.object({ */ maxRevisions: z.number().int().min(0).default(3) .describe('Max send-backs for revision before auto-reject (0 = send-back disabled)'), -}, { error: approvalNodeConfigUnknownKeyError }).strict()); +})); export type ApprovalNodeConfig = z.input; /** Post-parse shape of {@link ApprovalNodeConfig} — defaults applied, transforms run (ADR-0122). */ export type ApprovalNodeConfigParsed = z.infer; diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index 5e41fe8d3c..6bcfb1c033 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -1438,23 +1438,6 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { expect(message, `\`${key}\` should point at the start node`).toContain('START node'); } }); - - it('accepts every key the schema declares (guards FLOW_KEYS drift)', () => { - const probes: Record = { - description: 'd', successMessage: 's', errorMessage: 'e', version: 2, - status: 'active', template: false, variables: [{ name: 'v', type: 'text' }], - active: true, runAs: 'system', - errorHandling: { strategy: 'retry', maxRetries: 2 }, - protection: { lock: 'none' }, - }; - for (const [key, value] of Object.entries(probes)) { - const result = FlowSchema.safeParse({ ...minimalFlow, [key]: value }); - const unknown = result.success - ? undefined - : result.error.issues.find((i) => i.code === 'unrecognized_keys'); - expect(unknown, `\`${key}\` should be a declared Flow key`).toBeUndefined(); - } - }); }); describe('FlowNodeSchema', () => { @@ -1478,24 +1461,6 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { expect(unknownKeyIssue(FlowNodeSchema, { id: 'n1', type: 'script', label: 'S', inputs: {} })!.message) .toContain('`config`'); }); - - it('accepts every key the schema declares (guards FLOW_NODE_KEYS drift)', () => { - const probes: Record = { - config: { a: 1 }, - connectorConfig: { connectorId: 'c', actionId: 'a', input: {} }, - position: { x: 0, y: 0 }, timeoutMs: 100, - inputSchema: { p: { type: 'string' } }, outputSchema: { o: { type: 'number' } }, - waitEventConfig: { eventType: 'timer', timerDuration: 'PT1H' }, - boundaryConfig: { attachedToNodeId: 'n0', eventType: 'error' }, - }; - for (const [key, value] of Object.entries(probes)) { - const result = FlowNodeSchema.safeParse({ id: 'n1', type: 'script', label: 'S', [key]: value }); - const unknown = result.success - ? undefined - : result.error.issues.find((i) => i.code === 'unrecognized_keys'); - expect(unknown, `\`${key}\` should be a declared FlowNode key`).toBeUndefined(); - } - }); }); describe('FlowEdgeSchema', () => { diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 9fcd1b7730..c1b6bc2580 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -4,7 +4,6 @@ import { z } from 'zod'; import { ProtectionSchema } from '../shared/protection.zod'; import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; import { ExpressionInputSchema } from '../shared/expression.zod'; -import { strictUnknownKeyError } from '../shared/suggestions.zod'; /** * Flow Node Types — **built-in seed set** (ADR-0018). @@ -109,28 +108,24 @@ export const FLOW_STRUCTURAL_NODE_TYPES: readonly string[] = ['start', 'end']; * note). */ -/** Keys {@link FlowVariableSchema} declares (drift-guarded by flow.test.ts). */ -const FLOW_VARIABLE_KEYS = ['name', 'type', 'isInput', 'isOutput'] as const; - -const flowVariableUnknownKeyError = strictUnknownKeyError({ - surface: 'this flow variable', - knownKeys: FLOW_VARIABLE_KEYS, - aliases: { input: 'isInput', output: 'isOutput' }, - history: - 'Until #4001 these were dropped silently — the variable still parsed, so a ' + - 'mis-declared input/output contract shipped without a diagnostic.', -}); - /** * Flow Variable Schema * Variables available within the flow execution context. */ -export const FlowVariableSchema = lazySchema(() => z.object({ +export const FlowVariableSchema = lazySchema(() => strictObject( + { + surface: 'this flow variable', + aliases: { input: 'isInput', output: 'isOutput' }, + history: + 'Until #4001 these were dropped silently — the variable still parsed, so a ' + + 'mis-declared input/output contract shipped without a diagnostic.', + }, + { name: z.string().describe('Variable name'), type: z.string().describe('Data type (text, number, boolean, object, list)'), isInput: z.boolean().default(false).describe('Is input parameter'), isOutput: z.boolean().default(false).describe('Is output parameter'), -}, { error: flowVariableUnknownKeyError }).strict()); +})); /** * Flow Node Schema @@ -153,33 +148,6 @@ export const FlowVariableSchema = lazySchema(() => z.object({ * position: { x: 300, y: 200 } * } */ -/** Keys {@link FlowNodeSchema} declares (drift-guarded by flow.test.ts). */ -const FLOW_NODE_KEYS = [ - 'id', 'type', 'label', 'config', 'connectorConfig', 'position', 'timeoutMs', - 'inputSchema', 'outputSchema', 'waitEventConfig', 'boundaryConfig', -] as const; - -const flowNodeUnknownKeyError = strictUnknownKeyError({ - surface: 'this flow node', - knownKeys: FLOW_NODE_KEYS, - aliases: { - configuration: 'config', - settings: 'config', - properties: 'config', - options: 'config', - params: 'config', - parameters: 'config', - }, - guidance: { - inputs: - '`inputs` is not a FlowNode key — a node\'s runtime inputs live under `config` ' + - '(e.g. `config.inputs` for script/function nodes); `inputSchema` declares their types.', - }, - history: - 'Until #4001 these were dropped silently — the node still parsed, so a mis-placed ' + - 'config shipped as a step that quietly ignored it.', -}); - /** * A flow node — **including** whatever ADR-0031 region its `config` holds (#4415). * @@ -233,7 +201,27 @@ export const FlowNodeSchema = lazySchema(() => flowNodeObject().transform(parseF * the `lazySchema` factory runs at module-evaluation time, and a `const` arrow * declared after it would still be in its temporal dead zone. */ -function flowNodeObject() { return z.object({ +function flowNodeObject() { return strictObject( + { + surface: 'this flow node', + aliases: { + configuration: 'config', + settings: 'config', + properties: 'config', + options: 'config', + params: 'config', + parameters: 'config', + }, + guidance: { + inputs: + '`inputs` is not a FlowNode key — a node\'s runtime inputs live under `config` ' + + '(e.g. `config.inputs` for script/function nodes); `inputSchema` declares their types.', + }, + history: + 'Until #4001 these were dropped silently — the node still parsed, so a mis-placed ' + + 'config shipped as a step that quietly ignored it.', + }, + { id: z.string().describe('Node unique ID'), type: z.string().min(1).describe( 'Action type — a built-in FlowNodeAction id or a plugin-registered node type. ' + @@ -456,34 +444,30 @@ function flowNodeObject() { return z.object({ /** Signal name — only for signal boundary events */ signalName: z.string().optional().describe('Named signal to catch'), }).optional().describe('Configuration for boundary events attached to host nodes'), -}, { error: flowNodeUnknownKeyError }).strict(); } - -/** Keys {@link FlowEdgeSchema} declares (drift-guarded by flow.test.ts). */ -const FLOW_EDGE_KEYS = ['id', 'source', 'target', 'condition', 'type', 'label', 'isDefault'] as const; - -const flowEdgeUnknownKeyError = strictUnknownKeyError({ - surface: 'this flow edge', - knownKeys: FLOW_EDGE_KEYS, - aliases: { - // n8n / mermaid / BPMN-tool vocabulary an author (or AI) imports wholesale. - from: 'source', - to: 'target', - sourceid: 'source', - targetid: 'target', - expression: 'condition', - when: 'condition', - guard: 'condition', - }, - history: - 'Until #4001 these were dropped silently — the edge still parsed, so a branch ' + - 'predicate or endpoint the author wrote was quietly ignored.', -}); +}); } /** * Flow Edge Schema * Connections between nodes. */ -export const FlowEdgeSchema = lazySchema(() => z.object({ +export const FlowEdgeSchema = lazySchema(() => strictObject( + { + surface: 'this flow edge', + aliases: { + // n8n / mermaid / BPMN-tool vocabulary an author (or AI) imports wholesale. + from: 'source', + to: 'target', + sourceid: 'source', + targetid: 'target', + expression: 'condition', + when: 'condition', + guard: 'condition', + }, + history: + 'Until #4001 these were dropped silently — the edge still parsed, so a branch ' + + 'predicate or endpoint the author wrote was quietly ignored.', + }, + { id: z.string().describe('Edge unique ID'), source: z.string().describe('Source Node ID'), target: z.string().describe('Target Node ID'), @@ -521,7 +505,7 @@ export const FlowEdgeSchema = lazySchema(() => z.object({ 'BPMN default flow: traverse this edge only when no sibling conditional edge of the same ' + 'source node matched. Mutually exclusive with `condition`; at most one per source node.', ), -}, { error: flowEdgeUnknownKeyError }).strict()); +})); /** * Flow Schema @@ -549,46 +533,35 @@ export const FlowEdgeSchema = lazySchema(() => z.object({ * ] * } */ -/** Keys {@link FlowSchema} declares (drift-guarded by flow.test.ts). */ -const FLOW_KEYS = [ - 'name', 'label', 'description', 'successMessage', 'errorMessage', 'version', - 'status', 'template', 'type', 'variables', 'nodes', 'edges', 'active', 'runAs', - 'errorHandling', 'protection', - // ADR-0010 runtime protection envelope (MetadataProtectionFields spread). - '_lock', '_lockReason', '_lockSource', '_provenance', '_packageId', - '_packageVersion', '_lockDocsUrl', -] as const; - -const flowUnknownKeyError = strictUnknownKeyError({ - surface: 'this flow', - knownKeys: FLOW_KEYS, - aliases: { - steps: 'nodes', - connections: 'edges', - transitions: 'edges', - links: 'edges', - trigger: 'type', - triggertype: 'type', - title: 'label', - }, - guidance: { - object: - '`object` is not a Flow field — a record-change flow binds its object on the ' + - 'START node\'s `config` (`{ objectName, triggerType, condition }`), not at the ' + - 'flow top level.', - objectName: - '`objectName` is not a Flow field — it belongs on the START node\'s `config` ' + - '(`{ objectName, triggerType, condition }`), not at the flow top level.', - schedule: - '`schedule` is not a Flow field — a schedule flow declares its cron/interval as ' + - '`config.schedule` on the START node, not at the flow top level.', +export const FlowSchema = lazySchema(() => strictObject( + { + surface: 'this flow', + aliases: { + steps: 'nodes', + connections: 'edges', + transitions: 'edges', + links: 'edges', + trigger: 'type', + triggertype: 'type', + title: 'label', + }, + guidance: { + object: + '`object` is not a Flow field — a record-change flow binds its object on the ' + + 'START node\'s `config` (`{ objectName, triggerType, condition }`), not at the ' + + 'flow top level.', + objectName: + '`objectName` is not a Flow field — it belongs on the START node\'s `config` ' + + '(`{ objectName, triggerType, condition }`), not at the flow top level.', + schedule: + '`schedule` is not a Flow field — a schedule flow declares its cron/interval as ' + + '`config.schedule` on the START node, not at the flow top level.', + }, + history: + 'Until #4001 these were dropped silently — the flow still parsed, so a trigger ' + + 'binding or config the author wrote was quietly ignored.', }, - history: - 'Until #4001 these were dropped silently — the flow still parsed, so a trigger ' + - 'binding or config the author wrote was quietly ignored.', -}); - -export const FlowSchema = lazySchema(() => z.object({ + { /** Identity */ name: z.string().regex(/^[a-z_][a-z0-9_]*$/).describe('Machine name'), label: z.string().describe('Flow label'), @@ -804,7 +777,7 @@ export const FlowSchema = lazySchema(() => z.object({ // ADR-0010 — runtime protection envelope (internal — set by loader). ...MetadataProtectionFields, -}, { error: flowUnknownKeyError }).strict()); +})); /** * Type-safe factory for creating flow definitions. diff --git a/packages/spec/src/data/datasource.zod.ts b/packages/spec/src/data/datasource.zod.ts index da529cadcc..03cc659f70 100644 --- a/packages/spec/src/data/datasource.zod.ts +++ b/packages/spec/src/data/datasource.zod.ts @@ -7,7 +7,7 @@ import { z } from 'zod'; * Can be a built-in driver or a plugin-contributed driver (e.g., "com.vendor.snowflake"). */ import { lazySchema } from '../shared/lazy-schema'; -import { strictUnknownKeyError } from '../shared/suggestions.zod'; +import { strictObject } from '../shared/strict-object'; import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; import { validateDriverConfig } from './driver/config-registry.zod'; @@ -39,31 +39,6 @@ import { validateDriverConfig } from './driver/config-registry.zod'; * a verdict against a shape we do not have would be worse than the silence. */ -/** Keys {@link DriverDefinitionSchema} declares (drift-guarded by datasource.test.ts). */ -const DRIVER_DEFINITION_KEYS = ['id', 'label', 'description', 'icon', 'configSchema'] as const; - -/** Keys {@link ExternalDatasourceSettingsSchema} declares (drift-guarded by datasource.test.ts). */ -const EXTERNAL_SETTINGS_KEYS = [ - 'allowedSchemas', 'allowWrites', 'validation', - 'credentialsRef', 'queryTimeoutMs', -] as const; - -/** Keys the external `validation` block declares (drift-guarded by datasource.test.ts). */ -const EXTERNAL_VALIDATION_KEYS = ['onMismatch', 'checkOnBoot', 'checkIntervalMs'] as const; - -/** Keys {@link DatasourceSchema} declares (drift-guarded by datasource.test.ts). */ -const DATASOURCE_KEYS = [ - 'name', 'label', 'driver', 'config', 'pool', - 'ssl', 'description', 'active', 'autoConnect', - 'schemaMode', 'external', 'origin', -] as const; - -/** Keys the datasource `pool` block declares (drift-guarded by datasource.test.ts). */ -const POOL_KEYS = ['min', 'max', 'idleTimeoutMillis', 'connectionTimeoutMillis'] as const; - -/** Keys the datasource `ssl` block declares (drift-guarded by datasource.test.ts). */ -const SSL_KEYS = ['enabled', 'rejectUnauthorized', 'ca', 'cert', 'key'] as const; - const CAPABILITIES_REMOVED_PREFIX = '`datasource.capabilities` was removed in @objectstack/spec 17.0.0 (#4583, ADR-0049) — ' + 'all eleven flags were declared, strict-guarded and read by nobody. '; @@ -162,75 +137,6 @@ const belongsInConfig = (key: string, canonical: string = key) => + `config contract (\`PostgresConfigSchema\` / \`MysqlConfigSchema\` / \`SqliteConfigSchema\` / ` + `\`MongoConfigSchema\` / \`MemoryConfigSchema\`, exported from \`@objectstack/spec/data\`).`; -const driverDefinitionUnknownKeyError = strictUnknownKeyError({ - surface: 'this driver definition', - knownKeys: DRIVER_DEFINITION_KEYS, - aliases: { - name: 'id', - driver: 'id', - title: 'label', - config: 'configSchema', - schema: 'configSchema', - }, - guidance: { - capabilities: RETIRED_CAPABILITIES.capabilities, - capability: RETIRED_CAPABILITIES.capabilities, - }, - history: 'Until #4001 these were dropped silently — the driver still registered.', -}); - -const externalSettingsUnknownKeyError = strictUnknownKeyError({ - surface: "this datasource's external settings", - knownKeys: EXTERNAL_SETTINGS_KEYS, - aliases: { - schemas: 'allowedSchemas', - allowedschema: 'allowedSchemas', - writable: 'allowWrites', - allowwrite: 'allowWrites', - credentials: 'credentialsRef', - secretref: 'credentialsRef', - timeoutms: 'queryTimeoutMs', - querytimeout: 'queryTimeoutMs', - }, - guidance: { - label: RETIRED_DATASOURCE_BLOCKS.externalLabel, - requirePermission: RETIRED_DATASOURCE_BLOCKS.externalRequirePermission, - permission: RETIRED_DATASOURCE_BLOCKS.externalRequirePermission, - password: - '`password` must never be inlined. Put the secret in the secrets store and reference ' - + 'it with `credentialsRef` (e.g. `credentialsRef: "secret:warehouse/password"`).', - // #4487 corrected the second half of this line. It used to offer - // `capabilities.readOnly` as the place to "describe the driver" — a key the - // liveness audit found has NO reader (liveness/datasource.json), so an - // author who took the advice believed they had marked a datasource - // non-writable and had not. Same defect as the pre-#4410 `belongsInConfig` - // line documented above, on a property whose whole point is safety: a - // prescription must land somewhere enforced, and `allowWrites` is the only - // write gate there is. - readOnly: - '`readOnly` is not an external-settings key. Use `allowWrites: false` here — it is the ' - + 'enforced datasource-wide write gate (checked by the ObjectQL engine before any write ' - + 'to a federated datasource).', - }, - history: 'Until #4001 these were dropped silently — federation ran on the defaults instead.', -}); - -const externalValidationUnknownKeyError = strictUnknownKeyError({ - surface: "this datasource's external validation policy", - knownKeys: EXTERNAL_VALIDATION_KEYS, - aliases: { - onmismatch: 'onMismatch', - mismatch: 'onMismatch', - checkonboot: 'checkOnBoot', - validateonboot: 'checkOnBoot', - interval: 'checkIntervalMs', - checkinterval: 'checkIntervalMs', - }, - history: - 'Until #4001 these were dropped silently — drift checking ran on the defaults ' - + '(fail on mismatch, check at boot) regardless of what was written.', -}); - /** * `datasource.readReplicas` — retired (#4468, ADR-0049 enforce-or-remove). * @@ -259,91 +165,6 @@ const RETIRED_READ_REPLICAS = + '`config` at that endpoint, which is the only read-scaling path that works today. ' + 'Run `os migrate meta --from 16` to rewrite it automatically.'; -const datasourceUnknownKeyError = strictUnknownKeyError({ - surface: 'this datasource', - knownKeys: DATASOURCE_KEYS, - aliases: { - type: 'driver', - connection: 'config', - connectionconfig: 'config', - options: 'config', - enabled: 'active', - pooling: 'pool', - mode: 'schemaMode', - schema_mode: 'schemaMode', - federation: 'external', - tls: 'ssl', - }, - guidance: { - host: belongsInConfig('host'), - port: belongsInConfig('port'), - database: belongsInConfig('database'), - user: belongsInConfig('user', 'username'), - username: belongsInConfig('username'), - filename: belongsInConfig('filename'), - url: belongsInConfig('url'), - connectionString: belongsInConfig('connectionString', 'url'), - password: - '`password` must never be inlined on a datasource. Interpolate it from the environment ' - + 'inside `config`, or for an external datasource reference the secrets store via ' - + '`external.credentialsRef`.', - readReplicas: RETIRED_READ_REPLICAS, - replicas: RETIRED_READ_REPLICAS, - capabilities: RETIRED_CAPABILITIES.capabilities, - readOnly: RETIRED_CAPABILITIES.readOnly, - retryPolicy: RETIRED_DATASOURCE_BLOCKS.retryPolicy, - retry: RETIRED_DATASOURCE_BLOCKS.retryPolicy, - healthCheck: RETIRED_DATASOURCE_BLOCKS.healthCheck, - healthcheck: RETIRED_DATASOURCE_BLOCKS.healthCheck, - }, - history: - 'Until #4001 these were dropped silently — a connection key written one level too high ' - + 'left the datasource connecting on driver defaults rather than failing.', -}); - -const poolUnknownKeyError = strictUnknownKeyError({ - surface: "this datasource's pool config", - knownKeys: POOL_KEYS, - aliases: { - minimum: 'min', - maximum: 'max', - minconnections: 'min', - maxconnections: 'max', - idletimeout: 'idleTimeoutMillis', - idletimeoutms: 'idleTimeoutMillis', - connectiontimeout: 'connectionTimeoutMillis', - connectiontimeoutms: 'connectionTimeoutMillis', - acquiretimeoutmillis: 'connectionTimeoutMillis', - }, - history: - 'Until #4001 these were dropped silently — the pool ran on its defaults (min 0, max 10) ' - + 'no matter what was written. Note both timeouts end in `Millis`, not `Ms`.', -}); - -const sslUnknownKeyError = strictUnknownKeyError({ - surface: "this datasource's ssl config", - knownKeys: SSL_KEYS, - aliases: { - active: 'enabled', - ssl: 'enabled', - tls: 'enabled', - rejectunauthorised: 'rejectUnauthorized', - cacert: 'ca', - certificate: 'cert', - clientcert: 'cert', - privatekey: 'key', - clientkey: 'key', - }, - guidance: { - insecure: - '`insecure` is not an ssl key. To accept a self-signed certificate set ' - + '`rejectUnauthorized: false` — deliberately, and never against a production database.', - }, - history: - 'Until #4001 these were dropped silently — which meant a TLS setting that never took ' - + 'effect looked identical to one that did.', -}); - export const DriverType = z.string().describe('Underlying driver identifier'); /** @@ -351,7 +172,23 @@ export const DriverType = z.string().describe('Underlying driver identifier'); * Metadata describing a Database Driver. * Plugins use this to register new connectivity options. */ -export const DriverDefinitionSchema = lazySchema(() => z.object({ +export const DriverDefinitionSchema = lazySchema(() => strictObject( + { + surface: 'this driver definition', + aliases: { + name: 'id', + driver: 'id', + title: 'label', + config: 'configSchema', + schema: 'configSchema', + }, + guidance: { + capabilities: RETIRED_CAPABILITIES.capabilities, + capability: RETIRED_CAPABILITIES.capabilities, + }, + history: 'Until #4001 these were dropped silently — the driver still registered.', + }, + { id: z.string().describe('Unique driver identifier (e.g. "postgres")'), label: z.string().describe('Display label (e.g. "PostgreSQL")'), description: z.string().optional(), @@ -374,7 +211,7 @@ export const DriverDefinitionSchema = lazySchema(() => z.object({ */ configSchema: z.record(z.string(), z.unknown()).describe('JSON Schema for connection configuration'), -}, { error: driverDefinitionUnknownKeyError }).strict()); +})); /** A driver definition — {@link DriverDefinitionSchema}'s parsed shape. */ export type DriverDefinition = z.input; @@ -403,25 +240,75 @@ export type SchemaMode = z.input; * policy for a mature external database: write gating, schema whitelist, * boot/drift validation behaviour, credentials reference, and query caps. */ -export const ExternalDatasourceSettingsSchema = z.object({ +export const ExternalDatasourceSettingsSchema = strictObject( + { + surface: "this datasource's external settings", + aliases: { + schemas: 'allowedSchemas', + allowedschema: 'allowedSchemas', + writable: 'allowWrites', + allowwrite: 'allowWrites', + credentials: 'credentialsRef', + secretref: 'credentialsRef', + timeoutms: 'queryTimeoutMs', + querytimeout: 'queryTimeoutMs', + }, + guidance: { + label: RETIRED_DATASOURCE_BLOCKS.externalLabel, + requirePermission: RETIRED_DATASOURCE_BLOCKS.externalRequirePermission, + permission: RETIRED_DATASOURCE_BLOCKS.externalRequirePermission, + password: + '`password` must never be inlined. Put the secret in the secrets store and reference ' + + 'it with `credentialsRef` (e.g. `credentialsRef: "secret:warehouse/password"`).', + // #4487 corrected the second half of this line. It used to offer + // `capabilities.readOnly` as the place to "describe the driver" — a key the + // liveness audit found has NO reader (liveness/datasource.json), so an + // author who took the advice believed they had marked a datasource + // non-writable and had not. Same defect as the pre-#4410 `belongsInConfig` + // line documented above, on a property whose whole point is safety: a + // prescription must land somewhere enforced, and `allowWrites` is the only + // write gate there is. + readOnly: + '`readOnly` is not an external-settings key. Use `allowWrites: false` here — it is the ' + + 'enforced datasource-wide write gate (checked by the ObjectQL engine before any write ' + + 'to a federated datasource).', + }, + history: 'Until #4001 these were dropped silently — federation ran on the defaults instead.', + }, + { allowedSchemas: z.array(z.string()).optional() .describe('Whitelist of remote schemas/databases that may be exposed.'), allowWrites: z.boolean().default(false) .describe('Global write gate. Individual objects must also opt in via object.external.writable.'), - validation: z.object({ + validation: strictObject( + { + surface: "this datasource's external validation policy", + aliases: { + onmismatch: 'onMismatch', + mismatch: 'onMismatch', + checkonboot: 'checkOnBoot', + validateonboot: 'checkOnBoot', + interval: 'checkIntervalMs', + checkinterval: 'checkIntervalMs', + }, + history: + 'Until #4001 these were dropped silently — drift checking ran on the defaults ' + + '(fail on mismatch, check at boot) regardless of what was written.', + }, + { onMismatch: z.enum(['fail', 'warn', 'ignore']).default('fail') .describe('What to do when a federated object diverges from the remote table.'), checkOnBoot: z.boolean().default(true) .describe('Validate federated objects against the remote schema at boot.'), checkIntervalMs: z.number().optional() .describe('Optional background drift-check interval in milliseconds.'), - }, { error: externalValidationUnknownKeyError }).strict() + }) .default({ onMismatch: 'fail', checkOnBoot: true }).describe('Boot/drift validation policy'), credentialsRef: z.string().optional() .describe('Reference into the secrets store; never inline credentials.'), queryTimeoutMs: z.number().default(30_000) .describe('Hard cap on per-query execution time.'), -}, { error: externalSettingsUnknownKeyError }).strict() +}) .describe('External datasource federation settings (schemaMode != "managed")'); export type ExternalDatasourceSettings = z.input; @@ -456,7 +343,48 @@ function reportDriverConfigIssues( * Datasource Schema * Represents a connection to an external data store. */ -export const DatasourceSchema = lazySchema(() => z.object({ +export const DatasourceSchema = lazySchema(() => strictObject( + { + surface: 'this datasource', + aliases: { + type: 'driver', + connection: 'config', + connectionconfig: 'config', + options: 'config', + enabled: 'active', + pooling: 'pool', + mode: 'schemaMode', + schema_mode: 'schemaMode', + federation: 'external', + tls: 'ssl', + }, + guidance: { + host: belongsInConfig('host'), + port: belongsInConfig('port'), + database: belongsInConfig('database'), + user: belongsInConfig('user', 'username'), + username: belongsInConfig('username'), + filename: belongsInConfig('filename'), + url: belongsInConfig('url'), + connectionString: belongsInConfig('connectionString', 'url'), + password: + '`password` must never be inlined on a datasource. Interpolate it from the environment ' + + 'inside `config`, or for an external datasource reference the secrets store via ' + + '`external.credentialsRef`.', + readReplicas: RETIRED_READ_REPLICAS, + replicas: RETIRED_READ_REPLICAS, + capabilities: RETIRED_CAPABILITIES.capabilities, + readOnly: RETIRED_CAPABILITIES.readOnly, + retryPolicy: RETIRED_DATASOURCE_BLOCKS.retryPolicy, + retry: RETIRED_DATASOURCE_BLOCKS.retryPolicy, + healthCheck: RETIRED_DATASOURCE_BLOCKS.healthCheck, + healthcheck: RETIRED_DATASOURCE_BLOCKS.healthCheck, + }, + history: + 'Until #4001 these were dropped silently — a connection key written one level too high ' + + 'left the datasource connecting on driver defaults rather than failing.', + }, + { /** Machine Name */ name: z.string().regex(/^[a-z_][a-z0-9_]*$/).describe('Unique datasource identifier'), @@ -477,12 +405,30 @@ export const DatasourceSchema = lazySchema(() => z.object({ * Connection Pool Configuration * Standard connection pooling settings. */ - pool: z.object({ + pool: strictObject( + { + surface: "this datasource's pool config", + aliases: { + minimum: 'min', + maximum: 'max', + minconnections: 'min', + maxconnections: 'max', + idletimeout: 'idleTimeoutMillis', + idletimeoutms: 'idleTimeoutMillis', + connectiontimeout: 'connectionTimeoutMillis', + connectiontimeoutms: 'connectionTimeoutMillis', + acquiretimeoutmillis: 'connectionTimeoutMillis', + }, + history: + 'Until #4001 these were dropped silently — the pool ran on its defaults (min 0, max 10) ' + + 'no matter what was written. Note both timeouts end in `Millis`, not `Ms`.', + }, + { min: z.number().default(0).describe('Minimum connections'), max: z.number().default(10).describe('Maximum connections'), idleTimeoutMillis: z.number().default(30000).describe('Idle timeout'), connectionTimeoutMillis: z.number().default(3000).describe('Connection establishment timeout'), - }, { error: poolUnknownKeyError }).strict().optional().describe('Connection pool settings'), + }).optional().describe('Connection pool settings'), // `readReplicas` was removed here (#4468) — see RETIRED_READ_REPLICAS. It // declared replica connections nothing opened; read/write splitting does not @@ -495,13 +441,36 @@ export const DatasourceSchema = lazySchema(() => z.object({ /** SSL/TLS Configuration */ - ssl: z.object({ + ssl: strictObject( + { + surface: "this datasource's ssl config", + aliases: { + active: 'enabled', + ssl: 'enabled', + tls: 'enabled', + rejectunauthorised: 'rejectUnauthorized', + cacert: 'ca', + certificate: 'cert', + clientcert: 'cert', + privatekey: 'key', + clientkey: 'key', + }, + guidance: { + insecure: + '`insecure` is not an ssl key. To accept a self-signed certificate set ' + + '`rejectUnauthorized: false` — deliberately, and never against a production database.', + }, + history: + 'Until #4001 these were dropped silently — which meant a TLS setting that never took ' + + 'effect looked identical to one that did.', + }, + { enabled: z.boolean().default(false).describe('Enable SSL/TLS for database connection'), rejectUnauthorized: z.boolean().default(true).describe('Reject connections with invalid/self-signed certificates'), ca: z.string().optional().describe('CA certificate (PEM format or path to file)'), cert: z.string().optional().describe('Client certificate (PEM format or path to file)'), key: z.string().optional().describe('Client private key (PEM format or path to file)'), - }, { error: sslUnknownKeyError }).strict().optional().describe('SSL/TLS configuration for secure database connections'), + }).optional().describe('SSL/TLS configuration for secure database connections'), /** Description */ description: z.string().optional().describe('Internal description'), @@ -557,7 +526,7 @@ export const DatasourceSchema = lazySchema(() => z.object({ // same one `permission` hit as a 422 on the ADR-0094 overlay path before // Tier-A declared them (#4001 findings log, entries 2/8). ...MetadataProtectionFields, -}, { error: datasourceUnknownKeyError }).strict().superRefine((ds, ctx) => { +}).superRefine((ds, ctx) => { // The `config` gate (#4410). `config` is parsed against the contract for the // declared driver and every issue is re-pathed under the slot it came from — // the author sees `config.hostname`, not a detached message. diff --git a/packages/spec/src/data/driver/memory.zod.ts b/packages/spec/src/data/driver/memory.zod.ts index e445611d3c..b50600a8a9 100644 --- a/packages/spec/src/data/driver/memory.zod.ts +++ b/packages/spec/src/data/driver/memory.zod.ts @@ -2,7 +2,7 @@ import { z } from 'zod'; -import { strictUnknownKeyError } from '../../shared/suggestions.zod'; +import { strictObject } from '../../shared/strict-object'; import type { DriverDefinition } from '../datasource.zod'; import { driverConfigJsonSchema, @@ -167,51 +167,48 @@ export const MemoryPersistenceConfigSchema = lazySchema(() => z.union([ // 2. Connection Configuration // ========================================================================== -const MEMORY_CONFIG_KEYS = ['initialData', 'strictMode', 'persistence'] as const; - -/** - * Two keys were declared here and read by nobody: `indexes` and - * `maxRecordsPerObject`. `InMemoryDriverConfig` (`driver-memory`) has no field - * for either — the driver indexes nothing (its reads are a linear Mingo scan) - * and evicts nothing (there is no LRU) — so an author who bounded a store or - * asked for an index got a clean parse and no behaviour. #4410's enforce step - * is what surfaced them: giving `config` a gate means every key inside it now - * claims to be honoured, so a key that is not gets removed rather than blessed. - * Both are rejected with the prescription below (ADR-0049 enforce-or-remove). - */ -const memoryConfigUnknownKeyError = strictUnknownKeyError({ - surface: "this memory datasource's config", - knownKeys: MEMORY_CONFIG_KEYS, - aliases: { - data: 'initialData', - seed: 'initialData', - seeddata: 'initialData', - strict: 'strictMode', - persist: 'persistence', - persistent: 'persistence', - }, - guidance: { - indexes: - '`indexes` was declared but never read: the memory driver keeps no indexes — every read is ' - + 'a linear Mingo scan — so it changed nothing. Drop it, or move the datasource to a ' - + 'driver that indexes (`sqlite` / `postgres`), where object-level `indexes` apply.', - maxRecordsPerObject: - '`maxRecordsPerObject` was declared but never read: the memory driver evicts nothing, so a ' - + 'bound here was never enforced and the store grew unbounded regardless. Drop it and bound ' - + 'the data you load, or use a driver with real storage limits.', - filename: - '`filename` is a sqlite key. For a memory datasource that survives restarts set ' - + "`persistence: 'file'` (the file is scoped per datasource); for a real file-backed SQL " - + "database set `driver: 'sqlite'`.", - schemaMode: SCHEMA_MODE_BELONGS_ON_DATASOURCE, - readOnly: READ_ONLY_BELONGS_ON_DATASOURCE, +export const MemoryConfigSchema = lazySchema(() => strictObject( + /** + * Two keys were declared here and read by nobody: `indexes` and + * `maxRecordsPerObject`. `InMemoryDriverConfig` (`driver-memory`) has no field + * for either — the driver indexes nothing (its reads are a linear Mingo scan) + * and evicts nothing (there is no LRU) — so an author who bounded a store or + * asked for an index got a clean parse and no behaviour. #4410's enforce step + * is what surfaced them: giving `config` a gate means every key inside it now + * claims to be honoured, so a key that is not gets removed rather than blessed. + * Both are rejected with the prescription below (ADR-0049 enforce-or-remove). + */ + { + surface: "this memory datasource's config", + aliases: { + data: 'initialData', + seed: 'initialData', + seeddata: 'initialData', + strict: 'strictMode', + persist: 'persistence', + persistent: 'persistence', + }, + guidance: { + indexes: + '`indexes` was declared but never read: the memory driver keeps no indexes — every read is ' + + 'a linear Mingo scan — so it changed nothing. Drop it, or move the datasource to a ' + + 'driver that indexes (`sqlite` / `postgres`), where object-level `indexes` apply.', + maxRecordsPerObject: + '`maxRecordsPerObject` was declared but never read: the memory driver evicts nothing, so a ' + + 'bound here was never enforced and the store grew unbounded regardless. Drop it and bound ' + + 'the data you load, or use a driver with real storage limits.', + filename: + '`filename` is a sqlite key. For a memory datasource that survives restarts set ' + + "`persistence: 'file'` (the file is scoped per datasource); for a real file-backed SQL " + + "database set `driver: 'sqlite'`.", + schemaMode: SCHEMA_MODE_BELONGS_ON_DATASOURCE, + readOnly: READ_ONLY_BELONGS_ON_DATASOURCE, + }, + history: + 'Until #4410 nothing validated `datasource.config` at all — an unrecognised key was accepted ' + + 'in silence and the store came up on the driver defaults instead.', }, - history: - 'Until #4410 nothing validated `datasource.config` at all — an unrecognised key was accepted ' - + 'in silence and the store came up on the driver defaults instead.', -}); - -export const MemoryConfigSchema = lazySchema(() => z.object({ + { /** * Initial data to pre-populate the in-memory store. * Maps object/table names to arrays of records. @@ -291,7 +288,7 @@ export const MemoryConfigSchema = lazySchema(() => z.object({ * so two pools that DO opt in still need it to avoid aliasing one file. */ persistence: MemoryPersistenceConfigSchema.or(z.literal(false)).default(false).describe('Persistence configuration (opt-in; defaults to pure in-memory)'), -}, { error: memoryConfigUnknownKeyError }).strict() +}) .describe('Memory Driver Connection Configuration')); /** diff --git a/packages/spec/src/data/driver/mongo.zod.ts b/packages/spec/src/data/driver/mongo.zod.ts index c2bcc08db8..312596f33a 100644 --- a/packages/spec/src/data/driver/mongo.zod.ts +++ b/packages/spec/src/data/driver/mongo.zod.ts @@ -3,7 +3,7 @@ import { z } from 'zod'; import { lazySchema } from '../../shared/lazy-schema'; -import { strictUnknownKeyError } from '../../shared/suggestions.zod'; +import { strictObject } from '../../shared/strict-object'; import type { DriverDefinition } from '../datasource.zod'; import { driverConfigJsonSchema, @@ -29,46 +29,41 @@ import { // 1. Connection Configuration // ========================================================================== -const MONGO_CONFIG_KEYS = [ - 'url', 'host', 'port', 'database', 'username', 'password', 'authSource', 'options', -] as const; - -const mongoConfigUnknownKeyError = strictUnknownKeyError({ - surface: "this mongo datasource's config", - knownKeys: MONGO_CONFIG_KEYS, - aliases: { - uri: 'url', - connectionstring: 'url', - dsn: 'url', - hostname: 'host', - server: 'host', - dbname: 'database', - db: 'database', - user: 'username', - passwd: 'password', - pwd: 'password', - authdb: 'authSource', - authdatabase: 'authSource', - replicaset: 'options', +export const MongoConfigSchema = lazySchema(() => strictObject( + { + surface: "this mongo datasource's config", + aliases: { + uri: 'url', + connectionstring: 'url', + dsn: 'url', + hostname: 'host', + server: 'host', + dbname: 'database', + db: 'database', + user: 'username', + passwd: 'password', + pwd: 'password', + authdb: 'authSource', + authdatabase: 'authSource', + replicaset: 'options', + }, + guidance: { + pool: + '`pool` is not driver config — connection pooling is configured once for every driver in ' + + "the datasource's own `pool` block, which the factory maps onto the Mongo client's " + + '`minPoolSize`/`maxPoolSize`. Move it next to `driver`.', + schemaMode: SCHEMA_MODE_BELONGS_ON_DATASOURCE, + readOnly: READ_ONLY_BELONGS_ON_DATASOURCE, + ssl: + '`ssl` is not a top-level mongo key. TLS is a connection-string concern here: put it in ' + + '`url` (`?tls=true`) or in the `options` passthrough the Mongo client reads.', + }, + history: + 'Until #4410 nothing validated `datasource.config` at all — an unrecognised connection key ' + + 'was accepted in silence and the datasource then connected to mongodb://localhost:27017 ' + + 'rather than failing.', }, - guidance: { - pool: - '`pool` is not driver config — connection pooling is configured once for every driver in ' - + "the datasource's own `pool` block, which the factory maps onto the Mongo client's " - + '`minPoolSize`/`maxPoolSize`. Move it next to `driver`.', - schemaMode: SCHEMA_MODE_BELONGS_ON_DATASOURCE, - readOnly: READ_ONLY_BELONGS_ON_DATASOURCE, - ssl: - '`ssl` is not a top-level mongo key. TLS is a connection-string concern here: put it in ' - + '`url` (`?tls=true`) or in the `options` passthrough the Mongo client reads.', - }, - history: - 'Until #4410 nothing validated `datasource.config` at all — an unrecognised connection key ' - + 'was accepted in silence and the datasource then connected to mongodb://localhost:27017 ' - + 'rather than failing.', -}); - -export const MongoConfigSchema = lazySchema(() => z.object({ + { /** * Connection URI (standard connection string). When present it supersedes * `host`/`port`/`database`/`username`/`authSource` — those are only used to @@ -111,7 +106,7 @@ export const MongoConfigSchema = lazySchema(() => z.object({ */ options: z.record(z.string(), z.unknown()).optional() .describe('Extra MongoClient options (replicaSet, tls, timeouts, …)'), -}, { error: mongoConfigUnknownKeyError }).strict() +}) .describe('MongoDB Connection Configuration') .superRefine((cfg, ctx) => { if (!cfg.url && !cfg.database) { diff --git a/packages/spec/src/data/driver/mysql.zod.ts b/packages/spec/src/data/driver/mysql.zod.ts index 356614ba2e..aacc07e3ec 100644 --- a/packages/spec/src/data/driver/mysql.zod.ts +++ b/packages/spec/src/data/driver/mysql.zod.ts @@ -18,7 +18,7 @@ import { z } from 'zod'; import { lazySchema } from '../../shared/lazy-schema'; -import { strictUnknownKeyError } from '../../shared/suggestions.zod'; +import { strictObject } from '../../shared/strict-object'; import { driverConfigJsonSchema, DriverSslToggleSchema, @@ -28,51 +28,46 @@ import { SSL_DETAIL_BELONGS_ON_DATASOURCE, } from './common.zod'; -const MYSQL_CONFIG_KEYS = [ - 'url', 'host', 'port', 'database', 'username', 'password', 'ssl', 'autoMigrate', -] as const; - -const mysqlConfigUnknownKeyError = strictUnknownKeyError({ - surface: "this mysql datasource's config", - knownKeys: MYSQL_CONFIG_KEYS, - aliases: { - hostname: 'host', - server: 'host', - dbname: 'database', - db: 'database', - schema: 'database', - user: 'username', - passwd: 'password', - pwd: 'password', - connectionstring: 'url', - dsn: 'url', - uri: 'url', - sslmode: 'ssl', - tls: 'ssl', - usessl: 'ssl', - }, - guidance: { - pool: - '`pool` is not driver config — connection pooling is configured once for every driver in ' - + "the datasource's own `pool` block. Move it next to `driver`.", - schemaMode: SCHEMA_MODE_BELONGS_ON_DATASOURCE, - readOnly: READ_ONLY_BELONGS_ON_DATASOURCE, - ca: SSL_DETAIL_BELONGS_ON_DATASOURCE, - cert: SSL_DETAIL_BELONGS_ON_DATASOURCE, - key: SSL_DETAIL_BELONGS_ON_DATASOURCE, - rejectUnauthorized: SSL_DETAIL_BELONGS_ON_DATASOURCE, - charset: - '`charset` is not honoured: the factory builds the mysql2 connection from the keys listed ' - + 'here only. Put it in the `url` as a query parameter (`?charset=utf8mb4`) so the client ' - + 'actually receives it.', +export const MysqlConfigSchema = lazySchema(() => strictObject( + { + surface: "this mysql datasource's config", + aliases: { + hostname: 'host', + server: 'host', + dbname: 'database', + db: 'database', + schema: 'database', + user: 'username', + passwd: 'password', + pwd: 'password', + connectionstring: 'url', + dsn: 'url', + uri: 'url', + sslmode: 'ssl', + tls: 'ssl', + usessl: 'ssl', + }, + guidance: { + pool: + '`pool` is not driver config — connection pooling is configured once for every driver in ' + + "the datasource's own `pool` block. Move it next to `driver`.", + schemaMode: SCHEMA_MODE_BELONGS_ON_DATASOURCE, + readOnly: READ_ONLY_BELONGS_ON_DATASOURCE, + ca: SSL_DETAIL_BELONGS_ON_DATASOURCE, + cert: SSL_DETAIL_BELONGS_ON_DATASOURCE, + key: SSL_DETAIL_BELONGS_ON_DATASOURCE, + rejectUnauthorized: SSL_DETAIL_BELONGS_ON_DATASOURCE, + charset: + '`charset` is not honoured: the factory builds the mysql2 connection from the keys listed ' + + 'here only. Put it in the `url` as a query parameter (`?charset=utf8mb4`) so the client ' + + 'actually receives it.', + }, + history: + 'Until #4410 nothing validated `datasource.config` at all — an unrecognised connection key ' + + 'was accepted in silence and the datasource then connected on the client defaults ' + + '(localhost:3306) rather than failing.', }, - history: - 'Until #4410 nothing validated `datasource.config` at all — an unrecognised connection key ' - + 'was accepted in silence and the datasource then connected on the client defaults ' - + '(localhost:3306) rather than failing.', -}); - -export const MysqlConfigSchema = lazySchema(() => z.object({ + { /** * Connection URI, passed to `mysql2` as-is when present. * Format: `mysql://[user[:password]@][host][:port]/[dbname][?params]` @@ -105,7 +100,7 @@ export const MysqlConfigSchema = lazySchema(() => z.object({ /** Dev-only, loosen-only schema self-heal (#2186). */ autoMigrate: SqlAutoMigrateSchema.optional(), -}, { error: mysqlConfigUnknownKeyError }).strict() +}) .describe('MySQL / MariaDB connection configuration') .superRefine((cfg, ctx) => { if (!cfg.url && !cfg.database) { diff --git a/packages/spec/src/data/driver/postgres.zod.ts b/packages/spec/src/data/driver/postgres.zod.ts index 119402d8d6..437ca9debf 100644 --- a/packages/spec/src/data/driver/postgres.zod.ts +++ b/packages/spec/src/data/driver/postgres.zod.ts @@ -16,7 +16,7 @@ import { z } from 'zod'; import { lazySchema } from '../../shared/lazy-schema'; -import { strictUnknownKeyError } from '../../shared/suggestions.zod'; +import { strictObject } from '../../shared/strict-object'; import { driverConfigJsonSchema, DriverSslToggleSchema, @@ -26,61 +26,55 @@ import { SSL_DETAIL_BELONGS_ON_DATASOURCE, } from './common.zod'; -const POSTGRES_CONFIG_KEYS = [ - 'url', 'host', 'port', 'database', 'username', 'password', 'ssl', - 'schema', 'applicationName', 'statementTimeout', 'autoMigrate', -] as const; - /** Prescription for a pool knob written inside `config` instead of `pool`. */ const poolBelongsOnDatasource = (key: string, canonical: string) => `\`${key}\` is not driver config — connection pooling is configured once for every driver in ` + `the datasource's own \`pool\` block. Move it to \`pool: { ${canonical}: … }\`. ` + `(It was declared here and read by nothing until #4410.)`; -const postgresConfigUnknownKeyError = strictUnknownKeyError({ - surface: "this postgres datasource's config", - knownKeys: POSTGRES_CONFIG_KEYS, - aliases: { - hostname: 'host', - server: 'host', - dbname: 'database', - db: 'database', - user: 'username', - passwd: 'password', - pwd: 'password', - connectionstring: 'url', - dsn: 'url', - uri: 'url', - searchpath: 'schema', - applicationname: 'applicationName', - statementtimeout: 'statementTimeout', - sslmode: 'ssl', - tls: 'ssl', - usessl: 'ssl', - }, - guidance: { - pool: poolBelongsOnDatasource('pool', 'max'), - min: poolBelongsOnDatasource('min', 'min'), - max: poolBelongsOnDatasource('max', 'max'), - idleTimeoutMillis: poolBelongsOnDatasource('idleTimeoutMillis', 'idleTimeoutMillis'), - connectionTimeoutMillis: poolBelongsOnDatasource( - 'connectionTimeoutMillis', - 'connectionTimeoutMillis', - ), - schemaMode: SCHEMA_MODE_BELONGS_ON_DATASOURCE, - readOnly: READ_ONLY_BELONGS_ON_DATASOURCE, - ca: SSL_DETAIL_BELONGS_ON_DATASOURCE, - cert: SSL_DETAIL_BELONGS_ON_DATASOURCE, - key: SSL_DETAIL_BELONGS_ON_DATASOURCE, - rejectUnauthorized: SSL_DETAIL_BELONGS_ON_DATASOURCE, +export const PostgresConfigSchema = lazySchema(() => strictObject( + { + surface: "this postgres datasource's config", + aliases: { + hostname: 'host', + server: 'host', + dbname: 'database', + db: 'database', + user: 'username', + passwd: 'password', + pwd: 'password', + connectionstring: 'url', + dsn: 'url', + uri: 'url', + searchpath: 'schema', + applicationname: 'applicationName', + statementtimeout: 'statementTimeout', + sslmode: 'ssl', + tls: 'ssl', + usessl: 'ssl', + }, + guidance: { + pool: poolBelongsOnDatasource('pool', 'max'), + min: poolBelongsOnDatasource('min', 'min'), + max: poolBelongsOnDatasource('max', 'max'), + idleTimeoutMillis: poolBelongsOnDatasource('idleTimeoutMillis', 'idleTimeoutMillis'), + connectionTimeoutMillis: poolBelongsOnDatasource( + 'connectionTimeoutMillis', + 'connectionTimeoutMillis', + ), + schemaMode: SCHEMA_MODE_BELONGS_ON_DATASOURCE, + readOnly: READ_ONLY_BELONGS_ON_DATASOURCE, + ca: SSL_DETAIL_BELONGS_ON_DATASOURCE, + cert: SSL_DETAIL_BELONGS_ON_DATASOURCE, + key: SSL_DETAIL_BELONGS_ON_DATASOURCE, + rejectUnauthorized: SSL_DETAIL_BELONGS_ON_DATASOURCE, + }, + history: + 'Until #4410 nothing validated `datasource.config` at all — an unrecognised connection key ' + + 'was accepted in silence and the datasource then connected on the client defaults ' + + "(localhost:5432), which is #4001's original bug one level down.", }, - history: - 'Until #4410 nothing validated `datasource.config` at all — an unrecognised connection key ' - + 'was accepted in silence and the datasource then connected on the client defaults ' - + "(localhost:5432), which is #4001's original bug one level down.", -}); - -export const PostgresConfigSchema = lazySchema(() => z.object({ + { /** * Connection URI. When present it supersedes `host`/`port`/`database`/ * `username`, and a datasource secret (`external.credentialsRef`) still @@ -129,7 +123,7 @@ export const PostgresConfigSchema = lazySchema(() => z.object({ /** Dev-only, loosen-only schema self-heal (#2186). */ autoMigrate: SqlAutoMigrateSchema.optional(), -}, { error: postgresConfigUnknownKeyError }).strict() +}) .describe('PostgreSQL connection configuration') .superRefine((cfg, ctx) => { if (!cfg.url && !cfg.database) { diff --git a/packages/spec/src/data/driver/sqlite.zod.ts b/packages/spec/src/data/driver/sqlite.zod.ts index e680eb1895..f5863a10c9 100644 --- a/packages/spec/src/data/driver/sqlite.zod.ts +++ b/packages/spec/src/data/driver/sqlite.zod.ts @@ -20,7 +20,7 @@ import { z } from 'zod'; import { lazySchema } from '../../shared/lazy-schema'; -import { strictUnknownKeyError } from '../../shared/suggestions.zod'; +import { strictObject } from '../../shared/strict-object'; import { driverConfigJsonSchema, READ_ONLY_BELONGS_ON_DATASOURCE, @@ -28,8 +28,6 @@ import { SqlAutoMigrateSchema, } from './common.zod'; -const SQLITE_CONFIG_KEYS = ['filename', 'autoMigrate'] as const; - const FILENAME_ALIASES = { file: 'filename', filepath: 'filename', @@ -49,22 +47,21 @@ const IN_MEMORY_GUIDANCE = '`memory` is not a sqlite key. An ephemeral database is `filename: \':memory:\'`; for the ' + 'mingo in-memory engine (a different driver entirely) set `driver: \'memory\'`.'; -const sqliteConfigUnknownKeyError = strictUnknownKeyError({ - surface: "this sqlite datasource's config", - knownKeys: SQLITE_CONFIG_KEYS, - aliases: FILENAME_ALIASES, - guidance: { - schemaMode: SCHEMA_MODE_BELONGS_ON_DATASOURCE, - readOnly: READ_ONLY_BELONGS_ON_DATASOURCE, - memory: IN_MEMORY_GUIDANCE, - persist: - '`persist` is a `sqlite-wasm` key — the native sqlite driver writes through on every ' - + "statement and has nothing to schedule. Set `driver: 'sqlite-wasm'` to use it.", +export const SqliteConfigSchema = lazySchema(() => strictObject( + { + surface: "this sqlite datasource's config", + aliases: FILENAME_ALIASES, + guidance: { + schemaMode: SCHEMA_MODE_BELONGS_ON_DATASOURCE, + readOnly: READ_ONLY_BELONGS_ON_DATASOURCE, + memory: IN_MEMORY_GUIDANCE, + persist: + '`persist` is a `sqlite-wasm` key — the native sqlite driver writes through on every ' + + "statement and has nothing to schedule. Set `driver: 'sqlite-wasm'` to use it.", + }, + history: sqliteHistory, }, - history: sqliteHistory, -}); - -export const SqliteConfigSchema = lazySchema(() => z.object({ + { /** * Database file path, or `:memory:` for an ephemeral in-process database. * A relative path resolves against the server's working directory. @@ -75,7 +72,7 @@ export const SqliteConfigSchema = lazySchema(() => z.object({ /** Dev-only, loosen-only schema self-heal (#2186). */ autoMigrate: SqlAutoMigrateSchema.optional(), -}, { error: sqliteConfigUnknownKeyError }).strict() +}) .describe('SQLite connection configuration')); export type SqliteConfig = z.input; @@ -97,24 +94,21 @@ export const SqliteWasmPersistModeSchema = z.union([ export type SqliteWasmPersistMode = z.input; -const SQLITE_WASM_CONFIG_KEYS = ['filename', 'persist'] as const; - -const sqliteWasmConfigUnknownKeyError = strictUnknownKeyError({ - surface: "this sqlite-wasm datasource's config", - knownKeys: SQLITE_WASM_CONFIG_KEYS, - aliases: FILENAME_ALIASES, - guidance: { - schemaMode: SCHEMA_MODE_BELONGS_ON_DATASOURCE, - readOnly: READ_ONLY_BELONGS_ON_DATASOURCE, - memory: IN_MEMORY_GUIDANCE, - autoMigrate: - '`autoMigrate` is honoured by the native sqlite / postgres / mysql drivers only — the ' - + 'wasm driver is constructed without it, so writing it here would change nothing.', +export const SqliteWasmConfigSchema = lazySchema(() => strictObject( + { + surface: "this sqlite-wasm datasource's config", + aliases: FILENAME_ALIASES, + guidance: { + schemaMode: SCHEMA_MODE_BELONGS_ON_DATASOURCE, + readOnly: READ_ONLY_BELONGS_ON_DATASOURCE, + memory: IN_MEMORY_GUIDANCE, + autoMigrate: + '`autoMigrate` is honoured by the native sqlite / postgres / mysql drivers only — the ' + + 'wasm driver is constructed without it, so writing it here would change nothing.', + }, + history: sqliteHistory, }, - history: sqliteHistory, -}); - -export const SqliteWasmConfigSchema = lazySchema(() => z.object({ + { /** * Database file path, or `:memory:` for an ephemeral in-process database. * A file-backed wasm database persists according to {@link SqliteWasmPersistModeSchema}. @@ -128,7 +122,7 @@ export const SqliteWasmConfigSchema = lazySchema(() => z.object({ * filename is given; `:memory:` never persists. */ persist: SqliteWasmPersistModeSchema.optional().meta({ title: 'Persist mode' }), -}, { error: sqliteWasmConfigUnknownKeyError }).strict() +}) .describe('SQLite (WASM) connection configuration')); export type SqliteWasmConfig = z.input; diff --git a/packages/spec/src/data/hook-body.zod.ts b/packages/spec/src/data/hook-body.zod.ts index 03387ccbf2..0c91131c08 100644 --- a/packages/spec/src/data/hook-body.zod.ts +++ b/packages/spec/src/data/hook-body.zod.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { z } from 'zod'; -import { strictUnknownKeyError } from '../shared/suggestions.zod'; +import { strictObject } from '../shared/strict-object'; // Retired token prescription. Declared with `//` (never `/** */`) and ABOVE the // capability enum's JSDoc on purpose — see the placement note below: build-docs @@ -86,48 +86,10 @@ export type HookBodyCapability = z.input; * Keep declarations that carry JSDoc below the first exported symbol here. */ -/** Keys {@link ExpressionBodySchema} declares (drift-guarded by hook-body.test.ts). */ -const EXPRESSION_BODY_KEYS = ['language', 'source'] as const; - -/** Keys {@link ScriptBodySchema} declares (drift-guarded by hook-body.test.ts). */ -const SCRIPT_BODY_KEYS = ['language', 'source', 'capabilities', 'timeoutMs', 'memoryMb'] as const; - const L2_ONLY_ON_L1 = 'is an L2 key — it only applies to `language: "js"`. An expression body is a pure ' + 'formula: it performs no IO, so it has nothing to grant and no sandbox to bound.'; -const expressionBodyUnknownKeyError = strictUnknownKeyError({ - surface: 'this expression (L1) hook body', - knownKeys: EXPRESSION_BODY_KEYS, - aliases: { expression: 'source', formula: 'source', code: 'source', script: 'source' }, - guidance: { - capabilities: `\`capabilities\` ${L2_ONLY_ON_L1}`, - timeoutMs: `\`timeoutMs\` ${L2_ONLY_ON_L1}`, - memoryMb: `\`memoryMb\` ${L2_ONLY_ON_L1}`, - }, - history: 'Until #4001 these were dropped silently.', -}); - -const scriptBodyUnknownKeyError = strictUnknownKeyError({ - surface: 'this sandboxed JS (L2) hook body', - knownKeys: SCRIPT_BODY_KEYS, - aliases: { - capability: 'capabilities', - caps: 'capabilities', - permissions: 'capabilities', - timeout: 'timeoutMs', - timeoutms: 'timeoutMs', - memory: 'memoryMb', - memorymb: 'memoryMb', - code: 'source', - script: 'source', - body: 'source', - }, - history: - 'Until #4001 these were dropped silently — the body still ran, just not under the ' - + 'limits or grants that were written.', -}); - /** * L1 — Pure expression body. * @@ -139,11 +101,22 @@ const scriptBodyUnknownKeyError = strictUnknownKeyError({ * { "language": "expression", "source": "input.amount > 1000 && input.status == 'open'" } * ``` */ -export const ExpressionBodySchema = z.object({ +export const ExpressionBodySchema = strictObject( + { + surface: 'this expression (L1) hook body', + aliases: { expression: 'source', formula: 'source', code: 'source', script: 'source' }, + guidance: { + capabilities: `\`capabilities\` ${L2_ONLY_ON_L1}`, + timeoutMs: `\`timeoutMs\` ${L2_ONLY_ON_L1}`, + memoryMb: `\`memoryMb\` ${L2_ONLY_ON_L1}`, + }, + history: 'Until #4001 these were dropped silently.', + }, + { language: z.literal('expression'), /** Formula-engine expression. Pure, side-effect-free. */ source: z.string().min(1).describe('Formula expression source'), -}, { error: expressionBodyUnknownKeyError }).strict().describe('L1 expression body — pure formula, no IO'); +}).describe('L1 expression body — pure formula, no IO'); export type ExpressionBody = z.input; /** @@ -201,7 +174,26 @@ export type ExpressionBody = z.input; * } * ``` */ -export const ScriptBodySchema = z.object({ +export const ScriptBodySchema = strictObject( + { + surface: 'this sandboxed JS (L2) hook body', + aliases: { + capability: 'capabilities', + caps: 'capabilities', + permissions: 'capabilities', + timeout: 'timeoutMs', + timeoutms: 'timeoutMs', + memory: 'memoryMb', + memorymb: 'memoryMb', + code: 'source', + script: 'source', + body: 'source', + }, + history: + 'Until #4001 these were dropped silently — the body still ran, just not under the ' + + 'limits or grants that were written.', + }, + { language: z.literal('js'), /** Function body source (NOT a full module — no top-level imports). */ source: z.string().min(1).describe('Function body source'), @@ -221,7 +213,7 @@ export const ScriptBodySchema = z.object({ * Subject to engine support (isolated-vm enforces, quickjs approximates). */ memoryMb: z.number().int().positive().max(256).optional().describe('Per-invocation memory cap (MB)'), -}, { error: scriptBodyUnknownKeyError }).strict().describe('L2 sandboxed JS body — runs inside an isolated VM with declared capabilities'); +}).describe('L2 sandboxed JS body — runs inside an isolated VM with declared capabilities'); export type ScriptBody = z.input; /** Post-parse shape of {@link ScriptBody} — defaults applied, transforms run (ADR-0122). */ export type ScriptBodyParsed = z.infer; diff --git a/packages/spec/src/data/hook.zod.ts b/packages/spec/src/data/hook.zod.ts index 4a0f2e4c04..ea38b2fae3 100644 --- a/packages/spec/src/data/hook.zod.ts +++ b/packages/spec/src/data/hook.zod.ts @@ -9,7 +9,7 @@ import { ExpressionInputSchema } from '../shared/expression.zod'; */ import { lazySchema } from '../shared/lazy-schema'; import { retiredKey } from '../shared/retired-key'; -import { strictUnknownKeyError } from '../shared/suggestions.zod'; +import { strictObject } from '../shared/strict-object'; import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; import { HookBodySchema } from './hook-body.zod'; // Type-only, and it must stay that way: `contracts/` already imports `data/` @@ -68,63 +68,6 @@ const hookTargetError = + "`object: 'account'` or `object: ['account', 'contact']` — or, if firing on " + "every object really is the intent, write the wildcard explicitly: `object: '*'`."; -/** Keys {@link HookSchema} declares (drift-guarded by hook.test.ts). */ -const HOOK_KEYS = [ - 'name', 'label', 'object', 'events', 'handler', 'body', 'priority', - 'async', 'condition', 'description', 'retryPolicy', 'timeout', 'onError', -] as const; - -/** Keys the hook `retryPolicy` block declares (drift-guarded by hook.test.ts). */ -const HOOK_RETRY_POLICY_KEYS = ['maxRetries', 'backoffMs'] as const; - -const hookUnknownKeyError = strictUnknownKeyError({ - surface: 'this hook', - knownKeys: HOOK_KEYS, - aliases: { - hookname: 'name', - objectname: 'object', - objects: 'object', - event: 'events', - fn: 'handler', - callback: 'handler', - order: 'priority', - sequence: 'priority', - background: 'async', - isasync: 'async', - when: 'condition', - predicate: 'condition', - retry: 'retryPolicy', - timeoutms: 'timeout', - errorpolicy: 'onError', - onfailure: 'onError', - }, - guidance: { - enabled: - '`enabled` is not a hook key — a hook has no on/off switch. Gate it with `condition` ' - + '(the hook is skipped when the predicate is false), or remove the hook.', - active: - '`active` is not a hook key — a hook has no on/off switch. Gate it with `condition`, ' - + 'or remove the hook.', - }, - history: 'Until #4001 these were dropped silently — the hook still registered and ran.', -}); - -const hookRetryPolicyUnknownKeyError = strictUnknownKeyError({ - surface: "this hook's retryPolicy", - knownKeys: HOOK_RETRY_POLICY_KEYS, - aliases: { - retries: 'maxRetries', - attempts: 'maxRetries', - basedelayms: 'backoffMs', - backoff: 'backoffMs', - delayms: 'backoffMs', - }, - history: - 'Until #4001 these were dropped silently — the hook retried on the defaults rather ' - + 'than the policy that was written. Note a datasource retryPolicy spells its delay ' - + '`baseDelayMs`; a hook spells it `backoffMs`.', -}); - export const HookEvent = z.enum([ // Read — one event per read, regardless of shape. `beforeFind`/`afterFind` // fire for BOTH `find` and `findOne` (the event attaches to record @@ -165,7 +108,38 @@ export const HookEvent = z.enum([ * - Side Effects (Sending emails, Syncing to external systems) * - Security (Filtering data based on context) */ -export const HookSchema = lazySchema(() => z.object({ +export const HookSchema = lazySchema(() => strictObject( + { + surface: 'this hook', + aliases: { + hookname: 'name', + objectname: 'object', + objects: 'object', + event: 'events', + fn: 'handler', + callback: 'handler', + order: 'priority', + sequence: 'priority', + background: 'async', + isasync: 'async', + when: 'condition', + predicate: 'condition', + retry: 'retryPolicy', + timeoutms: 'timeout', + errorpolicy: 'onError', + onfailure: 'onError', + }, + guidance: { + enabled: + '`enabled` is not a hook key — a hook has no on/off switch. Gate it with `condition` ' + + '(the hook is skipped when the predicate is false), or remove the hook.', + active: + '`active` is not a hook key — a hook has no on/off switch. Gate it with `condition`, ' + + 'or remove the hook.', + }, + history: 'Until #4001 these were dropped silently — the hook still registered and ran.', + }, + { /** * Unique identifier for the hook * Required for debugging and overriding. @@ -185,8 +159,8 @@ export const HookSchema = lazySchema(() => z.object({ * - Wildcard: "*" (All objects) * * Must name at least one object. An empty target (`''`, `[]`, `['']`) is - * refused rather than widened to the wildcard — see the note above - * {@link HOOK_KEYS}. + * refused rather than widened to the wildcard — see {@link hookTargetError}, + * which carries the rejection text and the reason. */ object: z.union([z.string(), z.array(z.string())]) .refine( @@ -271,10 +245,25 @@ export const HookSchema = lazySchema(() => z.object({ /** * Retry Policy */ - retryPolicy: z.object({ + retryPolicy: strictObject( + { + surface: "this hook's retryPolicy", + aliases: { + retries: 'maxRetries', + attempts: 'maxRetries', + basedelayms: 'backoffMs', + backoff: 'backoffMs', + delayms: 'backoffMs', + }, + history: + 'Until #4001 these were dropped silently — the hook retried on the defaults rather ' + + 'than the policy that was written. Note a datasource retryPolicy spells its delay ' + + '`baseDelayMs`; a hook spells it `backoffMs`.', + }, + { maxRetries: z.number().default(3).describe('Maximum retry attempts on failure'), backoffMs: z.number().default(1000).describe('Backoff delay between retries in milliseconds'), - }, { error: hookRetryPolicyUnknownKeyError }).strict().optional().describe('Retry policy for failed hook executions'), + }).optional().describe('Retry policy for failed hook executions'), /** * Execution Timeout @@ -296,7 +285,7 @@ export const HookSchema = lazySchema(() => z.object({ // REJECTED here — the same live 422 that `permission` hit on the ADR-0094 // overlay path before Tier-A declared them (#4001 findings log, entries 2/8). ...MetadataProtectionFields, -}, { error: hookUnknownKeyError }).strict()); +})); /** * Hook Runtime Context diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index 7b7002456e..53da385766 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -12,7 +12,6 @@ import { ObjectListViewSchema } from '../ui/view.zod'; import { ExpressionInputSchema, TemplateExpressionInputSchema, type Expression, type ExpressionInput } from '../shared/expression.zod'; 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'; import { retiredKey } from '../shared/retired-key'; @@ -1045,34 +1044,6 @@ export type RowCrudActionOverride = z.input; /** Post-parse shape of {@link RowCrudActionOverride} — defaults applied, transforms run (ADR-0122). */ export type RowCrudActionOverrideParsed = z.infer; -/** - * Unknown-key error for {@link ObjectSchemaBase}, built on FIRST USE. - * - * Deferred because the pieces it needs — `UNKNOWN_KEY_GUIDANCE` and the shape's - * own key list — are declared *below* this schema, and `ObjectSchemaBase` is a - * plain `z.object` evaluated at module load. An error map only runs at parse - * time, so resolving them then sidesteps the temporal dead zone without moving - * several hundred lines around. `knownKeys` reads the shape rather than a - * transcription, for the reason `strictObject` exists. - */ -let objectUnknownKeyErrorImpl: z.core.$ZodErrorMap | undefined; -const objectUnknownKeyError: z.core.$ZodErrorMap = (issue) => - (objectUnknownKeyErrorImpl ??= strictUnknownKeyError({ - surface: 'this object', - knownKeys: Object.keys(ObjectSchemaBase.shape), - // The same semantic renames the WARNING layer already knew - // (`OBJECT_KEY_GUIDANCE`). Graduating a surface from warn to reject must - // not cost the author a prescription: `capabilities` → `enable` is a - // different word for the same intent, so edit distance cannot reach it and - // only an explicit entry can. - aliases: { capabilities: 'enable', features: 'enable' }, - guidance: UNKNOWN_KEY_GUIDANCE, - history: - 'Until #4001 closed this shape these were dropped silently on the PARSE path — ' - + '`ObjectSchema.create()` has rejected them since #1535, but `defineStack({ objects })`, ' - + '`/api/v1/meta/types/object` and the Studio form all go through `parse()`, which did not.', - }))(issue); - /* * ── Unknown-key strictness (#4001 registered-types line) ──────────────────── * @@ -1126,7 +1097,133 @@ const MANAGED_BY_SYSTEM_RETIRED = + 'is what a v16 `system` object already resolved to. Run `os migrate meta --from 16` to ' + 'rewrite it automatically.'; -const ObjectSchemaBase = z.object({ +/** + * Known-confusable schema keys → precise authoring guidance. + * + * ADR-0032's "no silent failure" principle applied to metadata *shape*: an + * unknown top-level key on `ObjectSchema.create()` used to be discarded by + * Zod's default `.strip()`, so a misauthored schema key vanished with no + * error, no warning, and a green `tsc` — shipping dead metadata the author + * believed they had wired up (issue #1535, object-level `workflows: [...]`). + * + * These entries turn the most likely mistakes into a fixable error that points + * at the *supported* mechanism rather than a generic "unknown key". + */ +const UNKNOWN_KEY_GUIDANCE: Record = { + workflows: + '`workflows` is not an ObjectSchema field. Object-level, record-triggered ' + + 'automation is authored as a lifecycle hook (`src/objects/.hook.ts`, ' + + 'wrapped in `defineHook()` from `@objectstack/spec/data`) or as a top-level ' + + '`record_change` flow — not as `workflows[]` on the object schema.', + workflow: + '`workflow` is not an ObjectSchema field. Record-triggered automation is ' + + 'authored as a lifecycle hook (`src/objects/.hook.ts`) or a top-level ' + + '`record_change` flow.', + hooks: + '`hooks` is not an ObjectSchema field. Lifecycle hooks live in their own ' + + '`src/objects/.hook.ts` module, wrapped in `defineHook()` from ' + + '`@objectstack/spec/data`.', + triggers: + '`triggers` is not an ObjectSchema field. Use a lifecycle hook ' + + '(`src/objects/.hook.ts`) or a top-level `record_change` flow.', + + // ── Tombstones for RETIRED keys (upgrade prescriptions) ──────────────── + // A retired key's error must carry the fix: the compile/validation error is + // the one upgrade channel every consumer is guaranteed to hit — an agent + // bumping @objectstack/spec sees THIS message, not our docs site. Each entry + // names what replaced the key and the version/decision that removed it. + // Tombstones age out too: drop an entry ~two majors after the removal + // (by then it's archaeology, not an upgrade; see CHANGELOG.md for history). + namespace: + '`namespace` was retired in ADR-0006 D4 — the object `name` IS the canonical id ' + + 'everywhere (API, ObjectQL, REST, SDK, DB table), so there is no separate namespace ' + + 'to declare. Embed the module prefix in the name instead: `namespace: "sys", ' + + 'name: "user"` becomes `name: "sys_user"`. Until #4001 closed this shape on the ' + + 'parse path it was stripped in silence, so an object declaring one shipped under ' + + 'the unprefixed name its author did not intend.', + compactLayout: + '`compactLayout` was renamed to `highlightFields` in @objectstack/spec 11.7.0 ' + + '(ADR-0085 semantic roles) and the alias was retired in 11.9.1 (#2536). ' + + 'Rename the key — the value shape (ordered field-name list) is unchanged.', + detail: + 'The `detail` UI-hints block was removed by ADR-0085 (spec 11.7.0). Its ' + + 'jobs moved to top-level semantic roles: `detail.stageField` → `stageField` ' + + '(string | false), `detail.highlightFields` → `highlightFields`, section ' + + 'layout → `fieldGroups` + `Field.group`. Whole-page customization is done ' + + 'by assigning a custom Page schema instead of per-page hint keys.', + views: + '`views` is not an ObjectSchema field: the object-level `views.form/*` and ' + + '`views.detail/*` UI-hint dialect was never part of the spec and its ' + + 'renderer support was removed (ADR-0085). Use the semantic roles ' + + '(`highlightFields`, `stageField`, `fieldGroups`) for hints and `listViews` ' + + 'for named list views.', + defaultDetailForm: + '`defaultDetailForm` was never implemented and was removed from the spec ' + + '(#2402). Curate the record page by assigning a custom Page schema; form ' + + 'layout derives from `fieldGroups` + `Field.group`.', + softDelete: + '`softDelete` was removed from the spec in 16.0 (#2377, ADR-0049 ' + + 'enforce-or-remove) — there is no soft-delete/recycle-bin runtime, so it ' + + 'stored nothing and implied restore semantics that do not exist. Deletes ' + + 'are hard deletes; remove the key.', + versioning: + '`versioning` was removed from the spec in 16.0 (#2377, ADR-0049) — no ' + + 'record-versioning engine ever read it (it snapshotted no history). Use ' + + 'per-field `Field.trackHistory` for field-level history, or a data ' + + 'lifecycle policy (`lifecycle`) for retention.', + search: + '`search` (the SearchConfig block) was removed from the spec in 16.0 ' + + '(#2377, ADR-0049) — no search-engine config was consumed. Declare the ' + + 'indexed fields with `searchableFields` (ADR-0061); records stay queryable ' + + 'via the normal data API regardless.', + recordName: + '`recordName` was removed from the spec in 16.0 (#2377, ADR-0049) — it was ' + + 'never read. Auto-naming is modelled as a `Field` of type \'autonumber\' ' + + '(with `autonumberFormat`) designated as the object\'s `nameField`.', + keyPrefix: + '`keyPrefix` was removed from the spec in 16.0 (#2377, ADR-0049) — record ' + + 'ids are not prefixed from it (no Salesforce-style key-prefix runtime). ' + + 'Remove the key; it had no effect.', + tags: + '`tags` (object-level categorization) was removed from the spec (#2377, ' + + 'ADR-0049) — it had no runtime reader. Remove the key; use `managedBy` for ' + + 'lifecycle bucketing or a real field for per-record tagging.', + active: + '`active` was removed from the spec (#2377, ADR-0049) — no runtime reader ' + + 'gated on it, so an "inactive" object was still fully queryable and usable. ' + + 'Remove the key; gate availability with permissions/sharing instead.', + abstract: + '`abstract` was removed from the spec (#2377, ADR-0049) — object ' + + 'inheritance/abstraction is not implemented, so an abstract object still ' + + 'got a table and was instantiable. Remove the key.', +}; + +// ⚠️ ORDER IS LOAD-BEARING (#5593). This map used to live ~700 lines BELOW +// `ObjectSchemaBase`, and the error map that reads it was built lazily +// (`objectUnknownKeyErrorImpl ??= …`) purely to step around the temporal dead +// zone that created. `strictObject` evaluates its options object at +// CONSTRUCTION — that is what lets the audit in `alias-integrity.test.ts` judge +// the table against the real `.shape` — so the deferral had to go, and the +// declaration order is what replaces it. Keep this block above +// `ObjectSchemaBase`; moving it back reintroduces the TDZ as a module-init +// crash under `OS_EAGER_SCHEMAS=1` (how `build-schemas.ts` runs), which the +// test suite does not reach because tests import lazily. +const ObjectSchemaBase = strictObject( + { + surface: 'this object', + // The same semantic renames the WARNING layer already knew + // (`OBJECT_KEY_GUIDANCE`). Graduating a surface from warn to reject must + // not cost the author a prescription: `capabilities` → `enable` is a + // different word for the same intent, so edit distance cannot reach it and + // only an explicit entry can. + aliases: { capabilities: 'enable', features: 'enable' }, + guidance: UNKNOWN_KEY_GUIDANCE, + history: + 'Until #4001 closed this shape these were dropped silently on the PARSE path — ' + + '`ObjectSchema.create()` has rejected them since #1535, but `defineStack({ objects })`, ' + + '`/api/v1/meta/types/object` and the Studio form all go through `parse()`, which did not.', + }, + { /** * Identity & Metadata */ @@ -1802,7 +1899,7 @@ const ObjectSchemaBase = z.object({ // ADR-0010 — runtime protection envelope (internal — set by loader). ...MetadataProtectionFields, -}, { error: objectUnknownKeyError }).strict(); +}); /** * Converts a snake_case name to a human-readable Title Case label. @@ -1815,107 +1912,6 @@ function snakeCaseToLabel(name: string): string { .join(' '); } -/** - * Known-confusable schema keys → precise authoring guidance. - * - * ADR-0032's "no silent failure" principle applied to metadata *shape*: an - * unknown top-level key on `ObjectSchema.create()` used to be discarded by - * Zod's default `.strip()`, so a misauthored schema key vanished with no - * error, no warning, and a green `tsc` — shipping dead metadata the author - * believed they had wired up (issue #1535, object-level `workflows: [...]`). - * - * These entries turn the most likely mistakes into a fixable error that points - * at the *supported* mechanism rather than a generic "unknown key". - */ -const UNKNOWN_KEY_GUIDANCE: Record = { - workflows: - '`workflows` is not an ObjectSchema field. Object-level, record-triggered ' + - 'automation is authored as a lifecycle hook (`src/objects/.hook.ts`, ' + - 'wrapped in `defineHook()` from `@objectstack/spec/data`) or as a top-level ' + - '`record_change` flow — not as `workflows[]` on the object schema.', - workflow: - '`workflow` is not an ObjectSchema field. Record-triggered automation is ' + - 'authored as a lifecycle hook (`src/objects/.hook.ts`) or a top-level ' + - '`record_change` flow.', - hooks: - '`hooks` is not an ObjectSchema field. Lifecycle hooks live in their own ' + - '`src/objects/.hook.ts` module, wrapped in `defineHook()` from ' + - '`@objectstack/spec/data`.', - triggers: - '`triggers` is not an ObjectSchema field. Use a lifecycle hook ' + - '(`src/objects/.hook.ts`) or a top-level `record_change` flow.', - - // ── Tombstones for RETIRED keys (upgrade prescriptions) ──────────────── - // A retired key's error must carry the fix: the compile/validation error is - // the one upgrade channel every consumer is guaranteed to hit — an agent - // bumping @objectstack/spec sees THIS message, not our docs site. Each entry - // names what replaced the key and the version/decision that removed it. - // Tombstones age out too: drop an entry ~two majors after the removal - // (by then it's archaeology, not an upgrade; see CHANGELOG.md for history). - namespace: - '`namespace` was retired in ADR-0006 D4 — the object `name` IS the canonical id ' + - 'everywhere (API, ObjectQL, REST, SDK, DB table), so there is no separate namespace ' + - 'to declare. Embed the module prefix in the name instead: `namespace: "sys", ' + - 'name: "user"` becomes `name: "sys_user"`. Until #4001 closed this shape on the ' + - 'parse path it was stripped in silence, so an object declaring one shipped under ' + - 'the unprefixed name its author did not intend.', - compactLayout: - '`compactLayout` was renamed to `highlightFields` in @objectstack/spec 11.7.0 ' + - '(ADR-0085 semantic roles) and the alias was retired in 11.9.1 (#2536). ' + - 'Rename the key — the value shape (ordered field-name list) is unchanged.', - detail: - 'The `detail` UI-hints block was removed by ADR-0085 (spec 11.7.0). Its ' + - 'jobs moved to top-level semantic roles: `detail.stageField` → `stageField` ' + - '(string | false), `detail.highlightFields` → `highlightFields`, section ' + - 'layout → `fieldGroups` + `Field.group`. Whole-page customization is done ' + - 'by assigning a custom Page schema instead of per-page hint keys.', - views: - '`views` is not an ObjectSchema field: the object-level `views.form/*` and ' + - '`views.detail/*` UI-hint dialect was never part of the spec and its ' + - 'renderer support was removed (ADR-0085). Use the semantic roles ' + - '(`highlightFields`, `stageField`, `fieldGroups`) for hints and `listViews` ' + - 'for named list views.', - defaultDetailForm: - '`defaultDetailForm` was never implemented and was removed from the spec ' + - '(#2402). Curate the record page by assigning a custom Page schema; form ' + - 'layout derives from `fieldGroups` + `Field.group`.', - softDelete: - '`softDelete` was removed from the spec in 16.0 (#2377, ADR-0049 ' + - 'enforce-or-remove) — there is no soft-delete/recycle-bin runtime, so it ' + - 'stored nothing and implied restore semantics that do not exist. Deletes ' + - 'are hard deletes; remove the key.', - versioning: - '`versioning` was removed from the spec in 16.0 (#2377, ADR-0049) — no ' + - 'record-versioning engine ever read it (it snapshotted no history). Use ' + - 'per-field `Field.trackHistory` for field-level history, or a data ' + - 'lifecycle policy (`lifecycle`) for retention.', - search: - '`search` (the SearchConfig block) was removed from the spec in 16.0 ' + - '(#2377, ADR-0049) — no search-engine config was consumed. Declare the ' + - 'indexed fields with `searchableFields` (ADR-0061); records stay queryable ' + - 'via the normal data API regardless.', - recordName: - '`recordName` was removed from the spec in 16.0 (#2377, ADR-0049) — it was ' + - 'never read. Auto-naming is modelled as a `Field` of type \'autonumber\' ' + - '(with `autonumberFormat`) designated as the object\'s `nameField`.', - keyPrefix: - '`keyPrefix` was removed from the spec in 16.0 (#2377, ADR-0049) — record ' + - 'ids are not prefixed from it (no Salesforce-style key-prefix runtime). ' + - 'Remove the key; it had no effect.', - tags: - '`tags` (object-level categorization) was removed from the spec (#2377, ' + - 'ADR-0049) — it had no runtime reader. Remove the key; use `managedBy` for ' + - 'lifecycle bucketing or a real field for per-record tagging.', - active: - '`active` was removed from the spec (#2377, ADR-0049) — no runtime reader ' + - 'gated on it, so an "inactive" object was still fully queryable and usable. ' + - 'Remove the key; gate availability with permissions/sharing instead.', - abstract: - '`abstract` was removed from the spec (#2377, ADR-0049) — object ' + - 'inheritance/abstraction is not implemented, so an abstract object still ' + - 'got a table and was instantiable. Remove the key.', -}; - /** Levenshtein edit distance — backs the "did you mean" hint for typo'd keys. */ function editDistance(a: string, b: string): number { const dp: number[][] = Array.from({ length: a.length + 1 }, () => diff --git a/packages/spec/src/identity/position.test.ts b/packages/spec/src/identity/position.test.ts index d71896af1e..606dc9e299 100644 --- a/packages/spec/src/identity/position.test.ts +++ b/packages/spec/src/identity/position.test.ts @@ -261,17 +261,4 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { expect(parsed._packageId).toBe('com.showcase'); expect(parsed._lock).toBe('full'); }); - - it('accepts every key the schema declares (guards POSITION_KEYS drift)', () => { - const probes: Record = { - description: 'd', delegatable: true, protection: { lock: 'none' }, - }; - for (const [key, value] of Object.entries(probes)) { - const result = PositionSchema.safeParse({ name: 'p', label: 'P', [key]: value }); - const unknown = result.success - ? undefined - : result.error.issues.find((i) => i.code === 'unrecognized_keys'); - expect(unknown, `\`${key}\` should be a declared Position key`).toBeUndefined(); - } - }); }); diff --git a/packages/spec/src/identity/position.zod.ts b/packages/spec/src/identity/position.zod.ts index 58c970cebf..0803ab6984 100644 --- a/packages/spec/src/identity/position.zod.ts +++ b/packages/spec/src/identity/position.zod.ts @@ -4,7 +4,7 @@ import { z } from 'zod'; import { SnakeCaseIdentifierSchema } from '../shared/identifiers.zod'; import { ProtectionSchema } from '../shared/protection.zod'; import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; -import { strictUnknownKeyError } from '../shared/suggestions.zod'; +import { strictObject } from '../shared/strict-object'; /** * Position Schema — the flat capability-distribution group (ADR-0090 D3). @@ -43,40 +43,31 @@ import { strictUnknownKeyError } from '../shared/suggestions.zod'; */ import { lazySchema } from '../shared/lazy-schema'; -/** Keys {@link PositionSchema} declares (drift-guarded by position.test.ts). */ -const POSITION_KEYS = [ - 'name', 'label', 'description', 'delegatable', 'protection', - // ADR-0010 runtime protection envelope (MetadataProtectionFields spread). - '_lock', '_lockReason', '_lockSource', '_provenance', '_packageId', - '_packageVersion', '_lockDocsUrl', -] as const; - -const positionUnknownKeyError = strictUnknownKeyError({ - surface: 'this position', - knownKeys: POSITION_KEYS, - aliases: { title: 'label' }, - guidance: { - permissionSets: - '`permissionSets` is not a Position field — a position is only the named ' + - 'distribution point (ADR-0090 D3); capability arrives via runtime bindings ' + - '(`sys_position_permission_set` rows, created in Setup or by an app\'s ' + - 'kernel:ready binder). Packages SUGGEST bindings via `isDefault` on a ' + - 'permission set, never by declaring them on the position.', - users: - '`users` is not a Position field — assignment is a runtime binding ' + - '(`sys_user_position` rows), never authored on the position (ADR-0090).', - parent: - '`parent` is not a Position field — positions are deliberately FLAT (ADR-0090 ' + - 'D3, finalizing ADR-0057 D5): the visibility hierarchy is the business-unit ' + - 'tree (`sys_business_unit`) and the manager chain (`sys_user.manager_id`), ' + - 'never a position tree.', +export const PositionSchema = lazySchema(() => strictObject( + { + surface: 'this position', + aliases: { title: 'label' }, + guidance: { + permissionSets: + '`permissionSets` is not a Position field — a position is only the named ' + + 'distribution point (ADR-0090 D3); capability arrives via runtime bindings ' + + '(`sys_position_permission_set` rows, created in Setup or by an app\'s ' + + 'kernel:ready binder). Packages SUGGEST bindings via `isDefault` on a ' + + 'permission set, never by declaring them on the position.', + users: + '`users` is not a Position field — assignment is a runtime binding ' + + '(`sys_user_position` rows), never authored on the position (ADR-0090).', + parent: + '`parent` is not a Position field — positions are deliberately FLAT (ADR-0090 ' + + 'D3, finalizing ADR-0057 D5): the visibility hierarchy is the business-unit ' + + 'tree (`sys_business_unit`) and the manager chain (`sys_user.manager_id`), ' + + 'never a position tree.', + }, + history: + 'Until #4001 these were dropped silently — the position still parsed, so the ' + + 'author believed a distribution property was declared that the runtime never saw.', }, - history: - 'Until #4001 these were dropped silently — the position still parsed, so the ' + - 'author believed a distribution property was declared that the runtime never saw.', -}); - -export const PositionSchema = lazySchema(() => z.object({ + { /** Identity */ name: SnakeCaseIdentifierSchema.describe('Unique position name (lowercase snake_case)'), label: z.string().describe('Display label (e.g. VP of Sales)'), @@ -118,7 +109,7 @@ export const PositionSchema = lazySchema(() => z.object({ // runtime — the schema just could not represent them (the same ADR-0078 §3 // inverse-drift class the permission schema had). ...MetadataProtectionFields, -}, { error: positionUnknownKeyError }).strict()); +})); /** * [ADR-0090 D5/D9] Built-in AUDIENCE ANCHOR positions. `everyone` is held diff --git a/packages/spec/src/security/permission.test.ts b/packages/spec/src/security/permission.test.ts index 70cb8d2d51..0d6db78338 100644 --- a/packages/spec/src/security/permission.test.ts +++ b/packages/spec/src/security/permission.test.ts @@ -622,24 +622,6 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { expect(message, `\`${key}\` should carry guidance`).toContain(`\`${key}\` is not a PermissionSet field`); } }); - - it('accepts every key the schema declares (guards PERMISSION_SET_KEYS drift)', () => { - const probes: Record = { - name: 'p', label: 'P', description: 'd', packageId: 'pkg', managedBy: 'user', - isDefault: true, - objects: { task: { allowRead: true } }, fields: { 'task.secret': { readable: false } }, - systemPermissions: ['manage_users'], tabPermissions: { app_crm: 'visible' }, - rowLevelSecurity: [], adminScope: { businessUnit: 'east' }, - protection: { lock: 'none' }, - }; - for (const [key, value] of Object.entries(probes)) { - const result = PermissionSetSchema.safeParse({ name: 'p', objects: {}, [key]: value }); - const unknown = result.success - ? undefined - : result.error.issues.find((i) => i.code === 'unrecognized_keys'); - expect(unknown, `\`${key}\` should be a declared PermissionSet key`).toBeUndefined(); - } - }); }); describe('ObjectPermissionSchema', () => { @@ -653,21 +635,6 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { expect(unknownKeyIssue(ObjectPermissionSchema, { viewAll: true })!.message) .toContain('`viewAll` → `viewAllRecords`'); }); - - it('accepts every key the schema declares (guards OBJECT_PERMISSION_KEYS drift)', () => { - const probes: Record = { - allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true, - allowExport: true, allowTransfer: true, allowRestore: true, allowPurge: true, - viewAllRecords: true, modifyAllRecords: true, readScope: 'org', writeScope: 'own', - }; - for (const [key, value] of Object.entries(probes)) { - const result = ObjectPermissionSchema.safeParse({ [key]: value }); - const unknown = result.success - ? undefined - : result.error.issues.find((i) => i.code === 'unrecognized_keys'); - expect(unknown, `\`${key}\` should be a declared ObjectPermission key`).toBeUndefined(); - } - }); }); describe('FieldPermissionSchema', () => { @@ -689,19 +656,5 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { expect(unknownKeyIssue(AdminScopeSchema, { businessUnit: 'east', business_unit: 'x' })!.message) .toContain('`business_unit` → `businessUnit`'); }); - - it('accepts every key the schema declares (guards ADMIN_SCOPE_KEYS drift)', () => { - const probes: Record = { - businessUnit: 'east', includeSubtree: false, manageAssignments: true, - manageBindings: true, authorEnvironmentSets: true, assignablePermissionSets: ['a'], - }; - for (const [key, value] of Object.entries(probes)) { - const result = AdminScopeSchema.safeParse({ businessUnit: 'east', [key]: value }); - const unknown = result.success - ? undefined - : result.error.issues.find((i) => i.code === 'unrecognized_keys'); - expect(unknown, `\`${key}\` should be a declared AdminScope key`).toBeUndefined(); - } - }); }); }); diff --git a/packages/spec/src/security/permission.zod.ts b/packages/spec/src/security/permission.zod.ts index efbb2a721b..dd2a1e4d45 100644 --- a/packages/spec/src/security/permission.zod.ts +++ b/packages/spec/src/security/permission.zod.ts @@ -17,7 +17,7 @@ import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; * - Purge (Hard delete / Compliance) */ import { lazySchema } from '../shared/lazy-schema'; -import { strictUnknownKeyError } from '../shared/suggestions.zod'; +import { strictObject } from '../shared/strict-object'; /** * [ADR-0057 D1] Object access DEPTH — the Dataverse "access level" axis, * layered on top of OWD. Widens the owner-match for owner-scoped objects. @@ -42,13 +42,6 @@ export type ObjectAccessScope = z.input; * a recognisable spelling of it. */ -/** Keys {@link ObjectPermissionSchema} declares (drift-guarded by permission.test.ts). */ -const OBJECT_PERMISSION_KEYS = [ - 'allowCreate', 'allowRead', 'allowEdit', 'allowDelete', 'allowExport', - 'allowTransfer', 'allowRestore', 'allowPurge', - 'viewAllRecords', 'modifyAllRecords', 'readScope', 'writeScope', -] as const; - /** * Semantic near-misses for object-permission bits — mostly the bare CRUD verbs * (Salesforce object-permission vocabulary) an author reaches for before @@ -77,23 +70,22 @@ const OBJECT_PERMISSION_KEY_ALIASES: Readonly> = { modifyalldata: 'modifyAllRecords', }; -const objectPermissionUnknownKeyError = strictUnknownKeyError({ - surface: 'this object permission', - knownKeys: OBJECT_PERMISSION_KEYS, - aliases: OBJECT_PERMISSION_KEY_ALIASES, - guidance: { - apiOperations: - '`apiOperations` is the server-resolved effective operation set (#3391) — it exists ' + - 'only on the RESPONSE surface (`/me/permissions`) and is never authored. Grant ' + - 'capability with the `allow*` bits here; tighten an object\'s exposure with ' + - '`apiMethods` on the object schema.', +export const ObjectPermissionSchema = lazySchema(() => strictObject( + { + surface: 'this object permission', + aliases: OBJECT_PERMISSION_KEY_ALIASES, + guidance: { + apiOperations: + '`apiOperations` is the server-resolved effective operation set (#3391) — it exists ' + + 'only on the RESPONSE surface (`/me/permissions`) and is never authored. Grant ' + + 'capability with the `allow*` bits here; tighten an object\'s exposure with ' + + '`apiMethods` on the object schema.', + }, + history: + 'Until #4001 these were dropped silently — the permission set still parsed, so the ' + + 'author believed a grant or restriction was in place that the runtime never saw.', }, - history: - 'Until #4001 these were dropped silently — the permission set still parsed, so the ' + - 'author believed a grant or restriction was in place that the runtime never saw.', -}); - -export const ObjectPermissionSchema = lazySchema(() => z.object({ + { /** C: Create */ allowCreate: z.boolean().default(false).describe('Create permission'), /** R: Read (Owned records or Shared records) */ @@ -190,7 +182,7 @@ export const ObjectPermissionSchema = lazySchema(() => z.object({ readScope: ObjectAccessScopeSchema.optional().describe('[ADR-0057 D1] Read depth: own|unit|unit_and_below|org'), /** [ADR-0057 D1] Write (edit/delete) access DEPTH — same enum as readScope. */ writeScope: ObjectAccessScopeSchema.optional().describe('[ADR-0057 D1] Write depth: own|unit|unit_and_below|org'), -}, { error: objectPermissionUnknownKeyError }).strict()); +})); /** * RESPONSE-side extension of {@link ObjectPermissionSchema} carrying the @@ -239,21 +231,14 @@ export type EffectiveObjectPermission = z.input z.object({ +export const AdminScopeSchema = lazySchema(() => strictObject( + { + surface: 'this admin scope', + history: + 'Until #4001 these were dropped silently — the scope still parsed, so a delegation ' + + 'boundary the author intended was never enforced.', + }, + { /** Root of the delegated subtree — `sys_business_unit.name` (machine name, portable across environments). */ businessUnit: z.string().describe('[ADR-0090 D12] Delegation boundary: sys_business_unit.name of the subtree root'), /** Whether the scope covers the whole subtree under `businessUnit` (default) or that single unit only. */ @@ -271,41 +256,40 @@ export const AdminScopeSchema = lazySchema(() => z.object({ * grantor's scope to STRICTLY contain the granted one. */ assignablePermissionSets: z.array(z.string()).default([]).describe('Allowlist of permission-set names the delegate may hand out'), -}, { error: adminScopeUnknownKeyError }).strict()); +})); export type AdminScope = z.input; /** Post-parse shape of {@link AdminScope} — defaults applied, transforms run (ADR-0122). */ export type AdminScopeParsed = z.infer; -const fieldPermissionUnknownKeyError = strictUnknownKeyError({ - surface: 'this field permission', - knownKeys: ['readable', 'editable'], - aliases: { - read: 'readable', - visible: 'readable', - write: 'editable', - edit: 'editable', - update: 'editable', - }, - guidance: { - hidden: - '`hidden` is not a FieldPermission key — FLS is declared positively: set ' + - '`readable: false` to hide the field.', - }, - history: - 'Until #4001 these were dropped silently — the entry still parsed, so field-level ' + - 'security the author intended was never applied.', -}); - /** * Field Level Security (FLS) */ -export const FieldPermissionSchema = lazySchema(() => z.object({ +export const FieldPermissionSchema = lazySchema(() => strictObject( + { + surface: 'this field permission', + aliases: { + read: 'readable', + visible: 'readable', + write: 'editable', + edit: 'editable', + update: 'editable', + }, + guidance: { + hidden: + '`hidden` is not a FieldPermission key — FLS is declared positively: set ' + + '`readable: false` to hide the field.', + }, + history: + 'Until #4001 these were dropped silently — the entry still parsed, so field-level ' + + 'security the author intended was never applied.', + }, + { /** Can see this field */ readable: z.boolean().default(true).describe('Field read access'), /** Can edit this field */ editable: z.boolean().default(false).describe('Field edit access'), -}, { error: fieldPermissionUnknownKeyError }).strict()); +})); /** * Permission Set Schema @@ -331,16 +315,6 @@ export const FieldPermissionSchema = lazySchema(() => z.object({ * - 'SystemAdmin' (mixed case) * - 'Read Only' (spaces) */ -/** Keys {@link PermissionSetSchema} declares (drift-guarded by permission.test.ts). */ -const PERMISSION_SET_KEYS = [ - 'name', 'label', 'description', 'packageId', 'managedBy', 'isDefault', 'objects', - 'fields', 'systemPermissions', 'tabPermissions', 'rowLevelSecurity', 'adminScope', - 'protection', - // ADR-0010 runtime protection envelope (MetadataProtectionFields spread). - '_lock', '_lockReason', '_lockSource', '_provenance', '_packageId', - '_packageVersion', '_lockDocsUrl', -] as const; - /** Semantic near-misses borrowed from neighbouring schemas / products. */ const PERMISSION_SET_KEY_ALIASES: Readonly> = { objectpermissions: 'objects', @@ -356,41 +330,40 @@ const PERMISSION_SET_KEY_ALIASES: Readonly> = { policies: 'rowLevelSecurity', }; -const permissionSetUnknownKeyError = strictUnknownKeyError({ - surface: 'this permission set', - knownKeys: PERMISSION_SET_KEYS, - aliases: PERMISSION_SET_KEY_ALIASES, - guidance: { - // ── Tombstones for RETIRED keys (upgrade prescriptions; they age out ~two - // majors after removal — pattern of data/object.zod.ts UNKNOWN_KEY_GUIDANCE). - contextVariables: - '`contextVariables` was removed by ADR-0105 D11 (enforce-or-remove, ADR-0049): it ' + - 'was authorable but had zero runtime consumers. A custom membership set a policy ' + - 'needs as `field IN (current_user.)` is staged by a registered rlsMembership ' + - 'resolver; a constant belongs inline in the policy\'s `using` expression.', - isProfile: - '`isProfile` was removed by ADR-0090 D2 — there is no Profile concept. Permission ' + - 'sets are the only capability container; use `isDefault` to mark the app baseline ' + - 'bound to the built-in `everyone` position.', - // ── Wrong-layer pointers for keys that are never authored on a set. - profiles: - '`profiles` is not a PermissionSet field (ADR-0090 D2: no Profile concept). ' + - 'Distribution is a runtime binding — positions bind sets to people ' + - '(`sys_position_permission_set`), never the set itself.', - roles: - '`roles` is not a PermissionSet field — ObjectStack has no role hierarchy: ' + - 'capability = permission sets (union-merged), distribution = positions, ' + - 'visibility depth = business units (ADR-0057 / ADR-0090).', - users: - '`users` is not a PermissionSet field — assignment is a runtime binding ' + - '(`sys_user_permission_set` / positions), never authored on the set (ADR-0090).', +export const PermissionSetSchema = lazySchema(() => strictObject( + { + surface: 'this permission set', + aliases: PERMISSION_SET_KEY_ALIASES, + guidance: { + // ── Tombstones for RETIRED keys (upgrade prescriptions; they age out ~two + // majors after removal — pattern of data/object.zod.ts UNKNOWN_KEY_GUIDANCE). + contextVariables: + '`contextVariables` was removed by ADR-0105 D11 (enforce-or-remove, ADR-0049): it ' + + 'was authorable but had zero runtime consumers. A custom membership set a policy ' + + 'needs as `field IN (current_user.)` is staged by a registered rlsMembership ' + + 'resolver; a constant belongs inline in the policy\'s `using` expression.', + isProfile: + '`isProfile` was removed by ADR-0090 D2 — there is no Profile concept. Permission ' + + 'sets are the only capability container; use `isDefault` to mark the app baseline ' + + 'bound to the built-in `everyone` position.', + // ── Wrong-layer pointers for keys that are never authored on a set. + profiles: + '`profiles` is not a PermissionSet field (ADR-0090 D2: no Profile concept). ' + + 'Distribution is a runtime binding — positions bind sets to people ' + + '(`sys_position_permission_set`), never the set itself.', + roles: + '`roles` is not a PermissionSet field — ObjectStack has no role hierarchy: ' + + 'capability = permission sets (union-merged), distribution = positions, ' + + 'visibility depth = business units (ADR-0057 / ADR-0090).', + users: + '`users` is not a PermissionSet field — assignment is a runtime binding ' + + '(`sys_user_permission_set` / positions), never authored on the set (ADR-0090).', + }, + history: + 'Until #4001 these were dropped silently — the set still parsed, so the author ' + + 'believed a capability boundary was declared that the runtime never saw.', }, - history: - 'Until #4001 these were dropped silently — the set still parsed, so the author ' + - 'believed a capability boundary was declared that the runtime never saw.', -}); - -export const PermissionSetSchema = lazySchema(() => z.object({ + { /** Unique permission set name */ name: SnakeCaseIdentifierSchema.describe('Permission set unique name (lowercase snake_case)'), @@ -542,7 +515,7 @@ export const PermissionSetSchema = lazySchema(() => z.object({ // ADR-0078 §3 inverse-drift class as `description`); the `.strict()` gate // turned that into a visible 422 and surfaced the gap. ...MetadataProtectionFields, -}, { error: permissionSetUnknownKeyError }).strict()); +})); export type PermissionSet = z.input; /** Post-parse shape of {@link PermissionSet} — defaults applied, transforms run (ADR-0122). */ diff --git a/packages/spec/src/security/rls.test.ts b/packages/spec/src/security/rls.test.ts index 20a3926b1f..b2bd5d2cc3 100644 --- a/packages/spec/src/security/rls.test.ts +++ b/packages/spec/src/security/rls.test.ts @@ -542,18 +542,4 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { expect(messages).toContain('#3896'); expect(messages).toContain('Delete the key'); }); - - it('accepts every key the schema declares (guards RLS_POLICY_KEYS drift)', () => { - const probes: Record = { - label: 'L', description: 'D', check: 'owner_id = current_user.id', - positions: ['manager'], enabled: false, tags: ['compliance'], - }; - for (const [key, value] of Object.entries(probes)) { - const result = RowLevelSecurityPolicySchema.safeParse({ ...policy, [key]: value }); - const unknown = result.success - ? undefined - : result.error.issues.find((i) => i.code === 'unrecognized_keys'); - expect(unknown, `\`${key}\` should be a declared RLS policy key`).toBeUndefined(); - } - }); }); diff --git a/packages/spec/src/security/rls.zod.ts b/packages/spec/src/security/rls.zod.ts index 44f7fd2ff9..0aefcbc848 100644 --- a/packages/spec/src/security/rls.zod.ts +++ b/packages/spec/src/security/rls.zod.ts @@ -2,7 +2,7 @@ import { z } from 'zod'; import { retiredKey } from '../shared/retired-key'; -import { strictUnknownKeyError } from '../shared/suggestions.zod'; +import { strictObject } from '../shared/strict-object'; /** * # Row-Level Security (RLS) Protocol @@ -212,37 +212,31 @@ export type RLSOperation = z.input; * } * ``` */ -/** - * Keys {@link RowLevelSecurityPolicySchema} declares (drift-guarded by - * rls.test.ts). `priority` is deliberately absent: it is a {@link retiredKey} - * tombstone in the shape — declared so its rejection carries the prescription, - * but never a suggestion target. - */ -const RLS_POLICY_KEYS = [ - 'name', 'label', 'description', 'object', 'operation', 'using', 'check', - 'positions', 'enabled', 'tags', -] as const; - -const rlsPolicyUnknownKeyError = strictUnknownKeyError({ - surface: 'this RLS policy', - knownKeys: RLS_POLICY_KEYS, - aliases: { - // ADR-0090 D3 renamed the pre-D3 `roles` vocabulary to `positions`. - roles: 'positions', - role: 'positions', - // PostgreSQL spells the write-side clause `WITH CHECK`. - withcheck: 'check', - // The read-side clause under other names an author reaches for first. - condition: 'using', - filter: 'using', - where: 'using', +export const RowLevelSecurityPolicySchema = lazySchema(() => strictObject( + { + surface: 'this RLS policy', + // The suggestion pool is `Object.keys(shape)` minus anything that accepts + // nothing (#5593). `priority` is exactly that case and the exclusion is + // deliberate: it is a {@link retiredKey} tombstone, declared so its + // rejection carries the upgrade prescription, never offered as a rename + // target. The hand-transcribed list this replaced had to state the same + // exclusion in prose and be trusted to keep it. + aliases: { + // ADR-0090 D3 renamed the pre-D3 `roles` vocabulary to `positions`. + roles: 'positions', + role: 'positions', + // PostgreSQL spells the write-side clause `WITH CHECK`. + withcheck: 'check', + // The read-side clause under other names an author reaches for first. + condition: 'using', + filter: 'using', + where: 'using', + }, + history: + 'Until #4001 these were dropped silently — the policy still parsed, so a ' + + 'row-level restriction the author wrote was never compiled into the filter.', }, - history: - 'Until #4001 these were dropped silently — the policy still parsed, so a ' + - 'row-level restriction the author wrote was never compiled into the filter.', -}); - -export const RowLevelSecurityPolicySchema = lazySchema(() => z.object({ + { /** * Unique identifier for this policy. * Must be unique within the object. @@ -445,7 +439,7 @@ export const RowLevelSecurityPolicySchema = lazySchema(() => z.object({ tags: z.array(z.string()) .optional() .describe('Policy categorization tags'), -}, { error: rlsPolicyUnknownKeyError }).strict().superRefine((data, ctx) => { +}).superRefine((data, ctx) => { // Ensure at least one of USING or CHECK is provided if (!data.using && !data.check) { ctx.addIssue({ diff --git a/packages/spec/src/security/sharing.test.ts b/packages/spec/src/security/sharing.test.ts index 4faef14dc4..fbff229574 100644 --- a/packages/spec/src/security/sharing.test.ts +++ b/packages/spec/src/security/sharing.test.ts @@ -460,17 +460,4 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { const issue = result.error!.issues.find((i) => i.code === 'unrecognized_keys'); expect(issue!.message).toContain('`id` → `value`'); }); - - it('accepts every key the schema declares (guards SHARING_RULE_KEYS drift)', () => { - const probes: Record = { - label: 'L', description: 'D', active: false, accessLevel: 'edit', - }; - for (const [key, value] of Object.entries(probes)) { - const result = SharingRuleSchema.safeParse({ ...rule, [key]: value }); - const unknown = result.success - ? undefined - : result.error.issues.find((i) => i.code === 'unrecognized_keys'); - expect(unknown, `\`${key}\` should be a declared sharing-rule key`).toBeUndefined(); - } - }); }); diff --git a/packages/spec/src/security/sharing.zod.ts b/packages/spec/src/security/sharing.zod.ts index d6a82bd457..75a2f1f40a 100644 --- a/packages/spec/src/security/sharing.zod.ts +++ b/packages/spec/src/security/sharing.zod.ts @@ -2,7 +2,7 @@ import { z } from 'zod'; import { ExpressionInputSchema } from '../shared/expression.zod'; -import { strictUnknownKeyError } from '../shared/suggestions.zod'; +import { strictObject } from '../shared/strict-object'; /** * Organization-Wide Defaults (OWD) @@ -95,55 +95,6 @@ export const ShareRecipientType = z.enum([ 'business_unit', ]); -/** - * Keys the sharing-rule surface declares — the base shape plus the - * `criteria`-variant extension keys (`type` / `condition`), since the strict - * error map rides {@link BaseSharingRuleSchema} into every extension - * (drift-guarded by sharing.test.ts). - */ -const SHARING_RULE_KEYS = [ - 'name', 'label', 'description', 'object', 'active', 'accessLevel', - 'sharedWith', 'type', 'condition', -] as const; - -const sharingRuleUnknownKeyError = strictUnknownKeyError({ - surface: 'this sharing rule', - knownKeys: SHARING_RULE_KEYS, - aliases: { - // The runtime/persisted rule row spells the compiled predicate `criteria` - // (`criteria_json`); the authored key is the CEL `condition` (#3896). - criteria: 'condition', - filter: 'condition', - when: 'condition', - access: 'accessLevel', - level: 'accessLevel', - recipient: 'sharedWith', - sharewith: 'sharedWith', - sharedto: 'sharedWith', - enabled: 'active', - }, - guidance: { - ownedBy: - '`ownedBy` belongs to the removed `owner`-type sharing rule — it depends on live ' + - 'team/position membership, which the static materialiser cannot track, so it was ' + - 'removed from the authoring surface (ADR-0078). Only `criteria` rules are ' + - 'authorable; express membership-shaped access via RLS dynamic membership ' + - '(§7.3.1) or business-unit depth scopes (ADR-0057).', - }, - history: - 'Until #4001 these were dropped silently — the rule still parsed, so a share the ' + - 'author intended was never materialised (or a constraint never applied).', -}); - -const sharingRecipientUnknownKeyError = strictUnknownKeyError({ - surface: 'this sharing-rule recipient', - knownKeys: ['type', 'value'], - aliases: { id: 'value', target: 'value' }, - history: - 'Until #4001 these were dropped silently — the recipient still parsed, so the ' + - 'grant could land on the wrong principal without a diagnostic.', -}); - /** * Base Sharing Rule * Common metadata for all sharing strategies. @@ -152,7 +103,45 @@ const sharingRecipientUnknownKeyError = strictUnknownKeyError({ * (zod carries the catchall and error through extension), so the * criteria rule below inherits both. */ -const BaseSharingRuleSchema = z.object({ +const BaseSharingRuleSchema = strictObject( + { + surface: 'this sharing rule', + // The strict error map rides `.extend()` into `CriteriaSharingRuleSchema`, + // which is the ONLY surface anything parses (`SharingRuleSchema` IS that + // extension; this base is module-private). Its two extension keys are named + // here so the suggestion pool on the extended surface is complete, and so + // the `criteria`/`filter`/`when` → `condition` aliases below point at a key + // the shape that actually runs them accepts. Before #5593 that was implicit + // in a hand-transcribed `SHARING_RULE_KEYS` array which quietly listed both + // the base's keys and the extension's; `extraKeys` is where that legitimate + // content goes now that the base's own keys come from `.shape`. + extraKeys: ['type', 'condition'], + aliases: { + // The runtime/persisted rule row spells the compiled predicate `criteria` + // (`criteria_json`); the authored key is the CEL `condition` (#3896). + criteria: 'condition', + filter: 'condition', + when: 'condition', + access: 'accessLevel', + level: 'accessLevel', + recipient: 'sharedWith', + sharewith: 'sharedWith', + sharedto: 'sharedWith', + enabled: 'active', + }, + guidance: { + ownedBy: + '`ownedBy` belongs to the removed `owner`-type sharing rule — it depends on live ' + + 'team/position membership, which the static materialiser cannot track, so it was ' + + 'removed from the authoring surface (ADR-0078). Only `criteria` rules are ' + + 'authorable; express membership-shaped access via RLS dynamic membership ' + + '(§7.3.1) or business-unit depth scopes (ADR-0057).', + }, + history: + 'Until #4001 these were dropped silently — the rule still parsed, so a share the ' + + 'author intended was never materialised (or a constraint never applied).', + }, + { // Identification name: z.string().regex(/^[a-z_][a-z0-9_]*$/).describe('Unique rule name (snake_case)'), label: z.string().optional().describe('Human-readable label'), @@ -166,10 +155,18 @@ const BaseSharingRuleSchema = z.object({ accessLevel: SharingLevel.default('read'), // Recipient (Whom to share with) - sharedWith: z.object({ + sharedWith: strictObject( + { + surface: 'this sharing-rule recipient', + aliases: { id: 'value', target: 'value' }, + history: + 'Until #4001 these were dropped silently — the recipient still parsed, so the ' + + 'grant could land on the wrong principal without a diagnostic.', + }, + { type: ShareRecipientType, value: z.string().describe('ID or code of the recipient (user / team / position / business unit)'), - }, { error: sharingRecipientUnknownKeyError }).strict().describe('The recipient of the shared access'), + }).describe('The recipient of the shared access'), // ADR-0010 — runtime protection envelope (internal — set by loader). // @@ -186,7 +183,7 @@ const BaseSharingRuleSchema = z.object({ // output fails to parse — a hard 422 on the overlay path") and prescribes // this spread as the fix. ...MetadataProtectionFields, -}, { error: sharingRuleUnknownKeyError }).strict(); +}); /** * 1. Criteria-Based Sharing Rule diff --git a/packages/spec/src/shared/alias-integrity.test.ts b/packages/spec/src/shared/alias-integrity.test.ts index 9d5925db8c..b9236add27 100644 --- a/packages/spec/src/shared/alias-integrity.test.ts +++ b/packages/spec/src/shared/alias-integrity.test.ts @@ -70,37 +70,48 @@ * and separators, a second spelling of one probe is **never** reachable. It is * dead either way; it is only sometimes also a defect. * - * ## The second batch: tables that never had a shape (#5483) + * ## The second batch: tables that never had a shape (#5483 → #5593) * - * `strictObject` is not the only way to get an alias table. The 44 call sites - * that predate the helper call `strictUnknownKeyError` directly and hand it a - * **hand-transcribed `knownKeys` array**, so there is no `.shape` to judge them - * against and no `strictObject` construction to register them. They were - * outside all three claims above. + * `strictObject` used not to be the only way to get an alias table. Forty-four + * call sites predated the helper and called `strictUnknownKeyError` directly, + * handing it a **hand-transcribed `knownKeys` array** — so there was no `.shape` + * to judge them against, and they sat outside all three claims above. * - * They are judged now, from a second registry the factory itself fills - * (`alias-table-registry.ts`) — no call site was touched, which is what keeps - * the migration in #5593 a separable change rather than one this guard has - * already half-done. What that buys differs sharply by claim: + * #5483 shipped a transitional guard: a second registry the factory itself + * filled, so those tables were judged *somehow* without a single call site being + * edited. What it could buy differed sharply by claim — claims 1 and 2 were + * answered against the TRANSCRIPTION, so a drifted array dragged both answers + * with it, while claim 3 lost nothing (an `aliasProbe` collision is a property + * of the table alone, and that sweep came back clean at 52 tables). * - * - claims 1 and 2 are answered against the **transcription**. If the array has - * drifted from the schema it describes, both answers inherit the drift. That - * gap is exactly what `strictObject` abolishes and only migration closes. - * - claim 3 loses **nothing**: an `aliasProbe` collision is a property of the - * table alone. These 44 were not "measured clean" on it — #5481 postdates the - * measurement recorded in #5483, so they were *unmeasured*. Now they are - * measured, and the sweep comes back clean at 52 tables. + * **#5593 migrated all 44 and deleted the registry.** The two weak answers are + * now strong ones: every table in this repo is judged against the shape its + * error map actually reads, including the "alias target must not be a tombstone" + * half that a flat `knownKeys` array cannot express at all. Two consequences + * worth naming, because both were predicted as failures and one of them was not: * - * Claim 2 did not come back clean, and the finding is filed rather than fixed - * here: `ui/app.zod.ts` shares four "start expanded" aliases across all nine - * navigation-item variants while `expanded` is declared on `group` alone, so on - * the other eight the suggestion names a key that variant also rejects (#5555). - * Pinned shrink-only below, structurally, because the fix rewrites author-facing - * message text in a call site this transitional guard may not touch. + * - `VARIANT_LEGAL_GUIDANCE` — #5483's exemption for the `children` prescription + * that is legally silent on the two nav variants declaring `children` — is + * **deleted, by a fix rather than a move**: `ui/app.zod.ts` now files that + * prescription only on the seven variants where it can fire, which a + * transcription-shaped table could not distinguish. Same demotion-not-tolerance + * move #5555 made on the "start expanded" aliases one field over. + * - `PROSE_ALIAS_TARGETS` **moved instead of dying**, and that is a correction to + * the migration's own forecast. The exemption existed because seven nav alias + * targets are deliberately prose (`type: 'dashboard' (with dashboardName)`) + * rather than key names; migrating the family did not make them key names, so + * claim 2 met the same 93 entries from the shape side and the tolerance had to + * come with it. It is strictly stronger where it now sits — judged against + * `.shape`, with the same staleness test — but it is one exemption this + * campaign did not get to delete. * - * The shrink-only ratchet also survives unchanged, because what it discourages - * — a NEW direct call site, carrying a fresh second copy of a key list — is - * exactly as undesirable as it was before these tables gained a guard. + * ## The ratchet inverted: `strictUnknownKeyError` is now internal-only in-repo + * + * The old shrink-only ratchet (`<= 44` direct call sites) is a **hard zero**: + * the factory stays PUBLISHED for external callers, but inside `packages/spec` + * the only caller is `strictObject`. A new direct call site would mint a fresh + * second copy of a key list and land outside the shape-backed audit, so it fails + * here rather than being counted. */ import fs from 'node:fs'; @@ -111,11 +122,6 @@ import { describe, it, expect, beforeAll } from 'vitest'; import ts from 'typescript'; import { aliasProbe } from './alias-probe'; -import { - directAliasTableOverflow, - directAliasTables, - type DirectAliasTableDeclaration, -} from './alias-table-registry'; import { acceptsNothing, strictObjectDeclarations, type StrictObjectDeclaration } from './strict-object'; const HERE = path.dirname(fileURLToPath(import.meta.url)); @@ -252,9 +258,6 @@ function force(root: unknown, seen: Set): void { /** Every declaration built by the forcing walk, de-duplicated by content. */ let SURFACES: StrictObjectDeclaration[] = []; -/** The same, for tables that reached `strictUnknownKeyError` directly (#5483). */ -let DIRECT: DirectAliasTableDeclaration[] = []; - beforeAll(async () => { const seen = new Set(); for (const file of MODULES) { @@ -269,6 +272,14 @@ beforeAll(async () => { // A factory (`actionObject()`) called by two schemas runs its `strictObject` // twice, registering two declarations from ONE call site. Same table, same // shape, same verdict — collapse them so a failure is reported once. + // + // The key includes the SHAPE's key list, not just the surface and aliases, + // because `ui/app.zod.ts`'s navigation family is the opposite case: one + // `navItemSurface(variant)` factory feeding nine `strictObject` calls whose + // tables are genuinely different (same surface template, different shape, + // different cross-variant aliases). Collapsing on surface alone would judge + // one of the nine and silently drop eight — the shape is what tells the two + // situations apart. const unique = new Map(); for (const d of strictObjectDeclarations()) { unique.set( @@ -277,25 +288,6 @@ beforeAll(async () => { ); } SURFACES = [...unique.values()]; - - // Same collapse for the direct batch, and for the same reason: the nav-item - // factory in `ui/app.zod.ts` is ONE call site that runs nine times (once per - // `type` variant), and each variant is a genuinely different table — same - // surface template, different `knownKeys`, different cross-variant aliases — - // so the key has to include the key list, not just the surface and aliases. - const uniqueDirect = new Map(); - for (const d of directAliasTables()) { - uniqueDirect.set( - JSON.stringify([ - d.options.surface, - d.options.aliases ?? {}, - d.options.guidance ? Object.keys(d.options.guidance).sort() : [], - [...d.options.knownKeys].sort(), - ]), - d, - ); - } - DIRECT = [...uniqueDirect.values()]; }, 180_000); // --------------------------------------------------------------------------- @@ -314,12 +306,15 @@ interface CallSite { } /** - * The two helper spellings, and where each keeps its options literal. - * `strictObject(options, shape)` vs `strictUnknownKeyError(options)`. + * Where the helper keeps its options literal — `strictObject(options, shape)`. + * + * A one-entry map since #5593 retired the `strictUnknownKeyError(options)` + * spelling from this package. Kept as a map rather than inlined because the + * arity is the load-bearing part (the scan reads `arguments[0]`), and a second + * helper would arrive with a different one. */ const CALLEES = { strictObject: 2, - strictUnknownKeyError: 1, } as const; function callSites(file: string, callee: keyof typeof CALLEES): CallSite[] { @@ -375,14 +370,6 @@ function callSites(file: string, callee: keyof typeof CALLEES): CallSite[] { const CALL_SITES = MODULES.flatMap((f) => callSites(f, 'strictObject')); -/** - * The pre-helper wiring's call sites (#5483) — the helper modules excluded, so - * `strictObject`'s own internal call is not mistaken for one of them. - */ -const DIRECT_CALL_SITES = MODULES - .filter((f) => !HELPER_MODULES.has(path.relative(SPEC_SRC, f))) - .flatMap((f) => callSites(f, 'strictUnknownKeyError')); - // --------------------------------------------------------------------------- // 1. Coverage — the walk reached every table the source declares // --------------------------------------------------------------------------- @@ -420,26 +407,57 @@ describe('alias integrity — coverage', () => { expect(unreached, 'these alias tables are not reachable from any module export, so nothing judges them').toEqual([]); }); - it('the surface this gate can only judge by transcription only ever shrinks', () => { - // `strictObject` is not the only way to get an alias table: the pre-helper - // wiring calls `strictUnknownKeyError` directly with a hand-transcribed - // `knownKeys` array. Since #5483 those tables ARE judged — the factory - // registers them and the block below reads them — so this number is no - // longer a measure of what nothing watches. It measures what is watched - // with the WEAKER instrument: three claims, two of them answered against - // the transcription rather than the shape, so a drifted array drags both - // answers with it and this file cannot tell. + it('NOTHING in packages/spec calls `strictUnknownKeyError` directly any more (#5593)', () => { + // This was a shrink-only ratchet at 44 — the pre-helper wiring, which hands + // the factory a hand-transcribed `knownKeys` array instead of a shape. + // #5593 migrated the last of them, so it is a hard ZERO and the assertion + // changed meaning with the number: it no longer measures "how much of this + // gate runs on the weaker instrument", it forbids the weaker instrument. + // + // Not a style rule. A direct call site is a second copy of a key list, and + // the two claims that matter most — "is this alias key really unknown here" + // and "is this alias target really a key this shape accepts" — can only be + // answered against a transcription, which inherits every drift. The + // tombstone half of the second claim cannot be answered at all: a flat + // string array has no schemas in it. `strictObject` derives the list from + // the shape, so there is nothing left to drift. // - // Which leaves the ratchet meaning exactly what it always meant. Migrating - // one call site to `strictObject` is free and moves it to the shape-backed - // half; adding a NEW one mints a fresh second copy of a key list and fails - // here, forcing the choice to be deliberate. The batched migration that - // takes this to 0 is #5593. - const uncovered = MODULES.filter((f) => !HELPER_MODULES.has(path.relative(SPEC_SRC, f))).flatMap((f) => { + // `strictUnknownKeyError` stays PUBLISHED for external callers (it is in + // `api-surface.json` under `./shared`); this is a rule about THIS package. + const direct = MODULES.filter((f) => !HELPER_MODULES.has(path.relative(SPEC_SRC, f))).flatMap((f) => { const source = fs.readFileSync(f, 'utf8'); return [...source.matchAll(/\bstrictUnknownKeyError\s*\(/g)].map(() => path.relative(SPEC_SRC, f)); }); - expect(uncovered.length).toBeLessThanOrEqual(44); + expect( + direct.sort(), + 'build the shape with `strictObject(options, shape)` instead — see the header of `strict-object.ts`', + ).toEqual([]); + }); + + it("the nav-item factory built one table per variant, and `data/object.zod.ts`'s reached the walk", () => { + // Two coverage facts #5483's direct-call block used to carry, kept because + // both name a mechanism that can break silently, and both moved into the + // shape-backed registry at #5593 rather than disappearing with it. + // + // (a) `ui/app.zod.ts`'s nine navigation branches share ONE + // `navItemSurface(variant)` options factory. Nine variants in, nine + // tables out — the count IS the coverage, and it is the assertion that + // fails if the de-duplication above ever collapses them onto one. + const navTables = SURFACES.filter((s) => PROSE_TARGET_SURFACE.test(s.options.surface)); + expect(navTables.length, 'one strict branch per nav-item `type`').toBe(9); + + // (b) `data/object.zod.ts` used to build its error map on FIRST USE, to + // step around a temporal dead zone, and needed a synthetic-issue poke in + // `force()` to register at all. #5593 removed the deferral — moving + // `UNKNOWN_KEY_GUIDANCE` above the shape is what replaced it, because + // `strictObject` evaluates its options at construction — so this is now + // an ordinary registration. Asserted anyway: if the declaration order + // ever gets shuffled back, the module crashes under `OS_EAGER_SCHEMAS=1` + // and this names the schema that did it. + expect( + SURFACES.map((s) => s.options.surface), + "`data/object.zod.ts`'s table did not register — check the declaration order of UNKNOWN_KEY_GUIDANCE", + ).toContain('this object'); }); it('the runtime walk sees tables the AST provably cannot read', () => { @@ -477,6 +495,63 @@ const entry = (s: StrictObjectDeclaration, written: string, target: string): str return `${where} — "${s.options.surface}": \`${written}\` -> \`${target}\``; }; +/** + * Alias targets that are deliberately **prose, not a key name** — the only + * place in `packages/spec` where that is true, and an explicit allowlist rather + * than a relaxed criterion because the two are not the same promise. + * + * `ui/app.zod.ts` builds one table per navigation-item `type`, and the + * commonest nav mistake is not a typo: it is `dashboardName` written on a `url` + * item — a real key, on the wrong variant. Naming a key there would be wrong + * twice over (the key IS spelled correctly, and writing it is still not enough + * without the matching `type`), so the target is a sentence: + * + * Did you mean `dashboardname` → `type: 'dashboard' (with dashboardName)`? + * + * Enumerated, not pattern-matched, and paired with the surface family that owns + * them, so a new prose target anywhere — including a second one on this very + * surface — fails the target criterion and has to be argued for here. The + * staleness test below is the other half: an entry nothing uses is deleted, so + * this list cannot quietly outlive the tables it excuses. + * + * The seventh entry arrived by that route. #5555 fixed the defect this gate + * found — the four "start expanded" spellings redirecting to `expanded` on the + * eight variants that lack it — and the fix is precisely a demotion from bare + * key name to prose, so the tolerance that pinned it was deleted and this list + * grew by one. It is one string for four alias keys because they share a single + * answer: the key you want lives on `group`. + * + * ⚠️ **Written for the direct-call guard at #5483, MOVED here at #5593 — and the + * move is the correction worth reading.** The migration forecast that this + * exemption would "lose its basis and go red, and should be deleted rather than + * rewritten", on the reasoning that its precondition was the tables still being + * in the direct registry. That reasoning was about where the tables were judged; + * the FACT it excuses is a property of the tables themselves, and migrating them + * did not turn seven sentences into key names. Measured on the migration: claim + * 2 below met the same seven targets from the shape side, 93 entries across the + * variants, so the exemption came with them. It is strictly stronger here — the + * criterion it relaxes is now `target in shape` rather than + * `knownKeys.includes(target)` — but it is the one exemption #5593 did not get + * to delete. Its sibling, `VARIANT_LEGAL_GUIDANCE`, genuinely died: that one was + * excusing a limitation of the transcription, and the shape-backed form let + * `ui/app.zod.ts` file the prescription only where it can fire. + */ +const PROSE_ALIAS_TARGETS: ReadonlySet = new Set([ + "type: 'object' (with objectName)", + "type: 'page' (with pageName)", + "type: 'url' (with url)", + "type: 'dashboard' (with dashboardName)", + "type: 'report' (with reportName)", + "type: 'component' (with componentRef)", + "type: 'group' (with expanded)", +]); + +/** The surface family the prose targets are allowed on, and nowhere else. */ +const PROSE_TARGET_SURFACE = /^this `[a-z]+` navigation item$/; + +const isProseTarget = (surface: string, target: string): boolean => + PROSE_TARGET_SURFACE.test(surface) && PROSE_ALIAS_TARGETS.has(target); + describe('alias integrity — every table is a true claim about its schema', () => { it('no alias key is itself a declared key (a dead entry that can never fire)', () => { // An alias is consulted only from the `unrecognized_keys` path. A key the @@ -498,12 +573,34 @@ describe('alias integrity — every table is a true claim about its schema', () // the alias table is consulted BEFORE that fallback and bypasses the filter // entirely. Pointing an alias at a tombstone is ledger finding 12 exactly — // the author is told to write the one key guaranteed to be rejected next. + // + // `extraKeys` counts as accepted, and the reason is the base/extension + // boundary rather than leniency: strictness and the error map RIDE + // `.extend()`, so a module-private base's table is consulted on a shape it + // does not itself declare (`security/sharing.zod.ts`'s base names `type` / + // `condition`, which only `CriteriaSharingRuleSchema` declares — and that + // extension is the only surface anything parses). `extraKeys` is the field + // `strictObject` provides for exactly that, and the suggester already reads + // it as a candidate, so refusing it here would forbid a pattern the helper + // documents. It IS the weaker half of this claim — an author-asserted key + // rather than a shape-backed one — which is why it is named in the failure + // text below and kept to the extension case. const broken: string[] = []; for (const s of SURFACES) { const shape = s.shape; + const extra = new Set(s.options.extraKeys ?? []); for (const [written, target] of Object.entries(s.options.aliases ?? {})) { + // The one sanctioned exception, enumerated above: a nav-item target + // that is a SENTENCE about the `type`, because the key the author wrote + // is spelled correctly and only the variant is wrong. + if (isProseTarget(s.options.surface, target)) continue; + if (extra.has(target)) continue; if (!(target in shape)) { - broken.push(`${entry(s, written, target)} — \`${target}\` is not declared here`); + broken.push( + `${entry(s, written, target)} — \`${target}\` is not declared here` + + ' (declare it, retarget the alias, or — only if the table rides `.extend()`' + + ' onto a surface that DOES declare it — name it in `extraKeys`)', + ); } else if (acceptsNothing(shape[target])) { broken.push(`${entry(s, written, target)} — \`${target}\` is a tombstone; it accepts nothing`); } @@ -539,6 +636,21 @@ describe('alias integrity — every table is a true claim about its schema', () expect(collisions.sort()).toEqual([]); }); + it('every prose-target exemption is still load-bearing', () => { + // An allowlist nobody reaches is the silent pass-through this exemption was + // written to avoid, one release later. If a nav variant is reworded or + // retired, the stale entries surface here instead of quietly widening what + // the target criterion above will forgive. + const used = new Set(); + for (const s of SURFACES) { + if (!PROSE_TARGET_SURFACE.test(s.options.surface)) continue; + for (const target of Object.values(s.options.aliases ?? {})) { + if (PROSE_ALIAS_TARGETS.has(target)) used.add(target); + } + } + expect([...PROSE_ALIAS_TARGETS].filter((x) => !used.has(x)).sort()).toEqual([]); + }); + it('no guidance key is itself a declared key (the same dead entry, other channel)', () => { // `guidance` is consulted from the same `unrecognized_keys` path, so a // prescription filed under a key the shape DECLARES is unreachable in @@ -557,254 +669,3 @@ describe('alias integrity — every table is a true claim about its schema', () expect(dead.sort()).toEqual([]); }); }); - -// --------------------------------------------------------------------------- -// 3. The same claims over the tables that never had a shape (#5483) -// --------------------------------------------------------------------------- - -/** - * Alias targets that are deliberately **prose, not a key name** — the only - * place in `packages/spec` where that is true, and an explicit allowlist rather - * than a relaxed criterion because the two are not the same promise. - * - * `ui/app.zod.ts` builds one table per navigation-item `type`, and the - * commonest nav mistake is not a typo: it is `dashboardName` written on a `url` - * item — a real key, on the wrong variant. Naming a key there would be wrong - * twice over (the key IS spelled correctly, and writing it is still not enough - * without the matching `type`), so the target is a sentence: - * - * Did you mean `dashboardname` → `type: 'dashboard' (with dashboardName)`? - * - * Enumerated, not pattern-matched, and paired with the surface family that owns - * them, so a new prose target anywhere — including a second one on this very - * surface — fails the target criterion and has to be argued for here. The - * staleness test below is the other half: an entry nothing uses is deleted, so - * this list cannot quietly outlive the tables it excuses. - * - * The seventh entry arrived by that route. #5555 fixed the defect this gate - * found — the four "start expanded" spellings redirecting to `expanded` on the - * eight variants that lack it — and the fix is precisely a demotion from bare - * key name to prose, so the tolerance that pinned it was deleted and this list - * grew by one. It is one string for four alias keys because they share a single - * answer: the key you want lives on `group`. - */ -const PROSE_ALIAS_TARGETS: ReadonlySet = new Set([ - "type: 'object' (with objectName)", - "type: 'page' (with pageName)", - "type: 'url' (with url)", - "type: 'dashboard' (with dashboardName)", - "type: 'report' (with reportName)", - "type: 'component' (with componentRef)", - "type: 'group' (with expanded)", -]); - -/** The surface family the prose targets are allowed on, and nowhere else. */ -const PROSE_TARGET_SURFACE = /^this `[a-z]+` navigation item$/; - -const isProseTarget = (surface: string, target: string): boolean => - PROSE_TARGET_SURFACE.test(surface) && PROSE_ALIAS_TARGETS.has(target); - -/* - * The shrink-only tolerance that used to sit here (`isPinnedExpandedDefect`, - * 32 occurrences) is GONE, not relaxed: #5555 fixed the defect it pinned. - * - * It held the four "start expanded" spellings that `NAV_ITEM_ALIASES` redirected - * to `expanded` on all nine nav variants, while `expanded` is declared on `group` - * alone — so on the other eight the redirect named a key that variant also - * rejected (ledger finding 7's second rejection, from the campaign built to end - * it). The fix moved those four into the per-variant assembly: `group` keeps the - * bare key name, the other eight answer with prose, so the seventh entry in - * PROSE_ALIAS_TARGETS above is where this debt went. Claim 2 below now judges - * the family with no tolerance at all — a re-introduction lands in `broken`. - */ - -/** - * Guidance filed once for a table stamped nine times, legal on two of them. - * - * `children` really is declared on the `object` and `group` nav variants, so - * the prescription "`children` is only meaningful on a `group` item…" correctly - * never fires there — it is written for the other seven, where it does. That is - * a criterion meeting a variant family, not a dead entry: the shape-backed half - * of this gate judges one authored table against one shape, and here one - * authored table is stamped against nine. - * - * Exempted rather than pinned as debt because there is nothing to fix — no - * author is misinformed and no prescription is lost. Enumerated by - * `(surface, key)` so it cannot cover a second guidance entry that IS dead. - */ -const VARIANT_LEGAL_GUIDANCE: ReadonlySet = new Set([ - 'this `object` navigation item::children', - 'this `group` navigation item::children', -]); - -/** `file:line — "surface": \`written\` -> \`target\``, for the direct batch. */ -const directEntry = (d: DirectAliasTableDeclaration, written: string, target: string): string => { - const site = DIRECT_CALL_SITES.find( - (c) => c.surface === d.options.surface - && Object.entries(c.aliases).every(([k, v]) => d.options.aliases?.[k] === v), - ); - const where = site ? `${site.file}:${site.line}` : '(location unresolved)'; - return `${where} — "${d.options.surface}": \`${written}\` -> \`${target}\``; -}; - -describe('alias integrity — direct `strictUnknownKeyError` tables (#5483)', () => { - it('the direct call sites really registered (self-test before the verdict)', () => { - // Same guard as the shape-backed half: state the scale before the verdict, - // so a registration that silently stopped working reads as a failure and - // not as forty-four clean tables. - expect(DIRECT_CALL_SITES.length).toBeGreaterThanOrEqual(40); - // One call site (the nav-item factory) runs nine times, so the registry is - // legitimately LARGER than the source count. It can never be smaller - // without a table having gone unjudged. - expect(DIRECT.length).toBeGreaterThanOrEqual(DIRECT_CALL_SITES.length); - expect(DIRECT.some((d) => Object.keys(d.options.aliases ?? {}).length > 0)).toBe(true); - // The registry is capped (it is filled by a PUBLISHED factory — see - // `alias-table-registry.ts`). Overflow would mean judging a prefix. - expect(directAliasTableOverflow(), 'the direct registry overflowed; raise CAPACITY').toBe(0); - }); - - it('every direct call site with an alias table was reached at runtime', () => { - const bySurface = new Map(); - for (const d of DIRECT) { - const list = bySurface.get(d.options.surface) ?? []; - list.push(d); - bySurface.set(d.options.surface, list); - } - const unreached: string[] = []; - for (const site of DIRECT_CALL_SITES) { - if (!site.hasAliases) continue; - const candidates = site.surface ? (bySurface.get(site.surface) ?? []) : DIRECT; - const matched = candidates.some((d) => - Object.entries(site.aliases).every(([k, v]) => d.options.aliases?.[k] === v)); - if (!matched) unreached.push(`${site.file}:${site.line} (${site.surface ?? 'assembled surface'})`); - } - expect(unreached, 'these alias tables never reached the registry, so nothing judges them').toEqual([]); - }); - - it('the deferred error maps were forced too', () => { - // `data/object.zod.ts` builds its map on FIRST USE, to step around a - // temporal dead zone. Nothing in this file parses anything, so without the - // synthetic-issue poke in `force()` that table never registers. - // - // Measured, not assumed: deleting the poke turns this red AND the coverage - // check above (that site's `surface` and alias entries are both literals, - // so the AST can see it and report it unreached). Two failures for one - // cause — so this assertion is not what makes the poke's absence *visible*, - // it is what makes it legible. "`this object` is missing" names the - // mechanism; "some site at object.zod.ts:962 is unreached" sends the next - // reader looking for a walk bug that is not there. - expect( - DIRECT.map((d) => d.options.surface), - "`data/object.zod.ts`'s deferred map did not register — did the forcing poke stop working?", - ).toContain('this object'); - }); - - it('the nav-item factory registered one table per variant', () => { - // The one direct site the AST reads as an empty assembled table: both its - // surface (a template) and its aliases (spreads) are computed. Nine - // variants in, nine tables out — the count is the coverage. - const navTables = DIRECT.filter((d) => PROSE_TARGET_SURFACE.test(d.options.surface)); - expect(navTables.length).toBe(9); - }); - - it('no alias key is itself a known key (a dead entry that can never fire)', () => { - // Claim 1, answered against the transcribed `knownKeys` rather than a - // shape. Weaker — a key list that has drifted from its schema drags the - // answer with it — but it is the list the SUGGESTER reads, so a hit here is - // a real dead entry either way. - const dead: string[] = []; - for (const d of DIRECT) { - const known = new Set(d.options.knownKeys); - for (const [written, target] of Object.entries(d.options.aliases ?? {})) { - if (known.has(written)) { - dead.push(`${directEntry(d, written, target)} — \`${written}\` is a known key here`); - } - } - } - expect(dead.sort()).toEqual([]); - }); - - it('every alias target is a key the table claims to accept', () => { - // Claim 2. The tombstone half of the shape-backed version has no analogue - // here: `knownKeys` is a flat array with no schemas in it, so "this target - // is declared but accepts nothing" is invisible until the call site - // migrates (#5593). - const broken: string[] = []; - for (const d of DIRECT) { - const known = new Set(d.options.knownKeys); - for (const [written, target] of Object.entries(d.options.aliases ?? {})) { - if (known.has(target) || isProseTarget(d.options.surface, target)) continue; - broken.push(`${directEntry(d, written, target)} — \`${target}\` is not a known key here`); - } - } - // No tolerance: #5555 closed the last one (see the note above - // `VARIANT_LEGAL_GUIDANCE`), so every direct table answers this claim - // outright — including the nav family that used to carry the 32. - expect(broken.sort()).toEqual([]); - }); - - it('every prose-target exemption is still load-bearing', () => { - // An allowlist nobody reaches is the silent pass-through this exemption was - // written to avoid, one release later. If a nav variant is reworded, or the - // family migrates to `strictObject`, the stale entries surface here instead - // of quietly widening what claim 2 will forgive. - const used = new Set(); - for (const d of DIRECT) { - if (!PROSE_TARGET_SURFACE.test(d.options.surface)) continue; - for (const target of Object.values(d.options.aliases ?? {})) { - if (PROSE_ALIAS_TARGETS.has(target)) used.add(target); - } - } - expect([...PROSE_ALIAS_TARGETS].filter((t) => !used.has(t)).sort()).toEqual([]); - }); - - it('no two alias keys in one table collapse onto the same probe (#5481)', () => { - // Claim 3, and the only one that loses NOTHING for lack of a shape: the - // probe reads the alias table alone. This dimension was never measured on - // these 44 tables — #5481 postdates #5483's "measured clean" note, which - // covered the other two claims — so this assertion is the measurement, not - // a re-statement of one. - const collisions: string[] = []; - for (const d of DIRECT) { - const byProbe = new Map(); - for (const key of Object.keys(d.options.aliases ?? {})) { - byProbe.set(aliasProbe(key), [...(byProbe.get(aliasProbe(key)) ?? []), key]); - } - for (const [probe, keys] of byProbe) { - if (keys.length < 2) continue; - const written = keys.map((k) => `\`${k}\` -> \`${d.options.aliases?.[k]}\``).join(', '); - collisions.push( - `${directEntry(d, keys[0], d.options.aliases?.[keys[0]] ?? '?')} — ${keys.length} keys share the probe \`${probe}\`: ${written}` - + ` — only \`${d.options.aliases?.[keys[keys.length - 1]]}\` survives`, - ); - } - } - expect(collisions.sort()).toEqual([]); - }); - - it('no guidance key is itself a known key (the same dead entry, other channel)', () => { - const dead: string[] = []; - for (const d of DIRECT) { - const known = new Set(d.options.knownKeys); - for (const written of Object.keys(d.options.guidance ?? {})) { - if (!known.has(written)) continue; - if (VARIANT_LEGAL_GUIDANCE.has(`${d.options.surface}::${written}`)) continue; - dead.push(`"${d.options.surface}": guidance for \`${written}\`, which is a known key here`); - } - } - expect(dead.sort()).toEqual([]); - }); - - it('every variant-legal guidance exemption is still load-bearing', () => { - // Same staleness rule the prose targets get: an exemption nobody reaches is - // deleted, not left widening what the criterion above forgives. - const reached = new Set(); - for (const d of DIRECT) { - const known = new Set(d.options.knownKeys); - for (const written of Object.keys(d.options.guidance ?? {})) { - if (known.has(written)) reached.add(`${d.options.surface}::${written}`); - } - } - expect([...VARIANT_LEGAL_GUIDANCE].filter((e) => !reached.has(e)).sort()).toEqual([]); - }); -}); diff --git a/packages/spec/src/shared/alias-table-registry.ts b/packages/spec/src/shared/alias-table-registry.ts deleted file mode 100644 index c1b472ff3b..0000000000 --- a/packages/spec/src/shared/alias-table-registry.ts +++ /dev/null @@ -1,118 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * The registry for alias tables that reach {@link strictUnknownKeyError} - * **directly** — the pre-`strictObject` wiring (#5483). - * - * ## Why a second registry - * - * `strictObject` records `{ options, shape }` at construction, so - * `alias-integrity.test.ts` can judge a table against the **shape** it makes - * claims about. The 44 call sites that predate the helper have no shape to - * offer: they hand `strictUnknownKeyError` a hand-transcribed `knownKeys` - * array. That is a weaker instrument — the array can drift from the schema it - * describes and nothing here can see it — and mixing the two into one registry - * would quietly relabel that weakness as shape-backed coverage. - * - * So they are kept apart, and the difference is the point: - * - * | claim | `strictObject` registry | this registry | - * |:--|:--|:--| - * | alias key is not a declared key | judged vs `.shape` | judged vs `knownKeys` | - * | alias target is a declared key | judged vs `.shape` (+ tombstone check) | judged vs `knownKeys` | - * | no two alias keys share a probe | judged | judged — **identically** | - * - * The third claim (#5481) is the one that loses nothing: `aliasProbe` - * collisions are a property of the table *alone*, so judging them here is the - * same measurement the covered surfaces get, not an approximation of it. That - * is what made the transitional guard worth shipping ahead of the migration — - * before it, those 44 tables were **unmeasured** on collisions, not clean. - * - * Retiring this registry is the migration in #5593: every call site that moves - * to `strictObject` leaves here and arrives there, shape-backed, and the last - * one takes this file with it. - * - * ## Not part of the package contract - * - * Deliberately **not** re-exported from `shared/index.ts`, like - * `strict-object.ts` and `alias-probe.ts`: it is an internal seam the gate - * reaches by relative path. Adding it to the barrel would publish a mutable - * process-global as API. - */ - -import type { StrictUnknownKeyErrorOptions } from './suggestions.zod'; - -/** One alias table recorded as its owning error map was built. */ -export interface DirectAliasTableDeclaration { - /** The authoring metadata the error map was built from. */ - readonly options: StrictUnknownKeyErrorOptions; -} - -/** - * Upper bound on recorded tables. - * - * `strictObject` is internal, so its registry can only ever grow with the - * schemas this repo declares. `strictUnknownKeyError` is **published** (it is - * in `api-surface.json` under `./shared`), and a consumer is free to build one - * error map per tenant, per request, in a loop — a registry that recorded every - * one of those would be an unbounded retainer in someone else's process for the - * sole benefit of a test in ours. - * - * The cap makes the failure mode a *measurement* problem rather than a memory - * one, and `overflowed()` makes even that loud: the gate asserts it is zero, so - * if this repo ever declares more tables than fit, the check fails instead of - * silently judging a prefix. In-repo occupancy is ~52 of 512. - */ -const CAPACITY = 512; - -const DECLARATIONS: DirectAliasTableDeclaration[] = []; -let overflow = 0; -let suppression = 0; - -/** - * Record a table built by a direct {@link strictUnknownKeyError} call. - * - * Called from the factory itself rather than from the 44 call sites, which is - * the whole reason the transitional guard costs no call-site edits. - */ -export function registerDirectAliasTable(options: StrictUnknownKeyErrorOptions): void { - if (suppression > 0) return; - if (DECLARATIONS.length >= CAPACITY) { - overflow++; - return; - } - DECLARATIONS.push({ options }); -} - -/** - * Run `build` with registration turned off — for `strictObject`, whose tables - * are already recorded **with their shape** in the richer registry. - * - * Without this, a `strictObject` surface would land in both registries the - * moment its deferred error map got built (which the gate's forcing pass does - * deliberately, and any parse failure does incidentally), and this registry's - * population would depend on whether some earlier test had rejected a key. - * A count that moves with unrelated test ordering is not a measurement. - * - * Depth-counted rather than a boolean because the suppressed callback is free - * to build another map; synchronous throughout, so there is no interleaving to - * lose track of. - */ -export function withoutDirectAliasTableRegistration(build: () => T): T { - suppression++; - try { - return build(); - } finally { - suppression--; - } -} - -/** Every direct table built **so far in this process** (#5483). */ -export function directAliasTables(): readonly DirectAliasTableDeclaration[] { - return DECLARATIONS; -} - -/** How many tables were dropped for want of {@link CAPACITY}. Asserted zero. */ -export function directAliasTableOverflow(): number { - return overflow; -} diff --git a/packages/spec/src/shared/strict-object.test.ts b/packages/spec/src/shared/strict-object.test.ts index 069dc230f0..7ada4744e7 100644 --- a/packages/spec/src/shared/strict-object.test.ts +++ b/packages/spec/src/shared/strict-object.test.ts @@ -1,5 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +import { execFileSync } from 'node:child_process'; + import { z } from 'zod'; import { describe, expect, it } from 'vitest'; @@ -350,3 +352,47 @@ describe('strictObject — the error map is lazy, so cycles cannot break it', () expect(aliasesRead).toBe(1); }); }); + +// ============================================================================ +// #5593 — this module survives being entered FIRST in its own import cycle. +// +// `strictObject` is called at MODULE SCOPE by schemas that sit inside the +// `field.zod` ↔ `suggestions.zod` ↔ `strict-object` cycle, so under +// `OS_EAGER_SCHEMAS=1` it can run while this module is still initializing. +// Everything it touches on the way in must therefore be reachable from the +// first instruction of module evaluation — which rules out a module-level +// `const` for the declaration registry, and is why `declarationStore()` is a +// hoisted `function` declaration. +// +// ⚠️ Why this needs its own subprocess rather than an ordinary assertion: +// `lazySchema` defers construction behind a Proxy, so a normal `vitest run` +// never evaluates a schema at import time and the hazard is invisible. #5593 +// shipped the regression and the ordinary suite stayed green — what caught it +// was CI's `check-test-completeness` gate noticing that +// `automation/flow-region-cycle.test.ts` was counted and never reported, +// because ITS subprocess died at import with `ReferenceError: Cannot access +// 'DECLARATIONS' before initialization` and vitest could not even format the +// stack. Two guards, one hazard: that file states the cycle it protects +// (#4415), this one states the rule this module has to keep. +// +// The entry point is deliberately `data/field.zod.ts` — the module whose +// own module-scope `strictObject(…)` call is the one that lands here +// mid-initialization — reached through a module that pulls THIS file first. +// ============================================================================ +describe('#5593 — eager construction with this module entered first', () => { + it('does not throw at import time under OS_EAGER_SCHEMAS=1', () => { + const barrel = new URL('../automation/index.ts', import.meta.url).href; + const run = (): string => + execFileSync( + process.execPath, + ['--import', 'tsx', '--input-type=module', '-e', + `import ${JSON.stringify(barrel)}; + console.log('ok');`], + { env: { ...process.env, OS_EAGER_SCHEMAS: '1' }, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }, + ).trim(); + expect( + run(), + 'a module-level `const` in strict-object.ts is in its TDZ here — use a hoisted function', + ).toBe('ok'); + }, 60_000); +}); diff --git a/packages/spec/src/shared/strict-object.ts b/packages/spec/src/shared/strict-object.ts index 4ef334cd99..4e70f672a4 100644 --- a/packages/spec/src/shared/strict-object.ts +++ b/packages/spec/src/shared/strict-object.ts @@ -60,7 +60,6 @@ import { z } from 'zod'; -import { withoutDirectAliasTableRegistration } from './alias-table-registry'; import { strictUnknownKeyError } from './suggestions.zod'; /** @@ -164,7 +163,41 @@ export interface StrictObjectDeclaration { readonly shape: z.ZodRawShape; } -const DECLARATIONS: StrictObjectDeclaration[] = []; +/** + * The registry array, owned by a HOISTED function declaration. + * + * ⚠️ **Deliberately not a module-level `const`, and this is load-bearing.** + * `strictObject` is called at MODULE SCOPE by schemas that sit inside the + * `field.zod` ↔ `suggestions.zod` ↔ this module import cycle, so under + * `OS_EAGER_SCHEMAS=1` it can run while this module is still initializing. A + * `const` is in its temporal dead zone until its own line executes, so the call + * throws `ReferenceError: Cannot access 'DECLARATIONS' before initialization` + * at IMPORT time — before a single test body runs. A hoisted `function` + * declaration is fully initialized from the first instruction of module + * evaluation, which is the same property `automation/flow.zod.ts`'s + * `flowNodeObject()` relies on for its own cycle (#4415, and its docblock says + * so out loud). + * + * Measured, not assumed. On `main` the cycle happened to be entered through + * `field.zod` first, which resolves this module fully before anything calls + * into it. #5593 moved `automation/`'s schemas from `strictUnknownKeyError` to + * this helper, and that one edge reordered the entry: the `automation` barrel + * now reaches THIS module first, then `suggestions.zod`, then `field.zod`, + * whose own module-scope `strictObject(…)` call lands here mid-initialization. + * + * ⚠️ The failure mode is why this is written down rather than left to the + * types. `lazySchema` defers construction behind a Proxy, so an ordinary + * `vitest run` never evaluates a schema at import time and stays GREEN; the + * eager subprocess dies before any test body runs, and vitest cannot even + * format the stack, so the owning file reports *no result at all*. What caught + * it was CI's `check-test-completeness` gate noticing that + * `automation/flow-region-cycle.test.ts` was counted and never reported. Pinned + * from this side too, in `strict-object.test.ts`. + */ +function declarationStore(): StrictObjectDeclaration[] { + const self = declarationStore as unknown as { list?: StrictObjectDeclaration[] }; + return (self.list ??= []); +} /** * Every authoring shape {@link strictObject} has built **so far in this @@ -188,7 +221,7 @@ const DECLARATIONS: StrictObjectDeclaration[] = []; * nothing judges, and that must fail loudly rather than pass quietly. */ export function strictObjectDeclarations(): readonly StrictObjectDeclaration[] { - return DECLARATIONS; + return declarationStore(); } /** @@ -229,14 +262,14 @@ export function strictObject(options: StrictObjectOptio typeof issue.input === 'string' ? retiredForms[issue.input] : undefined; if (prescription) return prescription; } - // Built WITHOUT registering in the direct-call registry (#5483). This table - // is already recorded below with its `shape`, which is the stronger record: - // letting it land in both would judge it twice — the second time against - // the transcription-shaped view (`knownKeys`, tombstones filtered out) - // rather than the shape — and would make that registry's population depend - // on whether anything happened to reject a key first, since this build is - // deferred to the first rejection. - return (build ??= withoutDirectAliasTableRegistration(() => strictUnknownKeyError({ + // The table is recorded ONCE, below, with its `shape` — the strong record + // the audit reads. Between #5483 and #5593 a second, transcription-shaped + // registry existed for the 44 call sites that predated this helper, and + // this build had to be run with that registry suppressed so a + // `strictObject` surface would not be judged twice (the second time against + // `knownKeys` rather than the shape). #5593 migrated the last of those call + // sites and deleted the registry, so the suppression went with it. + return (build ??= strictUnknownKeyError({ surface, // Declared-but-unwritable keys (tombstones) are excluded — see // `acceptsNothing`. They stay in the SHAPE, so writing one still raises @@ -249,10 +282,10 @@ export function strictObject(options: StrictObjectOptio history, aliases, guidance, - })))(issue); + }))(issue); }; - DECLARATIONS.push({ options, shape }); + declarationStore().push({ options, shape }); return z.object(shape, { error }).strict(); } diff --git a/packages/spec/src/shared/suggestions.zod.ts b/packages/spec/src/shared/suggestions.zod.ts index 313e056252..3dfee12854 100644 --- a/packages/spec/src/shared/suggestions.zod.ts +++ b/packages/spec/src/shared/suggestions.zod.ts @@ -4,7 +4,6 @@ import type { z } from 'zod'; import { FieldType } from '../data/field.zod'; import { aliasProbe } from './alias-probe'; -import { registerDirectAliasTable } from './alias-table-registry'; /** * "Did you mean?" Suggestion Utilities @@ -292,25 +291,23 @@ export interface StrictUnknownKeyErrorOptions { * ~220 on the single-line displays several consumers use. It is still emitted * verbatim and unconditionally — only its position moved. * - * ## The table is recorded as it is built (#5483) + * ## No in-repo caller passes `knownKeys` by hand any more (#5593) * - * Every call registers its `{ surface, knownKeys, aliases, guidance }` with - * `alias-table-registry`, which is what puts the 44 remaining direct call sites - * under `alias-integrity.test.ts`. Registering **here** rather than at the call - * sites is the entire trick: the gate gains 44 tables and the schemas gain no - * edit, so the migration to `strictObject` (#5593) stays a clean, separable - * change rather than something this guard has already half-done. + * `knownKeys` is a hand-transcribed array — a second copy of the shape it + * describes — so a table built this way could only ever be audited against the + * transcription, and a drifted array dragged the audit with it. #5483 shipped a + * transitional registry that at least put those 44 call sites under + * `alias-integrity.test.ts`; #5593 migrated every one of them to + * `strictObject`, which derives the candidate list from the shape, and + * deleted the registry with the last of them. * - * What the gate can judge from a registration is bounded by what a direct call - * offers. `knownKeys` is a hand-transcribed array, so "is this alias target a - * real key?" is answered against the transcription, not the shape — the - * array-vs-shape drift `strictObject` exists to abolish stays unguarded until - * the call site migrates. The `aliasProbe` collision claim has no such caveat: - * it reads the alias table alone, so it is judged here exactly as it is on a - * `strictObject` surface. + * This factory stays PUBLISHED and unchanged for external callers, but inside + * `packages/spec` the only caller is `strictObject` itself. That is enforced, + * not merely true: `alias-integrity.test.ts` fails on any new direct call site + * here, because a new one would mint a fresh second copy of a key list and + * arrive outside the shape-backed audit. */ export function strictUnknownKeyError(options: StrictUnknownKeyErrorOptions): z.core.$ZodErrorMap { - registerDirectAliasTable(options); const { surface, knownKeys, guidance = {}, history } = options; const aliases: Record = {}; for (const [key, canonical] of Object.entries(options.aliases ?? {})) { diff --git a/packages/spec/src/ui/action.test.ts b/packages/spec/src/ui/action.test.ts index 5743573de3..0c0121529f 100644 --- a/packages/spec/src/ui/action.test.ts +++ b/packages/spec/src/ui/action.test.ts @@ -157,26 +157,6 @@ describe('ActionParamSchema', () => { expect(message).toContain('`wibble`'); expect(message).not.toContain('Did you mean'); }); - - it('accepts every key the schema declares (guards ACTION_PARAM_KEYS drift)', () => { - // If a declared key were missing from the suggestion list, or a listed key - // were removed from the schema, one of these probes would be rejected. - const probes: Record = { - name: 'p', field: 'inspector', objectOverride: 'sys_member', label: 'P', - type: 'lookup', required: true, options: [{ label: 'A', value: 'a' }], - placeholder: 'ph', helpText: 'help', defaultValue: 'd', multiple: true, - accept: ['image/*'], maxSize: 1024, reference: 'sys_user', - defaultFromRow: true, visible: 'features.phoneNumber == true', - requiresFeature: 'phoneNumber', - }; - for (const [key, value] of Object.entries(probes)) { - const result = ActionParamSchema.safeParse({ name: 'p', [key]: value }); - const unknown = result.success - ? undefined - : result.error.issues.find((i) => i.code === 'unrecognized_keys'); - expect(unknown, `\`${key}\` should be a declared ActionParam key`).toBeUndefined(); - } - }); }); }); diff --git a/packages/spec/src/ui/action.zod.ts b/packages/spec/src/ui/action.zod.ts index 2cd4cdba9e..1cd64f9d68 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -50,20 +50,6 @@ import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; */ import { lazySchema } from '../shared/lazy-schema'; -/** - * Keys `ActionParamSchema` declares. - * - * Kept beside the schema rather than derived from `.shape`: the schema body is - * allocated lazily (see `lazySchema`), and the error map below has to name a - * canonical key *while* that first parse is still in flight. `action.zod.test.ts` - * asserts every entry here is really accepted, so the list cannot rot silently. - */ -const ACTION_PARAM_KEYS = [ - 'name', 'field', 'objectOverride', 'label', 'type', 'required', 'options', - 'placeholder', 'helpText', 'defaultValue', 'multiple', 'accept', 'maxSize', - 'reference', 'defaultFromRow', 'visible', 'requiresFeature', -] as const; - /** * Semantic near-misses — a different **word** for the same intent, usually * borrowed from a neighbouring schema where that word is correct. Edit distance @@ -164,16 +150,15 @@ const actionParamOptionUndeclaredAnywhere = (key: 'icon' | 'disabled'): string = + `\`SelectOptionMetadata\` type, which no metadata path populates and no widget reads. An ` + `action param's options are \`{ label, value, visibleWhen }\`; drop the key.`; -const actionParamUnknownKeyError = strictUnknownKeyError({ - surface: 'this action param', - knownKeys: ACTION_PARAM_KEYS, - aliases: ACTION_PARAM_KEY_ALIASES, - history: - 'Until #3405 these were dropped silently — the param still parsed, so a mis-spelled ' + - 'config shipped as a control that quietly ignored it.', -}); - -export const ActionParamSchema = lazySchema(() => z.object({ +export const ActionParamSchema = lazySchema(() => strictObject( + { + surface: 'this action param', + aliases: ACTION_PARAM_KEY_ALIASES, + history: + 'Until #3405 these were dropped silently — the param still parsed, so a mis-spelled ' + + 'config shipped as a control that quietly ignored it.', + }, + { /** Request-body key. Defaults to `field` when `field` is set. */ name: z.string().optional(), /** Reference an existing object field for label/type/validation/options. */ @@ -385,7 +370,7 @@ export const ActionParamSchema = lazySchema(() => z.object({ * enum-checked and the gate/registry stay in lockstep. */ requiresFeature: z.enum(PUBLIC_AUTH_FEATURE_NAMES).optional().describe('Public auth feature flag gating this param; lowered into `visible` at parse time.'), -}, { error: actionParamUnknownKeyError }).strict().refine( +}).refine( (p) => Boolean(p.name) || Boolean(p.field), { message: 'ActionParam requires either "name" or "field"' }, ).refine( diff --git a/packages/spec/src/ui/app.test.ts b/packages/spec/src/ui/app.test.ts index 5f46325bf2..4df2e5242c 100644 --- a/packages/spec/src/ui/app.test.ts +++ b/packages/spec/src/ui/app.test.ts @@ -1143,25 +1143,6 @@ describe('unknown keys are rejected, not stripped (#4001 PR B)', () => { expect(result.success).toBe(false); expect(result.error!.issues.map((i) => i.message).join('\n')).toContain('FormView.sharing'); }); - - it('accepts every key the schema declares (guards APP_KEYS drift)', () => { - const probes: Record = { - description: 'd', icon: 'briefcase', branding: { primaryColor: '#fff' }, - active: false, isDefault: true, hidden: true, - navigation: [{ id: 'nav_a', label: 'A', type: 'object', objectName: 'account' }], - areas: [{ id: 'area_a', label: 'A', navigation: [] }], - contextSelectors: [{ id: 'pkg', label: 'Package', optionsSource: { endpoint: '/api/v1/packages' } }], - requiredPermissions: ['app.access.x'], - defaultAgent: 'ask', protection: { lock: 'none' }, - }; - for (const [key, value] of Object.entries(probes)) { - const result = AppSchema.safeParse({ name: 'app_a', label: 'A', [key]: value }); - const unknown = result.success - ? undefined - : result.error.issues.find((i) => i.code === 'unrecognized_keys'); - expect(unknown, `\`${key}\` should be a declared App key`).toBeUndefined(); - } - }); }); describe('navigation items (discriminated union)', () => { diff --git a/packages/spec/src/ui/app.zod.ts b/packages/spec/src/ui/app.zod.ts index 877258686a..516034a9f5 100644 --- a/packages/spec/src/ui/app.zod.ts +++ b/packages/spec/src/ui/app.zod.ts @@ -5,7 +5,7 @@ import { SnakeCaseIdentifierSchema } from '../shared/identifiers.zod'; import { ExpressionInputSchema } from '../shared/expression.zod'; import { I18nLabelSchema } from './i18n.zod'; import { retiredKey } from '../shared/retired-key'; -import { strictUnknownKeyError } from '../shared/suggestions.zod'; +import { strictObject, type StrictObjectOptions } from '../shared/strict-object'; /** * Base Navigation Item Schema @@ -55,12 +55,6 @@ import { ProtectionSchema } from '../shared/protection.zod'; * dead surface (ADR-0049 enforce-or-remove). */ -/** Keys every nav-item variant shares (drift-guarded by app.test.ts). */ -const BASE_NAV_ITEM_KEYS = [ - 'id', 'label', 'icon', 'order', 'badge', 'badgeVariant', 'visible', - 'requiredPermissions', 'requiresObject', 'requiresService', 'type', -] as const; - /** * Semantic near-misses shared by every nav-item variant. * @@ -124,61 +118,142 @@ const NAV_EXPANDED_ALIASES_ELSEWHERE: Readonly> = { isopen: 'type: \'group\' (with expanded)', }; -/** Per-variant payload keys, for the error map's suggestion pool. */ -const NAV_VARIANT_KEYS: Readonly> = { - object: ['objectName', 'viewName', 'recordId', 'recordMode', 'filters', 'children'], - dashboard: ['dashboardName'], - page: ['pageName', 'params'], - url: ['url', 'target'], - report: ['reportName'], - action: ['actionDef'], - component: ['componentRef', 'params'], - group: ['expanded', 'children'], - separator: [], +/** Every `type` a navigation item can carry — one strict branch each. */ +type NavItemVariant = + | 'object' | 'dashboard' | 'page' | 'url' | 'report' + | 'action' | 'component' | 'group' | 'separator'; + +/** + * The two variants that ACCEPT `children`. + * + * Not on the branch schema itself — {@link NavigationItemSchema} `.extend()`s + * the recursive `children` onto these two members, and strictness plus the + * error map ride that extension, so `children` is legal on exactly these two at + * the door anything actually parses. Two facts follow, and they are the reason + * this set exists at all: + * + * 1. the `children` PRESCRIPTION must not be filed on them — it would be a dead + * entry on the extended surface, which `alias-integrity.test.ts` rejects; + * 2. `children` belongs in their `extraKeys`, so a near-miss (`childs`, + * `childrens`) still resolves on the extended surface even though the branch + * schema's own `.shape` has no such key. + * + * Kept as a two-entry set rather than the nine-entry key transcription this + * factory used to carry (#5593): everything else the suggestion pool needs is + * read from each branch's own `.shape`. Getting this set wrong in the dangerous + * direction — naming a variant whose member does accept `children` — fails the + * gate rather than shipping a dead entry. + */ +const NAV_VARIANTS_ACCEPTING_CHILDREN: ReadonlySet = new Set(['object', 'group']); + +/** + * The separator's own alias table — the subset of {@link NAV_ITEM_ALIASES} whose + * TARGET the separator branch actually declares. + * + * A separator is a divider: `type`, an optional `id`, an optional `order`, and + * nothing else. Every other entry in the shared table would name a key this + * branch rejects, so it is demoted to {@link SEPARATOR_NAV_ITEM_GUIDANCE}. + */ +const SEPARATOR_NAV_ITEM_ALIASES: Readonly> = { + name: 'id', + sort: 'order', + sortorder: 'order', + position: 'order', }; /** - * Build the strict error map for one nav-item variant. Each variant gets its - * own so the "did you mean" pool is that variant's real key set — suggesting - * `dashboardName` on a `url` item would be noise, not help. + * What a separator answers for the base nav keys it does NOT declare. + * + * One sentence, filed under each key an author is likely to reach for, because + * they all have the same answer: a divider carries no label, no icon, no badge + * and no gate — those belong on the items it separates, and a *titled* section + * is a `group`, not a separator. Filed as `guidance` rather than as aliases + * because there is no key here to rename onto (finding 7 is exactly the mistake + * of answering with one). */ -const navItemUnknownKeyError = (variant: keyof typeof NAV_VARIANT_KEYS) => - strictUnknownKeyError({ - surface: `this \`${variant}\` navigation item`, - knownKeys: [...BASE_NAV_ITEM_KEYS, ...NAV_VARIANT_KEYS[variant]], - aliases: { - ...NAV_ITEM_ALIASES, - // Cross-variant payloads: naming the right key on the wrong `type` is - // the commonest nav mistake, so point at the type that owns it. - ...(variant !== 'object' ? { objectname: 'type: \'object\' (with objectName)' } : {}), - ...(variant !== 'page' ? { pagename: 'type: \'page\' (with pageName)' } : {}), - ...(variant !== 'url' ? { url: 'type: \'url\' (with url)' } : {}), - ...(variant !== 'dashboard' ? { dashboardname: 'type: \'dashboard\' (with dashboardName)' } : {}), - ...(variant !== 'report' ? { reportname: 'type: \'report\' (with reportName)' } : {}), - ...(variant !== 'component' ? { componentref: 'type: \'component\' (with componentRef)' } : {}), - // `expanded` is a cross-variant key too — it just looks shared because - // "start expanded" is a sidebar-wide idea. It exists on `group` alone, so - // only `group` may answer with the bare key name (#5555). - ...(variant !== 'group' ? NAV_EXPANDED_ALIASES_ELSEWHERE : NAV_EXPANDED_ALIASES_ON_GROUP), - }, - guidance: { - children: - '`children` is only meaningful on a `group` item (or an `object` item nesting its ' + - 'views). Nest entries under `{ type: \'group\', children: [...] }`.', - }, - history: - 'Until #4001 these were dropped silently — the entry still parsed, so a mis-spelled ' + - 'config shipped as a nav item that quietly ignored it (a stripped `visible` renders ' + - 'an entry that should have been gated).', - }); - -const actionDefUnknownKeyError = strictUnknownKeyError({ - surface: "this nav item's action definition", - knownKeys: ['actionName', 'params'], - aliases: { action: 'actionName', name: 'actionName', args: 'params', input: 'params' }, +const SEPARATOR_NAV_ITEM_GUIDANCE: Readonly> = Object.fromEntries( + ['label', 'title', 'icon', 'badge', 'badgeVariant', 'visible', 'requiredPermissions', 'requiresObject', 'requiresService'] + .map((key) => [ + key, + `\`${key}\` is not a separator key — a separator is a divider and declares only ` + + '`id` and `order`. Put labels, icons, badges and visibility gating on the items it ' + + "separates, or use `{ type: 'group', label: '…' }` for a titled section.", + ]), +); + +/** + * Authoring-surface options for one nav-item variant. + * + * Each variant gets its own table so the "did you mean" pool is that variant's + * real key set — suggesting `dashboardName` on a `url` item would be noise, not + * help. Before #5593 the pool was a hand-transcribed + * `[...BASE_NAV_ITEM_KEYS, ...NAV_VARIANT_KEYS[variant]]`; `strictObject` reads + * it from the branch's `.shape` instead, so the two copies became one and the + * nine tables joined the shape-backed half of the alias-integrity gate — which + * is where the "alias target must be a key this shape accepts" claim finally + * reaches the family that historically broke it (#5555). + * + * ⚠️ The PROSE targets below survive that stronger judgement deliberately, and + * `alias-integrity.test.ts`'s `PROSE_ALIAS_TARGETS` allowlist matches these + * strings EXACTLY. They are not key names and must not become key names: the + * key an author wrote is spelled correctly, it is the `type` that is wrong, so + * answering with a bare key name would be ledger finding 7 (a rename onto a key + * this variant also rejects). + */ +const navItemSurface = (variant: NavItemVariant): StrictObjectOptions => ({ + surface: `this \`${variant}\` navigation item`, + aliases: { + // ⚠️ `separator` is the ONE branch that spreads nothing — it declares + // `type`/`id`/`order` and no more — so the shared table's targets (`label`, + // `visible`, `requiredPermissions`, `badgeVariant`, `requiresObject`) are + // keys IT REJECTS. Answering `title` with *"did you mean `label`?"* there + // was ledger finding 7, live on `main`: the author fixes the spelling as + // instructed and is rejected a second time, with no suggestion left. It + // survived #5483's guard because the hand-transcribed key list handed every + // variant `[...BASE_NAV_ITEM_KEYS, ...]`, base keys included, so the + // transcription said `label` was known here and the guard believed it — + // the exact array-vs-shape drift #5593 exists to abolish, found by the + // migration itself. The nine base-only spellings become a PRESCRIPTION + // below instead of a rename. + ...(variant === 'separator' ? SEPARATOR_NAV_ITEM_ALIASES : NAV_ITEM_ALIASES), + // Cross-variant payloads: naming the right key on the wrong `type` is + // the commonest nav mistake, so point at the type that owns it. + ...(variant !== 'object' ? { objectname: 'type: \'object\' (with objectName)' } : {}), + ...(variant !== 'page' ? { pagename: 'type: \'page\' (with pageName)' } : {}), + ...(variant !== 'url' ? { url: 'type: \'url\' (with url)' } : {}), + ...(variant !== 'dashboard' ? { dashboardname: 'type: \'dashboard\' (with dashboardName)' } : {}), + ...(variant !== 'report' ? { reportname: 'type: \'report\' (with reportName)' } : {}), + ...(variant !== 'component' ? { componentref: 'type: \'component\' (with componentRef)' } : {}), + // `expanded` is a cross-variant key too — it just looks shared because + // "start expanded" is a sidebar-wide idea. It exists on `group` alone, so + // only `group` may answer with the bare key name (#5555). + ...(variant !== 'group' ? NAV_EXPANDED_ALIASES_ELSEWHERE : NAV_EXPANDED_ALIASES_ON_GROUP), + }, + // The recursive `children` key, which lives on the UNION member rather than on + // the branch — see {@link NAV_VARIANTS_ACCEPTING_CHILDREN}. Naming it keeps a + // near-miss resolvable on the surface that really accepts it. + ...(NAV_VARIANTS_ACCEPTING_CHILDREN.has(variant) ? { extraKeys: ['children'] } : {}), + // Filed only where it can FIRE. `children` is legal on the `object` and + // `group` members, so the prescription is written for the other seven; #5483's + // guard had to exempt the two by name (`VARIANT_LEGAL_GUIDANCE`) because a + // transcription cannot tell a legal variant from a dead entry. The shape can, + // so the exemption is deleted and the table is simply correct per variant + // (#5593) — the same demotion-instead-of-tolerance move #5555 made one field + // over. The separator's own prescriptions ride alongside it. + guidance: { + ...(NAV_VARIANTS_ACCEPTING_CHILDREN.has(variant) + ? {} + : { + children: + '`children` is only meaningful on a `group` item (or an `object` item nesting its ' + + 'views). Nest entries under `{ type: \'group\', children: [...] }`.', + }), + ...(variant === 'separator' ? SEPARATOR_NAV_ITEM_GUIDANCE : {}), + }, history: - 'Until #4001 these were dropped silently — the definition still parsed, so clicking ' + - 'the entry dispatched a different action than the author declared.', + 'Until #4001 these were dropped silently — the entry still parsed, so a mis-spelled ' + + 'config shipped as a nav item that quietly ignored it (a stripped `visible` renders ' + + 'an entry that should have been gated).', }); /** @@ -194,9 +269,12 @@ const actionDefUnknownKeyError = strictUnknownKeyError({ * 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 — + * And every branch already applies its own `strictObject` with the curated + * per-variant table ({@link navItemSurface}; `navItemUnknownKeyError` until + * #5593), so every key this base contributes is ALREADY gated at all nine + * doors — and, since #5593, the "did you mean" pool each door offers is read + * from that branch's own shape rather than from a transcription that included + * these keys whether the branch spread them or not. 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"*). @@ -303,7 +381,7 @@ const BaseNavItemSchema = z.object({ * objectName: 'ticket', filters: { owner_id: '{current_user_id}', status: 'open' } } * ``` */ -export const ObjectNavItemSchema = lazySchema(() => z.object({ +export const ObjectNavItemSchema = lazySchema(() => strictObject(navItemSurface('object'), { ...BaseNavItemSchema.shape, type: z.literal('object'), objectName: z.string().describe('Target object name'), @@ -343,7 +421,7 @@ export const ObjectNavItemSchema = lazySchema(() => z.object({ filters: z.record(z.string(), z.string()).optional().describe( 'URL filter conditions — targets the /:objectName/data bare surface via filter[]= params instead of a saved view. Values support template vars {current_user_id}, {current_org_id}. Mutually exclusive with recordId/viewName.', ), -}, { error: navItemUnknownKeyError('object') }).strict()); +})); /** * Correct-by-construction guard (ADR-0053 philosophy): `filters` combined @@ -374,58 +452,66 @@ const objectNavTargetExclusivity = ( * 2. Dashboard Navigation Item * Navigates to a specific dashboard. */ -export const DashboardNavItemSchema = lazySchema(() => z.object({ +export const DashboardNavItemSchema = lazySchema(() => strictObject(navItemSurface('dashboard'), { ...BaseNavItemSchema.shape, type: z.literal('dashboard'), dashboardName: z.string().describe('Target dashboard name'), -}, { error: navItemUnknownKeyError('dashboard') }).strict()); +})); /** * 3. Page Navigation Item * Navigates to a custom UI page/component. */ -export const PageNavItemSchema = lazySchema(() => z.object({ +export const PageNavItemSchema = lazySchema(() => strictObject(navItemSurface('page'), { ...BaseNavItemSchema.shape, type: z.literal('page'), pageName: z.string().describe('Target custom page component name'), // OPEN by design: the page owns its own param contract. params: z.record(z.string(), z.unknown()).optional().describe('Parameters passed to the page context'), -}, { error: navItemUnknownKeyError('page') }).strict()); +})); /** * 4. URL Navigation Item * Navigates to an external or absolute URL. */ -export const UrlNavItemSchema = lazySchema(() => z.object({ +export const UrlNavItemSchema = lazySchema(() => strictObject(navItemSurface('url'), { ...BaseNavItemSchema.shape, type: z.literal('url'), url: z.string().describe('Target external URL'), target: z.enum(['_self', '_blank']).default('_self').describe('Link target window'), -}, { error: navItemUnknownKeyError('url') }).strict()); +})); /** * 5. Report Navigation Item * Navigates to a specific report. */ -export const ReportNavItemSchema = lazySchema(() => z.object({ +export const ReportNavItemSchema = lazySchema(() => strictObject(navItemSurface('report'), { ...BaseNavItemSchema.shape, type: z.literal('report'), reportName: z.string().describe('Target report name'), -}, { error: navItemUnknownKeyError('report') }).strict()); +})); /** * 6. Action Navigation Item * Triggers an action (e.g. opening a flow, running a script, or launching a screen action). */ -export const ActionNavItemSchema = lazySchema(() => z.object({ +export const ActionNavItemSchema = lazySchema(() => strictObject(navItemSurface('action'), { ...BaseNavItemSchema.shape, type: z.literal('action'), - actionDef: z.object({ + actionDef: strictObject( + { + surface: "this nav item's action definition", + aliases: { action: 'actionName', name: 'actionName', args: 'params', input: 'params' }, + history: + 'Until #4001 these were dropped silently — the definition still parsed, so clicking ' + + 'the entry dispatched a different action than the author declared.', + }, + { actionName: z.string().describe('Action machine name to execute'), // OPEN by design: the action owns its own param contract. params: z.record(z.string(), z.unknown()).optional().describe('Parameters passed to the action'), - }, { error: actionDefUnknownKeyError }).strict().describe('Action definition to execute when clicked'), -}, { error: navItemUnknownKeyError('action') }).strict()); + }).describe('Action definition to execute when clicked'), +})); /** * 7. Component Navigation Item @@ -446,25 +532,25 @@ export const ActionNavItemSchema = lazySchema(() => z.object({ * componentRef: 'metadata:resource', params: { type: 'object' } } * ``` */ -export const ComponentNavItemSchema = lazySchema(() => z.object({ +export const ComponentNavItemSchema = lazySchema(() => strictObject(navItemSurface('component'), { ...BaseNavItemSchema.shape, type: z.literal('component'), componentRef: z.string().describe('Component registry key (e.g. "metadata:directory")'), // OPEN by design: props are the component's own contract. params: z.record(z.string(), z.unknown()).optional().describe('Props passed to the component'), -}, { error: navItemUnknownKeyError('component') }).strict()); +})); /** * 8. Group Navigation Item * A container for child navigation items (Sub-menu). * Does not perform navigation itself. */ -export const GroupNavItemSchema = lazySchema(() => z.object({ +export const GroupNavItemSchema = lazySchema(() => strictObject(navItemSurface('group'), { ...BaseNavItemSchema.shape, type: z.literal('group'), expanded: z.boolean().default(false).describe('Default expansion state in sidebar'), // children property is added in the recursive definition below -}, { error: navItemUnknownKeyError('group') }).strict()); +})); /** * 9. Separator Navigation Item @@ -472,11 +558,11 @@ export const GroupNavItemSchema = lazySchema(() => z.object({ * match the objectui renderer's `item.type === 'separator'` branch * (inverse-drift fix, liveness audit #1878/#1891/#1894). */ -const SeparatorNavItemSchema = lazySchema(() => z.object({ +const SeparatorNavItemSchema = lazySchema(() => strictObject(navItemSurface('separator'), { type: z.literal('separator'), id: SnakeCaseIdentifierSchema.optional().describe('Optional id for the separator'), order: z.number().optional().describe('Sort order within the same level (lower = first)'), -}, { error: navItemUnknownKeyError('separator') }).strict()); +})); /** Separator branch — internal, mirrors {@link SeparatorNavItemSchema}. */ type SeparatorNavItem = z.infer; @@ -620,21 +706,20 @@ export const NavigationItemSchema: z.ZodType z.object({ - app: SnakeCaseIdentifierSchema.describe('Target app name to contribute navigation into (e.g. "setup")'), - group: SnakeCaseIdentifierSchema.optional().describe('Target group nav-item id to append into (e.g. "group_integrations"); omit to append at the app top level'), - priority: z.number().int().min(0).default(200).describe('Merge priority within the target group — lower applied first (matches object extender priority)'), - items: z.array(NavigationItemSchema).describe('Navigation items contributed into the target app/group'), -}, { - error: strictUnknownKeyError({ +export const NavigationContributionSchema = lazySchema(() => strictObject( + { surface: 'this navigation contribution', - knownKeys: ['app', 'group', 'priority', 'items'], aliases: { targetapp: 'app', appname: 'app', targetgroup: 'group', groupid: 'group', order: 'priority', navigation: 'items' }, history: 'Until #4001 these were dropped silently — the contribution still parsed, so a ' + 'package injected its menu into the wrong place, or nowhere.', - }), -}).strict().describe('A navigation contribution: a package injecting nav items into an app it does not own (ADR-0029 D7)')); + }, + { + app: SnakeCaseIdentifierSchema.describe('Target app name to contribute navigation into (e.g. "setup")'), + group: SnakeCaseIdentifierSchema.optional().describe('Target group nav-item id to append into (e.g. "group_integrations"); omit to append at the app top level'), + priority: z.number().int().min(0).default(200).describe('Merge priority within the target group — lower applied first (matches object extender priority)'), + items: z.array(NavigationItemSchema).describe('Navigation items contributed into the target app/group'), +}).describe('A navigation contribution: a package injecting nav items into an app it does not own (ADR-0029 D7)')); /** * The authoring shape of a contribution (#4195) — `priority` is `.default(200)` * and each item is a {@link NavigationItemInput}, so this is what a package @@ -649,21 +734,20 @@ export type NavigationContributionParsed = z.infer z.object({ - primaryColor: z.string().optional().describe('Primary theme color hex code'), - accentColor: z.string().optional().describe('Accent color hex code (highlights, active states). Declared to match the objectui ConsoleLayout read of branding.accentColor (inverse-drift fix, liveness audit #1878/#1891/#1894).'), - logo: z.string().optional().describe('Custom logo URL for this app'), - favicon: z.string().optional().describe('Custom favicon URL for this app'), -}, { - error: strictUnknownKeyError({ +export const AppBrandingSchema = lazySchema(() => strictObject( + { surface: "this app's branding block", - knownKeys: ['primaryColor', 'accentColor', 'logo', 'favicon'], aliases: { primary: 'primaryColor', accent: 'accentColor', color: 'primaryColor', logourl: 'logo', icon: 'favicon', theme: 'primaryColor' }, history: 'Until #4001 these were dropped silently — branding still parsed, so a theme the ' + 'author set never reached the shell.', - }), -}).strict()); + }, + { + primaryColor: z.string().optional().describe('Primary theme color hex code'), + accentColor: z.string().optional().describe('Accent color hex code (highlights, active states). Declared to match the objectui ConsoleLayout read of branding.accentColor (inverse-drift fix, liveness audit #1878/#1891/#1894).'), + logo: z.string().optional().describe('Custom logo URL for this app'), + favicon: z.string().optional().describe('Custom favicon URL for this app'), +})); /** * `app.areas[].order`, retired in 17.0.0 (#4667, ADR-0049). @@ -780,7 +864,29 @@ const AREA_REQUIRED_PERMISSIONS_RETIRED = * }; * ``` */ -export const NavigationAreaSchema = lazySchema(() => z.object({ +export const NavigationAreaSchema = lazySchema(() => strictObject( + { + surface: 'this navigation area', + // `sort: 'order'` retired with the key it pointed at (#4667); the three + // gating aliases (`visibleWhen`/`visibleOn`/`permissions`) retired with + // theirs (#4651). An alias must never rename onto a key that is itself + // gone — it would answer "unknown key" with a second unknown key — so each + // moves to `guidance` and carries the prescription instead. + aliases: { title: 'label', name: 'id', items: 'navigation', children: 'navigation' }, + guidance: { + order: AREA_ORDER_RETIRED, + sort: AREA_ORDER_RETIRED, + visible: AREA_VISIBLE_RETIRED, + visibleWhen: AREA_VISIBLE_RETIRED, + visibleOn: AREA_VISIBLE_RETIRED, + requiredPermissions: AREA_REQUIRED_PERMISSIONS_RETIRED, + permissions: AREA_REQUIRED_PERMISSIONS_RETIRED, + }, + history: + 'Until #4001 these were dropped silently — the area still parsed, so its gating or ' + + 'ordering was quietly ignored.', + }, + { /** Unique area identifier */ id: SnakeCaseIdentifierSchema.describe('Unique area identifier (lowercase snake_case)'), @@ -804,30 +910,7 @@ export const NavigationAreaSchema = lazySchema(() => z.object({ /** Navigation items within this area */ navigation: z.array(NavigationItemSchema).describe('Navigation items within this area'), -}, { - error: strictUnknownKeyError({ - surface: 'this navigation area', - knownKeys: ['id', 'label', 'icon', 'description', 'navigation'], - // `sort: 'order'` retired with the key it pointed at (#4667); the three - // gating aliases (`visibleWhen`/`visibleOn`/`permissions`) retired with - // theirs (#4651). An alias must never rename onto a key that is itself - // gone — it would answer "unknown key" with a second unknown key — so each - // moves to `guidance` and carries the prescription instead. - aliases: { title: 'label', name: 'id', items: 'navigation', children: 'navigation' }, - guidance: { - order: AREA_ORDER_RETIRED, - sort: AREA_ORDER_RETIRED, - visible: AREA_VISIBLE_RETIRED, - visibleWhen: AREA_VISIBLE_RETIRED, - visibleOn: AREA_VISIBLE_RETIRED, - requiredPermissions: AREA_REQUIRED_PERMISSIONS_RETIRED, - permissions: AREA_REQUIRED_PERMISSIONS_RETIRED, - }, - history: - 'Until #4001 these were dropped silently — the area still parsed, so its gating or ' + - 'ordering was quietly ignored.', - }), -}).strict()); +})); /** * App Context Selector Schema @@ -903,7 +986,16 @@ const CONTEXT_SELECTOR_RETIRED_KEY_GUIDANCE: Readonly> = + 'Delete the key.', }; -export const AppContextSelectorSchema = lazySchema(() => z.object({ +export const AppContextSelectorSchema = lazySchema(() => strictObject( + { + surface: 'this app context selector', + aliases: { name: 'id', title: 'label', source: 'optionsSource', options: 'optionsSource' }, + guidance: CONTEXT_SELECTOR_RETIRED_KEY_GUIDANCE, + history: + 'Until #4001 these were dropped silently — the selector still parsed, so its scope ' + + 'variable behaved differently than declared.', + }, + { /** * Identifier — also the template-variable name the selected value is * exposed under. Reference it in nav items as `{}` @@ -923,7 +1015,15 @@ export const AppContextSelectorSchema = lazySchema(() => z.object({ * Re-uses existing REST surfaces (e.g. `/api/v1/packages`) so no * bespoke option API is required. */ - optionsSource: z.object({ + optionsSource: strictObject( + { + surface: "this context selector's options source", + aliases: { url: 'endpoint', path: 'endpoint', value: 'valueKey', label: 'labelKey', filters: 'filter', where: 'filter' }, + history: + 'Until #4001 these were dropped silently — the source still parsed, so the ' + + 'dropdown resolved its options from a different shape than declared.', + }, + { endpoint: z.string().describe('REST endpoint returning the option rows (e.g. /api/v1/packages)'), valueKey: z.string().default('id').describe('Row property used as the option value (dotted path allowed, e.g. "manifest.id")'), labelKey: z.string().default('name').describe('Row property used as the option label (dotted path allowed, e.g. "manifest.name")'), @@ -943,32 +1043,22 @@ export const AppContextSelectorSchema = lazySchema(() => z.object({ * filter: [{ key: 'manifest.scope', op: 'nin', value: ['system', 'cloud'] }] * ``` */ - filter: z.array(z.object({ - key: z.string().describe('Dotted path on each row to compare (e.g. "manifest.scope")'), - op: z.enum(['eq', 'ne', 'in', 'nin']).default('eq') - .describe('Comparison operator: eq | ne | in | nin'), - value: z.union([z.string(), z.array(z.string())]) - .describe('Comparison value (string for eq/ne, string[] for in/nin)'), - }, { - error: strictUnknownKeyError({ + filter: z.array(strictObject( + { surface: 'this context-selector option filter', - knownKeys: ['key', 'op', 'value'], aliases: { field: 'key', path: 'key', operator: 'op', values: 'value' }, history: 'Until #4001 these were dropped silently — the predicate still parsed, so the ' + 'option list was not narrowed the way the author declared.', - }), - }).strict()).optional().describe('Predicates (AND) each option row must satisfy'), - }, { - error: strictUnknownKeyError({ - surface: "this context selector's options source", - knownKeys: ['endpoint', 'valueKey', 'labelKey', 'filter'], - aliases: { url: 'endpoint', path: 'endpoint', value: 'valueKey', label: 'labelKey', filters: 'filter', where: 'filter' }, - history: - 'Until #4001 these were dropped silently — the source still parsed, so the ' + - 'dropdown resolved its options from a different shape than declared.', - }), - }).strict().describe('Option data source'), + }, + { + key: z.string().describe('Dotted path on each row to compare (e.g. "manifest.scope")'), + op: z.enum(['eq', 'ne', 'in', 'nin']).default('eq') + .describe('Comparison operator: eq | ne | in | nin'), + value: z.union([z.string(), z.array(z.string())]) + .describe('Comparison value (string for eq/ne, string[] for in/nin)'), + })).optional().describe('Predicates (AND) each option row must satisfy'), + }).describe('Option data source'), // `includeAll` and `placement` were removed in 17.0.0 (#4509) — see // CONTEXT_SELECTOR_RETIRED_KEY_GUIDANCE above. @@ -989,17 +1079,7 @@ export const AppContextSelectorSchema = lazySchema(() => z.object({ /** How the selection is persisted across navigation. */ persist: z.enum(['query', 'session', 'none']).default('query') .describe('Persist selection via URL query, sessionStorage, or not at all'), -}, { - error: strictUnknownKeyError({ - surface: 'this app context selector', - knownKeys: ['id', 'label', 'icon', 'optionsSource', 'allValue', 'persist'], - aliases: { name: 'id', title: 'label', source: 'optionsSource', options: 'optionsSource' }, - guidance: CONTEXT_SELECTOR_RETIRED_KEY_GUIDANCE, - history: - 'Until #4001 these were dropped silently — the selector still parsed, so its scope ' + - 'variable behaved differently than declared.', - }), -}).strict()); +})); export type AppContextSelector = z.input; /** Post-parse shape of {@link AppContextSelector} — defaults applied, transforms run (ADR-0122). */ @@ -1046,19 +1126,6 @@ export type AppContextSelectorParsed = z.infer; * ] * } */ -/** Keys {@link AppSchema} declares (drift-guarded by app.test.ts). */ -const APP_KEYS = [ - 'name', 'label', 'description', 'icon', 'branding', 'active', 'isDefault', - 'hidden', 'navigation', 'areas', 'contextSelectors', 'homePageId', - 'requiredPermissions', 'defaultAgent', 'protection', - // ADR-0010 runtime protection envelope (MetadataProtectionFields spread). - '_lock', '_lockReason', '_lockSource', '_provenance', '_packageId', - '_packageVersion', '_lockDocsUrl', - // Tombstoned in PR A (#4142) — declared so the prescription, not a bare - // "unrecognized key", is what an upgrading author sees. - 'version', 'aria', 'objects', 'apis', 'sharing', 'embed', 'mobileNavigation', -] as const; - /** * `app.homePageId`, retired in 17.0.0 (#4667, ADR-0049) — **premise corrected in * #4709**, retirement itself upheld. @@ -1097,52 +1164,51 @@ const HOME_PAGE_ID_RETIRED = + 'should own the root landing. Run `os migrate meta --from 16` to rewrite existing sources ' + 'automatically.'; -const appUnknownKeyError = strictUnknownKeyError({ - surface: 'this app', - knownKeys: APP_KEYS, - aliases: { - title: 'label', - nav: 'navigation', - menu: 'navigation', - menus: 'navigation', - items: 'navigation', - sidebar: 'navigation', - tabs: 'navigation', - sections: 'areas', - groups: 'areas', - permissions: 'requiredPermissions', - // `home` / `homepage` / `landingpage` aliased `homePageId`, retired in - // 17.0.0 (#4667). They fall through to the tombstone's own prescription - // rather than renaming onto a key that no longer exists. - agent: 'defaultAgent', - logo: 'branding', - theme: 'branding', - enabled: 'active', - default: 'isDefault', - selectors: 'contextSelectors', - }, - guidance: { - pages: - '`pages` is not an App field — a page is its own metadata record; reference it from ' + - "navigation with `{ type: 'page', pageName: '' }`.", - views: - '`views` is not an App field — views belong to their object (`listViews`); reference ' + - "one from navigation with `{ type: 'object', objectName, viewName }`.", - flows: - '`flows` is not an App field — flows are top-level stack metadata ' + - '(`defineStack({ flows })`), not app-scoped.', - // The three retired `homePageId` aliases. `retiredKey` already answers the - // canonical spelling; these cover the spellings that used to route to it. - home: HOME_PAGE_ID_RETIRED, - homepage: HOME_PAGE_ID_RETIRED, - landingpage: HOME_PAGE_ID_RETIRED, +export const AppSchema = lazySchema(() => strictObject( + { + surface: 'this app', + aliases: { + title: 'label', + nav: 'navigation', + menu: 'navigation', + menus: 'navigation', + items: 'navigation', + sidebar: 'navigation', + tabs: 'navigation', + sections: 'areas', + groups: 'areas', + permissions: 'requiredPermissions', + // `home` / `homepage` / `landingpage` aliased `homePageId`, retired in + // 17.0.0 (#4667). They fall through to the tombstone's own prescription + // rather than renaming onto a key that no longer exists. + agent: 'defaultAgent', + logo: 'branding', + theme: 'branding', + enabled: 'active', + default: 'isDefault', + selectors: 'contextSelectors', + }, + guidance: { + pages: + '`pages` is not an App field — a page is its own metadata record; reference it from ' + + "navigation with `{ type: 'page', pageName: '' }`.", + views: + '`views` is not an App field — views belong to their object (`listViews`); reference ' + + "one from navigation with `{ type: 'object', objectName, viewName }`.", + flows: + '`flows` is not an App field — flows are top-level stack metadata ' + + '(`defineStack({ flows })`), not app-scoped.', + // The three retired `homePageId` aliases. `retiredKey` already answers the + // canonical spelling; these cover the spellings that used to route to it. + home: HOME_PAGE_ID_RETIRED, + homepage: HOME_PAGE_ID_RETIRED, + landingpage: HOME_PAGE_ID_RETIRED, + }, + history: + 'Until #4001 these were dropped silently — the app still parsed, so navigation or ' + + 'gating the author declared never reached the shell.', }, - history: - 'Until #4001 these were dropped silently — the app still parsed, so navigation or ' + - 'gating the author declared never reached the shell.', -}); - -export const AppSchema = lazySchema(() => z.object({ + { /** Machine name (id) */ name: SnakeCaseIdentifierSchema.describe('App unique machine name (lowercase snake_case)'), @@ -1354,7 +1420,7 @@ export const AppSchema = lazySchema(() => z.object({ // ADR-0010 — runtime protection envelope (internal — set by loader). ...MetadataProtectionFields, -}, { error: appUnknownKeyError }).strict()); +})); /** * App Factory Helper diff --git a/packages/spec/src/ui/bulk-action.zod.ts b/packages/spec/src/ui/bulk-action.zod.ts index 4772f5aa2a..5440cde65e 100644 --- a/packages/spec/src/ui/bulk-action.zod.ts +++ b/packages/spec/src/ui/bulk-action.zod.ts @@ -2,7 +2,7 @@ import { z } from 'zod'; import { lazySchema } from '../shared/lazy-schema'; -import { strictUnknownKeyError } from '../shared/suggestions.zod'; +import { strictObject } from '../shared/strict-object'; import { ExpressionInputSchema } from '../shared/expression.zod'; import { SnakeCaseIdentifierSchema } from '../shared/identifiers.zod'; import { FieldType } from '../data/field.zod'; @@ -133,56 +133,6 @@ export const BulkActionParamSchema = lazySchema(() => z.object({ }).passthrough()); export type BulkActionParam = z.input; -/** Declared keys of a bulk-action def — the "did you mean" pool. */ -const BULK_ACTION_DEF_KEYS = [ - 'name', 'label', 'icon', 'variant', 'operation', 'execution', 'patch', - 'params', 'confirmText', 'confirmLabel', 'visible', 'requiredPermissions', - 'maxRecords', 'batchSize', -] as const; - -const bulkActionDefUnknownKeyError = strictUnknownKeyError({ - surface: 'this bulk action definition', - knownKeys: BULK_ACTION_DEF_KEYS, - aliases: { - action: 'name', - actionname: 'name', - title: 'label', - op: 'operation', - mode: 'execution', - confirm: 'confirmText', - confirmmessage: 'confirmText', - limit: 'maxRecords', - max: 'maxRecords', - batch: 'batchSize', - // The capability gate IS a declared key here too — `requiredPermissions` - // (ADR-0066 D4, #6257) — so its near-misses RENAME onto it, exactly as - // they do on `ActionSchema`. - permissions: 'requiredPermissions', capabilities: 'requiredPermissions', - requiresPermissions: 'requiredPermissions', requiredCapabilities: 'requiredPermissions', - acl: 'requiredPermissions', - }, - guidance: { - // Not a typo — a real key the RENDERER attaches, which is exactly why an - // author reaching for it needs more than "did you mean". - actionDef: - '`actionDef` is attached by the renderer, not authored: `resolveBulkActions` looks the ' - + 'action up by `name` and inlines it. Writing it by hand smuggles an action definition ' - + 'past the action registry — no permission gate, no param contract, no lint. Declare the ' - + 'action normally and let this def name it.', - bulkEnabled: - '`action.bulkEnabled` was retired in spec 17: the selection bar is driven by the LIST ' - + "VIEW's `bulkActions` / `bulkActionDefs`, which is this array. There is nothing to set.", - recordIdParam: - '`recordIdParam` belongs on the ACTION, not on the def that names it — a per-record bulk ' - + "run reuses the action's own declaration, and an `execution: 'aggregate'` run carries " - + 'the whole selection in `params._selectedIds` instead of a single record id.', - }, - history: - 'Until #4457 the whole array was `z.array(z.record(z.string(), z.any()))` — every key parsed, ' - + 'so a mis-spelled one shipped as a button that silently ran the DEFAULT behaviour (or none ' - + 'at all).', -}); - /** * Rich, schema-driven definition of one button in the multi-select bar. * @@ -203,7 +153,49 @@ const bulkActionDefUnknownKeyError = strictUnknownKeyError({ * modes invisible from the authoring side, which is why they are caught here * rather than written down and hoped for. */ -export const BulkActionDefSchema = lazySchema(() => z.object({ +export const BulkActionDefSchema = lazySchema(() => strictObject( + { + surface: 'this bulk action definition', + aliases: { + action: 'name', + actionname: 'name', + title: 'label', + op: 'operation', + mode: 'execution', + confirm: 'confirmText', + confirmmessage: 'confirmText', + limit: 'maxRecords', + max: 'maxRecords', + batch: 'batchSize', + // The capability gate IS a declared key here too — `requiredPermissions` + // (ADR-0066 D4, #6257) — so its near-misses RENAME onto it, exactly as + // they do on `ActionSchema`. + permissions: 'requiredPermissions', capabilities: 'requiredPermissions', + requiresPermissions: 'requiredPermissions', requiredCapabilities: 'requiredPermissions', + acl: 'requiredPermissions', + }, + guidance: { + // Not a typo — a real key the RENDERER attaches, which is exactly why an + // author reaching for it needs more than "did you mean". + actionDef: + '`actionDef` is attached by the renderer, not authored: `resolveBulkActions` looks the ' + + 'action up by `name` and inlines it. Writing it by hand smuggles an action definition ' + + 'past the action registry — no permission gate, no param contract, no lint. Declare the ' + + 'action normally and let this def name it.', + bulkEnabled: + '`action.bulkEnabled` was retired in spec 17: the selection bar is driven by the LIST ' + + "VIEW's `bulkActions` / `bulkActionDefs`, which is this array. There is nothing to set.", + recordIdParam: + '`recordIdParam` belongs on the ACTION, not on the def that names it — a per-record bulk ' + + "run reuses the action's own declaration, and an `execution: 'aggregate'` run carries " + + 'the whole selection in `params._selectedIds` instead of a single record id.', + }, + history: + 'Until #4457 the whole array was `z.array(z.record(z.string(), z.any()))` — every key parsed, ' + + 'so a mis-spelled one shipped as a button that silently ran the DEFAULT behaviour (or none ' + + 'at all).', + }, + { name: SnakeCaseIdentifierSchema.describe('Stable identifier — the audit-log action key, and (for an aggregate def) the name of the object action to dispatch.'), label: z.string().optional().describe('Button + dialog-header text. Plain string: an authored def is not i18n-resolved (declare a real action and name it in `bulkActions` to get localization).'), icon: z.string().optional().describe('Lucide icon name (e.g. "user-check", "trash-2").'), @@ -218,7 +210,7 @@ export const BulkActionDefSchema = lazySchema(() => z.object({ requiredPermissions: z.array(z.string()).optional().describe("[ADR-0066 D4] Capability gate on the button, `action.requiredPermissions` semantics verbatim: absent or empty always passes, several are AND-ed, and a client that cannot resolve the caller's capabilities fails OPEN (the server stays the authority). This key exists for INLINE defs — notably the `update`/`delete` data-plane forms, which dispatch no action and so have nothing to inherit a gate from; a def promoted from `bulkActions: ['']` (or an aggregate def naming a declared action) inherits the action's own declaration instead. On a data-plane def the gate governs visibility only — the write itself is still authorized by the data API's object permissions and server hooks."), maxRecords: z.number().int().positive().optional().describe('Selection size above which the run is blocked. Set it on defs whose server work is expensive — an aggregate def carries every selected id in one request.'), batchSize: z.number().int().positive().optional().describe('Records per executor batch (default 200). Data-plane operations only — an aggregate run is a single call by definition.'), -}, { error: bulkActionDefUnknownKeyError }).strict() +}) .superRefine((def, ctx) => { // ── `custom` without `aggregate` is the historical no-op ────────────── // `useBulkExecutor`'s custom branch dispatches only when the def carries a diff --git a/packages/spec/src/ui/chart.test.ts b/packages/spec/src/ui/chart.test.ts index dce0a8cbf9..77f62e9646 100644 --- a/packages/spec/src/ui/chart.test.ts +++ b/packages/spec/src/ui/chart.test.ts @@ -279,8 +279,16 @@ describe('Chart ARIA Integration', () => { // UPDATE (#5020): the open half's verdict moved from `no gate` to `authorable` // — the react-page publish lint now PARSES `ChartAggregateSchema` instead of // re-deriving it, so a `strictObject` here would no longer gate nothing. The -// posture itself is unchanged, so every assertion below stands as written; the -// conversion is #5583, and that is the change that inverts the two STRIP pins. +// posture itself was unchanged by that step, which is the whole reason it was +// its own step. +// +// UPDATE (#5583): the posture moved too, and the file is now 0 strip. The split +// is history rather than a live classification — the `chart.zod.ts` row left the +// ledger's remaining-strip map on the reverse pin (a row that outlives its work +// fails), and the two "still STRIPS" pins below were INVERTED in place. What the +// pair still guards is the ORDER: parse first, posture second. A sweep that +// meets a `no gate` verdict elsewhere and closes it in passing is the failure +// this file was written to make visible (#4583). // ============================================================================ describe('#4001 批 15 — the five closed chart sites', () => { const reject = (schema: { safeParse: (v: unknown) => { success: boolean; error?: { issues: unknown } } }, value: unknown): string => { @@ -422,33 +430,72 @@ describe('#4001 批 15 — the five closed chart sites', () => { }); }); -describe('#4001 批 15 — the two chart sites deliberately LEFT OPEN (measured, not skipped)', () => { +describe('#4001 批 15 — the two chart sites left open on a measurement, CLOSED at #5583', () => { // `ChartAggregateSchema` and `ChartGroupBySchema`'s object arm have a LIVE // carrier — the react tier's `` prop, which // objectui's ObjectChart reads to run the query — and, as of **#5020**, a // PARSE: the react-page publish lint calls `ChartAggregateSchema.safeParse()` // instead of re-deriving the vocabulary and the count/field refinement by - // hand. That retires the 批 15 `no gate` verdict; both sites are now ordinary - // `authorable` ones. + // hand. That retired the 批 15 `no gate` verdict and made both sites ordinary + // `authorable` ones; **#5583** then moved the posture, which is what the two + // pins below now record. // - // The two pins below are therefore UNCHANGED and must stay GREEN: the posture - // did not move, only the parse did. `.strict()` is a property of a parse, and - // now that one exists, converting these two is a behaviour change with a gate - // to observe it — **#5583**, where these two assertions INVERT (the stripped - // key becomes a named rejection). Until then they record what the wired gate - // still cannot see, which is the difference between a gate and a closed door - // (#4583). The companion pins live in `packages/lint`'s - // `validate-react-page-props.test.ts`. - it('ChartAggregateSchema still STRIPS an undeclared key — deliberate', () => { - const parsed = ChartAggregateSchema.parse({ function: 'count', groupBy: 'status', groupby: 'status' }) as Record; - expect(parsed.groupby, 'if this is no longer stripped, re-read the header in chart.zod.ts').toBeUndefined(); - expect(parsed.groupBy).toBe('status'); - }); - - it('ChartGroupBySchema\'s object arm still STRIPS an undeclared key — deliberate', () => { - const parsed = ChartGroupBySchema.parse({ field: 'created_at', dateGranularty: 'month' }) as Record; - expect(parsed.dateGranularty).toBeUndefined(); - expect(parsed.field).toBe('created_at'); + // ⚠️ They are the SAME two assertions 批 15 wrote, INVERTED — deliberately + // rewritten rather than deleted, because the pair is what makes the two-step + // order legible: the key that used to come back stripped now comes back as a + // named rejection, from the same input, at the same site. A reader who lands + // here from a future sweep should be able to see both states. The companion + // pins live in `packages/lint`'s `validate-react-page-props.test.ts`, which + // inverted in the same PR. + it('ChartAggregateSchema REJECTS an undeclared key, by name (#5583 — was a silent strip)', () => { + const r = ChartAggregateSchema.safeParse({ function: 'count', groupBy: 'status', groupby: 'status' }); + expect(r.success, 'if this parses again the strictness was reverted — re-read the header in chart.zod.ts').toBe(false); + const issue = r.error!.issues[0]; + expect(issue.code).toBe('unrecognized_keys'); + // The three things a named rejection owes an author: the surface, the + // offending key echoed back, and the rename. `groupby` → `groupBy` comes + // from the FOLDED edit distance, not from an alias entry — asserted here so + // a later "curation" that adds the redundant alias has a reason not to. + expect(issue.message).toContain('this chart aggregate'); + expect(issue.message).toContain('`groupby`'); + expect(issue.message).toContain('`groupby` → `groupBy`'); + + // The curated half, on the key this file's own header named as the + // expensive one: written BESIDE `groupBy`, `dateGranularity` did nothing. + const wrongLayer = ChartAggregateSchema.safeParse({ function: 'count', groupBy: 'created_at', dateGranularity: 'month' }); + expect(wrongLayer.success).toBe(false); + expect(wrongLayer.error!.issues[0].message).toContain('goes INSIDE `groupBy`'); + + // Control, in the SAME run: a declaration that was legal before is legal + // now. A strictness pin that only shows rejections is satisfiable by a + // schema that rejects everything. + expect(ChartAggregateSchema.safeParse({ function: 'count', groupBy: 'status' }).success).toBe(true); + }); + + it("ChartGroupBySchema's object arm REJECTS an undeclared key — and the UNION collapses its message (#5583)", () => { + const r = ChartGroupBySchema.safeParse({ field: 'created_at', dateGranularty: 'month' }); + expect(r.success).toBe(false); + + // ⚠️ The zod-4 union collapse, pinned as a RAW SHAPE rather than described. + // `groupBy` is a union, so the arm's `unrecognized_keys` never reaches + // `error.issues`: what surfaces is ONE `invalid_union` whose own message is + // the bare string "Invalid input". A consumer that renders `issue.message` + // verbatim shows the author nothing at all (#5014), which is why + // `packages/lint/src/zod-issue-format.ts` unpacks `issue.errors` — and why + // that unpacking had to exist BEFORE this schema was closed. + const top = r.error!.issues[0] as { code: string; message: string; errors?: unknown[][] }; + expect(top.code, 'the strict arm does NOT surface as unrecognized_keys').toBe('invalid_union'); + expect(top.message, 'the collapsed message carries nothing an author can act on').toBe('Invalid input'); + + // The named rejection is reachable, one level in — this is exactly what the + // lint side reads, so if this shape ever changes the unpacking breaks with it. + const armMessages = (top.errors ?? []).flat().map((i) => (i as { message: string }).message); + expect(armMessages.some((m) => m.includes('this chart groupBy'))).toBe(true); + expect(armMessages.some((m) => m.includes('`dateGranularty` → `dateGranularity`'))).toBe(true); + + // Controls in the same run: both accepted forms still parse. + expect(ChartGroupBySchema.safeParse({ field: 'created_at', dateGranularity: 'month', alias: 'month' }).success).toBe(true); + expect(ChartGroupBySchema.safeParse('status').success).toBe(true); }); it('neither is REACHABLE from the metadata-type roots — the measurement, re-run every CI', () => { diff --git a/packages/spec/src/ui/chart.zod.ts b/packages/spec/src/ui/chart.zod.ts index 50ec8c2c84..cc2dc5043b 100644 --- a/packages/spec/src/ui/chart.zod.ts +++ b/packages/spec/src/ui/chart.zod.ts @@ -5,10 +5,12 @@ import { I18nLabelSchema, AriaPropsSchema } from './i18n.zod'; import { strictObject } from '../shared/strict-object'; // --------------------------------------------------------------------------- -// UNKNOWN-KEY POSTURE (#4001 批 15, ADR-0078) — this file is SPLIT, on a -// measurement, and the split is the point. Five of its seven object sites are -// closed; two are deliberately left open with the reason recorded, because -// closing them would gate nothing. +// UNKNOWN-KEY POSTURE (#4001 批 15 → #5583, ADR-0078) — this file is CLOSED, +// and the ORDER it was closed in is the point. 批 15 shut five of its object +// sites and deliberately left two open with the reason recorded, because +// closing them would have gated nothing; #5020 supplied the missing parse and +// #5583 then shut them. Both halves are kept below, because a later sweep +// meeting a `no gate` verdict elsewhere needs the refusal, not just the result. // // CLOSED (real door, three measurements, 2026-08-03): // `ChartConfigSchema`, `ChartAxisSchema`, `ChartSeriesSchema`, @@ -37,20 +39,37 @@ import { strictObject } from '../shared/strict-object'; // dimension/measure NAMES and adds no key of its own, so the inherited key // set is exactly right and no `extraKeys` entry is needed. // -// STILL OPEN (`ChartAggregateSchema`, `ChartGroupBySchema`) — but no longer for -// the reason 批 15 recorded. Their carrier is the REACT tier's -// `` prop, which had NO parse behind it; #5020 wired -// one (`packages/lint`'s react-page publish gate now calls +// CLOSED LAST, at #5583 (`ChartAggregateSchema`, `ChartGroupBySchema`'s object +// arm) — and the ORDER is the record worth keeping. Their carrier is the REACT +// tier's `` prop, which had no parse behind it, so +// 批 15 left them open rather than shipping a `.strict()` over nothing (#4583). +// #5020 wired the parse (`packages/lint`'s react-page publish gate now calls // `ChartAggregateSchema.safeParse()` instead of re-deriving the vocabulary and -// the count/field refinement by hand), so the `no gate` verdict is spent and -// these two are ordinary `authorable` sites. What remains is the posture: both -// are still STRIP, so an unknown key is dropped by that parse rather than -// reported, and `groupby` / `dateGranularty` still degrade a chart silently. -// Closing them is now a behaviour change with a gate to observe it — **#5583**, -// which also carries the one product question this pair raises (is an ungrouped -// single-value chart a supported shape? the renderer honours it, `groupBy` is -// declared required, and #5020's gate reports the absence at `warning` until -// that is answered). +// the count/field refinement by hand), which spent the `no gate` verdict and +// made these two ordinary `authorable` sites; #5583 then moved the POSTURE. +// `groupby` / `fn` / `dateGranularty` are now named rejections carrying a +// surface and a rename, and the file's ui/ row left the ledger's +// remaining-strip map. This file is 0 strip. +// +// ⚠️ The product question #5583 carried is ANSWERED, and the answer is the one +// that does NOT move this schema: **an ungrouped single-value chart is not a +// supported `` shape**, so `groupBy` stays REQUIRED. Measured +// rather than argued (2026-08-08): the example corpus authors exactly one +// `` and it carries `groupBy`; the single-value need +// is served by a DIFFERENT registered block, objectui's `object-metric` +// (`ObjectMetricWidget`), which the showcase authors seven times with +// `aggregate: { field, function }` and no `groupBy` at all. The three +// `schema.aggregate?.groupBy || schema.xAxisKey` reads in objectui's +// `ObjectChart.tsx` are optional-chained on `aggregate` itself, so what they +// serve is a chart with NO aggregate (a `data=` / `dataset=` binding) — they +// keep option-colour resolution, the comparison merge and the drill-down filter +// working there, and none of them makes an ungrouped aggregate draw. The one +// path that does aggregate client-side declares `groupBy: string` REQUIRED and, +// given `undefined`, buckets every record under `String(undefined)`. Declaring +// the shape optional would advertise what the renderer does not deliver +// (Prime Directive #10) and would make "forgot the category axis" a legal +// declaration. #5020's `warning`-level tolerance therefore stays a tolerance, +// not a blessing — see `validate-react-page-props.ts`. // --------------------------------------------------------------------------- /** @@ -626,11 +645,12 @@ export const ChartConfigSchema = lazySchema(() => strictObject( */ // --------------------------------------------------------------------------- -// THE TWO SITES BELOW ARE STILL OPEN — and as of #5020 the reason has CHANGED. -// Read this header as two layers: what 批 15 measured (still accurate as -// history), and what moved since (the parse exists now; the posture does not). +// THE TWO SITES BELOW WERE THE FILE'S LAST OPEN ONES, and they were closed in +// TWO STEPS on purpose — parse first (#5020), posture second (#5583). Read this +// header as three layers: what 批 15 measured (still accurate as history), what +// #5020 moved (the parse), and what #5583 moved (the posture). // -// ## What moved (#5020) +// ## What #5020 moved: the PARSE // // Point 3 below said "nothing parses these". That is no longer true. The // react-page publish gate — `packages/lint/src/validate-react-page-props.ts` — @@ -638,29 +658,36 @@ export const ChartConfigSchema = lazySchema(() => strictObject( // `aggregate={{…}}` literal, exactly as #5022 did for `ChartDrillDownSchema` // beside it, and the hand-derived `CHART_FUNCTIONS` list plus the hand-written // twin of the count/field refinement are DELETED: this file is the single source -// of both again. So the ledger's `no gate` verdict is spent, and these two rows -// are now ordinary `authorable` sites. +// of both again. So the ledger's `no gate` verdict was spent, and these two +// sites became ordinary `authorable` ones. +// +// ## What #5583 moved: the POSTURE // -// ## What did NOT move, and why it is a separate issue +// Both are now `strictObject`. An undeclared key is a named rejection carrying +// the surface, the offending key and a rename — `groupby` → `groupBy`, +// `dateGranularty` → `dateGranularity`, `fn` → `function` — instead of a silent +// strip that left a chart drawing one ungrouped point with `build`/`validate` +// green. The two "still STRIPS — deliberate" pins in `chart.test.ts` and the +// companion tolerance pin in `validate-react-page-props.test.ts` INVERTED with +// it; they are the same assertions, read from the other side. // -// Both are still STRIP-posture, so the parse the gate now runs still DROPS an -// unknown key instead of reporting it — `groupby` for `groupBy`, -// `dateGranularty` for `dateGranularity` — and a chart still degrades to a -// single ungrouped point with `build`/`validate` green. Converting them to -// `strictObject` is now a real behaviour change with a gate that observes it, -// which is the whole point of doing it in this order, and it is **#5583**. -// `validate-react-page-props.test.ts` pins today's tolerance out loud so the -// wired gate cannot be mistaken for a closed one (#4583); those pins invert -// when #5583 lands, as do the two "still STRIPS — deliberate" pins in -// `chart.test.ts`. +// ⚠️ **The zod-4 union collapse is why the lint side had to be built first, and +// it is load-bearing here.** `groupBy` is a UNION, so an `unrecognized_keys` +// raised inside its object arm never reaches `error.issues` on its own — the +// whole union is reported as one `invalid_union` whose message is the bare +// string "Invalid input", with the arm messages tucked inside `issue.errors`. +// A consumer that renders `issue.message` verbatim shows the author nothing. +// `packages/lint/src/zod-issue-format.ts`'s `describeIssue` unpacks the arms, +// which is what carries this schema's named rejection to the author, and +// `chart.test.ts` pins the raw arm shape so the two halves cannot drift apart. // -// ⚠️ #5583 also carries the one product question this pair raises, which is NOT -// a strictness question: `groupBy` is declared REQUIRED here and in the -// published react-blocks type, while objectui's `ObjectChart` honours its -// absence (`schema.aggregate?.groupBy || schema.xAxisKey`) and -// `chartAggregateCategoryKey` in `./chart-aggregate.ts` documents the ungrouped -// single-row result. Until that is answered, #5020's gate reports the absence at -// `warning` rather than gating a shape the platform itself delivers. +// ⚠️ The product question this pair raised is ANSWERED (2026-08-08) and the +// answer left this schema alone: `groupBy` stays REQUIRED. The measurement is +// in the file header at the top — the ungrouped single-value need is served by +// objectui's separate `object-metric` block, the corpus authors zero ungrouped +// `` aggregates, and the renderer's `|| schema.xAxisKey` reads +// serve charts with no aggregate at all rather than ungrouped ones. #5020's +// `warning`-level tolerance therefore stays a tolerance. // // ## What 批 15 measured (the history, unchanged) // @@ -704,11 +731,11 @@ export const ChartConfigSchema = lazySchema(() => strictObject( // // The batch's standing instruction — "do not convert these two to // `strictObject` before the parse exists, it would read as load-bearing while -// gating nothing" — is therefore SATISFIED, not repealed. The conversion is now -// the right next step and has its own issue (#5583). Anyone reaching this -// paragraph from a strictness sweep should go there rather than closing these -// two in passing: the sweep would also have to invert four pins and answer the -// `groupBy` product question above. +// gating nothing" — was SATISFIED rather than repealed, and #5583 then did the +// conversion in that order. The instruction is kept here because it is the +// reusable part: it is the reason this file's last two sites took two issues +// and eight days instead of one commit, and it is what any later sweep meeting +// a `no gate` verdict should do. // --------------------------------------------------------------------------- /** @@ -739,17 +766,59 @@ export const ChartAggregateFunctionSchema = lazySchema(() => export const ChartGroupBySchema = lazySchema(() => z.union([ z.string().describe('Field to group by'), - z.object({ - field: z.string().describe('Field to group by'), - dateGranularity: z - .enum(['day', 'week', 'month', 'quarter', 'year']) - .optional() - .describe('Bucket date values into uniform periods'), - alias: z - .string() - .optional() - .describe('Alias for the projected group value (defaults to `field`) — this becomes the category column'), - }), + strictObject( + { + surface: 'this chart groupBy', + history: + 'Until #5583 an undeclared key inside the structured groupBy was dropped at parse — `dateGranularty` for `dateGranularity` cost the date bucketing silently, and the chart drew one point per raw timestamp.', + // The near-misses edit distance cannot reach, each anchored to a + // neighbouring vocabulary this protocol really uses: + // `granularity` — `dateGranularity` is the only spelling in the + // protocol, but the WORD an author reaches for is the bare noun + // (`data/query.zod.ts`'s own `DateGranularity` type is named that + // way, so the file the shape is mirrored from teaches it); + // `name` — `DatasetDimension.name` is the ADR-0021 dataset path's + // spelling of exactly this slot, so an author moving between the + // two chart binding modes writes it; + // `as` / `label` — SQL's rename keyword and the display word. `alias` + // is neither, and getting it wrong is expensive: the category + // column keeps the raw field name and every axis binding written + // against the alias resolves to nothing. + // + // ⚠️ `dateGranularty` deliberately has NO entry: the folded edit + // distance already reaches it (1 against a budget of 5), and a second + // spelling of a probe the fallback covers is a dead entry (#5481). + aliases: { + granularity: 'dateGranularity', + bucket: 'dateGranularity', + name: 'field', + as: 'alias', + label: 'alias', + }, + // Wrong-LAYER keys: real protocol words, one level out. A rename would + // be ledger finding 7 (steering the author at a key this shape also + // refuses), so they get a prescription instead. Both are keys of the + // ENCLOSING aggregate, which is the only shape an author can confuse + // this one with. + guidance: { + function: + '`function` belongs on the aggregate, not inside `groupBy` — write `aggregate: { function, field, groupBy: { field, dateGranularity } }`.', + groupBy: + 'You are already inside `groupBy` — the structured form is `{ field, dateGranularity?, alias? }`, not a nested `groupBy`.', + }, + }, + { + field: z.string().describe('Field to group by'), + dateGranularity: z + .enum(['day', 'week', 'month', 'quarter', 'year']) + .optional() + .describe('Bucket date values into uniform periods'), + alias: z + .string() + .optional() + .describe('Alias for the projected group value (defaults to `field`) — this becomes the category column'), + }, + ), ]), ); @@ -762,15 +831,57 @@ export const ChartGroupBySchema = lazySchema(() => * renderer as `sum(undefined)` and render blank). */ export const ChartAggregateSchema = lazySchema(() => - z - .object({ + strictObject( + { + surface: 'this chart aggregate', + history: + 'Until #5583 an undeclared aggregate key was dropped at parse — `groupby` for `groupBy` degraded the chart to a single ungrouped point, and `fn` for `function` fell back to the default, both with `build`/`validate` fully green.', + // The near-misses edit distance cannot reach (the folded fallback already + // covers `groupby`, `Group_By` and `functoin`, so none of those appears + // here — a second spelling of a covered probe is a dead entry, #5481): + // `fn` / `agg` / `aggregation` — this file's own header names `fn` as + // one of the keys that was being dropped silently, and `aggregations` + // is the word `IDataEngine.aggregate()` uses for the same slot; + // `measure` / `dimension` — the ADR-0021 DATASET path's names for the + // value and the category (`DatasetMeasure`, `DatasetDimension`). The + // two binding modes sit on one component, so an author who has just + // written a dataset chart writes them here; + // `category` — what this protocol itself calls the result column + // (`chartAggregateCategoryKey`), so the file teaches the word. + aliases: { + fn: 'function', + agg: 'function', + aggregation: 'function', + measure: 'field', + dimension: 'groupBy', + category: 'groupBy', + }, + // Wrong-LAYER and retired-shape prescriptions. `dateGranularity` is the + // expensive one and is named in this file's header as a key that was + // being dropped: written BESIDE `groupBy` it does nothing, and the chart + // draws one point per raw timestamp. + guidance: { + dateGranularity: + '`dateGranularity` goes INSIDE `groupBy`, not beside it — write `groupBy: { field: "", dateGranularity: "month" }`.', + alias: + '`alias` renames the GROUP column and lives inside the structured `groupBy` node. The measure column is named after `field` (or the literal `count` for a fieldless count) and is not renameable here — see `chartAggregateResultKeys` in `./chart-aggregate.ts`.', + filter: + '`filter` is a prop on the chart itself (``), not part of the aggregate.', + objectName: + '`objectName` is the chart\'s own prop — the aggregate runs against it and does not name it again.', + measures: + 'An inline aggregate is SINGLE-MEASURE by design (one `function` over one `field`); two measures would collide on the result column name. Multi-measure is the dataset path\'s job — bind `dataset` + `values` instead (ADR-0021 Level B, see `./chart-aggregate.ts`).', + }, + }, + { field: z .string() .optional() .describe('Field to aggregate — required for sum/avg/min/max, optional for count'), function: ChartAggregateFunctionSchema.describe('Aggregation function'), groupBy: ChartGroupBySchema.describe('Field the rows are grouped by — the chart category axis'), - }) + }, + ) .superRefine((agg, ctx) => { if (agg.function !== 'count' && !agg.field) { ctx.addIssue({