From 899924ba987b2d2aca9e775f21e12ac93d5bf465 Mon Sep 17 00:00:00 2001 From: "dayuan.jiang" Date: Sun, 4 Oct 2026 07:38:17 +0900 Subject: [PATCH] fix(mcp-server): fix duplicate page exports and auto-save deleting user files - Preview page: keep an MCP export open until the server has its result. A poll answered before that still saw the request and started the same export again, so a parallel page export could write the previous page's image into its file - Auto-save only removes its own mcp-*.drawio files, so a DRAWIO_DATA_DIR that also holds the user's diagrams keeps them - screenshot_diagram captures a page that has no id attribute by loading just that page, like export_diagram - An empty no longer hides orphan mxPoints that come after it - POST /api/state refuses a push without xml, which used to wipe the stored diagram - Clear exportOptions when an export ends, reuse hasCells for the empty diagram check, and reword two log lines --- packages/mcp-server/src/http-server.ts | 5 +++ packages/mcp-server/src/index.ts | 42 ++++++++++--------- packages/mcp-server/src/persistence.ts | 6 ++- packages/mcp-server/src/preview/preview.js | 17 +++++--- packages/mcp-server/src/xml-validation.ts | 3 +- packages/mcp-server/tests/http-server.test.ts | 10 +++++ packages/mcp-server/tests/persistence.test.ts | 5 ++- .../mcp-server/tests/xml-validation.test.ts | 18 ++++++++ 8 files changed, 77 insertions(+), 29 deletions(-) diff --git a/packages/mcp-server/src/http-server.ts b/packages/mcp-server/src/http-server.ts index ad269a03..0e96f103 100644 --- a/packages/mcp-server/src/http-server.ts +++ b/packages/mcp-server/src/http-server.ts @@ -477,6 +477,11 @@ function handleStateApi( return } + if (typeof data.xml !== "string") { + res.writeHead(400, { "Content-Type": "application/json" }) + res.end(JSON.stringify({ error: "xml must be a string" })) + return + } const version = setState(sessionId, data.xml, data.svg, true) res.writeHead(200, { "Content-Type": "application/json" }) res.end(JSON.stringify({ success: true, version })) diff --git a/packages/mcp-server/src/index.ts b/packages/mcp-server/src/index.ts index faacb463..6f332cd1 100644 --- a/packages/mcp-server/src/index.ts +++ b/packages/mcp-server/src/index.ts @@ -59,7 +59,7 @@ import { serializeMxfile, wrapCellsInModel, } from "./pages.js" -import { Autosaver, defaultDataDir } from "./persistence.js" +import { Autosaver, defaultDataDir, hasCells } from "./persistence.js" import { getShapeLibrary, SHAPE_LIBRARY_GROUPS } from "./shape-library.js" import { validateAndFixXml } from "./xml-validation.js" @@ -670,8 +670,8 @@ server.registerTool( if (!gate.ok) { log.warn( gate.reason === "stale" - ? "edit_diagram called with unseen browser changes - rejecting to prevent data loss" - : "edit_diagram called without seeing the diagram - rejecting to prevent data loss", + ? "edit_diagram rejected: the browser has changes the model has not seen" + : "edit_diagram rejected: the model has not seen the diagram yet", ) // The error carries the current page, so the model has now // seen it and can retry without a get_diagram round-trip. @@ -926,6 +926,7 @@ function exportViaBrowser( live.exportData = undefined live.exportFormat = undefined live.exportXml = undefined + live.exportOptions = undefined } return exportData }) @@ -1009,12 +1010,7 @@ server.registerTool( return previewStalledError(currentSession.id) } const xml = getState(currentSession.id)?.xml || currentSession.xml - // Any cell besides the root cells "0" and "1" - if ( - !/<(mxCell\b[^>]*\bid="(?![01]")|UserObject\b|object\b)/.test( - xml, - ) - ) { + if (!hasCells(xml)) { return { content: [{ type: "text", text: "The diagram is empty." }], } @@ -1026,18 +1022,26 @@ server.registerTool( page_index, }) let pageId: string | undefined + let projectionXml: string | undefined if (hasPageSelector(pageSelector)) { - pageId = pageIdFor(normalizeToMxfile(xml) ?? xml, pageSelector) + const doc = normalizeToMxfile(xml) ?? xml + pageId = pageIdFor(doc, pageSelector) + // A page without an id: load just that page and capture + // it, as export_diagram does if (!pageId) { - return { - content: [ - { - type: "text", - text: `Error: Page ${describeSelector(pageSelector)} not found.`, - }, - ], - isError: true, + const projection = projectPage(doc, pageSelector) + if (!projection.ok) { + return { + content: [ + { + type: "text", + text: `Error: Page ${describeSelector(pageSelector)} not found.`, + }, + ], + isError: true, + } } + projectionXml = projection.xml } } @@ -1046,7 +1050,7 @@ server.registerTool( data = await exportViaBrowser( currentSession.id, "png", - undefined, + projectionXml, { width, pageId }, ) if (!data || data.length <= MAX_SCREENSHOT_CHARS) break diff --git a/packages/mcp-server/src/persistence.ts b/packages/mcp-server/src/persistence.ts index 02ec8208..947fbb8f 100644 --- a/packages/mcp-server/src/persistence.ts +++ b/packages/mcp-server/src/persistence.ts @@ -30,7 +30,7 @@ export function defaultDataDir(): string | null { } /** Any cell besides the root cells "0" and "1" */ -const hasCells = (xml: string) => +export const hasCells = (xml: string) => /<(mxCell\b[^>]*\bid="(?![01]")|UserObject\b|object\b)/.test(xml) export class Autosaver { @@ -89,8 +89,10 @@ export class Autosaver { private removeOldest(): void { if (!this.dir) return const dir = this.dir + // Only our own session files: DRAWIO_DATA_DIR may be a folder + // that also holds the user's diagrams const files = readdirSync(dir) - .filter((f) => f.endsWith(".drawio")) + .filter((f) => f.startsWith("mcp-") && f.endsWith(".drawio")) .map((f) => ({ f, mtime: statSync(join(dir, f)).mtimeMs })) .sort((a, b) => b.mtime - a.mtime) for (const { f } of files.slice(this.maxFiles)) { diff --git a/packages/mcp-server/src/preview/preview.js b/packages/mcp-server/src/preview/preview.js index e7d5213b..2917d2f9 100644 --- a/packages/mcp-server/src/preview/preview.js +++ b/packages/mcp-server/src/preview/preview.js @@ -51,15 +51,22 @@ window.addEventListener('message', (e) => { const isPng = pendingMcpExport === 'png' && d.startsWith('data:image/png'); const isSvg = (pendingMcpExport === 'svg' || pendingMcpExport === 'xmlsvg') && (d.startsWith('data:image/svg') || d.startsWith(' {}); - // Page-targeted export: restore the user's real - // multi-page document now that we have the image. - restoreFromProjection(); + }).catch(() => {}).finally(() => { + // The timeout already ended this export + if (seq !== mcpExportSeq) return; + pendingMcpExport = null; + // Page-targeted export: restore the user's real + // multi-page document now that we have the image. + restoreFromProjection(); + }); } return; } diff --git a/packages/mcp-server/src/xml-validation.ts b/packages/mcp-server/src/xml-validation.ts index 0c6b617c..0d4ffbdf 100644 --- a/packages/mcp-server/src/xml-validation.ts +++ b/packages/mcp-server/src/xml-validation.ts @@ -369,7 +369,8 @@ function findOrphanMxPoints( xml: string, ): Array<{ start: number; end: number }> { const arrays: Array<[number, number]> = [] - for (const m of xml.matchAll(/]*>[\s\S]*?<\/Array>/g)) { + // (?, which has no points inside + for (const m of xml.matchAll(/]*(?[\s\S]*?<\/Array>/g)) { arrays.push([m.index, m.index + m[0].length]) } const orphans: Array<{ start: number; end: number }> = [] diff --git a/packages/mcp-server/tests/http-server.test.ts b/packages/mcp-server/tests/http-server.test.ts index 12fbfb29..a52cca82 100644 --- a/packages/mcp-server/tests/http-server.test.ts +++ b/packages/mcp-server/tests/http-server.test.ts @@ -153,6 +153,16 @@ describe("request origin checks", () => { }) describe("POST /api/state", () => { + it("refuses a push without xml and keeps the diagram", async () => { + setState("mcp-no-xml", "kept") + const res = await postJson("/api/state", { + sessionId: "mcp-no-xml", + baseVersion: 99, + }) + expect(res.status).toBe(400) + expect(getState("mcp-no-xml")?.xml).toBe("kept") + }) + it("decodes UTF-8 characters split across body chunks", async () => { const xml = `${"数据".repeat(30000)}` const body = Buffer.from(JSON.stringify({ sessionId: "mcp-utf8", xml })) diff --git a/packages/mcp-server/tests/persistence.test.ts b/packages/mcp-server/tests/persistence.test.ts index a6dda00c..9c37d0f9 100644 --- a/packages/mcp-server/tests/persistence.test.ts +++ b/packages/mcp-server/tests/persistence.test.ts @@ -49,10 +49,10 @@ describe("Autosaver", () => { ) }) - it("keeps only the newest files", () => { + it("keeps only the newest session files and never touches other files", () => { const dir = tempDir() const saver = new Autosaver(dir, 10, 2) - for (const [i, id] of ["mcp-old", "mcp-mid"].entries()) { + for (const [i, id] of ["mine", "mcp-old", "mcp-mid"].entries()) { writeFileSync(join(dir, `${id}.drawio`), DIAGRAM) utimesSync(join(dir, `${id}.drawio`), 1000 + i, 1000 + i) } @@ -61,6 +61,7 @@ describe("Autosaver", () => { expect(readdirSync(dir).sort()).toEqual([ "mcp-mid.drawio", "mcp-new.drawio", + "mine.drawio", ]) }) diff --git a/packages/mcp-server/tests/xml-validation.test.ts b/packages/mcp-server/tests/xml-validation.test.ts index 479281f8..f42b9143 100644 --- a/packages/mcp-server/tests/xml-validation.test.ts +++ b/packages/mcp-server/tests/xml-validation.test.ts @@ -245,4 +245,22 @@ describe("validateAndFixXml strict checks", () => { expect(r.fixed).toContain('as="sourcePoint"') expect(r.fixed).toContain('') }) + + it("finds an orphan mxPoint after an empty ", () => { + const edge = (id: string, points: string) => + `${points}` + const r = validateAndFixXml( + model( + edge("e1", ``) + + `` + + edge( + "e2", + ``, + ), + ), + ) + expect(r.valid).toBe(true) + expect(r.fixed).not.toContain('') + expect(r.fixed).toContain('') + }) })