Skip to content

Commit 7de2ebe

Browse files
os-zhuangclaude
andcommitted
fix(metadata): package-scope the layered/Studio-editor read via ?package= (ADR-0048)
The ?layers=true single-item read (Studio editor's 3-state view) ignored packageId, so editing one of two same-named items from different packages resolved ambiguously. Thread packageId through getMetaItemLayered (code layer + overlay query), registry.getArtifactItem / lookupArtifactItem (prefer-local), and the rest-server layered branch. Verified live: the Studio editor at /apps/:app/metadata/doc/showcase_index loads each package's own item per ?package= (com.example.showcase vs com.objectstack.studio); layered REST endpoint disambiguates; objectql 602 / rest 108 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0856476 commit 7de2ebe

5 files changed

Lines changed: 96 additions & 22 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
"@objectstack/objectql": patch
3+
"@objectstack/rest": patch
4+
---
5+
6+
fix(metadata): package-scope the layered (Studio editor) read via `?package=` (ADR-0048)
7+
8+
The `?layers=true` single-item read (the Studio metadata editor's 3-state
9+
code/overlay/effective view) ignored `packageId`, so editing one of two
10+
same-named items from different packages resolved ambiguously (first match).
11+
12+
- `protocol.getMetaItemLayered` now threads `packageId` into the code layer
13+
(`metadataService.get` + `lookupArtifactItem` + `registry.getItem`) and the
14+
`sys_metadata` overlay query (`package_id` prefer-local).
15+
- `registry.getArtifactItem(type, name, currentPackageId?)` and
16+
`lookupArtifactItem` gained the optional package-scope hint.
17+
- `rest-server` threads `?package=` into the layered branch.
18+
19+
This completes the per-route package-scoped resolution audit: the runtime
20+
render surface (dashboard/report/page/doc) was already scoped; this closes the
21+
Studio editor (`/apps/:appName/metadata/:type/:name`). Frontend counterpart
22+
sends `?package=` from the metadata list row's owning package.

packages/objectql/src/protocol-layered-get.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,36 @@ describe('ObjectStackProtocolImplementation - getMetaItemLayered', () => {
9595
expect(result.overlay).toBeNull();
9696
expect(result.effective).toBeNull();
9797
});
98+
99+
it('scopes the code layer to the requested package on a same-name collision (ADR-0048)', async () => {
100+
// Two packages each ship view/clash; the registry stores them under
101+
// composite keys. The layered (Studio editor) read must resolve the
102+
// code baseline owned by the requested package.
103+
registry.registerItem('view', { name: 'clash', type: 'grid', object: 'task', label: 'Alpha view' }, 'name', 'com.acme.alpha');
104+
registry.registerItem('view', { name: 'clash', type: 'grid', object: 'task', label: 'Beta view' }, 'name', 'com.acme.beta');
105+
106+
const alpha = await protocol.getMetaItemLayered({ type: 'view', name: 'clash', packageId: 'com.acme.alpha' });
107+
const beta = await protocol.getMetaItemLayered({ type: 'view', name: 'clash', packageId: 'com.acme.beta' });
108+
109+
expect((alpha.code as any)?.label).toBe('Alpha view');
110+
expect((beta.code as any)?.label).toBe('Beta view');
111+
});
112+
113+
it('scopes the overlay query to the requested package (ADR-0048)', async () => {
114+
const rows: Record<string, any> = {
115+
'com.acme.alpha': { metadata: { name: 'clash', label: 'Alpha overlay' } },
116+
'com.acme.beta': { metadata: { name: 'clash', label: 'Beta overlay' } },
117+
};
118+
// The package-scoped query carries package_id; the package-less
119+
// fallback must miss (null) so the scoped row is the only hit.
120+
mockEngine.findOne.mockImplementation((_table: string, opts: any) =>
121+
Promise.resolve(opts.where?.package_id ? (rows[opts.where.package_id] ?? null) : null),
122+
);
123+
124+
const alpha = await protocol.getMetaItemLayered({ type: 'view', name: 'clash', packageId: 'com.acme.alpha' });
125+
const beta = await protocol.getMetaItemLayered({ type: 'view', name: 'clash', packageId: 'com.acme.beta' });
126+
127+
expect((alpha.overlay as any)?.label).toBe('Alpha overlay');
128+
expect((beta.overlay as any)?.label).toBe('Beta overlay');
129+
});
98130
});

