From 964a4b5f33deb7bf76cdda88b8087d512d87b0c9 Mon Sep 17 00:00:00 2001 From: Jack Date: Sun, 7 Jun 2026 20:05:10 +0800 Subject: [PATCH] fix(metadata-admin): client validation never stricter than the live server (spec-skew root-cure) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Studio editor validated drafts with the BUNDLED @objectstack/spec Zod schema, which can lag the running server. After a server-side schema relaxation (e.g. report.objectName/columns made optional for the ADR-0021 dataset dual- form), the editor showed false "required" errors on perfectly valid drafts. Root-cure: pass the live server schema (RichMetadataTypeEntry.schema, from /meta/types) into validateMetadataDraft and suppress "missing required field" issues for top-level fields the SERVER marks optional. Only suppresses when the field is actually absent (a present-but-invalid field still surfaces), so the client can never be stricter than the live server. Generic — any future schema relaxation is respected automatically, no per-change shim (cf. the existing FORWARD_COMPAT_FLOW_NODE_TYPES hack this generalises). Tests: clientValidation.skew (4) — suppresses stale required; never over- suppresses present-but-invalid; legacy strict behavior without a server schema. Co-Authored-By: Claude Opus 4.8 --- .../views/metadata-admin/ResourceEditPage.tsx | 6 +- .../clientValidation.skew.test.tsx | 64 +++++++++++++++++++ .../views/metadata-admin/clientValidation.ts | 29 +++++++++ 3 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 packages/app-shell/src/views/metadata-admin/clientValidation.skew.test.tsx diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx index c337ef9ad..d020e21bb 100644 --- a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx @@ -369,7 +369,9 @@ function MetadataResourceEditPageImpl({ if (!hasClientValidator(type)) return; let cancelled = false; const handle = window.setTimeout(() => { - void validateMetadataDraft(type, draft).then((res) => { + // Pass the live server schema so the client never flags fields the + // running server now treats as optional (cross-repo spec-skew root-cure). + void validateMetadataDraft(type, draft, entry?.schema as { required?: unknown } | undefined).then((res) => { if (cancelled) return; setIssues(res.issues); }); @@ -378,7 +380,7 @@ function MetadataResourceEditPageImpl({ cancelled = true; window.clearTimeout(handle); }; - }, [type, draft]); + }, [type, draft, entry?.schema]); // Per-item draft pending publish (mode=draft saves land here). // When non-null, the editor is "viewing the draft" and we surface // Publish / Discard-draft actions. diff --git a/packages/app-shell/src/views/metadata-admin/clientValidation.skew.test.tsx b/packages/app-shell/src/views/metadata-admin/clientValidation.skew.test.tsx new file mode 100644 index 000000000..8b569267a --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/clientValidation.skew.test.tsx @@ -0,0 +1,64 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Cross-repo spec-skew root-cure: the client (bundled @objectstack/spec) must + * never be STRICTER than the running server. validateMetadataDraft suppresses + * "missing required field" errors for fields the server schema marks optional. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Stub the bundled report schema to the STALE shape (objectName + columns +// required) so we can prove the suppression against a newer server schema. +vi.mock('@objectstack/spec/ui', () => ({ + ReportSchema: { + safeParse: (v: any) => { + const issues: Array<{ path: (string | number)[]; message: string }> = []; + if (v?.objectName === undefined) issues.push({ path: ['objectName'], message: 'Required' }); + if (v?.columns === undefined) issues.push({ path: ['columns'], message: 'Required' }); + // a present-but-invalid field error that must NEVER be suppressed + if (v?.label === '') issues.push({ path: ['label'], message: 'Label must not be empty' }); + return issues.length ? { success: false, error: { issues } } : { success: true }; + }, + }, +})); + +import { validateMetadataDraft } from './clientValidation'; + +beforeEach(() => vi.clearAllMocks()); + +// Server schema where objectName/columns are OPTIONAL (the dual-form, newer). +const serverSchema = { required: ['name', 'label'] }; + +describe('validateMetadataDraft — spec-skew suppression', () => { + it('suppresses stale "objectName/columns required" when the server marks them optional', async () => { + const draft = { name: 'rev', label: 'Revenue', dataset: 'sales', values: ['revenue'] }; + const res = await validateMetadataDraft('report', draft, serverSchema); + expect(res.ok).toBe(true); + expect(res.issues).toHaveLength(0); + }); + + it('still flags a present-but-invalid field (never over-suppresses)', async () => { + const draft = { name: 'rev', label: '', dataset: 'sales', values: ['revenue'] }; + const res = await validateMetadataDraft('report', draft, serverSchema); + expect(res.ok).toBe(false); + expect(res.issues.map((i) => i.path)).toContain('label'); + // objectName/columns are still suppressed (absent + server-optional) + expect(res.issues.map((i) => i.path)).not.toContain('objectName'); + }); + + it('without a server schema, keeps the legacy (strict bundled) behavior', async () => { + const draft = { name: 'rev', label: 'Revenue' }; // no objectName/columns + const res = await validateMetadataDraft('report', draft); + expect(res.ok).toBe(false); + expect(res.issues.map((i) => i.path)).toEqual(expect.arrayContaining(['objectName', 'columns'])); + }); + + it('does not suppress a required field that is still required by the server', async () => { + // server requires label; bundled flags objectName(optional-on-server) + ... ; + // here label is present so no label issue, objectName suppressed → ok. + const draft = { name: 'rev', label: 'Revenue', dataset: 'sales', values: ['revenue'] }; + const res = await validateMetadataDraft('report', draft, { required: ['name', 'label'] }); + expect(res.ok).toBe(true); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/clientValidation.ts b/packages/app-shell/src/views/metadata-admin/clientValidation.ts index 79e079759..3a8a103f2 100644 --- a/packages/app-shell/src/views/metadata-admin/clientValidation.ts +++ b/packages/app-shell/src/views/metadata-admin/clientValidation.ts @@ -160,6 +160,17 @@ export interface ValidateResult { export async function validateMetadataDraft( type: string, draft: unknown, + /** + * The live server JSON schema for this type (from `/meta/types`, i.e. + * `RichMetadataTypeEntry.schema`). When provided it ROOT-CURES cross-repo + * spec skew: the bundled `@objectstack/spec` may lag the running server, so + * we never let the client be STRICTER than the server — a "missing required + * field" flagged by the (possibly stale) bundled Zod is suppressed when the + * server marks that field optional. The server's own validation on save + * stays authoritative. This makes the editor track the live schema without a + * per-change shim (cf. `FORWARD_COMPAT_FLOW_NODE_TYPES`). + */ + serverSchema?: { required?: unknown }, ): Promise { const schema = await getSchemaForType(type); if (!schema) return { ok: true, issues: [] }; @@ -168,6 +179,24 @@ export async function validateMetadataDraft( if (result.success) return { ok: true, issues: [] }; let rawIssues = result.error?.issues ?? []; + + // Cross-repo skew root-cure — drop "missing required field" false positives + // for top-level fields the SERVER schema marks optional. Only suppresses when + // the field is actually absent in the draft (a present-but-invalid field + // still surfaces), so the client can never be stricter than the live server. + const serverRequired = Array.isArray(serverSchema?.required) + ? new Set((serverSchema!.required as unknown[]).map((x) => String(x))) + : undefined; + if (serverRequired && draft && typeof draft === 'object' && !Array.isArray(draft)) { + const d = draft as Record; + rawIssues = rawIssues.filter((i) => { + const path = i.path ?? []; + if (path.length !== 1) return true; // only top-level field issues + const field = String(path[0]); + const absent = d[field] === undefined || d[field] === null; + return !(absent && !serverRequired.has(field)); + }); + } // Forward-compat: don't let the published flow schema's closed node-type // enum reject node types the running server supports (see // FORWARD_COMPAT_FLOW_NODE_TYPES). Suppress only the `.type` enum mismatch