fix(app-shell): delete metadata items through the API's delete door, not a tombstone PUT

.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<void>`), 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.

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<string, unknown>));
+ 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');
+ });
+});