feat(spec)!: retire the flattened view overlay's owner and hidden keys (ADR-0049)

.changeset/view-overlay-owner-hidden-retired.md

@@ -0,0 +1,101 @@
+---
+'@objectstack/spec': minor
+---
+
+feat(spec)!: retire the flattened view overlay's `owner` and `hidden` keys — accepted at the save door, stored, and read by nothing (#20230)
+
+**BREAKING** — `owner` and `hidden` are removed from the flattened view overlay:
+the lean `view` body with no `config` that `PUT /api/v1/meta/view/:name` (the
+Studio and MCP save) accepts, members 3 and 4 of the `view` metadata door, and
+the same members in the assembled-manifest `viewItems:` channel. ADR-0049
+enforce-or-remove; triage direction, verbatim: 「follow #20085's disposition for
+the same key pair」. This completes the family: the view item record's `owner` /
+`hidden` are retired in this same release by its own entry, with the same texts.
+
+⚠️ **This supersedes one sentence of the view item retirement's note in this same
+release.** That note says the flattened overlay's own `owner` / `hidden` are
+untouched and that a `{ object, viewKind, hidden: true }` overlay still parses.
+True of that change alone; after this one, such an overlay is refused too. Read the
+two notes together: after this release, neither door accepts either key.
+
+Clause-②: no (narrowing)
+
+The overlay door declared both keys separately from the view item's pair. A bound
+overlay such as `{ object, viewKind, hidden: true }` saved clean and one row was
+stored with the key, and nothing ever read it. Both view-switcher read paths
+(`GET /meta/view?object=` and `getViewsByObject`) filter on `viewKind` + `object`
+and sort on `order`, so `hidden: true` hid nothing, and a view with `owner` set
+was listed for every user who can read the object.
+
+Writer census, taken before removal: no writer of either overlay key in this
+framework or its examples, in objectui at its pinned commit and at `main` (the
+toolbar writes only `rowHeight`, `sort`, `hiddenFields`, `columnState` and
+`inlineEdit`; the switcher only `label`, `isPinned`, `isDefault` and `sortOrder`),
+or in the HotCRM app. The cloud repository was not reachable from the census.
+
+### FROM → TO
+
+| removed | what to write instead |
+| --- | --- |
+| flattened overlay `owner` | delete the key. Nothing restricts a view to one user today; a view is visible to everyone who can read its object. |
+| flattened overlay `hidden` | delete the key. To take a view out of the switcher, delete the view item (or stop shipping it from source). |
+
+**The one-line fix: delete `owner:` and `hidden:` from every view body you save.**
+`os migrate meta --from 17` lists the mechanical edits for existing sources.
+
+⚠️ Runtime behaviour is deliberately **unchanged**. Neither key ever changed what
+a view showed or to whom. What changes is the answer an author gets: a save that
+carries either key is refused `422 INVALID_METADATA`, with the prescription
+located at the key, instead of being stored with no effect. The prescriptions are
+the view item's own texts, so the family answers with one voice on both doors.
+
+### Stored rows
+
+Every read of a stored `view` row replays the conversion chain before the row is
+served or badged, and the D2 conversion strips both keys there. What that leaves
+depends on what else the row holds:
+
+- **A row with any other view key** (a column state, a sort, a default flag, an
+  order): served and badged valid without the keys. A GET then a PUT of the whole
+  row saves (if it was otherwise valid), so the console's next read-merge-write of
+  it saves, and `os migrate meta --stored --apply` rewrites it.
+- **A hide-only row**, holding nothing but its identity (`name`, `object`,
+  `viewKind`, `label`) and `owner` / `hidden`, such as
+  `{ object, viewKind, hidden: true }`: the strip leaves identity only, which the
+  `view` door refuses ("only identity fields"). The row is served badged invalid
+  (it was badged valid before this release). A whole-row re-save, or one that adds
+  only identity (a rename sets `label`), answers `422 INVALID_METADATA`.
+  `--apply` reports it `failed` and leaves it as stored; every read strips it
+  again. A write that adds a real view key, such as a toolbar toggle, saves.
+  **Fix: delete the row** (it never changed what anyone saw), or add the
+  personalization setting its author meant and save that.
+
+### The retirement kit
+
+- **Tombstones on both overlay members.** `retiredKey()` in
+  `flattenedViewOverlayFields()`, with the view item's prescription texts. Both
+  members `.strip()`, so a bare deletion would have dropped the key in silence
+  (ADR-0104).
+- **D2 conversion `view-overlay-owner-hidden-removed`** (step 18, retired from the
+  load path). A lossless delete from the flattened spelling (no `config`, no
+  container slot) in `views` (stack sources and stored rows) and `viewItems`
+  (assembled artifacts). It is disjoint from `view-item-owner-hidden-removed` by
+  `config`, so no row is judged by both.
+- **D3 semantic entry `view-overlay-owner-hidden-retired`**: the family's one D3
+  record, naming its D2 conversion. The view item record's pair is a separate
+  family with its own conversion and its own D3 entry; the two share the
+  prescription texts.
+- **`RETIRED_KEYS_BY_MAJOR[18]`**: `ui/ViewMetadata:owner`, `ui/ViewMetadata:hidden`.
+  `ui/ViewMetadata` is unemitted (its `z.undefined()` guards have no JSON Schema
+  form), so no build gate judges these rows and the four surface ratchets are
+  byte-identical on this retirement. The rows are pinned by the retirement test.
+- **No liveness row**: the `view` ledger walks the container keys only.
+- **No deprecation window**, per the project's startup-stage posture.
+
+⚠️ **The out-of-repo population is NOT MEASURED.** `@objectstack/spec` is published,
+and production `sys_metadata` rows are not reachable from the repository. Stored
+rows are stripped on read by the conversion above, and a hide-only row among them
+needs the fix above. A client that still sends either key is refused at its next
+save.
+
+<!-- adr-0087: registered view-overlay-owner-hidden-removed, view-overlay-owner-hidden-retired -->

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

