Skip to content

Commit fc5a3a2

Browse files
os-zhuangclaude
andauthored
fix(spec): validate all three runtime view shapes in the view type-schema (#3095) (#3159)
* fix(spec): validate all three runtime view shapes in the `view` type-schema (#3095) The `view` metadata type mapped to the aggregate container `ViewSchema` (`{ list, form, listViews, formViews }`, all slots optional). Zod strips unknown keys, so the two non-container shapes a `view` body carries at runtime — a standalone ViewItem record (`{ name, object, viewKind, config }`) and a console personalization overlay (raw config + inherited identity, #2555) — both strip-parsed to `{}`. That made the 422 check in `saveMetaItem` and read-time `computeMetadataDiagnostics` a no-op for them: a broken `config` (e.g. a kanban missing `groupByField`) saved with a false 200 and badged valid. Map `view` to a new `ViewMetadataSchema` — a union over the three shapes, each validated genuinely: 1. non-empty defineView container (empty container rejected), 2. ViewItem record (nested config validated against ListView/FormView), 3. flattened personalization overlay (inline config + identity), with structural guards pinning config/list/form/listViews/formViews to undefined so a malformed record/container is never rescued through the lenient branch with its payload stripped. All members strip-parse (no .strict()) so aux Studio keys still ride along without a false 422; saveMetaItem keeps persisting verbatim. toJSONSchema emits an anyOf of the four members for /api/v1/meta/types/view. Adds regression tests: schema-level (all three shapes + JSON Schema), save-path 422 for a bad ViewItem, and computeMetadataDiagnostics badges. Fixes #3095. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LFmMsWLC3CVULh3Ke7rUdd * fix(spec): keep liveness gate + api-surface green for the `view` union schema Two CI gates broke on the #3095 change that maps `view` to a union: - Spec property liveness: `check-liveness.mts` walked each metadata type's top-level OBJECT shape; a union has none, so it threw "view is not an object schema". Teach `shapeOf` to resolve a union to its first object-typed member — the canonical authorable container (list/form/listViews/formViews) the ledger governs. Discriminated-union members (ViewItem) are skipped; their inner config is the same ListView/FormView surface already governed under the container's list/form children. - API-surface snapshot: record the two intended new public exports (`ViewMetadata` type, `ViewMetadataSchema` const) via gen:api-surface. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LFmMsWLC3CVULh3Ke7rUdd --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d7a6758 commit fc5a3a2

8 files changed

Lines changed: 441 additions & 5 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
**The `view` metadata type-schema now validates all three runtime `view` shapes instead of stripping two of them to `{}`.** `metadata-type-schemas.ts` mapped `view` to the aggregate container `ViewSchema` (`{ list, form, listViews, formViews }`, every slot optional). Zod strips unknown keys, so the two non-container shapes a `view` body actually carries at runtime — a standalone **ViewItem record** (`{ name, object, viewKind, config }`) and a **console personalization overlay** (raw view config + identity inherited by `normalizeViewMetadata`, #2555) — both strip-parsed to `{}`. That made the `422` check in `saveMetaItem` and read-time `computeMetadataDiagnostics` a **no-op** for those shapes: a broken `config` (e.g. a kanban missing `groupByField`) saved with a false `200` and badged valid, and the view create-seed test validated against nothing.
6+
7+
`view` now maps to a new `ViewMetadataSchema` — a union over the three shapes, each validated genuinely:
8+
9+
1. **defineView container** — non-empty (`ViewSchema` refined to require at least one of `list`/`form`/`listViews`/`formViews`; an empty container is rejected, mirroring `defineView`).
10+
2. **ViewItem record**`ViewItemSchema`; the nested `config` is validated against ListView/FormView.
11+
3. **Flattened personalization overlay** — inline ListView/FormView config plus optional identity fields. Structural guards pin `config`/`list`/`form`/`listViews`/`formViews` to `undefined` so a malformed record or container can never be rescued through this lenient branch with its real payload silently stripped.
12+
13+
All members strip-parse (no `.strict()`), so auxiliary Studio round-trip keys (`isPinned`, `sortOrder`, …) still ride along without a false `422`, and `saveMetaItem` keeps persisting the body verbatim. `z.toJSONSchema()` emits the schema as an `anyOf` of the four members, which `/api/v1/meta/types/view` serves to Studio's SchemaForm.
14+
15+
Fixes #3095.

packages/objectql/src/metadata-diagnostics.test.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*/
1111

1212
import { describe, expect, it } from 'vitest';
13-
import { computeViewReferenceDiagnostics } from '@objectstack/metadata-protocol';
13+
import { computeViewReferenceDiagnostics, computeMetadataDiagnostics } from '@objectstack/metadata-protocol';
1414

1515
const objectDef = {
1616
fields: {
@@ -81,3 +81,56 @@ describe('computeViewReferenceDiagnostics (ADR-0047)', () => {
8181
expect(computeViewReferenceDiagnostics({}, {}).valid).toBe(true);
8282
});
8383
});
84+
85+
/**
86+
* #3095 — `computeMetadataDiagnostics('view', …)` must see through the two
87+
* runtime `view` shapes the old container schema strip-parsed to `{}` (a
88+
* standalone ViewItem record and a personalization overlay). Before the fix it
89+
* returned `{ valid: true }` unconditionally for both, so Studio's badge never
90+
* flagged a broken enumerated ViewItem. These pin the schema-backed diagnostics.
91+
*/
92+
describe('computeMetadataDiagnostics — view shapes (#3095)', () => {
93+
it('flags a ViewItem whose kanban config is missing groupByField', () => {
94+
const diag = computeMetadataDiagnostics('view', {
95+
name: 'crm_lead.pipeline',
96+
object: 'crm_lead',
97+
viewKind: 'list',
98+
config: { type: 'kanban', columns: ['name'], kanban: { summarizeField: 'amount', columns: ['name'] } },
99+
});
100+
expect(diag?.valid).toBe(false);
101+
expect(diag?.errors?.length).toBeGreaterThan(0);
102+
});
103+
104+
it('passes a well-formed ViewItem', () => {
105+
const diag = computeMetadataDiagnostics('view', {
106+
name: 'crm_lead.all',
107+
object: 'crm_lead',
108+
viewKind: 'list',
109+
config: { type: 'grid', columns: ['name'], data: { provider: 'object', object: 'crm_lead' } },
110+
});
111+
expect(diag?.valid).toBe(true);
112+
});
113+
114+
it('passes a personalization overlay body (badges do not regress)', () => {
115+
const diag = computeMetadataDiagnostics('view', {
116+
type: 'grid',
117+
data: { provider: 'object', object: 'showcase_task' },
118+
columns: ['title'],
119+
sort: [{ id: 'x', field: 'estimate_hours', order: 'desc' }],
120+
name: 'showcase_task.default',
121+
viewKind: 'list',
122+
object: 'showcase_task',
123+
label: 'All Tasks',
124+
});
125+
expect(diag?.valid).toBe(true);
126+
});
127+
128+
it('still validates the defineView container shape', () => {
129+
expect(computeMetadataDiagnostics('view', {
130+
list: { type: 'grid', data: { provider: 'object', object: 'crm_lead' }, columns: ['name'] },
131+
})?.valid).toBe(true);
132+
expect(computeMetadataDiagnostics('view', {
133+
list: { type: 'grid', data: { provider: 'object', object: 'crm_lead' } }, // no columns
134+
})?.valid).toBe(false);
135+
});
136+
});

packages/objectql/src/protocol-view-identity-overlay.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,44 @@ describe('view overlay identity (#2555)', () => {
168168
expect(item.label).toBe('My Renamed Grid');
169169
});
170170

171+
// #3095 — a standalone ViewItem record's `config` used to strip to `{}`
172+
// under the container ViewSchema, so a broken config saved with a false 200.
173+
// The `view` type now maps to the union schema that validates it genuinely.
174+
it('write path rejects a ViewItem whose kanban config is broken (422)', async () => {
175+
const { engine } = makeStubEngine();
176+
const protocol = new ObjectStackProtocolImplementation(engine);
177+
await expect(
178+
protocol.saveMetaItem({
179+
type: 'view',
180+
name: 'crm_lead.pipeline',
181+
item: {
182+
name: 'crm_lead.pipeline',
183+
object: 'crm_lead',
184+
viewKind: 'list',
185+
// kanban config is missing the required groupByField.
186+
config: { type: 'kanban', columns: ['name'], kanban: { summarizeField: 'amount', columns: ['name'] } },
187+
},
188+
}),
189+
).rejects.toMatchObject({ code: 'invalid_metadata', status: 422 });
190+
});
191+
192+
it('write path accepts a well-formed ViewItem record', async () => {
193+
const { engine, rows } = makeStubEngine();
194+
const protocol = new ObjectStackProtocolImplementation(engine);
195+
const result = await protocol.saveMetaItem({
196+
type: 'view',
197+
name: 'crm_lead.all',
198+
item: {
199+
name: 'crm_lead.all',
200+
object: 'crm_lead',
201+
viewKind: 'list',
202+
config: { type: 'grid', columns: ['name'], data: { provider: 'object', object: 'crm_lead' } },
203+
},
204+
});
205+
expect(result.success).toBe(true);
206+
expect(Array.from(rows.values()).some((r) => r.type === 'view')).toBe(true);
207+
});
208+
171209
it('write path stays a plain name-stamp when the registry has no entry to inherit from', async () => {
172210
const { engine, rows } = makeStubEngine();
173211
const protocol = new ObjectStackProtocolImplementation(engine);

packages/spec/api-surface.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3471,6 +3471,8 @@
34713471
"ViewKeyCollision (interface)",
34723472
"ViewKind (type)",
34733473
"ViewKindSchema (const)",
3474+
"ViewMetadata (type)",
3475+
"ViewMetadataSchema (const)",
34743476
"ViewSchema (const)",
34753477
"ViewScope (type)",
34763478
"ViewScopeSchema (const)",

packages/spec/scripts/liveness/check-liveness.mts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,22 @@ function unwrap(s: any, depth = 0): any {
8787
function shapeOf(s: any): Record<string, any> | null {
8888
const u = unwrap(s);
8989
const def = defOf(u);
90-
if (def?.type !== 'object') return null;
91-
return def.shape ?? u.shape ?? null;
90+
if (def?.type === 'object') return def.shape ?? u.shape ?? null;
91+
// #3095 — a metadata type may register a UNION of shapes rather than a single
92+
// object (e.g. `view`: defineView container | ViewItem record | flattened
93+
// personalization overlay). The liveness ledger governs the canonical
94+
// authorable **container**, which is the first object-typed member of the
95+
// union. Discriminated-union members (ViewItem) are skipped — their inner
96+
// `config` is the same ListView/FormView surface already governed under the
97+
// container's `list` / `form` children — so this walks the container shape.
98+
if (def?.type === 'union' && Array.isArray(def.options)) {
99+
for (const opt of def.options) {
100+
const uo = unwrap(opt);
101+
const od = defOf(uo);
102+
if (od?.type === 'object') return od.shape ?? uo.shape ?? null;
103+
}
104+
}
105+
return null;
92106
}
93107
function descOf(s: any): string {
94108
let cur = s;

packages/spec/src/kernel/metadata-type-schemas.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import { DatasourceSchema } from '../data/datasource.zod';
3333
import { SeedSchema } from '../data/seed.zod';
3434
import { MappingSchema } from '../data/mapping.zod';
3535

36-
import { ViewSchema } from '../ui/view.zod';
36+
import { ViewMetadataSchema } from '../ui/view.zod';
3737
import { PageSchema } from '../ui/page.zod';
3838
import { DashboardSchema } from '../ui/dashboard.zod';
3939
import { AppSchema } from '../ui/app.zod';
@@ -75,7 +75,11 @@ const BUILTIN_METADATA_TYPE_SCHEMAS: Partial<Record<MetadataType, z.ZodType>> =
7575
mapping: MappingSchema as unknown as z.ZodType, // #2611: reusable import mapping; runtime-creatable so the wizard can save one
7676

7777
// UI Protocol
78-
view: ViewSchema,
78+
// #3095 — a union over the three runtime `view` shapes (defineView container,
79+
// standalone ViewItem record, flattened personalization overlay). The bare
80+
// container `ViewSchema` strip-parsed ViewItem/personalization bodies to `{}`,
81+
// making save-time 422 validation and read-time diagnostics a no-op for them.
82+
view: ViewMetadataSchema,
7983
page: PageSchema,
8084
dashboard: DashboardSchema,
8185
app: AppSchema,
Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #3095 — `ViewMetadataSchema` is the canonical schema the `view` metadata type
5+
* registers (save-time 422 validation + read-time diagnostics). It MUST validate
6+
* all three runtime `view` shapes GENUINELY, where the bare container
7+
* {@link ViewSchema} was a no-op (Zod strip-parsed ViewItem/personalization
8+
* bodies to `{}`, so a broken `config` sailed through "valid").
9+
*
10+
* 1. defineView aggregate container (non-empty)
11+
* 2. standalone ViewItem record ({ name, object, viewKind, config })
12+
* 3. flattened personalization overlay (raw config + inherited identity, #2555)
13+
*/
14+
15+
import { describe, it, expect } from 'vitest';
16+
import { z } from 'zod';
17+
import { ViewMetadataSchema } from './view.zod';
18+
19+
const PLACEHOLDER_DATA = { provider: 'object', object: 'crm_lead' } as const;
20+
21+
describe('ViewMetadataSchema — genuine validation across the three runtime shapes (#3095)', () => {
22+
// ── shape 2: standalone ViewItem record ───────────────────────────────────
23+
describe('ViewItem record form', () => {
24+
it('accepts a well-formed list ViewItem', () => {
25+
const r = ViewMetadataSchema.safeParse({
26+
name: 'crm_lead.all',
27+
object: 'crm_lead',
28+
viewKind: 'list',
29+
label: 'All Leads',
30+
config: { type: 'grid', columns: ['name'], data: PLACEHOLDER_DATA },
31+
});
32+
expect(r.success).toBe(true);
33+
});
34+
35+
it('REJECTS a ViewItem whose kanban config is missing groupByField (was a no-op under ViewSchema)', () => {
36+
const r = ViewMetadataSchema.safeParse({
37+
name: 'crm_lead.pipeline',
38+
object: 'crm_lead',
39+
viewKind: 'list',
40+
config: {
41+
type: 'kanban',
42+
columns: ['name'],
43+
// groupByField is required by KanbanConfigSchema — omit it.
44+
kanban: { summarizeField: 'amount', columns: ['name'] },
45+
},
46+
});
47+
expect(r.success).toBe(false);
48+
});
49+
50+
it('accepts a well-formed form ViewItem', () => {
51+
const r = ViewMetadataSchema.safeParse({
52+
name: 'crm_lead.edit',
53+
object: 'crm_lead',
54+
viewKind: 'form',
55+
config: { type: 'simple', sections: [{ label: 'Details', fields: ['name'] }] },
56+
});
57+
expect(r.success).toBe(true);
58+
});
59+
60+
it('REJECTS a form ViewItem carrying an invalid form `type` (config validated, not stripped)', () => {
61+
const r = ViewMetadataSchema.safeParse({
62+
name: 'crm_lead.edit',
63+
object: 'crm_lead',
64+
viewKind: 'form',
65+
config: { type: 'not_a_real_form_type' },
66+
});
67+
expect(r.success).toBe(false);
68+
});
69+
});
70+
71+
// ── shape 1: defineView container ─────────────────────────────────────────
72+
describe('defineView container form', () => {
73+
it('accepts a non-empty container', () => {
74+
const r = ViewMetadataSchema.safeParse({
75+
list: { type: 'grid', data: PLACEHOLDER_DATA, columns: [{ field: 'name' }] },
76+
});
77+
expect(r.success).toBe(true);
78+
});
79+
80+
it('REJECTS a container whose nested list is missing required columns', () => {
81+
const r = ViewMetadataSchema.safeParse({
82+
list: { type: 'grid', data: PLACEHOLDER_DATA },
83+
});
84+
expect(r.success).toBe(false);
85+
});
86+
87+
it('REJECTS an explicitly-empty container (zero views — mirrors defineView)', () => {
88+
// A body that names container slots but fills none of them registers no
89+
// view; the container member's non-empty refine rejects it, and the
90+
// flattened members reject it via their container-key guards.
91+
expect(ViewMetadataSchema.safeParse({ listViews: {}, formViews: {} }).success).toBe(false);
92+
expect(ViewMetadataSchema.safeParse({ listViews: {} }).success).toBe(false);
93+
});
94+
95+
it('accepts a bare `{}` (legacy-compatible — the old ViewSchema also accepted it)', () => {
96+
// Not a regression: a truly empty body carries no viewKind/object, so
97+
// every consumer that filters on identity drops it. Pinned so the lenient
98+
// flattened-overlay branch behaviour is intentional, not accidental.
99+
expect(ViewMetadataSchema.safeParse({}).success).toBe(true);
100+
});
101+
});
102+
103+
// ── shape 3: flattened personalization overlay (#2555) ────────────────────
104+
describe('flattened personalization overlay form', () => {
105+
it('accepts a raw list config with identity inherited from the shadowed entry', () => {
106+
// The exact shape normalizeViewMetadata persists on a console column-sort PUT.
107+
const r = ViewMetadataSchema.safeParse({
108+
type: 'grid',
109+
data: { provider: 'object', object: 'showcase_task' },
110+
columns: ['title'],
111+
sort: [{ id: '29200fa8-c416-471e-9ca3-913f9308ad89', field: 'estimate_hours', order: 'desc' }],
112+
name: 'showcase_task.default',
113+
viewKind: 'list',
114+
object: 'showcase_task',
115+
label: 'All Tasks',
116+
});
117+
expect(r.success).toBe(true);
118+
});
119+
120+
it('accepts a raw list config with NO identity (adhoc PUT, no registry entry to inherit from)', () => {
121+
const r = ViewMetadataSchema.safeParse({
122+
type: 'grid',
123+
data: { provider: 'object', object: 'showcase_task' },
124+
columns: ['title'],
125+
sort: [{ field: 'estimate_hours', order: 'desc' }],
126+
name: 'adhoc.view',
127+
});
128+
expect(r.success).toBe(true);
129+
});
130+
131+
it('accepts a raw form config overlay', () => {
132+
const r = ViewMetadataSchema.safeParse({
133+
type: 'simple',
134+
sections: [{ label: 'Details', fields: ['name'] }],
135+
name: 'crm_lead.edit',
136+
viewKind: 'form',
137+
object: 'crm_lead',
138+
});
139+
expect(r.success).toBe(true);
140+
});
141+
142+
it('REJECTS a flattened list overlay whose kanban binding is broken (genuine, not stripped)', () => {
143+
const r = ViewMetadataSchema.safeParse({
144+
type: 'kanban',
145+
columns: ['name'],
146+
kanban: { summarizeField: 'amount', columns: ['name'] }, // no groupByField
147+
name: 'crm_lead.pipeline',
148+
viewKind: 'list',
149+
object: 'crm_lead',
150+
});
151+
expect(r.success).toBe(false);
152+
});
153+
154+
it('preserves auxiliary Studio round-trip keys without a strict-mode 422', () => {
155+
// isPinned/sortOrder ride along on the overlay; the schema validates but
156+
// must not reject unknown top-level aux keys (saveMetaItem persists verbatim).
157+
const r = ViewMetadataSchema.safeParse({
158+
type: 'grid',
159+
data: PLACEHOLDER_DATA,
160+
columns: ['name'],
161+
name: 'crm_lead.all',
162+
viewKind: 'list',
163+
object: 'crm_lead',
164+
isPinned: true,
165+
sortOrder: 3,
166+
});
167+
expect(r.success).toBe(true);
168+
});
169+
});
170+
171+
// ── mutual exclusion: a broken record/container is never rescued ──────────
172+
describe('member exclusivity', () => {
173+
it('does not rescue a broken record via the flattened branch (config guard)', () => {
174+
// A record body carries a nested `config`; the flattened members pin
175+
// `config` to undefined, so a broken config cannot be silently stripped.
176+
const r = ViewMetadataSchema.safeParse({
177+
name: 'crm_lead.pipeline',
178+
object: 'crm_lead',
179+
viewKind: 'list',
180+
config: { type: 'grid', columns: 'not-an-array' },
181+
});
182+
expect(r.success).toBe(false);
183+
});
184+
185+
it('does not rescue a broken container via the flattened branch (list guard)', () => {
186+
const r = ViewMetadataSchema.safeParse({
187+
list: { type: 'grid', data: PLACEHOLDER_DATA }, // missing columns
188+
name: 'crm_lead.default',
189+
});
190+
expect(r.success).toBe(false);
191+
});
192+
});
193+
194+
// ── JSON Schema emission (/api/v1/meta/types/view) ────────────────────────
195+
it('converts to a JSON Schema anyOf (union → anyOf) without throwing', () => {
196+
const json = z.toJSONSchema(ViewMetadataSchema, { unrepresentable: 'any' }) as Record<string, unknown>;
197+
expect(Array.isArray(json.anyOf)).toBe(true);
198+
expect((json.anyOf as unknown[]).length).toBe(4);
199+
});
200+
});

0 commit comments

Comments
 (0)