fix(metadata-protocol): an object's overlay row is a base layer, not its resolved schema (#8027)

.changeset/object-overlay-row-is-a-base-layer.md

@@ -0,0 +1,97 @@
+---
+'@objectstack/metadata-protocol': patch
+'@objectstack/objectql': patch
+---
+
+fix(metadata-protocol): an object's overlay row is a base layer, not its resolved schema (#8027)
+
+**Behaviour change, and it is a payload gaining fields.** When a `sys_metadata`
+overlay row exists for an object, `GET /meta/object/:name`, `GET /meta/object`
+and the `effective` layer of `?layers=true` now serve that object's RESOLVED
+schema — the overlay row as the base layer with its `objectExtensions`
+contributors folded on (ADR-0029 D9.2) — where they previously served the stored
+row verbatim. Any consumer of those routes sees the extension's fields appear on
+customised objects. An object with no overlay row, and an object nothing
+extends, are byte-identical (measured; see below).
+
+**A second payload change, in the other direction:** on an in-process
+(`bridged`) boot the by-name read and the `code` layer previously served every
+extender-contributed `validation` and `index` TWICE. That duplication is
+removed. It was a live regression introduced by #7556 (PR #8015) and is
+explained under "the fold is not idempotent" below.
+
+The defect: an overlay row for an object — an admin renaming the object's label
+in Studio — was adopted as the resolved schema. `getMetaItem` took the stored row
+as `item` and returned it; `getMetaItems` did the same through
+`mergePackageAwareOverlay`, which picks a per-slot winner WHOLESALE rather than
+merging fields; and `getMetaItemLayered`'s `effective` is `overlay ?? code`, so
+it inherited the same body. D9.2 defines the resolution as `overlay ?? own` with
+the `extend` contributors folded ON, which is exactly what
+`SchemaRegistry.resolveObject` does for an overlay it knows about, and what
+#7556 made the by-name read do for the MetadataService copy. The `sys_metadata`
+path was the one adopter that never folded.
+
+Measured with one `extend` contributor (three fields) and one env-wide overlay
+row: `byName` and `listed` both served the object with NO extension fields,
+while `layers.code` served them (#7556 folds it) and `layers.effective` did not
+— so a single `?layers=true` response reported a `code` layer that has the
+fields and an `effective` layer that does not, with an `overlay` layer showing a
+customisation that explained none of the difference. The practical cost is the
+#7556 shape again: an admin who customises a label silently removes three
+extension-contributed fields from every writable form, while the data API keeps
+accepting and persisting them.
+
+**The fold is not idempotent, and that is the hazard this fix had to clear
+rather than assume away.** `mergeObjectDefinitions` CONCATENATES `validations`
+and `indexes` (`fields` is a key-keyed spread and the scalar props are
+last-writer-wins, so those were always safe), so folding a body that has already
+been through the fold duplicates both — and a duplicated index does not fail a
+test, it fails a deployment. The precondition #7556 documented ("callers must
+apply this only to a base that has not been through the fold") turned out to be
+one no caller can honour, and two shipped call sites already violated it:
+
+1. **The MetadataService body on an in-process boot.** ObjectQL's
+ `bridgeObjectsToMetadataService` seeds that service from
+ `registry.getAllObjects()` — bodies that are already resolved — so #7556's
+ fold ran on a folded base and served every extender validation and index
+ twice. Its own pin could not see this: it compares FIELD NAMES, and the field
+ spread is idempotent.
+2. **A stored overlay row.** The write path persists the request body verbatim
+ (ADR-0005 §Validation), so the ordinary Studio GET → edit → PUT round-trip
+ stores whatever the read served — and since #7556 that read is folded. The
+ row is *defined* by D9.2 as the base layer, but nothing enforces it, and
+ seeded / imported / migrated / pre-existing rows are unconstrained besides.
+
+So `SchemaRegistry.foldObjectExtendersOnto` was made IDEMPOTENT instead of
+documented harder: an entry the `extend` contributors are about to add, already
+present in the base, is removed first and then re-added by the fold exactly
+once. Extenders are still concatenated against each other — two contributors
+declaring an identical rule still yield two, matching `resolveObject` — so
+nothing the fold did on an unfolded base is narrowed, and such a base is
+returned by reference, byte-identical.
+
+Levels: `metadata-protocol` is `patch` — it restores the contract these routes
+were already specified to answer, and it is the same reasoning #7556 used for
+the same routes. `objectql` is `patch` — no new public API (`foldObjectExtendersOnto`
+already exists since #7556); its documented contract moves from "not idempotent,
+callers must guarantee an unfolded base" to "idempotent", which is a defect fix
+rather than a capability, and no caller can be relying on duplicated validations.
+
+Pinned against the REGISTRY'S RESOLVED SCHEMA, in
+`packages/rest/src/meta-object-overlay-extension-fold.test.ts`, deliberately not
+as agreement between the two routes: #7556's `byName === listed` pin is green
+throughout this defect, because here both routes agree — on a body that has
+already lost the fields. Its author said so explicitly rather than let the pin
+imply coverage it did not have. Eight cases over real handlers / real protocol /
+real registry: the overlay case on both routes and on `layers.effective`; `code`
+and `effective` agreeing when the row customises nothing; `layers.overlay` still
+reporting only what the tenant stored (an extension is not a tenant
+customisation — the boundary #7556 drew); an already-folded row and a bridged
+host holding the idempotency; the no-overlay and no-extension controls; and an
+anti-vacuity case pinning that the fixtures ARE discriminated.
+
+Byte-identity measured directly, by dumping all three surfaces for nine hosts
+under this branch and under the pre-fix behaviour: 8 of 9 identical. The one that
+differs is the extended object on a `bridged` host, where the pre-fix payload
+carries `['owner_rule','ext_rule','ext_rule']` / `['owner_idx','ext_idx','ext_idx']`
+and this branch carries each once — the #7556 regression, repaired.

packages/metadata-protocol/src/protocol.ts

@@ -4146,10 +4146,14 @@ export class ObjectStackProtocolImplementation implements
  * (see {@link SchemaRegistry.foldObjectExtendersOnto}), so applying it twice
  * would duplicate both.
  */
- private foldObjectExtendersFromRegistry(type: string, name: string, body: unknown): unknown {
+ private foldObjectExtendersFromRegistry(type: string, name: unknown, body: unknown): unknown {
  const singular = PLURAL_TO_SINGULAR[type] ?? type;
  if (singular !== 'object') return body;
  if (body === null || typeof body !== 'object') return body;
+ // [#8027] The list caller reads the name off the ROW rather than off a
+ // request, and a row with no usable `name` has no contributor list to
+ // look up — `mergePackageAwareOverlay` skips such rows too.
+ if (typeof name !== 'string' || name === '') return body;
  const registry = (this.engine as any)?.registry;
  // Partial registry doubles in tests predate this method; a host that
  // cannot fold answers exactly as it did before.
@@ -4380,7 +4384,19 @@ export class ObjectStackProtocolImplementation implements
  const patch = viewIdentityPatch(data as Record<string, unknown>, prev);
  if (patch) Object.assign(data as Record<string, unknown>, patch);
  }
- return data;
+ // [#8027] The list half of the same rule the by-name read
+ // applies to its own overlay adoption. This merge REPLACES a
+ // base item with the overlay body wholesale (it is a
+ // per-slot layer pick, not a field merge), so for `object` —
+ // whose resolved schema is D9.2's base-plus-extenders — the
+ // winning row has to be resolved the same way `resolveObject`
+ // resolves the base it displaced, or the two reads of one
+ // object disagree the moment a row exists. Both routes were
+ // wrong here together, which is why #7556's byName===listed
+ // pin stayed green through this defect.
+ return this.foldObjectExtendersFromRegistry(
+ request.type, (data as { name?: unknown } | null)?.name, data,
+ );
  });
 
  // Only hydrate the global registry for unscoped (control-plane)
@@ -4777,6 +4793,30 @@ export class ObjectStackProtocolImplementation implements
  };
  }
 
