From 9a674307a3a92e6f9021f2293ac5147d891b4230 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 04:02:48 +0000 Subject: [PATCH 1/2] fix(app-shell,designer): stop putting three object-level keys the spec refuses on the wire (objectui#6223) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ObjectSchema` refuses `group`, `sortOrder` and `relationships` BY NAME. Measured against the installed `@objectstack/spec` 17.2.0 (ESM build), whose accept set is 42 keys: const base = { name: 'account', label: 'Account', fields: { n: { type: 'text', label: 'N' } } }; ObjectSchema.safeParse(base) => success = true (control) ObjectSchema.safeParse({ ...base, isSystem: true }) => success = true (control) ObjectSchema.safeParse({ ...base, pluralLabel: 'A' }) => success = true (control) ObjectSchema.safeParse({ ...base, group: 'Sales' }) => unrecognized_keys ["group"] ObjectSchema.safeParse({ ...base, sortOrder: 3 }) => unrecognized_keys ["sortOrder"] ObjectSchema.safeParse({ ...base, relationships: [] }) => unrecognized_keys ["relationships"] The two controls are what make that a key-by-key result rather than a schema refusing everything. Each key is resolved on its own, as the objectui#5761 family ruling requires: group UI-only. The spec has no object-level grouping key (`fieldGroups` groups the fields INSIDE one object), so the Object Manager's grouping control and column stay and the value is DERIVED from the accepted key `isSystem` instead of round-tripped. `MetadataObjectsPage` also strips a `group` already stored by an earlier build — its save-back spreads the server document verbatim, so not writing the key is not the same as removing it. sortOrder Declaration removed. What populated it was the array index the converter happened to be at. The field-level `sortOrder` is a different key with its own card (objectui#6045) and is untouched. relationships Declaration removed from the payload. The spec models relationships on the FIELD (`reference` / `master_detail` plus object-level `indexes`); what the designer should author for a relationship is a data-model question this does not settle. The objectui#5761 parity gate gains a SECOND ORACLE: every entry in `PAYLOAD_SHAPES` names the schema that judges it, and reach is resolved WITHIN an oracle rather than across one — `group` is a legal `FieldSchema` key and a refused `ObjectSchema` key at the same time, so a pooled accept set would have stayed green on exactly these three. That extension surfaced two more, filed and not fixed here: `enabled` (objectui#6238, written by the soft-delete path) is ledgered, and `fields` sent as an array where the spec wants a map (objectui#6240) is a value-level rejection the key-name check cannot see — pinned as an assertion so it cannot change silently. Assertions are on captured PUT bytes, not the in-memory object: `undefined` is a key zod's strict object counts and `JSON.stringify` drops. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L --- .changeset/6223-object-payload-spec-keys.md | 54 ++++ ...tadataService.specKeyObjectPayload.test.ts | 250 +++++++++++++++++ .../app-shell/src/services/MetadataService.ts | 41 ++- .../app-shell/src/utils/metadataConverters.ts | 6 + .../MetadataObjectsPage.specKeyGroup.test.tsx | 254 ++++++++++++++++++ .../src/MetadataObjectsPage.tsx | 30 ++- .../check-designer-field-key-parity.test.ts | 185 ++++++++++++- scripts/check-designer-field-key-parity.mjs | 200 +++++++++++--- 8 files changed, 962 insertions(+), 58 deletions(-) create mode 100644 .changeset/6223-object-payload-spec-keys.md create mode 100644 packages/app-shell/src/services/MetadataService.specKeyObjectPayload.test.ts create mode 100644 packages/plugin-designer/src/MetadataObjectsPage.specKeyGroup.test.tsx diff --git a/.changeset/6223-object-payload-spec-keys.md b/.changeset/6223-object-payload-spec-keys.md new file mode 100644 index 0000000000..e6b7eb8071 --- /dev/null +++ b/.changeset/6223-object-payload-spec-keys.md @@ -0,0 +1,54 @@ +--- +'@object-ui/plugin-designer': minor +'@object-ui/app-shell': minor +--- + +Object-level metadata payloads no longer emit the three keys `ObjectSchema` refuses by +name — **group**, **sortOrder** and **relationships** (objectui#6223). + +Measured against the installed `@objectstack/spec` 17.2.0, whose `ObjectSchema` accept set +is 42 keys: + +``` +const base = { name: 'account', label: 'Account', fields: { n: { type: 'text', label: 'N' } } }; + +ObjectSchema.safeParse(base) => success = true (control) +ObjectSchema.safeParse({ ...base, isSystem: true }) => success = true (control) +ObjectSchema.safeParse({ ...base, pluralLabel: 'A' }) => success = true (control) + +ObjectSchema.safeParse({ ...base, group: 'Sales' }) => unrecognized_keys ["group"] +ObjectSchema.safeParse({ ...base, sortOrder: 3 }) => unrecognized_keys ["sortOrder"] +ObjectSchema.safeParse({ ...base, relationships: [ … ] }) => unrecognized_keys ["relationships"] +``` + +The two controls are what make that a key-by-key result rather than a schema refusing +everything. Each key was resolved on its own, as the objectui#5761 family ruling requires: + +- **group** — the Object Manager's grouping is a UI-only display category. The spec has no + object-level grouping key (`fieldGroups` groups the fields *inside* one object), so the + grouping control and its column stay, and the value is now DERIVED from the spec key that + is accepted (`isSystem`) instead of round-tripped. `MetadataObjectsPage` also strips a + `group` already stored by an earlier build, because its save-back spreads the server + document verbatim and would otherwise keep re-sending it forever. +- **sortOrder** — what populated it was the array index the converter happened to be at, + i.e. the order the list was already in. The declaration is removed from the object + payload. The field-level `sortOrder` is a different key with a different card + (objectui#6045) and is untouched. +- **relationships** — the spec models relationships on the FIELD (`reference` / + `master_detail`, plus object-level `indexes`). The object payload stops declaring and + sending an object-level relationship array; what the designer should author for a + relationship is a data-model question this change does not settle. + +**Breaking for TypeScript consumers of `ObjectMetadataPayload`** (exported from app-shell): +the three properties are gone from the published type, so code that set them stops +compiling. That is the point — setting any of them produced a payload the metadata route +refuses. `ObjectDefinition` (the designer's UI model) is unchanged and still carries all +three. + +The parity gate built for objectui#5761 now has a **second oracle**: every shape in +`PAYLOAD_SHAPES` names the schema that judges it, `ObjectSchema` alongside `FieldSchema`, +and reach is resolved within an oracle rather than across one — `group` is a legal +`FieldSchema` key and a refused `ObjectSchema` key at the same time. That extension found a +fourth object-level key (`enabled`, objectui#6238) and a value-level rejection the key-name +check cannot see (`fields` sent as an array where the spec wants a map, objectui#6240); +both are filed and ledgered rather than fixed here. diff --git a/packages/app-shell/src/services/MetadataService.specKeyObjectPayload.test.ts b/packages/app-shell/src/services/MetadataService.specKeyObjectPayload.test.ts new file mode 100644 index 0000000000..2c698ea2b8 --- /dev/null +++ b/packages/app-shell/src/services/MetadataService.specKeyObjectPayload.test.ts @@ -0,0 +1,250 @@ +/** + * 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#6223 — the object payload `MetadataService` PUTs carries no key + * `ObjectSchema` refuses BY NAME. + * + * Surfaced by the key-level parity gate built for objectui#5761 + * (`scripts/check-designer-field-key-parity.mjs`), once objectui#6223 gave it a + * SECOND ORACLE. Until then the gate compared field shapes against + * `FieldSchema` and nothing at all checked the parent document those fields are + * nested in, so three object-level keys sat on the wire while the gate was + * green. `ObjectMetadataPayload` is now one of that gate's object-level `wire` + * shapes: `toObjectPayload` builds it and `saveObject` PUTs it whole to + * `PUT /api/v1/meta/object/:name`. + * + * Measured against the installed `@objectstack/spec` 17.2.0 (ESM build), whose + * `ObjectSchema` accept set is 42 keys: + * + * ObjectSchema.safeParse({ ...base, group: 'Sales' }) => unrecognized_keys ["group"] + * ObjectSchema.safeParse({ ...base, sortOrder: 3 }) => unrecognized_keys ["sortOrder"] + * ObjectSchema.safeParse({ ...base, relationships: … }) => unrecognized_keys ["relationships"] + * + * The controls are what make that a KEY-BY-KEY result rather than a schema that + * refuses everything: `isSystem` and `pluralLabel` parse green on the same base + * document. Both are asserted below, first, before any claim about the fix. + * + * ## What this file does NOT claim + * + * It does not claim a reproduced HTTP 422. The card was explicit that whether + * the deployed route rejects these today depends on what that route parses + * with; the schema fact is the ground for the fix and is all that is asserted. + * + * ## Why the assertions are on bytes + * + * `undefined` is a key zod's strict object COUNTS and `JSON.stringify` DROPS. + * An in-memory assertion on the object handed to the client and a wire + * assertion therefore disagree exactly on the half-filled case, so every + * assertion here reads `JSON.parse` of the captured request body. + * + * ## One `it` per key, deliberately + * + * Three keys were resolved. A suite that exercised them together would stay + * green on a fix that landed two of three, so each key is pinned by name in its + * own case: reverting one resolution reds only that case. + */ + +import { describe, expect, it, vi } from 'vitest'; +import { ObjectSchema } from '@objectstack/spec/data'; +import { ObjectStackAdapter } from '@object-ui/data-objectstack'; +import type { ObjectDefinition } from '@object-ui/types'; +import { MetadataService } from './MetadataService'; + +/** The bodies of every PUT the SDK issued, exactly as they went over the wire. */ +function makeCapturingAdapter() { + const puts: Array> = []; + const adapter = new ObjectStackAdapter({ + baseUrl: 'http://test.local', + fetch: vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + if ((init?.method ?? 'GET').toUpperCase() === 'PUT') { + puts.push(JSON.parse(String(init?.body ?? '{}')) as Record); + } + return new Response(JSON.stringify({ success: true }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }) as unknown as typeof fetch, + }); + return { adapter, puts }; +} + +const unrecognizedKeys = (result: ReturnType): string[] => + result.success + ? [] + : result.error.issues + .filter((i) => i.code === 'unrecognized_keys') + .flatMap((i) => (i as unknown as { keys: string[] }).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' } } }; + +/** + * An object as the Object Manager holds it, with all three UI-only keys + * populated — the state a designer save actually starts from. + */ +const MANAGED: ObjectDefinition = { + id: 'account', + name: 'account', + label: 'Account', + pluralLabel: 'Accounts', + description: 'Customer accounts', + icon: 'Building', + group: 'Custom Objects', + sortOrder: 3, + isSystem: false, + fieldCount: 1, + relationships: [ + { relatedObject: 'contact', type: 'one-to-many', label: 'Contacts', foreignKey: 'account_id' }, + ], +}; + +async function putFor(obj: ObjectDefinition = MANAGED): Promise> { + const { adapter, puts } = makeCapturingAdapter(); + await new MetadataService(adapter).saveObject(obj, [{ name: 'name', type: 'text', label: 'Name' }]); + expect(puts).toHaveLength(1); + return puts[0]; +} + +describe('the instrument', () => { + it('is the installed spec schema and it is STRICT — unknown keys are refused, not stripped', () => { + // objectstack#4001 closed the silent-drop shape. Every parity assertion + // below depends on it: a stripping schema would make them all trivially + // green while the 422 still happened server-side. + 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', () => { + // Without these two, "ObjectSchema refuses `group`" would be worthless: a + // schema that refused every object would produce the same evidence. + expect(ObjectSchema.safeParse(BASE).success).toBe(true); + expect(ObjectSchema.safeParse({ ...BASE, isSystem: true }).success).toBe(true); + expect(ObjectSchema.safeParse({ ...BASE, pluralLabel: 'Accounts' }).success).toBe(true); + }); + + it('refuses each of the three keys BY NAME, one at a time', () => { + expect(unrecognizedKeys(ObjectSchema.safeParse({ ...BASE, group: 'Sales' }))).toEqual(['group']); + expect(unrecognizedKeys(ObjectSchema.safeParse({ ...BASE, sortOrder: 3 }))).toEqual(['sortOrder']); + expect( + unrecognizedKeys( + ObjectSchema.safeParse({ ...BASE, relationships: [{ relatedObject: 'contact', type: 'one-to-many' }] }), + ), + ).toEqual(['relationships']); + }); + + it('has no near-spelling for any of them — unlike objectui#6041, nothing here is a rename', () => { + const accept = new Set(Object.keys(ObjectSchema.shape as Record)); + expect(accept.size).toBe(42); + // `fieldGroups` is the only grouping key on the object, and it groups the + // FIELDS INSIDE one object — it is not a category for objects themselves, + // so `group` has no mapping target here. + expect(accept.has('fieldGroups')).toBe(true); + for (const key of ['group', 'sortOrder', 'relationships', 'order', 'category', 'sortField']) { + expect(accept.has(key), `ObjectSchema unexpectedly accepts \`${key}\``).toBe(false); + } + }); +}); + +describe('objectui#6223 · `group` — the manager’s display category, never the payload', () => { + it('does not put `group` on the wire even when the object carries one', async () => { + const put = await putFor(); + expect('group' in put).toBe(false); + // Falsification: the save really happened and really described this object. + expect(put.name).toBe('account'); + expect(put.label).toBe('Account'); + }); + + it('and `ObjectSchema` reports no `group` among the refused keys of that body', async () => { + expect(unrecognizedKeys(ObjectSchema.safeParse(await putFor()))).not.toContain('group'); + }); +}); + +describe('objectui#6223 · `sortOrder` — list order, not object metadata', () => { + it('does not put `sortOrder` on the wire even when the object carries one', async () => { + const put = await putFor(); + expect('sortOrder' in put).toBe(false); + expect(put.name).toBe('account'); + }); + + it('and `ObjectSchema` reports no `sortOrder` among the refused keys of that body', async () => { + expect(unrecognizedKeys(ObjectSchema.safeParse(await putFor()))).not.toContain('sortOrder'); + }); + + it('leaves the FIELD-level `sortOrder` alone — that key is objectui#6045 and is not this card', async () => { + // The two keys share a spelling and nothing else. Reverting the object-level + // resolution must not read as progress on the field-level one, and this + // assertion is what keeps the two cards independently measurable. + const { adapter, puts } = makeCapturingAdapter(); + await new MetadataService(adapter).saveFields('account', [ + { id: 'name', name: 'name', label: 'Name', type: 'text', sortOrder: 7 }, + ]); + const fields = puts[puts.length - 1].fields as Record[]; + expect(fields[0].sortOrder).toBe(7); + }); +}); + +describe('objectui#6223 · `relationships` — the spec models these on the FIELD', () => { + it('does not put `relationships` on the wire even when the object carries them', async () => { + const put = await putFor(); + expect('relationships' in put).toBe(false); + expect(put.name).toBe('account'); + }); + + it('and `ObjectSchema` reports no `relationships` among the refused keys of that body', async () => { + expect(unrecognizedKeys(ObjectSchema.safeParse(await putFor()))).not.toContain('relationships'); + }); +}); + +describe('objectui#6223 · the whole body, and the honest limit of this fix', () => { + it('carries NO key `ObjectSchema` refuses by name', async () => { + // The claim of this card, stated once over the whole document rather than + // key by key. Before the fix this was ["group", "sortOrder", "relationships"]. + expect(unrecognizedKeys(ObjectSchema.safeParse(await putFor()))).toEqual([]); + }); + + it('still carries everything the spec DOES accept — the fix removed keys, it did not empty the payload', async () => { + // Falsification for the assertion above: a payload of `{}` would also carry + // no refused key, and that is not the fix. + const put = await putFor(); + expect(put).toMatchObject({ + name: 'account', + label: 'Account', + pluralLabel: 'Accounts', + description: 'Customer accounts', + icon: 'Building', + }); + }); + + it('does NOT parse green as a whole — `fields` is an array where the spec wants a map (objectui#6240)', async () => { + // Stated as an assertion rather than left as prose, because a reader who + // saw only the `unrecognized_keys` case above would reasonably conclude the + // designer's object payload is now spec-valid. It is not: this fix closes + // the KEY-NAME class, which is the class objectui#6223 and its parity gate + // are about. What remains is a VALUE-level rejection — the gate's coverage + // note 4 — and it is filed separately as objectui#6240. + const result = ObjectSchema.safeParse(await putFor()); + expect(result.success).toBe(false); + expect(unrecognizedKeys(result)).toEqual([]); + expect(result.error?.issues.map((i) => `${i.code} @ ${i.path.join('.')}`)).toEqual([ + 'invalid_type @ fields', + ]); + }); + + it('a half-filled object — no group, no sortOrder, no relationships — puts identical bytes, as it always did', async () => { + // ⚠ This case would still pass on a revert, deliberately. `undefined` is + // dropped by `JSON.stringify`, so an object that never had these keys + // populated produced byte-identical output before and after this fix. It is + // here to prove the fix did not newly break the untouched half, which is a + // claim about what did NOT change. + const put = await putFor({ id: 'lead', name: 'lead', label: 'Lead' }); + expect(Object.keys(put).sort()).toEqual(['fields', 'label', 'name']); + expect(unrecognizedKeys(ObjectSchema.safeParse(put))).toEqual([]); + }); +}); diff --git a/packages/app-shell/src/services/MetadataService.ts b/packages/app-shell/src/services/MetadataService.ts index 03c1e93f31..53da577dae 100644 --- a/packages/app-shell/src/services/MetadataService.ts +++ b/packages/app-shell/src/services/MetadataService.ts @@ -30,16 +30,27 @@ export interface ObjectMetadataPayload { pluralLabel?: string; description?: string; icon?: string; - group?: string; - sortOrder?: number; + // No `group` (objectui#6223): `ObjectSchema` has no object-level grouping + // key — its 42-key accept set contains `fieldGroups`, which groups the FIELDS + // inside one object, and nothing that categorises objects against each other. + // The designer's grouping IS a real feature, but a UI-only one: the Object + // Manager's group column and its group select are display categories derived + // from the object itself (`sys_` prefix / `isSystem` -> `System Objects` vs + // `Custom Objects`), never authored data the server round-trips. Writing it + // made `PUT /api/v1/meta/object/:name` refuse the key by name. + // No `sortOrder` (objectui#6223): `ObjectSchema` has no object-level ordering + // key either. What populated it was the ARRAY INDEX the converter happened to + // be at (`sortOrder: index`), i.e. the order the list was already in — a + // display concern of the manager, not object metadata. (Distinct from the + // field-level `sortOrder`, objectui#6045, which is still declared below.) enabled?: boolean; fields?: FieldMetadataPayload[]; - relationships?: Array<{ - relatedObject: string; - type: string; - label?: string; - foreignKey?: string; - }>; + // No `relationships` (objectui#6223): the spec models relationships on the + // FIELD — `reference` / `master_detail` plus object-level `indexes` — and + // `ObjectSchema` refuses an object-level `relationships` array by name. What + // the designer should author for a relationship is a data-model question + // that this card does not settle; what it settles is that this shape must + // stop putting the key on the wire. } /** Shape written to the metadata API for a field definition. */ @@ -76,7 +87,16 @@ export interface FieldMetadataPayload { // Converters: UI types → API payloads // --------------------------------------------------------------------------- -/** Convert an `ObjectDefinition` (UI) to the API payload shape. */ +/** + * Convert an `ObjectDefinition` (UI) to the API payload shape. + * + * `ObjectDefinition` carries three keys that deliberately do NOT cross into the + * payload (objectui#6223): `group` and `sortOrder` are the Object Manager's own + * display category and display order, and `relationships` has no object-level + * home in the spec. `ObjectSchema` refuses all three BY NAME, so copying them + * across is what turned a designer save into a 422. The UI model keeps them; + * the wire shape does not. + */ function toObjectPayload(obj: ObjectDefinition, fields?: FieldMetadataPayload[]): ObjectMetadataPayload { return { name: obj.name, @@ -84,10 +104,7 @@ function toObjectPayload(obj: ObjectDefinition, fields?: FieldMetadataPayload[]) pluralLabel: obj.pluralLabel, description: obj.description, icon: obj.icon, - group: obj.group, - sortOrder: obj.sortOrder, fields, - relationships: obj.relationships, }; } diff --git a/packages/app-shell/src/utils/metadataConverters.ts b/packages/app-shell/src/utils/metadataConverters.ts index 8a6aae2e39..36e085f972 100644 --- a/packages/app-shell/src/utils/metadataConverters.ts +++ b/packages/app-shell/src/utils/metadataConverters.ts @@ -83,6 +83,12 @@ export function toObjectDefinition(obj: MetadataObject, index: number): ObjectDe pluralLabel: obj.pluralLabel || obj.plural_label || undefined, description: typeof obj.description === 'object' ? obj.description.defaultValue : (obj.description || undefined), icon: obj.icon || undefined, + // `group` and `sortOrder` are DISPLAY values of the Object Manager, derived + // here and belonging to the UI model only (objectui#6223). `ObjectSchema` + // has no object-level grouping or ordering key and refuses both BY NAME, so + // they must never be copied into an object payload — see the tombstones on + // `ObjectMetadataPayload`. Note what populates them: the `sys_` prefix and + // the array index, i.e. facts about this list, not about the object. group: obj.name?.startsWith('sys_') ? 'System Objects' : 'Custom Objects', sortOrder: index, isSystem: obj.name?.startsWith('sys_') || false, diff --git a/packages/plugin-designer/src/MetadataObjectsPage.specKeyGroup.test.tsx b/packages/plugin-designer/src/MetadataObjectsPage.specKeyGroup.test.tsx new file mode 100644 index 0000000000..38ba711265 --- /dev/null +++ b/packages/plugin-designer/src/MetadataObjectsPage.specKeyGroup.test.tsx @@ -0,0 +1,254 @@ +/** + * 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#6223 — the Object Manager's grouping stays a DISPLAY category and + * never reaches `PUT /api/v1/meta/object/:name`. + * + * Surfaced by the key-level parity gate built for objectui#5761 + * (`scripts/check-designer-field-key-parity.mjs`) once objectui#6223 gave it a + * second oracle. `ServerObjectSchema` is one of that gate's object-level `wire` + * shapes: `handleObjectsChange` merges the manager's edits onto the raw server + * document and PUTs the result. + * + * `group` is not in `ObjectSchema`'s 42-key accept set. Measured against the + * installed `@objectstack/spec` 17.2.0: + * + * ObjectSchema.safeParse({ ...base, group: 'Sales' }) => success = false + * unrecognized_keys keys=["group"] + * ObjectSchema.safeParse({ ...base, isSystem: true }) => success = true (control) + * + * The object-level `isSystem` control matters twice over: it is what proves + * this is a key-by-key result, and it is the key this file now DERIVES the + * display group from. + * + * ## Two halves, and the second is why a write-only fix would not do + * + * WRITE — the merged save-back carried `group` verbatim, so a designer save + * put a key the schema refuses on the wire. + * SPREAD — `merged` is built by spreading the raw server document, so an + * object that already had `group` stored from before this fix would + * spread it straight back out and stay permanently unsaveable. Not + * writing the key is not the same as removing it, and the second case + * below is the one that distinguishes them. + * + * ## Assertions are on captured PUT bytes + * + * `undefined` is a key zod's strict object COUNTS and `JSON.stringify` DROPS, + * so an assertion on the object handed to the client and an assertion on the + * wire disagree on exactly the case this card cares about. + * + * This file names neither `sortOrder` nor `relationships` anywhere: those two + * keys are resolved in `MetadataService` and pinned in its own sibling file, so + * reverting one key's resolution reds only that key's assertions. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { act, render, waitFor } from '@testing-library/react'; +import { ObjectSchema } from '@objectstack/spec/data'; +import { MetadataClient } from '@object-ui/data-objectstack'; +import type { ObjectDefinition } from '@object-ui/types'; + +/** + * Two objects as the server holds them. + * + * `account` is what a SPEC-PARSED server sends: no `group`, because the + * schema refuses the key and so it was never stored. + * `legacy_widget` carries a `group` a pre-fix designer build could have left + * behind. The merge spreads the previous document verbatim, so without the + * strip this key rides straight back out to the route that rejects it. + */ +const ACCOUNT = { + name: 'account', + label: 'Account', + pluralLabel: 'Accounts', + icon: 'Building', + isSystem: false, + fields: { name: { type: 'text', label: 'Name' } }, +}; + +const LEGACY = { + name: 'legacy_widget', + label: 'Legacy Widget', + group: 'Integration', + isSystem: false, + fields: { name: { type: 'text', label: 'Name' } }, +}; + +/** A system object, so both derived categories are exercised on real data. */ +const SYS_USER = { + name: 'sys_user', + label: 'User', + isSystem: true, + fields: { name: { type: 'text', label: 'Name' } }, +}; + +interface RecordedManagerProps { + objects: ObjectDefinition[]; + onObjectsChange?: (objects: ObjectDefinition[]) => void; + showSystemObjects?: boolean; + readOnly?: boolean; +} + +let managerProps: RecordedManagerProps | null = null; + +vi.mock('./ObjectManager', () => ({ + ObjectManager: (props: RecordedManagerProps) => { + managerProps = props; + return null; + }, +})); + +import { MetadataObjectsPage } from './MetadataObjectsPage'; + +let puts: Array> = []; + +function json(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); +} + +function realClient(): MetadataClient { + return new MetadataClient({ + baseUrl: 'http://localhost:3000', + fetch: (async (input: RequestInfo | URL, init?: RequestInit) => { + const method = (init?.method ?? 'GET').toUpperCase(); + if (method === 'PUT') { + puts.push(JSON.parse(String(init?.body ?? '{}')) as Record); + return json({ success: true }); + } + void input; + return json({ items: [ACCOUNT, LEGACY, SYS_USER] }); + }) as unknown as typeof fetch, + }); +} + +async function renderPage() { + render(); + await waitFor(() => expect(managerProps).not.toBeNull()); + await waitFor(() => expect(managerProps!.objects).toHaveLength(3)); +} + +/** Edit one object through the manager and wait for the resulting PUT. */ +async function editObject(name: string, patch: Partial) { + const next = managerProps!.objects.map((o) => (o.name === name ? { ...o, ...patch } : o)); + await act(async () => { + managerProps!.onObjectsChange!(next); + }); + await waitFor(() => expect(puts.length).toBeGreaterThan(0)); + return puts[puts.length - 1]; +} + +const unrecognizedKeys = (result: ReturnType): string[] => + result.success + ? [] + : result.error.issues + .filter((i) => i.code === 'unrecognized_keys') + .flatMap((i) => (i as unknown as { keys: string[] }).keys); + +beforeEach(() => { + puts = []; + managerProps = null; +}); + +afterEach(() => { + managerProps = null; +}); + +describe('the instrument', () => { + it('is the installed spec schema and it is STRICT — unknown keys are refused, not stripped', () => { + const base = { name: 'account', label: 'Account', fields: { n: { type: 'text', label: 'N' } } }; + const result = ObjectSchema.safeParse({ ...base, zzzDefinitelyNotAKey: 1 }); + expect(result.success).toBe(false); + expect(unrecognizedKeys(result)).toContain('zzzDefinitelyNotAKey'); + }); + + it('refuses `group` by name and accepts `isSystem` — the two states this file distinguishes', () => { + const base = { name: 'account', label: 'Account', fields: { n: { type: 'text', label: 'N' } } }; + expect(unrecognizedKeys(ObjectSchema.safeParse({ ...base, group: 'Sales' }))).toEqual(['group']); + expect(ObjectSchema.safeParse({ ...base, isSystem: true }).success).toBe(true); + }); +}); + +describe('objectui#6223 · the grouping control still works — it is DERIVED, not round-tripped', () => { + it('hands a group down to the manager for every object, from the key the spec DOES accept', async () => { + await renderPage(); + const account = managerProps!.objects.find((o) => o.name === 'account')!; + // Before this fix the page read `raw.group` — a key `ObjectSchema` refuses, + // so the server never stored one and this rendered EMPTY on every row. + expect(account.group).toBe('Custom Objects'); + expect(account.isSystem).toBe(false); + }); + + it('derives the system category too, so BOTH values are reachable and neither is a constant', async () => { + // Falsification for the case above: a hardcoded 'Custom Objects' would + // satisfy it. This one shows the derivation actually reads the object, and + // reads it from `isSystem` — a key `ObjectSchema` accepts, so the server + // really does send it. + await renderPage(); + const sys = managerProps!.objects.find((o) => o.name === 'sys_user')!; + const custom = managerProps!.objects.find((o) => o.name === 'account')!; + expect(sys.group).toBe('System Objects'); + expect(custom.group).toBe('Custom Objects'); + expect(sys.isSystem).toBe(true); + }); + + it('ignores a legacy stored `group` on the way IN as well — the derived value wins', async () => { + // `legacy_widget` has `group: 'Integration'` stored. Reading it back would + // re-admit a key the schema refuses into the UI model, and from there into + // the next save. The derivation is unconditional for exactly that reason. + await renderPage(); + const legacy = managerProps!.objects.find((o) => o.name === 'legacy_widget')!; + expect(legacy.group).toBe('Custom Objects'); + expect(legacy.group).not.toBe('Integration'); + }); +}); + +describe('objectui#6223 · WRITE — the merged save-back carries no `group`', () => { + it('does not put `group` on the wire when the manager edits an object', async () => { + await renderPage(); + const put = await editObject('account', { label: 'Customer Account' }); + expect('group' in put).toBe(false); + // Falsification: the edit really was saved, and the rest of the document + // survived the merge. + expect(put.label).toBe('Customer Account'); + expect(put.name).toBe('account'); + expect(put.pluralLabel).toBe('Accounts'); + expect(put.fields).toBeDefined(); + }); + + it('and `ObjectSchema` reports no refused key at all in that body', async () => { + await renderPage(); + const put = await editObject('account', { label: 'Customer Account' }); + expect(unrecognizedKeys(ObjectSchema.safeParse(put))).toEqual([]); + }); +}); + +describe('objectui#6223 · SPREAD — a `group` already stored on the server is stripped, not re-sent', () => { + it('drops a legacy stored `group` instead of spreading it back out', async () => { + // The half a write-only fix would miss. `merged` is built from `...base`, + // the raw server document, so an object saved by a pre-fix build would keep + // failing forever: every later save re-sent the stored key. + await renderPage(); + const put = await editObject('legacy_widget', { label: 'Renamed Widget' }); + expect('group' in put).toBe(false); + expect(put.label).toBe('Renamed Widget'); + expect(unrecognizedKeys(ObjectSchema.safeParse(put))).toEqual([]); + }); + + it('the fixture really did carry the key — otherwise the case above proves nothing', () => { + // Non-vacuity: if `LEGACY` ever lost its `group` the assertion above would + // stay green while testing nothing at all. + expect(LEGACY.group).toBe('Integration'); + expect( + unrecognizedKeys(ObjectSchema.safeParse({ ...LEGACY, label: 'Renamed Widget' })), + ).toEqual(['group']); + }); +}); diff --git a/packages/plugin-designer/src/MetadataObjectsPage.tsx b/packages/plugin-designer/src/MetadataObjectsPage.tsx index 72c35d11ad..33e2fde1a0 100644 --- a/packages/plugin-designer/src/MetadataObjectsPage.tsx +++ b/packages/plugin-designer/src/MetadataObjectsPage.tsx @@ -38,14 +38,22 @@ import type { ObjectDefinition } from '@object-ui/types'; import { MetadataClient, type MetadataClientConfig } from '@object-ui/data-objectstack'; import { ObjectManager } from './ObjectManager'; -/** Minimal shape we consume from a framework ObjectSchema payload. */ +/** + * Minimal shape we consume from a framework ObjectSchema payload — and, merged + * back in {@link MetadataObjectsPage}, the body of `PUT /api/v1/meta/object/:name`. + */ interface ServerObjectSchema { name: string; label?: string; pluralLabel?: string; description?: string; icon?: string; - group?: string; + // No `group` (objectui#6223): `ObjectSchema` has no object-level grouping key + // and refuses this one BY NAME, so the merged save-back made the whole PUT a + // 422 `INVALID_METADATA` — which then blocks EVERY later save of that object. + // Nothing is lost by dropping it: a key the schema refuses was never stored, + // so `raw.group` was always absent on the way back in. The manager's grouping + // is a display category and `toObjectDefinition` derives it below. isSystem?: boolean; fields?: Record; [key: string]: unknown; @@ -87,7 +95,13 @@ function toObjectDefinition(raw: ServerObjectSchema): ObjectDefinition { pluralLabel: raw.pluralLabel, description: raw.description, icon: raw.icon, - group: raw.group, + // Derived, never round-tripped (objectui#6223). `group` is the Object + // Manager's display category; `ObjectSchema` has no object-level grouping + // key, so the server neither stores nor serves one. Deriving it from the + // spec key that IS accepted (`isSystem`) keeps the grouping control + // populated with the same two `OBJECT_GROUPS` entries the sibling + // converter uses, instead of reading a key that can only ever be absent. + group: (raw.isSystem ?? false) ? 'System Objects' : 'Custom Objects', isSystem: raw.isSystem ?? false, fieldCount, }; @@ -191,9 +205,16 @@ export function MetadataObjectsPage({ pluralLabel: updated.pluralLabel, description: updated.description, icon: updated.icon, - group: updated.group, + // `group` is deliberately not merged back (objectui#6223) — see the + // note on `ServerObjectSchema`. isSystem: updated.isSystem, }; + // `...base` is a verbatim spread of whatever the server sent, so simply + // not writing `group` is not enough: an object that already has the key + // stored from before this fix would spread it straight back out and stay + // permanently unsaveable. Strip it on the way out too — the objectui#4644 + // strip-on-load shape, applied on the write side where the spread is. + delete merged.group; // Don't issue redundant saves if nothing visible changed. if ( prev[updated.name] @@ -201,7 +222,6 @@ export function MetadataObjectsPage({ && prev[updated.name].pluralLabel === merged.pluralLabel && prev[updated.name].description === merged.description && prev[updated.name].icon === merged.icon - && prev[updated.name].group === merged.group ) { continue; } diff --git a/scripts/__tests__/check-designer-field-key-parity.test.ts b/scripts/__tests__/check-designer-field-key-parity.test.ts index addfce874c..bf8561d93e 100644 --- a/scripts/__tests__/check-designer-field-key-parity.test.ts +++ b/scripts/__tests__/check-designer-field-key-parity.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { FieldSchema } from '@objectstack/spec/data'; +import { FieldSchema, ObjectSchema } from '@objectstack/spec/data'; // Plain-JS CI helper. Its types are INFERRED from the .mjs source by // `tsconfig.scripts.json` (`allowJs`), so no `@ts-expect-error` here. @@ -13,6 +13,7 @@ import { analyze, declaredKeys, fieldSchemaAcceptSet, + objectSchemaAcceptSet, } from '../check-designer-field-key-parity.mjs'; /** @@ -179,6 +180,145 @@ describe('reach classification — a UI-only key is not a wire violation', () => }); }); +describe('the second oracle — objectui#6223', () => { + const OBJECT_WIRE = { + id: 'FixtureObjectPayload', + file: 'object-payload.ts', + interface: 'FixtureObjectPayload', + schema: 'ObjectSchema' as const, + reach: 'wire' as const, + writer: 'fixture', + }; + const OBJECT_UI = { + id: 'FixtureObjectUi', + file: 'object-ui.ts', + interface: 'FixtureObjectUi', + schema: 'ObjectSchema' as const, + reach: 'ui' as const, + writer: 'fixture', + }; + + it('resolves the very same `ObjectSchema` object `@objectstack/spec/data` exports', async () => { + // Reference identity, exactly as for the field oracle: `plugin-designer` + // declares its own `ServerObjectSchema` subset type, which is one of this + // gate's INPUTS and must never be mistaken for its oracle. A structural + // check could not tell the two apart; `===` can. This also keeps the second + // oracle on the ESM build for the same dual-package reason. + const { schema } = await objectSchemaAcceptSet(); + expect(schema).toBe(ObjectSchema); + }); + + it('reads an object accept set that is REAL and DIFFERENT from the field one', async () => { + // If the two oracles ever resolved to the same schema the object-level + // check would be vacuous while looking like it ran, so the difference is + // asserted rather than assumed. + const { accept: objectKeys } = await objectSchemaAcceptSet(); + const { accept: fieldKeys } = await fieldSchemaAcceptSet(); + expect(objectKeys.has('fields')).toBe(true); + expect(objectKeys.has('isSystem')).toBe(true); + expect(objectKeys.has('group')).toBe(false); + expect(objectKeys.has('zzzDefinitelyNotAKey')).toBe(false); + expect(objectKeys.size).toBeGreaterThan(20); + expect(objectKeys.size).not.toBe(fieldKeys.size); + }); + + it('is STRICT — an object document refuses unknown keys rather than stripping them', () => { + const base = { name: 'account', label: 'Account', fields: { n: { type: 'text', label: 'N' } } }; + const stripped = ObjectSchema.safeParse({ ...base, zzzDefinitelyNotAKey: 1 }); + expect(stripped.success).toBe(false); + expect(stripped.error?.issues.map((i) => i.code)).toContain('unrecognized_keys'); + }); + + it('goes red on an object-level key the OBJECT schema refuses', async () => { + // The objectui#6223 instance, as a negative control: `group` on an + // object-level wire shape. + await withFixture( + { 'object-payload.ts': 'export interface FixtureObjectPayload {\n name?: string;\n label?: string;\n group?: string;\n}\n' }, + async (dir) => { + const { violations } = await analyze(dir, { shapes: [OBJECT_WIRE], ledger: {} }); + expect(violations.map((v) => `${v.key}:${v.oracle}`)).toEqual(['group:ObjectSchema']); + }, + ); + }); + + it('the same key really is refused by the real ObjectSchema, with `unrecognized_keys`', async () => { + // The control above proves the GATE reports it. This one proves the key is + // genuinely refused, so the gate is not red about something harmless. + const parsed = ObjectSchema.safeParse({ + name: 'account', + label: 'Account', + fields: { n: { type: 'text', label: 'N' } }, + group: 'Sales', + }); + expect(parsed.success).toBe(false); + const issue = parsed.error?.issues.find((i) => i.code === 'unrecognized_keys'); + expect((issue as { keys: string[] }).keys).toContain('group'); + }); + + it('routes each shape to ITS OWN oracle — `group` is legal on a field and refused on an object', async () => { + // The one assertion that a single pooled accept set could not satisfy, and + // the reason the second oracle is a per-shape property rather than a union. + // `FieldSchema` accepts `group`; `ObjectSchema` does not. A gate that + // checked every shape against a merged set would have stayed green on + // exactly the three keys objectui#6223 found. + expect(FieldSchema.safeParse({ type: 'text', label: 'L', group: 'Details' }).success).toBe(true); + await withFixture( + { + 'payload.ts': 'export interface FixturePayload {\n type?: string;\n label?: string;\n group?: string;\n}\n', + 'object-payload.ts': 'export interface FixtureObjectPayload {\n name?: string;\n label?: string;\n group?: string;\n}\n', + }, + async (dir) => { + const { violations } = await analyze(dir, { shapes: [WIRE_SHAPE, OBJECT_WIRE], ledger: {} }); + expect(violations.map((v) => `${v.shape}.${v.key}`)).toEqual(['FixtureObjectPayload.group']); + }, + ); + }); + + it('resolves REACH within an oracle, never across one', async () => { + // `group` on an object UI model is uiOnly — but only because no OBJECT wire + // shape declares it. A field wire shape declaring the same spelling must + // not launder it into a violation, nor a violation into a uiOnly. + await withFixture( + { + 'payload.ts': 'export interface FixturePayload {\n type?: string;\n label?: string;\n group?: string;\n}\n', + 'object-ui.ts': 'export interface FixtureObjectUi {\n name?: string;\n label?: string;\n group?: string;\n}\n', + }, + async (dir) => { + const { violations, uiOnly } = await analyze(dir, { shapes: [WIRE_SHAPE, OBJECT_UI], ledger: {} }); + expect(violations).toEqual([]); + expect(uiOnly.map((u) => `${u.shape}.${u.key}`)).toEqual(['FixtureObjectUi.group']); + }, + ); + }); + + it('and the object UI key becomes a violation the moment an object WIRE shape declares it', async () => { + await withFixture( + { + 'object-payload.ts': 'export interface FixtureObjectPayload {\n name?: string;\n label?: string;\n group?: string;\n}\n', + 'object-ui.ts': 'export interface FixtureObjectUi {\n name?: string;\n label?: string;\n group?: string;\n}\n', + }, + async (dir) => { + const { violations, uiOnly } = await analyze(dir, { shapes: [OBJECT_WIRE, OBJECT_UI], ledger: {} }); + expect(uiOnly).toEqual([]); + expect(violations.map((v) => `${v.shape}.${v.key}`)).toEqual([ + 'FixtureObjectPayload.group', + 'FixtureObjectUi.group', + ]); + }, + ); + }); + + it('throws when the object oracle cannot be resolved — a missing schema is never a pass', async () => { + await expect( + analyze(repoRoot, { + shapes: [OBJECT_WIRE], + ledger: {}, + importSpec: async () => ({ FieldSchema }), + }), + ).rejects.toThrow(/no longer exports `ObjectSchema`/); + }); +}); + describe('the ledger ratchets in both directions', () => { const LEDGER = { zzzLedgeredKey: { card: 'objectui#0000', spec: null, note: 'fixture' } }; @@ -258,6 +398,49 @@ describe('the real shapes, on the real tree', () => { expect(declaredKeys(repoRoot, server!).indexSignature).toBe(true); }); + it('carries an object-level oracle over both object wire shapes — objectui#6223', async () => { + // The gate shipped for objectui#5761 had three field shapes and no object + // shape, so the parent document those fields are nested in was unchecked. + // If that ever regresses this assertion says so, rather than the next + // object-level key being found by a user hitting a save-blocking 422. + const objectShapes = PAYLOAD_SHAPES.filter((s) => s.schema === 'ObjectSchema'); + expect(objectShapes.map((s) => s.id).sort()).toEqual([ + 'ObjectDefinition', + 'ObjectMetadataPayload', + 'ServerObjectSchema', + ]); + expect(objectShapes.filter((s) => s.reach === 'wire').map((s) => s.id).sort()).toEqual([ + 'ObjectMetadataPayload', + 'ServerObjectSchema', + ]); + // Every shape names its oracle explicitly — an entry that forgot to would + // silently fall back to the field schema and be checked against the wrong + // accept set. + for (const shape of PAYLOAD_SHAPES) { + expect(shape.schema, `${shape.id} names no oracle`).toBeTruthy(); + } + }); + + it('keeps `group`, `sortOrder` and `relationships` on the UI model and off every object wire shape', async () => { + // The structural claim objectui#6223 landed, asserted on the real tree + // rather than on a fixture: the Object Manager may hold all three (they are + // its display category, its display order, and a UI-model relationship + // list), and no shape that becomes a PUT body may declare any of them. + const { uiOnly, violations } = await analyze(repoRoot); + const onObjectDefinition = uiOnly.filter((u) => u.shape === 'ObjectDefinition').map((u) => u.key); + for (const key of ['group', 'sortOrder', 'relationships']) { + expect(onObjectDefinition, `${key} left the UI model`).toContain(key); + expect(violations.map((v) => v.key), `${key} is back on a wire shape`).not.toContain(key); + } + for (const id of ['ObjectMetadataPayload', 'ServerObjectSchema']) { + const shape = PAYLOAD_SHAPES.find((s) => s.id === id)!; + const { keys } = declaredKeys(repoRoot, shape); + expect(keys, `${id} declares group`).not.toContain('group'); + expect(keys, `${id} declares sortOrder`).not.toContain('sortOrder'); + expect(keys, `${id} declares relationships`).not.toContain('relationships'); + } + }); + it('is green — every refused key on the tree is filed and ledgered', async () => { const { violations, staleLedger } = await analyze(repoRoot); expect(violations).toEqual([]); diff --git a/scripts/check-designer-field-key-parity.mjs b/scripts/check-designer-field-key-parity.mjs index bb23ee72aa..08a778bd1d 100644 --- a/scripts/check-designer-field-key-parity.mjs +++ b/scripts/check-designer-field-key-parity.mjs @@ -1,10 +1,18 @@ #!/usr/bin/env node /** - * Every key a field designer's statically declared payload shape can emit must - * be a key the installed `FieldSchema` accepts by name. + * Every key a designer's statically declared payload shape can emit must be a + * key the installed spec schema that judges that shape accepts by name. * - * The failure class (objectui#5761): a field designer offers a control that - * writes a key `FieldSchema` refuses BY NAME. The author sees the control work + * Two oracles, because the designers PUT a two-level document. A field shape is + * judged by `FieldSchema`, the parent object document by `ObjectSchema`, and + * each shape in {@link PAYLOAD_SHAPES} names the one that judges it. The gate + * shipped for objectui#5761 carried only the field oracle, so nothing checked + * the document those fields are nested in — and objectui#6223 then found three + * object-level keys (`group`, `sortOrder`, `relationships`) that `ObjectSchema` + * refuses by name, sitting on the wire the whole time the gate was green. + * + * The failure class (objectui#5761): a designer offers a control that + * writes a key the spec refuses BY NAME. The author sees the control work * — and, in metadata-admin, sees the preview render it — then * `PUT /api/v1/meta/object/:name` returns a hard 422 `INVALID_METADATA` that * blocks EVERY subsequent save of that object until the key is stripped. The @@ -73,10 +81,10 @@ * refinement) is green here and still a 422 in production. * * ── The accept set is read from the schema, never listed here ─────────────── - * `FieldSchema` is a strict zod object: it refuses unknown keys with - * `unrecognized_keys` rather than stripping them (objectstack#4001 closed the - * silent-drop shape). Its accept set is read off the schema's own `shape` at - * run time. + * `FieldSchema` and `ObjectSchema` are strict zod objects: they refuse unknown + * keys with `unrecognized_keys` rather than stripping them (objectstack#4001 + * closed the silent-drop shape). Each accept set is read off the schema's own + * `shape` at run time. * * It is read through a dynamic `import()`, NOT `createRequire`, and that is * load-bearing rather than stylistic. `@objectstack/spec` is a dual-package @@ -170,6 +178,7 @@ export const PAYLOAD_SHAPES = [ id: "FieldMetadataPayload", file: "packages/app-shell/src/services/MetadataService.ts", interface: "FieldMetadataPayload", + schema: "FieldSchema", reach: "wire", // `toFieldPayload` builds it; `saveFields` PUTs `fields.map(toFieldPayload)` // and `saveObject` PUTs it through `toObjectPayload`. @@ -179,6 +188,7 @@ export const PAYLOAD_SHAPES = [ id: "ServerFieldSchema", file: "packages/plugin-designer/src/MetadataFieldsPage.tsx", interface: "ServerFieldSchema", + schema: "FieldSchema", reach: "wire", // `fromDesignerField` builds it; `MetadataFieldsPage` PUTs the assembled // `fields` map. Carries an index signature — see coverage note 2. @@ -188,9 +198,40 @@ export const PAYLOAD_SHAPES = [ id: "DesignerFieldDefinition", file: "packages/types/src/designer.ts", interface: "DesignerFieldDefinition", + schema: "FieldSchema", reach: "ui", writer: "FieldDesigner (in-memory model)", }, + { + id: "ObjectMetadataPayload", + file: "packages/app-shell/src/services/MetadataService.ts", + interface: "ObjectMetadataPayload", + schema: "ObjectSchema", + reach: "wire", + // `toObjectPayload` builds it; `saveObject` PUTs it whole to + // `PUT /api/v1/meta/object/:name`, fields nested inside. + writer: "MetadataService.saveObject", + }, + { + id: "ServerObjectSchema", + file: "packages/plugin-designer/src/MetadataObjectsPage.tsx", + interface: "ServerObjectSchema", + schema: "ObjectSchema", + reach: "wire", + // `handleObjectsChange` merges the manager's edits onto the raw server + // payload and PUTs the result. Carries an index signature — coverage + // note 2 applies here too, and with more force: this shape is BUILT by + // spreading the server's own document. + writer: "MetadataObjectsPage.handleObjectsChange", + }, + { + id: "ObjectDefinition", + file: "packages/types/src/designer.ts", + interface: "ObjectDefinition", + schema: "ObjectSchema", + reach: "ui", + writer: "ObjectManager (in-memory model)", + }, ]; /** @@ -215,16 +256,28 @@ export const KNOWN_UNPARSEABLE_KEYS = { spec: null, // the spec has `sortable` (a boolean), and no field-level ordering key note: "Latent: declared and written by `toFieldPayload`, but nothing populates it, so JSON drops the undefined. One reorder feature away from live.", }, + enabled: { + card: "objectui#6238", + // `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.", + }, }; +/** The oracle names a shape may name, in the order they are reported. */ +export const ORACLES = ["FieldSchema", "ObjectSchema"]; + /** - * The keys the INSTALLED `FieldSchema` accepts, read off the schema itself. + * The keys the INSTALLED schema named by `exportName` accepts, read off the + * schema itself. * * Resolved through `@objectstack/spec/data` — the published subpath, the same * one the runtime parses with — so this is the schema that actually judges a * `PUT`, not a local look-alike. Extraction failure throws; see the header. */ -export async function fieldSchemaAcceptSet(importSpec = (id) => import(id)) { +export async function schemaAcceptSet(exportName, importSpec = (id) => import(id)) { let data; try { data = await importSpec("@objectstack/spec/data"); @@ -235,17 +288,17 @@ export async function fieldSchemaAcceptSet(importSpec = (id) => import(id)) { ` (${err && err.message})` ); } - const schema = data.FieldSchema; + const schema = data[exportName]; if (!schema) { fail( - "@objectstack/spec/data no longer exports `FieldSchema` — the accept set cannot be derived.\n" + - " Re-point this gate at the schema that judges a field payload; do NOT hardcode a key list." + `@objectstack/spec/data no longer exports \`${exportName}\` — the accept set cannot be derived.\n` + + " Re-point this gate at the schema that judges that payload; do NOT hardcode a key list." ); } const keys = shapeKeys(schema); if (!keys || keys.length === 0) { fail( - "could not resolve `FieldSchema`'s shape from @objectstack/spec/data.\n" + + `could not resolve \`${exportName}\`'s shape from @objectstack/spec/data.\n` + " The schema's internal representation changed. Fix the walk — falling back to a\n" + " hardcoded key list would make this gate the stale copy it exists to prevent." ); @@ -253,6 +306,16 @@ export async function fieldSchemaAcceptSet(importSpec = (id) => import(id)) { return { schema, accept: new Set(keys), origin: specOrigin() }; } +/** {@link schemaAcceptSet} pinned to the field oracle. */ +export async function fieldSchemaAcceptSet(importSpec = (id) => import(id)) { + return schemaAcceptSet("FieldSchema", importSpec); +} + +/** {@link schemaAcceptSet} pinned to the object oracle. */ +export async function objectSchemaAcceptSet(importSpec = (id) => import(id)) { + return schemaAcceptSet("ObjectSchema", importSpec); +} + /** * The file the accept set was read from, for the run log. `createRequire` is * used ONLY here — `import.meta.resolve` is not available in every Node this @@ -323,56 +386,110 @@ export function declaredKeys(root, shape) { return { keys, indexSignature }; } +/** The oracle a shape names, defaulting to the field one for older entries. */ +const oracleOf = (shape) => shape.schema ?? "FieldSchema"; + /** - * Compare every declared payload key against the accept set. + * Compare every declared payload key against the accept set OF THE ORACLE THAT + * JUDGES ITS SHAPE. + * + * Returns `{ accept, accepts, origin, shapes, violations, uiOnly, staleLedger }`. + * `violations` is what makes the gate red; `uiOnly` and `staleLedger` are + * reported too — `staleLedger` is red as well (see the header's + * both-directions ratchet). * - * Returns `{ accept, shapes, violations, uiOnly, staleLedger }`. `violations` - * is what makes the gate red; `uiOnly` and `staleLedger` are reported too — - * `staleLedger` is red as well (see the header's both-directions ratchet). + * Reach is resolved WITHIN an oracle, never across one. A key is `uiOnly` when + * no wire shape *judged by the same schema* declares it: `group` is a legal + * `FieldSchema` key and a refused `ObjectSchema` key at the same time, so a + * single pooled wire-key set would have let an object-level key hide behind a + * field-level shape that legitimately declares the same spelling. */ export async function analyze(root = REPO_ROOT, options = {}) { const shapes = options.shapes ?? PAYLOAD_SHAPES; const ledger = options.ledger ?? KNOWN_UNPARSEABLE_KEYS; - const { accept, origin } = options.acceptSet - ? { accept: options.acceptSet, origin: "(injected)" } - : await fieldSchemaAcceptSet(options.importSpec); + const needed = [...new Set(shapes.map(oracleOf))]; + + /** oracle name -> accept set. `acceptSet` (singular) applies to every oracle. */ + const accepts = new Map(); + let origin = "(injected)"; + for (const name of needed) { + if (options.acceptSets && options.acceptSets[name]) { + accepts.set(name, options.acceptSets[name]); + } else if (options.acceptSet) { + accepts.set(name, options.acceptSet); + } else { + const resolved = await schemaAcceptSet(name, options.importSpec); + accepts.set(name, resolved.accept); + origin = resolved.origin; + } + } const read = shapes.map((shape) => ({ shape, ...declaredKeys(root, shape) })); - const wireKeys = new Set(read.filter((r) => r.shape.reach === "wire").flatMap((r) => r.keys)); + + /** oracle name -> every key declared on a wire shape judged by that oracle. */ + const wireKeysByOracle = new Map( + needed.map((name) => [ + name, + new Set(read.filter((r) => r.shape.reach === "wire" && oracleOf(r.shape) === name).flatMap((r) => r.keys)), + ]) + ); const violations = []; const uiOnly = []; const ledgered = new Set(); for (const { shape, keys } of read) { + const oracle = oracleOf(shape); + const accept = accepts.get(oracle); for (const key of keys) { if (accept.has(key)) continue; - if (shape.reach === "ui" && !wireKeys.has(key)) { - uiOnly.push({ shape: shape.id, file: shape.file, key }); + if (shape.reach === "ui" && !wireKeysByOracle.get(oracle).has(key)) { + uiOnly.push({ shape: shape.id, file: shape.file, key, oracle }); continue; } if (Object.prototype.hasOwnProperty.call(ledger, key)) { ledgered.add(key); continue; } - violations.push({ shape: shape.id, file: shape.file, writer: shape.writer, key }); + violations.push({ shape: shape.id, file: shape.file, writer: shape.writer, key, oracle }); } } // Both-directions ratchet: an entry that no longer applies must not survive. const declaredEverywhere = new Set(read.flatMap((r) => r.keys)); + const acceptedBy = (key) => + [...new Set(read.filter((r) => r.keys.includes(key)).map((r) => oracleOf(r.shape)))].filter((name) => + accepts.get(name).has(key) + ); const staleLedger = Object.keys(ledger) .filter((key) => !ledgered.has(key)) - .map((key) => ({ - key, - reason: !declaredEverywhere.has(key) - ? "no payload shape declares it any more" - : accept.has(key) - ? "`FieldSchema` now accepts it" - : "it is no longer reachable from a wire-bound shape", - })); + .map((key) => { + if (!declaredEverywhere.has(key)) { + return { key, reason: "no payload shape declares it any more" }; + } + const accepting = acceptedBy(key); + // Every shape that still declares it is judged by a schema that now + // accepts it — the objectui#4676 shape, where the producer moved upstream. + const stillRefused = read.some( + (r) => r.keys.includes(key) && !accepts.get(oracleOf(r.shape)).has(key) + ); + if (accepting.length > 0 && !stillRefused) { + return { key, reason: `\`${accepting.join("` / `")}\` now accepts it` }; + } + return { key, reason: "it is no longer reachable from a wire-bound shape" }; + }); - return { accept, origin, shapes: read, violations, uiOnly, staleLedger }; + // `accept` is kept as the field oracle's set for callers that predate the + // second oracle; `accepts` is the full map. + return { + accept: accepts.get("FieldSchema") ?? accepts.get(needed[0]), + accepts, + origin, + shapes: read, + violations, + uiOnly, + staleLedger, + }; } async function main() { @@ -388,18 +505,20 @@ async function main() { throw err; } - const { accept, origin, shapes, violations, uiOnly, staleLedger } = result; - console.log(`designer-field-key-parity: FieldSchema accepts ${accept.size} keys`); + const { accepts, origin, shapes, violations, uiOnly, staleLedger } = result; + for (const [name, accept] of accepts) { + console.log(`designer-field-key-parity: ${name} accepts ${accept.size} keys`); + } console.log(` oracle: ${origin}`); for (const { shape, keys, indexSignature } of shapes) { console.log( - ` ${shape.id.padEnd(24)} ${String(keys.length).padStart(2)} declared [${shape.reach}]` + + ` ${shape.id.padEnd(24)} ${String(keys.length).padStart(2)} declared [${shape.reach}] vs ${(shape.schema ?? "FieldSchema").padEnd(12)}` + (indexSignature ? " (+ index signature — see coverage note 2)" : "") ); } if (uiOnly.length) { - console.log("\n UI-only keys (declared on no wire-bound shape, so out of reach of a PUT):"); - for (const u of uiOnly) console.log(` ${u.key} (${u.shape})`); + console.log("\n UI-only keys (declared on no wire-bound shape of the same oracle, so out of reach of a PUT):"); + for (const u of uiOnly) console.log(` ${u.key.padEnd(16)} (${u.shape}, vs ${u.oracle})`); } const ledgerKeys = Object.keys(KNOWN_UNPARSEABLE_KEYS); if (ledgerKeys.length) { @@ -422,7 +541,7 @@ async function main() { } if (violations.length) { - console.error("\ndesigner-field-key-parity: KEYS `FieldSchema` REFUSES BY NAME\n"); + console.error("\ndesigner-field-key-parity: KEYS THE SPEC REFUSES BY NAME\n"); // Grouped by KEY, not by site: one key declared on three shapes is one // decision to make, and reading it three times obscures that. const byKey = new Map(); @@ -435,6 +554,7 @@ async function main() { for (const v of sites) { console.error(` declared on ${v.shape} (${v.file})`); console.error(` written by ${v.writer}`); + console.error(` refused by ${v.oracle}`); } } console.error( From fcca348f5cfdd3789ada8c2534d6f6ac7922869f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 04:09:10 +0000 Subject: [PATCH 2/2] fix(scripts): scope each parity-ledger entry to the oracle that refuses its key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by objectui#6223's own per-key ablation, and it is a hole the second oracle opened. Re-declaring the OBJECT-level `sortOrder` left the gate GREEN: the ledger is keyed by key NAME, so objectui#6045's FIELD-level entry absorbed an object-level reappearance in silence — the ledger becoming the hiding place the gate's header says it must never be. Entries now name their oracle (defaulting to `FieldSchema`) and are matched on `(key, oracle)`. Staleness is scoped the same way in both directions: an entry whose oracle no shape declares the key under is stale even when a shape of the OTHER oracle still declares that spelling. Two executable controls carry it, both derived from the measurement above, plus an assertion that every real entry names an oracle rather than defaulting into one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L --- .../check-designer-field-key-parity.test.ts | 54 +++++++++++++++++++ scripts/check-designer-field-key-parity.mjs | 47 +++++++++++++--- 2 files changed, 94 insertions(+), 7 deletions(-) diff --git a/scripts/__tests__/check-designer-field-key-parity.test.ts b/scripts/__tests__/check-designer-field-key-parity.test.ts index bf8561d93e..cbab0cf6e5 100644 --- a/scripts/__tests__/check-designer-field-key-parity.test.ts +++ b/scripts/__tests__/check-designer-field-key-parity.test.ts @@ -308,6 +308,57 @@ describe('the second oracle — objectui#6223', () => { ); }); + it('scopes a ledger entry to ITS oracle — one level\'s card must not absorb the other level\'s key', async () => { + // Measured during objectui#6223's ablation, which is why it is pinned here. + // `sortOrder` is refused at BOTH levels by two different schemas and is two + // different cards (objectui#6045 field-level, objectui#6223 object-level). + // With a name-keyed ledger, re-declaring the OBJECT-level key stayed green: + // the field-level card's entry absorbed it in silence. That is the ledger + // becoming a hiding place, which the header says it must never be. + const FIELD_LEDGER = { + zzzTwoLevelKey: { card: 'objectui#0000', oracle: 'FieldSchema', spec: null, note: 'fixture' }, + }; + await withFixture( + { + 'payload.ts': 'export interface FixturePayload {\n type?: string;\n label?: string;\n zzzTwoLevelKey?: string;\n}\n', + 'object-payload.ts': 'export interface FixtureObjectPayload {\n name?: string;\n label?: string;\n zzzTwoLevelKey?: string;\n}\n', + }, + async (dir) => { + const { violations, staleLedger } = await analyze(dir, { + shapes: [WIRE_SHAPE, OBJECT_WIRE], + ledger: FIELD_LEDGER, + }); + // The field-level occurrence is covered by its card... + expect(staleLedger).toEqual([]); + // ...and the object-level one is NOT, because the entry is not scoped to + // that oracle. Exactly one violation, on the object shape. + expect(violations.map((v) => `${v.shape}.${v.key}`)).toEqual([ + 'FixtureObjectPayload.zzzTwoLevelKey', + ]); + }, + ); + }); + + it('an entry scoped to an oracle no shape of that oracle declares is stale', async () => { + // The other direction of the same scope. An ObjectSchema-scoped entry is + // not kept alive by a FIELD shape that happens to declare the same + // spelling, or the entry would outlive the refusal it was filed for. + await withFixture( + { 'payload.ts': 'export interface FixturePayload {\n type?: string;\n label?: string;\n zzzTwoLevelKey?: string;\n}\n' }, + async (dir) => { + const { staleLedger } = await analyze(dir, { + shapes: [WIRE_SHAPE], + ledger: { + zzzTwoLevelKey: { card: 'objectui#0000', oracle: 'ObjectSchema', spec: null, note: 'fixture' }, + }, + }); + expect(staleLedger).toEqual([ + { key: 'zzzTwoLevelKey', reason: 'no payload shape declares it any more' }, + ]); + }, + ); + }); + it('throws when the object oracle cannot be resolved — a missing schema is never a pass', async () => { await expect( analyze(repoRoot, { @@ -453,6 +504,9 @@ describe('the real shapes, on the real tree', () => { for (const [key, entry] of entries) { 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); } }); }); diff --git a/scripts/check-designer-field-key-parity.mjs b/scripts/check-designer-field-key-parity.mjs index 08a778bd1d..94d962b3d5 100644 --- a/scripts/check-designer-field-key-parity.mjs +++ b/scripts/check-designer-field-key-parity.mjs @@ -239,6 +239,12 @@ export const PAYLOAD_SHAPES = [ * owns its resolution. NOT a suppression list: see the header's ratchet note — * an entry that stops applying is as red as a key that is missing one. * + * `oracle` scopes the entry to the schema that refuses the key, defaulting to + * `FieldSchema`. It is load-bearing, not decoration: `sortOrder` is refused at + * BOTH levels by two different schemas, and they are two different cards with + * two different resolutions. Without the scope, one card's entry would absorb + * the other level's occurrence and the gate would stay green over it. + * * `spec` records the accepted spelling where the spec has one, because that is * the fact a resolver needs first and the fact most likely to be wrong in a * hurry. It is documentation for the card, never an instruction to rename: @@ -248,16 +254,24 @@ export const PAYLOAD_SHAPES = [ export const KNOWN_UNPARSEABLE_KEYS = { formula: { card: "objectui#6043", + oracle: "FieldSchema", spec: "expression (+ returnType)", note: "LIVE. FieldDesigner renders a textarea for it on `type == 'formula'`. Not a rename: the spec's `expression` is CEL, so the key and the expression LANGUAGE move together.", }, sortOrder: { card: "objectui#6045", + // Scoped to the FIELD oracle deliberately. `sortOrder` is refused at BOTH + // levels and the two are different cards with different resolutions + // (objectui#6223 removed the object-level one). An unscoped entry would let + // this card's entry absorb an object-level reappearance in silence, which + // is the ledger becoming the hiding place the header says it must not be. + oracle: "FieldSchema", spec: null, // the spec has `sortable` (a boolean), and no field-level ordering key note: "Latent: declared and written by `toFieldPayload`, but nothing populates it, so JSON drops the undefined. One reorder feature away from live.", }, 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. @@ -447,8 +461,9 @@ export async function analyze(root = REPO_ROOT, options = {}) { uiOnly.push({ shape: shape.id, file: shape.file, key, oracle }); continue; } - if (Object.prototype.hasOwnProperty.call(ledger, key)) { - ledgered.add(key); + const entry = Object.prototype.hasOwnProperty.call(ledger, key) ? ledger[key] : null; + if (entry && (entry.oracle ?? "FieldSchema") === oracle) { + ledgered.add(`${key}\u0000${oracle}`); continue; } violations.push({ shape: shape.id, file: shape.file, writer: shape.writer, key, oracle }); @@ -456,22 +471,37 @@ export async function analyze(root = REPO_ROOT, options = {}) { } // Both-directions ratchet: an entry that no longer applies must not survive. - const declaredEverywhere = new Set(read.flatMap((r) => r.keys)); + const ledgerOracle = (key) => ledger[key].oracle ?? "FieldSchema"; + /** key -> the oracles whose shapes still declare it. */ + const declaredUnder = new Map(); + for (const r of read) { + for (const key of r.keys) { + if (!declaredUnder.has(key)) declaredUnder.set(key, new Set()); + declaredUnder.get(key).add(oracleOf(r.shape)); + } + } + const acceptedBy = (key) => [...new Set(read.filter((r) => r.keys.includes(key)).map((r) => oracleOf(r.shape)))].filter((name) => accepts.get(name).has(key) ); const staleLedger = Object.keys(ledger) - .filter((key) => !ledgered.has(key)) + .filter((key) => !ledgered.has(`${key}\u0000${ledgerOracle(key)}`)) .map((key) => { - if (!declaredEverywhere.has(key)) { + // Scoped to the entry's own oracle: a key still declared somewhere, but + // no longer on any shape THIS entry could apply to, is exactly as stale + // as one nothing declares at all. + if (!declaredUnder.get(key)?.has(ledgerOracle(key))) { return { key, reason: "no payload shape declares it any more" }; } const accepting = acceptedBy(key); // Every shape that still declares it is judged by a schema that now // accepts it — the objectui#4676 shape, where the producer moved upstream. const stillRefused = read.some( - (r) => r.keys.includes(key) && !accepts.get(oracleOf(r.shape)).has(key) + (r) => + r.keys.includes(key) && + oracleOf(r.shape) === ledgerOracle(key) && + !accepts.get(oracleOf(r.shape)).has(key) ); if (accepting.length > 0 && !stillRefused) { return { key, reason: `\`${accepting.join("` / `")}\` now accepts it` }; @@ -525,7 +555,10 @@ async function main() { console.log("\n Ledgered — refused, filed, resolution owned by its card:"); for (const key of ledgerKeys) { const e = KNOWN_UNPARSEABLE_KEYS[key]; - console.log(` ${key.padEnd(14)} ${e.card}` + (e.spec ? ` (spec spells it \`${e.spec}\`)` : " (no spec equivalent)")); + console.log( + ` ${key.padEnd(14)} ${e.card} [${e.oracle ?? "FieldSchema"}]` + + (e.spec ? ` (spec spells it \`${e.spec}\`)` : " (no spec equivalent)") + ); } }