Skip to content

Commit 6b83a55

Browse files
os-zhuangclaude
andauthored
fix(types): Page/App/Dashboard validate the spec's own fields instead of passing them through (objectstack#4115 group C) (#3063)
These three renderer nodes extend BaseSchema, which is `.passthrough()`, while their spec counterparts are strict. Every spec-only key therefore rode through objectui completely unvalidated — 23 on App (`branding`, `sharing`, `embed`, `objects`, `apis`, `requiredPermissions`, `homePageId`, `protection`, the whole `_lock*`/`_package*`/`_provenance` package envelope), 10 on Dashboard (`header`, `refreshInterval`, `performance`, `aria`, `protection`, …) and 6 on Page (`interfaceConfig`, `kind`, `slots`, `source`, `requires`, `aria`). A typo in any of them (`brading: {…}`) was indistinguishable from the real key. This is the objectstack#4120 silent-drop failure mode, objectui side. Page carried a live consequence: `source` was never declared, so a `kind: 'html' | 'react'` page — whose body lives in `source` — could not be expressed at all. The fix is NOT `.strict()`: these are component nodes and the open envelope is deliberate. Spec fields now flow in **by reference**, the pattern `zod/objectql.zod.ts`'s ListViewSchema already uses, via a new `specFieldsExcept()` helper in base.zod.ts. The helper reads `.shape` rather than calling `SpecSchema.omit({…}).partial()` because zod 4 refuses `.omit()` on an object carrying refinements — spec's PageSchema has one, so the idiomatic form throws at import time. Each omission is documented at its site: component-envelope keys go to BaseSchema; Page's `type` genuinely collides (spec's is the page KIND, objectui's is the component discriminator and the kind lives on `pageType`); and the element schemas that are their own ledger entries (navigation/areas/contextSelectors, widgets/globalFilters/dateRange, regions) stay local with the migration deferred. `.partial()` on the imported shape guarantees no future spec field can become required and invalidate payloads already stored. Mutation-tested: dropping a spec field from a derivation fails 3 tests by name, and adding an untriaged objectui-only key fails the rogue-key assertion. 78/78 type-check, 271 types + 3626 downstream assertions green. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent c41174a commit 6b83a55

5 files changed

Lines changed: 331 additions & 8 deletions

