diff --git a/.gitignore b/.gitignore index 131d9031..3eb7e75b 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,11 @@ .env.dev .env.prod data +# Exception: the committed draw.io shape catalog (issue #424) lives in a `data/` +# dir, but the bare `data` ignore above is meant for runtime state, not this +# bundled build asset. Re-include the directory and its contents. +!packages/mcp/data/ +!packages/mcp/data/** # compiled output /dist node_modules 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 a2bd5be4..24b8e746 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 @@ -24,6 +24,14 @@ import { SHARED_TOOL_SPECS } from '../../../../../../packages/mcp/src/tool-specs const mockLoaded = (DocmostClient: loader.DocmostClientCtor) => ({ DocmostClient, sharedToolSpecs: SHARED_TOOL_SPECS as Record, + // Pure no-network draw.io helpers (#424). Type-correct stubs: these tests + // never execute the drawio_shapes / drawio_guide tool bodies. + searchShapes: (() => []) as unknown as loader.SearchShapesFn, + getGuideSection: (() => ({ + section: 'index', + content: '', + sections: [], + })) as unknown as loader.GetGuideSectionFn, }); /** @@ -847,3 +855,109 @@ 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[][] = []; + + // FakeDocmostClient (not Partial): since #446 derived + // DocmostClientLike from the real client, its drawioCreate/drawioUpdate return + // the concrete result shape, so a minimal stub object would not be assignable. + // FakeDocmostClient types every method as (...args) => Promise, which is + // exactly what these arg-capturing doubles need. + const fakeClient: FakeDocmostClient = { + 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 c40bdbfd..d0f87270 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 @@ -111,10 +111,12 @@ function __assertClientCallContract(client: DocmostClientLike): void { afterText: s, }); void client.replaceImage(s, s, s, { align, alt: s }); - // --- draw.io diagrams (#423) --- + // --- draw.io diagrams (#423 stage 1, #424 stage 2) --- + // The 5th `layout` arg (#424) is exercised so this parity assertion fails if the + // client signature drops it — it must reach the client from the shared execute. void client.drawioGet(s, s, 'xml'); - void client.drawioCreate(s, { position: 'append', anchorNodeId: s }, s, s); - void client.drawioUpdate(s, s, s, s); + void client.drawioCreate(s, { position: 'append', anchorNodeId: s }, s, s, 'elk'); + void client.drawioUpdate(s, s, s, s, 'elk'); // --- write (comment) --- void client.createComment(s, s, 'inline', s, s, s); void client.resolveComment(s, true); @@ -263,8 +265,19 @@ export class AiChatToolsService { // provenance tokens) and load the shared tool-spec registry. Client // construction is shared with the page-change detection path (#274) via // buildDocmostClient so both go over the exact same authenticated route. - const { sharedToolSpecs, createCommentSignalTracker } = - await loadDocmostMcp(); + // searchShapes / getGuideSection (#424) are the PURE, no-network helpers + // backing drawio_shapes / drawio_guide. They are `inlineBothHosts` specs (no + // canonical execute — their catalog loader uses import.meta and can't be + // value-imported into the zod-agnostic tool-specs.ts under the server's + // commonjs type-check), so the shared registry loop below SKIPS them and this + // service wires them inline (see drawioShapes/drawioGuide entries), mirroring + // how index.ts registers them on the standalone MCP host. + const { + sharedToolSpecs, + createCommentSignalTracker, + searchShapes, + getGuideSection, + } = await loadDocmostMcp(); const client = await this.buildDocmostClient( user, sessionId, @@ -555,6 +568,8 @@ export class AiChatToolsService { // WHICH mapping to run and returns its value directly (no envelope). For each // spec: // - skip `mcpOnly` specs (they belong to the standalone MCP host only); + // - skip `inlineBothHosts` specs (drawio_shapes / drawio_guide): they carry + // no execute and are wired INLINE just below, calling the pure helpers; // - use `inAppExecute` when the spec declares a DELIBERATE per-layer // difference (a projected result shape, a different guardrail message); // - otherwise use the canonical `execute` (raw client result, identical to @@ -564,6 +579,7 @@ export class AiChatToolsService { // arg mapping lives — it can no longer silently drift from the MCP host. for (const spec of Object.values(sharedToolSpecs)) { if (spec.mcpOnly) continue; + if (spec.inlineBothHosts) continue; const run = spec.inAppExecute ?? spec.execute; if (!run) continue; // defensive: a shared spec always carries one of them. tools[spec.inAppKey] = sharedTool( @@ -573,6 +589,25 @@ export class AiChatToolsService { ); } + // drawio_shapes / drawio_guide (#424): `inlineBothHosts` registry specs wired + // here with the SAME schema+description the shared spec pins, but calling the + // pure searchShapes / getGuideSection helpers off the loaded @docmost/mcp + // module — they are not client methods and their catalog loader uses + // import.meta, so they cannot live in the zod-agnostic shared execute. The raw + // result is identical to the MCP host's (which wraps it as JSON text); here + // the in-app host returns it plain, exactly like every other shared tool. + tools[sharedToolSpecs.drawioShapes.inAppKey] = sharedTool( + sharedToolSpecs.drawioShapes, + async ({ query, category, limit }) => { + const results = searchShapes(query, { category, limit }); + return { query, count: results.length, results }; + }, + ); + tools[sharedToolSpecs.drawioGuide.inAppKey] = sharedTool( + sharedToolSpecs.drawioGuide, + async ({ section }) => getGuideSection(section), + ); + // Passive "new comments: N" signal (#417). PER-TURN state (forUser runs once // per turn), so the watermark starts now and only comments a human leaves // WHILE this turn runs are signalled — exactly the mid-turn loop; between-turn diff --git a/apps/server/src/core/ai-chat/tools/comment-signal-inapp.spec.ts b/apps/server/src/core/ai-chat/tools/comment-signal-inapp.spec.ts index 1df0fb85..b9e9cacc 100644 --- a/apps/server/src/core/ai-chat/tools/comment-signal-inapp.spec.ts +++ b/apps/server/src/core/ai-chat/tools/comment-signal-inapp.spec.ts @@ -287,6 +287,14 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => { // Wire the REAL factory so the in-app path is exercised end to end. createCommentSignalTracker: createCommentSignalTracker as unknown as loader.CommentSignalTrackerFactory, + // Pure no-network draw.io helpers (#424) — required on the loader return; + // this comment-signal test doesn't exercise them, so no-op stubs suffice. + searchShapes: (() => []) as unknown as loader.SearchShapesFn, + getGuideSection: (() => ({ + section: '', + content: '', + sections: [], + })) as unknown as loader.GetGuideSectionFn, }); return new AiChatToolsService( tokenServiceStub as never, 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 4573199b..d9419a7e 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 @@ -62,7 +62,10 @@ type DocmostClientMethod = | 'insertImage' | 'replaceImage' | 'insertFootnote' - // --- draw.io diagrams (#423, stage 1) --- + // --- draw.io diagrams (#423 stage 1, #424 stage 2) --- + // DERIVED from the real DocmostClient (#446): drawioCreate/drawioUpdate carry + // the optional layout:"elk" 5th arg in the real signature, so the layout parity + // (#440) is inherited automatically — no hand-written mirror to keep in sync. | 'drawioGet' | 'drawioCreate' | 'drawioUpdate' @@ -141,6 +144,19 @@ export type CommentSignalTrackerFactory = (options: { debounceMs?: number; }) => CommentSignalTrackerLike; +// Pure, no-network draw.io helpers (#424). These are plain functions on the +// module (NOT DocmostClient methods) — the in-app AI-SDK service calls them +// directly to wire drawio_shapes / drawio_guide, mirroring the MCP server. +export type SearchShapesFn = ( + query: string, + opts?: { category?: string; limit?: number }, +) => Array>; +export type GetGuideSectionFn = (section?: string) => { + section: string; + content: string; + sections: string[]; +}; + interface DocmostMcpModule { DocmostClient: DocmostClientCtor; SHARED_TOOL_SPECS: Record; @@ -153,6 +169,15 @@ interface DocmostMcpModule { // the mocked loader in unit tests) — the stale-check below is a NO-OP when it // is missing, so an older build never wrongly fails startup. REGISTRY_STAMP?: string; + // Pure, no-network draw.io helpers (#424) backing drawio_shapes / drawio_guide. + // Those two specs are `inlineBothHosts` (they stay in SHARED_TOOL_SPECS for the + // shared contract but carry no execute — their catalog loader uses import.meta + // and can't be value-imported into the zod-agnostic tool-specs.ts), so the + // in-app service wires them INLINE off these helpers, mirroring the standalone + // MCP host. Exposed off the loaded module so the service and its test mocks can + // reach them. + searchShapes: SearchShapesFn; + getGuideSection: GetGuideSectionFn; } /** @@ -216,6 +241,8 @@ export async function loadDocmostMcp(): Promise<{ DocmostClient: DocmostClientCtor; sharedToolSpecs: Record; createCommentSignalTracker?: CommentSignalTrackerFactory; + searchShapes: SearchShapesFn; + getGuideSection: GetGuideSectionFn; }> { if (!modulePromise) { modulePromise = (async () => { @@ -261,5 +288,8 @@ export async function loadDocmostMcp(): Promise<{ // Optional: forwarded when present so the in-app layer can build the passive // comment signal (#417); undefined on a stale build => signal disabled. createCommentSignalTracker: mod.createCommentSignalTracker, + // Pure no-network draw.io helpers (#424); not client methods. + searchShapes: mod.searchShapes, + getGuideSection: mod.getGuideSection, }; } diff --git a/apps/server/src/core/ai-chat/tools/shared-tool-specs.contract.spec.ts b/apps/server/src/core/ai-chat/tools/shared-tool-specs.contract.spec.ts index f62f0cc7..3d708db0 100644 --- a/apps/server/src/core/ai-chat/tools/shared-tool-specs.contract.spec.ts +++ b/apps/server/src/core/ai-chat/tools/shared-tool-specs.contract.spec.ts @@ -45,6 +45,16 @@ describe('SHARED_TOOL_SPECS contract parity', () => { string, loader.SharedToolSpec >, + // Pure no-network draw.io helpers (#424). The contract test never executes + // a tool body, so type-correct stubs suffice (the real functions can't be + // imported here — drawio-shapes.ts uses import.meta, incompatible with the + // CommonJS jest transform). + searchShapes: (() => []) as unknown as loader.SearchShapesFn, + getGuideSection: (() => ({ + section: 'index', + content: '', + sections: [], + })) as unknown as loader.GetGuideSectionFn, }); const service = new AiChatToolsService( tokenServiceStub as never, diff --git a/apps/server/src/core/ai-chat/tools/tool-tiers.spec.ts b/apps/server/src/core/ai-chat/tools/tool-tiers.spec.ts index 130e9b06..3849af12 100644 --- a/apps/server/src/core/ai-chat/tools/tool-tiers.spec.ts +++ b/apps/server/src/core/ai-chat/tools/tool-tiers.spec.ts @@ -124,6 +124,13 @@ describe('deferred catalog ↔ live forUser() toolset partition (#332, F3)', () return {} as DocmostClientLike; } as unknown as loader.DocmostClientCtor, sharedToolSpecs: SHARED_TOOL_SPECS as Record, + // Pure no-network draw.io helpers (#424); tool bodies are never executed here. + searchShapes: (() => []) as unknown as loader.SearchShapesFn, + getGuideSection: (() => ({ + section: 'index', + content: '', + sections: [], + })) as unknown as loader.GetGuideSectionFn, }); const service = new AiChatToolsService( { diff --git a/packages/mcp/data/drawio-shape-index.json.gz b/packages/mcp/data/drawio-shape-index.json.gz new file mode 100644 index 00000000..91d30d8e Binary files /dev/null and b/packages/mcp/data/drawio-shape-index.json.gz differ diff --git a/packages/mcp/package.json b/packages/mcp/package.json index 66386299..b5400dab 100644 --- a/packages/mcp/package.json +++ b/packages/mcp/package.json @@ -57,6 +57,7 @@ "@tiptap/starter-kit": "3.20.4", "@types/jsdom": "^27.0.0", "axios": "^1.6.0", + "elkjs": "^0.11.1", "form-data": "^4.0.0", "jsdom": "^27.4.0", "marked": "^17.0.1", diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index a2312ce4..89891063 100644 --- a/packages/mcp/src/client.ts +++ b/packages/mcp/src/client.ts @@ -55,6 +55,7 @@ import { countUserCells, } from "./lib/drawio-xml.js"; import { renderDiagramShapes } from "./lib/drawio-preview.js"; +import { applyElkLayout } from "./lib/drawio-layout.js"; import { applyTextEdits, TextEdit, @@ -3984,6 +3985,7 @@ export class DocmostClient { }, xml: string, title?: string, + layout?: "elk", ): Promise<{ success: boolean; nodeId: string; @@ -4014,8 +4016,12 @@ export class DocmostClient { } } + // Optional server-side ELK auto-layout: the model declares structure with + // rough coords, ELK computes the pixels (best-effort — returns the input + // unchanged on any layout failure). + const laidOutXml = layout === "elk" ? await applyElkLayout(xml) : xml; // Pre-write pipeline (throws a structured DrawioLintError on any violation). - const prepared = prepareModel(xml); + const prepared = prepareModel(laidOutXml); const inner = renderDiagramShapes(prepared.cells, prepared.bbox); const diagramTitle = title || "Page-1"; const svg = buildDrawioSvg(prepared.modelXml, inner, prepared.bbox, diagramTitle); @@ -4129,6 +4135,7 @@ export class DocmostClient { node: string, xml: string, baseHash: string, + layout?: "elk", ): Promise<{ success: boolean; nodeId: string; @@ -4166,8 +4173,10 @@ export class DocmostClient { ); } + // Optional server-side ELK auto-layout (best-effort; see drawioCreate). + const laidOutXml = layout === "elk" ? await applyElkLayout(xml) : xml; // Pipeline for the new content (throws a structured DrawioLintError). - const prepared = prepareModel(xml); + const prepared = prepareModel(laidOutXml); const inner = renderDiagramShapes(prepared.cells, prepared.bbox); const diagramTitle = oldAttrs.title || "Page-1"; const svg = buildDrawioSvg(prepared.modelXml, inner, prepared.bbox, diagramTitle); diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index a1ca00c4..4c22ea6f 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -5,6 +5,8 @@ import { fileURLToPath } from "url"; import { dirname, join } from "path"; import { DocmostClient, DocmostMcpConfig } from "./client.js"; import { parseNodeArg } from "@docmost/prosemirror-markdown"; +import { searchShapes } from "./lib/drawio-shapes.js"; +import { getGuideSection } from "./lib/drawio-guide.js"; import { SHARED_TOOL_SPECS, SharedToolSpec } from "./tool-specs.js"; import { SERVER_INSTRUCTIONS } from "./server-instructions.js"; import { @@ -55,6 +57,13 @@ export type { CommentSignalProbeResult, CommentSignalTrackerOptions, } from "./comment-signal.js"; +// Re-export the pure, no-network draw.io helpers (#424) so the in-app AI-SDK +// service can wire drawio_shapes / drawio_guide off the loaded module. These are +// NOT client methods (no page/backend hit) — the in-app handler calls them +// directly, mirroring how the standalone MCP server wires them here. +export { searchShapes } from "./lib/drawio-shapes.js"; +export type { SearchShapesOptions } from "./lib/drawio-shapes.js"; +export { getGuideSection } from "./lib/drawio-guide.js"; // Read version from package.json const __filename = fileURLToPath(import.meta.url); @@ -80,6 +89,11 @@ const VERSION = packageJson.version; // (SHARED_TOOL_SPECS + INLINE_MCP_INVENTORY), so it can no longer drift out of // sync with the registered tools. Re-exported here (its old home) so existing // importers are unaffected; the composition lives in server-instructions.ts. +// The drawio_shapes / drawio_guide tools (#424) stay in SHARED_TOOL_SPECS (so the +// generated picks them up from their catalogLine automatically) +// but are flagged `inlineBothHosts` and registered inline below (their pure +// helpers can't cross into tool-specs.ts); only the hand-written routing prose in +// server-instructions.ts is updated to mention them. export { SERVER_INSTRUCTIONS }; // Helper to format JSON responses @@ -272,6 +286,11 @@ export function createDocmostMcpServer(config: DocmostMcpConfig): McpServer { // the wrapping is typed loosely and cast — runtime behaviour is unchanged. const registerSharedFromSpec = (spec: SharedToolSpec) => { if (spec.inAppOnly) return; + // `inlineBothHosts` specs (drawio_shapes / drawio_guide) carry no execute — + // their pure helper cannot cross into the zod-agnostic tool-specs.ts, so they + // are registered INLINE below (searchShapes / getGuideSection). Skip them here + // so the loop never dereferences a missing `execute`. + if (spec.inlineBothHosts) return; const handler = async (args: any) => { if (spec.mcpExecute) { // The override owns the full MCP result envelope (not re-wrapped). @@ -296,6 +315,45 @@ export function createDocmostMcpServer(config: DocmostMcpConfig): McpServer { registerSharedFromSpec(spec as SharedToolSpec); } + // --- INLINE drawio helper tools (IN the shared registry, but inlineBothHosts) --- + // drawio_shapes / drawio_guide (#424) live in SHARED_TOOL_SPECS (so the shared + // contract pins their name/description/schema across both hosts) but carry the + // `inlineBothHosts` flag and NO execute: their pure backing helpers + // (searchShapes / getGuideSection) cannot be value-imported into the + // zod-agnostic tool-specs.ts without breaking the in-app server's commonjs + // type-check (searchShapes' catalog loader uses import.meta). So both hosts wire + // them directly. Here on the MCP host they reuse the spec's name/description/ + // schema and wrap the raw helper result as JSON text content — byte-identical to + // what the registry loop would have produced. The in-app host mirrors this in + // ai-chat-tools.service.ts. + { + // Cast registerTool like the loop's registerSharedFromSpec does: the spec's + // buildShape returns the loose zod-agnostic ZodRawShape (Record) and the handler args are the SDK-validated, type-erased input. + const registerInline = server.registerTool as any; + const shapesSpec = SHARED_TOOL_SPECS.drawioShapes as SharedToolSpec; + registerInline( + shapesSpec.mcpName, + { + description: shapesSpec.description, + inputSchema: shapesSpec.buildShape!(z), + }, + async ({ query, category, limit }: any) => { + const results = searchShapes(query, { category, limit }); + return jsonContent({ query, count: results.length, results }); + }, + ); + const guideSpec = SHARED_TOOL_SPECS.drawioGuide as SharedToolSpec; + registerInline( + guideSpec.mcpName, + { + description: guideSpec.description, + inputSchema: guideSpec.buildShape!(z), + }, + async ({ section }: any) => jsonContent(getGuideSection(section)), + ); + } + // --- INLINE tools kept per-transport (NOT in the shared registry) --- // Each stays inline for a documented reason: a snake_case/camelCase naming // clash the registry convention forbids (table_get), an intentional diff --git a/packages/mcp/src/lib/drawio-guide.ts b/packages/mcp/src/lib/drawio-guide.ts new file mode 100644 index 00000000..cc0e5d2d --- /dev/null +++ b/packages/mcp/src/lib/drawio-guide.ts @@ -0,0 +1,228 @@ +// Progressive-disclosure authoring reference for the `drawio_guide` tool +// (issue #424, stage 2). The FULL draw.io authoring guide would bloat every +// context window, so it is split into small sections the model reads on demand: +// skeleton | layout | containers | icons-aws | icons-azure +// Content is written directly from the issue #424 appendix (the layout +// heuristics, container rules, AWS icon patterns + gotchas + blocklist, and +// Azure image-style paths). ACCEPTANCE: each section stays <= ~4 KB so pulling +// one is cheap. + +export type GuideSection = + | "skeleton" + | "layout" + | "containers" + | "icons-aws" + | "icons-azure"; + +export const GUIDE_SECTIONS: GuideSection[] = [ + "skeleton", + "layout", + "containers", + "icons-aws", + "icons-azure", +]; + +const SKELETON = `# drawio_guide: skeleton + +Canonical mxGraph skeleton. id="0" and id="1" are MANDATORY sentinels; every +real cell has parent="1" (or a container id). Set adaptiveColors="auto" on the +model so Docmost's dark theme adapts strokeColor/fillColor/fontColor="default". + +\`\`\`xml + + + + + + + + + + + + + + + +\`\`\` + +Three accepted inputs to drawio_create/drawio_update: a bare , a +full (decoded to its first page), or a raw list of (the server +wraps it and adds the id=0/id=1 sentinels). + +Hard rules: a cell is vertex="1" XOR edge="1" (a container/group is neither); +every edge has a child ; ids are unique; +no XML comments; put html=1 in styles and XML-escape value (& -> &, +< -> <); a newline in a label is , never a literal \\n. Don't guess +shape=mxgraph.* names — call drawio_shapes first (a wrong name renders empty).`; + +const LAYOUT = `# drawio_guide: layout + +Turn "make it look good" into checkable numbers. Or pass layout:"elk" to +drawio_create/drawio_update and the server computes coordinates for you (ELK +layered layout, honouring nested containers) — you declare structure, it places +pixels. + +Spacing (when placing by hand): +- Horizontal gap between shapes 200-220px; vertical between rows/lanes 250px; + auxiliary services (monitoring, DLQ) sit below the main flow with 280px+ gap. +- Coordinates are multiples of 10 (grid). Base sizes: rectangle 140x60, diamond + 140x80, circle 60x60; cloud icons 78x78 primary / 65x65 secondary; font 12px. +- Main flow left-to-right, one primary axis; <=3-4 lanes/zones; one icon/service. + +Edges: +- <=1 bend per edge (ideally 0); an edge must not cross another shape's bbox; + two edges must not lie on top of each other. +- Give explicit exitX/exitY/entryX/entryY for every non-straight link or the + orthogonal router drives lines through shapes. Vertical link: + exitX=0.5;exitY=1 -> entryX=0.5;entryY=0. For 2+ links on one node, spread the + attach points 0.25 / 0.5 / 0.75. +- Base edge style: + edgeStyle=orthogonalEdgeStyle;rounded=1;orthogonalLoop=1;jettySize=auto;html=1;strokeWidth=2;exitX=1;exitY=0.5;entryX=0;entryY=0.5; +- Edge labels: 1-2 words max, labelBackgroundColor=#F5F5F5;fontSize=11;. Don't + label an obvious flow (Lambda->DynamoDB needs no "Write"); prefer numbering + stages (1,2,3) over many labels. +- Line semantics: solid = main/sync; dashed=1 = async; red dashed + strokeColor=#DD344C = error path. + +Alignment: centre a child under its parent by math, not by eye: +child.x = parent.center_x - child.width/2. + +The linter returns quality WARNINGS (bbox overlap, edge through a shape, +edge-on-edge, gap <150px, label wider than its shape, negative/off-page coords). +They do not block the write — fix them and retry, max 2 iterations.`; + +const CONTAINERS = `# drawio_guide: containers + +Groups/zones are TRANSPARENT containers. A coloured group fill is an instant +"AI-generated" tell — never fill a group. + +- Every group: container=1;dropTarget=1;fillColor=none;. It is a cell with + vertex unset AND edge unset. +- Children set parent="" and their coordinates are RELATIVE to the + group's top-left, not absolute. +- An edge between cells in DIFFERENT containers must be parent="1" (the layer), + otherwise it is clipped to one container and disappears. +- Keep the group title off the group icon: + spacingLeft=40;spacingTop=-4;. +- Leave >=30px padding between children and the group frame. +- Draw edges on the BACK layer (place their BEFORE the shapes in XML) + and keep >=20px between an arrow and a label. + +Swimlanes: style=swimlane;horizontal=0;startSize=110;. Lanes are parent="1"; +their members are children of the lane. + +Example (transparent zone with two children and an internal edge): +\`\`\`xml + + + + + + + + + + + + +\`\`\``; + +const ICONS_AWS = `# drawio_guide: icons-aws + +Two mutually-exclusive AWS icon patterns — mixing them is the #1 cause of empty +boxes. Always call drawio_shapes for the exact resIcon name; do not guess. + +| Level | style | strokeColor | +|---|---|---| +| Service | shape=mxgraph.aws4.resourceIcon;resIcon=mxgraph.aws4. | #ffffff (required) | +| Resource | shape=mxgraph.aws4. | none (required) | + +Full service-level template (fillColor is REQUIRED — the glyph is invisible in +PNG export without it): +\`\`\` +sketch=0;outlineConnect=0;fontColor=#232F3E;fillColor=;strokeColor=#ffffff;dashed=0;verticalLabelPosition=bottom;verticalAlign=top;align=center;html=1;fontSize=12;aspect=fixed;shape=mxgraph.aws4.resourceIcon;resIcon=mxgraph.aws4. +\`\`\` + +Category fillColor: Compute #ED7100, Networking #8C4FFF, Database #C925D1, +Storage #3F8624, Security #DD344C, Integration #E7157B, AI/ML #01A88D. + +Rebrandings (stencil name lags the product name): +- Amazon OpenSearch -> resIcon elasticsearch_service (renamed 2021) +- Amazon EventBridge -> resIcon eventbridge (was CloudWatch Events) +- VPC Peering -> resIcon peering (NOT vpc_peering -> empty box) +- Amazon MSK -> resIcon managed_streaming_for_kafka (NOT msk) +- IAM Identity Center -> resIcon single_sign_on (NOT iam_identity_center) + +Blocklist -> replacement: dynamodb_table -> dynamodb; general_saml_token -> +traditional_server; kinesis_data_streams is unreliable. An unknown service -> +generic resIcon=mxgraph.aws4.general_AWScloud WITH a label; an unnamed coloured +rectangle is forbidden. + +Group stencils (transparent containers): AWS Cloud group_aws_cloud_alt, VPC +group_vpc2, Subnet group_security_group, Account group_account; subnets use +shape=mxgraph.aws4.group;grIcon=mxgraph.aws4.group_public_subnet;.`; + +const ICONS_AZURE = `# drawio_guide: icons-azure + +shape=mxgraph.azure2.* does NOT render in every host. Use the portable +image-style instead: +\`\`\` +image;aspect=fixed;html=1;image=img/lib/azure2//.svg; +\`\`\` + +Known working paths: +- networking/Front_Doors.svg +- app_services/API_Management_Services.svg +- databases/Azure_Cosmos_DB.svg +- identity/Managed_Identities.svg +- management_governance/Monitor.svg +- devops/Application_Insights.svg + +For maximum robustness (e.g. PNG export on a host without the bundled lib), use +an absolute URL fallback for the image: +\`\`\` +https://raw.githubusercontent.com/jgraph/drawio/dev/src/main/webapp/img/lib/azure2//.svg +\`\`\` + +Call drawio_shapes with the service name (e.g. "cosmos", "api management", +"front door") to get the exact image-style string and default 68x68 size.`; + +const CONTENT: Record = { + skeleton: SKELETON, + layout: LAYOUT, + containers: CONTAINERS, + "icons-aws": ICONS_AWS, + "icons-azure": ICONS_AZURE, +}; + +/** + * Return one guide section, or (when `section` is omitted/unknown) an index + * listing the available sections plus a one-line summary each. Each section is + * kept under ~4 KB so pulling it does not bloat the model's context. + */ +export function getGuideSection(section?: string): { + section: string; + content: string; + sections: GuideSection[]; +} { + const key = (section ?? "").trim().toLowerCase() as GuideSection; + if (section && GUIDE_SECTIONS.includes(key)) { + return { section: key, content: CONTENT[key], sections: GUIDE_SECTIONS }; + } + const index = + "# drawio_guide\n\nProgressive-disclosure draw.io authoring reference. " + + "Call drawio_guide(section) with one of:\n" + + "- skeleton — canonical mxGraph XML, sentinels, the three accepted inputs, hard rules\n" + + "- layout — spacing heuristics, edge routing, the layout:\"elk\" option, quality warnings\n" + + "- containers — transparent groups, relative child coords, cross-container edges, swimlanes\n" + + "- icons-aws — the service/resource icon patterns, category colors, rebrandings, blocklist\n" + + "- icons-azure — the portable image-style paths\n\n" + + "Also call drawio_shapes(query) for verified stencil style-strings."; + return { section: "index", content: index, sections: GUIDE_SECTIONS }; +} diff --git a/packages/mcp/src/lib/drawio-layout.ts b/packages/mcp/src/lib/drawio-layout.ts new file mode 100644 index 00000000..e3ff71e4 --- /dev/null +++ b/packages/mcp/src/lib/drawio-layout.ts @@ -0,0 +1,240 @@ +// ELK auto-layout for draw.io models (issue #424, stage 2). The model declares +// the LOGICAL structure (which nodes exist, which containers nest which +// children, which edges connect what) with rough or arbitrary coordinates; this +// module runs an Eclipse Layout Kernel "layered" pass (via elkjs — a pure-JS +// port, no native/browser deps) that HONOURS nested containers as compound +// nodes, then rewrites every vertex's with the computed pixels. +// +// Principle: "the model declares logical structure, the server computes pixels." +// Coordinates ELK returns for a node are relative to its parent, which is +// exactly mxGraph's convention for a child of a container, so they map across +// directly. Container sizes are computed by ELK; leaf sizes are preserved. + +import ELK from "elkjs/lib/elk.bundled.js"; +import { JSDOM } from "jsdom"; +import { normalizeInput, parseCells, type DrawioCell } from "./drawio-xml.js"; + +// Default sizes when a vertex declares no geometry (appendix base sizes). +const DEFAULT_W = 140; +const DEFAULT_H = 60; + +// DoS bounds for the in-process ELK layout. The mxGraph XML is LLM-supplied +// (layout:"elk" in drawio_create/drawio_update) and elkjs runs synchronously on +// the MCP server's event loop, so an unbounded graph would block it for +// seconds-to-minutes. A ~1MB XML (well under the stage-1 16MB cap) can carry +// thousands of nodes. We cap the graph size and race the layout against a +// wall-clock timeout; on either bound we fall back to the ORIGINAL model, the +// same best-effort contract the catch already honours. +// - 500 nodes lays out in well under a second; beyond that ELK cost climbs +// steeply, so refuse and leave the (already-valid) model untouched. +// - Edges dominate the layered-crossing cost, so allow a bit more headroom +// (1000) than nodes but still bound them. +// - 5s is generous for any graph within the caps yet short enough that a +// pathological input can never wedge the server. +const ELK_MAX_NODES = 500; +const ELK_MAX_EDGES = 1000; +const ELK_TIMEOUT_MS = 5000; + +// Spacing is set >=150px on purpose so an ELK layout never trips the linter's +// "gap between adjacent shapes < 150px" quality warning (acceptance #3). +const LAYOUT_OPTIONS: Record = { + "elk.algorithm": "layered", + "elk.direction": "RIGHT", + // Route edges across container boundaries in a single hierarchical pass. + "elk.hierarchyHandling": "INCLUDE_CHILDREN", + "elk.layered.spacing.nodeNodeBetweenLayers": "170", + "elk.spacing.nodeNode": "170", + "elk.spacing.edgeNode": "40", + "elk.spacing.edgeEdge": "30", + "elk.padding": "[top=20,left=20,bottom=20,right=20]", +}; + +// Per-container options: pad children >=30px off the frame (appendix rule) and +// carry the same generous spacing so nested nodes never trip the "gap <150px" +// warning either. +const CONTAINER_OPTIONS: Record = { + "elk.algorithm": "layered", + "elk.direction": "RIGHT", + "elk.padding": "[top=40,left=30,bottom=30,right=30]", + "elk.layered.spacing.nodeNodeBetweenLayers": "170", + "elk.spacing.nodeNode": "170", +}; + +interface ElkNode { + id: string; + width?: number; + height?: number; + x?: number; + y?: number; + children?: ElkNode[]; + layoutOptions?: Record; +} +interface ElkEdge { + id: string; + sources: string[]; + targets: string[]; +} +interface ElkGraph extends ElkNode { + edges?: ElkEdge[]; +} + +/** + * Apply an ELK layered layout to a drawio input and return a full mxGraphModel + * string with rewritten geometry. Accepts the same three input forms as + * drawio_create (a bare model, an , or a list). Async because + * elkjs' layout() is promise-based. On any layout failure the ORIGINAL + * (normalized) model is returned unchanged — layout is best-effort polish, never + * a reason to fail the write. + */ +export async function applyElkLayout(inputXml: string): Promise { + const modelXml = normalizeInput(inputXml); + let cells: DrawioCell[]; + try { + cells = parseCells(modelXml); + } catch { + return modelXml; // unparseable -> let the linter report it downstream + } + + const byId = new Map(cells.map((c) => [c.id, c])); + const vertices = cells.filter( + (c) => c.vertex && c.id !== "0" && c.id !== "1", + ); + if (vertices.length === 0) return modelXml; + + // A vertex is a CONTAINER iff some other vertex names it as parent. + const childrenOf = new Map(); + for (const v of vertices) { + const p = v.parent && byId.get(v.parent)?.vertex ? v.parent : "__root__"; + if (!childrenOf.has(p)) childrenOf.set(p, []); + childrenOf.get(p)!.push(v); + } + const isContainer = (id: string) => childrenOf.has(id); + + const buildNode = (v: DrawioCell): ElkNode => { + const kids = childrenOf.get(v.id); + const node: ElkNode = { id: v.id }; + if (kids && kids.length > 0) { + node.children = kids.map(buildNode); + node.layoutOptions = { ...CONTAINER_OPTIONS }; + } else { + node.width = v.geometry.width ?? DEFAULT_W; + node.height = v.geometry.height ?? DEFAULT_H; + } + return node; + }; + + const roots = (childrenOf.get("__root__") ?? []).map(buildNode); + + // All edges at the root; INCLUDE_CHILDREN lets them span the hierarchy. Only + // edges whose endpoints are laid-out vertices are handed to ELK. + const vertexIds = new Set(vertices.map((v) => v.id)); + const edges: ElkEdge[] = []; + for (const c of cells) { + if (!c.edge || !c.source || !c.target) continue; + if (!vertexIds.has(c.source) || !vertexIds.has(c.target)) continue; + edges.push({ id: c.id || `e${edges.length}`, sources: [c.source], targets: [c.target] }); + } + + // DoS guard: refuse to lay out an oversized LLM-supplied graph. elkjs runs + // in-process on the event loop, so bound the work before we ever call it and + // return the original model unchanged (best-effort, same as the catch below). + if (vertices.length > ELK_MAX_NODES || edges.length > ELK_MAX_EDGES) { + return modelXml; + } + + const graph: ElkGraph = { + id: "root", + layoutOptions: LAYOUT_OPTIONS, + children: roots, + edges, + }; + + let laid: ElkGraph; + let timer: ReturnType | undefined; + try { + // elkjs ships a CJS default export whose interop shape varies across + // module systems; resolve the real constructor at runtime, then cast (the + // runtime call is verified — see the layout unit test). + const Ctor: any = (ELK as any).default ?? ELK; + const elk = new Ctor(); + // Race the layout against a wall-clock timeout so a graph that is under the + // node/edge caps but still pathologically slow can never wedge the server. + const timeout = new Promise((_, reject) => { + timer = setTimeout( + () => reject(new Error("ELK layout timed out")), + ELK_TIMEOUT_MS, + ); + }); + laid = (await Promise.race([elk.layout(graph as any), timeout])) as ElkGraph; + } catch { + return modelXml; // best-effort: keep the model as-is on timeout or ELK failure + } finally { + if (timer) clearTimeout(timer); + } + + // Collect computed geometry per node id (coords are parent-relative already). + const geo = new Map(); + const walk = (n: ElkNode) => { + if (n.id !== "root") { + geo.set(n.id, { + x: Math.round(n.x ?? 0), + y: Math.round(n.y ?? 0), + w: Math.round(n.width ?? DEFAULT_W), + h: Math.round(n.height ?? DEFAULT_H), + }); + } + for (const c of n.children ?? []) walk(c); + }; + walk(laid); + + return rewriteGeometry(modelXml, geo, isContainer); +} + +/** + * Rewrite each vertex cell's x/y (and width/height for containers, + * whose size ELK computed) using the DOM, then serialize back. Leaf sizes are + * left untouched. Edges and non-geometry attributes are preserved verbatim. + */ +function rewriteGeometry( + modelXml: string, + geo: Map, + isContainer: (id: string) => boolean, +): string { + const dom = new JSDOM(""); + const parser = new dom.window.DOMParser(); + const doc = parser.parseFromString(modelXml, "application/xml"); + if (doc.getElementsByTagName("parsererror").length > 0) return modelXml; + + const cellEls = doc.getElementsByTagName("mxCell"); + for (let i = 0; i < cellEls.length; i++) { + const el = cellEls[i]; + const id = el.getAttribute("id") || ""; + const g = geo.get(id); + if (!g) continue; + let geoEl: any = null; + for (let j = 0; j < el.childNodes.length; j++) { + const ch = el.childNodes[j]; + if (ch.nodeType === 1 && (ch as any).tagName === "mxGeometry") { + geoEl = ch; + break; + } + } + if (!geoEl) { + geoEl = doc.createElement("mxGeometry"); + geoEl.setAttribute("as", "geometry"); + el.appendChild(geoEl); + } + geoEl.setAttribute("x", String(g.x)); + geoEl.setAttribute("y", String(g.y)); + // Containers take ELK's computed size; leaves keep their authored size. + if (isContainer(id) || !geoEl.hasAttribute("width")) { + geoEl.setAttribute("width", String(g.w)); + } + if (isContainer(id) || !geoEl.hasAttribute("height")) { + geoEl.setAttribute("height", String(g.h)); + } + } + + const ser = new dom.window.XMLSerializer(); + return ser.serializeToString(doc.documentElement); +} diff --git a/packages/mcp/src/lib/drawio-shapes.ts b/packages/mcp/src/lib/drawio-shapes.ts new file mode 100644 index 00000000..7ca6e9a4 --- /dev/null +++ b/packages/mcp/src/lib/drawio-shapes.ts @@ -0,0 +1,420 @@ +// Verified draw.io shape catalog for the `drawio_shapes` tool (issue #424, +// stage 2). This is the fix for AI-generated diagrams' #1 defect: guessed +// `shape=mxgraph.*` names that render as EMPTY BOXES because the stencil does +// not exist. Instead of guessing, the model queries this catalog and gets back +// an exact, verified style-string + the stencil's default width/height. +// +// DATA SOURCE — the bundled index is the REAL jgraph/drawio-mcp shape index +// (`shape-search/search-index.json`, Apache-2.0, ~10 446 shapes), fetched +// verbatim and gzip-compressed to `packages/mcp/data/drawio-shape-index.json.gz` +// (~4.7 MB -> ~430 KB). Each record is `{ style, w, h, title, tags, type }`. +// +// REGENERATING THE INDEX (keeps the catalog from going stale as draw.io ships +// new stencils): jgraph publishes `shape-search/generate-index.js`, which +// rebuilds `search-index.json` from a draw.io release's `app.min.js`. To update: +// 1. clone https://github.com/jgraph/drawio-mcp (Apache-2.0) +// 2. run `node shape-search/generate-index.js` per its README +// 3. `gzip -9 -c search-index.json > packages/mcp/data/drawio-shape-index.json.gz` +// The record shape and this module's search stay unchanged. +// +// CURATED OVERLAY — on top of the raw index this module carries a small, +// hand-maintained overlay drawn from the issue #424 appendix (the aws- +// architecture-diagram-skill knowledge): AWS service rebrandings whose stencil +// name lags the product name, a BLOCKLIST of known-broken stencils mapped to +// working replacements, the category fillColor palette, the AWS group/subnet +// stencils, and the Azure image-style paths. The overlay is applied BEFORE the +// raw search so a query for a rebranded/blocked name returns the correct answer +// with an explanatory note instead of the empty-box stencil. + +import { readFileSync } from "node:fs"; +import { gunzipSync } from "node:zlib"; + +/** A single catalog record as returned to the model. */ +export interface ShapeResult { + /** The exact draw.io style-string to put on the cell. */ + style: string; + /** Default width in px for this stencil. */ + w: number; + /** Default height in px for this stencil. */ + h: number; + /** Human-readable stencil name. */ + title: string; + /** "vertex" | "edge" (from the index). */ + type: string; + /** AWS category (Compute/Database/…) when derivable, else undefined. */ + category?: string; + /** + * Present when the overlay rewrote/annotated the answer: a rebrand, a + * blocklist replacement, or a usage hint. The model should surface it. + */ + note?: string; +} + +/** Raw record shape in the bundled index. */ +interface IndexRecord { + style: string; + w: number; + h: number; + title: string; + tags: string; + type: string; +} + +// --- AWS category fillColor palette (appendix) ----------------------------- +// Service-level icons MUST carry a fillColor (invisible in PNG export +// otherwise); the color is the AWS category color. +export const AWS_CATEGORY_FILL: Record = { + Compute: "#ED7100", + Networking: "#8C4FFF", + Database: "#C925D1", + Storage: "#3F8624", + Security: "#DD344C", + Integration: "#E7157B", + "AI/ML": "#01A88D", +}; + +/** Reverse lookup: fillColor hex -> category name (for annotating results). */ +const FILL_TO_CATEGORY: Record = Object.fromEntries( + Object.entries(AWS_CATEGORY_FILL).map(([k, v]) => [v.toLowerCase(), k]), +); + +/** + * Build the canonical service-level AWS icon style for a resIcon name. Mirrors + * the appendix's full template: strokeColor=#ffffff is MANDATORY and fillColor + * is the category color (defaults to AWS ink #232F3E when the category is + * unknown, so the glyph is never invisible). + */ +export function awsServiceStyle(resIcon: string, category?: string): string { + const fill = (category && AWS_CATEGORY_FILL[category]) || "#232F3E"; + return ( + "sketch=0;outlineConnect=0;fontColor=#232F3E;gradientColor=none;" + + `fillColor=${fill};strokeColor=#ffffff;dashed=0;verticalLabelPosition=bottom;` + + "verticalAlign=top;align=center;html=1;fontSize=12;fontStyle=0;aspect=fixed;" + + `shape=mxgraph.aws4.resourceIcon;resIcon=mxgraph.aws4.${resIcon}` + ); +} + +// --- AWS rebrandings (appendix "gotcha" table) ----------------------------- +// The stencil name lags the AWS product name; a naive query for the product +// name would miss (or return an empty box). Each alias maps to the REAL resIcon. +interface Rebrand { + aliases: string[]; + resIcon: string; + category?: string; + note: string; +} +export const AWS_REBRANDS: Rebrand[] = [ + { + aliases: ["opensearch", "open search", "amazon opensearch"], + resIcon: "elasticsearch_service", + category: "Database", + note: "Amazon OpenSearch's stencil is still named `elasticsearch_service` (renamed in 2021).", + }, + { + aliases: ["eventbridge", "event bridge", "cloudwatch events"], + resIcon: "eventbridge", + category: "Integration", + note: "Amazon EventBridge uses resIcon `eventbridge` (formerly CloudWatch Events).", + }, + { + aliases: ["vpc peering", "peering"], + resIcon: "peering", + category: "Networking", + note: "VPC Peering is resIcon `peering`, NOT `vpc_peering` (which renders empty).", + }, + { + aliases: ["msk", "kafka", "managed streaming", "amazon msk"], + resIcon: "managed_streaming_for_kafka", + category: "Integration", + note: "Amazon MSK is resIcon `managed_streaming_for_kafka`, NOT `msk`.", + }, + { + aliases: ["iam identity center", "identity center", "sso", "single sign on"], + resIcon: "single_sign_on", + category: "Security", + note: "IAM Identity Center is resIcon `single_sign_on`, NOT `iam_identity_center`.", + }, +]; + +// --- BLOCKLIST of broken stencils (appendix) ------------------------------- +// A query that names one of these gets the working replacement + a note; the +// broken stencil is never returned. +interface Blocked { + bad: string; + good: string; + goodStyle?: (idx: IndexRecord[]) => ShapeResult | null; + note: string; +} +export const AWS_BLOCKLIST: Blocked[] = [ + { + bad: "dynamodb_table", + good: "dynamodb", + note: "`dynamodb_table` renders as an empty box; use resIcon `dynamodb`.", + }, + { + bad: "general_saml_token", + good: "traditional_server", + note: "`general_saml_token` is broken; use resIcon `traditional_server`.", + }, + { + bad: "kinesis_data_streams", + good: "kinesis_data_streams", + note: "`kinesis_data_streams` is unreliable across draw.io versions; verify it renders, or fall back to resIcon `kinesis`.", + }, +]; + +// --- AWS group / container stencils (appendix) ----------------------------- +// Groups are transparent containers; these are the verified stencil names. +export const AWS_GROUP_STENCILS: ShapeResult[] = [ + { + title: "AWS Cloud (group)", + style: + "points=[[0,0],[0.25,0],[0.5,0],[0.75,0],[1,0],[1,0.25],[1,0.5],[1,0.75],[1,1],[0.75,1],[0.5,1],[0.25,1],[0,1],[0,0.75],[0,0.5],[0,0.25]];" + + "outlineConnect=0;gradientColor=none;html=1;whiteSpace=wrap;fontSize=12;fontStyle=0;container=1;" + + "pointerEvents=0;collapsible=0;recursiveResize=0;shape=mxgraph.aws4.group;grIcon=mxgraph.aws4.group_aws_cloud_alt;" + + "strokeColor=#232F3E;fillColor=none;verticalAlign=top;align=left;spacingLeft=30;fontColor=#232F3E;dashed=0;", + w: 400, + h: 300, + type: "vertex", + note: "AWS Cloud boundary — transparent container (grIcon=group_aws_cloud_alt).", + }, + { + title: "VPC (group)", + style: + "points=[[0,0],[0.25,0],[0.5,0],[0.75,0],[1,0],[1,0.25],[1,0.5],[1,0.75],[1,1],[0.75,1],[0.5,1],[0.25,1],[0,1],[0,0.75],[0,0.5],[0,0.25]];" + + "outlineConnect=0;gradientColor=none;html=1;whiteSpace=wrap;fontSize=12;fontStyle=0;container=1;" + + "pointerEvents=0;collapsible=0;recursiveResize=0;shape=mxgraph.aws4.group;grIcon=mxgraph.aws4.group_vpc2;" + + "strokeColor=#8C4FFF;fillColor=none;verticalAlign=top;align=left;spacingLeft=30;fontColor=#8C4FFF;dashed=0;", + w: 350, + h: 250, + type: "vertex", + note: "VPC boundary — transparent container (grIcon=group_vpc2).", + }, + { + title: "Public Subnet (group)", + style: + "sketch=0;outlineConnect=0;gradientColor=none;html=1;whiteSpace=wrap;fontSize=12;fontStyle=0;container=1;" + + "pointerEvents=0;collapsible=0;recursiveResize=0;shape=mxgraph.aws4.group;grIcon=mxgraph.aws4.group_public_subnet;" + + "grStroke=0;strokeColor=none;fillColor=#E9F3E6;verticalAlign=top;align=left;spacingLeft=30;fontColor=#248814;dashed=0;", + w: 300, + h: 200, + type: "vertex", + note: "Public subnet — transparent container (grIcon=group_public_subnet).", + }, + { + title: "Private Subnet (group)", + style: + "sketch=0;outlineConnect=0;gradientColor=none;html=1;whiteSpace=wrap;fontSize=12;fontStyle=0;container=1;" + + "pointerEvents=0;collapsible=0;recursiveResize=0;shape=mxgraph.aws4.group;grIcon=mxgraph.aws4.group_private_subnet;" + + "grStroke=0;strokeColor=none;fillColor=#E6F2F8;verticalAlign=top;align=left;spacingLeft=30;fontColor=#147EBA;dashed=0;", + w: 300, + h: 200, + type: "vertex", + note: "Private subnet — transparent container (grIcon=group_private_subnet).", + }, +]; + +// --- Azure image-style stencils (appendix) --------------------------------- +// `shape=mxgraph.azure2.*` does not render in every host; the image-style path +// is the portable form. These are the verified known-working paths. +interface AzureIcon { + aliases: string[]; + path: string; + title: string; +} +const AZURE_ICONS: AzureIcon[] = [ + { aliases: ["front door", "front doors"], path: "networking/Front_Doors.svg", title: "Azure Front Door" }, + { aliases: ["api management", "apim"], path: "app_services/API_Management_Services.svg", title: "Azure API Management" }, + { aliases: ["cosmos", "cosmos db"], path: "databases/Azure_Cosmos_DB.svg", title: "Azure Cosmos DB" }, + { aliases: ["managed identity", "managed identities"], path: "identity/Managed_Identities.svg", title: "Azure Managed Identity" }, + { aliases: ["azure monitor", "monitor"], path: "management_governance/Monitor.svg", title: "Azure Monitor" }, + { aliases: ["application insights", "app insights"], path: "devops/Application_Insights.svg", title: "Azure Application Insights" }, +]; + +/** Build the portable Azure image-style for a lib path (appendix template). */ +export function azureImageStyle(path: string): string { + return `sketch=0;points=[[0,0,0],[0.25,0,0],[0.5,0,0],[0.75,0,0],[1,0,0],[0,1,0],[0.25,1,0],[0.5,1,0],[0.75,1,0],[1,1,0],[0,0.25,0],[0,0.5,0],[0,0.75,0],[1,0.25,0],[1,0.5,0],[1,0.75,0]];shadow=0;dashed=0;html=1;strokeColor=none;fillColor=#5E9BD9;labelPosition=center;verticalLabelPosition=bottom;verticalAlign=top;align=center;outlineConnect=0;image;aspect=fixed;image=img/lib/azure2/${path};`; +} + +// --- index loading (lazy, cached) ------------------------------------------ + +let _index: IndexRecord[] | null = null; + +/** Path to the bundled gzipped index, resolved relative to the built module. */ +function indexPath(): URL { + // build/lib/drawio-shapes.js -> ../../data/… -> packages/mcp/data/… + return new URL("../../data/drawio-shape-index.json.gz", import.meta.url); +} + +/** Load + decompress + parse the bundled index once, then cache it. */ +export function loadShapeIndex(): IndexRecord[] { + if (_index) return _index; + const gz = readFileSync(indexPath()); + const json = gunzipSync(gz).toString("utf-8"); + const arr = JSON.parse(json) as IndexRecord[]; + _index = arr; + return arr; +} + +/** Derive an AWS category from a service-level icon's fillColor, if present. */ +function categoryOf(style: string): string | undefined { + const m = /fillColor=(#[0-9a-fA-F]{6})/.exec(style); + if (!m) return undefined; + return FILL_TO_CATEGORY[m[1].toLowerCase()]; +} + +function toResult(r: IndexRecord): ShapeResult { + return { + style: r.style, + w: r.w, + h: r.h, + title: r.title, + type: r.type, + category: categoryOf(r.style), + }; +} + +/** Find the best index record whose style carries `resIcon=`. */ +function findByResIcon(idx: IndexRecord[], name: string): IndexRecord | null { + const needle = `resIcon=mxgraph.aws4.${name}`; + // Prefer the service-level resourceIcon form; fall back to any style match. + let fallback: IndexRecord | null = null; + for (const r of idx) { + if (r.style.includes(needle) && r.style.includes("resourceIcon")) return r; + if (!fallback && r.style.includes(needle)) fallback = r; + } + return fallback; +} + +/** + * Score a record against a lowercased query. Higher is better; 0 = no match. + * Exact title match ranks highest, then title substring, tag word, then a loose + * style/tag substring. This is a cheap substring+token scorer, not a real fuzzy + * matcher, which is plenty for the "give me the lambda icon" use case. + */ +function score(r: IndexRecord, q: string): number { + const title = r.title.toLowerCase(); + const tags = r.tags.toLowerCase(); + const style = r.style.toLowerCase(); + let s = title === q ? 100 : 0; + if (title !== q && title.includes(q)) s += 40 - Math.min(20, title.length - q.length); + const words = q.split(/\s+/).filter(Boolean); + for (const w of words) { + if (title.includes(w)) s += 12; + if (new RegExp(`(^|\\W)${escapeRe(w)}(\\W|$)`).test(tags)) s += 8; + else if (tags.includes(w)) s += 4; + if (style.includes(w)) s += 2; + } + // Prefer the current AWS icon generation (aws4) over the deprecated aws3 + // stencils, which are the older visual style and often not what's wanted. + if (s > 0) { + if (style.includes("mxgraph.aws4")) s += 6; + else if (style.includes("mxgraph.aws3")) s -= 12; + } + return s; +} + +function escapeRe(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +export interface SearchShapesOptions { + category?: string; + limit?: number; +} + +/** + * Search the catalog. Applies the curated overlay first (blocklist replacement, + * AWS rebrand, AWS group stencils, Azure image-style), then substring/tag/fuzzy + * search over the bundled ~10 446-shape index. Returns up to `limit` results + * (default 12) with exact style-strings and default sizes. + */ +export function searchShapes( + query: string, + opts: SearchShapesOptions = {}, +): ShapeResult[] { + const limit = Math.max(1, Math.min(50, opts.limit ?? 12)); + const q = query.trim().toLowerCase(); + if (q === "") return []; + const idx = loadShapeIndex(); + const out: ShapeResult[] = []; + const seen = new Set(); + const push = (r: ShapeResult) => { + if (seen.has(r.style)) return; + seen.add(r.style); + out.push(r); + }; + + // 1. BLOCKLIST: a query naming a broken stencil returns the replacement. + for (const b of AWS_BLOCKLIST) { + if (q.includes(b.bad) || b.bad.includes(q.replace(/\s+/g, "_"))) { + const rec = findByResIcon(idx, b.good); + if (rec) push({ ...toResult(rec), note: b.note }); + } + } + + // 2. AWS rebrandings: surface the correct resIcon with the rename note. + for (const rb of AWS_REBRANDS) { + if (rb.aliases.some((a) => q === a || q.includes(a) || a.includes(q))) { + const rec = findByResIcon(idx, rb.resIcon); + if (rec) { + push({ ...toResult(rec), category: rec ? categoryOf(rec.style) ?? rb.category : rb.category, note: rb.note }); + } else { + push({ + style: awsServiceStyle(rb.resIcon, rb.category), + w: 78, + h: 78, + title: rb.resIcon, + type: "vertex", + category: rb.category, + note: rb.note, + }); + } + } + } + + // 3. Azure image-style icons. + for (const az of AZURE_ICONS) { + if (az.aliases.some((a) => q.includes(a) || a.includes(q))) { + push({ + style: azureImageStyle(az.path), + w: 68, + h: 68, + title: az.title, + type: "vertex", + category: "Azure", + note: "Azure: portable image-style (shape=mxgraph.azure2.* does not render in every host).", + }); + } + } + + // 4. AWS group/container stencils. + if (/\b(group|container|boundary|vpc|subnet|cloud|account)\b/.test(q)) { + for (const g of AWS_GROUP_STENCILS) { + if (g.title.toLowerCase().includes(q) || q.split(/\s+/).some((w) => g.title.toLowerCase().includes(w))) { + push(g); + } + } + } + + // 5. General index search (substring + tags + loose fuzzy). + const catFilter = opts.category?.toLowerCase(); + const scored: { r: IndexRecord; s: number }[] = []; + for (const r of idx) { + const s = score(r, q); + if (s <= 0) continue; + if (catFilter) { + const cat = categoryOf(r.style)?.toLowerCase(); + const inStyle = r.style.toLowerCase().includes(catFilter); + if (cat !== catFilter && !inStyle) continue; + } + scored.push({ r, s }); + } + scored.sort((a, b) => b.s - a.s || a.r.title.length - b.r.title.length); + for (const { r } of scored) { + if (out.length >= limit) break; + push(toResult(r)); + } + + return out.slice(0, limit); +} diff --git a/packages/mcp/src/lib/drawio-xml.ts b/packages/mcp/src/lib/drawio-xml.ts index 56f1b840..feb9df50 100644 --- a/packages/mcp/src/lib/drawio-xml.ts +++ b/packages/mcp/src/lib/drawio-xml.ts @@ -461,6 +461,308 @@ export function absolutePos( return { x, y }; } +// --- quality warnings (geometry, non-blocking) ----------------------------- +// +// These are computed purely from geometry — NO rendering — and are returned as +// WARNINGS (never errors): they do not block the write, they nudge the model to +// self-correct ("fix the warnings and retry, max 2 iterations"). They replace +// the vision-self-check a render backend would have done. + +interface Rect { + x: number; + y: number; + w: number; + h: number; +} + +/** Absolute rect of a vertex (following the container chain), or null. */ +function rectOf(cell: DrawioCell, byId: Map): Rect | null { + if (!cell.vertex || !cell.geometry.hasGeometry) return null; + const g = cell.geometry; + if (g.width == null || g.height == null) return null; + const { x, y } = absolutePos(cell, byId); + return { x, y, w: g.width, h: g.height }; +} + +/** True if `ancestorId` is somewhere up `cell`'s parent chain. */ +function isAncestor( + ancestorId: string, + cell: DrawioCell, + byId: Map, +): boolean { + const seen = new Set([cell.id]); + let p = cell.parent; + while (p && !seen.has(p)) { + if (p === ancestorId) return true; + seen.add(p); + p = byId.get(p)?.parent; + } + return false; +} + +/** Strict interior overlap of two rects (touching edges do NOT count). */ +function rectsOverlap(a: Rect, b: Rect): boolean { + return a.x < b.x + b.w && b.x < a.x + a.w && a.y < b.y + b.h && b.y < a.y + a.h; +} + +function center(r: Rect): { x: number; y: number } { + return { x: r.x + r.w / 2, y: r.y + r.h / 2 }; +} + +/** + * Liang-Barsky: does segment p->q pass through the INTERIOR of rect r? Used to + * detect an edge crossing a shape that is not one of its endpoints. + */ +function segCrossesRect( + a: { x: number; y: number }, + b: { x: number; y: number }, + r: Rect, +): boolean { + const dx = b.x - a.x; + const dy = b.y - a.y; + // Canonical Liang-Barsky: for each of the 4 slabs, p*t <= q. + const p = [-dx, dx, -dy, dy]; + const q = [a.x - r.x, r.x + r.w - a.x, a.y - r.y, r.y + r.h - a.y]; + let t0 = 0; + let t1 = 1; + for (let i = 0; i < 4; i++) { + if (p[i] === 0) { + if (q[i] < 0) return false; // parallel to this slab AND outside it + continue; + } + const t = q[i] / p[i]; + if (p[i] < 0) { + if (t > t1) return false; + if (t > t0) t0 = t; + } else { + if (t < t0) return false; + if (t < t1) t1 = t; + } + } + return t1 > t0; // strictly non-degenerate overlap with the rect interior +} + +function cross( + ox: number, + oy: number, + ax: number, + ay: number, + bx: number, + by: number, +): number { + return (ax - ox) * (by - oy) - (ay - oy) * (bx - ox); +} + +/** Collinear + overlapping test for two straight segments (edge-on-edge). */ +function segmentsOverlap( + a1: { x: number; y: number }, + a2: { x: number; y: number }, + b1: { x: number; y: number }, + b2: { x: number; y: number }, +): boolean { + const EPS = 1; + // b1 and b2 must be (near-)collinear with segment a. + if ( + Math.abs(cross(a1.x, a1.y, a2.x, a2.y, b1.x, b1.y)) > EPS * dist(a1, a2) || + Math.abs(cross(a1.x, a1.y, a2.x, a2.y, b2.x, b2.y)) > EPS * dist(a1, a2) + ) { + return false; + } + // Project all four points onto the dominant axis and test 1-D overlap length. + const horizontal = Math.abs(a2.x - a1.x) >= Math.abs(a2.y - a1.y); + const pa = horizontal ? [a1.x, a2.x] : [a1.y, a2.y]; + const pb = horizontal ? [b1.x, b2.x] : [b1.y, b2.y]; + const loA = Math.min(pa[0], pa[1]); + const hiA = Math.max(pa[0], pa[1]); + const loB = Math.min(pb[0], pb[1]); + const hiB = Math.max(pb[0], pb[1]); + const overlap = Math.min(hiA, hiB) - Math.max(loA, loB); + return overlap > 5; // >5px of shared collinear run +} + +function dist( + a: { x: number; y: number }, + b: { x: number; y: number }, +): number { + return Math.hypot(a.x - b.x, a.y - b.y) || 1; +} + +/** Approximate rendered text width (px) of a cell value at a font size. */ +function estimateLabelWidth(value: string, fontSize: number): number { + // Decode explicit line breaks, strip tags/entities, take the longest line. + const lines = value + .replace(/ /gi, "\n") + .replace(//gi, "\n") + .replace(/<[^>]+>/g, "") + .replace(/&[a-z]+;/gi, "x") + .split("\n"); + let longest = 0; + for (const l of lines) longest = Math.max(longest, l.trim().length); + // ~0.6em per glyph is a decent average for proportional fonts. + return longest * fontSize * 0.6; +} + +/** Page size declared on the model root, defaulting to Letter (850x1100). */ +function parsePageSize(modelXml: string): { w: number; h: number } { + const w = /pageWidth="(\d+)"/.exec(modelXml); + const h = /pageHeight="(\d+)"/.exec(modelXml); + return { + w: w ? Number(w[1]) : 850, + h: h ? Number(h[1]) : 1100, + }; +} + +/** Minimum required gap between adjacent shapes (appendix heuristic). */ +export const MIN_SHAPE_GAP = 150; + +/** + * Compute the geometry-derived quality warnings for a parsed model. Each is a + * `[rule] message` string. Pure — no rendering, no I/O. + */ +export function computeQualityWarnings( + cells: DrawioCell[], + modelXml?: string, +): string[] { + const warnings: string[] = []; + const byId = new Map(cells.map((c) => [c.id, c])); + const verts = cells.filter((c) => c.vertex && c.id !== "0" && c.id !== "1"); + const isContainer = (id: string) => + verts.some((v) => v.parent === id); + const rects = new Map(); + for (const v of verts) { + const r = rectOf(v, byId); + if (r) rects.set(v.id, r); + } + + // 1. Shape bbox overlap (excluding a container overlapping its own child). + for (let i = 0; i < verts.length; i++) { + for (let j = i + 1; j < verts.length; j++) { + const a = verts[i]; + const b = verts[j]; + const ra = rects.get(a.id); + const rb = rects.get(b.id); + if (!ra || !rb) continue; + if (isAncestor(a.id, b, byId) || isAncestor(b.id, a, byId)) continue; + if (rectsOverlap(ra, rb)) { + warnings.push( + `[shape-overlap] shapes "${a.id}" and "${b.id}" overlap; separate them (>=${MIN_SHAPE_GAP}px apart) or use layout:"elk"`, + ); + } + } + } + + // 2. Edge passing through a non-endpoint LEAF shape's bbox. + const edges = cells.filter((c) => c.edge); + for (const e of edges) { + if (!e.source || !e.target) continue; + const rs = rects.get(e.source); + const rt = rects.get(e.target); + if (!rs || !rt) continue; + const p = center(rs); + const q = center(rt); + for (const v of verts) { + if (v.id === e.source || v.id === e.target) continue; + if (isContainer(v.id)) continue; // an edge legitimately crosses container frames + const rv = rects.get(v.id); + if (!rv) continue; + // shrink to avoid flagging a graze at a shared layer boundary + const shrunk: Rect = { x: rv.x + 6, y: rv.y + 6, w: rv.w - 12, h: rv.h - 12 }; + if (shrunk.w <= 0 || shrunk.h <= 0) continue; + if (segCrossesRect(p, q, shrunk)) { + warnings.push( + `[edge-through-shape] edge "${e.id}" passes through shape "${v.id}" (not its source/target); add exitX/exitY/entryX/entryY or a waypoint`, + ); + break; + } + } + } + + // 3. Edge-on-edge overlap (parallel duplicates or collinear shared runs). + const edgeSegs: { id: string; a: any; b: any; key: string }[] = []; + for (const e of edges) { + if (!e.source || !e.target) continue; + const rs = rects.get(e.source); + const rt = rects.get(e.target); + if (!rs || !rt) continue; + const key = [e.source, e.target].sort().join("::"); + edgeSegs.push({ id: e.id, a: center(rs), b: center(rt), key }); + } + for (let i = 0; i < edgeSegs.length; i++) { + for (let j = i + 1; j < edgeSegs.length; j++) { + const ea = edgeSegs[i]; + const eb = edgeSegs[j]; + const dup = ea.key === eb.key; + if (dup || segmentsOverlap(ea.a, ea.b, eb.a, eb.b)) { + warnings.push( + `[edge-overlap] edges "${ea.id}" and "${eb.id}" lie on top of each other; offset one (distinct exit/entry points) or reroute`, + ); + } + } + } + + // 4. Adjacent SIBLING leaf shapes closer than MIN_SHAPE_GAP. + for (let i = 0; i < verts.length; i++) { + for (let j = i + 1; j < verts.length; j++) { + const a = verts[i]; + const b = verts[j]; + if ((a.parent ?? "") !== (b.parent ?? "")) continue; + if (isContainer(a.id) || isContainer(b.id)) continue; + const ra = rects.get(a.id); + const rb = rects.get(b.id); + if (!ra || !rb || rectsOverlap(ra, rb)) continue; + const yOverlap = ra.y < rb.y + rb.h && rb.y < ra.y + ra.h; + const xOverlap = ra.x < rb.x + rb.w && rb.x < ra.x + ra.w; + let gap = Infinity; + if (yOverlap) { + gap = Math.min( + gap, + ra.x >= rb.x ? ra.x - (rb.x + rb.w) : rb.x - (ra.x + ra.w), + ); + } + if (xOverlap) { + gap = Math.min( + gap, + ra.y >= rb.y ? ra.y - (rb.y + rb.h) : rb.y - (ra.y + ra.h), + ); + } + if (gap > 0 && gap < MIN_SHAPE_GAP) { + warnings.push( + `[gap-too-small] shapes "${a.id}" and "${b.id}" are ${Math.round(gap)}px apart (<${MIN_SHAPE_GAP}px); increase spacing`, + ); + } + } + } + + // 5. Label visibly wider than its shape (skip labels drawn OUTSIDE the shape). + for (const v of verts) { + if (!v.value || isContainer(v.id)) continue; + if (v.styleMap.verticalLabelPosition || v.styleMap.labelPosition) continue; + const r = rects.get(v.id); + if (!r) continue; + const fontSize = Number(v.styleMap.fontSize) || 12; + const est = estimateLabelWidth(v.value, fontSize); + if (est > r.w * 1.15) { + warnings.push( + `[label-overflow] label of "${v.id}" (~${Math.round(est)}px) is wider than its shape (${r.w}px); widen it, shorten the text, or wrap with `, + ); + } + } + + // 6. Negative / off-page (top-left) coordinates. + const page = parsePageSize(modelXml ?? ""); + for (const v of verts) { + const r = rects.get(v.id); + if (!r) continue; + if (r.x < 0 || r.y < 0) { + warnings.push( + `[out-of-bounds] shape "${v.id}" has negative coordinates (${Math.round(r.x)},${Math.round(r.y)}); move it into the positive quadrant (page ${page.w}x${page.h})`, + ); + } + } + + return warnings; +} + // --- linter ---------------------------------------------------------------- /** @@ -755,12 +1057,15 @@ export function prepareModel(inputXml: string): PreparedModel { const modelXml = normalizeXml(rawModel); const bbox = computeBBox(cells); const cellCount = cells.filter((c) => c.id !== "0" && c.id !== "1").length; + // Geometry quality warnings (non-blocking) are appended to any structural + // warnings from the linter. The model surfaces these and can self-correct. + const quality = computeQualityWarnings(cells, modelXml); return { modelXml, cells, bbox, cellCount, - warnings, + warnings: [...warnings, ...quality], hash: mxHash(modelXml), }; } diff --git a/packages/mcp/src/server-instructions.ts b/packages/mcp/src/server-instructions.ts index eda032ed..704dfa3b 100644 --- a/packages/mcp/src/server-instructions.ts +++ b/packages/mcp/src/server-instructions.ts @@ -41,7 +41,7 @@ import { SHARED_TOOL_SPECS, SharedToolSpec } from "./tool-specs.js"; export const ROUTING_PROSE = "Docmost editing guide — choose the tool by intent. The at the end lists every tool with a one-line purpose; the notes below are the routing hints for WHEN to reach for each.\n" + "READ: find a page -> search (workspace-wide full-text); list -> list_pages / list_spaces. Locate blocks and their ids CHEAPLY -> get_outline (compact top-level map; start here, not get_page_json). One block's subtree -> get_node (by attrs.id, or \"#\" for tables, which carry no id). Find every occurrence of a string/regex ON a page (and where each is) -> search_in_page, NOT block-by-block get_node — it returns each hit's node ref + block index + context for a targeted comment. Whole page -> get_page (Markdown, lossy; inline tags are comment anchors — markup, not text) or get_page_json (lossless ProseMirror with block ids). Hand a huge page (with images) to an external consumer without pulling it through the model context -> stash_page (returns a short-lived anonymous URL).\n" + - "EDIT: fix wording/typos/numbers -> edit_page_text (find/replace inside blocks, no node id needed). Change ONE block (paragraph/heading/callout/etc.) structurally -> patch_node (by attrs.id from get_outline). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Tables -> table_get / table_update_cell / table_insert_row / table_delete_row (address by \"#\" from get_outline; table nodes have no attrs.id). Images -> insert_image (add from a web URL) / replace_image (swap an existing image). Draw.io diagrams -> drawio_create (create from mxGraph XML and insert), drawio_get (read a diagram as mxGraph XML + a hash), drawio_update (replace a diagram; pass the hash from drawio_get as baseHash for optimistic locking). Footnotes -> insert_footnote. Bulk/structural rewrite -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" + + "EDIT: fix wording/typos/numbers -> edit_page_text (find/replace inside blocks, no node id needed). Change ONE block (paragraph/heading/callout/etc.) structurally -> patch_node (by attrs.id from get_outline). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Tables -> table_get / table_update_cell / table_insert_row / table_delete_row (address by \"#\" from get_outline; table nodes have no attrs.id). Images -> insert_image (add from a web URL) / replace_image (swap an existing image). Draw.io diagrams -> drawio_create (create from mxGraph XML and insert), drawio_get (read a diagram as mxGraph XML + a hash), drawio_update (replace a diagram; pass the hash from drawio_get as baseHash for optimistic locking); before authoring a diagram, drawio_shapes (look up verified stencil style-strings so a shape name never renders as an empty box) and drawio_guide (on-demand authoring reference: skeleton/layout/containers/icons-aws/icons-azure), and pass layout:\"elk\" to drawio_create/drawio_update to auto-place nodes. Footnotes -> insert_footnote. Bulk/structural rewrite -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" + "PAGES: new -> create_page (Markdown). Rename (title only) -> rename_page. Move -> move_page. Delete -> delete_page (SOFT delete — the page goes to trash and is restorable; nothing is permanent). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Sharing -> share_page / unshare_page / list_shares; share_page makes the page PUBLICLY accessible — do it only when explicitly asked.\n" + "COMMENTS: create_comment is always inline and requires an EXACT selection — contiguous text from a single block, <=250 chars (fails rather than leaving an unanchored comment); reply to a thread via parentCommentId. Propose a concrete text fix for one-click human approval -> create_comment with suggestedText (the exact plain-text replacement for the selection; the selection must then be UNIQUE in the page — extend it with context if needed); prefer this over editing directly when the change is subjective or needs the author's sign-off. Manage -> list_comments, update_comment, resolve_comment (resolve/reopen, reversible — prefer over delete to close), delete_comment, check_new_comments.\n" + "HISTORY: review what changed -> diff_page_versions (a historyId vs current, or two versions). List saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown."; @@ -106,6 +106,8 @@ const TOOL_FAMILY: Record = { drawio_get: "EDIT", drawio_create: "EDIT", drawio_update: "EDIT", + drawio_shapes: "EDIT", + drawio_guide: "EDIT", docmost_transform: "EDIT", // PAGES create_page: "PAGES", diff --git a/packages/mcp/src/tool-specs.ts b/packages/mcp/src/tool-specs.ts index f0702f7a..1db87be6 100644 --- a/packages/mcp/src/tool-specs.ts +++ b/packages/mcp/src/tool-specs.ts @@ -172,6 +172,18 @@ export interface SharedToolSpec { mcpOnly?: boolean; /** Registered only on the in-app host (skipped by the MCP registry loop). */ inAppOnly?: boolean; + /** + * The spec stays in the registry (so the shared contract still pins its name / + * description / schema across both hosts) but carries NO `execute`/override and + * is registered INLINE by BOTH hosts instead of through the registry loop. Used + * for tools whose implementation cannot cross into this zod-agnostic file — the + * drawio_shapes / drawio_guide pure helpers, whose backing module resolves a + * bundled data file via `import.meta` and so cannot be value-imported here + * without breaking the in-app server's commonjs type-check of this source. Both + * registry loops SKIP a spec with this flag; the per-host inline registrations + * own it (index.ts on MCP, ai-chat-tools.service.ts in-app). + */ + inlineBothHosts?: boolean; } // --- Shared execute helpers ------------------------------------------------- @@ -192,6 +204,29 @@ const mcpJson = (data: unknown) => ({ content: [{ type: 'text' as const, text: JSON.stringify(data, null, 2) }], }); +/** + * Compact HARD-RULES block injected into the drawio_create / drawio_update + * descriptions (issue #424 — the jgraph/drawio-mcp pattern of putting the + * must-follow rules right where the model reads them at call time). Deliberately + * terse; the long-form authoring guidance lives in drawio_guide. + */ +export const DRAWIO_HARD_RULES = + ' RULES: id="0" and id="1"(parent="0") sentinels are MANDATORY; each cell is ' + + 'vertex="1" XOR edge="1" (a container/group is neither); every edge carries a ' + + 'child ; ids are unique; NO XML ' + + 'comments; put html=1 in styles and XML-escape value (& -> &, < -> <); a ' + + "newline in a label is , never a literal \\n; containers are TRANSPARENT " + + "(fillColor=none;container=1;dropTarget=1;) with children set parent= " + + 'and RELATIVE coords, and an edge between different containers is parent="1"; set ' + + 'adaptiveColors="auto" on (free dark-theme adaptation for ' + + 'strokeColor/fillColor/fontColor="default"); do NOT guess shape=mxgraph.* names ' + + "(a wrong name renders as an empty box) — call drawio_shapes first; call " + + "drawio_guide(section) for authoring help. Pass layout:\"elk\" to let the server " + + "compute coordinates from your rough placement. The result carries geometry " + + "WARNINGS (overlaps, an edge through a shape, edge-on-edge, gaps <150px, a label " + + "wider than its shape, negative coords) — they do NOT block the write; fix them " + + "and retry, max 2 iterations."; + export const SHARED_TOOL_SPECS = { // --- no-argument read tools --- @@ -1619,7 +1654,7 @@ export const SHARED_TOOL_SPECS = { }), }, - // --- draw.io diagrams (issue #423, stage 1) --- + // --- draw.io diagrams (issue #423 stage 1, #424 stage 2) --- drawioGet: { mcpName: 'drawio_get', @@ -1673,7 +1708,8 @@ export const SHARED_TOOL_SPECS = { 'back into drawio_get / drawio_update for THIS document. It is positional, ' + 'so if you add or remove blocks before it, re-resolve via get_outline. The ' + 'diagram is editable in the draw.io editor and can be re-read with ' + - 'drawio_get.', + 'drawio_get.' + + DRAWIO_HARD_RULES, tier: 'deferred', catalogLine: 'drawioCreate — create a draw.io diagram from mxGraph XML and insert it.', @@ -1697,9 +1733,19 @@ export const SHARED_TOOL_SPECS = { .optional() .describe('Anchor text fragment (for before/after).'), title: z.string().optional().describe('Optional diagram title.'), + layout: z + .enum(['elk']) + .optional() + .describe( + 'Optional: "elk" runs an ELK layered auto-layout (honouring nested ' + + 'containers) and rewrites all coordinates — give rough placement and ' + + 'let the server compute pixels.', + ), }), // The flat schema fields are regrouped into the client's `where` object. - execute: (client, { pageId, xml, position, anchorNodeId, anchorText, title }) => + // `layout` is the 5th arg: dropping it silently disables ELK auto-layout + // (a reviewed #440 parity fix — MUST reach the client on both hosts). + execute: (client, { pageId, xml, position, anchorNodeId, anchorText, title, layout }) => client.drawioCreate( pageId as string, { @@ -1709,6 +1755,7 @@ export const SHARED_TOOL_SPECS = { }, xml as string, title as string | undefined, + layout as 'elk' | undefined, ), }, @@ -1722,7 +1769,8 @@ export const SHARED_TOOL_SPECS = { '(a human or another agent edited it) the hash mismatches and the update ' + 'is refused with a conflict error — re-read with drawio_get and retry. On ' + 'success it overwrites the diagram attachment and updates the node ' + - 'width/height. `node` is the drawio node attrs.id or "#".', + 'width/height. `node` is the drawio node attrs.id or "#".' + + DRAWIO_HARD_RULES, tier: 'deferred', catalogLine: 'drawioUpdate — replace a draw.io diagram (optimistic-locked by baseHash).', @@ -1742,13 +1790,101 @@ export const SHARED_TOOL_SPECS = { .string() .min(1) .describe('The meta.hash from the drawio_get this edit is based on.'), + layout: z + .enum(['elk']) + .optional() + .describe( + 'Optional: "elk" runs an ELK layered auto-layout and rewrites all ' + + 'coordinates before writing.', + ), }), - execute: (client, { pageId, node, xml, baseHash }) => + // `layout` is the 5th arg: forward layout:"elk" (a reviewed #440 parity fix) + // so both hosts run the ELK auto-layout before writing. + execute: (client, { pageId, node, xml, baseHash, layout }) => client.drawioUpdate( pageId as string, node as string, xml as string, baseHash as string, + layout as 'elk' | undefined, ), }, + + drawioShapes: { + mcpName: 'drawio_shapes', + inAppKey: 'drawioShapes', + description: + 'Look up VERIFIED draw.io stencil style-strings so you never guess a ' + + '`shape=mxgraph.*` name (a wrong name renders as an EMPTY BOX). Searches a ' + + 'bundled catalog of ~10 400 shapes (the jgraph/drawio-mcp index) by ' + + 'substring, tags and loose fuzzy match, plus a curated overlay for AWS ' + + 'icons: it returns the correct resIcon for rebranded services (OpenSearch ' + + '-> elasticsearch_service, MSK -> managed_streaming_for_kafka, VPC Peering ' + + '-> peering, IAM Identity Center -> single_sign_on) and maps known-broken ' + + 'stencils to working replacements (e.g. dynamodb_table -> dynamodb) with a ' + + 'note. Each hit is { style, w, h, title, type, category?, note? } — copy ' + + '`style` verbatim onto the cell and use w/h as the default size. Call this ' + + 'BEFORE drawio_create/drawio_update whenever you need a specific icon ' + + '(AWS/Azure/GCP/network/UML/flowchart).', + tier: 'deferred', + catalogLine: + 'drawioShapes — look up verified draw.io stencil style-strings (no empty boxes).', + buildShape: (z) => ({ + query: z + .string() + .min(1) + .describe('What to find, e.g. "lambda", "s3", "azure cosmos", "vpc group".'), + category: z + .string() + .optional() + .describe('Optional filter, e.g. an AWS category name ("Compute").'), + limit: z + .number() + .optional() + .describe('Max results (default 12, capped at 50).'), + }), + // INLINE on both hosts (no `execute`): drawio_shapes calls the PURE helper + // searchShapes, which is NOT a client method — it reads the bundled shape + // catalog via `import.meta.url` (drawio-shapes.ts). tool-specs.ts is + // type-checked FROM SOURCE by the in-app server under module:commonjs, where a + // static value-import of that `import.meta` module is a compile error + // (TS1343), so its execute CANNOT live here. `inlineBothHosts` tells BOTH + // registry loops to skip it; index.ts (MCP) and ai-chat-tools.service.ts + // (in-app) each register it directly, calling searchShapes from the loaded + // module. It STAYS in this registry so the shared-tool-specs contract still + // pins its name/description/schema across both hosts. + inlineBothHosts: true, + }, + + drawioGuide: { + mcpName: 'drawio_guide', + inAppKey: 'drawioGuide', + description: + 'Progressive-disclosure draw.io authoring reference. Call with a `section` ' + + 'to pull one focused, <=4KB chapter instead of bloating context: ' + + '"skeleton" (canonical mxGraph XML, sentinels, the accepted inputs, hard ' + + 'rules), "layout" (spacing heuristics, edge routing, the layout:"elk" ' + + 'option, the quality warnings), "containers" (transparent groups, relative ' + + 'child coords, cross-container edges, swimlanes), "icons-aws" (the ' + + 'service/resource icon patterns, category colors, rebrandings, blocklist), ' + + '"icons-azure" (portable image-style paths). Omit `section` to get the ' + + 'index of sections. Pair with drawio_shapes for exact stencil styles.', + tier: 'deferred', + catalogLine: + 'drawioGuide — on-demand draw.io authoring reference (skeleton/layout/containers/icons).', + buildShape: (z) => ({ + section: z + .enum(['skeleton', 'layout', 'containers', 'icons-aws', 'icons-azure']) + .optional() + .describe('Which section to read; omit for the section index.'), + }), + // INLINE on both hosts (no `execute`) — same reason as drawio_shapes above: + // drawio_guide calls the PURE helper getGuideSection (drawio-guide.ts, no + // client, no network). getGuideSection itself has no `import.meta`, but it is + // kept inline for SYMMETRY with drawio_shapes (both drawio helper tools wired + // the same way in one place) and to avoid pulling any drawio lib source into + // the in-app server's commonjs type-check. `inlineBothHosts` makes both loops + // skip it; index.ts and ai-chat-tools.service.ts register it directly. + inlineBothHosts: true, + }, } satisfies Record; diff --git a/packages/mcp/test/unit/drawio-guide.test.mjs b/packages/mcp/test/unit/drawio-guide.test.mjs new file mode 100644 index 00000000..cc6dbf6e --- /dev/null +++ b/packages/mcp/test/unit/drawio-guide.test.mjs @@ -0,0 +1,51 @@ +// Unit tests for the drawio_guide progressive-disclosure reference (issue #424). +// Acceptance #2: every section is returned and each is <= ~4KB so pulling one +// does not bloat the model's context. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + getGuideSection, + GUIDE_SECTIONS, +} from "../../build/lib/drawio-guide.js"; + +const MAX_BYTES = 4096; // "<= ~4KB" acceptance bound. + +test("every section is returned and is under ~4KB", () => { + assert.deepEqual(GUIDE_SECTIONS, [ + "skeleton", + "layout", + "containers", + "icons-aws", + "icons-azure", + ]); + for (const s of GUIDE_SECTIONS) { + const { section, content } = getGuideSection(s); + assert.equal(section, s); + assert.ok(content.length > 200, `${s}: suspiciously short`); + const bytes = Buffer.byteLength(content, "utf8"); + assert.ok(bytes <= MAX_BYTES, `${s}: ${bytes} bytes exceeds ${MAX_BYTES}`); + } +}); + +test("each section's content matches its topic", () => { + assert.match(getGuideSection("skeleton").content, /mxGraphModel/); + assert.match(getGuideSection("skeleton").content, /adaptiveColors="auto"/); + assert.match(getGuideSection("layout").content, /elk/i); + assert.match(getGuideSection("layout").content, /150px|<150/); + assert.match(getGuideSection("containers").content, /fillColor=none/); + assert.match(getGuideSection("icons-aws").content, /resourceIcon/); + assert.match(getGuideSection("icons-aws").content, /elasticsearch_service/); + assert.match(getGuideSection("icons-azure").content, /img\/lib\/azure2/); +}); + +test("omitting the section returns the index of sections", () => { + const idx = getGuideSection(); + assert.equal(idx.section, "index"); + for (const s of GUIDE_SECTIONS) assert.ok(idx.content.includes(s)); + assert.ok(Buffer.byteLength(idx.content, "utf8") <= MAX_BYTES); +}); + +test("an unknown section falls back to the index", () => { + const idx = getGuideSection("nonsense"); + assert.equal(idx.section, "index"); +}); diff --git a/packages/mcp/test/unit/drawio-layout.test.mjs b/packages/mcp/test/unit/drawio-layout.test.mjs new file mode 100644 index 00000000..a6998d17 --- /dev/null +++ b/packages/mcp/test/unit/drawio-layout.test.mjs @@ -0,0 +1,111 @@ +// Unit tests for the ELK auto-layout (issue #424, part 4). Acceptance #3: a +// 10+ node graph with rough/overlapping coordinates, laid out with ELK, has no +// bbox overlaps and produces no quality warnings. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { applyElkLayout } from "../../build/lib/drawio-layout.js"; +import { prepareModel, parseCells } from "../../build/lib/drawio-xml.js"; + +/** Build a model where every vertex starts stacked at (10,10). */ +function stackedGraph(n, edges) { + let cells = ""; + for (let i = 2; i < 2 + n; i++) { + cells += + `` + + ``; + } + let ei = 0; + for (const [s, t] of edges) { + cells += + `` + + ``; + } + return ( + '' + + cells + + "" + ); +} + +test("acceptance #3: a 10-node graph with rough coords lays out with no warnings", async () => { + const edges = [ + [2, 3], [2, 4], [3, 5], [4, 5], [5, 6], + [6, 7], [6, 8], [7, 9], [8, 10], [9, 11], [10, 11], + ]; + const model = stackedGraph(10, edges); + + // Before: everything is stacked at (10,10) -> lots of overlap warnings. + const before = prepareModel(model); + assert.ok(before.warnings.length > 0, "the stacked input should warn"); + + const laid = await applyElkLayout(model); + const after = prepareModel(laid); + assert.equal( + after.warnings.length, + 0, + `ELK layout should clear all warnings, got: ${after.warnings.join(" | ")}`, + ); + // Same number of user cells survived the layout. + assert.equal(after.cellCount, before.cellCount); +}); + +test("ELK honours nested containers as compound nodes (no warnings, children stay nested)", async () => { + const model = + '' + + '' + + '' + + '' + + '' + + '' + + '' + + ""; + const laid = await applyElkLayout(model); + const cells = parseCells(laid); + const byId = Object.fromEntries(cells.map((c) => [c.id, c])); + // Children keep their container parent; the container was sized to hold them. + assert.equal(byId.a.parent, "g"); + assert.equal(byId.b.parent, "g"); + assert.ok((byId.g.geometry.width ?? 0) >= 260, "container widened to fit children"); + const after = prepareModel(laid); + assert.equal(after.warnings.length, 0, after.warnings.join(" | ")); +}); + +test("edges and cell count are preserved by layout", async () => { + const model = stackedGraph(4, [[2, 3], [3, 4], [4, 5]]); + const laid = await applyElkLayout(model); + const cells = parseCells(laid); + assert.equal(cells.filter((c) => c.edge).length, 3); + assert.equal(cells.filter((c) => c.vertex).length, 4); +}); + +test("DoS guard: a graph over the node cap is returned unchanged, quickly", async () => { + // 600 vertices > ELK_MAX_NODES (500): the layout must be SKIPPED and the + // input returned verbatim, without ever handing the graph to elkjs. This + // exercises the cap path that bounds the in-process, event-loop-blocking + // layout on LLM-supplied XML. + const model = stackedGraph(600, []); + const t0 = Date.now(); + const laid = await applyElkLayout(model); + const dt = Date.now() - t0; + // normalizeInput may reserialize, but geometry must be untouched: every + // vertex is still stacked at (10,10), i.e. no ELK coordinates were applied. + const cells = parseCells(laid); + const verts = cells.filter((c) => c.vertex); + assert.equal(verts.length, 600, "all vertices survived"); + for (const v of verts) { + assert.equal(v.geometry.x, 10, "x untouched -> layout was skipped"); + assert.equal(v.geometry.y, 10, "y untouched -> layout was skipped"); + } + // Returning the input without an ELK pass is essentially instant; assert it + // did not hang. Generous bound to stay non-flaky on a loaded CI box. + assert.ok(dt < 2000, `cap path should be fast, took ${dt}ms`); +}); + +test("layout is best-effort: an empty/degenerate model is returned intact", async () => { + const model = + ''; + const laid = await applyElkLayout(model); + // No vertices -> unchanged, still lints clean. + const after = prepareModel(laid); + assert.equal(after.cellCount, 0); +}); diff --git a/packages/mcp/test/unit/drawio-quality-warnings.test.mjs b/packages/mcp/test/unit/drawio-quality-warnings.test.mjs new file mode 100644 index 00000000..e58e3a5a --- /dev/null +++ b/packages/mcp/test/unit/drawio-quality-warnings.test.mjs @@ -0,0 +1,101 @@ +// Unit tests for the geometry quality-warnings (issue #424, part 5). Acceptance +// #4: every warning has a positive AND a negative case, and warnings NEVER block +// the write (prepareModel returns them, it does not throw). +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { prepareModel } from "../../build/lib/drawio-xml.js"; + +function model(cells) { + return ( + '' + + cells + + "" + ); +} +function warnings(cells) { + return prepareModel(model(cells)).warnings; +} +function has(ws, rule) { + return ws.some((w) => w.startsWith(`[${rule}]`)); +} +function v(id, x, y, w = 120, h = 60, value = "", style = "rounded=1;html=1;", parent = "1") { + return ( + `` + + `` + ); +} +function edge(id, s, t, parent = "1") { + return ( + `` + + `` + ); +} + +test("shape-overlap: positive and negative", () => { + assert.ok(has(warnings(v("a", 0, 0) + v("b", 50, 20)), "shape-overlap")); + assert.ok(!has(warnings(v("a", 0, 0) + v("b", 300, 0)), "shape-overlap")); +}); + +test("shape-overlap: a container over its own child does NOT warn", () => { + const cells = + '' + + v("a", 30, 40, 120, 60, "", "rounded=1;", "g"); + assert.ok(!has(warnings(cells), "shape-overlap")); +}); + +test("edge-through-shape: positive and negative", () => { + // A -> B passes straight through C sitting on the line. + const pos = + v("a", 0, 0, 60, 60) + v("c", 200, 0, 60, 60) + v("b", 400, 0, 60, 60) + edge("e", "a", "b"); + assert.ok(has(warnings(pos), "edge-through-shape")); + // C moved off the line -> no crossing. + const neg = + v("a", 0, 0, 60, 60) + v("c", 200, 300, 60, 60) + v("b", 400, 0, 60, 60) + edge("e", "a", "b"); + assert.ok(!has(warnings(neg), "edge-through-shape")); +}); + +test("edge-overlap: positive (duplicate) and negative", () => { + const pos = v("a", 0, 0) + v("b", 300, 0) + edge("e1", "a", "b") + edge("e2", "a", "b"); + assert.ok(has(warnings(pos), "edge-overlap")); + const neg = + v("a", 0, 0) + v("b", 300, 0) + v("c", 300, 300) + edge("e1", "a", "b") + edge("e2", "a", "c"); + assert.ok(!has(warnings(neg), "edge-overlap")); +}); + +test("gap-too-small: positive and negative", () => { + assert.ok(has(warnings(v("a", 0, 0) + v("b", 220, 0)), "gap-too-small")); // 100px gap + assert.ok(!has(warnings(v("a", 0, 0) + v("b", 300, 0)), "gap-too-small")); // 180px gap +}); + +test("label-overflow: positive and negative", () => { + const pos = v("a", 0, 0, 40, 60, "A very long label that does not fit"); + assert.ok(has(warnings(pos), "label-overflow")); + const neg = v("a", 0, 0, 300, 60, "Short"); + assert.ok(!has(warnings(neg), "label-overflow")); +}); + +test("label-overflow: a label drawn OUTSIDE the shape (AWS icon) does NOT warn", () => { + const cells = v( + "a", + 0, + 0, + 60, + 60, + "A very long service label below the icon", + "shape=mxgraph.aws4.resourceIcon;verticalLabelPosition=bottom;verticalAlign=top;html=1;", + ); + assert.ok(!has(warnings(cells), "label-overflow")); +}); + +test("out-of-bounds: positive (negative coords) and negative", () => { + assert.ok(has(warnings(v("a", -50, 10)), "out-of-bounds")); + assert.ok(!has(warnings(v("a", 10, 10)), "out-of-bounds")); +}); + +test("warnings never block the write (prepareModel returns, does not throw)", () => { + const messy = v("a", 0, 0) + v("b", 30, 20) + v("c", 40, 40); // heavy overlap + const prepared = prepareModel(model(messy)); + assert.ok(prepared.warnings.length > 0, "expected warnings"); + assert.ok(prepared.modelXml.includes("mxGraphModel"), "still produced a model"); + assert.equal(prepared.cellCount, 3); +}); diff --git a/packages/mcp/test/unit/drawio-shapes.test.mjs b/packages/mcp/test/unit/drawio-shapes.test.mjs new file mode 100644 index 00000000..1521a744 --- /dev/null +++ b/packages/mcp/test/unit/drawio-shapes.test.mjs @@ -0,0 +1,97 @@ +// Unit tests for the drawio_shapes verified-stencil catalog (issue #424). +// Covers acceptance #1: a "lambda" query returns a valid mxgraph.aws4 icon with +// the right service/resource pattern + sizes; a blocklisted stencil query +// returns its working replacement. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + searchShapes, + awsServiceStyle, + azureImageStyle, + loadShapeIndex, + AWS_CATEGORY_FILL, +} from "../../build/lib/drawio-shapes.js"; + +test("the bundled index loads and is the real ~10k-shape catalog", () => { + const idx = loadShapeIndex(); + assert.ok(Array.isArray(idx)); + assert.ok(idx.length > 10000, `expected >10000 shapes, got ${idx.length}`); + // Record shape { style, w, h, title, tags, type }. + for (const k of ["style", "w", "h", "title", "tags", "type"]) { + assert.ok(k in idx[0], `record missing key ${k}`); + } +}); + +test('drawio_shapes("lambda") returns a valid mxgraph.aws4 service icon', () => { + const results = searchShapes("lambda", { limit: 5 }); + assert.ok(results.length > 0); + // Acceptance #1: a valid aws4 service-level icon (resourceIcon + resIcon) + // for lambda, with sensible default sizes, is present. + const svc = results.find( + (r) => + /shape=mxgraph\.aws4\.resourceIcon/.test(r.style) && + /resIcon=mxgraph\.aws4\.lambda(_function)?\b/.test(r.style), + ); + assert.ok(svc, `no aws4 lambda service icon in ${JSON.stringify(results.map((r) => r.style.slice(-40)))}`); + assert.ok(svc.w > 0 && svc.h > 0, "icon must carry default w/h"); + // The current-generation aws4 icon must outrank the deprecated aws3 one. + assert.match(results[0].style, /mxgraph\.aws4/); +}); + +test("a blocklisted stencil query returns its replacement + a note", () => { + const results = searchShapes("dynamodb_table", { limit: 3 }); + assert.ok(results.length > 0); + const rep = results[0]; + // dynamodb_table (empty box) -> dynamodb. + assert.match(rep.style, /resIcon=mxgraph\.aws4\.dynamodb\b/); + assert.ok(rep.note && /dynamodb_table/.test(rep.note), "note must explain the replacement"); + // The broken stencil name must NOT be returned as a usable style. + assert.ok( + !results.some((r) => /resIcon=mxgraph\.aws4\.dynamodb_table\b/.test(r.style)), + "the broken dynamodb_table stencil must not be returned", + ); +}); + +test("an AWS rebranding query returns the real (renamed) resIcon", () => { + const os = searchShapes("opensearch", { limit: 3 }); + assert.ok( + os.some((r) => /resIcon=mxgraph\.aws4\.elasticsearch_service\b/.test(r.style) && r.note), + "OpenSearch must map to elasticsearch_service with a note", + ); + const msk = searchShapes("msk", { limit: 3 }); + assert.ok( + msk.some((r) => /managed_streaming_for_kafka/.test(r.style)), + "MSK must map to managed_streaming_for_kafka", + ); +}); + +test("category filter narrows results", () => { + const all = searchShapes("database", { limit: 20 }); + const dbOnly = searchShapes("database", { category: "Database", limit: 20 }); + assert.ok(dbOnly.length <= all.length); +}); + +test("limit is honoured and capped", () => { + assert.equal(searchShapes("aws", { limit: 3 }).length, 3); + assert.ok(searchShapes("aws", { limit: 999 }).length <= 50); +}); + +test("empty query returns nothing", () => { + assert.deepEqual(searchShapes(" "), []); +}); + +test("style builders match the appendix templates", () => { + const s = awsServiceStyle("lambda", "Compute"); + assert.match(s, /strokeColor=#ffffff/); // mandatory for service-level + assert.match(s, new RegExp(`fillColor=${AWS_CATEGORY_FILL.Compute}`)); + assert.match(s, /shape=mxgraph\.aws4\.resourceIcon;resIcon=mxgraph\.aws4\.lambda$/); + const az = azureImageStyle("databases/Azure_Cosmos_DB.svg"); + assert.match(az, /image=img\/lib\/azure2\/databases\/Azure_Cosmos_DB\.svg/); +}); + +test("azure and group queries surface the curated overlay", () => { + const cosmos = searchShapes("cosmos", { limit: 5 }); + assert.ok(cosmos.some((r) => /azure2\/databases\/Azure_Cosmos_DB\.svg/.test(r.style))); + const vpc = searchShapes("vpc group", { limit: 5 }); + assert.ok(vpc.some((r) => /grIcon=mxgraph\.aws4\.group_vpc2/.test(r.style))); +}); diff --git a/packages/mcp/test/unit/drawio-stage2-registration.test.mjs b/packages/mcp/test/unit/drawio-stage2-registration.test.mjs new file mode 100644 index 00000000..37b40a4c --- /dev/null +++ b/packages/mcp/test/unit/drawio-stage2-registration.test.mjs @@ -0,0 +1,62 @@ +// Drift guards for the stage-2 drawio tools (issue #424): the new tools must be +// wired into the shared registry AND routed in SERVER_INSTRUCTIONS, and the +// hard-rules block must be injected into the create/update descriptions. These +// complement the generic server-instructions.test.mjs / tool-specs.test.mjs. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { SERVER_INSTRUCTIONS } from "../../build/index.js"; +import { SHARED_TOOL_SPECS } from "../../build/tool-specs.js"; + +test("drawio_shapes and drawio_guide are in the shared registry", () => { + assert.equal(SHARED_TOOL_SPECS.drawioShapes.mcpName, "drawio_shapes"); + assert.equal(SHARED_TOOL_SPECS.drawioGuide.mcpName, "drawio_guide"); + // Deferred tier, matching the stage-1 drawio tools. + assert.equal(SHARED_TOOL_SPECS.drawioShapes.tier, "deferred"); + assert.equal(SHARED_TOOL_SPECS.drawioGuide.tier, "deferred"); +}); + +test("the new tools are routed in SERVER_INSTRUCTIONS", () => { + for (const name of ["drawio_shapes", "drawio_guide"]) { + assert.match(SERVER_INSTRUCTIONS, new RegExp(`\\b${name}\\b`), `${name} missing from guide`); + } +}); + +test("the hard-rules block is injected into create/update descriptions", () => { + for (const key of ["drawioCreate", "drawioUpdate"]) { + const d = SHARED_TOOL_SPECS[key].description; + assert.match(d, /sentinels are MANDATORY/); + assert.match(d, /vertex="1" XOR edge="1"/); + assert.match(d, /call drawio_shapes first/); + assert.match(d, /adaptiveColors="auto"/); + assert.match(d, / /); + } +}); + +test("create/update expose the layout:\"elk\" parameter", () => { + const { z } = { z: makeZodStub() }; + for (const key of ["drawioCreate", "drawioUpdate"]) { + const shape = SHARED_TOOL_SPECS[key].buildShape(z); + assert.ok("layout" in shape, `${key} missing layout param`); + } +}); + +// Tiny zod stub: buildShape only calls z.string/enum/number + chained +// .min/.optional/.describe, all of which return `this`. +function makeZodStub() { + const chain = new Proxy( + {}, + { + get: (_t, prop) => { + if (prop === "parse") return () => ({}); + return () => chain; + }, + }, + ); + return { + string: () => chain, + number: () => chain, + enum: () => chain, + array: () => chain, + object: () => chain, + }; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cbe77728..1ab1c069 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1044,6 +1044,9 @@ importers: axios: specifier: 1.16.0 version: 1.16.0 + elkjs: + specifier: ^0.11.1 + version: 0.11.1 form-data: specifier: ^4.0.0 version: 4.0.5 @@ -6876,6 +6879,9 @@ packages: electron-to-chromium@1.5.286: resolution: {integrity: sha512-9tfDXhJ4RKFNerfjdCcZfufu49vg620741MNs26a9+bhLThdB+plgMeou98CAaHu/WATj2iHOOHTp1hWtABj2A==} + elkjs@0.11.1: + resolution: {integrity: sha512-zxxR9k+rx5ktMwT/FwyLdPCrq7xN6e4VGGHH8hA01vVYKjTFik7nHOxBnAYtrgYUB1RpAiLvA1/U2YraWxyKKg==} + emittery@0.13.1: resolution: {integrity: sha512-DeWwawk6r5yR9jFgnDKYt4sLS0LmHJJi3ZOnb5/JdbYwj3nW+FxQnHIjhBKz8YLC7oRNPVM9NQ47I3CVx34eqQ==} engines: {node: '>=12'} @@ -17642,6 +17648,8 @@ snapshots: electron-to-chromium@1.5.286: {} + elkjs@0.11.1: {} + emittery@0.13.1: {} emoji-regex@8.0.0: {}