test(mcp): покрыть error-ветки drawio-тулов + escaping round-trip; удалить мёртвый freshBlockId (ревью #434)

- drawio_get: битый ref -> 'no node found'; drawio-нода без src -> 'has no src';
  drawio_update: узел не drawio -> чистая ошибка + ноль upload/mutate;
  drawio_create: anchor не найден -> ошибка 'unreferenced orphan' + attachment
  залит и назван в сообщении, drawio-узел не записан.
- escaping round-trip: title с < > " & переживает encode/build/decode байт-в-байт,
  внешний content="..." остаётся well-formed.
- удалён неиспользуемый freshBlockId() (0 call-site после фикса #index-хендла).

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

packages/mcp/package.json

@@ -52,6 +52,7 @@
     "form-data": "^4.0.0",
     "jsdom": "^27.4.0",
     "marked": "^17.0.1",
+    "pako": "^2.0.3",
     "re2": "^1.21.0",
     "ws": "^8.19.0",
     "y-prosemirror": "1.3.7",

packages/mcp/src/client.ts

@@ -8,6 +8,10 @@ import {
   filterComment,
   filterSearchResult,
 } from "./lib/filters.js";
+import { HocuspocusProvider } from "@hocuspocus/provider";
+import { TiptapTransformer } from "@hocuspocus/transformer";
+import * as Y from "yjs";
+import WebSocket from "ws";
 import { convertProseMirrorToMarkdown } from "./lib/markdown-converter.js";
 import {
   collectInternalFileNodes,
@@ -20,10 +24,11 @@ import {
   markdownToProseMirror,
   markdownToProseMirrorCanonical,
   mutatePageContent,
+  buildCollabWsUrl,
   assertYjsEncodable,
+  applyDocToFragment,
   MutationResult,
 } from "./lib/collaboration.js";
-import { acquireCollabSession } from "./lib/collab-session.js";
 import { footnoteWarningsField } from "./lib/footnote-analyze.js";
 import { buildPageTree } from "./lib/tree.js";
 import {
@@ -44,6 +49,15 @@ import {
 } from "./lib/node-ops.js";
 import { searchInDoc, SearchOptions } from "./lib/page-search.js";
 import { withPageLock } from "./lib/page-lock.js";
+import {
+  prepareModel,
+  decodeDrawioSvg,
+  buildDrawioSvg,
+  mxHash,
+  normalizeXml,
+  countUserCells,
+} from "./lib/drawio-xml.js";
+import { renderDiagramShapes } from "./lib/drawio-preview.js";
 import {
   applyTextEdits,
   TextEdit,
@@ -412,32 +426,177 @@ export class DocmostClient {
    * change report. The report is computed AFTER the atomic read->write and
    * never throws.
    */
-  private async mutateLiveContentUnlocked(
+  private mutateLiveContentUnlocked(
     pageId: string,
     collabToken: string,
     transform: (liveDoc: any) => any | null,
   ): Promise<MutationResult> {
-    // Reuse a live CollabSession for the page (issue #400) instead of opening a
-    // fresh provider per op. acquireCollabSession does NOT take the per-page
-    // lock — the caller (replaceImage) already holds ONE withPageLock across its
-    // scan -> upload -> write sequence, and the mutex is not reentrant, so
-    // taking it here would deadlock. The synchronous read->write section and the
-    // unsyncedChanges/connectionLost ack logic live in CollabSession.mutate,
-    // preserved verbatim from the old inline machine (incl. the #152 structural
-    // diff that keeps a live editor's cursor anchored).
-    const session = await acquireCollabSession(pageId, collabToken, this.apiUrl, {
-      // Only the actual 25s collab connect timeout emits this — the connect-vs-
-      // unload signal; the other failure paths must NOT emit it.
-      onConnectTimeout: () =>
-        this.onMetricFn?.("collab_connect_timeouts_total", 1),
+    const CONNECT_TIMEOUT_MS = 25000;
+    const PERSIST_TIMEOUT_MS = 20000;
+    const ydoc = new Y.Doc();
+    const wsUrl = buildCollabWsUrl(this.apiUrl);
+
+    return new Promise<MutationResult>((resolve, reject) => {
+      let provider: HocuspocusProvider | undefined;
+      let applied = false; // onSynced may fire again on reconnect — apply once.
+      let settled = false;
+      let connectionLost = false;
+      let connectTimer: ReturnType<typeof setTimeout> | undefined;
+      let persistTimer: ReturnType<typeof setTimeout> | undefined;
+      let unsyncedHandler: ((data: { number: number }) => void) | undefined;
+      // The verifiable result resolved on every success/abort path. Set on abort
+      // (no-op report) and after a real write (computed change report).
+      let mutationResult: MutationResult;
+
+      const cleanup = () => {
+        if (connectTimer) clearTimeout(connectTimer);
+        if (persistTimer) clearTimeout(persistTimer);
+        if (provider) {
+          if (unsyncedHandler) {
+            try {
+              provider.off("unsyncedChanges", unsyncedHandler);
+            } catch (err) {}
+          }
+          try {
+            provider.destroy();
+          } catch (err) {}
+        }
+      };
+
+      const finish = (err: Error | null, value?: MutationResult) => {
+        if (settled) return;
+        settled = true;
+        cleanup();
+        if (err) reject(err);
+        else resolve(value as MutationResult);
+      };
+
+      connectTimer = setTimeout(() => {
+        // Only the actual 25s collab connect timeout fires here — the agent's
+        // collab connection to the server never became ready. This is the
+        // connect-vs-unload signal; the other finish() paths must NOT emit it.
+        this.onMetricFn?.("collab_connect_timeouts_total", 1);
+        finish(new Error("Connection timeout to collaboration server"));
+      }, CONNECT_TIMEOUT_MS);
+
+      const waitForPersistence = () => {
+        if (settled) return;
+        if (!provider) {
+          finish(new Error("collab provider gone before persistence"));
+          return;
+        }
+        if (provider.unsyncedChanges === 0) {
+          finish(null, mutationResult);
+          return;
+        }
+        persistTimer = setTimeout(() => {
+          finish(
+            new Error(
+              "Timeout waiting for collaboration server to persist the update",
+            ),
+          );
+        }, PERSIST_TIMEOUT_MS);
+        unsyncedHandler = (data: { number: number }) => {
+          if (data.number === 0 && !connectionLost) {
+            finish(null, mutationResult);
+          }
+        };
+        provider.on("unsyncedChanges", unsyncedHandler);
+      };
+
+      provider = new HocuspocusProvider({
+        url: wsUrl,
+        name: `page.${pageId}`,
+        document: ydoc,
+        token: collabToken,
+        // @ts-ignore - Required for Node.js environment
+        WebSocketPolyfill: WebSocket,
+        onDisconnect: () => {
+          connectionLost = true;
+          finish(
+            new Error(
+              "Collaboration connection closed before the update was persisted/synced",
+            ),
+          );
+        },
+        onClose: () => {
+          connectionLost = true;
+          finish(
+            new Error(
+              "Collaboration connection closed before the update was persisted/synced",
+            ),
+          );
+        },
+        onSynced: () => {
+          if (applied || settled) return;
+          applied = true;
+
+          // CRITICAL: keep everything between reading and writing the live doc
+          // synchronous (no await) so no remote update can interleave.
+          let newDoc: any;
+          let beforeDoc: any;
+          try {
+            let liveDoc = TiptapTransformer.fromYdoc(ydoc, "default");
+            if (
+              !liveDoc ||
+              typeof liveDoc !== "object" ||
+              !Array.isArray(liveDoc.content)
+            ) {
+              liveDoc = { type: "doc", content: [] };
+            }
+
+            // Snapshot the before-doc for the change report (safe deep clone).
+            beforeDoc = JSON.parse(JSON.stringify(liveDoc));
+
+            newDoc = transform(liveDoc);
+
+            if (newDoc == null) {
+              // Transform aborted — write nothing, return the live doc with a
+              // no-op change report.
+              mutationResult = {
+                doc: liveDoc,
+                verify: {
+                  changed: false,
+                  textInserted: 0,
+                  textDeleted: 0,
+                  blocksChanged: 0,
+                  marks: {},
+                  summary: "no changes (transform aborted)",
+                },
+              };
+              finish(null, mutationResult);
+              return;
+            }
+
+            // Structural diff into the live fragment (issue #152), mirroring
+            // the main write path: preserves the Yjs ids of unchanged nodes so
+            // an open editor's cursor is not yanked to the end of the document.
+            // The previous destructive rewrite (delete-all + applyUpdate of a
+            // fresh Y.Doc) discarded every node id, so replaceImage — the only
+            // caller of this method — still reproduced the #152 cursor jump
+            // (#164). applyDocToFragment runs its own atomic `transact`.
+            applyDocToFragment(ydoc, newDoc);
+          } catch (e) {
+            finish(e instanceof Error ? e : new Error(String(e)));
+            return;
+          }
+
+          // Compute the verifiable change report AFTER the transact write: it
+          // only needs the JSON before/after, so it cannot affect the atomic
+          // read->write window, and summarizeChange never throws.
+          mutationResult = {
+            doc: newDoc,
+            verify: summarizeChange(beforeDoc, newDoc),
+          };
+          waitForPersistence();
+        },
+        onAuthenticationFailed: () => {
+          finish(
+            new Error("Authentication failed for collaboration connection"),
+          );
+        },
+      });
     });
-    try {
-      return await session.mutate(transform);
-    } catch (e) {
-      // Drop the session on any failure so the next call reconnects fresh.
-      session.destroy("mutate failed");
-      throw e;
-    }
   }
 
   /**
@@ -3282,6 +3441,453 @@ export class DocmostClient {
     });
   }
 
+  // --- draw.io diagrams (issue #423) ---
+
+  /**
+   * Upload a ready-made byte buffer as a page attachment via the same
+   * multipart /files/upload endpoint uploadImage uses. Split out as its own
+   * (overridable) seam so drawio_create/update can upload the generated
+   * `.drawio.svg` without going through the URL-fetch path, and so tests can
+   * stub the network. Mirrors uploadImage's fresh-FormData + one-shot 401/403
+   * re-auth handling (a FormData body is single-use, so it must be rebuilt per
+   * attempt).
+   */
+  protected async uploadAttachmentBuffer(
+    pageId: string,
+    buffer: Buffer,
+    fileName: string,
+    mime: string,
+  ): Promise<{ id: string; fileName: string; fileSize: number }> {
+    await this.ensureAuthenticated();
+    const buildForm = () => {
+      const form = new FormData();
+      form.append("pageId", pageId);
+      form.append("file", buffer, { filename: fileName, contentType: mime });
+      return form;
+    };
+    const uploadUrl = `${this.apiUrl}/files/upload`;
+    let response;
+    try {
+      const form = buildForm();
+      response = await axios.post(uploadUrl, form, {
+        headers: {
+          ...form.getHeaders(),
+          Authorization: this.client.defaults.headers.common["Authorization"],
+        },
+        timeout: 60000,
+      });
+    } catch (error) {
+      if (
+        axios.isAxiosError(error) &&
+        (error.response?.status === 401 || error.response?.status === 403)
+      ) {
+        await this.login();
+        const form2 = buildForm();
+        response = await axios.post(uploadUrl, form2, {
+          headers: {
+            ...form2.getHeaders(),
+            Authorization: this.client.defaults.headers.common["Authorization"],
+          },
+          timeout: 60000,
+        });
+      } else if (axios.isAxiosError(error)) {
+        if (process.env.DEBUG) {
+          console.error(
+            "Attachment upload failed; response body:",
+            JSON.stringify(error.response?.data),
+          );
+        }
+        throw new Error(
+          `Attachment upload failed: ${error.response?.status} ${error.response?.statusText}`,
+        );
+      } else {
+        throw error;
+      }
+    }
+    const att = response.data?.data ?? response.data;
+    if (!att?.id || !att?.fileName) {
+      throw new Error(
+        "Unexpected /files/upload response: " + JSON.stringify(response.data),
+      );
+    }
+    return {
+      id: att.id,
+      fileName: att.fileName,
+      fileSize: att.fileSize ?? buffer.length,
+    };
+  }
+
+  /**
+   * Fetch a stored `.drawio.svg` attachment as text. Overridable seam over
+   * fetchInternalFile (the authed loopback fetch, which also rejects any
+   * traversal/SSRF src) so drawio_get/update can read the current diagram and
+   * tests can stub the bytes.
+   */
+  protected async fetchAttachmentText(src: string): Promise<string> {
+    const { buffer } = await this.fetchInternalFile(src);
+    return buffer.toString("utf-8");
+  }
+
+  /**
+   * Resolve a drawio node on a page by `attrs.id` or `#<index>` and return the
+   * node plus its ref. Throws a clear error if the ref does not resolve to a
+   * drawio node.
+   */
+  private async resolveDrawioNode(
+    pageId: string,
+    node: string,
+  ): Promise<{ node: any; ref: string }> {
+    const data = await this.getPageRaw(pageId);
+    const hit = getNodeByRef(
+      data.content ?? { type: "doc", content: [] },
+      node,
+    );
+    if (!hit) {
+      throw new Error(
+        `drawio: no node found for "${node}" on page ${pageId} (use the drawio node's attrs.id or "#<index>" from get_outline)`,
+      );
+    }
+    if (hit.type !== "drawio") {
+      throw new Error(
+        `drawio: node "${node}" on page ${pageId} is a ${hit.type}, not a drawio diagram`,
+      );
+    }
+    return { node: hit.node, ref: node };
+  }
+
+  /**
+   * Read a drawio diagram as mxGraph XML (default) or as the raw `.drawio.svg`.
+   * Runs the decode chain (base64/entity content= → drawio file → nested XML or
+   * pako-inflated compressed <diagram>). The returned `hash` is the
+   * optimistic-lock key for drawio_update.
+   */
+  async drawioGet(
+    pageId: string,
+    node: string,
+    format: "xml" | "svg" = "xml",
+  ): Promise<{
+    pageId: string;
+    nodeId: string;
+    format: "xml" | "svg";
+    content: string;
+    meta: {
+      attachmentId: string | null;
+      title: string | null;
+      width: number | null;
+      height: number | null;
+      cellCount: number;
+      hash: string;
+    };
+  }> {
+    await this.ensureAuthenticated();
+    const { node: drawio } = await this.resolveDrawioNode(pageId, node);
+    const attrs = drawio.attrs || {};
+    const src = attrs.src;
+    if (!src) {
+      throw new Error(
+        `drawio: node "${node}" on page ${pageId} has no src to read`,
+      );
+    }
+    const svg = await this.fetchAttachmentText(src);
+    const modelXml = decodeDrawioSvg(svg);
+    const meta = {
+      attachmentId: attrs.attachmentId ?? null,
+      title: attrs.title ?? null,
+      width: attrs.width != null ? Number(attrs.width) : null,
+      height: attrs.height != null ? Number(attrs.height) : null,
+      cellCount: countUserCells(modelXml),
+      hash: mxHash(modelXml),
+    };
+    return {
+      pageId,
+      nodeId: attrs.id ?? node,
+      format,
+      content: format === "svg" ? svg : normalizeXml(modelXml),
+      meta,
+    };
+  }
+
+  /**
+   * Create a drawio diagram from mxGraph XML: lint → schematic SVG preview
+   * (pure TS) → build the `.drawio.svg` (createDrawioSvg contract) → create the
+   * attachment → insert a `drawio` node before/after an anchor or appended.
+   * `xml` is a bare `<mxGraphModel>` or a list of `<mxCell>` (the server wraps
+   * it and adds the id=0/id=1 sentinels).
+   */
+  async drawioCreate(
+    pageId: string,
+    where: {
+      position: "before" | "after" | "append";
+      anchorNodeId?: string;
+      anchorText?: string;
+    },
+    xml: string,
+    title?: string,
+  ): Promise<{
+    success: boolean;
+    nodeId: string;
+    attachmentId: string;
+    warnings: string[];
+    verify?: any;
+  }> {
+    await this.ensureAuthenticated();
+    if (
+      !where ||
+      (where.position !== "before" &&
+        where.position !== "after" &&
+        where.position !== "append")
+    ) {
+      throw new Error(
+        'drawio_create: `where.position` must be one of "before", "after", "append"',
+      );
+    }
+    if (where.position === "before" || where.position === "after") {
+      const hasId =
+        typeof where.anchorNodeId === "string" && where.anchorNodeId.length > 0;
+      const hasText =
+        typeof where.anchorText === "string" && where.anchorText.length > 0;
+      if (hasId === hasText) {
+        throw new Error(
+          `drawio_create: position "${where.position}" requires exactly one of anchorNodeId or anchorText`,
+        );
+      }
+    }
+
+    // Pre-write pipeline (throws a structured DrawioLintError on any violation).
+    const prepared = prepareModel(xml);
+    const inner = renderDiagramShapes(prepared.cells, prepared.bbox);
+    const diagramTitle = title || "Page-1";
+    const svg = buildDrawioSvg(prepared.modelXml, inner, prepared.bbox, diagramTitle);
+
+    const att = await this.uploadAttachmentBuffer(
+      pageId,
+      Buffer.from(svg, "utf-8"),
+      "diagram.drawio.svg",
+      "image/svg+xml",
+    );
+
+    // NOTE: no `id` attribute is set here. The vendored `drawio` node schema
+    // (diagramAttributes) declares no `id`, so any block id would be silently
+    // dropped by PMNode.fromJSON on save and the returned handle would fail to
+    // resolve. The addressable handle is the node's "#<index>" (like image/table
+    // nodes), computed after the insert below.
+    const drawioNode: any = {
+      type: "drawio",
+      attrs: {
+        src: `/api/files/${att.id}/${att.fileName}`,
+        attachmentId: att.id,
+        width: prepared.bbox.width,
+        height: prepared.bbox.height,
+        align: "center",
+      },
+    };
+    if (title) drawioNode.attrs.title = title;
+    // Reuse the existing URL trust boundary (rejects unsafe src schemes).
+    this.validateDocUrls(drawioNode);
+
+    const collabToken = await this.getCollabTokenWithReauth();
+    const pageUuid = await this.resolvePageId(pageId);
+
+    let inserted = false;
+    let insertedIndex = -1;
+    const mutation = await this.mutatePage(
+      pageUuid,
+      collabToken,
+      this.apiUrl,
+      (liveDoc) => {
+        inserted = false;
+        insertedIndex = -1;
+        const { doc: nd, inserted: ins } = insertNodeRelative(
+          liveDoc,
+          drawioNode,
+          where,
+        );
+        inserted = ins;
+        if (!inserted) return null; // anchor not found -> skip the write
+        // Locate the freshly-inserted node to derive its "#<index>" handle. The
+        // just-uploaded attachmentId is unique, so it identifies our node.
+        if (Array.isArray(nd.content)) {
+          insertedIndex = nd.content.findIndex(
+            (b: any) =>
+              b &&
+              b.type === "drawio" &&
+              b.attrs &&
+              b.attrs.attachmentId === att.id,
+          );
+        }
+        return nd;
+      },
+    );
+
+    if (!inserted) {
+      const anchorDesc = where.anchorNodeId
+        ? `anchorNodeId "${where.anchorNodeId}"`
+        : `anchorText "${where.anchorText}"`;
+      throw new Error(
+        `drawio_create: anchor not found (${anchorDesc}) on page ${pageId}. The diagram attachment ${att.id} is now an unreferenced orphan.`,
+      );
+    }
+
+    if (insertedIndex < 0) {
+      // The node was inserted nested (e.g. inside a callout/table cell via an
+      // anchor), where "#<index>" — which addresses only top-level blocks —
+      // cannot reference it. drawio nodes carry no persisted id, so there is no
+      // stable handle for a nested diagram.
+      throw new Error(
+        `drawio_create: the diagram was inserted on page ${pageId} but not as a ` +
+          `top-level block, so it has no addressable "#<index>" handle. Anchor ` +
+          `on a top-level block (or append) so the diagram can be re-read.`,
+      );
+    }
+
+    // The returned handle is POSITIONAL ("#<index>"): valid for the immediate
+    // create -> get/update flow, but re-resolve via get_outline if the document
+    // structure changes (blocks added/removed before it shift the index).
+    const nodeId = `#${insertedIndex}`;
+
+    return {
+      success: true,
+      nodeId,
+      attachmentId: att.id,
+      warnings: prepared.warnings,
+      verify: mutation.verify,
+    };
+  }
+
+  /**
+   * Full-replacement update of a drawio diagram. `baseHash` is MANDATORY: it is
+   * compared against the hash of the diagram's CURRENT XML (from drawio_get);
+   * any mismatch means a human or another agent edited the diagram after the
+   * read, so the write is refused with a conflict error. On success the new
+   * `.drawio.svg` is uploaded as a FRESH attachment (in-place byte overwrite is
+   * avoided — some Docmost versions corrupt an attachment on overwrite, exactly
+   * as replaceImage documents) and the node is repointed with new dimensions.
+   */
+  async drawioUpdate(
+    pageId: string,
+    node: string,
+    xml: string,
+    baseHash: string,
+  ): Promise<{
+    success: boolean;
+    nodeId: string;
+    attachmentId: string;
+    warnings: string[];
+    verify?: any;
+  }> {
+    await this.ensureAuthenticated();
+    if (typeof baseHash !== "string" || baseHash.length === 0) {
+      throw new Error(
+        "drawio_update: baseHash is mandatory — read the diagram with drawio_get first and pass back its meta.hash",
+      );
+    }
+
+    // Resolve the node and read the CURRENT diagram to enforce the optimistic
+    // lock before doing any write or upload.
+    const { node: drawio, ref } = await this.resolveDrawioNode(pageId, node);
+    const oldAttrs = drawio.attrs || {};
+    const oldSrc = oldAttrs.src;
+    // The returned handle is the caller-supplied reference. drawio nodes carry
+    // no persisted id, so `ref` (an "#<index>" or a rare legacy attrs.id) is the
+    // honest identifier to hand back.
+    const nodeId = oldAttrs.id ?? ref;
+    if (!oldSrc) {
+      throw new Error(
+        `drawio_update: node "${node}" on page ${pageId} has no src to compare against`,
+      );
+    }
+    const currentSvg = await this.fetchAttachmentText(oldSrc);
+    const currentHash = mxHash(decodeDrawioSvg(currentSvg));
+    if (currentHash !== baseHash) {
+      throw new Error(
+        `drawio_update: conflict — the diagram changed since it was read ` +
+          `(baseHash ${baseHash} != current ${currentHash}). Re-read it with drawio_get and retry.`,
+      );
+    }
+
+    // Pipeline for the new content (throws a structured DrawioLintError).
+    const prepared = prepareModel(xml);
+    const inner = renderDiagramShapes(prepared.cells, prepared.bbox);
+    const diagramTitle = oldAttrs.title || "Page-1";
+    const svg = buildDrawioSvg(prepared.modelXml, inner, prepared.bbox, diagramTitle);
+
+    const att = await this.uploadAttachmentBuffer(
+      pageId,
+      Buffer.from(svg, "utf-8"),
+      "diagram.drawio.svg",
+      "image/svg+xml",
+    );
+    const newSrc = `/api/files/${att.id}/${att.fileName}`;
+
+    const collabToken = await this.getCollabTokenWithReauth();
+    const pageUuid = await this.resolvePageId(pageId);
+
+    let repointed = 0;
+    const repoint = (n: any) => {
+      n.attrs = {
+        ...n.attrs,
+        src: newSrc,
+        attachmentId: att.id,
+        width: prepared.bbox.width,
+        height: prepared.bbox.height,
+      };
+      repointed++;
+    };
+
+    const mutation = await this.mutatePage(
+      pageUuid,
+      collabToken,
+      this.apiUrl,
+      (liveDoc) => {
+        repointed = 0;
+        const doc =
+          liveDoc && liveDoc.type === "doc"
+            ? liveDoc
+            : { type: "doc", content: [] };
+        if (!Array.isArray(doc.content)) doc.content = [];
+        // Repoint ONLY the resolved node — never every node that happens to
+        // share this attachmentId (a copied diagram is two nodes with one
+        // attachmentId; keying on it would clobber both). Re-resolve the same
+        // handle against the live doc and walk to its exact position.
+        const hit = getNodeByRef(doc, ref);
+        if (!hit || hit.type !== "drawio") return null; // vanished/changed -> skip
+        let target: any = doc;
+        for (const idx of hit.path) {
+          if (!target || !Array.isArray(target.content)) {
+            target = null;
+            break;
+          }
+          target = target.content[idx];
+        }
+        if (!target || target.type !== "drawio") return null;
+        repoint(target);
+        if (repointed === 0) return null; // node vanished concurrently -> skip
+        return doc;
+      },
+    );
+
+    if (repointed === 0) {
+      return {
+        success: true,
+        nodeId,
+        attachmentId: att.id,
+        warnings: [
+          ...prepared.warnings,
+          "target drawio node was removed concurrently; uploaded attachment is unreferenced",
+        ],
+        verify: mutation.verify,
+      };
+    }
+
+    return {
+      success: true,
+      nodeId,
+      attachmentId: att.id,
+      warnings: prepared.warnings,
+      verify: mutation.verify,
+    };
+  }
+
   // --- Page history / diff / transform ---
 
   /**

packages/mcp/src/index.ts

@@ -13,11 +13,6 @@ import { SHARED_TOOL_SPECS, SharedToolSpec } from "./tool-specs.js";
 export { DocmostClient } from "./client.js";
 export type { DocmostMcpConfig } from "./client.js";
 
-// Teardown for the live per-page CollabSession cache (issue #400). An embedding
-// HTTP host (the gitmost NestJS server) should call this from its own shutdown
-// hook so no cached collab provider outlives the process.
-export { destroyAllSessions } from "./lib/collab-session.js";
-
 // Re-export the zod-agnostic shared tool-spec registry so the in-app AI-SDK
 // service can read it off the loaded module (it cannot import the ESM package's
 // internals directly; it goes through loadDocmostMcp()).
@@ -52,7 +47,7 @@ const VERSION = packageJson.version;
 export const SERVER_INSTRUCTIONS =
   "Docmost editing guide — choose the tool by intent.\n" +
   "READ: find a page -> search (workspace-wide full-text); list -> list_pages / list_spaces. Locate blocks and their ids CHEAPLY -> get_outline (compact top-level map; start here, not get_page_json). One block's subtree -> get_node (by attrs.id, or \"#<index>\" for tables, which carry no id). Find every occurrence of a string/regex ON a page (and where each is) -> search_in_page, NOT block-by-block get_node — it returns each hit's node ref + block index + context for a targeted comment. Whole page -> get_page (Markdown, lossy; inline <span data-comment-id> tags are comment anchors — markup, not text) or get_page_json (lossless ProseMirror with block ids). Hand a huge page (with images) to an external consumer without pulling it through the model context -> stash_page (returns a short-lived anonymous URL).\n" +
-  "EDIT: fix wording/typos/numbers -> edit_page_text (find/replace inside blocks, no node id needed). Change ONE block (paragraph/heading/callout/etc.) structurally -> patch_node (by attrs.id from get_outline). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Tables -> table_get / table_update_cell / table_insert_row / table_delete_row (address by \"#<index>\" from get_outline; table nodes have no attrs.id). Images -> insert_image (add from a web URL) / replace_image (swap an existing image). Footnotes -> insert_footnote. Bulk/structural rewrite -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" +
+  "EDIT: fix wording/typos/numbers -> edit_page_text (find/replace inside blocks, no node id needed). Change ONE block (paragraph/heading/callout/etc.) structurally -> patch_node (by attrs.id from get_outline). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Tables -> table_get / table_update_cell / table_insert_row / table_delete_row (address by \"#<index>\" from get_outline; table nodes have no attrs.id). Images -> insert_image (add from a web URL) / replace_image (swap an existing image). Draw.io diagrams -> drawio_create (create from mxGraph XML and insert), drawio_get (read a diagram as mxGraph XML + a hash), drawio_update (replace a diagram; pass the hash from drawio_get as baseHash for optimistic locking). Footnotes -> insert_footnote. Bulk/structural rewrite -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" +
   "PAGES: new -> create_page (Markdown). Rename (title only) -> rename_page. Move -> move_page. Delete -> delete_page (SOFT delete — the page goes to trash and is restorable; nothing is permanent). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Sharing -> share_page / unshare_page / list_shares; share_page makes the page PUBLICLY accessible — do it only when explicitly asked.\n" +
   "COMMENTS: create_comment is always inline and requires an EXACT selection — contiguous text from a single block, <=250 chars (fails rather than leaving an unanchored comment); reply to a thread via parentCommentId. Propose a concrete text fix for one-click human approval -> create_comment with suggestedText (the exact plain-text replacement for the selection; the selection must then be UNIQUE in the page — extend it with context if needed); prefer this over editing directly when the change is subjective or needs the author's sign-off. Manage -> list_comments, update_comment, resolve_comment (resolve/reopen, reversible — prefer over delete to close), delete_comment, check_new_comments.\n" +
   "HISTORY: review what changed -> diff_page_versions (a historyId vs current, or two versions). List saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown.";
@@ -465,6 +460,38 @@ registerShared(
   },
 );
 
+// Tool: drawio_get — read a draw.io diagram as mxGraph XML (or the raw SVG).
+registerShared(
+  SHARED_TOOL_SPECS.drawioGet,
+  async ({ pageId, node, format }) => {
+    const result = await docmostClient.drawioGet(pageId, node, format ?? "xml");
+    return jsonContent(result);
+  },
+);
+
+// Tool: drawio_create — lint mxGraph XML, build the .drawio.svg, insert a node.
+registerShared(
+  SHARED_TOOL_SPECS.drawioCreate,
+  async ({ pageId, xml, position, anchorNodeId, anchorText, title }) => {
+    const result = await docmostClient.drawioCreate(
+      pageId,
+      { position, anchorNodeId, anchorText },
+      xml,
+      title,
+    );
+    return jsonContent(result);
+  },
+);
+
+// Tool: drawio_update — optimistic-locked full replacement of a diagram.
+registerShared(
+  SHARED_TOOL_SPECS.drawioUpdate,
+  async ({ pageId, node, xml, baseHash }) => {
+    const result = await docmostClient.drawioUpdate(pageId, node, xml, baseHash);
+    return jsonContent(result);
+  },
+);
+
 // Tool: share_page
 // Schema + description now live in the shared registry (#294). The execute body
 // keeps this transport's own `searchIndexing ?? true` default.

packages/mcp/src/lib/collab-session.ts

@@ -1,668 +0,0 @@
-import { HocuspocusProvider } from "@hocuspocus/provider";
-import { TiptapTransformer } from "@hocuspocus/transformer";
-import * as Y from "yjs";
-import WebSocket from "ws";
-import {
-  buildCollabWsUrl,
-  applyDocToFragment,
-  MutationResult,
-} from "./collaboration.js";
-import { summarizeChange } from "./diff.js";
-
-/**
- * Live per-page collaboration session cache (issue #400).
- *
- * The one-shot write path (collaboration.mutatePageContent /
- * client.mutateLiveContentUnlocked) used to open a NEW HocuspocusProvider, run
- * the full connect -> auth -> onLoadDocument -> initial-sync handshake, apply a
- * single edit, wait for persistence, and then `provider.destroy()` — for EVERY
- * content mutation. Disconnecting after every edit means that once the pause
- * between calls exceeds the server's write debounce, the server does a full
- * store -> unload -> reload per cell, causing 25s connect timeouts and
- * event-loop lag under a burst of edits on one page.
- *
- * This module keeps ONE live provider + ydoc per (wsUrl, pageId, token) alive
- * across a SERIES of edits. While the provider stays connected the server never
- * enters store -> unload -> reload, its debounce coalesces N writes into 1-2
- * stores, and the repeated auth/load/initial-sync disappears.
- *
- * The synchronous read -> transform -> write section and the per-edit
- * persistence-ack logic are preserved VERBATIM from the one-shot machine — the
- * only change is that they run on a persistent provider instead of a throwaway
- * one. See CollabSession.mutate.
- */
-
-/** Time we wait for the initial handshake/sync before giving up. */
-const CONNECT_TIMEOUT_MS = 25000;
-/** Time we wait for the server to acknowledge our write before giving up. */
-const PERSIST_TIMEOUT_MS = 20000;
-
-/**
- * Tunables, read fresh from the environment on every acquire so tests (and a
- * live rollback) can change them without reloading the module. Mirrors how
- * http.ts parses MCP_SESSION_IDLE_MS.
- *   - MCP_COLLAB_SESSION_IDLE_MS: idle TTL, reset after every op. Default 60s.
- *     0 (or negative) DISABLES the cache — every op opens its own provider and
- *     destroys it after the op, i.e. the exact legacy per-op-provider behavior
- *     (the rollback path).
- *   - MCP_COLLAB_SESSION_MAX_AGE_MS: hard lifetime checked at acquire; bounds
- *     the permission-staleness window. Default 10 min.
- *   - MCP_COLLAB_SESSION_MAX_ENTRIES: registry cap; the least-recently-used
- *     session is destroy-evicted when the cap is reached. Default 32.
- */
-interface SessionConfig {
-  idleMs: number;
-  maxAgeMs: number;
-  maxEntries: number;
-}
-
-function parseEnvInt(value: string | undefined, fallback: number): number {
-  const parsed = parseInt(value ?? "", 10);
-  return Number.isFinite(parsed) ? parsed : fallback;
-}
-
-function readConfig(): SessionConfig {
-  // idleMs: allow 0 (disable). A malformed value falls back to the default.
-  const idleRaw = parseInt(process.env.MCP_COLLAB_SESSION_IDLE_MS ?? "", 10);
-  const idleMs = Number.isFinite(idleRaw) ? Math.max(0, idleRaw) : 60 * 1000;
-  const maxAgeMs = Math.max(
-    0,
-    parseEnvInt(process.env.MCP_COLLAB_SESSION_MAX_AGE_MS, 10 * 60 * 1000),
-  );
-  const maxEntriesRaw = parseEnvInt(
-    process.env.MCP_COLLAB_SESSION_MAX_ENTRIES,
-    32,
-  );
-  const maxEntries = maxEntriesRaw > 0 ? maxEntriesRaw : 32;
-  return { idleMs, maxAgeMs, maxEntries };
-}
-
-/**
- * The subset of HocuspocusProvider this module depends on, so the provider can
- * be replaced with a fake in unit tests (there is no server in the test env).
- */
-export interface CollabProviderLike {
-  synced: boolean;
-  unsyncedChanges: number;
-  destroy(): void;
-  on(event: "unsyncedChanges", handler: (data: { number: number }) => void): void;
-  off(event: "unsyncedChanges", handler: (data: { number: number }) => void): void;
-}
-
-/** The configuration object passed to the provider factory. */
-export interface CollabProviderConfig {
-  url: string;
-  name: string;
-  document: Y.Doc;
-  token: string;
-  WebSocketPolyfill: unknown;
-  onConnect: () => void;
-  onSynced: () => void;
-  onDisconnect: () => void;
-  onClose: () => void;
-  onAuthenticationFailed: () => void;
-}
-
-export type CollabProviderFactory = (
-  config: CollabProviderConfig,
-) => CollabProviderLike;
-
-const defaultProviderFactory: CollabProviderFactory = (config) =>
-  // @ts-ignore - WebSocketPolyfill is required for the Node.js environment.
-  new HocuspocusProvider(config) as unknown as CollabProviderLike;
-
-let providerFactory: CollabProviderFactory = defaultProviderFactory;
-
-/**
- * TEST SEAM: swap the provider factory (pass null to restore the real one).
- * Not part of the public API — used only by the unit tests, which cannot reach
- * a real collaboration server.
- */
-export function __setCollabProviderFactory(
-  factory: CollabProviderFactory | null,
-): void {
-  providerFactory = factory ?? defaultProviderFactory;
-}
-
-/** Optional per-acquire hooks (metrics), passed through from the call site. */
-export interface AcquireOptions {
-  /** Invoked when the initial connect handshake times out (CONNECT_TIMEOUT_MS). */
-  onConnectTimeout?: () => void;
-}
-
-type SessionState = "connecting" | "ready" | "dead";
-
-/**
- * One live provider + ydoc for a single (wsUrl, pageId, token) triple.
- *
- * Lifecycle: connecting -> ready -> dead. A session becomes `dead` on the first
- * disconnect/close/auth-failure at ANY time, on an idle/eviction/max-age
- * teardown, or on an explicit destroy(); death is terminal and removes the
- * session from the registry so the next acquire opens a fresh one. We never use
- * the provider's auto-reconnect — destroying on the first disconnect closes the
- * "reconnect drove unsyncedChanges to 0 without retransmitting our write" class
- * of false success.
- */
-export class CollabSession {
-  readonly key: string;
-  readonly pageId: string;
-  readonly wsUrl: string;
-  readonly token: string;
-  readonly createdAt: number;
-  state: SessionState = "connecting";
-  /**
-   * Set true on disconnect/close/auth-failure so a reconnect-driven
-   * unsyncedChanges->0 cannot be mistaken for a successful persist of our
-   * write (preserved verbatim from the one-shot machine).
-   */
-  connectionLost = false;
-
-  provider: CollabProviderLike | undefined;
-  private readonly ydoc: Y.Doc;
-  private readonly cfg: SessionConfig;
-  /**
-   * Ephemeral sessions (cache disabled, MCP_COLLAB_SESSION_IDLE_MS<=0) are never
-   * registered and self-destroy after their single op — the legacy
-   * provider-per-op behavior.
-   */
-  private readonly ephemeral: boolean;
-  private readonly opts: AcquireOptions | undefined;
-
-  private dead = false;
-  private connectTimer: ReturnType<typeof setTimeout> | undefined;
-  private idleTimer: ReturnType<typeof setTimeout> | undefined;
-  private openPromise: Promise<void> | undefined;
-  private openResolve: (() => void) | undefined;
-  private openReject: ((err: Error) => void) | undefined;
-  private openSettled = false;
-  /**
-   * The rejector of the CURRENT in-flight mutate, if any. A disconnect/close/
-   * auth-failure or timeout at ANY time rejects the in-flight op through this
-   * with the SAME error text the one-shot machine emitted.
-   */
-  private inflightReject: ((err: Error) => void) | undefined;
-
-  constructor(
-    key: string,
-    pageId: string,
-    wsUrl: string,
-    token: string,
-    cfg: SessionConfig,
-    ephemeral: boolean,
-    opts: AcquireOptions | undefined,
-  ) {
-    this.key = key;
-    this.pageId = pageId;
-    this.wsUrl = wsUrl;
-    this.token = token;
-    this.cfg = cfg;
-    this.ephemeral = ephemeral;
-    this.opts = opts;
-    this.createdAt = Date.now();
-    this.ydoc = new Y.Doc();
-  }
-
-  /**
-   * A cached session may be reused only when it is fully ready, still synced,
-   * has not lost its connection, and has not exceeded its max age (invariant 5
-   * "validate on reuse" + the max-age acquire check).
-   */
-  isReusable(): boolean {
-    return (
-      !this.dead &&
-      this.state === "ready" &&
-      !this.connectionLost &&
-      !!this.provider &&
-      this.provider.synced === true &&
-      Date.now() - this.createdAt < this.cfg.maxAgeMs
-    );
-  }
-
-  /**
-   * Connect and wait for the initial sync (onSynced) within CONNECT_TIMEOUT_MS.
-   * Idempotent: repeated calls return the same in-flight/settled promise.
-   */
-  open(): Promise<void> {
-    if (this.openPromise) return this.openPromise;
-    this.openPromise = new Promise<void>((resolve, reject) => {
-      this.openResolve = resolve;
-      this.openReject = reject;
-
-      this.connectTimer = setTimeout(() => {
-        // The 25s connect timeout: the collab connection never became ready.
-        this.opts?.onConnectTimeout?.();
-        this.teardown(
-          new Error("Connection timeout to collaboration server"),
-          false,
-        );
-      }, CONNECT_TIMEOUT_MS);
-
-      if (process.env.DEBUG)
-        console.error(`Connecting to WebSocket: ${this.wsUrl}`);
-
-      this.provider = providerFactory({
-        url: this.wsUrl,
-        name: `page.${this.pageId}`,
-        document: this.ydoc,
-        token: this.token,
-        WebSocketPolyfill: WebSocket,
-        onConnect: () => {
-          if (process.env.DEBUG) console.error("WS Connect");
-        },
-        // An unexpected disconnect/close at ANY time (during the connect-wait,
-        // between edits, or during a persistence wait) makes the session dead:
-        // surface it now instead of hanging, reject any in-flight op with the
-        // same error text as the one-shot machine, and remove ourselves from
-        // the registry so the next acquire opens fresh. `teardown` is idempotent
-        // so the onClose our own destroy() triggers is a harmless no-op.
-        onDisconnect: () => {
-          if (process.env.DEBUG) console.error("WS Disconnect");
-          this.teardown(
-            new Error(
-              "Collaboration connection closed before the update was persisted/synced",
-            ),
-            true,
-          );
-        },
-        onClose: () => {
-          if (process.env.DEBUG) console.error("WS Close");
-          this.teardown(
-            new Error(
-              "Collaboration connection closed before the update was persisted/synced",
-            ),
-            true,
-          );
-        },
-        onSynced: () => {
-          if (this.dead || this.openSettled) return;
-          if (process.env.DEBUG) console.error("Connected and synced!");
-          if (this.connectTimer) {
-            clearTimeout(this.connectTimer);
-            this.connectTimer = undefined;
-          }
-          this.state = "ready";
-          this.openSettled = true;
-          this.openResolve?.();
-        },
-        onAuthenticationFailed: () => {
-          this.teardown(
-            new Error("Authentication failed for collaboration connection"),
-            true,
-          );
-        },
-      });
-    });
-    return this.openPromise;
-  }
-
-  /**
-   * Run one atomic read -> transform -> write against the LIVE doc and wait for
-   * the server to acknowledge the write.
-   *
-   * INVARIANT 1 (read->write atomicity): between `TiptapTransformer.fromYdoc`
-   * and `applyDocToFragment` there is NO `await`. Yjs applies remote updates
-   * only when the event loop yields, so this synchronous block sees a consistent
-   * live doc and no concurrent human edit can interleave and be clobbered —
-   * exactly as in the one-shot onSynced code, just on a persistent provider.
-   *
-   * INVARIANT 2 (per-edit ack): after the write, resolve immediately if
-   * unsyncedChanges is already 0, else wait for the unsyncedChanges->0 event
-   * (PERSIST_TIMEOUT_MS), guarded by connectionLost so a reconnect handshake
-   * cannot report a false success.
-   *
-   * CONCURRENCY: not safe to invoke concurrently on ONE session — the caller
-   * MUST serialize (hold the per-page lock), mirroring acquireCollabSession.
-   * The in-flight op is tracked in a single `inflightReject` field, so an
-   * overlapping second call would clobber the first's rejector and leave it
-   * hanging on disconnect. A fail-fast guard below rejects the overlap instead.
-   * Sequential (awaited) mutates are fine: localFinish clears inflightReject
-   * before the promise settles, so the guard is clear by the time the next runs.
-   */
-  mutate(
-    transform: (liveDoc: any) => any | null,
-  ): Promise<MutationResult> {
-    // Belt-and-suspenders (acquire already validated): refuse to write on a
-    // session that is not in a live, synced, ready state.
-    if (
-      this.dead ||
-      this.state !== "ready" ||
-      this.connectionLost ||
-      !this.provider ||
-      this.provider.synced !== true
-    ) {
-      return Promise.reject(
-        new Error("Collaboration session is not in a ready state"),
-      );
-    }
-
-    // Fail-fast on concurrent use: a second overlapping mutate would overwrite
-    // the first's inflightReject, so a disconnect would only reject the second
-    // and hang the first until PERSIST_TIMEOUT_MS. Reject the overlap WITHOUT
-    // touching the in-flight op's state (no localFinish/teardown here).
-    if (this.inflightReject) {
-      return Promise.reject(
-        new Error(
-          "mutate already in-flight; caller must serialize (hold the page lock)",
-        ),
-      );
-    }
-
-    return new Promise<MutationResult>((resolve, reject) => {
-      let settled = false;
-      let persistTimer: ReturnType<typeof setTimeout> | undefined;
-      let unsyncedHandler:
-        | ((data: { number: number }) => void)
-        | undefined;
-      // The verifiable result resolved on every success/abort path. Set on
-      // abort (no-op report) and after a real write (computed change report).
-      let mutationResult: MutationResult;
-
-      const localFinish = (err: Error | null, value?: MutationResult) => {
-        if (settled) return;
-        settled = true;
-        if (persistTimer) clearTimeout(persistTimer);
-        if (unsyncedHandler && this.provider) {
-          try {
-            this.provider.off("unsyncedChanges", unsyncedHandler);
-          } catch (e) {}
-        }
-        this.inflightReject = undefined;
-        if (err) reject(err);
-        else resolve(value as MutationResult);
-        // Post-settle lifecycle: an ephemeral (cache-disabled) session dies with
-        // its single op; a cached session that is still alive re-arms its idle
-        // TTL so the clock starts from the LAST op.
-        if (this.ephemeral) {
-          this.destroy("ephemeral op complete");
-        } else if (!this.dead) {
-          this.armIdle();
-        }
-      };
-
-      // Register so a disconnect/close/auth-failure/teardown rejects THIS op
-      // with the connection-loss error text. localFinish's `settled` guard makes
-      // a racing teardown + normal resolve safe (first one wins).
-      this.inflightReject = (e: Error) => localFinish(e);
-
-      // Resolve once the server acknowledges our update: the provider increments
-      // unsyncedChanges when the local update is sent and decrements it on the
-      // server's SyncStatus(applied=true); reaching 0 means the authoritative
-      // in-memory ydoc on the server now contains our write.
-      const waitForPersistence = () => {
-        if (settled) return;
-        // A missing provider is a failure, not a success: without it the write
-        // can never have been acknowledged.
-        if (!this.provider) {
-          localFinish(new Error("collab provider gone before persistence"));
-          return;
-        }
-        if (this.provider.unsyncedChanges === 0) {
-          localFinish(null, mutationResult);
-          return;
-        }
-        persistTimer = setTimeout(() => {
-          localFinish(
-            new Error(
-              "Timeout waiting for collaboration server to persist the update",
-            ),
-          );
-        }, PERSIST_TIMEOUT_MS);
-        unsyncedHandler = (data: { number: number }) => {
-          // Only treat unsyncedChanges->0 as success when the connection is
-          // still up. A transient disconnect + reconnect handshake can drive the
-          // counter back to 0 without our write being re-transmitted; in that
-          // case let the disconnect/close error win instead.
-          if (data.number === 0 && !this.connectionLost) {
-            localFinish(null, mutationResult);
-          }
-        };
-        this.provider.on("unsyncedChanges", unsyncedHandler);
-      };
-
-      // CRITICAL: everything between reading the live doc and writing it back
-      // must stay synchronous (no await). While the JS event loop is not
-      // yielded, no incoming remote update can interleave, so any already-synced
-      // concurrent edits are preserved in liveDoc.
-      let newDoc: any;
-      let beforeDoc: any;
-      try {
-        let liveDoc = TiptapTransformer.fromYdoc(this.ydoc, "default");
-        if (
-          !liveDoc ||
-          typeof liveDoc !== "object" ||
-          !Array.isArray(liveDoc.content)
-        ) {
-          liveDoc = { type: "doc", content: [] };
-        }
-
-        // Snapshot the before-doc for the change report. Docs are
-        // JSON-serializable, so this is a safe deep clone.
-        beforeDoc = JSON.parse(JSON.stringify(liveDoc));
-
-        newDoc = transform(liveDoc);
-
-        if (newDoc == null) {
-          // Transform aborted — write nothing, return the live doc with a no-op
-          // change report.
-          mutationResult = {
-            doc: liveDoc,
-            verify: {
-              changed: false,
-              textInserted: 0,
-              textDeleted: 0,
-              blocksChanged: 0,
-              marks: {},
-              summary: "no changes (transform aborted)",
-            },
-          };
-          localFinish(null, mutationResult);
-          return;
-        }
-
-        // Structural diff into the live fragment (issue #152): preserves the Yjs
-        // ids of unchanged nodes, so an open editor's cursor is not yanked to the
-        // end of the document on every agent write.
-        applyDocToFragment(this.ydoc, newDoc);
-      } catch (e) {
-        // Includes errors thrown by transform (e.g. "afterText not found",
-        // "text not found"): propagate them verbatim to the caller.
-        localFinish(e instanceof Error ? e : new Error(String(e)));
-        return;
-      }
-
-      // Compute the verifiable change report AFTER the transact write: it only
-      // needs the JSON before/after, so it cannot affect the atomic read->write
-      // window, and summarizeChange never throws.
-      mutationResult = {
-        doc: newDoc,
-        verify: summarizeChange(beforeDoc, newDoc),
-      };
-      if (process.env.DEBUG)
-        console.error("Content written, waiting for server to persist...");
-      waitForPersistence();
-    });
-  }
-
-  /** (Re)arm the idle TTL so the clock starts from the most recent activity. */
-  armIdle(): void {
-    if (this.dead || this.ephemeral) return;
-    if (this.idleTimer) clearTimeout(this.idleTimer);
-    if (this.cfg.idleMs > 0) {
-      this.idleTimer = setTimeout(() => {
-        this.destroy("idle timeout");
-      }, this.cfg.idleMs);
-      // Never let the idle timer keep the process alive.
-      (this.idleTimer as any).unref?.();
-    }
-  }
-
-  /**
-   * Idempotent teardown: mark dead, clear timers, remove from the registry, fail
-   * any pending open/in-flight op, and destroy the provider. `inflightError` is
-   * the error a pending open or in-flight op is rejected with; `connectionLoss`
-   * marks the session as connection-lost so the ack guard cannot report a false
-   * success on a racing unsyncedChanges->0.
-   */
-  private teardown(inflightError: Error | null, connectionLoss: boolean): void {
-    if (this.dead) return;
-    this.dead = true;
-    this.state = "dead";
-    if (connectionLoss) this.connectionLost = true;
-
-    if (this.connectTimer) {
-      clearTimeout(this.connectTimer);
-      this.connectTimer = undefined;
-    }
-    if (this.idleTimer) {
-      clearTimeout(this.idleTimer);
-      this.idleTimer = undefined;
-    }
-
-    // Remove ourselves from the registry (only if we are still the live entry —
-    // a re-open under the same key must not be evicted by our teardown).
-    if (sessions.get(this.key) === this) {
-      sessions.delete(this.key);
-    }
-
-    // Fail a pending open() and any in-flight mutate with the terminal error.
-    if (!this.openSettled) {
-      this.openSettled = true;
-      this.openReject?.(
-        inflightError ?? new Error("Collaboration session destroyed"),
-      );
-    }
-    if (this.inflightReject) {
-      const rej = this.inflightReject;
-      this.inflightReject = undefined;
-      rej(inflightError ?? new Error("Collaboration session destroyed"));
-    }
-
-    if (this.provider) {
-      try {
-        this.provider.destroy();
-      } catch (e) {}
-      this.provider = undefined;
-    }
-  }
-
-  /**
-   * Public idempotent teardown used by the acquire/eviction paths and by a
-   * caller that wants the session dropped after a failed op ("next call
-   * reconnects fresh").
-   */
-  destroy(reason: string): void {
-    if (this.dead) return;
-    if (process.env.DEBUG)
-      console.error(`Destroying collab session ${this.pageId}: ${reason}`);
-    this.teardown(new Error(`Collaboration session destroyed: ${reason}`), false);
-  }
-}
-
-/** key = wsUrl + pageId + collabToken (identity isolation: invariant 4). */
-const sessions = new Map<string, CollabSession>();
-
-function sessionKey(wsUrl: string, pageId: string, token: string): string {
-  // The token is part of the key so sessions are NEVER shared between different
-  // users' MCP sessions (HTTP mode), and a token rotation makes a new entry
-  // while the old one idles out.
-  return `${wsUrl}${pageId}${token}`;
-}
-
-/**
- * Get a live, synced CollabSession for a page, reusing a cached one when it is
- * still valid or opening a fresh one otherwise. Does NOT take the per-page lock
- * — the caller MUST already hold it (both call sites run inside withPageLock,
- * which is not reentrant, so acquiring the lock here would deadlock
- * mutateLiveContentUnlocked).
- */
-export async function acquireCollabSession(
-  pageId: string,
-  collabToken: string,
-  baseUrl: string,
-  opts?: AcquireOptions,
-): Promise<CollabSession> {
-  const cfg = readConfig();
-  const wsUrl = buildCollabWsUrl(baseUrl);
-
-  // Cache disabled (rollback path): open an unregistered ephemeral session that
-  // self-destroys after its single op — the exact legacy per-op-provider flow.
-  if (cfg.idleMs <= 0) {
-    const session = new CollabSession(
-      sessionKey(wsUrl, pageId, collabToken),
-      pageId,
-      wsUrl,
-      collabToken,
-      cfg,
-      true,
-      opts,
-    );
-    await session.open();
-    return session;
-  }
-
-  const key = sessionKey(wsUrl, pageId, collabToken);
-  const existing = sessions.get(key);
-  if (existing) {
-    if (existing.isReusable()) {
-      // Reuse. Refresh LRU order (re-insert = most recently used) and re-arm the
-      // idle TTL so the reuse counts as activity.
-      sessions.delete(key);
-      sessions.set(key, existing);
-      existing.armIdle();
-      if (process.env.DEBUG)
-        console.error(`Reusing collab session for page ${pageId}`);
-      return existing;
-    }
-    // Stale (not synced / past max age / lost): drop it and open fresh.
-    existing.destroy("stale on reuse");
-  }
-
-  // Enforce the registry cap before inserting: destroy-evict the least recently
-  // used (the first entry in insertion order) until there is room.
-  while (sessions.size >= cfg.maxEntries) {
-    const oldestKey: string | undefined = sessions.keys().next().value;
-    if (oldestKey === undefined) break;
-    const victim = sessions.get(oldestKey);
-    if (victim) victim.destroy("evicted (LRU cap)");
-    // destroy() removes it from the map; guard against a no-op destroy.
-    if (sessions.has(oldestKey)) sessions.delete(oldestKey);
-  }
-
-  const session = new CollabSession(
-    key,
-    pageId,
-    wsUrl,
-    collabToken,
-    cfg,
-    false,
-    opts,
-  );
-  sessions.set(key, session);
-  try {
-    await session.open();
-  } catch (e) {
-    // Failed connect/sync: make sure it is not left cached.
-    session.destroy("open failed");
-    throw e;
-  }
-  session.armIdle();
-  if (process.env.DEBUG)
-    console.error(`Opened new collab session for page ${pageId}`);
-  return session;
-}
-
-/**
- * Destroy every cached session. Wired into the process shutdown so a hanging
- * session does not keep a doc loaded on the server past exit.
- */
-export function destroyAllSessions(): void {
-  for (const session of [...sessions.values()]) {
-    session.destroy("process shutdown");
-  }
-  sessions.clear();
-}
-
-/** TEST-ONLY: number of currently cached sessions. */
-export function __sessionCountForTests(): number {
-  return sessions.size;
-}

packages/mcp/src/lib/collaboration.ts

@@ -1,3 +1,4 @@
+import { HocuspocusProvider } from "@hocuspocus/provider";
 import { TiptapTransformer } from "@hocuspocus/transformer";
 import * as Y from "yjs";
 import WebSocket from "ws";
@@ -15,8 +16,7 @@ import { docmostExtensions, docmostSchema } from "./docmost-schema.js";
 import { withPageLock } from "./page-lock.js";
 import { sanitizeForYjs, findUnstorableAttr } from "./node-ops.js";
 import { canonicalizeFootnotes } from "./footnote-canonicalize.js";
-import { VerifyReport } from "./diff.js";
-import { acquireCollabSession } from "./collab-session.js";
+import { summarizeChange, VerifyReport } from "./diff.js";
 
 export { markdownToProseMirror };
 
@@ -194,27 +194,26 @@ export function assertYjsEncodable(doc: any): void {
   }
 }
 
+/** Time we wait for the initial handshake/sync before giving up. */
+const CONNECT_TIMEOUT_MS = 25000;
+/** Time we wait for the server to acknowledge our write before giving up. */
+const PERSIST_TIMEOUT_MS = 20000;
+
 /**
  * Safely mutate the live content of a page over the collaboration websocket.
  *
  * This is the single safe write path for every MCP content mutation. It:
  *   1. serializes per-page writes through withPageLock (no two MCP writes on
  *      the same page overlap);
- *   2. acquires a LIVE, synced CollabSession for the page (issue #400) — a
- *      cached provider whose local ydoc mirrors the authoritative server doc
- *      (INCLUDING edits/comments/images not yet in the debounced REST snapshot),
- *      reused across a series of edits instead of a fresh connect/auth/sync per
- *      call;
- *   3. SYNCHRONOUSLY reads the live doc, runs `transform`, and writes the result
- *      back — with no `await` between read and write so no remote update can
- *      interleave and clobber concurrent human edits (CollabSession.mutate);
+ *   2. connects to Hocuspocus and waits for the initial sync so the local ydoc
+ *      mirrors the authoritative server doc — INCLUDING edits/comments/images
+ *      that are not yet in the debounced REST snapshot;
+ *   3. inside onSynced, SYNCHRONOUSLY reads the live doc, runs `transform`, and
+ *      writes the result back — with no `await` between read and write so no
+ *      remote update can interleave and clobber concurrent human edits;
  *   4. waits for the server to acknowledge the write (unsyncedChanges -> 0)
  *      before resolving, so the next operation observes our change.
  *
- * On any mutate failure the session is destroyed so the next call reconnects
- * fresh; the page lock is held for the whole acquire+mutate so the session's
- * synchronous read->write window never overlaps another MCP write on the page.
- *
  * `transform` receives the live ProseMirror doc and returns the NEW full
  * ProseMirror doc to write, or `null` to abort with no write (a no-op). If
  * `transform` throws, the error is propagated to the caller (not swallowed).
@@ -231,7 +230,7 @@ export async function mutatePageContent(
   baseUrl: string,
   transform: (liveDoc: any) => any | null,
 ): Promise<MutationResult> {
-  return withPageLock(pageId, async () => {
+  return withPageLock(pageId, () => {
     if (process.env.DEBUG) {
       console.error(`Starting realtime content mutate for page ${pageId}`);
       // Token prefix is sensitive; only log it under DEBUG.
@@ -240,15 +239,202 @@ export async function mutatePageContent(
       );
     }
 
-    const session = await acquireCollabSession(pageId, collabToken, baseUrl);
-    try {
-      return await session.mutate(transform);
-    } catch (e) {
-      // Drop the session on any failure so the next call reconnects fresh (this
-      // also closes the "reconnect drove the counter to 0" false-success class).
-      session.destroy("mutate failed");
-      throw e;
-    }
+    const ydoc = new Y.Doc();
+    const wsUrl = buildCollabWsUrl(baseUrl);
+    if (process.env.DEBUG) console.error(`Connecting to WebSocket: ${wsUrl}`);
+
+    return new Promise<MutationResult>((resolve, reject) => {
+      let provider: HocuspocusProvider | undefined;
+      let applied = false; // onSynced may fire again on reconnect — apply once.
+      let settled = false;
+      // Set true on disconnect/close so a reconnect-driven unsyncedChanges->0
+      // cannot be mistaken for a successful persist of our write.
+      let connectionLost = false;
+      let connectTimer: ReturnType<typeof setTimeout> | undefined;
+      let persistTimer: ReturnType<typeof setTimeout> | undefined;
+      let unsyncedHandler: ((data: { number: number }) => void) | undefined;
+
+      const cleanup = () => {
+        if (connectTimer) clearTimeout(connectTimer);
+        if (persistTimer) clearTimeout(persistTimer);
+        if (provider) {
+          if (unsyncedHandler) {
+            try {
+              provider.off("unsyncedChanges", unsyncedHandler);
+            } catch (err) {}
+          }
+          try {
+            provider.destroy();
+          } catch (err) {}
+        }
+      };
+
+      const finish = (err: Error | null, value?: MutationResult) => {
+        if (settled) return;
+        settled = true;
+        cleanup();
+        if (err) reject(err);
+        else resolve(value as MutationResult);
+      };
+
+      connectTimer = setTimeout(() => {
+        finish(new Error("Connection timeout to collaboration server"));
+      }, CONNECT_TIMEOUT_MS);
+
+      // Resolve once the server has acknowledged our update. The provider
+      // increments unsyncedChanges when our local update is sent and
+      // decrements it when the server replies with a SyncStatus(applied=true);
+      // reaching 0 means the authoritative in-memory ydoc on the server now
+      // contains our write.
+      const waitForPersistence = () => {
+        if (settled) return;
+        // A missing provider is a failure, not a success: without it the write
+        // can never have been acknowledged. Only an actual unsyncedChanges===0
+        // on a live provider counts as persisted.
+        if (!provider) {
+          finish(new Error("collab provider gone before persistence"));
+          return;
+        }
+        if (provider.unsyncedChanges === 0) {
+          finish(null, mutationResult);
+          return;
+        }
+        persistTimer = setTimeout(() => {
+          finish(
+            new Error(
+              "Timeout waiting for collaboration server to persist the update",
+            ),
+          );
+        }, PERSIST_TIMEOUT_MS);
+        unsyncedHandler = (data: { number: number }) => {
+          // Only treat unsyncedChanges->0 as success when the connection is
+          // still up. A transient disconnect + reconnect handshake can drive
+          // the counter back to 0 without our write being re-transmitted; in
+          // that case let the disconnect/close error win instead.
+          if (data.number === 0 && !connectionLost) {
+            finish(null, mutationResult);
+          }
+        };
+        provider.on("unsyncedChanges", unsyncedHandler);
+      };
+
+      // The verifiable result resolved on every success/abort path. Set on
+      // abort (no-op report) and after a real write (computed change report).
+      let mutationResult: MutationResult;
+
+      provider = new HocuspocusProvider({
+        url: wsUrl,
+        name: `page.${pageId}`,
+        document: ydoc,
+        token: collabToken,
+        // @ts-ignore - Required for Node.js environment
+        WebSocketPolyfill: WebSocket,
+        onConnect: () => {
+          if (process.env.DEBUG) console.error("WS Connect");
+        },
+        // An unexpected disconnect/close while we are still waiting (during the
+        // connect-wait before onSynced, or during the persistence wait after the
+        // write) means the update will never be acknowledged — surface it now
+        // instead of hanging until the connect/persist timeout fires. `finish`
+        // is idempotent via the `settled` flag, so the onClose that our own
+        // cleanup()->provider.destroy() triggers (after settled=true is set) is
+        // a harmless no-op and cannot cause a double-resolve.
+        onDisconnect: () => {
+          if (process.env.DEBUG) console.error("WS Disconnect");
+          // Mark BEFORE finish so the unsyncedChanges handler (if it races)
+          // sees the connection as lost and won't report a false success.
+          connectionLost = true;
+          finish(
+            new Error(
+              "Collaboration connection closed before the update was persisted/synced",
+            ),
+          );
+        },
+        onClose: () => {
+          if (process.env.DEBUG) console.error("WS Close");
+          // Mark BEFORE finish so the unsyncedChanges handler (if it races)
+          // sees the connection as lost and won't report a false success.
+          connectionLost = true;
+          finish(
+            new Error(
+              "Collaboration connection closed before the update was persisted/synced",
+            ),
+          );
+        },
+        onSynced: () => {
+          if (applied || settled) return;
+          applied = true;
+          if (process.env.DEBUG) console.error("Connected and synced!");
+
+          // CRITICAL: everything between reading the live doc and writing it
+          // back must stay synchronous (no await). While the JS event loop is
+          // not yielded, no incoming remote update can interleave, so any
+          // already-synced concurrent edits are preserved in liveDoc.
+          let newDoc: any;
+          let beforeDoc: any;
+          try {
+            let liveDoc = TiptapTransformer.fromYdoc(ydoc, "default");
+            if (
+              !liveDoc ||
+              typeof liveDoc !== "object" ||
+              !Array.isArray(liveDoc.content)
+            ) {
+              liveDoc = { type: "doc", content: [] };
+            }
+
+            // Snapshot the before-doc for the change report. Docs are
+            // JSON-serializable, so this is a safe deep clone.
+            beforeDoc = JSON.parse(JSON.stringify(liveDoc));
+
+            newDoc = transform(liveDoc);
+
+            if (newDoc == null) {
+              // Transform aborted — write nothing, return the live doc with a
+              // no-op change report.
+              mutationResult = {
+                doc: liveDoc,
+                verify: {
+                  changed: false,
+                  textInserted: 0,
+                  textDeleted: 0,
+                  blocksChanged: 0,
+                  marks: {},
+                  summary: "no changes (transform aborted)",
+                },
+              };
+              finish(null, mutationResult);
+              return;
+            }
+
+            // Structural diff into the live fragment (issue #152): preserves
+            // the Yjs ids of unchanged nodes, so an open editor's cursor is not
+            // yanked to the end of the document on every agent write.
+            applyDocToFragment(ydoc, newDoc);
+          } catch (e) {
+            // Includes errors thrown by transform (e.g. "afterText not found",
+            // "text not found"): propagate them verbatim to the caller.
+            finish(e instanceof Error ? e : new Error(String(e)));
+            return;
+          }
+
+          // Compute the verifiable change report AFTER the transact write: it
+          // only needs the JSON before/after, so it cannot affect the atomic
+          // read->write window, and summarizeChange never throws.
+          mutationResult = {
+            doc: newDoc,
+            verify: summarizeChange(beforeDoc, newDoc),
+          };
+          if (process.env.DEBUG)
+            console.error("Content written, waiting for server to persist...");
+          waitForPersistence();
+        },
+        onAuthenticationFailed: () => {
+          finish(
+            new Error("Authentication failed for collaboration connection"),
+          );
+        },
+      });
+    });
   });
 }
 

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

@@ -0,0 +1,193 @@
+// Pure-TS schematic SVG preview for draw.io diagrams (issue #423, stage 1).
+//
+// HARD CONSTRAINT: no backend rendering. This is a dependency-free string
+// builder — given the parsed mxGraph cells it draws a rough schematic (rects,
+// ellipses, diamonds, edges + labels) that stands in as the diagram's visible
+// image UNTIL a human first opens it in the draw.io editor and saves, at which
+// point the client replaces this with the pixel-perfect export SVG. It is
+// deliberately approximate: it exists so a freshly-agent-created diagram is not
+// an empty box in the page.
+
+import type { DrawioCell, DrawioBBox } from "./drawio-xml.js";
+import { absolutePos } from "./drawio-xml.js";
+
+function esc(s: string): string {
+  return s
+    .replace(/&/g, "&")
+    .replace(/</g, "&lt;")
+    .replace(/>/g, "&gt;")
+    .replace(/"/g, "&quot;");
+}
+
+/**
+ * Strip HTML markup from a cell value (draw.io labels are HTML when html=1),
+ * decode the handful of entities we care about, and collapse whitespace so the
+ * label fits on the schematic. `<br>` becomes a space (this is a one-line
+ * preview label, not a faithful multi-line render).
+ */
+function labelText(value: string): string {
+  return value
+    .replace(/<br\s*\/?>/gi, " ")
+    .replace(/<[^>]+>/g, "")
+    .replace(/&lt;/g, "<")
+    .replace(/&gt;/g, ">")
+    .replace(/&quot;/g, '"')
+    .replace(/&#39;|&apos;/g, "'")
+    .replace(/&#xa;|&#10;/gi, " ")
+    .replace(/&amp;/g, "&")
+    .replace(/\s+/g, " ")
+    .trim();
+}
+
+function centeredLabel(cx: number, cy: number, value: string, color = "#000000"): string {
+  const text = labelText(value);
+  if (!text) return "";
+  return (
+    `<text x="${round(cx)}" y="${round(cy)}" ` +
+    `font-family="Helvetica, Arial, sans-serif" font-size="12" ` +
+    `text-anchor="middle" dominant-baseline="middle" fill="${esc(color)}">` +
+    `${esc(text)}</text>`
+  );
+}
+
+function round(n: number): number {
+  return Math.round(n * 100) / 100;
+}
+
+interface ShapeKind {
+  kind: "ellipse" | "rhombus" | "triangle" | "rect";
+}
+
+/**
+ * Decide which schematic primitive to draw for a vertex. A shape can be named
+ * either as the style's base token (e.g. "ellipse;…") or as a key (e.g.
+ * "shape=rhombus" / "ellipse=1"), so both the base style and the map are
+ * checked.
+ */
+function shapeKind(
+  styleMap: Record<string, string>,
+  baseStyle?: string,
+): ShapeKind {
+  const shape = styleMap.shape ?? baseStyle;
+  const has = (name: string) => shape === name || styleMap[name] != null;
+  if (has("ellipse")) return { kind: "ellipse" };
+  if (has("rhombus")) return { kind: "rhombus" };
+  if (has("triangle")) return { kind: "triangle" };
+  // Everything else — including unknown stencils (shape=mxgraph.*), swimlanes,
+  // and plain boxes — is drawn as a (rounded) rectangle.
+  return { kind: "rect" };
+}
+
+function fill(styleMap: Record<string, string>): string {
+  const c = styleMap.fillColor;
+  if (!c || c.toLowerCase() === "none") return "#ffffff";
+  return c;
+}
+
+function stroke(styleMap: Record<string, string>): string {
+  const c = styleMap.strokeColor;
+  if (!c || c.toLowerCase() === "none") return "#000000";
+  return c;
+}
+
+/**
+ * Render the schematic shapes as the INNER content of the `.drawio.svg` (the
+ * outer <svg> wrapper is added by drawio-xml.buildDrawioSvg). Coordinates are
+ * absolute (container children are resolved via the parent chain).
+ */
+export function renderDiagramShapes(cells: DrawioCell[], _bbox: DrawioBBox): string {
+  const byId = new Map(cells.map((c) => [c.id, c]));
+  const parts: string[] = [];
+
+  // Edges first so vertices sit on top of their connectors.
+  for (const c of cells) {
+    if (!c.edge) continue;
+    parts.push(renderEdge(c, byId));
+  }
+
+  for (const c of cells) {
+    if (!c.vertex || !c.geometry.hasGeometry) continue;
+    const g = c.geometry;
+    if (g.width == null || g.height == null) continue;
+    const { x, y } = absolutePos(c, byId);
+    parts.push(renderVertex(c, x, y, g.width, g.height));
+  }
+
+  return `<g>${parts.filter(Boolean).join("")}</g>`;
+}
+
+function renderVertex(
+  c: DrawioCell,
+  x: number,
+  y: number,
+  w: number,
+  h: number,
+): string {
+  const f = esc(fill(c.styleMap));
+  const s = esc(stroke(c.styleMap));
+  const { kind } = shapeKind(c.styleMap, c.baseStyle);
+  const cx = x + w / 2;
+  const cy = y + h / 2;
+  let shape = "";
+  switch (kind) {
+    case "ellipse":
+      shape =
+        `<ellipse cx="${round(cx)}" cy="${round(cy)}" rx="${round(w / 2)}" ` +
+        `ry="${round(h / 2)}" fill="${f}" stroke="${s}"/>`;
+      break;
+    case "rhombus": {
+      const pts = [
+        `${round(cx)},${round(y)}`,
+        `${round(x + w)},${round(cy)}`,
+        `${round(cx)},${round(y + h)}`,
+        `${round(x)},${round(cy)}`,
+      ].join(" ");
+      shape = `<polygon points="${pts}" fill="${f}" stroke="${s}"/>`;
+      break;
+    }
+    case "triangle": {
+      const pts = [
+        `${round(x)},${round(y)}`,
+        `${round(x + w)},${round(cy)}`,
+        `${round(x)},${round(y + h)}`,
+      ].join(" ");
+      shape = `<polygon points="${pts}" fill="${f}" stroke="${s}"/>`;
+      break;
+    }
+    default: {
+      const rounded = c.styleMap.rounded === "1";
+      const rx = rounded ? Math.min(12, w / 2, h / 2) : 0;
+      shape =
+        `<rect x="${round(x)}" y="${round(y)}" width="${round(w)}" ` +
+        `height="${round(h)}" rx="${round(rx)}" ry="${round(rx)}" ` +
+        `fill="${f}" stroke="${s}"/>`;
+    }
+  }
+  return shape + centeredLabel(cx, cy, c.value, c.styleMap.fontColor || "#000000");
+}
+
+function renderEdge(c: DrawioCell, byId: Map<string, DrawioCell>): string {
+  const src = c.source != null ? byId.get(c.source) : undefined;
+  const tgt = c.target != null ? byId.get(c.target) : undefined;
+  const p1 = anchorPoint(src, byId);
+  const p2 = anchorPoint(tgt, byId);
+  if (!p1 || !p2) return ""; // a floating endpoint with no fixed point: skip
+  const line =
+    `<line x1="${round(p1.x)}" y1="${round(p1.y)}" ` +
+    `x2="${round(p2.x)}" y2="${round(p2.y)}" ` +
+    `stroke="#000000" stroke-width="1"/>`;
+  const mid = { x: (p1.x + p2.x) / 2, y: (p1.y + p2.y) / 2 };
+  return line + centeredLabel(mid.x, mid.y, c.value);
+}
+
+/** Center point of a vertex used as an edge anchor (approximate). */
+function anchorPoint(
+  cell: DrawioCell | undefined,
+  byId: Map<string, DrawioCell>,
+): { x: number; y: number } | null {
+  if (!cell || !cell.geometry.hasGeometry) return null;
+  const g = cell.geometry;
+  if (g.width == null || g.height == null) return null;
+  const { x, y } = absolutePos(cell, byId);
+  return { x: x + g.width / 2, y: y + g.height / 2 };
+}

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

@@ -0,0 +1,771 @@
+// draw.io (mxGraph) XML support for the MCP drawio tools (issue #423, stage 1).
+//
+// This module owns everything that is pure data-plumbing for draw.io diagrams:
+//   - the DECODE CHAIN that turns a stored `diagram.drawio.svg` attachment back
+//     into mxGraph XML (handles both the plain nested-XML form Docmost writes
+//     and draw.io's own COMPRESSED `<diagram>` payload — base64 + raw-deflate);
+//   - the ENCODE side that wraps mxGraph XML into the `.drawio.svg` attachment
+//     using the exact same contract as the import service's createDrawioSvg;
+//   - a deterministic LINTER that rejects the structural mistakes generators
+//     make before anything is written (each violation carries the offending
+//     cellId + position so the model can auto-retry);
+//   - a stable HASH over the normalized XML, used as the optimistic-lock key.
+//
+// HARD CONSTRAINT: no backend rendering. Nothing here shells out or renders a
+// bitmap; the only runtime dependencies are jsdom (already used across this
+// package for XML parsing) and pako (raw-inflate for the compressed format).
+
+import { createHash } from "node:crypto";
+import { JSDOM } from "jsdom";
+import pako from "pako";
+
+// --- shared XML parser -----------------------------------------------------
+
+// A single reusable JSDOM window; constructing one per parse is wasteful and
+// these tools are low-frequency. Only the DOMParser is used.
+let _window: any = null;
+function xmlWindow(): any {
+  if (!_window) _window = new JSDOM("").window;
+  return _window;
+}
+
+/** Default mxGraphModel attributes used when the server wraps a cell list. */
+const DEFAULT_MODEL_ATTRS =
+  'dx="0" dy="0" grid="1" gridSize="10" page="1" pageWidth="850" pageHeight="1100"';
+
+// --- structured lint errors ------------------------------------------------
+
+export interface DrawioLintIssue {
+  /** Machine-readable rule id, e.g. "edge-geometry". */
+  rule: string;
+  /** Human-readable explanation the model can act on. */
+  message: string;
+  /** The offending cell's id, when the rule is cell-scoped. */
+  cellId?: string;
+  /** Extra location info: cell index in <root>, or a parser line:col. */
+  position?: string;
+}
+
+/**
+ * Thrown by the linter and by decode/prepare when the input is unusable. Carries
+ * the full list of issues so the caller can surface a structured tool-error the
+ * model auto-retries against.
+ */
+export class DrawioLintError extends Error {
+  issues: DrawioLintIssue[];
+  constructor(issues: DrawioLintIssue[]) {
+    const summary = issues
+      .map((i) => {
+        const where = [
+          i.cellId != null ? `cellId=${i.cellId}` : null,
+          i.position != null ? `at ${i.position}` : null,
+        ]
+          .filter(Boolean)
+          .join(", ");
+        return `[${i.rule}] ${i.message}${where ? ` (${where})` : ""}`;
+      })
+      .join("; ");
+    super(`drawio lint failed: ${summary}`);
+    this.name = "DrawioLintError";
+    this.issues = issues;
+  }
+}
+
+// --- parsed-cell model -----------------------------------------------------
+
+export interface DrawioGeometry {
+  x?: number;
+  y?: number;
+  width?: number;
+  height?: number;
+  relative: boolean;
+  hasGeometry: boolean;
+}
+
+export interface DrawioCell {
+  id: string;
+  parent?: string;
+  source?: string;
+  target?: string;
+  vertex: boolean;
+  edge: boolean;
+  value: string;
+  style: string;
+  styleMap: Record<string, string>;
+  /** Non-key/value leading token of the style (a base stylename), if any. */
+  baseStyle?: string;
+  geometry: DrawioGeometry;
+}
+
+export interface DrawioBBox {
+  width: number;
+  height: number;
+}
+
+// --- style parsing ---------------------------------------------------------
+
+/**
+ * Parse a draw.io style string into { baseStyle, map }. Grammar:
+ *   [stylename;]key=value;key=value;...
+ * A single leading token without '=' is the base stylename (e.g. "text" or
+ * "ellipse"). Every other non-empty segment must be exactly one key=value pair.
+ * Returns `null` (the segment index) on the first malformed segment so the
+ * linter can report a precise error.
+ */
+export function parseStyle(
+  style: string,
+): { baseStyle?: string; map: Record<string, string>; badSegment?: string } {
+  const map: Record<string, string> = {};
+  let baseStyle: string | undefined;
+  const segments = style.split(";");
+  for (let i = 0; i < segments.length; i++) {
+    const seg = segments[i].trim();
+    if (seg === "") continue; // trailing/empty segments are fine
+    const eq = seg.indexOf("=");
+    if (eq === -1) {
+      // A bare token is only valid as the FIRST meaningful segment (base style).
+      if (baseStyle === undefined && Object.keys(map).length === 0) {
+        baseStyle = seg;
+        continue;
+      }
+      return { baseStyle, map, badSegment: seg };
+    }
+    // A second '=' inside the same segment is malformed.
+    if (seg.indexOf("=", eq + 1) !== -1) {
+      return { baseStyle, map, badSegment: seg };
+    }
+    const key = seg.slice(0, eq).trim();
+    const val = seg.slice(eq + 1).trim();
+    if (key === "") return { baseStyle, map, badSegment: seg };
+    map[key] = val;
+  }
+  return { baseStyle, map };
+}
+
+// --- low-level XML helpers -------------------------------------------------
+
+function parseXml(xml: string): { doc: any; error: string | null } {
+  const parser = new (xmlWindow().DOMParser)();
+  const doc = parser.parseFromString(xml, "application/xml");
+  const err = doc.getElementsByTagName("parsererror");
+  if (err.length > 0) {
+    // jsdom prefixes the message with "line:col:" — keep it as the position.
+    return { doc, error: (err[0].textContent || "malformed XML").trim() };
+  }
+  return { doc, error: null };
+}
+
+function num(v: string | null): number | undefined {
+  if (v == null || v === "") return undefined;
+  const n = Number(v);
+  return Number.isFinite(n) ? n : undefined;
+}
+
+/** Extract the raw `<mxGraphModel …>…</mxGraphModel>` substring, or null. */
+function sliceModel(xml: string): string | null {
+  const open = xml.indexOf("<mxGraphModel");
+  if (open === -1) return null;
+  const close = xml.indexOf("</mxGraphModel>", open);
+  if (close === -1) {
+    // Self-closed empty model, e.g. `<mxGraphModel .../>`.
+    const selfClose = xml.indexOf("/>", open);
+    if (selfClose !== -1) return xml.slice(open, selfClose + 2);
+    return null;
+  }
+  return xml.slice(open, close + "</mxGraphModel>".length);
+}
+
+// --- 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.
+ */
+export function extractContentAttr(svg: string): string {
+  const { doc, error } = parseXml(svg);
+  if (!error) {
+    const root = doc.documentElement;
+    if (root && root.hasAttribute && root.hasAttribute("content")) {
+      return root.getAttribute("content") || "";
+    }
+  }
+  // Fallback for a malformed wrapper: pull the attribute directly. The content
+  // 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.
+    return m[1]
+      .replace(/&lt;/g, "<")
+      .replace(/&gt;/g, ">")
+      .replace(/&quot;/g, '"')
+      .replace(/&#39;/g, "'")
+      .replace(/&amp;/g, "&");
+  }
+  throw new Error("drawio: SVG has no content= attribute to decode");
+}
+
+/**
+ * Turn a decoded draw.io file (`<mxfile>` or a bare `<mxGraphModel>`, possibly
+ * with a COMPRESSED `<diagram>` payload) into the mxGraphModel XML. For the
+ * plain form the raw substring is returned verbatim so a round-trip stays
+ * byte-stable; the compressed form is inflated (base64 → raw-deflate →
+ * decodeURIComponent), which is how draw.io stores diagrams by default.
+ */
+export function decodeDrawioFileToModel(fileXml: string): string {
+  // Plain, nested XML: return the model substring untouched (byte-stable).
+  const sliced = sliceModel(fileXml);
+  if (sliced) return sliced;
+
+  // Otherwise it must be the compressed `<diagram>…</diagram>` text payload.
+  const open = fileXml.indexOf("<diagram");
+  if (open !== -1) {
+    const gt = fileXml.indexOf(">", open);
+    const close = fileXml.indexOf("</diagram>", gt);
+    if (gt !== -1 && close !== -1) {
+      const payload = fileXml.slice(gt + 1, close).trim();
+      if (payload) {
+        const inflated = inflateDiagramPayload(payload);
+        const model = sliceModel(inflated);
+        if (model) return model;
+        return inflated;
+      }
+    }
+  }
+  throw new Error(
+    "drawio: could not decode file — no <mxGraphModel> and no compressed <diagram> payload",
+  );
+}
+
+/**
+ * Upper bound on the inflated size of a compressed `<diagram>` payload
+ * (decompression-bomb guard). `fetchInternalFile` caps the DOWNLOAD at 64 MiB,
+ * but a tiny crafted compressed payload can inflate to gigabytes and OOM the
+ * process. A real diagram's mxGraphModel XML is small (KBs to low MBs even for
+ * large diagrams), so 16 MiB is far above any legitimate payload while keeping
+ * memory bounded. Chars ~= bytes for the (mostly ASCII) URI-encoded XML.
+ */
+export const MAX_INFLATED_DIAGRAM_BYTES = 16 * 1024 * 1024;
+
+/**
+ * Inflate draw.io's compressed diagram payload:
+ *   base64-decode → raw-inflate (raw deflate, windowBits -15) →
+ *   decodeURIComponent.
+ *
+ * Uses pako's streaming Inflate so we can abort as soon as the decompressed
+ * output exceeds MAX_INFLATED_DIAGRAM_BYTES — the full bomb is never
+ * materialised in memory.
+ */
+export function inflateDiagramPayload(base64: string): string {
+  const bytes = Buffer.from(base64, "base64");
+  const inflator = new pako.Inflate({ raw: true, to: "string" });
+  let total = 0;
+  const passthrough = inflator.onData.bind(inflator);
+  inflator.onData = (chunk: string | Uint8Array) => {
+    total += chunk.length;
+    if (total > MAX_INFLATED_DIAGRAM_BYTES) {
+      // Throwing here propagates out of push(), aborting inflation immediately.
+      throw new Error(
+        `drawio: refusing to decode diagram — decompressed size exceeds ` +
+          `${MAX_INFLATED_DIAGRAM_BYTES} bytes (possible decompression bomb)`,
+      );
+    }
+    passthrough(chunk);
+  };
+  inflator.push(bytes, true);
+  if (inflator.err) {
+    throw new Error(
+      `drawio: failed to inflate compressed <diagram> payload (${inflator.msg || inflator.err})`,
+    );
+  }
+  const uriEncoded = inflator.result as string;
+  return decodeURIComponent(uriEncoded);
+}
+
+/** Full decode chain: `.drawio.svg` string → mxGraphModel XML. */
+export function decodeDrawioSvg(svg: string): string {
+  const content = extractContentAttr(svg).trim();
+  const fileXml = content.startsWith("<")
+    ? content
+    : Buffer.from(content, "base64").toString("utf-8");
+  return decodeDrawioFileToModel(fileXml);
+}
+
+// --- encode side -----------------------------------------------------------
+
+/**
+ * Wrap an mxGraphModel in the plain (uncompressed) `<mxfile><diagram>` envelope.
+ * draw.io opens uncompressed XML fine, and staying uncompressed keeps the
+ * write path deterministic and the round-trip byte-stable.
+ */
+export function encodeDrawioFile(modelXml: string, title = "Page-1"): string {
+  const safeTitle = xmlEscape(title);
+  return `<mxfile host="drawio"><diagram id="page-1" name="${safeTitle}">${modelXml}</diagram></mxfile>`;
+}
+
+/**
+ * Build the `diagram.drawio.svg` attachment. Mirrors the import service's
+ * createDrawioSvg contract exactly:
+ *   <svg xmlns=… xmlns:xlink=… content="${base64(drawioFile)}">${inner}</svg>
+ * plus width/height/viewBox from the diagram bounding box and the schematic
+ * preview as the visible children (`inner`).
+ */
+export function buildDrawioSvg(
+  modelXml: string,
+  inner: string,
+  bbox: DrawioBBox,
+  title = "Page-1",
+): string {
+  const file = encodeDrawioFile(modelXml, title);
+  const base64 = Buffer.from(file, "utf-8").toString("base64");
+  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>`
+  );
+}
+
+function xmlEscape(s: string): string {
+  return s
+    .replace(/&/g, "&amp;")
+    .replace(/</g, "&lt;")
+    .replace(/>/g, "&gt;")
+    .replace(/"/g, "&quot;");
+}
+
+// --- normalization + hash --------------------------------------------------
+
+/**
+ * Normalize mxGraph XML for hashing / stable comparison: drop the whitespace
+ * between tags and trim. This is intentionally conservative — it never reorders
+ * attributes or cells (that would be lossy) — so two documents hash equal iff
+ * they differ only in inter-tag formatting.
+ */
+export function normalizeXml(xml: string): string {
+  return xml.replace(/>\s+</g, "><").trim();
+}
+
+/** Stable optimistic-lock hash over the normalized model XML (sha256, hex). */
+export function mxHash(modelXml: string): string {
+  return createHash("sha256").update(normalizeXml(modelXml), "utf-8").digest("hex");
+}
+
+// --- cell parsing ----------------------------------------------------------
+
+/** Parse every `<mxCell>` in a model into a structured DrawioCell list. */
+export function parseCells(modelXml: string): DrawioCell[] {
+  const { doc, error } = parseXml(modelXml);
+  if (error) {
+    throw new DrawioLintError([
+      { rule: "well-formed-xml", message: error, position: firstLineCol(error) },
+    ]);
+  }
+  const cells: DrawioCell[] = [];
+  const els = doc.getElementsByTagName("mxCell");
+  for (let i = 0; i < els.length; i++) {
+    cells.push(readCell(els[i]));
+  }
+  return cells;
+}
+
+function readCell(el: any): DrawioCell {
+  const style = el.getAttribute("style") || "";
+  const parsed = parseStyle(style);
+  const geoEl = firstChildByTag(el, "mxGeometry");
+  const geometry: DrawioGeometry = geoEl
+    ? {
+        x: num(geoEl.getAttribute("x")),
+        y: num(geoEl.getAttribute("y")),
+        width: num(geoEl.getAttribute("width")),
+        height: num(geoEl.getAttribute("height")),
+        relative: geoEl.getAttribute("relative") === "1",
+        hasGeometry: true,
+      }
+    : { relative: false, hasGeometry: false };
+  return {
+    id: el.getAttribute("id") ?? "",
+    parent: el.getAttribute("parent") ?? undefined,
+    source: el.getAttribute("source") ?? undefined,
+    target: el.getAttribute("target") ?? undefined,
+    vertex: el.getAttribute("vertex") === "1",
+    edge: el.getAttribute("edge") === "1",
+    value: el.getAttribute("value") ?? "",
+    style,
+    styleMap: parsed.map,
+    baseStyle: parsed.baseStyle,
+    geometry,
+  };
+}
+
+function firstChildByTag(el: any, tag: string): any {
+  for (let i = 0; i < el.childNodes.length; i++) {
+    const c = el.childNodes[i];
+    if (c.nodeType === 1 && c.tagName === tag) return c;
+  }
+  return null;
+}
+
+function firstLineCol(msg: string): string | undefined {
+  const m = /^(\d+:\d+)/.exec(msg);
+  return m ? m[1] : undefined;
+}
+
+// --- bounding box ----------------------------------------------------------
+
+/**
+ * Absolute bounding box of the diagram from its vertex geometries. Container
+ * children are relative, so absolute positions are resolved along the parent
+ * chain before taking the extent. Falls back to a default canvas when empty.
+ */
+export function computeBBox(cells: DrawioCell[]): DrawioBBox {
+  const byId = new Map(cells.map((c) => [c.id, c]));
+  let maxX = 0;
+  let maxY = 0;
+  let any = false;
+  for (const c of cells) {
+    if (!c.vertex || !c.geometry.hasGeometry) continue;
+    const g = c.geometry;
+    if (g.width == null || g.height == null) continue;
+    const { x, y } = absolutePos(c, byId);
+    maxX = Math.max(maxX, x + g.width);
+    maxY = Math.max(maxY, y + g.height);
+    any = true;
+  }
+  if (!any) return { width: 300, height: 200 };
+  // A small margin so borders/labels are not clipped at the edge.
+  return { width: Math.ceil(maxX) + 20, height: Math.ceil(maxY) + 20 };
+}
+
+/** Absolute (x,y) of a vertex, following its parent chain (containers). */
+export function absolutePos(
+  cell: DrawioCell,
+  byId: Map<string, DrawioCell>,
+): { x: number; y: number } {
+  let x = cell.geometry.x ?? 0;
+  let y = cell.geometry.y ?? 0;
+  const seen = new Set<string>([cell.id]);
+  let parentId = cell.parent;
+  while (parentId && !seen.has(parentId)) {
+    seen.add(parentId);
+    const p = byId.get(parentId);
+    // Sentinels (0/1) carry no geometry; stop there.
+    if (!p || !p.vertex || !p.geometry.hasGeometry) break;
+    x += p.geometry.x ?? 0;
+    y += p.geometry.y ?? 0;
+    parentId = p.parent;
+  }
+  return { x, y };
+}
+
+// --- linter ----------------------------------------------------------------
+
+/**
+ * Run every deterministic pre-write rule over a full mxGraphModel string. On any
+ * violation it throws a DrawioLintError carrying one issue per violation, each
+ * with the offending cellId + position. Returns the parsed cells on success.
+ */
+export function lintModel(modelXml: string): {
+  cells: DrawioCell[];
+  warnings: string[];
+} {
+  const issues: DrawioLintIssue[] = [];
+  const warnings: string[] = [];
+
+  // Rule: no XML comments. Checked on the raw string (a comment survives DOM
+  // parsing as a comment node, but the intent is to reject them outright — they
+  // routinely wrap "TODO" cruft that breaks downstream tooling).
+  if (modelXml.includes("<!--")) {
+    issues.push({
+      rule: "no-comments",
+      message: "XML comments (<!-- -->) are not allowed in diagram XML",
+    });
+  }
+
+  // Rule: value escaping + literal newline. Scan raw <mxCell> tags so the error
+  // can name the cell id even when the whole document is otherwise malformed.
+  scanRawValues(modelXml, issues);
+
+  // Well-formedness — everything below needs a parsed DOM.
+  const { doc, error } = parseXml(modelXml);
+  if (error) {
+    issues.push({
+      rule: "well-formed-xml",
+      message: error,
+      position: firstLineCol(error),
+    });
+    throw new DrawioLintError(issues);
+  }
+
+  const root = doc.documentElement;
+  if (!root || root.tagName !== "mxGraphModel") {
+    issues.push({
+      rule: "structure",
+      message: `root element must be <mxGraphModel>, got <${root ? root.tagName : "?"}>`,
+    });
+    throw new DrawioLintError(issues);
+  }
+  if (!firstChildByTag(root, "root")) {
+    issues.push({
+      rule: "structure",
+      message: "<mxGraphModel> must contain a <root> element",
+    });
+    throw new DrawioLintError(issues);
+  }
+
+  const cells = parseCells(modelXml);
+  const ids = new Set<string>();
+
+  // Rule: sentinel cells id="0" and id="1"(parent="0").
+  const cell0 = cells.find((c) => c.id === "0");
+  const cell1 = cells.find((c) => c.id === "1");
+  if (!cell0) {
+    issues.push({
+      rule: "sentinel-cells",
+      message: 'missing the root sentinel cell <mxCell id="0"/>',
+      cellId: "0",
+    });
+  }
+  if (!cell1) {
+    issues.push({
+      rule: "sentinel-cells",
+      message: 'missing the layer sentinel cell <mxCell id="1" parent="0"/>',
+      cellId: "1",
+    });
+  } else if (cell1.parent !== "0") {
+    issues.push({
+      rule: "sentinel-cells",
+      message: 'the layer sentinel <mxCell id="1"> must have parent="0"',
+      cellId: "1",
+    });
+  }
+
+  cells.forEach((c, index) => {
+    const pos = `cell #${index}`;
+    const isSentinel = c.id === "0" || c.id === "1";
+
+    // Rule: unique, non-empty ids; user cells must not reuse 0/1.
+    if (c.id === "") {
+      issues.push({ rule: "cell-id", message: "cell has an empty id", position: pos });
+    } else if (ids.has(c.id)) {
+      issues.push({
+        rule: "duplicate-id",
+        message: `duplicate cell id "${c.id}"`,
+        cellId: c.id,
+        position: pos,
+      });
+    }
+    ids.add(c.id);
+
+    if (isSentinel) return; // sentinels are exempt from the shape rules below
+
+    // Rule: vertex XOR edge (a cell may be neither: groups/containers).
+    if (c.vertex && c.edge) {
+      issues.push({
+        rule: "vertex-edge-exclusive",
+        message: 'a cell cannot be both vertex="1" and edge="1"',
+        cellId: c.id,
+        position: pos,
+      });
+    }
+
+    // Rule: every edge has a child <mxGeometry as="geometry"/>.
+    if (c.edge && !c.geometry.hasGeometry) {
+      issues.push({
+        rule: "edge-geometry",
+        message:
+          'edge is missing its child <mxGeometry relative="1" as="geometry"/> — it will not render',
+        cellId: c.id,
+        position: pos,
+      });
+    }
+
+    // Rule: edge endpoints resolve to existing ids.
+    if (c.edge) {
+      for (const end of ["source", "target"] as const) {
+        const ref = c[end];
+        if (ref != null && ref !== "" && !cellExists(cells, ref)) {
+          issues.push({
+            rule: "edge-endpoint",
+            message: `edge ${end} "${ref}" does not resolve to any cell`,
+            cellId: c.id,
+            position: pos,
+          });
+        }
+      }
+    }
+
+    // Rule: parent must exist.
+    if (c.parent != null && c.parent !== "" && !cellExists(cells, c.parent)) {
+      issues.push({
+        rule: "parent-exists",
+        message: `parent "${c.parent}" does not resolve to any cell`,
+        cellId: c.id,
+        position: pos,
+      });
+    }
+
+    // Rule: style parses as key=value; pairs.
+    if (c.style !== "") {
+      const parsed = parseStyle(c.style);
+      if (parsed.badSegment !== undefined) {
+        issues.push({
+          rule: "style-format",
+          message: `malformed style segment "${parsed.badSegment}" (expected key=value)`,
+          cellId: c.id,
+          position: pos,
+        });
+      }
+    }
+  });
+
+  if (issues.length > 0) throw new DrawioLintError(issues);
+  return { cells, warnings };
+}
+
+function cellExists(cells: DrawioCell[], id: string): boolean {
+  return cells.some((c) => c.id === id);
+}
+
+/**
+ * Raw-string scan of every `value="…"`/`value='…'` on an mxCell tag. Catches an
+ * unescaped `&`/`<`/`>` and a literal newline character inside a value, keyed to
+ * the cell's id. Runs before DOM parsing so a value bug is reported with its
+ * cellId even when the document is otherwise malformed.
+ */
+function scanRawValues(xml: string, issues: DrawioLintIssue[]): void {
+  const tagRe = /<mxCell\b([^>]*?)\/?>/g;
+  let m: RegExpExecArray | null;
+  while ((m = tagRe.exec(xml)) !== null) {
+    const attrs = m[1];
+    const idM = /\bid\s*=\s*"([^"]*)"/.exec(attrs);
+    const cellId = idM ? idM[1] : undefined;
+    const valM = /\bvalue\s*=\s*"([^"]*)"/.exec(attrs) || /\bvalue\s*=\s*'([^']*)'/.exec(attrs);
+    if (!valM) continue;
+    const raw = valM[1];
+    // Literal newline (0x0A / 0x0D) inside the attribute value.
+    if (/[\n\r]/.test(raw)) {
+      issues.push({
+        rule: "value-newline",
+        message:
+          "value contains a literal newline; use &#xa; (or <br> with html=1) instead",
+        cellId,
+      });
+    }
+    // Unescaped '<' or '>' inside a value.
+    if (raw.includes("<") || raw.includes(">")) {
+      issues.push({
+        rule: "value-escaping",
+        message: "value contains an unescaped '<' or '>'; use &lt; / &gt;",
+        cellId,
+      });
+    }
+    // '&' that does not begin a valid entity.
+    const badAmp = /&(?!(amp|lt|gt|quot|apos|#[0-9]+|#x[0-9a-fA-F]+);)/.test(raw);
+    if (badAmp) {
+      issues.push({
+        rule: "value-escaping",
+        message: "value contains an unescaped '&'; use &amp;",
+        cellId,
+      });
+    }
+  }
+}
+
+// --- input normalization + prepare -----------------------------------------
+
+/**
+ * Normalize an accepted tool input into a full mxGraphModel string:
+ *   - a bare `<mxGraphModel>` is used as-is;
+ *   - an `<mxfile>` is decoded to its first page's model;
+ *   - a list of `<mxCell>` is wrapped with the mxGraphModel/root envelope and
+ *     the sentinel cells (id=0, id=1 parent=0) are added when absent.
+ */
+export function normalizeInput(inputXml: string): string {
+  let xml = inputXml.trim();
+  // Strip an optional XML prolog.
+  if (xml.startsWith("<?xml")) {
+    const end = xml.indexOf("?>");
+    if (end !== -1) xml = xml.slice(end + 2).trim();
+  }
+  if (xml.startsWith("<mxfile")) {
+    return decodeDrawioFileToModel(xml);
+  }
+  if (xml.startsWith("<mxGraphModel")) {
+    return xml;
+  }
+  if (xml.includes("<mxCell")) {
+    return wrapCellFragment(xml);
+  }
+  throw new DrawioLintError([
+    {
+      rule: "unrecognized-input",
+      message:
+        "input must be a <mxGraphModel>, an <mxfile>, or a list of <mxCell> elements",
+    },
+  ]);
+}
+
+function wrapCellFragment(fragment: string): string {
+  // Validate the fragment is well-formed (wrapped so a bare list parses) and
+  // discover which sentinels are already present.
+  const { doc, error } = parseXml(`<root>${fragment}</root>`);
+  if (error) {
+    throw new DrawioLintError([
+      {
+        rule: "well-formed-xml",
+        message: error,
+        position: firstLineCol(error),
+      },
+    ]);
+  }
+  const existing = new Set<string>();
+  const els = doc.getElementsByTagName("mxCell");
+  for (let i = 0; i < els.length; i++) {
+    existing.add(els[i].getAttribute("id") ?? "");
+  }
+  let prefix = "";
+  if (!existing.has("0")) prefix += '<mxCell id="0"/>';
+  if (!existing.has("1")) prefix += '<mxCell id="1" parent="0"/>';
+  return `<mxGraphModel ${DEFAULT_MODEL_ATTRS}><root>${prefix}${fragment}</root></mxGraphModel>`;
+}
+
+export interface PreparedModel {
+  /** Canonical (normalized) mxGraphModel XML that gets written. */
+  modelXml: string;
+  cells: DrawioCell[];
+  bbox: DrawioBBox;
+  /** Number of user cells (excludes the id=0/id=1 sentinels). */
+  cellCount: number;
+  warnings: string[];
+  hash: string;
+}
+
+/**
+ * Full pre-write pipeline for create/update: normalize the input into a model,
+ * lint it (throws DrawioLintError on any violation), then compute the canonical
+ * form, bounding box, cell count and hash. Never touches the network.
+ */
+export function prepareModel(inputXml: string): PreparedModel {
+  const rawModel = normalizeInput(inputXml);
+  const { cells, warnings } = lintModel(rawModel);
+  const modelXml = normalizeXml(rawModel);
+  const bbox = computeBBox(cells);
+  const cellCount = cells.filter((c) => c.id !== "0" && c.id !== "1").length;
+  return {
+    modelXml,
+    cells,
+    bbox,
+    cellCount,
+    warnings,
+    hash: mxHash(modelXml),
+  };
+}
+
+/** Cell count of a decoded model (user cells only) — used by drawio_get meta. */
+export function countUserCells(modelXml: string): number {
+  return parseCells(modelXml).filter((c) => c.id !== "0" && c.id !== "1").length;
+}

packages/mcp/src/lib/pako.d.ts

@@ -0,0 +1,61 @@
+// Minimal ambient type declaration for `pako` (no @types/pako is installed and
+// pako 2.x ships no bundled .d.ts). We only use the raw-deflate codec to read
+// draw.io's compressed `<diagram>` payload, so declare just that surface.
+declare module "pako" {
+  interface RawOptions {
+    /** When "string", the result is returned as a (binary/UTF-8) string. */
+    to?: "string";
+    /** Raw-deflate window bits; draw.io uses raw deflate (no zlib header). */
+    windowBits?: number;
+    level?: number;
+  }
+
+  /** Raw-inflate (windowBits: -15). `to:"string"` yields a string. */
+  export function inflateRaw(
+    data: Uint8Array | ArrayBuffer | number[],
+    options: RawOptions & { to: "string" },
+  ): string;
+  export function inflateRaw(
+    data: Uint8Array | ArrayBuffer | number[],
+    options?: RawOptions,
+  ): Uint8Array;
+
+  /** Raw-deflate (windowBits: -15). Used only by tests to build fixtures. */
+  export function deflateRaw(
+    data: Uint8Array | string,
+    options?: RawOptions,
+  ): Uint8Array;
+
+  interface InflateStreamOptions {
+    to?: "string";
+    windowBits?: number;
+    /** Raw deflate (no zlib header) — equivalent to windowBits: -15. */
+    raw?: boolean;
+    chunkSize?: number;
+  }
+
+  /**
+   * Streaming inflate. We use it to bound the decompressed size: `onData` is
+   * invoked per output chunk, letting us abort a decompression bomb before the
+   * full output is materialised.
+   */
+  export class Inflate {
+    constructor(options?: InflateStreamOptions);
+    onData: (chunk: string | Uint8Array) => void;
+    onEnd: (status: number) => void;
+    push(
+      data: Uint8Array | ArrayBuffer | number[] | string,
+      flushMode?: boolean | number,
+    ): boolean;
+    result: string | Uint8Array;
+    err: number;
+    msg: string;
+  }
+
+  const _default: {
+    inflateRaw: typeof inflateRaw;
+    deflateRaw: typeof deflateRaw;
+    Inflate: typeof Inflate;
+  };
+  export default _default;
+}

packages/mcp/src/stdio.ts

@@ -1,7 +1,6 @@
 #!/usr/bin/env node
 import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
 import { createDocmostMcpServer } from "./index.js";
-import { destroyAllSessions } from "./lib/collab-session.js";
 
 // Standalone stdio entrypoint. This restores the original behavior of the
 // package when run as a CLI (`docmost-mcp`): it reads credentials from the
@@ -34,20 +33,6 @@ async function run() {
     console.error("Uncaught exception:", error);
   });
 
-  // Teardown hook (issue #400): destroy every cached live CollabSession on exit
-  // so a hanging session does not keep a doc loaded on the server (which would
-  // also defer the server's afterUnloadDocument cleanup). `exit` runs the
-  // synchronous idempotent teardown; SIGINT/SIGTERM also run it, then exit.
-  process.on("exit", () => {
-    destroyAllSessions();
-  });
-  for (const signal of ["SIGINT", "SIGTERM"] as const) {
-    process.on(signal, () => {
-      destroyAllSessions();
-      process.exit(0);
-    });
-  }
-
   const server = createDocmostMcpServer({
     apiUrl: API_URL!,
     email: EMAIL!,

packages/mcp/src/tool-specs.ts

@@ -1115,4 +1115,112 @@ export const SHARED_TOOL_SPECS = {
       alt: z.string().optional(),
     }),
   },
+
+  // --- draw.io diagrams (issue #423, stage 1) ---
+
+  drawioGet: {
+    mcpName: 'drawio_get',
+    inAppKey: 'drawioGet',
+    description:
+      'Read a draw.io diagram on a page as mxGraph XML (default) or as its raw ' +
+      '`.drawio.svg`. `node` is the drawio node\'s attrs.id (from get_outline / ' +
+      'get_page_json) or "#<index>" for a top-level block. Returns the decoded ' +
+      'mxGraphModel XML plus meta { attachmentId, title, width, height, ' +
+      'cellCount, hash }. `hash` is the optimistic-lock key you MUST pass back ' +
+      'as baseHash to drawio_update. Diagrams a human saved from the editor ' +
+      '(including draw.io\'s compressed format) decode losslessly.',
+    tier: 'deferred',
+    catalogLine:
+      'drawioGet — read a draw.io diagram as mxGraph XML (+ hash for updates).',
+    buildShape: (z) => ({
+      pageId: z.string().min(1),
+      node: z
+        .string()
+        .min(1)
+        .describe('The drawio node attrs.id, or "#<index>" for a top-level block.'),
+      format: z
+        .enum(['xml', 'svg'])
+        .optional()
+        .describe('"xml" (default) for mxGraph XML, or "svg" for the raw .drawio.svg.'),
+    }),
+  },
+
+  drawioCreate: {
+    mcpName: 'drawio_create',
+    inAppKey: 'drawioCreate',
+    description:
+      'Create a draw.io diagram from mxGraph XML and insert it as a diagram ' +
+      'block. `xml` is a bare `<mxGraphModel>` OR a list of `<mxCell>` elements ' +
+      '(the server wraps it and adds the id=0 / id=1 sentinel cells). The XML is ' +
+      'LINTED first (well-formedness, sentinel cells, unique ids, vertex XOR ' +
+      'edge, every edge has a child <mxGeometry as="geometry"/>, edge ' +
+      'source/target and every parent resolve, style parses, no XML comments, ' +
+      'value escaping) — a violation returns a structured error naming the rule ' +
+      'and cellId so you can fix and retry. `where` positions the block like ' +
+      'insert_node: position before/after (with exactly one of anchorNodeId or ' +
+      'anchorText) or append. Returns { nodeId, attachmentId, warnings }. The ' +
+      'returned `nodeId` is an index-based "#<index>" handle (drawio nodes carry ' +
+      'no attrs.id): it addresses the new top-level block and can be fed straight ' +
+      'back into drawio_get / drawio_update for THIS document. It is positional, ' +
+      'so if you add or remove blocks before it, re-resolve via get_outline. The ' +
+      'diagram is editable in the draw.io editor and can be re-read with ' +
+      'drawio_get.',
+    tier: 'deferred',
+    catalogLine:
+      'drawioCreate — create a draw.io diagram from mxGraph XML and insert it.',
+    buildShape: (z) => ({
+      pageId: z.string().min(1),
+      xml: z
+        .string()
+        .min(1)
+        .describe(
+          'mxGraph XML: a bare <mxGraphModel> or a list of <mxCell> elements.',
+        ),
+      position: z
+        .enum(['before', 'after', 'append'])
+        .describe('Where to insert relative to the anchor.'),
+      anchorNodeId: z
+        .string()
+        .optional()
+        .describe('Anchor block id (for before/after).'),
+      anchorText: z
+        .string()
+        .optional()
+        .describe('Anchor text fragment (for before/after).'),
+      title: z.string().optional().describe('Optional diagram title.'),
+    }),
+  },
+
+  drawioUpdate: {
+    mcpName: 'drawio_update',
+    inAppKey: 'drawioUpdate',
+    description:
+      'Replace a draw.io diagram\'s content with new mxGraph XML (same lint ' +
+      'pipeline as drawio_create). `baseHash` is MANDATORY: pass the hash from ' +
+      'the drawio_get you based the edit on. If the diagram changed since ' +
+      '(a human or another agent edited it) the hash mismatches and the update ' +
+      'is refused with a conflict error — re-read with drawio_get and retry. On ' +
+      'success it overwrites the diagram attachment and updates the node ' +
+      'width/height. `node` is the drawio node attrs.id or "#<index>".',
+    tier: 'deferred',
+    catalogLine:
+      'drawioUpdate — replace a draw.io diagram (optimistic-locked by baseHash).',
+    buildShape: (z) => ({
+      pageId: z.string().min(1),
+      node: z
+        .string()
+        .min(1)
+        .describe('The drawio node attrs.id, or "#<index>" for a top-level block.'),
+      xml: z
+        .string()
+        .min(1)
+        .describe(
+          'New mxGraph XML: a bare <mxGraphModel> or a list of <mxCell> elements.',
+        ),
+      baseHash: z
+        .string()
+        .min(1)
+        .describe('The meta.hash from the drawio_get this edit is based on.'),
+    }),
+  },
 } satisfies Record<string, SharedToolSpec>;

packages/mcp/test/mock/drawio-tools.test.mjs

@@ -0,0 +1,467 @@
+// Contract tests for the drawio_get / drawio_create / drawio_update client
+// methods (issue #423). Follows the repo's seam-override pattern (see
+// full-doc-write-canonicalize.test.mjs): a DocmostClient subclass stubs the I/O
+// seams (auth, collab token, page read, attachment upload/fetch, the mutatePage
+// write) so the tool logic is exercised without a live Docmost or collab socket.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import pako from "pako";
+import { DocmostClient } from "../../build/client.js";
+import {
+  buildDrawioSvg,
+  encodeDrawioFile,
+  normalizeXml,
+  mxHash,
+  decodeDrawioSvg,
+} from "../../build/lib/drawio-xml.js";
+
+const MODEL =
+  '<mxGraphModel><root>' +
+  '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+  '<mxCell id="2" value="Hi" style="rounded=1;" vertex="1" parent="1">' +
+  '<mxGeometry x="20" y="20" width="120" height="60" as="geometry"/></mxCell>' +
+  '</root></mxGraphModel>';
+
+// Build a Docmost-style `.drawio.svg` (base64 content) for a model.
+function svgFor(model) {
+  return buildDrawioSvg(normalizeXml(model), "<g/>", { width: 200, height: 120 });
+}
+
+// Build a human/compressed-export `.drawio.svg` (base64 content wrapping a
+// compressed <diagram> payload), mimicking a diagram a person saved.
+function compressedSvgFor(model) {
+  const compressed = Buffer.from(
+    pako.deflateRaw(encodeURIComponent(normalizeXml(model))),
+  ).toString("base64");
+  const file = `<mxfile host="Electron"><diagram id="a" name="Page-1">${compressed}</diagram></mxfile>`;
+  const content = Buffer.from(file, "utf-8").toString("base64");
+  return `<svg xmlns="http://www.w3.org/2000/svg" content="${content}"><image href="x"/></svg>`;
+}
+
+// The vendored `drawio` node schema (diagramAttributes) declares ONLY these
+// attributes; PMNode.fromJSON drops anything else on save. Mirror that here so
+// the mock write path behaves like the real one — in particular, a block `id`
+// set on a drawio node does NOT survive the save, so a handle keyed on it is
+// un-resolvable. This is exactly what the production bug (issue #423 Fix 1) was.
+const DRAWIO_SCHEMA_ATTRS = new Set([
+  "src",
+  "title",
+  "alt",
+  "width",
+  "height",
+  "size",
+  "aspectRatio",
+  "align",
+  "attachmentId",
+]);
+
+function applyDrawioSchemaDrop(node) {
+  if (!node || typeof node !== "object") return;
+  if (node.type === "drawio" && node.attrs && typeof node.attrs === "object") {
+    for (const key of Object.keys(node.attrs)) {
+      if (!DRAWIO_SCHEMA_ATTRS.has(key)) delete node.attrs[key];
+    }
+  }
+  if (Array.isArray(node.content)) for (const c of node.content) applyDrawioSchemaDrop(c);
+}
+
+function makeClient({ pageDoc, attachmentSvg } = {}) {
+  const calls = { uploads: [], mutations: [] };
+  class TestClient extends DocmostClient {
+    async ensureAuthenticated() {}
+    async getCollabTokenWithReauth() {
+      return "collab-token";
+    }
+    async resolvePageId(pageId) {
+      return `uuid-${pageId}`;
+    }
+    async getPageRaw(pageId) {
+      return {
+        id: pageId,
+        slugId: "s",
+        title: "P",
+        spaceId: "sp",
+        content: pageDoc ?? { type: "doc", content: [] },
+      };
+    }
+    async uploadAttachmentBuffer(pageId, buffer, fileName, mime) {
+      const id = `att-${calls.uploads.length + 1}`;
+      calls.uploads.push({ pageId, fileName, mime, svg: buffer.toString("utf-8") });
+      return { id, fileName, fileSize: buffer.length };
+    }
+    async fetchAttachmentText(src) {
+      return attachmentSvg;
+    }
+    mutatePage(pageId, token, apiUrl, transform) {
+      // Run the transform against a clone of the source doc, capture the result.
+      const clone = structuredClone(pageDoc ?? { type: "doc", content: [] });
+      const doc = transform(clone);
+      // Mirror the real schema: unknown drawio attrs (e.g. a block `id`) are
+      // dropped on save, so callers can never rely on them to address the node.
+      if (doc) applyDrawioSchemaDrop(doc);
+      calls.mutations.push({ pageId, doc });
+      return Promise.resolve({ doc, verify: { changed: doc != null } });
+    }
+  }
+  const client = new TestClient("http://127.0.0.1:1/api", "e@x.com", "pw");
+  return { client, calls };
+}
+
+function findDrawio(node, acc = []) {
+  if (!node || typeof node !== "object") return acc;
+  if (node.type === "drawio") acc.push(node);
+  if (Array.isArray(node.content)) for (const c of node.content) findDrawio(c, acc);
+  return acc;
+}
+
+// --- drawio_create ---------------------------------------------------------
+
+test("drawio_create: lints, builds the .drawio.svg, uploads and inserts a node", async () => {
+  const pageDoc = {
+    type: "doc",
+    content: [{ type: "paragraph", attrs: { id: "p1" }, content: [] }],
+  };
+  const { client, calls } = makeClient({ pageDoc });
+  const res = await client.drawioCreate("page1", { position: "append" }, MODEL, "My diagram");
+
+  assert.equal(res.success, true);
+  // The returned handle is an index-based "#<index>" ref (drawio nodes carry no
+  // persisted attrs.id), addressing the appended top-level block (index 1, after
+  // the existing paragraph).
+  assert.equal(res.nodeId, "#1");
+  assert.equal(res.attachmentId, "att-1");
+  assert.equal(calls.uploads.length, 1);
+  assert.equal(calls.uploads[0].fileName, "diagram.drawio.svg");
+  assert.equal(calls.uploads[0].mime, "image/svg+xml");
+  // The uploaded SVG carries the model back (round-trips through the decode chain).
+  assert.equal(decodeDrawioSvg(calls.uploads[0].svg), normalizeXml(MODEL));
+
+  // A drawio node was appended with src/attachmentId/dimensions and the title.
+  const drawios = findDrawio(calls.mutations[0].doc);
+  assert.equal(drawios.length, 1);
+  const n = drawios[0];
+  // No `id` attribute is set/persisted on the node (schema has none).
+  assert.equal(n.attrs.id, undefined);
+  assert.equal(n.attrs.attachmentId, "att-1");
+  assert.match(n.attrs.src, /^\/api\/files\/att-1\//);
+  assert.ok(n.attrs.width > 0 && n.attrs.height > 0);
+  assert.equal(n.attrs.title, "My diagram");
+});
+
+test("drawio_create: a lint violation throws before any upload", async () => {
+  const { client, calls } = makeClient({ pageDoc: { type: "doc", content: [] } });
+  // Edge with no child geometry -> edge-geometry rule.
+  const bad =
+    '<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"/></mxCell>' +
+    '<mxCell id="3" edge="1" parent="1" source="2" target="2"/></root></mxGraphModel>';
+  await assert.rejects(
+    () => client.drawioCreate("page1", { position: "append" }, bad, undefined),
+    /edge-geometry/,
+  );
+  assert.equal(calls.uploads.length, 0, "no attachment uploaded on lint failure");
+});
+
+test("drawio_create: before/after requires exactly one anchor", async () => {
+  const { client } = makeClient({ pageDoc: { type: "doc", content: [] } });
+  await assert.rejects(
+    () => client.drawioCreate("page1", { position: "before" }, MODEL),
+    /exactly one of anchorNodeId or anchorText/,
+  );
+});
+
+// --- drawio_get ------------------------------------------------------------
+
+test("drawio_get: decodes the model and returns meta with a hash", async () => {
+  const pageDoc = {
+    type: "doc",
+    content: [
+      {
+        type: "drawio",
+        attrs: {
+          id: "d1",
+          src: "/api/files/att-1/diagram.drawio.svg",
+          attachmentId: "att-1",
+          title: "T",
+          width: 200,
+          height: 120,
+        },
+      },
+    ],
+  };
+  const { client } = makeClient({ pageDoc, attachmentSvg: svgFor(MODEL) });
+  const res = await client.drawioGet("page1", "d1", "xml");
+  assert.equal(res.content, normalizeXml(MODEL));
+  assert.equal(res.meta.attachmentId, "att-1");
+  assert.equal(res.meta.title, "T");
+  assert.equal(res.meta.cellCount, 1);
+  assert.equal(res.meta.hash, mxHash(normalizeXml(MODEL)));
+});
+
+test("drawio_get: format=svg returns the raw .drawio.svg", async () => {
+  const svg = svgFor(MODEL);
+  const pageDoc = {
+    type: "doc",
+    content: [
+      { type: "drawio", attrs: { id: "d1", src: "/api/files/att-1/x.svg", attachmentId: "att-1" } },
+    ],
+  };
+  const { client } = makeClient({ pageDoc, attachmentSvg: svg });
+  const res = await client.drawioGet("page1", "d1", "svg");
+  assert.equal(res.content, svg);
+});
+
+test("drawio_get: reads a HUMAN-saved compressed diagram losslessly (pako)", async () => {
+  const pageDoc = {
+    type: "doc",
+    content: [
+      { type: "drawio", attrs: { id: "d1", src: "/api/files/att-1/x.svg", attachmentId: "att-1" } },
+    ],
+  };
+  const { client } = makeClient({ pageDoc, attachmentSvg: compressedSvgFor(MODEL) });
+  const res = await client.drawioGet("page1", "d1", "xml");
+  assert.equal(res.content, normalizeXml(MODEL));
+});
+
+// --- drawio_update ---------------------------------------------------------
+
+const UPDATED_MODEL =
+  '<mxGraphModel><root>' +
+  '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+  '<mxCell id="2" value="Changed" style="rounded=1;" vertex="1" parent="1">' +
+  '<mxGeometry x="20" y="20" width="300" height="200" as="geometry"/></mxCell>' +
+  '</root></mxGraphModel>';
+
+function updatePageDoc() {
+  return {
+    type: "doc",
+    content: [
+      {
+        type: "drawio",
+        attrs: {
+          id: "d1",
+          src: "/api/files/att-1/diagram.drawio.svg",
+          attachmentId: "att-1",
+          width: 200,
+          height: 120,
+        },
+      },
+    ],
+  };
+}
+
+test("drawio_update: stale baseHash -> conflict, no upload", async () => {
+  const { client, calls } = makeClient({
+    pageDoc: updatePageDoc(),
+    attachmentSvg: svgFor(MODEL),
+  });
+  await assert.rejects(
+    () => client.drawioUpdate("page1", "d1", UPDATED_MODEL, "deadbeef-stale"),
+    /conflict/,
+  );
+  assert.equal(calls.uploads.length, 0, "no upload on conflict");
+});
+
+test("drawio_update: current baseHash -> uploads new attachment and repoints node dims", async () => {
+  const currentHash = mxHash(normalizeXml(MODEL));
+  const { client, calls } = makeClient({
+    pageDoc: updatePageDoc(),
+    attachmentSvg: svgFor(MODEL),
+  });
+  const res = await client.drawioUpdate("page1", "d1", UPDATED_MODEL, currentHash);
+  assert.equal(res.success, true);
+  assert.equal(res.attachmentId, "att-1"); // fresh id from the stub sequence
+  assert.equal(calls.uploads.length, 1);
+  // The uploaded SVG carries the NEW model.
+  assert.equal(decodeDrawioSvg(calls.uploads[0].svg), normalizeXml(UPDATED_MODEL));
+  // The node was repointed with the new bounding-box dimensions:
+  // vertex maxX=320,maxY=220 + the 20px preview margin -> 340 x 240.
+  const n = findDrawio(calls.mutations[0].doc)[0];
+  assert.equal(n.attrs.attachmentId, "att-1");
+  assert.equal(n.attrs.width, 340);
+  assert.equal(n.attrs.height, 240);
+  // The block `id` used as the legacy resolution handle is dropped on save
+  // (schema declares no `id`); the update still targeted the correct node.
+  assert.equal(n.attrs.id, undefined);
+});
+
+test("drawio_update: baseHash is mandatory", async () => {
+  const { client } = makeClient({ pageDoc: updatePageDoc(), attachmentSvg: svgFor(MODEL) });
+  await assert.rejects(
+    () => client.drawioUpdate("page1", "d1", UPDATED_MODEL, ""),
+    /baseHash is mandatory/,
+  );
+});
+
+// --- Fix 1: the create handle must resolve on the SAVED doc (no id) ---------
+
+test("drawio_create -> get/update: returned #<index> handle resolves on the saved doc (id dropped)", async () => {
+  // Create appends a drawio node after the existing paragraph.
+  const createDoc = {
+    type: "doc",
+    content: [{ type: "paragraph", attrs: { id: "p1" }, content: [] }],
+  };
+  const create = makeClient({ pageDoc: createDoc });
+  const res = await create.client.drawioCreate(
+    "page1",
+    { position: "append" },
+    MODEL,
+    "T",
+  );
+  // The handle is index-based, not a block id.
+  assert.equal(res.nodeId, "#1");
+
+  // Take the document EXACTLY as it was saved: the schema drop stripped the
+  // node's id, so no id-based handle could ever resolve against it.
+  const savedDoc = create.calls.mutations[0].doc;
+  assert.equal(findDrawio(savedDoc)[0].attrs.id, undefined);
+
+  // drawio_get with the returned handle resolves the just-created node.
+  const getClient = makeClient({ pageDoc: savedDoc, attachmentSvg: svgFor(MODEL) });
+  const got = await getClient.client.drawioGet("page1", res.nodeId, "xml");
+  assert.equal(got.nodeId, res.nodeId);
+  assert.equal(got.content, normalizeXml(MODEL));
+
+  // drawio_update with the same handle + the hash from get repoints that node.
+  const upClient = makeClient({ pageDoc: savedDoc, attachmentSvg: svgFor(MODEL) });
+  const upd = await upClient.client.drawioUpdate(
+    "page1",
+    res.nodeId,
+    UPDATED_MODEL,
+    got.meta.hash,
+  );
+  assert.equal(upd.success, true);
+  assert.equal(upd.nodeId, res.nodeId);
+  const updated = findDrawio(upClient.calls.mutations[0].doc)[0];
+  assert.equal(
+    decodeDrawioSvg(upClient.calls.uploads[0].svg),
+    normalizeXml(UPDATED_MODEL),
+  );
+  assert.equal(updated.attrs.width, 340);
+});
+
+// --- error paths: the LLM must get a clean error, not a crash --------------
+
+test("drawio_get: a bad node ref -> clean 'no node found' error", async () => {
+  // Page has one paragraph; the requested ref resolves to nothing.
+  const pageDoc = {
+    type: "doc",
+    content: [{ type: "paragraph", attrs: { id: "p1" }, content: [] }],
+  };
+  const { client } = makeClient({ pageDoc, attachmentSvg: svgFor(MODEL) });
+  await assert.rejects(
+    () => client.drawioGet("page1", "does-not-exist", "xml"),
+    /no node found for "does-not-exist"/,
+  );
+});
+
+test("drawio_get: a drawio node with no src -> clean 'has no src to read' error", async () => {
+  const pageDoc = {
+    type: "doc",
+    content: [
+      // A drawio node that carries no `src` (e.g. a half-written node).
+      { type: "drawio", attrs: { id: "d1", attachmentId: "att-1" } },
+    ],
+  };
+  const { client } = makeClient({ pageDoc, attachmentSvg: svgFor(MODEL) });
+  await assert.rejects(
+    () => client.drawioGet("page1", "d1", "xml"),
+    /node "d1" on page page1 has no src to read/,
+  );
+});
+
+test("drawio_update: the resolved node is NOT a drawio node -> clean error, no upload", async () => {
+  // "#0" resolves to a paragraph. The update must refuse cleanly rather than
+  // crash or repoint the wrong node.
+  const pageDoc = {
+    type: "doc",
+    content: [{ type: "paragraph", attrs: { id: "p1" }, content: [] }],
+  };
+  const { client, calls } = makeClient({ pageDoc, attachmentSvg: svgFor(MODEL) });
+  await assert.rejects(
+    () => client.drawioUpdate("page1", "#0", UPDATED_MODEL, "any-nonempty-hash"),
+    /node "#0" on page page1 is a paragraph, not a drawio diagram/,
+  );
+  assert.equal(calls.uploads.length, 0, "no upload when the node is not a diagram");
+  assert.equal(calls.mutations.length, 0, "no write when the node is not a diagram");
+});
+
+test("drawio_create: anchor not found -> clean error that reports the orphan attachment", async () => {
+  // The upload happens before the mutate transform; when the anchor cannot be
+  // found the write is skipped and the (now unreferenced) attachment is named
+  // in the error, exactly as the code documents.
+  const pageDoc = {
+    type: "doc",
+    content: [{ type: "paragraph", attrs: { id: "p1" }, content: [] }],
+  };
+  const { client, calls } = makeClient({ pageDoc });
+  await assert.rejects(
+    () =>
+      client.drawioCreate(
+        "page1",
+        { position: "after", anchorNodeId: "nope" },
+        MODEL,
+        "T",
+      ),
+    (err) =>
+      /anchor not found/.test(err.message) &&
+      /unreferenced orphan/.test(err.message) &&
+      /att-1/.test(err.message),
+  );
+  // The orphan was uploaded (and reported), but no node was written.
+  assert.equal(calls.uploads.length, 1, "attachment uploaded before the failed insert");
+  const drawios = calls.mutations.length ? findDrawio(calls.mutations[0].doc) : [];
+  assert.equal(drawios.length, 0, "no drawio node written when the anchor is missing");
+});
+
+// --- Fix 2: update targets ONLY the resolved node --------------------------
+
+test("drawio_update: repoints ONLY the addressed node, not siblings sharing an attachmentId", async () => {
+  // A copied diagram: two drawio nodes share one attachmentId. Updating via the
+  // "#0" handle must touch node #0 only, never the sibling copy.
+  const shared = {
+    type: "doc",
+    content: [
+      {
+        type: "drawio",
+        attrs: {
+          src: "/api/files/shared/x.svg",
+          attachmentId: "shared",
+          width: 200,
+          height: 120,
+        },
+      },
+      {
+        type: "drawio",
+        attrs: {
+          src: "/api/files/shared/x.svg",
+          attachmentId: "shared",
+          width: 200,
+          height: 120,
+        },
+      },
+    ],
+  };
+  const { client, calls } = makeClient({
+    pageDoc: shared,
+    attachmentSvg: svgFor(MODEL),
+  });
+  const res = await client.drawioUpdate(
+    "page1",
+    "#0",
+    UPDATED_MODEL,
+    mxHash(normalizeXml(MODEL)),
+  );
+  assert.equal(res.success, true);
+
+  const drawios = findDrawio(calls.mutations[0].doc);
+  assert.equal(drawios.length, 2);
+  // Node #0 repointed to the NEW attachment ("att-1" from the stub) and dims.
+  assert.equal(drawios[0].attrs.attachmentId, "att-1");
+  assert.equal(drawios[0].attrs.width, 340);
+  assert.match(drawios[0].attrs.src, /^\/api\/files\/att-1\//);
+  // Node #1 (the sibling copy) is untouched despite sharing the old attachmentId.
+  assert.equal(drawios[1].attrs.attachmentId, "shared");
+  assert.equal(drawios[1].attrs.width, 200);
+  assert.equal(drawios[1].attrs.src, "/api/files/shared/x.svg");
+});

packages/mcp/test/mock/resolve-page-id-collab-doc-name.test.mjs

@@ -19,7 +19,6 @@ import { WebSocketServer } from "ws";
 import { Hocuspocus } from "@hocuspocus/server";
 import { DocmostClient } from "../../build/client.js";
 import { buildYDoc } from "../../build/lib/collaboration.js";
-import { destroyAllSessions } from "../../build/lib/collab-session.js";
 // Import the SAME page-lock module instance that build/client.js imports. ESM
 // caches modules by resolved URL, so this `withPageLock` shares the very
 // per-page mutex map (`chains`) the client uses — letting the replaceImage test
@@ -189,10 +188,6 @@ async function spawnCollabStack(opts = {}) {
 
 const openStacks = [];
 after(async () => {
-  // #400: tests now leave a cached live CollabSession per page. Destroy them
-  // first (closes the client ws) so the server.close() below is not racing an
-  // open collab connection.
-  destroyAllSessions();
   await Promise.all(
     openStacks.map(
       ({ server, hocuspocus }) =>
@@ -275,23 +270,17 @@ test("a UUID input is passed through unchanged and triggers NO /pages/info fetch
   );
 });
 
-test("repeated slugId edits reuse ONE live collab session and resolve the UUID only once (#400 cache)", async () => {
+test("a repeated slugId edit resolves the UUID only once (cache)", async () => {
   const { state, baseURL } = await spawnCollabStack();
   const client = new DocmostClient(baseURL, "user@example.com", "pw");
 
-  // #400: a series of edits on the same page reuses ONE live CollabSession, so
-  // the connect/handshake happens once and the collab doc is OPENED a single
-  // time (not per edit). The live ydoc persists between edits (the whole point),
-  // so the second edit sees the first edit's result: after "hello" -> "hi world"
-  // it targets the still-present "world".
+  // Each mock connection re-seeds a fresh "hello world" doc (the mock does not
+  // persist across connects), so both edits target "hello". The cache assertion
+  // only concerns the slugId->uuid resolution, not the document content.
   await client.editPageText(SLUG, [{ find: "hello", replace: "hi" }]);
-  await client.editPageText(SLUG, [{ find: "world", replace: "planet" }]);
+  await client.editPageText(SLUG, [{ find: "hello", replace: "hey" }]);
 
-  assert.deepEqual(
-    state.docNames,
-    [`page.${UUID}`],
-    "the two edits must reuse one live collab session -> a single collab-doc open (#400)",
-  );
+  assert.deepEqual(state.docNames, [`page.${UUID}`, `page.${UUID}`]);
   assert.equal(
     state.pagesInfoCalls.length,
     1,
@@ -336,9 +325,8 @@ test("replaceImage opens by the resolved UUID AND keys its page lock by that UUI
   await uploadStarted; // deterministic: replaceImage now holds its page lock.
 
   // (a) OPEN BY UUID: the only collab doc opened so far (the scan pass) used the
-  // canonical UUID, never the slugId. (#400: the write pass will REUSE this same
-  // live session rather than reopen, so docNames stays a single entry — asserted
-  // at the end.)
+  // canonical UUID, never the slugId. (The write pass opens a second time after
+  // we release the gate; asserted at the end.)
   assert.deepEqual(
     state.docNames,
     [`page.${UUID}`],
@@ -390,9 +378,8 @@ test("replaceImage opens by the resolved UUID AND keys its page lock by that UUI
 
   assert.equal(res.success, true);
   assert.equal(res.replaced, 1, "the one seeded image must be repointed");
-  // #400: the write pass REUSES the scan pass's live session, so the collab doc
-  // is opened ONCE across both passes (never reopened, never by the slugId).
-  assert.deepEqual(state.docNames, [`page.${UUID}`]);
+  // Both opens (scan pass + write pass) used the UUID; the slugId never appears.
+  assert.deepEqual(state.docNames, [`page.${UUID}`, `page.${UUID}`]);
   assert.ok(
     !state.docNames.includes(`page.${SLUG}`),
     "replaceImage must NEVER open the collab doc by the slugId (the #260 bug)",

packages/mcp/test/unit/collab-session.test.mjs

@@ -1,348 +0,0 @@
-import { test, beforeEach, afterEach, mock } from "node:test";
-import assert from "node:assert/strict";
-import { EventEmitter } from "node:events";
-
-import {
-  acquireCollabSession,
-  destroyAllSessions,
-  __setCollabProviderFactory,
-  __sessionCountForTests,
-} from "../../build/lib/collab-session.js";
-import { withPageLock } from "../../build/lib/page-lock.js";
-
-// A stand-in for HocuspocusProvider: it shares the ydoc (so the real yjs
-// read/transform/write in CollabSession.mutate runs unchanged), auto-completes
-// the connect+sync handshake on a microtask (unaffected by mock timers), and
-// exposes hooks to drive disconnect/close/auth-failure and the unsyncedChanges
-// ack. There is no collaboration server in the test env, so every test drives
-// the provider through this fake.
-class FakeProvider extends EventEmitter {
-  static instances = [];
-  static connectCount = 0;
-
-  static reset() {
-    FakeProvider.instances = [];
-    FakeProvider.connectCount = 0;
-  }
-
-  static last() {
-    return FakeProvider.instances[FakeProvider.instances.length - 1];
-  }
-
-  constructor(config, opts = {}) {
-    super();
-    this.config = config;
-    this.ydoc = config.document;
-    this.synced = false;
-    this.unsyncedChanges = opts.unsynced ?? 0;
-    this.destroyed = false;
-    FakeProvider.instances.push(this);
-    FakeProvider.connectCount += 1;
-    if (opts.autoSync !== false) {
-      // Real HocuspocusProvider fires onSynced asynchronously after the
-      // handshake; a microtask reproduces that without depending on timers.
-      queueMicrotask(() => {
-        if (this.destroyed) return;
-        this.config.onConnect?.();
-        this.synced = true;
-        this.config.onSynced?.();
-      });
-    }
-  }
-
-  destroy() {
-    this.destroyed = true;
-  }
-
-  // --- test drivers ---
-  _disconnect() {
-    this.config.onDisconnect?.();
-  }
-  _close() {
-    this.config.onClose?.();
-  }
-  _authFail() {
-    this.config.onAuthenticationFailed?.();
-  }
-  _ack() {
-    this.unsyncedChanges = 0;
-    this.emit("unsyncedChanges", { number: 0 });
-  }
-}
-
-/** Build a provider factory that stamps every provider with the given opts. */
-function factory(opts = {}) {
-  return (config) => new FakeProvider(config, opts);
-}
-
-/** A minimal, schema-valid ProseMirror doc for a write. */
-function docWith(text) {
-  return {
-    type: "doc",
-    content: [
-      { type: "paragraph", content: [{ type: "text", text: String(text) }] },
-    ],
-  };
-}
-
-const ENV_KEYS = [
-  "MCP_COLLAB_SESSION_IDLE_MS",
-  "MCP_COLLAB_SESSION_MAX_AGE_MS",
-  "MCP_COLLAB_SESSION_MAX_ENTRIES",
-];
-let savedEnv;
-
-beforeEach(() => {
-  savedEnv = {};
-  for (const k of ENV_KEYS) savedEnv[k] = process.env[k];
-  FakeProvider.reset();
-  __setCollabProviderFactory(factory());
-});
-
-afterEach(() => {
-  destroyAllSessions();
-  __setCollabProviderFactory(null);
-  mock.timers.reset();
-  for (const k of ENV_KEYS) {
-    if (savedEnv[k] === undefined) delete process.env[k];
-    else process.env[k] = savedEnv[k];
-  }
-});
-
-test("acquire opens one provider; reuse returns the SAME live session", async () => {
-  const a = await acquireCollabSession("page-1", "tok", "http://h/api");
-  const b = await acquireCollabSession("page-1", "tok", "http://h/api");
-  assert.equal(a, b, "same (wsUrl,page,token) must reuse the session");
-  assert.equal(FakeProvider.connectCount, 1, "exactly one connect/sync");
-  assert.equal(__sessionCountForTests(), 1);
-});
-
-test("N mutates on one page open the provider ONCE (coalesced series)", async () => {
-  const session = await acquireCollabSession("page-1", "tok", "http://h/api");
-  for (let i = 0; i < 20; i++) {
-    const r = await session.mutate(() => docWith(`edit ${i}`));
-    assert.ok(r && r.doc, "each mutate resolves a MutationResult");
-  }
-  assert.equal(
-    FakeProvider.connectCount,
-    1,
-    "20 mutates must not reconnect — one live provider for the whole series",
-  );
-});
-
-test("mutate preserves the transform-abort no-op report (null transform)", async () => {
-  const session = await acquireCollabSession("page-1", "tok", "http://h/api");
-  const r = await session.mutate(() => null);
-  assert.equal(r.verify.changed, false);
-  assert.equal(r.verify.summary, "no changes (transform aborted)");
-});
-
-test("registry key includes the token: different tokens => different sessions", async () => {
-  const a = await acquireCollabSession("page-1", "tok-A", "http://h/api");
-  const b = await acquireCollabSession("page-1", "tok-B", "http://h/api");
-  assert.notEqual(a, b, "different tokens must never share a session");
-  assert.equal(FakeProvider.connectCount, 2);
-  assert.equal(__sessionCountForTests(), 2);
-  // Same token reuses.
-  const a2 = await acquireCollabSession("page-1", "tok-A", "http://h/api");
-  assert.equal(a, a2);
-  assert.equal(FakeProvider.connectCount, 2);
-});
-
-test("disconnect at any time kills the session and removes it from the registry", async () => {
-  const a = await acquireCollabSession("page-1", "tok", "http://h/api");
-  assert.equal(__sessionCountForTests(), 1);
-  FakeProvider.last()._disconnect();
-  assert.equal(__sessionCountForTests(), 0, "dead session is deregistered");
-  // Re-acquire opens a brand new provider.
-  const b = await acquireCollabSession("page-1", "tok", "http://h/api");
-  assert.notEqual(a, b);
-  assert.equal(FakeProvider.connectCount, 2);
-});
-
-test("an in-flight mutate rejects with the connection-closed text on disconnect", async () => {
-  __setCollabProviderFactory(factory({ unsynced: 1 })); // stay pending after write
-  const session = await acquireCollabSession("page-1", "tok", "http://h/api");
-  const p = session.mutate(() => docWith("x"));
-  // The write happened synchronously; persistence is pending. Now the socket drops.
-  FakeProvider.last()._disconnect();
-  await assert.rejects(
-    p,
-    /Collaboration connection closed before the update was persisted\/synced/,
-  );
-});
-
-test("auth failure rejects the pending open with the auth error text", async () => {
-  __setCollabProviderFactory(factory({ autoSync: false }));
-  const p = acquireCollabSession("page-1", "tok", "http://h/api");
-  // Let the provider be constructed, then fail auth.
-  await Promise.resolve();
-  FakeProvider.last()._authFail();
-  await assert.rejects(
-    p,
-    /Authentication failed for collaboration connection/,
-  );
-  assert.equal(__sessionCountForTests(), 0);
-});
-
-test("mutate-error invalidates the session (caller destroys; re-acquire is fresh)", async () => {
-  const session = await acquireCollabSession("page-1", "tok", "http://h/api");
-  await assert.rejects(
-    session.mutate(() => {
-      throw new Error("afterText not found");
-    }),
-    /afterText not found/,
-  );
-  // The production caller destroys on failure; mirror that here.
-  session.destroy("mutate failed");
-  assert.equal(__sessionCountForTests(), 0);
-  const fresh = await acquireCollabSession("page-1", "tok", "http://h/api");
-  assert.notEqual(fresh, session);
-  assert.equal(FakeProvider.connectCount, 2);
-});
-
-test("a pending write resolves when the server acks (unsyncedChanges -> 0)", async () => {
-  __setCollabProviderFactory(factory({ unsynced: 1 }));
-  const session = await acquireCollabSession("page-1", "tok", "http://h/api");
-  const p = session.mutate(() => docWith("y"));
-  FakeProvider.last()._ack();
-  const r = await p;
-  assert.ok(r.doc);
-});
-
-test("concurrent mutate on one session: the second rejects, the first is unaffected", async () => {
-  __setCollabProviderFactory(factory({ unsynced: 1 })); // first stays pending after write
-  const session = await acquireCollabSession("page-1", "tok", "http://h/api");
-  // First mutate: write happens synchronously, persistence ack still pending.
-  const first = session.mutate(() => docWith("first"));
-  // Second overlapping mutate on the SAME session must fail fast.
-  await assert.rejects(
-    session.mutate(() => docWith("second")),
-    /mutate already in-flight; caller must serialize \(hold the page lock\)/,
-  );
-  // The first op is untouched: its rejector was not clobbered. Ack it now.
-  FakeProvider.last()._ack();
-  const r = await first;
-  assert.ok(r.doc, "the first in-flight mutate still resolves on its own ack");
-  assert.equal(FakeProvider.connectCount, 1, "no reconnect from the rejected overlap");
-});
-
-test("sequential mutates on one session both succeed (the guard doesn't break serialized use)", async () => {
-  const session = await acquireCollabSession("page-1", "tok", "http://h/api");
-  // Await the first fully, then run the second: inflightReject was cleared by
-  // localFinish before the first settled, so the guard is clear for the second.
-  const r1 = await session.mutate(() => docWith("one"));
-  assert.ok(r1.doc, "first sequential mutate resolves");
-  const r2 = await session.mutate(() => docWith("two"));
-  assert.ok(r2.doc, "second sequential mutate resolves — guard not tripped");
-  assert.equal(FakeProvider.connectCount, 1, "sequential mutates reuse one provider");
-});
-
-test("connect timeout rejects with the connect-timeout text and fires the metric hook", async () => {
-  mock.timers.enable({ apis: ["setTimeout"] });
-  __setCollabProviderFactory(factory({ autoSync: false }));
-  let metricFired = 0;
-  const p = acquireCollabSession("page-1", "tok", "http://h/api", {
-    onConnectTimeout: () => {
-      metricFired += 1;
-    },
-  });
-  mock.timers.tick(25000);
-  await assert.rejects(p, /Connection timeout to collaboration server/);
-  assert.equal(metricFired, 1);
-  assert.equal(__sessionCountForTests(), 0);
-});
-
-test("idle TTL destroys the session; re-acquire reconnects", async () => {
-  mock.timers.enable({ apis: ["setTimeout"] });
-  process.env.MCP_COLLAB_SESSION_IDLE_MS = "1000";
-  const a = await acquireCollabSession("page-1", "tok", "http://h/api");
-  assert.equal(__sessionCountForTests(), 1);
-  mock.timers.tick(1000); // idle fires
-  assert.equal(__sessionCountForTests(), 0, "idle timeout destroyed it");
-  const b = await acquireCollabSession("page-1", "tok", "http://h/api");
-  assert.notEqual(a, b);
-  assert.equal(FakeProvider.connectCount, 2);
-});
-
-test("max age is enforced at acquire (destroy + open fresh), idle held off", async () => {
-  mock.timers.enable({ apis: ["setTimeout", "Date"] });
-  process.env.MCP_COLLAB_SESSION_MAX_AGE_MS = "1000";
-  process.env.MCP_COLLAB_SESSION_IDLE_MS = "10000000"; // never fires in this test
-  const a = await acquireCollabSession("page-1", "tok", "http://h/api");
-  mock.timers.tick(2000); // past max age, below idle
-  const b = await acquireCollabSession("page-1", "tok", "http://h/api");
-  assert.notEqual(a, b, "a session past its max age must be replaced");
-  assert.equal(FakeProvider.connectCount, 2);
-});
-
-test("registry cap: least-recently-used session is destroy-evicted", async () => {
-  process.env.MCP_COLLAB_SESSION_MAX_ENTRIES = "2";
-  const s1 = await acquireCollabSession("page-1", "tok", "http://h/api");
-  const p1prov = FakeProvider.last();
-  const s2 = await acquireCollabSession("page-2", "tok", "http://h/api");
-  const p2prov = FakeProvider.last();
-  assert.equal(__sessionCountForTests(), 2);
-
-  // Touch page-1 so page-2 becomes the least-recently-used entry.
-  assert.equal(await acquireCollabSession("page-1", "tok", "http://h/api"), s1);
-  assert.equal(FakeProvider.connectCount, 2, "reuse must not reconnect");
-
-  // A third distinct page (cap 2) must evict the LRU (page-2), not page-1.
-  const s3 = await acquireCollabSession("page-3", "tok", "http://h/api");
-  assert.equal(__sessionCountForTests(), 2);
-  assert.equal(FakeProvider.connectCount, 3);
-  assert.equal(p2prov.destroyed, true, "the LRU provider was destroy-evicted");
-  assert.equal(p1prov.destroyed, false, "the MRU page-1 survived");
-
-  // page-1 still reused (survived), page-3 still reused.
-  assert.equal(await acquireCollabSession("page-1", "tok", "http://h/api"), s1);
-  assert.equal(await acquireCollabSession("page-3", "tok", "http://h/api"), s3);
-  assert.equal(FakeProvider.connectCount, 3, "no extra reconnects");
-});
-
-test("MCP_COLLAB_SESSION_IDLE_MS=0 disables the cache (legacy provider-per-op)", async () => {
-  process.env.MCP_COLLAB_SESSION_IDLE_MS = "0";
-  const s1 = await acquireCollabSession("page-1", "tok", "http://h/api");
-  // Never registered (ephemeral).
-  assert.equal(__sessionCountForTests(), 0);
-  const r1 = await s1.mutate(() => docWith("a"));
-  assert.ok(r1.doc);
-  // After its single op the ephemeral session self-destroyed.
-  assert.equal(FakeProvider.instances[0].destroyed, true);
-  // A second op opens a BRAND NEW provider (no reuse).
-  const s2 = await acquireCollabSession("page-1", "tok", "http://h/api");
-  assert.notEqual(s1, s2);
-  assert.equal(FakeProvider.connectCount, 2);
-});
-
-test("replaceImage-shaped flow: acquire under an EXTERNAL page lock does not deadlock and reuses one session", async () => {
-  const pageId = "page-lock";
-  // Mirror replaceImage: hold ONE withPageLock across scan (read-only) + write,
-  // each going through the non-locking acquireCollabSession.
-  const result = await withPageLock(pageId, async () => {
-    // pass 1: read-only scan (transform returns null -> no write)
-    const scan = await acquireCollabSession(pageId, "tok", "http://h/api");
-    const scanRes = await scan.mutate(() => null);
-    assert.equal(scanRes.verify.changed, false);
-    // pass 2: the actual write, same held lock
-    const write = await acquireCollabSession(pageId, "tok", "http://h/api");
-    return write.mutate(() => docWith("repointed"));
-  });
-  assert.ok(result.doc, "the locked scan+write completed without deadlock");
-  assert.equal(
-    FakeProvider.connectCount,
-    1,
-    "both passes under the held lock reuse ONE live session",
-  );
-});
-
-test("destroyAllSessions tears down every cached session", async () => {
-  await acquireCollabSession("page-1", "tok", "http://h/api");
-  await acquireCollabSession("page-2", "tok", "http://h/api");
-  assert.equal(__sessionCountForTests(), 2);
-  const provs = [...FakeProvider.instances];
-  destroyAllSessions();
-  assert.equal(__sessionCountForTests(), 0);
-  assert.ok(provs.every((p) => p.destroyed), "all providers destroyed");
-});

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

@@ -0,0 +1,84 @@
+// Unit tests for the pure-TS schematic SVG preview (issue #423). Asserts that
+// the preview emits well-formed SVG covering each primitive (rect/ellipse/
+// rhombus/edge), resolves container-relative coordinates to absolute, and that
+// the full `.drawio.svg` wrapper parses.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { JSDOM } from "jsdom";
+import { renderDiagramShapes } from "../../build/lib/drawio-preview.js";
+import {
+  parseCells,
+  computeBBox,
+  buildDrawioSvg,
+  normalizeXml,
+} from "../../build/lib/drawio-xml.js";
+
+const { window } = new JSDOM("");
+function parseSvg(svg) {
+  const doc = new window.DOMParser().parseFromString(svg, "application/xml");
+  const err = doc.getElementsByTagName("parsererror");
+  assert.equal(err.length, 0, `SVG did not parse: ${err[0]?.textContent}`);
+  return doc;
+}
+
+const MODEL =
+  '<mxGraphModel><root>' +
+  '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+  '<mxCell id="2" value="Box &amp; Co" style="rounded=1;fillColor=#dae8fc;strokeColor=#6c8ebf;" vertex="1" parent="1">' +
+  '<mxGeometry x="40" y="40" width="120" height="60" as="geometry"/></mxCell>' +
+  '<mxCell id="3" value="Circle" style="ellipse;fillColor=#d5e8d4;" vertex="1" parent="1">' +
+  '<mxGeometry x="240" y="40" width="80" height="80" as="geometry"/></mxCell>' +
+  '<mxCell id="5" value="Dec" style="rhombus;" vertex="1" parent="1">' +
+  '<mxGeometry x="40" y="160" width="100" height="60" as="geometry"/></mxCell>' +
+  '<mxCell id="4" value="link" edge="1" parent="1" source="2" target="3"><mxGeometry relative="1" as="geometry"/></mxCell>' +
+  '</root></mxGraphModel>';
+
+test("renderDiagramShapes emits rect, ellipse, polygon and a line", () => {
+  const cells = parseCells(MODEL);
+  const inner = renderDiagramShapes(cells, computeBBox(cells));
+  assert.ok(inner.includes("<rect"), "has a rect");
+  assert.ok(inner.includes("<ellipse"), "has an ellipse");
+  assert.ok(inner.includes("<polygon"), "has a polygon (rhombus)");
+  assert.ok(inner.includes("<line"), "has an edge line");
+  // Labels are HTML-escaped.
+  assert.ok(inner.includes("Box &amp; Co"));
+});
+
+test("the full .drawio.svg wrapper parses as valid XML", () => {
+  const cells = parseCells(MODEL);
+  const bbox = computeBBox(cells);
+  const inner = renderDiagramShapes(cells, bbox);
+  const svg = buildDrawioSvg(normalizeXml(MODEL), inner, bbox);
+  const doc = parseSvg(svg);
+  assert.equal(doc.documentElement.tagName, "svg");
+  assert.ok(doc.documentElement.getAttribute("content"), "carries content=");
+  // The visible children exist.
+  assert.ok(doc.getElementsByTagName("rect").length >= 1);
+});
+
+test("container children resolve to absolute coordinates", () => {
+  // A group at (100,100) with a child rect at relative (10,10,20,20) -> abs 110,110.
+  const model =
+    '<mxGraphModel><root>' +
+    '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="g" style="group;" vertex="1" parent="1"><mxGeometry x="100" y="100" width="200" height="200" as="geometry"/></mxCell>' +
+    '<mxCell id="c" value="in" vertex="1" parent="g"><mxGeometry x="10" y="10" width="20" height="20" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  const cells = parseCells(model);
+  const inner = renderDiagramShapes(cells, computeBBox(cells));
+  // The child rect must be placed at absolute x=110,y=110.
+  assert.ok(/<rect x="110" y="110"/.test(inner), `expected abs child rect, got: ${inner}`);
+});
+
+test("unknown stencil (shape=mxgraph.*) degrades to a labeled rectangle", () => {
+  const model =
+    '<mxGraphModel><root>' +
+    '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="2" value="AWS" style="shape=mxgraph.aws4.lambda;" vertex="1" parent="1">' +
+    '<mxGeometry x="0" y="0" width="60" height="60" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  const cells = parseCells(model);
+  const inner = renderDiagramShapes(cells, computeBBox(cells));
+  assert.ok(inner.includes("<rect"), "unknown stencil -> rect");
+  assert.ok(inner.includes(">AWS<"), "keeps the label");
+});

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

@@ -0,0 +1,360 @@
+// Unit tests for the drawio-xml module (issue #423): the linter (a positive
+// baseline + a negative case per rule, each asserting rule + cellId), the
+// decode chain (plain nested XML AND draw.io's compressed <diagram> via pako),
+// encode/round-trip byte-stability, hash stability, style parsing and the
+// bounding box.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import pako from "pako";
+import {
+  parseStyle,
+  lintModel,
+  prepareModel,
+  normalizeInput,
+  normalizeXml,
+  mxHash,
+  computeBBox,
+  parseCells,
+  decodeDrawioSvg,
+  decodeDrawioFileToModel,
+  buildDrawioSvg,
+  encodeDrawioFile,
+  countUserCells,
+  DrawioLintError,
+  inflateDiagramPayload,
+  MAX_INFLATED_DIAGRAM_BYTES,
+} from "../../build/lib/drawio-xml.js";
+
+// A well-formed model with one vertex and a valid edge to it.
+const VALID_MODEL =
+  '<mxGraphModel><root>' +
+  '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+  '<mxCell id="2" value="Hello" style="rounded=1;whiteSpace=wrap;html=1;" vertex="1" parent="1">' +
+  '<mxGeometry x="100" y="100" width="120" height="60" as="geometry"/></mxCell>' +
+  '<mxCell id="3" value="Two" style="ellipse;" vertex="1" parent="1">' +
+  '<mxGeometry x="300" y="100" width="80" height="80" as="geometry"/></mxCell>' +
+  '<mxCell id="4" edge="1" parent="1" source="2" target="3">' +
+  '<mxGeometry relative="1" as="geometry"/></mxCell>' +
+  '</root></mxGraphModel>';
+
+function issuesOf(fn) {
+  try {
+    fn();
+    return null;
+  } catch (e) {
+    assert.ok(e instanceof DrawioLintError, `expected DrawioLintError, got ${e}`);
+    return e.issues;
+  }
+}
+const hasRule = (issues, rule, cellId) =>
+  issues.some(
+    (i) => i.rule === rule && (cellId === undefined || i.cellId === cellId),
+  );
+
+// --- style parsing ---------------------------------------------------------
+
+test("parseStyle: base stylename + key=value pairs", () => {
+  const r = parseStyle("ellipse;fillColor=#ff0000;whiteSpace=wrap;");
+  assert.equal(r.baseStyle, "ellipse");
+  assert.equal(r.map.fillColor, "#ff0000");
+  assert.equal(r.map.whiteSpace, "wrap");
+  assert.equal(r.badSegment, undefined);
+});
+
+test("parseStyle: flags a segment with two '='", () => {
+  const r = parseStyle("a=b=c;");
+  assert.equal(r.badSegment, "a=b=c");
+});
+
+test("parseStyle: a second bare token is malformed", () => {
+  const r = parseStyle("rounded=1;bareword");
+  assert.equal(r.badSegment, "bareword");
+});
+
+// --- linter: positive baseline ---------------------------------------------
+
+test("lintModel: the canonical valid model passes", () => {
+  const { cells } = lintModel(VALID_MODEL);
+  assert.equal(cells.length, 5);
+});
+
+// --- linter: one negative case per rule ------------------------------------
+
+test("rule well-formed-xml: malformed XML", () => {
+  const issues = issuesOf(() => lintModel("<mxGraphModel><root><mxCell id=\"0\"></root>"));
+  assert.ok(hasRule(issues, "well-formed-xml"));
+  assert.ok(issues[0].position, "carries a line:col position");
+});
+
+test("rule structure: root is not mxGraphModel", () => {
+  const issues = issuesOf(() => lintModel("<foo><root/></foo>"));
+  assert.ok(hasRule(issues, "structure"));
+});
+
+test("rule sentinel-cells: missing id=0 / id=1", () => {
+  const m =
+    '<mxGraphModel><root>' +
+    '<mxCell id="2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  const issues = issuesOf(() => lintModel(m));
+  assert.ok(hasRule(issues, "sentinel-cells", "0"));
+  assert.ok(hasRule(issues, "sentinel-cells", "1"));
+});
+
+test("rule duplicate-id: two cells share an id", () => {
+  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"/></mxCell>' +
+    '<mxCell id="2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  const issues = issuesOf(() => lintModel(m));
+  assert.ok(hasRule(issues, "duplicate-id", "2"));
+});
+
+test("rule vertex-edge-exclusive: cell is both vertex and edge", () => {
+  const m =
+    '<mxGraphModel><root>' +
+    '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="2" vertex="1" edge="1" parent="1"><mxGeometry as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  const issues = issuesOf(() => lintModel(m));
+  assert.ok(hasRule(issues, "vertex-edge-exclusive", "2"));
+});
+
+test("rule edge-geometry: self-closed edge without child mxGeometry", () => {
+  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"/></mxCell>' +
+    '<mxCell id="3" vertex="1" parent="1"><mxGeometry x="30" y="0" width="10" height="10" as="geometry"/></mxCell>' +
+    '<mxCell id="4" edge="1" parent="1" source="2" target="3"/>' +
+    '</root></mxGraphModel>';
+  const issues = issuesOf(() => lintModel(m));
+  assert.ok(hasRule(issues, "edge-geometry", "4"));
+});
+
+test("rule edge-endpoint: source/target does not resolve", () => {
+  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"/></mxCell>' +
+    '<mxCell id="4" edge="1" parent="1" source="2" target="99"><mxGeometry relative="1" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  const issues = issuesOf(() => lintModel(m));
+  assert.ok(hasRule(issues, "edge-endpoint", "4"));
+});
+
+test("rule parent-exists: parent points at a missing id", () => {
+  const m =
+    '<mxGraphModel><root>' +
+    '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="2" vertex="1" parent="42"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  const issues = issuesOf(() => lintModel(m));
+  assert.ok(hasRule(issues, "parent-exists", "2"));
+});
+
+test("rule no-comments: XML comment present", () => {
+  const m =
+    '<mxGraphModel><root>' +
+    '<!-- a comment --><mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '</root></mxGraphModel>';
+  const issues = issuesOf(() => lintModel(m));
+  assert.ok(hasRule(issues, "no-comments"));
+});
+
+test("rule style-format: malformed style segment (cellId reported)", () => {
+  const m =
+    '<mxGraphModel><root>' +
+    '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="2" style="rounded=1;a=b=c;" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  const issues = issuesOf(() => lintModel(m));
+  assert.ok(hasRule(issues, "style-format", "2"));
+});
+
+test("rule value-newline: literal newline in a value (cellId reported)", () => {
+  const m =
+    '<mxGraphModel><root>' +
+    '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="2" value="line1\nline2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  const issues = issuesOf(() => lintModel(m));
+  assert.ok(hasRule(issues, "value-newline", "2"));
+});
+
+test("rule value-escaping: unescaped ampersand in a value (cellId reported)", () => {
+  const m =
+    '<mxGraphModel><root>' +
+    '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="2" value="A & B" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  const issues = issuesOf(() => lintModel(m));
+  assert.ok(hasRule(issues, "value-escaping", "2"));
+});
+
+test("rule reserved id: escaped entity value passes (no false positive)", () => {
+  const m =
+    '<mxGraphModel><root>' +
+    '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="2" value="A &amp; B &lt;ok&gt;" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
+    '</root></mxGraphModel>';
+  assert.doesNotThrow(() => lintModel(m));
+});
+
+// --- input normalization ---------------------------------------------------
+
+test("normalizeInput: a list of <mxCell> is wrapped and sentinels added", () => {
+  const frag =
+    '<mxCell id="2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>';
+  const model = normalizeInput(frag);
+  assert.ok(model.startsWith("<mxGraphModel"));
+  assert.ok(model.includes('<mxCell id="0"/>'));
+  assert.ok(model.includes('<mxCell id="1" parent="0"/>'));
+  // And it lints clean.
+  assert.doesNotThrow(() => lintModel(model));
+});
+
+test("normalizeInput: an existing sentinel is not duplicated", () => {
+  const frag =
+    '<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"/></mxCell>';
+  const model = normalizeInput(frag);
+  const count0 = (model.match(/id="0"/g) || []).length;
+  assert.equal(count0, 1);
+});
+
+test("prepareModel: returns bbox, cellCount, hash and lints", () => {
+  const p = prepareModel(VALID_MODEL);
+  assert.equal(p.cellCount, 3); // 2, 3, 4 (sentinels excluded)
+  assert.ok(p.bbox.width > 0 && p.bbox.height > 0);
+  assert.equal(p.hash, mxHash(normalizeXml(VALID_MODEL)));
+});
+
+// --- decode chain: plain -----------------------------------------------------
+
+test("decode chain (plain): buildDrawioSvg -> decodeDrawioSvg round-trips byte-stable", () => {
+  const model = normalizeXml(VALID_MODEL);
+  const svg = buildDrawioSvg(model, "<g/>", { width: 400, height: 200 });
+  const decoded = decodeDrawioSvg(svg);
+  assert.equal(decoded, model);
+});
+
+test("decode chain: entity-encoded content= (draw.io export style) is read directly", () => {
+  const model = normalizeXml(VALID_MODEL);
+  const file = encodeDrawioFile(model, "Page-1");
+  const escaped = file
+    .replace(/&/g, "&amp;")
+    .replace(/</g, "&lt;")
+    .replace(/>/g, "&gt;")
+    .replace(/"/g, "&quot;");
+  const svg = `<svg xmlns="http://www.w3.org/2000/svg" content="${escaped}"></svg>`;
+  const decoded = decodeDrawioSvg(svg);
+  assert.equal(decoded, model);
+});
+
+// --- decode chain: compressed (pako) ---------------------------------------
+
+test("decode chain (compressed pako): human-saved <diagram> payload decodes losslessly", () => {
+  const model = normalizeXml(VALID_MODEL);
+  // Reproduce draw.io's compression: encodeURIComponent -> raw deflate -> base64.
+  const compressed = Buffer.from(
+    pako.deflateRaw(encodeURIComponent(model)),
+  ).toString("base64");
+  const file = `<mxfile host="Electron"><diagram id="abc" name="Page-1">${compressed}</diagram></mxfile>`;
+  // Docmost stores the file base64 in content=.
+  const contentB64 = Buffer.from(file, "utf-8").toString("base64");
+  const svg = `<svg xmlns="http://www.w3.org/2000/svg" content="${contentB64}"><image href="x"/></svg>`;
+  const decoded = decodeDrawioSvg(svg);
+  assert.equal(decoded, model);
+});
+
+test("decodeDrawioFileToModel: bare mxGraphModel file returns the model substring", () => {
+  const model = normalizeXml(VALID_MODEL);
+  assert.equal(decodeDrawioFileToModel(model), model);
+});
+
+// --- hash stability --------------------------------------------------------
+
+test("mxHash: stable across inter-tag whitespace, sensitive to content", () => {
+  const a = VALID_MODEL;
+  const b = VALID_MODEL.replace(/></g, ">\n  <"); // reformat only
+  assert.equal(mxHash(a), mxHash(b));
+  const c = VALID_MODEL.replace('value="Hello"', 'value="Changed"');
+  assert.notEqual(mxHash(a), mxHash(c));
+});
+
+// --- bounding box + cell count ---------------------------------------------
+
+test("computeBBox + countUserCells", () => {
+  const cells = parseCells(VALID_MODEL);
+  const bbox = computeBBox(cells);
+  // Vertex 3 spans to x=380,y=180; plus the 20px margin.
+  assert.equal(bbox.width, 400);
+  assert.equal(bbox.height, 200);
+  assert.equal(countUserCells(VALID_MODEL), 3);
+});
+
+// --- decompression-bomb guard (Fix 3) --------------------------------------
+
+test("inflateDiagramPayload: a small legitimate payload inflates fine", () => {
+  const xml = normalizeXml(VALID_MODEL);
+  const base64 = Buffer.from(
+    pako.deflateRaw(encodeURIComponent(xml)),
+  ).toString("base64");
+  assert.equal(inflateDiagramPayload(base64), xml);
+});
+
+test("inflateDiagramPayload: rejects an over-cap decompression bomb", () => {
+  // A tiny compressed payload that inflates to just over the cap. Highly
+  // compressible (all one byte) -> the base64 is small, but the inflated output
+  // exceeds MAX_INFLATED_DIAGRAM_BYTES and must be refused before it is fully
+  // materialised.
+  const bombSize = MAX_INFLATED_DIAGRAM_BYTES + 1024;
+  const base64 = Buffer.from(
+    pako.deflateRaw(Buffer.alloc(bombSize, 0x41 /* 'A' */)),
+  ).toString("base64");
+  assert.ok(
+    base64.length < 1024 * 1024,
+    "the compressed bomb is tiny relative to its inflated size",
+  );
+  assert.throws(
+    () => inflateDiagramPayload(base64),
+    /decompression bomb/,
+  );
+});
+
+test("encode/build: a title with < > \" & round-trips without corrupting the SVG", async () => {
+  // A user-supplied title full of XML metacharacters must be escaped so the
+  // inner <mxfile> stays well-formed and the outer content="..." attribute is
+  // never broken out of. Prove it survives the encode -> build -> decode chain.
+  const title = 'A < B > C " D & E';
+  const model = normalizeXml(VALID_MODEL);
+  const svg = buildDrawioSvg(model, "<g/>", { width: 200, height: 120 }, title);
+
+  // 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).
+  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);
+
+  // 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 nameMatch = /<diagram id="[^"]*" name="([^"]*)">/.exec(fileXml);
+  assert.ok(nameMatch, "the diagram name attribute is intact and quote-safe");
+  const decodedTitle = nameMatch[1]
+    .replace(/&lt;/g, "<")
+    .replace(/&gt;/g, ">")
+    .replace(/&quot;/g, '"')
+    .replace(/&amp;/g, "&");
+  assert.equal(decodedTitle, title);
+
+  // encodeDrawioFile alone produces the same escaped, well-formed envelope.
+  const file = encodeDrawioFile(model, title);
+  assert.match(file, /name="A &lt; B &gt; C &quot; D &amp; E">/);
+});

pnpm-lock.yaml

@@ -1041,6 +1041,9 @@ importers:
       marked:
         specifier: ^17.0.1
         version: 17.0.5
+      pako:
+        specifier: ^2.0.3
+        version: 2.0.3
       re2:
         specifier: ^1.21.0
         version: 1.25.0