spec: view union identity precondition (#5599)

.changeset/view-union-identity-precondition.md

@@ -0,0 +1,96 @@
+---
+"@objectstack/spec": major
+---
+
+feat(spec)!: a `view` body must be a view before the union judges it (#5599)
+
+`ViewMetadataSchema` — the schema the `view` metadata type registers, and so the
+one both `saveMetaItem`'s 422 gate and the read-time `_diagnostics` badge consult
+— accepted **any object at all**. Measured on `origin/main`:
+
+```
+getMetadataTypeSchema('view').safeParse({ nope: 1 })  ->  success, data = { type: 'simple' }
+getMetadataTypeSchema('view').safeParse({})           ->  success, data = { type: 'simple' }
+
+saveMetaItem({ type: 'view', name: 'garbage_view', item: { nope: 1 } })
+  ->  { success: true, state: 'active', seq: 1 }
+      persisted body = {"nope":1,"name":"garbage_view"}
+```
+
+The union's fourth member (`FormViewSchema.extend(…).strip()`) both strips
+unknown keys **and** declares no required key — `type` even carries a `'simple'`
+default — so it matched every object and handed the whole union a wildcard. The
+`.strip()` is deliberate and load-bearing (#5074: it is what carries Studio's
+round-trip keys); the defect is that an arm which strips *and* requires nothing
+is a universal match. So `view` was the one common overlay type whose declared
+write-path spec validation (ADR-0005 §Validation) could be bypassed outright — a
+`declared ≠ enforced` gap at union **member selection**, one level above the
+object schemas #4001 closed.
+
+Because `saveMetaItem` persists the *original* body rather than the parse output,
+a wrong-shaped view — an AI-generated body in the wrong dialect, a hand-written
+one with every key misspelled — did not fail loudly. It became an **active** view
+overlay that renders nothing, and the read path then re-parsed it through the same
+schema and badged it `_diagnostics.valid: true` (#5598), so Studio agreed it was
+fine.
+
+**The fix.** A minimal identity precondition now runs ahead of all four arms: a
+`view` body must carry at least one key some member declares, discounting the
+keys the write path stamps onto every body itself (`name` always, plus
+`viewKind`/`object`/`label` inherited from a shadowed registry entry — #2555).
+The bar is *shape*, not completeness: `{ isPinned: true }` is not a renderable
+view either, but it is unambiguously a view operation and still saves. No arm's
+`.strip()` changed, and `/api/v1/meta/types/view` emits a byte-identical
+`anyOf` of four in both the output and input directions, so Studio's SchemaForm
+renders exactly as before.
+
+**Behaviour change** (why this is major — it is an enforcement close, not a new
+capability):
+
+| `view` body | Before | After |
+|:--|:--|:--|
+| `{ nope: 1 }`, `{ id: 'x' }` — no recognized key | saved, stored **active** | **422** |
+| `{}` | saved, stored active | **422** |
+| identity only (`{ name }`, `{ name, object, viewKind, label }`) | saved | **422** |
+| `{ isPinned: true }`, `{ hidden: true }`, `{ sortOrder: 3 }`, `{ order: 2 }` | saved | unchanged — saved |
+| any container / ViewItem record / flattened overlay | as before | unchanged |
+| a body mixing garbage **with** a real view key | stripped and saved | unchanged — still stripped and saved |
+
+That last row is the deliberate residue of the minimal fix: the precondition asks
+"is this a view", never "is every key meaningful". Closing it means closing the
+arms, which would break the round-trip capability #5074 exists to protect.
+
+**FROM → TO.** Existing projects whose stored views carry stray-key bodies will
+start seeing 422 on the next save of those views. Reads are unaffected — nothing
+is deleted or rewritten — but the same documents now badge `valid: false`, which
+is how you find them. The platform ships a sweep endpoint for exactly this:
+
+```bash
+curl -s "$OS_URL/api/v1/meta/diagnostics?type=view" -H "Authorization: Bearer $TOKEN" \
+  | jq -r '.entries[] | "\(.name)\t\(.diagnostics.errors[0].message)"'
+```
+
+Each row names the view and why it is rejected. The fix is per row: give the body
+a real view shape, or delete the overlay if it was never a view to begin with.
+
+```diff
+- { "nope": 1, "name": "crm_lead.all" }
++ { "name": "crm_lead.all", "object": "crm_lead", "viewKind": "list",
++   "config": { "type": "grid", "columns": ["name"] } }
+```
+
+The rejection carries its own prescription rather than a rootless
+`Invalid input` — it names the key classes a view may open with, separates keys
+it does not recognize from identity keys it recognizes but discounts, and it is
+one issue, not one plus four `invalid_union` branches.
+
+**New export.** `VIEW_WRITE_PATH_IDENTITY_KEYS` (`@objectstack/spec/ui`) — the
+discounted set, exported so the producer side can be pinned against it. It is:
+`normalizeViewMetadata` must never stamp a key absent from that set, or the key
+silently becomes evidence again and re-opens this hole; a behavioural test in
+`@objectstack/metadata-protocol` fails in the file that would introduce it.
+
+Direction A from the issue — giving the form arm a required floor — remains
+deliberately **not** taken. It needs Studio's flattened round-trip bodies
+measured first, or it 422s writes the platform itself makes; the ruling on #5599
+deferred it as a possible second tightening on top of this one.

packages/metadata-protocol/src/metadata-diagnostics.union-issues.test.ts

@@ -136,3 +136,71 @@ describe('#5598 the entries that never went through a union are unchanged', () =
         expect(computeMetadataDiagnostics('service', { name: 'whatever' })).toBeUndefined();
     });
 });
+
+/**
+ * #5599 — the OTHER half of the same badge, closed in `packages/spec`.
+ *
+ * #5598 fixed a stored view whose defect collapsed to one rootless line. It could
+ * not touch the worse case one row over: a stored view that is not a view at all
+ * got `valid: true`. `ViewMetadataSchema`'s fourth union member both stripped
+ * unknown keys and required none, so `{ nope: 1 }` MATCHED, and the badge this
+ * module computes from that same schema said the document was fine. The two bugs
+ * are one mechanism seen from both ends — a union that explains its rejections
+ * badly, and a union that does not reject at all — which is why the ruling on
+ * #5599 asked for the disappearance of this false `valid: true` to be asserted
+ * from the READ path, not only from the schema's own unit tests.
+ */
+describe('#5599 a stored `view` that is not a view is no longer badged valid', () => {
+    it('`{ nope: 1 }` — the issue\'s headline document — is now `valid: false`', () => {
+        // On `origin/main` this returned exactly `{ valid: true }`.
+        const diag = computeMetadataDiagnostics('view', { nope: 1 });
+        expect(diag?.valid).toBe(false);
+        expect(diag?.errors?.length).toBeGreaterThan(0);
+    });
+
+    it('…and the badge names WHY, so Studio has something to render', () => {
+        const diag = computeMetadataDiagnostics('view', { nope: 1 });
+        expect(diag?.errors?.[0]?.message).toContain('no recognized `view` key');
+        expect(diag?.errors?.[0]?.code).toBe('custom');
+    });
+
+    it('an empty stored `view` body is `valid: false` too', () => {
+        expect(computeMetadataDiagnostics('view', {})?.valid).toBe(false);
+    });
+
+    it('reaches Studio through `decorateMetadataItem`, like every other verdict', () => {
+        const decorated = decorateMetadataItem('view', { nope: 1 }) as {
+            _diagnostics?: { valid: boolean };
+        };
+        expect(decorated._diagnostics?.valid).toBe(false);
+    });
+
+    it('read and save still agree — one ranking, applied to the new rejection', () => {
+        // The #5598 invariant, re-proved on the issue class #5599 introduces:
+        // a document must not be "valid to open, invalid to save" or vice versa.
+        const schema = getMetadataTypeSchema('view') as z.ZodTypeAny;
+        const parsed = schema.safeParse({ nope: 1 });
+        expect(parsed.success).toBe(false);
+        const fromSharedRanking = zodIssuesToMetadataIssues(
+            (parsed as { error: { issues: unknown[] } }).error.issues,
+        );
+        expect(computeMetadataDiagnostics('view', { nope: 1 })?.errors).toEqual(fromSharedRanking);
+    });
+
+    it('a legitimately-lean overlay is still valid — no collateral badge', () => {
+        // The precondition asks "is this a view at all", never "is it complete".
+        expect(computeMetadataDiagnostics('view', { isPinned: true })).toEqual({ valid: true });
+        expect(computeMetadataDiagnostics('view', { hidden: true })).toEqual({ valid: true });
+    });
+
+    it('a stored row of pure identity is no longer valid either', () => {
+        // The stored twin of the write-path case: `{ nope: 1 }` was persisted as
+        // `{ nope: 1, name: … }` (plus inherited identity where a registry entry
+        // existed), so every such row read back `valid: true`. Those rows are
+        // exactly the ones an operator now has to find — see the changeset.
+        expect(computeMetadataDiagnostics('view', { nope: 1, name: 'garbage_view' })?.valid).toBe(false);
+        expect(computeMetadataDiagnostics('view', {
+            nope: 1, name: 'showcase_task.default', viewKind: 'list', object: 'showcase_task', label: 'All Tasks',
+        })?.valid).toBe(false);
+    });
+});

packages/metadata-protocol/src/protocol.lock-gate-fail-closed.test.ts

@@ -185,8 +185,17 @@ function protocolFor(h: Harness) {
     return new ObjectStackProtocolImplementation(h.engine, undefined, 'env_1');
 }
 
+// [#5599] The body carries a real view key (`type` / `columns`), not identity
+// alone. It used to be `{ name, label }`, which the `view` schema accepted only
+// because its union had a member that stripped unknown keys and required none —
+// the hole #5599 closed. Nothing here needs a contentless body: this file's
+// subject is WHICH writes the lock gate admits, not what a view looks like.
 const save = (p: ObjectStackProtocolImplementation) =>
-    p.saveMetaItem({ type: 'view', name: 'v1', item: { name: 'v1', label: 'Edited' } } as any);
+    p.saveMetaItem({
+        type: 'view',
+        name: 'v1',
+        item: { name: 'v1', label: 'Edited', type: 'grid', columns: ['name'] },
+    } as any);
 
 const remove = (p: ObjectStackProtocolImplementation) =>
     p.deleteMetaItem({ type: 'view', name: 'v1' } as any);

packages/metadata-protocol/src/view-write-path-identity.test.ts

@@ -0,0 +1,81 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #5599 — the producer half of the identity precondition, pinned where the
+ * producer lives.
+ *
+ * `ViewMetadataSchema`'s precondition asks "did the AUTHOR send something that
+ * is a view?". It cannot ask that directly, because `saveMetaItem` normalizes
+ * before it validates: by the time the schema sees the body,
+ * {@link normalizeViewMetadata} has already stamped keys onto it. The schema
+ * therefore discounts a fixed set — `VIEW_WRITE_PATH_IDENTITY_KEYS` — when
+ * judging evidence.
+ *
+ * That makes the two files a matched pair with no compiler link between them:
+ * the day this function learns to stamp a fifth key, that key silently becomes
+ * "evidence the author sent a view" over in `packages/spec`, and `{ nope: 1 }`
+ * starts passing the gate again — the exact defect #5599 closed, reopened by an
+ * edit that looks entirely reasonable and touches neither the schema nor this
+ * test's subject.
+ *
+ * So the pin is behavioural, not a copy of the list: it feeds the normalizer the
+ * emptiest possible body together with a maximal baseline, and asserts that
+ * everything it stamps is discounted. A new stamped key fails here, in the file
+ * that introduced it, with the remedy named.
+ */
+import { describe, expect, it } from 'vitest';
+import { VIEW_WRITE_PATH_IDENTITY_KEYS } from '@objectstack/spec/ui';
+import { getMetadataTypeSchema } from '@objectstack/spec/kernel';
+import { normalizeViewMetadata } from './protocol.js';
+
+/** A registry entry carrying every identity field `viewIdentityPatch` inherits. */
+const baseline = {
+    name: 'showcase_task.default',
+    object: 'showcase_task',
+    viewKind: 'list',
+    label: 'All Tasks',
+    scope: 'package',
+    config: { type: 'grid', data: { provider: 'object', object: 'showcase_task' }, columns: ['title'] },
+};
+
+describe('#5599 the write path stamps only keys the spec discounts as identity', () => {
+    it('every key stamped onto an empty body is in VIEW_WRITE_PATH_IDENTITY_KEYS', () => {
+        const stamped = normalizeViewMetadata('view', {}, 'showcase_task.default', baseline) as Record<string, unknown>;
+        const unaccounted = Object.keys(stamped).filter((k) => !VIEW_WRITE_PATH_IDENTITY_KEYS.has(k));
+        expect(
+            unaccounted,
+            'normalizeViewMetadata stamped a key the #5599 identity precondition does not discount. '
+            + 'That key now counts as evidence that the author sent a view, which re-opens #5599. '
+            + 'Add it to VIEW_WRITE_PATH_IDENTITY_KEYS in packages/spec/src/ui/view.zod.ts.',
+        ).toEqual([]);
+    });
+
+    it('…and with no baseline it stamps only `name`', () => {
+        const stamped = normalizeViewMetadata('view', {}, 'adhoc.view', undefined) as Record<string, unknown>;
+        expect(Object.keys(stamped)).toEqual(['name']);
+        expect(VIEW_WRITE_PATH_IDENTITY_KEYS.has('name')).toBe(true);
+    });
+
+    it('the normalized garbage body is REJECTED — the two halves compose', () => {
+        // This is the end-to-end statement of the fix, at the seam: the body the
+        // schema actually receives for the issue's headline input, in both the
+        // baseline and no-baseline cases.
+        const schema = getMetadataTypeSchema('view')!;
+        for (const withBaseline of [undefined, baseline]) {
+            const normalized = normalizeViewMetadata('view', { nope: 1 }, 'garbage_view', withBaseline);
+            expect(schema.safeParse(normalized).success).toBe(false);
+        }
+    });
+
+    it('…while a real personalization PUT survives the same seam', () => {
+        const schema = getMetadataTypeSchema('view')!;
+        const personalization = {
+            type: 'grid',
+            data: { provider: 'object', object: 'showcase_task' },
+            columns: ['title'],
+            sort: [{ field: 'estimate_hours', order: 'desc' }],
+        };
+        const normalized = normalizeViewMetadata('view', personalization, 'showcase_task.default', baseline);
+        expect(schema.safeParse(normalized).success).toBe(true);
+    });
+});

packages/objectql/src/protocol-view-identity-overlay.test.ts

@@ -231,4 +231,62 @@ describe('view overlay identity (#2555)', () => {
         expect(persisted.name).toBe('adhoc.view');
         expect('viewKind' in persisted).toBe(false);
     });
+
+    // ── #5599 — the write path's spec gate was bypassable by ANY body ────────
+    //
+    // #3095 (above) closed the case where a view's nested `config` was stripped
+    // to `{}`. #5599 is the case one level further out: the union's fourth
+    // member both `.strip()`s and requires nothing, so `{ nope: 1 }` MATCHED it,
+    // the gate reported success, and — because `saveMetaItem` persists the
+    // ORIGINAL body, not the parse output — `{"nope":1,"name":"garbage_view"}`
+    // landed in `sys_metadata` as an ACTIVE view. `view` was the one common
+    // overlay type whose declared spec validation (ADR-0005 §Validation) could
+    // be bypassed outright: Prime Directive #10's "declared ≠ enforced", at the
+    // union's member-selection layer rather than inside any member.
+    //
+    // Measured on `origin/main` before the fix, this exact call returned
+    // `{ success: true, state: 'active', seq: 1 }`.
+    it('#5599 write path REJECTS a body that is not a view at all (was: success + stored active)', async () => {
+        const { engine, rows } = makeStubEngine();
+        const protocol = new ObjectStackProtocolImplementation(engine);
+        await expect(
+            protocol.saveMetaItem({ type: 'view', name: 'garbage_view', item: { nope: 1 } }),
+        ).rejects.toMatchObject({ code: 'INVALID_METADATA', status: 422 });
+        // The half that made this a data bug rather than a validation nit:
+        // nothing may reach the store.
+        expect(Array.from(rows.values()).some((r) => r.type === 'view')).toBe(false);
+    });
+
+    it('#5599 write path REJECTS an empty body, and stores nothing', async () => {
+        const { engine, rows } = makeStubEngine();
+        const protocol = new ObjectStackProtocolImplementation(engine);
+        await expect(
+            protocol.saveMetaItem({ type: 'view', name: 'empty_view', item: {} }),
+        ).rejects.toMatchObject({ code: 'INVALID_METADATA', status: 422 });
+        expect(Array.from(rows.values()).some((r) => r.type === 'view')).toBe(false);
+    });
+
+    it('#5599 the 422 carries the prescription, not a rootless "Invalid input"', async () => {
+        const { engine } = makeStubEngine();
+        const protocol = new ObjectStackProtocolImplementation(engine);
+        const failure = await protocol
+            .saveMetaItem({ type: 'view', name: 'garbage_view', item: { nope: 1 } })
+            .then(() => null, (e: unknown) => e);
+        expect(failure).toBeTruthy();
+        expect(JSON.stringify(failure)).toContain('no recognized `view` key');
+    });
+
+    it('#5599 …while the personalization PUT this file exists for still saves', async () => {
+        // The regression this precondition must never cause: a 422 on a body the
+        // platform itself writes. `personalization` is the captured console PUT.
+        const { engine, rows } = makeStubEngine({ 'showcase_task.default': flattened });
+        const protocol = new ObjectStackProtocolImplementation(engine);
+        const result = await protocol.saveMetaItem({
+            type: 'view',
+            name: 'showcase_task.default',
+            item: { ...personalization },
+        });
+        expect(result.success).toBe(true);
+        expect(Array.from(rows.values()).some((r) => r.type === 'view')).toBe(true);
+    });
 });

packages/spec/api-surface/ui.json

@@ -338,6 +338,7 @@
     "VIEW_CONSOLE_ROW_DECORATIONS (const)",
     "VIEW_FILTER_OPERATORS (const)",
     "VIEW_FILTER_OPERATOR_ALIASES (const)",
+    "VIEW_WRITE_PATH_IDENTITY_KEYS (const)",
     "View (type)",
     "ViewData (type)",
     "ViewDataParsed (type)",