feat(ai-chat): пробросить drawio_shapes/drawio_guide in-app — восстановить SHARED_TOOL_SPECS-паритет (#424)

Стадия-1 (#434) уже была довяжена in-app в develop (f46d89ea, agent_vscode)
для CRUD-тулов; два новых чистых read-only хелпера стадии-2 остались
незаброшенными → contract-parity спека падала 6 ассертами (по 3 на
drawio_shapes/drawio_guide). В отличие от CRUD-тулов это ЧИСТЫЕ функции без
сетевого вызова, поэтому НЕ client-методы:

- реэкспорт searchShapes / getGuideSection (+ тип SearchShapesOptions) из
  entry пакета @docmost/mcp; loadDocmostMcp() пробрасывает их так же, как
  sharedToolSpecs (типы SearchShapesFn/GetGuideSectionFn);
- две записи sharedTool(...) в forUser() после drawioUpdate: drawioShapes
  повторяет серверный вызов searchShapes(query,{category,limit}) и форму
  { query, count, results }; drawioGuide — getGuideSection(section)
  (omit section -> index); голый объект без jsonContent-envelope, как у
  соседних in-app хендлеров;
- DocmostClientLike и HOST_CONTRACT_METHODS-вайтлист НЕ тронуты (это не
  методы клиента);
- три тест-мока (contract/service/tool-tiers) получили type-only no-op
  заглушки под расширенный тип loadDocmostMcp() — тела инструментов в этих
  тестах не исполняются, contract-спека реально гоняет настоящий
  SHARED_TOOL_SPECS.

Внутреннее ревью обвязки: APPROVE, 0 находок. shared-tool-specs.contract:
211/211 (было 6 падений); client-host-contract drift-guard 3/0; tsc EXIT 0.

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

.gitignore

@@ -2,6 +2,11 @@
 .env.dev
 .env.prod
 data
+# Exception: the committed draw.io shape catalog (issue #424) lives in a `data/`
+# dir, but the bare `data` ignore above is meant for runtime state, not this
+# bundled build asset. Re-include the directory and its contents.
+!packages/mcp/data/
+!packages/mcp/data/**
 # compiled output
 /dist
 node_modules

apps/client/src/features/ai-chat/components/ai-chat-window.tsx

@@ -86,11 +86,19 @@ const MIN_HEIGHT = 400;
 // Margin kept between the window and the viewport edges while dragging.
 const EDGE_MARGIN = 8;
 
-// #184 phase 1.5: hard cap on the degraded-poll fallback. The poll is armed when
-// a resume attempt could not attach to the live run and disarmed by the thread on
-// settle / local stream; this cap is the ONLY backstop against an endless tick
-// (a stuck 'streaming' row before the boot-sweep, or a user-tail 204 with no run).
-const DEGRADED_POLL_MAX_MS = 10 * 60_000;
+// #184 phase 1.5 / #430: backstop for the degraded-poll fallback. The poll is
+// armed when a resume attempt could not attach to the live run and disarmed by the
+// thread on settle / local stream; this cap is the ONLY backstop against an endless
+// tick (a stuck 'streaming' row before the boot-sweep, or a user-tail 204 with no
+// run).
+//
+// #430: measured from RUN ACTIVITY, not from arm-time. A real autonomous run takes
+// 11-25 min — longer than a fixed 10-min-from-start cap, which used to cut the poll
+// off mid-run. Instead we cap on INACTIVITY: keep polling as long as the run is
+// still making progress (its persisted rows keep changing), and only give up after
+// this long with NO new activity. A genuinely stuck run produces no row changes, so
+// the idle cap still bounds it; a long-but-progressing run polls to completion.
+const DEGRADED_POLL_IDLE_MAX_MS = 10 * 60_000;
 
 /** Compact token formatter: 1.2M / 3.4k / 950. */
 function formatTokens(n: number): string {
@@ -254,9 +262,12 @@ export default function AiChatWindow() {
   // onResumeFallback(true); the thread disarms it on settle / local stream. The
   // window only OWNS the timer (armedAtRef stamps when it was armed for the cap).
   const [degradedPoll, setDegradedPoll] = useState(false);
-  const armedAtRef = useRef(0);
+  // #430: timestamp of the LAST run activity while the poll is armed — stamped on
+  // arm and re-stamped whenever the polled rows change (see the effect below). The
+  // idle cap is measured from this, so a long-but-progressing run keeps polling.
+  const lastActivityAtRef = useRef(0);
   const onResumeFallback = useCallback((active: boolean): void => {
-    if (active) armedAtRef.current = Date.now();
+    if (active) lastActivityAtRef.current = Date.now();
     setDegradedPoll(active);
   }, []);
   // Reset the degraded poll whenever the open chat changes: it is scoped to the
@@ -269,18 +280,28 @@ export default function AiChatWindow() {
     useAiChatMessagesQuery(
       activeChatId ?? undefined,
       // DELIBERATELY DUMB (invariant 8 / task 2.4): poll every 2.5s while armed
-      // and under the 10-min cap; otherwise off. NO error checks (TanStack v5
-      // resets fetchFailureCount each fetch, so consecutive errors are not
-      // expressible — and the poll must survive a server restart) and NO tail
-      // checks (the settled/local-stream semantics live in ChatThread, which
-      // disarms via onResumeFallback(false)). The time cap is the only backstop.
+      // and while the run is still active (#430: under the INACTIVITY cap, not a
+      // fixed-from-start cap); otherwise off. NO error checks (TanStack v5 resets
+      // fetchFailureCount each fetch, so consecutive errors are not expressible —
+      // and the poll must survive a server restart) and NO tail checks (the
+      // settled/local-stream semantics live in ChatThread, which disarms via
+      // onResumeFallback(false)). The idle cap is the only backstop.
       () =>
         degradedPoll === true &&
-        Date.now() - armedAtRef.current < DEGRADED_POLL_MAX_MS
+        Date.now() - lastActivityAtRef.current < DEGRADED_POLL_IDLE_MAX_MS
           ? 2500
           : false,
     );
 
+  // #430: re-stamp the activity clock whenever the polled rows change while the
+  // poll is armed. TanStack keeps the same `messageRows` reference across refetches
+  // that return deep-equal data (structural sharing), so a new reference means the
+  // run genuinely progressed — which extends the inactivity cap above. A stuck run
+  // yields no reference change, so the cap eventually fires and stops the poll.
+  useEffect(() => {
+    if (degradedPoll) lastActivityAtRef.current = Date.now();
+  }, [degradedPoll, messageRows]);
+
   // #184 reconnect-and-live-follow. Whether detached agent runs are enabled for
   // this workspace. When the feature is off no runs are ever created, so the
   // resume attempt would only ever 204; gating ChatThread's resume on it avoids a

apps/client/src/features/ai-chat/components/chat-thread.test.tsx

@@ -739,3 +739,170 @@ function renderResumable(initialRows: IAiChatMessageRow[]) {
     act(() => view.rerender(<Wrapper rows={rows} />));
   return { rerender, onResumeFallback };
 }
+
+// #430: auto-reconnect to a DETACHED run after a LIVE SSE disconnect. The mount
+// path only resumes on mount/reload; these cover the missing trigger — a live
+// `isDisconnect` on onFinish must (backoff-)re-attach WITHOUT a reload, pin+strip
+// the live row to avoid duplicates, fall back to the degraded poll on a 204, and
+// exhaust to a manual Retry.
+describe("ChatThread — live reconnect after isDisconnect (#430)", () => {
+  // A LIVE local turn that just dropped: the settled tail existed before, and the
+  // partial assistant row lives only in `messages` (not persisted as a tail).
+  const settledTail = () => [
+    row("u1", "user", undefined, "hi"),
+    row("a1", "assistant", "succeeded", "done"),
+  ];
+  // The partial assistant message onFinish hands us for the dropped LIVE turn.
+  const liveMsg = {
+    id: "a2",
+    role: "assistant",
+    parts: [{ type: "text", text: "partial live answer" }],
+  };
+
+  beforeEach(() => {
+    resetState();
+    // status "ready": with a live disconnect the mock is not streaming, so the
+    // status==="streaming" auto-clear effect stays out of the way.
+    h.state.status = "ready";
+    vi.useFakeTimers();
+  });
+  afterEach(() => {
+    vi.useRealTimers();
+    cleanup();
+  });
+
+  // Render a NON-resuming mount (settled tail -> no mount resume) with autonomous
+  // runs on, then simulate a live disconnect via onFinish.
+  function renderLiveThenDisconnect() {
+    const view = renderThread({
+      autonomousRunsEnabled: true,
+      initialRows: settledTail(),
+    });
+    // The settled tail must NOT have triggered a mount resume.
+    expect(h.state.resumeStream).not.toHaveBeenCalled();
+    act(() => {
+      h.state.onFinish?.({
+        message: liveMsg,
+        isAbort: false,
+        isDisconnect: true,
+        isError: false,
+      });
+    });
+    return view;
+  }
+
+  // Fire the pending (scheduled) attempt for `attempt` (backoff = 1s,2s,4s,...).
+  function advanceToAttempt(attempt: number) {
+    act(() => {
+      vi.advanceTimersByTime(1000 * 2 ** (attempt - 1));
+    });
+  }
+
+  // Simulate the reconnect GET returning 204 (nothing live) so the transport's
+  // no-active-stream recovery runs.
+  async function reconnect204() {
+    vi.stubGlobal(
+      "fetch",
+      vi.fn().mockResolvedValue({ status: 204, ok: false }),
+    );
+    await act(async () => {
+      await h.state.transport!.fetch!("http://x", { method: "GET" });
+    });
+  }
+
+  // Simulate the reconnect GET returning a live 2xx stream.
+  async function reconnect200() {
+    vi.stubGlobal(
+      "fetch",
+      vi.fn().mockResolvedValue({ status: 200, ok: true }),
+    );
+    await act(async () => {
+      await h.state.transport!.fetch!("http://x", { method: "GET" });
+    });
+  }
+
+  it("calls resumeStream POST-mount (a live disconnect triggers a backoff reconnect)", () => {
+    renderLiveThenDisconnect();
+    // The banner shows immediately; the attach itself fires after the first backoff.
+    expect(screen.getByText(/reconnecting/i)).toBeTruthy();
+    expect(h.state.resumeStream).not.toHaveBeenCalled();
+    advanceToAttempt(1);
+    // resumeStream is now called AFTER mount — the bug was it only ever fired once
+    // on mount. The reconnect URL pins expect=live&anchor to OUR run.
+    expect(h.state.resumeStream).toHaveBeenCalledTimes(1);
+    expect(h.state.transport!.prepareReconnectToStreamRequest!().api).toBe(
+      "/api/ai-chat/runs/c1/stream?expect=live&anchor=a2",
+    );
+  });
+
+  it("strips the pinned live row before replay so content is NOT duplicated", () => {
+    renderLiveThenDisconnect();
+    advanceToAttempt(1);
+    // The attempt strips the anchor row from the store (the live replay rebuilds
+    // it). Apply the setMessages updater to prove it removes exactly the anchor.
+    const updater = h.state.setMessages.mock.calls.at(-1)![0] as (
+      prev: { id: string }[],
+    ) => { id: string }[];
+    expect(updater([{ id: "u1" }, { id: "a2" }])).toEqual([{ id: "u1" }]);
+  });
+
+  it("a live re-attach (2xx) clears the reconnect banner", async () => {
+    renderLiveThenDisconnect();
+    advanceToAttempt(1);
+    await reconnect200();
+    expect(screen.queryByText(/reconnecting/i)).toBeNull();
+  });
+
+  it("a 204 arms the degraded poll and backs off to the next attempt", async () => {
+    const { onResumeFallback } = renderLiveThenDisconnect();
+    advanceToAttempt(1);
+    expect(h.state.resumeStream).toHaveBeenCalledTimes(1);
+    await reconnect204();
+    // Fallback engaged: the degraded poll is armed (204 -> onNoActiveStream).
+    expect(onResumeFallback).toHaveBeenCalledWith(true);
+    // Still reconnecting — the banner advanced to attempt 2/5.
+    expect(screen.getByText(/reconnecting.*2\/5/i)).toBeTruthy();
+    // The next backoff fires attempt 2 (another resumeStream).
+    advanceToAttempt(2);
+    expect(h.state.resumeStream).toHaveBeenCalledTimes(2);
+  });
+
+  it("exhausts the attempt limit into a manual Retry, which restarts the sequence", async () => {
+    renderLiveThenDisconnect();
+    // Drive all 5 attempts, each failing with a 204.
+    for (let n = 1; n <= 5; n++) {
+      advanceToAttempt(n);
+      expect(h.state.resumeStream).toHaveBeenCalledTimes(n);
+      await reconnect204();
+    }
+    // The 5th 204 exhausted the cap -> the manual Retry replaces the banner.
+    expect(screen.queryByText(/reconnecting/i)).toBeNull();
+    const retry = screen.getByText("Retry");
+    expect(retry).toBeTruthy();
+    // Retry fires attempt 1 immediately (no backoff) — a 6th resumeStream.
+    act(() => {
+      fireEvent.click(retry);
+    });
+    expect(h.state.resumeStream).toHaveBeenCalledTimes(6);
+    expect(screen.getByText(/reconnecting/i)).toBeTruthy();
+  });
+
+  it("does NOT reconnect when autonomous runs are disabled", () => {
+    renderThread({ autonomousRunsEnabled: false, initialRows: settledTail() });
+    act(() => {
+      h.state.onFinish?.({
+        message: liveMsg,
+        isAbort: false,
+        isDisconnect: true,
+        isError: false,
+      });
+    });
+    expect(screen.queryByText(/reconnecting/i)).toBeNull();
+    // The terminal "connection lost" notice is shown instead (unchanged behavior).
+    expect(
+      screen.getByText("Connection lost — the answer was interrupted."),
+    ).toBeTruthy();
+    advanceToAttempt(1);
+    expect(h.state.resumeStream).not.toHaveBeenCalled();
+  });
+});

apps/client/src/features/ai-chat/components/chat-thread.tsx

@@ -1,7 +1,17 @@
 import { useCallback, useEffect, useMemo, useRef, useState } from "react";
 import { useQueryClient } from "@tanstack/react-query";
 import { generateId } from "ai";
-import { ActionIcon, Box, Group, Stack, Text, Tooltip } from "@mantine/core";
+import {
+  ActionIcon,
+  Alert,
+  Box,
+  Button,
+  Group,
+  Loader,
+  Stack,
+  Text,
+  Tooltip,
+} from "@mantine/core";
 import {
   IconClockHour4,
   IconPlayerPlayFilled,
@@ -51,6 +61,15 @@ import classes from "@/features/ai-chat/components/ai-chat.module.css";
 // from the token rate.
 const STREAM_THROTTLE_MS = 50;
 
+// #430: auto-reconnect after a LIVE SSE disconnect of a DETACHED (autonomous) run.
+// The run keeps executing server-side, so instead of a dead "Lost connection"
+// banner we re-attach to the live tail through the SAME resumable machinery the
+// mount path uses. Attempts back off exponentially and are capped; on exhaustion
+// the user gets a manual Retry (the degraded poll keeps catching up underneath).
+const RECONNECT_MAX_ATTEMPTS = 5;
+// Backoff before attempt N (1-based): 1s, 2s, 4s, 8s, 16s.
+const RECONNECT_BASE_DELAY_MS = 1000;
+
 /** The page the user is currently viewing, sent as chat context. */
 export interface OpenPageContext {
   id: string;
@@ -175,6 +194,10 @@ export default function ChatThread({
   const reconcileTailRef = useRef(false);
   const noStreamHandledRef = useRef(false);
   const onNoActiveStreamRef = useRef<(() => void) | null>(null);
+  // #430: called from the transport's reconnect-GET success branch when a live
+  // stream re-attached (2xx, not 204) — clears the reconnect banner. Kept in a ref
+  // because the transport's fetch closure (useMemo([])) reads it live.
+  const onReconnectAttachedRef = useRef<(() => void) | null>(null);
   // Live mount flag. The attach GET and the resumed `onFinish` are async and can
   // land AFTER this thread unmounts (the parent remounts per chat via `key`); with
   // chatIdRef then pointing at the NEW chat, an ungated late callback would arm a
@@ -378,6 +401,10 @@ export default function ChatThread({
             // NOT drop the in-progress row or stop tracking the durable run.
             if (response.status === 204 || !response.ok)
               onNoActiveStreamRef.current?.();
+            // #430: a 2xx stream re-attached (live tail or finished-replay). Signal
+            // the reconnect controller to clear its banner. No-op outside an active
+            // reconnect sequence (e.g. the mount attach), so it is safe here.
+            else onReconnectAttachedRef.current?.();
             return response;
           } catch (err) {
             // Network throw: same no-onFinish recovery, then rethrow so the SDK
@@ -481,6 +508,31 @@ export default function ChatThread({
           );
         }
       }
+      // (2b) #430: a LIVE (non-resumed) detached run whose SSE just dropped. The
+      // server run keeps executing, so instead of a dead "Lost connection" banner
+      // start a reconnect sequence: pin the CURRENT streaming assistant row as the
+      // strip/anchor (the live tail is the already-shown partial in `messages`, not
+      // a persistent row) and re-attach to the live tail via the resumable machinery.
+      const startedReconnect =
+        isDisconnect &&
+        !wasResumed &&
+        autonomousRunsEnabled === true &&
+        mountedRef.current &&
+        message?.role === "assistant" &&
+        typeof message.id === "string";
+      if (startedReconnect) {
+        beginReconnect({
+          id: message.id,
+          role: "assistant",
+          content: "",
+          status: "streaming",
+          createdAt: new Date().toISOString(),
+          // Preserve the partial parts so a 204 restore (onNoActiveStream) re-shows
+          // what was on screen while the degraded poll catches the run up to
+          // terminal (rowToUiMessage prefers metadata.parts).
+          metadata: { parts: message.parts },
+        });
+      }
       // (3) Standard branches.
       // Forward the authoritative server chatId (streamed on the assistant
       // message metadata) so the parent adopts the REAL created chat id for a new
@@ -490,9 +542,11 @@ export default function ChatThread({
       onTurnFinished(extractServerChatId(message), threadKey);
       // Show a neutral "stopped" marker for an aborted turn; the red error banner
       // (via `error`) already covers isError, and a clean finish clears any marker.
+      // On a live disconnect that STARTED a reconnect, suppress the terminal
+      // "connection lost" notice — the reconnect banner takes over (#430).
       if (isError) setStopNotice(null);
       else if (isAbort) setStopNotice("manual");
-      else if (isDisconnect) setStopNotice("disconnect");
+      else if (isDisconnect) setStopNotice(startedReconnect ? null : "disconnect");
       else setStopNotice(null);
       // A resumed turn NEVER flushes the queue (invariant 7): skip BOTH the
       // flush-on-abort branch and the plain flush. The local streamer is the only
@@ -579,6 +633,106 @@ export default function ChatThread({
 
   const isStreaming = status === "submitted" || status === "streaming";
 
+  // #430: live-disconnect reconnect controller. `null` = idle; `{ trying, attempt }`
+  // = a backoff sequence is running (drives the "reconnecting… (N/max)" banner);
+  // `{ failed }` = attempts exhausted (drives the manual Retry). Mirrored into a ref
+  // so the transport/onNoActiveStream closures branch on the LIVE value.
+  type ReconnectState =
+    | null
+    | { phase: "trying"; attempt: number }
+    | { phase: "failed" };
+  const [reconnectState, setReconnectState] = useState<ReconnectState>(null);
+  const reconnectStateRef = useRef<ReconnectState>(null);
+  const setReconnectStatePair = useCallback((s: ReconnectState) => {
+    reconnectStateRef.current = s;
+    setReconnectState(s);
+  }, []);
+  const reconnectTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+  const clearReconnectTimer = useCallback(() => {
+    if (reconnectTimerRef.current) {
+      clearTimeout(reconnectTimerRef.current);
+      reconnectTimerRef.current = null;
+    }
+  }, []);
+
+  // One reconnect attempt — MIRRORS the mount strip/anchor path for the LIVE case.
+  // beginReconnect pinned strippedRowRef/stripRef to the run's assistant row, so:
+  //  - remove that row from the store (the mount path strips it from the SEED; here
+  //    it is already shown, so filter it out) — the live replay's `text-start` then
+  //    rebuilds it without DUPLICATING parts (the main dedup risk, #430);
+  //  - reset the one-shot 204 guard so onNoActiveStream can fire for THIS attempt;
+  //  - mark the turn resumed (invariant 7/8) so onFinish runs the recovery block and
+  //    never flushes the queue;
+  //  - resumeStream() -> prepareReconnectToStreamRequest builds
+  //    ?expect=live&anchor=<pinned id>, pinning the replay to OUR run (invariant 6).
+  const attemptReconnectOnce = useCallback(
+    (attempt: number) => {
+      if (!mountedRef.current) return;
+      const anchor = strippedRowRef.current;
+      if (anchor) {
+        setMessages((prev) => prev.filter((m) => m.id !== anchor.id));
+      }
+      noStreamHandledRef.current = false;
+      setResumedTurnPair(true);
+      setReconnectStatePair({ phase: "trying", attempt });
+      void resumeStream();
+    },
+    [setMessages, setResumedTurnPair, setReconnectStatePair, resumeStream],
+  );
+
+  // Schedule attempt `attempt` after an exponential backoff.
+  const scheduleReconnectAttempt = useCallback(
+    (attempt: number) => {
+      clearReconnectTimer();
+      setReconnectStatePair({ phase: "trying", attempt });
+      reconnectTimerRef.current = setTimeout(
+        () => attemptReconnectOnce(attempt),
+        RECONNECT_BASE_DELAY_MS * 2 ** (attempt - 1),
+      );
+    },
+    [clearReconnectTimer, setReconnectStatePair, attemptReconnectOnce],
+  );
+
+  // Start a fresh reconnect sequence, pinning `anchorRow` (the live run's assistant
+  // row) as the strip/anchor reused by every attempt.
+  const beginReconnect = useCallback(
+    (anchorRow: IAiChatMessageRow) => {
+      if (!autonomousRunsEnabled || !mountedRef.current) return;
+      strippedRowRef.current = anchorRow;
+      stripRef.current = true;
+      scheduleReconnectAttempt(1);
+    },
+    [autonomousRunsEnabled, scheduleReconnectAttempt],
+  );
+
+  // Manual Retry (shown once attempts are exhausted): restart at attempt 1 and fire
+  // immediately (the user asked for it now — no backoff).
+  const retryReconnect = useCallback(() => {
+    clearReconnectTimer();
+    attemptReconnectOnce(1);
+  }, [clearReconnectTimer, attemptReconnectOnce]);
+
+  // Live SSE re-attached (the reconnect GET returned a 2xx stream): clear the
+  // banner + any pending backoff. No-op outside a sequence (e.g. the mount attach).
+  const onReconnectAttached = useCallback(() => {
+    if (!mountedRef.current || !reconnectStateRef.current) return;
+    clearReconnectTimer();
+    setReconnectStatePair(null);
+  }, [clearReconnectTimer, setReconnectStatePair]);
+  onReconnectAttachedRef.current = onReconnectAttached;
+
+  // The reconnect GET could not attach (204 / error). onNoActiveStream has already
+  // armed the degraded poll (the robust fallback that drives the row to terminal
+  // from the DB), so this only decides the LIVE-attach retry: back off and try
+  // again up to the cap, else surface the manual Retry.
+  const onReconnectNoStream = useCallback(() => {
+    const s = reconnectStateRef.current;
+    if (s?.phase !== "trying") return;
+    if (s.attempt < RECONNECT_MAX_ATTEMPTS)
+      scheduleReconnectAttempt(s.attempt + 1);
+    else setReconnectStatePair({ phase: "failed" });
+  }, [scheduleReconnectAttempt, setReconnectStatePair]);
+
   // 204-handler (`onNoActiveStream`): the attach returned 204 — nothing live to
   // resume (overflow / begin-failure / after retention / anchor-mismatch). One-
   // shot via noStreamHandledRef (we do NOT null onNoActiveStreamRef). Exactly four
@@ -610,7 +764,17 @@ export default function ChatThread({
     // (d) 204 means onFinish will NOT fire — clear the suppression flag so it
     // cannot swallow the NEXT local turn's queue flush.
     setResumedTurnPair(false);
-  }, [setMessages, queryClient, onResumeFallback, setResumedTurnPair]);
+    // (e) #430: if this 204/error landed during a live-disconnect reconnect
+    // sequence, back off and retry the live attach (or give up to the manual
+    // Retry). The degraded poll armed in (c) is the fallback either way.
+    onReconnectNoStream();
+  }, [
+    setMessages,
+    queryClient,
+    onResumeFallback,
+    setResumedTurnPair,
+    onReconnectNoStream,
+  ]);
   onNoActiveStreamRef.current = onNoActiveStream;
 
   // Mount effect: kick off the resume attempt for a non-settled tail. Marking the
@@ -628,6 +792,9 @@ export default function ChatThread({
     return () => {
       mountedRef.current = false;
       attachAbortRef.current?.abort();
+      // #430: drop any pending reconnect backoff so it can't fire against the next
+      // chat this thread's refs are reused for.
+      if (reconnectTimerRef.current) clearTimeout(reconnectTimerRef.current);
     };
     // Mount-only by design; the parent remounts per chat via `key`.
     // eslint-disable-next-line react-hooks/exhaustive-deps
@@ -666,12 +833,27 @@ export default function ChatThread({
     if (tail.status !== "streaming") {
       reconcileTailRef.current = false;
       onResumeFallback?.(false);
+      // #430: the run reached its terminal state via the degraded poll — there is
+      // no live tail left to reconnect to, so drop any reconnect banner / Retry.
+      clearReconnectTimer();
+      setReconnectStatePair(null);
     }
     // onResumeFallback intentionally omitted (parent-stable callback); deps are
     // fixed by the resume design.
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [initialRows, isStreaming, setMessages]);
 
+  // #430: a real stream is live again — the reconnect re-attached to the live tail
+  // (status -> "streaming") OR the user started a new local turn. Either way clear
+  // the reconnect banner + any pending backoff. Gated on "streaming" (not the
+  // broader "submitted") so a still-pending attach GET does not clear prematurely.
+  useEffect(() => {
+    if (status === "streaming") {
+      clearReconnectTimer();
+      setReconnectStatePair(null);
+    }
+  }, [status, clearReconnectTimer, setReconnectStatePair]);
+
   // "Send now" on a queued message: interrupt the current turn and immediately
   // send THIS message, keeping the agent's partial output. Other queued messages
   // stay queued and flush normally after the new turn. Reuses the existing
@@ -719,6 +901,9 @@ export default function ChatThread({
     // observer's Stop would otherwise leave the attach fetch running.
     attachAbortRef.current?.abort();
     stop();
+    // #430: pressing Stop also cancels an in-progress reconnect sequence.
+    clearReconnectTimer();
+    setReconnectStatePair(null);
     if (!autonomousRunsEnabled) return;
     if (chatIdRef.current) {
       onServerStop?.(chatIdRef.current);
@@ -740,7 +925,13 @@ export default function ChatThread({
       // for this fix. Documented so a future change can address the abort-ordering.
       stopPendingRef.current = true;
     }
-  }, [stop, autonomousRunsEnabled, onServerStop]);
+  }, [
+    stop,
+    autonomousRunsEnabled,
+    onServerStop,
+    clearReconnectTimer,
+    setReconnectStatePair,
+  ]);
 
   // Clear the stopped marker as soon as a new turn begins streaming, and drop any
   // stale "Send now" interrupt flags. On the legit interrupt path both refs are
@@ -825,6 +1016,43 @@ export default function ChatThread({
           detail={errorView.detail}
           mb="xs"
         />
+      ) : reconnectState ? (
+        // #430: while auto-reconnecting to a detached run's live tail, show progress
+        // instead of a dead "Lost connection" banner; once attempts are exhausted,
+        // offer a manual Retry (the degraded poll keeps catching up underneath).
+        <Alert
+          variant="light"
+          color="gray"
+          p="xs"
+          mb="xs"
+          style={{ flexShrink: 0 }}
+        >
+          <Group gap={8} wrap="nowrap" align="center">
+            {reconnectState.phase === "trying" ? (
+              <>
+                <Loader size={14} color="gray" style={{ flex: "none" }} />
+                <Text size="sm" lh={1.3} c="dimmed">
+                  {t("Connection lost — reconnecting…")}
+                  {` (${reconnectState.attempt}/${RECONNECT_MAX_ATTEMPTS})`}
+                </Text>
+              </>
+            ) : (
+              <>
+                <Text size="sm" lh={1.3} c="dimmed" style={{ flex: 1 }}>
+                  {t("Couldn't reconnect to the answer.")}
+                </Text>
+                <Button
+                  size="compact-xs"
+                  variant="light"
+                  color="gray"
+                  onClick={retryReconnect}
+                >
+                  {t("Retry")}
+                </Button>
+              </>
+            )}
+          </Group>
+        </Alert>
       ) : stopNotice ? (
         <ChatStoppedNotice
           text={

apps/client/src/features/editor/components/footnote/footnote-definition-view.tsx

@@ -49,19 +49,14 @@ export default function FootnoteDefinitionView(props: NodeViewProps) {
       className={classes.definition}
       style={{ ["--footnote-number" as any]: `"${number}"` }}
     >
-      {/* #146: contentDOM MUST be the first child — a non-editable marker before
+      {/* #146: contentDOM MUST be the first child — non-editable chrome before
           it makes click hit-testing snap the caret above. Content first; the
-          marker + back-link follow in DOM and are placed left/right via CSS
-          flex `order`. The second #146 mitigation lives in
+          back-link follows in DOM and is placed on the right via CSS flex. The
+          decorative "N." number is rendered inline via the .definitionContent
+          ::before rule (from the --footnote-number var), so no marker element
+          precedes the content. The second #146 mitigation lives in
           editor-paste-handler.tsx (reflowAfterPaste). */}
       <NodeViewContent className={classes.definitionContent} />
-      <span
-        className={classes.definitionMarker}
-        contentEditable={false}
-        aria-hidden="true"
-      >
-        {number}.
-      </span>
       {refCount > 1 ? (
         // Multiple references -> ↩ followed by one lettered link per occurrence.
         <span

apps/client/src/features/editor/components/footnote/footnote.module.css

@@ -81,34 +81,34 @@
 .definition {
   display: flex;
   align-items: flex-start;
-  /* Tight number→text spacing (~one space) so it reads like "1. text"
-     instead of leaving a wide gap after the period. */
-  gap: 0.4em;
+  /* Tight spacing between the content and the trailing ↩ back-link. */
+  gap: 0.3em;
   padding: 2px 0;
+  /* Footnotes read smaller than body text (16px). Matches .listHeading. */
+  font-size: var(--mantine-font-size-sm);
 }
 
-.definitionMarker {
-  order: -1; /* keep the "N." marker on the LEFT though it follows content in DOM (#146) */
-  flex: 0 0 auto;
-  min-width: 1.5em;
-  /* Right-align within the narrow column so the period sits next to the text
-     and multi-digit numbers (10, 11, …) stay aligned on their right edge. */
-  text-align: right;
-  font-variant-numeric: tabular-nums;
-  color: var(--mantine-color-dimmed);
-  user-select: none;
-}
-
+/* The "N." number is decorative (from the --footnote-number CSS var on the
+   wrapper, never in the document model) and is rendered inline at the start of
+   the first content line via ::before. This keeps text and wrapped lines flush
+   to the left margin — no hanging indent — while the editable contentDOM stays
+   the FIRST DOM child (#146). */
 .definitionContent {
   flex: 1 1 auto;
   min-width: 0;
 }
 
-/* The inner editable paragraph inherits `.ProseMirror p { margin: 0.5em 0 }`,
-   which pushes the first text line ~0.5em below the "N." marker (aligned to
-   flex-start), making the number float above the text. Drop the outer margins
-   so the marker and the first line share the same top edge — same approach
-   used for callouts in core.css. */
+.definitionContent > :first-child::before {
+  content: var(--footnote-number, "?") ". ";
+  color: var(--mantine-color-dimmed);
+  font-variant-numeric: tabular-nums;
+  user-select: none;
+}
+
+/* The inner editable paragraph inherits `.ProseMirror p { margin: 0.5em 0 }`.
+   Drop the outer margins so the definition sits tight to the heading above and
+   the ::before number aligns with the top of the row — same approach used for
+   callouts in core.css. */
 .definitionContent > :first-child {
   margin-top: 0;
 }

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

@@ -17,10 +17,24 @@ import { Injectable, Logger, OnModuleDestroy } from '@nestjs/common';
 /** How long a finished entry is retained for late attach (replay + immediate end). */
 export const RUN_STREAM_RETAIN_FINISHED_MS = 30_000;
 
-/** Per-run replay buffer cap. Past this the buffer is dropped (attach -> 204). */
-export const RUN_STREAM_MAX_BUFFER_BYTES = 4 * 1024 * 1024;
+/**
+ * Per-run replay buffer cap. Past this the buffer is dropped (attach -> 204, and
+ * the client falls back to its restore + degraded-poll path, #430).
+ *
+ * Raised from 4MB to 32MB (#430): marathon autonomous runs (11-25 min observed)
+ * stream far more than 4MB of SSE frames, so a live disconnect mid-run would find
+ * an already-overflowed buffer and could only degrade-poll instead of re-attaching
+ * to the live tail. 32MB comfortably covers those runs while staying bounded.
+ *
+ * Memory cost: this is the WORST-CASE retained size PER ACTIVE run (the buffer is
+ * freed on finish + retention, or dropped immediately on overflow). With the small
+ * number of concurrent autonomous runs a single workspace realistically has, 32MB
+ * each is an acceptable ceiling; the overflow->204->degraded-poll fallback remains
+ * the backstop for anything larger, so correctness never depends on this bound.
+ */
+export const RUN_STREAM_MAX_BUFFER_BYTES = 32 * 1024 * 1024;
 
-// 2x the replay cap: a just-written 4MB replay burst alone can never trip the
+// 2x the replay cap: a just-written full-replay burst alone can never trip the
 // per-subscriber cap (see controller); only a genuinely stalled socket can.
 export const SUBSCRIBER_MAX_BUFFERED_BYTES = 2 * RUN_STREAM_MAX_BUFFER_BYTES;
 

apps/server/src/core/ai-chat/ai-chat-stream-registry.spec.ts

@@ -2,6 +2,7 @@ import {
   AiChatStreamRegistryService,
   RUN_STREAM_MAX_BUFFER_BYTES,
   RUN_STREAM_RETAIN_FINISHED_MS,
+  SUBSCRIBER_MAX_BUFFERED_BYTES,
   RunStreamCallbacks,
 } from './ai-chat-stream-registry.service';
 
@@ -210,9 +211,10 @@ describe('AiChatStreamRegistryService', () => {
     const att = (await registry.attach(CHAT, false, undefined, c.cb))!;
     att.start();
 
-    const oneMb = 'x'.repeat(1024 * 1024);
-    // 5 x 1MB = 5MB > 4MB cap; the 5th frame is the one that crosses.
-    for (let i = 0; i < 5; i++) src.push(oneMb + i);
+    // Cap-relative so it survives a buffer-cap change (#430): a quarter-cap frame
+    // means 5 frames comfortably exceed the replay cap; the last one crosses.
+    const chunk = 'x'.repeat(Math.floor(RUN_STREAM_MAX_BUFFER_BYTES / 4));
+    for (let i = 0; i < 5; i++) src.push(chunk + i);
     await flush();
 
     const entry = (registry as any).entries.get(CHAT);
@@ -220,7 +222,7 @@ describe('AiChatStreamRegistryService', () => {
     expect(entry.bytes).toBeGreaterThan(RUN_STREAM_MAX_BUFFER_BYTES);
     // The live subscriber received ALL 5 frames, including the crossing one.
     expect(c.frames).toHaveLength(5);
-    expect(c.frames[4]).toBe(oneMb + 4);
+    expect(c.frames[4]).toBe(chunk + 4);
 
     // A NEW attach after overflow gets null (replay buffer is gone).
     const c2 = collector();
@@ -240,9 +242,11 @@ describe('AiChatStreamRegistryService', () => {
     const attB = (await registry.attach(CHAT, false, undefined, b.cb))!;
     attB.start();
 
-    const oneMb = 'x'.repeat(1024 * 1024);
-    // 9 x 1MB = 9MB > 8MB per-subscriber cap; A's pending overflows, B streams live.
-    for (let i = 0; i < 9; i++) src.push(oneMb + i);
+    // Cap-relative so it survives a buffer-cap change (#430): a quarter-of-the-
+    // per-subscriber-cap frame means 5 frames exceed A's paused-pending cap while
+    // B streams every frame live.
+    const chunk = 'x'.repeat(Math.floor(SUBSCRIBER_MAX_BUFFERED_BYTES / 4));
+    for (let i = 0; i < 5; i++) src.push(chunk + i);
     await flush();
 
     const entry = (registry as any).entries.get(CHAT);
@@ -250,7 +254,7 @@ describe('AiChatStreamRegistryService', () => {
     expect(entry.subscribers.size).toBe(1);
     expect(a.frames).toEqual([]); // paused + overflowed: nothing was delivered
     // B received every frame live (delivery unaffected by A's overflow).
-    expect(b.frames).toHaveLength(9);
+    expect(b.frames).toHaveLength(5);
 
     // A's start() (arriving late) degrades to an immediate end, not a partial replay.
     attA.start();

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

@@ -148,6 +148,53 @@ describe('assistantParts', () => {
     expect(toolPart).not.toHaveProperty('output');
   });
 
+  it('replays the REAL error text for a THROWN tool (tool-error part)', () => {
+    const steps = [
+      {
+        text: '',
+        toolCalls: [
+          { toolCallId: 'c7', toolName: 'editPageText', input: { id: 'p1' } },
+        ],
+        // A thrown tool is a `tool-error` content part; toolResults holds only
+        // successes and stays empty for this call.
+        toolResults: [],
+        content: [
+          {
+            type: 'tool-error',
+            toolCallId: 'c7',
+            toolName: 'editPageText',
+            input: { id: 'p1' },
+            error: new Error('page is locked'),
+          },
+        ],
+      },
+    ];
+    const parts = assistantParts(steps, '') as AnyPart[];
+    const toolPart = parts.find((p) => p.type === 'tool-editPageText');
+    expect(toolPart).toBeDefined();
+    expect(toolPart!.state).toBe('output-error');
+    // The REAL error is replayed, NOT the 'Tool call did not complete.' placeholder.
+    expect(toolPart!.errorText).toBe('page is locked');
+    expect(toolPart).not.toHaveProperty('output');
+  });
+
+  it('keeps the placeholder ONLY for a call with neither result nor tool-error', () => {
+    const steps = [
+      {
+        text: '',
+        toolCalls: [
+          { toolCallId: 'c8', toolName: 'insertNode', input: { node: {} } },
+        ],
+        toolResults: [],
+        content: [], // aborted mid-step: no result AND no tool-error
+      },
+    ];
+    const parts = assistantParts(steps, '') as AnyPart[];
+    const toolPart = parts.find((p) => p.type === 'tool-insertNode');
+    expect(toolPart!.state).toBe('output-error');
+    expect(toolPart!.errorText).toBe('Tool call did not complete.');
+  });
+
   it('skips malformed tool-calls (missing toolName or toolCallId)', () => {
     const steps = [
       {
@@ -195,6 +242,45 @@ describe('serializeSteps', () => {
     expect(trace[0]).toEqual({ toolName: 'getPage', input: { id: 'p1' } });
     expect(trace[1]).toEqual({ toolName: 'getPage', output: { title: 'T' } });
   });
+
+  it('records a THROWN tool failure (tool-error part) with its error message', () => {
+    const trace = serializeSteps([
+      {
+        toolCalls: [{ toolName: 'editPageText', input: { id: 'p1' } }],
+        toolResults: [],
+        content: [
+          {
+            type: 'tool-error',
+            toolName: 'editPageText',
+            error: new Error('page is locked'),
+          },
+        ],
+      },
+    ]) as Array<Record<string, unknown>>;
+    // The call element is followed by a paired error element (mirroring how a
+    // successful result is appended), so the failure survives in the trace.
+    expect(trace).toHaveLength(2);
+    expect(trace[0]).toEqual({ toolName: 'editPageText', input: { id: 'p1' } });
+    expect(trace[1]).toEqual({
+      toolName: 'editPageText',
+      error: 'page is locked',
+    });
+  });
+
+  it('truncates a very long tool-error message to the tool-output limit', () => {
+    const long = 'x'.repeat(5000);
+    const trace = serializeSteps([
+      {
+        toolCalls: [{ toolName: 'editPageText', input: {} }],
+        toolResults: [],
+        content: [{ type: 'tool-error', toolName: 'editPageText', error: long }],
+      },
+    ]) as Array<Record<string, unknown>>;
+    const errorText = trace[1].error as string;
+    // Truncated (not the full 5000 chars) and carries the omission marker.
+    expect(errorText.length).toBeLessThan(long.length);
+    expect(errorText).toContain('chars omitted');
+  });
 });
 
 describe('rowToUiMessage', () => {

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

@@ -1637,6 +1637,17 @@ type StepLike = {
     toolName?: string;
     output?: unknown;
   }>;
+  // ai@6.0.134: a tool that THREW surfaces as a `tool-error` content part
+  // ({ type:'tool-error', toolCallId, toolName, input, error }), NOT as a
+  // `toolResults` entry (which holds only successes). Read from here so failed
+  // calls are persisted with their real error instead of being dropped.
+  content?: ReadonlyArray<{
+    type?: string;
+    toolCallId?: string;
+    toolName?: string;
+    input?: unknown;
+    error?: unknown;
+  }>;
 };
 
 /**
@@ -1739,6 +1750,26 @@ function compactValue(value: unknown, depth: number): unknown {
   return value;
 }
 
+/**
+ * Extract a bounded string message from a `tool-error` part's `error` field for
+ * persistence and history replay. The field may be an `Error`, a string, or an
+ * arbitrary object, so pull a message robustly. The result is passed through
+ * `compactValue` so a very long error honors the SAME truncation limits the file
+ * already applies to tool outputs (no new limit is introduced here).
+ */
+function normalizeToolError(error: unknown): string {
+  const message =
+    error instanceof Error
+      ? error.message
+      : typeof error === 'string'
+        ? error
+        : error != null &&
+            typeof (error as { message?: unknown }).message === 'string'
+          ? (error as { message: string }).message
+          : String(error);
+  return compactValue(message, 0) as string;
+}
+
 /**
  * Rebuild the FULL UIMessage `parts` for an assistant turn from the SDK steps,
  * so multi-turn history replays prior tool-calls/results to the model (not just
@@ -1771,6 +1802,14 @@ export function assistantParts(
     for (const r of step.toolResults ?? []) {
       if (r.toolCallId) resultsById.set(r.toolCallId, r.output);
     }
+    // Index this step's THROWN tool failures (ai@6 `tool-error` content parts)
+    // by tool call id, so a call that failed replays with its real error text.
+    const errorsById = new Map<string, unknown>();
+    for (const part of step.content ?? []) {
+      if (part.type === 'tool-error' && part.toolCallId) {
+        errorsById.set(part.toolCallId, part.error);
+      }
+    }
     for (const call of step.toolCalls ?? []) {
       if (!call.toolName || !call.toolCallId) continue;
       const hasResult = resultsById.has(call.toolCallId);
@@ -1783,9 +1822,21 @@ export function assistantParts(
           input: call.input,
           output: compactToolOutput(resultsById.get(call.toolCallId)),
         });
+      } else if (errorsById.has(call.toolCallId)) {
+        // The tool THREW: replay the REAL error so the model on the next turn
+        // knows WHY the call failed (and does not blindly repeat it). An
+        // output-error round-trips through convertToModelMessages as a balanced
+        // tool-call + tool-result, keeping the rebuilt history valid.
+        parts.push({
+          type: `tool-${call.toolName}`,
+          toolCallId: call.toolCallId,
+          state: 'output-error',
+          input: call.input,
+          errorText: normalizeToolError(errorsById.get(call.toolCallId)),
+        });
       } else {
-        // No paired result (e.g. aborted mid-step). Persisting a bare
-        // tool-call (input-available) would replay as an unpaired call and
+        // No paired result AND no tool-error (e.g. aborted mid-step). Persisting
+        // a bare tool-call (input-available) would replay as an unpaired call and
         // throw MissingToolResultsError on the next turn (convertToModelMessages
         // emits no tool-result for it). Emit a SYNTHETIC paired result instead:
         // an output-error round-trips through convertToModelMessages as a
@@ -2021,10 +2072,19 @@ export function serializeSteps(
   steps: ReadonlyArray<{
     toolCalls?: ReadonlyArray<{ toolName?: string; input?: unknown }>;
     toolResults?: ReadonlyArray<{ toolName?: string; output?: unknown }>;
+    content?: ReadonlyArray<{
+      type?: string;
+      toolName?: string;
+      error?: unknown;
+    }>;
   }>,
 ): unknown {
-  const calls: Array<{ toolName?: string; input?: unknown; output?: unknown }> =
-    [];
+  const calls: Array<{
+    toolName?: string;
+    input?: unknown;
+    output?: unknown;
+    error?: string;
+  }> = [];
   for (const step of steps ?? []) {
     for (const call of step.toolCalls ?? []) {
       calls.push({ toolName: call.toolName, input: call.input });
@@ -2032,6 +2092,18 @@ export function serializeSteps(
     for (const r of step.toolResults ?? []) {
       calls.push({ toolName: r.toolName, output: compactToolOutput(r.output) });
     }
+    // ai@6 surfaces a THROWN tool failure as a `tool-error` content part, NOT as
+    // a `toolResults` entry. Record it as its own paired element (mirroring how a
+    // successful result is appended) so the failure and its reason survive in the
+    // trace instead of leaving an orphaned call with no result.
+    for (const part of step.content ?? []) {
+      if (part.type === 'tool-error') {
+        calls.push({
+          toolName: part.toolName,
+          error: normalizeToolError(part.error),
+        });
+      }
+    }
   }
   return calls.length > 0 ? calls : null;
 }

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

@@ -13,6 +13,14 @@ import { SHARED_TOOL_SPECS } from '../../../../../../packages/mcp/src/tool-specs
 const mockLoaded = (DocmostClient: loader.DocmostClientCtor) => ({
   DocmostClient,
   sharedToolSpecs: SHARED_TOOL_SPECS as Record<string, loader.SharedToolSpec>,
+  // Pure no-network draw.io helpers (#424). Type-correct stubs: these tests
+  // never execute the drawio_shapes / drawio_guide tool bodies.
+  searchShapes: (() => []) as unknown as loader.SearchShapesFn,
+  getGuideSection: (() => ({
+    section: 'index',
+    content: '',
+    sections: [],
+  })) as unknown as loader.GetGuideSectionFn,
 });
 
 /**

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

@@ -168,7 +168,8 @@ export class AiChatToolsService {
     // provenance tokens) and load the shared tool-spec registry. Client
     // construction is shared with the page-change detection path (#274) via
     // buildDocmostClient so both go over the exact same authenticated route.
-    const { sharedToolSpecs } = await loadDocmostMcp();
+    const { sharedToolSpecs, searchShapes, getGuideSection } =
+      await loadDocmostMcp();
     const client = await this.buildDocmostClient(
       user,
       sessionId,
@@ -729,6 +730,53 @@ export class AiChatToolsService {
           }),
       ),
 
+      // Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#423).
+      // meta.hash in the result is the baseHash drawioUpdate requires.
+      drawioGet: sharedTool(
+        sharedToolSpecs.drawioGet,
+        async ({ pageId, node, format }) =>
+          await client.drawioGet(pageId, node, format ?? 'xml'),
+      ),
+
+      // Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#423).
+      // The flat schema fields are regrouped into the client's `where` object.
+      drawioCreate: sharedTool(
+        sharedToolSpecs.drawioCreate,
+        async ({ pageId, xml, position, anchorNodeId, anchorText, title }) =>
+          await client.drawioCreate(
+            pageId,
+            { position, anchorNodeId, anchorText },
+            xml,
+            title,
+          ),
+      ),
+
+      // Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#423).
+      // baseHash is the optimistic lock: mismatch => structured conflict error.
+      drawioUpdate: sharedTool(
+        sharedToolSpecs.drawioUpdate,
+        async ({ pageId, node, xml, baseHash }) =>
+          await client.drawioUpdate(pageId, node, xml, baseHash),
+      ),
+
+      // Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#424).
+      // Pure no-network helper — calls searchShapes directly, no client method.
+      // Result shape mirrors the MCP server: { query, count, results }.
+      drawioShapes: sharedTool(
+        sharedToolSpecs.drawioShapes,
+        async ({ query, category, limit }) => {
+          const results = searchShapes(query, { category, limit });
+          return { query, count: results.length, results };
+        },
+      ),
+
+      // Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#424).
+      // Pure no-network helper — calls getGuideSection directly, no client method.
+      drawioGuide: sharedTool(
+        sharedToolSpecs.drawioGuide,
+        async ({ section }) => getGuideSection(section),
+      ),
+
       // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
       // The table reference parameter was unified to `table` (was `tableRef`).
       tableInsertRow: sharedTool(

apps/server/src/core/ai-chat/tools/docmost-client.loader.ts

@@ -168,6 +168,32 @@ export interface DocmostClientLike {
     url: string,
     opts?: { align?: 'left' | 'center' | 'right'; alt?: string },
   ): Promise<Record<string, unknown>>;
+  // --- draw.io diagrams (#423, stage 1) ---
+  // Read a diagram as decoded mxGraph XML (default) or the raw .drawio.svg.
+  // meta.hash is the optimistic-lock key drawioUpdate expects as baseHash.
+  drawioGet(
+    pageId: string,
+    node: string,
+    format?: 'xml' | 'svg',
+  ): Promise<Record<string, unknown>>;
+  // Lint mxGraph XML, build the .drawio.svg attachment and insert a drawio node.
+  drawioCreate(
+    pageId: string,
+    where: {
+      position: 'before' | 'after' | 'append';
+      anchorNodeId?: string;
+      anchorText?: string;
+    },
+    xml: string,
+    title?: string,
+  ): Promise<Record<string, unknown>>;
+  // Optimistic-locked full replacement of a diagram (baseHash from drawioGet).
+  drawioUpdate(
+    pageId: string,
+    node: string,
+    xml: string,
+    baseHash: string,
+  ): Promise<Record<string, unknown>>;
   tableInsertRow(
     pageId: string,
     tableRef: string,
@@ -278,9 +304,24 @@ export interface SharedToolSpec {
   buildShape?: (z: any) => Record<string, unknown>;
 }
 
+// Pure, no-network draw.io helpers (#424). These are plain functions on the
+// module (NOT DocmostClient methods) — the in-app AI-SDK service calls them
+// directly to wire drawio_shapes / drawio_guide, mirroring the MCP server.
+export type SearchShapesFn = (
+  query: string,
+  opts?: { category?: string; limit?: number },
+) => Array<Record<string, unknown>>;
+export type GetGuideSectionFn = (section?: string) => {
+  section: string;
+  content: string;
+  sections: string[];
+};
+
 interface DocmostMcpModule {
   DocmostClient: DocmostClientCtor;
   SHARED_TOOL_SPECS: Record<string, SharedToolSpec>;
+  searchShapes: SearchShapesFn;
+  getGuideSection: GetGuideSectionFn;
 }
 
 // TS with module:commonjs downlevels a literal `import()` to `require()`, which
@@ -304,6 +345,8 @@ let modulePromise: Promise<DocmostMcpModule> | null = null;
 export async function loadDocmostMcp(): Promise<{
   DocmostClient: DocmostClientCtor;
   sharedToolSpecs: Record<string, SharedToolSpec>;
+  searchShapes: SearchShapesFn;
+  getGuideSection: GetGuideSectionFn;
 }> {
   if (!modulePromise) {
     modulePromise = (async () => {
@@ -329,5 +372,8 @@ export async function loadDocmostMcp(): Promise<{
   return {
     DocmostClient: mod.DocmostClient,
     sharedToolSpecs: mod.SHARED_TOOL_SPECS,
+    // Pure no-network draw.io helpers (#424); not client methods.
+    searchShapes: mod.searchShapes,
+    getGuideSection: mod.getGuideSection,
   };
 }

apps/server/src/core/ai-chat/tools/shared-tool-specs.contract.spec.ts

@@ -45,6 +45,16 @@ describe('SHARED_TOOL_SPECS contract parity', () => {
         string,
         loader.SharedToolSpec
       >,
+      // Pure no-network draw.io helpers (#424). The contract test never executes
+      // a tool body, so type-correct stubs suffice (the real functions can't be
+      // imported here — drawio-shapes.ts uses import.meta, incompatible with the
+      // CommonJS jest transform).
+      searchShapes: (() => []) as unknown as loader.SearchShapesFn,
+      getGuideSection: (() => ({
+        section: 'index',
+        content: '',
+        sections: [],
+      })) as unknown as loader.GetGuideSectionFn,
     });
     const service = new AiChatToolsService(
       tokenServiceStub as never,

apps/server/src/core/ai-chat/tools/tool-tiers.spec.ts

@@ -124,6 +124,13 @@ describe('deferred catalog ↔ live forUser() toolset partition (#332, F3)', ()
         return {} as DocmostClientLike;
       } as unknown as loader.DocmostClientCtor,
       sharedToolSpecs: SHARED_TOOL_SPECS as Record<string, loader.SharedToolSpec>,
+      // Pure no-network draw.io helpers (#424); tool bodies are never executed here.
+      searchShapes: (() => []) as unknown as loader.SearchShapesFn,
+      getGuideSection: (() => ({
+        section: 'index',
+        content: '',
+        sections: [],
+      })) as unknown as loader.GetGuideSectionFn,
     });
     const service = new AiChatToolsService(
       {

docs/reading-ai-logs.md

@@ -13,12 +13,14 @@ Read the **Gotchas** section before you trust any error count.
 - Agent chats live in Postgres, DB `docmost`, tables `ai_chat_*`.
 - Each tool invocation is stored as **two** array elements (a `tool-call` part and
   a `tool-result` part), so naive counting double-counts.
-- **A tool that *throws* writes no result part at all.** Its error text is nowhere
-  in the DB — not in `tool_calls`, `content`, or `metadata`. It is shown live in
-  the UI only. So `isError` / `success=false` scans under-report by design.
-- To find where agents fail you need **three** sources: (1) soft-failure markers in
-  `tool_calls`, (2) the orphan-gap proxy for thrown errors, (3) server logs / the
-  live UI for the actual error text.
+- **A tool that *throws* writes no result part.** Since the #407 fix its error is
+  persisted as a dedicated `{toolName, error}` element in `tool_calls` (queryable +
+  replayed to the model). **Rows written before #407 still drop it** — the error is
+  nowhere in the DB and shows only in the live UI. So `isError` / `success=false`
+  scans under-report by design, and pre-#407 thrown errors are invisible.
+- To find where agents fail: (1) soft-failure markers in `tool_calls`, (2) the new
+  `error` field for thrown errors (new rows) / the orphan-gap proxy (old rows),
+  (3) server logs / the live UI for full stack traces beyond the truncated message.
 
 ## Where the data lives
 
@@ -61,13 +63,17 @@ index 0: { "toolName": "getPage",  "input":  { "pageId": "…" } }   ← tool-ca
 index 1: { "toolName": "getPage",  "output": { … } }                ← tool-result (has output, NO input)
 ```
 
-The **only** keys that ever appear on an element are `toolName`, `input`, `output`.
-There is no `state`, no `errorText`, no `type`. Consequences:
+The keys that appear on an element are `toolName`, `input`, `output`, and — for a
+**thrown** failure on rows written after the #407 fix — `error` (the tool's error
+message; see the "Hard failures" section below). There is no `state`, no `errorText`,
+no `type`. On pre-#407 rows a thrown failure has NO paired result element at all
+(silent orphan). Consequences:
 
-1. **Real invocation count = elements that have `output`.** Counting every element
-   double-counts (you get ~2× and a spurious "~50% of every tool has no output").
-2. **Pairing:** a successful call = a `tool-call` part followed by its `tool-result`
-   part. Both carry `toolName`, so you can group by tool on either.
+1. **Real invocation count = elements that have `output` or `error`.** Counting every
+   element double-counts (you get ~2× and a spurious "~50% of every tool has no output").
+2. **Pairing:** a call = a `tool-call` part followed by its result part. A success
+   carries `output`; a thrown failure (post-#407) carries `error` instead. Both carry
+   `toolName`, so you can group by tool on either.
 
 ## The two classes of failure (and which the DB can see)
 
@@ -85,25 +91,37 @@ These are visible in the `tool-result` `output`. The marker differs per tool:
 Note `editPageText` returns `failed: []` on success — filtering on the *presence*
 of the key gives false positives; filter on **non-empty**.
 
-### 2. Hard failures — tool THREW → NOT PERSISTED ❌ (the trap)
+### 2. Hard failures — tool THREW → NOW PERSISTED ✅ (since the #407 fix)
 
 When a tool throws (the classic one is `patchNode` / `insertNode` / `tableUpdateCell`
 → `Failed to encode document to Yjs (fromJSON): Unknown node type: undefined`), the
-runtime writes **no `tool-result` part**. The orphaned `tool-call` part stays, but
-the error text is **nowhere in the DB**. It is streamed to the UI live and (until
-rotation) to server logs — that is it.
+runtime still writes **no `tool-result` part** — the failure is an ai@6 `tool-error`
+content part instead. **Since the #407 fix, that error is persisted**: `serializeSteps`
+appends a dedicated element `{toolName, error: "<message>"}` right after the failed
+call, mirroring how a successful `{toolName, output}` element is appended. So a thrown
+error now leaves a queryable `error` field carrying its (truncated) reason, and the
+same real text is replayed to the model on the next turn (an `output-error` part with
+the real `errorText`, no longer the `'Tool call did not complete.'` placeholder).
 
-So any query like `count(*) FILTER (WHERE output.success = false)` will happily
-return **0** for `patchNode` even when the chat is visibly full of red failures.
-That is survivorship bias, not reliability.
+**Cutover caveat — old rows keep the old blind shape.** Rows written **before** this
+change have the two-part shape (`call` + `output` only) and simply **drop** thrown
+errors, leaving a silent **orphan** (a `call` with no `output` *and* no `error`). Rows
+written **after** the fix additionally carry the `error` element. So:
 
-The only DB-side proxy for a thrown error is an **orphan**: a `tool-call` part with
-no matching `tool-result`. Caveat: orphans also appear when a run is **aborted**
-mid-flight (server restart), so a high-volume tool (`createComment`, `searchInPage`,
-`Search_web_search`) shows orphans from aborts, not from real errors. Treat the
-orphan gap as an *upper bound* on hard errors, and cross-check the tool: a gap on a
-structural editor (`patchNode`, `insertNode`, `updatePageJson`, `transformPage`) is
-almost certainly a thrown Yjs-encode error; a gap on `createComment` is mostly aborts.
+- **New rows:** query the `error` field directly (see the hard-error query below) — no
+  orphan heuristic needed for thrown failures.
+- **Old rows (pre-#407):** the only DB-side proxy is still an **orphan**: a `tool-call`
+  part with no matching `tool-result` *and* no `error`. Orphans also appear when a run
+  is **aborted** mid-flight (server restart), so a high-volume tool (`createComment`,
+  `searchInPage`, `Search_web_search`) shows orphans from aborts, not real errors on
+  old rows. Treat the orphan gap as an *upper bound*, and cross-check the tool: a gap on
+  a structural editor (`patchNode`, `insertNode`, `updatePageJson`, `transformPage`) is
+  almost certainly a thrown Yjs-encode error; a gap on `createComment` is mostly aborts.
+
+A note on the aborted-call fallback: a call with **neither** a result **nor** a
+`tool-error` (genuinely interrupted mid-step) still replays with the
+`'Tool call did not complete.'` placeholder and persists as an orphan — that path is
+unchanged, and is distinct from a real thrown error, which now carries `error`.
 
 ### 3. Run-level failures → `ai_chat_runs`
 
@@ -164,14 +182,28 @@ WHERE jsonb_typeof(o->'failed') = 'array'
 GROUP BY 1 ORDER BY 2 DESC;
 ```
 
-**Hard-error proxy — orphan gap per tool, WITH a spread column** (call parts minus
-result parts, plus how many distinct chats the gap is spread across):
+**Hard errors — persisted `error` field per tool (NEW rows, since #407)** — thrown
+tool failures now carry their real reason, so query them directly:
+
+```sql
+SELECT elem->>'toolName' AS tool, count(*) AS thrown_errors,
+       min(elem->>'error') AS sample_error
+FROM ai_chat_messages m, jsonb_array_elements(m.tool_calls) elem
+WHERE jsonb_typeof(m.tool_calls) = 'array' AND elem ? 'error'
+GROUP BY 1 ORDER BY 2 DESC;
+```
+
+**Hard-error proxy for OLD rows (pre-#407) — orphan gap per tool, WITH a spread column**
+(call parts minus result parts, plus how many distinct chats the gap is spread across).
+This covers rows written before thrown errors were persisted; on new rows a thrown
+failure now has its own `error` element (use the query above) and an orphan means only
+a genuinely aborted mid-step call:
 
 ```sql
 WITH parts AS (
   SELECT m.chat_id, elem->>'toolName' AS tool,
          (elem ? 'input' AND NOT (elem ? 'output')) AS is_call,
-         (elem ? 'output')                          AS is_result
+         (elem ? 'output' OR elem ? 'error')        AS is_result
   FROM ai_chat_messages m, jsonb_array_elements(m.tool_calls) elem
   WHERE jsonb_typeof(m.tool_calls) = 'array' AND m.role = 'assistant'
 ),
@@ -188,8 +220,12 @@ HAVING sum(gap) FILTER (WHERE gap > 0) > 0
 ORDER BY missing_results DESC;
 ```
 
-**`missing_results` mixes thrown errors AND aborted/interrupted runs — you cannot
-split them from `output` alone** (a positional "what follows the orphan" heuristic
+The `is_result` predicate counts an `error` element as a paired result too, so on new
+rows a persisted thrown error no longer inflates the orphan gap; a remaining gap is an
+aborted/interrupted call.
+
+**On OLD rows, `missing_results` mixes thrown errors AND aborted/interrupted runs — you
+cannot split them from `output` alone** (a positional "what follows the orphan" heuristic
 breaks on parallel tool batches, which persist as `call,call,…,result,result`). Use
 `chats_spread` to disambiguate:
 
@@ -244,18 +280,20 @@ docker compose -p gitmost logs -f --tail=100 # whole stack
 ```
 
 Logging is `json-file`, `max-size=10m max-file=5` → ~50 MB retained, then rotated,
-and **wiped on container recreate**. So thrown-tool error text is only reliably
-caught **in real time** (or in the live chat UI, which renders the failed part with
-its message). There is no durable, queryable store of hard tool errors today — if you
-need one, that is a feature to add (persist `output-error` parts, or emit a
-`tool_calls_total{tool,status}` metric to VictoriaMetrics).
+and **wiped on container recreate**. Since the #407 fix, thrown-tool error text is
+**persisted in the `error` field** of `tool_calls` (see the hard-error query above), so
+you no longer depend on live logs for it. Logs/live UI remain useful for **pre-#407
+rows** (whose thrown errors were dropped) and for full stack traces beyond the
+truncated stored message. A per-tool `tool_calls_total{tool,status}` metric to
+VictoriaMetrics is still a possible future add for aggregate dashboards.
 
 ## Gotchas checklist
 
-- [ ] Counting every `tool_calls` element → **2× overcount**. Count elements with `output`.
-- [ ] `isError` / `success=false` ≈ 0 does **not** mean "no errors" — thrown errors aren't persisted.
+- [ ] Counting every `tool_calls` element → **overcount**. Count `output` elements; add `error` elements for thrown failures (new rows), but don't count both as invocations.
+- [ ] `isError` / `success=false` ≈ 0 does **not** mean "no errors" — thrown errors are a separate `error` element (new rows) or dropped entirely (pre-#407 rows).
+- [ ] Thrown errors persist only on rows written **after the #407 fix** — pre-#407 rows still drop them (orphan only). Mind the cutover when trending over time.
 - [ ] `editPageText.failed` is `[]` on success — test for **non-empty**, not presence.
-- [ ] Orphan gap mixes thrown errors **and** aborted runs — split by tool before concluding.
+- [ ] Orphan gap on OLD rows mixes thrown errors **and** aborted runs — split by tool. On NEW rows a thrown error is its own `error` element, so a gap ≈ aborted call.
 - [ ] `aborted` runs = server restarts, `failed` runs = provider overload — not agent mistakes.
 - [ ] Never dump a raw `tool_calls` cell — it can be hundreds of KB.
 - [ ] Logs are ephemeral (≤50 MB, wiped on recreate) — grab hard-error text live.

packages/mcp/package.json

@@ -49,9 +49,11 @@
     "@tiptap/starter-kit": "3.20.4",
     "@types/jsdom": "^27.0.0",
     "axios": "^1.6.0",
+    "elkjs": "^0.11.1",
     "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,10 +8,6 @@ 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,
@@ -24,11 +20,10 @@ 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 {
@@ -40,6 +35,7 @@ import {
   deleteNodeById,
   assertUnambiguousMatch,
   insertNodeRelative,
+  blockPlainText,
   buildOutline,
   getNodeByRef,
   readTable,
@@ -49,6 +45,16 @@ 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 { applyElkLayout } from "./lib/drawio-layout.js";
 import {
   applyTextEdits,
   TextEdit,
@@ -59,10 +65,12 @@ import { getCollabToken, performLogin } from "./lib/auth-utils.js";
 import { diffDocs, summarizeChange } from "./lib/diff.js";
 import {
   applyAnchorInDoc,
-  canAnchorInDoc,
   countAnchorMatches,
   getAnchoredText,
+  resolveAnchorSelection,
+  normalizeForMatch,
 } from "./lib/comment-anchor.js";
+import { closestBlockHint } from "./lib/text-normalize.js";
 import {
   blockText,
   walk,
@@ -417,177 +425,32 @@ export class DocmostClient {
    * change report. The report is computed AFTER the atomic read->write and
    * never throws.
    */
-  private mutateLiveContentUnlocked(
+  private async mutateLiveContentUnlocked(
     pageId: string,
     collabToken: string,
     transform: (liveDoc: any) => any | null,
   ): Promise<MutationResult> {
-    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"),
-          );
-        },
-      });
+    // 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),
     });
+    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;
+    }
   }
 
   /**
@@ -2474,6 +2337,64 @@ export class DocmostClient {
     };
   }
 
+  /** Plain text of each TOP-LEVEL block of `doc`, for anchor-failure hints. */
+  private topLevelBlockTexts(doc: any): string[] {
+    const content = doc && Array.isArray(doc.content) ? doc.content : [];
+    return content
+      .map((b: any) => blockPlainText(b))
+      .filter((t: string) => t.length > 0);
+  }
+
+  /**
+   * True when per-block anchoring failed but the (normalized) selection DOES
+   * appear in the blocks' joined plain text — i.e. it straddles a block
+   * boundary. Blocks are joined with a newline (collapsed to one space by
+   * normalizeForMatch) so a selection whose parts are separated by a paragraph
+   * break still matches. Callers only reach here after single-block anchoring
+   * (incl. the markdown-strip fallback) has already failed.
+   */
+  private selectionSpansMultipleBlocks(
+    blockTexts: string[],
+    selection: string,
+  ): boolean {
+    const normSel = normalizeForMatch(selection).norm.trim();
+    if (normSel.length === 0) return false;
+    const joined = normalizeForMatch(blockTexts.join("\n")).norm;
+    return joined.indexOf(normSel) !== -1;
+  }
+
+  /**
+   * Build the actionable error for a create_comment anchor MISS, porting
+   * edit_page_text's self-correction affordances: an explicit "spans multiple
+   * blocks" message when the selection straddles a block boundary, otherwise a
+   * "closest block text" hint quoting the block that holds the selection's
+   * longest token. `live` switches the wording between the pre-check (reading the
+   * persisted page) and the post-create live-anchor failure (which rolls back).
+   */
+  private anchorNotFoundError(
+    doc: any,
+    selection: string,
+    live: boolean,
+  ): Error {
+    const blockTexts = this.topLevelBlockTexts(doc);
+    const rolled = live ? " The comment was rolled back." : "";
+    if (this.selectionSpansMultipleBlocks(blockTexts, selection)) {
+      return new Error(
+        "create_comment: the selection spans multiple blocks; anchor on a " +
+          "contiguous fragment within a SINGLE paragraph/block (<=250 chars)." +
+          rolled,
+      );
+    }
+    const where = live ? "in the live document" : "in the page";
+    return new Error(
+      `create_comment: could not find the selection text ${where} to anchor ` +
+        "the comment. Provide the EXACT contiguous text from a single " +
+        "paragraph/block (<=250 chars)." +
+        closestBlockHint(blockTexts, selection) +
+        rolled,
+    );
+  }
+
   /**
    * Create an inline comment anchored to its `selection` text, or a reply.
    *
@@ -2535,6 +2456,10 @@ export class DocmostClient {
     // Captured in the pre-check below (which already reads the page) and used as
     // payload.selection. Ordinary comments keep sending the raw agent selection.
     let anchoredSelection: string | null = null;
+    // Set when the anchor matched only after stripping markdown from the
+    // selection (the strip fallback); surfaced as a soft warning like
+    // edit_page_text does, so a stale-markdown selection is flagged.
+    let anchorNormalized = false;
 
     // For a top-level comment, fail BEFORE creating anything when the selection
     // is not present in the persisted document — this avoids leaving an orphan
@@ -2550,10 +2475,7 @@ export class DocmostClient {
           // rejected BEFORE creating the comment.
           const matches = countAnchorMatches(page.content, selection);
           if (matches === 0) {
-            throw new Error(
-              "create_comment: could not find the selection text in the page to anchor the comment. " +
-                "Provide the EXACT contiguous text from a single paragraph/block (<=250 chars).",
-            );
+            throw this.anchorNotFoundError(page.content, selection, false);
           }
           if (matches >= 2) {
             throw new Error(
@@ -2567,18 +2489,27 @@ export class DocmostClient {
           // null despite countAnchorMatches===1 (shouldn't happen), fall back to
           // the raw agent selection below rather than crash.
           anchoredSelection = getAnchoredText(page.content, selection);
-        } else if (!canAnchorInDoc(page.content, selection)) {
-          throw new Error(
-            "create_comment: could not find the selection text in the page to anchor the comment. " +
-              "Provide the EXACT contiguous text from a single paragraph/block (<=250 chars).",
-          );
+          anchorNormalized = resolveAnchorSelection(
+            page.content,
+            selection,
+          ).normalized;
+        } else {
+          const resolved = resolveAnchorSelection(page.content, selection);
+          if (!resolved.found) {
+            throw this.anchorNotFoundError(page.content, selection, false);
+          }
+          anchorNormalized = resolved.normalized;
         }
       } catch (e) {
-        // Rethrow our own "not found"/"ambiguous" errors; swallow read/network
-        // errors so the live anchor step can still try (and enforce) anchoring.
+        // Rethrow our own "not found"/"ambiguous"/"spans multiple blocks" errors;
+        // swallow read/network errors so the live anchor step can still try (and
+        // enforce) anchoring.
         if (
           e instanceof Error &&
           (e.message.startsWith("create_comment: could not find the selection") ||
+            e.message.startsWith(
+              "create_comment: the selection spans multiple blocks",
+            ) ||
             e.message.startsWith(
               "create_comment: the suggestion's selection is ambiguous",
             ))
@@ -2650,6 +2581,10 @@ export class DocmostClient {
     // Set inside the transform when a suggestion's live anchor is ambiguous
     // (>=2 occurrences), so the rollback path can surface the right error.
     let ambiguousInLiveDoc = false;
+    // Captured inside the transform on a not-found abort, so the rollback path
+    // can surface the closest-block / spans-multiple-blocks hint built from the
+    // LIVE document (the pre-check page is not in scope there).
+    let liveNotFoundError: Error | null = null;
     try {
       const collabToken = await this.getCollabTokenWithReauth();
       // Open the collab doc by the canonical UUID, never the slugId (#260). The
@@ -2677,6 +2612,13 @@ export class DocmostClient {
             const liveCount = countAnchorMatches(doc, selection as string);
             if (liveCount !== 1) {
               ambiguousInLiveDoc = liveCount >= 2;
+              if (liveCount === 0) {
+                liveNotFoundError = this.anchorNotFoundError(
+                  doc,
+                  selection as string,
+                  true,
+                );
+              }
               return null;
             }
           }
@@ -2686,6 +2628,11 @@ export class DocmostClient {
           }
           // Selection text not found in the LIVE document: abort the write. The
           // rollback + throw below turns this into a hard error.
+          liveNotFoundError = this.anchorNotFoundError(
+            doc,
+            selection as string,
+            true,
+          );
           return null;
         },
       );
@@ -2702,13 +2649,28 @@ export class DocmostClient {
       // suggestion, was ambiguous) in the live document. Roll back the comment
       // and surface a hard error.
       await this.safeDeleteComment(newCommentId);
-      throw new Error(
-        ambiguousInLiveDoc
-          ? "create_comment: the suggestion's selection is ambiguous in the live document (multiple occurrences); the comment was rolled back. Expand the selection with surrounding context so it is unique."
-          : "create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back",
+      if (ambiguousInLiveDoc) {
+        throw new Error(
+          "create_comment: the suggestion's selection is ambiguous in the live document (multiple occurrences); the comment was rolled back. Expand the selection with surrounding context so it is unique.",
+        );
+      }
+      throw (
+        liveNotFoundError ??
+        new Error(
+          "create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back",
+        )
       );
     }
 
+    // Soft warning (like edit_page_text): the selection only matched after
+    // stripping markdown, so the caller likely quoted a styled fragment.
+    if (anchorNormalized) {
+      result.warning =
+        "The selection matched only after stripping markdown syntax; the comment " +
+        "was anchored on the document's plain text. Copy the selection verbatim " +
+        "from get_page / search_in_page output to avoid this.";
+    }
+
     result.anchored = true;
     return result;
   }
@@ -3432,6 +3394,461 @@ 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,
+    layout?: "elk",
+  ): 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`,
+        );
+      }
+    }
+
+    // Optional server-side ELK auto-layout: the model declares structure with
+    // rough coords, ELK computes the pixels (best-effort — returns the input
+    // unchanged on any layout failure).
+    const laidOutXml = layout === "elk" ? await applyElkLayout(xml) : xml;
+    // Pre-write pipeline (throws a structured DrawioLintError on any violation).
+    const prepared = prepareModel(laidOutXml);
+    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,
+    layout?: "elk",
+  ): 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.`,
+      );
+    }
+
+    // Optional server-side ELK auto-layout (best-effort; see drawioCreate).
+    const laidOutXml = layout === "elk" ? await applyElkLayout(xml) : xml;
+    // Pipeline for the new content (throws a structured DrawioLintError).
+    const prepared = prepareModel(laidOutXml);
+    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

@@ -5,6 +5,8 @@ import { fileURLToPath } from "url";
 import { dirname, join } from "path";
 import { DocmostClient, DocmostMcpConfig } from "./client.js";
 import { parseNodeArg } from "./lib/parse-node-arg.js";
+import { searchShapes } from "./lib/drawio-shapes.js";
+import { getGuideSection } from "./lib/drawio-guide.js";
 import { SHARED_TOOL_SPECS, SharedToolSpec } from "./tool-specs.js";
 
 // Re-export the client and its config type so embedding hosts (e.g. the gitmost
@@ -13,12 +15,25 @@ 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()).
 export { SHARED_TOOL_SPECS } from "./tool-specs.js";
 export type { SharedToolSpec } from "./tool-specs.js";
 
+// Re-export the pure, no-network draw.io helpers (#424) so the in-app AI-SDK
+// service can wire drawio_shapes / drawio_guide off the loaded module. These are
+// NOT client methods (no page/backend hit) — the in-app handler calls them
+// directly, mirroring how the standalone MCP server wires them here.
+export { searchShapes } from "./lib/drawio-shapes.js";
+export type { SearchShapesOptions } from "./lib/drawio-shapes.js";
+export { getGuideSection } from "./lib/drawio-guide.js";
+
 // Read version from package.json
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = dirname(__filename);
@@ -47,7 +62,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); before authoring a diagram, drawio_shapes (look up verified stencil style-strings so a shape name never renders as an empty box) and drawio_guide (on-demand authoring reference: skeleton/layout/containers/icons-aws/icons-azure), and pass layout:\"elk\" to drawio_create/drawio_update to auto-place nodes. 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.";
@@ -460,6 +475,56 @@ 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, layout }) => {
+    const result = await docmostClient.drawioCreate(
+      pageId,
+      { position, anchorNodeId, anchorText },
+      xml,
+      title,
+      layout,
+    );
+    return jsonContent(result);
+  },
+);
+
+// Tool: drawio_update — optimistic-locked full replacement of a diagram.
+registerShared(
+  SHARED_TOOL_SPECS.drawioUpdate,
+  async ({ pageId, node, xml, baseHash, layout }) => {
+    const result = await docmostClient.drawioUpdate(
+      pageId,
+      node,
+      xml,
+      baseHash,
+      layout,
+    );
+    return jsonContent(result);
+  },
+);
+
+// Tool: drawio_shapes — verified stencil-style lookup (no network; #424).
+registerShared(SHARED_TOOL_SPECS.drawioShapes, async ({ query, category, limit }) => {
+  const results = searchShapes(query, { category, limit });
+  return jsonContent({ query, count: results.length, results });
+});
+
+// Tool: drawio_guide — on-demand draw.io authoring reference (no network; #424).
+registerShared(SHARED_TOOL_SPECS.drawioGuide, async ({ section }) => {
+  return jsonContent(getGuideSection(section));
+});
+
 // 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

@@ -0,0 +1,668 @@
+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,4 +1,3 @@
-import { HocuspocusProvider } from "@hocuspocus/provider";
 import { TiptapTransformer } from "@hocuspocus/transformer";
 import * as Y from "yjs";
 import WebSocket from "ws";
@@ -16,7 +15,8 @@ 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 { summarizeChange, VerifyReport } from "./diff.js";
+import { VerifyReport } from "./diff.js";
+import { acquireCollabSession } from "./collab-session.js";
 
 export { markdownToProseMirror };
 
@@ -194,26 +194,27 @@ 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. 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;
+ *   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);
  *   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).
@@ -230,7 +231,7 @@ export async function mutatePageContent(
   baseUrl: string,
   transform: (liveDoc: any) => any | null,
 ): Promise<MutationResult> {
-  return withPageLock(pageId, () => {
+  return withPageLock(pageId, async () => {
     if (process.env.DEBUG) {
       console.error(`Starting realtime content mutate for page ${pageId}`);
       // Token prefix is sensitive; only log it under DEBUG.
@@ -239,202 +240,15 @@ export async function mutatePageContent(
       );
     }
 
-    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"),
-          );
-        },
-      });
-    });
+    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;
+    }
   });
 }
 

packages/mcp/src/lib/comment-anchor.ts

@@ -17,8 +17,23 @@
  * comparing and match across maximal runs of consecutive text nodes within a
  * single block, while mapping every normalized character back to its raw index
  * so the mark lands on the exact original characters.
+ *
+ * MARKDOWN-STRIP FALLBACK: when the agent copies a selection that still carries
+ * inline markdown (`**bold**`, `` `code` ``, `[t](u)`), the raw locator will not
+ * match the document's plain text. Exactly like edit_page_text's json-edit
+ * fallback, we first try the verbatim selection and, ONLY if it anchors nowhere
+ * in the whole document, retry with `stripInlineMarkdown` applied. `canAnchorInDoc`,
+ * `getAnchoredText` and `applyAnchorInDoc` share this decision via
+ * `resolveAnchorSelection`. `countAnchorMatches` keeps its OWN parallel exact-wins
+ * implementation (it needs a raw match COUNT, not a single resolved locator), kept
+ * deliberately in sync with `resolveAnchorSelection`: raw match ⇒ use raw, else fall
+ * back to the stripped count. All four therefore agree on which locator matched —
+ * the suggestion-uniqueness gate depends on count and can/get never disagreeing, so
+ * these two exact-wins implementations MUST stay in sync if either is changed.
  */
 
+import { stripInlineMarkdown } from "./text-normalize.js";
+
 /** Typographic double-quote variants mapped to ASCII `"`. */
 const DOUBLE_QUOTES = "«»„“”‟〝〞＂";
 /** Typographic single-quote/apostrophe variants mapped to ASCII `'`. */
@@ -214,15 +229,17 @@ function reconstructRawText(blockContent: any[], match: AnchorMatch): string {
  * un-appliable (spurious 409).
  */
 export function getAnchoredText(doc: any, selection: string): string | null {
+  const { selection: effective, found } = resolveAnchorSelection(doc, selection);
+  if (!found) return null;
   const visit = (node: any, depth: number): string | null => {
     if (depth > MAX_DEPTH || !node || typeof node !== "object") return null;
     if (!Array.isArray(node.content)) return null;
-    const match = findAnchorInBlock(node.content, selection);
+    const match = findAnchorInBlock(node.content, effective);
     if (match) return reconstructRawText(node.content, match);
     for (const child of node.content) {
       if (child && typeof child === "object" && Array.isArray(child.content)) {
-        const found = visit(child, depth + 1);
-        if (found !== null) return found;
+        const foundText = visit(child, depth + 1);
+        if (foundText !== null) return foundText;
       }
     }
     return null;
@@ -231,12 +248,11 @@ export function getAnchoredText(doc: any, selection: string): string | null {
 }
 
 /**
- * Depth-first, document-order check for whether `selection` can be anchored
- * anywhere in `doc`. At each node with an array `content`, first try to match
- * within that node's own content, then recurse into children that themselves
- * have a `content` array.
+ * RAW (no markdown-strip fallback) depth-first check that `selection` anchors
+ * somewhere in `doc`. This is the primitive `resolveAnchorSelection` builds on;
+ * public callers should use `canAnchorInDoc`, which adds the strip fallback.
  */
-export function canAnchorInDoc(doc: any, selection: string): boolean {
+function rawCanAnchorInDoc(doc: any, selection: string): boolean {
   const visit = (node: any, depth: number): boolean => {
     if (depth > MAX_DEPTH || !node || typeof node !== "object") return false;
     if (!Array.isArray(node.content)) return false;
@@ -251,6 +267,43 @@ export function canAnchorInDoc(doc: any, selection: string): boolean {
   return visit(doc, 0);
 }
 
+/**
+ * Decide the locator that ACTUALLY anchors `selection` in `doc`, applying the
+ * markdown-strip fallback once (so every public entry point agrees):
+ *  - EXACT WINS: if the verbatim selection anchors anywhere, use it as-is.
+ *  - FALLBACK: only if the verbatim selection anchors nowhere, and the
+ *    markdown-stripped form differs and DOES anchor, use the stripped form and
+ *    flag `normalized` so callers can surface a soft warning.
+ *  - otherwise `found` is false and `selection` is returned unchanged.
+ *
+ * The stripped form is used ONLY to LOCATE the anchor; getAnchoredText still
+ * reconstructs and stores the RAW document substring, so the strip never leaks
+ * into what gets persisted.
+ */
+export function resolveAnchorSelection(
+  doc: any,
+  selection: string,
+): { selection: string; found: boolean; normalized: boolean } {
+  if (rawCanAnchorInDoc(doc, selection)) {
+    return { selection, found: true, normalized: false };
+  }
+  const stripped = stripInlineMarkdown(selection);
+  if (stripped !== selection && rawCanAnchorInDoc(doc, stripped)) {
+    return { selection: stripped, found: true, normalized: true };
+  }
+  return { selection, found: false, normalized: false };
+}
+
+/**
+ * Depth-first, document-order check for whether `selection` can be anchored
+ * anywhere in `doc` (with the markdown-strip fallback). At each node with an
+ * array `content`, first try to match within that node's own content, then
+ * recurse into children that themselves have a `content` array.
+ */
+export function canAnchorInDoc(doc: any, selection: string): boolean {
+  return resolveAnchorSelection(doc, selection).found;
+}
+
 /**
  * Split the matched text nodes and splice the comment mark across the range.
  * `blockContent` is mutated IN PLACE. `match.startChild..endChild` are all text
@@ -315,7 +368,7 @@ function spliceCommentMark(
  * not use this. (Note: counts OCCURRENCES, not just matching blocks, so two
  * occurrences inside one block are correctly reported as 2.)
  */
-export function countAnchorMatches(doc: any, selection: string): number {
+function rawCountAnchorMatches(doc: any, selection: string): number {
   const normSel = normalizeForMatch(selection).norm.trim();
   if (normSel.length === 0) return 0;
 
@@ -369,6 +422,25 @@ export function countAnchorMatches(doc: any, selection: string): number {
   return total;
 }
 
+/**
+ * Uniqueness gate for suggestions, with the SAME markdown-strip fallback as the
+ * other entry points so count never disagrees with can/get/apply. EXACT WINS: if
+ * the verbatim selection occurs at all, return its raw occurrence count (so a
+ * selection that is unique raw stays unique — the fallback never runs and cannot
+ * introduce a spurious second match). Only when the verbatim selection is absent
+ * do we count occurrences of the markdown-stripped form.
+ */
+export function countAnchorMatches(doc: any, selection: string): number {
+  const raw = rawCountAnchorMatches(doc, selection);
+  if (raw > 0) return raw;
+  const stripped = stripInlineMarkdown(selection);
+  if (stripped !== selection) {
+    const strippedCount = rawCountAnchorMatches(doc, stripped);
+    if (strippedCount > 0) return strippedCount;
+  }
+  return 0;
+}
+
 /**
  * Depth-first (same order as canAnchorInDoc) over `doc`; on the FIRST block
  * whose content matches `selection`, splice the comment mark across the matched
@@ -380,10 +452,12 @@ export function applyAnchorInDoc(
   selection: string,
   commentId: string,
 ): boolean {
+  const { selection: effective, found } = resolveAnchorSelection(doc, selection);
+  if (!found) return false;
   const visit = (node: any, depth: number): boolean => {
     if (depth > MAX_DEPTH || !node || typeof node !== "object") return false;
     if (!Array.isArray(node.content)) return false;
-    const match = findAnchorInBlock(node.content, selection);
+    const match = findAnchorInBlock(node.content, effective);
     if (match) {
       spliceCommentMark(node.content, match, commentId);
       return true;

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

@@ -0,0 +1,228 @@
+// Progressive-disclosure authoring reference for the `drawio_guide` tool
+// (issue #424, stage 2). The FULL draw.io authoring guide would bloat every
+// context window, so it is split into small sections the model reads on demand:
+//   skeleton | layout | containers | icons-aws | icons-azure
+// Content is written directly from the issue #424 appendix (the layout
+// heuristics, container rules, AWS icon patterns + gotchas + blocklist, and
+// Azure image-style paths). ACCEPTANCE: each section stays <= ~4 KB so pulling
+// one is cheap.
+
+export type GuideSection =
+  | "skeleton"
+  | "layout"
+  | "containers"
+  | "icons-aws"
+  | "icons-azure";
+
+export const GUIDE_SECTIONS: GuideSection[] = [
+  "skeleton",
+  "layout",
+  "containers",
+  "icons-aws",
+  "icons-azure",
+];
+
+const SKELETON = `# drawio_guide: skeleton
+
+Canonical mxGraph skeleton. id="0" and id="1" are MANDATORY sentinels; every
+real cell has parent="1" (or a container id). Set adaptiveColors="auto" on the
+model so Docmost's dark theme adapts strokeColor/fillColor/fontColor="default".
+
+\`\`\`xml
+<mxGraphModel dx="800" dy="600" grid="1" gridSize="10" adaptiveColors="auto"
+    page="1" pageWidth="850" pageHeight="1100">
+  <root>
+    <mxCell id="0"/>
+    <mxCell id="1" parent="0"/>
+    <mxCell id="2" value="Start" style="rounded=1;whiteSpace=wrap;html=1;"
+        vertex="1" parent="1">
+      <mxGeometry x="40" y="40" width="140" height="60" as="geometry"/>
+    </mxCell>
+    <mxCell id="3" value="Store" style="shape=cylinder3;whiteSpace=wrap;html=1;"
+        vertex="1" parent="1">
+      <mxGeometry x="40" y="200" width="80" height="80" as="geometry"/>
+    </mxCell>
+    <mxCell id="e1" edge="1" parent="1" source="2" target="3"
+        style="edgeStyle=orthogonalEdgeStyle;rounded=1;html=1;">
+      <mxGeometry relative="1" as="geometry"/>
+    </mxCell>
+  </root>
+</mxGraphModel>
+\`\`\`
+
+Three accepted inputs to drawio_create/drawio_update: a bare <mxGraphModel>, a
+full <mxfile> (decoded to its first page), or a raw list of <mxCell> (the server
+wraps it and adds the id=0/id=1 sentinels).
+
+Hard rules: a cell is vertex="1" XOR edge="1" (a container/group is neither);
+every edge has a child <mxGeometry relative="1" as="geometry"/>; ids are unique;
+no XML comments; put html=1 in styles and XML-escape value (& -> &amp;,
+< -> &lt;); a newline in a label is &#xa;, never a literal \\n. Don't guess
+shape=mxgraph.* names — call drawio_shapes first (a wrong name renders empty).`;
+
+const LAYOUT = `# drawio_guide: layout
+
+Turn "make it look good" into checkable numbers. Or pass layout:"elk" to
+drawio_create/drawio_update and the server computes coordinates for you (ELK
+layered layout, honouring nested containers) — you declare structure, it places
+pixels.
+
+Spacing (when placing by hand):
+- Horizontal gap between shapes 200-220px; vertical between rows/lanes 250px;
+  auxiliary services (monitoring, DLQ) sit below the main flow with 280px+ gap.
+- Coordinates are multiples of 10 (grid). Base sizes: rectangle 140x60, diamond
+  140x80, circle 60x60; cloud icons 78x78 primary / 65x65 secondary; font 12px.
+- Main flow left-to-right, one primary axis; <=3-4 lanes/zones; one icon/service.
+
+Edges:
+- <=1 bend per edge (ideally 0); an edge must not cross another shape's bbox;
+  two edges must not lie on top of each other.
+- Give explicit exitX/exitY/entryX/entryY for every non-straight link or the
+  orthogonal router drives lines through shapes. Vertical link:
+  exitX=0.5;exitY=1 -> entryX=0.5;entryY=0. For 2+ links on one node, spread the
+  attach points 0.25 / 0.5 / 0.75.
+- Base edge style:
+  edgeStyle=orthogonalEdgeStyle;rounded=1;orthogonalLoop=1;jettySize=auto;html=1;strokeWidth=2;exitX=1;exitY=0.5;entryX=0;entryY=0.5;
+- Edge labels: 1-2 words max, labelBackgroundColor=#F5F5F5;fontSize=11;. Don't
+  label an obvious flow (Lambda->DynamoDB needs no "Write"); prefer numbering
+  stages (1,2,3) over many labels.
+- Line semantics: solid = main/sync; dashed=1 = async; red dashed
+  strokeColor=#DD344C = error path.
+
+Alignment: centre a child under its parent by math, not by eye:
+child.x = parent.center_x - child.width/2.
+
+The linter returns quality WARNINGS (bbox overlap, edge through a shape,
+edge-on-edge, gap <150px, label wider than its shape, negative/off-page coords).
+They do not block the write — fix them and retry, max 2 iterations.`;
+
+const CONTAINERS = `# drawio_guide: containers
+
+Groups/zones are TRANSPARENT containers. A coloured group fill is an instant
+"AI-generated" tell — never fill a group.
+
+- Every group: container=1;dropTarget=1;fillColor=none;. It is a cell with
+  vertex unset AND edge unset.
+- Children set parent="<groupId>" and their coordinates are RELATIVE to the
+  group's top-left, not absolute.
+- An edge between cells in DIFFERENT containers must be parent="1" (the layer),
+  otherwise it is clipped to one container and disappears.
+- Keep the group title off the group icon:
+  spacingLeft=40;spacingTop=-4;.
+- Leave >=30px padding between children and the group frame.
+- Draw edges on the BACK layer (place their <mxCell> BEFORE the shapes in XML)
+  and keep >=20px between an arrow and a label.
+
+Swimlanes: style=swimlane;horizontal=0;startSize=110;. Lanes are parent="1";
+their members are children of the lane.
+
+Example (transparent zone with two children and an internal edge):
+\`\`\`xml
+<mxCell id="z1" value="VPC" style="rounded=0;container=1;dropTarget=1;fillColor=none;verticalAlign=top;spacingLeft=40;spacingTop=-4;html=1;"
+    vertex="1" parent="1">
+  <mxGeometry x="40" y="40" width="320" height="200" as="geometry"/>
+</mxCell>
+<mxCell id="a" value="App" style="rounded=1;html=1;" vertex="1" parent="z1">
+  <mxGeometry x="30" y="40" width="120" height="60" as="geometry"/>
+</mxCell>
+<mxCell id="b" value="DB" style="shape=cylinder3;html=1;" vertex="1" parent="z1">
+  <mxGeometry x="30" y="120" width="80" height="60" as="geometry"/>
+</mxCell>
+<mxCell id="ab" edge="1" parent="z1" source="a" target="b">
+  <mxGeometry relative="1" as="geometry"/>
+</mxCell>
+\`\`\``;
+
+const ICONS_AWS = `# drawio_guide: icons-aws
+
+Two mutually-exclusive AWS icon patterns — mixing them is the #1 cause of empty
+boxes. Always call drawio_shapes for the exact resIcon name; do not guess.
+
+| Level | style | strokeColor |
+|---|---|---|
+| Service | shape=mxgraph.aws4.resourceIcon;resIcon=mxgraph.aws4.<NAME> | #ffffff (required) |
+| Resource | shape=mxgraph.aws4.<NAME> | none (required) |
+
+Full service-level template (fillColor is REQUIRED — the glyph is invisible in
+PNG export without it):
+\`\`\`
+sketch=0;outlineConnect=0;fontColor=#232F3E;fillColor=<category>;strokeColor=#ffffff;dashed=0;verticalLabelPosition=bottom;verticalAlign=top;align=center;html=1;fontSize=12;aspect=fixed;shape=mxgraph.aws4.resourceIcon;resIcon=mxgraph.aws4.<NAME>
+\`\`\`
+
+Category fillColor: Compute #ED7100, Networking #8C4FFF, Database #C925D1,
+Storage #3F8624, Security #DD344C, Integration #E7157B, AI/ML #01A88D.
+
+Rebrandings (stencil name lags the product name):
+- Amazon OpenSearch -> resIcon elasticsearch_service (renamed 2021)
+- Amazon EventBridge -> resIcon eventbridge (was CloudWatch Events)
+- VPC Peering -> resIcon peering (NOT vpc_peering -> empty box)
+- Amazon MSK -> resIcon managed_streaming_for_kafka (NOT msk)
+- IAM Identity Center -> resIcon single_sign_on (NOT iam_identity_center)
+
+Blocklist -> replacement: dynamodb_table -> dynamodb; general_saml_token ->
+traditional_server; kinesis_data_streams is unreliable. An unknown service ->
+generic resIcon=mxgraph.aws4.general_AWScloud WITH a label; an unnamed coloured
+rectangle is forbidden.
+
+Group stencils (transparent containers): AWS Cloud group_aws_cloud_alt, VPC
+group_vpc2, Subnet group_security_group, Account group_account; subnets use
+shape=mxgraph.aws4.group;grIcon=mxgraph.aws4.group_public_subnet;.`;
+
+const ICONS_AZURE = `# drawio_guide: icons-azure
+
+shape=mxgraph.azure2.* does NOT render in every host. Use the portable
+image-style instead:
+\`\`\`
+image;aspect=fixed;html=1;image=img/lib/azure2/<category>/<Icon>.svg;
+\`\`\`
+
+Known working paths:
+- networking/Front_Doors.svg
+- app_services/API_Management_Services.svg
+- databases/Azure_Cosmos_DB.svg
+- identity/Managed_Identities.svg
+- management_governance/Monitor.svg
+- devops/Application_Insights.svg
+
+For maximum robustness (e.g. PNG export on a host without the bundled lib), use
+an absolute URL fallback for the image:
+\`\`\`
+https://raw.githubusercontent.com/jgraph/drawio/dev/src/main/webapp/img/lib/azure2/<category>/<Icon>.svg
+\`\`\`
+
+Call drawio_shapes with the service name (e.g. "cosmos", "api management",
+"front door") to get the exact image-style string and default 68x68 size.`;
+
+const CONTENT: Record<GuideSection, string> = {
+  skeleton: SKELETON,
+  layout: LAYOUT,
+  containers: CONTAINERS,
+  "icons-aws": ICONS_AWS,
+  "icons-azure": ICONS_AZURE,
+};
+
+/**
+ * Return one guide section, or (when `section` is omitted/unknown) an index
+ * listing the available sections plus a one-line summary each. Each section is
+ * kept under ~4 KB so pulling it does not bloat the model's context.
+ */
+export function getGuideSection(section?: string): {
+  section: string;
+  content: string;
+  sections: GuideSection[];
+} {
+  const key = (section ?? "").trim().toLowerCase() as GuideSection;
+  if (section && GUIDE_SECTIONS.includes(key)) {
+    return { section: key, content: CONTENT[key], sections: GUIDE_SECTIONS };
+  }
+  const index =
+    "# drawio_guide\n\nProgressive-disclosure draw.io authoring reference. " +
+    "Call drawio_guide(section) with one of:\n" +
+    "- skeleton — canonical mxGraph XML, sentinels, the three accepted inputs, hard rules\n" +
+    "- layout — spacing heuristics, edge routing, the layout:\"elk\" option, quality warnings\n" +
+    "- containers — transparent groups, relative child coords, cross-container edges, swimlanes\n" +
+    "- icons-aws — the service/resource icon patterns, category colors, rebrandings, blocklist\n" +
+    "- icons-azure — the portable image-style paths\n\n" +
+    "Also call drawio_shapes(query) for verified stencil style-strings.";
+  return { section: "index", content: index, sections: GUIDE_SECTIONS };
+}

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

@@ -0,0 +1,205 @@
+// ELK auto-layout for draw.io models (issue #424, stage 2). The model declares
+// the LOGICAL structure (which nodes exist, which containers nest which
+// children, which edges connect what) with rough or arbitrary coordinates; this
+// module runs an Eclipse Layout Kernel "layered" pass (via elkjs — a pure-JS
+// port, no native/browser deps) that HONOURS nested containers as compound
+// nodes, then rewrites every vertex's <mxGeometry> with the computed pixels.
+//
+// Principle: "the model declares logical structure, the server computes pixels."
+// Coordinates ELK returns for a node are relative to its parent, which is
+// exactly mxGraph's convention for a child of a container, so they map across
+// directly. Container sizes are computed by ELK; leaf sizes are preserved.
+
+import ELK from "elkjs/lib/elk.bundled.js";
+import { JSDOM } from "jsdom";
+import { normalizeInput, parseCells, type DrawioCell } from "./drawio-xml.js";
+
+// Default sizes when a vertex declares no geometry (appendix base sizes).
+const DEFAULT_W = 140;
+const DEFAULT_H = 60;
+
+// Spacing is set >=150px on purpose so an ELK layout never trips the linter's
+// "gap between adjacent shapes < 150px" quality warning (acceptance #3).
+const LAYOUT_OPTIONS: Record<string, string> = {
+  "elk.algorithm": "layered",
+  "elk.direction": "RIGHT",
+  // Route edges across container boundaries in a single hierarchical pass.
+  "elk.hierarchyHandling": "INCLUDE_CHILDREN",
+  "elk.layered.spacing.nodeNodeBetweenLayers": "170",
+  "elk.spacing.nodeNode": "170",
+  "elk.spacing.edgeNode": "40",
+  "elk.spacing.edgeEdge": "30",
+  "elk.padding": "[top=20,left=20,bottom=20,right=20]",
+};
+
+// Per-container options: pad children >=30px off the frame (appendix rule) and
+// carry the same generous spacing so nested nodes never trip the "gap <150px"
+// warning either.
+const CONTAINER_OPTIONS: Record<string, string> = {
+  "elk.algorithm": "layered",
+  "elk.direction": "RIGHT",
+  "elk.padding": "[top=40,left=30,bottom=30,right=30]",
+  "elk.layered.spacing.nodeNodeBetweenLayers": "170",
+  "elk.spacing.nodeNode": "170",
+};
+
+interface ElkNode {
+  id: string;
+  width?: number;
+  height?: number;
+  x?: number;
+  y?: number;
+  children?: ElkNode[];
+  layoutOptions?: Record<string, string>;
+}
+interface ElkEdge {
+  id: string;
+  sources: string[];
+  targets: string[];
+}
+interface ElkGraph extends ElkNode {
+  edges?: ElkEdge[];
+}
+
+/**
+ * Apply an ELK layered layout to a drawio input and return a full mxGraphModel
+ * string with rewritten geometry. Accepts the same three input forms as
+ * drawio_create (a bare model, an <mxfile>, or a <mxCell> list). Async because
+ * elkjs' layout() is promise-based. On any layout failure the ORIGINAL
+ * (normalized) model is returned unchanged — layout is best-effort polish, never
+ * a reason to fail the write.
+ */
+export async function applyElkLayout(inputXml: string): Promise<string> {
+  const modelXml = normalizeInput(inputXml);
+  let cells: DrawioCell[];
+  try {
+    cells = parseCells(modelXml);
+  } catch {
+    return modelXml; // unparseable -> let the linter report it downstream
+  }
+
+  const byId = new Map(cells.map((c) => [c.id, c]));
+  const vertices = cells.filter(
+    (c) => c.vertex && c.id !== "0" && c.id !== "1",
+  );
+  if (vertices.length === 0) return modelXml;
+
+  // A vertex is a CONTAINER iff some other vertex names it as parent.
+  const childrenOf = new Map<string, DrawioCell[]>();
+  for (const v of vertices) {
+    const p = v.parent && byId.get(v.parent)?.vertex ? v.parent : "__root__";
+    if (!childrenOf.has(p)) childrenOf.set(p, []);
+    childrenOf.get(p)!.push(v);
+  }
+  const isContainer = (id: string) => childrenOf.has(id);
+
+  const buildNode = (v: DrawioCell): ElkNode => {
+    const kids = childrenOf.get(v.id);
+    const node: ElkNode = { id: v.id };
+    if (kids && kids.length > 0) {
+      node.children = kids.map(buildNode);
+      node.layoutOptions = { ...CONTAINER_OPTIONS };
+    } else {
+      node.width = v.geometry.width ?? DEFAULT_W;
+      node.height = v.geometry.height ?? DEFAULT_H;
+    }
+    return node;
+  };
+
+  const roots = (childrenOf.get("__root__") ?? []).map(buildNode);
+
+  // All edges at the root; INCLUDE_CHILDREN lets them span the hierarchy. Only
+  // edges whose endpoints are laid-out vertices are handed to ELK.
+  const vertexIds = new Set(vertices.map((v) => v.id));
+  const edges: ElkEdge[] = [];
+  for (const c of cells) {
+    if (!c.edge || !c.source || !c.target) continue;
+    if (!vertexIds.has(c.source) || !vertexIds.has(c.target)) continue;
+    edges.push({ id: c.id || `e${edges.length}`, sources: [c.source], targets: [c.target] });
+  }
+
+  const graph: ElkGraph = {
+    id: "root",
+    layoutOptions: LAYOUT_OPTIONS,
+    children: roots,
+    edges,
+  };
+
+  let laid: ElkGraph;
+  try {
+    // elkjs ships a CJS default export whose interop shape varies across
+    // module systems; resolve the real constructor at runtime, then cast (the
+    // runtime call is verified — see the layout unit test).
+    const Ctor: any = (ELK as any).default ?? ELK;
+    const elk = new Ctor();
+    laid = (await elk.layout(graph as any)) as ElkGraph;
+  } catch {
+    return modelXml; // best-effort: keep the model as-is on any ELK failure
+  }
+
+  // Collect computed geometry per node id (coords are parent-relative already).
+  const geo = new Map<string, { x: number; y: number; w: number; h: number }>();
+  const walk = (n: ElkNode) => {
+    if (n.id !== "root") {
+      geo.set(n.id, {
+        x: Math.round(n.x ?? 0),
+        y: Math.round(n.y ?? 0),
+        w: Math.round(n.width ?? DEFAULT_W),
+        h: Math.round(n.height ?? DEFAULT_H),
+      });
+    }
+    for (const c of n.children ?? []) walk(c);
+  };
+  walk(laid);
+
+  return rewriteGeometry(modelXml, geo, isContainer);
+}
+
+/**
+ * Rewrite each vertex cell's <mxGeometry> x/y (and width/height for containers,
+ * whose size ELK computed) using the DOM, then serialize back. Leaf sizes are
+ * left untouched. Edges and non-geometry attributes are preserved verbatim.
+ */
+function rewriteGeometry(
+  modelXml: string,
+  geo: Map<string, { x: number; y: number; w: number; h: number }>,
+  isContainer: (id: string) => boolean,
+): string {
+  const dom = new JSDOM("");
+  const parser = new dom.window.DOMParser();
+  const doc = parser.parseFromString(modelXml, "application/xml");
+  if (doc.getElementsByTagName("parsererror").length > 0) return modelXml;
+
+  const cellEls = doc.getElementsByTagName("mxCell");
+  for (let i = 0; i < cellEls.length; i++) {
+    const el = cellEls[i];
+    const id = el.getAttribute("id") || "";
+    const g = geo.get(id);
+    if (!g) continue;
+    let geoEl: any = null;
+    for (let j = 0; j < el.childNodes.length; j++) {
+      const ch = el.childNodes[j];
+      if (ch.nodeType === 1 && (ch as any).tagName === "mxGeometry") {
+        geoEl = ch;
+        break;
+      }
+    }
+    if (!geoEl) {
+      geoEl = doc.createElement("mxGeometry");
+      geoEl.setAttribute("as", "geometry");
+      el.appendChild(geoEl);
+    }
+    geoEl.setAttribute("x", String(g.x));
+    geoEl.setAttribute("y", String(g.y));
+    // Containers take ELK's computed size; leaves keep their authored size.
+    if (isContainer(id) || !geoEl.hasAttribute("width")) {
+      geoEl.setAttribute("width", String(g.w));
+    }
+    if (isContainer(id) || !geoEl.hasAttribute("height")) {
+      geoEl.setAttribute("height", String(g.h));
+    }
+  }
+
+  const ser = new dom.window.XMLSerializer();
+  return ser.serializeToString(doc.documentElement);
+}

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-shapes.ts

@@ -0,0 +1,420 @@
+// Verified draw.io shape catalog for the `drawio_shapes` tool (issue #424,
+// stage 2). This is the fix for AI-generated diagrams' #1 defect: guessed
+// `shape=mxgraph.*` names that render as EMPTY BOXES because the stencil does
+// not exist. Instead of guessing, the model queries this catalog and gets back
+// an exact, verified style-string + the stencil's default width/height.
+//
+// DATA SOURCE — the bundled index is the REAL jgraph/drawio-mcp shape index
+// (`shape-search/search-index.json`, Apache-2.0, ~10 446 shapes), fetched
+// verbatim and gzip-compressed to `packages/mcp/data/drawio-shape-index.json.gz`
+// (~4.7 MB -> ~430 KB). Each record is `{ style, w, h, title, tags, type }`.
+//
+// REGENERATING THE INDEX (keeps the catalog from going stale as draw.io ships
+// new stencils): jgraph publishes `shape-search/generate-index.js`, which
+// rebuilds `search-index.json` from a draw.io release's `app.min.js`. To update:
+//   1. clone https://github.com/jgraph/drawio-mcp (Apache-2.0)
+//   2. run `node shape-search/generate-index.js` per its README
+//   3. `gzip -9 -c search-index.json > packages/mcp/data/drawio-shape-index.json.gz`
+// The record shape and this module's search stay unchanged.
+//
+// CURATED OVERLAY — on top of the raw index this module carries a small,
+// hand-maintained overlay drawn from the issue #424 appendix (the aws-
+// architecture-diagram-skill knowledge): AWS service rebrandings whose stencil
+// name lags the product name, a BLOCKLIST of known-broken stencils mapped to
+// working replacements, the category fillColor palette, the AWS group/subnet
+// stencils, and the Azure image-style paths. The overlay is applied BEFORE the
+// raw search so a query for a rebranded/blocked name returns the correct answer
+// with an explanatory note instead of the empty-box stencil.
+
+import { readFileSync } from "node:fs";
+import { gunzipSync } from "node:zlib";
+
+/** A single catalog record as returned to the model. */
+export interface ShapeResult {
+  /** The exact draw.io style-string to put on the cell. */
+  style: string;
+  /** Default width in px for this stencil. */
+  w: number;
+  /** Default height in px for this stencil. */
+  h: number;
+  /** Human-readable stencil name. */
+  title: string;
+  /** "vertex" | "edge" (from the index). */
+  type: string;
+  /** AWS category (Compute/Database/…) when derivable, else undefined. */
+  category?: string;
+  /**
+   * Present when the overlay rewrote/annotated the answer: a rebrand, a
+   * blocklist replacement, or a usage hint. The model should surface it.
+   */
+  note?: string;
+}
+
+/** Raw record shape in the bundled index. */
+interface IndexRecord {
+  style: string;
+  w: number;
+  h: number;
+  title: string;
+  tags: string;
+  type: string;
+}
+
+// --- AWS category fillColor palette (appendix) -----------------------------
+// Service-level icons MUST carry a fillColor (invisible in PNG export
+// otherwise); the color is the AWS category color.
+export const AWS_CATEGORY_FILL: Record<string, string> = {
+  Compute: "#ED7100",
+  Networking: "#8C4FFF",
+  Database: "#C925D1",
+  Storage: "#3F8624",
+  Security: "#DD344C",
+  Integration: "#E7157B",
+  "AI/ML": "#01A88D",
+};
+
+/** Reverse lookup: fillColor hex -> category name (for annotating results). */
+const FILL_TO_CATEGORY: Record<string, string> = Object.fromEntries(
+  Object.entries(AWS_CATEGORY_FILL).map(([k, v]) => [v.toLowerCase(), k]),
+);
+
+/**
+ * Build the canonical service-level AWS icon style for a resIcon name. Mirrors
+ * the appendix's full template: strokeColor=#ffffff is MANDATORY and fillColor
+ * is the category color (defaults to AWS ink #232F3E when the category is
+ * unknown, so the glyph is never invisible).
+ */
+export function awsServiceStyle(resIcon: string, category?: string): string {
+  const fill = (category && AWS_CATEGORY_FILL[category]) || "#232F3E";
+  return (
+    "sketch=0;outlineConnect=0;fontColor=#232F3E;gradientColor=none;" +
+    `fillColor=${fill};strokeColor=#ffffff;dashed=0;verticalLabelPosition=bottom;` +
+    "verticalAlign=top;align=center;html=1;fontSize=12;fontStyle=0;aspect=fixed;" +
+    `shape=mxgraph.aws4.resourceIcon;resIcon=mxgraph.aws4.${resIcon}`
+  );
+}
+
+// --- AWS rebrandings (appendix "gotcha" table) -----------------------------
+// The stencil name lags the AWS product name; a naive query for the product
+// name would miss (or return an empty box). Each alias maps to the REAL resIcon.
+interface Rebrand {
+  aliases: string[];
+  resIcon: string;
+  category?: string;
+  note: string;
+}
+export const AWS_REBRANDS: Rebrand[] = [
+  {
+    aliases: ["opensearch", "open search", "amazon opensearch"],
+    resIcon: "elasticsearch_service",
+    category: "Database",
+    note: "Amazon OpenSearch's stencil is still named `elasticsearch_service` (renamed in 2021).",
+  },
+  {
+    aliases: ["eventbridge", "event bridge", "cloudwatch events"],
+    resIcon: "eventbridge",
+    category: "Integration",
+    note: "Amazon EventBridge uses resIcon `eventbridge` (formerly CloudWatch Events).",
+  },
+  {
+    aliases: ["vpc peering", "peering"],
+    resIcon: "peering",
+    category: "Networking",
+    note: "VPC Peering is resIcon `peering`, NOT `vpc_peering` (which renders empty).",
+  },
+  {
+    aliases: ["msk", "kafka", "managed streaming", "amazon msk"],
+    resIcon: "managed_streaming_for_kafka",
+    category: "Integration",
+    note: "Amazon MSK is resIcon `managed_streaming_for_kafka`, NOT `msk`.",
+  },
+  {
+    aliases: ["iam identity center", "identity center", "sso", "single sign on"],
+    resIcon: "single_sign_on",
+    category: "Security",
+    note: "IAM Identity Center is resIcon `single_sign_on`, NOT `iam_identity_center`.",
+  },
+];
+
+// --- BLOCKLIST of broken stencils (appendix) -------------------------------
+// A query that names one of these gets the working replacement + a note; the
+// broken stencil is never returned.
+interface Blocked {
+  bad: string;
+  good: string;
+  goodStyle?: (idx: IndexRecord[]) => ShapeResult | null;
+  note: string;
+}
+export const AWS_BLOCKLIST: Blocked[] = [
+  {
+    bad: "dynamodb_table",
+    good: "dynamodb",
+    note: "`dynamodb_table` renders as an empty box; use resIcon `dynamodb`.",
+  },
+  {
+    bad: "general_saml_token",
+    good: "traditional_server",
+    note: "`general_saml_token` is broken; use resIcon `traditional_server`.",
+  },
+  {
+    bad: "kinesis_data_streams",
+    good: "kinesis_data_streams",
+    note: "`kinesis_data_streams` is unreliable across draw.io versions; verify it renders, or fall back to resIcon `kinesis`.",
+  },
+];
+
+// --- AWS group / container stencils (appendix) -----------------------------
+// Groups are transparent containers; these are the verified stencil names.
+export const AWS_GROUP_STENCILS: ShapeResult[] = [
+  {
+    title: "AWS Cloud (group)",
+    style:
+      "points=[[0,0],[0.25,0],[0.5,0],[0.75,0],[1,0],[1,0.25],[1,0.5],[1,0.75],[1,1],[0.75,1],[0.5,1],[0.25,1],[0,1],[0,0.75],[0,0.5],[0,0.25]];" +
+      "outlineConnect=0;gradientColor=none;html=1;whiteSpace=wrap;fontSize=12;fontStyle=0;container=1;" +
+      "pointerEvents=0;collapsible=0;recursiveResize=0;shape=mxgraph.aws4.group;grIcon=mxgraph.aws4.group_aws_cloud_alt;" +
+      "strokeColor=#232F3E;fillColor=none;verticalAlign=top;align=left;spacingLeft=30;fontColor=#232F3E;dashed=0;",
+    w: 400,
+    h: 300,
+    type: "vertex",
+    note: "AWS Cloud boundary — transparent container (grIcon=group_aws_cloud_alt).",
+  },
+  {
+    title: "VPC (group)",
+    style:
+      "points=[[0,0],[0.25,0],[0.5,0],[0.75,0],[1,0],[1,0.25],[1,0.5],[1,0.75],[1,1],[0.75,1],[0.5,1],[0.25,1],[0,1],[0,0.75],[0,0.5],[0,0.25]];" +
+      "outlineConnect=0;gradientColor=none;html=1;whiteSpace=wrap;fontSize=12;fontStyle=0;container=1;" +
+      "pointerEvents=0;collapsible=0;recursiveResize=0;shape=mxgraph.aws4.group;grIcon=mxgraph.aws4.group_vpc2;" +
+      "strokeColor=#8C4FFF;fillColor=none;verticalAlign=top;align=left;spacingLeft=30;fontColor=#8C4FFF;dashed=0;",
+    w: 350,
+    h: 250,
+    type: "vertex",
+    note: "VPC boundary — transparent container (grIcon=group_vpc2).",
+  },
+  {
+    title: "Public Subnet (group)",
+    style:
+      "sketch=0;outlineConnect=0;gradientColor=none;html=1;whiteSpace=wrap;fontSize=12;fontStyle=0;container=1;" +
+      "pointerEvents=0;collapsible=0;recursiveResize=0;shape=mxgraph.aws4.group;grIcon=mxgraph.aws4.group_public_subnet;" +
+      "grStroke=0;strokeColor=none;fillColor=#E9F3E6;verticalAlign=top;align=left;spacingLeft=30;fontColor=#248814;dashed=0;",
+    w: 300,
+    h: 200,
+    type: "vertex",
+    note: "Public subnet — transparent container (grIcon=group_public_subnet).",
+  },
+  {
+    title: "Private Subnet (group)",
+    style:
+      "sketch=0;outlineConnect=0;gradientColor=none;html=1;whiteSpace=wrap;fontSize=12;fontStyle=0;container=1;" +
+      "pointerEvents=0;collapsible=0;recursiveResize=0;shape=mxgraph.aws4.group;grIcon=mxgraph.aws4.group_private_subnet;" +
+      "grStroke=0;strokeColor=none;fillColor=#E6F2F8;verticalAlign=top;align=left;spacingLeft=30;fontColor=#147EBA;dashed=0;",
+    w: 300,
+    h: 200,
+    type: "vertex",
+    note: "Private subnet — transparent container (grIcon=group_private_subnet).",
+  },
+];
+
+// --- Azure image-style stencils (appendix) ---------------------------------
+// `shape=mxgraph.azure2.*` does not render in every host; the image-style path
+// is the portable form. These are the verified known-working paths.
+interface AzureIcon {
+  aliases: string[];
+  path: string;
+  title: string;
+}
+const AZURE_ICONS: AzureIcon[] = [
+  { aliases: ["front door", "front doors"], path: "networking/Front_Doors.svg", title: "Azure Front Door" },
+  { aliases: ["api management", "apim"], path: "app_services/API_Management_Services.svg", title: "Azure API Management" },
+  { aliases: ["cosmos", "cosmos db"], path: "databases/Azure_Cosmos_DB.svg", title: "Azure Cosmos DB" },
+  { aliases: ["managed identity", "managed identities"], path: "identity/Managed_Identities.svg", title: "Azure Managed Identity" },
+  { aliases: ["azure monitor", "monitor"], path: "management_governance/Monitor.svg", title: "Azure Monitor" },
+  { aliases: ["application insights", "app insights"], path: "devops/Application_Insights.svg", title: "Azure Application Insights" },
+];
+
+/** Build the portable Azure image-style for a lib path (appendix template). */
+export function azureImageStyle(path: string): string {
+  return `sketch=0;points=[[0,0,0],[0.25,0,0],[0.5,0,0],[0.75,0,0],[1,0,0],[0,1,0],[0.25,1,0],[0.5,1,0],[0.75,1,0],[1,1,0],[0,0.25,0],[0,0.5,0],[0,0.75,0],[1,0.25,0],[1,0.5,0],[1,0.75,0]];shadow=0;dashed=0;html=1;strokeColor=none;fillColor=#5E9BD9;labelPosition=center;verticalLabelPosition=bottom;verticalAlign=top;align=center;outlineConnect=0;image;aspect=fixed;image=img/lib/azure2/${path};`;
+}
+
+// --- index loading (lazy, cached) ------------------------------------------
+
+let _index: IndexRecord[] | null = null;
+
+/** Path to the bundled gzipped index, resolved relative to the built module. */
+function indexPath(): URL {
+  // build/lib/drawio-shapes.js -> ../../data/… -> packages/mcp/data/…
+  return new URL("../../data/drawio-shape-index.json.gz", import.meta.url);
+}
+
+/** Load + decompress + parse the bundled index once, then cache it. */
+export function loadShapeIndex(): IndexRecord[] {
+  if (_index) return _index;
+  const gz = readFileSync(indexPath());
+  const json = gunzipSync(gz).toString("utf-8");
+  const arr = JSON.parse(json) as IndexRecord[];
+  _index = arr;
+  return arr;
+}
+
+/** Derive an AWS category from a service-level icon's fillColor, if present. */
+function categoryOf(style: string): string | undefined {
+  const m = /fillColor=(#[0-9a-fA-F]{6})/.exec(style);
+  if (!m) return undefined;
+  return FILL_TO_CATEGORY[m[1].toLowerCase()];
+}
+
+function toResult(r: IndexRecord): ShapeResult {
+  return {
+    style: r.style,
+    w: r.w,
+    h: r.h,
+    title: r.title,
+    type: r.type,
+    category: categoryOf(r.style),
+  };
+}
+
+/** Find the best index record whose style carries `resIcon=<name>`. */
+function findByResIcon(idx: IndexRecord[], name: string): IndexRecord | null {
+  const needle = `resIcon=mxgraph.aws4.${name}`;
+  // Prefer the service-level resourceIcon form; fall back to any style match.
+  let fallback: IndexRecord | null = null;
+  for (const r of idx) {
+    if (r.style.includes(needle) && r.style.includes("resourceIcon")) return r;
+    if (!fallback && r.style.includes(needle)) fallback = r;
+  }
+  return fallback;
+}
+
+/**
+ * Score a record against a lowercased query. Higher is better; 0 = no match.
+ * Exact title match ranks highest, then title substring, tag word, then a loose
+ * style/tag substring. This is a cheap substring+token scorer, not a real fuzzy
+ * matcher, which is plenty for the "give me the lambda icon" use case.
+ */
+function score(r: IndexRecord, q: string): number {
+  const title = r.title.toLowerCase();
+  const tags = r.tags.toLowerCase();
+  const style = r.style.toLowerCase();
+  let s = title === q ? 100 : 0;
+  if (title !== q && title.includes(q)) s += 40 - Math.min(20, title.length - q.length);
+  const words = q.split(/\s+/).filter(Boolean);
+  for (const w of words) {
+    if (title.includes(w)) s += 12;
+    if (new RegExp(`(^|\\W)${escapeRe(w)}(\\W|$)`).test(tags)) s += 8;
+    else if (tags.includes(w)) s += 4;
+    if (style.includes(w)) s += 2;
+  }
+  // Prefer the current AWS icon generation (aws4) over the deprecated aws3
+  // stencils, which are the older visual style and often not what's wanted.
+  if (s > 0) {
+    if (style.includes("mxgraph.aws4")) s += 6;
+    else if (style.includes("mxgraph.aws3")) s -= 12;
+  }
+  return s;
+}
+
+function escapeRe(s: string): string {
+  return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+}
+
+export interface SearchShapesOptions {
+  category?: string;
+  limit?: number;
+}
+
+/**
+ * Search the catalog. Applies the curated overlay first (blocklist replacement,
+ * AWS rebrand, AWS group stencils, Azure image-style), then substring/tag/fuzzy
+ * search over the bundled ~10 446-shape index. Returns up to `limit` results
+ * (default 12) with exact style-strings and default sizes.
+ */
+export function searchShapes(
+  query: string,
+  opts: SearchShapesOptions = {},
+): ShapeResult[] {
+  const limit = Math.max(1, Math.min(50, opts.limit ?? 12));
+  const q = query.trim().toLowerCase();
+  if (q === "") return [];
+  const idx = loadShapeIndex();
+  const out: ShapeResult[] = [];
+  const seen = new Set<string>();
+  const push = (r: ShapeResult) => {
+    if (seen.has(r.style)) return;
+    seen.add(r.style);
+    out.push(r);
+  };
+
+  // 1. BLOCKLIST: a query naming a broken stencil returns the replacement.
+  for (const b of AWS_BLOCKLIST) {
+    if (q.includes(b.bad) || b.bad.includes(q.replace(/\s+/g, "_"))) {
+      const rec = findByResIcon(idx, b.good);
+      if (rec) push({ ...toResult(rec), note: b.note });
+    }
+  }
+
+  // 2. AWS rebrandings: surface the correct resIcon with the rename note.
+  for (const rb of AWS_REBRANDS) {
+    if (rb.aliases.some((a) => q === a || q.includes(a) || a.includes(q))) {
+      const rec = findByResIcon(idx, rb.resIcon);
+      if (rec) {
+        push({ ...toResult(rec), category: rec ? categoryOf(rec.style) ?? rb.category : rb.category, note: rb.note });
+      } else {
+        push({
+          style: awsServiceStyle(rb.resIcon, rb.category),
+          w: 78,
+          h: 78,
+          title: rb.resIcon,
+          type: "vertex",
+          category: rb.category,
+          note: rb.note,
+        });
+      }
+    }
+  }
+
+  // 3. Azure image-style icons.
+  for (const az of AZURE_ICONS) {
+    if (az.aliases.some((a) => q.includes(a) || a.includes(q))) {
+      push({
+        style: azureImageStyle(az.path),
+        w: 68,
+        h: 68,
+        title: az.title,
+        type: "vertex",
+        category: "Azure",
+        note: "Azure: portable image-style (shape=mxgraph.azure2.* does not render in every host).",
+      });
+    }
+  }
+
+  // 4. AWS group/container stencils.
+  if (/\b(group|container|boundary|vpc|subnet|cloud|account)\b/.test(q)) {
+    for (const g of AWS_GROUP_STENCILS) {
+      if (g.title.toLowerCase().includes(q) || q.split(/\s+/).some((w) => g.title.toLowerCase().includes(w))) {
+        push(g);
+      }
+    }
+  }
+
+  // 5. General index search (substring + tags + loose fuzzy).
+  const catFilter = opts.category?.toLowerCase();
+  const scored: { r: IndexRecord; s: number }[] = [];
+  for (const r of idx) {
+    const s = score(r, q);
+    if (s <= 0) continue;
+    if (catFilter) {
+      const cat = categoryOf(r.style)?.toLowerCase();
+      const inStyle = r.style.toLowerCase().includes(catFilter);
+      if (cat !== catFilter && !inStyle) continue;
+    }
+    scored.push({ r, s });
+  }
+  scored.sort((a, b) => b.s - a.s || a.r.title.length - b.r.title.length);
+  for (const { r } of scored) {
+    if (out.length >= limit) break;
+    push(toResult(r));
+  }
+
+  return out.slice(0, limit);
+}

packages/mcp/src/lib/json-edit.ts

@@ -12,7 +12,11 @@
  * re-import for small wording fixes.
  */
 
-import { stripInlineMarkdown, stripBalancedWrappers } from "./text-normalize.js";
+import {
+  stripInlineMarkdown,
+  stripBalancedWrappers,
+  closestBlockHint,
+} from "./text-normalize.js";
 
 export interface TextEdit {
   find: string;
@@ -381,29 +385,9 @@ export function applyTextEdits(
       } else {
         // Append a bounded "closest text" hint: find the FIRST block that
         // contains the longest whitespace-delimited token (>= 3 chars) of the
-        // (stripped, then raw) locator, and quote that block's plain text.
-        reason = "text not found in the document.";
-        const tokenSource = stripped.length > 0 ? stripped : edit.find;
-        const longestToken = tokenSource
-          .split(/\s+/)
-          .filter((t) => t.length >= 3)
-          .sort((a, b) => b.length - a.length)[0];
-        if (longestToken) {
-          const hitBlock = blockPlain.find((plain) =>
-            plain.includes(longestToken),
-          );
-          if (hitBlock) {
-            // Truncate by code point (spread iterates by code point) so a
-            // surrogate pair is never split; append the ellipsis only when the
-            // text was actually longer than the limit.
-            const points = [...hitBlock];
-            const snippet =
-              points.length > 120
-                ? points.slice(0, 120).join("") + "…"
-                : hitBlock;
-            reason += ` Closest block text: "${snippet}".`;
-          }
-        }
+        // (stripped, then raw) locator, and quote that block's plain text. Shared
+        // with create_comment via closestBlockHint so both give the same hint.
+        reason = "text not found in the document." + closestBlockHint(blockPlain, edit.find);
       }
       failed.push({ find: edit.find, reason });
       continue;

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/lib/text-normalize.ts

@@ -114,3 +114,37 @@ export function stripInlineMarkdown(s: string): string {
 
   return out;
 }
+
+/**
+ * Build a bounded "closest text" hint for an anchor/find MISS, shared by
+ * edit_page_text (json-edit) and create_comment (client) so both surface the
+ * same self-correction affordance.
+ *
+ * Take the longest whitespace-delimited token (>= 3 chars) of the locator
+ * (markdown-stripped first, so `**bold**` contributes `bold`), find the FIRST
+ * of `blockTexts` that contains it, and return ` Closest block text: "…".` with
+ * the block quoted (truncated to 120 code points + ellipsis). Returns "" when
+ * no token qualifies or no block contains it, so the caller can append it
+ * unconditionally.
+ */
+export function closestBlockHint(
+  blockTexts: string[],
+  locator: string,
+): string {
+  if (typeof locator !== "string" || locator.length === 0) return "";
+  const stripped = stripInlineMarkdown(locator);
+  const tokenSource = stripped.length > 0 ? stripped : locator;
+  const longestToken = tokenSource
+    .split(/\s+/)
+    .filter((t) => t.length >= 3)
+    .sort((a, b) => b.length - a.length)[0];
+  if (!longestToken) return "";
+  const hitBlock = blockTexts.find((plain) => plain.includes(longestToken));
+  if (!hitBlock) return "";
+  // Truncate by code point (spread iterates by code point) so a surrogate pair
+  // is never split; append the ellipsis only when the text was actually longer.
+  const points = [...hitBlock];
+  const snippet =
+    points.length > 120 ? points.slice(0, 120).join("") + "…" : hitBlock;
+  return ` Closest block text: "${snippet}".`;
+}

packages/mcp/src/stdio.ts

@@ -1,6 +1,7 @@
 #!/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
@@ -33,6 +34,20 @@ 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

@@ -56,6 +56,29 @@ export interface SharedToolSpec {
   buildShape?: (z: ZodLike) => Record<string, unknown>;
 }
 
+/**
+ * Compact HARD-RULES block injected into the drawio_create / drawio_update
+ * descriptions (issue #424 — the jgraph/drawio-mcp pattern of putting the
+ * must-follow rules right where the model reads them at call time). Deliberately
+ * terse; the long-form authoring guidance lives in drawio_guide.
+ */
+export const DRAWIO_HARD_RULES =
+  ' RULES: id="0" and id="1"(parent="0") sentinels are MANDATORY; each cell is ' +
+  'vertex="1" XOR edge="1" (a container/group is neither); every edge carries a ' +
+  'child <mxGeometry relative="1" as="geometry"/>; ids are unique; NO XML ' +
+  'comments; put html=1 in styles and XML-escape value (& -> &amp;, < -> &lt;); a ' +
+  "newline in a label is &#xa;, never a literal \\n; containers are TRANSPARENT " +
+  "(fillColor=none;container=1;dropTarget=1;) with children set parent=<groupId> " +
+  'and RELATIVE coords, and an edge between different containers is parent="1"; set ' +
+  'adaptiveColors="auto" on <mxGraphModel> (free dark-theme adaptation for ' +
+  'strokeColor/fillColor/fontColor="default"); do NOT guess shape=mxgraph.* names ' +
+  "(a wrong name renders as an empty box) — call drawio_shapes first; call " +
+  "drawio_guide(section) for authoring help. Pass layout:\"elk\" to let the server " +
+  "compute coordinates from your rough placement. The result carries geometry " +
+  "WARNINGS (overlaps, an edge through a shape, edge-on-edge, gaps <150px, a label " +
+  "wider than its shape, negative coords) — they do NOT block the write; fix them " +
+  "and retry, max 2 iterations.";
+
 export const SHARED_TOOL_SPECS = {
   // --- no-argument read tools ---
 
@@ -771,9 +794,13 @@ export const SHARED_TOOL_SPECS = {
       'The comment is anchored inline to the given exact `selection` text ' +
       '(which gets highlighted); page-level comments are NOT supported. A ' +
       'new top-level comment REQUIRES a `selection`. Replies inherit the ' +
-      "parent's anchor and take no selection. If the call fails with a " +
-      '"selection not found" error, retry with a corrected EXACT selection ' +
-      'copied verbatim from a single paragraph/block. You may also attach a ' +
+      "parent's anchor and take no selection. Always COPY the `selection` " +
+      'VERBATIM from get_page / search_in_page output — do NOT quote it from ' +
+      'memory (stale-memory quoting is the top cause of anchor misses). If the ' +
+      'call fails with a "selection not found" error, the error quotes the ' +
+      "closest block text (or says the selection spans multiple blocks); retry " +
+      "with a corrected EXACT selection copied verbatim from a single " +
+      'paragraph/block. You may also attach a ' +
       '`suggestedText` proposing a replacement for the `selection` (a human ' +
       'applies it from the UI); when set, the `selection` must occur exactly ' +
       'once in the page. Reversible via the comment UI.',
@@ -1115,4 +1142,188 @@ export const SHARED_TOOL_SPECS = {
       alt: z.string().optional(),
     }),
   },
+
+  // --- draw.io diagrams (issue #423 stage 1, #424 stage 2) ---
+
+  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.' +
+      DRAWIO_HARD_RULES,
+    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.'),
+      layout: z
+        .enum(['elk'])
+        .optional()
+        .describe(
+          'Optional: "elk" runs an ELK layered auto-layout (honouring nested ' +
+            'containers) and rewrites all coordinates — give rough placement and ' +
+            'let the server compute pixels.',
+        ),
+    }),
+  },
+
+  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>".' +
+      DRAWIO_HARD_RULES,
+    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.'),
+      layout: z
+        .enum(['elk'])
+        .optional()
+        .describe(
+          'Optional: "elk" runs an ELK layered auto-layout and rewrites all ' +
+            'coordinates before writing.',
+        ),
+    }),
+  },
+
+  drawioShapes: {
+    mcpName: 'drawio_shapes',
+    inAppKey: 'drawioShapes',
+    description:
+      'Look up VERIFIED draw.io stencil style-strings so you never guess a ' +
+      '`shape=mxgraph.*` name (a wrong name renders as an EMPTY BOX). Searches a ' +
+      'bundled catalog of ~10 400 shapes (the jgraph/drawio-mcp index) by ' +
+      'substring, tags and loose fuzzy match, plus a curated overlay for AWS ' +
+      'icons: it returns the correct resIcon for rebranded services (OpenSearch ' +
+      '-> elasticsearch_service, MSK -> managed_streaming_for_kafka, VPC Peering ' +
+      '-> peering, IAM Identity Center -> single_sign_on) and maps known-broken ' +
+      'stencils to working replacements (e.g. dynamodb_table -> dynamodb) with a ' +
+      'note. Each hit is { style, w, h, title, type, category?, note? } — copy ' +
+      '`style` verbatim onto the cell and use w/h as the default size. Call this ' +
+      'BEFORE drawio_create/drawio_update whenever you need a specific icon ' +
+      '(AWS/Azure/GCP/network/UML/flowchart).',
+    tier: 'deferred',
+    catalogLine:
+      'drawioShapes — look up verified draw.io stencil style-strings (no empty boxes).',
+    buildShape: (z) => ({
+      query: z
+        .string()
+        .min(1)
+        .describe('What to find, e.g. "lambda", "s3", "azure cosmos", "vpc group".'),
+      category: z
+        .string()
+        .optional()
+        .describe('Optional filter, e.g. an AWS category name ("Compute").'),
+      limit: z
+        .number()
+        .optional()
+        .describe('Max results (default 12, capped at 50).'),
+    }),
+  },
+
+  drawioGuide: {
+    mcpName: 'drawio_guide',
+    inAppKey: 'drawioGuide',
+    description:
+      'Progressive-disclosure draw.io authoring reference. Call with a `section` ' +
+      'to pull one focused, <=4KB chapter instead of bloating context: ' +
+      '"skeleton" (canonical mxGraph XML, sentinels, the accepted inputs, hard ' +
+      'rules), "layout" (spacing heuristics, edge routing, the layout:"elk" ' +
+      'option, the quality warnings), "containers" (transparent groups, relative ' +
+      'child coords, cross-container edges, swimlanes), "icons-aws" (the ' +
+      'service/resource icon patterns, category colors, rebrandings, blocklist), ' +
+      '"icons-azure" (portable image-style paths). Omit `section` to get the ' +
+      'index of sections. Pair with drawio_shapes for exact stencil styles.',
+    tier: 'deferred',
+    catalogLine:
+      'drawioGuide — on-demand draw.io authoring reference (skeleton/layout/containers/icons).',
+    buildShape: (z) => ({
+      section: z
+        .enum(['skeleton', 'layout', 'containers', 'icons-aws', 'icons-azure'])
+        .optional()
+        .describe('Which section to read; omit for the section index.'),
+    }),
+  },
 } satisfies Record<string, SharedToolSpec>;

packages/mcp/test/mock/create-comment.test.mjs

@@ -548,3 +548,94 @@ test("suggestedText: the stored selection is the doc's RAW typographic substring
   );
   assert.equal(createPayload.suggestedText, "goodbye");
 });
+
+// -----------------------------------------------------------------------------
+// 8) #408: a not-found selection error QUOTES the closest block text so the
+//    model can self-correct instead of blind-retrying.
+// -----------------------------------------------------------------------------
+test("a not-found selection error includes a 'Closest block text' hint", async () => {
+  let createCalls = 0;
+  const { baseURL } = await spawn(async (req, res) => {
+    await readBody(req);
+    if (req.url === "/api/auth/login") {
+      sendJson(res, 200, { success: true }, {
+        "Set-Cookie": "authToken=t; Path=/; HttpOnly",
+      });
+      return;
+    }
+    if (req.url === "/api/pages/info") {
+      sendJson(res, 200, {
+        data: {
+          id: "page-1",
+          content: {
+            type: "doc",
+            content: [
+              { type: "paragraph", content: [{ type: "text", text: "The quick brown fox jumps" }] },
+            ],
+          },
+        },
+      });
+      return;
+    }
+    if (req.url === "/api/comments/create") {
+      createCalls++;
+      sendJson(res, 200, { data: { id: "should-not-happen" } });
+      return;
+    }
+    sendJson(res, 404, { message: "not found" });
+  });
+
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+  await assert.rejects(
+    () => client.createComment("page-1", "body", "inline", "quick brown cat"),
+    /Closest block text: "The quick brown fox jumps"/,
+    "a not-found selection must quote the closest block text",
+  );
+  assert.equal(createCalls, 0, "/comments/create must NOT be called on a miss");
+});
+
+// -----------------------------------------------------------------------------
+// 9) #408: a selection that straddles two blocks gets the explicit
+//    "spans multiple blocks" message instead of a bare not-found.
+// -----------------------------------------------------------------------------
+test("a selection spanning multiple blocks gets the explicit spans-multiple-blocks message", async () => {
+  let createCalls = 0;
+  const { baseURL } = await spawn(async (req, res) => {
+    await readBody(req);
+    if (req.url === "/api/auth/login") {
+      sendJson(res, 200, { success: true }, {
+        "Set-Cookie": "authToken=t; Path=/; HttpOnly",
+      });
+      return;
+    }
+    if (req.url === "/api/pages/info") {
+      sendJson(res, 200, {
+        data: {
+          id: "page-1",
+          content: {
+            type: "doc",
+            content: [
+              { type: "paragraph", content: [{ type: "text", text: "the quick brown" }] },
+              { type: "paragraph", content: [{ type: "text", text: "fox jumps over" }] },
+            ],
+          },
+        },
+      });
+      return;
+    }
+    if (req.url === "/api/comments/create") {
+      createCalls++;
+      sendJson(res, 200, { data: { id: "should-not-happen" } });
+      return;
+    }
+    sendJson(res, 404, { message: "not found" });
+  });
+
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+  await assert.rejects(
+    () => client.createComment("page-1", "body", "inline", "brown fox"),
+    /spans multiple blocks/,
+    "a cross-block selection must report the spans-multiple-blocks hint",
+  );
+  assert.equal(createCalls, 0, "/comments/create must NOT be called on a miss");
+});

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,6 +19,7 @@ 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
@@ -188,6 +189,10 @@ 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 }) =>
@@ -270,17 +275,23 @@ test("a UUID input is passed through unchanged and triggers NO /pages/info fetch
   );
 });
 
-test("a repeated slugId edit resolves the UUID only once (cache)", async () => {
+test("repeated slugId edits reuse ONE live collab session and resolve the UUID only once (#400 cache)", async () => {
   const { state, baseURL } = await spawnCollabStack();
   const client = new DocmostClient(baseURL, "user@example.com", "pw");
 
-  // 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.
+  // #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".
   await client.editPageText(SLUG, [{ find: "hello", replace: "hi" }]);
-  await client.editPageText(SLUG, [{ find: "hello", replace: "hey" }]);
+  await client.editPageText(SLUG, [{ find: "world", replace: "planet" }]);
 
-  assert.deepEqual(state.docNames, [`page.${UUID}`, `page.${UUID}`]);
+  assert.deepEqual(
+    state.docNames,
+    [`page.${UUID}`],
+    "the two edits must reuse one live collab session -> a single collab-doc open (#400)",
+  );
   assert.equal(
     state.pagesInfoCalls.length,
     1,
@@ -325,8 +336,9 @@ 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. (The write pass opens a second time after
-  // we release the gate; asserted at the end.)
+  // 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.)
   assert.deepEqual(
     state.docNames,
     [`page.${UUID}`],
@@ -378,8 +390,9 @@ 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");
-  // Both opens (scan pass + write pass) used the UUID; the slugId never appears.
-  assert.deepEqual(state.docNames, [`page.${UUID}`, `page.${UUID}`]);
+  // #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}`]);
   assert.ok(
     !state.docNames.includes(`page.${SLUG}`),
     "replaceImage must NEVER open the collab doc by the slugId (the #260 bug)",

packages/mcp/test/unit/client-host-contract.test.mjs

@@ -81,6 +81,10 @@ const HOST_CONTRACT_METHODS = [
   "insertImage",
   "replaceImage",
   "insertFootnote",
+  // draw.io diagrams (#423, stage 1) — read + create + optimistic-locked update
+  "drawioGet",
+  "drawioCreate",
+  "drawioUpdate",
   // write (comment)
   "createComment",
   "resolveComment",

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

@@ -0,0 +1,348 @@
+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/comment-anchor.test.mjs

@@ -8,6 +8,7 @@ import {
   applyAnchorInDoc,
   countAnchorMatches,
   getAnchoredText,
+  resolveAnchorSelection,
 } from "../../build/lib/comment-anchor.js";
 
 const COMMENT_ID = "cmt-123";
@@ -308,3 +309,70 @@ test("getAnchoredText returns null when the selection does not anchor", () => {
   const doc = paragraphDoc([{ type: "text", text: "hello world" }]);
   assert.equal(getAnchoredText(doc, "not present"), null);
 });
+
+// ---------------------------------------------------------------------------
+// #408 MARKDOWN-STRIP FALLBACK. A selection copied with inline markdown still
+// carries `**`/`` ` ``/`[t](u)` markers the plain document text lacks. When the
+// verbatim selection anchors nowhere, all four entry points retry with the
+// markdown stripped — consistently, so the suggestion-uniqueness gate stays
+// coherent — while what gets STORED remains the raw document substring.
+// ---------------------------------------------------------------------------
+test("a markdown-styled selection anchors against plain doc text via the strip fallback", () => {
+  const doc = paragraphDoc([{ type: "text", text: "a bold word here" }]);
+  // The agent quoted "**bold** word" from a styled view; the doc is plain text.
+  const sel = "**bold** word";
+  const resolved = resolveAnchorSelection(doc, sel);
+  assert.equal(resolved.found, true, "strip fallback finds the anchor");
+  assert.equal(resolved.normalized, true, "reports the soft-warning flag");
+  assert.equal(canAnchorInDoc(doc, sel), true);
+  assert.equal(countAnchorMatches(doc, sel), 1);
+
+  const ok = applyAnchorInDoc(doc, sel, COMMENT_ID);
+  assert.equal(ok, true);
+  const marked = doc.content[0].content.filter((p) => commentMark(p));
+  assert.equal(marked.map((m) => m.text).join(""), "bold word",
+    "the mark lands on the plain-text span");
+});
+
+test("getAnchoredText stores the RAW doc substring even when matched via the strip fallback", () => {
+  // Doc uses a smart apostrophe; the agent typed ASCII + markdown emphasis.
+  const doc = paragraphDoc([{ type: "text", text: "it’s bold now" }]);
+  const stored = getAnchoredText(doc, "it's **bold**");
+  assert.equal(stored, "it’s bold",
+    "stored selection is the raw document text, not the stripped/ASCII locator");
+});
+
+test("the strip fallback does not flip a raw-unique selection to ambiguous", () => {
+  // "config" appears twice, but the raw phrase "config value" appears once.
+  const doc = {
+    type: "doc",
+    content: [
+      { type: "paragraph", content: [{ type: "text", text: "the config value here" }] },
+      { type: "paragraph", content: [{ type: "text", text: "another config here" }] },
+    ],
+  };
+  // Raw phrase is unique -> exactly 1, and no strip happens (nothing to strip).
+  assert.equal(countAnchorMatches(doc, "config value"), 1);
+  assert.equal(resolveAnchorSelection(doc, "config value").normalized, false);
+});
+
+test("EXACT WINS: a raw match short-circuits the strip fallback (count reflects raw)", () => {
+  // A literal "**" run exists raw once; its stripped form would also appear.
+  const doc = paragraphDoc([{ type: "text", text: "use **stars** and stars" }]);
+  // Raw "**stars**" occurs once -> count 1 from the verbatim locator; the
+  // fallback (which would find two "stars") never runs.
+  assert.equal(countAnchorMatches(doc, "**stars**"), 1);
+  assert.equal(resolveAnchorSelection(doc, "**stars**").normalized, false);
+});
+
+test("a markdown selection whose stripped form is ambiguous is counted as ambiguous", () => {
+  const doc = {
+    type: "doc",
+    content: [
+      { type: "paragraph", content: [{ type: "text", text: "first config here" }] },
+      { type: "paragraph", content: [{ type: "text", text: "second config here" }] },
+    ],
+  };
+  // Verbatim "**config**" matches nothing; stripped "config" matches twice.
+  assert.equal(countAnchorMatches(doc, "**config**"), 2);
+});

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

@@ -0,0 +1,51 @@
+// Unit tests for the drawio_guide progressive-disclosure reference (issue #424).
+// Acceptance #2: every section is returned and each is <= ~4KB so pulling one
+// does not bloat the model's context.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import {
+  getGuideSection,
+  GUIDE_SECTIONS,
+} from "../../build/lib/drawio-guide.js";
+
+const MAX_BYTES = 4096; // "<= ~4KB" acceptance bound.
+
+test("every section is returned and is under ~4KB", () => {
+  assert.deepEqual(GUIDE_SECTIONS, [
+    "skeleton",
+    "layout",
+    "containers",
+    "icons-aws",
+    "icons-azure",
+  ]);
+  for (const s of GUIDE_SECTIONS) {
+    const { section, content } = getGuideSection(s);
+    assert.equal(section, s);
+    assert.ok(content.length > 200, `${s}: suspiciously short`);
+    const bytes = Buffer.byteLength(content, "utf8");
+    assert.ok(bytes <= MAX_BYTES, `${s}: ${bytes} bytes exceeds ${MAX_BYTES}`);
+  }
+});
+
+test("each section's content matches its topic", () => {
+  assert.match(getGuideSection("skeleton").content, /mxGraphModel/);
+  assert.match(getGuideSection("skeleton").content, /adaptiveColors="auto"/);
+  assert.match(getGuideSection("layout").content, /elk/i);
+  assert.match(getGuideSection("layout").content, /150px|<150/);
+  assert.match(getGuideSection("containers").content, /fillColor=none/);
+  assert.match(getGuideSection("icons-aws").content, /resourceIcon/);
+  assert.match(getGuideSection("icons-aws").content, /elasticsearch_service/);
+  assert.match(getGuideSection("icons-azure").content, /img\/lib\/azure2/);
+});
+
+test("omitting the section returns the index of sections", () => {
+  const idx = getGuideSection();
+  assert.equal(idx.section, "index");
+  for (const s of GUIDE_SECTIONS) assert.ok(idx.content.includes(s));
+  assert.ok(Buffer.byteLength(idx.content, "utf8") <= MAX_BYTES);
+});
+
+test("an unknown section falls back to the index", () => {
+  const idx = getGuideSection("nonsense");
+  assert.equal(idx.section, "index");
+});

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

@@ -0,0 +1,88 @@
+// Unit tests for the ELK auto-layout (issue #424, part 4). Acceptance #3: a
+// 10+ node graph with rough/overlapping coordinates, laid out with ELK, has no
+// bbox overlaps and produces no quality warnings.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { applyElkLayout } from "../../build/lib/drawio-layout.js";
+import { prepareModel, parseCells } from "../../build/lib/drawio-xml.js";
+
+/** Build a model where every vertex starts stacked at (10,10). */
+function stackedGraph(n, edges) {
+  let cells = "";
+  for (let i = 2; i < 2 + n; i++) {
+    cells +=
+      `<mxCell id="${i}" value="N${i}" style="rounded=1;html=1;" vertex="1" parent="1">` +
+      `<mxGeometry x="10" y="10" width="120" height="60" as="geometry"/></mxCell>`;
+  }
+  let ei = 0;
+  for (const [s, t] of edges) {
+    cells +=
+      `<mxCell id="e${ei++}" edge="1" parent="1" source="${s}" target="${t}">` +
+      `<mxGeometry relative="1" as="geometry"/></mxCell>`;
+  }
+  return (
+    '<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    cells +
+    "</root></mxGraphModel>"
+  );
+}
+
+test("acceptance #3: a 10-node graph with rough coords lays out with no warnings", async () => {
+  const edges = [
+    [2, 3], [2, 4], [3, 5], [4, 5], [5, 6],
+    [6, 7], [6, 8], [7, 9], [8, 10], [9, 11], [10, 11],
+  ];
+  const model = stackedGraph(10, edges);
+
+  // Before: everything is stacked at (10,10) -> lots of overlap warnings.
+  const before = prepareModel(model);
+  assert.ok(before.warnings.length > 0, "the stacked input should warn");
+
+  const laid = await applyElkLayout(model);
+  const after = prepareModel(laid);
+  assert.equal(
+    after.warnings.length,
+    0,
+    `ELK layout should clear all warnings, got: ${after.warnings.join(" | ")}`,
+  );
+  // Same number of user cells survived the layout.
+  assert.equal(after.cellCount, before.cellCount);
+});
+
+test("ELK honours nested containers as compound nodes (no warnings, children stay nested)", async () => {
+  const model =
+    '<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    '<mxCell id="g" value="VPC" style="container=1;dropTarget=1;fillColor=none;" vertex="1" parent="1"><mxGeometry x="0" y="0" width="100" height="100" as="geometry"/></mxCell>' +
+    '<mxCell id="a" value="A" style="rounded=1;" vertex="1" parent="g"><mxGeometry width="120" height="60" as="geometry"/></mxCell>' +
+    '<mxCell id="b" value="B" style="rounded=1;" vertex="1" parent="g"><mxGeometry width="120" height="60" as="geometry"/></mxCell>' +
+    '<mxCell id="c" value="C" style="rounded=1;" vertex="1" parent="1"><mxGeometry width="120" height="60" as="geometry"/></mxCell>' +
+    '<mxCell id="ab" edge="1" parent="g" source="a" target="b"><mxGeometry relative="1" as="geometry"/></mxCell>' +
+    '<mxCell id="bc" edge="1" parent="1" source="b" target="c"><mxGeometry relative="1" as="geometry"/></mxCell>' +
+    "</root></mxGraphModel>";
+  const laid = await applyElkLayout(model);
+  const cells = parseCells(laid);
+  const byId = Object.fromEntries(cells.map((c) => [c.id, c]));
+  // Children keep their container parent; the container was sized to hold them.
+  assert.equal(byId.a.parent, "g");
+  assert.equal(byId.b.parent, "g");
+  assert.ok((byId.g.geometry.width ?? 0) >= 260, "container widened to fit children");
+  const after = prepareModel(laid);
+  assert.equal(after.warnings.length, 0, after.warnings.join(" | "));
+});
+
+test("edges and cell count are preserved by layout", async () => {
+  const model = stackedGraph(4, [[2, 3], [3, 4], [4, 5]]);
+  const laid = await applyElkLayout(model);
+  const cells = parseCells(laid);
+  assert.equal(cells.filter((c) => c.edge).length, 3);
+  assert.equal(cells.filter((c) => c.vertex).length, 4);
+});
+
+test("layout is best-effort: an empty/degenerate model is returned intact", async () => {
+  const model =
+    '<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/></root></mxGraphModel>';
+  const laid = await applyElkLayout(model);
+  // No vertices -> unchanged, still lints clean.
+  const after = prepareModel(laid);
+  assert.equal(after.cellCount, 0);
+});

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-quality-warnings.test.mjs

@@ -0,0 +1,101 @@
+// Unit tests for the geometry quality-warnings (issue #424, part 5). Acceptance
+// #4: every warning has a positive AND a negative case, and warnings NEVER block
+// the write (prepareModel returns them, it does not throw).
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { prepareModel } from "../../build/lib/drawio-xml.js";
+
+function model(cells) {
+  return (
+    '<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/>' +
+    cells +
+    "</root></mxGraphModel>"
+  );
+}
+function warnings(cells) {
+  return prepareModel(model(cells)).warnings;
+}
+function has(ws, rule) {
+  return ws.some((w) => w.startsWith(`[${rule}]`));
+}
+function v(id, x, y, w = 120, h = 60, value = "", style = "rounded=1;html=1;", parent = "1") {
+  return (
+    `<mxCell id="${id}" value="${value}" style="${style}" vertex="1" parent="${parent}">` +
+    `<mxGeometry x="${x}" y="${y}" width="${w}" height="${h}" as="geometry"/></mxCell>`
+  );
+}
+function edge(id, s, t, parent = "1") {
+  return (
+    `<mxCell id="${id}" edge="1" parent="${parent}" source="${s}" target="${t}">` +
+    `<mxGeometry relative="1" as="geometry"/></mxCell>`
+  );
+}
+
+test("shape-overlap: positive and negative", () => {
+  assert.ok(has(warnings(v("a", 0, 0) + v("b", 50, 20)), "shape-overlap"));
+  assert.ok(!has(warnings(v("a", 0, 0) + v("b", 300, 0)), "shape-overlap"));
+});
+
+test("shape-overlap: a container over its own child does NOT warn", () => {
+  const cells =
+    '<mxCell id="g" value="G" style="container=1;fillColor=none;" vertex="1" parent="1"><mxGeometry x="0" y="0" width="400" height="200" as="geometry"/></mxCell>' +
+    v("a", 30, 40, 120, 60, "", "rounded=1;", "g");
+  assert.ok(!has(warnings(cells), "shape-overlap"));
+});
+
+test("edge-through-shape: positive and negative", () => {
+  // A -> B passes straight through C sitting on the line.
+  const pos =
+    v("a", 0, 0, 60, 60) + v("c", 200, 0, 60, 60) + v("b", 400, 0, 60, 60) + edge("e", "a", "b");
+  assert.ok(has(warnings(pos), "edge-through-shape"));
+  // C moved off the line -> no crossing.
+  const neg =
+    v("a", 0, 0, 60, 60) + v("c", 200, 300, 60, 60) + v("b", 400, 0, 60, 60) + edge("e", "a", "b");
+  assert.ok(!has(warnings(neg), "edge-through-shape"));
+});
+
+test("edge-overlap: positive (duplicate) and negative", () => {
+  const pos = v("a", 0, 0) + v("b", 300, 0) + edge("e1", "a", "b") + edge("e2", "a", "b");
+  assert.ok(has(warnings(pos), "edge-overlap"));
+  const neg =
+    v("a", 0, 0) + v("b", 300, 0) + v("c", 300, 300) + edge("e1", "a", "b") + edge("e2", "a", "c");
+  assert.ok(!has(warnings(neg), "edge-overlap"));
+});
+
+test("gap-too-small: positive and negative", () => {
+  assert.ok(has(warnings(v("a", 0, 0) + v("b", 220, 0)), "gap-too-small")); // 100px gap
+  assert.ok(!has(warnings(v("a", 0, 0) + v("b", 300, 0)), "gap-too-small")); // 180px gap
+});
+
+test("label-overflow: positive and negative", () => {
+  const pos = v("a", 0, 0, 40, 60, "A very long label that does not fit");
+  assert.ok(has(warnings(pos), "label-overflow"));
+  const neg = v("a", 0, 0, 300, 60, "Short");
+  assert.ok(!has(warnings(neg), "label-overflow"));
+});
+
+test("label-overflow: a label drawn OUTSIDE the shape (AWS icon) does NOT warn", () => {
+  const cells = v(
+    "a",
+    0,
+    0,
+    60,
+    60,
+    "A very long service label below the icon",
+    "shape=mxgraph.aws4.resourceIcon;verticalLabelPosition=bottom;verticalAlign=top;html=1;",
+  );
+  assert.ok(!has(warnings(cells), "label-overflow"));
+});
+
+test("out-of-bounds: positive (negative coords) and negative", () => {
+  assert.ok(has(warnings(v("a", -50, 10)), "out-of-bounds"));
+  assert.ok(!has(warnings(v("a", 10, 10)), "out-of-bounds"));
+});
+
+test("warnings never block the write (prepareModel returns, does not throw)", () => {
+  const messy = v("a", 0, 0) + v("b", 30, 20) + v("c", 40, 40); // heavy overlap
+  const prepared = prepareModel(model(messy));
+  assert.ok(prepared.warnings.length > 0, "expected warnings");
+  assert.ok(prepared.modelXml.includes("mxGraphModel"), "still produced a model");
+  assert.equal(prepared.cellCount, 3);
+});

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

@@ -0,0 +1,97 @@
+// Unit tests for the drawio_shapes verified-stencil catalog (issue #424).
+// Covers acceptance #1: a "lambda" query returns a valid mxgraph.aws4 icon with
+// the right service/resource pattern + sizes; a blocklisted stencil query
+// returns its working replacement.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import {
+  searchShapes,
+  awsServiceStyle,
+  azureImageStyle,
+  loadShapeIndex,
+  AWS_CATEGORY_FILL,
+} from "../../build/lib/drawio-shapes.js";
+
+test("the bundled index loads and is the real ~10k-shape catalog", () => {
+  const idx = loadShapeIndex();
+  assert.ok(Array.isArray(idx));
+  assert.ok(idx.length > 10000, `expected >10000 shapes, got ${idx.length}`);
+  // Record shape { style, w, h, title, tags, type }.
+  for (const k of ["style", "w", "h", "title", "tags", "type"]) {
+    assert.ok(k in idx[0], `record missing key ${k}`);
+  }
+});
+
+test('drawio_shapes("lambda") returns a valid mxgraph.aws4 service icon', () => {
+  const results = searchShapes("lambda", { limit: 5 });
+  assert.ok(results.length > 0);
+  // Acceptance #1: a valid aws4 service-level icon (resourceIcon + resIcon)
+  // for lambda, with sensible default sizes, is present.
+  const svc = results.find(
+    (r) =>
+      /shape=mxgraph\.aws4\.resourceIcon/.test(r.style) &&
+      /resIcon=mxgraph\.aws4\.lambda(_function)?\b/.test(r.style),
+  );
+  assert.ok(svc, `no aws4 lambda service icon in ${JSON.stringify(results.map((r) => r.style.slice(-40)))}`);
+  assert.ok(svc.w > 0 && svc.h > 0, "icon must carry default w/h");
+  // The current-generation aws4 icon must outrank the deprecated aws3 one.
+  assert.match(results[0].style, /mxgraph\.aws4/);
+});
+
+test("a blocklisted stencil query returns its replacement + a note", () => {
+  const results = searchShapes("dynamodb_table", { limit: 3 });
+  assert.ok(results.length > 0);
+  const rep = results[0];
+  // dynamodb_table (empty box) -> dynamodb.
+  assert.match(rep.style, /resIcon=mxgraph\.aws4\.dynamodb\b/);
+  assert.ok(rep.note && /dynamodb_table/.test(rep.note), "note must explain the replacement");
+  // The broken stencil name must NOT be returned as a usable style.
+  assert.ok(
+    !results.some((r) => /resIcon=mxgraph\.aws4\.dynamodb_table\b/.test(r.style)),
+    "the broken dynamodb_table stencil must not be returned",
+  );
+});
+
+test("an AWS rebranding query returns the real (renamed) resIcon", () => {
+  const os = searchShapes("opensearch", { limit: 3 });
+  assert.ok(
+    os.some((r) => /resIcon=mxgraph\.aws4\.elasticsearch_service\b/.test(r.style) && r.note),
+    "OpenSearch must map to elasticsearch_service with a note",
+  );
+  const msk = searchShapes("msk", { limit: 3 });
+  assert.ok(
+    msk.some((r) => /managed_streaming_for_kafka/.test(r.style)),
+    "MSK must map to managed_streaming_for_kafka",
+  );
+});
+
+test("category filter narrows results", () => {
+  const all = searchShapes("database", { limit: 20 });
+  const dbOnly = searchShapes("database", { category: "Database", limit: 20 });
+  assert.ok(dbOnly.length <= all.length);
+});
+
+test("limit is honoured and capped", () => {
+  assert.equal(searchShapes("aws", { limit: 3 }).length, 3);
+  assert.ok(searchShapes("aws", { limit: 999 }).length <= 50);
+});
+
+test("empty query returns nothing", () => {
+  assert.deepEqual(searchShapes("   "), []);
+});
+
+test("style builders match the appendix templates", () => {
+  const s = awsServiceStyle("lambda", "Compute");
+  assert.match(s, /strokeColor=#ffffff/); // mandatory for service-level
+  assert.match(s, new RegExp(`fillColor=${AWS_CATEGORY_FILL.Compute}`));
+  assert.match(s, /shape=mxgraph\.aws4\.resourceIcon;resIcon=mxgraph\.aws4\.lambda$/);
+  const az = azureImageStyle("databases/Azure_Cosmos_DB.svg");
+  assert.match(az, /image=img\/lib\/azure2\/databases\/Azure_Cosmos_DB\.svg/);
+});
+
+test("azure and group queries surface the curated overlay", () => {
+  const cosmos = searchShapes("cosmos", { limit: 5 });
+  assert.ok(cosmos.some((r) => /azure2\/databases\/Azure_Cosmos_DB\.svg/.test(r.style)));
+  const vpc = searchShapes("vpc group", { limit: 5 });
+  assert.ok(vpc.some((r) => /grIcon=mxgraph\.aws4\.group_vpc2/.test(r.style)));
+});

packages/mcp/test/unit/drawio-stage2-registration.test.mjs

@@ -0,0 +1,62 @@
+// Drift guards for the stage-2 drawio tools (issue #424): the new tools must be
+// wired into the shared registry AND routed in SERVER_INSTRUCTIONS, and the
+// hard-rules block must be injected into the create/update descriptions. These
+// complement the generic server-instructions.test.mjs / tool-specs.test.mjs.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { SERVER_INSTRUCTIONS } from "../../build/index.js";
+import { SHARED_TOOL_SPECS } from "../../build/tool-specs.js";
+
+test("drawio_shapes and drawio_guide are in the shared registry", () => {
+  assert.equal(SHARED_TOOL_SPECS.drawioShapes.mcpName, "drawio_shapes");
+  assert.equal(SHARED_TOOL_SPECS.drawioGuide.mcpName, "drawio_guide");
+  // Deferred tier, matching the stage-1 drawio tools.
+  assert.equal(SHARED_TOOL_SPECS.drawioShapes.tier, "deferred");
+  assert.equal(SHARED_TOOL_SPECS.drawioGuide.tier, "deferred");
+});
+
+test("the new tools are routed in SERVER_INSTRUCTIONS", () => {
+  for (const name of ["drawio_shapes", "drawio_guide"]) {
+    assert.match(SERVER_INSTRUCTIONS, new RegExp(`\\b${name}\\b`), `${name} missing from guide`);
+  }
+});
+
+test("the hard-rules block is injected into create/update descriptions", () => {
+  for (const key of ["drawioCreate", "drawioUpdate"]) {
+    const d = SHARED_TOOL_SPECS[key].description;
+    assert.match(d, /sentinels are MANDATORY/);
+    assert.match(d, /vertex="1" XOR edge="1"/);
+    assert.match(d, /call drawio_shapes first/);
+    assert.match(d, /adaptiveColors="auto"/);
+    assert.match(d, /
/);
+  }
+});
+
+test("create/update expose the layout:\"elk\" parameter", () => {
+  const { z } = { z: makeZodStub() };
+  for (const key of ["drawioCreate", "drawioUpdate"]) {
+    const shape = SHARED_TOOL_SPECS[key].buildShape(z);
+    assert.ok("layout" in shape, `${key} missing layout param`);
+  }
+});
+
+// Tiny zod stub: buildShape only calls z.string/enum/number + chained
+// .min/.optional/.describe, all of which return `this`.
+function makeZodStub() {
+  const chain = new Proxy(
+    {},
+    {
+      get: (_t, prop) => {
+        if (prop === "parse") return () => ({});
+        return () => chain;
+      },
+    },
+  );
+  return {
+    string: () => chain,
+    number: () => chain,
+    enum: () => chain,
+    array: () => chain,
+    object: () => chain,
+  };
+}

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

@@ -1035,6 +1035,9 @@ importers:
       axios:
         specifier: 1.16.0
         version: 1.16.0
+      elkjs:
+        specifier: ^0.11.1
+        version: 0.11.1
       form-data:
         specifier: ^4.0.0
         version: 4.0.5
@@ -1044,6 +1047,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
@@ -6842,6 +6848,9 @@ packages:
   electron-to-chromium@1.5.286:
     resolution: {integrity: sha512-9tfDXhJ4RKFNerfjdCcZfufu49vg620741MNs26a9+bhLThdB+plgMeou98CAaHu/WATj2iHOOHTp1hWtABj2A==}
 
+  elkjs@0.11.1:
+    resolution: {integrity: sha512-zxxR9k+rx5ktMwT/FwyLdPCrq7xN6e4VGGHH8hA01vVYKjTFik7nHOxBnAYtrgYUB1RpAiLvA1/U2YraWxyKKg==}
+
   emittery@0.13.1:
     resolution: {integrity: sha512-DeWwawk6r5yR9jFgnDKYt4sLS0LmHJJi3ZOnb5/JdbYwj3nW+FxQnHIjhBKz8YLC7oRNPVM9NQ47I3CVx34eqQ==}
     engines: {node: '>=12'}
@@ -17547,6 +17556,8 @@ snapshots:
 
   electron-to-chromium@1.5.286: {}
 
+  elkjs@0.11.1: {}
+
   emittery@0.13.1: {}
 
   emoji-regex@8.0.0: {}