+ // [#8027] An OBJECT's overlay row is a BASE LAYER, not a resolved
+ // schema. ADR-0029 D9.2 defines the resolution as `overlay ?? own` with
+ // the `extend` contributors folded ON — which is what
+ // `SchemaRegistry.resolveObject` does for an overlay it knows about, and
+ // what step 2 below already does (#7556) for the MetadataService copy.
+ // Step 1 was the one adopter that served its layer verbatim, so a single
+ // customisation row — an admin renaming the object's label in Studio —
+ // silently dropped every extension-contributed field from this read, and
+ // therefore from every writable form derived from it, while the data API
+ // kept accepting and persisting those same fields.
+ //
+ // Placed AFTER the draft return on purpose: a draft is a pending edit of
+ // the base layer, and folding it would show an author extension fields
+ // inside the document they are editing and about to PUT back. Drafts were
+ // never folded (before #7556 nothing was), so this leaves that asymmetry
+ // exactly where it already was rather than widening this fix into it.
+ //
+ // No-op for every type but `object`, for an object nothing extends, and
+ // for an item with no overlay row (`item` is still undefined here, and
+ // step 2 / step 3 fold their own sources).
+ if (item !== undefined) {
+ item = this.foldObjectExtendersFromRegistry(request.type, request.name, item);
+ }
+
  // 2. MetadataService (runtime-registered items: HMR-updated view/page/
  // dashboard/agent/tool, plus FilesystemLoader-sourced items). This
  // is consulted BEFORE the in-memory SchemaRegistry because the
