From fe30936e2f6aae4b55643ade31f209026209f7dc Mon Sep 17 00:00:00 2001
From: agent_coder <agent_coder@vvzvlad.xyz>
Date: Sun, 12 Jul 2026 04:38:38 +0300
Subject: [PATCH] =?UTF-8?q?fix(mcp):=20drawio=20=E2=80=94=20=D0=B0=D0=B2?=
 =?UTF-8?q?=D1=82=D0=BE-=D1=81=D1=82=D1=80=D0=B8=D0=BF=20XML-=D0=BA=D0=BE?=
 =?UTF-8?q?=D0=BC=D0=BC=D0=B5=D0=BD=D1=82=D0=B0=D1=80=D0=B8=D0=B5=D0=B2=20?=
 =?UTF-8?q?=D0=B2=D0=BC=D0=B5=D1=81=D1=82=D0=BE=20lint-=D0=BE=D1=88=D0=B8?=
 =?UTF-8?q?=D0=B1=D0=BA=D0=B8=20(#505)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Модель органически вставляет `<!-- ... -->` в mxGraph-XML вопреки запрету
в описании тула (природа LLM, промптингом не лечится). Жёсткая ошибка
линтера [no-comments] заставляла её ПОЛНОСТЬЮ перегенерировать диаграмму —
впустую потраченный tool-call на почти каждой первой генерации.

Теперь в общем prepare-пути (`prepareModel`, через который идут
drawioCreate/drawioUpdate/drawioEditCells) комментарии срезаются ДО линта:
`xml.replace(/<!--[\s\S]*?-->/g, "")`. Комментарии не несут семантики, так
что стрип всегда безопасен. Число срезанных комментариев уходит в
`warnings[]` как `stripped N XML comment(s)` (только при N > 0) — модель это
видит, но НЕ ретраит.

Стрип ГЕЙТИТСЯ на well-formedness: сначала парсим модель, и режем
комментарии только если XML корректен. На malformed-входе (например сырой
неэкранированный `<!--` внутри значения атрибута на пути create/update, где
normalizeInput отдаёт строку без парсинга) стрип НЕ выполняется — иначе он
молча вырезал бы текст автора и принял битую диаграмму. Вместо этого правила
линтера value-escaping / well-formed-xml корректно отклоняют такой вход, и
автор видит реальную ошибку. В корректном XML литеральный `<!--` в значении
обязан быть entity-escaped, а CDATA drawio не использует, поэтому после
проверки well-formedness регэксп бьёт только по настоящим comment-нодам.

Правило `no-comments` в линтере оставлено как defense-in-depth backstop.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 packages/mcp/src/lib/drawio-xml.ts         | 31 ++++++++-
 packages/mcp/test/unit/drawio-xml.test.mjs | 79 ++++++++++++++++++++++
 2 files changed, 107 insertions(+), 3 deletions(-)

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. `&lt;!--`) 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 `<!--` inside a value on the <mxGraphModel> create/update
+  // path, which normalizeInput passes through unparsed) we must NOT strip:
+  // truncating that value would silently corrupt the author's label. Instead we
+  // leave it intact and let lintModel's value-escaping / well-formed-xml rules
+  // reject it so the author sees the real error.
+  const { error: parseError } = parseXml(rawModel);
+  const commentMatches =
+    parseError === null ? (rawModel.match(/<!--[\s\S]*?-->/g) ?? []) : [];
+  const commentCount = commentMatches.length;
+  const strippedModel =
+    commentCount > 0 ? rawModel.replace(/<!--[\s\S]*?-->/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 =
+    '<mxGraphModel><root>' +
+    '<!-- header comment -->' +
+    '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="2" value="Hello" vertex="1" parent="1">' +
+    '<!-- inline note -->' +
+    '<mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  // Would fail [no-comments] if not stripped first.
+  const p = prepareModel(m);
+  // Comments are gone from the canonical model.
+  assert.ok(!p.modelXml.includes("<!--"));
+  assert.ok(!p.modelXml.includes("header comment"));
+  assert.ok(!p.modelXml.includes("inline note"));
+  // No [no-comments] error was raised (prepareModel returned instead of throwing).
+  // A single strip warning naming the count (2) is present.
+  assert.ok(p.warnings.includes("stripped 2 XML comment(s)"));
+});
+
+test("prepareModel: zero comments emits no strip warning", () => {
+  const p = prepareModel(VALID_MODEL);
+  assert.ok(!p.warnings.some((w) => w.startsWith("stripped")));
+});
+
+test("prepareModel: an entity-escaped <!-- inside a value is NOT stripped", () => {
+  const m =
+    '<mxGraphModel><root>' +
+    '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="2" value="a &lt;!-- x --&gt; b" vertex="1" parent="1">' +
+    '<mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  const p = prepareModel(m);
+  // Nothing was mistaken for a comment node.
+  assert.ok(!p.warnings.some((w) => w.startsWith("stripped")));
+  // The escaped sequence survives round-trip in the value.
+  const cell = p.cells.find((c) => c.id === "2");
+  assert.equal(cell.value, "a <!-- x --> b");
+});
+
+test("prepareModel: malformed input with a raw <!-- in a value is NOT stripped (no silent truncation)", () => {
+  // A literal, UNescaped `<!--` inside an attribute value makes the XML
+  // malformed (raw `<` is illegal in a value). The comment-strip is gated on
+  // well-formedness, so it must NOT touch this input — otherwise the author's
+  // label ` secret ` would be silently deleted and the corrupt diagram accepted.
+  // Instead lintModel's value-escaping / well-formed-xml rules must reject it.
+  const m =
+    '<mxGraphModel><root>' +
+    '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="2" value="a <!-- secret --> b" vertex="1" parent="1">' +
+    '<mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  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 =
+    '<mxGraphModel><root>' +
+    '<!-- unstripped --><mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '</root></mxGraphModel>';
+  const issues = issuesOf(() => lintModel(m));
+  assert.ok(hasRule(issues, "no-comments"));
+});
+
 // --- decode chain: plain -----------------------------------------------------
 
 test("decode chain (plain): buildDrawioSvg -> decodeDrawioSvg round-trips byte-stable", () => {