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..cbab0cf6e5 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,196 @@ 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('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, { + 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 +449,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([]); @@ -270,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 bb23ee72aa..94d962b3d5 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)", + }, ]; /** @@ -198,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: @@ -207,24 +254,44 @@ 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. + 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 +302,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 +320,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 +400,126 @@ 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); + 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 }); + 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 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)) - .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", - })); + .filter((key) => !ledgered.has(`${key}\u0000${ledgerOracle(key)}`)) + .map((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) && + 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` }; + } + 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,25 +535,30 @@ 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) { 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)") + ); } } @@ -422,7 +574,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 +587,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(