@@ -193,7 +193,18 @@ describe('#5599 a stored `view` that is not a view is no longer badged valid', (
         // inherits `object`/`viewKind` from the shadowed entry (#2555), so a
         // stored lean overlay of a REAL view looks exactly like this.
         expect(computeMetadataDiagnostics('view', { isPinned: true, object: 'task', viewKind: 'list' })).toEqual({ valid: true });
-        expect(computeMetadataDiagnostics('view', { hidden: true, object: 'task', viewKind: 'list' })).toEqual({ valid: true });
+        expect(computeMetadataDiagnostics('view', { order: 2, object: 'task', viewKind: 'list' })).toEqual({ valid: true });
+        // [#20230] `hidden` used to stand in the line above. The overlay's
+        // `owner` / `hidden` are retired (ADR-0049): a body that still carries
+        // one is badged invalid with the retirement prescription at the key.
+        // A STORED row never reaches this badge with the key — the read path
+        // replays the chain first (`convertStoredItem`), and
+        // `view-overlay-owner-hidden-removed` strips it.
+        const bound = { object: 'task', viewKind: 'list' } as const;
+        const retired = computeMetadataDiagnostics('view', { ...bound, hidden: true });
+        expect(retired?.valid).toBe(false);
+        const atKey = retired?.errors?.find((e) => e.path === 'hidden');
+        expect(atKey?.message).toMatch(/^`view\.hidden` was removed in @objectstack\/spec/);
         // …while a stored row with NO binding is a row no object-bound read
         // path can serve — the #7741 dead row — and is badged invalid now.
         expect(computeMetadataDiagnostics('view', { isPinned: true })?.valid).toBe(false);

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

@@ -31,6 +31,7 @@ import { describe, expect, it } from 'vitest';
 // of this package's (file, verb) pairs sat in the gate's DEBT ledger until
 // #5619 sank the two predicates into a package both sides already depend on.
 import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';
+import { applyConversionsToStoredItem } from '@objectstack/spec';
 import { getMetadataTypeSchema } from '@objectstack/spec/kernel';
 import { ObjectStackProtocolImplementation, zodIssuesToMetadataIssues } from './protocol.js';
 
@@ -46,8 +47,16 @@ interface Row {
 const keyOf = (w: Record<string, unknown>) =>
     `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}`;
 
-/** The engine surface the repository write path touches. */
-function makeProtocol() {
+/**
+ * The engine surface the repository write path touches.
+ *
+ * [#20230] `seed` (optional; default none, so every other test's double is
+ * unchanged) makes `findOne` answer the READ a `getMetaItem` performs against
+ * `sys_metadata` with a stored row, and gives the registry the two read verbs
+ * that path consults — answering nothing, so the served item is the stored row
+ * after the rehydration seam and nothing else.
+ */
+function makeProtocol(seed: Array<{ type: string; name: string; metadata: Record<string, unknown> }> = []) {
     // ⚠️ Keyed BY TABLE. `find`/`findOne` below answer nothing, so this harness
     // cannot serve a `sys_metadata_history` row as a `sys_metadata` row the way
     // #16223 measured — but one flat map still made `rows.size` the total of
@@ -65,7 +74,15 @@ function makeProtocol() {
     let nextId = 0;
     const engine: any = {
         async findOne(object: string, query?: EngineFindOneQueryInput) {
-                          assertEngineFindOnePredicate(object, query); return null; },
+            assertEngineFindOnePredicate(object, query);
+            if (object !== 'sys_metadata' || seed.length === 0) return null;
+            const where = ((query as { where?: Record<string, unknown> } | undefined)?.where ?? {});
+            const hit = seed.find((r) => r.type === where.type && r.name === where.name
+                && (where.state ?? 'active') === 'active' && (where.organization_id ?? null) === null);
+            return hit
+                ? { id: `seed_${hit.name}`, type: hit.type, name: hit.name, organization_id: null, state: 'active', metadata: JSON.stringify(hit.metadata) }
+                : null;
+        },
         async find() { return []; },
         async insert(table: string, data: Record<string, unknown>) {
             nextId += 1;
@@ -81,7 +98,9 @@ function makeProtocol() {
             assertEngineDeleteDispatch(opts);
             return { deleted: 0 };
         },
-        registry: { registerItem: () => {}, registerObject: () => {} },
+        registry: seed.length === 0
+            ? { registerItem: () => {}, registerObject: () => {} }
+            : { registerItem: () => {}, registerObject: () => {}, getItem: () => undefined, getObject: () => undefined },
     };
     const protocol: any = new ObjectStackProtocolImplementation(engine, () => new Map());
     return { protocol, rows };
@@ -363,3 +382,107 @@ describe('#5364 zodIssuesToMetadataIssues — the shared ranking, verbatim', ()
         expect(zodIssuesToMetadataIssues(null)).toEqual([]);
     });
 });
+
+/**
+ * #20230 — the flattened overlay's retired `owner` / `hidden`, at the door the
+ * retirement exists for: `saveMetaItem`, the `PUT /api/v1/meta/view/:name` write
+ * path the console and an MCP author reach.
+ *
+ * Before: a bound lean overlay `{ object, viewKind, hidden: true }` saved, one
+ * row persisted with the key, and nothing ever read it — measured on this same
+ * harness by the #20085 dev. After: refused with the ADR-0112 envelope, nothing
+ * persisted, and the retirement prescription located at the key. Pinned HERE
+ * and not only in spec because this envelope is what Studio and an MCP caller
+ * receive — a spec tombstone that did not reach it would be a refusal nobody sees.
+ */
+describe('#20230 a flattened overlay carrying a retired owner/hidden is refused at the save door', () => {
+    // Spread, not a literal: the spec's tree-scoped absence pin reads object
+    // literals, and these bodies are refusals, not authorings.
+    const BOUND_LIST = { object: 'task', viewKind: 'list' } as const;
+    const BOUND_FORM = { object: 'task', viewKind: 'form' } as const;
+    const PRESCRIPTION: Record<'owner' | 'hidden', RegExp> = {
+        owner: /^`view\.owner` was removed in @objectstack\/spec 17\.5\.0 \(ADR-0049/,
+        hidden: /^`view\.hidden` was removed in @objectstack\/spec 17\.5\.0 \(ADR-0049/,
+    };
+
+    for (const [key, value] of [['owner', 'usr_7'], ['hidden', true]] as const) {
+        for (const [family, bound] of [['list', BOUND_LIST], ['form', BOUND_FORM]] as const) {
+            it(`a bound ${family} overlay with \`${key}\` answers 422 INVALID_METADATA and persists nothing`, async () => {
+                const { protocol, rows } = makeProtocol();
+
+                const err = await rejection(save(protocol, { name: 'task_list', ...bound, [key]: value }));
+
+                expect(err.code).toBe('INVALID_METADATA');
+                expect(err.status).toBe(422);
+                expect(rows.size).toBe(0);
+                // The prescription, located at the key the author sent.
+                const atKey = err.issues.find((i: any) => i.path === key);
+                expect(atKey, `an issue located at \`${key}\``).toBeDefined();
+                expect(atKey.code).toBe('invalid_type');
+                expect(atKey.message).toMatch(PRESCRIPTION[key]);
+                expect(err.message).toContain(`\`view.${key}\` was removed`);
+            });
+        }
+    }
+
+    /**
+     * The hide-only residue, at the door it is refused by. The card's measured
+     * stored shape `{ object, viewKind, hidden: true }` (plus the stamped
+     * `name`) is served stripped by the rehydration seam — identity only — and
+     * a whole-row PUT of what was served is refused by the identity
+     * precondition. Stated in the D2 docblock, the D3 acceptance criteria and
+     * the changeset; the remedy is to delete the row or add the setting its
+     * author meant.
+     */
+    it('RESIDUE: a whole-row PUT of a stripped hide-only row answers 422 INVALID_METADATA ("only identity fields")', async () => {
+        const stored = { name: 'task_list', ...BOUND_LIST, hidden: true };
+        const served = applyConversionsToStoredItem('view', stored) as Record<string, unknown>;
+        expect(served).toEqual({ name: 'task_list', ...BOUND_LIST });
+
+        const { protocol, rows } = makeProtocol();
+        const err = await rejection(save(protocol, served));
+
+        expect(err.code).toBe('INVALID_METADATA');
+        expect(err.status).toBe(422);
+        expect(rows.size).toBe(0);
+        expect(err.message).toContain('only identity fields');
+        // Not the retirement prescription: the key is already gone.
+        expect(err.message).not.toContain('was removed in @objectstack/spec');
+    });
+
+    it('RESIDUE CONTROL: the same stripped row plus a real view key (a toolbar toggle) saves', async () => {
+        const served = applyConversionsToStoredItem('view', { name: 'task_list', ...BOUND_LIST, hidden: true }) as Record<string, unknown>;
+        const { protocol, rows } = makeProtocol();
+        const result = await save(protocol, { ...served, isDefault: true });
+        expect(result.success).toBe(true);
+        expect(rows.size).toBe(1);
+    });
+
+    it('READ PATH: `getMetaItem` serves a stored overlay without `owner` / `hidden` — valid with content, invalid when hide-only', async () => {
+        const { protocol } = makeProtocol([
+            { type: 'view', name: 'task_list', metadata: { name: 'task_list', ...BOUND_LIST, isDefault: true, order: 2, owner: 'usr_7', hidden: true } },
+            { type: 'view', name: 'task_hidden', metadata: { name: 'task_hidden', ...BOUND_FORM, hidden: true } },
+        ]);
+
+        const content = (await protocol.getMetaItem({ type: 'view', name: 'task_list' })).item;
+        expect(content).not.toHaveProperty('owner');
+        expect(content).not.toHaveProperty('hidden');
+        expect(content.isDefault).toBe(true);
+        expect(content.order).toBe(2);
+        expect(content._diagnostics).toEqual({ valid: true });
+
+        const hideOnly = (await protocol.getMetaItem({ type: 'view', name: 'task_hidden' })).item;
+        expect(hideOnly).not.toHaveProperty('hidden');
+        expect(hideOnly._diagnostics?.valid).toBe(false);
+        expect(JSON.stringify(hideOnly._diagnostics)).toContain('only identity fields');
+    });
+
+    it('CONTROL: the same bound overlays without the keys still save, one row each', async () => {
+        for (const bound of [BOUND_LIST, BOUND_FORM]) {
+            const { protocol, rows } = makeProtocol();
+            const result = await save(protocol, { name: 'task_list', ...bound, isDefault: true, order: 2 });
+            expect(result.success).toBe(true);
+            expect(rows.size).toBe(1);
+        }
+    });
+});