packages/objectql/src/protocol.ts

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1698,10 +1698,12 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
16981698
const services = this.getServicesRegistry?.();
16991699
const metadataService = services?.get('metadata');
17001700
if (metadataService && typeof metadataService.get === 'function') {
1701-
let fromService = await metadataService.get(request.type, request.name);
1701+
// ADR-0048 — package-scope the code layer so a same-name
1702+
// collision resolves to the requested package's artifact.
1703+
let fromService = await metadataService.get(request.type, request.name, request.packageId);
17021704
if (fromService === undefined || fromService === null) {
17031705
const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type];
1704-
if (alt) fromService = await metadataService.get(alt, request.name);
1706+
if (alt) fromService = await metadataService.get(alt, request.name, request.packageId);
17051707
}
17061708
if (fromService !== undefined && fromService !== null) code = fromService;
17071709
}
@@ -1712,11 +1714,11 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
17121714
// Prefer the artifact-only lookup so an overlay row hydrated
17131715
// into the registry's plain key can't masquerade as the "code
17141716
// default" layer; fall back to getItem for runtime-only items.
1715-
let regItem = this.lookupArtifactItem(request.type, request.name)
1716-
?? this.engine.registry.getItem(request.type, request.name);
1717+
let regItem = this.lookupArtifactItem(request.type, request.name, request.packageId)
1718+
?? this.engine.registry.getItem(request.type, request.name, request.packageId);
17171719
if (regItem === undefined) {
17181720
const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type];
1719-
if (alt) regItem = this.engine.registry.getItem(alt, request.name);
1721+
if (alt) regItem = this.engine.registry.getItem(alt, request.name, request.packageId);
17201722
}
17211723
if (regItem !== undefined) code = regItem;
17221724
}
@@ -1726,20 +1728,24 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
17261728
let overlayScope: 'org' | 'env' | null = null;
17271729
try {
17281730
const findOverlay = async (oid: string | null) => {
1729-
const where: Record<string, unknown> = {
1730-
type: request.type,
1731-
name: request.name,
1732-
state: 'active',
1733-
organization_id: oid,
1731+
// ADR-0048 prefer-local: when a package is supplied, the row
1732+
// owned by that package wins over a package-less first match.
1733+
const lookup = async (t: string) => {
1734+
const base: Record<string, unknown> = {
1735+
type: t, name: request.name, state: 'active', organization_id: oid,
1736+
};
1737+
if (request.packageId) {
1738+
const scoped = await this.engine.findOne('sys_metadata', {
1739+
where: { ...base, package_id: request.packageId },
1740+
});
1741+
if (scoped) return scoped;
1742+
}
1743+
return await this.engine.findOne('sys_metadata', { where: base });
17341744
};
1735-
let rec = await this.engine.findOne('sys_metadata', { where });
1745+
let rec = await lookup(request.type);
17361746
if (!rec) {
17371747
const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type];
1738-
if (alt) {
1739-
rec = await this.engine.findOne('sys_metadata', {
1740-
where: { ...where, type: alt },
1741-
});
1742-
}
1748+
if (alt) rec = await lookup(alt);
17431749
}
17441750
return rec;
17451751
};
@@ -3143,7 +3149,7 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
31433149
* type and its singular/plural twin. Returns `undefined` when the
31443150
* registry is unavailable or the item is not artifact-backed.
31453151
*/
3146-
private lookupArtifactItem(type: string, name: string): unknown {
3152+
private lookupArtifactItem(type: string, name: string, currentPackageId?: string): unknown {
31473153
const registry = (this.engine as any)?.registry;
31483154
if (!registry) return undefined;
31493155
const singular = PLURAL_TO_SINGULAR[type] ?? type;
@@ -3152,15 +3158,16 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
31523158
// into the plain key (getMetaItems / loadMetaFromDb) can never
31533159
// shadow the packaged artifact's protection envelope (ADR-0010
31543160
// §3.3 — pre-fix, that shadow made a `_lock: full` app read back
3155-
// as unlocked after PUT+GET until restart).
3161+
// as unlocked after PUT+GET until restart). `currentPackageId`
3162+
// (ADR-0048) makes that scan package-scoped (prefer-local).
31563163
if (typeof registry.getArtifactItem === 'function') {
3157-
return registry.getArtifactItem(singular, name)
3158-
?? registry.getArtifactItem(type, name);
3164+
return registry.getArtifactItem(singular, name, currentPackageId)
3165+
?? registry.getArtifactItem(type, name, currentPackageId);
31593166
}
31603167
// Partial registry mocks in tests — fall back to getItem and apply
31613168
// the same package-provenance filter inline.
31623169
if (typeof registry.getItem !== 'function') return undefined;
3163-
const item = registry.getItem(singular, name) ?? registry.getItem(type, name);
3170+
const item = registry.getItem(singular, name, currentPackageId) ?? registry.getItem(type, name, currentPackageId);
31643171
if (!item || !(item as any)._packageId || (item as any)._packageId === 'sys_metadata') {
31653172
return undefined;
31663173
}

packages/objectql/src/registry.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -965,7 +965,7 @@ export class SchemaRegistry {
965965
* it (that masking is exactly the "registry pollution" bug where a
966966
* locked app's `_lock` read back as undefined after a PUT+GET).
967967
*/
968-
getArtifactItem<T>(type: string, name: string): T | undefined {
968+
getArtifactItem<T>(type: string, name: string, currentPackageId?: string): T | undefined {
969969
if (type === 'object' || type === 'objects') {
970970
const obj = this.getObject(name) as any;
971971
return obj && obj._packageId && obj._packageId !== 'sys_metadata'
@@ -974,6 +974,14 @@ export class SchemaRegistry {
974974
}
975975
const collection = this.metadata.get(type);
976976
if (!collection) return undefined;
977+
// ADR-0048 prefer-local: when the caller resolves within a package, the
978+
// artifact owned by that package wins over a first-match composite scan,
979+
// so two installed packages shipping the same name don't resolve by Map
980+
// iteration order.
981+
if (currentPackageId) {
982+
const local = collection.get(`${currentPackageId}:${name}`) as any;
983+
if (local && local._packageId && local._packageId !== 'sys_metadata') return local as T;
984+
}
977985
for (const [key, item] of collection) {
978986
if (key !== name && key.endsWith(`:${name}`)) {
979987
const it = item as any;

packages/rest/src/rest-server.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1917,9 +1917,14 @@ export class RestServer {
19171917
// diagnostic endpoint, not on the hot read path.
19181918
const wantLayered = req.query?.layers !== undefined && req.query?.layers !== '';
19191919
if (wantLayered && typeof (p as any).getMetaItemLayered === 'function') {
1920+
// ADR-0048 — thread `?package=` so the layered (Studio
1921+
// editor) view is package-scoped; the editor passes the
1922+
// edited item's owning package, not the studio app's.
1923+
const layeredPackageId = req.query?.package || undefined;
19201924
const layered = await (p as any).getMetaItemLayered({
19211925
type: req.params.type,
19221926
name: req.params.name,
1927+
...(layeredPackageId ? { packageId: layeredPackageId } : {}),
19231928
...(environmentId ? { environmentId } : {}),
19241929
});
19251930
res.json(layered);

0 commit comments

Comments
 (0)