Skip to content

Commit dbe92a7

Browse files
baozhoutaoclaude
andauthored
fix(metadata-protocol): boot 重水合按行的真实 package_id 登记对象归属 (#4636) (#6261)
`loadMetaFromDb` 的 object 分支从 `engine.find` 的行上读 `record.packageId`, 而 `sys_metadata` 的列是 snake_case 的 `package_id`,该表达式恒为 `undefined || 'sys_metadata'` —— 每次重启都把绑定了包的对象 overlay 登记在 哨兵下。改读 `package_id`,与写路径(#4636 PR1)、`getMetaItems` 以及相邻的 非 object 分支一致。 归属键同时是包过滤键(`getAllObjects(packageId)`),所以此前对象在创建时 出现在自己所属包的侧边栏过滤里、重启后消失;更要紧的是重启后的第一次编辑: boot 登记 `'sys_metadata'`、保存登记 `app.<slug>`,`registerObject` 抛 `already owned by package …` 被 `applyObjectRegistryMutation` 吞成 warn, 保存回 `success: true` 而内存 schema 停在重启时的版本,该笔编辑被静默丢弃 (cloud#970 的重启面)。 同时把 `objectql/src/registry.ts` `isTenantAuthored` 的契约注释收尾:摘掉 PR1 加的「这半句描述的是契约,还不是代码」标注 —— 两侧现已一致。 Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We Co-authored-by: Claude <noreply@anthropic.com>
1 parent 018440f commit dbe92a7

4 files changed

Lines changed: 341 additions & 7 deletions

File tree

.changeset/lucky-moons-smoke.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
'@objectstack/objectql': patch
4+
---
5+
6+
fix(metadata-protocol): boot 重水合按行的真实 package 绑定登记对象归属(#4636 裁 B 收官)
7+
8+
`loadMetaFromDb` 的 object 分支从 `engine.find` 返回的行上读 `record.packageId`,而 `sys_metadata` 的列是 snake_case 的 `package_id` —— 该表达式恒为 `undefined || 'sys_metadata'`,于是每次重启都把**绑定了包**的对象 overlay 登记在 `'sys_metadata'` 哨兵下。改为读 `package_id`,与写路径、`getMetaItems`、以及相邻的非 object 分支一致。
9+
10+
用户可见的行为差异:归属键同时就是包过滤键(`getAllObjects(packageId)`),所以此前一个对象在**创建时**出现在自己所属包的侧边栏过滤里,**重启之后就消失**;更要紧的是重启后的第一次编辑——boot 登记 `'sys_metadata'`、保存登记 `app.<slug>`,`registerObject``already owned by package …``applyObjectRegistryMutation` 吞成 `console.warn`,保存回 `success: true` 而内存 schema 停在重启时的版本,这一笔编辑被静默丢弃(cloud#970 的重启面)。两侧统一到真实 id 后,过滤与编辑都跨重启成立。
11+
12+
`@objectstack/objectql` 仅同步 `registry.ts``isTenantAuthored` 的契约注释:PR1 标注的「这半句描述的是契约,还不是代码」随本次落地摘除。

packages/metadata-protocol/src/protocol.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10760,9 +10760,23 @@ export class ObjectStackProtocolImplementation implements
1076010760
// the very next write with `not_overridable`. An app the user
1076110761
// had just built became un-editable at the first kernel
1076210762
// rebuild (cloud#970).
10763+
//
10764+
// The ownership key is the row's REAL package binding
10765+
// (#4636 PR2). These rows come off `engine.find`, so
10766+
// their columns are snake_case — `package_id`, never
10767+
// `packageId`, exactly as `getMetaItems` and the
10768+
// sibling branch below already read them. Reading the
10769+
// camelCase key made the expression `undefined ||
10770+
// 'sys_metadata'`, so every boot registered even a
10771+
// package-bound object under the sentinel and the
10772+
// sidebar's `getAllObjects(packageId)` filter lost it
10773+
// across a restart. `||` and not `??`, symmetric with
10774+
// the write path's `request.packageId || 'sys_metadata'`:
10775+
// an empty binding is "no package", and the sentinel
10776+
// marks exactly that one thing.
1076310777
this.engine.registry.registerObject(
1076410778
{ ...(data as Record<string, unknown>), _provenance: 'org' } as any,
10765-
record.packageId || 'sys_metadata',
10779+
(record as { package_id?: string | null }).package_id || 'sys_metadata',
1076610780
);
1076710781
} else {
1076810782
// Same rule as the getMetaItems read-side hydration and
Lines changed: 310 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,310 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #4636 PR2 — BOOT re-hydration (`loadMetaFromDb`) keys object-overlay
5+
* ownership by the row's REAL package binding, closing the restart half of
6+
* the same defect PR1 closed on the write path.
7+
*
8+
* ## What was wrong
9+
*
10+
* The object branch of `loadMetaFromDb` read `record.packageId` off a row
11+
* that came straight out of `engine.find('sys_metadata', …)` — and those
12+
* rows are keyed by the object's SNAKE_CASE column names. `sys_metadata`
13+
* declares `package_id`; the repository writes `package_id`; `getMetaItems`
14+
* reads `r.package_id`; the sibling (non-object) branch three lines below
15+
* reads `(record as …).package_id`. Only this one branch spelled it
16+
* camelCase, so the expression was permanently `undefined || 'sys_metadata'`
17+
* and EVERY boot-hydrated object registered under the sentinel, package-bound
18+
* or not.
19+
*
20+
* ## The behaviour that pins it: create, restart, edit
21+
*
22+
* The ownership key is the package-filter key — `getAllObjects(packageId)`
23+
* matches `contributor.packageId` and the runtime sidebar consumes it. After
24+
* PR1 the write path records `app.<slug>`, so the surviving defect was
25+
* exactly restart-shaped: an object was in its package's filter when you
26+
* created it and gone after a reboot.
27+
*
28+
* Worse than the missing filter row, and the reason this file simulates a
29+
* real restart rather than asserting the key in isolation: with the two sides
30+
* disagreeing, the FIRST edit after a restart re-claimed ownership under a
31+
* different key, `registerObject` threw `already owned by package
32+
* "sys_metadata"`, and `applyObjectRegistryMutation` catches that into a
33+
* `console.warn`. The save answered `success: true` while the in-memory
34+
* schema stayed at the pre-edit version — the edit was dropped silently. It
35+
* is cloud#970's restart surface in its post-PR1 form: not a `403`, because
36+
* both sides do stamp `_provenance: 'org'` and the overlay gate stays open,
37+
* but a swallowed ownership clash. So the assertions below run a full
38+
* session-1-writes / session-2-boots / session-2-edits cycle against the
39+
* real `SchemaRegistry`, and check the evolved field actually lands.
40+
*
41+
* This file lives in `@objectstack/objectql` for the same reason PR1's
42+
* `protocol-writepath-object-ownership.test.ts` does: the subject is the REAL
43+
* `SchemaRegistry` (contributor ownership, `getAllObjects` filtering), and
44+
* `@objectstack/objectql` depends on `@objectstack/metadata-protocol` — only
45+
* this direction can hold both halves without closing a cycle turbo rejects.
46+
*/
47+
48+
import { describe, expect, it } from 'vitest';
49+
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
50+
import { SchemaRegistry } from './registry.js';
51+
// [#4550 / #5480] The producer's OWN write-verb dispatch decisions, so this
52+
// double cannot accept a call `ObjectQL.delete` / `ObjectQL.update` refuses.
53+
import { assertEngineDeleteDispatch } from './engine-delete-dispatch.js';
54+
import { assertEngineUpdateDispatch } from './engine-update-dispatch.js';
55+
56+
/** A Studio authoring workspace id — writable under ADR-0070. */
57+
const APP_PKG = 'app.myapp';
58+
const OTHER_PKG = 'app.otherapp';
59+
/** The key an overlay row bound to NO package keeps. */
60+
const SENTINEL = 'sys_metadata';
61+
62+
interface Row {
63+
id: string;
64+
type: string;
65+
name: string;
66+
organization_id: string | null;
67+
package_id: string | null;
68+
state: string;
69+
metadata: string;
70+
checksum?: string;
71+
version?: number;
72+
}
73+
74+
interface HistoryRow {
75+
id: string;
76+
event_seq: number;
77+
type: string;
78+
name: string;
79+
version: number;
80+
operation_type: string;
81+
metadata: string | null;
82+
checksum: string | null;
83+
organization_id: string | null;
84+
recorded_at: string;
85+
}
86+
87+
function matches(r: Record<string, unknown>, where: Record<string, unknown>): boolean {
88+
for (const [k, v] of Object.entries(where)) {
89+
if (v === undefined) continue;
90+
if ((r as any)[k] !== v) return false;
91+
}
92+
return true;
93+
}
94+
95+
function keyOf(w: Record<string, unknown>) {
96+
return `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`;
97+
}
98+
99+
/**
100+
* One kernel process: a fresh `SchemaRegistry` + protocol over a
101+
* `sys_metadata` table. `seed` is how a RESTART is expressed — the rows a
102+
* previous process persisted, handed to a brand-new registry that knows
103+
* nothing about them until `loadMetaFromDb` runs.
104+
*/
105+
function makeSession(seed: { rows?: Row[]; history?: HistoryRow[] } = {}) {
106+
const registry = new SchemaRegistry({ multiTenant: false });
107+
registry.logLevel = 'silent';
108+
const rows = new Map<string, Row>();
109+
for (const r of seed.rows ?? []) rows.set(keyOf(r), { ...r });
110+
const historyRows: HistoryRow[] = (seed.history ?? []).map((h) => ({ ...h }));
111+
const synced: string[] = [];
112+
let nextId = 0;
113+
const findRow = (w: Record<string, unknown>) => {
114+
for (const [k, r] of rows) if (matches(r, w)) return { key: k, row: r };
115+
return null;
116+
};
117+
const engine: any = {
118+
registry,
119+
async findOne(table: string, opts: { where: Record<string, unknown> }) {
120+
if (table === 'sys_metadata_history') {
121+
return historyRows.find((h) => matches(h as any, opts.where)) ?? null;
122+
}
123+
return findRow(opts.where)?.row ?? null;
124+
},
125+
async find(table: string, opts: { where: Record<string, unknown> }) {
126+
if (table === 'sys_metadata_history') {
127+
return historyRows.filter((h) => matches(h as any, opts.where));
128+
}
129+
return Array.from(rows.values()).filter((r) => matches(r, opts.where));
130+
},
131+
async insert(table: string, data: Record<string, unknown>) {
132+
if (table === 'sys_metadata_history') {
133+
const h = { id: `h_${++nextId}`, ...(data as any) } as HistoryRow;
134+
historyRows.push(h);
135+
return { id: h.id };
136+
}
137+
if (table !== 'sys_metadata') return { id: 'side_table' };
138+
const row = { id: `r_${++nextId}`, ...(data as any) } as Row;
139+
rows.set(keyOf(data), row);
140+
return { id: row.id };
141+
},
142+
async update(table: string, data: Record<string, unknown>, opts: { where: Record<string, unknown> }) {
143+
assertEngineUpdateDispatch(data, opts);
144+
if (table !== 'sys_metadata') return { id: null };
145+
const found = findRow(opts.where);
146+
if (!found) return { id: null };
147+
const merged = { ...found.row, ...(data as any) };
148+
rows.delete(found.key);
149+
rows.set(keyOf(merged), merged);
150+
return { id: found.row.id };
151+
},
152+
async delete(_t: string, opts?: Record<string, unknown>) {
153+
assertEngineDeleteDispatch(opts);
154+
return { deleted: 0 };
155+
},
156+
async syncObjectSchema(name: string) { synced.push(name); },
157+
};
158+
// A PROJECT kernel (`environmentId` set) — the topology cloud#970 was
159+
// reported on, and the only one where `saveMetaItem`'s overlay gate is
160+
// engaged at all.
161+
const protocol = new ObjectStackProtocolImplementation(engine, undefined, 'env_test');
162+
return { registry, protocol, rows, historyRows, synced };
163+
}
164+
165+
function objectBody(name: string, extra?: Record<string, unknown>) {
166+
return {
167+
name,
168+
label: 'Invoice',
169+
fields: {
170+
name: { name: 'name', type: 'text', label: 'Name' },
171+
amount: { name: 'amount', type: 'number', label: 'Amount' },
172+
},
173+
...extra,
174+
};
175+
}
176+
177+
/** The owning contributor recorded for `name` (no namespace → fqn === name). */
178+
const owner = (registry: SchemaRegistry, name: string) => registry.getObjectOwner(name);
179+
180+
/**
181+
* Session 1: author the object through the REAL write path, then hand its
182+
* persisted rows to a brand-new process. Hand-crafting the row would let the
183+
* fixture drift from what the repository actually writes — the column
184+
* spelling is the entire subject of this test.
185+
*/
186+
async function persistThenRestart(opts: { name: string; packageId?: string }) {
187+
const first = makeSession();
188+
const res = await first.protocol.saveMetaItem({
189+
type: 'object',
190+
name: opts.name,
191+
...(opts.packageId ? { packageId: opts.packageId } : {}),
192+
item: objectBody(opts.name),
193+
});
194+
expect(res.success).toBe(true);
195+
const persisted = Array.from(first.rows.values());
196+
return {
197+
persisted,
198+
reboot: () => makeSession({ rows: persisted, history: first.historyRows }),
199+
};
200+
}
201+
202+
describe('#4636 PR2 — boot re-hydration keys object ownership by the row\'s package_id', () => {
203+
it('registers a package-bound row under its REAL package id, not the sentinel', async () => {
204+
const { persisted, reboot } = await persistThenRestart({
205+
name: 'myapp_invoice',
206+
packageId: APP_PKG,
207+
});
208+
209+
// The row the previous process left behind carries the binding in the
210+
// snake_case column this branch has to read.
211+
expect(persisted).toHaveLength(1);
212+
expect(persisted[0].package_id).toBe(APP_PKG);
213+
expect((persisted[0] as any).packageId).toBeUndefined();
214+
215+
const second = reboot();
216+
const res = await second.protocol.loadMetaFromDb();
217+
218+
expect(res.loaded).toBe(1);
219+
expect(res.errors).toBe(0);
220+
// Pre-fix: `'sys_metadata'` — `record.packageId` was undefined and the
221+
// `|| 'sys_metadata'` fallback always won.
222+
expect(owner(second.registry, 'myapp_invoice')?.packageId).toBe(APP_PKG);
223+
});
224+
225+
it('still stamps `_provenance: \'org\'` on the hydrated body (cloud#970 unchanged)', async () => {
226+
const { reboot } = await persistThenRestart({
227+
name: 'myapp_invoice',
228+
packageId: APP_PKG,
229+
});
230+
231+
const second = reboot();
232+
await second.protocol.loadMetaFromDb();
233+
234+
// Unchanged by PR2 and deliberately so: the row is tenant-authored
235+
// whatever package it is bound to, and this stamp — not the sentinel
236+
// string — is what keeps `isArtifactBacked` false so the overlay gate
237+
// lets the next write through.
238+
expect((owner(second.registry, 'myapp_invoice')?.definition as any)?._provenance).toBe('org');
239+
});
240+
241+
it('the sidebar package filter finds the object again after a restart', async () => {
242+
const { reboot } = await persistThenRestart({
243+
name: 'myapp_invoice',
244+
packageId: APP_PKG,
245+
});
246+
247+
const second = reboot();
248+
await second.protocol.loadMetaFromDb();
249+
250+
// runtime `meta.ts` → `getAllObjects(packageId)`. Pre-fix this was
251+
// empty after every restart even though the object was there before it.
252+
expect(second.registry.getAllObjects(APP_PKG).map((o: any) => o.name)).toEqual(['myapp_invoice']);
253+
expect(second.registry.getAllObjects(OTHER_PKG)).toEqual([]);
254+
});
255+
256+
it('the FIRST edit after a restart lands in the schema (cloud#970 restart surface)', async () => {
257+
const { reboot } = await persistThenRestart({
258+
name: 'myapp_invoice',
259+
packageId: APP_PKG,
260+
});
261+
262+
const second = reboot();
263+
await second.protocol.loadMetaFromDb();
264+
265+
// The user comes back the next morning and adds a field.
266+
const evolved = objectBody('myapp_invoice');
267+
(evolved.fields as any).due_date = { name: 'due_date', type: 'date', label: 'Due' };
268+
const saved = await second.protocol.saveMetaItem({
269+
type: 'object',
270+
name: 'myapp_invoice',
271+
packageId: APP_PKG,
272+
item: evolved,
273+
});
274+
275+
expect(saved.success).toBe(true);
276+
// THE load-bearing assertion. Pre-fix, `success: true` was already
277+
// true — boot claimed `'sys_metadata'`, this save claimed `app.myapp`,
278+
// `registerObject` threw `already owned by package "sys_metadata"`, and
279+
// `applyObjectRegistryMutation` swallowed it into a `console.warn`. The
280+
// write reached the DB and the in-memory schema stayed at the boot
281+
// version, so CRUD on the new field failed until the NEXT restart.
282+
expect(Object.keys((second.registry.getObject('myapp_invoice') as any).fields)).toContain('due_date');
283+
// Ownership survives the re-registration on the same key.
284+
expect(owner(second.registry, 'myapp_invoice')?.packageId).toBe(APP_PKG);
285+
// On disk: still one row, still bound to its package.
286+
const stored = Array.from(second.rows.values()).filter((r) => r.name === 'myapp_invoice');
287+
expect(stored).toHaveLength(1);
288+
expect(stored[0].package_id).toBe(APP_PKG);
289+
});
290+
});
291+
292+
describe('#4636 PR2 — a package-less row keeps the sentinel (regression)', () => {
293+
it('hydrates an unbound row under the sentinel, exactly as before', async () => {
294+
const { persisted, reboot } = await persistThenRestart({ name: 'global_invoice' });
295+
296+
expect(persisted[0].package_id ?? null).toBeNull();
297+
298+
const second = reboot();
299+
const res = await second.protocol.loadMetaFromDb();
300+
301+
expect(res.loaded).toBe(1);
302+
// `||`, not `??`: no binding — including the empty-string spelling —
303+
// means "no package", and the sentinel marks that one thing. Same
304+
// normalisation the write path applies to `request.packageId`.
305+
expect(owner(second.registry, 'global_invoice')?.packageId).toBe(SENTINEL);
306+
expect((owner(second.registry, 'global_invoice')?.definition as any)?._provenance).toBe('org');
307+
// It is not smuggled into any package's filter.
308+
expect(second.registry.getAllObjects(APP_PKG)).toEqual([]);
309+
});
310+
});

packages/objectql/src/registry.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -799,12 +799,10 @@ export class NamespaceConflictError extends Error {
799799
*
800800
* `_packageId !== 'sys_metadata'` alone cannot answer it. That sentinel marks
801801
* one thing only — an overlay row bound to no package. A row that IS bound to
802-
* one is keyed by its real package id on the save path (#4636 PR1) and by the
803-
* boot-time rehydration of `sys_metadata` (#4636 PR2 — the branch still reads a
804-
* camelCase key off a snake_case row, so today it falls back to the sentinel;
805-
* that half of this paragraph describes the contract, not yet the code). Either
806-
* way the key is `app.<slug>`, which is exactly what every code-shipped item
807-
* carries too, so the sentinel test cannot tell them apart. A tenant's own
802+
* one is keyed by its real package id on BOTH sides that register it: the save
803+
* path (#4636 PR1) and the boot-time rehydration of `sys_metadata` (#4636 PR2).
804+
* Either way the key is `app.<slug>`, which is exactly what every code-shipped
805+
* item carries too, so the sentinel test cannot tell them apart. A tenant's own
808806
* overlay came back from a kernel rebuild looking like a code
809807
* artifact, and the protocol's overlay gate refused the next write to it with
810808
* `not_overridable` — an app the user had just built through Studio/AI became

0 commit comments

Comments
 (0)