From 50d27b121d119cd7e98a873c6127ab10d544e915 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 13:14:45 +0000 Subject: [PATCH] fix(metadata-protocol): an object's overlay row is a base layer, not its resolved schema (#8027) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `sys_metadata` overlay row for an object — an admin renaming the object's label in Studio — was adopted as the object's 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 that body. 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 #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 row: `byName` and `listed` served the object with NO extension fields while `layers.code` served them and `layers.effective` did not — one `?layers=true` response reporting a `code` layer that has the fields and an `effective` layer that does not, with an `overlay` layer explaining none of the difference. Same user-visible shape as #7556: 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 had to be cleared rather than assumed away. `mergeObjectDefinitions` CONCATENATES `validations` and `indexes` (`fields` is a key-keyed spread, the scalars are last-writer-wins). The precondition #7556 documented — "apply this only to a base that has not been through the fold" — is one no caller can honour, and two shipped call sites already violated it: 1. The MetadataService body on an in-process boot. `bridgeObjectsToMetadataService` seeds that service from `getAllObjects()`, so #7556's fold ran on a folded base and served every extender validation and index TWICE. Live on the parent commit; invisible to #7556's pin, which compares field names, and the field spread is idempotent. 2. A stored overlay row. The write path persists the body verbatim (ADR-0005 §Validation), so the Studio GET → edit → PUT round-trip stores whatever the read served — folded since #7556. D9.2 defines the row as the base layer but nothing enforces it, and seeded/imported/migrated rows are unconstrained besides. So `foldObjectExtendersOnto` is IDEMPOTENT instead of documented harder: an entry the extenders are about to add, already in the base, is removed first and re-added by the fold exactly once. Extenders still concatenate against each other (two contributors declaring an identical rule still yield two, matching `resolveObject`), and an unfolded base is returned by reference, unchanged. The `overlay` layer is deliberately left raw — it reports what a tenant customised, and a code-declared extension is not that (#7556's boundary). The draft read is deliberately left unfolded: a draft is a pending edit of the base layer and is PUT back verbatim. Pinned against the REGISTRY'S RESOLVED SCHEMA, not as agreement between the two routes: #7556's `byName === listed` pin is green throughout this defect, because both routes agree here — on a body that has already lost the fields. Eight cases over real handlers / real protocol / real registry, plus an anti-vacuity case pinning that the fixtures are discriminated. Byte-identity measured 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 `ext_rule`/`ext_idx` twice and this branch carries each once. Co-authored-by: Claude --- .../object-overlay-row-is-a-base-layer.md | 97 ++++ packages/metadata-protocol/src/protocol.ts | 60 ++- packages/objectql/src/registry.ts | 96 +++- ...meta-object-overlay-extension-fold.test.ts | 492 ++++++++++++++++++ 4 files changed, 736 insertions(+), 9 deletions(-) create mode 100644 .changeset/object-overlay-row-is-a-base-layer.md create mode 100644 packages/rest/src/meta-object-overlay-extension-fold.test.ts diff --git a/.changeset/object-overlay-row-is-a-base-layer.md b/.changeset/object-overlay-row-is-a-base-layer.md new file mode 100644 index 0000000000..91bc467a6c --- /dev/null +++ b/.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. diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index b2108ff969..650df716b9 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/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, prev); if (patch) Object.assign(data as Record, 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 diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index e8e6996ec4..b7dc17badb 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -141,6 +141,27 @@ function mergeObjectDefinitions(base: ServiceObject, extension: Partial) + .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(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)[listKey]; + if (!Array.isArray(baseList) || baseList.length === 0) continue; + const contributed = new Set(); + for (const contrib of contributors) { + if (contrib.ownership !== 'extend') continue; + const list = (contrib.definition as unknown as Record)[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)[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 diff --git a/packages/rest/src/meta-object-overlay-extension-fold.test.ts b/packages/rest/src/meta-object-overlay-extension-fold.test.ts new file mode 100644 index 0000000000..aa1dba35c3 --- /dev/null +++ b/packages/rest/src/meta-object-overlay-extension-fold.test.ts @@ -0,0 +1,492 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#8027] A tenant customisation is a BASE LAYER. It does not decide which +// fields the object has. +// +// The defect: `sys_metadata` holds an overlay row for an object (an admin +// renamed the object's label in Studio). `getMetaItem` adopted that row as the +// resolved schema, and `getMetaItems` did the same through +// `mergePackageAwareOverlay`, which picks a per-slot winner WHOLESALE rather +// than merging fields. 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 +// #7556 made the by-name read do for the MetadataService copy. The overlay path +// was the one adopter that still served its layer verbatim. +// +// Measured with one `extend` contributor and one env-wide overlay row: `byName` +// and `listed` both lost every extension field, `layers.code` kept 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 explains +// none of the difference. An admin who renames a label silently removes three +// extension-contributed fields from every writable form, while the data API +// keeps accepting and persisting them. +// +// WHY THIS FILE DOES NOT PIN AGREEMENT. #7556's pin is `byName === listed`, and +// it CANNOT catch this: here both routes agree — on a body that has already +// lost the fields. Its author said so rather than let the pin imply coverage it +// did not have. So the load-bearing assertion here is against the REGISTRY'S +// RESOLVED SCHEMA — what D9.2 defines — and never against the other route. The +// agreement dimension is kept as a second, weaker check. +// +// ⭐ AND THE FOLD IS NOT IDEMPOTENT, which is the trap this fix had to clear +// rather than assume away: `mergeObjectDefinitions` CONCATENATES `validations` +// and `indexes`. 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. Two shipped call sites already hand it a folded base (the +// `bridged` host below, live on `main`; and a stored row saved from a folded +// read), so `foldObjectExtendersOnto` was made idempotent instead. The +// `prefolded` and `bridged` hosts here are what hold that. + +import { describe, it, expect, vi } from 'vitest'; +import { SchemaRegistry } from '@objectstack/objectql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server.js'; + +/** The three fields the showcase's `objectExtensions` entry contributes. */ +const EXTENSION_FIELDS = ['loyalty_tier', 'linkedin_url', 'csat_score'] as const; + +/** The owner package's declaration — what a compiled artifact stores under `objects`. */ +const OWNER_DECLARATION = { + name: 'showcase_account', + label: 'Account', + fields: { + name: { name: 'name', label: 'Name', type: 'text' }, + industry: { name: 'industry', label: 'Industry', type: 'text' }, + }, + validations: [ + { name: 'owner_rule', type: 'script', message: 'Name is required', condition: 'record.name == null' }, + ], + indexes: [{ name: 'owner_idx', fields: ['name'] }], +}; + +/** + * What the artifact stores under `objectExtensions` — a SEPARATE collection. + * + * It contributes `validations` and `indexes` as well as fields, deliberately: + * those are the two keys the fold CONCATENATES, so they are the only place a + * second fold is observable. A fixture contributing fields alone would make + * every idempotency case below pass vacuously (#7556's pin compares field + * names, which is exactly why the double-fold on `bridged` survived it). + */ +const EXTENSION_DECLARATION = { + name: 'showcase_account', + // Deliberately declares NO `label`. `mergeObjectDefinitions` applies an + // extender's scalar props last-writer-wins, so an extension that relabels + // its target overrides the TENANT's overlay label — D9.2 as written, and + // visible here only because this file (unlike #7556's) compares more than + // field names. A contributor that adds fields is the population this issue + // is about; keeping a label out of it lets the cases below pin that the + // tenant's own customisation survives the fold. + fields: { + loyalty_tier: { name: 'loyalty_tier', label: 'Loyalty Tier', type: 'text' }, + linkedin_url: { name: 'linkedin_url', label: 'LinkedIn URL', type: 'url' }, + csat_score: { name: 'csat_score', label: 'CSAT Score', type: 'number' }, + }, + validations: [ + { name: 'ext_rule', type: 'script', message: 'CSAT is 0-5', condition: 'record.csat_score > 5' }, + ], + indexes: [{ name: 'ext_idx', fields: ['loyalty_tier'] }], +}; + +/** An object NOTHING extends — the control that keeps every other payload honest. */ +const UNEXTENDED_DECLARATION = { + name: 'showcase_task', + label: 'Task', + fields: { title: { name: 'title', label: 'Title', type: 'text' } }, + validations: [ + { name: 'task_rule', type: 'script', message: 'Title is required', condition: 'record.title == null' }, + ], + indexes: [{ name: 'task_idx', fields: ['title'] }], +}; + +const clone = (v: T): T => JSON.parse(JSON.stringify(v)) as T; + +function createMockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), + use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +/** Field names off a served body, tolerant of both wire shapes. */ +function fieldNamesOf(item: unknown): string[] { + const fields = (item as { fields?: unknown } | null | undefined)?.fields; + if (!fields) return []; + const names = Array.isArray(fields) + ? (fields as Array<{ name?: unknown }>).map((f) => String(f?.name)) + : Object.keys(fields as Record); + return [...names].sort(); +} + +/** + * The columns the platform injects into every served object body + * (`governServedItem` / #6562), listed here for the same reason #6562's own + * suite lists them. + * + * Filtered off BOTH sides of every comparison below, so each pin is about the + * AUTHORED field set — which is what an `extend` contributor contributes to and + * what this issue is about. They are not filtered off the `toContain` checks: + * an extension field must be present in the payload as actually served. + */ +const PLATFORM_COLUMNS: readonly string[] = [ + 'id', 'created_at', 'created_by', 'updated_at', 'updated_by', + 'organization_id', 'owner_id', 'owning_business_unit_id', +]; + +/** Served field names minus {@link PLATFORM_COLUMNS}. */ +function declaredFieldsOf(item: unknown): string[] { + return fieldNamesOf(item).filter((n) => !PLATFORM_COLUMNS.includes(n)); +} + +/** + * `validations` / `indexes` entry names IN ORDER, with duplicates preserved. + * Order and multiplicity are the whole point: a second fold shows up here and + * nowhere else. + */ +function entryNames(item: unknown, key: 'validations' | 'indexes'): string[] { + const list = (item as Record | null | undefined)?.[key]; + return Array.isArray(list) ? list.map((v) => String((v as { name?: unknown })?.name)) : []; +} + +/** How this host's `metadata` service was populated. */ +type ServiceMode = + /** Artifact ingest: owner declaration only; extensions live in their own collection. */ + | 'artifact' + /** In-process boot: `bridgeObjectsToMetadataService` seeds it from the MERGED registry. */ + | 'bridged' + /** No `metadata` service at all — the read falls through to the registry. */ + | 'absent'; + +/** What, if anything, `sys_metadata` holds for the object. */ +type OverlayMode = + /** No row. The ordinary shape, and the one that must stay byte-identical. */ + | 'none' + /** A row that customises the label — the tenant edit this issue is about. */ + | 'customised' + /** A row byte-identical to the owner's declaration — no meaningful customisation. */ + | 'verbatim' + /** + * ⭐ A row whose body has ALREADY been through the fold — what the Studio + * GET → edit → PUT round-trip persists, because the write path stores the + * request body verbatim (ADR-0005 §Validation) and the read it came from is + * folded. The row a naive fix duplicates every extender validation on. + */ + | 'prefolded'; + +interface Host { + listed: unknown; + byName: unknown; + layerCode: unknown; + layerOverlay: unknown; + layerEffective: unknown; + /** The registry's resolution OF THIS HOST'S BASE — what D9.2 defines as correct. */ + registryResolved: unknown; + /** The raw row as stored, for the `overlay` layer comparison. */ + storedRow: unknown; +} + +/** + * Boot a REST server over the REAL protocol over a REAL registry and read all + * three surfaces off it: the list route, the by-name route, and `?layers=true`. + */ +async function measure(opts: { + serviceMode?: ServiceMode; + overlay?: OverlayMode; + extended?: boolean; +} = {}): Promise { + const serviceMode = opts.serviceMode ?? 'artifact'; + const overlayMode = opts.overlay ?? 'none'; + const extended = opts.extended !== false; + const declaration = extended ? OWNER_DECLARATION : UNEXTENDED_DECLARATION; + const objectName = declaration.name; + + const registry = new SchemaRegistry(); + registry.registerObject(clone(declaration) as never, 'showcase', undefined, 'own'); + if (extended) { + registry.registerObject( + clone(EXTENSION_DECLARATION) as never, 'showcase-success', undefined, 'extend', 210, + ); + } + + // The stored row's BODY. `customised` renames the object — the edit an admin + // actually makes — and carries the owner's declaration otherwise, because a + // Studio save PUTs the whole document back. + let storedBody: Record | undefined; + if (overlayMode === 'customised') { + storedBody = { ...clone(declaration), label: 'Customer' }; + } else if (overlayMode === 'verbatim') { + storedBody = clone(declaration); + } else if (overlayMode === 'prefolded') { + storedBody = registry.foldObjectExtendersOnto(objectName, clone(declaration)) as Record; + } + + const rows = storedBody === undefined ? [] : [{ + id: 'row_1', + type: 'object', + name: objectName, + state: 'active', + organization_id: null, + package_id: null, + metadata: JSON.stringify(storedBody), + }]; + const matches = (r: Record, w: Record) => + Object.entries(w).every(([k, v]) => (r[k] ?? null) === (v ?? null)); + + const engine = { + registry, + find: async (table: string, q: { where: Record }) => + table === 'sys_metadata' ? rows.filter((r) => matches(r, q.where)) : [], + findOne: async (table: string, q: { where: Record }) => + table === 'sys_metadata' ? rows.find((r) => matches(r, q.where)) : undefined, + }; + + const services = new Map(); + if (serviceMode !== 'absent') { + // 'artifact' registers the OWNER declaration (what the compiled + // artifact's `objects` collection holds); 'bridged' registers what + // `getAllObjects()` returns, which is the ALREADY-MERGED body. + const body = serviceMode === 'artifact' ? clone(declaration) : registry.getObject(objectName); + services.set('metadata', { + get: async (type: string, name: string) => + (type === 'object' || type === 'objects') && name === objectName + ? clone(body) + : undefined, + }); + } + + const protocol = new ObjectStackProtocolImplementation( + engine as never, + () => services as Map, + ); + + const rest = new RestServer( + createMockServer() as never, + protocol as never, + { api: { requireAuth: false } } as never, + ); + (rest as unknown as { resolveExecCtx: () => Promise }).resolveExecCtx = + async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const routes = rest.getRouteManager(); + + const run = async (path: string, params: Record, query: Record) => { + const entry = routes.get('GET', path); + if (!entry) throw new Error(`route not registered: ${path}`); + let body: unknown; + const res = { + status: () => res, header: () => res, + json: (b: unknown) => { body = b; }, send: (b: unknown) => { body = b; }, + } as unknown as Parameters[1]; + await entry.handler( + { params, query, headers: {}, method: 'GET', path } as unknown as Parameters[0], + res, + ); + return body as Record | undefined; + }; + + const listBody = await run('/api/v1/meta/:type', { type: 'object' }, {}); + const listItems = (Array.isArray(listBody) + ? listBody + : ((listBody?.items ?? listBody?.data ?? []) as unknown[])) as Array<{ name?: string }>; + + const singleBody = await run('/api/v1/meta/:type/:name', { type: 'object', name: objectName }, {}); + const layeredBody = await run( + '/api/v1/meta/:type/:name', { type: 'object', name: objectName }, { layers: 'true' }, + ); + + return { + listed: listItems.find((o) => o?.name === objectName), + byName: singleBody?.item, + layerCode: layeredBody?.code, + layerOverlay: layeredBody?.overlay, + layerEffective: layeredBody?.effective, + // D9.2 applied to THIS host's base: the overlay row when one exists, + // else the owner's declaration. This is the definition the served body + // is checked against — never the other route. + registryResolved: registry.foldObjectExtendersOnto( + objectName, clone(storedBody ?? declaration), + ), + storedRow: storedBody, + }; +} + +describe('[#8027] an object overlay row is a base layer, not the resolved schema', () => { + it('serves the extension fields on BOTH routes when a customisation row exists', async () => { + const host = await measure({ overlay: 'customised' }); + + // The defect, on the two reads every writable form derives from. + for (const field of EXTENSION_FIELDS) { + expect(fieldNamesOf(host.byName)).toContain(field); + expect(fieldNamesOf(host.listed)).toContain(field); + } + + // ⭐ THE LOAD-BEARING ASSERTION: what is served is the registry's + // resolution of this host's base — D9.2 — not merely what the other + // route happens to say. #7556's `byName === listed` pin is green + // THROUGHOUT this defect, because both routes lost the fields together. + expect(declaredFieldsOf(host.byName)).toEqual(declaredFieldsOf(host.registryResolved)); + expect(declaredFieldsOf(host.listed)).toEqual(declaredFieldsOf(host.registryResolved)); + + // The customisation itself still wins where it actually speaks. A fold + // that resolved the object by discarding the overlay would satisfy every + // field assertion above and be a worse bug than the one being fixed. + expect((host.byName as { label?: string }).label).toBe('Customer'); + expect((host.listed as { label?: string }).label).toBe('Customer'); + }); + + it('resolves `layers.effective`, and leaves `layers.overlay` the tenant\'s own row', async () => { + const host = await measure({ overlay: 'customised' }); + + for (const field of EXTENSION_FIELDS) { + expect(fieldNamesOf(host.layerEffective)).toContain(field); + } + expect(declaredFieldsOf(host.layerEffective)).toEqual(declaredFieldsOf(host.registryResolved)); + + // ⛔ The boundary #7556 drew deliberately: an extension is a CODE + // declaration, not something this tenant customised, so it must not + // appear in the layer whose entire job is "what you changed". Studio's + // diff tab reads this. + for (const field of EXTENSION_FIELDS) { + expect(fieldNamesOf(host.layerOverlay)).not.toContain(field); + } + expect(host.layerOverlay).toEqual(host.storedRow); + }); + + it('stops `code` and `effective` contradicting each other in one response', async () => { + // A row that customises NOTHING — byte-identical to the owner's + // declaration. The two layers describe the same object from the same + // base, so any difference between them is the bug itself rather than a + // customisation. This is the shape the issue called out: `code` had the + // fields, `effective` did not, and the `overlay` layer explained nothing. + const host = await measure({ overlay: 'verbatim' }); + + expect(fieldNamesOf(host.layerEffective)).toEqual(fieldNamesOf(host.layerCode)); + for (const field of EXTENSION_FIELDS) { + expect(fieldNamesOf(host.layerCode)).toContain(field); + expect(fieldNamesOf(host.layerEffective)).toContain(field); + } + }); + + // ── ⭐ §2: the fold is applied to bases that have already been folded ── + + it('does not duplicate validations or indexes on an ALREADY-FOLDED overlay row', async () => { + const host = await measure({ overlay: 'prefolded' }); + + // The row itself genuinely carries the extender's entries — without + // this the case proves nothing (see the anti-vacuity test). + expect(entryNames(host.storedRow, 'validations')).toEqual(['owner_rule', 'ext_rule']); + + // Folding it again must not append a second copy. `fields` would look + // perfect either way — the spread is idempotent — which is exactly how + // this class of bug survives a field-name pin. + expect(entryNames(host.byName, 'validations')).toEqual(['owner_rule', 'ext_rule']); + expect(entryNames(host.byName, 'indexes')).toEqual(['owner_idx', 'ext_idx']); + expect(entryNames(host.listed, 'validations')).toEqual(['owner_rule', 'ext_rule']); + expect(entryNames(host.listed, 'indexes')).toEqual(['owner_idx', 'ext_idx']); + expect(entryNames(host.layerEffective, 'validations')).toEqual(['owner_rule', 'ext_rule']); + expect(entryNames(host.layerEffective, 'indexes')).toEqual(['owner_idx', 'ext_idx']); + }); + + it('does not duplicate them on a BRIDGED host either — the in-process dev boot', async () => { + // ObjectQL's `bridgeObjectsToMetadataService` seeds the `metadata` + // service from `registry.getAllObjects()` — bodies that are already + // resolved — so #7556's fold ran on a folded base here and served every + // extender validation and index TWICE. That regression is live on the + // commit this branch starts from; it survived #7556's own pin because + // that pin compares field names. + const host = await measure({ serviceMode: 'bridged', overlay: 'none' }); + + expect(entryNames(host.byName, 'validations')).toEqual(['owner_rule', 'ext_rule']); + expect(entryNames(host.byName, 'indexes')).toEqual(['owner_idx', 'ext_idx']); + expect(entryNames(host.layerCode, 'validations')).toEqual(['owner_rule', 'ext_rule']); + expect(entryNames(host.layerCode, 'indexes')).toEqual(['owner_idx', 'ext_idx']); + expect(entryNames(host.layerEffective, 'validations')).toEqual(['owner_rule', 'ext_rule']); + }); + + // ── the two populations that must not move at all ── + + it('leaves an object with NO overlay row exactly as it was', async () => { + const host = await measure({ overlay: 'none' }); + + // Identical to the registry's resolution, with no entry appearing twice. + expect(declaredFieldsOf(host.byName)).toEqual(declaredFieldsOf(host.registryResolved)); + expect(declaredFieldsOf(host.listed)).toEqual(declaredFieldsOf(host.registryResolved)); + expect(entryNames(host.byName, 'validations')).toEqual(['owner_rule', 'ext_rule']); + expect(entryNames(host.byName, 'indexes')).toEqual(['owner_idx', 'ext_idx']); + // #7556's own pin, restated: this fix must not cost that agreement. + expect(fieldNamesOf(host.byName)).toEqual(fieldNamesOf(host.listed)); + expect(host.layerOverlay).toBeNull(); + }); + + it('leaves an object with NO extension contributor exactly as it was, row or no row', async () => { + const [withRow, withoutRow] = await Promise.all([ + measure({ extended: false, overlay: 'customised' }), + measure({ extended: false, overlay: 'none' }), + ]); + + // Nothing grafted on: a fold that ran unconditionally, or that failed to + // filter its contributor list, would be invisible to every case above + // and would change every object's payload to correct three fields. + for (const host of [withRow, withoutRow]) { + expect(declaredFieldsOf(host.byName)).toEqual(declaredFieldsOf(host.registryResolved)); + expect(fieldNamesOf(host.byName)).toContain('title'); + for (const field of EXTENSION_FIELDS) { + expect(fieldNamesOf(host.byName)).not.toContain(field); + } + expect(entryNames(host.byName, 'validations')).toEqual(['task_rule']); + expect(entryNames(host.byName, 'indexes')).toEqual(['task_idx']); + } + // The row still customises what it customises. + expect((withRow.byName as { label?: string }).label).toBe('Customer'); + expect((withoutRow.byName as { label?: string }).label).toBe('Task'); + }); + + // ── anti-vacuity ── + + it('anti-vacuity: the fixtures genuinely differ, so none of the above holds by emptiness', async () => { + const [artifact, bridged, prefolded, unextended, none] = await Promise.all([ + measure({ overlay: 'customised' }), + measure({ serviceMode: 'bridged', overlay: 'none' }), + measure({ overlay: 'prefolded' }), + measure({ extended: false, overlay: 'customised' }), + measure({ overlay: 'none' }), + ]); + + // 1. The overlay row is REAL and is genuinely a bare base layer: it + // carries none of the extension fields. If the fixture ever started + // seeding the row from the merged registry, every case above would + // still pass while proving nothing — so that is pinned here. + for (const field of EXTENSION_FIELDS) { + expect(fieldNamesOf(artifact.storedRow)).not.toContain(field); + } + expect(fieldNamesOf(artifact.storedRow)).toEqual(['industry', 'name']); + + // 2. …and the `prefolded` row is NOT that row — it really has been + // through the fold. Without this the idempotency case would be a + // second copy of the ordinary one. + for (const field of EXTENSION_FIELDS) { + expect(fieldNamesOf(prefolded.storedRow)).toContain(field); + } + expect(entryNames(prefolded.storedRow, 'validations')) + .not.toEqual(entryNames(artifact.storedRow, 'validations')); + + // 3. The `bridged` host's service body really is the merged one, which + // is what made it fold a folded base. Held against the registry so + // the two cannot drift into being the same host. + expect(entryNames(bridged.layerCode, 'validations')).toContain('ext_rule'); + + // 4. The extended and unextended objects are genuinely different + // shapes, so "equals the registry's resolution" is a statement about + // a non-constant set. + expect(fieldNamesOf(none.byName)).not.toEqual(fieldNamesOf(unextended.byName)); + expect(fieldNamesOf(unextended.byName)).not.toContain('loyalty_tier'); + + // 5. The customised row really does change something, so "the overlay + // still wins" is not vacuous either. + expect((artifact.byName as { label?: string }).label).toBe('Customer'); + expect((none.byName as { label?: string }).label).toBe('Account'); + }); +});