From 8d690503d4ff5ccef8e63e493df1889b90773446 Mon Sep 17 00:00:00 2001 From: caoxiaole07 Date: Sun, 11 Oct 2026 09:03:13 +0800 Subject: [PATCH] feat(mcp): load_diagram accepts inline 'xml' content as an alternative to 'path' (#946) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agents frequently hold .drawio content in memory (another tool's output, a repository read, an API response) and previously had to write it to a temporary file just so load_diagram could read it back. Add a mutually exclusive 'xml' argument that goes through the same parser as the 'path' branch, so both plain XML and draw.io's compressed save format work. Edit-gate semantics by source: - 'path': unchanged — the model has not seen the content, one get_diagram round-trip is still required before editing. - plain 'xml': the model supplied the exact content (same rationale as create_new_diagram), so it is recorded as seen and can be edited immediately. - compressed 'xml': the session stores the decompressed form, which the model cannot derive from the compressed input — the gate is kept. Argument validation (mutual exclusion / presence) fires before the session check so callers get useful errors regardless of session state. parseDrawioFileContent now reports whether any page was decompressed, which drives the gate decision. The diagram-workflow prompt is updated to document the new argument. Co-authored-by: caoxiaole07 <254405386+caoxiaole07@users.noreply.github.com> --- packages/mcp-server/src/drawing-guide.ts | 2 +- packages/mcp-server/src/index.ts | 121 +++++++++++++----- packages/mcp-server/src/load-diagram.ts | 8 +- .../mcp-server/tests/load-diagram.test.ts | 39 +++++- .../mcp-server/tests/server-wiring.test.ts | 48 +++++++ 5 files changed, 178 insertions(+), 40 deletions(-) diff --git a/packages/mcp-server/src/drawing-guide.ts b/packages/mcp-server/src/drawing-guide.ts index ba967578..b63ce546 100644 --- a/packages/mcp-server/src/drawing-guide.ts +++ b/packages/mcp-server/src/drawing-guide.ts @@ -18,7 +18,7 @@ import { export const DRAWING_GUIDE = `# Draw.io drawing guide ## Workflow -- create_new_diagram draws a new diagram and REPLACES the whole document. add_page adds another tab. edit_diagram changes cells of an existing page. load_diagram opens a .drawio file (the server reads the file itself). get_diagram returns the current XML, including the user's manual edits. export_diagram saves to a file. +- create_new_diagram draws a new diagram and REPLACES the whole document. add_page adds another tab. edit_diagram changes cells of an existing page. load_diagram opens a .drawio file (the server reads the file itself, or takes the file's content as its 'xml' argument when you already have it in hand). get_diagram returns the current XML, including the user's manual edits. export_diagram saves to a file. - Before drawing, describe your layout plan in 2-3 sentences, so shapes do not overlap and edges do not cross shapes. - Send XML only through tool calls, never in chat text. Never draw a box just to send the user a message. - Before using any icon library (AWS, Azure, GCP, Kubernetes, Cisco, BPMN, Material Design, web icons...), call get_shape_library and use the exact style names it returns. NEVER guess icon style names. For AWS, use the AWS 2025 icons (library aws4). diff --git a/packages/mcp-server/src/index.ts b/packages/mcp-server/src/index.ts index a6182094..09993564 100644 --- a/packages/mcp-server/src/index.ts +++ b/packages/mcp-server/src/index.ts @@ -455,21 +455,54 @@ registerWriteTool( { title: "Load .drawio file", description: - "Load a .drawio file from disk into the current session, REPLACING the entire diagram (all pages). " + - "The server reads the file directly — you do NOT need to read the file yourself or pass its XML through create_new_diagram. " + - "Handles both plain-XML and draw.io's compressed save format.\n\n" + - "After loading, call get_diagram before edit_diagram — you haven't seen the file's cell IDs or structure yet.", + "Load a .drawio diagram into the current session, REPLACING the entire diagram (all pages). " + + "Provide ONE of two mutually exclusive sources: 'path' (the server reads the file from disk — you do NOT need to read the file yourself or pass its XML through create_new_diagram) " + + "or 'xml' (the raw file content you already have — from another tool, a repository read, or an API response — so no temporary file needs to be written first). " + + "Both accept plain XML and draw.io's compressed save format.\n\n" + + "After loading from 'path' (or from compressed 'xml'), call get_diagram before edit_diagram — you haven't seen the file's cell IDs yet. " + + "Plain-XML 'xml' content you supplied yourself is already known and can be edited immediately.", inputSchema: { path: z .string() + .optional() .describe( - "Absolute path to the .drawio file to load (e.g. /Users/me/diagram.drawio or ~/diagram.drawio). Relative paths resolve against the MCP server's working directory, which is often not your project.", + "Path to the .drawio file to load (e.g. /Users/me/diagram.drawio or ~/diagram.drawio). Relative paths resolve against the MCP server's working directory, which is often not your project. Mutually exclusive with 'xml'.", + ), + xml: z + .string() + .optional() + .describe( + "Raw .drawio file content: a plain /, or draw.io's compressed save format. Use when the content is already in hand (another tool's output, a repository read, an API response). Mutually exclusive with 'path'.", ), }, annotations: { openWorldHint: false }, }, - async ({ path }) => { + async ({ path, xml: inlineXml }) => { try { + // Argument validation comes before the session check: a bad + // argument is a caller error and should be reported as such. + if (path !== undefined && inlineXml !== undefined) { + return { + content: [ + { + type: "text", + text: "Error: Provide either 'path' or 'xml', not both.", + }, + ], + isError: true, + } + } + if (path === undefined && inlineXml === undefined) { + return { + content: [ + { + type: "text", + text: "Error: Provide either 'path' (a .drawio file to read) or 'xml' (the file's content).", + }, + ], + isError: true, + } + } if (!currentSession) { return { content: [ @@ -482,29 +515,37 @@ registerWriteTool( } } - const fs = await import("node:fs/promises") - const nodePath = await import("node:path") - const absolutePath = nodePath.resolve(expandHome(path)) + // Exactly one of path/xml is present (validated above). + let content = "" + let sourceLabel = "" + if (inlineXml !== undefined) { + content = inlineXml + sourceLabel = "inline XML" + } else if (path !== undefined) { + const fs = await import("node:fs/promises") + const nodePath = await import("node:path") + const absolutePath = nodePath.resolve(expandHome(path)) - let content: string - try { - // A pipe or device could be read forever, and the other - // write tools wait for this one - if (!(await fs.stat(absolutePath)).isFile()) { - throw new Error("not a regular file") - } - content = await fs.readFile(absolutePath, "utf-8") - } catch (e) { - const msg = e instanceof Error ? e.message : String(e) - return { - content: [ - { - type: "text", - text: `Error: Cannot read file ${absolutePath}: ${msg}`, - }, - ], - isError: true, + try { + // A pipe or device could be read forever, and the other + // write tools wait for this one + if (!(await fs.stat(absolutePath)).isFile()) { + throw new Error("not a regular file") + } + content = await fs.readFile(absolutePath, "utf-8") + } catch (e) { + const msg = e instanceof Error ? e.message : String(e) + return { + content: [ + { + type: "text", + text: `Error: Cannot read file ${absolutePath}: ${msg}`, + }, + ], + isError: true, + } } + sourceLabel = absolutePath } const loaded = parseDrawioFileContent(content) @@ -517,7 +558,7 @@ registerWriteTool( const xml = loaded.xml log.info( - `Loading diagram from ${absolutePath} (${xml.length} chars)`, + `Loading diagram from ${sourceLabel} (${xml.length} chars)`, ) // Save the user's current state before replacing (same flow as @@ -537,10 +578,17 @@ registerWriteTool( currentSession.xml = xml currentSession.version++ setState(currentSession.id, xml) - // Deliberately NOT marking the loaded XML as seen: the model only - // supplied a path, so it doesn't know the file's cell IDs. The - // edit gate will require one get_diagram before edits. - currentSession.lastSeenXml = "" + // Edit-gate semantics by source: + // - 'path': the model only supplied a path, so it doesn't know + // the file's cell IDs — keep the gate (one get_diagram first). + // - plain 'xml': the model supplied the exact content, same + // rationale as create_new_diagram — record it as seen. + // - compressed 'xml': the session now holds the decompressed + // form, which the model cannot derive from the compressed + // input — keep the gate. + const markSeen = + inlineXml !== undefined && !loaded.hadCompressedPages + currentSession.lastSeenXml = markSeen ? xml : "" addHistory(currentSession.id, xml, "") @@ -551,13 +599,17 @@ registerWriteTool( ? `Pages (${pages.length}): ${pages.map((p) => `[${p.index}] id=${p.id} name="${p.name}" cells=${p.cellCount}`).join(" | ")}` : "no pages parsed" - log.info(`Diagram loaded from file (${pageSummary})`) + log.info(`Diagram loaded (${pageSummary})`) + + const gateHint = markSeen + ? "" + : "\n\nCall get_diagram before edit_diagram — you haven't seen this file's cell IDs yet." return { content: [ { type: "text", - text: `Diagram loaded from ${absolutePath}!\n\nThe diagram is now visible in your browser.\n\n${pageSummary}\n\nCall get_diagram before edit_diagram — you haven't seen this file's cell IDs yet.`, + text: `Diagram loaded from ${sourceLabel}!\n\nThe diagram is now visible in your browser.\n\n${pageSummary}${gateHint}`, }, ], } @@ -572,7 +624,6 @@ registerWriteTool( } }, ) - // Tool: edit_diagram registerWriteTool( "edit_diagram", diff --git a/packages/mcp-server/src/load-diagram.ts b/packages/mcp-server/src/load-diagram.ts index f1609854..9dbcb5f9 100644 --- a/packages/mcp-server/src/load-diagram.ts +++ b/packages/mcp-server/src/load-diagram.ts @@ -18,7 +18,7 @@ import { import { getXmlSyntaxError } from "./xml-syntax.ts" export type LoadResult = - | { ok: true; xml: string } + | { ok: true; xml: string; hadCompressedPages: boolean } | { ok: false; error: string } /** @@ -101,5 +101,9 @@ export function parseDrawioFileContent(content: string): LoadResult { decompressedAny = true } // Nothing changed — keep the file's own serialisation. - return { ok: true, xml: decompressedAny ? serializeMxfile(doc) : trimmed } + return { + ok: true, + xml: decompressedAny ? serializeMxfile(doc) : trimmed, + hadCompressedPages: decompressedAny, + } } diff --git a/packages/mcp-server/tests/load-diagram.test.ts b/packages/mcp-server/tests/load-diagram.test.ts index d0264e6e..afce3463 100644 --- a/packages/mcp-server/tests/load-diagram.test.ts +++ b/packages/mcp-server/tests/load-diagram.test.ts @@ -54,7 +54,11 @@ describe("decompressPageContent", () => { describe("parseDrawioFileContent", () => { it("passes a plain-XML mxfile through unchanged", () => { const r = parseDrawioFileContent(PLAIN_MXFILE) - expect(r).toEqual({ ok: true, xml: PLAIN_MXFILE }) + expect(r).toEqual({ + ok: true, + xml: PLAIN_MXFILE, + hadCompressedPages: false, + }) }) it("wraps a bare mxGraphModel into a one-page mxfile", () => { @@ -103,7 +107,11 @@ describe("parseDrawioFileContent", () => { it("keeps empty pages as-is", () => { const withEmpty = `${MODEL_XML}` const r = parseDrawioFileContent(withEmpty) - expect(r).toEqual({ ok: true, xml: withEmpty }) + expect(r).toEqual({ + ok: true, + xml: withEmpty, + hadCompressedPages: false, + }) }) it("rejects empty files", () => { @@ -124,3 +132,30 @@ describe("parseDrawioFileContent", () => { if (!r.ok) expect(r.error).toContain('"Broken"') }) }) + +describe("hadCompressedPages", () => { + it("is false for a plain-XML mxfile", () => { + const r = parseDrawioFileContent(PLAIN_MXFILE) + expect(r.ok).toBe(true) + if (r.ok) expect(r.hadCompressedPages).toBe(false) + }) + + it("is false for a bare mxGraphModel", () => { + const r = parseDrawioFileContent(MODEL_XML) + expect(r.ok).toBe(true) + if (r.ok) expect(r.hadCompressedPages).toBe(false) + }) + + it("is true for a fully compressed mxfile", () => { + const r = parseDrawioFileContent(COMPRESSED_MXFILE) + expect(r.ok).toBe(true) + if (r.ok) expect(r.hadCompressedPages).toBe(true) + }) + + it("is true for a mixed plain/compressed file", () => { + const mixed = `${MODEL_XML}${drawioCompress(MODEL_XML)}` + const r = parseDrawioFileContent(mixed) + expect(r.ok).toBe(true) + if (r.ok) expect(r.hadCompressedPages).toBe(true) + }) +}) diff --git a/packages/mcp-server/tests/server-wiring.test.ts b/packages/mcp-server/tests/server-wiring.test.ts index 33abe65d..a8f52b32 100644 --- a/packages/mcp-server/tests/server-wiring.test.ts +++ b/packages/mcp-server/tests/server-wiring.test.ts @@ -188,3 +188,51 @@ describe("MCP server wiring", () => { expect(text.length).toBeLessThanOrEqual(15000) }) }) + +describe("load_diagram dual-source arguments", () => { + it("advertises both optional 'path' and 'xml' sources", async () => { + const resp = await send("tools/list", {}) + const load = resp.result.tools.find( + (t: { name: string }) => t.name === "load_diagram", + ) + const props = load?.inputSchema?.properties ?? {} + expect(props.path).toBeTruthy() + expect(props.xml).toBeTruthy() + const required: string[] = load?.inputSchema?.required ?? [] + expect(required).not.toContain("path") + expect(required).not.toContain("xml") + }) + + it("rejects passing both 'path' and 'xml'", async () => { + // Argument validation fires before the session check: no session + // exists in this harness, so a both-args call must report the + // mutual-exclusion error, not "No active session". + const resp = await send("tools/call", { + name: "load_diagram", + arguments: { path: "/tmp/x.drawio", xml: "" }, + }) + expect(resp.error, JSON.stringify(resp.error)).toBeUndefined() + expect(resp.result?.isError).toBe(true) + expect(resp.result?.content?.[0]?.text).toContain("not both") + }) + + it("rejects passing neither 'path' nor 'xml'", async () => { + const resp = await send("tools/call", { + name: "load_diagram", + arguments: {}, + }) + expect(resp.result?.isError).toBe(true) + expect(resp.result?.content?.[0]?.text).toContain("either 'path'") + }) + + it("accepts 'xml' alone as a source (fails only on the missing session)", async () => { + const resp = await send("tools/call", { + name: "load_diagram", + arguments: { + xml: '', + }, + }) + expect(resp.result?.isError).toBe(true) + expect(resp.result?.content?.[0]?.text).toContain("No active session") + }) +})