refactor(mcp-server): remove dead code and fold repeated tool plumbing

- index.ts: registerTool() does the session check, try/catch and the
  write queue for all 15 tools; fail()/ok(), syncFromBrowser(),
  commitDocument() and describePages() replace the copied blocks; drop
  pickPageSelector, currentSession.version, the input ?? {} defaults;
  add_page reuses prepareNewModel() split out of prepareNewDiagram
- xml-validation.ts: drop the hand-written syntax checks that saxes
  already reports, MAX_XML_SIZE and the stale header
- http-server.ts: one pendingExport object, no unread response fields,
  no CORS/OPTIONS branch for same-origin requests, fewer exports
- diagram-operations parseNewCell(), addHistory returns void,
  normalizeToMxfile without host, foldCells without layer, expandStyles
  removed, unused preview.css rule removed, no .d.ts output
- xml-examples.ts styleRules() shares the Styles paragraphs with the
  web prompt
- tests: setupFiles installs the DOM polyfill once, server-wiring uses
  the SDK client, mcp-core.test.ts drops the cases MCP tests already
  cover
This commit is contained in:
dayuan.jiang
2026-10-12 08:25:47 +09:00
parent 165175be20
commit fc8ffc5655
35 changed files with 1068 additions and 2185 deletions
@@ -5,13 +5,7 @@
*/
import { deflateRawSync } from "node:zlib"
import { beforeAll, describe, expect, it, vi } from "vitest"
import { installDomPolyfill } from "../src/dom.ts"
beforeAll(() => {
installDomPolyfill()
})
import { describe, expect, it, vi } from "vitest"
import { applyDiagramOperations } from "../src/diagram-operations.ts"
const DOC = `<mxfile><diagram id="p" name="Page-1"><mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/><UserObject id="a" label="A" link="https://example.com"><mxCell vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell></UserObject><mxCell id="b" value="B" vertex="1" parent="1"><mxGeometry as="geometry"/></mxCell><object id="e1" label="" tooltip="t"><mxCell edge="1" source="b" target="a" parent="1"><mxGeometry relative="1" as="geometry"/></mxCell></object><mxCell id="child" value="C" vertex="1" parent="a"><mxGeometry as="geometry"/></mxCell></root></mxGraphModel></diagram></mxfile>`
@@ -2,13 +2,7 @@
* Tests for the all-or-nothing edit_diagram core (src/edit-diagram.ts).
*/
import { beforeAll, describe, expect, it } from "vitest"
import { installDomPolyfill } from "../src/dom.ts"
beforeAll(() => {
installDomPolyfill()
})
import { describe, expect, it } from "vitest"
import { editDiagram, targetPageXml } from "../src/edit-diagram.ts"
import { validateMxCellStructure } from "../src/xml-validation.ts"
+1 -7
View File
@@ -9,13 +9,7 @@
* reads as a user edit.
*/
import { beforeAll, describe, expect, it } from "vitest"
import { installDomPolyfill } from "../src/dom.ts"
beforeAll(() => {
installDomPolyfill()
})
import { describe, expect, it } from "vitest"
import {
checkEditGate,
contentFingerprint,
+1 -7
View File
@@ -2,8 +2,7 @@
* Tests for the diagram history: the versions restore_version can go to.
*/
import { beforeAll, describe, expect, it } from "vitest"
import { installDomPolyfill } from "../src/dom.ts"
import { describe, expect, it } from "vitest"
import {
addHistory,
clearHistory,
@@ -12,11 +11,6 @@ import {
otherVersions,
} from "../src/history.ts"
beforeAll(() => {
// XML parsing for the content fingerprints
installDomPolyfill()
})
const page = (cellId: string) =>
`<mxfile><diagram id="p" name="P"><mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/><mxCell id="${cellId}" vertex="1" parent="1"/></root></mxGraphModel></diagram></mxfile>`
+7 -10
View File
@@ -8,7 +8,6 @@
import http from "node:http"
import { afterAll, beforeAll, describe, expect, it } from "vitest"
import { installDomPolyfill } from "../src/dom.ts"
import { addHistory, getHistory } from "../src/history.ts"
import {
drawioEmbedParams,
@@ -28,8 +27,6 @@ import {
let port = 0
beforeAll(async () => {
// XML parsing, as the server installs it at startup
installDomPolyfill()
port = await startHttpServer(40000 + Math.floor(Math.random() * 10000))
})
@@ -330,15 +327,15 @@ describe("export requests", () => {
const poll = JSON.parse(
(await request(`/api/state?sessionId=${id}`)).body,
)
expect(poll.exportFormat).toBe("png")
expect(poll.exportOptions).toEqual({ width: 1000, pageId: "p2" })
expect(poll.export.format).toBe("png")
expect(poll.export.options).toEqual({ width: 1000, pageId: "p2" })
await postJson("/api/state", {
sessionId: id,
exportData: "data:image/png;base64,AAAA",
exportId: poll.exportId,
exportId: poll.export.id,
})
expect(getState(id)?.exportOptions).toBeUndefined()
expect(getState(id)?.pendingExport).toBeUndefined()
})
it("ignores a late result of an export that already timed out", async () => {
@@ -353,10 +350,10 @@ describe("export requests", () => {
await postJson("/api/state", {
sessionId: id,
exportData: "data:image/png;base64,LATE",
exportId: first.exportId,
exportId: first.export.id,
})
expect(getState(id)?.exportData).toBeUndefined()
expect(getState(id)?.exportFormat).toBe("svg")
expect(getState(id)?.pendingExport?.format).toBe("svg")
})
})
@@ -388,7 +385,7 @@ describe("a session state recreated after it was lost", () => {
stateId,
})
expect(res.status).toBe(409)
expect(JSON.parse(res.body).stateChanged).toBe(true)
expect(JSON.parse(res.body).error).toBe("Session was recreated")
// The saved file was recovered first and is kept
expect(getState("mcp-sid-restart")?.xml).toBe(SAVED)
}
+1 -9
View File
@@ -15,15 +15,7 @@
* (diagram-operations.ts) — i.e. the layers underneath the MCP tool surface.
*/
import { beforeAll, describe, expect, it } from "vitest"
import { installDomPolyfill } from "../src/dom.ts"
// Install the DOM polyfill exactly as index.ts does at runtime — the
// helpers under test rely on it.
beforeAll(() => {
installDomPolyfill()
})
import { describe, expect, it } from "vitest"
import { applyDiagramOperations } from "../src/diagram-operations.ts"
import {
addPageToDoc,
@@ -4,13 +4,7 @@
* never sent.
*/
import { beforeAll, describe, expect, it } from "vitest"
import { installDomPolyfill } from "../src/dom.ts"
beforeAll(() => {
installDomPolyfill()
})
import { describe, expect, it } from "vitest"
import {
isMxCellXmlComplete,
prepareNewDiagram,
@@ -15,14 +15,9 @@ import {
} from "node:fs"
import { homedir, tmpdir } from "node:os"
import { join } from "node:path"
import { afterEach, beforeAll, describe, expect, it } from "vitest"
import { installDomPolyfill } from "../src/dom.ts"
import { afterEach, describe, expect, it } from "vitest"
import { Autosaver, defaultDataDir } from "../src/persistence.ts"
beforeAll(() => {
installDomPolyfill()
})
const DIAGRAM = `<mxfile><diagram id="p" name="P"><mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/><mxCell id="a" vertex="1" parent="1"/></root></mxGraphModel></diagram></mxfile>`
const BLANK = `<mxfile><diagram id="page-1" name="Page-1"><mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/></root></mxGraphModel></diagram></mxfile>`
const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms))
+78 -164
View File
@@ -1,7 +1,7 @@
/**
* Server-wiring test: boot the actual MCP stdio server (from source via tsx)
* and drive it the way a real MCP client does — initialize handshake,
* tools/list — to catch registration/schema regressions that the unit tests
* and drive it with the SDK's own client — initialize handshake, tools/list,
* tools/call — to catch registration/schema regressions that the unit tests
* (which import helpers directly) can't see.
*
* This replaces the old standalone tests/smoke.mjs, which spawned the BUILT
@@ -13,11 +13,13 @@
* window via open(). The browser bridge is covered by the Playwright e2e suite.
*/
import { type ChildProcessWithoutNullStreams, spawn } from "node:child_process"
import { mkdtempSync, rmSync, writeFileSync } from "node:fs"
import { tmpdir } from "node:os"
import path from "node:path"
import { fileURLToPath } from "node:url"
import { Client } from "@modelcontextprotocol/sdk/client/index.js"
import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"
import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"
import { afterAll, beforeAll, describe, expect, it } from "vitest"
const __dirname = path.dirname(fileURLToPath(import.meta.url))
@@ -60,115 +62,69 @@ const MAX_DESCRIPTION = 2048
// developer's real ~/.next-ai-drawio/instructions.md must not leak in
const CUSTOM_RULE = "Always use blue fill #dae8fc."
let proc: ChildProcessWithoutNullStreams
let stdoutBuf = ""
const pending = new Map<
number,
{ resolve: (m: any) => void; reject: (e: Error) => void; timeout: any }
>()
let nextId = 1
let initResp: any
let client: Client
function send(method: string, params: unknown, isNotification = false) {
const msg: Record<string, unknown> = { jsonrpc: "2.0", method, params }
if (!isNotification) msg.id = nextId++
proc.stdin.write(`${JSON.stringify(msg)}\n`)
if (isNotification) return Promise.resolve(undefined)
return new Promise<any>((resolve, reject) => {
const id = msg.id as number
const timeout = setTimeout(() => {
pending.delete(id)
reject(new Error(`Timed out waiting for response to ${method}`))
}, 15000)
pending.set(id, { resolve, reject, timeout })
})
}
const listTools = async () => (await client.listTools()).tools
const findTool = async (name: string) =>
(await listTools()).find((t) => t.name === name)
const properties = async (name: string) =>
(await findTool(name))?.inputSchema?.properties ?? {}
/** A tool call's result; the SDK throws on a JSON-RPC error */
const callTool = async (name: string, args: Record<string, unknown>) =>
(await client.callTool({ name, arguments: args })) as CallToolResult
const textOf = (result: CallToolResult) =>
(result.content[0] as { text: string }).text
beforeAll(async () => {
writeFileSync(path.join(dataDir, "instructions.md"), CUSTOM_RULE)
writeFileSync(path.join(dataDir, `${SAVED_ID}.drawio`), SAVED_DIAGRAM)
proc = spawn(tsxBin, [entry], {
stdio: ["pipe", "pipe", "pipe"],
env: { ...process.env, DRAWIO_DATA_DIR: dataDir },
}) as ChildProcessWithoutNullStreams
proc.stdout.on("data", (chunk: Buffer) => {
stdoutBuf += chunk.toString()
const lines = stdoutBuf.split("\n")
stdoutBuf = lines.pop() || ""
for (const line of lines) {
const trimmed = line.trim()
if (!trimmed) continue
let msg: any
try {
msg = JSON.parse(trimmed)
} catch {
// Non-JSON-RPC log line — ignore.
continue
}
const p = msg.id !== undefined ? pending.get(msg.id) : undefined
if (p) {
clearTimeout(p.timeout)
pending.delete(msg.id)
p.resolve(msg)
}
}
})
initResp = await send("initialize", {
protocolVersion: "2024-11-05",
capabilities: {},
clientInfo: { name: "wiring-test", version: "0.0.0" },
})
expect(initResp.error, JSON.stringify(initResp.error)).toBeUndefined()
expect(initResp.result?.serverInfo?.name).toBeTruthy()
await send("notifications/initialized", {}, true)
client = new Client({ name: "wiring-test", version: "0.0.0" })
await client.connect(
new StdioClientTransport({
command: tsxBin,
args: [entry],
env: { ...process.env, DRAWIO_DATA_DIR: dataDir } as Record<
string,
string
>,
// The server's log lines
stderr: "ignore",
}),
)
expect(client.getServerVersion()?.name).toBeTruthy()
}, 30000)
afterAll(() => {
proc?.kill("SIGTERM")
afterAll(async () => {
await client?.close()
rmSync(dataDir, { recursive: true, force: true })
})
describe("MCP server wiring", () => {
it("registers all tools", async () => {
const resp = await send("tools/list", {})
expect(resp.error, JSON.stringify(resp.error)).toBeUndefined()
const names: string[] = (resp.result?.tools ?? []).map(
(t: { name: string }) => t.name,
)
const names = (await listTools()).map((t) => t.name)
for (const expected of EXPECTED_TOOLS) {
expect(names, `missing tool: ${expected}`).toContain(expected)
}
})
it("advertises page-selector params on edit_diagram", async () => {
const resp = await send("tools/list", {})
const edit = resp.result.tools.find(
(t: { name: string }) => t.name === "edit_diagram",
)
const props = edit?.inputSchema?.properties ?? {}
const props = await properties("edit_diagram")
expect(props.page_id).toBeTruthy()
expect(props.page_name).toBeTruthy()
expect(props.page_index).toBeTruthy()
})
it("advertises session_id on start_session", async () => {
const resp = await send("tools/list", {})
const start = resp.result.tools.find(
(t: { name: string }) => t.name === "start_session",
)
const start = await findTool("start_session")
expect(start?.inputSchema?.properties?.session_id).toBeTruthy()
expect(start?.inputSchema?.required ?? []).not.toContain("session_id")
})
it("lists the saved diagrams with their pages, without a session", async () => {
const resp = await send("tools/call", {
name: "list_saved_diagrams",
arguments: {},
})
expect(resp.result.isError).toBeFalsy()
const text: string = resp.result.content[0].text
const result = await callTool("list_saved_diagrams", {})
expect(result.isError).toBeFalsy()
const text = textOf(result)
const line = text.split("\n").find((l) => l.startsWith(SAVED_ID))
expect(line, text).toBeTruthy()
expect(line).toContain("pages: Flow (3 cells)")
@@ -177,81 +133,59 @@ describe("MCP server wiring", () => {
})
it("advertises screenshot on create_new_diagram and edit_diagram", async () => {
const resp = await send("tools/list", {})
for (const name of ["create_new_diagram", "edit_diagram"]) {
const tool = resp.result.tools.find(
(t: { name: string }) => t.name === name,
)
const props = tool?.inputSchema?.properties ?? {}
expect(props.screenshot?.type, `${name} screenshot`).toBe("boolean")
const props = await properties(name)
const screenshot = props.screenshot as { type?: string } | undefined
expect(screenshot?.type, `${name} screenshot`).toBe("boolean")
}
})
it("advertises name/id/xml on add_page", async () => {
const resp = await send("tools/list", {})
const addPage = resp.result.tools.find(
(t: { name: string }) => t.name === "add_page",
)
const props = addPage?.inputSchema?.properties ?? {}
const props = await properties("add_page")
expect(props.name).toBeTruthy()
expect(props.id).toBeTruthy()
expect(props.xml).toBeTruthy()
})
it("advertises steps_back on restore_version", async () => {
const resp = await send("tools/list", {})
const restore = resp.result.tools.find(
(t: { name: string }) => t.name === "restore_version",
)
expect(restore?.inputSchema?.properties?.steps_back).toBeTruthy()
const props = await properties("restore_version")
expect(props.steps_back).toBeTruthy()
})
it("refuses restore_version without a session", async () => {
const resp = await send("tools/call", {
name: "restore_version",
arguments: {},
})
expect(resp.result.isError).toBe(true)
expect(resp.result.content[0].text).toContain("start_session")
const result = await callTool("restore_version", {})
expect(result.isError).toBe(true)
expect(textOf(result)).toContain("start_session")
})
it("keeps every description and the instructions within the host limit", async () => {
const resp = await send("tools/list", {})
for (const tool of resp.result.tools) {
for (const tool of await listTools()) {
expect(
tool.description.length,
tool.description?.length,
`${tool.name} description length`,
).toBeLessThanOrEqual(MAX_DESCRIPTION)
}
const instructions: string = initResp.result.instructions
const instructions = client.getInstructions() ?? ""
expect(instructions).toContain("start_session")
expect(instructions.length).toBeLessThanOrEqual(MAX_DESCRIPTION)
})
it("serves a shape library without a session", async () => {
const resp = await send("tools/call", {
name: "get_shape_library",
arguments: { library: "AWS4" },
})
expect(resp.result.isError).toBeFalsy()
expect(resp.result.content[0].text).toContain("mxgraph.aws4")
const result = await callTool("get_shape_library", { library: "AWS4" })
expect(result.isError).toBeFalsy()
expect(textOf(result)).toContain("mxgraph.aws4")
})
it("reports an unknown shape library with the available names", async () => {
const resp = await send("tools/call", {
name: "get_shape_library",
arguments: { library: "../secrets" },
const result = await callTool("get_shape_library", {
library: "../secrets",
})
expect(resp.result.isError).toBe(true)
expect(resp.result.content[0].text).toContain("kubernetes")
expect(result.isError).toBe(true)
expect(textOf(result)).toContain("kubernetes")
})
it("serves the drawing guide without a session", async () => {
const resp = await send("tools/call", {
name: "get_drawing_guide",
arguments: {},
})
const text: string = resp.result.content[0].text
const text = textOf(await callTool("get_drawing_guide", {}))
expect(text).toContain("Edge routing rules")
expect(text).toContain("do not describe the diagram")
expect(text).toContain("clear the canvas")
@@ -259,31 +193,21 @@ describe("MCP server wiring", () => {
})
it("appends the user's instructions.md to the drawing guide", async () => {
const resp = await send("tools/call", {
name: "get_drawing_guide",
arguments: {},
})
const text: string = resp.result.content[0].text
const text = textOf(await callTool("get_drawing_guide", {}))
expect(
text.endsWith(`\n\n## Custom Instructions\n${CUSTOM_RULE}`),
).toBe(true)
})
it("advertises topic on get_drawing_guide", async () => {
const resp = await send("tools/list", {})
const guide = resp.result.tools.find(
(t: { name: string }) => t.name === "get_drawing_guide",
)
expect(guide?.inputSchema?.properties?.topic).toBeTruthy()
const props = await properties("get_drawing_guide")
expect(props.topic).toBeTruthy()
})
it("serves the table XML reference with topic=tables", async () => {
const resp = await send("tools/call", {
name: "get_drawing_guide",
arguments: { topic: "tables" },
})
expect(resp.result.isError).toBeFalsy()
const text: string = resp.result.content[0].text
const result = await callTool("get_drawing_guide", { topic: "tables" })
expect(result.isError).toBeFalsy()
const text = textOf(result)
expect(text).toContain("shape=table")
expect(text).not.toContain("Edge routing rules")
})
@@ -291,10 +215,7 @@ describe("MCP server wiring", () => {
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 load = await findTool("load_diagram")
const props = load?.inputSchema?.properties ?? {}
expect(props.path).toBeTruthy()
expect(props.xml).toBeTruthy()
@@ -307,32 +228,25 @@ describe("load_diagram dual-source arguments", () => {
// 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: "<mxfile/>" },
const result = await callTool("load_diagram", {
path: "/tmp/x.drawio",
xml: "<mxfile/>",
})
expect(resp.error, JSON.stringify(resp.error)).toBeUndefined()
expect(resp.result?.isError).toBe(true)
expect(resp.result?.content?.[0]?.text).toContain("not both")
expect(result.isError).toBe(true)
expect(textOf(result)).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'")
const result = await callTool("load_diagram", {})
expect(result.isError).toBe(true)
expect(textOf(result)).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: '<mxfile><diagram id="p" name="P"><mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/></root></mxGraphModel></diagram></mxfile>',
},
const result = await callTool("load_diagram", {
xml: '<mxfile><diagram id="p" name="P"><mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/></root></mxGraphModel></diagram></mxfile>',
})
expect(resp.result?.isError).toBe(true)
expect(resp.result?.content?.[0]?.text).toContain("No active session")
expect(result.isError).toBe(true)
expect(textOf(result)).toContain("No active session")
})
})
+5
View File
@@ -0,0 +1,5 @@
// The DOMParser and XMLSerializer globals the XML helpers need, installed
// before every test file as index.ts does at startup
import { installDomPolyfill } from "../src/dom.ts"
installDomPolyfill()
+17 -7
View File
@@ -3,13 +3,7 @@
* a page, like in the web app, and the server adds the wrapper and root cells.
*/
import { beforeAll, describe, expect, it } from "vitest"
import { installDomPolyfill } from "../src/dom.ts"
beforeAll(() => {
installDomPolyfill()
})
import { describe, expect, it } from "vitest"
import { prepareNewDiagram, reservedIdError } from "../src/new-diagram.ts"
import { hasCells, wrapCellsInModel } from "../src/pages.ts"
import { validateAndFixXml } from "../src/xml-validation.ts"
@@ -100,6 +94,22 @@ describe("hasCells", () => {
hasCells(`<mxCell id = "0"/><mxCell id = "1" parent="0"/>`),
).toBe(false)
})
it("does not mistake ids that start with 0 or 1 for root cells", () => {
expect(
hasCells(
'<mxCell id="0"/><mxCell id="1" parent="0"/><mxCell id="10"/>',
),
).toBe(true)
})
it("counts a cell wrapped in a UserObject", () => {
expect(
hasCells(
'<mxCell id="0"/><mxCell id="1" parent="0"/><UserObject id="u" link="x"><mxCell vertex="1" parent="1"/></UserObject>',
),
).toBe(true)
})
})
describe("reservedIdError", () => {
@@ -4,13 +4,7 @@
* whether draw.io renders it as intended is checked by hand.
*/
import { beforeAll, describe, expect, it } from "vitest"
import { installDomPolyfill } from "../src/dom.ts"
beforeAll(() => {
installDomPolyfill()
})
import { describe, expect, it } from "vitest"
import { prepareNewDiagram } from "../src/new-diagram.ts"
import { hasCells } from "../src/pages.ts"
import { XML_REFERENCE } from "../src/xml-reference.ts"
@@ -7,15 +7,9 @@
* leave valid parts of the document untouched.
*/
import { beforeAll, describe, expect, it } from "vitest"
import { installDomPolyfill } from "../src/dom.ts"
import { getXmlSyntaxError } from "../src/xml-syntax.ts"
beforeAll(() => {
installDomPolyfill()
})
import { describe, expect, it } from "vitest"
import { addPageToDoc, parseMxfile, serializeMxfile } from "../src/pages.ts"
import { getXmlSyntaxError } from "../src/xml-syntax.ts"
import { validateAndFixXml } from "../src/xml-validation.ts"
/** Bare model with the root cells plus the given cells. */