From cad596fb0bb392be40000656aeb08e7452d458ac Mon Sep 17 00:00:00 2001 From: agent_coder Date: Fri, 10 Jul 2026 14:28:39 +0300 Subject: [PATCH] =?UTF-8?q?fix(ai-chat):=20=D0=BF=D1=80=D0=BE=D0=B1=D1=80?= =?UTF-8?q?=D0=BE=D1=81=D0=B8=D1=82=D1=8C=20layout:elk=20=D0=B2=20in-app?= =?UTF-8?q?=20drawioCreate/drawioUpdate=20=E2=80=94=20=D0=BF=D0=B0=D1=80?= =?UTF-8?q?=D0=B8=D1=82=D0=B5=D1=82=20=D1=81=20MCP=20(=D1=80=D0=B5=D0=B2?= =?UTF-8?q?=D1=8C=D1=8E=20#440)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In-app хендлеры drawio_create/drawio_update деструктурировали args БЕЗ layout и не передавали его клиенту 5-м аргументом → layout:"elk" (схема его принимает — общий buildShape) ТИХО терялся, ELK-автолейаут работал только по MCP-хосту. Корень: ручное зеркало DocmostClientLike (loader) отстало от реального client.ts — у его drawioCreate/drawioUpdate не было параметра layout (то, что #446 чинит деривацией типа, но #446 ещё не влит). Добавил layout?:'elk' в обе сигнатуры зеркала + проброс в обоих хендлерах. Тест (пропущенный зелёным гейтом пробел — не было теста на in-app passthrough): in-app drawioCreate/drawioUpdate с layout:'elk' → фейк-клиент получает layout 5-м позиционным аргументом; omit-кейс → undefined. Мутационно: убрать проброс в drawioCreate → layout-create-тест краснеет. Гейт: mcp build чисто; tsc -p apps/server без новых ошибок; jest ai-chat-tools.service (35) + shared-tool-specs.contract + tool-tiers + comment-signal-inapp → 273 passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tools/ai-chat-tools.service.spec.ts | 101 ++++++++++++++++++ .../ai-chat/tools/ai-chat-tools.service.ts | 26 ++++- .../ai-chat/tools/docmost-client.loader.ts | 4 + 3 files changed, 128 insertions(+), 3 deletions(-) diff --git a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts index 3cba8c6d..094a0413 100644 --- a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts +++ b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts @@ -844,3 +844,104 @@ describe('AiChatToolsService getCurrentPage selection (#388)', () => { ); }); }); + +/** + * #440 review: the in-app drawio_create / drawio_update handlers must forward + * the optional `layout:"elk"` param to the client (5th positional arg), exactly + * like the MCP host. It was silently dropped, so ELK auto-layout worked only via + * the standalone MCP server, not in-app. These tests pin per-host parity. + */ +describe('AiChatToolsService drawio layout passthrough (#440)', () => { + const createCalls: unknown[][] = []; + const updateCalls: unknown[][] = []; + + const fakeClient: Partial = { + drawioCreate: (...args: unknown[]) => { + createCalls.push(args); + return Promise.resolve({ success: true, nodeId: '#0' }); + }, + drawioUpdate: (...args: unknown[]) => { + updateCalls.push(args); + return Promise.resolve({ success: true, nodeId: '#0' }); + }, + }; + + const tokenServiceStub = { + generateAccessToken: jest.fn().mockResolvedValue('access-token'), + generateCollabToken: jest.fn().mockResolvedValue('collab-token'), + }; + + let service: AiChatToolsService; + + beforeEach(() => { + createCalls.length = 0; + updateCalls.length = 0; + jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue( + mockLoaded(function () { + return fakeClient as DocmostClientLike; + } as unknown as loader.DocmostClientCtor), + ); + service = new AiChatToolsService( + tokenServiceStub as never, + {} as never, + {} as never, + {} as never, + {} as never, + { + asSink: () => ({ put: jest.fn(), has: jest.fn(), evict: jest.fn() }), + } as never, + ); + }); + + afterEach(() => jest.restoreAllMocks()); + + const buildTools = () => + service.forUser( + { id: 'user-1', email: 'u@example.com', workspaceId: 'ws-1' } as never, + 'session-1', + 'ws-1', + 'chat-1', + ); + + it('forwards layout:"elk" to client.drawioCreate as the 5th positional arg', async () => { + const tools = await buildTools(); + await tools.drawioCreate.execute( + { + pageId: 'p-1', + xml: '', + position: 'append', + layout: 'elk', + } as never, + {} as never, + ); + expect(createCalls).toHaveLength(1); + // drawioCreate(pageId, where, xml, title, layout) — layout is args[4]. + expect(createCalls[0][4]).toBe('elk'); + }); + + it('forwards layout:"elk" to client.drawioUpdate as the 5th positional arg', async () => { + const tools = await buildTools(); + await tools.drawioUpdate.execute( + { + pageId: 'p-1', + node: '#0', + xml: '', + baseHash: 'h', + layout: 'elk', + } as never, + {} as never, + ); + expect(updateCalls).toHaveLength(1); + // drawioUpdate(pageId, node, xml, baseHash, layout) — layout is args[4]. + expect(updateCalls[0][4]).toBe('elk'); + }); + + it('omits layout (undefined 5th arg) when not requested', async () => { + const tools = await buildTools(); + await tools.drawioCreate.execute( + { pageId: 'p-1', xml: '', position: 'append' } as never, + {} as never, + ); + expect(createCalls[0][4]).toBeUndefined(); + }); +}); diff --git a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts index 12cf9df7..a5d0a53e 100644 --- a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts +++ b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts @@ -747,12 +747,24 @@ export class AiChatToolsService { // The flat schema fields are regrouped into the client's `where` object. drawioCreate: sharedTool( sharedToolSpecs.drawioCreate, - async ({ pageId, xml, position, anchorNodeId, anchorText, title }) => + async ({ + pageId, + xml, + position, + anchorNodeId, + anchorText, + title, + layout, + }) => await client.drawioCreate( pageId, { position, anchorNodeId, anchorText }, xml, title, + // Forward layout:"elk" so in-app auto-placement matches the MCP host + // (the shared schema accepts it; dropping it silently disabled ELK + // auto-layout for the in-app agent). + layout as "elk" | undefined, ), ), @@ -760,8 +772,16 @@ export class AiChatToolsService { // baseHash is the optimistic lock: mismatch => structured conflict error. drawioUpdate: sharedTool( sharedToolSpecs.drawioUpdate, - async ({ pageId, node, xml, baseHash }) => - await client.drawioUpdate(pageId, node, xml, baseHash), + async ({ pageId, node, xml, baseHash, layout }) => + // Forward layout:"elk" (5th arg) so in-app auto-placement matches the + // MCP host; dropping it silently disabled ELK auto-layout in-app. + await client.drawioUpdate( + pageId, + node, + xml, + baseHash, + layout as "elk" | undefined, + ), ), // Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#424). diff --git a/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts b/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts index e1165996..50efd939 100644 --- a/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts +++ b/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts @@ -190,6 +190,9 @@ export interface DocmostClientLike { }, xml: string, title?: string, + // #424: layout:"elk" runs elkjs auto-placement before the write. Mirror the + // real client signature so the in-app handler can forward it (parity #440). + layout?: 'elk', ): Promise>; // Optimistic-locked full replacement of a diagram (baseHash from drawioGet). drawioUpdate( @@ -197,6 +200,7 @@ export interface DocmostClientLike { node: string, xml: string, baseHash: string, + layout?: 'elk', ): Promise>; tableInsertRow( pageId: string,