Skip to content

Commit e67ac4b

Browse files
fix(metadata-protocol): duplicatePackage renames a name that is also a field key only at object-reference sites (#24475)
Fixes #23561 Clause-②: no ## What this does `duplicatePackage` (`packages/metadata-protocol/src/protocol.ts`, ADR-0070 D4) renamed a token that is BOTH a prefixed package object's name AND a field key as if it were the object, in free text. A field keyed `dupsrc_customer` on the object `dupsrc_customer` kept its key on the copy, but a copied list view's `columns` and a copied form's `sections[].fields` were rewritten to `dupdst_customer`, a field the copy does not have. Since PR #23559 the list view was refused at the copy (`list-view-field-unknown`). The form copied silently, with a dangling field. Triage ruling (comment 6181968665), direction B: "Rename such a name only at object-reference sites, never in free text. That extends the landed bare-name policy". C (also renaming the field) is ruled out. The mechanism: before the free-text regex is built, `duplicatePackage` collects the `fields` record keys of every object row in the source package. A prefixed renamed object name that is also one of those keys is left out of the free-text regex. Such a name is then rewritten only by the existing object-reference path (`REFERENCE_SITES` target `object`: `object`, `reference`, `objectName`, a permission set's `objects` keys, and the rest), which is exactly how a bare object name is already handled. No second mechanism is added, and `reference-sites.ts` is not edited. - **Scope of "a field key": package-wide.** The set holds a field key on ANY object of the package, not only on the same-named object. For example, a lookup field `acme_customer` on `acme_order` raises the same ambiguity in that order's list view. Package-wide is the superset of the two readings. - **Consequence of the ruling, pinned:** free text (an object's `description`) naming such an object is now left as stored. A prefixed object that no field key spells is still renamed in free text, as a whole identifier, as before. - No accept/refuse change beyond the copy no longer refusing that list view. ## Pins (`packages/objectql/src/protocol-duplicate-package-names-in-step.test.ts`, real kernel: ObjectKernel + ObjectQLPlugin, memory driver) - **New case:** a name that is both a prefixed object and a field key is renamed only at object-reference properties. It asserts: - `res.failed` is empty (the list view is no longer refused) and both views are copied; - the list-view columns stay `['full_name', 'dupsrc_customer', 'x_dupsrc_customer']`; - the form's `sections[0].fields` stay `['full_name', 'dupsrc_customer']`; - the object references DO follow the copy: the view `object` and `config.data.object`, two lookup `reference`s, the action `objectName`, and the permission set's `objects` key; - the description is left as stored; - `engine.insert` through the copied field key lands. - **Flipped pin, load-bearing:** the landed case-2 assertion that the description reads `Linked from dupdst_customer` now reads the description unchanged, by the ruled semantics. The whole-identifier (left-boundary) rule it also guarded is re-pinned on a new prefixed object with no matching field key, `dupsrc_note`. That case expects `Attached to dupdst_note; see x_dupsrc_note.`, and the field `x_dupsrc_note` keeps both its key and its name. So the guarded surface does not shrink. - **Negative control:** `dupsrc_note`, an ordinary prefixed object, is still renamed in free text. **Ablation** (through `scripts/ablation-replace.mjs`). The anchor `renameMap.has(name) && !fieldKeys.has(name)` went from 1 hit to 0 and was replaced by a marker literal (blob `49c970959c61` to `1ea7b74ba948`). Then: rebuild `@objectstack/metadata-protocol`, and `ablation-dist-preflight` found the marker in 2 built files. The pin file then read `Tests 5 failed | 1 passed (6)`. That is MORE diagnostics than the 2 red cases predicted: the copied list view's `dupdst_customer` column is refused at the copy, so `failed[]` is non-empty and cases 1, 3 and 4 go red as well. Restore: blob == HEAD, `git diff HEAD` empty, rebuild, `--absent` preflight (marker absent from all 24 built files), then the pin file read `Tests 6 passed (6)`. ## Tests and gates (head `e7a7b375ed`, after merging `origin/main` `26501ae24f`) - Build: the `@objectstack/objectql` closure was rebuilt after the merge (VERDICT command-exit 0). - Five duplicate suites (`names-in-step`, `unprefixed-object`, `permission-field-keys`, `objects-key-collision`, `package-lifecycle`): `Test Files 5 passed (5)`, `Tests 30 passed (30)`. - `pnpm --filter @objectstack/metadata-protocol exec vitest run`: `Test Files 246 passed | 3 skipped (249)`, `Tests 20655 passed | 19 skipped`. - Typecheck: `pnpm --filter @objectstack/metadata-protocol typecheck` and `pnpm --filter @objectstack/objectql typecheck` both exited 0 (`check:test-typecheck: OK`). - ESLint, a proven narrowing to the 2 touched source files: - `--no-inline-config --format json` gives 2 files, 0 errors, 0 warnings. - The config never enables type-aware linting (`eslint.config.mjs`, no `parserOptions.project`), so this diff cannot move any untouched file's verdict. - `@objectstack/spec` `test:repo`: `Test Files 61 passed (61)`, `Tests 1033 passed (1033)`. This was measured at `efed56ba5b`, before the merge. This diff touches no `packages/spec` file. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 69 commands at `e7a7b375ed`. Every exit was captured before any pipe. `--ran` printed: `69 derived famil(ies) accounted for — 68 run, 1 NOT-MEASURED`. - The 68 that ran exited 0. - NOT MEASURED: `pnpm check:dual-build-cjs-loads`, exit 3, PREREQUISITE NOT MET. It needs every package's `dist/`, and a full build is outside this card's local scope. CI's `Lint & Repo Gates` runs it. ## Acceptance notes - **File surface:** the claimed `protocol.ts` region, the sibling pin file in `packages/objectql` (where the family's pins live, see PR #23559), and the changeset. `reference-sites.ts` needed no edit. - **Recorded only, as the card and triage say:** a view CONTAINER name with no dot (for example `ticket_views`) is still renamed by the prefix swap alone. It is not addressed here. - **Pull:** as the card measured, no field key equals an object name across the three example apps, so the repro is synthetic. Changeset: `.changeset/23561-duplicate-object-name-token.md`, `@objectstack/metadata-protocol` `patch`. Session `session_01D6F9axfoPzR7t8G1J7Ytzz`, os-dev for `domain:spec`. --- _Generated by [Claude Code](https://claude.ai/code/session_01D6F9axfoPzR7t8G1J7Ytzz)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4bd7215 commit e67ac4b

3 files changed

Lines changed: 89 additions & 7 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
`duplicatePackage` no longer renames, in free text, a name that is both a prefixed package object and a field key of one of the package's objects. Such a name (for example a field keyed `acme_deal` on the object `acme_deal`) may name the field, so it is now rewritten only at object-reference properties (`object`, `reference`, `objectName`, a permission set's `objects` keys, …), the way a bare object name already is. A copied list view's `columns` and a copied form's `sections[].fields` keep naming the field the copy has, instead of a field named after the renamed object: the list view is no longer refused at the copy with `list-view-field-unknown`, and the form no longer copies a field the copy does not have. Free text naming such an object (a description) is left as stored; a prefixed object no field key spells is still renamed in free text as before.

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27888,7 +27888,32 @@ export class ObjectStackProtocolImplementation implements
2788827888
// and `name` disagreed. `x_iojn_task` is another identifier, never the
2788927889
// object: `renameName` swaps the source prefix only at the START of a
2789027890
// name, and this rewrite now matches only a whole identifier too.
27891-
const olds = prefixed.filter((name) => renameMap.has(name)).sort((a, b) => b.length - a.length);
27891+
//
27892+
// ⭐ A PREFIXED NAME THAT IS ALSO A FIELD KEY IS NOT DISTINCTIVE. When a
27893+
// package object declares a field keyed like a prefixed package object
27894+
// (`fields.iojn_customer` beside the object `iojn_customer`), a free-text
27895+
// value spelled that way may name the field — a list view's `columns`,
27896+
// a form's `sections[].fields` — and the field keeps its key. Renaming
27897+
// it as the object copied a column or form field the copy does not
27898+
// have. Such a name is treated as a bare name is: rewritten only at an
27899+
// object-reference property (below), never in free text.
27900+
const fieldKeys = new Set<string>();
27901+
for (const row of rows) {
27902+
if (row?.type !== 'object') continue;
27903+
let body: any;
27904+
try {
27905+
body = typeof row.metadata === 'string' ? JSON.parse(row.metadata) : row.metadata;
27906+
} catch {
27907+
continue; // the copy loop reports an unparseable row at its own name
27908+
}
27909+
const fields = body?.fields;
27910+
if (fields && typeof fields === 'object' && !Array.isArray(fields)) {
27911+
for (const key of Object.keys(fields)) fieldKeys.add(key);
27912+
}
27913+
}
27914+
const olds = prefixed
27915+
.filter((name) => renameMap.has(name) && !fieldKeys.has(name))
27916+
.sort((a, b) => b.length - a.length);
2789227917
const esc = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
2789327918
const re = olds.length
2789427919
? new RegExp(`(?<![A-Za-z0-9_])(${olds.map(esc).join('|')})(?![A-Za-z0-9_])`, 'g')

