Skip to content

Commit e59786e

Browse files
os-zhuangclaude
andauthored
fix(spec): five exported symbols resolved to any — type the recursive schemas and gate it in CI (#4171) (#4194)
A recursive Zod schema needs an explicit annotation to break its circular inference, and five of them took the cheapest one available: export const NavigationItemSchema: z.ZodType<any> = z.lazy(() => …); export type NavigationItem = z.infer<typeof NavigationItemSchema>; // → any It compiles, it validates correctly at runtime, and it silently throws the type away. `NavigationItem`, `FormField`, `JoinNode` and `NormalizedFilter` were all `any` on the published surface, plus `FieldNodeSchema` — which had no exported type alias yet, so `z.infer<typeof FieldNodeSchema>` was `any` and `QueryAST['fields']` with it. That is worse than a missing export. #4115 tells every consumer that a local declaration under a spec export's name must be replaced by a binding to the spec — and for these, obeying it replaced a precise type with `any`. It is hard to catch by inspection because `any` is mutually assignable with everything, so the natural "are these the same type?" check answers yes in both directions and recommends precisely the wrong action — the same failure family as #4075's `[key: string]: any` on `ActionDef`. Now annotated with the real type, using the pattern `QueryAST` already follows in `data/query.zod.ts`: infer the non-recursive part, tie the recursive knot in the type, so the keys stay derived from the schema. Runtime validation is unchanged — every schema parses exactly what it parsed before. What the types immediately caught, none of it visible while they were `any`: - `account.app.ts` set `defaultOpen` on three nav groups, a key the spec has never declared; it worked only because objectui's NavigationRenderer still falls back to that legacy alias. Fixed at the producer per Prime Directive #12 — the canonical key is `expanded`. - The MongoDB driver built its projection with `projection[field] = 1` over `query.fields`, so a relationship `FieldNode` would have keyed the projection on `"[object Object]"`. It now reads the node's field name. - `setup.app.ts`, `studio.app.ts` and `setup-nav.contributions.ts` are annotated with the PARSED `App` / `NavigationContribution` types but omitted `.default()`ed keys (`expanded`, `target`), as did the form fields `metadata-protocol` synthesizes for `getUiView` (`span`). Each now states the default it was relying on, matching what the surrounding literals already do for `active` / `isDefault` / `collapsible` / `collapsed` / `columns`. Gated, not just fixed (`check:exported-any`, wired into the required TypeScript Type Check job). `api-surface.json` records that an export exists and never what it resolves to, which is how these survived a whole major with every gate green. The new scan reads the built `.d.ts` a consumer's import actually resolves to and fails on any exported type that resolves to `any` — or any exported schema whose output is `any`, the root cause, and the only reason `FieldNodeSchema` was visible at all. Its `KNOWN_ANY` ledger is shrink-only and currently empty. It self-tests against the real zod first, so if the internals it reads are ever renamed the gate fails loudly instead of quietly passing everything forever. Claude-Session: https://claude.ai/code/session_01XY3swmCsdYx6AMGiRmFt1H Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6eec18c commit e59786e

18 files changed

Lines changed: 680 additions & 62 deletions

File tree

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/platform-objects": patch
4+
"@objectstack/metadata-protocol": patch
5+
"@objectstack/driver-mongodb": patch
6+
---
7+
8+
fix(spec): five exported symbols resolved to `any` — type the recursive schemas and gate it in CI (#4171)
9+
10+
A recursive Zod schema needs an explicit annotation to break its circular
11+
inference, and five of them took the cheapest one available:
12+
13+
```ts
14+
export const NavigationItemSchema: z.ZodType<any> = z.lazy(() => …);
15+
export type NavigationItem = z.infer<typeof NavigationItemSchema>; // → any
16+
```
17+
18+
It compiles, it validates correctly at runtime, and it silently throws the type
19+
away. `NavigationItem`, `FormField`, `JoinNode` and `NormalizedFilter` were all
20+
`any` on the published surface, plus `FieldNodeSchema` — which had no exported
21+
type alias yet, so `z.infer<typeof FieldNodeSchema>` was `any` and
22+
`QueryAST['fields']` with it.
23+
24+
That is worse than a missing export. #4115 tells every consumer that a local
25+
declaration under a spec export's name must be replaced by a binding to the
26+
spec — and for these, obeying it **replaced a precise type with `any`**.
27+
objectui's `NavigationItem` is a 118-line documented interface (`recordId`
28+
template variables, `requiresObject` / `requiresService` capability gates,
29+
`filters` precedence); every key of it exists in the spec's version, so by every
30+
available signal it read as a redundant fork safe to delete. Deleting it swapped
31+
a fully-typed interface for `any`, with no compile error anywhere to say so.
32+
33+
It is hard to catch by inspection because `any` is mutually assignable with
34+
everything, so the natural "are these the same type?" check answers *yes* in both
35+
directions and recommends precisely the wrong action. Same failure family as
36+
#4075's `[key: string]: any` on `ActionDef`: a type that agrees with everything
37+
reads as agreement.
38+
39+
**Now annotated with the real type**, using the pattern `QueryAST` already
40+
follows in `data/query.zod.ts` — infer the non-recursive part, tie the recursive
41+
knot in the type, so the keys stay derived from the schema instead of being
42+
hand-maintained beside it:
43+
44+
```ts
45+
const BaseXSchema = z.object({ …every non-recursive key });
46+
export type X = z.infer<typeof BaseXSchema> & { children?: X[] };
47+
export const XSchema: z.ZodType<X> = z.lazy(() => BaseXSchema.extend({
48+
children: z.array(XSchema).optional(),
49+
}));
50+
```
51+
52+
`z.infer` now resolves to the type it should always have been: `NavigationItem`
53+
is the nine-branch discriminated union, `FormField` the 30-key form-field
54+
contract (with `visibleOn` absent by construction — ADR-0089 D2 folds it into
55+
`visibleWhen` at the boundary), `JoinNode` and the newly exported `FieldNode`
56+
the query AST nodes, `NormalizedFilter` the normalized filter AST. Runtime
57+
validation is unchanged: every schema parses exactly what it parsed before.
58+
59+
**What the types immediately caught**, none of it visible while they were `any`:
60+
61+
- `account.app.ts` set `defaultOpen` on three nav groups — a key the spec has
62+
never declared. It worked only because objectui's `NavigationRenderer` still
63+
falls back to that legacy alias. Fixed at the producer per Prime Directive
64+
#12: the canonical key is `expanded`.
65+
- The MongoDB driver built its projection with `projection[field] = 1` over
66+
`query.fields`, so a relationship `FieldNode` would have keyed the projection
67+
on `"[object Object]"`. It now reads the node's field name.
68+
- `setup.app.ts`, `studio.app.ts` and `setup-nav.contributions.ts` are annotated
69+
with the PARSED `App` / `NavigationContribution` types but omitted
70+
`.default()`ed keys (`expanded`, `target`), as did the form fields
71+
`metadata-protocol` synthesizes for `getUiView` (`span`). Each now states the
72+
default it was relying on, matching what the surrounding literals already do
73+
for `active` / `isDefault` / `collapsible` / `collapsed` / `columns`.
74+
75+
**Gated, not just fixed** (`check:exported-any`, wired into the required
76+
`TypeScript Type Check` job). `api-surface.json` records that an export *exists*
77+
and never what it *resolves to*, which is how these survived a whole major with
78+
every gate green. The new scan reads the built `.d.ts` a consumer's import
79+
actually resolves to and fails on any exported type that resolves to `any` — or
80+
any exported schema whose output is `any`, the root cause, and the only reason
81+
`FieldNodeSchema` was visible at all. Its `KNOWN_ANY` ledger is shrink-only and
82+
currently empty. It self-tests against the real zod first, so if the internals it
83+
reads are ever renamed the gate fails loudly instead of quietly passing
84+
everything forever.

.github/workflows/lint.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,19 @@ jobs:
284284
- name: Check @objectstack/spec public API surface
285285
run: pnpm --filter @objectstack/spec run check:api-surface
286286

287+
# Same surface, the other axis: api-surface.json records that an export
288+
# EXISTS, never what it resolves to — so four exported types sat at `any`
289+
# across a whole major with every gate green (#4171). #4115 tells consumers
290+
# to replace a local declaration with the spec import, which for those four
291+
# traded a precise type for one that constrains nothing, silently: `any` is
292+
# mutually assignable with everything, so the check that would catch the
293+
# swap reports "identical, safe to re-export". Reads the built dist a
294+
# consumer's import actually resolves to, so it runs after the build step
295+
# with the other consumer gates. Self-tests first — a scan whose green
296+
# result is "nothing found" has to prove it can still find something.
297+
- name: Check no exported spec type resolves to `any`
298+
run: pnpm --filter @objectstack/spec run check:exported-any
299+
287300
# Anti-drift for the skill EXAMPLES, not just the skill reference indexes
288301
# (#3094). The TypeScript in skills/ is the first thing an AI copies when
289302
# authoring metadata, yet nothing type-checked it — so it rotted silently

AGENTS.md

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,10 +274,20 @@ believe it, and before you file a bug about `main` being red. (Two phantom "brea
274274
removals" this way while writing this section; `check:generated` now prints this caveat
275275
inline when that gate is the one failing.)
276276

277-
`check:liveness`, `check:empty-state`, `check:skill-examples` and
278-
`check:react-conformance` are pure checks with no generator — a failure there is a real
279-
finding to fix, not an artifact to regenerate. `check:generated` names them as
280-
deliberately not run, so its "all up to date" never reads as "everything passed".
277+
`check:liveness`, `check:empty-state`, `check:skill-examples`,
278+
`check:react-conformance` and `check:exported-any` are pure checks with no generator — a
279+
failure there is a real finding to fix, not an artifact to regenerate. `check:generated`
280+
names them as deliberately not run, so its "all up to date" never reads as "everything
281+
passed".
282+
283+
`check:exported-any` is the one of those that also reads the built `dist/*.d.ts`, so the
284+
stale-`dist` caveat above applies to it too. It asks the other half of the
285+
`api-surface.json` question: that snapshot records an export *exists*, never what it
286+
*resolves to*, which is how five exported symbols sat at `any` for a whole major with
287+
every gate green (#4171). A recursive Zod schema needs an annotation to break its
288+
circular inference, and `z.ZodType<any>` compiles, validates correctly, and silently
289+
throws the type away — annotate with the type instead (`QueryAST` in
290+
`src/data/query.zod.ts` is the pattern).
281291

282292
Two generators have **no** gate at all — `gen:openapi` and `gen:sbom`. Nothing verifies
283293
their output is current; the script reports that each run rather than staying silent

content/docs/references/ui/view.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,12 +158,12 @@ Column footer summary configuration
158158
| **span** | `Enum<'auto' \| 'full'>` | optional | Relative field width. 'auto' (default — omit it): the renderer sizes the field from its widget type × the current column count (wide widgets like textarea/richtext/json/file/subform take the whole row). 'full': whole row at any column count. Prefer this over the absolute `colSpan`. |
159159
| **widget** | `string` | optional | Custom widget/component name (overrides type-based inference) |
160160
| **language** | `string` | optional | Code editor language (for type=code) |
161-
| **fields** | `[FormField](#formfield)[]` | optional | Sub-fields for composite/repeater/record types |
162161
| **keyField** | `{ field?: string; label?: string; placeholder?: string; helpText?: string; … }` | optional | Key column config for record-typed fields |
163162
| **dependsOn** | `string` | optional | Parent field name for cascading |
164163
| **visibleWhen** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Visibility predicate (CEL) — field shown only when TRUE. Root: `record`+`current_user` (runtime forms) or `data` (metadata forms). e.g. P`record.priority == 'urgent'` |
165164
| **visibleOn** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | [DEPRECATED → `visibleWhen`] Visibility predicate (CEL). Normalized to `visibleWhen` at parse. |
166165
| **disclosure** | `Enum<'inline' \| 'popover'>` | optional | Composite rendering: inline bordered box (default) or a summary line + gear popover (progressive disclosure). |
166+
| **fields** | `[FormField](#formfield)[]` | optional | Sub-fields for composite/repeater/record types |
167167

168168

169169
---

packages/metadata-protocol/src/protocol.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2894,7 +2894,14 @@ export class ObjectStackProtocolImplementation implements
28942894
readonly: fields[f]?.readonly,
28952895
type: fields[f]?.type,
28962896
// Default to 2 columns for most, 1 for textareas
2897-
colSpan: (fields[f]?.type === 'textarea' || fields[f]?.type === 'html') ? 2 : 1
2897+
colSpan: (fields[f]?.type === 'textarea' || fields[f]?.type === 'html') ? 2 : 1,
2898+
// `FormField.span` defaults to 'auto'; this view is hand-built
2899+
// rather than run through `FormFieldSchema.parse()`, so the
2900+
// default is spelled out to make the returned object a real
2901+
// parsed `View` (the same reason the section below states its
2902+
// own `collapsible` / `collapsed` / `columns` defaults). Was
2903+
// unnoticed while `FormField` resolved to `any` (#4171).
2904+
span: 'auto' as const
28982905
}));
28992906

