fix(mcp): drawio — авто-стрип XML-комментариев вместо lint-ошибки (#505)

Модель органически вставляет `<!-- ... -->` в 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) — модель это видит, но НЕ
ретраит.

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

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

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CHANGELOG.md

@@ -392,6 +392,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   `@docmost/token-estimate` package) so they can never diverge. Deferred-tool
   activation is also cached in the chat metadata to avoid re-resolving it each
   turn. (#490)
+- **Cyrillic (and any non-ASCII) draw.io labels no longer turn into mojibake
+  when a diagram is opened in the draw.io editor.** Agent-created diagrams
+  (`drawioCreate`) and Confluence-imported diagrams stored their model in the
+  SVG's `content=` attribute as base64; the draw.io editor decodes that via
+  Latin-1 `atob` (no UTF-8 step), so every non-ASCII char (e.g. `Старт-бит`,
+  `ё`, `—`) split into garbage and the editor's autosave then persisted the
+  corrupted model, breaking the page preview too. Both write paths
+  (`buildDrawioSvg`, the import service's `createDrawioSvg`) now write `content=`
+  as XML-entity-escaped mxfile XML — draw.io's own native form, decoded by the
+  DOM as UTF-8 — so labels open intact. The decoder reads both the new
+  entity-encoded form and the old base64 form, so existing diagrams still open.
+  *Healing pre-fix diagrams:* only a diagram that still holds its original
+  (correct-UTF-8) base64 — i.e. one not yet opened/autosaved in the draw.io
+  editor — can be repaired in place by `drawioGet` → `drawioUpdate` with the
+  same XML (rewrites the attachment in the new form); no migration script is
+  needed. A diagram that was already opened in the editor persisted the
+  mojibake at rest, so `drawioGet` reads the already-corrupted text and
+  `drawioUpdate` faithfully rewrites it — that text is lost and is not
+  recoverable by a rewrite. (#507)
 - **A chat with one malformed message part no longer 500s on every turn, and a
   failed send no longer duplicates the user's message.** Incoming client parts
   are now whitelisted to `text` (a forged tool-result part can no longer reach

apps/server/src/core/ai-chat/ai-chat.service.ts

@@ -189,10 +189,11 @@ export function stepBudgetWarning(stepNumber: number): string {
 //
 // `system` is the in-scope system prompt; we CONCATENATE so the original
 // persona/context is preserved — a bare `system` override would REPLACE the
-// whole system prompt for the step. `activatedTools` is PER-TURN mutable state
-// owned by the streaming loop (a closure Set grown by loadTools); it is passed
-// in (not module-global, not persisted) so this stays a pure function of its
-// arguments.
+// whole system prompt for the step. `activatedTools` is a closure Set grown by
+// loadTools and owned by the streaming loop; the caller seeds it from and
+// persists it to the chat's metadata across turns (#490), but this function only
+// READS the Set it is handed, so it stays a pure function of its arguments (not
+// module-global).
 //
 // NOTE: at AI SDK v7 the per-step `system` field is renamed to `instructions`.
 // On v6 (`^6.0.134`) `system` is the correct field — adjust when bumping.
@@ -1410,10 +1411,11 @@ export class AiChatService implements OnModuleInit, OnModuleDestroy {
       const baseTools = { ...external.tools, ...docmostTools };
 
       // Deferred tool loading state (#332), scoped to THIS streaming loop:
-      //  - `activatedTools` is per-TURN mutable state — a fresh closure Set created
-      //    per streamText call, NOT module-global and NOT persisted, so a new turn
-      //    starts cold. loadTools.execute adds to it; prepareAgentStep reads it to
-      //    widen `activeTools` on the NEXT step.
+      //  - `activatedTools` is a fresh closure Set per streamText call (not
+      //    module-global), SEEDED from the chat's persisted metadata.activatedTools
+      //    (#490, just below) so activation carries across turns. loadTools.execute
+      //    adds to it; prepareAgentStep reads it to widen `activeTools` on the NEXT
+      //    step; turn end persists it back.
       //  - `validDeferredNames` = every tool that is NOT core (the in-app deferred
       //    tools + ALL external MCP tools), computed from the ACTUAL toolset so an
       //    external tool is loadable by its namespaced name. loadTools rejects any

apps/server/src/integrations/import/services/import-attachment.service.drawio-svg.spec.ts

@@ -0,0 +1,131 @@
+// p-limit and @sindresorhus/slugify are ESM-only and not in jest's transform
+// allowlist; both are irrelevant to createDrawioSvg (a pure fs + string method),
+// so they are mocked out to keep the module graph loadable under ts-jest.
+jest.mock('p-limit', () => ({
+  __esModule: true,
+  default: () => (fn: () => unknown) => fn(),
+}));
+jest.mock('@sindresorhus/slugify', () => ({
+  __esModule: true,
+  default: (input: string) => String(input),
+}));
+
+import { promises as fs } from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import { ImportAttachmentService } from './import-attachment.service';
+
+/**
+ * Unit test for ImportAttachmentService.createDrawioSvg (issue #507).
+ *
+ * The Confluence import wraps a `.drawio` file into a `.drawio.svg` attachment.
+ * The `content=` payload MUST be the mxfile XML entity-escaped (draw.io's native
+ * form), NOT base64 — draw.io's editor decodes a base64 content= via Latin-1
+ * atob, mangling every non-ASCII char into mojibake. createDrawioSvg touches no
+ * injected dependency, so the service is built with placeholder deps.
+ */
+describe('ImportAttachmentService.createDrawioSvg (#507)', () => {
+  const service = new ImportAttachmentService(
+    {} as any,
+    {} as any,
+    {} as any,
+  );
+  const call = (p: string): Promise<Buffer> =>
+    (service as any).createDrawioSvg(p);
+
+  let tmpDir: string;
+  beforeAll(async () => {
+    tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'drawio-507-'));
+  });
+  afterAll(async () => {
+    await fs.rm(tmpDir, { recursive: true, force: true });
+  });
+
+  const writeDrawio = async (name: string, xml: string): Promise<string> => {
+    const p = path.join(tmpDir, name);
+    await fs.writeFile(p, xml, 'utf-8');
+    return p;
+  };
+
+  it('writes content= as entity-encoded XML, not base64', async () => {
+    const drawio =
+      '<mxfile host="Confluence"><diagram name="Схема — ёж">' +
+      '<mxGraphModel><root><mxCell id="0"/>' +
+      '<mxCell id="2" value="Старт-бит" vertex="1" parent="0"/>' +
+      '</root></mxGraphModel></diagram></mxfile>';
+    const p = await writeDrawio('cyrillic.drawio', drawio);
+
+    const svg = (await call(p)).toString('utf-8');
+    const content = /content="([^"]*)"/.exec(svg)?.[1];
+    expect(content).toBeDefined();
+
+    // Entity-encoded XML form, starting with &lt;mxfile — never a base64 blob.
+    expect(content).toMatch(/^&lt;mxfile/);
+    expect(content).toContain('&lt;');
+    // Non-ASCII survives as raw UTF-8, with no Latin-1 mojibake.
+    expect(content).toContain('Старт-бит');
+    expect(content).toContain('Схема — ёж');
+    expect(content).not.toContain('Ð');
+
+    // Decoding the attribute (un-escaping) yields the original drawio file.
+    const decoded = content!
+      .replace(/&lt;/g, '<')
+      .replace(/&gt;/g, '>')
+      .replace(/&quot;/g, '"')
+      .replace(/&amp;/g, '&');
+    expect(decoded).toBe(drawio);
+  });
+
+  it('encodes literal tab/newline/CR as numeric char-refs, not literal control chars (#507 F1)', async () => {
+    // A literal tab/newline/CR inside the mxfile XML would be collapsed to a
+    // single space by XML attribute-value normalization when the draw.io editor
+    // reads content=, silently flattening multi-line labels and tab-bearing
+    // values. They must be emitted as numeric char-refs instead.
+    const drawio =
+      '<mxfile><diagram name="p">' +
+      '<mxGraphModel><root><mxCell id="0"/>' +
+      '<mxCell id="2" value="col1\tcol2" style="html=1;\nshadow=0" vertex="1" parent="0"/>' +
+      '<mxCell id="3" value="Line1\nLine2\rLine3" vertex="1" parent="0"/>' +
+      '</root></mxGraphModel></diagram></mxfile>';
+    const p = await writeDrawio('ctrl.drawio', drawio);
+
+    const svg = (await call(p)).toString('utf-8');
+    const content = /content="([^"]*)"/.exec(svg)?.[1];
+    expect(content).toBeDefined();
+    // No literal control chars survive in the attribute value.
+    expect(content).not.toMatch(/[\t\n\r]/);
+    // They round-trip as numeric char-refs.
+    expect(content).toContain('&#x9;');
+    expect(content).toContain('&#xa;');
+    expect(content).toContain('&#xd;');
+    // Decoding (char-refs back to literal, entities back) recovers the file.
+    const decoded = content!
+      .replace(/&lt;/g, '<')
+      .replace(/&gt;/g, '>')
+      .replace(/&quot;/g, '"')
+      .replace(/&#39;/g, "'")
+      .replace(/&#x9;/gi, '\t')
+      .replace(/&#xa;/gi, '\n')
+      .replace(/&#xd;/gi, '\r')
+      .replace(/&amp;/g, '&');
+    expect(decoded).toBe(drawio);
+  });
+
+  it('escapes XML metacharacters in the drawio payload', async () => {
+    const drawio = '<mxfile><diagram name="a &amp; b">"q" &lt;x&gt;</diagram></mxfile>';
+    const p = await writeDrawio('meta.drawio', drawio);
+
+    const svg = (await call(p)).toString('utf-8');
+    const content = /content="([^"]*)"/.exec(svg)?.[1];
+    expect(content).toBeDefined();
+    // The attribute value must contain no bare `<`, `>` or `"` that would break
+    // out of the content="..." attribute or the SVG element.
+    expect(content).not.toMatch(/[<>"]/);
+    const decoded = content!
+      .replace(/&lt;/g, '<')
+      .replace(/&gt;/g, '>')
+      .replace(/&quot;/g, '"')
+      .replace(/&amp;/g, '&');
+    expect(decoded).toBe(drawio);
+  });
+});

apps/server/src/integrations/import/services/import-attachment.service.ts

@@ -8,6 +8,7 @@ import { createReadStream } from 'node:fs';
 import { promises as fs } from 'fs';
 import { Readable } from 'stream';
 import { getMimeType, sanitizeFileName } from '../../../common/helpers';
+import { htmlEscape } from '../../../common/helpers/html-escaper';
 import { v7 } from 'uuid';
 import { FileTask } from '@docmost/db/types/entity.types';
 import { getAttachmentFolderPath } from '../../../core/attachment/attachment.utils';
@@ -849,7 +850,12 @@ export class ImportAttachmentService {
   ): Promise<Buffer> {
     try {
       const drawioContent = await fs.readFile(drawioPath, 'utf-8');
-      const drawioBase64 = Buffer.from(drawioContent).toString('base64');
+      // Write the mxfile XML XML-entity-escaped (draw.io's native content= form),
+      // NOT base64. draw.io's editor decodes a base64 content= via Latin-1 atob
+      // (no UTF-8 step), turning every non-ASCII char (Cyrillic, ё, —) into
+      // mojibake; the entity-encoded form is decoded by the DOM as UTF-8 and
+      // opens intact. Docmost's own decoder reads both forms.
+      const drawioEscaped = this.xmlEscapeContent(drawioContent);
 
       let imageElement = '';
       // If we have a PNG, include it in the SVG
@@ -875,7 +881,7 @@ export class ImportAttachmentService {
       width="600"
       height="400"
       viewBox="0 0 600 400"
-      content="${drawioBase64}">${imageElement}</svg>`;
+      content="${drawioEscaped}">${imageElement}</svg>`;
 
       return Buffer.from(svgContent, 'utf-8');
     } catch (error) {
@@ -884,6 +890,24 @@ export class ImportAttachmentService {
     }
   }
 
+  /**
+   * Escape a string so it is safe as the value of a double-quoted XML attribute
+   * (the `content=` payload of a `.drawio.svg`). The shared `htmlEscape` covers
+   * `& < > " '` (a strict superset of what this attribute needs; the extra `'`
+   * escape is harmless in a `"`-delimited value). On top of that, the numeric
+   * char-refs for tab/newline/CR are required: a literal tab/newline/CR inside
+   * an attribute value is collapsed to a single space by XML attribute-value
+   * normalization on DOM read (both our decoder and the real draw.io editor),
+   * silently flattening multi-line labels and tab-bearing values. Char-refs
+   * survive that normalization (#507).
+   */
+  private xmlEscapeContent(s: string): string {
+    return htmlEscape(s)
+      .replace(/\t/g, '&#x9;')
+      .replace(/\n/g, '&#xa;')
+      .replace(/\r/g, '&#xd;');
+  }
+
   private async uploadWithRetry(opts: {
     abs: string;
     storageFilePath: string;

apps/server/test/integration/ai-chat-stream.int-spec.ts

@@ -322,20 +322,21 @@ describe('AiChatService.stream [integration]', () => {
   });
 
   /**
-   * #332 deferred tool loading, the ON path. The riskiest property is that the
-   * per-turn `activatedTools` Set is created FRESH inside each stream() call, so a
-   * tool a previous turn activated via loadTools is NOT still active when the next
-   * turn starts — the new turn begins "cold" (CORE + loadTools only). The unit
-   * tests only exercise pure prepareAgentStep with hand-fed Sets; this pins the
-   * real wiring end-to-end (loadTools.execute -> activatedTools -> prepareStep ->
-   * per-step activeTools) against the real streamText loop, and proves there is no
-   * cross-turn leak. We drive a MockLanguageModelV3 whose step 1 calls
-   * loadTools(['createPage']) and assert, via the model's recorded per-step
-   * CallOptions.tools (the AI SDK filters the provider tool list by activeTools),
-   * that the deferred tool becomes active on the SAME turn's next step but NOT on a
-   * fresh turn's first step.
+   * #332 + #490 deferred tool loading, the ON path. Turn 1 starts COLD (CORE +
+   * loadTools only) and activates a deferred tool via loadTools; that activation
+   * is PERSISTED into the chat's metadata.activatedTools (#490) so the NEXT turn
+   * SEEDS from it and the tool is active from the fresh turn's FIRST step — the
+   * model never re-runs loadTools to re-activate the same tool. The unit tests
+   * only exercise pure prepareAgentStep with hand-fed Sets; this pins the real
+   * wiring end-to-end (loadTools.execute -> activatedTools -> persist -> next-turn
+   * seed -> prepareStep -> per-step activeTools) against the real streamText loop.
+   * We drive a MockLanguageModelV3 whose step 1 calls loadTools(['createPage'])
+   * and assert, via the model's recorded per-step CallOptions.tools (the AI SDK
+   * filters the provider tool list by activeTools), that the deferred tool becomes
+   * active on the SAME turn's next step AND, seeded from metadata, on the next
+   * turn's first step.
    */
-  describe('deferred tool loading ON — per-turn activation, no leak (#332)', () => {
+  describe('deferred tool loading ON — cross-turn activation persistence (#332 + #490)', () => {
     // A stub deferred (non-core) tool the agent can activate. Its execute is never
     // called — the model only needs to SEE it become active — but it must be a
     // valid AI-SDK tool so the SDK includes it in a step's tool list once active.
@@ -451,7 +452,7 @@ describe('AiChatService.stream [integration]', () => {
       } as any);
     }
 
-    it('activates a deferred tool for the SAME turn, and a NEW turn starts cold (no leak)', async () => {
+    it('activates a deferred tool for the SAME turn, and a NEW turn SEEDS it from persisted chat metadata (#490)', async () => {
       const chatId = (await createChat(db, { workspaceId, creatorId: userId })).id;
 
       // --- Turn 1: loadTools(createPage) on step 1, then answer on step 2. ---
@@ -474,7 +475,7 @@ describe('AiChatService.stream [integration]', () => {
       // Step 2 of the SAME turn sees the just-activated deferred tool.
       expect(step2Tools).toContain('createPage');
 
-      // --- Turn 2 on the SAME chat: must start cold again. ---
+      // --- Turn 2 on the SAME chat: seeds the persisted activation (#490). ---
       const model2 = new MockLanguageModelV3({
         doStream: async () => ({ stream: successStream() }),
       } as any);
@@ -485,9 +486,10 @@ describe('AiChatService.stream [integration]', () => {
 
       const nextTurnFirstStep = toolNames(model2.doStreamCalls[0]);
       expect(nextTurnFirstStep).toContain('loadTools');
-      // The activated set is per-turn: the prior turn's createPage did NOT leak,
-      // so the fresh turn's first step sees it deferred again.
-      expect(nextTurnFirstStep).not.toContain('createPage');
+      // #490: activation PERSISTS across turns — turn 1 wrote createPage into the
+      // chat's metadata.activatedTools, so the next turn seeds from it and the
+      // deferred tool is active from the FIRST step (no need to re-run loadTools).
+      expect(nextTurnFirstStep).toContain('createPage');
     });
   });
 });

packages/mcp/src/lib/drawio-xml.ts

@@ -178,10 +178,11 @@ function sliceModel(xml: string): string | null {
 // --- decode chain ----------------------------------------------------------
 
 /**
- * Read the `content=` attribute out of a `.drawio.svg` string. Docmost stores a
- * base64 payload there (createDrawioSvg); draw.io's own SVG export may store the
- * XML entity-encoded instead. The DOM decodes entities for us, so the caller
- * only has to distinguish "starts with '<'" (raw XML) from base64.
+ * Read the `content=` attribute out of a `.drawio.svg` string. Docmost writes
+ * the mxfile XML entity-encoded there (buildDrawioSvg / createDrawioSvg), which
+ * is also how draw.io's own SVG export stores it; older attachments stored a
+ * base64 payload instead. The DOM decodes entities for us, so the caller only
+ * has to distinguish "starts with '<'" (raw XML) from base64.
  */
 export function extractContentAttr(svg: string): string {
   const { doc, error } = parseXml(svg);
@@ -195,12 +196,23 @@ export function extractContentAttr(svg: string): string {
   // value itself never contains a double-quote (base64 / entity-encoded XML).
   const m = /content="([^"]*)"/.exec(svg);
   if (m) {
-    // Decode the handful of XML entities a raw regex would leave encoded.
+    // Decode the handful of XML entities a raw regex would leave encoded. The
+    // numeric char-refs for tab/newline/CR MUST be decoded here too: the DOM
+    // path above turns them back into the literal control chars, so this
+    // regex fallback has to agree or the two decode paths diverge (#507).
+    // `&amp;` is decoded last so an escaped `&amp;#x9;` reads back as the
+    // literal text `&#x9;`, not a tab.
     return m[1]
       .replace(/&lt;/g, "<")
       .replace(/&gt;/g, ">")
       .replace(/&quot;/g, '"')
       .replace(/&#39;/g, "'")
+      .replace(/&#x9;/gi, "\t")
+      .replace(/&#9;/g, "\t")
+      .replace(/&#xa;/gi, "\n")
+      .replace(/&#10;/g, "\n")
+      .replace(/&#xd;/gi, "\r")
+      .replace(/&#13;/g, "\r")
       .replace(/&amp;/g, "&");
   }
   throw new Error("drawio: SVG has no content= attribute to decode");
@@ -307,9 +319,16 @@ export function encodeDrawioFile(modelXml: string, title = "Page-1"): string {
 /**
  * Build the `diagram.drawio.svg` attachment. Mirrors the import service's
  * createDrawioSvg contract exactly:
- *   <svg xmlns=… xmlns:xlink=… content="${base64(drawioFile)}">${inner}</svg>
+ *   <svg xmlns=… xmlns:xlink=… content="${xmlEscape(drawioFile)}">${inner}</svg>
  * plus width/height/viewBox from the diagram bounding box and the schematic
  * preview as the visible children (`inner`).
+ *
+ * The `content=` value is the mxfile XML XML-entity-escaped (draw.io's own
+ * native form), NOT base64. draw.io's editor decodes a base64 content= via
+ * Latin-1 atob (no UTF-8 step), turning every non-ASCII char (e.g. Cyrillic,
+ * ё, —) into mojibake; the entity-encoded form is decoded by the DOM as UTF-8
+ * and opens intact. Our decoder (decodeDrawioSvg) reads both forms, so old
+ * base64 attachments still round-trip.
  */
 export function buildDrawioSvg(
   modelXml: string,
@@ -318,14 +337,14 @@ export function buildDrawioSvg(
   title = "Page-1",
 ): string {
   const file = encodeDrawioFile(modelXml, title);
-  const base64 = Buffer.from(file, "utf-8").toString("base64");
+  const content = xmlEscape(file);
   const w = Math.max(1, Math.round(bbox.width));
   const h = Math.max(1, Math.round(bbox.height));
   return (
     `<svg xmlns="http://www.w3.org/2000/svg" ` +
     `xmlns:xlink="http://www.w3.org/1999/xlink" ` +
     `width="${w}" height="${h}" viewBox="0 0 ${w} ${h}" ` +
-    `content="${base64}">${inner}</svg>`
+    `content="${content}">${inner}</svg>`
   );
 }
 
@@ -334,7 +353,15 @@ function xmlEscape(s: string): string {
     .replace(/&/g, "&amp;")
     .replace(/</g, "&lt;")
     .replace(/>/g, "&gt;")
-    .replace(/"/g, "&quot;");
+    .replace(/"/g, "&quot;")
+    // A literal tab/newline/CR inside an attribute value is collapsed to a
+    // single space by XML attribute-value normalization on DOM read (both jsdom
+    // here and the real draw.io editor), silently flattening multi-line labels
+    // and tab-bearing values. Numeric char-refs survive that normalization, so
+    // emit them the way draw.io's own native export does (#507).
+    .replace(/\t/g, "&#x9;")
+    .replace(/\n/g, "&#xa;")
+    .replace(/\r/g, "&#xd;");
 }
 
 // --- normalization + hash --------------------------------------------------
@@ -1053,8 +1080,43 @@ 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 two conditions so the regex only ever targets real
+  // comment nodes:
+  //   1. Well-formedness. Well-formed XML forbids a bare `<` inside an attribute
+  //      value (it must be entity-escaped, e.g. `&lt;!--`), so a literal `<!--`
+  //      cannot hide in a value. 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. We leave it intact
+  //      and let lintModel's value-escaping / well-formed-xml rules reject it so
+  //      the author sees the real error.
+  //   2. No CDATA. A CDATA section is well-formed yet its text may contain a
+  //      literal `<!-- ... -->` that is NOT a comment node; stripping it would
+  //      silently delete author content. Real drawio labels live in `value=`
+  //      attributes (CDATA impossible), so excluding CDATA is free on legitimate
+  //      models; a CDATA-bearing model with a genuine comment then falls to the
+  //      retained no-comments lint backstop (an explicit error) rather than being
+  //      silently corrupted.
+  const { error: parseError } = parseXml(rawModel);
+  const hasCdata = rawModel.includes("<![CDATA[");
+  const commentMatches =
+    parseError === null && !hasCdata
+      ? (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 +1127,7 @@ export function prepareModel(inputXml: string): PreparedModel {
     cells,
     bbox,
     cellCount,
-    warnings: [...warnings, ...quality],
+    warnings: [...stripWarnings, ...warnings, ...quality],
     hash: mxHash(modelXml),
   };
 }

packages/mcp/test/unit/drawio-xml.test.mjs

@@ -51,6 +51,14 @@ const hasRule = (issues, rule, cellId) =>
     (i) => i.rule === rule && (cellId === undefined || i.cellId === cellId),
   );
 
+// Reverse the attribute-value XML escaping used in the `content=` payload.
+const unescapeAttr = (s) =>
+  s
+    .replace(/&lt;/g, "<")
+    .replace(/&gt;/g, ">")
+    .replace(/&quot;/g, '"')
+    .replace(/&amp;/g, "&");
+
 // --- style parsing ---------------------------------------------------------
 
 test("parseStyle: base stylename + key=value pairs", () => {
@@ -232,6 +240,107 @@ 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("prepareModel: a <!-- --> inside a CDATA section is NOT stripped (no silent corruption)", () => {
+  // A CDATA section is well-formed, so the well-formedness gate alone would let
+  // the regex delete a literal `<!-- x -->` living inside CDATA text — silent
+  // content loss with a false "stripped" success. The CDATA sub-gate must skip
+  // the strip; the model then hits the retained no-comments backstop (an explicit
+  // error) instead of being silently accepted with a strip warning.
+  const m =
+    '<mxGraphModel><root>' +
+    '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/>' +
+    '<foo><![CDATA[keep<!-- x -->keep]]></foo></mxCell>' +
+    '</root></mxGraphModel>';
+  const issues = issuesOf(() => prepareModel(m));
+  // Rejected (threw), not silently accepted with a strip warning.
+  assert.ok(issues, "expected prepareModel to reject rather than silently strip");
+  // The no-comments backstop caught the literal comment sequence.
+  assert.ok(
+    hasRule(issues, "no-comments"),
+    `expected no-comments backstop, 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", () => {
@@ -335,16 +444,20 @@ test("encode/build: a title with < > \" & round-trips without corrupting the SVG
 
   // The outer content="..." attribute must not be broken by the title: the raw
   // title metacharacters never appear literally in the SVG markup (they are
-  // base64-encoded inside content=, and escaped inside the file XML).
+  // entity-escaped inside content=, doubly so where the file XML already escaped
+  // them inside name="...").
   const contentMatch = /content="([^"]*)"/.exec(svg);
   assert.ok(contentMatch, "SVG has a single well-formed content= attribute");
 
   // The diagram model still decodes losslessly despite the exotic title.
   assert.equal(decodeDrawioSvg(svg), model);
 
+  // content= is now entity-encoded XML (draw.io's native form), never base64.
+  assert.match(contentMatch[1], /^&lt;mxfile/);
+
   // The file XML is well-formed: the title lives in name="..." as escaped
   // entities, so unescaping recovers the original title byte-for-byte.
-  const fileXml = Buffer.from(contentMatch[1], "base64").toString("utf-8");
+  const fileXml = unescapeAttr(contentMatch[1]);
   const nameMatch = /<diagram id="[^"]*" name="([^"]*)">/.exec(fileXml);
   assert.ok(nameMatch, "the diagram name attribute is intact and quote-safe");
   const decodedTitle = nameMatch[1]
@@ -358,3 +471,112 @@ test("encode/build: a title with < > \" & round-trips without corrupting the SVG
   const file = encodeDrawioFile(model, title);
   assert.match(file, /name="A &lt; B &gt; C &quot; D &amp; E">/);
 });
+
+// --- #507: content= is entity-encoded XML, never base64 --------------------
+
+// A model whose cell values carry Cyrillic, ё and an em dash — exactly the
+// characters draw.io's Latin-1 atob mangles when content= is base64.
+const CYRILLIC_MODEL =
+  "<mxGraphModel><root>" +
+  '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+  '<mxCell id="2" value="Старт-бит — ёж" style="rounded=1;html=1;" vertex="1" parent="1">' +
+  '<mxGeometry x="100" y="100" width="120" height="60" as="geometry"/></mxCell>' +
+  "</root></mxGraphModel>";
+
+test("#507: buildDrawioSvg writes content= as entity-encoded XML (not base64)", () => {
+  const model = normalizeXml(CYRILLIC_MODEL);
+  const svg = buildDrawioSvg(model, "<g/>", { width: 200, height: 120 }, "Диаграмма");
+
+  const contentMatch = /content="([^"]*)"/.exec(svg);
+  assert.ok(contentMatch, "SVG has a single well-formed content= attribute");
+  const content = contentMatch[1];
+
+  // The content= value is the entity-encoded mxfile XML — starts with `&lt;mxfile`.
+  assert.match(content, /^&lt;mxfile/, "content= is entity-encoded mxfile XML");
+  // It must NOT be a base64 blob: base64 has no XML entities and no literal `<`.
+  assert.ok(content.includes("&lt;"), "content= carries XML entities, not base64");
+
+  // Cyrillic / ё / — survive verbatim in the attribute (raw UTF-8, not atob-mangled).
+  assert.ok(content.includes("Старт-бит — ёж"), "non-ASCII value is raw UTF-8 in content=");
+  assert.ok(content.includes("Диаграмма"), "non-ASCII title is raw UTF-8 in content=");
+  // The mojibake that base64+atob would have produced must be absent.
+  assert.ok(!content.includes("Ð"), "no Latin-1 mojibake in content=");
+});
+
+test("#507: Cyrillic model round-trips byte-stable through buildDrawioSvg -> decodeDrawioSvg", () => {
+  const model = normalizeXml(CYRILLIC_MODEL);
+  const svg = buildDrawioSvg(model, "<g/>", { width: 200, height: 120 }, "Заголовок — ё");
+  assert.equal(decodeDrawioSvg(svg), model);
+});
+
+test("#507 back-compat: an OLD base64-form .drawio.svg still decodes losslessly", () => {
+  const model = normalizeXml(CYRILLIC_MODEL);
+  // Reproduce the pre-fix write path: encodeDrawioFile -> base64 in content=.
+  const file = encodeDrawioFile(model, "Старая диаграмма");
+  const base64 = Buffer.from(file, "utf-8").toString("base64");
+  const svg =
+    `<svg xmlns="http://www.w3.org/2000/svg" content="${base64}"><g/></svg>`;
+  // No XML entities, purely base64 alphabet — this is the legacy form.
+  assert.ok(!base64.includes("<") && !base64.includes("&"));
+  assert.equal(decodeDrawioSvg(svg), model);
+});
+
+test("#507 negative: non-ASCII in a cell value AND in the title round-trip clean", () => {
+  const model = normalizeXml(CYRILLIC_MODEL);
+  const title = "Тест — ёмкость № 5";
+  const svg = buildDrawioSvg(model, "<g/>", { width: 200, height: 120 }, title);
+
+  // Model recovered byte-for-byte.
+  assert.equal(decodeDrawioSvg(svg), model);
+
+  // Title lands in the <diagram name="..."> of the decoded file XML, intact.
+  const content = /content="([^"]*)"/.exec(svg)[1];
+  const fileXml = unescapeAttr(content);
+  const nameMatch = /<diagram id="[^"]*" name="([^"]*)">/.exec(fileXml);
+  assert.ok(nameMatch, "diagram name attribute present");
+  assert.equal(unescapeAttr(nameMatch[1]), title);
+});
+
+// --- #507 F1: literal tab/newline/CR in an attribute value survive the
+// content= round-trip. The whole mxfile XML lives in one content="..." attr;
+// XML attribute-value normalization collapses a LITERAL tab/newline/CR to a
+// single space on DOM read, so the escape must emit numeric char-refs instead.
+const CTRL_MODEL =
+  "<mxGraphModel><root>" +
+  '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+  '<mxCell id="2" value="col1\tcol2" style="html=1;\nshadow=0" vertex="1" parent="1">' +
+  '<mxGeometry x="10" y="10" width="120" height="60" as="geometry"/></mxCell>' +
+  '<mxCell id="3" value="Line1\nLine2\rLine3" vertex="1" parent="1">' +
+  '<mxGeometry x="10" y="100" width="120" height="60" as="geometry"/></mxCell>' +
+  "</root></mxGraphModel>";
+
+test("#507 F1: literal tab/newline/CR in a value round-trip byte-stable (DOM decode)", () => {
+  const svg = buildDrawioSvg(CTRL_MODEL, "<g/>", { width: 200, height: 200 });
+  const content = /content="([^"]*)"/.exec(svg)[1];
+  // The tab/newline/CR must be emitted as numeric char-refs, never as literal
+  // control chars (which DOM attribute-value normalization would eat).
+  assert.ok(
+    !/[\t\n\r]/.test(content),
+    "no literal tab/newline/CR survive in the content= attribute",
+  );
+  assert.ok(
+    content.includes("&#x9;") &&
+      content.includes("&#xa;") &&
+      content.includes("&#xd;"),
+    "tab/newline/CR are emitted as numeric char-refs",
+  );
+  // Full DOM decode recovers the model byte-for-byte, control chars intact.
+  assert.equal(decodeDrawioSvg(svg), CTRL_MODEL);
+});
+
+test("#507 F1: regex fallback decodes tab/newline/CR char-refs (agrees with DOM path)", () => {
+  const svg = buildDrawioSvg(CTRL_MODEL, "<g/>", { width: 200, height: 200 });
+  const content = /content="([^"]*)"/.exec(svg)[1];
+  // A bare `&` makes the SVG wrapper malformed, forcing extractContentAttr onto
+  // its regex fallback branch. That branch must decode the tab/newline/CR
+  // char-refs exactly like the DOM path, or the two decoders diverge.
+  const malformedSvg = `<svg content="${content}">&</svg>`;
+  assert.equal(decodeDrawioSvg(malformedSvg), CTRL_MODEL);
+  // The well-formed (DOM) path yields the identical result.
+  assert.equal(decodeDrawioSvg(svg), CTRL_MODEL);
+});