diff --git a/.changeset/adr-0048-studio-editor-package-scope.md b/.changeset/adr-0048-studio-editor-package-scope.md new file mode 100644 index 0000000000..7b42231875 --- /dev/null +++ b/.changeset/adr-0048-studio-editor-package-scope.md @@ -0,0 +1,22 @@ +--- +"@objectstack/objectql": patch +"@objectstack/rest": patch +--- + +fix(metadata): package-scope the layered (Studio editor) read via `?package=` (ADR-0048) + +The `?layers=true` single-item read (the Studio metadata editor's 3-state +code/overlay/effective view) ignored `packageId`, so editing one of two +same-named items from different packages resolved ambiguously (first match). + +- `protocol.getMetaItemLayered` now threads `packageId` into the code layer + (`metadataService.get` + `lookupArtifactItem` + `registry.getItem`) and the + `sys_metadata` overlay query (`package_id` prefer-local). +- `registry.getArtifactItem(type, name, currentPackageId?)` and + `lookupArtifactItem` gained the optional package-scope hint. +- `rest-server` threads `?package=` into the layered branch. + +This completes the per-route package-scoped resolution audit: the runtime +render surface (dashboard/report/page/doc) was already scoped; this closes the +Studio editor (`/apps/:appName/metadata/:type/:name`). Frontend counterpart +sends `?package=` from the metadata list row's owning package. diff --git a/packages/objectql/src/protocol-layered-get.test.ts b/packages/objectql/src/protocol-layered-get.test.ts index 3d5a67bfe2..0e74d22cab 100644 --- a/packages/objectql/src/protocol-layered-get.test.ts +++ b/packages/objectql/src/protocol-layered-get.test.ts @@ -95,4 +95,36 @@ describe('ObjectStackProtocolImplementation - getMetaItemLayered', () => { expect(result.overlay).toBeNull(); expect(result.effective).toBeNull(); }); + + it('scopes the code layer to the requested package on a same-name collision (ADR-0048)', async () => { + // Two packages each ship view/clash; the registry stores them under + // composite keys. The layered (Studio editor) read must resolve the + // code baseline owned by the requested package. + registry.registerItem('view', { name: 'clash', type: 'grid', object: 'task', label: 'Alpha view' }, 'name', 'com.acme.alpha'); + registry.registerItem('view', { name: 'clash', type: 'grid', object: 'task', label: 'Beta view' }, 'name', 'com.acme.beta'); + + const alpha = await protocol.getMetaItemLayered({ type: 'view', name: 'clash', packageId: 'com.acme.alpha' }); + const beta = await protocol.getMetaItemLayered({ type: 'view', name: 'clash', packageId: 'com.acme.beta' }); + + expect((alpha.code as any)?.label).toBe('Alpha view'); + expect((beta.code as any)?.label).toBe('Beta view'); + }); + + it('scopes the overlay query to the requested package (ADR-0048)', async () => { + const rows: Record = { + 'com.acme.alpha': { metadata: { name: 'clash', label: 'Alpha overlay' } }, + 'com.acme.beta': { metadata: { name: 'clash', label: 'Beta overlay' } }, + }; + // The package-scoped query carries package_id; the package-less + // fallback must miss (null) so the scoped row is the only hit. + mockEngine.findOne.mockImplementation((_table: string, opts: any) => + Promise.resolve(opts.where?.package_id ? (rows[opts.where.package_id] ?? null) : null), + ); + + const alpha = await protocol.getMetaItemLayered({ type: 'view', name: 'clash', packageId: 'com.acme.alpha' }); + const beta = await protocol.getMetaItemLayered({ type: 'view', name: 'clash', packageId: 'com.acme.beta' }); + + expect((alpha.overlay as any)?.label).toBe('Alpha overlay'); + expect((beta.overlay as any)?.label).toBe('Beta overlay'); + }); }); diff --git a/packages/objectql/src/protocol.ts b/packages/objectql/src/protocol.ts index 07caf2a9d4..65ba2f5d60 100644 --- a/packages/objectql/src/protocol.ts +++ b/packages/objectql/src/protocol.ts @@ -1698,10 +1698,12 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { const services = this.getServicesRegistry?.(); const metadataService = services?.get('metadata'); if (metadataService && typeof metadataService.get === 'function') { - let fromService = await metadataService.get(request.type, request.name); + // ADR-0048 — package-scope the code layer so a same-name + // collision resolves to the requested package's artifact. + let fromService = await metadataService.get(request.type, request.name, request.packageId); if (fromService === undefined || fromService === null) { const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type]; - if (alt) fromService = await metadataService.get(alt, request.name); + if (alt) fromService = await metadataService.get(alt, request.name, request.packageId); } if (fromService !== undefined && fromService !== null) code = fromService; } @@ -1712,11 +1714,11 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { // Prefer the artifact-only lookup so an overlay row hydrated // into the registry's plain key can't masquerade as the "code // default" layer; fall back to getItem for runtime-only items. - let regItem = this.lookupArtifactItem(request.type, request.name) - ?? this.engine.registry.getItem(request.type, request.name); + let regItem = this.lookupArtifactItem(request.type, request.name, request.packageId) + ?? this.engine.registry.getItem(request.type, request.name, request.packageId); if (regItem === undefined) { const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type]; - if (alt) regItem = this.engine.registry.getItem(alt, request.name); + if (alt) regItem = this.engine.registry.getItem(alt, request.name, request.packageId); } if (regItem !== undefined) code = regItem; } @@ -1726,20 +1728,24 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { let overlayScope: 'org' | 'env' | null = null; try { const findOverlay = async (oid: string | null) => { - const where: Record = { - type: request.type, - name: request.name, - state: 'active', - organization_id: oid, + // ADR-0048 prefer-local: when a package is supplied, the row + // owned by that package wins over a package-less first match. + const lookup = async (t: string) => { + const base: Record = { + type: t, name: request.name, state: 'active', organization_id: oid, + }; + if (request.packageId) { + const scoped = await this.engine.findOne('sys_metadata', { + where: { ...base, package_id: request.packageId }, + }); + if (scoped) return scoped; + } + return await this.engine.findOne('sys_metadata', { where: base }); }; - let rec = await this.engine.findOne('sys_metadata', { where }); + let rec = await lookup(request.type); if (!rec) { const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type]; - if (alt) { - rec = await this.engine.findOne('sys_metadata', { - where: { ...where, type: alt }, - }); - } + if (alt) rec = await lookup(alt); } return rec; }; @@ -3143,7 +3149,7 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { * type and its singular/plural twin. Returns `undefined` when the * registry is unavailable or the item is not artifact-backed. */ - private lookupArtifactItem(type: string, name: string): unknown { + private lookupArtifactItem(type: string, name: string, currentPackageId?: string): unknown { const registry = (this.engine as any)?.registry; if (!registry) return undefined; const singular = PLURAL_TO_SINGULAR[type] ?? type; @@ -3152,15 +3158,16 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { // into the plain key (getMetaItems / loadMetaFromDb) can never // shadow the packaged artifact's protection envelope (ADR-0010 // §3.3 — pre-fix, that shadow made a `_lock: full` app read back - // as unlocked after PUT+GET until restart). + // as unlocked after PUT+GET until restart). `currentPackageId` + // (ADR-0048) makes that scan package-scoped (prefer-local). if (typeof registry.getArtifactItem === 'function') { - return registry.getArtifactItem(singular, name) - ?? registry.getArtifactItem(type, name); + return registry.getArtifactItem(singular, name, currentPackageId) + ?? registry.getArtifactItem(type, name, currentPackageId); } // Partial registry mocks in tests — fall back to getItem and apply // the same package-provenance filter inline. if (typeof registry.getItem !== 'function') return undefined; - const item = registry.getItem(singular, name) ?? registry.getItem(type, name); + const item = registry.getItem(singular, name, currentPackageId) ?? registry.getItem(type, name, currentPackageId); if (!item || !(item as any)._packageId || (item as any)._packageId === 'sys_metadata') { return undefined; } diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index 1431b8077d..6201392a4e 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -965,7 +965,7 @@ export class SchemaRegistry { * it (that masking is exactly the "registry pollution" bug where a * locked app's `_lock` read back as undefined after a PUT+GET). */ - getArtifactItem(type: string, name: string): T | undefined { + getArtifactItem(type: string, name: string, currentPackageId?: string): T | undefined { if (type === 'object' || type === 'objects') { const obj = this.getObject(name) as any; return obj && obj._packageId && obj._packageId !== 'sys_metadata' @@ -974,6 +974,14 @@ export class SchemaRegistry { } const collection = this.metadata.get(type); if (!collection) return undefined; + // ADR-0048 prefer-local: when the caller resolves within a package, the + // artifact owned by that package wins over a first-match composite scan, + // so two installed packages shipping the same name don't resolve by Map + // iteration order. + if (currentPackageId) { + const local = collection.get(`${currentPackageId}:${name}`) as any; + if (local && local._packageId && local._packageId !== 'sys_metadata') return local as T; + } for (const [key, item] of collection) { if (key !== name && key.endsWith(`:${name}`)) { const it = item as any; diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index c2eb1b6d41..6f316bc8e1 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -1917,9 +1917,14 @@ export class RestServer { // diagnostic endpoint, not on the hot read path. const wantLayered = req.query?.layers !== undefined && req.query?.layers !== ''; if (wantLayered && typeof (p as any).getMetaItemLayered === 'function') { + // ADR-0048 — thread `?package=` so the layered (Studio + // editor) view is package-scoped; the editor passes the + // edited item's owning package, not the studio app's. + const layeredPackageId = req.query?.package || undefined; const layered = await (p as any).getMetaItemLayered({ type: req.params.type, name: req.params.name, + ...(layeredPackageId ? { packageId: layeredPackageId } : {}), ...(environmentId ? { environmentId } : {}), }); res.json(layered);