‎packages/objectql/src/protocol-duplicate-package-names-in-step.test.ts‎

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,19 @@
2525
* 3. **A view-item id inside a value.** A form action's `target`
2626
* (`ticket.entry`) kept the bare object's name, so the copy's button opened
2727
* the source's form.
28+
* 4. **A name that is both a prefixed object and a field key.** The free-text
29+
* rewrite renamed a value equal to `dupsrc_customer` as the object even
30+
* where it named the field of that key: a copied list view's `columns`
31+
* named `dupdst_customer`, a field the copy does not have (refused at the
32+
* copy, `list-view-field-unknown`), and a form's `sections[].fields` copied
33+
* it silently.
2834
*
2935
* ## The rules now
3036
*
3137
* A record entry's `name` that echoes its key follows the key; free text
32-
* renames an object's name only as a whole identifier; a view item's object
38+
* renames an object's name only as a whole identifier, and never a name that
39+
* is also a package object's field key (that one is renamed only at an
40+
* object-reference property, as a bare name is); a view item's object
3341
* part takes `renameMap`'s rename, whole, and its view key is left as stored;
3442
* a value that IS such a name, whole, follows it.
3543
*
@@ -130,6 +138,7 @@ const Q = 'com.qa.dupdst';
130138
const R = 'com.qa.dupext';
131139

