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

* fix(metadata-protocol): draft-preview diagnostics must not judge the injected _draft badge

Fixes #7656

* chore: changeset for #7656

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: huangyiirene

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