diff --git a/.changeset/6238-object-payload-enabled.md b/.changeset/6238-object-payload-enabled.md new file mode 100644 index 000000000..fc618f3cd --- /dev/null +++ b/.changeset/6238-object-payload-enabled.md @@ -0,0 +1,70 @@ +--- +'@object-ui/app-shell': minor +--- + +`MetadataService`'s two delete methods no longer PUT a hand-written tombstone. They call +the metadata API's own delete door instead, and the latent `enabled?: boolean` on +`ObjectMetadataPayload` is gone with it (objectui#6238). Object-level member of the +objectui#5761 family, surfaced by the `ObjectSchema` oracle objectui#6223 added to +`scripts/check-designer-field-key-parity.mjs`. + +**What the tombstone actually did.** `deleteObject` and `deleteMetadataItem` wrote +`{ name, enabled: false, _deleted: true }` through `client.meta.saveItem`, i.e. +`PUT /api/v1/meta/:type/:name`. Measured against the installed `@objectstack/spec` 17.2.0 +using `getMetadataTypeSchema` — the registry the framework's own `saveMetaItem` resolves a +PUT's validator from — across all 26 registered overlay schemas: + +``` +ObjectSchema.safeParse({ name, label, fields }) => success = true (control) +ObjectSchema.safeParse({ name, label, fields, isSystem: true }) => success = true (control) +ObjectSchema.safeParse({ name, enabled: false, _deleted: true }) => unrecognized_keys ["enabled","_deleted"] + +25 of 26 registered overlay schemas refuse `enabled` and/or `_deleted` BY NAME + 1 of 26 (`view`) tolerates them; 4 kinds have no registered schema at all + 0 of 26 strip them +``` + +So there were two failure modes, not one. Where the type has a strict schema — `object` +among them — the delete was a hard `422 INVALID_METADATA`, so nothing was ever recorded. +Where the schema is tolerant or absent, the framework stores the request item **verbatim** +(it deliberately persists the body rather than `parsed.data`), and `_deleted` has no reader +anywhere on the platform — so the "soft delete" was a silent no-op that left the item live +carrying two junk keys. Neither outcome deleted or disabled anything. + +**The resolution is a mechanism change, not a rename**, and there was nothing to rename to. +`ObjectSchema`'s 42-key accept set has no on/off flag; the near-spelling `enable` is +`ObjectCapabilities`, a system-features *module object*, so `enabled: false` → `enable: +false` fails on the value where it passes on the name. No wire key was invented: a metadata +soft-delete convention would be a `@objectstack/spec` contract addition, and the platform +does not have one. + +**Both sites now call `client.meta.deleteItem(type, name)`** — `DELETE +/api/v1/meta/:type/:name`, the same request `MetadataClient.reset` issues, which is the +mechanism `MetadataObjectsPage.handleObjectsChange` and `ResourceEditPage` already used for +deletes. Two mechanisms for one operation had disagreed; now there is one. The delete route +is generic over `:type` on the same route family and capability gate as the PUT, so this +holds for every category the generic `deleteMetadataItem` serves, not just `object`. The +doc comment claiming the API "exposes `saveItem` but no dedicated `deleteItem`" was stale: +`@objectstack/client` 17.2.0 declares `meta.deleteItem` on the very client this service +already holds. + +`reset` semantics are the overlay's, and that is the governed answer rather than a +shortfall: it removes the customization row — which *is* deletion for an object the +designer authored — and restores the artifact for one a package declares, an object you are +not allowed to delete. Which of the two an item is, is what the API's own `deletable` / +`resettable` verdicts report, not something a client-side flag should decide. + +**No published type changed.** `ObjectMetadataPayload` is exported from its module but that +module is not re-exported by `packages/app-shell/src/index.ts`, the package's only entry, so +the removed `enabled?: boolean` was never on the published surface and no `**/src/index.ts` +is touched. What consumers *can* observe is behaviour: `MetadataService` is reachable +through the published `useMetadataService()` hook, both method signatures are unchanged +(`Promise`), and the HTTP request they issue changes from a `PUT` with a body to a +`DELETE`. + +The `KNOWN_UNPARSEABLE_KEYS` entry in `scripts/check-designer-field-key-parity.mjs` goes +with the fix — that ledger ratchets in both directions, so an entry left behind for a +resolved key is as red as a missing one. It is now empty for the first time, which is the +ratchet arriving where it was pointed; the self-test's non-vacuity guard moved onto a +fixture accordingly, so an empty ledger reads as success rather than as a demand that some +key stay unresolved. diff --git a/packages/app-shell/src/services/MetadataService.retiredObjectEnabled.test.ts b/packages/app-shell/src/services/MetadataService.retiredObjectEnabled.test.ts new file mode 100644 index 000000000..fc8c45c1e --- /dev/null +++ b/packages/app-shell/src/services/MetadataService.retiredObjectEnabled.test.ts @@ -0,0 +1,267 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#6238 — `MetadataService`'s delete path no longer PUTs a tombstone. + * + * `deleteObject` and `deleteMetadataItem` used to write + * `{ name, enabled: false, _deleted: true }` through `client.meta.saveItem`, + * i.e. `PUT /api/v1/meta/:type/:name`. Neither key is a metadata convention, + * and the two halves of that failed differently — which is why this file + * carries both a schema oracle and a WIRE oracle. + * + * ## What was measured before the fix + * + * Against the installed `@objectstack/spec` 17.2.0 (ESM build), using + * `getMetadataTypeSchema` — the registry the framework's own `saveMetaItem` + * resolves a PUT's validator from (`resolveOverlaySchema`): + * + * - 25 of the 26 registered overlay schemas refuse `enabled` and/or + * `_deleted` BY NAME, `object` among them (`422 INVALID_METADATA`, + * `unrecognized_keys ["enabled","_deleted"]`). + * - `view` is the exception, and it is the WORSE one: its schema tolerates + * unknown keys, and the framework persists the request item verbatim rather + * than `parsed.data`, so the tombstone was STORED. So were the four kinds + * with no registered schema at all (`analytics_cube`, `connector`, + * `sharing_rule`, `webhook`), which fall through unvalidated. + * - `_deleted` has no reader anywhere in the platform. So on exactly the + * categories that did not 422, the "soft delete" was a silent no-op that + * left the item live carrying two junk keys. + * + * Nothing strips them, and there is no spec surface for "this item exists but + * is off" — `ObjectSchema`'s accept set has no such flag, and the near-spelling + * `enable` is a capabilities MODULE OBJECT, so `enabled: false` -> `enable: + * false` fails on the value where it passes on the name. There was therefore no + * spelling to rename to and no convention to converge onto: the delete door + * `DELETE /meta/:type/:name` is the whole answer, and it is the same request + * `MetadataClient.reset` already issued for `MetadataObjectsPage`'s object + * deletes. + * + * ## Why a wire oracle as well as the schema one + * + * `scripts/check-designer-field-key-parity.mjs` reads DECLARATIONS. It saw the + * latent `enabled?: boolean` on `ObjectMetadataPayload` and it could not see + * the tombstone at all — that body is a literal, not a declared shape, which is + * the gate's own coverage note 1. So restoring only the WRITER, with the + * declaration still deleted, leaves that gate green. Every case below that + * asserts on a captured request is covering that half, and is the half that + * reds if the tombstone comes back. + * + * ## Why the assertions are on captured requests rather than spies + * + * The claim is about bytes on the wire, and the two disagree in the case that + * matters: a spy sees an in-memory body, and `JSON.stringify` drops + * `undefined`. Every wire case here reads the real `RequestInit` the SDK handed + * `fetch`. + */ + +import { describe, expect, it, vi } from 'vitest'; +import { ObjectSchema } from '@objectstack/spec/data'; +import { getMetadataTypeSchema, listMetadataTypeSchemaTypes } from '@objectstack/spec/kernel'; +import { ObjectStackAdapter, MetadataClient } from '@object-ui/data-objectstack'; +import { MetadataService } from './MetadataService'; + +const BASE_URL = 'http://test.local'; + +/** One request exactly as the SDK issued it. */ +interface CapturedRequest { + method: string; + url: string; + body: string | undefined; +} + +function makeCapturingAdapter() { + const requests: CapturedRequest[] = []; + const adapter = new ObjectStackAdapter({ + baseUrl: BASE_URL, + fetch: vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + requests.push({ + method: (init?.method ?? 'GET').toUpperCase(), + url: String(input), + body: init?.body == null ? undefined : String(init.body), + }); + return new Response(JSON.stringify({ type: 'object', name: 'account', deleted: true }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }) as unknown as typeof fetch, + }); + return { adapter, requests }; +} + +const unrecognizedKeys = (result: { success: boolean; error?: unknown }): string[] => + result.success + ? [] + : ((result as { error: { issues: Array<{ code: string; keys?: string[] }> } }).error.issues + .filter((i) => i.code === 'unrecognized_keys') + .flatMap((i) => i.keys ?? [])); + +/** A base document `ObjectSchema` accepts, for probing one key at a time. */ +const BASE = { name: 'account', label: 'Account', fields: { n: { type: 'text', label: 'N' } } }; + +/** The body the two delete methods used to PUT. */ +const TOMBSTONE = { name: 'account', enabled: false, _deleted: true }; + +describe('the instrument', () => { + it('is the installed spec schema and it is STRICT — unknown keys are refused, not stripped', () => { + // Without this, every "refuses `enabled`" assertion below could be produced + // by a schema that silently dropped the key instead (objectstack#4001). + const result = ObjectSchema.safeParse({ ...BASE, zzzDefinitelyNotAKey: 1 }); + expect(result.success).toBe(false); + expect(unrecognizedKeys(result)).toContain('zzzDefinitelyNotAKey'); + }); + + it('accepts the controls — this is a key-by-key result, not a schema refusing everything', () => { + expect(ObjectSchema.safeParse(BASE).success).toBe(true); + expect(ObjectSchema.safeParse({ ...BASE, isSystem: true }).success).toBe(true); + }); +}); + +describe('objectui#6238 · the schema oracle — both tombstone keys are refused BY NAME', () => { + it('refuses `enabled` and `_deleted` individually, on a document it otherwise accepts', () => { + expect(unrecognizedKeys(ObjectSchema.safeParse({ ...BASE, enabled: false }))).toEqual(['enabled']); + expect(unrecognizedKeys(ObjectSchema.safeParse({ ...BASE, _deleted: true }))).toEqual(['_deleted']); + }); + + it('refuses the whole tombstone body, naming both keys at once', () => { + expect(unrecognizedKeys(ObjectSchema.safeParse(TOMBSTONE)).sort()).toEqual(['_deleted', 'enabled']); + }); + + it('has no on/off flag to rename to — `enable` is a capabilities object, not a boolean', () => { + // This is the trap the card names, and it is a VALUE-level fact, so the + // key-name gate could never have stated it. `enable` passes on the name and + // fails on the value, which is why this is objectui#4687's resolution + // (delete the declaration) rather than objectui#6041's (rename it). + const accept = new Set(Object.keys(ObjectSchema.shape as Record)); + expect(accept.size).toBe(42); + expect(accept.has('enable')).toBe(true); + expect(accept.has('enabled')).toBe(false); + expect(accept.has('_deleted')).toBe(false); + for (const key of ['disabled', 'active', 'isActive', 'deleted', 'isDeleted', 'softDeleted']) { + expect(accept.has(key), `ObjectSchema unexpectedly accepts \`${key}\``).toBe(false); + } + + const asBoolean = ObjectSchema.safeParse({ ...BASE, enable: false }); + expect(asBoolean.success).toBe(false); + // The refusal is NOT a name refusal — that is the whole point. + expect(unrecognizedKeys(asBoolean)).not.toContain('enable'); + }); + + it('is a platform-wide fact, not an `object`-only one — the generic method serves every category', () => { + // `deleteMetadataItem` is generic over category, so the measurement has to + // be too. `getMetadataTypeSchema` is the registry the framework's + // `saveMetaItem` resolves a PUT's validator from, so this is the same + // oracle the server would have used, type by type. + const types = listMetadataTypeSchemaTypes(); + expect(types.length).toBeGreaterThan(20); + + const refusedByName: string[] = []; + const tolerated: string[] = []; + for (const type of types) { + const schema = getMetadataTypeSchema(type); + if (!schema) continue; + const parsed = schema.safeParse({ name: 'probe_name', enabled: false, _deleted: true }); + const refused = unrecognizedKeys(parsed).filter((k) => k === 'enabled' || k === '_deleted'); + if (refused.length) refusedByName.push(type); + else tolerated.push(type); + } + + // The overwhelming majority refuse by name — a hard 422 on the delete. + expect(refusedByName).toContain('object'); + expect(refusedByName.length).toBeGreaterThanOrEqual(20); + // And at least one does NOT, which is the case that made this a silent + // no-op rather than a loud failure. Recorded as a measurement rather than + // an exact list so a spec release that tightens a schema does not red this. + expect(refusedByName.length + tolerated.length).toBe(types.length); + }); +}); + +describe('objectui#6238 · the wire oracle — the delete path issues DELETE, never a tombstone PUT', () => { + it('deleteObject issues DELETE /api/v1/meta/object/:name and no PUT at all', async () => { + const { adapter, requests } = makeCapturingAdapter(); + await new MetadataService(adapter).deleteObject('account'); + + const writes = requests.filter((r) => r.method !== 'GET'); + expect(writes).toHaveLength(1); + expect(writes[0].method).toBe('DELETE'); + expect(writes[0].url).toBe(`${BASE_URL}/api/v1/meta/object/account`); + // A DELETE carries no body, so there is nothing left for a refused key to + // ride on. Asserted rather than assumed: the defect was a body. + expect(writes[0].body).toBeUndefined(); + expect(requests.some((r) => r.method === 'PUT')).toBe(false); + }); + + it('puts neither `enabled` nor `_deleted` anywhere in any request it makes', async () => { + // Deliberately a scan of every captured request rather than of one body: + // the claim is that the two keys left the wire, not that one call stopped + // sending them. + const { adapter, requests } = makeCapturingAdapter(); + const service = new MetadataService(adapter); + await service.deleteObject('account'); + await service.deleteMetadataItem('flow', 'nightly_purge'); + + const wire = requests.map((r) => `${r.method} ${r.url} ${r.body ?? ''}`).join('\n'); + expect(wire).not.toContain('enabled'); + expect(wire).not.toContain('_deleted'); + // Falsification: the requests really happened and really named the items. + expect(wire).toContain('DELETE http://test.local/api/v1/meta/object/account'); + expect(wire).toContain('DELETE http://test.local/api/v1/meta/flow/nightly_purge'); + }); + + it('covers the categories the old tombstone did NOT 422 on — where it was a silent no-op', async () => { + // `view` (tolerant schema) and `webhook` (no registered schema) are the two + // shapes of "stored verbatim". Those are the categories where the old + // tombstone persisted junk keys onto a still-live item, so they are the + // ones worth pinning by name rather than trusting the generic case to cover. + for (const category of ['view', 'webhook']) { + const { adapter, requests } = makeCapturingAdapter(); + await new MetadataService(adapter).deleteMetadataItem(category, 'probe_name'); + + const writes = requests.filter((r) => r.method !== 'GET'); + expect(writes).toHaveLength(1); + expect(writes[0].method).toBe('DELETE'); + expect(writes[0].url).toBe(`${BASE_URL}/api/v1/meta/${category}/probe_name`); + expect(writes[0].body).toBeUndefined(); + } + }); + + it('issues the SAME request `MetadataClient.reset` does — one operation, one mechanism', async () => { + // The convergence claim, measured rather than asserted in prose. `reset` is + // the door `MetadataObjectsPage.handleObjectsChange` and + // `ResourceEditPage` already delete through; `client.meta.deleteItem` is + // its name on the SDK client `MetadataService` already holds. If they were + // two different endpoints, converging on one of them would not be + // convergence — so the endpoints are compared, not the call sites. + const { adapter, requests: viaService } = makeCapturingAdapter(); + await new MetadataService(adapter).deleteObject('account'); + + const viaReset: CapturedRequest[] = []; + await new MetadataClient({ + baseUrl: BASE_URL, + fetch: vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + viaReset.push({ + method: (init?.method ?? 'GET').toUpperCase(), + url: String(input), + body: init?.body == null ? undefined : String(init.body), + }); + return new Response(JSON.stringify({ reset: true }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }) as unknown as typeof fetch, + }).reset('object', 'account'); + + const serviceWrite = viaService.filter((r) => r.method !== 'GET'); + expect(serviceWrite).toHaveLength(1); + expect(viaReset).toHaveLength(1); + expect(serviceWrite[0].method).toBe(viaReset[0].method); + expect(serviceWrite[0].url).toBe(viaReset[0].url); + // Falsification: neither is a GET that happened to match. + expect(viaReset[0].method).toBe('DELETE'); + }); +}); diff --git a/packages/app-shell/src/services/MetadataService.ts b/packages/app-shell/src/services/MetadataService.ts index 19a8a1d46..27c4cf65d 100644 --- a/packages/app-shell/src/services/MetadataService.ts +++ b/packages/app-shell/src/services/MetadataService.ts @@ -44,7 +44,14 @@ export interface ObjectMetadataPayload { // display concern of the manager, not object metadata. (Distinct from the // field-level `sortOrder`, which objectui#6045 has since removed for its own // reasons — `FieldSchema` refuses that spelling too, at the other level.) - enabled?: boolean; + // No `enabled` (objectui#6238): `ObjectSchema` refuses it BY NAME and the + // spec has no object-level on/off flag at all. The near-spelling `enable` is + // NOT it — that is `ObjectCapabilities`, a system-features module object, so + // `enabled: false` -> `enable: false` fails on the VALUE where it passes on + // the name. This declaration was objectui#4687's shape (never populated by + // `toObjectPayload`); the key reached the wire only through the tombstone + // bodies the two delete methods wrote by hand, and those now go through the + // metadata API's own delete door instead — see `deleteMetadataItem`. fields?: FieldMetadataPayload[]; // No `relationships` (objectui#6223): the spec models relationships on the // FIELD — `reference` / `master_detail` plus object-level `indexes` — and @@ -214,15 +221,38 @@ export class MetadataService { } /** - * Soft-delete a metadata item by persisting it with `enabled: false` and - * `_deleted: true`. Works for any metadata category. + * Delete a metadata item through the metadata API's own delete door + * (`DELETE /api/v1/meta/:type/:name`). Works for any metadata category. * - * **Not wired to the view seam, and the reason is structural** (objectui#4373): - * both view cache keys are OBJECT-scoped, this signature has no object - * parameter, and the tombstone body it writes (`{ name, enabled, _deleted }`) - * carries no object binding either — so unlike {@link saveMetadataItem} there - * is nothing here to derive one from. Splitting `name` on `.` would be a - * second, silently-wrong identity rule (a source-declared view's name is not + * **It used to PUT a hand-written tombstone** — `{ name, enabled: false, + * _deleted: true }` — and objectui#6238 measured what the server does with + * that body. Neither key is a metadata convention: + * + * - `enabled` and `_deleted` are refused BY NAME by 25 of the 26 overlay + * schemas the framework validates a PUT against (`getMetadataTypeSchema`, + * the registry `saveMetaItem`'s `resolveOverlaySchema` reads), `object` + * among them: `422 INVALID_METADATA`, `unrecognized_keys`. + * - Where the type's schema is tolerant (`view`) or unregistered + * (`analytics_cube`, `connector`, `sharing_rule`, `webhook`) the body is + * STORED VERBATIM — the framework persists the request item, never + * `parsed.data` — and `_deleted` has no reader anywhere in the platform. + * So on exactly the categories that did not 422, the "soft delete" was a + * silent no-op that left the item live carrying two junk keys. + * + * There was no third outcome: nothing strips the keys, and no spec surface + * expresses "this item exists but is off" (`ObjectSchema`'s 42-key accept set + * has no such flag), so there is no correct spelling this could be renamed + * to. The delete door is `DELETE /:type/:name` — generic over `:type` on the + * same route family and capability gate as the PUT — which is the same + * request `MetadataClient.reset` issues for `MetadataObjectsPage`'s object + * deletes and `ResourceEditPage`'s generic ones. One operation, one mechanism. + * + * **Still not wired to the view seam, and the reason is unchanged and + * structural** (objectui#4373): both view cache keys are OBJECT-scoped and + * this signature has no object parameter. The old tombstone body carried no + * object binding to derive one from; a DELETE has no body at all, so it + * carries even less. Splitting `name` on `.` would be a second, + * silently-wrong identity rule (a source-declared view's name is not * qualified), and inventing an object argument for a method with no callers * is a surface we would be guessing at. If a `'view'` caller ever appears, * the fix is to give it the object it already knows and call @@ -230,7 +260,7 @@ export class MetadataService { */ async deleteMetadataItem(category: string, name: string): Promise { const client = this.adapter.getClient(); - await client.meta.saveItem(category, name, { name, enabled: false, _deleted: true }); + await client.meta.deleteItem(category, name); this.adapter.invalidateCache(`${category}:${name}`); } @@ -250,16 +280,30 @@ export class MetadataService { } /** - * Delete an object definition from the backend. + * Delete an object definition from the backend + * (`DELETE /api/v1/meta/object/:name`). + * + * The note this replaces said the metadata API "currently exposes `saveItem` + * but no dedicated `deleteItem`", and that a tombstone PUT recorded the + * intent until a real delete existed. Both halves were stale by the time + * objectui#6238 measured them: `@objectstack/client` 17.2.0 declares + * `meta.deleteItem(type, name)` on the very client this service already + * holds, and it issues the SAME `DELETE /api/v1/meta/:type/:name` that + * `MetadataClient.reset` does — the mechanism `MetadataObjectsPage` has been + * using for designer object deletes all along. Nothing was recording an + * intent in the meantime: `ObjectSchema` refuses `enabled` and `_deleted` by + * name, so this call returned `422 INVALID_METADATA` every time it ran. * - * NOTE: The ObjectStack metadata API currently exposes `saveItem` but no - * dedicated `deleteItem`. We persist the object with `enabled: false` so - * the intent is recorded and the object is hidden from active use. - * A full hard-delete can be added once the backend supports it. + * `reset` semantics are the overlay's, and that is the governed answer rather + * than a shortfall: it removes the customization row, which IS deletion for + * an object the designer authored, and restores the artifact for one a + * package declares — an object you are not allowed to delete. Which of the + * two an item is, is what the API's own `deletable` / `resettable` verdicts + * report (`MetadataClient`), not something a client-side flag should decide. */ async deleteObject(objectName: string): Promise { const client = this.adapter.getClient(); - await client.meta.saveItem('object', objectName, { name: objectName, enabled: false, _deleted: true }); + await client.meta.deleteItem('object', objectName); this.adapter.invalidateCache(`object:${objectName}`); } diff --git a/packages/app-shell/src/services/MetadataService.viewInvalidation.test.ts b/packages/app-shell/src/services/MetadataService.viewInvalidation.test.ts index 2ce2298e6..c801236ab 100644 --- a/packages/app-shell/src/services/MetadataService.viewInvalidation.test.ts +++ b/packages/app-shell/src/services/MetadataService.viewInvalidation.test.ts @@ -42,16 +42,17 @@ function makeAdapter() { const saveItem = vi.fn( async (_category: string, _name: string, _data: Record) => ({ success: true }), ); + const deleteItem = vi.fn(async (type: string, name: string) => ({ type, name, deleted: true })); const adapter = { - getClient: () => ({ meta: { saveItem } }), + getClient: () => ({ meta: { saveItem, deleteItem } }), invalidateCache: (key: string) => invalidatedKeys.push(key), invalidateViewKeys: (objectName: string, viewName: string) => { viewSeamCalls.push([objectName, viewName]); }, }; - return { adapter, invalidatedKeys, viewSeamCalls, saveItem }; + return { adapter, invalidatedKeys, viewSeamCalls, saveItem, deleteItem }; } describe('MetadataService routes view writes through the adapter seam (#4373)', () => { @@ -133,25 +134,26 @@ describe('MetadataService routes view writes through the adapter seam (#4373)', }); it('deleteMetadataItem cannot name the view keys, and the reason is its payload', async () => { - // Pinned as a decision, not left as an absence. The tombstone this method - // writes is `{ name, enabled: false, _deleted: true }` — no object binding - // — and the signature has no object parameter either, so unlike the save - // half there is nothing here to derive the keys from. Inventing an object - // argument for a method with no callers would be guessing at a surface. - // If a `'view'` caller ever appears, give it the object it already knows - // and call `adapter.invalidateViewKeys(objectName, name)` here. - const { adapter, invalidatedKeys, viewSeamCalls, saveItem } = makeAdapter(); + // Pinned as a decision, not left as an absence — and objectui#6238 made the + // reason STRONGER rather than obsolete. This method used to PUT a tombstone + // `{ name, enabled: false, _deleted: true }`, whose defect was that it + // carried no object binding to derive the two object-scoped view keys from; + // it now issues `DELETE /meta/view/:name`, which carries no body at all. + // The signature still has no object parameter, so there is still nothing + // here to derive the keys from, and inventing an object argument for a + // method with no callers would still be guessing at a surface. If a + // `'view'` caller ever appears, give it the object it already knows and + // call `adapter.invalidateViewKeys(objectName, name)` here. + const { adapter, invalidatedKeys, viewSeamCalls, saveItem, deleteItem } = makeAdapter(); await new MetadataService(adapter as unknown as ObjectStackAdapter).deleteMetadataItem('view', 'account.mine'); - expect(saveItem).toHaveBeenCalledWith('view', 'account.mine', { - name: 'account.mine', - enabled: false, - _deleted: true, - }); - // The written body carries no object — this is the measurement, not a wish. - const [, , written] = saveItem.mock.calls[0]; - expect(written.object).toBeUndefined(); + expect(deleteItem).toHaveBeenCalledWith('view', 'account.mine'); + // The delete door takes `(type, name)` and nothing else, so there is no + // body to read an object out of — this is the measurement, not a wish. + expect(deleteItem.mock.calls[0]).toHaveLength(2); + // And no tombstone PUT rides along beside it. + expect(saveItem).not.toHaveBeenCalled(); expect(viewSeamCalls).toEqual([]); expect(invalidatedKeys).toEqual(['view:account.mine']); }); diff --git a/scripts/__tests__/check-designer-field-key-parity.test.ts b/scripts/__tests__/check-designer-field-key-parity.test.ts index cbab0cf6e..0972a8ecf 100644 --- a/scripts/__tests__/check-designer-field-key-parity.test.ts +++ b/scripts/__tests__/check-designer-field-key-parity.test.ts @@ -499,14 +499,37 @@ describe('the real shapes, on the real tree', () => { }); it('every ledger entry names the card that owns its resolution', async () => { - const entries = Object.entries(KNOWN_UNPARSEABLE_KEYS); - expect(entries.length).toBeGreaterThan(0); - for (const [key, entry] of entries) { + // The ledger is REMOVE-only and it is currently EMPTY — every key it ever + // held has been resolved (objectui#4676 `placeholder`, objectui#6043 + // `formula`, objectui#6045 `sortOrder`, objectui#6238 `enabled`). That is + // the ratchet arriving where it was pointed, so an empty ledger is the + // success state and must not be a failure. + // + // This case used to open `expect(entries.length).toBeGreaterThan(0)` as its + // non-vacuity guard, and when objectui#6238 emptied the ledger that + // assertion inverted into a demand that some key stay UNRESOLVED — with a + // failure message (`expected 0 to be greater than 0`) whose obvious remedy + // is to add a row back, i.e. the one edit the header forbids. The guard is + // kept, pointed at the right thing: the validation must be exercised, and + // the fixture below is what exercises it when the real ledger is empty. + const validate = (key: string, entry: { card?: string; note?: string; oracle?: string }) => { expect(entry.card, `${key} has no card`).toMatch(/^objectui#\d+$/); expect(entry.note, `${key} has no note`).toBeTruthy(); // objectui#6223: with two oracles, an entry that names none silently // defaults to the field one and can absorb an object-level key. expect(['FieldSchema', 'ObjectSchema'], `${key} names no oracle`).toContain(entry.oracle); - } + }; + + // Non-vacuity, on a fixture rather than on the real tree: the loop body + // really does reject a malformed entry, whatever the live ledger holds. + validate('zzzWellFormedFixture', { + card: 'objectui#0000', + note: 'fixture', + oracle: 'FieldSchema', + }); + expect(() => validate('zzzNoCardFixture', { note: 'fixture', oracle: 'FieldSchema' })).toThrow(); + expect(() => validate('zzzNoOracleFixture', { card: 'objectui#0000', note: 'fixture' })).toThrow(); + + for (const [key, entry] of Object.entries(KNOWN_UNPARSEABLE_KEYS)) validate(key, entry); }); }); diff --git a/scripts/check-designer-field-key-parity.mjs b/scripts/check-designer-field-key-parity.mjs index 5409469c5..2326d74ac 100644 --- a/scripts/check-designer-field-key-parity.mjs +++ b/scripts/check-designer-field-key-parity.mjs @@ -276,15 +276,30 @@ export const KNOWN_UNPARSEABLE_KEYS = { // (objectui#6223 kept it there as the Object Manager's display order), where // the gate reports it as `uiOnly`. That is why this entry was oracle-scoped: // removing it must not, and does not, quiet the other level. - enabled: { - card: "objectui#6238", - oracle: "ObjectSchema", - // `ObjectSchema` DOES have `enable` — but it is `ObjectCapabilities`, a - // system-features module object, not a boolean on/off flag. Recorded here - // so the next reader does not mistake the near-spelling for a rename. - spec: null, - note: "Object-level, surfaced by the ObjectSchema oracle added in objectui#6223. `ObjectMetadataPayload` declares it and `deleteObject` / `deleteMetadataItem` write `{ enabled: false, _deleted: true }` directly, so the SOFT-DELETE path — not `toObjectPayload` — is what puts it on the wire. Not a rename: the spec's `enable` is a capabilities object, so what a soft delete should write is its own question.", - }, + // objectui#6238 `enabled` (OBJECT level) was resolved and its entry removed. + // The resolution was objectui#4687's for the declaration and a mechanism + // change for the writers, and it needed both because the two halves failed + // differently: `ObjectMetadataPayload` declared the key and `toObjectPayload` + // never populated it (latent), while `deleteObject` / `deleteMetadataItem` + // put it on the wire inside a hand-written tombstone `{ name, enabled: false, + // _deleted: true }` that no declared shape described — coverage note 1, the + // hole this gate states rather than hides. + // + // The `spec` column recorded no equivalent and there was none to take. The + // near-spelling `enable` is `ObjectCapabilities`, a system-features module + // object, so `enabled: false` -> `enable: false` fails on the VALUE where it + // passes on the name; and the 42-key accept set has no on/off flag at all. + // Measured before the fix: 25 of the 26 registered overlay schemas refuse + // `enabled`/`_deleted` by name, and where a type's schema is tolerant + // (`view`) or unregistered the framework stores the body verbatim while + // nothing on the platform reads `_deleted` — a 422 on one side, a silent + // no-op on the other, and no soft-delete convention to converge onto. Both + // writers now call `client.meta.deleteItem`, i.e. the same + // `DELETE /meta/:type/:name` `MetadataClient.reset` issues. + // + // Nothing about this entry's removal touches the FIELD oracle, and the + // spelling is unrelated to `ObjectDefinition.isSystem` / the UI-only keys the + // gate still reports below. }; /** The oracle names a shape may name, in the order they are reported. */