From ff195fafdcebcf38ec2ae67abd182e413b334a14 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 21:26:40 +0000 Subject: [PATCH 1/2] fix(metadata-protocol): duplicatePackage refuses an item whose renamed reference meets a name it already holds WIP: the collision detection in deepRewrite and the failed[] call site. Claude-Session: https://claude.ai/code/session_01YRp5z4kK6vgYkrdH97ztsy Co-authored-by: Claude --- packages/metadata-protocol/src/protocol.ts | 76 ++++++++++++++++++++-- 1 file changed, 72 insertions(+), 4 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 1c6a4accdb3..73a5c5e26cf 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -26327,6 +26327,11 @@ export class ObjectStackProtocolImplementation implements * A copied permission set's field rules follow too: in `fields`, keyed * `.`, a key's object part takes the renamed object's name * and its field part is left as stored. + * + * A rename at a reference site that meets a name the item already holds + * there (a permission set's or a translation's `objects` key, a hook's + * `object` entry) is never merged: the item is refused in `failed[]`, + * naming both, before anything is written for it. */ async duplicatePackage(request: { sourcePackageId: string; @@ -26550,26 +26555,75 @@ export class ObjectStackProtocolImplementation implements // KEY (`permission.objects` is keyed by object name) or a `.name`. The // record key holds for prefixed names too: a copied permission set // otherwise kept `objects.`, a grant on the SOURCE. + // + // ⛔ REFUSED, NEVER MERGED: at a reference site, a renamed name can + // meet a name the item already holds there — the source also names + // the object the copy is about to be called (`dupdst_customer`, an + // object outside the package or not yet created) beside its own + // (`dupsrc_customer`). Two record KEYS that land on one key keep only + // the last entry: a permission set's `objects` dropped a grant (and + // the copied object could gain edit and delete its source never gave + // it), a translation's `objects` dropped a label. Two array ELEMENTS + // that land on one name bind it twice: a hook's handler ran twice per + // event on the copied object. So every such meeting is recorded in + // `collisions` — the path, the name that moved, the name it met — and + // the call site refuses the item in `failed[]` before anything is + // written for it. `renameMap` is one-to-one, so a meeting is always a + // renamed name and a name this copy does not rename; an element the + // source itself repeats is copied as stored. const objectReferenceProperties = new Set( (REFERENCE_SITES.byTarget.get('object') ?? []).map((site) => site.property), ); - const deepRewrite = (v: any, atReference = false): any => { + type ReferenceCollision = { at: string; shape: 'key' | 'entry'; moved: string; met: string }; + const deepRewrite = (v: any, collisions: ReferenceCollision[], atReference = false, at = ''): any => { if (typeof v === 'string') { const whole = atReference ? renameMap.get(v) : undefined; if (whole !== undefined) return whole; return re ? v.replace(re, (m) => renameMap.get(m) ?? m) : v; } - if (Array.isArray(v)) return v.map((element) => deepRewrite(element, atReference)); + if (Array.isArray(v)) { + const out = v.map((element, i) => deepRewrite(element, collisions, atReference, `${at}[${i}]`)); + if (atReference) { + const firstAt = new Map(); + out.forEach((element, i) => { + if (typeof element !== 'string') return; + const j = firstAt.get(element); + if (j === undefined) { + firstAt.set(element, i); + } else if (v[j] !== v[i]) { + collisions.push({ at, shape: 'entry', moved: v[i] !== element ? v[i] : v[j], met: element }); + } + }); + } + return out; + } if (v && typeof v === 'object') { const o: any = {}; + const from = new Map(); for (const [k, val] of Object.entries(v)) { const key = atReference ? (renameMap.get(k) ?? k) : k; - o[key] = deepRewrite(val, objectReferenceProperties.has(k) || (atReference && k === 'name')); + const other = from.get(key); + if (other !== undefined) { + collisions.push({ at, shape: 'key', moved: key === k ? other : k, met: key }); + } + from.set(key, k); + o[key] = deepRewrite( + val, collisions, + objectReferenceProperties.has(k) || (atReference && k === 'name'), + at ? `${at}.${k}` : k, + ); } return o; } return v; }; + const describeCollision = (c: ReferenceCollision): string => + `its \`${c.at}\` ${c.shape} '${c.moved}' follows the copied object to '${c.met}', ` + + `${c.shape === 'key' ? 'a key' : 'an entry'} it already holds there, which this copy does not rename. ` + + (c.shape === 'key' + ? 'A record keeps one entry per key, so the copy would drop one of the two. ' + : `The copy would name '${c.met}' twice. `) + + `Remove '${c.moved}' or '${c.met}' on the source, then duplicate again.`; // ⭐ A COPIED PERMISSION SET'S FIELD RULES FOLLOW THE RENAMED OBJECTS. // `PermissionSetSchema.fields` is a record keyed `.` // (`packages/spec/src/security/permission.zod.ts`), and the security @@ -26738,7 +26792,21 @@ export class ObjectStackProtocolImplementation implements continue; } } - const rewritten = deepRewrite(item); + const collisions: ReferenceCollision[] = []; + const rewritten = deepRewrite(item, collisions); + if (collisions.length > 0) { + const noun = singular === 'permission' + ? 'Permission set' + : `${singular.charAt(0).toUpperCase()}${singular.slice(1)}`; + const more = collisions.length - 1; + failed.push({ + type: row.type, + name: row.name, + error: `${noun} '${row.name}' is not copied: ${describeCollision(collisions[0]!)}` + + (more > 0 ? ` It has ${more} more such collision${more === 1 ? '' : 's'}.` : ''), + }); + continue; + } if (rewritten && typeof rewritten === 'object' && !Array.isArray(rewritten)) { rewritten.name = newName; // [ADR-0086 D3] A permission set that declares its SOURCE From 1b8e8e13bdf223390c4601214ba6c3086574ff3b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 21:29:13 +0000 Subject: [PATCH 2/2] test(objectql): pin duplicatePackage's refusal of a renamed reference that meets a held name; changeset Claude-Session: https://claude.ai/code/session_01YRp5z4kK6vgYkrdH97ztsy Co-authored-by: Claude --- ...7-duplicate-package-reference-collision.md | 15 ++ ...cate-package-objects-key-collision.test.ts | 248 ++++++++++++++++++ 2 files changed, 263 insertions(+) create mode 100644 .changeset/23537-duplicate-package-reference-collision.md create mode 100644 packages/objectql/src/protocol-duplicate-package-objects-key-collision.test.ts diff --git a/.changeset/23537-duplicate-package-reference-collision.md b/.changeset/23537-duplicate-package-reference-collision.md new file mode 100644 index 00000000000..2ac997b548e --- /dev/null +++ b/.changeset/23537-duplicate-package-reference-collision.md @@ -0,0 +1,15 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +fix(metadata-protocol): `duplicatePackage` refuses a copied item whose renamed object reference meets a name the item already holds, instead of merging the two + +Clause-②: no + +`duplicatePackage` (ADR-0070 D4, the package duplicate route's method) renames every copied object into the target namespace and rewrites each reference to it where a property names an object: a string, an array entry, or a record key (a permission set's `objects`, a translation's `objects`). + +**FROM.** When a copied item also named the object the copy is about to be called, the renamed name landed on a name the item already held, and the copy merged the two with `failed: []`. A permission set holding `objects: { crm_customer: { allowRead: true }, crm2_customer: { allowRead: true, allowEdit: true, allowDelete: true } }`, duplicated into namespace `crm2`, was copied as `objects: { crm2_customer: { allowRead: true, allowEdit: true, allowDelete: true } }`: whichever entry came last was kept, so one grant was dropped, and here the copied object gained edit and delete the source set never granted on its own object. A translation's `objects` dropped one object's labels the same way. A hook whose `object` list held both names was copied as `['crm2_customer', 'crm2_customer']`, so its handler ran twice per event on the copied object. + +**TO.** Such an item is listed in `failed[]`, and nothing of it is written (no item row, no history entry). The entry names the item, the property, the name that moved and the name it met, and the source edit that clears it, for example: "Permission set 'crm_access' is not copied: its `objects` key 'crm_customer' follows the copied object to 'crm2_customer', a key it already holds there, which this copy does not rename. … Remove 'crm_customer' or 'crm2_customer' on the source, then duplicate again." Remove one of the two names on the source item, then duplicate again. + +**Unchanged.** An item with no such collision copies as before, its references following the copied objects. An entry the source item itself repeats is copied as stored. The source package is never modified. diff --git a/packages/objectql/src/protocol-duplicate-package-objects-key-collision.test.ts b/packages/objectql/src/protocol-duplicate-package-objects-key-collision.test.ts new file mode 100644 index 00000000000..8054b108065 --- /dev/null +++ b/packages/objectql/src/protocol-duplicate-package-objects-key-collision.test.ts @@ -0,0 +1,248 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0070 D4 — `duplicatePackage` never merges a renamed reference into a + * name the copied item already holds there. + * + * ## The defect + * + * At an object-reference property, `duplicatePackage` renames a name whole — + * a string, an array element, a record KEY. When the source names the object + * the copy is about to be called (`dupdst_customer`, not yet an object) beside + * the package's own (`dupsrc_customer`), the rename lands on a name the item + * already holds: + * + * - a permission set's `objects` kept whichever entry came last, so one grant + * was dropped with `failed: []`, and the copied object could gain edit and + * delete the source set never granted on its own object; + * - a translation's `objects` dropped one object's labels the same way; + * - a hook's `object` list named the copied object twice, so its handler ran + * twice per event there. + * + * Now the item is refused in `failed[]`, naming the path, both names and the + * source edit that clears it, and nothing of it is written. + * + * ## Why a real kernel + * + * `ObjectQLPlugin` binds the real protocol to the real engine and registry, so + * every pin reads the stored rows out of the driver, never a payload. + */ + +import { randomUUID } from 'node:crypto'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { ObjectKernel } from '@objectstack/core'; +import { ObjectQL } from './engine.js'; +import { ObjectQLPlugin } from './plugin.js'; + +type Stores = Map>>; + +/** In-memory driver, copied from `protocol-duplicate-package-permission-field-keys.test.ts`; equality-only WHERE. */ +function makeMemoryDriver(stores: Stores) { + const storeFor = (obj: string) => { + let s = stores.get(obj); + if (!s) { s = new Map(); stores.set(obj, s); } + return s; + }; + // `$and` / `$or` are conjoined WITH their sibling keys, the way a real + // driver ANDs them. + const matchesWhere = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where)) { + if (k === '$and' && Array.isArray(v)) { + if (!v.every((w: any) => matchesWhere(row, w))) return false; + continue; + } + if (k === '$or' && Array.isArray(v)) { + if (!v.some((w: any) => matchesWhere(row, w))) return false; + continue; + } + if (k.startsWith('$')) continue; + const rowVal = row[k]; + const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v; + const a = rowVal === undefined ? null : rowVal; + const b = expected === undefined ? null : expected; + if (a !== b) return false; + } + return true; + }; + const driver: any = { + name: 'memory', version: '0.0.0', supports: {} as any, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, + async find(object: string, ast: any) { + const rows = Array.from(storeFor(object).values()).filter((r) => matchesWhere(r, ast?.where)); + // The caller's bound, after the filter, by presence (`check:objectql-double-limit`). + return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; + }, + async findOne(object: string, ast: any) { + for (const r of storeFor(object).values()) if (matchesWhere(r, ast?.where)) return r; + return null; + }, + async create(object: string, data: Record) { + const id = (data.id as string) ?? `r_${randomUUID()}`; + const row = { ...data, id }; + storeFor(object).set(id, row); + return row; + }, + async update(object: string, id: string, data: Record) { + const s = storeFor(object); + const cur = s.get(id); + if (!cur) throw new Error(`not found: ${object}/${id}`); + const updated = { ...cur, ...data, id }; + s.set(id, updated); + return updated; + }, + async upsert(object: string, data: Record) { + const id = data.id as string | undefined; + if (id && storeFor(object).has(id)) return this.update(object, id, data); + return this.create(object, data); + }, + async delete(object: string, id: string) { return storeFor(object).delete(id); }, + async count(object: string, ast: any) { return (await this.find(object, ast)).length; }, + async bulkCreate(object: string, rows: Record[]) { + return Promise.all(rows.map((r) => this.create(object, r))); + }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return driver; +} + +/** Source package; `installPackage` derives its namespace `dupsrc`. */ +const P = 'com.qa.dupsrc'; +/** Target package; namespace `dupdst`, derived the same way. */ +const Q = 'com.qa.dupdst'; + +const NARROW = { allowRead: true }; +const WIDE = { allowRead: true, allowEdit: true, allowDelete: true }; + +const customerBody = { + name: 'dupsrc_customer', label: 'Customer', sharingModel: 'private', + fields: { full_name: { name: 'full_name', type: 'text', label: 'Full name' } }, +}; + +describe('ADR-0070 D4 — duplicatePackage refuses a renamed reference that meets a name the item holds', () => { + let kernel: ObjectKernel; + let stores: Stores; + let engine: ObjectQL; + let protocol: any; + + beforeEach(async () => { + // `logger.level` is the real config key, and gracefulShutdown:false keeps + // process signal handlers out of vitest's fork workers (#4250). + kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false }); + stores = new Map(); + engine = new ObjectQL(); + engine.registerDriver(makeMemoryDriver(stores), true); + await kernel.use(new ObjectQLPlugin({ ql: engine })); + await kernel.bootstrap(); + protocol = kernel.getService('protocol'); + }); + afterEach(async () => { + if (kernel.getState() === 'running') await kernel.shutdown(); + }); + + const storedBody = (type: string, name: string, packageId: string): any => { + const row = Array.from(stores.get('sys_metadata')?.values() ?? []) + .find((r) => r.type === type && r.name === name && r.package_id === packageId); + if (!row) return undefined; + return typeof row.metadata === 'string' ? JSON.parse(row.metadata) : row.metadata; + }; + /** Every table the copy writes (the item, its history, commit and audit rows) that mentions `name`. */ + const tablesMentioning = (name: string): string[] => + Array.from(stores.entries()) + .filter(([, rows]) => Array.from(rows.values()).some((row) => JSON.stringify(row).includes(`"${name}"`))) + .map(([table]) => table) + .sort(); + + async function save(type: string, item: { name: string }): Promise { + const res = await protocol.saveMetaItem({ type, name: item.name, packageId: P, item }); + expect(res.success).toBe(true); + } + async function seed(type: string, item: { name: string }): Promise { + await protocol.installPackage({ manifest: { id: P, name: 'Source', type: 'application' } }); + expect(engine.registry.getPackage(P)?.manifest?.namespace).toBe('dupsrc'); + await save('object', customerBody); + await save(type, item); + } + + for (const [order, objects] of [ + ['the renamed key first', { dupsrc_customer: NARROW, dupdst_customer: WIDE }], + ['the key it meets first', { dupdst_customer: WIDE, dupsrc_customer: NARROW }], + ] as const) { + it(`a permission set whose renamed \`objects\` key meets one it holds is refused, never merged (${order})`, async () => { + await seed('permission', { name: 'dupsrc_objs', label: 'Objects', objects } as { name: string }); + + const res = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: Q }); + + expect(res.failed).toHaveLength(1); + const [refusal] = res.failed; + expect(refusal).toMatchObject({ type: 'permission', name: 'dupsrc_objs' }); + // Located: the set, the property, the key that moved, the key it met, and the remedy. + expect(refusal.error).toContain("Permission set 'dupsrc_objs' is not copied"); + expect(refusal.error).toContain("`objects` key 'dupsrc_customer' follows the copied object to 'dupdst_customer'"); + expect(refusal.error).toContain("Remove 'dupsrc_customer' or 'dupdst_customer' on the source, then duplicate again."); + // Nothing of the set was written: no item row, no history, commit or audit row. + expect(storedBody('permission', 'dupdst_objs', Q)).toBeUndefined(); + expect(tablesMentioning('dupdst_objs')).toEqual([]); + // NEGATIVE CONTROL: the object still copied, and the scan finds what a copy writes. + expect(res.copied).toEqual([{ type: 'object', name: 'dupdst_customer' }]); + expect(tablesMentioning('dupdst_customer')).toEqual(expect.arrayContaining(['sys_metadata', 'sys_metadata_history'])); + // The source set is untouched. + expect(storedBody('permission', 'dupsrc_objs', P).objects).toEqual(objects); + }); + } + + it('NEGATIVE CONTROL: the same set without the second key copies, and its one grant follows the renamed object', async () => { + await seed('permission', { name: 'dupsrc_objs', label: 'Objects', objects: { dupsrc_customer: NARROW } } as { name: string }); + + const res = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: Q }); + + expect(res.failed).toEqual([]); + expect(res.copied).toEqual(expect.arrayContaining([ + { type: 'object', name: 'dupdst_customer' }, + { type: 'permission', name: 'dupdst_objs' }, + ])); + expect(storedBody('permission', 'dupdst_objs', Q).objects).toEqual({ dupdst_customer: NARROW }); + expect(storedBody('permission', 'dupsrc_objs', P).objects).toEqual({ dupsrc_customer: NARROW }); + }); + + it('a translation whose renamed `objects` key meets one it holds is refused, never merged', async () => { + const objects = { dupsrc_customer: { label: 'Customer A' }, dupdst_customer: { label: 'Customer B' } }; + await seed('translation', { name: 'dupsrc_zh', locale: 'zh-CN', objects } as { name: string }); + + const res = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: Q }); + + expect(res.failed).toHaveLength(1); + expect(res.failed[0]).toMatchObject({ type: 'translation', name: 'dupsrc_zh' }); + expect(res.failed[0].error).toContain("Translation 'dupsrc_zh' is not copied"); + expect(res.failed[0].error).toContain("`objects` key 'dupsrc_customer' follows the copied object to 'dupdst_customer'"); + expect(tablesMentioning('dupdst_zh')).toEqual([]); + // NEGATIVE CONTROL: the object still copied; the source is untouched. + expect(res.copied).toEqual([{ type: 'object', name: 'dupdst_customer' }]); + expect(storedBody('translation', 'dupsrc_zh', P).objects).toEqual(objects); + }); + + it('a hook whose renamed `object` entry meets one it holds is refused; without the second entry it copies', async () => { + const hook = (object: string[]) => ({ + name: 'dupsrc_stamp', object, events: ['beforeInsert'], + body: { language: 'js', source: "ctx.input.full_name = 'stamped';" }, + }); + await seed('hook', hook(['dupsrc_customer', 'dupdst_customer'])); + + const res = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: Q }); + + expect(res.failed).toHaveLength(1); + expect(res.failed[0]).toMatchObject({ type: 'hook', name: 'dupsrc_stamp' }); + expect(res.failed[0].error).toContain("Hook 'dupsrc_stamp' is not copied"); + expect(res.failed[0].error).toContain("`object` entry 'dupsrc_customer' follows the copied object to 'dupdst_customer'"); + expect(tablesMentioning('dupdst_stamp')).toEqual([]); + + // NEGATIVE CONTROL: the same hook without the second entry copies, and its entry follows the renamed object. + await save('hook', hook(['dupsrc_customer'])); + const again = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: 'com.qa.dupdst2' }); + expect(again.failed).toEqual([]); + expect(storedBody('hook', 'dupdst2_stamp', 'com.qa.dupdst2').object).toEqual(['dupdst2_customer']); + }); +});