diff --git a/.changeset/20051-persist-parsed-view-body.md b/.changeset/20051-persist-parsed-view-body.md new file mode 100644 index 00000000000..3a91e7b95f0 --- /dev/null +++ b/.changeset/20051-persist-parsed-view-body.md @@ -0,0 +1,41 @@ +--- +'@objectstack/metadata-protocol': minor +'@objectstack/spec': minor +--- + +fix(metadata-protocol,spec)!: a saved view stores the parsed value of every key its body carried, and a ViewItem record's top-level `options` bag is refused by name (#20051) + +**BREAKING** — two narrowings on the `view` write door (`PUT /api/v1/meta/view/:name`, the Studio and MCP save). They ship as `minor` under the repo's launch-window convention for breaking changes. This is stage (iv), the last stage, of ruling 甲 on #20051. The storage half follows the maintainer's ruling on its Q2, letter B (「同意 批次 #256」). + +Clause-②: no (narrowing) + +## What changes + +**1. What a saved view stores.** `saveMetaItem` used to validate a `view` body and then store the request body as sent, with two normalizations grafted back (filter operator spellings, and form `groups` → `sections`). It now stores the parsed value of every key the body carried, and nothing else: + +- **An undeclared key is dropped.** The members' top-level `.strip()` used to drop it from the parse only, so it lived in the store and nowhere in the contract. Every key the console reads back off a stored row is declared (`VIEW_CONSOLE_ROUND_TRIP_KEYS`), so nothing the console relies on is lost. The keys the console writes that are dropped are the ones mapped rather than declared in that record: a sort row's `id` (the builders mint a fresh one), a top-level `id` (read only when a row has no `name`), and `objectName` (every reader falls back to `object`, which both writers stamp). +- **A declared key keeps its normalized value.** Examples: `notEquals` → `not_equals`, `exportOptions: ['csv']` → `{ formats: ['csv'] }`, and a CEL string → its expression object. +- **A moved key is stored under its canonical spelling.** Examples: `groups` → `sections`, and `visibleOn` → `visibleWhen`. The second was never grafted before, so a form stored with `visibleOn` kept the legacy spelling. +- **A schema default the author did not write is NOT stored.** This is ADR-0087's `storable` rule, the one flows already follow: a stored row never pins the day's default. A console toolbar save (sort, density, hidden fields, column widths, inline edit) stores no `type: 'grid'`. A form stores no `sharing.enabled` and no section `collapsible` / `collapsed` / `columns` it did not write. Storing the whole parse output instead would also have minted rows that fail their own re-save: a column-less toolbar patch carrying the `grid` default is refused as "sets `type` but lists no `columns`". + +The stored row re-parses to exactly what the save accepted, so a GET → PUT of it is judged the same. Every other metadata type keeps its request body, with the two grafts, as before. + +**2. A ViewItem record's top-level `options` bag is refused.** On a record (`{ name, object, viewKind, config }`), the member's top-level strip used to drop `options` from the parse unread while the save stored it. The interface page then rendered it and the object page did not. It is now refused by name with `422 INVALID_METADATA` at `options`, and the message prescribes `config.KIND`. No console write puts the bag on a record. The flattened list overlay's legacy `options` bag is unchanged: it is judged key by key, and objectui pins it. + +## FROM → TO + +| you wrote | the stored row / the door now | +|:--|:--| +| a view body with a key its member does not declare (`objectName`, a form-only `layout` on a list view, `isPinned` on a form) | the key is not stored: write only declared keys (`object`, not `objectName`) | +| a view body relying on a schema default being written into the row | the row carries only what you wrote; the parse applies the default on every read | +| `sort: [{ id, field, order }]` | stored as `sort: [{ field, order }]` | +| `groups: [...]` / `visibleOn: '…'` on a form | stored as `sections: [...]` / `visibleWhen: { dialect: 'cel', source: '…' }` | +| a ViewItem record with `options: { kanban: {...} }` | `422` at `options`: move it to `config: { kanban: {...} }` (`config.KIND`), or remove it | + +**The one-line fix:** write the declared spelling. A stored view you read back is what the contract accepts, and a record's per-kind blocks live under `config`. + +## Existing rows + +Stored rows are not migrated and not re-read differently. The maintainer's word on this card is 「20051 不考虑现有的数据」. A row keeps its bytes until its next save, and that save stores it as described above. A record carrying a top-level `options` is refused on its next save and served as stored until then. + + diff --git a/content/docs/references/ui/view.mdx b/content/docs/references/ui/view.mdx index 18a962ee89c..560de2073e4 100644 --- a/content/docs/references/ui/view.mdx +++ b/content/docs/references/ui/view.mdx @@ -2260,6 +2260,7 @@ This schema accepts one of the following structures: | **sortOrder** | `integer` | optional | Console round-trip: the view's position among the object's saved views in the switcher, 0-based and counted over saved views only (a code-defined view carries none). Written by the console's drag-reorder and read back to order the tabs. Not authored: `order` is the authored default position. | | **visibility** | `Enum<'private' \| 'team' \| 'organization' \| 'public'>` | optional | Console round-trip: the group the switcher files this view's tab under (private, team, organization or public). Display grouping only, NOT access control: nothing restricts who can list or open the view by this value. No console control sets it; the console carries a stored value forward when it re-saves the row. Not authored. | | **columnState** | `{ order?: string[]; widths?: Record }` | optional | Studio round-trip: column order/widths (runtime-only state, written by the console grid and stored on the view's row, which has no per-user scope — not authored) | +| **options** | `never` | optional | Refused: a view item record carries no top-level `options` bag. Its per-kind blocks live under `config` (`config.kanban`, `config.timeline`, …); the legacy `options.KIND` bag belongs to the flattened list overlay only. | ### Nested Shape: `ViewItemWire[viewKind='list'].config` @@ -2361,6 +2362,7 @@ This schema accepts one of the following structures: | **sortOrder** | `integer` | optional | Console round-trip: the view's position among the object's saved views in the switcher, 0-based and counted over saved views only (a code-defined view carries none). Written by the console's drag-reorder and read back to order the tabs. Not authored: `order` is the authored default position. | | **visibility** | `Enum<'private' \| 'team' \| 'organization' \| 'public'>` | optional | Console round-trip: the group the switcher files this view's tab under (private, team, organization or public). Display grouping only, NOT access control: nothing restricts who can list or open the view by this value. No console control sets it; the console carries a stored value forward when it re-saves the row. Not authored. | | **columnState** | `{ order?: string[]; widths?: Record }` | optional | Studio round-trip: column order/widths (runtime-only state, written by the console grid and stored on the view's row, which has no per-user scope — not authored) | +| **options** | `never` | optional | Refused: a view item record carries no top-level `options` bag. Its per-kind blocks live under `config` (`config.kanban`, `config.timeline`, …); the legacy `options.KIND` bag belongs to the flattened list overlay only. | ### Nested Shape: `ViewItemWire[viewKind='form'].config` diff --git a/packages/lint/src/runtime-gate.view-writes.test.ts b/packages/lint/src/runtime-gate.view-writes.test.ts index 54bd2ca570e..ae25afc35c9 100644 --- a/packages/lint/src/runtime-gate.view-writes.test.ts +++ b/packages/lint/src/runtime-gate.view-writes.test.ts @@ -48,9 +48,10 @@ const objects = [ /** * A flattened standalone list overlay, as the wire carries it: a raw ListView * config at the TOP level, `object` + `viewKind` required (#7741), plus the - * console decorations a personalization PUT persists — `sort[].id` is - * objectui's `crypto.randomUUID()` row id (#5074), stored verbatim because - * `saveMetaItem` persists the original body. + * console decorations a personalization PUT carries — `sort[].id` is + * objectui's `crypto.randomUUID()` row id (#5074). It was stored verbatim + * until #20051 made a view store its parsed body; rows stored before that + * still carry it, so the rules keep being judged on it. */ const overlay = (patch: Record) => ({ name: 'crm_case.custom', @@ -264,7 +265,7 @@ describe('a flattened list overlay at the runtime publish gate (#9313)', () => { // (`view.zod.ts`'s #5074 trace), so every pin/reorder toggle round-trips the // whole record, `config` included. - /** A ViewItem record as `saveMetaItem` stores it (original body, verbatim). */ + /** A ViewItem record as the console PUTs it back (the stored record plus the patch). */ const record = ( configPatch: Record, patch: Record = {}, diff --git a/packages/metadata-protocol/src/protocol.graft-folded-form-sections.test.ts b/packages/metadata-protocol/src/protocol.graft-folded-form-sections.test.ts index 24764d95dd4..eafd030797d 100644 --- a/packages/metadata-protocol/src/protocol.graft-folded-form-sections.test.ts +++ b/packages/metadata-protocol/src/protocol.graft-folded-form-sections.test.ts @@ -27,9 +27,22 @@ * helper directly for the structural cases a save cannot reach (a `groups` key * the schema KEEPS, a mismatched parsed tree), mirroring * `protocol.graft-normalized-operators.test.ts`. + * + * ## Since #20051's stage (iv): a `view` stores its parsed body + * + * A `view` no longer runs this graft on a verbatim body. `saveMetaItem` stores + * the parsed value of every key the body carried (`projectStorableViewBody`), + * which subsumes the fold, the operator graft beside it, and every other key + * move; the graft keeps serving the other types. The save-path blocks above + * therefore now pin the projection's behaviour on the same fixtures, and the + * pins that asserted the verbatim save were re-judged where they stand. The + * stage's own pins are the last three blocks, riding this file's pinned engine + * double for the reason the #20051 door-half block below gives. */ -import { describe, expect, it } from 'vitest'; -import { ViewMetadataSchema } from '@objectstack/spec/ui'; +import { describe, expect, it, vi } from 'vitest'; +import { ViewMetadataSchema, VIEW_CONSOLE_ROUND_TRIP_KEYS } from '@objectstack/spec/ui'; +import { getMetadataTypeSchema, registerMetadataTypeSchema } from '@objectstack/spec/kernel'; +import { z } from 'zod'; import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate, @@ -201,45 +214,63 @@ describe('#7134 the save path persists the folded `sections`, at every depth a f expect(body).not.toHaveProperty('groups'); }); - it('the fold and the Studio round-trip keys COEXIST on one save', async () => { - // The combined statement, and the reason a wholesale `parsed.data` swap - // was never an option: that swap would fold the key and strip - // `isPinned` / `isDefault` / `sortOrder`; persisting verbatim keeps them - // and folds nothing. Only the graft does both. Evidence, not a guard — - // the `sections` half goes red on revert. + it('[#20051] the fold and the form overlay\'s declared keys coexist on one save; its undeclared ones do not', async () => { + // Re-judged at stage (iv) of #20051, which stores a view's parsed + // body. The fold is stored, and so is `isDefault`, which the flattened + // FORM overlay declares. `isPinned` and `sortOrder` are the list + // switcher's row state: the form member declares neither and the + // console writes neither on a form row (`VIEW_CONSOLE_ROUND_TRIP_KEYS` + // maps no key to `formOverlay`), so the stored row drops both. const body = await storedViewBody( 'contact_us', flatForm({ groups: [SECTION], isPinned: true, isDefault: false, sortOrder: 3 }), ); expect(body.sections).toEqual([SECTION]); expect(body).not.toHaveProperty('groups'); - expect({ isPinned: body.isPinned, isDefault: body.isDefault, sortOrder: body.sortOrder }) - .toEqual({ isPinned: true, isDefault: false, sortOrder: 3 }); + expect(body.isDefault).toBe(false); + expect(body).not.toHaveProperty('isPinned'); + expect(body).not.toHaveProperty('sortOrder'); }); }); describe('#7134 what the save path must NOT change', () => { - it('GUARD: Studio-only round-trip keys still survive the save', async () => { - // GUARD, green in BOTH directions — the reason `parsed.data` is - // discarded at all (ADR-0005 §Validation). It asserts ONLY the - // round-trip keys: a `sections` assertion here would be evidence for - // the fix wearing a guard's label, which is the mislabel this file's - // reverse-verification caught. The two claims are pinned together in - // the coexistence case above, where the combined statement belongs. - const body = await storedViewBody( - 'contact_us', - flatForm({ groups: [SECTION], isPinned: true, isDefault: false, sortOrder: 3 }), - ); - expect(body.isPinned).toBe(true); - expect(body.isDefault).toBe(false); - expect(body.sortOrder).toBe(3); + it('GUARD: the console\'s round-trip keys survive the save on the member that declares them', async () => { + // GUARD, green in BOTH directions: stored verbatim before #20051's stage + // (iv), stored because declared after it. Re-judged from a FORM overlay + // to the flat LIST overlay, the row these keys are written on + // (`VIEW_CONSOLE_ROUND_TRIP_KEYS`): every key that record maps to + // `listOverlay`, with a value the console writes. + const authored = { + name: 'crm_lead.all', + object: 'crm_lead', + viewKind: 'list', + sort: [{ field: 'name', order: 'asc' }], + isPinned: true, + isDefault: false, + sortOrder: 3, + visibility: 'team', + columnState: { order: ['name'], widths: { name: 180 } }, + _isOverride: true, + }; + expect(Object.keys(VIEW_CONSOLE_ROUND_TRIP_KEYS).filter( + (k) => (VIEW_CONSOLE_ROUND_TRIP_KEYS as Record)[k].includes('listOverlay'), + ).every((k) => k in authored)).toBe(true); + expect(await storedViewBody(authored.name, authored)).toEqual(authored); }); - it('GUARD: a form authored with canonical `sections` is stored byte-identical', async () => { - // Also green in both directions: nothing folded, so nothing to graft. + it('[#20051] a form authored with canonical `sections` is stored as authored — no section default, no `sharing.enabled`, no undeclared key', async () => { + // Re-judged at stage (iv): was "stored byte-identical", with an + // `isPinned` the form member does not declare riding along. The parse + // adds `collapsible` / `collapsed` / `columns` to the section and + // `enabled: false` to `sharing`; ruling B stores none of them (ADR-0087's + // `storable` rule), and the undeclared `isPinned` is dropped. const authored = flatForm({ sections: [SECTION], isPinned: true }); + const parsed = (ViewMetadataSchema as any).parse(authored); + expect(parsed.sections[0]).toMatchObject({ collapsible: false, collapsed: false, columns: 1 }); + expect(parsed.sharing.enabled).toBe(false); const body = await storedViewBody('contact_us', authored); - expect(body).toEqual(authored); + const { isPinned: _dropped, ...declared } = authored as Record; + expect(body).toEqual(declared); }); it('GUARD: a LIST overlay is untouched — this walk is form-shaped only', async () => { @@ -507,3 +538,246 @@ describe('#20186 the save door judges a flattened overlay by the member its view expect(await storedViewBody(list.name, list)).toEqual(list); }); }); + +/** + * [#20051] Stage (iv), ruling 甲 under letter B: a saved view stores the parsed + * value of every key its request body carried — undeclared keys dropped, + * schema defaults NOT materialised (ADR-0087's `storable` rule, the one flows + * follow). Driven through the REAL `saveMetaItem`, reading the stored row. + * + * The toolbar bodies are the console's own writes at the `.objectui-sha` pin, + * measured on the card (W1–W5): `buildPersistedViewBody`'s overlay branch plus + * the identity and marker `updateViewConfig` stamps, and a sort entry carrying + * the row id `ListView`'s header sort mints. + */ +describe('[#20051] stage (iv): a saved view stores the parsed value of every key its body carried', () => { + const OVERLAY_ID = { viewKind: 'list', object: 'crm_lead', name: 'crm_lead.all', _isOverride: true } as const; + const SORT_ROW_ID = '6f1c1e7a-0000-4000-8000-000000000001'; + const TOOLBAR_SAVES: Array<[string, Record]> = [ + ['W1 sort', { sort: [{ id: SORT_ROW_ID, field: 'name', order: 'desc' }] }], + ['W2 density', { rowHeight: 'compact' }], + ['W3 hidden fields', { hiddenFields: ['status'] }], + ['W4 column state', { columnState: { order: ['status', 'name'], widths: { name: 180 } } }], + ['W5 inline edit', { inlineEdit: false }], + ]; + + it.each(TOOLBAR_SAVES)('%s: the stored row carries no `type` the author did not write', async (_label, patch) => { + const body = { ...patch, ...OVERLAY_ID }; + // The default is real: the parse of this very body names a list type. + expect((ViewMetadataSchema as any).parse(body).type).toBe('grid'); + const stored = await storedViewBody(body.name, body); + expect(stored).not.toHaveProperty('type'); + // Everything the patch said is stored, and nothing else. + const expected = 'sort' in patch + ? { ...body, sort: [{ field: 'name', order: 'desc' }] } + : body; + expect(stored).toEqual(expected); + }); + + it('W1: a sort row id the console mints is not stored (`VIEW_CONSOLE_ROW_DECORATIONS`)', async () => { + const body = { ...TOOLBAR_SAVES[0][1], ...OVERLAY_ID }; + const stored = await storedViewBody(body.name, body); + expect(stored.sort).toEqual([{ field: 'name', order: 'desc' }]); + }); + + it('an undeclared key is dropped: the saved-view toolbar toggle\'s `id` / `objectName`, and a form-only `layout`', async () => { + // W6b's shape: `buildPersistedViewBody`'s saved-view branch writes the + // whole tab back, and the tab carries `id` and `objectName`. + const declared = { + name: 'crm_lead.my_leads', + object: 'crm_lead', + viewKind: 'list', + label: 'My leads', + type: 'grid', + columns: ['name', 'status'], + data: { provider: 'object', object: 'crm_lead' }, + isDefault: false, + }; + const stored = await storedViewBody(declared.name, { + ...declared, id: 'crm_lead.my_leads', objectName: 'crm_lead', layout: 'diagonal', + }); + expect(stored).toEqual(declared); + }); + + it('a declared key keeps its normalised value', async () => { + const stored = await storedViewBody('crm_lead.open', { + name: 'crm_lead.open', + object: 'crm_lead', + viewKind: 'list', + type: 'grid', + columns: ['name'], + exportOptions: ['csv'], + filter: [{ field: 'status', operator: 'notEquals', value: 'done' }], + }); + expect(stored.exportOptions).toEqual({ formats: ['csv'] }); + expect(stored.filter).toEqual([{ field: 'status', operator: 'not_equals', value: 'done' }]); + }); + + it('a key moved inside a moved key is stored under its canonical spelling, with no default beside it', async () => { + // Two levels of key move: `groups` → `sections`, and within each section + // and field `visibleOn` → `visibleWhen`. Neither graft covered the + // second; the projection finds both by re-parsing, so the stored row + // keeps the predicates and drops the section / field defaults. + const stored = await storedViewBody('contact_us', flatForm({ + groups: [{ + label: 'About you', + visibleOn: 'record.stage == "open"', + fields: ['name', { field: 'email', visibleOn: 'record.opted_in == true' }], + }], + })); + expect(stored).not.toHaveProperty('groups'); + expect(stored.sections).toEqual([{ + label: 'About you', + visibleWhen: { dialect: 'cel', source: 'record.stage == "open"' }, + fields: ['name', { field: 'email', visibleWhen: { dialect: 'cel', source: 'record.opted_in == true' } }], + }]); + expect(stored.sharing).toEqual(SHARING); + }); + + it('the persisted body parses to exactly what the save parsed', async () => { + const bodies: Array> = [ + ...TOOLBAR_SAVES.map(([, patch]) => ({ ...patch, ...OVERLAY_ID })), + flatForm({ groups: [SECTION], isPinned: true }), + { + name: 'lead.contact_us', object: 'lead', viewKind: 'form', label: 'Contact us', + config: { type: 'simple', data: DATA, sharing: SHARING, groups: [SECTION] }, + }, + { + name: 'lead_views', + list: { type: 'grid', data: DATA, columns: ['name'] }, + formViews: { intake: { type: 'simple', data: DATA, groups: [SECTION] } }, + }, + ]; + for (const body of bodies) { + const stored = await storedViewBody(body.name as string, body); + expect((ViewMetadataSchema as any).parse(stored), JSON.stringify(body)) + .toEqual((ViewMetadataSchema as any).parse(body)); + } + }); +}); + +/** + * [#20051] Stage (iv), Q3: a top-level `options` bag on a ViewItem RECORD is + * refused by name, with the prescription to write `config.KIND`. Before, the + * record member's `.strip()` dropped it from the parse unread while the save + * stored it; with the parsed body stored it would vanish on a `200`. + */ +describe('[#20051] stage (iv): a ViewItem record\'s top-level `options` bag is refused by name', () => { + const RECORD = { + name: 'crm_lead.board', + object: 'crm_lead', + viewKind: 'list', + label: 'Board', + config: { + type: 'kanban', + data: { provider: 'object', object: 'crm_lead' }, + columns: ['name'], + kanban: { groupByField: 'stage', columns: ['name'] }, + }, + }; + + async function refusedRecord(item: Record) { + const { protocol, rows } = makeProtocol(); + let err: any; + try { + await (protocol as any).saveMetaItem({ type: 'view', name: item.name as string, item }); + } catch (e) { err = e; } + expect(err?.code).toBe('INVALID_METADATA'); + expect(err?.status).toBe(422); + expect(Array.from(rows.values()).filter((r) => r.type === 'view')).toHaveLength(0); + const issue = (err.issues as Array<{ path: string; code?: string; message: string }>) + .find((i) => i.path === 'options'); + expect(issue, JSON.stringify(err.issues)).toBeDefined(); + return issue!; + } + + it('on the list arm: 422 at `options`, naming the move to `config.KIND`', async () => { + const issue = await refusedRecord({ ...RECORD, options: { kanban: { groupByField: 'stage' } } }); + expect(issue.message).toMatch(/^A view item record carries no top-level `options` bag/); + expect(issue.message).toContain('Move each `options.KIND` block to `config.KIND`'); + }); + + it('on the form arm too', async () => { + const issue = await refusedRecord({ + name: 'lead.contact_us', object: 'lead', viewKind: 'form', label: 'Contact us', + config: { type: 'simple', data: DATA, sections: [SECTION] }, + options: { timeline: { titleField: 'name' } }, + }); + expect(issue.message).toMatch(/^A view item record carries no top-level `options` bag/); + }); + + it('control: the same record without the bag saves, and the row is the record', async () => { + expect(await storedViewBody(RECORD.name, RECORD)).toEqual(RECORD); + }); +}); + +/** + * [#20051] Stage (iv) is gated on the `view` type. Every other type keeps its + * request body (with the two grafts), which a page shows: its parse turns a + * component's CEL string into an expression object and adds `properties: {}`, + * and neither reaches the row. + */ +describe('[#20051] stage (iv): the projection is view-only', () => { + it('a page is stored as sent, not as parsed', async () => { + const component = { type: 'record:details', visibleWhen: 'record.stage == "open"' }; + const page = { name: 'lead_record', label: 'Lead', type: 'record', object: 'lead', regions: [{ name: 'main', components: [component] }] }; + const { protocol, rows } = makeProtocol(); + const result: any = await (protocol as any).saveMetaItem({ type: 'page', name: page.name, item: page }); + expect(result.success, JSON.stringify(result)).toBe(true); + const row = Array.from(rows.values()).find((r) => r.type === 'page'); + const stored = JSON.parse(row!.metadata); + expect(stored.regions[0].components[0]).toEqual(component); + }); +}); + +/** + * [#20051] The save's fail-safe arm, driven through the REAL `saveMetaItem`: + * when {@link projectStorableViewBody} does not converge, the view is stored + * as its whole parse output and a warning is logged ONCE per view name. + * + * No `view` schema reaches that arm today, so the seam is the production API + * a plugin uses to replace a type's schema, `registerMetadataTypeSchema` — + * never a test-only hook. The stand-in schema's parse is not idempotent (it + * bumps a counter on every parse), so no body re-parses to what the save + * parsed. The built-in `view` schema is registered back in `finally`. + */ +describe('[#20051] stage (iv): the non-converging fallback stores the parse output and warns once per view', () => { + const nonConverging = z.object({ + name: z.string(), + object: z.string(), + viewKind: z.literal('list'), + n: z.number().transform((v) => v + 1), + }); + const FALLBACK_WARNING = 'no body of only the keys the request carried re-parses to the same view'; + + it('two saves of one view warn once; a second view warns on its own', async () => { + const builtin = getMetadataTypeSchema('view')!; + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + registerMetadataTypeSchema('view', nonConverging); + try { + const { protocol, rows } = makeProtocol(); + const save = (name: string, n: number) => (protocol as any).saveMetaItem({ + type: 'view', name, item: { name, object: 'crm_lead', viewKind: 'list', n }, + }); + await save('crm_lead.a', 1); + await save('crm_lead.a', 5); + const fallbackWarnings = () => warn.mock.calls + .map((call) => String(call[0])) + .filter((line) => line.includes(FALLBACK_WARNING)); + expect(fallbackWarnings()).toHaveLength(1); + expect(fallbackWarnings()[0]).toContain('view/crm_lead.a'); + // The row is the whole parse output of the LAST save — not the + // authored body (`n: 5`), and no key lost. + const row = Array.from(rows.values()).find((r) => r.type === 'view' && r.name === 'crm_lead.a'); + expect(JSON.parse(row!.metadata)).toEqual({ name: 'crm_lead.a', object: 'crm_lead', viewKind: 'list', n: 6 }); + + await save('crm_lead.b', 1); + expect(fallbackWarnings()).toHaveLength(2); + expect(fallbackWarnings()[1]).toContain('view/crm_lead.b'); + } finally { + registerMetadataTypeSchema('view', builtin); + warn.mockRestore(); + } + expect(getMetadataTypeSchema('view')).toBe(builtin); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts b/packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts index b2a445ec8fc..c79df0b955c 100644 --- a/packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts +++ b/packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts @@ -10,6 +10,11 @@ * table that keeps growing can never be retired. * * `graftNormalizedOperators` copies the normalization back on and nothing else. + * [#20051] A `view` save no longer runs it: a view stores the parsed value of + * every key its body carried (`projectStorableViewBody`), which normalises the + * operator the same way. The graft serves every other type, and these helper + * pins keep using view bodies because `ViewMetadataSchema` is the schema whose + * operator normalisation the graft was written for. * The tests below pin both halves of that claim: operators DO change, and * everything else — auxiliary fields, unknown keys, `$`-token conditions, array * order, absent optionals — does NOT. @@ -124,7 +129,8 @@ describe('graftNormalizedOperators — through the real view metadata schema', ( describe('graftNormalizedOperators — what it must never touch', () => { it('keeps Studio-only auxiliary fields a `parsed.data` swap would strip', () => { - // The exact reason saveMeta persists verbatim (ADR-0005 §Validation). + // Why the graft, not a `parsed.data` swap, is the helper's contract (ADR-0005 + // appendix (c) for every type but `view`). const authored = view( [{ field: 'status', operator: 'gt', value: 1 }], { isPinned: true, isDefault: false, sortOrder: 3 }, diff --git a/packages/metadata-protocol/src/protocol.project-storable-view-body.test.ts b/packages/metadata-protocol/src/protocol.project-storable-view-body.test.ts new file mode 100644 index 00000000000..602def565c1 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.project-storable-view-body.test.ts @@ -0,0 +1,169 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20051] `projectStorableViewBody`, driven directly: the `view` write door's + * storable body, with the re-parse injected, so no save and no schema is + * involved. + * + * The save-path pins, on real `view` bodies, live in + * `protocol.graft-folded-form-sections.test.ts`, where they ride that file's + * pinned engine double. This file pins the helper's own arms. It pins the + * `converged: false` fail-safe above all: no `view` schema reaches that arm + * today, so nothing else would notice if it regressed into storing the + * authored body, or into dropping keys. + * + * The injected `reparse` stands in for the schema's `safeParse`: it returns + * `{ ok: true, data }` for an accepted body and `{ ok: false }` for a refused + * one. Each stub below is a small, explicit model of one behaviour a parse can + * have: applying a default, folding a key, refusing, or never settling. + */ +import { describe, expect, it } from 'vitest'; +import { projectStorableViewBody } from './protocol.js'; + +type Reparse = Parameters[2]; + +/** Wraps a stub parse function and counts how often the projection calls it. */ +function counted(parse: (body: Record) => ReturnType) { + const calls: unknown[] = []; + const reparse: Reparse = (body) => { + calls.push(body); + return parse(body as Record); + }; + return { reparse, calls }; +} + +/** + * A parse that applies `defaults` to every key the body omits and folds + * `moves` (old key → canonical key) the way `groups` → `sections` folds. + */ +function modelParse(opts: { defaults?: Record; moves?: Record }) { + return (body: Record): ReturnType => { + const out: Record = { ...body }; + for (const [from, to] of Object.entries(opts.moves ?? {})) { + if (out[from] !== undefined) { + if (out[to] === undefined) out[to] = out[from]; + delete out[from]; + } + } + for (const [key, value] of Object.entries(opts.defaults ?? {})) { + if (out[key] === undefined) out[key] = value; + } + return { ok: true, data: out }; + }; +} + +describe('projectStorableViewBody — the converged arms', () => { + it('(c) a key the parse adds and the re-parse reproduces is a default, and is not stored', () => { + const parse = modelParse({ defaults: { type: 'grid' } }); + const authored = { name: 'crm_lead.all', sort: [{ field: 'name', order: 'asc' }] }; + const parsed = (parse(authored) as { data: unknown }).data; + expect(parsed).toHaveProperty('type', 'grid'); + const { reparse, calls } = counted(parse); + + const out = projectStorableViewBody(authored, parsed, reparse); + + expect(out.converged).toBe(true); + expect(out.body).not.toHaveProperty('type'); + expect(out.body).toEqual(authored); + // One verify parse, no restoration round: the common case's cost. + expect(calls).toHaveLength(1); + }); + + it('(d) a key the parse adds and the re-parse does NOT reproduce is a moved key, stored under its canonical spelling', () => { + const parse = modelParse({ moves: { visibleOn: 'visibleWhen' } }); + const authored = { name: 'contact_us', visibleOn: 'record.a == 1' }; + const parsed = (parse(authored) as { data: unknown }).data; + const { reparse } = counted(parse); + + const out = projectStorableViewBody(authored, parsed, reparse); + + expect(out.converged).toBe(true); + expect(out.body).toEqual({ name: 'contact_us', visibleWhen: 'record.a == 1' }); + expect(out.body).not.toHaveProperty('visibleOn'); + }); + + it('(d) a moved container is projected against the key it moved from: its nested defaults are not stored', () => { + // `groups` → `sections`, where each section gains a `collapsed` default + // the author did not write. + const sectionDefaults = (s: Record) => ({ collapsed: false, ...s }); + const reparse: Reparse = (body) => { + const b = body as Record; + const { groups, ...rest } = b; + const sections = (rest.sections ?? groups) as Array> | undefined; + return { ok: true, data: sections ? { ...rest, sections: sections.map(sectionDefaults) } : rest }; + }; + const authored = { name: 'contact_us', groups: [{ label: 'About you', fields: ['name'] }] }; + const parsed = (reparse(authored) as { data: unknown }).data; + expect(parsed).toEqual({ name: 'contact_us', sections: [{ collapsed: false, label: 'About you', fields: ['name'] }] }); + + const out = projectStorableViewBody(authored, parsed, reparse); + + expect(out.converged).toBe(true); + expect(out.body).toEqual({ name: 'contact_us', sections: [{ label: 'About you', fields: ['name'] }] }); + }); + + it('an authored key is stored with its PARSED value; a value whose shape the parse changed is stored as parsed', () => { + const parsed = { operator: 'not_equals', exportOptions: { formats: ['csv'] }, l: [1] }; + const out = projectStorableViewBody( + { operator: 'notEquals', exportOptions: ['csv'], l: [1, 2], undeclared: true }, + parsed, + (body) => ({ ok: true, data: body }), + ); + expect(out).toEqual({ body: parsed, converged: true }); + }); +}); + +describe('projectStorableViewBody — the converged: false fail-safe', () => { + // Authored and parsed differ in every way the arm could get wrong: a + // normalised value, a dropped undeclared key, and an added default. + const authored = { name: 'crm_lead.all', operator: 'notEquals', undeclared: 1 }; + const parsed = { name: 'crm_lead.all', operator: 'not_equals', type: 'grid' }; + + it('(a) a re-parse that refuses the body on the first round: the stored body is the parse output, not the authored body', () => { + const { reparse, calls } = counted(() => ({ ok: false })); + + const out = projectStorableViewBody(authored, parsed, reparse); + + expect(out.converged).toBe(false); + expect(out.body).toEqual(parsed); + expect(out.body).not.toEqual(authored); + expect(out.body).not.toHaveProperty('undeclared'); + expect(calls).toHaveLength(1); + }); + + it('(b) a re-parse that never reproduces the parse and makes no progress: converged false, bounded, no throw', () => { + const { reparse, calls } = counted((body) => ({ ok: true, data: { ...body, operator: 'drifted' } })); + + let out: ReturnType | undefined; + expect(() => { out = projectStorableViewBody(authored, parsed, reparse); }).not.toThrow(); + + expect(out!.converged).toBe(false); + expect(out!.body).toEqual(parsed); + // A round that adds nothing ends the loop at once. + expect(calls.length).toBeGreaterThanOrEqual(1); + expect(calls.length).toBeLessThanOrEqual(8); + }); + + it('(b) a re-parse that reveals one more needed key every round stops at the round bound, not at convergence', () => { + // Twenty keys the parse adds. Each re-parse withholds the first one the + // body does not carry yet, so every round finds exactly one "moved" + // key and adds it. Convergence would take 21 rounds; the bound + // (`STORABLE_VIEW_MAX_ROUNDS`, 8) must stop it first. + const keys = Array.from({ length: 20 }, (_, i) => `k${String(i).padStart(2, '0')}`); + const wide: Record = { name: 'crm_lead.all' }; + for (const k of keys) wide[k] = k.toUpperCase(); + const { reparse, calls } = counted((body) => { + const missing = keys.find((k) => body[k] === undefined); + const data: Record = { ...wide }; + if (missing) delete data[missing]; + return { ok: true, data }; + }); + + const out = projectStorableViewBody({ name: 'crm_lead.all' }, wide, reparse); + + expect(out.converged).toBe(false); + expect(out.body).toEqual(wide); + expect(calls.length).toBeLessThanOrEqual(8); + expect(calls.length).toBeGreaterThan(1); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.read-decorations.test.ts b/packages/metadata-protocol/src/protocol.read-decorations.test.ts index 1da14cf24e8..c47b81adf42 100644 --- a/packages/metadata-protocol/src/protocol.read-decorations.test.ts +++ b/packages/metadata-protocol/src/protocol.read-decorations.test.ts @@ -4,9 +4,10 @@ * #4326 — read decorations must not round-trip into the persisted body. * * `getMetaItem`/`getMetaItems` stamp `_diagnostics` on every served document - * (and `_draft` on draft reads), while the write path persists the request body - * VERBATIM by design (ADR-0005 §Validation — `parsed.data` would strip - * Studio-only auxiliary fields). The standard Studio round-trip therefore used + * (and `_draft` on draft reads), while the write path persisted the request body + * VERBATIM by design (ADR-0005 appendix (c); since #20051 only for types other + * than `view`, which stores its parsed body — undeclared keys, these among + * them, never reach the row). The standard Studio round-trip therefore used * to bake a read-time verdict into `sys_metadata.metadata`, into its checksum, * and into every history diff. * diff --git a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts index a13868f96ad..32f19d5b498 100644 --- a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts +++ b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts @@ -780,9 +780,9 @@ describe('runtime authoring gate on OBJECT writes (#4716)', () => { // THE MEASUREMENT THIS BLOCK EXISTS TO TAKE. The entry's `surfaceReason` held // the crossing back on an open question: does the gate's `body` reach the rule // BEFORE the per-type `safeParse`, whose residue stage strips the only evidence -// the rule reads? It does — and not by luck. `saveMetaItem` keeps the AUTHORED -// body verbatim on purpose (`parsed.data` would strip the Studio-only auxiliary -// fields an overlay rides with) and grafts back exactly two normalizations, +// the rule reads? It does — and not by luck. For every type but `view` (a +// permission set among them; a `view` stores its parsed body since #20051), +// `saveMetaItem` keeps the AUTHORED body verbatim and grafts back exactly two normalizations, // each a walk over the authored keys that adds nothing and drops nothing else. // So the residue is still there at the gate call, and the persisted row proves // it from the other side. Post-parse the rule would indeed be structurally diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 8eeae62ea43..e2c307e8c6d 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -364,7 +364,8 @@ function governServedItem(type: string, item: T): T { * * Exactly the shape, and exactly the reason, of the `stripReadDecorations` call * beside it in `saveMetaItem` (#4326) — the write path persists the request body - * verbatim by design (ADR-0005 §Validation), so anything the READ adds must come + * verbatim by design for every type but `view` (ADR-0005 appendix (c), the + * "Addendum — 2026-05-16 (c)"), so anything the READ adds must come * off again on the way in or it is baked into `sys_metadata.metadata`, into its * checksum, and into every history diff. Kept a SEPARATE strip from that one * rather than folded into `METADATA_READ_DECORATIONS`, because the two lists are @@ -691,14 +692,21 @@ const HAND_CRAFTED_SCHEMAS: Record> = { * * Validation policy: * - `safeParse` is used so we can craft a 422 with structured `issues`. - * - We do NOT replace the persisted document with `parsed.data`; the - * original payload is stored verbatim so Studio-only auxiliary fields - * (e.g. `isPinned`, `isDefault`, `sortOrder`) survive the round-trip. - * The one exception is filter `operator` spellings, which are grafted back - * from `parsed.data` so a save stops minting new legacy-alias rows — see - * {@link graftNormalizedOperators}. [#20101] A page that omits `type` is - * stored with the default `PageSchema` declares for it — see - * {@link withDeclaredPageTypeDefault}. + * - [#20051] What is persisted depends on the type — ADR-0005 appendix (c), + * the "Addendum — 2026-05-16 (c): spec validation on overlay save": + * - `view` stores the parsed value of every key its body carried: + * undeclared keys are dropped, schema defaults are not materialised + * (ADR-0087's `storable` rule, the one flows follow). Every key the + * console reads back off a stored row is declared on the contract + * (`VIEW_CONSOLE_ROUND_TRIP_KEYS`), so the parse keeps it. See + * {@link projectStorableViewBody}. + * - Every other type stores its original payload verbatim, with two + * normalizations grafted back from `parsed.data` so a save stops + * minting legacy-alias rows — filter `operator` spellings + * ({@link graftNormalizedOperators}) and the form `groups` → `sections` + * move ({@link graftFoldedFormSections}). [#20101] A page that omits + * `type` is stored with the default `PageSchema` declares for it — see + * {@link withDeclaredPageTypeDefault}. * - Types without a registered schema (the wiring-layer types * `function`/`service`/`router`, and any plugin types that have not * yet called `registerMetadataTypeSchema()`) fall through unvalidated. @@ -1329,14 +1337,174 @@ export function graftFoldedFormSections(authored: unknown, parsed: unknown): unk return rest.sections !== undefined ? rest : { ...rest, sections: groups }; } +/** + * How many verify-and-restore rounds {@link projectStorableViewBody} may take. + * One round per level of key-moved nesting is what convergence costs: a + * `groups` → `sections` fold whose sections carry a `visibleOn` → + * `visibleWhen` fold is two levels. A `view` body is a bounded document, so a + * body still diverging after this many rounds is not converging at all. + */ +const STORABLE_VIEW_MAX_ROUNDS = 8; + +/** A JSON object — not an array, not `null`. */ +function isPlainRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** + * Equality as the store sees it: `JSON.stringify` semantics, so a key whose + * value is `undefined` is the same as an absent key, and key ORDER does not + * count (the list overlay's `type` default is re-applied in declaration + * position, so a re-parse can reorder keys it did not change). + */ +function storableEqual(a: unknown, b: unknown): boolean { + if (a === b) return true; + if (Array.isArray(a) || Array.isArray(b)) { + if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) return false; + return a.every((entry, i) => storableEqual(entry, b[i])); + } + if (isPlainRecord(a) && isPlainRecord(b)) { + const keysA = Object.keys(a).filter((k) => a[k] !== undefined); + const keysB = Object.keys(b).filter((k) => b[k] !== undefined); + if (keysA.length !== keysB.length) return false; + return keysA.every((k) => storableEqual(a[k], b[k])); + } + return false; +} + +/** + * The one authored key, among those the parse did not keep at this position, + * whose value has the shape of `value` — the key a moved key was moved FROM. + * `undefined` when there is none, or more than one. + */ +function soleMovedFrom(dropped: readonly string[], authored: Record, value: unknown): string | undefined { + const sameShape = (candidate: unknown): boolean => { + if (Array.isArray(value)) return Array.isArray(candidate) && candidate.length === value.length; + if (isPlainRecord(value)) return isPlainRecord(candidate); + return typeof candidate === typeof value && !isPlainRecord(candidate) && !Array.isArray(candidate); + }; + const matches = dropped.filter((key) => sameShape(authored[key])); + return matches.length === 1 ? matches[0] : undefined; +} + +/** + * One round of {@link projectStorableViewBody}: the storable node at one + * position, built from the authored node, the parsed node, and — from the + * second round on — the re-parse of the previous round's body (`reparsed`) and + * that body itself (`previous`). + * + * - A key the author wrote and the parse kept is stored with its PARSED value, + * recursively. That is what normalises (`notEquals` → `not_equals`, + * `exportOptions: ['csv']` → `{ formats: ['csv'] }`) without storing a + * default a nested parse added. + * - A key the author wrote and the parse did not keep is not stored: an + * undeclared key, a console decoration, or the old spelling of a moved key. + * - A key the parse added is stored only on evidence. The previous round's body + * did not carry it, so if the re-parse of that body produced the same value, + * the parse applies it by itself — a schema DEFAULT, left to the parse + * (ADR-0087's `storable` rule). If the re-parse did NOT reproduce it, the value + * came from something the author wrote under another key — a MOVED key + * (`groups` → `sections`, `visibleOn` → `visibleWhen`) — and it is stored, + * projected against the authored key it moved from when that key is the only + * one of its shape, and stored whole otherwise. Once stored it stays stored + * in later rounds, so the rounds only ever add. + * - Anything else — a value whose shape the parse changed (`['csv']` → + * `{ formats }`), an array whose length it changed, a leaf — is stored as + * parsed. + */ +function buildStorableViewNode(authored: unknown, parsed: unknown, reparsed: unknown, previous: unknown): unknown { + if (isPlainRecord(parsed) && isPlainRecord(authored)) { + const a = authored; + const r = isPlainRecord(reparsed) ? reparsed : undefined; + const prev = isPlainRecord(previous) ? previous : undefined; + const out: Record = {}; + const dropped: string[] = []; + for (const [key, value] of Object.entries(a)) { + if (value === undefined) continue; + if (parsed[key] === undefined) { + dropped.push(key); + continue; + } + out[key] = buildStorableViewNode(value, parsed[key], r?.[key], prev?.[key]); + } + for (const [key, value] of Object.entries(parsed)) { + if (value === undefined || key in out || a[key] !== undefined) continue; + const storedBefore = prev !== undefined && prev[key] !== undefined; + if (!storedBefore && (r === undefined || storableEqual(r[key], value))) continue; + // Projected against the key it moved from when that key is the only + // one of its shape; otherwise (a `visibleOn` string folded into a + // `visibleWhen` expression object) the parsed value is the moved + // key's value, and it is stored whole. + const from = soleMovedFrom(dropped, a, value); + out[key] = from === undefined ? value : buildStorableViewNode(a[from], value, r?.[key], prev?.[key]); + } + return out; + } + if (Array.isArray(parsed) && Array.isArray(authored) && authored.length === parsed.length) { + const r = Array.isArray(reparsed) && reparsed.length === parsed.length ? reparsed : undefined; + const prev = Array.isArray(previous) && previous.length === parsed.length ? previous : undefined; + return parsed.map((entry, i) => buildStorableViewNode(authored[i], entry, r?.[i], prev?.[i])); + } + return parsed; +} + +/** + * [#20051] What the `view` write door STORES: the parsed value of every key the + * request body carried — undeclared keys dropped, schema defaults NOT + * materialised. Ruling 甲's end state ("a saved view is the parsed body") under + * letter B of its follow-up ruling, which puts views under the rule ADR-0087 + * (addendum 2026-08-01b) records for flows: `storable` excludes schema + * defaults, so a stored row never pins the day's default. + * + * It replaces, for `view`, the two grafts that ran over the verbatim body — + * {@link graftNormalizedOperators} (a value normalised in place) and + * {@link graftFoldedFormSections} (a key moved). Both are special cases of it: + * a parsed value is stored for every authored key, and a moved key is found by + * evidence rather than by name, so `visibleOn` → `visibleWhen` (never grafted) + * is covered too. Every other type keeps the two grafts over its verbatim body. + * + * ## Why a re-parse, and what it proves + * + * The parse output alone cannot say which of the keys it added are defaults and + * which are moved keys: `sections` folded from `groups` and `collapsible` + * defaulted to `false` look the same. So the body built from the authored keys + * is parsed again, and a key the parse added is kept exactly when that + * re-parse does NOT reproduce it (see {@link buildStorableViewNode}). The loop + * ends when the re-parse of the stored body EQUALS the parse of the request + * body — the invariant the stored row is held to: a GET → PUT of it is judged + * the same, and every reader that parses it sees what the save accepted. + * + * `converged: false` — the loop found no body meeting that invariant (a + * re-parse refused an intermediate body, or a round added nothing). The caller + * then stores `parsed` itself: no key is lost, only that row's defaults are + * pinned. Nothing in the `view` schema reaches that arm today; it is the + * fail-safe for a schema change that makes one of the rounds' bodies invalid. + */ +export function projectStorableViewBody( + authored: unknown, + parsed: unknown, + reparse: (body: unknown) => { ok: true; data: unknown } | { ok: false }, +): { body: unknown; converged: boolean } { + let candidate = buildStorableViewNode(authored, parsed, undefined, undefined); + for (let round = 0; round < STORABLE_VIEW_MAX_ROUNDS; round++) { + const again = reparse(candidate); + if (!again.ok) break; + if (storableEqual(again.data, parsed)) return { body: candidate, converged: true }; + const next = buildStorableViewNode(authored, parsed, again.data, candidate); + if (storableEqual(next, candidate)) break; + candidate = next; + } + return { body: parsed, converged: false }; +} + /** * [#20101] Give a page body that omits `type` the default `PageSchema` * declares for it, and change nothing else. * * `PageSchema` declares `type: PageTypeSchema.default('record')`, so a page * authored without `type` IS a record page. The save gate parses with that - * default and then persists the authored body verbatim (ADR-0005 - * §"Validation", {@link resolveOverlaySchema}), and every read serves a stored + * default and then persists the authored body verbatim (ADR-0005 appendix + * (c), {@link resolveOverlaySchema}), and every read serves a stored * row without parsing it. A record page written the natural way, with the * defaulted key left out, therefore reached every reader of the served body * with no `type` at all. A consumer that picks an object's record page by @@ -4716,6 +4884,14 @@ export class ObjectStackProtocolImplementation implements */ private flowCanonicalizeFallbackWarned = new Set(); + /** + * [#20051] Once-per-process dedupe (`type|name`) for the warning + * `saveMetaItem` emits when {@link projectStorableViewBody} does not converge + * and a `view` is stored as its whole parse output. Same reason as the flow + * dedupe above: Studio autosaves the same draft over and over. + */ + private storableViewFallbackWarned = new Set(); + /** * Canonicalize a stored `sys_metadata` body on rehydration (#3903; * ADR-0087 addendum "stored metadata replays the chain"). @@ -7336,7 +7512,8 @@ export class ObjectStackProtocolImplementation implements * [#8268, generalising #8038] The write-side counterpart of * {@link materializeFromRegistry}, owed for the reason * {@link stripServedSystemColumns} is owed one field family over: the write - * path persists the request body verbatim (ADR-0005 §Validation), so a + * path persists an `object`'s request body verbatim (ADR-0005 appendix + * (c), the "Addendum — 2026-05-16 (c)"), so a * document this service's read added a stamp to would otherwise be handed * straight back and stored carrying it. * @@ -16144,7 +16321,9 @@ export class ObjectStackProtocolImplementation implements // stays something the server states, not something a caller claims. const writeSource = request.source ?? 'protocol.saveMetaItem'; // Drop OUR OWN read decorations before anything reads the body (#4326). - // The write path persists verbatim by design (ADR-0005 §Validation), so + // The write path persists verbatim by design for every type but `view` + // (ADR-0005 appendix (c), the "Addendum — 2026-05-16 (c)"), and a view + // is gated on this same body before it is projected, so // the standard Studio round-trip — GET (decorated) → edit → PUT the whole // body — would otherwise bake a read-time verdict into the row, its // checksum, and every history diff. See {@link stripReadDecorations} for @@ -16590,10 +16769,12 @@ export class ObjectStackProtocolImplementation implements // overlay type (see OVERLAY_VALIDATION_SCHEMAS), validate the payload // before persisting. We surface invalid payloads as `422 // invalid_metadata` with structured Zod issues so the Studio form can - // highlight the offending field. The original `item` is kept verbatim - // — `parsed.data` would strip Studio-only auxiliary fields (e.g. - // isPinned, isDefault, sortOrder) that intentionally ride along with - // the overlay document. ADR-0005 §"Validation". + // highlight the offending field. What is then STORED depends on the + // type (ADR-0005 appendix (c), "Addendum — 2026-05-16 (c): spec + // validation on overlay save"): a `view` stores the parsed value of + // every key its body carried and nothing else + // ({@link projectStorableViewBody}, ADR-0087's `storable` rule); every + // other type keeps its request body, with the grafts below. // // [#5364] "so the Studio form can highlight the offending field" was // the promise; a top-level `z.union` broke it. Mapping only the issues @@ -16633,25 +16814,53 @@ export class ObjectStackProtocolImplementation implements (err as any).issues = issues; throw err; } - // Keep the body verbatim, but not its *legacy spellings*: the - // schema just folded them to canonical and the result would - // otherwise be discarded, so every save minted new alias rows. - // Two normalizations are grafted back, each by its own walk — - // filter `operator` values ({@link graftNormalizedOperators}, - // objectui#2945) and the form `groups` → `sections` key move - // ({@link graftFoldedFormSections}, #7134). A key move is not - // expressible in the scalar walk, which is why there are two. - // - // The fold runs FIRST so the operator walk meets `sections` - // lined up with the parsed tree rather than a `groups` key the - // parsed side no longer has. Form sections carry no `operator` - // today (`visibleWhen` is a CEL string), so this ordering is - // structural hygiene rather than a measured fix — but it is the - // ordering that stays correct if one ever does. - request.item = graftNormalizedOperators( - graftFoldedFormSections(request.item, parsed.data), - parsed.data, - ); + if (singularType === 'view') { + // [#20051] A saved view is the parsed body: the parsed + // value of every key the request carried, undeclared keys + // dropped, schema defaults left to the parse. The two + // grafts below are special cases of this projection, so a + // view does not run them. See {@link projectStorableViewBody}. + const storable = projectStorableViewBody(request.item, parsed.data, (body) => { + const again = schema.safeParse(body); + return again.success ? { ok: true, data: again.data } : { ok: false }; + }); + if (!storable.converged) { + // Functional, not durability: the fallback stores the + // whole parse output, so nothing the author wrote is + // lost; only this row's schema defaults are pinned. + const key = `${singularType}|${request.name}`; + if (!this.storableViewFallbackWarned.has(key)) { + this.storableViewFallbackWarned.add(key); + console.warn( + `[Protocol] view/${request.name}: the stored body is the whole parse output, schema ` + + `defaults included, because no body of only the keys the request carried re-parses ` + + `to the same view. Nothing the author wrote is lost; the defaults in this row no ` + + `longer follow the schema's. Re-save the view after a platform upgrade to re-derive it.`, + ); + } + } + request.item = storable.body; + } else { + // Every other type keeps its body verbatim, but not its *legacy spellings*: the + // schema just folded them to canonical and the result would + // otherwise be discarded, so every save minted new alias rows. + // Two normalizations are grafted back, each by its own walk — + // filter `operator` values ({@link graftNormalizedOperators}, + // objectui#2945) and the form `groups` → `sections` key move + // ({@link graftFoldedFormSections}, #7134). A key move is not + // expressible in the scalar walk, which is why there are two. + // + // The fold runs FIRST so the operator walk meets `sections` + // lined up with the parsed tree rather than a `groups` key the + // parsed side no longer has. Form sections carry no `operator` + // today (`visibleWhen` is a CEL string), so this ordering is + // structural hygiene rather than a measured fix — but it is the + // ordering that stays correct if one ever does. + request.item = graftNormalizedOperators( + graftFoldedFormSections(request.item, parsed.data), + parsed.data, + ); + } // [#20101] …and a page's declared `type` default, the one // default that a reader of the served body selects on. It is // stored so that the served document and the stored row are diff --git a/packages/objectql/src/protocol-meta.test.ts b/packages/objectql/src/protocol-meta.test.ts index a8b407862ee..f62eee1f31f 100644 --- a/packages/objectql/src/protocol-meta.test.ts +++ b/packages/objectql/src/protocol-meta.test.ts @@ -583,10 +583,14 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => { expect(mockEngine.insert).toHaveBeenCalled(); }); - it('preserves Studio-only auxiliary fields verbatim (not stripped)', async () => { - // isPinned / isDefault / sortOrder are not in ListViewSchema; - // we must NOT replace the persisted document with parsed.data, - // or these fields would be silently dropped on every save. + it('[#20051] stores the console\'s declared round-trip keys, and drops the `objectName` alias', async () => { + // Re-judged at stage (iv) of #20051: was "preserves Studio-only + // auxiliary fields verbatim". A saved view now stores the parsed + // value of every key its body carried. `isPinned` / `isDefault` / + // `sortOrder` survive because the list overlay DECLARES them + // (`VIEW_CONSOLE_ROUND_TRIP_KEYS`); `objectName` is the + // console's alias of `object`, which the member does not + // declare, so it is dropped and `object` carries the binding. const itemWithExtras = { ...validView, isPinned: true, @@ -603,10 +607,10 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => { const insertCall = mockEngine.insert.mock.calls[0]; const persisted = JSON.parse(insertCall[1].metadata); - expect(persisted.isPinned).toBe(true); - expect(persisted.isDefault).toBe(false); - expect(persisted.sortOrder).toBe(5); - expect(persisted.objectName).toBe('lead'); + const { objectName: _alias, ...declared } = itemWithExtras; + expect(persisted).toEqual(declared); + expect(persisted).not.toHaveProperty('objectName'); + expect(persisted.object).toBe('lead'); }); it('rejects an invalid view (container with a listView missing `columns`) with 422', async () => { diff --git a/packages/objectql/src/protocol-publish-rollback.test.ts b/packages/objectql/src/protocol-publish-rollback.test.ts index 28d401b7efe..8b14beb2155 100644 --- a/packages/objectql/src/protocol-publish-rollback.test.ts +++ b/packages/objectql/src/protocol-publish-rollback.test.ts @@ -337,13 +337,17 @@ describe('publishMetaItem / rollbackMetaItem / diffMetaItem', () => { }); await protocol.saveMetaItem({ type: 'view', name: 'case_grid', organizationId: 'org_alpha', - item: { name: 'case_grid', type: 'grid', label: 'B', columns: ['id', 'title'], extra: 1, object: 'case', viewKind: 'list' }, // [#7741] the inline arm requires the object binding pair + item: { name: 'case_grid', type: 'grid', label: 'B', columns: ['id', 'title'], rowHeight: 'compact', extra: 1, object: 'case', viewKind: 'list' }, // [#7741] the inline arm requires the object binding pair }); const diff = await protocol.diffMetaItem({ type: 'view', name: 'case_grid', organizationId: 'org_alpha', fromVersion: 1, toVersion: 2, }); - expect((diff as any).added.map((e: any) => e.path)).toContain('extra'); + // [#20051] Re-judged at stage (iv): the added key is a DECLARED one + // (`rowHeight`). The undeclared `extra` never reaches a version — a + // view stores the parsed value of every key its body carried — so no + // diff can show it. + expect((diff as any).added.map((e: any) => e.path)).toEqual(['rowHeight']); expect((diff as any).changed.map((e: any) => e.path).sort()).toEqual(['columns', 'label']); }); }); diff --git a/packages/objectql/src/protocol-view-identity-overlay.test.ts b/packages/objectql/src/protocol-view-identity-overlay.test.ts index 416fd758ef4..706947ec07a 100644 --- a/packages/objectql/src/protocol-view-identity-overlay.test.ts +++ b/packages/objectql/src/protocol-view-identity-overlay.test.ts @@ -154,8 +154,11 @@ describe('view overlay identity (#2555)', () => { expect(persisted.object).toBe('showcase_task'); expect(persisted.label).toBe('All Tasks'); expect(persisted.name).toBe('showcase_task.default'); - // …and the personalization survives untouched. - expect(persisted.sort).toEqual(personalization.sort); + // …and the personalization survives. [#20051] Minus the sort row's + // `id`: a console row key (`VIEW_CONSOLE_ROW_DECORATIONS`) the save + // strips before the parse, and a view now stores its parsed body. + expect(persisted.sort).toEqual([{ field: 'estimate_hours', order: 'desc' }]); + expect(persisted.columns).toEqual(personalization.columns); }); it('read path: getMetaItems heals a pre-fix identity-less overlay row from the shadowed entry', async () => { diff --git a/packages/spec/src/migrations/entries/semantic/18.view-item-options-bag-refused.ts b/packages/spec/src/migrations/entries/semantic/18.view-item-options-bag-refused.ts new file mode 100644 index 00000000000..a1b8229b338 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.view-item-options-bag-refused.ts @@ -0,0 +1,39 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// Stage (iv) of ruling 甲 on #20051. A write-time narrowing of the ViewItem wire +// member: a stored record it newly refuses is read and served exactly as before +// and fails only on its next save, which is why this is a semantic entry and +// not a conversion — whether a record's top-level bag duplicates, contradicts +// or extends its `config` blocks is a fact only its author holds. +export const entry: SemanticMigration = { + id: 'view-item-options-bag-refused', + surface: + 'A top-level `options` bag on a `view` item RECORD (`{ name, object, viewKind, config }`) saved through ' + + 'the metadata write door (`PUT /api/v1/meta/view/:name`, the Studio / MCP save): `options.kanban`, ' + + '`options.calendar`, `options.timeline` and every other key in it, on either `viewKind`.', + replacement: + 'The same per-kind blocks under the record\'s `config` — `options.kanban` becomes `config.kanban`, ' + + '`options.timeline` becomes `config.timeline` — where each is judged by its kind\'s own block schema, ' + + 'and a key that block does not declare is re-spelled or deleted as its refusal says. A key `config` ' + + 'already sets wins; the bag\'s copy is deleted. The flattened list overlay (no `config`) keeps its ' + + 'legacy `options` bag, judged key by key, as before.', + reason: + 'The record member re-opens its top level with a strip so the console\'s round-trip keys survive, and ' + + 'that strip dropped a top-level `options` bag from the parse without looking inside it, while the save ' + + 'stored the request body. The console reads a record\'s body from `config` on the object page but ' + + 'spreads the whole stored record on the interface page, so the same saved view rendered two ways. ' + + 'Now that the save stores the parsed body, the bag would instead vanish silently on the next save. ' + + 'The maintainer\'s ruling of 2026-09-30 refuses it by name with the prescription to ' + + 'write `config.KIND`; declaring it would have kept a second spelling of one block on a second member. ' + + 'No console write puts the bag on a record. Not convertible: which of two spellings of one block the ' + + 'author meant, where both are set, is the author\'s call.', + acceptanceCriteria: + 'Every stored `view` record carrying a top-level `options` saves again after its blocks move under ' + + '`config`. A record that still carries the bag is refused `422 INVALID_METADATA` on its next save, with ' + + 'an issue at `options` whose message prescribes `config.KIND`. Nothing is rewritten on read and nothing ' + + 'is refused on read: a record that fails is served exactly as stored until it is saved. Verify by ' + + 're-saving each stored record that carries `options` (a GET then a PUT of the same body) and reading ' + + 'a `200`.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index e3e761e3dbf..12971175a78 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -18319,6 +18319,41 @@ const step18: MigrationStep = { + 'equals, and operator: "in" with value: ["won"] — select the same rows, so the result ' + 'set cannot tell you which the metadata meant, and only the author knows.', }, + // Stage (iv) of ruling 甲 on #20051. A write-time narrowing of the ViewItem wire + // member: a stored record it newly refuses is read and served exactly as before + // and fails only on its next save, which is why this is a semantic entry and + // not a conversion — whether a record's top-level bag duplicates, contradicts + // or extends its `config` blocks is a fact only its author holds. + { + id: 'view-item-options-bag-refused', + surface: + 'A top-level `options` bag on a `view` item RECORD (`{ name, object, viewKind, config }`) saved through ' + + 'the metadata write door (`PUT /api/v1/meta/view/:name`, the Studio / MCP save): `options.kanban`, ' + + '`options.calendar`, `options.timeline` and every other key in it, on either `viewKind`.', + replacement: + 'The same per-kind blocks under the record\'s `config` — `options.kanban` becomes `config.kanban`, ' + + '`options.timeline` becomes `config.timeline` — where each is judged by its kind\'s own block schema, ' + + 'and a key that block does not declare is re-spelled or deleted as its refusal says. A key `config` ' + + 'already sets wins; the bag\'s copy is deleted. The flattened list overlay (no `config`) keeps its ' + + 'legacy `options` bag, judged key by key, as before.', + reason: + 'The record member re-opens its top level with a strip so the console\'s round-trip keys survive, and ' + + 'that strip dropped a top-level `options` bag from the parse without looking inside it, while the save ' + + 'stored the request body. The console reads a record\'s body from `config` on the object page but ' + + 'spreads the whole stored record on the interface page, so the same saved view rendered two ways. ' + + 'Now that the save stores the parsed body, the bag would instead vanish silently on the next save. ' + + 'The maintainer\'s ruling of 2026-09-30 refuses it by name with the prescription to ' + + 'write `config.KIND`; declaring it would have kept a second spelling of one block on a second member. ' + + 'No console write puts the bag on a record. Not convertible: which of two spellings of one block the ' + + 'author meant, where both are set, is the author\'s call.', + acceptanceCriteria: + 'Every stored `view` record carrying a top-level `options` saves again after its blocks move under ' + + '`config`. A record that still carries the bag is refused `422 INVALID_METADATA` on its next save, with ' + + 'an issue at `options` whose message prescribes `config.KIND`. Nothing is rewritten on read and nothing ' + + 'is refused on read: a record that fails is served exactly as stored until it is saved. Verify by ' + + 're-saving each stored record that carries `options` (a GET then a PUT of the same body) and reading ' + + 'a `200`.', + }, // #20085 (ADR-0049 enforce-or-remove; triage direction 「retire both keys」) — // the D3 entry of the `view-item-owner-hidden-removed` family (ruling B on // #17152: one D3 entry per retirement family, even when D2 is lossless). diff --git a/packages/spec/src/ui/view-console-round-trip-keys.test.ts b/packages/spec/src/ui/view-console-round-trip-keys.test.ts index 2a6d8cf50f9..b3c47f921e0 100644 --- a/packages/spec/src/ui/view-console-round-trip-keys.test.ts +++ b/packages/spec/src/ui/view-console-round-trip-keys.test.ts @@ -7,12 +7,12 @@ * * ## Why this is a closure pin * - * `saveMetaItem` stores the request body verbatim (ADR-0005 appendix (c)), so a + * `saveMetaItem` stored the request body verbatim (ADR-0005 appendix (c)), so a * key the console writes and reads back lived in the store while the members' * `.strip()` dropped it from the parse. Nothing failed: the parse output was - * simply never persisted. The end state the ruling names is the parsed body - * becoming the stored one, and it may land only once every round-trip key is - * declared. This file is what "every" means. + * simply never persisted. Stage (iv) made the parsed body the stored one — the + * parsed value of every key a body carries — so a round-trip key survives a + * save only if it is declared. This file is what "every" means. * * `CENSUS` below is the measurement, written down independently of the spec's * own record: the keys objectui's console writes onto a stored `view` row and diff --git a/packages/spec/src/ui/view-overlay-options-bag.test.ts b/packages/spec/src/ui/view-overlay-options-bag.test.ts index 0cc50925608..0cbf7ea505a 100644 --- a/packages/spec/src/ui/view-overlay-options-bag.test.ts +++ b/packages/spec/src/ui/view-overlay-options-bag.test.ts @@ -244,3 +244,58 @@ describe('[#20051] the form overlay cannot take the bag the list overlay refused expect(ViewMetadataSchema.safeParse(body).success).toBe(true); }); }); + +/** + * [#20051] Stage (iv), Q3: the same legacy bag on a ViewItem RECORD + * (`{ name, object, viewKind, config }`) is refused by name, on both arms, with + * the prescription to write `config.KIND`. The record member's top-level + * `.strip()` used to drop it unread; with the save storing the parsed body it + * would have vanished on a `200` instead. + */ +describe('[#20051] a ViewItem record carries no top-level `options` bag', () => { + const record = (extra: Record = {}) => ({ + name: 'crm_lead.board', + object: 'crm_lead', + viewKind: 'list', + config: { + type: 'kanban', + data: { provider: 'object', object: 'crm_lead' }, + columns: ['name'], + kanban: { groupByField: 'stage', columns: ['name'] }, + }, + ...extra, + }); + + function recordIssueAtOptions(body: unknown): Issue { + const d = diagnoseViewMetadata(body); + expect(d.success, `expected a refusal for ${JSON.stringify(body)}`).toBe(false); + if (d.success) throw new Error('unreachable'); + expect(d.branch).toBe('viewItem'); + const hit = (d.issues as unknown as Issue[]).find((i) => i.path.length === 1 && i.path[0] === 'options'); + expect(hit, JSON.stringify(d.issues)).toBeDefined(); + return hit!; + } + + it('on the list arm: refused at `options`, prescribing `config.KIND`', () => { + const hit = recordIssueAtOptions(record({ options: { kanban: { groupByField: 'stage' } } })); + expect(hit.message).toMatch(/^A view item record carries no top-level `options` bag/); + expect(hit.message).toContain('Move each `options.KIND` block to `config.KIND`'); + expect(VIEW_METADATA_MEMBERS.viewItem.safeParse(record({ options: {} })).success).toBe(false); + }); + + it('on the form arm too', () => { + const hit = recordIssueAtOptions({ + name: 'lead.contact_us', + object: 'lead', + viewKind: 'form', + config: { type: 'simple', data: { provider: 'object', object: 'lead' } }, + options: { timeline: { titleField: 'name' } }, + }); + expect(hit.message).toMatch(/^A view item record carries no top-level `options` bag/); + }); + + it('CONTROL: the record without the bag parses, and `config.kanban` is where the block lives', () => { + const parsed = ViewMetadataSchema.safeParse(record()); + expect(parsed.success).toBe(true); + }); +}); diff --git a/packages/spec/src/ui/view-union-retirement-prescription.test.ts b/packages/spec/src/ui/view-union-retirement-prescription.test.ts index cb0074e9d58..00d2f9568b3 100644 --- a/packages/spec/src/ui/view-union-retirement-prescription.test.ts +++ b/packages/spec/src/ui/view-union-retirement-prescription.test.ts @@ -190,9 +190,25 @@ function walkNeverLeaves(): NeverLeaf[] { return out; } +/** + * [#20051] The one closed family of non-catchall `never` leaves that is NOT a + * retirement: a key pinned ABSENT on a member with a curated refusal, because + * it was never that member's key. It rides the same lift (a claimed-branch + * `invalid_type` / `expected: never` issue), so it is held to the same bar — + * its message is a prescription, never zod's default text. Closed: a new entry + * is a deliberate edit here, and every entry must still be reached. + */ +const REFUSED_ABSENT_LEAVES: Readonly> = { + // The ViewItem record's top-level `options` bag, on both arms: the record's + // per-kind blocks live under `config` (stage (iv) of #20051, Q3). + 'viewItem.|0.options': /^A view item record carries no top-level `options` bag: .*Move each `options\.KIND` block to `config\.KIND`/, + 'viewItem.|1.options': /^A view item record carries no top-level `options` bag: .*Move each `options\.KIND` block to `config\.KIND`/, +}; + describe('§2 population — every non-catchall `never` leaf on this surface is a prescription', () => { const leaves = walkNeverLeaves(); - const tombstones = leaves.filter((l) => !l.isCatchall); + const pinned = leaves.filter((l) => !l.isCatchall && l.path in REFUSED_ABSENT_LEAVES); + const tombstones = leaves.filter((l) => !l.isCatchall && !(l.path in REFUSED_ABSENT_LEAVES)); const catchalls = leaves.filter((l) => l.isCatchall); it('the walk reached both populations — the lit control for every zero below', () => { @@ -206,6 +222,14 @@ describe('§2 population — every non-catchall `never` leaf on this surface is expect(leaf.message, leaf.path).not.toContain('expected never, received'); } }); + + it('[#20051] every pinned-absent key is reached, and carries its own curated refusal', () => { + expect(pinned.map((l) => l.path).sort()).toEqual(Object.keys(REFUSED_ABSENT_LEAVES).sort()); + for (const leaf of pinned) { + expect(leaf.message, leaf.path).toMatch(REFUSED_ABSENT_LEAVES[leaf.path]!); + expect(leaf.message, leaf.path).not.toContain('expected never, received'); + } + }); }); // ─────────────────────────────────────────────────────────────────────────── diff --git a/packages/spec/src/ui/view.zod.ts b/packages/spec/src/ui/view.zod.ts index 6b06e119b34..783c1dd43db 100644 --- a/packages/spec/src/ui/view.zod.ts +++ b/packages/spec/src/ui/view.zod.ts @@ -398,9 +398,10 @@ export function normalizeFilterOperator(op: unknown): string { // (`components/src/custom/filter-builder.tsx:228`, re-stamped on read-back at // `plugin-view/src/config/view-config-utils.ts:146`/`:160`) and the sort builder // (`components/src/custom/sort-builder.tsx:68`/`:94`) both stamp -// `id: crypto.randomUUID()` on every row they render. `saveMetaItem` validates -// the PUT body and persists the AUTHORED body verbatim, so those ids reach the -// wire and the store. +// `id: crypto.randomUUID()` on every row they render, so those ids reach the +// wire. [#20051] They no longer reach the store: `saveMetaItem` stores the +// parsed value of every key a body carries, and the ids are removed before the +// parse. // // ⚠️ Why a `.strip()` on the wire member cannot do this job — the #4001 批 18 / // #5114 finding, and the reason this vocabulary exists at all: **`.strip()` does @@ -487,8 +488,10 @@ function stripRowDecorations(value: unknown, isRow: boolean, depth: number): unk * authoring doors keep rejecting the key by name, which is the whole point of * the split. * - * Nothing is lost at rest: `saveMetaItem` persists the ORIGINAL body, so the - * console still reads its own ids back. + * [#20051] The ids do not reach the store: `saveMetaItem` stores the parsed + * body, and this runs before the parse. Nothing the console needs is lost, as + * {@link VIEW_CONSOLE_ROUND_TRIP_KEYS} records: the builders mint a fresh id + * for a row that has none. * * Returns the SAME reference when there is nothing to strip, so the common path * allocates nothing. Non-object inputs pass through — the schema owns those. @@ -903,8 +906,8 @@ function foldAuthoredViewFilterOperator(op: ViewFilterOperator): string { * (`components/src/custom/filter-builder.tsx:228`; stamped again when a stored * filter is read back into the builder — * `plugin-view/src/config/view-config-utils.ts:146`/`:160`). `saveMetaItem` - * validates the PUT body and then persists the AUTHORED body verbatim, so that - * `id` is on the wire and in the store. Closed *without* a wire route, this + * validated the PUT body and then persisted the AUTHORED body verbatim (until + * #20051), so that `id` was on the wire and in the store. Closed *without* a wire route, this * shape turned every filter write carrying one into a 422 — measured on all * three paths, including the flattened personalization overlay that is the body * the console actually PUTs. @@ -1631,10 +1634,10 @@ export const UserFilterFieldSchema = lazySchema(() => strictObject({ * `:742`); its own `UserFiltersSchema` declared the key. The spec's did not — * and the difference between the two shapes was *exactly* that one key. * - * That gap was not inert, because the metadata write path does NOT persist - * `parsed.data`: `saveMetaItem` validates with `safeParse` and then stores the - * ORIGINAL body verbatim, precisely so Studio-only auxiliary keys survive - * (`metadata-protocol/src/protocol.ts`, "Validation policy"). So an authored + * That gap was not inert, because the metadata write path did NOT persist + * `parsed.data` then: `saveMetaItem` validated with `safeParse` and stored the + * ORIGINAL body verbatim, precisely so Studio-only auxiliary keys survived + * (a `view` stores its parsed body since #20051). So an authored * `allowAddTab` was stripped only from the parse RESULT, which is discarded — * the stored document kept it and the renderer read it. **The capability * worked.** Closing the shape without declaring the key would have converted a @@ -2611,8 +2614,8 @@ const ListViewShapeSchema = lazySchema(() => strictObject({ * normalizeViewMetadata persists on a console column-sort PUT"*, and `id` is * a UI row identity objectui stamps per row * (`components/src/custom/sort-builder.tsx:68`, `:94` — - * `crypto.randomUUID()`), persisted verbatim because `saveMetaItem` stores the - * original body. **`.strip()` on a wire member could not rescue it** — it + * `crypto.randomUUID()`), persisted verbatim because `saveMetaItem` stored the + * original body (until #20051). **`.strip()` on a wire member could not rescue it** — it * re-opens the TOP level only, and this is a nested block reached through that * member. See {@link stripViewConsoleDecorations}, which removes the * decoration on the wire door instead, at every depth. @@ -5095,8 +5098,9 @@ type ViewItemWireArmShape = { * - **Not `.strict()`**: the console owns the internals of this per-user * state (objectui's `gridNonAuthorKeys` ruling, 2026-08-18), so a future * console key here must not 422 against an older server. The two known - * keys are typed; unknown inner keys parse (and `saveMetaItem` stores the - * original body verbatim anyway). + * keys are typed; unknown inner keys parse. [#20051] They are not stored: + * the save keeps only the keys the schema declares, so a new console key + * is declared here before the console relies on reading it back. * - **Not exported, not on {@link ListViewSchema}**: exporting would mint a * protocol def / authorable-surface entries, and declaring it on the * authoring shape would bless hand-authoring a payload the product writes @@ -5202,8 +5206,8 @@ const VIEW_SWITCHER_VISIBILITY_GROUPS = ['private', 'team', 'organization', 'pub * stored-view readers at the `.objectui-sha` pin and ran the console's write * bodies through {@link ViewMetadataSchema}. Before this declaration the list * overlay's `.strip()` dropped `isPinned` and `sortOrder` from the parse, and - * both members dropped `visibility`: `saveMetaItem` stores the request body - * verbatim (ADR-0005 appendix (c)), so the keys lived in the store and nowhere + * both members dropped `visibility`: `saveMetaItem` stored the request body + * verbatim then (ADR-0005 appendix (c)), so the keys lived in the store and nowhere * in the contract. * * None of the three is per-user. A stored `view` row is environment metadata @@ -5239,13 +5243,32 @@ function viewSwitcherRowStateFields() { }; } +/** + * [#20051] The refusal the ViewItem wire member gives a top-level `options` bag. + * + * A record's view body lives under `config`, and `config` declares each + * per-kind block (`kanban`, `calendar`, `timeline`, …) itself, so `config.KIND` + * is the record's one spelling. The legacy `options.KIND` bag belongs to the + * FLATTENED list overlay, where it is judged key by key and objectui pins it; + * on a record the member's top-level `.strip()` used to drop it from the parse + * unread while the save stored it, and one console reader (the interface page) + * rendered it while the object page did not. No console write puts it on a + * record. Declared ABOVE {@link viewItemWireFields}, its first reader, for the + * `OS_EAGER_SCHEMAS=1` TDZ reason given on {@link VIEW_SWITCHER_VISIBILITY_GROUPS}. + */ +const VIEW_ITEM_OPTIONS_REFUSED = + 'A view item record carries no top-level `options` bag: its view body lives under `config`, and `config` ' + + 'declares each per-kind block itself. Move each `options.KIND` block to `config.KIND` (for example ' + + '`options.kanban` to `config.kanban`), or remove `options`.'; + /** * Auxiliary Studio round-trip keys, given an explicit DECLARED home on the wire * variant (#5074) instead of living implicitly on "the member nobody closed". * * The console writes them through the `view` metadata API and reads them back; - * `saveMetaItem` persists the body verbatim, so they are on the wire and in the - * store. They are deliberately declared HERE and not on {@link ViewItemSchema}: + * [#20051] the save stores the parsed value of every key a body carries, so a + * key is kept in the store only because it is declared here. They are + * deliberately declared HERE and not on {@link ViewItemSchema}: * an author who writes `isPinned` in a `*.view.ts` gets a named rejection * pointing at `order`, while the console's own PUT parses. [#20456] The * switcher's three keys come from {@link viewSwitcherRowStateFields}, the one @@ -5262,6 +5285,17 @@ function viewItemWireFields() { // shape where `.strip()` used to let it ride through unchecked. columnState: ViewColumnStateSchema.optional() .describe('Studio round-trip: column order/widths (runtime-only state, written by the console grid and stored on the view\'s row, which has no per-user scope — not authored)'), + // [#20051] Pinned ABSENT, refused by name — see + // {@link VIEW_ITEM_OPTIONS_REFUSED}. `z.never()` rather than the form + // overlay's `z.undefined()` ({@link FORM_OVERLAY_OPTIONS_REFUSED}): this + // member is published on its own as `ViewItemWire.json`, and `never` has a + // JSON Schema form (`not: {}`) where `undefined` has none. + options: z.never({ error: () => VIEW_ITEM_OPTIONS_REFUSED }).optional() + .describe( + 'Refused: a view item record carries no top-level `options` bag. Its per-kind blocks live under ' + + '`config` (`config.kanban`, `config.timeline`, …); the legacy `options.KIND` bag belongs to the ' + + 'flattened list overlay only.', + ), }; } @@ -5346,9 +5380,9 @@ export function defineViewItem(config: z.input): ViewItem // record/container can never be rescued by this lenient branch. // // Auxiliary Studio round-trip keys (`isPinned`, `sortOrder`, …) ride along on -// the shapes Studio actually round-trips, matching the "persist the payload -// verbatim" contract in `saveMetaItem` (it validates but stores the original -// item). ⚠️ [#5074] The line that used to stand here said "all four members +// the shapes Studio actually round-trips, DECLARED there, because the save +// stores the parsed value of every key a body carries and nothing else +// ([#20051]; it stored the original item until then). ⚠️ [#5074] The line that used to stand here said "all four members // strip-parse (no `.strict()`)". It was true when written, then half-false and // half-load-bearing (批 18 measured it), and is now replaced by the split. As // it stands, measured: @@ -5377,12 +5411,12 @@ export function defineViewItem(config: z.input): ViewItem // below existed: member 4 both `.strip()`s AND declares no required key // (`FormViewSchema.type` even carries a `'simple'` default), so it matched // ANY object. `{ nope: 1 }` did not merely pass — it passed as a *view*, -// reduced to `{ type: 'simple' }`, and `saveMetaItem` (which persists the +// reduced to `{ type: 'simple' }`, and `saveMetaItem` (which then persisted the // ORIGINAL body, not the parse output) wrote `{"nope":1,"name":"…"}` into // `sys_metadata` as an ACTIVE view overlay. The read path then re-parsed the // same body through the same schema and badged it `_diagnostics.valid: true` // (#5598), so Studio agreed. `view` was the one common overlay type whose -// declared write-path spec gate (ADR-0005 §Validation) could be bypassed by +// declared write-path spec gate (ADR-0005 appendix (c)) could be bypassed by // an arbitrary body — Prime Directive #10's "declared ≠ enforced", one layer // above the object schemas #4001 closed: at union-MEMBER SELECTION, not at // any single member. The fix is a precondition, NOT a strictness flip on the @@ -6111,8 +6145,8 @@ function formOverlayColumnsField(): z.ZodOptional { * `.strip()` is load-bearing, not leftover. `.extend()` INHERITS strictness, so * closing `ListViewSchema` for authoring (#4001) silently made this overlay * strict too — and this member exists precisely to carry Studio's auxiliary - * round-trip keys (`isPinned`, `sortOrder`, …) that `saveMetaItem` persists - * verbatim. Strict here is a 422 on a shape the platform itself writes. The + * round-trip keys (`isPinned`, `sortOrder`, …), which the save keeps because + * they are declared. Strict here is a 422 on a shape the platform itself writes. The * ledger names this as the trap to watch while batching: a response-side * extension of an authoring schema must strip back, or an upstream field * addition becomes a crash. diff --git a/scripts/adr-anchors/packages__metadata-protocol__src__protocol.ts.json b/scripts/adr-anchors/packages__metadata-protocol__src__protocol.ts.json index fa87f55ed6f..ff850c0536b 100644 --- a/scripts/adr-anchors/packages__metadata-protocol__src__protocol.ts.json +++ b/scripts/adr-anchors/packages__metadata-protocol__src__protocol.ts.json @@ -1,8 +1,10 @@ { "file": "packages/metadata-protocol/src/protocol.ts", "adrs": [ + "ADR-0005", "ADR-0029", + "ADR-0087", "ADR-0119" ], - "invariant": "ADR-0029 D9.8/D9.9 — the two `sys_metadata` object-hydration seams (`applyObjectRegistryMutation` and `loadMetaFromDb`) CHOOSE the contributor kind instead of defaulting to `'own'`: a packaged owner already registered for the name means the row is that object's `overlay` LAYER, and no packaged owner means the row IS the object. Registering `'own'` unconditionally is what spliced the packaged contributor out and destroyed the code definition at write time, replayed silently on every boot (#6853). The row's `package_id` is provenance ON the layer and never an ownership claim (D9.9): same package is normal, a package-less sentinel is ACCEPTED (the old throw was an artefact of the borrowed `own` slot, #6995), and a row bound to a DIFFERENT package is refused at the producer with `OBJECT_OVERLAY_PACKAGE_MISMATCH` / 422 and counted in `loadMetaFromDb`'s per-record `errors` at boot — because the overlay-uniqueness index can hold two rows for one (type, name) bound to two packages and `computeFQN` is identity, so the registry could never serve both. The delete heal subtracts the LAYER (`removeObjectOverlay`) BEFORE the tier walk, because tier 1 returns as soon as it heals the generic metadata map and would otherwise leave the object's layer registered.\n\nADR-0119 — `batchData`'s `atomic` is REAL or REFUSED, never silent best-effort. An explicitly atomic batch runs inside ONE `engine.transaction()`, so a failure rolls back every prior write and the response reports zero successes (rows marked ROLLED_BACK / NOT_ATTEMPTED — reporting a rolled-back row as `success: true` was the original bug, not merely the missing transaction). A runtime that cannot roll back gets 501 NOT_IMPLEMENTED; degrading to best-effort is how the flag came to lie." + "invariant": "ADR-0029 D9.8/D9.9 — the two `sys_metadata` object-hydration seams (`applyObjectRegistryMutation` and `loadMetaFromDb`) CHOOSE the contributor kind instead of defaulting to `'own'`: a packaged owner already registered for the name means the row is that object's `overlay` LAYER, and no packaged owner means the row IS the object. Registering `'own'` unconditionally is what spliced the packaged contributor out and destroyed the code definition at write time, replayed silently on every boot (#6853). The row's `package_id` is provenance ON the layer and never an ownership claim (D9.9): same package is normal, a package-less sentinel is ACCEPTED (the old throw was an artefact of the borrowed `own` slot, #6995), and a row bound to a DIFFERENT package is refused at the producer with `OBJECT_OVERLAY_PACKAGE_MISMATCH` / 422 and counted in `loadMetaFromDb`'s per-record `errors` at boot — because the overlay-uniqueness index can hold two rows for one (type, name) bound to two packages and `computeFQN` is identity, so the registry could never serve both. The delete heal subtracts the LAYER (`removeObjectOverlay`) BEFORE the tier walk, because tier 1 returns as soon as it heals the generic metadata map and would otherwise leave the object's layer registered.\n\nADR-0119 — `batchData`'s `atomic` is REAL or REFUSED, never silent best-effort. An explicitly atomic batch runs inside ONE `engine.transaction()`, so a failure rolls back every prior write and the response reports zero successes (rows marked ROLLED_BACK / NOT_ATTEMPTED — reporting a rolled-back row as `success: true` was the original bug, not merely the missing transaction). A runtime that cannot roll back gets 501 NOT_IMPLEMENTED; degrading to best-effort is how the flag came to lie.\n\nADR-0005 appendix (c) (\"Addendum — 2026-05-16 (c): spec validation on overlay save\", as amended by ruling 甲 on #20051) + ADR-0087 (addendum 2026-08-01b, `storable` excludes schema defaults) — `saveMetaItem` stores a `view` as the parsed value of every key its request body carried (`projectStorableViewBody`): an undeclared key is dropped, a schema default the author did not write is NOT stored, and a moved key (`groups` → `sections`, `visibleOn` → `visibleWhen`) is stored under its canonical spelling, so the stored row re-parses to exactly what the save accepted. Every key the console reads back off a stored view row is declared on the contract (`VIEW_CONSOLE_ROUND_TRIP_KEYS`), which is what makes dropping the rest lossless. Every OTHER type still stores its request body verbatim, with only the operator and `groups` → `sections` grafts applied: the switch is gated on `singularType === 'view'`, and widening it to another type is its own decision, not a refactor." }