fix(ai-chat): пробросить layout:elk в in-app drawioCreate/drawioUpdate — паритет с MCP (ревью #440)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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<DocmostClientLike> = {
|
||||
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: '<mxGraphModel/>',
|
||||
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: '<mxGraphModel/>',
|
||||
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: '<mxGraphModel/>', position: 'append' } as never,
|
||||
{} as never,
|
||||
);
|
||||
expect(createCalls[0][4]).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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<Record<string, unknown>>;
|
||||
// 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<Record<string, unknown>>;
|
||||
tableInsertRow(
|
||||
pageId: string,
|
||||
|
||||
Reference in New Issue
Block a user