Skip to content

Commit 320ec35

Browse files
fix(plugin-gantt, app-shell): refuse an invalid gantt block; designer spec derivations fail loudly (#13490)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} --> _Requested by **Elon** · [project thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DHcWWbiJqPPiDYgiSMxhTQo)_ Fixes #13459 Clause-②: no Metadata-protocol: narrowed - [x] Adds no product-specific widget, dialog, hook or string (see AGENTS.md › Product-neutral renderer) ## What changes **1. `ObjectGantt` refuses a `gantt` block that fails `GanttConfigSchema`** (`packages/plugin-gantt/src/ObjectGantt.tsx`). Before: `safeParse` failure → `console.warn` → the unvalidated block was returned and drawn, so a misspelt or missing key still rendered a plausible chart. Now `getGanttConfig` returns `{ config: null, invalidBlock }` on that branch and the component renders a refusal (`role="alert"`, `data-testid="gantt-invalid-config"`) naming every parse issue by its path in the block, e.g. `gantt.endDateField: …`, `gantt: Unrecognized key: "startField"`. The console warning and the shadowed-flat-keys warning still fire. Unchanged: a valid block, the flat ObjectView / ListView spelling (still not parsed, as before), and the "Gantt configuration required" screen for a view with no binding. **2. The five designer spec derivations fail loudly** (`packages/app-shell/src/views/metadata-admin/{dashboard,package,page,report,view}-schema.ts`). The ten `try { … } catch { console.warn(…) }` sites (seven `z.toJSONSchema` derivations, three form clones) and their `_…Failed` memo flags are removed. Their only input is the bundled `@objectstack/spec`, so a failed derivation is a build-time fact: it fails the tests beside these files, and at runtime it propagates to the route's `ErrorBoundary` (`console/AppContent.tsx`), which shows the error and reports it, instead of rendering a form silently missing fields. The getters' return types narrow from `JsonSchema | undefined` to `JsonSchema`. No runtime input reached those catches: the dashboard form's only runtime argument, `locale`, goes through `localizeMetadataForm`, which does not throw on an unknown locale. `tzShift.ts` (`timeZone` fallback) is out of scope per the claim (it waits on a spec-row decision). ## Tests On head `e26d1cd`: - New `packages/plugin-gantt/src/ObjectGantt.invalidBlockRefusal-13459.test.tsx` (4): an undeclared key and a missing required field are each refused with the path named and no chart; the console diagnostic stays; CONTROL: a valid block renders its tasks. - New `packages/app-shell/src/views/metadata-admin/spec-derivation-13459.test.ts` (14): each of the seven getters derives an object schema from the bundled spec, and each THROWS (and does not warn) when `z.toJSONSchema` throws. The failing `toJSONSchema` is a scoped `vi.doMock` + `doUnmock`: a first version used a hoisted `vi.mock`, which leaked into `view-schema.test.ts` in the same worker (3 failures, fixed in `a3b60c1`). - Ablation (each leg committed first, restored with `git checkout HEAD --`, `git diff HEAD` empty after): `ObjectGantt.tsx` reverted to `origin/main` → the three refusal cases fail, CONTROL passes; `view-schema.ts` reverted → the three `view-schema.*` throw cases fail, the rest pass. - `pnpm exec vitest run packages/plugin-gantt/` → Test Files 100 passed | 1 skipped (101), Tests 775 passed | 114 skipped. - App-shell, narrowed: the 88 test files that import a changed module or a component calling one (`git grep -lE` over the five module names and their callers `PackageFormDialog`, `*DefaultInspector`, `ViewColumnInspector`, `ViewVariantInspector`, `ReportConfigPanel`) → Test Files 88 passed (88), Tests 721 passed. Blind spot: a test reaching these modules through another import path is not in that set; CI's sharded `pnpm test` runs the whole package. - `type-check`: `@object-ui/plugin-gantt` exit 0, `@object-ui/app-shell` exit 0 (turbo, both `tsc --noEmit` and `tsconfig.test.json`). - Gates, local: `eslint --no-inline-config` on the touched files 0 errors; `check:new-line-citations`, `check:changeset-claims`, `check-changeset-no-major`, `check-changeset-presence`, `check:i18n-keys`, `check:vi-mock-specifiers`, `check:vi-mock-inherit`, `check:vi-mock-override-shape`, `check:control-bytes`, `check:test-path-roots` all exit 0. `check:readme-exports` NOT MEASURED (needs every package's dist; CI builds first). ## Acceptance notes - The refusal copy is English, like the neighbouring "Gantt configuration required" refusal: both address the view's author. Localizing both would touch the ten locale packs, outside the claimed file surface. - `ObjectTimeline` still only warns on a failed `TimelineExtensionSchema.safeParse` (its refusal covers a missing date axis only). Same shape as this card's gantt half; not changed here (outside the claimed surface). - The `if (!specXForm || typeof specXForm !== 'object') return undefined` guards in the form getters remain: they are not catch-and-warn sites and the existing tests assert the forms are defined. --- _Generated by [Claude Code](https://claude.ai/code/session_01AdaeVx4N1y9QsiBkJ31nfJ)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01AdaeVx4N1y9QsiBkJ31nfJ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 84ce59c commit 320ec35

9 files changed

Lines changed: 357 additions & 175 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@object-ui/plugin-gantt": patch
3+
"@object-ui/app-shell": patch
4+
---
5+
6+
Clause-②: no
7+
Metadata-protocol: narrowed
8+
9+
An invalid `gantt` block is refused instead of drawn (objectui#13459). `ObjectGantt` used to log `Invalid gantt configuration` and render the block anyway, so a block with a misspelt or missing key still drew a chart. It now renders a refusal that names each `GanttConfigSchema` issue by its path in the block (`gantt.endDateField: …`); a valid block and the flat ObjectView / ListView spelling render as before.
10+
11+
The Studio designers' spec derivations (`dashboard`, `package`, `page`, `report` and `view` schemas in `metadata-admin`) no longer catch a failed derivation and return `undefined`, which rendered a designer form silently missing fields. Their only input is the bundled spec, so a failure is a build-time fact that fails their tests; at runtime it reaches the route's error boundary.

‎packages/app-shell/src/views/metadata-admin/dashboard-schema.ts‎

Lines changed: 29 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,19 @@ const FORM_FIELDS_OWNED_ELSEWHERE = new Set([
3939
'name', // record identity — not user-editable here
4040
]);
4141

42+
/**
43+
* No `try`/`catch` around the spec derivations in this file (objectui#13459).
44+
* Their only input is the bundled `@objectstack/spec`, so a derivation that
45+
* fails is a build-time fact: the tests beside this file go red, and at runtime
46+
* the throw reaches the route's error boundary. It used to be caught, warned
47+
* about in the console and turned into `undefined`, which rendered a designer
48+
* form silently missing fields that the author could then save past.
49+
*/
4250
let _dashboardDocSchema: JsonSchema | undefined;
43-
let _dashboardDocFailed = false;
4451

4552
/** JSONSchema for the whole (flat) Dashboard document. */
46-
export function getDashboardSchema(): JsonSchema | undefined {
47-
if (_dashboardDocSchema || _dashboardDocFailed) return _dashboardDocSchema;
48-
try {
49-
_dashboardDocSchema = z.toJSONSchema(DashboardSchema, TO_JSON_OPTS) as JsonSchema;
50-
} catch (err) {
51-
_dashboardDocFailed = true;
52-
if (typeof console !== 'undefined') {
53-
console.warn('[dashboard-schema] failed to derive Dashboard JSONSchema from spec', err);
54-
}
55-
}
53+
export function getDashboardSchema(): JsonSchema {
54+
_dashboardDocSchema ??= z.toJSONSchema(DashboardSchema, TO_JSON_OPTS) as JsonSchema;
5655
return _dashboardDocSchema;
5756
}
5857

@@ -79,31 +78,24 @@ export function getDashboardForm(locale?: SupportedLocale | string): FormViewSpe
7978
const cacheKey = String(locale ?? '');
8079
if (_dashboardForm.has(cacheKey)) return _dashboardForm.get(cacheKey);
8180
if (!specDashboardForm || typeof specDashboardForm !== 'object') return undefined;
82-
try {
83-
const clone = JSON.parse(JSON.stringify(specDashboardForm)) as FormViewSpec;
84-
for (const section of clone.sections ?? []) {
85-
section.fields = (section.fields ?? []).filter((f: any) => {
86-
const name = typeof f === 'string' ? f : f?.field;
87-
return !FORM_FIELDS_OWNED_ELSEWHERE.has(name);
88-
});
89-
}
90-
// Drop now-empty sections so we don't render bare headers.
91-
clone.sections = (clone.sections ?? []).filter(
92-
(s: any) => (s.fields ?? []).length > 0,
93-
);
94-
// Localize AFTER pruning: the overlay only has to reach what renders, and
95-
// pruning cannot drop a section the overlay just renamed.
96-
const localized = localizeMetadataForm(
97-
clone as unknown as Record<string, unknown>,
98-
'dashboard',
99-
locale,
100-
) as unknown as FormViewSpec;
101-
_dashboardForm.set(cacheKey, localized);
102-
return localized;
103-
} catch (err) {
104-
if (typeof console !== 'undefined') {
105-
console.warn('[dashboard-schema] failed to prepare dashboardForm from spec', err);
106-
}
107-
return undefined;
81+
const clone = JSON.parse(JSON.stringify(specDashboardForm)) as FormViewSpec;
82+
for (const section of clone.sections ?? []) {
83+
section.fields = (section.fields ?? []).filter((f: any) => {
84+
const name = typeof f === 'string' ? f : f?.field;
85+
return !FORM_FIELDS_OWNED_ELSEWHERE.has(name);
86+
});
10887
}
88+
// Drop now-empty sections so we don't render bare headers.
89+
clone.sections = (clone.sections ?? []).filter(
90+
(s: any) => (s.fields ?? []).length > 0,
91+
);
92+
// Localize AFTER pruning: the overlay only has to reach what renders, and
93+
// pruning cannot drop a section the overlay just renamed.
94+
const localized = localizeMetadataForm(
95+
clone as unknown as Record<string, unknown>,
96+
'dashboard',
97+
locale,
98+
) as unknown as FormViewSpec;
99+
_dashboardForm.set(cacheKey, localized);
100+
return localized;
109101
}

‎packages/app-shell/src/views/metadata-admin/package-schema.ts‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,19 @@ export const PACKAGE_FORM_FIELDS = [
5050
'dependencies',
5151
] as const;
5252

53+
/**
54+
* No `try`/`catch` around the spec derivations in this file (objectui#13459).
55+
* Their only input is the bundled `@objectstack/spec`, so a derivation that
56+
* fails is a build-time fact: the tests beside this file go red, and at runtime
57+
* the throw reaches the route's error boundary. It used to be caught, warned
58+
* about in the console and turned into `undefined`, which rendered a designer
59+
* form silently missing fields that the author could then save past.
60+
*/
5361
let _schema: JsonSchema | undefined;
54-
let _schemaFailed = false;
5562

5663
/** JSONSchema for the whole package manifest (memoised, spec-derived). */
57-
export function getPackageSchema(): JsonSchema | undefined {
58-
if (_schema || _schemaFailed) return _schema;
59-
try {
60-
_schema = z.toJSONSchema(ManifestSchema, TO_JSON_OPTS) as JsonSchema;
61-
} catch (err) {
62-
_schemaFailed = true;
63-
if (typeof console !== 'undefined') {
64-
console.warn('[package-schema] failed to derive manifest JSONSchema from spec', err);
65-
}
66-
}
64+
export function getPackageSchema(): JsonSchema {
65+
_schema ??= z.toJSONSchema(ManifestSchema, TO_JSON_OPTS) as JsonSchema;
6766
return _schema;
6867
}
6968

‎packages/app-shell/src/views/metadata-admin/page-schema.ts‎

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,19 @@ const TO_JSON_OPTS = { io: 'input', unrepresentable: 'any' } as const;
3939
*/
4040
export const PAGE_FIELDS_OWNED_ELSEWHERE = new Set(['name', 'regions', 'children']);
4141

42+
/**
43+
* No `try`/`catch` around the spec derivations in this file (objectui#13459).
44+
* Their only input is the bundled `@objectstack/spec`, so a derivation that
45+
* fails is a build-time fact: the tests beside this file go red, and at runtime
46+
* the throw reaches the route's error boundary. It used to be caught, warned
47+
* about in the console and turned into `undefined`, which rendered a designer
48+
* form silently missing fields that the author could then save past.
49+
*/
4250
let _pageSchema: JsonSchema | undefined;
43-
let _pageSchemaFailed = false;
4451

4552
/** JSONSchema for the whole Page document (memoised, spec-derived). */
46-
export function getPageSchema(): JsonSchema | undefined {
47-
if (_pageSchema || _pageSchemaFailed) return _pageSchema;
48-
try {
49-
_pageSchema = z.toJSONSchema(PageSchema, TO_JSON_OPTS) as JsonSchema;
50-
} catch (err) {
51-
_pageSchemaFailed = true;
52-
if (typeof console !== 'undefined') {
53-
console.warn('[page-schema] failed to derive Page JSONSchema from spec', err);
54-
}
55-
}
53+
export function getPageSchema(): JsonSchema {
54+
_pageSchema ??= z.toJSONSchema(PageSchema, TO_JSON_OPTS) as JsonSchema;
5655
return _pageSchema;
5756
}
5857

@@ -67,21 +66,14 @@ let _pageForm: FormViewSpec | undefined;
6766
export function getPageForm(): FormViewSpec | undefined {
6867
if (_pageForm) return _pageForm;
6968
if (!specPageForm || typeof specPageForm !== 'object') return undefined;
70-
try {
71-
const clone = JSON.parse(JSON.stringify(specPageForm)) as FormViewSpec;
72-
for (const section of clone.sections ?? []) {
73-
section.fields = (section.fields ?? []).filter((f: any) => {
74-
const name = typeof f === 'string' ? f : f?.field;
75-
return !PAGE_FIELDS_OWNED_ELSEWHERE.has(name);
76-
});
77-
}
78-
clone.sections = (clone.sections ?? []).filter((s) => (s.fields ?? []).length > 0);
79-
_pageForm = clone;
80-
} catch (err) {
81-
if (typeof console !== 'undefined') {
82-
console.warn('[page-schema] failed to prune Page authoring form from spec', err);
83-
}
84-
_pageForm = specPageForm as FormViewSpec;
69+
const clone = JSON.parse(JSON.stringify(specPageForm)) as FormViewSpec;
70+
for (const section of clone.sections ?? []) {
71+
section.fields = (section.fields ?? []).filter((f: any) => {
72+
const name = typeof f === 'string' ? f : f?.field;
73+
return !PAGE_FIELDS_OWNED_ELSEWHERE.has(name);
74+
});
8575
}
76+
clone.sections = (clone.sections ?? []).filter((s) => (s.fields ?? []).length > 0);
77+
_pageForm = clone;
8678
return _pageForm;
8779
}

‎packages/app-shell/src/views/metadata-admin/report-schema.ts‎

Lines changed: 42 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,19 @@ const FORM_FIELDS_OWNED_ELSEWHERE = new Set([
3838
'name', // record identity — not user-editable here
3939
]);
4040

41+
/**
42+
* No `try`/`catch` around the spec derivations in this file (objectui#13459).
43+
* Their only input is the bundled `@objectstack/spec`, so a derivation that
44+
* fails is a build-time fact: the tests beside this file go red, and at runtime
45+
* the throw reaches the route's error boundary. It used to be caught, warned
46+
* about in the console and turned into `undefined`, which rendered a designer
47+
* form silently missing fields that the author could then save past.
48+
*/
4149
let _reportDocSchema: JsonSchema | undefined;
42-
let _reportDocFailed = false;
4350

4451
/** JSONSchema for the whole (flat) Report document. */
45-
export function getReportSchema(): JsonSchema | undefined {
46-
if (_reportDocSchema || _reportDocFailed) return _reportDocSchema;
47-
try {
48-
_reportDocSchema = z.toJSONSchema(ReportSchema, TO_JSON_OPTS) as JsonSchema;
49-
} catch (err) {
50-
_reportDocFailed = true;
51-
if (typeof console !== 'undefined') {
52-
console.warn('[report-schema] failed to derive Report JSONSchema from spec', err);
53-
}
54-
}
52+
export function getReportSchema(): JsonSchema {
53+
_reportDocSchema ??= z.toJSONSchema(ReportSchema, TO_JSON_OPTS) as JsonSchema;
5554
return _reportDocSchema;
5655
}
5756

@@ -80,48 +79,41 @@ let _reportForm: FormViewSpec | undefined;
8079
export function getReportForm(): FormViewSpec | undefined {
8180
if (_reportForm) return _reportForm;
8281
if (!specReportForm || typeof specReportForm !== 'object') return undefined;
83-
try {
84-
const clone = JSON.parse(JSON.stringify(specReportForm)) as FormViewSpec;
85-
const schemaProps = getReportSchema()?.properties as Record<string, unknown> | undefined;
86-
for (const section of clone.sections ?? []) {
87-
section.fields = (section.fields ?? []).filter((f: any) => {
88-
const name = typeof f === 'string' ? f : f?.field;
89-
if (FORM_FIELDS_OWNED_ELSEWHERE.has(name)) return false;
90-
// Schema-subset pass: drop form fields the schema no longer carries.
91-
if (schemaProps && !(name in schemaProps)) return false;
92-
return true;
93-
});
94-
}
95-
if (schemaProps && 'runtimeFilter' in schemaProps) {
96-
const declared = new Set<string>();
97-
for (const s of clone.sections ?? []) {
98-
for (const f of s.fields ?? []) {
99-
declared.add(typeof f === 'string' ? f : (f as any)?.field);
100-
}
101-
}
102-
if (!declared.has('runtimeFilter')) {
103-
clone.sections = [
104-
...(clone.sections ?? []),
105-
{
106-
label: 'Filter',
107-
description: 'Render-time scope filter, ANDed at query time.',
108-
collapsible: true,
109-
collapsed: true,
110-
fields: [{ field: 'runtimeFilter', widget: 'json' }],
111-
},
112-
];
82+
const clone = JSON.parse(JSON.stringify(specReportForm)) as FormViewSpec;
83+
const schemaProps = getReportSchema().properties as Record<string, unknown> | undefined;
84+
for (const section of clone.sections ?? []) {
85+
section.fields = (section.fields ?? []).filter((f: any) => {
86+
const name = typeof f === 'string' ? f : f?.field;
87+
if (FORM_FIELDS_OWNED_ELSEWHERE.has(name)) return false;
88+
// Schema-subset pass: drop form fields the schema no longer carries.
89+
if (schemaProps && !(name in schemaProps)) return false;
90+
return true;
91+
});
92+
}
93+
if (schemaProps && 'runtimeFilter' in schemaProps) {
94+
const declared = new Set<string>();
95+
for (const s of clone.sections ?? []) {
96+
for (const f of s.fields ?? []) {
97+
declared.add(typeof f === 'string' ? f : (f as any)?.field);
11398
}
11499
}
115-
// Drop now-empty sections so we don't render bare headers.
116-
clone.sections = (clone.sections ?? []).filter(
117-
(s: any) => (s.fields ?? []).length > 0,
118-
);
119-
_reportForm = clone;
120-
} catch (err) {
121-
if (typeof console !== 'undefined') {
122-
console.warn('[report-schema] failed to prepare reportForm from spec', err);
100+
if (!declared.has('runtimeFilter')) {
101+
clone.sections = [
102+
...(clone.sections ?? []),
103+
{
104+
label: 'Filter',
105+
description: 'Render-time scope filter, ANDed at query time.',
106+
collapsible: true,
107+
collapsed: true,
108+
fields: [{ field: 'runtimeFilter', widget: 'json' }],
109+
},
110+
];
123111
}
124-
return undefined;
125112
}
113+
// Drop now-empty sections so we don't render bare headers.
114+
clone.sections = (clone.sections ?? []).filter(
115+
(s: any) => (s.fields ?? []).length > 0,
116+
);
117+
_reportForm = clone;
126118
return _reportForm;
127119
}
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectui#13459 — the designer forms' spec derivations fail LOUDLY.
5+
*
6+
* The five `*-schema.ts` modules derive their JSONSchema from the bundled
7+
* `@objectstack/spec`. That input is fixed at build time, so a derivation that
8+
* fails is a build-time fact: this file goes red for it. At runtime the throw
9+
* reaches the route's error boundary instead of being caught, warned about and
10+
* turned into `undefined` — which used to render a designer form silently
11+
* missing fields.
12+
*
13+
* Two halves: every derivation succeeds against the bundled spec, and a
14+
* derivation that throws reaches the caller instead of becoming `undefined`.
15+
*/
16+
17+
import { describe, it, expect, vi, afterEach } from 'vitest';
18+
19+
const DERIVATIONS = [
20+
['dashboard-schema', 'getDashboardSchema'],
21+
['package-schema', 'getPackageSchema'],
22+
['page-schema', 'getPageSchema'],
23+
['report-schema', 'getReportSchema'],
24+
['view-schema', 'getListVariantSchema'],
25+
['view-schema', 'getFormVariantSchema'],
26+
['view-schema', 'getListColumnSchema'],
27+
] as const;
28+
29+
const LOADERS: Record<(typeof DERIVATIONS)[number][0], () => Promise<unknown>> = {
30+
'dashboard-schema': () => import('./dashboard-schema'),
31+
'package-schema': () => import('./package-schema'),
32+
'page-schema': () => import('./page-schema'),
33+
'report-schema': () => import('./report-schema'),
34+
'view-schema': () => import('./view-schema'),
35+
};
36+
37+
/** A fresh module instance per case, so a memoised result cannot answer. */
38+
async function derive(file: (typeof DERIVATIONS)[number][0], getter: string): Promise<unknown> {
39+
vi.resetModules();
40+
const mod = (await LOADERS[file]()) as Record<string, () => unknown>;
41+
return mod[getter]();
42+
}
43+
44+
// `vi.doMock`, not a hoisted `vi.mock`: the failing `toJSONSchema` is scoped to
45+
// the cases that ask for it, and undone after each, so it cannot reach another
46+
// test file sharing this worker's module registry.
47+
afterEach(() => {
48+
vi.doUnmock('zod');
49+
vi.resetModules();
50+
});
51+
52+
describe('designer spec derivations (objectui#13459)', () => {
53+
it.each(DERIVATIONS)('%s.%s derives an object schema from the bundled spec', async (file, getter) => {
54+
const schema = (await derive(file, getter)) as { type?: unknown; properties?: unknown } | undefined;
55+
expect(schema).toBeDefined();
56+
expect(schema?.type).toBe('object');
57+
expect(schema?.properties).toBeTypeOf('object');
58+
});
59+
60+
it.each(DERIVATIONS)('%s.%s throws when the derivation fails, never returns undefined', async (file, getter) => {
61+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
62+
vi.doMock('zod', async (importOriginal) => {
63+
const actual = await importOriginal<typeof import('zod')>();
64+
const toJSONSchema = (): never => {
65+
throw new Error('derivation failed (test)');
66+
};
67+
return { ...actual, z: { ...actual.z, toJSONSchema } };
68+
});
69+
await expect(derive(file, getter)).rejects.toThrow('derivation failed (test)');
70+
expect(warn).not.toHaveBeenCalled();
71+
warn.mockRestore();
72+
});
73+
});

0 commit comments

Comments
 (0)