fix(app-shell): PUT the object payload's fields as the spec's name-keyed map

.changeset/6240-object-payload-fields-map.md

@@ -0,0 +1,33 @@
+---
+'@object-ui/app-shell': patch
+---
+
+`MetadataService`'s object writers PUT `fields` as the name-keyed MAP `ObjectSchema`
+requires, not an array (objectui#6240). Both of the designer's write paths were affected,
+and `saveFields` ran the conversion in the wrong direction outright: the server's own
+document arrives with `fields` as a map, and `fields.map(toFieldPayload)` turned it into an
+array on every field save.
+
+Measured against the installed `@objectstack/spec` 17.2.0 and against the framework's own
+write door. `ObjectSchema.fields` is a required record: an array — empty or not — is
+refused `invalid_type @ fields`, a map parses. `metadata-protocol`'s `saveMetaItem`
+resolves metadata type `object` to that same `ObjectSchema`, `safeParse`s the whole item
+and throws `422 INVALID_METADATA` **before** persisting, so the array was refused rather
+than stripped or stored: every designer object save and every designer field save that went
+through this service was a 422 that wrote nothing.
+
+This is the value-level half of the objectui#5761 parity family and is invisible to that
+family's key-name gate — `fields` sits in the accept set under either shape, which is the
+gate's own coverage note 4. The pins are runtime assertions on the captured request bytes.
+
+The conversion refuses, loudly, what it cannot key: a field with a missing or blank `name`
+throws instead of writing a `{ undefined: … }` entry (measured: the spec ACCEPTS that
+document, so nothing downstream would have caught it), and a duplicate name throws instead
+of letting the later field silently replace the earlier — a loss an array does not have.
+`saveFields` keeps preserving unknown keys of the fetched server document, which now
+actually reaches storage. `saveObject` with no `existingFields` still omits the key rather
+than writing `{}`: a PUT is an upsert, so `{}` would delete every field of an object on a
+save that only meant to rename it.
+
+`saveObject(obj, existingFields)` keeps its `FieldMetadataPayload[]` parameter type — the
+array is converted inside — so no caller's call site changes.

packages/app-shell/src/services/MetadataService.retiredFieldSortOrder.test.ts

@@ -96,9 +96,18 @@ function makeCapturingAdapter() {
  return { adapter, puts };
 }
 
-/** The field defs of the last PUT, in wire order. */
+/**
+ * The field defs of the last PUT, in wire order.
+ *
+ * `fields` is a name-keyed MAP on the wire (objectui#6240 — `ObjectSchema`
+ * refuses an array at the value level), and this file's subject is what is
+ * INSIDE one field def, so it reads the map's values in insertion order, which
+ * is the only field order the spec has. The CONTAINER shape is pinned by
+ * `MetadataService.objectPayloadFieldsMap.test.ts`, deliberately not here.
+ */
 function savedFields(puts: Array<Record<string, unknown>>): Record<string, unknown>[] {
- return puts[puts.length - 1].fields as Record<string, unknown>[];
+ const fields = puts[puts.length - 1].fields as Record<string, Record<string, unknown>>;
+ return Object.values(fields);
 }
 
 const unrecognizedKeys = (result: ReturnType<typeof FieldSchema.safeParse>): string[] =>

packages/app-shell/src/services/MetadataService.retiredFormula.test.ts

@@ -69,9 +69,18 @@ function makeCapturingAdapter() {
  return { adapter, puts };
 }
 
-/** The field defs of the last PUT, in wire order. */
+/**
+ * The field defs of the last PUT, in wire order.
+ *
+ * `fields` is a name-keyed MAP on the wire (objectui#6240 — `ObjectSchema`
+ * refuses an array at the value level), and this file's subject is what is
+ * INSIDE one field def, so it reads the map's values in insertion order, which
+ * is the only field order the spec has. The CONTAINER shape is pinned by
+ * `MetadataService.objectPayloadFieldsMap.test.ts`, deliberately not here.
+ */
 function savedFields(puts: Array<Record<string, unknown>>): Record<string, unknown>[] {
- return puts[puts.length - 1].fields as Record<string, unknown>[];
+ const fields = puts[puts.length - 1].fields as Record<string, Record<string, unknown>>;
+ return Object.values(fields);
 }
 
 const unrecognizedKeys = (result: ReturnType<typeof FieldSchema.safeParse>): string[] =>

packages/app-shell/src/services/MetadataService.specKeyObjectPayload.test.ts

@@ -232,19 +232,33 @@ describe('objectui#6223 · the whole body, and the honest limit of this fix', ()
  });
  });
 
