From 8171693ef66b38952c29970a000feea8bd2f8a49 Mon Sep 17 00:00:00 2001 From: "dayuan.jiang" Date: Sun, 11 Oct 2026 12:11:10 +0900 Subject: [PATCH] fix(mcp-server): review fixes for the draw.io file list and its guard - The list now ships the templates of Insert > Template, the PlantUML parser of Insert > Advanced and the template dialog's icon: the menu items were shown but failed with 404s. 15.1 MB packed; the tarball cap goes from 40 MB to 20 MB, where it still catches a list that grew by a whole js/ directory. - The guard drives both features, fails when the copy it runs against lacks files the editor asked for or when an export does not answer, checks that the copy is the pinned draw.io version, and runs in CI (npm run check-drawio, one E2E shard) so a list regression cannot reach a release. --- .github/workflows/test.yml | 7 +++ packages/mcp-server/drawio-files.txt | 8 ++- packages/mcp-server/package.json | 1 + .../mcp-server/scripts/check-drawio-files.mjs | 63 ++++++++++++++++--- packages/mcp-server/scripts/check-package.mjs | 6 +- 5 files changed, 72 insertions(+), 13 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fc2d7798..f36935a4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -81,6 +81,13 @@ jobs: - name: Build app run: npm run build + # The MCP package ships a trimmed draw.io; this drives the full copy + # (public/drawio, downloaded by the build) in Chromium and fails when + # the editor requests a file its list does not have. Once is enough. + - name: Check the MCP draw.io file list + if: matrix.shard == 1 + run: npm --prefix packages/mcp-server run check-drawio + - name: Run E2E tests run: npm run test:e2e -- --shard=${{ matrix.shard }}/6 env: diff --git a/packages/mcp-server/drawio-files.txt b/packages/mcp-server/drawio-files.txt index 5afded21..12264fd6 100644 --- a/packages/mcp-server/drawio-files.txt +++ b/packages/mcp-server/drawio-files.txt @@ -3,15 +3,19 @@ # a path segment, `**` across segments. # # Always included: the language files, the image icon libraries (loaded one -# icon at a time, as diagrams use them), the stylesheets, and the licenses of -# the icon sets. +# icon at a time, as diagrams use them), the stylesheets, the licenses of the +# icon sets, the templates of Insert > Template (the guard only sees the +# previews it scrolled to) and the PlantUML parser of Insert > Advanced. +images/osa_drive-harddisk.png img/LICENSE img/lib/** +js/plantuml/drawio-plantuml.min.js mxgraph/css/** resources/dia*.txt shapes/LICENSE stencils/LICENSE styles/** +templates/** # Requested by the editor; rewritten by scripts/check-drawio-files.mjs --update images/droptarget.png diff --git a/packages/mcp-server/package.json b/packages/mcp-server/package.json index 7919c195..1c4433bb 100644 --- a/packages/mcp-server/package.json +++ b/packages/mcp-server/package.json @@ -9,6 +9,7 @@ }, "scripts": { "build": "tsc && node scripts/copy-assets.mjs && node scripts/fetch-drawio.mjs", + "check-drawio": "node scripts/check-drawio-files.mjs", "check-package": "node scripts/check-package.mjs", "dev": "tsx watch src/index.ts", "start": "node dist/index.js", diff --git a/packages/mcp-server/scripts/check-drawio-files.mjs b/packages/mcp-server/scripts/check-drawio-files.mjs index 480692d0..733871ac 100644 --- a/packages/mcp-server/scripts/check-drawio-files.mjs +++ b/packages/mcp-server/scripts/check-drawio-files.mjs @@ -23,6 +23,7 @@ import { import http from "node:http" import path from "node:path" import { fileURLToPath, pathToFileURL } from "node:url" +import { readDrawioVersion } from "../../../scripts/drawio-zip.mjs" import { formatFileList, parseFileList, @@ -44,6 +45,18 @@ if (!existsSync(path.join(drawioDir, "index.html"))) { ) process.exit(2) } +// The list is for the pinned release; the fetch scripts stamp the copy +const { version } = readDrawioVersion() +const stampFile = path.join(drawioDir, ".version") +const stamp = existsSync(stampFile) + ? readFileSync(stampFile, "utf8").trim() + : null +if (stamp !== version) { + console.error( + `${drawioDir} is draw.io ${stamp ?? "(no .version stamp)"}; the package pins ${version}. Delete it and run "npm run dev" at the repository root again, or pass --drawio .`, + ) + process.exit(2) +} const playwrightEntry = path.join( PKG, "../../node_modules/playwright/index.mjs", @@ -172,9 +185,9 @@ const MATH_DOC = doc( ) // Dialogs and toggles, by draw.io action name (Actions.js); each is closed -// again right after it opened. Left out on purpose, to keep the package -// small: Insert > Template (5.6 MB of templates and previews) and the -// Insert > Advanced tools that load code when used (Mermaid, PlantUML). +// again right after it opened. Insert > Template and Insert > Advanced > +// PlantUML, which load files when used, are driven below. Left out on +// purpose, to keep the package small: Mermaid (it loads its own code). const ACTIONS = [ "editData", "editDiagram", @@ -408,16 +421,48 @@ async function drive(browser, ui, full) { report("page tabs") if (full) { - for (const format of ["png", "svg", "xmlsvg", "xml"]) { + // Insert > Advanced > PlantUML: the parser is loaded on first use + await frame.evaluate(() => window.__ui.actions.get("plantUml").funct()) + await frame + .locator(".geDialog textarea") + .first() + .fill("@startuml\nA -> B\n@enduml") + await frame + .locator('.geDialog button:text-is("Insert")') + .first() + .click() + await wait(page, 3000) + report("plantUml insert") + await closeAll() + + // Insert > Template: the index and the previews on screen + await frame.evaluate(() => + window.__ui.actions.get("insertTemplate").funct(), + ) + await wait(page, 2000) + report("template dialog") + await closeAll() + + const formats = ["png", "svg", "xmlsvg", "xml"] + for (const [i, format] of formats.entries()) { await page.evaluate( (f) => window.send({ action: "export", format: f, scale: 2 }), format, ) - await wait(page, 1500) + await page + .waitForFunction((n) => window.exports >= n, i + 1, { + timeout: 10000, + }) + .catch(() => {}) report(`export ${format}`) } const exports = await page.evaluate(() => window.exports) - if (exports < 4) console.log(` only ${exports} of 4 exports answered`) + if (exports < formats.length) { + console.error( + ` only ${exports} of ${formats.length} exports answered`, + ) + process.exitCode = 1 + } } await page.close() } @@ -430,10 +475,12 @@ try { server.close() } +// A full copy has every file the editor asks for; 404s mean this is not one if (notFound.size > 0) { - console.log( - `\nNot in the full copy either (ignored): ${[...notFound].join(", ")}`, + console.error( + `\nThe editor requested ${notFound.size} files that ${drawioDir} does not have (not a full draw.io copy?):\n ${[...notFound].join("\n ")}`, ) + process.exit(1) } // Compare with the list diff --git a/packages/mcp-server/scripts/check-package.mjs b/packages/mcp-server/scripts/check-package.mjs index 992233d5..274b2fda 100644 --- a/packages/mcp-server/scripts/check-package.mjs +++ b/packages/mcp-server/scripts/check-package.mjs @@ -13,9 +13,9 @@ const REQUIRED = [ "dist/drawio/js/app.min.js", "dist/drawio/LICENSE", ] -// The bundled draw.io is most of the package; a jump means the file list -// grew more than intended -const MAX_TARBALL_BYTES = 40 * 1024 * 1024 +// The bundled draw.io is most of the package (15.1 MB packed with draw.io +// v32.0.2); a jump means the file list grew more than intended +const MAX_TARBALL_BYTES = 20 * 1024 * 1024 const output = JSON.parse( execSync("npm pack --dry-run --json", { encoding: "utf8" }),