@@ -5152,7 +5192,21 @@ export class ObjectStackProtocolImplementation implements
  // customised), and a Studio diff showing `code`'s declaration next to
  // `effective`'s governed value is the platform override made visible,
  // not a defect.
- const effective: unknown | null = governServedItem(request.type, overlay ?? code);
+ // [#8027] …and `effective` is "what `getMetaItem` would return", so when
+ // the overlay wins it is that read's BASE LAYER, resolved the same way:
+ // D9.2's base-plus-extenders. Without this the single `?layers=true`
+ // response contradicted itself — `code` carried the extension fields
+ // (#7556 folds it) and `effective` did not, with the `overlay` layer
+ // showing a customisation that explained none of the difference.
+ //
+ // ⛔ The fold lands on the EFFECTIVE base only. `overlay` stays the row
+ // the tenant actually stored: a code-declared extension is not a tenant
+ // customisation, and #7556 drew that boundary deliberately (the same
+ // reason `governServedItem` is called here and never on `overlay`).
+ const effectiveBase: unknown | null = overlay !== null
+ ? this.foldObjectExtendersFromRegistry(request.type, request.name, overlay)
+ : code;
+ const effective: unknown | null = governServedItem(request.type, effectiveBase);
 
  const _diagnostics =
  effective !== null && effective !== undefined

packages/objectql/src/registry.ts

@@ -141,6 +141,27 @@ function mergeObjectDefinitions(base: ServiceObject, extension: Partial<ServiceO
  return merged;
 }
 
