fix(objectql): correct mergeObjectDefinitions docblock to the real, closed merge set (#12680)

.changeset/registry-merge-object-definitions-docblock-closed-set.md

@@ -0,0 +1,51 @@
+---
+"@objectstack/objectql": patch
+---
+
+fix(objectql): correct `mergeObjectDefinitions`'s docblock to the real, closed merge set (#12680)
+
+The docblock on `mergeObjectDefinitions` (`packages/objectql/src/registry.ts`)
+said:
+
+> Fields are merged additively. **Other props: later value wins.**
+
+The implementation has never done the second half. It merges exactly:
+`fields` (additively), `validations` (additively), `indexes` (additively),
+and the three guarded scalars `label` / `pluralLabel` / `description`
+(last-writer-wins, subject to the `tenantAuthored` yield rule). **Every
+other top-level prop an `extend` contributor carries is silently
+discarded** — `merged` starts as `{ ...base }` and nothing outside that list
+is ever copied onto it.
+
+This is a **docs-only correction plus a regression pin — zero runtime
+behaviour changed.** `mergeObjectDefinitions` is not exported; nothing about
+what it does was touched, only what the comment above it claims. Shipped as
+`patch` rather than omitted because the file's behaviour is now backed by an
+enforced pin (see below) where before it was backed by nothing but an
+inaccurate comment — that is a real (if internal-only) improvement to the
+package worth a version bump, and this repo's convention reserves "no
+changeset" for changes with no user-facing effect of any kind, not for
+"no runtime diff." There is no public API surface to widen or narrow (the
+function is module-private), so there is nothing here for `check:*` gates
+that watch exported shapes to see.
+
+Why this matters: an author reading the old docblock and shipping an
+`objectExtensions` entry carrying, say, `tenancy: { enabled: false }` would
+have gotten a **silent no-op** on a security-relevant key — no error, no
+warning, the base's existing value simply wins as if the extension had never
+named the key. That near-miss is the reason this card exists (found while
+resolving cloud#1653's investigation into exactly that override path). The
+corrected docblock says the discard out loud; a new pin
+(`registry-object-extension-nonenumerated-prop-discard.test.ts`) hands
+`mergeObjectDefinitions` (via the public `SchemaRegistry` API) an `extend`
+contributor carrying a non-enumerated top-level prop and asserts the merged
+result does not carry it, with a guarded scalar as a positive control in the
+same fold. `icon` is the pin's fixture — a real, spec-legal, security-neutral
+top-level prop — deliberately **not** `tenancy`, so the pin does not read as
+license to special-case that key elsewhere.
+
+⛔ Out of scope, explicitly: implementing "later value wins" for the
+undocumented remainder (making `tenancy` / `permissions` extender-writable is
+a separate, much larger decision triage fenced off this card) and adding a
+runtime warning on the silent drop (a real question, filed separately rather
+than folded in here).

packages/objectql/src/registry-object-extension-nonenumerated-prop-discard.test.ts

@@ -0,0 +1,98 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#12680] `mergeObjectDefinitions`'s docblock used to promise "other props:
+ * later value wins" while the implementation only ever merged `fields` /
+ * `validations` / `indexes` (additively) and the three guarded scalars
+ * `label` / `pluralLabel` / `description` (last-writer-wins). Every other
+ * top-level prop an `extend` contributor carries was — and still is —
+ * silently discarded: `merged = { ...base }` and nothing else is copied.
+ *
+ * That mismatch is what let cloud#1653's investigation nearly conclude that a
+ * host could override a framework object's `tenancy` declaration through the
+ * extension seam — the docblock said so. The docblock was corrected (this
+ * PR); THIS FILE pins the closed merge set as a fact of the implementation so
+ * prose and behaviour cannot silently drift apart again.
+ *
+ * `icon` is the fixture: a real, spec-legal top-level `ServiceObject` prop
+ * with no security weight (`tenancy` — the motivating example — is
+ * deliberately NOT baked into a test, to keep this pin from reading as
+ * license to write a `tenancy`-specific test elsewhere).
+ *
+ * `label` is the positive control: it IS in the guarded-scalar set, so it
+ * must change under the very same extend-fold. Without it, a fold that did
+ * nothing at all would pass the negative assertion for the wrong reason.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { SchemaRegistry } from './registry.js';
+
+const OWNER_PKG = 'app.owner';
+const EXTENDER_PKG = 'app.extender';
+const OBJECT_NAME = 'merge_fold_nonenumerated_prop_probe';
+
+function silentRegistry(): SchemaRegistry {
+ const r = new SchemaRegistry({ multiTenant: false });
+ r.logLevel = 'silent';
+ return r;
+}
+
+describe('mergeObjectDefinitions — closed merge set (#12680)', () => {
+ it('discards a non-enumerated top-level prop (`icon`) from an extend contributor silently', () => {
+ const r = silentRegistry();
+ r.registerObject(
+ {
+ name: OBJECT_NAME,
+ label: 'Base Label',
+ icon: 'base-icon',
+ fields: { name: { name: 'name', type: 'text', label: 'Name' } },
+ } as any,
+ OWNER_PKG,
+ );
+ r.registerObject(
+ {
+ name: OBJECT_NAME,
+ // Enumerated scalar — must win (positive control).
+ label: 'Extender Label',
+ // NON-enumerated top-level prop — not in the closed merge set, must
+ // NOT survive the fold, and no error/warning marks the drop.
+ icon: 'extender-icon',
+ } as any,
+ EXTENDER_PKG,
+ undefined,
+ 'extend',
+ );
+
+ const resolved = r.getObject(OBJECT_NAME) as any;
+
+ // Positive control: the guarded scalar DID fold — proves the extend
+ // contributor was actually applied, not skipped for some unrelated reason.
+ expect(resolved.label).toBe('Extender Label');
+
+ // The pin: the non-enumerated prop was silently discarded. The base's
+ // value survives untouched — the extension's value never lands.
+ expect(resolved.icon).toBe('base-icon');
+ });
+
+ it('discards a non-enumerated prop the BASE never declared, rather than materializing it from the extension', () => {
+ const r = silentRegistry();
+ r.registerObject(
+ {
+ name: OBJECT_NAME,
+ label: 'Base Label',
+ fields: { name: { name: 'name', type: 'text', label: 'Name' } },
+ // no `icon` at all on the base
+ } as any,
+ OWNER_PKG,
+ );
+ r.registerObject(
+ { name: OBJECT_NAME, icon: 'extender-icon' } as any,
+ EXTENDER_PKG,
+ undefined,
+ 'extend',
+ );
+
+ const resolved = r.getObject(OBJECT_NAME) as any;
+ expect(resolved.icon).toBeUndefined();
+ });
+});

packages/objectql/src/registry.ts

@@ -126,11 +126,49 @@ type ObjectFoldScalarKey = (typeof OBJECT_FOLD_SCALAR_KEYS)[number];
 
 /**
  * Deep merge two ServiceObject definitions.
- * Fields are merged additively. Other props: later value wins.
  *
- * [#8460] …except that "later value wins" is now conditional for the three
- * SCALARS. `tenantAuthored` names the scalars the fold's BASE has authored away
- * from the packaged owner's value; an extender yields on those. See
+ * The merge set is CLOSED and enumerable — this is NOT "every prop, later
+ * value wins":
+ * - `fields`, `validations`, `indexes` are merged ADDITIVELY.
+ * - The three {@link OBJECT_FOLD_SCALAR_KEYS} (`label`, `pluralLabel`,
+ * `description`) are overridden last-writer-wins, subject to the
+ * `tenantAuthored` yield rule below.
+ *
+ * **Every other top-level prop on `extension` is silently discarded.**
+ * `merged` starts as `{ ...base }` and nothing outside the list above is ever
+ * copied onto it — a caller that hands this function an extension body
+ * carrying an undeclared key, e.g. `tenancy: { enabled: false }`, gets a
+ * valid-but-inert no-op HERE: no error, no warning from this function, the
+ * base's existing value simply wins as if the extension had never named the
+ * key (issue #12680).
+ *
+ * Declarative authors never reach that silence, though: `ObjectExtensionSchema`
+ * (`packages/spec/src/data/object.zod.ts`) is `.strict()` — its shape is
+ * exactly `extend` / `fields` / `label` / `pluralLabel` / `description` /
+ * `validations` / `indexes` / `priority`, so an `objectExtensions` entry
+ * naming `tenancy`, `permissions`, or any other key outside that list fails
+ * LOUDLY at authoring time with a prescription (#4001; the same rule is
+ * documented at `content/docs/data-modeling/object-extensions.mdx`). This
+ * function's discard is reachable only by a caller that bypasses that
+ * schema — a direct, programmatic `registry.registerObject(def, pkg,
+ * undefined, 'extend', …)` call, which is how this package's own pin
+ * (`registry-object-extension-nonenumerated-prop-discard.test.ts`) exercises
+ * the rule. Extender-writable `tenancy`/`permissions` would in any case be a
+ * separate, much bigger decision that has NOT been made.
+ *
+ * Unlike `tenancy`/`permissions`, this one is not blocked by schema:
+ * `_provenance` is a real top-level prop an `extend` contributor carries
+ * TODAY. Copying it through (as "later value
+ * wins" would) lets a third-party extender flip a tenant-authored object's
+ * `_provenance` to the extending package's — reaching, by a different route,
+ * the exact outcome ADR-0029 D9.3's priority-reranking guard exists to
+ * prevent. `registry-object-overlay-layer.test.ts` ("an extender declaring
+ * priority 140 does not become the base layer") fails if this merge set is
+ * widened to copy it through.
+ *
+ * [#8460] …the SCALAR override above is conditional. `tenantAuthored` names
+ * the scalars the fold's BASE has authored away from the packaged owner's
+ * value; an extender yields on those. See
  * {@link SchemaRegistry.tenantAuthoredScalars} for why the set is computed once
  * over the base rather than re-derived from the running `merged`.
  */