132140
const PROSE = 'Linked from dupsrc_customer; see x_dupsrc_customer.';
141+
const NOTE_PROSE = 'Attached to dupsrc_note; see x_dupsrc_note.';
133142
const RULE = { readable: true, editable: false };
134143

135144
const grid = (object: string, columns: string[]) => ({ type: 'grid', data: { provider: 'object', object }, columns });
@@ -149,6 +158,14 @@ const customerBody = {
149158
x_dupsrc_customer: { name: 'x_dupsrc_customer', type: 'text', label: 'Extra' },
150159
},
151160
};
161+
/** A prefixed object no field key spells: free text naming it follows the copy. */
162+
const noteBody = {
163+
name: 'dupsrc_note', label: 'Note', description: NOTE_PROSE, sharingModel: 'private',
164+
fields: {
165+
x_dupsrc_note: { name: 'x_dupsrc_note', type: 'text', label: 'Extra' },
166+
customer: { name: 'customer', type: 'lookup', reference: 'dupsrc_customer', label: 'Customer' },
167+
},
168+
};
152169
/** The object whose name lacks the `dupsrc_` prefix, with a lookup to the prefixed one. */
153170
const ticketBody = {
154171
name: 'ticket', label: 'Ticket', sharingModel: 'private',
@@ -224,12 +241,13 @@ describe('ADR-0070 D4 — duplicatePackage keeps a copied name in step with what
224241
expect(engine.registry.getPackage(P)?.manifest?.namespace).toBe('dupsrc');
225242
await save(P, 'object', customerBody);
226243
await save(P, 'object', ticketBody);
244+
await save(P, 'object', noteBody);
227245
await save(P, 'view', viewItem('ticket', 'board', 'list', grid('ticket', ['title'])));
228-
await save(P, 'view', viewItem('dupsrc_customer', 'board', 'list', grid('dupsrc_customer', ['full_name', 'x_dupsrc_customer'])));
246+
await save(P, 'view', viewItem('dupsrc_customer', 'board', 'list', grid('dupsrc_customer', ['full_name', 'dupsrc_customer', 'x_dupsrc_customer'])));
229247
await save(P, 'view', viewItem('dupext_account', 'board', 'list', grid('dupext_account', ['name'])));
230248
await save(P, 'view', viewItem('dupsrc_ext', 'board', 'list', grid('dupsrc_ext', ['name'])));
231249
await save(P, 'view', viewItem('ticket', 'entry', 'form', form('ticket', ['title'])));
232-
await save(P, 'view', viewItem('dupsrc_customer', 'entry', 'form', form('dupsrc_customer', ['full_name'])));
250+
await save(P, 'view', viewItem('dupsrc_customer', 'entry', 'form', form('dupsrc_customer', ['full_name', 'dupsrc_customer'])));
233251
await save(P, 'action', formAction('dupsrc_new_ticket', 'ticket', 'ticket.entry'));
234252
await save(P, 'action', formAction('dupsrc_new_customer', 'dupsrc_customer', 'dupsrc_customer.entry'));
235253
await save(P, 'action', formAction('dupsrc_new_account', 'dupext_account', 'dupext_account.entry'));
@@ -265,14 +283,48 @@ describe('ADR-0070 D4 — duplicatePackage keeps a copied name in step with what
265283
it('free text renames an object\'s name only as a whole identifier, never inside a longer one', async () => {
266284
await duplicate();
267285

268-
expect(storedBody('object', 'dupdst_customer', Q).description)
269-
.toBe('Linked from dupdst_customer; see x_dupsrc_customer.');
286+
expect(storedBody('object', 'dupdst_note', Q).description)
287+
.toBe('Attached to dupdst_note; see x_dupsrc_note.');
288+
expect(Object.keys(storedBody('object', 'dupdst_note', Q).fields)).toEqual(['x_dupsrc_note', 'customer']);
289+
expect(storedBody('object', 'dupdst_note', Q).fields.x_dupsrc_note.name).toBe('x_dupsrc_note');
270290
// A copied view's column naming the field stays in step with the field.
271-
expect(storedBody('view', 'dupdst_customer.board', Q).config.columns).toEqual(['full_name', 'x_dupsrc_customer']);
291+
expect(storedBody('view', 'dupdst_customer.board', Q).config.columns)
292+
.toEqual(['full_name', 'dupsrc_customer', 'x_dupsrc_customer']);
272293
// The source is untouched.
273294
expect(storedBody('object', 'dupsrc_customer', P)).toMatchObject({ description: PROSE, fields: customerBody.fields });
274295
});
275296

297+
it('a name that is both a prefixed object and a field key is renamed only at object-reference properties', async () => {
298+
const res = await duplicate();
299+
300+
// The copied list view is no longer refused for a column the copy lacks.
301+
expect(res.failed).toEqual([]);
302+
expect(res.copied).toEqual(expect.arrayContaining([
303+
{ type: 'view', name: 'dupdst_customer.board' },
304+
{ type: 'view', name: 'dupdst_customer.entry' },
305+
]));
306+
// A value naming the FIELD keeps naming it: list-view columns and form fields.
307+
const board = storedBody('view', 'dupdst_customer.board', Q);
308+
expect(board.config.columns).toEqual(['full_name', 'dupsrc_customer', 'x_dupsrc_customer']);
309+
const entry = storedBody('view', 'dupdst_customer.entry', Q);
310+
expect(entry.config.sections[0].fields).toEqual(['full_name', 'dupsrc_customer']);
311+
expect(Object.keys(storedBody('object', 'dupdst_customer', Q).fields)).toContain('dupsrc_customer');
312+
// A value naming the OBJECT at a reference property follows the copy.
313+
expect(board).toMatchObject({ object: 'dupdst_customer', config: { data: { object: 'dupdst_customer' } } });
314+
expect(entry).toMatchObject({ object: 'dupdst_customer', config: { data: { object: 'dupdst_customer' } } });
315+
expect(storedBody('object', 'dupdst_ticket', Q).fields.customer.reference).toBe('dupdst_customer');
316+
expect(storedBody('object', 'dupdst_note', Q).fields.customer.reference).toBe('dupdst_customer');
317+
expect(storedBody('action', 'dupdst_new_customer', Q).objectName).toBe('dupdst_customer');
318+
expect(Object.keys(storedBody('permission', 'dupdst_ops', Q).objects)).toEqual(['dupdst_customer']);
319+
// Free text naming it is left as stored (the ruled semantics: such a name
320+
// may name the field); free text naming an ordinary prefixed object still
321+
// follows the copy (the negative control, `dupsrc_note`, in the case above).
322+
expect(storedBody('object', 'dupdst_customer', Q).description).toBe(PROSE);
323+
// The copied record can be written through the copied form's fields.
324+
await engine.insert('dupdst_customer', { id: 'c2', full_name: 'Ada', dupsrc_customer: 'C-2' });
325+
expect(stores.get('dupdst_customer')?.get('c2')).toMatchObject({ full_name: 'Ada', dupsrc_customer: 'C-2' });
326+
});
327+
276328
it('the copy of a bare object\'s view item takes the renamed object\'s prefix, and is the one a by-name read serves', async () => {
277329
const res = await duplicate();
278330

0 commit comments

Comments
 (0)