test(spec): pin the input half of every recursive schema, so the third omission fails instead of shipping (#3786) - #4252
Merged
Conversation
A recursive Zod schema carries a hand-written `z.ZodType<...>` annotation. `z.ZodType` takes <Output, Input> and Input DEFAULTS TO `unknown`, so naming only the first compiles, validates correctly at runtime, and silently un-types every authoring path through the schema. This package has made that mistake twice. #4171 fixed the output half of the nav annotation and check-exported-any.ts, built to hold that fix, reads output only — so it reported green over the broken half. #4221 found the consequence (`defineApp` compiled `navigation: [{ totally: 'made up' }, 42, 'nonsense']`) and #4227 named both parameters on the six remaining recursive schemas. Both fixes are correct and both are unpinned. #4227 declined a .d.ts scanner for a good reason — separating a deliberate single-parameter `z.ZodType<T>` (the generic in contracts/llm-adapter.ts takes a caller-supplied schema) from an omission needs heuristics on emitted type names, against check-exported-any.ts's zero-false-positive rule — and nominated #4221's assertion pattern instead. This applies it to the eight schemas #4221 did not cover: Query, JoinNode, FieldNode, FilterCondition, NormalizedFilter, StateNode, ValidationRule, FormField. Each gets a positive probe (the authoring shape still compiles, guarding an input type drawn too tight) and a negative probe reached through `z.input<typeof Schema>` — the way a consumer gets there. The negative is load-bearing: a value `unknown` would accept and the real type rejects, so dropping a type parameter turns the suppression unused and tsc fails by name. Mutation-tested in both shapes a regression can take: - QuerySchema back to one parameter → the pin fires AND JoinNodeSchema cascades, because JoinNodeInput.subquery is QueryInput. - StateNodeSchema back to one parameter → ONLY the pin fires. Nothing else references its input, so without this file that regression is silent. That is the case the file exists for. No runtime change and no new public export (check:api-surface reports no diff); the module is referenced by no tsup entry and re-exported by no barrel. Also classifies check:strictness-ledger in check-generated.ts's ledger — it landed in #4232 with no entry, so check:generated was failing on main itself, the same cross-PR race the check:variant-docs entry above it documents. Verified: tsc --noEmit clean, spec suite 275 files / 7142 tests green, check:exported-any / liveness / strictness-ledger / docs / variant-docs / api-surface / generated all PASS against a fresh DTS build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
July 31, 2026 01:45
os-zhuang
pushed a commit
that referenced
this pull request
Jul 31, 2026
…d FieldNode form #4252 landed while this branch was open and pinned the INPUT half of every recursive schema — including two probes that assert the `{ field, fields, alias }` select entry is assignable. Both changes merge without a git conflict and are jointly wrong: the exact shape AGENTS.md's multi-agent discipline #10 warns about. `FieldNode` is no longer recursive, so `FieldNodeSchema` infers both halves itself rather than carrying a `z.ZodType<Output, Input>` annotation. Its probes stay anyway — this file's job is that the input side is CHECKED, and a `z.string()` someone re-widens to `z.ZodType<FieldNode>` fails here exactly as a dropped type parameter would. The positive probe becomes a dotted path (the replacement the removal prescribes) and the object form joins the negative side. `wiredFieldNode` needs no change: `z.input<typeof FieldNodeSchema>` is `string`, so `42` is still the error the suppression expects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EskBioEkCCJpGz3SvPQDxx
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…ect object form is removed (#4196) (#4268) `FieldNodeSchema` declared two forms for one entry of `QueryAST['fields']`: a string, and `{ field, fields?, alias? }` for a nested select on a relationship. The object form was declared-but-inert. Nothing produced it, and nothing read `.fields` or `.alias`: - `objectql/engine.ts` — formula projection and both known-field filters treat the list as `string[]`; one already reached for `String(f).split('.')[0]`, which is what defensiveness against an unhandled shape looks like. - `driver-sql` `select()`, `driver-memory` `projectFields` — same. - `driver-mongodb` keyed its projection with the entry itself, so an object entry asked for a column literally named "[object Object]". #4194 taught it to read `.field` because the newly-precise type made the old line a compile error — defensive handling of a shape nothing sends, not evidence of life. - The REST ingress stringified each entry before comparing it to the field map, so the same entry came back as `400 INVALID_FIELD: Unknown field '[object Object]'` — a rejection naming something the caller never wrote. So `fields: [{ field: 'owner', fields: ['name'] }]` was accepted by validation and then dropped or mangled depending on the driver (ADR-0078 silently-inert declaration; ADR-0049 enforce-or-remove). The capability it described is already served by `expand`, resolved through batch `$in` queries. Removing the second spelling rather than lowering it into the first is Prime Directive #12: one capability, one contract. `FieldNode` is now `string`; `FieldNodeSchema` is `z.string()` with an error map that answers an object entry with the FROM → TO prescription instead of zod's "expected string, received object" — and only an object, since telling the author of `fields: [42]` that their entry "was removed" would misinform. `z.input` becomes `string`, so `tsc` fails at the authoring site first. `assertProjectionFieldsExist` gains the shape axis, judged BEFORE the object's field map: the entry is wrong about its shape, not about this object, and a registry-less host (which the name gate gives up on) would otherwise hand it to a driver that cannot read it. Registered as an ADR-0087 D3 semantic migration (`query-field-node-object-form-retired`) rather than a D2 conversion: `QueryAST` is a request shape, never stored in stack metadata, so the chain has no source to rewrite. Same treatment as `EnhancedApiError.fieldErrors` and `BatchOptions.validateOnly` on this step. Two spec tests that pinned the object form as parseable are inverted — they proved the DECLARATION, which is exactly how a declared-but-inert shape survives a green suite. New pins cover the prescription, the non-object mistake keeping zod's own message, and the `expand` replacement; the list path gets three more in the #4226 conformance suite. Merging `main` also caught a jointly-wrong pair with no git conflict: #4252's recursive-schema input pins assert the retired object form is assignable. Those two probes move to the negative side (`FieldNode` is no longer recursive, so `FieldNodeSchema` infers both halves itself) — the AGENTS.md §10 case. No runtime behaviour changes for anything that ever worked; the defensive unwrapping the drivers had grown against a shape nothing sends goes with it. Closes #4196. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
os-zhuang
pushed a commit
that referenced
this pull request
Jul 31, 2026
…y key — the last derivable copies from the #3786 sweep Re-ran the sweep across all 72 packages. The earlier pass globbed `packages/*/src`, one level deep, so it missed everything under `packages/plugins/` and `packages/adapters/` — "the sweep is basically clean" was based on an incomplete scan. A STALE CORS DEFAULT, on the one description callers actually read. `HonoCorsOptions.allowHeaders`' TSDoc promised `['Content-Type', 'Authorization', 'X-Requested-With']` "sufficient for cookie and bearer-token auth". The real default carries three more: `X-Tenant-ID`, `X-Environment-Id` (multi-tenant routing) and `If-Match` (OCC token on record PATCHes, objectui#2572). Sizing a custom allowHeaders against that sentence drops all three and breaks every cross-origin save. Three Hono CORS sites each kept their own copy under "keep in sync" comments. The copies agreed; the DOC — the only description with no counterpart to be diffed against, and the only one a caller reads — is what drifted. Both defaults are now single constants exported from plugin-hono-server and imported by the adapter (which already depends on it). The TSDoc links them instead of restating, and documents the asymmetry it never mentioned: allowHeaders REPLACES the default, exposeHeaders MERGES. hono-plugin.test.ts stopped stubbing ./adapter wholesale and keeps the real constants via importOriginal — it asserts exact header lists, so a mocked copy would make the test agree with itself rather than with what ships. Verified: removing `If-Match` fails `should allow If-Match by default`, by name. A HAND-COPIED REGISTRY KEY. runtime's share-links domain resolved 'shareLinks' as a literal, copied from SHARE_LINK_SERVICE — whose own comment says "keep in sync with the SharingPlugin registration". Now imports the constant. A drifted copy resolves nothing, so every share link 501s "Sharing is not configured for this environment" on an environment where it is. PLUS A DUPLICATE LEDGER ENTRY, the same defect one level up: check-generated.ts carried two NO_GENERATOR entries for check:strictness-ledger, because #4203 and #4252 each added one without seeing the other. Harmless (the ledger is read into a Set) but two comments told overlapping versions of one story. #4203's is kept — the fuller account, from the PR that fixed the underlying problem. NOT included, because main already has it: wiring the reconciliation into CI. #4203 landed it as `check:generated --reconcile-only` in lint.yml's unfiltered job, which is strictly better than what this branch first proposed — it runs only the reconciliation rather than re-running all eight artifact gates that already have their own steps, and it sits in an unfiltered required job so it cannot go dormant behind a paths filter. Checked and left alone: ApprovalStatus (5 values) and ApprovalActionKind (12 values) vs their plugin-approvals selects — diffed verbatim, no drift today, still hand-copied across a package boundary. Verified: 28/28 turbo test tasks green (plugin-hono-server 149, hono 73, runtime 67 files), affected builds clean, check:generated --reconcile-only PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb
os-zhuang
pushed a commit
that referenced
this pull request
Jul 31, 2026
…y key — the last derivable copies from the #3786 sweep Re-ran the sweep across all 72 packages. The earlier pass globbed `packages/*/src`, one level deep, so it missed everything under `packages/plugins/` and `packages/adapters/` — "the sweep is basically clean" was based on an incomplete scan. A STALE CORS DEFAULT, on the one description callers actually read. `HonoCorsOptions.allowHeaders`' TSDoc promised `['Content-Type', 'Authorization', 'X-Requested-With']` "sufficient for cookie and bearer-token auth". The real default carries three more: `X-Tenant-ID`, `X-Environment-Id` (multi-tenant routing) and `If-Match` (OCC token on record PATCHes, objectui#2572). Sizing a custom allowHeaders against that sentence drops all three and breaks every cross-origin save. Three Hono CORS sites each kept their own copy under "keep in sync" comments. The copies agreed; the DOC — the only description with no counterpart to be diffed against, and the only one a caller reads — is what drifted. Both defaults are now single constants exported from plugin-hono-server and imported by the adapter (which already depends on it). The TSDoc links them instead of restating, and documents the asymmetry it never mentioned: allowHeaders REPLACES the default, exposeHeaders MERGES. hono-plugin.test.ts stopped stubbing ./adapter wholesale and keeps the real constants via importOriginal — it asserts exact header lists, so a mocked copy would make the test agree with itself rather than with what ships. Verified: removing `If-Match` fails `should allow If-Match by default`, by name. A HAND-COPIED REGISTRY KEY. runtime's share-links domain resolved 'shareLinks' as a literal, copied from SHARE_LINK_SERVICE — whose own comment says "keep in sync with the SharingPlugin registration". Now imports the constant. A drifted copy resolves nothing, so every share link 501s "Sharing is not configured for this environment" on an environment where it is. PLUS A DUPLICATE LEDGER ENTRY, the same defect one level up: check-generated.ts carried two NO_GENERATOR entries for check:strictness-ledger, because #4203 and #4252 each added one without seeing the other. Harmless (the ledger is read into a Set) but two comments told overlapping versions of one story. #4203's is kept — the fuller account, from the PR that fixed the underlying problem. NOT included, because main already has it: wiring the reconciliation into CI. #4203 landed it as `check:generated --reconcile-only` in lint.yml's unfiltered job, which is strictly better than what this branch first proposed — it runs only the reconciliation rather than re-running all eight artifact gates that already have their own steps, and it sits in an unfiltered required job so it cannot go dormant behind a paths filter. Checked and left alone: ApprovalStatus (5 values) and ApprovalActionKind (12 values) vs their plugin-approvals selects — diffed verbatim, no drift today, still hand-copied across a package boundary. Verified: 28/28 turbo test tasks green (plugin-hono-server 149, hono 73, runtime 67 files), affected builds clean, check:generated --reconcile-only PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…y key — the last derivable copies from the #3786 sweep (#4282) Re-ran the sweep across all 72 packages. The earlier pass globbed `packages/*/src`, one level deep, so it missed everything under `packages/plugins/` and `packages/adapters/`. A STALE CORS DEFAULT, on the one description callers actually read. HonoCorsOptions.allowHeaders' TSDoc promised ['Content-Type', 'Authorization', 'X-Requested-With'] "sufficient for cookie and bearer-token auth". The real default carries three more: X-Tenant-ID, X-Environment-Id (multi-tenant routing) and If-Match (OCC token on record PATCHes, objectui#2572). Sizing a custom allowHeaders against that sentence drops all three and breaks every cross-origin save. Three Hono CORS sites each kept their own copy under "keep in sync" comments. The copies agreed; the two prose descriptions each drifted separately — because the copies had each other to be diffed against and the prose had nothing. Both defaults are now single constants exported from plugin-hono-server and imported by the adapter (which already depends on it). The TSDoc links them instead of restating, and documents the asymmetry it never mentioned: allowHeaders REPLACES the default, exposeHeaders MERGES. A THIRD COPY, in the public protocol docs: http-protocol.mdx advertised "Access-Control-Allow-Headers: Authorization, Content-Type" — two of six — and methods missing PUT and HEAD, with no mention of exposed headers. That is the copy an integrator builds a client against: reading it you would not know If-Match is permitted, or that set-auth-token is readable. Corrected, with the non-obvious entries explained and a pointer to the constants. hono-plugin.test.ts stopped stubbing ./adapter wholesale and keeps the real constants via importOriginal — it asserts exact header lists, so a mocked copy would make the test agree with itself rather than with what ships. Verified: removing If-Match fails `should allow If-Match by default`, by name. A HAND-COPIED REGISTRY KEY. runtime's share-links domain resolved 'shareLinks' as a literal, copied from SHARE_LINK_SERVICE — whose own comment says "keep in sync with the SharingPlugin registration". Now imports the constant. A drifted copy resolves nothing, so every share link 501s "Sharing is not configured for this environment" on an environment where it is. PLUS A DUPLICATE LEDGER ENTRY, the same defect one level up: check-generated.ts carried two NO_GENERATOR entries for check:strictness-ledger, because #4203 and #4252 each added one without seeing the other. #4203's is kept. NOT included, because main already has it: wiring the reconciliation into CI. #4203 landed `check:generated --reconcile-only` in lint.yml's unfiltered job, strictly better than what this branch first proposed. Checked and left alone: ApprovalStatus (5 values) and ApprovalActionKind (12 values) vs their plugin-approvals selects — diffed verbatim, no drift today, still hand-copied across a package boundary. Verified: 28/28 turbo test tasks green (cache bypassed), 17/17 CI checks green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A recursive Zod schema cannot infer its own type, so it carries a hand-written
z.ZodType<...>annotation.z.ZodTypetakes two type parameters,<Output, Input>, andInputdefaults tounknown. Naming only the first compiles, validates correctly at runtime, and silently un-types every authoring path through the schema —unknownaccepts everything.This package has made that mistake twice:
any, so "bind the spec-named symbol to the spec" actively degrades the consumer that obeys it #4171 replacedz.ZodType<any>on the nav union withz.ZodType<NavigationItem>, fixing the output half.check-exported-any.tswas built to hold that fix and reads output only, so it reported green over the half still broken.defineApp, the documented authoring entry point, compilednavigation: [{ totally: 'made up' }, 42, 'nonsense']clean — and fix(spec): the remaining six recursive schemas name both type parameters, and the authoring artifacts stop spelling out defaults (#4195) #4227 then named both parameters on the six remaining recursive schemas.Both fixes are correct. Both are currently unpinned: nothing fails if a type parameter goes missing again.
Why probes rather than a scanner
#4227 considered a
.d.ts-level gate and declined it, correctly. Separating a deliberate single-parameterz.ZodType<T>— the generic incontracts/llm-adapter.tstakes a caller-supplied schema, where the input side is genuinely nobody's business — from an omission needs heuristics on emitted type names, andcheck-exported-any.ts's own rule is zero false positives so red keeps meaning broken. Its commit nominated #4221's assertion-file pattern instead.This is that pattern, applied to the eight schemas #4221 did not cover:
QuerySchema,JoinNodeSchema,FieldNodeSchema,FilterConditionSchema,NormalizedFilterSchema,StateNodeSchema,ValidationRuleSchema,FormFieldSchema.Each gets:
z.input<typeof Schema>, the way a consumer gets there. This is the load-bearing one: a valueunknownwould accept and the real type rejects, so dropping a type parameter turns the suppression unused andtsc --noEmitfails on that line, by name.The wiring matters as much as the types. A perfectly-written
QueryInputthatQuerySchemadoes not reference leavesz.input<typeof QuerySchema>atunknownand every authoring path unchecked — exactly the state being fixed — so the negatives go through the schema, not the type alias. (Same lesson as #4221, where my first cut pinned only the unions and the mutation sailed through.)Verification
Mutation-tested in both shapes a regression can take:
QuerySchemaback to one parameterJoinNodeSchemacascades a type error, sinceJoinNodeInput.subqueryisQueryInputStateNodeSchemaback to one parameterThe second row is the case the file exists for.
Also:
tsc --noEmit— cleancheck:exported-any/check:liveness/check:strictness-ledger/check:docs/check:variant-docs/check:api-surface/check:generated— all PASS against a fresh DTS build (a staledistreports a spurious 34-breaking diff oncheck:api-surface; that is the documented caveat, not a finding)No runtime change and no new public export —
check:api-surfacereports no diff, and the module is referenced by no tsup entry and re-exported by no barrel.Drive-by
check:strictness-ledgerlanded in #4232 without an entry incheck-generated.ts's ledger, socheck:generatedfails onmainitself. Classified here asNO_GENERATOR(the ledger it audits is hand-written prose). This is the second occurrence of the same cross-PR race thecheck:variant-docsentry directly above it already documents — worth noting if that keeps happening.Generated by Claude Code