Skip to content

Commit d6938bf

Browse files
os-zhuangclaude
andauthored
fix(spec): the remaining six recursive schemas name both type parameters, 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>
1 parent 1ddf479 commit d6938bf

12 files changed

Lines changed: 178 additions & 62 deletions

File tree

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/platform-objects": patch
4+
---
5+
6+
fix(spec): the remaining six recursive schemas name both type parameters, and the authoring artifacts stop spelling out defaults (#4195)
7+
8+
#4221 fixed `NavigationItemSchema` — the worst instance, and the one with a
9+
reproducible "`defineApp` compiles `navigation: [42, 'nonsense']`" demo. This
10+
finishes the sweep: **six more schemas** had the same shape, and the authoring
11+
artifacts that #4171 had to work around can now be typed honestly.
12+
13+
`z.ZodType` takes `<Output, Input>` and `Input` defaults to `unknown`, so naming
14+
only the first parameter leaves `z.input` of anything embedding that schema at
15+
`unknown`. Measured with a type probe:
16+
17+
| | was | now |
18+
|---|---|---|
19+
| `QueryInput['joins']` | `unknown[]` | `JoinNodeInput[]` |
20+
| `QueryInput['fields']` | `unknown[]` | `FieldNode[]` |
21+
| `z.input<typeof FormFieldSchema>` | `unknown` | `FormFieldInput` |
22+
| `z.input<typeof QuerySchema>` | `unknown` | `QueryInput` |
23+
| `z.input<typeof StateNodeSchema>` | `unknown` | `StateNodeConfig` |
24+
| `z.input<typeof ValidationRuleSchema>` | `unknown` | `BaseValidationRuleShape` |
25+
26+
New exported types: `FormFieldInput`, `JoinNodeInput`, `NavigationContributionInput`.
27+
`FilterCondition`, `NormalizedFilter` and `FieldNode` carry no `.default()` or
28+
`.transform()`, so their input is their output and the second parameter is the
29+
first.
30+
31+
**The `z.ZodType<T>` single-parameter form is now absent from the codebase.**
32+
33+
## 26 hand-written defaults deleted
34+
35+
This is the half #4221 left on the table. #4171 had to spell out
36+
`expanded: false` (×16) and `target: '_self'` (×10) across `setup.app.ts`,
37+
`studio.app.ts` and `setup-nav.contributions.ts`, because those artifacts are
38+
annotated with the PARSED type where a `.default()`ed key is required — and
39+
retyping them to the input surface would have traded eight loud errors for no
40+
checking at all.
41+
42+
With `NavigationItemInput` landed (#4221) and `NavigationContributionInput`
43+
added here, they are annotated `AppInput` / `NavigationContributionInput`, the
44+
defaults are defaults again, and the literals are checked for the first time.
45+
Net across those four files: 21 lines added, 54 removed.
46+
47+
Verified live, not nominal: a literal omitting `expanded`/`target` compiles, and
48+
one writing `defaultOpen` — the non-spec key #4171 found in `account.app.ts`
49+
is a compile error whose suggestion list names `expanded`.
50+
51+
## Two typed with a documented caveat
52+
53+
`StateNodeSchema` and `ValidationRuleSchema` reuse their hand-written type for
54+
both parameters: exact on the input side, loose on the output side.
55+
`StateNodeConfig` marks `type` optional though `.default('atomic')` makes it
56+
always present; `BaseValidationRuleShape` carries a `[key: string]: unknown`
57+
index signature. Both were already that loose — input went from `unknown` (types
58+
nothing) to a real type, output is untouched. Making them exact means deriving
59+
those types from their schemas instead of maintaining them beside one, which is
60+
separate work; the caveat is written at each declaration rather than left for a
61+
reader to find.
62+
63+
## Why there is still no CI gate for this
64+
65+
Worth recording, since #4195 proposed one: extend `check:exported-any` to fail on
66+
"output precise but input `unknown`". Measured after this change — exactly two
67+
schemas match, `TranslationItemSchema` and `InlineActionSchema`, and **both are
68+
correct**: they are `z.preprocess(...)`, where an `unknown` input is zod's
69+
semantics rather than a missing annotation. Separating those from a genuinely
70+
missing parameter needs heuristics on emitted type names, and per the rule in
71+
that script's own header — zero false positives, so red keeps meaning broken — a
72+
gate that cannot be made reliable is worse than none. #4221's
73+
`app.nav-type-assertions.ts` is the better pattern where it applies: pin the
74+
contract at compile level rather than infer intent from shape.

packages/platform-objects/src/apps/account.app.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@
2727
* self-service view, platform admins get the browsable tables.
2828
*/
2929

30-
import type { App } from '@objectstack/spec/ui';
30+
import type { AppInput } from '@objectstack/spec/ui';
3131

32-
export const ACCOUNT_APP: App = {
32+
export const ACCOUNT_APP: AppInput = {
3333
name: 'account',
3434
label: 'Account',
3535
description: 'Personal security and identity settings',
@@ -55,12 +55,9 @@ export const ACCOUNT_APP: App = {
5555
// manage their own linked accounts / personal OAuth apps. RLS on each
5656
// object scopes rows to the caller.
5757
//
58-
// Group open-state is `expanded` — the spec key. These entries said
59-
// `defaultOpen`, objectui's legacy alias, which the spec has never declared
60-
// and `.strict()` would reject; it worked only because objectui's
61-
// NavigationRenderer still falls back to it, and because `NavigationItem`
62-
// resolved to `any` so nothing checked the key here (#4171). Per Prime
63-
// Directive #12 the producer is the place to fix that, not the renderer.
58+
// Group open-state is `expanded` — the spec key. These said `defaultOpen`,
59+
// objectui's legacy alias the spec never declared; #4171 fixed that at the
60+
// producer per Prime Directive #12.
6461
navigation: [
6562
// Profile is the canonical landing — a hand-written React settings card
6663
// (Vercel/Linear style) registered in the Console SPA as

packages/platform-objects/src/apps/setup-nav.contributions.ts

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,14 @@
2222
* contributions in the same group (mirrors object owner priority).
2323
*/
2424

25-
import type { NavigationContribution } from '@objectstack/spec/ui';
25+
import type { NavigationContributionInput } from '@objectstack/spec/ui';
2626

2727
const BASE_PRIORITY = 100;
2828

29-
// `target: '_self'` on every `url` entry is the schema's own default, spelled
30-
// out: this array is annotated with the PARSED type, where a `.default()`ed key
31-
// is required. Omitting it was only possible while `NavigationItem` resolved to
32-
// `any` and nothing checked these literals at all (#4171).
33-
3429
// Marketplace entries (browse / installed) moved to
3530
// @objectstack/cloud-connection's marketplace plugins (cloud ADR-0009:
3631
// the nav lives and dies with the capability — no plugin, no entry).
37-
export const SETUP_NAV_CONTRIBUTIONS: NavigationContribution[] = [
32+
export const SETUP_NAV_CONTRIBUTIONS: NavigationContributionInput[] = [
3833
{
3934
app: 'setup',
4035
group: 'group_overview',
@@ -102,7 +97,7 @@ export const SETUP_NAV_CONTRIBUTIONS: NavigationContribution[] = [
10297
group: 'group_configuration',
10398
priority: BASE_PRIORITY,
10499
items: [
105-
{ id: 'nav_settings_hub', type: 'url', target: '_self', label: 'All Settings', url: '/apps/setup/system/settings', icon: 'settings-2', requiredPermissions: ['manage_platform_settings'] },
100+
{ id: 'nav_settings_hub', type: 'url', label: 'All Settings', url: '/apps/setup/system/settings', icon: 'settings-2', requiredPermissions: ['manage_platform_settings'] },
106101
// Workspace identity first — Localization (order 2) and Company (order 3)
107102
// are the lowest-`order` settings manifests and the first thing a new
108103
// company admin configures. They ship as `service-settings` manifests
@@ -111,15 +106,15 @@ export const SETUP_NAV_CONTRIBUTIONS: NavigationContribution[] = [
111106
// admin consoles (Salesforce "Company Information", ServiceNow) surface
112107
// both directly. No `requiredPermissions` — matches Branding (read perm is
113108
// the app's base `setup.access`).
114-
{ id: 'nav_settings_localization', type: 'url', target: '_self', label: 'Localization', url: '/apps/setup/system/settings/localization', icon: 'globe' },
115-
{ id: 'nav_settings_company', type: 'url', target: '_self', label: 'Company', url: '/apps/setup/system/settings/company', icon: 'building-2' },
116-
{ id: 'nav_settings_branding', type: 'url', target: '_self', label: 'Branding', url: '/apps/setup/system/settings/branding', icon: 'palette' },
117-
{ id: 'nav_settings_auth', type: 'url', target: '_self', label: 'Authentication', url: '/apps/setup/system/settings/auth', icon: 'lock-keyhole', requiredPermissions: ['manage_platform_settings'] },
118-
{ id: 'nav_settings_mail', type: 'url', target: '_self', label: 'Email', url: '/apps/setup/system/settings/mail', icon: 'mail', requiredPermissions: ['manage_platform_settings'] },
119-
{ id: 'nav_settings_storage', type: 'url', target: '_self', label: 'File Storage', url: '/apps/setup/system/settings/storage', icon: 'hard-drive', requiredPermissions: ['manage_platform_settings'] },
120-
{ id: 'nav_settings_ai', type: 'url', target: '_self', label: 'AI & Embedder', url: '/apps/setup/system/settings/ai', icon: 'sparkles', requiredPermissions: ['manage_platform_settings'] },
121-
{ id: 'nav_settings_knowledge', type: 'url', target: '_self', label: 'Knowledge', url: '/apps/setup/system/settings/knowledge', icon: 'book-open', requiredPermissions: ['manage_platform_settings'] },
122-
{ id: 'nav_settings_feature_flags', type: 'url', target: '_self', label: 'Feature Flags', url: '/apps/setup/system/settings/feature_flags', icon: 'flag' },
109+
{ id: 'nav_settings_localization', type: 'url', label: 'Localization', url: '/apps/setup/system/settings/localization', icon: 'globe' },
110+
{ id: 'nav_settings_company', type: 'url', label: 'Company', url: '/apps/setup/system/settings/company', icon: 'building-2' },
111+
{ id: 'nav_settings_branding', type: 'url', label: 'Branding', url: '/apps/setup/system/settings/branding', icon: 'palette' },
112+
{ id: 'nav_settings_auth', type: 'url', label: 'Authentication', url: '/apps/setup/system/settings/auth', icon: 'lock-keyhole', requiredPermissions: ['manage_platform_settings'] },
113+
{ id: 'nav_settings_mail', type: 'url', label: 'Email', url: '/apps/setup/system/settings/mail', icon: 'mail', requiredPermissions: ['manage_platform_settings'] },
114+
{ id: 'nav_settings_storage', type: 'url', label: 'File Storage', url: '/apps/setup/system/settings/storage', icon: 'hard-drive', requiredPermissions: ['manage_platform_settings'] },
115+
{ id: 'nav_settings_ai', type: 'url', label: 'AI & Embedder', url: '/apps/setup/system/settings/ai', icon: 'sparkles', requiredPermissions: ['manage_platform_settings'] },
116+
{ id: 'nav_settings_knowledge', type: 'url', label: 'Knowledge', url: '/apps/setup/system/settings/knowledge', icon: 'book-open', requiredPermissions: ['manage_platform_settings'] },
117+
{ id: 'nav_settings_feature_flags', type: 'url', label: 'Feature Flags', url: '/apps/setup/system/settings/feature_flags', icon: 'flag' },
123118
],
124119
},
125120
{

packages/platform-objects/src/apps/setup.app.ts

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@
2525
* matching the convention used by the HotCRM reference app.
2626
*/
2727

28-
import type { App } from '@objectstack/spec/ui';
28+
import type { AppInput } from '@objectstack/spec/ui';
2929

30-
export const SETUP_APP: App = {
30+
export const SETUP_APP: AppInput = {
3131
name: 'setup',
3232
label: 'Setup',
3333
description: 'Platform settings and administration',
@@ -47,18 +47,12 @@ export const SETUP_APP: App = {
4747
requiredPermissions: ['setup.access'],
4848
// Shell only — the stable group anchors. Children are supplied by
4949
// `navigationContributions` from the packages that own the objects.
50-
//
51-
// `expanded: false` is the schema's own default, spelled out: this literal is
52-
// annotated with the PARSED `App` type, where a `.default()`ed key is
53-
// required. Omitting it was only possible while `NavigationItem` resolved to
54-
// `any` and nothing checked these entries at all (#4171).
5550
navigation: [
5651
{
5752
id: 'group_overview',
5853
type: 'group',
5954
label: 'Overview',
6055
icon: 'layout-dashboard',
61-
expanded: false,
6256
requiredPermissions: ['manage_platform_settings'],
6357
children: [],
6458
},
@@ -67,31 +61,27 @@ export const SETUP_APP: App = {
6761
type: 'group',
6862
label: 'Apps',
6963
icon: 'package',
70-
expanded: false,
7164
children: [],
7265
},
7366
{
7467
id: 'group_people_org',
7568
type: 'group',
7669
label: 'People & Organization',
7770
icon: 'users',
78-
expanded: false,
7971
children: [],
8072
},
8173
{
8274
id: 'group_access_control',
8375
type: 'group',
8476
label: 'Access Control',
8577
icon: 'shield',
86-
expanded: false,
8778
children: [],
8879
},
8980
{
9081
id: 'group_approvals',
9182
type: 'group',
9283
label: 'Approvals',
9384
icon: 'check-circle',
94-
expanded: false,
9585
requiredPermissions: ['manage_platform_settings'],
9686
children: [],
9787
},
@@ -100,15 +90,13 @@ export const SETUP_APP: App = {
10090
type: 'group',
10191
label: 'Configuration',
10292
icon: 'sliders-horizontal',
103-
expanded: false,
10493
children: [],
10594
},
10695
{
10796
id: 'group_diagnostics',
10897
type: 'group',
10998
label: 'Diagnostics',
11099
icon: 'stethoscope',
111-
expanded: false,
112100
requiredPermissions: ['manage_platform_settings'],
113101
children: [],
114102
},
@@ -117,7 +105,6 @@ export const SETUP_APP: App = {
117105
type: 'group',
118106
label: 'Integrations',
119107
icon: 'plug',
120-
expanded: false,
121108
requiredPermissions: ['manage_platform_settings'],
122109
children: [],
123110
},

packages/platform-objects/src/apps/studio.app.ts

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@
2525
* the box whenever the auth/security trio is loaded.
2626
*/
2727

28-
import type { App } from '@objectstack/spec/ui';
28+
import type { AppInput } from '@objectstack/spec/ui';
2929

30-
export const STUDIO_APP: App = {
30+
export const STUDIO_APP: AppInput = {
3131
name: 'studio',
3232
label: 'Studio',
3333
description: 'Metadata workbench for developers, analysts, and implementers',
@@ -77,17 +77,12 @@ export const STUDIO_APP: App = {
7777
placement: 'sidebar_header',
7878
},
7979
],
80-
// `expanded: false` on each group is the schema's own default, spelled out:
81-
// this literal is annotated with the PARSED `App` type, where a `.default()`ed
82-
// key is required. Omitting it was only possible while `NavigationItem`
83-
// resolved to `any` and nothing checked these entries at all (#4171).
8480
navigation: [
8581
{
8682
id: 'group_overview',
8783
type: 'group',
8884
label: 'Overview',
8985
icon: 'layout-dashboard',
90-
expanded: false,
9186
children: [
9287
{
9388
// The application builder's front door (ADR-0080/0084): pick or
@@ -126,7 +121,6 @@ export const STUDIO_APP: App = {
126121
type: 'group',
127122
label: 'Data Model',
128123
icon: 'database',
129-
expanded: false,
130124
children: [
131125
{
132126
id: 'nav_objects',
@@ -152,7 +146,6 @@ export const STUDIO_APP: App = {
152146
type: 'group',
153147
label: 'User Experience',
154148
icon: 'layout',
155-
expanded: false,
156149
children: [
157150
{
158151
id: 'nav_apps',
@@ -211,7 +204,6 @@ export const STUDIO_APP: App = {
211204
type: 'group',
212205
label: 'Logic',
213206
icon: 'function-square',
214-
expanded: false,
215207
children: [
216208
{
217209
id: 'nav_actions',
@@ -237,7 +229,6 @@ export const STUDIO_APP: App = {
237229
type: 'group',
238230
label: 'Automation',
239231
icon: 'workflow',
240-
expanded: false,
241232
children: [
242233
{
243234
id: 'nav_flows',
@@ -262,7 +253,6 @@ export const STUDIO_APP: App = {
262253
type: 'group',
263254
label: 'AI',
264255
icon: 'sparkles',
265-
expanded: false,
266256
children: [
267257
{
268258
id: 'nav_agents',
@@ -299,7 +289,6 @@ export const STUDIO_APP: App = {
299289
type: 'group',
300290
label: 'Developer',
301291
icon: 'terminal',
302-
expanded: false,
303292
children: [
304293
{
305294
id: 'nav_api_console',
@@ -333,7 +322,6 @@ export const STUDIO_APP: App = {
333322
type: 'group',
334323
label: 'Integration',
335324
icon: 'plug',
336-
expanded: false,
337325
children: [
338326
{
339327
id: 'nav_email_templates',

packages/spec/api-surface.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,7 @@
397397
"JSONValidation (type)",
398398
"JSONValidationSchema (const)",
399399
"JoinNode (type)",
400+
"JoinNodeInput (type)",
400401
"JoinNodeSchema (const)",
401402
"JoinStrategy (const)",
402403
"JoinType (const)",
@@ -3302,6 +3303,7 @@
33023303
"FormButtonConfig (type)",
33033304
"FormButtonConfigSchema (const)",
33043305
"FormField (type)",
3306+
"FormFieldInput (type)",
33053307
"FormFieldSchema (const)",
33063308
"FormSection (type)",
33073309
"FormSectionSchema (const)",
@@ -3362,6 +3364,7 @@
33623364
"NavigationConfig (type)",
33633365
"NavigationConfigSchema (const)",
33643366
"NavigationContribution (type)",
3367+
"NavigationContributionInput (type)",
33653368
"NavigationContributionSchema (const)",
33663369
"NavigationItem (type)",
33673370
"NavigationItemInput (type)",

packages/spec/src/automation/state-machine.zod.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,20 @@ export type StateNodeConfig = {
7979

8080
/**
8181
* State Node Definition
82+
*
83+
* Both type arguments are given (#4195) so `z.input` is not `unknown` — a state
84+
* machine is hand-authored, and an `unknown` input type means nothing checks
85+
* what an author writes into `states`.
86+
*
87+
* Caveat worth knowing before trusting it: {@link StateNodeConfig} is written by
88+
* hand in the AUTHORING shape (`type?` is optional), while `type` is
89+
* `.default('atomic')` and so always present once parsed. Using it for both
90+
* arguments is therefore exact on the input side and slightly loose on the
91+
* output side — which is what this schema already claimed before, so nothing
92+
* regressed. Making the output exact means re-deriving `StateNodeConfig` from
93+
* the schema rather than maintaining it beside one; that is a separate change.
8294
*/
83-
export const StateNodeSchema: z.ZodType<StateNodeConfig> = z.lazy(() => z.object({
95+
export const StateNodeSchema: z.ZodType<StateNodeConfig, StateNodeConfig> = z.lazy(() => z.object({
8496
/** Type of state */
8597
type: z.enum(['atomic', 'compound', 'parallel', 'final', 'history']).default('atomic'),
8698

0 commit comments

Comments
 (0)