+/**
+ * [#8027] Key-order-insensitive JSON, for comparing two `validations` /
+ * `indexes` entries BY VALUE — see
+ * {@link SchemaRegistry.subtractExtenderContributions}.
+ *
+ * Plain `JSON.stringify` would report the same rule as two different rules
+ * whenever the two copies were built by different code paths (a stored overlay
+ * row that went through `JSON.parse` vs the contributor's in-memory literal),
+ * which is exactly the pair being compared, so key order cannot be assumed.
+ * Arrays keep their order — an index's `fields` list is ordered and two
+ * spellings of it are genuinely two different indexes.
+ */
+function stableStringify(value: unknown): string {
+ if (value === null || typeof value !== 'object') return JSON.stringify(value) ?? 'null';
+ if (Array.isArray(value)) return `[${value.map(stableStringify).join(',')}]`;
+ const entries = Object.entries(value as Record<string, unknown>)
+ .filter(([, v]) => v !== undefined)
+ .sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0));
+ return `{${entries.map(([k, v]) => `${JSON.stringify(k)}:${stableStringify(v)}`).join(',')}}`;
+}
+
 /**
  * Global Schema Registry
  * Unified storage for all metadata types (Objects, Apps, Flows, Layouts, etc.)
@@ -1502,11 +1523,36 @@ export class SchemaRegistry {
  * Returns `base` untouched when nothing extends the name, so a caller may
  * apply it unconditionally.
  *
- * NOT idempotent, by construction: {@link mergeObjectDefinitions} CONCATENATES
- * `validations` and `indexes`, so folding an already-folded body would
- * duplicate both. Callers must apply this only to a base that has not been
- * through the fold — which is why the protocol applies it to the
- * MetadataService body and never to a registry-resolved one.
+ * [#8027] IDEMPOTENT, by construction — and it has to be, because the
+ * "callers must only pass an unfolded base" precondition this method carried
+ * when #7556 introduced it is not one any caller can honour.
+ *
+ * {@link mergeObjectDefinitions} CONCATENATES `validations` and `indexes`
+ * (`fields` is a key-keyed spread and the scalar props are last-writer-wins,
+ * so those were always safe). A second fold therefore used to duplicate both,
+ * and two shipped call sites already handed it a folded base:
+ *
+ * 1. The MetadataService body on an IN-PROCESS boot. ObjectQL's
+ * `bridgeObjectsToMetadataService` seeds that service from
+ * `registry.getAllObjects()` — bodies that are already resolved — so the
+ * by-name read and the layered `code` layer folded a folded body and
+ * served every extender-contributed validation and index TWICE. #7556's
+ * own pin could not see it: it compares FIELD NAMES, and the field spread
+ * is idempotent.
+ * 2. A `sys_metadata` overlay row (#8027). The write path persists the
+ * request body verbatim (ADR-0005 §Validation), so the ordinary Studio
+ * GET → edit → PUT round-trip stores whatever the read served — and since
+ * #7556 that read is folded. The row is DEFINED by D9.2 as the base
+ * layer, but nothing enforces it, and legacy/seeded/imported rows are
+ * unconstrained besides.
+ *
+ * So the precondition is dropped rather than documented harder: an entry the
+ * `extend` contributors are about to add, already sitting in `base`, is
+ * removed from the base first and then re-added by the fold exactly once.
+ * Extenders are still concatenated against EACH OTHER — two contributors
+ * declaring an identical rule still yield two, matching {@link resolveObject}
+ * — so this narrows nothing the fold was doing on an unfolded base, and such
+ * a base is returned byte-identically to before.
  */
  foldObjectExtendersOnto<T>(name: string, base: T): T {
  if (base === null || typeof base !== 'object') return base;
@@ -1516,10 +1562,48 @@ export class SchemaRegistry {
  if (!contributors || !contributors.some((c) => c.ownership === 'extend')) return base;
  return this.foldExtendersOntoDefinition(
  contributors,
- base as unknown as ServiceObject,
+this.subtractExtenderContributions(contributors, base as unknown as ServiceObject),
  ) as unknown as T;
  }
 
+ /**
+ * [#8027] Remove from `base` the `validations` / `indexes` entries the
+ * `extend` contributors are about to contribute — the half of
+ * {@link foldObjectExtendersOnto} that makes the fold idempotent.
+ *
+ * Only the two CONCATENATING keys are considered; `fields` and the scalar
+ * props already re-apply cleanly. Matching is by deep value equality on the
+ * serialised entry, so it fires only on an entry byte-identical to the one
+ * the extender declares — an entry the author merely gave the same `name` is
+ * a different rule and is left where it is.
+ *
+ * Returns `base` BY REFERENCE when nothing matched, which is the ordinary
+ * case (an unfolded base), so the pre-#8027 body is preserved exactly.
+ */
+ private subtractExtenderContributions(
+ contributors: ObjectContributor[],
+ base: ServiceObject,
+ ): ServiceObject {
+ const key = (entry: unknown): string => stableStringify(entry);
+ let out: ServiceObject | undefined;
+ for (const listKey of ['validations', 'indexes'] as const) {
+ const baseList = (base as Record<string, unknown>)[listKey];
+ if (!Array.isArray(baseList) || baseList.length === 0) continue;
+ const contributed = new Set<string>();
+ for (const contrib of contributors) {
+ if (contrib.ownership !== 'extend') continue;
+ const list = (contrib.definition as unknown as Record<string, unknown>)[listKey];
+ if (Array.isArray(list)) for (const entry of list) contributed.add(key(entry));
+ }
+ if (contributed.size === 0) continue;
+ const kept = baseList.filter((entry) => !contributed.has(key(entry)));
+ if (kept.length === baseList.length) continue;
+ out ??= { ...base };
+ (out as unknown as Record<string, unknown>)[listKey] = kept;
+ }
+ return out ?? base;
+ }
+
  /**
  * [ADR-0029 D9.6] The CODE-LAYER resolution of an object: the OWNER's
  * declaration with its extenders folded on, deliberately ignoring any tenant