diff --git a/packages/pen-mcp/src/__tests__/element-tools-contract.test.ts b/packages/pen-mcp/src/__tests__/element-tools-contract.test.ts index ea44388e1..ed53c7800 100644 --- a/packages/pen-mcp/src/__tests__/element-tools-contract.test.ts +++ b/packages/pen-mcp/src/__tests__/element-tools-contract.test.ts @@ -152,4 +152,51 @@ describe('element tools — silent no-op guards (P0.1 + post-insert check)', () const after = await readFile(fp, 'utf-8'); expect(after).toBe(before); }); + + it('throws when DSL-resolved parent_id collides with a different node (wrong-parent insert)', async () => { + // Seed the document with TWO frames: + // - intended parent, id = `A"B` (3 chars: A, ", B) — the user's target + // - decoy parent, id = `A\"B` (4 chars: A, \, ", B) — happens to + // be what batch_design's resolveRef produces after quote-strip on + // our JSON.stringify output for 'A"B' + // Expected: ensureParentExists finds the intended parent (raw string + // match ✓), but insertNodeInTree resolves to the decoy (literal match + // on the 4-char id). Without parent-location verification, the node + // would silently land under the wrong parent. + const fp = join(TMP, 'quoted-parent.op'); + await writeFile( + fp, + JSON.stringify({ + version: '1.0.0', + children: [ + { + id: 'A"B', + type: 'frame', + name: 'Intended Parent', + width: 1200, + height: 0, + layout: 'vertical', + children: [], + }, + { + id: 'A\\"B', + type: 'frame', + name: 'Decoy Parent', + width: 1200, + height: 0, + layout: 'vertical', + children: [], + }, + ], + }), + 'utf-8', + ); + await expect( + handleAddBottomNavV0({ + filePath: fp, + parent_id: 'A"B', + items: [{ title: 'Home', icon: 'home' }], + }), + ).rejects.toThrow(/wrong parent|not present/); + }); }); diff --git a/packages/pen-mcp/src/tools/element-tool-helpers.ts b/packages/pen-mcp/src/tools/element-tool-helpers.ts index ab8314f93..6c8c2e3fa 100644 --- a/packages/pen-mcp/src/tools/element-tool-helpers.ts +++ b/packages/pen-mcp/src/tools/element-tool-helpers.ts @@ -1,5 +1,5 @@ import { openDocument, resolveDocPath } from '../document-manager'; -import { findNodeInTree, getDocChildren } from '../utils/node-operations'; +import { findNodeInTree, findParentInTree, getDocChildren } from '../utils/node-operations'; import { generateId } from '../utils/id'; import { handleBatchDesign } from './batch-design'; @@ -147,5 +147,25 @@ export async function insertElementTree(args: { `parent_id=${JSON.stringify(args.parent_id)}, pageId=${JSON.stringify(args.pageId)}.`, ); } + // Parent-location verification: node exists in tree but may have landed + // under the wrong parent. Can happen if batch_design's resolveRef + // quote-strip produces a literal that matches a DIFFERENT node than the + // one pre-check validated. Example: doc has both `A"B` (user intent) + // AND `A\"B` (literal 4-char id with backslash); after JSON.stringify + // + quote-strip, parser resolves to `A\"B` and inserts under it. + // ensureParentExists and the "landed in tree" check both pass, but + // the insert went to the wrong place. + if (args.parent_id) { + const actualParent = findParentInTree(postChildren, insertedId); + const actualParentId = actualParent?.id ?? null; + if (actualParentId !== args.parent_id) { + throw new Error( + `Element tool insert landed under the wrong parent: expected ` + + `${JSON.stringify(args.parent_id)}, got ${JSON.stringify(actualParentId ?? 'root')}. ` + + `This is typically a DSL-parser escape mismatch where the resolved parent id ` + + `happens to collide with a different node's literal id.`, + ); + } + } return result; }