File tree

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* Page / App / Dashboard ↔ `@objectstack/spec` drift guard (objectstack#4115).
11+
*
12+
* These three renderer nodes extend `BaseSchema`, which is `.passthrough()`,
13+
* while their spec counterparts are strict. That combination is the objectui
14+
* equivalent of the objectstack#4120 silent-drop failure: every spec-only key
15+
* — `branding`, `sharing`, `interfaceConfig`, `source`, `header`,
16+
* `performance`, the whole `_lock*` package envelope — rode through
17+
* *completely unvalidated*, so `brading: {…}` was indistinguishable from
18+
* `branding: {…}` and a spec-authored `kind: 'html'` page could not even
19+
* express its body (`source` was never declared).
20+
*
21+
* The fix is NOT `.strict()` — these are component nodes and the open envelope
22+
* is deliberate. It is to let the spec's own fields flow in by reference
23+
* (`SpecXFields = SpecXSchema.omit({…}).partial()`), the pattern
24+
* `zod/objectql.zod.ts`'s `ListViewSchema` already uses.
25+
*
26+
* The derivation picks up new spec fields automatically, but can still break
27+
* silently in three ways, which these tests catch:
28+
*
29+
* 1. The spec grows a field objectui never triaged. Asserted: every spec key
30+
* is present in the objectui shape, or envelope-owned by BaseSchema.
31+
* 2. The spec renames/removes a field objectui deliberately omits and
32+
* re-declares locally — the local override would then shadow nothing.
33+
* Asserted: every omitted anchor still exists upstream.
34+
* 3. Someone adds an objectui-local key that should have been a spec field.
35+
* Asserted: every objectui-only key is in the sanctioned set below.
36+
*
37+
* When one fails, do NOT edit the sets to make it green — decide whether the
38+
* field belongs upstream in `@objectstack/spec` or is a genuine objectui-only
39+
* extension, and record the reason.
40+
*/
41+
42+
import { describe, it, expect } from 'vitest';
43+
import {
44+
AppSchema as SpecAppSchema,
45+
DashboardSchema as SpecDashboardSchema,
46+
PageSchema as SpecPageSchema,
47+
} from '@objectstack/spec/ui';
48+
import { AppSchema as OuiAppSchema } from '../zod/app.zod.js';
49+
import { DashboardSchema as OuiDashboardSchema } from '../zod/complex.zod.js';
50+
import { PageSchema as OuiPageSchema } from '../zod/layout.zod.js';
51+
import { BaseSchema } from '../zod/base.zod.js';
52+
53+
const shapeOf = (s: unknown) => (s as { shape: Record<string, unknown> }).shape;
54+
55+
/** Component-envelope keys owned by BaseSchema — derived, never hand-listed. */
56+
const ENVELOPE = new Set(Object.keys(shapeOf(BaseSchema)));
57+
58+
interface Case {
59+
spec: unknown;
60+
oui: unknown;
61+
/** Spec keys objectui deliberately omits and re-declares locally. */
62+
omitted: string[];
63+
/** objectui-only keys, each a conscious local-vs-upstream decision. */
64+
local: string[];
65+
}
66+
67+
const CASES: Record<string, Case> = {
68+
Page: {
69+
spec: SpecPageSchema,
70+
oui: OuiPageSchema,
71+
// `type` collides semantically (spec = page kind, objectui = component
72+
// discriminator, kind lives on `pageType`); `regions` is a local fork.
73+
omitted: ['type', 'regions'],
74+
local: ['title', 'pageType', 'body', 'children'],
75+
},
76+
App: {
77+
spec: SpecAppSchema,
78+
oui: OuiAppSchema,
79+
omitted: ['navigation', 'areas', 'contextSelectors'],
80+
local: ['title', 'logo', 'favicon', 'layout', 'menu', 'actions'],
81+
},
82+
Dashboard: {
83+
spec: SpecDashboardSchema,
84+
oui: OuiDashboardSchema,
85+
omitted: ['widgets', 'globalFilters', 'dateRange'],
86+
local: [],
87+
},
88+
};
89+
90+
describe.each(Object.keys(CASES))('%s spec parity (objectstack#4115 drift guard)', (name) => {
91+
const { spec, oui, omitted, local } = CASES[name];
92+
const specShape = shapeOf(spec);
93+
const ouiShape = shapeOf(oui);
94+
const ouiKeys = new Set(Object.keys(ouiShape));
95+
96+
it('covers every spec field — the spec cannot grow a key objectui ignores', () => {
97+
const missing = Object.keys(specShape).filter((k) => !ouiKeys.has(k) && !ENVELOPE.has(k));
98+
expect(missing).toEqual([]);
99+
});
100+
101+
it('keeps the upstream anchors it deliberately omits and re-declares', () => {
102+
// A rename upstream would orphan the local override, silently reopening
103+
// the very drift this derivation closed.
104+
for (const key of omitted) {
105+
expect(specShape, `spec no longer declares '${key}'`).toHaveProperty(key);
106+
}
107+
});
108+
109+
it('declares no objectui-only key outside the sanctioned set', () => {
110+
const rogue = [...ouiKeys].filter(
111+
(k) => !(k in specShape) && !ENVELOPE.has(k) && !local.includes(k),
112+
);
113+
expect(rogue).toEqual([]);
114+
});
115+
});
116+
117+
describe('spec-only keys are now VALIDATED, not passed through (objectstack#4115)', () => {
118+
it('Page declares the five keys that used to ride through unchecked', () => {
119+
const keys = Object.keys(shapeOf(OuiPageSchema));
120+
for (const key of ['interfaceConfig', 'kind', 'slots', 'source', 'requires']) {
121+
expect(keys, `'${key}' is still undeclared`).toContain(key);
122+
}
123+
});
124+
125+
it('Page can finally express a source-authored page (kind html/react)', () => {
126+
// Before the derivation `source` was not declared at all, so `kind: 'html'`
127+
// was unauthorable — the body had nowhere to live.
128+
const ok = OuiPageSchema.safeParse({ type: 'page', kind: 'html', source: '<h1>hi</h1>' });
129+
expect(ok.success).toBe(true);
130+
expect(ok.success && (ok.data as { source?: string }).source).toBe('<h1>hi</h1>');
131+
});
132+
133+
it('Page rejects a bad `kind` instead of waving it through', () => {
134+
expect(OuiPageSchema.safeParse({ type: 'page', kind: 'nonsense' }).success).toBe(false);
135+
});
136+
137+
it('App declares the package-lock and access keys that used to ride through', () => {
138+
const keys = Object.keys(shapeOf(OuiAppSchema));
139+
for (const key of ['branding', 'sharing', 'embed', 'objects', 'apis', 'requiredPermissions', 'homePageId', '_packageId']) {
140+
expect(keys, `'${key}' is still undeclared`).toContain(key);
141+
}
142+
});
143+
144+
it('App validates branding instead of accepting any shape', () => {
145+
expect(OuiAppSchema.safeParse({ type: 'app', branding: { primaryColor: '#fff' } }).success).toBe(true);
146+
// A wrong-typed branding used to be indistinguishable from a correct one.
147+
expect(OuiAppSchema.safeParse({ type: 'app', branding: 'blue' }).success).toBe(false);
148+
});
149+
150+
it('Dashboard declares header/refreshInterval/performance', () => {
151+
const keys = Object.keys(shapeOf(OuiDashboardSchema));
152+
for (const key of ['header', 'refreshInterval', 'performance', 'protection']) {
153+
expect(keys, `'${key}' is still undeclared`).toContain(key);
154+
}
155+
});
156+
157+
it('Dashboard validates header instead of accepting any shape', () => {
158+
expect(
159+
OuiDashboardSchema.safeParse({ type: 'dashboard', widgets: [], header: { showTitle: true } }).success,
160+
).toBe(true);
161+
expect(
162+
OuiDashboardSchema.safeParse({ type: 'dashboard', widgets: [], header: 'yes' }).success,
163+
).toBe(false);
164+
});
165+
});
166+
167+
describe('the derivation stays permissive where objectui needs it', () => {
168+
it('no spec field became required — stored payloads keep validating', () => {
169+
// `.partial()` on the imported shape is what guarantees this; without it a
170+
// spec field gaining `required` would invalidate every stored node.
171+
expect(OuiPageSchema.safeParse({ type: 'page' }).success).toBe(true);
172+
expect(OuiAppSchema.safeParse({ type: 'app' }).success).toBe(true);
173+
expect(OuiDashboardSchema.safeParse({ type: 'dashboard', widgets: [] }).success).toBe(true);
174+
});
175+
176+
it('the local vocabulary still parses', () => {
177+
expect(
178+
OuiPageSchema.safeParse({ type: 'page', pageType: 'record', title: 'T', regions: [] }).success,
179+
).toBe(true);
180+
expect(
181+
OuiAppSchema.safeParse({ type: 'app', title: 'T', logo: '/l.png', layout: 'sidebar' }).success,
182+
).toBe(true);
183+
});
184+
185+
it('the component envelope still passes unknown renderer props through', () => {
186+
// BaseSchema is deliberately open for type-specific extensions; the fix
187+
// narrows what the SPEC owns, it does not close the node.
188+
const r = OuiPageSchema.safeParse({ type: 'page', someRendererProp: 42 });
189+
expect(r.success).toBe(true);
190+
});
191+
});

packages/types/src/zod/app.zod.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
*/
1818

1919
import { z } from 'zod';
20-
import { BaseSchema } from './base.zod.js';
20+
import { AppSchema as SpecAppSchema } from '@objectstack/spec/ui';
21+
import { BaseSchema, specFieldsExcept } from './base.zod.js';
2122

2223
// ============================================================================
2324
// Unified NavigationItem Schema
@@ -151,7 +152,42 @@ export const AppContextSelectorSchema = z.object({
151152
/**
152153
* App Schema - Top-level application configuration
153154
*/
154-
export const AppSchema = BaseSchema.extend({
155+
/**
156+
* Spec-owned App fields, flowing in **by reference** (objectstack#4115).
157+
*
158+
* `BaseSchema` is `.passthrough()` while the spec's `AppSchema` is strict, so
159+
* before this derivation 23 spec-only keys rode through objectui completely
160+
* unvalidated — `branding`, `sharing`, `embed`, `objects`, `apis`,
161+
* `requiredPermissions`, `homePageId`, `protection` and the whole
162+
* `_lock*`/`_package*`/`_provenance` package-lock envelope. A typo in any of
163+
* them (`brading: {…}`) was invisible, and a packaged app round-tripped
164+
* through this schema lost nothing only by luck.
165+
*
166+
* Omitted, each for a stated reason:
167+
* - `name`/`label`/`description` — component-envelope keys owned by BaseSchema;
168+
* - `navigation`/`areas`/`contextSelectors` — objectui's element schemas are
169+
* their own ledger entries (they drift from the spec's on `badgeVariant`,
170+
* `expanded`/`defaultOpen`, `visible` and target-field requiredness);
171+
* migrating them is a separate, larger decision.
172+
*
173+
* `.partial()` guarantees no *future* spec field can become required and
174+
* silently invalidate stored objectui apps.
175+
*/
176+
const SpecAppFields = specFieldsExcept(SpecAppSchema.shape, [
177+
'name',
178+
'label',
179+
'description',
180+
'navigation',
181+
'areas',
182+
'contextSelectors',
183+
] as const);
184+
185+
/**
186+
* App Schema — the objectui app-shell renderer node, derived from
187+
* `@objectstack/spec/ui` `AppSchema` (see {@link SpecAppFields}). The drift
188+
* guard is `__tests__/page-app-dashboard-spec-parity.test.ts`.
189+
*/
190+
export const AppSchema = BaseSchema.extend(SpecAppFields.shape).extend({
155191
type: z.literal('app'),
156192
name: z.string().optional().describe('Application name (system ID)'),
157193
title: z.string().optional().describe('Display title'),

packages/types/src/zod/base.zod.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,36 @@ const BaseSchemaCore = z.object({
147147
*/
148148
export const BaseSchema = BaseSchemaCore;
149149

150+
/**
151+
* A spec schema's fields, minus the keys objectui declares locally, as an
152+
* all-optional shape ready for `BaseSchema.extend(…)` (objectstack#4115).
153+
*
154+
* `BaseSchema` is `.passthrough()` while the spec's document schemas are
155+
* strict, so a renderer node that does not declare the spec's fields lets
156+
* every one of them ride through *unvalidated*. Spreading this result back in
157+
* makes them validated again without closing the node to renderer props.
158+
*
159+
* Two deliberate properties:
160+
* - **by reference** — a field the spec adds is picked up automatically
161+
* rather than re-typed (and the per-node drift guard fails if it was never
162+
* triaged);
163+
* - **`.partial()`** — no future spec field can become required and silently
164+
* invalidate payloads objectui has already stored.
165+
*
166+
* Reads `.shape` rather than calling `SpecSchema.omit({…}).partial()` because
167+
* zod 4 refuses `.omit()` on an object carrying refinements — spec's
168+
* `PageSchema` has one, so the idiomatic form throws at import time.
169+
*/
170+
export function specFieldsExcept<T extends z.ZodRawShape, K extends keyof T & string>(
171+
shape: T,
172+
omit: readonly K[],
173+
) {
174+
const kept = Object.fromEntries(
175+
Object.entries(shape).filter(([key]) => !omit.includes(key as K)),
176+
) as Omit<T, K>;
177+
return z.object(kept).partial();
178+
}
179+
150180
/**
151181
* Component Input Configuration
152182
*/

packages/types/src/zod/complex.zod.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
*/
1818

1919
import { z } from 'zod';
20-
import { BaseSchema, SchemaNodeSchema } from './base.zod.js';
20+
import { DashboardSchema as SpecDashboardSchema } from '@objectstack/spec/ui';
21+
import { BaseSchema, SchemaNodeSchema, specFieldsExcept } from './base.zod.js';
2122
import { DASHBOARD_COLOR_VARIANTS, DASHBOARD_WIDGET_TYPES } from '../designer.js';
2223

2324
/**
@@ -322,7 +323,40 @@ export const GlobalFilterSchema = z.object({
322323
/**
323324
* Dashboard Schema - Dashboard component
324325
*/
325-
export const DashboardSchema = BaseSchema.extend({
326+
/**
327+
* Spec-owned Dashboard fields, flowing in **by reference** (objectstack#4115).
328+
*
329+
* `BaseSchema` is `.passthrough()` while the spec's `DashboardSchema` is
330+
* strict, so before this derivation every spec-only key rode through objectui
331+
* unvalidated — `header`, `refreshInterval`, `performance`, `aria`,
332+
* `protection` and the `_lock*`/`_package*`/`_provenance` package-lock
333+
* envelope were neither checked nor declared.
334+
*
335+
* Omitted, each for a stated reason:
336+
* - `name`/`label`/`description` — component-envelope keys owned by BaseSchema;
337+
* - `widgets`/`globalFilters`/`dateRange` — objectui's element schemas are
338+
* their own ledger entries (the local widget still carries the legacy
339+
* `component` envelope the spec has no room for, and both local configs are
340+
* deliberately looser than spec's); migration deferred.
341+
*
342+
* `.partial()` guarantees no *future* spec field can become required and
343+
* silently invalidate stored objectui dashboards.
344+
*/
345+
const SpecDashboardFields = specFieldsExcept(SpecDashboardSchema.shape, [
346+
'name',
347+
'label',
348+
'description',
349+
'widgets',
350+
'globalFilters',
351+
'dateRange',
352+
] as const);
353+
354+
/**
355+
* Dashboard Schema — the objectui dashboard renderer node, derived from
356+
* `@objectstack/spec/ui` `DashboardSchema` (see {@link SpecDashboardFields}).
357+
* The drift guard is `__tests__/page-app-dashboard-spec-parity.test.ts`.
358+
*/
359+
export const DashboardSchema = BaseSchema.extend(SpecDashboardFields.shape).extend({
326360
type: z.literal('dashboard'),
327361
columns: z.number().optional().describe('Number of columns'),
328362
gap: z.number().optional().describe('Grid gap'),

packages/types/src/zod/layout.zod.ts

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@
1818

1919
import { z } from 'zod';
2020
import {
21+
PageSchema as SpecPageSchema,
2122
PageTypeSchema as SpecPageTypeSchema,
2223
PageVariableSchema as SpecPageVariableSchema,
2324
} from '@objectstack/spec/ui';
24-
import { BaseSchema, SchemaNodeSchema } from './base.zod.js';
25+
import { BaseSchema, SchemaNodeSchema, specFieldsExcept } from './base.zod.js';
2526

2627
/**
2728
* Div Schema - Basic HTML container
@@ -273,10 +274,41 @@ export const PageVariableSchema = SpecPageVariableSchema;
273274
export const PageTypeSchema = SpecPageTypeSchema;
274275

275276
/**
276-
* Page Schema - Top-level page layout
277-
* Aligned with @objectstack/spec PageSchema
277+
* Spec-owned Page fields, flowing in **by reference** (objectstack#4115).
278+
*
279+
* `BaseSchema` is `.passthrough()` while the spec's `PageSchema` is strict, so
280+
* before this derivation every spec-only key rode through objectui completely
281+
* unvalidated — `interfaceConfig`, `kind`, `slots`, `source`, `requires` and
282+
* `aria` were neither checked nor even declared. `source` was the sharpest
283+
* hole: `kind: 'html' | 'react'` pages carry their body in `source`, so a
284+
* source-authored page could not be expressed here at all.
285+
*
286+
* Omitted, each for a stated reason:
287+
* - `name`/`label`/`description` — component-envelope keys owned by BaseSchema;
288+
* - `type` — the names genuinely collide: spec's `type` IS the page kind
289+
* (`record|app|utility|list|home`), objectui's is the component
290+
* discriminator (`'page'`) and the kind lives on `pageType` below.
291+
* Reconciling the two is a rename decision tracked separately;
292+
* - `regions` — objectui's `PageRegionSchema` adds `type`/`className` and
293+
* widens `width`; migration deferred (it is its own ledger entry).
294+
*
295+
* `.partial()` guarantees no *future* spec field can become required and
296+
* silently invalidate stored objectui pages.
297+
*/
298+
const SpecPageFields = specFieldsExcept(SpecPageSchema.shape, [
299+
'name',
300+
'label',
301+
'description',
302+
'type',
303+
'regions',
304+
] as const);
305+
306+
/**
307+
* Page Schema — top-level page layout, derived from `@objectstack/spec/ui`
308+
* `PageSchema` (see {@link SpecPageFields}). The drift guard is
309+
* `__tests__/page-app-dashboard-spec-parity.test.ts`.
278310
*/
279-
export const PageSchema = BaseSchema.extend({
311+
export const PageSchema = BaseSchema.extend(SpecPageFields.shape).extend({
280312
type: z.literal('page'),
281313
title: z.string().optional().describe('Page title'),
282314
icon: z.string().optional().describe('Page icon (Lucide icon name)'),

0 commit comments

Comments
 (0)