refactor(web): validate and repair diagram XML with the MCP server's engine

- Delete the web app's own copy of the XML checks and repairs from
  lib/utils.ts (1,074 lines). loadDiagram now uses the MCP server's
  validateAndFixXml without the strict checks, because the XML may hold
  the user's own diagram
- display_diagram and append_diagram prepare the model's XML with the new
  shared prepareNewDiagram, also used by the MCP create_new_diagram: wrap,
  validate strictly and auto-fix while it is still a bare model (where
  duplicate ids are renamed), then turn it into an mxfile
- The streaming preview of display_diagram no longer redraws the model's
  raw cells after the tool handler loaded the checked diagram, and drops
  a queued preview once the input is complete. That redraw lost
  auto-fixes and UserObject/object wrappers, so a linked cell lost its
  label; it also showed a second error toast
- The web repair regression tests now run against the MCP functions
- New e2e test checks the canvas content after display_diagram
- Fix the e2e upload tests, whose file input locator also matched the
  template import input
This commit is contained in:
dayuan.jiang
2026-10-04 12:44:01 +09:00
parent b7c543ca70
commit e49e9f0c47
14 changed files with 276 additions and 1303 deletions
+81
View File
@@ -0,0 +1,81 @@
import { expect, test } from "@playwright/test"
import { getIframe, sendMessage, waitForCompleteCount } from "./lib/fixtures"
/**
* Checks what draw.io actually shows after display_diagram, not only the
* tool card. The tool input is streamed in chunks like a real model, and
* the browser tool handler (not the server) completes the tool call.
*/
function streamedToolCall(xml: string) {
const toolCallId = `call_${Math.random().toString(36).slice(2)}`
const input = JSON.stringify({ xml })
const chunks = input.match(/[\s\S]{1,40}/g) ?? []
const events = [
{ type: "start", messageId: `msg_${toolCallId}` },
{ type: "tool-input-start", toolCallId, toolName: "display_diagram" },
...chunks.map((inputTextDelta) => ({
type: "tool-input-delta",
toolCallId,
inputTextDelta,
})),
{
type: "tool-input-available",
toolCallId,
toolName: "display_diagram",
input: { xml },
},
{ type: "finish" },
]
return `${events.map((e) => `data: ${JSON.stringify(e)}\n\n`).join("")}data: [DONE]\n\n`
}
const cell = (id: string, label: string, x: number) =>
`<mxCell id="${id}" value="${label}" style="rounded=1;" vertex="1" parent="1"><mxGeometry x="${x}" y="40" width="120" height="60" as="geometry"/></mxCell>`
const page = (id: string, cells: string) =>
`<diagram id="${id}" name="${id}"><mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/>${cells}</root></mxGraphModel></diagram>`
const TWO_PAGES = `<mxfile>${page("First", cell("a", "Old A", 40))}${page("Second", cell("b", "Old B", 40))}</mxfile>`
// Bare cells with a duplicate id and an unescaped &, which get fixed, and
// a linked cell whose label lives on its UserObject wrapper
const NEW_CELLS =
cell("2", "Alpha", 40) +
cell("2", "Beta", 220) +
cell("3", "R&D", 400) +
`<UserObject id="4" label="Docs" link="https://example.com"><mxCell style="rounded=1;" vertex="1" parent="1"><mxGeometry x="580" y="40" width="120" height="60" as="geometry"/></mxCell></UserObject>`
test("display_diagram replaces the document with the fixed diagram", async ({
page: p,
}) => {
const replies = [TWO_PAGES, NEW_CELLS]
await p.route("**/api/chat", async (route) => {
const xml = replies.shift()
await route.fulfill({
status: 200,
contentType: "text/event-stream",
body: xml
? streamedToolCall(xml)
: 'data: {"type":"start"}\n\ndata: {"type":"finish"}\n\ndata: [DONE]\n\n',
})
})
await p.goto("/", { waitUntil: "networkidle" })
await getIframe(p).waitFor({ state: "visible", timeout: 30000 })
const canvas = p.frameLocator("iframe")
await sendMessage(p, "Draw two pages")
await waitForCompleteCount(p, 1)
await expect(canvas.getByText("Old A")).toBeVisible({ timeout: 15000 })
await expect(canvas.getByText("Second", { exact: true })).toBeVisible()
await sendMessage(p, "Start over with three boxes")
await waitForCompleteCount(p, 2)
// Give a late preview time to redraw the raw cells, as it used to
await p.waitForTimeout(1000)
for (const label of ["Alpha", "Beta", "R&D", "Docs"]) {
await expect(canvas.getByText(label, { exact: true })).toBeVisible({
timeout: 15000,
})
}
// The old pages are gone
await expect(canvas.getByText("Old A")).toHaveCount(0)
await expect(canvas.getByText("Second", { exact: true })).toHaveCount(0)
})
+8 -4
View File
@@ -24,7 +24,8 @@ test.describe("File Upload", () => {
await page.goto("/", { waitUntil: "networkidle" })
await getIframe(page).waitFor({ state: "visible", timeout: 30000 })
const fileInput = page.locator('input[type="file"]')
// The chat attachment input; the template panel has its own file input
const fileInput = page.locator('input[type="file"][multiple]')
await fileInput.setInputFiles({
name: "test-image.png",
@@ -44,7 +45,8 @@ test.describe("File Upload", () => {
await page.goto("/", { waitUntil: "networkidle" })
await getIframe(page).waitFor({ state: "visible", timeout: 30000 })
const fileInput = page.locator('input[type="file"]')
// The chat attachment input; the template panel has its own file input
const fileInput = page.locator('input[type="file"][multiple]')
await fileInput.setInputFiles({
name: "test-image.png",
@@ -91,7 +93,8 @@ test.describe("File Upload", () => {
await page.goto("/", { waitUntil: "networkidle" })
await getIframe(page).waitFor({ state: "visible", timeout: 30000 })
const fileInput = page.locator('input[type="file"]')
// The chat attachment input; the template panel has its own file input
const fileInput = page.locator('input[type="file"][multiple]')
await fileInput.setInputFiles({
name: "architecture.png",
@@ -115,7 +118,8 @@ test.describe("File Upload", () => {
await page.goto("/", { waitUntil: "networkidle" })
await getIframe(page).waitFor({ state: "visible", timeout: 30000 })
const fileInput = page.locator('input[type="file"]')
// The chat attachment input; the template panel has its own file input
const fileInput = page.locator('input[type="file"][multiple]')
const largeBuffer = Buffer.alloc(3 * 1024 * 1024, "x")
await fileInput.setInputFiles({
+93 -1
View File
@@ -14,7 +14,11 @@ import {
wrapCellsInModel,
} from "@/packages/mcp-server/src/pages.ts"
import { getXmlSyntaxError } from "@/packages/mcp-server/src/xml-syntax.ts"
import { validateAndFixXml } from "@/packages/mcp-server/src/xml-validation.ts"
import {
autoFixXml,
validateAndFixXml,
validateMxCellStructure,
} from "@/packages/mcp-server/src/xml-validation.ts"
const box = (id: string, parent = "1") =>
`<mxCell id="${id}" value="${id}" vertex="1" parent="${parent}"><mxGeometry x="0" y="0" width="80" height="40" as="geometry"/></mxCell>`
@@ -88,3 +92,91 @@ describe("MCP diagram modules with a browser DOM", () => {
expect(decompressPageContent("not compressed")).toBeNull()
})
})
// Repair cases fixed in the web app's own copy before it moved here
const page = (id: string, cells: string) =>
`<diagram name="${id}" id="${id}"><mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/>${cells}</root></mxGraphModel></diagram>`
describe("duplicate ids in multi-page documents", () => {
const shape = (id: string, value = "Box") =>
`<mxCell id="${id}" value="${value}" vertex="1" parent="1"><mxGeometry x="0" y="0" width="80" height="40" as="geometry"/></mxCell>`
it("accepts the same ids on different pages", () => {
const xml = `<mxfile>${page("p1", shape("2"))}${page("p2", shape("2"))}</mxfile>`
expect(validateMxCellStructure(xml)).toBeNull()
})
it("still reports duplicate ids within one page", () => {
const xml = `<mxfile>${page("p1", shape("2") + shape("2"))}${page("p2", "")}</mxfile>`
expect(validateMxCellStructure(xml)).toMatch(/duplicate cell ID/i)
})
it("does not rename the root cells of other pages when fixing", () => {
const xml = `<mxfile>${page("p1", shape("2", "R&D"))}${page("p2", shape("3"))}</mxfile>`
const result = validateAndFixXml(xml)
expect(result.valid).toBe(true)
expect(result.fixed).not.toContain("_dup")
expect(result.fixed).toContain("R&amp;D")
})
it("renames a duplicate id in a bare model, as display_diagram has", () => {
// In an <mxfile> the duplicate is reported instead (above)
const xml = `<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/>${shape("d") + shape("d")}</root></mxGraphModel>`
const { fixed } = autoFixXml(xml)
expect(fixed).toContain('<mxCell id="d" ')
expect(fixed).toContain('<mxCell id="d_dup1" ')
})
})
describe("autoFixXml", () => {
it("does not insert a space at the start of style values", () => {
const xml = `<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/><mxCell id="2" value="R&D" style="rounded=1;whiteSpace=wrap;" vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell></root></mxGraphModel>`
const { fixed } = autoFixXml(xml)
expect(fixed).toContain('style="rounded=1;whiteSpace=wrap;"')
})
it("adds a missing space between attributes", () => {
const xml = `<mxCell id="2" vertex="1"parent="1"/>`
expect(autoFixXml(xml).fixed).toContain('vertex="1" parent="1"')
})
it("keeps &quot; inside rich text labels", () => {
const label = "&lt;font color=&quot;#ff0000&quot;&gt;Hello&lt;/font&gt;"
const xml = `<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/><mxCell id="2" value="${label}" style="html=1;" vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell><mxCell id="3" value="Q&A" vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell></root></mxGraphModel>`
const result = validateAndFixXml(xml)
expect(result.valid).toBe(true)
expect(result.fixed).toContain(`value="${label}"`)
})
it("fixes an attribute delimited by &quot;", () => {
const xml = `<mxCell id="2" dashPattern=&quot;1 1;&quot; vertex="1" parent="1"/>`
expect(autoFixXml(xml).fixed).toContain('dashPattern="1 1;"')
})
it("keeps cells written on one line next to multi-line cells", () => {
const xml = `<mxGraphModel><root>
<mxCell id="0"/>
<mxCell id="1" parent="0"/>
<mxCell id="2" value="Q&A" vertex="1" parent="1">
<mxGeometry x="0" y="0" width="80" height="40" as="geometry"/>
</mxCell>
<mxCell id="e1" edge="1" parent="1" source="2" target="3"><mxGeometry relative="1" as="geometry"/></mxCell>
<mxCell id="3" value="B" vertex="1" parent="1">
<mxGeometry x="200" y="0" width="80" height="40" as="geometry"/>
</mxCell>
</root></mxGraphModel>`
const result = validateAndFixXml(xml)
expect(result.valid).toBe(true)
for (const id of ["2", "e1", "3"]) {
expect(result.fixed).toContain(`<mxCell id="${id}"`)
}
})
it("keeps object and UserObject wrappers", () => {
const xml = `<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/><UserObject id="2" label="Docs" link="https://example.com"><mxCell vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell></UserObject><object id="3" label="A&B" owner="me"><mxCell vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell></object></root></mxGraphModel>`
const result = validateAndFixXml(xml)
expect(result.valid).toBe(true)
expect(result.fixed).toContain('<UserObject id="2"')
expect(result.fixed).toContain('<object id="3"')
})
})
-89
View File
@@ -1,12 +1,9 @@
import { describe, expect, it } from "vitest"
import {
applyDiagramOperations,
autoFixXml,
cn,
extractCompleteMxCells,
isMxCellXmlComplete,
validateAndFixXml,
validateMxCellStructure,
wrapWithMxFile,
} from "@/lib/utils"
@@ -136,92 +133,6 @@ describe("extractCompleteMxCells", () => {
})
})
const page = (id: string, cells: string) =>
`<diagram name="${id}" id="${id}"><mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/>${cells}</root></mxGraphModel></diagram>`
describe("duplicate ids in multi-page documents", () => {
const shape = (id: string, value = "Box") =>
`<mxCell id="${id}" value="${value}" vertex="1" parent="1"><mxGeometry x="0" y="0" width="80" height="40" as="geometry"/></mxCell>`
it("accepts the same ids on different pages", () => {
const xml = `<mxfile>${page("p1", shape("2"))}${page("p2", shape("2"))}</mxfile>`
expect(validateMxCellStructure(xml)).toBeNull()
})
it("still reports duplicate ids within one page", () => {
const xml = `<mxfile>${page("p1", shape("2") + shape("2"))}${page("p2", "")}</mxfile>`
expect(validateMxCellStructure(xml)).toContain("duplicate ID")
})
it("does not rename the root cells of other pages when fixing", () => {
const xml = `<mxfile>${page("p1", shape("2", "R&D"))}${page("p2", shape("3"))}</mxfile>`
const result = validateAndFixXml(xml)
expect(result.valid).toBe(true)
expect(result.fixed).not.toContain("_dup")
expect(result.fixed).toContain("R&amp;D")
})
it("renames a duplicate id within a page", () => {
const xml = `<mxfile>${page("p1", shape("d") + shape("d"))}</mxfile>`
const { fixed } = autoFixXml(xml)
expect(fixed).toContain('<mxCell id="d" ')
expect(fixed).toContain('<mxCell id="d_dup1" ')
})
})
describe("autoFixXml", () => {
it("does not insert a space at the start of style values", () => {
const xml = `<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/><mxCell id="2" value="R&D" style="rounded=1;whiteSpace=wrap;" vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell></root></mxGraphModel>`
const { fixed } = autoFixXml(xml)
expect(fixed).toContain('style="rounded=1;whiteSpace=wrap;"')
})
it("adds a missing space between attributes", () => {
const xml = `<mxCell id="2" vertex="1"parent="1"/>`
expect(autoFixXml(xml).fixed).toContain('vertex="1" parent="1"')
})
it("keeps &quot; inside rich text labels", () => {
const label = "&lt;font color=&quot;#ff0000&quot;&gt;Hello&lt;/font&gt;"
const xml = `<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/><mxCell id="2" value="${label}" style="html=1;" vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell><mxCell id="3" value="Q&A" vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell></root></mxGraphModel>`
const result = validateAndFixXml(xml)
expect(result.valid).toBe(true)
expect(result.fixed).toContain(`value="${label}"`)
})
it("fixes an attribute delimited by &quot;", () => {
const xml = `<mxCell id="2" dashPattern=&quot;1 1;&quot; vertex="1" parent="1"/>`
expect(autoFixXml(xml).fixed).toContain('dashPattern="1 1;"')
})
it("keeps cells written on one line next to multi-line cells", () => {
const xml = `<mxGraphModel><root>
<mxCell id="0"/>
<mxCell id="1" parent="0"/>
<mxCell id="2" value="Q&A" vertex="1" parent="1">
<mxGeometry x="0" y="0" width="80" height="40" as="geometry"/>
</mxCell>
<mxCell id="e1" edge="1" parent="1" source="2" target="3"><mxGeometry relative="1" as="geometry"/></mxCell>
<mxCell id="3" value="B" vertex="1" parent="1">
<mxGeometry x="200" y="0" width="80" height="40" as="geometry"/>
</mxCell>
</root></mxGraphModel>`
const result = validateAndFixXml(xml)
expect(result.valid).toBe(true)
for (const id of ["2", "e1", "3"]) {
expect(result.fixed).toContain(`<mxCell id="${id}"`)
}
})
it("keeps object and UserObject wrappers", () => {
const xml = `<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/><UserObject id="2" label="Docs" link="https://example.com"><mxCell vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell></UserObject><object id="3" label="A&B" owner="me"><mxCell vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell></object></root></mxGraphModel>`
const result = validateAndFixXml(xml)
expect(result.valid).toBe(true)
expect(result.fixed).toContain('<UserObject id="2"')
expect(result.fixed).toContain('<object id="3"')
})
})
describe("applyDiagramOperations with wrapped cells", () => {
const xml = `<mxfile><diagram id="p1"><mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/><UserObject id="5" label="Docs" link="https://example.com"><mxCell vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell></UserObject><mxCell id="6" value="B" vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell><mxCell id="e1" edge="1" parent="1" source="5" target="6"><mxGeometry relative="1" as="geometry"/></mxCell></root></mxGraphModel></diagram></mxfile>`