fix(metadata-protocol): draft preview no longer reports itself invalid over its own _draft badge

.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/<type>/<name>?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.

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<string, unknown>)
- ? stripDiagnostics(item as Record<string, unknown>)
- : 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<string, unknown>): Record<string, unknown> {
- 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

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);
+ });
+ });
+});