Skip to content

Commit 4c5a584

Browse files
os-zhuangclaude
andauthored
fix(seed-loader): resolve seed references for marketplace-installed objects (engine-registry fallback) (#3422)
* fix(seed-loader): resolve references for objects only in the engine registry (marketplace installs) Marketplace package objects register through the manifest service straight into the ObjectQL registry — after the boot-time bridgeObjectsToMetadataService pass — so the metadata service never lists them. buildDependencyGraph consulted only metadata.getObject(), got an empty reference graph for crm_*, and wrote every lookup/master_detail seed value verbatim (the raw externalId string, e.g. crm_contact.crm_account = 'Acme Corporation') instead of the target record's id. Under sharingModel: controlled_by_parent the dangling parent join then hid every contact from every user (REST total=0, GET 404) while the rows sat in the table. The loader now falls back to the engine's feature-detected getSchema() when the metadata service has no definition for a seeded object. Metadata stays the preferred source; contract-minimal engines keep the old behavior. Regression tests: a metadata-protocol unit spec (engine-registry-only objects resolve, metadata still wins when present, no-getSchema engines degrade as before) and a cloud-connection integration spec driving the real install endpoint + real SeedLoaderService, pinning that reference columns hold ids. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(cloud-connection): raise seed-lookup install spec timeout for cold CI runners The spec drives the real install handler, whose dynamic import('@objectstack/runtime') alone can take several seconds on a fresh runner — the 5s vitest default timed out on CI while passing warm locally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent fa3d0cf commit 4c5a584

4 files changed

Lines changed: 531 additions & 1 deletion

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(seed-loader): resolve lookup/master_detail references for objects that only live in the engine registry (marketplace installs)
6+
7+
`SeedLoaderService.buildDependencyGraph` consulted only `metadata.getObject()`
8+
when building the reference graph. Marketplace-installed packages register
9+
their objects through the `manifest` service straight into the ObjectQL
10+
registry — after the boot-time `bridgeObjectsToMetadataService` pass — so the
11+
metadata service never lists them. The reference graph came back empty for
12+
those objects and every lookup / master_detail seed value was written
13+
verbatim: `crm_contact.crm_account` held the authored natural key
14+
(`"Acme Corporation"`) instead of the target record's id.
15+
16+
The damage compounded under RLS: `crm_contact` declares
17+
`sharingModel: controlled_by_parent`, whose row filter compiles to a join on
18+
the parent reference. With every reference dangling, the join matched nothing
19+
and the whole object went invisible to everyone — platform admins included —
20+
while the rows sat in the table (REST list `total=0`, single GET 404).
21+
22+
The loader now falls back to the engine's own schema registry
23+
(feature-detected `engine.getSchema()`, which the ObjectQL engine exposes)
24+
whenever the metadata service has no definition for a seeded object. The
25+
metadata service remains the preferred source; engines without a schema
26+
registry keep the old behavior.
Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Marketplace install MUST resolve seed lookup/master_detail natural keys
5+
* into target record ids.
6+
*
7+
* The bug this pins: marketplace package objects are registered through the
8+
* `manifest` service straight into the ObjectQL registry — AFTER the boot-time
9+
* `bridgeObjectsToMetadataService` pass — so the metadata service never lists
10+
* them. The SeedLoaderService built its reference graph from the metadata
11+
* service only, saw no lookup/master_detail fields for crm_*, and wrote every
12+
* reference value verbatim: `crm_contact.crm_account = 'Acme Corporation'`
13+
* instead of the crm_account row's id. With `sharingModel:
14+
* controlled_by_parent` the dangling parent join then hid EVERY contact from
15+
* EVERY user (REST list total=0, single GET 404) while the rows sat in the DB.
16+
*
17+
* Unlike marketplace-install-local-reseed.test.ts, this test does NOT mock
18+
* @objectstack/runtime — the real SeedLoaderService runs against a faithful
19+
* engine stub whose `getSchema` serves exactly what `manifest.register`
20+
* (ql.registerApp) put into the engine registry, and a metadata service that
21+
* has never heard of the package's objects (the marketplace reality).
22+
*/
23+
24+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
25+
import { mkdtempSync, rmSync } from 'node:fs';
26+
import { join } from 'node:path';
27+
import { tmpdir } from 'node:os';
28+
29+
import { MarketplaceInstallLocalPlugin } from './marketplace-install-local-plugin.js';
30+
31+
type Handler = (c: any) => Promise<any>;
32+
33+
function makeRawApp() {
34+
const routes = new Map<string, Handler>();
35+
return {
36+
routes,
37+
get: (p: string, h: Handler) => routes.set(`GET ${p}`, h),
38+
post: (p: string, h: Handler) => routes.set(`POST ${p}`, h),
39+
delete: (p: string, h: Handler) => routes.set(`DELETE ${p}`, h),
40+
};
41+
}
42+
43+
function makeCtx(rawApp: any, services: Record<string, any>) {
44+
const hooks = new Map<string, any>();
45+
return {
46+
ctx: {
47+
hook: (e: string, h: any) => hooks.set(e, h),
48+
getService: (name: string) => {
49+
if (name === 'http-server') return { getRawApp: () => rawApp };
50+
const svc = services[name];
51+
if (svc === undefined) throw new Error(`no ${name}`);
52+
return svc;
53+
},
54+
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
55+
},
56+
fire: async () => { await hooks.get('kernel:ready')?.(); },
57+
};
58+
}
59+
60+
function makeC(body: any, manifestId?: string) {
61+
const json = vi.fn((payload: any, status?: number) => ({ payload, status: status ?? 200 }));
62+
return {
63+
req: {
64+
url: 'http://localhost:3000/api/v1/marketplace/install-local',
65+
raw: new Request('http://localhost:3000/x'),
66+
json: async () => body,
67+
param: (k: string) => (k === 'manifestId' ? manifestId : undefined),
68+
header: () => undefined,
69+
},
70+
json,
71+
};
72+
}
73+
74+
/** Engine stub faithful where it matters: find() filters `where`, insert()
75+
* assigns ids, and `getSchema` reads the registry that `manifest.register`
76+
* fills — the exact surface the real ObjectQL engine gives the seed loader. */
77+
function makeEngine() {
78+
const store: Record<string, any[]> = {};
79+
const registry: Record<string, any> = {};
80+
let idCounter = 0;
81+
const engine: any = {
82+
find: async (objectName: string, query?: any) => {
83+
let records = store[objectName] || [];
84+
if (query?.where) {
85+
records = records.filter((r) =>
86+
Object.entries(query.where).every(([k, v]) => r[k] === v),
87+
);
88+
}
89+
if (typeof query?.limit === 'number') records = records.slice(0, query.limit);
90+
return records;
91+
},
92+
insert: async (objectName: string, data: any) => {
93+
if (!store[objectName]) store[objectName] = [];
94+
if (Array.isArray(data)) {
95+
const records = data.map((d) => ({ id: `row-${++idCounter}`, ...d }));
96+
store[objectName].push(...records);
97+
return records;
98+
}
99+
const record = { id: `row-${++idCounter}`, ...data };
100+
store[objectName].push(record);
101+
return record;
102+
},
103+
update: async (objectName: string, data: any) => {
104+
const records = store[objectName] || [];
105+
const idx = records.findIndex((r) => r.id === data.id);
106+
if (idx >= 0) {
107+
records[idx] = { ...records[idx], ...data };
108+
return records[idx];
109+
}
110+
return data;
111+
},
112+
delete: async () => ({ deleted: 1 }),
113+
count: async (objectName: string) => (store[objectName] || []).length,
114+
aggregate: async () => [],
115+
getSchema: (name: string) => registry[name],
116+
syncSchemas: async () => undefined,
117+
registerApp: (manifest: any) => {
118+
for (const obj of manifest?.objects ?? []) {
119+
if (obj?.name) registry[obj.name] = obj;
120+
}
121+
},
122+
};
123+
return { engine, store, registry };
124+
}
125+
126+
/** Trimmed from the shipped app.objectstack.hotcrm@2.2.2 artifact: the
127+
* master_detail child under controlled_by_parent whose install corrupted. */
128+
const HOTCRM_MANIFEST = {
129+
id: 'app.test.hotcrm',
130+
name: 'HotCRM Test',
131+
version: '9.9.9',
132+
objects: [
133+
{
134+
name: 'crm_account',
135+
label: 'Account',
136+
fields: {
137+
name: { type: 'text', label: 'Name', required: true },
138+
industry: { type: 'text', label: 'Industry' },
139+
},
140+
},
141+
{
142+
name: 'crm_contact',
143+
label: 'Contact',
144+
sharingModel: 'controlled_by_parent',
145+
fields: {
146+
last_name: { type: 'text', label: 'Last Name', required: true },
147+
email: { type: 'text', label: 'Email' },
148+
crm_account: { type: 'master_detail', label: 'Account', reference: 'crm_account', required: true },
149+
},
150+
},
151+
{
152+
name: 'crm_opportunity',
153+
label: 'Opportunity',
154+
fields: {
155+
name: { type: 'text', label: 'Name', required: true },
156+
crm_account: { type: 'lookup', label: 'Account', reference: 'crm_account', required: true },
157+
},
158+
},
159+
],
160+
data: [
161+
{
162+
object: 'crm_account',
163+
externalId: 'name',
164+
mode: 'upsert',
165+
records: [
166+
{ name: 'Acme Corporation', industry: 'technology' },
167+
{ name: 'Globex Inc', industry: 'manufacturing' },
168+
],
169+
},
170+
{
171+
object: 'crm_contact',
172+
externalId: 'email',
173+
mode: 'upsert',
174+
records: [
175+
{ last_name: 'Smith', email: 'john.smith@acme.example.com', crm_account: 'Acme Corporation' },
176+
{ last_name: 'Lee', email: 'maria.lee@globex.example.com', crm_account: 'Globex Inc' },
177+
],
178+
},
179+
{
180+
object: 'crm_opportunity',
181+
externalId: 'name',
182+
mode: 'upsert',
183+
records: [{ name: 'Acme Expansion', crm_account: 'Acme Corporation' }],
184+
},
185+
],
186+
};
187+
188+
let dir: string;
189+
beforeEach(() => { dir = mkdtempSync(join(tmpdir(), 'mil-seed-lookup-')); });
190+
afterEach(() => { rmSync(dir, { recursive: true, force: true }); vi.restoreAllMocks(); });
191+
192+
describe('marketplace install — seed lookup resolution', () => {
193+
// Generous timeout: the install handler dynamically imports the real
194+
// @objectstack/runtime (unmocked on purpose), and that cold import alone
195+
// can eat several seconds on a fresh CI runner.
196+
it('writes target record ids (never raw externalId strings) for package objects unknown to the metadata service', { timeout: 30_000 }, async () => {
197+
const { engine, store, registry } = makeEngine();
198+
const rawApp = makeRawApp();
199+
const { ctx, fire } = makeCtx(rawApp, {
200+
// The real wiring: manifest.register → ql.registerApp → engine
201+
// registry ONLY. Nothing reaches the metadata service.
202+
manifest: { register: (m: any) => engine.registerApp(m) },
203+
auth: { api: { getSession: async () => ({ user: { id: 'admin' } }) } },
204+
objectql: engine,
205+
metadata: { getObject: vi.fn(async () => undefined), list: vi.fn(async () => []) },
206+
});
207+
const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir });
208+
await plugin.start(ctx as any);
209+
await fire();
210+
211+
const res = await rawApp.routes.get('POST /api/v1/marketplace/install-local')!(
212+
makeC({ manifest: HOTCRM_MANIFEST }),
213+
);
214+
expect(res.payload?.success).toBe(true);
215+
// Objects really did register only into the engine registry.
216+
expect(registry.crm_contact).toBeDefined();
217+
218+
// The inline seed ran and landed every row.
219+
expect(res.payload?.data?.seeded?.mode).toBe('inline');
220+
expect(res.payload?.data?.seeded?.errors).toBe(0);
221+
expect(store.crm_account).toHaveLength(2);
222+
expect(store.crm_contact).toHaveLength(2);
223+
expect(store.crm_opportunity).toHaveLength(1);
224+
225+
// THE regression: reference columns must hold the parent row's id.
226+
const acme = store.crm_account.find((r) => r.name === 'Acme Corporation')!;
227+
const globex = store.crm_account.find((r) => r.name === 'Globex Inc')!;
228+
const john = store.crm_contact.find((r) => r.email === 'john.smith@acme.example.com')!;
229+
const maria = store.crm_contact.find((r) => r.email === 'maria.lee@globex.example.com')!;
230+
const opp = store.crm_opportunity[0];
231+
232+
expect(john.crm_account).toBe(acme.id);
233+
expect(maria.crm_account).toBe(globex.id);
234+
expect(opp.crm_account).toBe(acme.id);
235+
236+
// Belt and braces: no reference column anywhere still carries the
237+
// authored natural-key string.
238+
for (const row of [...store.crm_contact, ...store.crm_opportunity]) {
239+
expect(String(row.crm_account)).not.toMatch(/Acme Corporation|Globex Inc/);
240+
}
241+
});
242+
});

0 commit comments

Comments
 (0)