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 <Array as="points"/> 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
This commit is contained in:
dayuan.jiang
2026-10-04 07:38:17 +09:00
parent 392c8af84a
commit 899924ba98
8 changed files with 77 additions and 29 deletions
+5
View File
@@ -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 }))
+23 -19
View File
@@ -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
+4 -2
View File
@@ -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)) {
+12 -5
View File
@@ -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('<svg'));
if (isPng || isSvg) {
pendingMcpExport = null;
// Keep pendingMcpExport set until the server has the
// result: a poll answered before that still sees the
// request and would start the same export again.
const seq = msg.message.mcpExport;
fetch('/api/state', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ sessionId, exportData: d })
}).catch(() => {});
// 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;
}
+2 -1
View File
@@ -369,7 +369,8 @@ function findOrphanMxPoints(
xml: string,
): Array<{ start: number; end: number }> {
const arrays: Array<[number, number]> = []
for (const m of xml.matchAll(/<Array\b[^>]*>[\s\S]*?<\/Array>/g)) {
// (?<!\/) skips an empty <Array/>, which has no points inside
for (const m of xml.matchAll(/<Array\b[^>]*(?<!\/)>[\s\S]*?<\/Array>/g)) {
arrays.push([m.index, m.index + m[0].length])
}
const orphans: Array<{ start: number; end: number }> = []
@@ -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", "<mxfile>kept</mxfile>")
const res = await postJson("/api/state", {
sessionId: "mcp-no-xml",
baseVersion: 99,
})
expect(res.status).toBe(400)
expect(getState("mcp-no-xml")?.xml).toBe("<mxfile>kept</mxfile>")
})
it("decodes UTF-8 characters split across body chunks", async () => {
const xml = `<mxfile>${"数据".repeat(30000)}</mxfile>`
const body = Buffer.from(JSON.stringify({ sessionId: "mcp-utf8", xml }))
@@ -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",
])
})
@@ -245,4 +245,22 @@ describe("validateAndFixXml strict checks", () => {
expect(r.fixed).toContain('as="sourcePoint"')
expect(r.fixed).toContain('<mxPoint x="1" y="2"/>')
})
it("finds an orphan mxPoint after an empty <Array/>", () => {
const edge = (id: string, points: string) =>
`<mxCell id="${id}" edge="1" parent="1"><mxGeometry relative="1" as="geometry">${points}</mxGeometry></mxCell>`
const r = validateAndFixXml(
model(
edge("e1", `<Array as="points"/>`) +
`<mxCell id="v" vertex="1" parent="1"><mxGeometry x="1" y="1" width="9" height="9" as="geometry"><mxPoint x="5" y="5"/></mxGeometry></mxCell>` +
edge(
"e2",
`<Array as="points"><mxPoint x="1" y="2"/></Array>`,
),
),
)
expect(r.valid).toBe(true)
expect(r.fixed).not.toContain('<mxPoint x="5" y="5"/>')
expect(r.fixed).toContain('<mxPoint x="1" y="2"/>')
})
})