Skip to content

Commit bcea363

Browse files
huangyiireneclaude
huangyiirene
and
claude
authored
fix(metadata-protocol): let an org-scoped caller see env-wide sys_metadata rows (#7819 tier 2) (#7871)
`duplicatePackage` and `reassignOrphanedMetadata` scanned `sys_metadata` with a strict `organization_id` equality, which matches no row whose column is NULL, so an org-scoped caller could not see any row recorded env-wide. Both scans now accept org-scoped or env-wide rows — the same `$or` the rest of this family carries. These two sites were filed UNVERIFIED, so step one was a reachability measurement on a real engine, not an edit. Both halves came back live: each has one production caller forwarding `resolveActiveOrganizationId`, and env-wide rows in that table are ordinary — a `saveMetaItem` naming no package at all still succeeds today and lands `package_id = null, organization_id = null`, which is exactly the orphan the scan could not see. Both symptoms were worse than projected: - `duplicatePackage` answered `{success: true, copiedCount: 1, failedCount: 0}` over a mixed-scope source — and because the rename map is built only from the rows the scan returns, a copied view kept pointing at the SOURCE package's object: a duplicate silently wired back to the base it was cloned from. - Widening the scan alone was not a fix. `object` is `allowOrgOverride=false`, so stamping the request's org onto a copy is refused with `NOT_OVERRIDABLE`. An object therefore cannot exist org-scoped, so every object row is env-wide and an org-scoped `duplicatePackage` could never copy one — ADR-0070 D4's "duplicate base" could not duplicate what a base is mostly made of whenever an org was active. Each copy now lands in the scope of the row it came from, the same rule #7559 gave `revertCommit`. - `reassignOrphanedMetadata` left env-wide orphans at `package_id = null` and reported success. Finding orphans is its entire purpose, and ADR-0070 D5 makes the unit explicitly the ENVIRONMENT, so the strict equality made it inert for an org-scoped caller in the deployment it was designed for. The no-org branch is deliberately NOT narrowed on either site, and both doors are pinned as they stand. Whether the orphan door should be as wide as it is remains #7780's open product question, which is not decided here. Pinned on a real ObjectQL over a real SqlDriver in `packages/runtime` (12 cases). Reverse verification predicted 5 red and measured 5 failed | 7 passed. Fixes #7819 Claude-Session: https://claude.ai/code/session_01DPXtEEaejmVQDY7GPUWv4z Co-authored-by: Claude <noreply@anthropic.com>
1 parent b313fde commit bcea363

3 files changed

Lines changed: 740 additions & 4 deletions

File tree

‎.changeset/duplicate-adopt-org-scope.md‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(metadata-protocol): let an org-scoped caller see env-wide `sys_metadata` rows in `duplicatePackage` / `reassignOrphanedMetadata` (#7819, tier 2)
6+
7+
Both methods scanned `sys_metadata` with a strict `organization_id` equality:
8+
9+
```ts
10+
if (request.organizationId) where.organization_id = request.organizationId;
11+
```
12+
13+
`organization_id = 'org'` matches no row whose column is NULL, so an org-scoped
14+
caller could not see any row recorded env-wide. Both scans now accept org-scoped
15+
**or** env-wide rows — the same `$or` `deletePackage` (#7705), `listCommits`
16+
(#7779) and the tier-1 sites (#7857) already carry.
17+
18+
## These two were filed UNVERIFIED, so step one was a measurement
19+
20+
#7819 carried four sites. Tier 1 shipped on measured evidence; these two were a
21+
grep match with a plausible mechanism, on a **different table** (`sys_metadata`,
22+
not `sys_metadata_commit`) with callers nobody had driven. "Latent, not live"
23+
would have been a complete outcome and no fix. Reachability was checked on a
24+
real engine before a line was edited, and both halves came back live:
25+
26+
1. **A caller passes an org.** One production caller each, both in
27+
`packages/runtime/src/domains/packages.ts`: `POST /packages/:id/duplicate`
28+
and `POST /packages/:id/adopt-orphans`, each forwarding
29+
`resolveActiveOrganizationId` — the same door tier 1 measured.
30+
2. **Env-wide rows exist in that table.** Not incidentally: a `saveMetaItem`
31+
from a session with no active org writes `organization_id = NULL`, and
32+
`resolveActiveOrganizationId` answers `undefined` both for such a session and
33+
for any throw on the auth seam. For the orphan site, a `saveMetaItem` naming
34+
**no package at all still succeeds today** and lands `package_id = null,
35+
organization_id = null` — the current write path mints exactly the orphan the
36+
scan could not see, so that population is live rather than the legacy residue
37+
the docstring can be read as describing.
38+
39+
Both projected symptoms then reproduced, and both were worse than projected.
40+
41+
## `duplicatePackage` — a partial copy reporting success, and a copy wired back to its source
42+
43+
Measured before the fix: a source package holding one env-wide row and one
44+
org-scoped row, duplicated by an org caller, answered
45+
`{success: true, copiedCount: 1, failedCount: 0}`.
46+
47+
The sharper consequence is the **rename map**, which is built only from the rows
48+
the scan returns. With the env-wide `object` rows missing it came out empty, so a
49+
copied view was renamed `iojn2_list` while its `data.object` still read
50+
`iojn_widget` — a duplicate silently wired back to the base it was cloned from,
51+
reporting success. An all-env-wide source degraded just as quietly the other way:
52+
`{success: false, copiedCount: 0, failedCount: 0}`, nothing copied and nothing
53+
named as failed.
54+
55+
### Widening the scan alone was **not** a fix
56+
57+
With the scan widened and the write left as it was, the object copy landed in
58+
`failed[]` with `NOT_OVERRIDABLE`: `object` is declared `allowOrgOverride=false`,
59+
so stamping the request's org onto the copy is refused — boot hydration loads
60+
env-wide rows only, and an org-scoped `object` row would vanish on the next
61+
restart (ADR-0005, #6190).
62+
63+
Since an `object` therefore **cannot exist org-scoped**, every object row in a
64+
source package is env-wide, and an org-scoped `duplicatePackage` could never copy
65+
a single one. Objects being what a base is mostly made of, ADR-0070 D4's
66+
"duplicate base" gesture was structurally unable to duplicate a base whenever an
67+
org was active — a larger defect than the card projected.
68+
69+
So each copy now lands in **the scope of the row it came from**, not the
70+
request's: the same rule #7559 gave `revertCommit`, for the same stated reason —
71+
this loop now processes a batch that "legitimately mixes an env-wide artifact
72+
with an org overlay". Scoped to the org-scoped door alone; with no
73+
`organizationId` every copy is still written env-wide exactly as before.
74+
75+
### One hazard this fix introduces rather than inherits
76+
77+
Widening the scan makes a collision newly possible: an item can now appear twice,
78+
as an env-wide row **plus** this org's overlay of it. Both copies would land on
79+
the same target key (`type, name, organization_id, COALESCE(package_id, '')`), so
80+
the surviving body would be decided by driver row order. The caller's own org now
81+
shadows env-wide — ADR-0005 overlay precedence, the same order
82+
`resolveMetaItemOrgScope` applies — and that is pinned as its own case.
83+
84+
## `reassignOrphanedMetadata` — the sharper member
85+
86+
Measured before the fix: two orphans, one env-wide and one org-scoped, adopted by
87+
an org caller answered `{success: true, reassignedCount: 1}`, leaving the
88+
env-wide orphan at `package_id = null` with nothing reporting it skipped.
89+
**Finding orphans is this method's entire purpose**, so a class of orphan it
90+
structurally cannot see is a wrong answer, not a partial one.
91+
92+
ADR-0070 D5 settles the scope question the widening raises (an org-scoped caller
93+
now rebinds rows every org can see): the unit is explicitly the **environment** —
94+
"bulk-assign legacy orphans to a default base named for the environment",
95+
completing when "an environment has no orphans" — in a deployment model whose own
96+
words are "there is no per-org overlay dimension here… the relevant axis is code
97+
package vs writable base, not 'org'". Under the model this method was designed
98+
for, every orphan is env-wide, so the strict equality made it **inert** for an
99+
org-scoped caller in precisely that deployment.
100+
101+
## The no-org branch is deliberately NOT narrowed
102+
103+
On both sites, exactly as #7705, #7779 and tier 1 left theirs. The exposure is
104+
worst at `reassignOrphanedMetadata`, whose no-org `where` is `{}` and already
105+
scans every organization's rows; narrowing either door to `organization_id IS
106+
NULL` would re-create this bug pointed the other way. Both doors are pinned as
107+
they stand so they cannot drift silently. Whether the orphan door *should* be
108+
that wide is #7780's open product question — a maintainer call, not decided here.
109+
110+
## Pin
111+
112+
`packages/runtime/src/package-duplicate-adopt-org-scope.integration.test.ts` — a
113+
real `ObjectQL` over a real `SqlDriver` on better-sqlite3, seeded through the real
114+
publish path, because the question is whether `organization_id = 'org'` matches a
115+
NULL column: a property of the driver's SQL, not of a stub's `filter()`. Every
116+
existing suite over these two methods either stubs `engine.find`
117+
(`packages/objectql/src/protocol-package-lifecycle.test.ts`) or never passes an
118+
org (the ADR-0070 dogfood), which is exactly why none could see this family. It
119+
lives in `packages/runtime` because `metadata-protocol` cannot import `objectql`
120+
(dependency cycle).
121+
122+
Twelve cases: the premise measured out of SQLite; the live-orphan producer; the
123+
positive for each site; the reference-rewrite consequence; the org-shadows-env
124+
precedence; both negative directions per site (another organization's rows,
125+
another package's rows, owned rows); and the no-org door on each site.
126+
127+
**Reverse verification**, direction predicted before running: restoring the strict
128+
equality turns red exactly the two positives, the reference-rewrite case, and the
129+
two orphan cases that assert the env-wide orphan is adopted — five — leaving the
130+
negative directions and both no-org doors green, since strict equality is
131+
*narrower* than the `$or`. Measured: **5 failed | 7 passed**, exactly those five.
132+
133+
⚠️ These suites resolve `@objectstack/metadata-protocol` through its **`dist`**
134+
and source-map traces back to `src`, so a source-only revert measures nothing
135+
while looking like it measured something. The package was rebuilt between every
136+
measurement above.

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 131 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11789,8 +11789,67 @@ export class ObjectStackProtocolImplementation implements
1178911789
request.targetNamespace ?? (request.targetPackageId.split('.').pop() ?? request.targetPackageId);
1179011790

1179111791
const where: Record<string, unknown> = { package_id: request.sourcePackageId, state: 'active' };
11792-
if (request.organizationId) where.organization_id = request.organizationId;
11793-
const rows = (await this.engine.find('sys_metadata', { where })) as any[];
11792+
// [#7819 tier 2] Copy the source's env-wide (`organization_id IS NULL`)
11793+
// rows too, not just the ones this org happens to own — the same `$or`
11794+
// {@link deletePackage} (#7705) and {@link listCommits} (#7779) carry.
11795+
// Unlike the tier-1 sites this really is plain scan scoping (`where` is
11796+
// keyed on package + state, not on `id`), so the family remedy applies
11797+
// without their authorization question.
11798+
//
11799+
// Measured on a real driver before the fix: a source package holding one
11800+
// env-wide row and one org-scoped row duplicated by an org caller
11801+
// answered `{success: true, copiedCount: 1, failedCount: 0}` — a PARTIAL
11802+
// copy reported as a whole one, because `organization_id = <org>` matches
11803+
// no NULL column. The mixed state is ordinary, not contrived: a publish
11804+
// made before an active org was selected lands its `sys_metadata` row
11805+
// env-wide (`saveMetaItem` writes `organization_id = NULL`), and
11806+
// `resolveActiveOrganizationId` yields `undefined` for such a session
11807+
// *and* for any throw on the auth seam.
11808+
//
11809+
// The sharper consequence is the rename map below, which is built ONLY
11810+
// from the rows this scan returns. With the env-wide OBJECT rows missing
11811+
// it came out empty, so a copied view was renamed `iojn2_list` while its
11812+
// `data.object` still pointed at the SOURCE package's `iojn_widget` — a
11813+
// duplicate silently wired back to the base it was cloned from, reporting
11814+
// success. An all-env-wide source degraded differently and just as
11815+
// quietly: `{success: false, copiedCount: 0, failedCount: 0}`, nothing
11816+
// copied and nothing named as failed.
11817+
//
11818+
// The no-org branch is deliberately NOT narrowed to `organization_id IS
11819+
// NULL`, exactly as #7705 / #7779 / tier 1 left theirs: that door copies
11820+
// every scope today, and restricting it to env-wide rows would drop every
11821+
// org-scoped row from the copy — the same bug pointed the other way.
11822+
if (request.organizationId) {
11823+
where.$or = [
11824+
{ organization_id: request.organizationId },
11825+
{ organization_id: null },
11826+
];
11827+
}
11828+
const scanned = (await this.engine.find('sys_metadata', { where })) as any[];
11829+
11830+
// [#7819 tier 2] ADR-0005 overlay precedence — the caller's OWN org
11831+
// shadows env-wide ({@link resolveMetaItemOrgScope} states the same rule
11832+
// for history lineages). Widening the scan makes a collision newly
11833+
// possible that could not occur while it was a strict equality: one item
11834+
// can now appear TWICE, as an env-wide row PLUS this org's overlay of it.
11835+
// Every copy is written under `request.organizationId`, so both would
11836+
// land on the same target key — overlay uniqueness is
11837+
// `(type, name, organization_id, COALESCE(package_id, ''))` — and which
11838+
// body survived would be decided by driver row order. Keep the org
11839+
// overlay: it is what this caller already reads everywhere else.
11840+
let rows = scanned;
11841+
if (request.organizationId) {
11842+
const byKey = new Map<string, any>();
11843+
for (const row of scanned) {
11844+
const key = `${row?.type}\u0000${row?.name}`;
11845+
const kept = byKey.get(key);
11846+
const keptIsEnvWide = kept != null && (kept.organization_id ?? null) === null;
11847+
if (kept == null || (keptIsEnvWide && (row?.organization_id ?? null) !== null)) {
11848+
byKey.set(key, row);
11849+
}
11850+
}
11851+
rows = [...byKey.values()];
11852+
}
1179411853

1179511854
// Map only OBJECT names that carry the source namespace prefix; views/etc.
1179611855
// are renamed by the same prefix swap and reference-rewritten via the map.
@@ -11932,14 +11991,44 @@ export class ObjectStackProtocolImplementation implements
1193211991
}
1193311992
const rewritten = deepRewrite(item);
1193411993
if (rewritten && typeof rewritten === 'object' && !Array.isArray(rewritten)) rewritten.name = newName;
11994+
// [#7819 tier 2] The copy lands in the SCOPE OF THE ROW IT CAME
11995+
// FROM, not the request's — the same rule #7559 gave `revertCommit`
11996+
// ({@link resolveMetaItemOrgScope}) for the same reason, now that
11997+
// widening the scan above means this loop, too, processes a batch
11998+
// that "legitimately mixes an env-wide artifact with an org
11999+
// overlay".
12000+
//
12001+
// Not cosmetic: without it the read fix alone cannot produce a
12002+
// working duplicate. Stamping the request's org on every copy is
12003+
// REFUSED for any type the metadata-type registry declares
12004+
// `allowOrgOverride=false` — `object` among them — with
12005+
// `NOT_OVERRIDABLE`, because boot hydration loads env-wide rows
12006+
// only and an org-scoped `object` row would vanish on the next
12007+
// restart (ADR-0005, #6190). Since an `object` therefore CANNOT
12008+
// exist org-scoped, every object row in a source package is
12009+
// env-wide, and an org-scoped `duplicatePackage` could not copy a
12010+
// single one: before this card the strict equality hid them, and
12011+
// with only the scan widened they would land in `failed[]`
12012+
// instead. Objects being what a base is mostly made of, ADR-0070
12013+
// D4's "duplicate base" gesture was structurally unable to
12014+
// duplicate a base whenever an org was active.
12015+
//
12016+
// Scoped to the org-scoped door alone. With no `organizationId` on
12017+
// the request the scan returns every organization's rows and each
12018+
// copy is written env-wide exactly as before — that door's
12019+
// behaviour is deliberately left byte-identical, as this card
12020+
// leaves all of its no-org branches.
12021+
const copyOrgId: string | null = request.organizationId
12022+
? ((row?.organization_id ?? null) as string | null)
12023+
: null;
1193512024
try {
1193612025
await this.saveMetaItem({
1193712026
type: row.type,
1193812027
name: newName,
1193912028
item: rewritten,
1194012029
mode: 'publish',
1194112030
packageId: request.targetPackageId,
11942-
...(request.organizationId ? { organizationId: request.organizationId } : {}),
12031+
...(copyOrgId ? { organizationId: copyOrgId } : {}),
1194312032
...(request.actor ? { actor: request.actor } : {}),
1194412033
});
1194512034
copied.push({ type: row.type, name: newName });
@@ -11978,7 +12067,45 @@ export class ObjectStackProtocolImplementation implements
1197812067
targetPackageId: string;
1197912068
}> {
1198012069
const where: Record<string, unknown> = {};
11981-
if (request.organizationId) where.organization_id = request.organizationId;
12070+
// [#7819 tier 2] See env-wide (`organization_id IS NULL`) orphans too.
12071+
// This is the sharper member of the family, because FINDING ORPHANS IS
12072+
// THE ENTIRE PURPOSE of this method: a class of orphan it structurally
12073+
// cannot see is not a partial answer, it is a wrong one. Measured on a
12074+
// real driver before the fix — two orphans, one env-wide and one
12075+
// org-scoped, adopted by an org caller: `{success: true,
12076+
// reassignedCount: 1}`, with the env-wide orphan left at
12077+
// `package_id = null` and nothing reporting that it was skipped.
12078+
//
12079+
// Not a legacy-only population, which is what makes this live rather
12080+
// than latent. The docstring above calls orphans a pre-package-first
12081+
// residue, and ADR-0070 D1 does reject NEW orphans that name a
12082+
// read-only package (`WRITABLE_PACKAGE_REQUIRED`) — but a
12083+
// `saveMetaItem` that names NO package at all still succeeds today and
12084+
// lands `package_id = null, organization_id = null`, i.e. the current
12085+
// write path mints exactly the orphan this scan could not see.
12086+
//
12087+
// ADR-0070 D5 settles the scope question this widening raises (an
12088+
// org-scoped caller now rebinds rows every org can see): the unit is
12089+
// explicitly the ENVIRONMENT — "bulk-assign legacy orphans to a default
12090+
// base named for the environment", completing when "an environment has
12091+
// no orphans", in a deployment model whose own words are "there is no
12092+
// per-org overlay dimension here… the relevant axis is code package vs
12093+
// writable base, not 'org'". Under that model every orphan is env-wide,
12094+
// so the strict equality made this method inert for an org-scoped
12095+
// caller in precisely the deployment it was designed for.
12096+
//
12097+
// ⛔ The no-org branch stays `{}` — deliberately un-narrowed, and this
12098+
// is the exposure the card flagged as worst: that door already scans
12099+
// EVERY organization's rows. Narrowing it to `organization_id IS NULL`
12100+
// would re-create this same bug pointed the other way. Whether that
12101+
// door should be that wide is #7780's open product question, which is
12102+
// a maintainer call and explicitly NOT decided here.
12103+
if (request.organizationId) {
12104+
where.$or = [
12105+
{ organization_id: request.organizationId },
12106+
{ organization_id: null },
12107+
];
12108+
}
1198212109
const rows = (await this.engine.find('sys_metadata', { where })) as any[];
1198312110
const orphans = rows.filter(
1198412111
(r) => r?.package_id == null || r.package_id === '' || r.package_id === 'sys_metadata',

0 commit comments

Comments
 (0)