diff --git a/packages/mcp/src/lib/drawio-xml.ts b/packages/mcp/src/lib/drawio-xml.ts index 84b2c5a3..887a7e28 100644 --- a/packages/mcp/src/lib/drawio-xml.ts +++ b/packages/mcp/src/lib/drawio-xml.ts @@ -1053,8 +1053,33 @@ export interface PreparedModel { */ export function prepareModel(inputXml: string): PreparedModel { const rawModel = normalizeInput(inputXml); - const { cells, warnings } = lintModel(rawModel); - const modelXml = normalizeXml(rawModel); + // Auto-strip XML comments before linting. The model routinely inserts + // `` into the diagram XML despite the prohibition in the tool + // description; a hard [no-comments] lint failure would force it to regenerate + // the whole diagram (a wasted tool call). Comments carry no diagram semantics, + // so stripping them is always safe. The linter's no-comments rule stays as a + // defense-in-depth backstop for any path that reaches it without this strip. + // + // The strip is GATED on well-formedness: the regex only reliably targets real + // comment nodes once the XML is known to be well-formed, because well-formed + // XML forbids a bare `<` inside an attribute value (it must be entity-escaped, + // e.g. `<!--`) and drawio XML uses no CDATA sections, so nothing but a + // comment node can hold the `` sequence. On MALFORMED input (e.g. + // a raw unescaped `/g) ?? []) : []; + const commentCount = commentMatches.length; + const strippedModel = + commentCount > 0 ? rawModel.replace(//g, "") : rawModel; + const stripWarnings = + commentCount > 0 ? [`stripped ${commentCount} XML comment(s)`] : []; + const { cells, warnings } = lintModel(strippedModel); + const modelXml = normalizeXml(strippedModel); const bbox = computeBBox(cells); const cellCount = cells.filter((c) => c.id !== "0" && c.id !== "1").length; // Geometry quality warnings (non-blocking) are appended to any structural @@ -1065,7 +1090,7 @@ export function prepareModel(inputXml: string): PreparedModel { cells, bbox, cellCount, - warnings: [...warnings, ...quality], + warnings: [...stripWarnings, ...warnings, ...quality], hash: mxHash(modelXml), }; } diff --git a/packages/mcp/test/unit/drawio-xml.test.mjs b/packages/mcp/test/unit/drawio-xml.test.mjs index 8d6e83ec..89e352ec 100644 --- a/packages/mcp/test/unit/drawio-xml.test.mjs +++ b/packages/mcp/test/unit/drawio-xml.test.mjs @@ -232,6 +232,85 @@ test("prepareModel: returns bbox, cellCount, hash and lints", () => { assert.equal(p.hash, mxHash(normalizeXml(VALID_MODEL))); }); +// --- comment auto-strip (issue #505) --------------------------------------- + +// The model organically inserts `` into diagram XML. prepareModel +// (the shared path for drawioCreate/drawioUpdate/drawioEditCells) strips them +// BEFORE the lint runs and reports the count as a non-blocking warning, so the +// model never hits a hard [no-comments] error and never regenerates the diagram. +test("prepareModel: strips XML comments, lint passes, warns with the count", () => { + const m = + '' + + '' + + '' + + '' + + '' + + '' + + ''; + // Would fail [no-comments] if not stripped first. + const p = prepareModel(m); + // Comments are gone from the canonical model. + assert.ok(!p.modelXml.includes(" b"); +}); + +test("prepareModel: malformed input with a raw b" vertex="1" parent="1">' + + '' + + ''; + const issues = issuesOf(() => prepareModel(m)); + // Rejected (threw DrawioLintError) rather than silently accepted. + assert.ok(issues, "expected prepareModel to reject malformed input"); + // By a real content rule, not swallowed by the strip. + assert.ok( + hasRule(issues, "value-escaping", "2") || hasRule(issues, "well-formed-xml"), + `expected value-escaping/well-formed-xml, got ${JSON.stringify(issues)}`, + ); +}); + +test("no-comments rule still fires when a comment reaches the linter directly", () => { + // Defense-in-depth: prepareModel strips first, but lintModel itself (the + // backstop) must still reject a raw comment-bearing model. + const m = + '' + + '' + + ''; + const issues = issuesOf(() => lintModel(m)); + assert.ok(hasRule(issues, "no-comments")); +}); + // --- decode chain: plain ----------------------------------------------------- test("decode chain (plain): buildDrawioSvg -> decodeDrawioSvg round-trips byte-stable", () => {