- 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);
+ it('parses green as a whole — the VALUE-level half closed too (objectui#6240)', async () => {
+ // ⭐ THIS CASE IS THE RED-TO-GREEN WITNESS OF objectui#6240, and it used to
+ // assert the OPPOSITE:
+ //
+ // expect(result.success).toBe(false);
+ // expect(issues).toEqual(['invalid_type @ fields']);
+ //
+ // That was correct and load-bearing while objectui#6240 was open. This file
+ // closed the KEY-NAME class (objectui#6223) and deliberately pinned the
+ // remaining VALUE-level rejection — the parity gate's coverage note 4 — so
+ // that it could not silently change. It has now changed, on purpose:
+ // `toObjectPayload` emits `fields` as the name-keyed MAP `ObjectSchema`
+ // requires instead of an array, so the whole document parses.
+ //
+ // The pin is REPLACED rather than deleted, because the claim it was really
+ // making — "judge the whole body, not just its key names" — is still the
+ // claim, and it is stronger green than red. The container shape itself,
+ // both writers, and the failure modes the conversion introduces are pinned
+ // in `MetadataService.objectPayloadFieldsMap.test.ts`.
+ const put = await putFor();
+ const result = ObjectSchema.safeParse(put);
  expect(unrecognizedKeys(result)).toEqual([]);
- expect(result.error?.issues.map((i) => `${i.code} @ ${i.path.join('.')}`)).toEqual([
- 'invalid_type @ fields',
- ]);
+ expect(result.error?.issues.map((i) => `${i.code} @ ${i.path.join('.')}`)).toBeUndefined();
+ expect(result.success).toBe(true);
+ // Falsification: green over a body that really carries the field, rather
+ // than over an emptied one.
+ expect(put.fields).toEqual({ name: { name: 'name', type: 'text', label: 'Name' } });
  });
 
  it('a half-filled object — no group, no sortOrder, no relationships — puts identical bytes, as it always did', async () => {

packages/app-shell/src/services/MetadataService.specKeyReference.test.ts

@@ -70,10 +70,18 @@ function makeCapturingAdapter() {
  return { adapter, puts };
 }
 
-/** The field defs of the last PUT, keyed by field name. */
+/**
+ * The field defs of the last PUT, in wire order.
+ *
+ * `fields` is a name-keyed MAP on the wire (objectui#6240 — `ObjectSchema`
+ * refuses an array at the value level), and this file's subject is what is
+ * INSIDE one field def, so it reads the map's values in insertion order, which
+ * is the only field order the spec has. The CONTAINER shape is pinned by
+ * `MetadataService.objectPayloadFieldsMap.test.ts`, deliberately not here.
+ */
 function savedFields(puts: Array<Record<string, unknown>>): Record<string, unknown>[] {
- const last = puts[puts.length - 1];
- return last.fields as Record<string, unknown>[];
+ const fields = puts[puts.length - 1].fields as Record<string, Record<string, unknown>>;
+ return Object.values(fields);
 }
 
 const unrecognizedKeys = (result: ReturnType<typeof FieldSchema.safeParse>): string[] =>

packages/app-shell/src/services/MetadataService.ts

@@ -52,7 +52,36 @@ export interface ObjectMetadataPayload {
  // `toObjectPayload`); the key reached the wire only through the tombstone
  // bodies the two delete methods wrote by hand, and those now go through the
  // metadata API's own delete door instead — see `deleteMetadataItem`.
- fields?: FieldMetadataPayload[];
+ /**
+ * The object's fields, keyed by field NAME — the map `ObjectSchema` requires
+ * (objectui#6240). This used to be `FieldMetadataPayload[]`.
+ *
+ * An array is a VALUE-level refusal, which is the class the key-name parity
+ * gate (`scripts/check-designer-field-key-parity.mjs`, coverage note 4)
+ * cannot see: `fields` is in `ObjectSchema`'s accept set under either shape,
+ * so the gate was green for as long as the array was on the wire.
+ *
+ * Measured against the installed `@objectstack/spec` 17.2.0 (ESM build):
+ *
+ * fields: [{ name: 'n', type: 'text', label: 'N' }]
+ * => invalid_type @ fields ("expected record, received array")
+ * fields: [] => invalid_type @ fields
+ * fields: { n: { type: 'text' } } => parses green
+ * fields: {} => parses green
+ * (key absent) => invalid_type @ fields — it is REQUIRED
+ *
+ * And the route is not lenient about it. `metadata-protocol`'s
+ * `saveMetaItem` resolves type `object` to this very `ObjectSchema`
+ * (`spec/kernel/metadata-type-schemas.ts`), `safeParse`s the whole item and
+ * THROWS `422 INVALID_METADATA` before anything is persisted — the array was
+ * refused, never stripped and never stored.
+ *
+ * Still OPTIONAL, deliberately — see `toObjectPayload`. `undefined` here
+ * means "the caller did not say what the fields are", and that must NOT be
+ * spelled `{}`: `{}` parses green and a PUT is an upsert, so it would land as
+ * "this object has no fields" and wipe them.
+ */
+ fields?: Record<string, FieldMetadataPayload>;
  // 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
@@ -133,10 +162,88 @@ function toObjectPayload(obj: ObjectDefinition, fields?: FieldMetadataPayload[])
  pluralLabel: obj.pluralLabel,
  description: obj.description,
  icon: obj.icon,
- fields,
+ // No fields supplied means the CALLER DID NOT SAY, which is not the same
+ // statement as `{}` ("this object has no fields") — and a PUT is an upsert,
+ // so writing `{}` here would delete every field of the object on a save
+ // that only meant to rename it. The body then still fails `ObjectSchema`
+ // (`fields` is required), which is a loud 422 that persists nothing — the
+ // right outcome for a caller that under-specified an upsert, and the same
+ // outcome as before this change. `saveFields` is the opposite case and
+ // treats its argument as authoritative; see there.
+ fields: fields ? toFieldsMap(fields) : undefined,
  };
 }
 
+/**
+ * Key a list of field payloads by field NAME — the shape `ObjectSchema.fields`
+ * requires (objectui#6240).
+ *
+ * Declaration order is preserved, and that is load-bearing rather than
+ * incidental: the spec models field order as DECLARATION ORDER in this record
+ * and has no field-level ordering key at all (objectui#6045), so insertion
+ * order here IS the designer's order.
+ *
+ * ## Why a missing name THROWS instead of writing `{ undefined: … }`
+ *
+ * The key can only come from the field's `name`. Both writer inputs declare it
+ * required (`FieldMetadataPayload.name`, `DesignerFieldDefinition.name`), but
+ * neither writer owns its input at runtime: `saveObject`'s `existingFields` is
+ * public API (`MetadataService` is reachable from app-shell's barrel through
+ * `useMetadataService`) and `saveFields` is handed whatever the designer's
+ * in-memory model holds. A nameless field keys as the literal string
+ * `"undefined"` — and the spec does NOT catch that. Measured on 17.2.0:
+ *
+ * ObjectSchema.safeParse({ …, fields: { undefined: { type: 'text', label: 'N' } } })
+ * => success = true
+ *
+ * So the loud, immediate, harmless array-shaped 422 would have been traded for
+ * a silently corrupt STORED document. That is the AI-authored-metadata failure
+ * mode this repo keeps closing, and this conversion is exactly where it would
+ * have been opened.
+ *
+ * ## Why a duplicate name throws too
+ *
+ * That one is the conversion's OWN hazard rather than an inherited one: an
+ * array can carry two entries named `n` and a map cannot, so the later entry
+ * would silently swallow the earlier. Refusing is the only reading that does
+ * not lose a field the caller declared.
+ *
+ * ## Why `Object.fromEntries` and not assignment into a literal
+ *
+ * `map['__proto__'] = field` does not create a key — it invokes the prototype
+ * setter — and `__proto__` is a SPEC-LEGAL field name (the record's key schema
+ * is `/^[a-z_][a-z0-9_]*$/`). The assignment form would therefore drop such a
+ * field silently, which is this function's whole subject wearing a different
+ * spelling. `Object.fromEntries` defines an own property instead.
+ */
+function toFieldsMap(fields: FieldMetadataPayload[]): Record<string, FieldMetadataPayload> {
+ const entries: Array<[string, FieldMetadataPayload]> = [];
+ const seen = new Set<string>();
+
+ fields.forEach((field, index) => {
+ const name = field?.name;
+ if (typeof name !== 'string' || name.trim() === '') {
+ throw new Error(
+ `[MetadataService] cannot build the object's \`fields\` map: the field at index ${index} has no ` +
+ '`name`. `ObjectSchema.fields` is keyed by field name, so a nameless field would be written under ' +
+ 'the literal key "undefined" — which the spec ACCEPTS, leaving a corrupt document stored with ' +
+ 'nothing to report it. Give the field a name.',
+ );
+ }
+ if (seen.has(name)) {
+ throw new Error(
+ `[MetadataService] cannot build the object's \`fields\` map: duplicate field name \`${name}\` at ` +
+ `index ${index}. A name-keyed map cannot carry two fields under one name, so the later one would ` +
+ 'silently replace the earlier. Rename or remove one of them.',
+ );
+ }
+ seen.add(name);
+ entries.push([name, field]);
+ });
+
+ return Object.fromEntries(entries);
+}
+
 /**
  * Convert a `DesignerFieldDefinition` (UI) to the API payload shape.
  *
@@ -314,8 +421,38 @@ export class MetadataService {
  /**
  * Persist updated fields for an object.
  *
- * Fetches the current object metadata, replaces its `fields` array with the
+ * Fetches the current object metadata, replaces its `fields` MAP with the
  * provided designer fields, and saves the whole object back.
+ *
+ * It used to write an ARRAY here (objectui#6240), and note which direction
+ * that ran in: the server's own document arrives with `fields` as a map, and
+ * `fields.map(toFieldPayload)` converted the correct shape INTO the refused
+ * one on every field save. `ObjectSchema` requires a record, so the resulting
+ * PUT was answered `422 INVALID_METADATA` (`invalid_type @ fields`) and
+ * nothing persisted — measured, not inferred: `metadata-protocol`'s
+ * `saveMetaItem` parses the whole item against `ObjectSchema` and throws
+ * before it writes.
+ *
+ * Two properties of this body are deliberate and are pinned in
+ * `MetadataService.objectPayloadFieldsMap.test.ts`:
+ *
+ * - **The spread still preserves unknown server keys.** `...existingObject`
+ * is what carries every key of the fetched document this service does not
+ * model, and reshaping `fields` must not cost that. It now matters more
+ * than it did, not less: while the body was refused, nothing it preserved
+ * ever reached storage.
+ * - **The field list is AUTHORITATIVE, so an empty one writes `{}`.** That
+ * is the opposite of `saveObject`'s optional `existingFields`, and the
+ * asymmetry is the point: here the designer is stating the object's
+ * complete field set, so "no fields" is a thing it can mean; there, a
+ * missing argument means the caller did not say.
+ *
+ * ⚠ Per-FIELD unknown keys are still not carried over — the entries are built
+ * fresh from the designer model, so a key the server sent inside one field
+ * (an `expression`, a `precision`) is dropped. That is unchanged by this
+ * card and its sibling writer already solves it (`MetadataFieldsPage`'s
+ * `carryOver`), but it becomes REACHABLE here for the first time now that the
+ * body is no longer refused. Filed separately rather than folded in.
  */
  async saveFields(objectName: string, fields: DesignerFieldDefinition[]): Promise<void> {
  const client = this.adapter.getClient();
@@ -332,7 +469,7 @@ export class MetadataService {
  const updatedObject = {
  ...existingObject,
  name: objectName,
- fields: fields.map(toFieldPayload),
+ fields: toFieldsMap(fields.map(toFieldPayload)),
  };
 
  await client.meta.saveItem('object', objectName, updatedObject);