Skip to content

Commit 01a7337

Browse files
os-zhuangclaude
andauthored
fix(objectql): a tenant's rename beats a package extension's scalar in the object fold (#8460) (#8620)
* fix(objectql): a tenant's rename beats a package extension's scalar in the object fold (#8460) ADR-0029 D9.2 makes the tenant's overlay the BASE of the object fold, and mergeObjectDefinitions applied an extender's label / pluralLabel / description LAST onto whatever base it was handed. So a rename saved through Studio answered 200, showed up under ?layers=true, and was overwritten inside the fold before any read served it. Per the 2026-08-13 maintainer ruling (option A), an extender's scalar now applies only while the fold's base still carries the packaged owner's value; a diverged base was authored by the tenant and the extender yields. This is deliberately the SAME comparison-based mechanism #8284 established one layer up for the i18n catalog — scalarOverridesPackagedBase is exported from @objectstack/spec and imported by the registry rather than re-spelled, so one sentence governs both layers: an explicit override beats a packaged default. No provenance flags, no migration, no escape hatch. The set is computed once over the base the fold starts from, never re-derived from the running merge, so extender-vs-extender precedence (D9.3) is untouched. Idempotence (#8027) is preserved by construction. Amends ADR-0029 D9.2 (new D9.2a). Flips #8284's staged acceptance pin in showcase-object-extension-scalar-divergence.dogfood.test.ts to green, unmodified in its assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy * chore(spec): record the newly exported predicate in the api-surface snapshot (#8460) The `TypeScript Type Check` job's final step is `check:api-surface`, which failed on the export #8460 adds: "0 breaking (removed/narrowed), 1 added". Purely additive — `scalarOverridesPackagedBase` is exported so the object fold can reuse the #8284 predicate instead of re-spelling it, per the ruling. Sibling of the export-origins regeneration already in this branch; the two snapshots are separate gates over the same export. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy * ci: retrigger after an unrelated service-job flake (#8460) `Test Core (2/3)` reddened at a1a1fc8 on `db-job-adapter.timeout.test.ts` — a wall-clock flake filed as #8628: the case registers a real every-minute cron and asserts exactly one run row, so it fails when the shard straddles a minute boundary (CI: file started 03:02:55, assertion failed 03:03:00.17). Not reachable from this diff — `@objectstack/service-job` does not depend on `@objectstack/objectql`. Verified green locally 5/5 in isolation and 71/71 for the package, plus a full sweep of the fold's dependent class. No re-run permission from this seat, so this empty commit retriggers CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 98eec7e commit 01a7337

7 files changed

Lines changed: 261 additions & 30 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
'@objectstack/objectql': patch
3+
'@objectstack/spec': patch
4+
---
5+
6+
A tenant's own object rename now wins over a package extension's scalar in the object fold (#8460).
7+
8+
`mergeObjectDefinitions` applied an `objectExtensions` entry's `label` /
9+
`pluralLabel` / `description` last, onto whatever base it was handed — and
10+
ADR-0029 D9.2 makes the tenant's overlay that base. So a rename saved through
11+
Studio answered `200`, was visible under `?layers=true`, and was overwritten
12+
inside the fold before any read served it.
13+
14+
Per the 2026-08-13 maintainer ruling (ADR-0029 D9.2a), an extender's scalar now
15+
applies only while the fold's base still carries the packaged owner's value; a
16+
base that has diverged was authored by the tenant, and the extender yields. This
17+
is the same comparison-based mechanism #8284 established for the i18n catalog
18+
one layer up — the same predicate, now exported from `@objectstack/spec` and
19+
imported by the registry rather than re-spelled — so one rule covers both
20+
layers: an explicit override beats a packaged default.
21+
22+
No provenance flags and no migration: the question is answered from two values
23+
at fold time. The accepted cost is deliberate — a package can no longer relabel
24+
an object a tenant has deliberately renamed.

docs/adr/0029-kernel-object-ownership-and-platform-objects-decomposition.md

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,68 @@ the one the ruling asked for:
472472
nobody asked for, arriving as a silent re-appearance of deleted fields.
473473
- **A second `own` is refused** — see § 4.
474474

475+
##### D9.2a — AMENDMENT (2026-08-13, #8460): an extender's SCALAR yields to a diverged base
476+
477+
D9.2 above says the fold runs "exactly as it does today", and for `fields`,
478+
`validations` and `indexes` it still does. For the three **scalars**`label`,
479+
`pluralLabel`, `description` — it no longer does, and this clause is the
480+
difference.
481+
482+
**Ruling (maintainer, 2026-08-13, option A — "tenant wins"):** an extender's
483+
scalar applies only while the fold's base still carries the **packaged owner's**
484+
value. A base whose scalar has diverged from the owner's has been authored by
485+
the tenant, and the extender **yields**.
486+
487+
Why this had to be decided rather than left to D9.2's "last writer wins": D9.2
488+
makes the tenant's overlay the *base* of the fold, so last-writer-wins meant a
489+
code package's `objectExtensions` scalar overwrote the tenant's own Studio
490+
rename *inside* the fold. The tenant's value was then absent from the document
491+
every read serves — `PUT /meta/object/:name` answered `200`, `?layers=true`
492+
showed the saved value under `overlay`, and no read a writable form derives from
493+
ever showed it (#8037, #8027/#8045, and the severe half of #8284).
494+
495+
The mechanism is **comparison-based provenance**, and is deliberately the *same*
496+
mechanism [#8284](https://github.com/objectstack-ai/objectstack/issues/8284)
497+
established one layer up for the i18n catalog — the same predicate, imported by
498+
`SchemaRegistry` from `@objectstack/spec`, not a second copy free to drift. One
499+
sentence now governs both layers: **an explicit override beats a packaged
500+
default.**
501+
502+
Binding consequences:
503+
504+
- **No provenance flag and no migration.** Nothing is stamped on the document;
505+
the question is answered from two values at fold time. A flag threaded through
506+
the fold was explicitly rejected.
507+
- **The comparison is against the packaged owner ALONE**, never against the
508+
owner with extenders already folded on (D9.6's `resolveOwnerLayer`) — that
509+
body reports every extender's scalar as "unchanged" and would yield nothing,
510+
ever.
511+
- **Computed once, over the base the fold starts from**, never re-derived from
512+
the running merge. Re-deriving would make one extender's scalar look
513+
"authored" to the next and silently invert extender-vs-extender precedence,
514+
which D9.3 reserves to declared priority.
515+
- **Conservative edges** (inherited from the shared predicate): an absent base,
516+
a non-string or empty value, and inexact equality all mean "no opinion", so
517+
this can only ever *withhold* an extender's scalar from a value that provably
518+
diverged. A tenant who renames an object to exactly the packaged string is a
519+
no-op, by construction.
520+
- **Idempotence (#8027) is preserved.** A base that already carries an
521+
extender's scalar reads as diverged, so the extenders yield and the value
522+
stays what the first fold produced — the same answer, reached by yielding
523+
instead of by re-applying.
524+
- **The accepted cost is the point, not a regression:** a package can no longer
525+
relabel an object a tenant has deliberately renamed. There is **no escape
526+
hatch**, by ruling. Note the honest edge this implies: because the write path
527+
persists the served body verbatim (ADR-0005 §Validation), a tenant who
528+
round-trips an object *without* renaming it freezes the extender's current
529+
scalar into the overlay row, and a later change to the package's extension
530+
scalar will not reach that tenant. That follows from comparison-based
531+
provenance with no flags, which is what the ruling required; it is recorded
532+
here rather than papered over.
533+
- Options B (status quo — the extension keeps winning) and C (refuse the write)
534+
were considered and **rejected**. Dropping scalars from the fold entirely
535+
(#8284's arm B) remains rejected and is not this clause.
536+
475537
#### D9.3 — selection is by KIND; priority stays descriptive
476538

477539
`contributors.sort((a, b) => a.priority - b.priority)` (`:1189`) totals the whole

packages/objectql/src/registry.ts

Lines changed: 112 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ import {
1717
ITEM_KEY_DISCRIMINATORS,
1818
readDiscriminatorValue as discriminatorValue,
1919
} from '@objectstack/metadata-core';
20-
import { SystemFieldName } from '@objectstack/spec/system';
20+
// [#8460] `scalarOverridesPackagedBase` is the #8284 comparison, imported rather
21+
// than re-spelled: the object FOLD asks the same question one layer down (has
22+
// this scalar been authored away from the packaged default?), and the ruling
23+
// required the same mechanism, not a second comparison shape.
24+
import { SystemFieldName, scalarOverridesPackagedBase } from '@objectstack/spec/system';
2125
import { resolveTenancyPosture, resolveSearchPinyinEnabled } from '@objectstack/types';
2226
import { postureEnforcesWall } from '@objectstack/spec/security';
2327
import { provisionSearchCompanion, SEARCH_COMPANION_FIELD } from './search-companion.js';
@@ -111,11 +115,30 @@ export function parseFQN(fqn: string): { namespace: string | undefined; shortNam
111115
};
112116
}
113117

118+
/**
119+
* The three SCALAR props {@link mergeObjectDefinitions} resolves last-writer-wins
120+
* — the exact set the #8284 and #8460 rulings both cover, and the same three
121+
* {@link scalarOverridesPackagedBase} answers for.
122+
*/
123+
const OBJECT_FOLD_SCALAR_KEYS = ['label', 'pluralLabel', 'description'] as const;
124+
125+
type ObjectFoldScalarKey = (typeof OBJECT_FOLD_SCALAR_KEYS)[number];
126+
114127
/**
115128
* Deep merge two ServiceObject definitions.
116129
* Fields are merged additively. Other props: later value wins.
130+
*
131+
* [#8460] …except that "later value wins" is now conditional for the three
132+
* SCALARS. `tenantAuthored` names the scalars the fold's BASE has authored away
133+
* from the packaged owner's value; an extender yields on those. See
134+
* {@link SchemaRegistry.tenantAuthoredScalars} for why the set is computed once
135+
* over the base rather than re-derived from the running `merged`.
117136
*/
118-
function mergeObjectDefinitions(base: ServiceObject, extension: Partial<ServiceObject>): ServiceObject {
137+
function mergeObjectDefinitions(
138+
base: ServiceObject,
139+
extension: Partial<ServiceObject>,
140+
tenantAuthored?: ReadonlySet<ObjectFoldScalarKey>,
141+
): ServiceObject {
119142
const merged = { ...base };
120143

121144
// Merge fields additively
@@ -133,10 +156,16 @@ function mergeObjectDefinitions(base: ServiceObject, extension: Partial<ServiceO
133156
merged.indexes = [...(base.indexes || []), ...extension.indexes];
134157
}
135158

136-
// Override scalar props (last writer wins)
137-
if (extension.label !== undefined) merged.label = extension.label;
138-
if (extension.pluralLabel !== undefined) merged.pluralLabel = extension.pluralLabel;
139-
if (extension.description !== undefined) merged.description = extension.description;
159+
// Override scalar props (last writer wins) — [#8460] unless the base has been
160+
// authored by the tenant, in which case the extender's packaged default yields.
161+
const yields = (key: ObjectFoldScalarKey): boolean => tenantAuthored?.has(key) === true;
162+
if (extension.label !== undefined && !yields('label')) merged.label = extension.label;
163+
if (extension.pluralLabel !== undefined && !yields('pluralLabel')) {
164+
merged.pluralLabel = extension.pluralLabel;
165+
}
166+
if (extension.description !== undefined && !yields('description')) {
167+
merged.description = extension.description;
168+
}
140169

141170
return merged;
142171
}
@@ -1614,15 +1643,91 @@ export class SchemaRegistry {
16141643
contributors: ObjectContributor[],
16151644
baseDefinition: ServiceObject,
16161645
): ServiceObject {
1646+
// [#8460] Computed ONCE, over the base the fold starts from — never
1647+
// re-derived from the running `merged`, which would make an extender's own
1648+
// scalar look "authored" to the next extender and silently invert
1649+
// extender-vs-extender precedence (D9.3: declared numbers order peers).
1650+
const tenantAuthored = this.tenantAuthoredScalars(contributors, baseDefinition);
16171651
let merged = { ...baseDefinition };
16181652
for (const contrib of contributors) {
16191653
if (contrib.ownership === 'extend') {
1620-
merged = mergeObjectDefinitions(merged, contrib.definition);
1654+
merged = mergeObjectDefinitions(merged, contrib.definition, tenantAuthored);
16211655
}
16221656
}
16231657
return merged;
16241658
}
16251659

1660+
/**
1661+
* [#8460] Which of the three fold scalars the BASE layer carries a
1662+
* TENANT-AUTHORED value for — i.e. one that no longer equals the packaged
1663+
* owner's.
1664+
*
1665+
* Maintainer ruling, 2026-08-13 (option A, "tenant wins"): an extender's
1666+
* scalar applies only while the fold's base still carries the packaged
1667+
* owner's value; a diverged base has been authored by the tenant and the
1668+
* extender yields. This **amends ADR-0029 D9.2**, whose fold was
1669+
* unconditionally last-writer-wins on the scalars, and it is deliberately the
1670+
* SAME comparison-based mechanism the #8284 fix applies one layer up — the
1671+
* predicate is literally {@link scalarOverridesPackagedBase}, imported from
1672+
* `@objectstack/spec`, not a second copy of the shape. One sentence at both
1673+
* layers: *an explicit override beats a packaged default.*
1674+
*
1675+
* ⛔ No provenance flag is stamped on the document and no migration is
1676+
* implied: the question is answered from two values, at fold time.
1677+
*
1678+
* The accepted cost is the point, not a regression — a package can no longer
1679+
* relabel an object a tenant has deliberately renamed. There is no escape
1680+
* hatch for it, by ruling.
1681+
*
1682+
* Conservative in the same three ways the predicate is (absent base, non-string
1683+
* or empty value, exact equality), plus two of this layer's own:
1684+
*
1685+
* - **no `own` contributor → no opinion.** An orphan overlay (D9.5) keeps
1686+
* today's fold exactly.
1687+
* - **base IS the packaged owner → no opinion, by reference.** That is
1688+
* {@link resolveObject} with no overlay registered and every
1689+
* {@link resolveOwnerLayer} call (D9.6), so the common shape pays one
1690+
* identity comparison and cannot change answer. It is an OPTIMISATION, not
1691+
* a correctness edge: comparing that base against itself answers "not
1692+
* diverged" for every key anyway. ⛔ So it must never be relaxed into
1693+
* something weaker than identity — a value-equality test here would start
1694+
* answering for bodies that merely LOOK like the owner.
1695+
*
1696+
* ⛔ The owner is read as the `own` CONTRIBUTOR, deliberately NOT through
1697+
* {@link getPackagedObjectOwner} — whose extra `isCodeArtifactBody` test
1698+
* (D9.8) would make this decline to protect a RUNTIME-authored object, i.e.
1699+
* exactly the object whose owner row the tenant wrote by hand. The two agree
1700+
* wherever a packaged owner exists, which is every shape #8460 measured; they
1701+
* differ only on a tenant-authored owner, and there the ruled sentence still
1702+
* reads the same way — the tenant's own row is the explicit override and a
1703+
* package's `objectExtensions` entry is the packaged default. The rejected
1704+
* alternative is the trap PR #8454 named one layer up, in its own form:
1705+
* comparing against a body that already has extenders folded onto it
1706+
* ({@link resolveOwnerLayer}) would report every extender's scalar as
1707+
* "unchanged" and yield nothing, ever.
1708+
*
1709+
* IDEMPOTENCE is preserved, which matters because
1710+
* {@link foldObjectExtendersOnto} is documented as idempotent (#8027) and two
1711+
* shipped call sites really do hand it an already-folded base. A base that
1712+
* already carries an extender's scalar reads as "diverged", so every extender
1713+
* yields and the value stays exactly what the first fold produced — the same
1714+
* answer, reached by yielding instead of by re-applying.
1715+
*/
1716+
private tenantAuthoredScalars(
1717+
contributors: ObjectContributor[],
1718+
baseDefinition: ServiceObject,
1719+
): ReadonlySet<ObjectFoldScalarKey> | undefined {
1720+
const owner = contributors.find((c) => c.ownership === 'own')?.definition;
1721+
if (!owner || owner === baseDefinition) return undefined;
1722+
let authored: Set<ObjectFoldScalarKey> | undefined;
1723+
for (const key of OBJECT_FOLD_SCALAR_KEYS) {
1724+
if (scalarOverridesPackagedBase(owner, key, baseDefinition[key])) {
1725+
(authored ??= new Set()).add(key);
1726+
}
1727+
}
1728+
return authored;
1729+
}
1730+
16261731
/**
16271732
* [#7556] Fold this object's `extend` contributors onto a base body the
16281733
* CALLER supplies — the same fold {@link resolveObject} (D9.2) and

0 commit comments

Comments
 (0)