fix(spec): type-check defineApp's navigation again — #4171 fixed only half the annotation (#3786) - #4221
Merged
os-zhuang merged 2 commits intoJul 30, 2026
Conversation
… half the annotation (#3786) `NavigationItemSchema` is recursive, so it cannot infer its own type and carries a hand-written annotation. #4171 fixed that annotation's Output half (it was `z.ZodType<any>`, making the exported `NavigationItem` `any` for everyone). `z.ZodType` takes `<Output, Input>` and Input DEFAULTS TO `unknown`. Naming only the first parameter left the input half at that default, so `z.input<typeof AppSchema>` resolved `navigation` to `unknown` — and `unknown` accepts everything. `defineApp(config: z.input<typeof AppSchema>)`, the documented authoring entry point, compiled `navigation: [{ totally: 'made up' }, 42, 'nonsense']` clean. Parsing was never affected; only the compile-time contract lied, which is why no runtime test noticed. #4171 was verified through `z.infer` and nobody re-measured `z.input` — half a fix looks identical to a whole one. Adds `NavigationItemInput` and names both parameters. The two unions must stay distinct: `GroupNavItemSchema.expanded` and `UrlNavItemSchema.target` carry `.default()`, so those keys are required in the parsed output and omissible when authoring; reusing `NavigationItem` for input would force authors to write values the schema exists to supply. `app.nav-type-assertions.ts` is the mechanism, not a comment: a non-test src module (the package's `tsc --noEmit` gate excludes test files) pinning both unions AND the wiring between them and the schema. A correct `NavigationItemInput` that no schema references would leave every authoring path back at `unknown`, so the load-bearing probes go through `AppInput`. Mutation-tested — dropping the Input parameter, removing the `expanded` default, and leaking `children` onto a flat nav branch each turn the matching assertion red. 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:
|
…ed (#3786) The comment on PLATFORM_ALWAYS_ON_CAPABILITIES named `sms`, `messaging` and `analytics` as the hosted slate's gaps. Re-measured against cloud's actual wiring, `analytics` was never one: it is force-mounted through the host's `defaultRequires`. The error came from diffing the nearest list-shaped thing rather than what mounts. The array cited, `mountDefaultEnvironmentPlugins`' `ORDER`, has no production call site in cloud — so the diff compared against a list nobody reads, which is the same defect #3786 is about, committed while describing it. Cloud has no rival slate array. Its hosted set is assembled from the host's `defaultRequires` plus the loader's dependency patches, so the comparison has to be against effective wiring. Measured that way: `messaging` loads only when `audit` does, and `sms` has no provider package in that image. Comment only — no behaviour change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb
os-zhuang
marked this pull request as ready for review
July 30, 2026 16:49
os-zhuang
pushed a commit
that referenced
this pull request
Jul 30, 2026
…arallel #4221 ("type-check defineApp's navigation again — #4171 fixed only half the annotation") fixed `NavigationItemSchema` while this branch was in review. Both declarations of `NavigationItemInput` are byte-identical — two independent derivations of the same union — so the only conflict was the doc comment above it, and #4221's is kept: it is the landed version, it carries the `defineApp(navigation: [42, 'nonsense'])` repro, and it references `app.nav-type-assertions.ts`, the compile-level pin it added. What this branch still carries that #4221 did not touch: - the other six schemas with the same defect — FormFieldSchema, JoinNodeSchema, FieldNodeSchema, QuerySchema, FilterConditionSchema, NormalizedFilterSchema, plus StateNodeSchema and ValidationRuleSchema with a documented caveat. The `z.ZodType<T>` single-parameter form is now absent from the codebase. - `NavigationContributionInput`, which #4221's `NavigationItemInput` needs before a contribution array can be typed as authoring input. - the downstream close-out: 26 `.default()`ed keys that #4171 had to spell out are deleted, and the four authoring artifacts are annotated `AppInput` / `NavigationContributionInput` instead of the parsed types. Changeset rewritten to describe this branch's remaining scope rather than restating #4221's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XY3swmCsdYx6AMGiRmFt1H
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…ers, and the authoring artifacts stop spelling out defaults (#4195) (#4227) `z.ZodType` takes `<Output, Input>` and `Input` DEFAULTS TO `unknown`, so naming only the first parameter leaves `z.input` of anything embedding that schema degraded to `unknown` — the authoring side of the contract, unchecked. #4221 fixed `NavigationItemSchema`, the worst instance. This finishes the sweep: the same defect was on six more — FormFieldSchema, JoinNodeSchema, FieldNodeSchema, QuerySchema, FilterConditionSchema, NormalizedFilterSchema — plus StateNodeSchema and ValidationRuleSchema with a documented caveat. The `z.ZodType<T>` single-parameter form is now absent from the codebase. Measured with a type probe: `QueryInput['joins']` and `['fields']` were `unknown[]`; `z.input` of the FormField, Query, StateNode and ValidationRule schemas was `unknown`. New exported types: `FormFieldInput`, `JoinNodeInput`, `NavigationContributionInput`. `FilterCondition`, `NormalizedFilter` and `FieldNode` carry no `.default()` or `.transform()`, so their input is their output and the second parameter is simply the first. The downstream close-out, which is the user-visible cost of the defect: #4171 had to spell out `expanded: false` (x16) and `target: '_self'` (x10) across setup.app.ts, studio.app.ts and setup-nav.contributions.ts, because those artifacts are annotated with the PARSED type where a `.default()`ed key is required — and retyping them to the input surface would have traded eight loud errors for no checking at all. With the input types real they are annotated `AppInput` / `NavigationContributionInput`, the defaults are defaults again, and the literals are checked for the first time (net -33 lines across the four). Verified live rather than nominal: a literal omitting `expanded`/`target` compiles, and one writing `defaultOpen` — the non-spec key #4171 found in account.app.ts — is a compile error whose suggestion list names `expanded`. StateNodeSchema and ValidationRuleSchema reuse their hand-written type for both parameters: exact on the input side, loose on the output side (`StateNodeConfig` marks `type` optional though `.default('atomic')` makes it always present; `BaseValidationRuleShape` carries a `[key: string]: unknown` index signature). Both were already that loose — input went from `unknown` to a real type, output is untouched. The caveat is written at each declaration. No CI gate, deliberately. #4195 proposed failing on "output precise but input `unknown`"; measured after the fix, exactly two schemas match and both are CORRECT — TranslationItemSchema and InlineActionSchema are `z.preprocess(...)`, where an `unknown` input is zod's semantics, not a missing annotation. Separating those from a real omission needs heuristics on emitted type names, and per the rule in check-exported-any.ts's own header — zero false positives, so red keeps meaning broken — a gate that cannot be made reliable is worse than none. #4221's app.nav-type-assertions.ts is the better pattern where it applies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…d omission fails instead of shipping (#3786) (#4252) 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 half still broken. #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; neither was pinned. #4227 declined a .d.ts scanner for a good reason — separating a deliberate single-parameter `z.ZodType<T>` (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, so the pin covers the WIRING and not just the type alias. 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, since 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. No runtime change and no new public export; 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.
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.
The bug
NavigationItemSchemais recursive, so it cannot infer its own type and carries a hand-written annotation instead. #4171 fixed that annotation's Output half — it had beenz.ZodType<any>, which made the exportedNavigationItemanyfor every consumer.z.ZodTypetakes two parameters,<Output, Input>, andInputdefaults tounknown. Naming only the first left the input half at that default, soz.input<typeof AppSchema>resolvednavigationtounknown— andunknownaccepts everything.defineApp(config: z.input<typeof AppSchema>)is the documented authoring entry point, and this compiled clean:Every authoring path through the app schema —
AppInput,NavigationAreaSchema,NavigationContributionSchema— was unchecked the same way.Parsing was never affected: the schema rejects all of the above at runtime. Only the compile-time contract lied, which is why nothing in the 7165-test suite noticed. #4171's fix was verified through
z.infer; nobody re-measuredz.input, and half a fix looks identical to a whole one.This is the #3786 shape at the type level: two descriptions of one key set, where the second one silently constrains nothing.
The fix
A new exported
NavigationItemInputdescribes the authoring side, and the annotation now names both parameters.The two unions genuinely differ and one cannot serve both:
GroupNavItemSchema.expandedandUrlNavItemSchema.targetcarry.default(), so those keys are required in the parsed output and omissible when authoring. ReusingNavigationItemas the input type would force authors to write values the schema exists to supply.The mechanism, not a comment
packages/spec/src/ui/app.nav-type-assertions.tsis a non-testsrcmodule — the package'stsc --noEmitCI gate excludes test files, so probes must live outside them to be checked at all. It is referenced by no tsup entry and re-exported by no barrel, so it adds nothing to any build.It pins both unions and, critically, the wiring between them and the schema. My first cut only pinned the unions, and mutation testing caught that: a correct
NavigationItemInputthat no schema references still leaves every authoring path atunknown— precisely the state being fixed. The load-bearing assertions therefore go throughAppInput.Every probe was mutation-tested:
Inputparameter (z.ZodType<NavigationItem>)expanded: .default(false)→.optional()childrenonto a flat (page) input branchCompatibility
Nothing changes at runtime, and nothing changes for code reading a parsed app. Code that authors navigation is now type-checked where it was not, so genuinely malformed navigation that used to compile will surface as a compile error — pre-existing bugs becoming visible, not new restrictions.
Two notes on what the checked type says:
AppInput,NavigationItemInput) let you omitexpandedandtarget; parsed types (App,NavigationItem) still guarantee both are present.const APP: App = {…}) still requires spelling out every defaulted key. That was already true — this PR just makes the authoring alternative actually check its contents.Verification
tsc --noEmit(@objectstack/spec) — cleancheck:api-surface— 1 added (NavigationItemInput), 0 breaking; snapshot regeneratedcheck:docs— 250 generated files in sync@objectstack/downstream-contracttypecheck — cleanexamples/*typecheck (4 packages) — cleanNote on scope
This was found while investigating why cloud's admin app stopped compiling after #4194. That specific failure turned out to be a different thing — cloud annotates a hand-authored literal with the output type
Appand omitsexpandedon five groups, which is cloud's fix to make and matches whatsetup.app.ts/studio.app.tsalready do in-repo. I initially misdiagnosed it as a missinggroupbranch in theNavigationItemrecursion knot and wrote a fix for that; measurement showed theGroupNavItemalias already carried the knot, so that edit was a semantic no-op and was reverted rather than shipped with a comment claiming it fixed something. The unchecked input type is the real defect the investigation surfaced.Generated by Claude Code