29002907
return {

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,13 @@ export const ACCOUNT_APP: App = {
5454
// No `requiredPermissions`: any authenticated user must be able to
5555
// manage their own linked accounts / personal OAuth apps. RLS on each
5656
// object scopes rows to the caller.
57+
//
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.
5764
navigation: [
5865
// Profile is the canonical landing — a hand-written React settings card
5966
// (Vercel/Linear style) registered in the Console SPA as

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

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ import type { NavigationContribution } 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+
2934
// Marketplace entries (browse / installed) moved to
3035
// @objectstack/cloud-connection's marketplace plugins (cloud ADR-0009:
3136
// the nav lives and dies with the capability — no plugin, no entry).
@@ -97,7 +102,7 @@ export const SETUP_NAV_CONTRIBUTIONS: NavigationContribution[] = [
97102
group: 'group_configuration',
98103
priority: BASE_PRIORITY,
99104
items: [
100-
{ id: 'nav_settings_hub', type: 'url', label: 'All Settings', url: '/apps/setup/system/settings', icon: 'settings-2', requiredPermissions: ['manage_platform_settings'] },
105+
{ id: 'nav_settings_hub', type: 'url', target: '_self', label: 'All Settings', url: '/apps/setup/system/settings', icon: 'settings-2', requiredPermissions: ['manage_platform_settings'] },
101106
// Workspace identity first — Localization (order 2) and Company (order 3)
102107
// are the lowest-`order` settings manifests and the first thing a new
103108
// company admin configures. They ship as `service-settings` manifests
@@ -106,15 +111,15 @@ export const SETUP_NAV_CONTRIBUTIONS: NavigationContribution[] = [
106111
// admin consoles (Salesforce "Company Information", ServiceNow) surface
107112
// both directly. No `requiredPermissions` — matches Branding (read perm is
108113
// the app's base `setup.access`).
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' },
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' },
118123
],
119124
},
120125
{

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,18 @@ 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).
5055
navigation: [
5156
{
5257
id: 'group_overview',
5358
type: 'group',
5459
label: 'Overview',
5560
icon: 'layout-dashboard',
61+
expanded: false,
5662
requiredPermissions: ['manage_platform_settings'],
5763
children: [],
5864
},
@@ -61,27 +67,31 @@ export const SETUP_APP: App = {
6167
type: 'group',
6268
label: 'Apps',
6369
icon: 'package',
70+
expanded: false,
6471
children: [],
6572
},
6673
{
6774
id: 'group_people_org',
6875
type: 'group',
6976
label: 'People & Organization',
7077
icon: 'users',
78+
expanded: false,
7179
children: [],
7280
},
7381
{
7482
id: 'group_access_control',
7583
type: 'group',
7684
label: 'Access Control',
7785
icon: 'shield',
86+
expanded: false,
7887
children: [],
7988
},
8089
{
8190
id: 'group_approvals',
8291
type: 'group',
8392
label: 'Approvals',
8493
icon: 'check-circle',
94+
expanded: false,
8595
requiredPermissions: ['manage_platform_settings'],
8696
children: [],
8797
},
@@ -90,13 +100,15 @@ export const SETUP_APP: App = {
90100
type: 'group',
91101
label: 'Configuration',
92102
icon: 'sliders-horizontal',
103+
expanded: false,
93104
children: [],
94105
},
95106
{
96107
id: 'group_diagnostics',
97108
type: 'group',
98109
label: 'Diagnostics',
99110
icon: 'stethoscope',
111+
expanded: false,
100112
requiredPermissions: ['manage_platform_settings'],
101113
children: [],
102114
},
@@ -105,6 +117,7 @@ export const SETUP_APP: App = {
105117
type: 'group',
106118
label: 'Integrations',
107119
icon: 'plug',
120+
expanded: false,
108121
requiredPermissions: ['manage_platform_settings'],
109122
children: [],
110123
},

0 commit comments

Comments
 (0)