diff --git a/.changeset/strictness-ledger-remeasure.md b/.changeset/strictness-ledger-remeasure.md new file mode 100644 index 0000000000..406fd11fb1 --- /dev/null +++ b/.changeset/strictness-ledger-remeasure.md @@ -0,0 +1,28 @@ +--- +'@objectstack/spec': patch +--- + +Re-measure the #4001 unknown-key strictness ledger, and fix the instrument that was measuring it. + +No schema changed posture in this change — it is the measurement step the +2026-08-03 ruling asked for before the remaining batches are cut. + +**The site counter now reads the AST instead of matching source text**, because +the textual method was wrong in both directions at once. It counted +`z.object({ … })` written inside JSDoc prose (`ui/action.zod.ts` declared 9 sites +and has 8), and it missed both the prettier-wrapped `z\n .object({` form +(`ui/chart.zod.ts`, 6 → 7) and `z.looseObject(` (`data/field-value.zod.ts`, +1 → 2). On `ui/` the two errors cancelled exactly, so a correct section total sat +over two wrong rows. + +The worst case was `automation/time-relative-trigger.zod.ts`: its only site is +written wrapped, so it counted **zero** — and a zero-site file is deliberately +skipped by the coverage walk, so an authorable schema stayed outside the ledger +while the gate printed "no undeclared schema files". It is now classified. + +**`check:strictness-ledger` gained a remaining-strip-site map** — per file, how +many object sites still silently discard unknown keys, which is the number batch +plans are actually scheduled against and which nothing measured before. It is +gated in both directions: a file with strip sites must have a row, and a row +whose file reaches zero strip sites fails, so a closed file drops out of the +worklist rather than outliving it. diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index a7e61cbeb4..0151f81526 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -387,6 +387,49 @@ dropped at parse, and nothing failed. closing a registered type is every package that parses that type**, not the package that declares it. +19. **The site counter itself was wrong in both directions, and the two errors + cancelled.** Found by the 2026-08-03 re-measurement, by building a second, + independent counter and making the two disagree — seven files, in the gate + this campaign wrote specifically so its map could not go stale. + + It **counted prose**: a `z.object({ … })` inside a JSDoc example is not a + site, and `ui/action.zod.ts` declared 9 where 8 exist + (`kernel/metadata-protection.zod.ts` and `shared/suggestions.zod.ts` were + entirely comment). It **missed the wrapped call**: prettier writes a long + chain as `z\n .object({`, which `z\.object\(` cannot match — one site in + `ui/chart.zod.ts`, two in `kernel/manifest.zod.ts`. And it **did not know + `z.looseObject(`**, so `data/field-value.zod.ts` declared 1 of its 2. + + On `ui/` the miscounts were −1 and +1, so the section total balanced + perfectly over two wrong rows — the gate's arithmetic check passing *because + of* a second error. + + The consequential one was `automation/time-relative-trigger.zod.ts`. Its only + site is written wrapped, so it counted **zero**, and a zero-site file is + deliberately SKIPPED by the coverage walk ("nothing to classify"). An + authorable schema — a declarative trigger authors write by hand into a flow + start node — sat outside the map while the gate printed *"no undeclared + schema files"*. Note this is not the `data/driver/` failure repeated: that + walk was blind, whereas this walk was fine and the **counter feeding it** + returned a zero the walk then correctly honoured. A blind spot one layer + further in, reached through a correct code path. + + Fifth instance of the pattern, and the reason the fix is structural rather + than another spelling bolted onto a regex: the counter now reads the AST, so + it cannot be fooled by formatting or by comments, and it stops needing to + learn each new idiom separately — the same move the envelope probe made in + finding 9 when it stopped needing a valid instance and started walking the + schema. + + The same walk yields **posture** per site, which closes the gap that made + this re-measurement necessary at all: the ledger could say a file contained + `.strict()` *somewhere*, never how many of its sites were still open. The + 2026-08-03 ruling was consequently scheduled against counts of `strictObject(` + occurrences — an idiom that misses every schema closed with the older + `z.object(…).strict()` spelling, reading `automation/` as **0 strict** when it + has 8, and `ui/` as 49 when it has 72. Both the map and the gate now carry the + open-site count directly (see the remaining-strip map below). + ## Where this ended up **24 of 25 registered types closed** (from 9 when the line started), and the @@ -431,16 +474,19 @@ block) when `position` joined the ratchet. ## File-level triage — the five authorable directories -Site counts are object sites — `z.object(` or `strictObject(` — per file (2026-07-30, this branch). -Classification is per the rule above; **(p)** marks a provisional call made -from the file's exports/JSDoc rather than a full read — verify before -tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). +Site counts are object sites — every `z.object(` / `strictObject(` / +`z.strictObject(` / `z.looseObject(` CALL, read from the AST rather than matched +textually (see `scripts/lib/strictness-ledger.ts` for why the textual method was +wrong in both directions at once). Classification is per the rule above; **(p)** +marks a provisional call made from the file's exports/JSDoc rather than a full +read — verify before tightening (the #4001 "sharing-rule lesson": candidates, +not verdicts). ### `ui/` — 198 sites | File | Sites | Class | Note / next action | |---|---|---|---| -| `action.zod.ts` | 9 | authorable | param schema strict (#3746); remaining blocks ride later steps | +| `action.zod.ts` | 8 | authorable | param schema strict (#3746); remaining blocks ride later steps. **9 → 8 at the #4001 re-measurement** — no schema changed: the ninth "site" was a `z.object(…)` inside a JSDoc paragraph, which the old textual counter could not tell from code | | `view.zod.ts` | 50 | authorable | partially strict (ADR-0089); long tail of sub-blocks. `bulkActionDefs` left this file in #4457 — see the row below | | `bulk-action.zod.ts` | 3 | authorable | **strict as of #4457** — `BulkActionDefSchema` (the def itself). It was `z.array(z.record(z.string(), z.any()))` inline in `view.zod.ts`: a selection-bar button with **no shape at all**, so `opeartion` / `excution: 'aggregate'` parsed and shipped as a button that ran the default behaviour. Its two other sites are `BulkActionParamSchema` and that param's `options` entry, both deliberately **open**: objectui's `BulkActionParam` declares a `[key: string]: unknown` catch-all for widget config (min/max/step/format), so `.passthrough()` is the honest mirror and strictness there would reject valid config — same call as `dashboard.zod.ts`'s widget `config`. The def also refuses the combinations the executor never reads (`patch` outside an update, `execution` outside a custom, `batchSize` on an aggregate) and a hand-written `actionDef`, which is renderer-attached | | `component.zod.ts` | 29 | authorable | **next candidate** — SDUI component defs; check React-prop open slots first (p) | @@ -449,13 +495,13 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). | `dashboard.zod.ts` | 11 | authorable | partially strict | | `widget.zod.ts` | 9 | authorable (p) | | | `page.zod.ts` | 7 | authorable | partially strict (ADR-0089) | -| `chart.zod.ts` / `i18n.zod.ts` / `responsive.zod.ts` | 6+6+4 | authorable (p) | i18n label shapes are wide-open records by design — verify | +| `chart.zod.ts` / `i18n.zod.ts` / `responsive.zod.ts` | 7+6+4 | authorable (p) | i18n label shapes are wide-open records by design — verify. **`chart` 6 → 7 at the re-measurement** — again no schema changed: `ChartAggregateSchema` is written `z\n .object({`, and the old counter's `z\.object\(` could not match across the line break | | `dataset.zod.ts` / `animation.zod.ts` / `dnd.zod.ts` / `keyboard.zod.ts` / `touch.zod.ts` | 4+4+4+4+7 | authorable (p) | interaction configs | | `offline.zod.ts` / `report.zod.ts` | 3 ea | authorable (p) | | | `notification.zod.ts` | 1 | authorable (p) | **#4610 dropped two sites** — the `./ui` `Notification` (toast/banner instance) and `NotificationConfig` (toaster global config) shapes were removed: zero importers in all three repos, and both shadowed live names owned elsewhere (`./api` owns the inbox row). What remains is `NotificationActionSchema`, part of the presentation vocabulary the ui entry keeps | | `sharing.zod.ts` | 2 | authorable (p) | public-sharing config | -### `data/` — 161 sites +### `data/` — 162 sites | File | Sites | Class | Note | |---|---|---|---| @@ -475,9 +521,9 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). | `mapping.zod.ts` | 3 | authorable (p) | | | `external-catalog.zod.ts` | 4 | wire (p) | | | `validation.zod.ts` | 6 | authorable | **strict as of #4001 batch 3b** — a `z.lazy()` discriminated union, so the one-call conversion does not apply: each of the six variants builds its own `strictObject` from a shared `BASE_VALIDATION_SHAPE`. Closing the base alone would have rejected correctly but suggested from the SHARED keys only, so a typo of a variant's own key (`transtions` → `transitions`) would get no rename. Site count 1 → 6 because the six variants are now object sites in their own right. The ADR-0010 envelope lives in the shared shape, so all six inherit it | -| `field-value.zod.ts` / `seed.zod.ts` | 1+1 | mixed (p) | `seed` is strict (registered-types batch) | +| `field-value.zod.ts` / `seed.zod.ts` | 2+1 | mixed (p) | `seed` is strict (registered-types batch). **`field-value` 1 → 2 at the re-measurement**: `FileValueSchema` is `z.looseObject(` — a THIRD object idiom the old counter did not know, so the site was invisible rather than classified. It is deliberately open (an uploaded file's metadata bag); `LocationValueSchema` beside it is the strip site | -### `automation/` — 74 sites +### `automation/` — 75 sites | File | Sites | Class | Note | |---|---|---|---| @@ -493,6 +539,7 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). | `builtin-node-config.zod.ts` | 8 | authorable | Same family (#4045): the CRUD quartet, `screen`, `map`. Written from what the executors read rather than from the descriptors' `configSchema` literals, and reconciled bidirectionally by `builtin-node-form-zod-ledger.test.ts` — so unlike most rows here, this one already has a drift check of its own. Same candidacy note as `io-node-config` | | `schemaless-node-config.zod.ts` | 4 | authorable | Same family, third panel (#4278): `script` / `subflow` / `decision` (+ the decision branch item) — the descriptor-schemaless nodes whose form lives in objectui's hand-written table. Written from the executors; the drift check is objectui's `flow-node-config.spec-reconciliation` test (cross-repo, via the published exports). Since #4343 `script` and `subflow` ARE parsed at execute time (`parse-config.ts`) — `script` once retiring its `actionType` branches left it flat — so strictness candidacy now follows `io-node-config` on the same terms rather than being moot; `decision` stays export-only | | `webhook.zod.ts` | 1 | authorable (p) | spec-only (#3461) | +| `time-relative-trigger.zod.ts` | 1 | authorable | **Undeclared until the #4001 re-measurement, and invisible for the worst possible reason**: `TimeRelativeTriggerSchema` is written `z\n .object({`, the old textual counter matched zero sites, and a zero-site file is SKIPPED by the coverage walk as "nothing to classify". So the gate whose whole promise is "no undeclared surface" reported green over an authorable schema — the same shape as `data/driver/`, one layer subtler, because this time the file was not hidden by the walk but by the counter feeding it. Classification is not a guess: the file's own `@example` blocks author it by hand into a flow start node (`config: { timeRelative: { object, dateField, offsetDays, filter } }`), which is the authoring door. A stripped key here means the sweep silently never matches — `offsetDay` for `offsetDays` returns a trigger that never fires, reported as configured | | `flow-function.zod.ts` | 1 | authorable | `FlowFunctionDeclarationSchema` (#4396) — the `{ handler, effect }` form of a `defineStack({ functions })` entry. Authored, but note what an undeclared key here would be: a sibling of a **live function**, not data. `defineStack`'s union already rejects a record whose `handler` is not callable, and the boot-path reader is the hand-written `normalizeFlowFunctionEntry` rather than a `.parse()` (re-validating a live handler every boot buys nothing), so strictness would bind at authoring only. Candidate on the same verify-first rule as its `*-node-config` neighbours | `trigger-registry.zod.ts` had a row here (11 sites, "mixed — descriptors are code-registered (wire-ish); bindings authored") until #4499 deleted the file: all 11 sites were the third connector-vocabulary declaration (`ConnectorSchema` / `Authentication*` / `Operation*` / `ConnectorInstance`), and the old row's classification was optimistic twice over — nothing was ever code-registered against these descriptors and no binding was ever authored. The engine registers against `integration/connector.zod.ts` (ADR-0097), which keeps its own row. @@ -534,6 +581,114 @@ the silent narrowing it replaces. That is the trade this whole campaign makes, and the residual risk is named here so a reader with `objectui` access can close it rather than rediscover it. +## Remaining strip sites — the batch-planning map (re-measured 2026-08-03) + +The tables above answer "how much surface is here". They never answered **"how +much of it is still open"** — the `Class` column is a verdict about who writes +the input, and the `Note` column said things like *"partially strict"*, which is +prose. So the number every batch plan actually needs was, until this +re-measurement, unmeasured: the 2026-08-03 maintainer ruling was scheduled +against counts of `strictObject(` occurrences, which undercount strict sites by +every schema closed with the OLDER `z.object(…).strict()` idiom — reading +`automation/` as **0 strict** when it has 8, and `ui/` as 49 when it has 72. + +This section is that number, per file, and **it is gated** (`check:strictness-ledger`): + +- every file with at least one strip site must have a row here, with matching counts; +- a row whose file reaches **zero** strip sites **fails the gate** — a closed file + drops out of this table. That reverse pin is what makes the table a ratchet + rather than a snapshot, and it is the lesson from the ADR-0010 debt list applied + one level up: *a worklist that can outlive its work will.* + +`Class` here is per SCHEMA, not per file, so a row's strip count can span two +classes; where it does, the split is stated. **Only the authorable half is in the +2026-08-03 ruling's forced scope** — wire/open rows are listed so the arithmetic +is complete and so nobody re-triages them from scratch next batch. + +#### `automation/` — 67 strip of 75 + +| File | Strip | Sites | Class | Batch | +|---|---|---|---|---| +| `execution.zod.ts` | 13 | 13 | wire | **out of scope** — engine-emitted run state; the ledger row already says "never strict" | +| `etl.zod.ts` | 10 | 10 | mixed | 7 authorable (`ETLSource` + `.incremental`, `ETLDestination`, `ETLTransformation`, `ETLPipeline` + `.retry` + `.notifications`), 3 wire (`ETLPipelineRun` + `.stats` + `.error` — run state) | +| `builtin-node-config.zod.ts` | 8 | 8 | authorable | CRUD quartet + `Screen` (+ `.options`) + `Map`; already has a bidirectional drift check (`builtin-node-form-zod-ledger.test.ts`) | +| `flow.zod.ts` | 7 | 11 | mixed | 6 authorable (`FlowNode.connectorConfig` / `.position` / `.inputSchema` / `.waitEventConfig` / `.boundaryConfig`, `Flow.errorHandling`), 1 wire (`FlowVersionHistorySchema` — the ledger row already exempts it) | +| `state-machine.zod.ts` | 6 | 6 | authorable (p) | `ActionRef` / `GuardRef` / `Transition` / `StateNode` + `.meta` / `StateMachine` | +| `bpmn-interop.zod.ts` | 5 | 5 | wire (p) | **out of scope** — third-party BPMN import/export shapes; strictness turns an upstream addition into our parse crash | +| `control-flow.zod.ts` | 5 | 5 | authorable (p) | `FlowRegion` / `Loop` / `ParallelBranch` / `Parallel` / `TryCatch` — validated structurally by `validateControlFlow` today, which is a sibling guard, not a key gate | +| `node-executor.zod.ts` | 4 | 4 | wire | **out of scope** — executor registration contract, code-to-code | +| `schemaless-node-config.zod.ts` | 4 | 4 | authorable | `Script` / `Subflow` / `DecisionCondition` / `Decision`; `script` + `subflow` ARE parsed at execute time since #4343 | +| `io-node-config.zod.ts` | 2 | 2 | authorable | `NotifyConfig` / `HttpConfig` — the sibling contracts for the deliberately-open flow node `config` slot | +| `flow-function.zod.ts` | 1 | 1 | authorable | `FlowFunctionDeclarationSchema`; binds at authoring only (the boot reader is `normalizeFlowFunctionEntry`, not a `.parse()`) | +| `time-relative-trigger.zod.ts` | 1 | 1 | authorable | `TimeRelativeTriggerSchema` — **newly visible** (see its triage row); a stripped `offsetDay`/`withinDay` yields a trigger that never fires, reported as configured | +| `webhook.zod.ts` | 1 | 1 | authorable (p) | `WebhookSchema`, spec-only (#3461) | + +**Authorable strip in `automation/`: 41 of 67.** This is the ruling's "known main body". + +#### `ui/` — 124 strip of 198 + +| File | Strip | Sites | Class | Batch | +|---|---|---|---|---| +| `component.zod.ts` | 29 | 29 | authorable (p) | Largest single block left. SDUI component props — **verify the React-prop open slots first**; `check:react-declaration-parity` compares two DECLARATIONS and cannot tell you which props a renderer reads | +| `view.zod.ts` | 20 | 50 | mixed | Top level and the form/page shapes are closed (ADR-0089 + the final batch). Remaining are sub-blocks; `UserFiltersSchema` is the one the last batch **named as deliberately left open** — it strips page-only keys with a test pinning that, so closing it needs its own verification | +| `theme.zod.ts` | 14 | 14 | authorable (p) | Authored themes; `Typography` / `Animation` sub-blocks dominate | +| `widget.zod.ts` | 9 | 9 | authorable (p) | Widget manifest + lifecycle/event/property/source | +| `chart.zod.ts` | 7 | 7 | authorable (p) | Axis / series / annotation / interaction / config / groupBy / aggregate | +| `touch.zod.ts` | 7 | 7 | authorable (p) | Gesture configs | +| `i18n.zod.ts` | 6 | 6 | authorable (p) | ⚠️ the triage row warns label shapes are wide-open records **by design** — verify before closing | +| `animation.zod.ts` | 4 | 4 | authorable (p) | | +| `dnd.zod.ts` | 4 | 4 | authorable (p) | | +| `keyboard.zod.ts` | 4 | 4 | authorable (p) | | +| `responsive.zod.ts` | 4 | 4 | authorable (p) | | +| `dataset.zod.ts` | 3 | 4 | authorable (p) | `DatasetDimension` / `DatasetMeasure` + `.derived` | +| `offline.zod.ts` | 3 | 3 | authorable (p) | | +| `dashboard.zod.ts` | 2 | 11 | authorable | Only `DashboardWidget.compareTo` and `.layout` left; `DashboardWidgetOptionsSchema` stays `passthrough` **deliberately** (renderer escape hatch — see the triage row) | +| `report.zod.ts` | 2 | 3 | authorable (p) | `ReportSort` / `JoinedReportBlock` | +| `sharing.zod.ts` | 2 | 2 | authorable (p) | `SharingConfig` / `EmbedConfig` | +| `action.zod.ts` | 1 | 8 | authorable | `ActionParamSchema.options` — a plain `{ label, value }` pair; the cheapest win in the directory | +| `app.zod.ts` | 1 | 18 | verify | `BaseNavItemSchema` — the base the strict discriminated-union members extend. Closing a base that is `.extend()`ed is the #4001 trap that bit `view` (finding 16); confirm the members' strictness is not already covering it before touching | +| `bulk-action.zod.ts` | 1 | 3 | open | `BulkActionParamSchema.options`. ⚠️ **The triage row calls both this and its parent "deliberately open", but only the PARENT is `passthrough` — this one is plain strip.** Same intent, two postures; decide which the intent actually was | +| `notification.zod.ts` | 1 | 1 | authorable (p) | `NotificationActionSchema` | + +**Authorable strip in `ui/`: 123 of 124** — everything except `bulk-action.zod.ts`'s +`options`, which is `open`. Of those 123, `app.zod.ts`'s single site is held pending +the finding-16 `.extend()` check rather than counted as ready. + +#### `data/` — 121 strip of 162 + +| File | Strip | Sites | Class | Batch | +|---|---|---|---|---| +| `object.zod.ts` | 14 | 20 | authorable | The registered type's top level is closed (#1535/#4519/#4522); these are inner blocks — `Index`, `ObjectAccessConfig`, `Lifecycle` (+4 sub-blocks), `ObjectFieldGroup`, `ObjectExternalBinding`, `userActions`, `systemFields`, `activityMilestones`, `publicSharing`, `ObjectExtension`. Highest author volume in the repo | +| `data-engine.zod.ts` | 13 | 13 | wire | **out of scope** — engine request/response contracts | +| `external-lookup.zod.ts` | 12 | 12 | mixed (p) | `ExternalDataSource` + `.authentication` and the `ExternalLookup` tree are authored config; needs the per-schema read the ledger never did | +| `seed-loader.zod.ts` | 12 | 12 | mixed (p) | Split is real: `SeedLoaderConfig` / `SeedIdentity` (+`.user`/`.org`) / `ReferenceResolution` are authored; `SeedLoadResult` / `SeedLoaderResult` (+`.summary`) / `ReferenceResolutionError` / `ObjectDependencyNode` / `ObjectDependencyGraph` / `SeedLoaderRequest` are loader runtime | +| `filter.zod.ts` | 11 | 11 | open | **out of scope** — query dialect; user data flows through, validated semantically elsewhere | +| `driver-nosql.zod.ts` | 10 | 10 | wire | **out of scope** | +| `driver.zod.ts` | 9 | 9 | wire | **out of scope** — driver capability contract | +| `analytics.zod.ts` | 8 | 8 | mixed (p) | `Metric` / `Dimension` / `Cube` / `AnalyticsQuery` — cube definitions are authored; needs a per-schema read | +| `document.zod.ts` | 8 | 8 | wire (p) | `DocumentTemplate` / `ESignatureConfig` read authorable on their face — the `(p)` is unresolved, verify before scheduling either way | +| `driver/memory.zod.ts` | 5 | 6 | authorable | The persistence-adapter union under `datasource.config`; `datasource.config` HAS been parsed against these since #4410, so strictness here now binds | +| `query.zod.ts` | 5 | 5 | open | ⚠️ **classification conflict — see #4721.** The row calls the query dialect `open`; #4721 asks for `SortNodeSchema.strict()`. Both cannot be right. Resolve the class before writing code | +| `external-catalog.zod.ts` | 4 | 4 | wire (p) | **out of scope** | +| `hook.zod.ts` | 4 | 6 | wire | **out of scope** — `HookContextSchema` + `.session`/`.provenance`/`.user` are the runtime shape handed to a handler; verified in the data step | +| `field.zod.ts` | 3 | 11 | authorable | `LocationCoordinates` / `CurrencyValue` / `Address` — field VALUE shapes, not field config; check whether they are record data (→ open) before closing | +| `driver-sql.zod.ts` | 2 | 2 | wire | **out of scope** | +| `field-value.zod.ts` | 1 | 2 | mixed (p) | `LocationValueSchema` — record data, very likely **open**; its sibling `FileValueSchema` is already `z.looseObject` | + +**Authorable strip in `data/`: ~22 firm** (`object` 14 + `driver/memory` 5 + `field` 3), **plus ~33 needing a per-schema verdict** (`external-lookup` 12, `seed-loader` 12, `analytics` 8, `field-value` 1). 66 are wire/open and out of the ruling's forced scope. + +#### `security/` — 13 strip of 20 + +| File | Strip | Sites | Class | Batch | +|---|---|---|---|---| +| `explain.zod.ts` | 11 | 11 | wire | **out of scope** — permission-explain responses; the triage row already says "never strict" | +| `rls.zod.ts` | 2 | 3 | wire | **out of scope** — `RLSUserContext` / `RLSEvaluationResult` are runtime shapes; the POLICY shape is closed | + +**Authorable strip in `security/`: 0. This directory is DONE.** + +`studio/` has **0 strip of 27** and so has no table here — batch 7 landed, and this +is the confirmation the campaign's own progress log was missing. + ## Other directories (coarse; classify per schema before touching) | Dir | Sites | Dominant class | Rationale | @@ -611,6 +766,33 @@ it rather than rediscover it. about who consumes the output. When a future triage row needs promoting out of `(p)`, look for the factory first. +3. **The 2026-08-03 ruling: close the remaining authorable surface inside the + v17 window** (wire/open shapes stay out of forced scope, decided by the + classification rule at the top of this file). The worklist is the + **remaining-strip map** above, which is measured and gated rather than + estimated — read it, do not re-derive it, and do not plan off `strictObject(` + occurrence counts (finding 19 explains what that undercounts). + + Two things in that map need a decision before any code is written, and both + are classification questions rather than implementation ones: + + - **`data/query.zod.ts` is classed `open`, and #4721 asks for + `SortNodeSchema.strict()`.** Both cannot be right. Measured, so the decision + is made against facts rather than recollection: `SortNodeSchema.parse({ + field, direction: 'desc' })` returns `{ field, order: 'asc' }` — the wrong + rows, with no signal — and #4721's premise that the top level already + rejects unknown option keys is true but of a **different mechanism**: + #4371's check is a hand-written allowlist in `objectql/src/engine.ts` + (`rejectUnknownEngineOptions`) that iterates `Object.entries(bag)` at the + top level only. It is a bespoke guard at one door, which is this campaign's + finding 17 exactly, so "same invariant, one level down" is not available as + a justification — closing the sort node is a *new* door, not the completion + of an existing one. + - **`ui/app.zod.ts`'s `BaseNavItemSchema`** is the base that the strict + discriminated-union members `.extend()`. Finding 16 is the warning: closing + a base closes every extension of it, including any that is deliberately a + wire shape. + Done in the registered-types batch: `strictObject` (`shared/strict-object.ts`) replaced the four-part wiring recipe, and `seed` + `doc` became the first two conversions built on it — chosen by the registered-type lens above rather than diff --git a/packages/spec/scripts/check-strictness-ledger.mts b/packages/spec/scripts/check-strictness-ledger.mts index 532c2e8c93..a5fa94545f 100644 --- a/packages/spec/scripts/check-strictness-ledger.mts +++ b/packages/spec/scripts/check-strictness-ledger.mts @@ -16,11 +16,27 @@ // claims in it that are mechanically checkable. // // WHAT IT CHECKS -// 1. Site counts. The ledger states its own method — "site counts are `z.object(` -// occurrences per file" — so every count is verifiable. A count that no longer -// matches means someone added or removed a schema without reclassifying it, and -// the row's `Class` verdict now covers sites nobody triaged. This is the ratchet: -// touching a file forces you back through the ledger. +// 1. Site counts. Every object-constructing CALL in the file, read from the AST. +// A count that no longer matches means someone added or removed a schema without +// reclassifying it, and the row's `Class` verdict now covers sites nobody +// triaged. This is the ratchet: touching a file forces you back through the +// ledger. +// The counting used to be a regex over the source text, and the #4001 +// re-measurement found it wrong in BOTH directions on seven files: it counted +// `z.object({…})` written inside JSDoc prose, and it missed the prettier-wrapped +// `z\n .object({` form and `z.looseObject(` entirely. The worst case was +// `automation/time-relative-trigger.zod.ts`, which read as ZERO sites — and a +// zero-site file is deliberately SKIPPED by check 2, so an authorable schema sat +// outside the map while this gate printed "no undeclared schema files". Same +// shape as the non-recursive walk below, one layer further in: not the walk +// blind, but the counter feeding it. See `lib/strictness-ledger.ts`. +// 5. The remaining-strip map. The triage tables say how much surface exists; that +// never answered how much is still OPEN, which is the number batches are planned +// against. Gated in both directions: a file with strip sites must have a row with +// matching counts, AND a row whose file has reached zero strip sites FAILS, so a +// closed file drops out. The reverse pin matters more than the forward one — this +// ledger has already had to record that it once listed a shipped feature as a +// TODO, and a worklist that can outlive its work will. // 2. Coverage. Every `*.zod.ts` under a triaged directory that HAS `z.object(` sites // must appear in that directory's table. A new one is undeclared surface — // exactly what the ledger exists to prevent. The walk is RECURSIVE; nested files @@ -52,7 +68,7 @@ import fs from 'node:fs'; import path from 'node:path'; import url from 'node:url'; -import { countSites, listSchemaFiles } from './lib/strictness-ledger'; +import { analyzeSites, countSites, countStripSites, listSchemaFiles } from './lib/strictness-ledger'; const HERE = path.dirname(url.fileURLToPath(import.meta.url)); const SPEC = path.resolve(HERE, '..'); @@ -193,6 +209,111 @@ for (const [d, declared] of declaredByDir) { } } +// ── The remaining-strip-site map (#4001 re-measurement) ───────────────────── +// +// The triage tables above say how much surface exists; they never said how much +// of it is still OPEN. That number is what every batch plan is scheduled +// against, and until it was measured the campaign was planning off counts of +// `strictObject(` occurrences — which miss every schema closed with the older +// `z.object(…).strict()` idiom, reading `automation/` as 0 strict when it has 8. +// +// So it is a table now, and gated in BOTH directions: +// forward — a file with strip sites must have a row with the right counts; +// reverse — a row whose file has reached zero strip sites FAILS. +// +// The reverse pin is the important half. A worklist that can outlive its work +// will: this ledger has already had to record that it once listed a shipped +// feature as a TODO, and the ADR-0010 debt list needed the same pin for the same +// reason. A row that cannot survive its own completion cannot rot. +interface StripRow { dir: string; file: string; strip: number; total: number; line: number } +const stripRows: StripRow[] = []; +const stripTotals = new Map(); +let sdir: string | null = null; + +for (let i = 0; i < md.length; i++) { + const line = md[i]; + const header = line.match(/^#### `([a-z-]+)\/` — (\d+) strip of (\d+)/); + if (header) { + sdir = header[1]; + stripTotals.set(sdir, { strip: Number(header[2]), total: Number(header[3]), line: i + 1 }); + continue; + } + if (/^#{2,4} /.test(line) && !header) sdir = null; + if (!sdir || !line.startsWith('|')) continue; + const cells = line.split('|').slice(1, -1).map((c) => c.trim()); + if (cells.length < 4) continue; + const file = cells[0].match(/^`([^`]+\.zod\.ts)`$/)?.[1]; + if (!file || !/^\d+$/.test(cells[1]) || !/^\d+$/.test(cells[2])) continue; + stripRows.push({ dir: sdir, file, strip: Number(cells[1]), total: Number(cells[2]), line: i + 1 }); +} + +const stripDeclared = new Map>(); +for (const row of stripRows) { + const perDir = stripDeclared.get(row.dir) ?? new Map(); + stripDeclared.set(row.dir, perDir); + perDir.set(row.file, row); + + const abs = path.join(SRC, row.dir, row.file); + if (!fs.existsSync(abs)) { + errors.push( + `ledger:${row.line} — remaining-strip map lists \`${row.dir}/${row.file}\`, which does not exist.\n` + + ` → the file moved or was deleted; drop the row.`, + ); + continue; + } + const sites = analyzeSites(abs); + const strip = sites.filter((s) => s.posture === 'strip').length; + if (strip === 0) { + errors.push( + `ledger:${row.line} — \`${row.dir}/${row.file}\` has NO strip sites left, but still has a row in the\n` + + ` remaining-strip map.\n` + + ` → the file is CLOSED: delete the row and decrement the "${row.dir}/ — N strip of M" header.\n` + + ` This table is a worklist; a row that outlives its work is how this ledger rotted before.`, + ); + } else if (strip !== row.strip || sites.length !== row.total) { + errors.push( + `ledger:${row.line} — \`${row.dir}/${row.file}\` declares ${row.strip} strip of ${row.total}, found ${strip} of ${sites.length}.\n` + + ` → ${strip < row.strip ? 'sites were closed' : 'sites were opened or added'}. Update the row (and the section header),\n` + + ` and confirm the Class verdict still covers what is left.`, + ); + } +} + +for (const [d, perDir] of stripDeclared) { + const dirPath = path.join(SRC, d); + if (!fs.existsSync(dirPath)) continue; + const missing = listSchemaFiles(dirPath) + .filter((f) => !perDir.has(f) && countStripSites(path.join(dirPath, f)) > 0); + if (missing.length) { + errors.push( + `\`${d}/\` has ${missing.length} file(s) with strip sites missing from the remaining-strip map: ` + + `${missing.map((f) => `${f} (${countStripSites(path.join(dirPath, f))})`).join(', ')}\n` + + ` → add a row under \`#### \\\`${d}/\\\`\` with its strip/total counts and a per-schema Class verdict.`, + ); + } +} + +for (const [d, { strip, total, line }] of stripTotals) { + const rows = stripRows.filter((r) => r.dir === d); + const sumStrip = rows.reduce((a, r) => a + r.strip, 0); + const dirPath = path.join(SRC, d); + const actualTotal = fs.existsSync(dirPath) + ? listSchemaFiles(dirPath).reduce((a, f) => a + countSites(path.join(dirPath, f)), 0) + : 0; + if (sumStrip !== strip) { + errors.push( + `ledger:${line} — \`${d}/\` remaining-strip header says ${strip} strip, rows sum to ${sumStrip}.\n` + + ` → update the header to match the rows.`, + ); + } + if (total !== actualTotal) { + errors.push( + `ledger:${line} — \`${d}/\` remaining-strip header says "of ${total}", the directory has ${actualTotal} sites.\n` + + ` → update the header; it must match the triage section total for the same directory.`, + ); + } +} + // Section arithmetic. for (const [d, { declared, line }] of sectionTotals) { const sum = rows.filter((r) => r.dir === d).reduce((a, r) => a + r.counts.reduce((x, y) => x + y, 0), 0); @@ -212,7 +333,13 @@ if (errors.length) { } const fileCount = rows.reduce((a, r) => a + r.files.length, 0); +const openStrip = stripRows.reduce((a, r) => a + r.strip, 0); console.log( `✓ strictness ledger: ${fileCount} file(s) across ${sectionTotals.size} triaged director(ies) — ` + `site counts match, no undeclared schema files, section totals balance.`, ); +console.log( + `✓ remaining-strip map: ${stripRows.length} open file(s) / ${openStrip} strip site(s) across ` + + `${stripTotals.size} director(ies) — counts match, no file with strip sites is missing a row, ` + + `no closed file still carries one.`, +); diff --git a/packages/spec/scripts/lib/strictness-ledger.ts b/packages/spec/scripts/lib/strictness-ledger.ts index ef94bb29e3..8dd510cbe2 100644 --- a/packages/spec/scripts/lib/strictness-ledger.ts +++ b/packages/spec/scripts/lib/strictness-ledger.ts @@ -3,30 +3,166 @@ /** * Shared pieces of the #4001 strictness-ledger gate * (`../check-strictness-ledger.mts`), extracted so the gate and its regression - * test cannot drift apart on the one property that already broke silently: - * whether the coverage walk descends into subdirectories. + * test cannot drift apart on the properties that have already broken silently: + * whether the coverage walk descends into subdirectories, and whether the site + * counter recognises the way schemas are actually written. */ import fs from 'node:fs'; import path from 'node:path'; +import ts from 'typescript'; + /** - * Object sites — `z.object(` **or** `strictObject(` — the ledger's own stated - * counting method. - * - * `strictObject(` counts because it *is* an object site; it is what a converted - * schema looks like. Counting only `z.object(` would have made every conversion - * silently shrink the ledger's measured surface, so a directory being solved and - * a directory being deleted would read identically — and a genuinely new, - * un-triaged `strictObject` schema would never register as undeclared surface. - * - * The gate caught this itself on the first conversion (`data/seed.zod.ts`, 1 → 0), - * which is the behaviour to preserve: a change in how schemas are written must - * fail this check rather than quietly rebase what it measures. + * How an object site treats keys it does not declare. + * + * `strip` is zod's default and the thing #4001 exists to remove from authorable + * surface: the key is discarded and the parse still succeeds. */ -export function countSites(file: string): number { +export type Posture = 'strict' | 'passthrough' | 'catchall' | 'strip'; + +/** One object site, located and classified. */ +export interface Site { + /** Path relative to the walked root, `/`-separated (`driver/postgres.zod.ts`). */ + file: string; + line: number; + /** Best-effort dotted name: enclosing `const`, plus object-literal property path. */ + name: string; + posture: Posture; + /** `z.object` | `strictObject` | `z.strictObject` | `z.looseObject`. */ + idiom: string; +} + +/** + * Which object-constructing call this is, or `null` if it is not one. + * + * `strictObject(` is the campaign's helper; the rest are zod's own. Adding a + * spelling here is the ONLY place the instrument learns an idiom — see the note + * on {@link analyzeSites} for why that used to be three places. + */ +function idiomOf(call: ts.CallExpression): string | null { + const e = call.expression; + if (ts.isIdentifier(e) && e.text === 'strictObject') return 'strictObject'; + if (ts.isPropertyAccessExpression(e) && ts.isIdentifier(e.expression) && e.expression.text === 'z') { + if (e.name.text === 'object') return 'z.object'; + if (e.name.text === 'strictObject') return 'z.strictObject'; + if (e.name.text === 'looseObject') return 'z.looseObject'; + } + return null; +} + +/** Posture from the method chain applied to the site (`.strict()`, `.passthrough()`, …). */ +function postureOf(call: ts.CallExpression, idiom: string): Posture { + if (idiom === 'strictObject' || idiom === 'z.strictObject') return 'strict'; + if (idiom === 'z.looseObject') return 'passthrough'; + let node: ts.Node = call; + let posture: Posture = 'strip'; + for (;;) { + const parent: ts.Node | undefined = node.parent; + if (!parent) return posture; + if (ts.isPropertyAccessExpression(parent) && parent.expression === node) { + const m = parent.name.text; + if (m === 'strict') posture = 'strict'; + else if (m === 'passthrough' || m === 'loose') posture = 'passthrough'; + else if (m === 'strip') posture = 'strip'; + else if (m === 'catchall') posture = 'catchall'; + node = parent; + } else if (ts.isCallExpression(parent) && parent.expression === node) { + node = parent; + } else { + return posture; + } + } +} + +/** Enclosing `const X` plus any object-literal property path down to the site. */ +function nameOf(call: ts.CallExpression): string { + const segs: string[] = []; + let node: ts.Node = call; + let root = ''; + while (node.parent) { + const p = node.parent; + if (ts.isPropertyAssignment(p) && p.initializer === node) { + const n = p.name; + segs.unshift(ts.isIdentifier(n) || ts.isStringLiteral(n) ? n.text : '?'); + } else if (ts.isVariableDeclaration(p) && ts.isIdentifier(p.name)) { + root = p.name.text; + break; + } else if (ts.isFunctionDeclaration(p) && p.name) { + root = `${p.name.text}()`; + break; + } + node = p; + } + if (!root) root = '(anonymous)'; + return segs.length ? `${root}.${segs.join('.')}` : root; +} + +/** + * Every object site in `file`, with its posture — read from the AST. + * + * ## Why this is not a regex any more + * + * The ledger's stated method was "`z.object(` or `strictObject(` occurrences per + * file", matched textually. That method was wrong in BOTH directions at once, + * and the #4001 re-measurement found seven files where it disagreed with the + * source: + * + * - **It counted prose.** A `z.object({ … })` inside a JSDoc example is not a + * site. `ui/action.zod.ts` declared 9 and has 8; `kernel/metadata-protection` + * and `shared/suggestions` were entirely comment. + * - **It missed the wrapped call.** Prettier breaks a long chain as `z\n + * .object({`, which `z\.object\(` cannot match. That hid a site in + * `ui/chart.zod.ts`, two in `kernel/manifest.zod.ts`, and — worst — + * `automation/time-relative-trigger.zod.ts` read as ZERO sites, which made the + * coverage walk SKIP the file as "nothing to classify". An undeclared + * authorable schema in `automation/`, invisible behind a zero. + * - **It did not know `z.looseObject(`.** `data/field-value.zod.ts` has two + * sites and declared one. + * + * On `ui/` the two errors cancelled: `action` over by one, `chart` under by one, + * section total exactly right. A green section header over two wrong rows is the + * campaign's own subject matter reproduced in its own instrument — the fourth + * time a measuring tool here reported coverage it did not have (the ledger's + * findings 9 and its non-recursive walk being the earlier ones). + * + * The AST cannot be fooled by formatting or comments, and — the point — it stops + * the instrument needing to learn each new spelling separately. Posture comes + * from the same walk, which is what makes a per-file strict/strip split + * measurable at all; the old method could only ask "does this FILE contain any + * `.strict()`", never "how many of its sites are still open". + */ +export function analyzeSites(file: string, rel = path.basename(file)): Site[] { const src = fs.readFileSync(file, 'utf-8'); - return (src.match(/z\.object\(|(? { + if (ts.isCallExpression(node)) { + const idiom = idiomOf(node); + if (idiom) { + out.push({ + file: rel, + line: sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1, + name: nameOf(node), + posture: postureOf(node, idiom), + idiom, + }); + } + } + ts.forEachChild(node, visit); + }; + visit(sf); + return out.sort((a, b) => a.line - b.line); +} + +/** Object sites in `file` — the ledger's per-file count. */ +export function countSites(file: string): number { + return analyzeSites(file).length; +} + +/** Object sites in `file` that still silently discard unknown keys. */ +export function countStripSites(file: string): number { + return analyzeSites(file).filter((s) => s.posture === 'strip').length; } /** @@ -51,3 +187,8 @@ export function listSchemaFiles(dir: string): string[] { .filter((f) => f.endsWith('.zod.ts')) .sort(); } + +/** Every site under `dir`, recursively, with `file` relative to `dir`. */ +export function analyzeTree(dir: string): Site[] { + return listSchemaFiles(dir).flatMap((rel) => analyzeSites(path.join(dir, rel), rel)); +} diff --git a/packages/spec/scripts/strictness-ledger.test.ts b/packages/spec/scripts/strictness-ledger.test.ts index 0e9f6fe70d..1627b10543 100644 --- a/packages/spec/scripts/strictness-ledger.test.ts +++ b/packages/spec/scripts/strictness-ledger.test.ts @@ -23,17 +23,21 @@ */ import fs from 'node:fs'; +import os from 'node:os'; import path from 'node:path'; import url from 'node:url'; import { describe, expect, it } from 'vitest'; -import { countSites, listSchemaFiles } from './lib/strictness-ledger'; +import { analyzeSites, countSites, countStripSites, listSchemaFiles } from './lib/strictness-ledger'; const HERE = path.dirname(url.fileURLToPath(import.meta.url)); const SPEC = path.resolve(HERE, '..'); const SRC = path.join(SPEC, 'src'); const LEDGER = path.resolve(SPEC, '../../docs/audits/2026-07-unknown-key-strictness-ledger.md'); +/** Absolute path of a source file, relative to `packages/spec/src`. */ +const at = (rel: string) => path.join(SRC, rel); + /** The directories the ledger triages file-by-file. */ const TRIAGED = ['ui', 'data', 'automation', 'security', 'studio'] as const; @@ -71,3 +75,113 @@ describe('strictness-ledger coverage walk', () => { expect(undeclared).toEqual([]); }); }); + +/** + * The site counter's own regression cover. + * + * Written after the #4001 re-measurement found the previous, textual counter + * wrong in both directions at once — counting `z.object({…})` that appeared in + * JSDoc prose, and missing both the prettier-wrapped `z\n .object({` form and + * `z.looseObject(`. On `ui/` the two errors happened to cancel, so the section + * header was correct over two wrong rows. + * + * Every case below names a REAL site that the old counter got wrong, so the test + * is a red-on-known-input check rather than a restatement of current output. + */ +describe('site counting reads the AST, not the source text', () => { + it('does not count a z.object( that appears inside a comment', () => { + // Both "sites" the old regex found here are inside one JSDoc example. + expect(countSites(at('kernel/metadata-protection.zod.ts'))).toBe(0); + expect(countSites(at('shared/suggestions.zod.ts'))).toBe(0); + // And the case that mattered, because this file IS triaged: 9 → 8. + expect(countSites(at('ui/action.zod.ts'))).toBe(8); + }); + + it('counts a call the source wraps across lines (`z\\n .object({`)', () => { + // ChartAggregateSchema is written wrapped; the old `z\.object\(` missed it. + const chart = analyzeSites(at('ui/chart.zod.ts')); + expect(chart).toHaveLength(7); + expect(chart.map((s) => s.name)).toContain('ChartAggregateSchema'); + + // The one that was hidden ENTIRELY: a wrapped call was this file's only + // site, so it counted zero — and a zero-site file is skipped by the coverage + // walk, which is how an authorable schema stayed out of the ledger while the + // gate reported "no undeclared schema files". + const trigger = analyzeSites(at('automation/time-relative-trigger.zod.ts')); + expect(trigger).toHaveLength(1); + expect(trigger[0].name).toBe('TimeRelativeTriggerSchema'); + }); + + it('knows every object idiom, including z.looseObject(', () => { + const fv = analyzeSites(at('data/field-value.zod.ts')); + expect(fv).toHaveLength(2); + expect(fv.find((s) => s.name === 'FileValueSchema')?.idiom).toBe('z.looseObject'); + }); +}); + +/** + * Posture reading — the property that makes a remaining-work map possible at + * all. The gate could previously only ask "does this FILE contain any + * `.strict()`", which is why the campaign was still planning off counts of + * `strictObject(` occurrences: that idiom misses every schema closed with the + * OLDER `z.object(…).strict()` spelling, and reads `automation/` as 0 strict + * when it has 8. + * + * Each case pairs a real reading with a MUTATED copy of the same file, so the + * reading is shown to flip. A posture check that has only ever been seen green + * is the exact instrument this campaign keeps getting burned by. + */ +describe('posture reading, with a red control for each', () => { + /** Analyze a mutated copy of a real source file. */ + const mutate = (rel: string, from: string, to: string) => { + const src = fs.readFileSync(path.join(SRC, rel), 'utf-8'); + expect(src, `mutation anchor missing in ${rel}`).toContain(from); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'strictness-')); + const tmp = path.join(dir, path.basename(rel)); + fs.writeFileSync(tmp, src.replace(from, to)); + try { + return analyzeSites(tmp, rel); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }; + + it('reads the strictObject( helper as strict — and as strip once un-converted', () => { + const rel = 'ui/action.zod.ts'; + const live = analyzeSites(at(rel)).find((s) => s.name === 'ActionAiSchema'); + expect(live?.posture).toBe('strict'); + expect(live?.idiom).toBe('strictObject'); + + const red = mutate(rel, 'export const ActionAiSchema = strictObject({', + 'export const ActionAiSchema = z.object({').find((s) => s.name === 'ActionAiSchema'); + expect(red?.posture).toBe('strip'); + }); + + 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. + 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(countStripSites(at('security/permission.zod.ts'))).toBe(0); + }); + + it('reads a default site as strip — and as strict once closed', () => { + const rel = 'automation/execution.zod.ts'; + const live = analyzeSites(at(rel)).find((s) => s.name === 'ExecutionStepLogSchema'); + expect(live?.posture).toBe('strip'); + + const red = mutate(rel, 'export const ExecutionStepLogSchema = lazySchema(() => z.object({', + 'export const ExecutionStepLogSchema = lazySchema(() => z.object({}).strict().extend({') + .find((s) => s.name === 'ExecutionStepLogSchema'); + expect(red?.posture).toBe('strict'); + }); + + it('reads .passthrough() as passthrough, not as closed', () => { + // The deliberate renderer escape hatch. If this ever read as `strict` the + // remaining-work map would under-report, which is the failure direction that + // retires a reader's suspicion. + const dash = analyzeSites(at('ui/dashboard.zod.ts')); + expect(dash.filter((s) => s.posture === 'passthrough')).toHaveLength(1); + }); +});