From 70db6d1c09363dc5c1d09f677f034d57cd731f13 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 18:15:50 +0000 Subject: [PATCH 1/2] fix(metadata-protocol): draft-preview diagnostics must not judge the injected _draft badge Fixes #7656 --- .../src/metadata-diagnostics.ts | 36 ++-- .../src/protocol.read-decorations.test.ts | 187 +++++++++++++++++- 2 files changed, 208 insertions(+), 15 deletions(-) diff --git a/packages/metadata-protocol/src/metadata-diagnostics.ts b/packages/metadata-protocol/src/metadata-diagnostics.ts index 15c814b24b..cb507c8098 100644 --- a/packages/metadata-protocol/src/metadata-diagnostics.ts +++ b/packages/metadata-protocol/src/metadata-diagnostics.ts @@ -24,7 +24,7 @@ */ import type { z } from 'zod'; -import { getMetadataTypeSchema } from '@objectstack/spec/kernel'; +import { getMetadataTypeSchema, stripReadDecorations } from '@objectstack/spec/kernel'; import type { MetadataValidationResult } from '@objectstack/spec/kernel'; import { PLURAL_TO_SINGULAR } from '@objectstack/spec/shared'; // [#5598] The READ path's share of the #5364 expansion. `zodIssuesToMetadataIssues` @@ -67,12 +67,28 @@ export function computeMetadataDiagnostics( }; } - // Strip our own decoration before re-validating so it never becomes - // a false-positive "unrecognized_keys" failure on schemas that grow - // a `.strict()` mode in the future. - const candidate = '_diagnostics' in (item as Record) - ? stripDiagnostics(item as Record) - : item; + // [#7656] Strip EVERY read decoration — the shared + // `METADATA_READ_DECORATIONS` list — before re-validating, not just the + // `_diagnostics` key this function stamps itself. + // + // This is a re-parse of a SERVED document in exactly the sense the module + // header of `spec/kernel/metadata-read-decorations.ts` means, so it is the + // third consumer of that list (after the write path's verbatim persist and + // the cold-boot flow bind) and must read it rather than keep a private + // one-key copy. The private copy predated `_draft` joining the list, and + // the schemas being closed since #4001 turned that gap into a verdict about + // the READER: `?preview=draft` stamps `_draft:true` on the item (both the + // single-item exit and the list overlay) and then decorates it, so the + // strict schema rejected our own badge BY NAME and every valid draft came + // back `valid:false / unrecognized_keys: ["_draft"]`. + // + // ⛔ The item schema is NOT the thing to loosen here: `_draft` is not a + // document key and must stay rejected when it appears in a stored body. It + // is the response's badge, which is precisely what the decoration list + // says. Same class as #6810 (`indexed`), different remedy — that key did + // not belong on the served body at all and left at its injection site, + // whereas this one is read by the UI and belongs on the response. + const candidate = stripReadDecorations(item); const parsed = (schema as z.ZodTypeAny).safeParse(candidate); if (parsed.success) { @@ -102,12 +118,6 @@ export function computeMetadataDiagnostics( return { valid: false, errors }; } -function stripDiagnostics(item: Record): Record { - const { _diagnostics: _drop, ...rest } = item; - void _drop; - return rest; -} - /** * Attach `_diagnostics` to a single metadata item. Returns the item * unchanged when no diagnostics could be computed (unknown type) or diff --git a/packages/metadata-protocol/src/protocol.read-decorations.test.ts b/packages/metadata-protocol/src/protocol.read-decorations.test.ts index 8756190fed..cf6c4fd614 100644 --- a/packages/metadata-protocol/src/protocol.read-decorations.test.ts +++ b/packages/metadata-protocol/src/protocol.read-decorations.test.ts @@ -31,8 +31,12 @@ import { describe, expect, it } from 'vitest'; // #5619 sank the two predicates into a package both sides already depend on. import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { FlowSchema } from '@objectstack/spec/automation'; -import { METADATA_READ_DECORATIONS } from '@objectstack/spec/kernel'; -import { ObjectStackProtocolImplementation, stripReadDecorations } from './index.js'; +import { METADATA_READ_DECORATIONS, getMetadataTypeSchema } from '@objectstack/spec/kernel'; +import { + ObjectStackProtocolImplementation, + computeMetadataDiagnostics, + stripReadDecorations, +} from './index.js'; interface Row { id: string; @@ -326,3 +330,182 @@ describe('a served document survives its own (closed) schema — cloud#971', () ).toEqual([]); }); }); + +/** + * #7656 — the read must not judge its own badge. + * + * The THIRD consumer of the same invariant, and the one where the served + * document never leaves the response: `decorateMetadataItem` re-parses the item + * to compute `_diagnostics`, which is a re-parse of a served document in exactly + * the sense the module header of `spec/kernel/metadata-read-decorations.ts` + * means. It stripped `_diagnostics` (its own key, by hand) and nothing else, so + * a `?preview=draft` read — which stamps `_draft:true` BEFORE decorating, on + * both exits — validated the badge it had just added against a closed schema + * and answered `_diagnostics.valid:false / unrecognized_keys: ["_draft"]` for a + * perfectly valid draft. The verdict was about the reader, not the document. + * + * Same class as #6810 (`indexed`, rejected by name on a served object) but not + * the same fix: `indexed` did not belong on the served body at all and was + * removed at the injection site, whereas `_draft` is the preview badge the UI + * reads — it belongs on the RESPONSE and is already a declared member of + * `METADATA_READ_DECORATIONS`. So this closes where the list is consumed, not + * where the badge is stamped. + * + * The anti-vacuity cases are the point of this block: "no `_draft` complaint" + * is also satisfied by a read that stopped computing diagnostics at all, which + * would be a strictly worse regression wearing a green test. Each side pairs a + * valid draft (must be `valid:true`) with a genuinely broken one (must still be + * `valid:false`, naming its OWN defect). + */ +describe('draft preview diagnostics do not judge the injected `_draft` badge (#7656)', () => { + /** The symptom, verbatim from the card: a complaint naming `_draft`. */ + const draftKeyComplaints = (diagnostics: any): string[] => + (diagnostics?.errors ?? []) + .map((e: { message?: string }) => String(e?.message ?? '')) + .filter((m: string) => m.includes('_draft')); + + /** + * Seed a stored draft row directly. The save path refuses an invalid body + * with 422, so a genuinely-broken draft cannot be authored through + * `saveMetaItem` — which is the whole reason read-time diagnostics exist: + * they badge rows that are already in the table (authored before a schema + * tightened, or written by the ADR-0033 AI apply loop). + */ + const seedDraft = async (engine: any, name: string, body: unknown) => { + await engine.insert('sys_metadata', { + type: 'object', + name, + organization_id: null, + package_id: null, + state: 'draft', + metadata: JSON.stringify(body), + }); + }; + + /** Valid except for one deliberately-planted defect: `type` is not a field type. */ + const brokenBody = (name: string) => ({ + name, + label: 'Broken', + fields: { amount: { type: 'not_a_real_field_type', label: 'Amount' } }, + }); + + describe('single-item read (`getMetaItem`, previewDrafts)', () => { + it('a valid draft reads back `_diagnostics.valid:true`', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await protocol.saveMetaItem({ + type: 'object', name: 'crm_quote', item: objectBody('crm_quote'), mode: 'draft', + }); + + const served: any = (await protocol.getMetaItem({ + type: 'object', name: 'crm_quote', previewDrafts: true, + })).item; + + expect(served._draft, 'precondition — the preview read badges').toBe(true); + expect(draftKeyComplaints(served._diagnostics)).toEqual([]); + expect( + served._diagnostics.valid, + `draft preview reported invalid: ${JSON.stringify(served._diagnostics?.errors)}`, + ).toBe(true); + }); + + it('a genuinely broken draft still reports its OWN error (anti-vacuity)', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await seedDraft(engine, 'crm_broken', brokenBody('crm_broken')); + + const served: any = (await protocol.getMetaItem({ + type: 'object', name: 'crm_broken', previewDrafts: true, + })).item; + + expect(served._draft, 'precondition — the preview read badges').toBe(true); + // Still computed, still false — the fix must not silence the path. + expect(served._diagnostics.valid).toBe(false); + expect(served._diagnostics.errors?.length).toBeGreaterThan(0); + // …and false for the DOCUMENT's reason, not for the reader's badge. + expect(draftKeyComplaints(served._diagnostics)).toEqual([]); + expect( + JSON.stringify(served._diagnostics.errors), + 'the real defect must still be named', + ).toContain('amount'); + }); + }); + + describe('list overlay (`getMetaItems`, previewDrafts)', () => { + /** The overlaid draft entry for `name`, as the Studio list receives it. */ + const listed = async (protocol: any, name: string) => { + const res: any = await protocol.getMetaItems({ type: 'object', previewDrafts: true }); + const items: any[] = Array.isArray(res) ? res : (res?.items ?? []); + const served = items.find((i) => i?.name === name); + expect(served, `getMetaItems('object') overlaid the draft ${name}`).toBeDefined(); + return served; + }; + + it('a valid draft reads back `_diagnostics.valid:true`', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await protocol.saveMetaItem({ + type: 'object', name: 'crm_quote', item: objectBody('crm_quote'), mode: 'draft', + }); + + const served = await listed(protocol, 'crm_quote'); + expect(served._draft, 'precondition — the overlay badges').toBe(true); + expect(draftKeyComplaints(served._diagnostics)).toEqual([]); + expect( + served._diagnostics.valid, + `draft overlay reported invalid: ${JSON.stringify(served._diagnostics?.errors)}`, + ).toBe(true); + }); + + it('a genuinely broken draft still reports its OWN error (anti-vacuity)', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await seedDraft(engine, 'crm_broken', brokenBody('crm_broken')); + + const served = await listed(protocol, 'crm_broken'); + expect(served._draft, 'precondition — the overlay badges').toBe(true); + expect(served._diagnostics.valid).toBe(false); + expect(served._diagnostics.errors?.length).toBeGreaterThan(0); + expect(draftKeyComplaints(served._diagnostics)).toEqual([]); + expect( + JSON.stringify(served._diagnostics.errors), + 'the real defect must still be named', + ).toContain('amount'); + }); + }); + + describe('the verdict is computed from the list, and the schema stays closed', () => { + it('every declared read decoration is invisible to the verdict (drift guard)', () => { + // The mirror of the cloud#971 drift guard above, one layer down: a + // FOURTH decoration added to `METADATA_READ_DECORATIONS` must not + // have to remember this consumer. It fails here, on a unit, instead + // of as `valid:false` on somebody's badge. + const body = objectBody('crm_invoice'); + expect(computeMetadataDiagnostics('object', body)?.valid).toBe(true); + + for (const key of METADATA_READ_DECORATIONS) { + const verdict = computeMetadataDiagnostics('object', { ...body, [key]: true }); + expect( + verdict?.valid, + `read decoration \`${key}\` leaked into the verdict: ` + + `${JSON.stringify(verdict?.errors)}`, + ).toBe(true); + } + }); + + it('the object schema itself still rejects `_draft` — only the strip moved', () => { + // ⛔ The remedy is NOT a looser item schema. `_draft` is a response + // badge; a STORED body carrying it is a polluted row and must keep + // failing by name, which is what makes the #4326 write-path strip + // load-bearing rather than cosmetic. + const schema = getMetadataTypeSchema('object'); + expect(schema, 'precondition — `object` has a registered schema').toBeDefined(); + + const parsed = (schema as any).safeParse({ ...objectBody('crm_invoice'), _draft: true }); + expect(parsed.success, 'the closed schema must still reject the badge').toBe(false); + expect( + parsed.error.issues.some((i: { code: string }) => i.code === 'unrecognized_keys'), + ).toBe(true); + }); + }); +}); From abfbf4f2d0e278a0007be1d0915b72a5fbefe5f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 18:42:26 +0000 Subject: [PATCH 2/2] chore: changeset for #7656 --- .../draft-preview-diagnostics-draft-badge.md | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .changeset/draft-preview-diagnostics-draft-badge.md diff --git a/.changeset/draft-preview-diagnostics-draft-badge.md b/.changeset/draft-preview-diagnostics-draft-badge.md new file mode 100644 index 0000000000..1774574bce --- /dev/null +++ b/.changeset/draft-preview-diagnostics-draft-badge.md @@ -0,0 +1,27 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): a draft preview no longer reports itself invalid because of its own `_draft` badge (#7656) + +`GET /api/v1/meta//?preview=draft` answered with `_diagnostics.valid: +false` and *"Unrecognized key(s) on this object: `_draft`"* for drafts that were +perfectly valid — the read stamped `_draft:true` onto the item so the console +could badge it, then validated the item **with that key still on it** against a +closed schema. The verdict was about the reader, not the document, and it reached +both exits: the single-item preview read and the draft overlay in the list. + +`computeMetadataDiagnostics` now removes every key on the shared +`METADATA_READ_DECORATIONS` list before its re-parse, instead of the private +one-key copy it carried (which removed `_diagnostics` only, and predated `_draft` +joining that list). That list exists precisely so the read path's own annotations +cannot be mistaken for document content by anything that re-parses a served +document — the write path's verbatim persist (#4326) and the cold-boot flow bind +(cloud#971) are the other two consumers; read-time diagnostics are the third. + +The item schema is **unchanged and still closed**: `_draft` remains rejected by +name when it appears in a stored body, which is what keeps the write-path strip +load-bearing. Only the reader stopped feeding its own badge to it. + +Genuinely invalid drafts are unaffected — they still read back `valid:false` with +their own errors, on both exits.