refactor(mcp): вписать drawio-тулы в реестровый цикл (Фаза 1б смержена) (#440)

Фаза 1б (#445/#446/#447/#448) влилась ПЕРЕД этим PR, поэтому явная проводка
drawio через registerShared/sharedTool конфликтовала с реестровыми циклами.
Фолд:
- drawioGet/Create/Update → канонический execute в спеке (это client-методы):
  execute возвращает сырой результат, цикл оборачивает jsonContent на MCP и
  отдаёт как есть in-app — байт-в-байт как старые тела, overrides не нужны.
  layout сохранён: добавлен в buildShape create/update + 5-м аргументом
  (layout as 'elk'|undefined) в обоих execute.
- drawioShapes/drawioGuide → остаются inline на ОБОИХ хостах. Гайд архитектора
  «execute в спеке с импортом searchShapes/getGuideSection в tool-specs.ts»
  оказался невозможен: drawio-shapes.ts использует import.meta.url, а
  tool-specs.ts тайпчекается из исходника под module:commonjs (contract-спека)
  → TS1343 на статический value-import, а import.meta не индиректится. Введён
  флаг спеки inlineBothHosts: спеки остаются в SHARED_TOOL_SPECS (contract
  пинит имя/описание/схему), но без execute; ОБА цикла их пропускают (добавлен
  симметричный guard в MCP-цикл), каждый хост регистрирует их inline через
  чистые хелперы — поведение байт-в-байт как до ребейза.
- docmost-client.loader: взята develop-форма DocmostClientLike = Pick<
  DocmostClient> (#446); ручное зеркало убрано, паритет layout наследуется из
  реальной сигнатуры client.
- ROUTING_PROSE: drawio intent-подсказки (shapes-first, guide, layout:elk);
  инвентарные строки убраны (генерируются из catalogLine).

Гейт: mcp node --test 674/674; server jest ai-chat-tools.service + contract
(211, все 5 drawio на обоих хостах, идентичная схема) + tool-tiers → 264;
tsc чисто; layout-passthrough тест 3/3. ELK DoS-кап и layout-фикс из round 3/4
сохранены.

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

.env.example

@@ -217,6 +217,17 @@ MCP_DOCMOST_PASSWORD=
 # active" behavior.
 # AI_CHAT_DEFERRED_TOOLS=true
 
+# Final-step lockdown for the in-app agent loop (#444). Default OFF. When ON
+# (legacy), the LAST allowed step forces a text-only answer: the model's tools are
+# stripped (toolChoice=none) and a synthesis instruction is appended. That
+# tool-stripping caused a token-degeneration incident — robbed of its tools on the
+# final step mid-work, the model emitted a ~255KB block repeating a single token —
+# so the default is now OFF: the last step keeps its tools and gets only a SOFT
+# nudge to finish with a text summary, and a token-degeneration detector is the
+# universal anti-babble guard. Enable this ONLY for a model that reliably ends its
+# turns with a clear text answer.
+# AI_CHAT_FINAL_STEP_LOCKDOWN=false
+
 # --- Autonomous / detached agent runs (settings.ai.autonomousRuns) ---
 # Opt-in per workspace (AI settings; off by default). When on, a chat turn becomes
 # a server-side RUN that survives a browser disconnect — only an explicit Stop ends

.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

AGENTS.md

@@ -338,7 +338,7 @@ Vite SPA. Code is organized by feature under `apps/client/src/features/*` (mirro
 - The version string shown in the UI comes from `APP_VERSION` (CI/Docker) or `git describe --tags --always` (local), resolved in `vite.config.ts` — not from `package.json`.
 - Server TS config is permissive (`noImplicitAny: false`, `strictNullChecks: false`, `no-explicit-any` lint disabled). Follow the existing relaxed style rather than tightening types broadly.
 - Dependency versions are heavily pinned via `pnpm.overrides` and `pnpm.patchedDependencies` (`scimmy`, `yjs`, `ai`) in the root `package.json`. Don't bump pinned/patched deps casually; the patches and overrides exist for compatibility/security reasons. The `ai@6.0.134` patch disables the SDK's O(n²) cumulative `partialOutput` accumulation when no output strategy is requested (server heap OOM on long agent runs, #184; tripwire test: `apps/server/src/integrations/ai/ai-sdk-partial-output.patch.spec.ts`) — it MUST be re-created via `pnpm patch` when bumping `ai`.
-- **Adding/renaming/removing an MCP tool requires updating `SERVER_INSTRUCTIONS`** in `packages/mcp/src/index.ts` — the intent-routing guide MCP clients receive on initialize. This applies both to inline `server.registerTool(...)` calls in `index.ts` and to specs in `packages/mcp/src/tool-specs.ts`. Enforced by `packages/mcp/test/unit/server-instructions.test.mjs`, which fails when a registered tool is not mentioned in the guide (deliberate opt-outs go into its `EXCEPTIONS` list). `packages/mcp/build/` is gitignored and rebuilt in CI/Docker via `pnpm build` (same convention as `git-sync`/`prosemirror-markdown`) — never commit it; rebuild locally after editing to run the tests.
+- **The MCP tool inventory in `SERVER_INSTRUCTIONS` is GENERATED from the registry** (`packages/mcp/src/server-instructions.ts`: `buildToolInventory()` over `SHARED_TOOL_SPECS`) and spliced into the hand-written routing prose (`ROUTING_PROSE`). So adding/renaming/removing a **shared** spec in `packages/mcp/src/tool-specs.ts` auto-updates the `<tool_inventory>` — no manual `SERVER_INSTRUCTIONS` edit needed. Only an **inline** MCP-only tool (those registered via `server.registerTool(...)` in `index.ts`, not through the registry) needs a one-line entry in `INLINE_MCP_INVENTORY`. Enforced by `packages/mcp/test/unit/tool-inventory.test.mjs`, which fails when a registered tool is missing from the generated inventory (there is no `EXCEPTIONS` opt-out anymore — every tool must appear). Update `ROUTING_PROSE` when a tool's *intent guidance* (when-to-use) changes. `packages/mcp/build/` is gitignored and rebuilt in CI/Docker via `pnpm build` (same convention as `git-sync`/`prosemirror-markdown`) — never commit it; rebuild locally after editing to run the tests.
 
 ## CI / release
 

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

@@ -28,7 +28,10 @@ const h = vi.hoisted(() => ({
         body: Record<string, unknown>;
       }) => { body: Record<string, unknown> };
       prepareReconnectToStreamRequest?: () => { api?: string };
-      fetch?: (input: unknown, init?: { method?: string }) => Promise<unknown>;
+      fetch?: (
+        input: unknown,
+        init?: { method?: string; body?: unknown },
+      ) => Promise<unknown>;
     },
   },
 }));
@@ -200,6 +203,244 @@ describe("ChatThread — send now (#198)", () => {
   });
 });
 
+// #396: in autonomous mode a live sendNow must additionally request the
+// AUTHORITATIVE server stop of the detached run (a local abort is only a client
+// disconnect the server ignores) and arm a bounded 409 retry so the re-POST
+// converges once the one-active-run slot frees. Legacy mode is unchanged.
+describe("ChatThread — send now server-stop + supersede retry (#396)", () => {
+  beforeEach(resetState);
+  afterEach(cleanup);
+
+  // A settled assistant tail => no mount resume (attemptResumeRef false), so the
+  // "Send now" button is visible for the NEW local streaming turn while
+  // autonomous runs are enabled.
+  const settledTail = () => [
+    row("u1", "user", undefined, "hi"),
+    row("a1", "assistant", "succeeded", "done"),
+  ];
+
+  it("autonomous: sendNow during a live stream calls onServerStop with the chat id", () => {
+    const { onServerStop } = renderThread({
+      autonomousRunsEnabled: true,
+      initialRows: settledTail(),
+    });
+    fireEvent.click(screen.getByTestId("queue-btn"));
+    fireEvent.click(screen.getByLabelText("Send now"));
+
+    expect(h.state.stop).toHaveBeenCalledTimes(1);
+    expect(onServerStop).toHaveBeenCalledWith("c1");
+  });
+
+  it("legacy (autonomous off): sendNow does NOT call onServerStop and does NOT retry the send", async () => {
+    const { onServerStop } = renderThread({
+      autonomousRunsEnabled: false,
+      initialRows: settledTail(),
+    });
+    fireEvent.click(screen.getByTestId("queue-btn"));
+    fireEvent.click(screen.getByLabelText("Send now"));
+    expect(onServerStop).not.toHaveBeenCalled();
+
+    // The supersede retry must NOT be armed: a POST that 409s is returned as-is
+    // (single fetch, no retry).
+    const fetchMock = vi
+      .fn()
+      .mockResolvedValue(
+        new Response(JSON.stringify({ code: "A_RUN_ALREADY_ACTIVE" }), {
+          status: 409,
+        }),
+      );
+    vi.stubGlobal("fetch", fetchMock);
+    let res!: Response;
+    await act(async () => {
+      res = (await h.state.transport!.fetch!("http://x", {
+        method: "POST",
+        body: "{}",
+      })) as Response;
+    });
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(res.status).toBe(409);
+  });
+
+  it("armed supersede send retries 409 A_RUN_ALREADY_ACTIVE and succeeds once the slot frees", async () => {
+    renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
+    // Arm the retry by performing a live sendNow (autonomous branch sets the ref).
+    fireEvent.click(screen.getByTestId("queue-btn"));
+    fireEvent.click(screen.getByLabelText("Send now"));
+
+    const fetchMock = vi
+      .fn()
+      // First POST: the old detached run still holds the slot -> 409.
+      .mockResolvedValueOnce(
+        new Response(JSON.stringify({ code: "A_RUN_ALREADY_ACTIVE" }), {
+          status: 409,
+        }),
+      )
+      // Retry: the server stop settled the old run -> 200.
+      .mockResolvedValueOnce(new Response("ok", { status: 200 }));
+    vi.stubGlobal("fetch", fetchMock);
+
+    let res!: Response;
+    await act(async () => {
+      res = (await h.state.transport!.fetch!("http://x", {
+        method: "POST",
+        body: "{}",
+      })) as Response;
+    });
+    expect(fetchMock).toHaveBeenCalledTimes(2);
+    expect(res.status).toBe(200);
+  });
+
+  it("supersede retry is one-shot: a later send (ref cleared) does NOT retry a 409", async () => {
+    renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
+    fireEvent.click(screen.getByTestId("queue-btn"));
+    fireEvent.click(screen.getByLabelText("Send now")); // arms the one-shot
+
+    // First armed send: immediately succeeds, consuming the arm.
+    let fetchMock = vi
+      .fn()
+      .mockResolvedValue(new Response("ok", { status: 200 }));
+    vi.stubGlobal("fetch", fetchMock);
+    await act(async () => {
+      await h.state.transport!.fetch!("http://x", {
+        method: "POST",
+        body: "{}",
+      });
+    });
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+
+    // A subsequent send is NOT armed -> a 409 is returned as-is (no retry).
+    fetchMock = vi.fn().mockResolvedValue(
+      new Response(JSON.stringify({ code: "A_RUN_ALREADY_ACTIVE" }), {
+        status: 409,
+      }),
+    );
+    vi.stubGlobal("fetch", fetchMock);
+    let res!: Response;
+    await act(async () => {
+      res = (await h.state.transport!.fetch!("http://x", {
+        method: "POST",
+        body: "{}",
+      })) as Response;
+    });
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(res.status).toBe(409);
+  });
+
+  it("supersede retry is bounded: exhaustion surfaces the 409 error", async () => {
+    renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
+    fireEvent.click(screen.getByTestId("queue-btn"));
+    fireEvent.click(screen.getByLabelText("Send now"));
+
+    // Every attempt 409s -> after 4 attempts the last 409 surfaces.
+    const fetchMock = vi.fn().mockResolvedValue(
+      new Response(JSON.stringify({ code: "A_RUN_ALREADY_ACTIVE" }), {
+        status: 409,
+      }),
+    );
+    vi.stubGlobal("fetch", fetchMock);
+    let res!: Response;
+    await act(async () => {
+      res = (await h.state.transport!.fetch!("http://x", {
+        method: "POST",
+        body: "{}",
+      })) as Response;
+    });
+    // 4 attempts total (1 immediate + 3 backoff retries), then give up.
+    expect(fetchMock).toHaveBeenCalledTimes(4);
+    expect(res.status).toBe(409);
+  });
+
+  it("armed supersede send does NOT retry a non-409 status", async () => {
+    renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
+    fireEvent.click(screen.getByTestId("queue-btn"));
+    fireEvent.click(screen.getByLabelText("Send now"));
+    const fetchMock = vi
+      .fn()
+      .mockResolvedValue(new Response("boom", { status: 500 }));
+    vi.stubGlobal("fetch", fetchMock);
+    let res!: Response;
+    await act(async () => {
+      res = (await h.state.transport!.fetch!("http://x", {
+        method: "POST",
+        body: "{}",
+      })) as Response;
+    });
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(res.status).toBe(500);
+  });
+
+  // Strand-path regression: sendNow arms the supersede retry, but if the promoted
+  // head is removed before the abort's onFinish lands, flushNext() sends nothing
+  // (returns false) and NO re-POST consumes the arm. The arm must be disarmed on
+  // that no-send branch so the NEXT unrelated NORMAL send does not inherit it and
+  // silently retry a genuine 409 (e.g. a legitimate two-tab conflict) 4x instead
+  // of surfacing it immediately.
+  it("strand-path: a stranded supersede arm (flushNext no-send) does NOT retry a later normal 409", async () => {
+    renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
+    // Arm the retry via a live autonomous sendNow (promotes the head + arms).
+    fireEvent.click(screen.getByTestId("queue-btn"));
+    fireEvent.click(screen.getByLabelText("Send now"));
+
+    // Remove the promoted head BEFORE the abort lands, so flushNext() returns
+    // false (no POST) and the arm would strand without the disarm fix.
+    fireEvent.click(screen.getByLabelText("Remove queued message"));
+
+    // The abort's onFinish now takes the flushOnAbortRef branch, calls flushNext()
+    // which finds an empty queue and returns false -> the no-send disarm must run.
+    act(() => {
+      h.state.onFinish?.({
+        message: { id: "a1", role: "assistant", parts: [] },
+        isAbort: true,
+        isDisconnect: false,
+        isError: false,
+      });
+    });
+    // No re-POST was sent (nothing to flush).
+    expect(h.state.sendMessage).not.toHaveBeenCalled();
+
+    // A subsequent NORMAL send that 409s must be returned as-is (exactly 1 fetch):
+    // the stranded arm must NOT cause the genuine 409 to be retried.
+    const fetchMock = vi
+      .fn()
+      .mockResolvedValue(
+        new Response(JSON.stringify({ code: "A_RUN_ALREADY_ACTIVE" }), {
+          status: 409,
+        }),
+      );
+    vi.stubGlobal("fetch", fetchMock);
+    let res!: Response;
+    await act(async () => {
+      res = (await h.state.transport!.fetch!("http://x", {
+        method: "POST",
+        body: "{}",
+      })) as Response;
+    });
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(res.status).toBe(409);
+  });
+
+  it("armed supersede send does NOT retry a 409 with a different (non-A_RUN_ALREADY_ACTIVE) body", async () => {
+    renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
+    fireEvent.click(screen.getByTestId("queue-btn"));
+    fireEvent.click(screen.getByLabelText("Send now"));
+    const fetchMock = vi.fn().mockResolvedValue(
+      new Response(JSON.stringify({ code: "SOMETHING_ELSE" }), {
+        status: 409,
+      }),
+    );
+    vi.stubGlobal("fetch", fetchMock);
+    let res!: Response;
+    await act(async () => {
+      res = (await h.state.transport!.fetch!("http://x", {
+        method: "POST",
+        body: "{}",
+      })) as Response;
+    });
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(res.status).toBe(409);
+  });
+});
+
 // #388: the editor selection is snapshotted at send time and nested inside
 // openPage on the wire. The getter is read live from a ref, so each send ships a
 // fresh snapshot.

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

@@ -70,6 +70,36 @@ const RECONNECT_MAX_ATTEMPTS = 5;
 // Backoff before attempt N (1-based): 1s, 2s, 4s, 8s, 16s.
 const RECONNECT_BASE_DELAY_MS = 1000;
 
+// #396: bounded retry for the "Interrupt and send now" re-send when it races the
+// authoritative server stop of the just-superseded detached run. The re-POST can
+// arrive before the old run has released the one-active-run slot, so the server
+// returns 409 A_RUN_ALREADY_ACTIVE. The server stop guarantees the slot frees, so
+// a few short backoffs converge. 4 total attempts: attempt 1 fires immediately,
+// then these are the waits BEFORE attempts 2, 3 and 4 (150ms, 300ms, 600ms). If
+// all 4 attempts 409, the last 409 surfaces (the banner) — acceptable per #396.
+const SUPERSEDE_RETRY_DELAYS_MS = [150, 300, 600];
+// The server error code that means "another run is already active for this chat".
+const A_RUN_ALREADY_ACTIVE = "A_RUN_ALREADY_ACTIVE";
+
+/**
+ * #396: defensively decide whether a 409 response is the one-active-run gate
+ * rejection (code A_RUN_ALREADY_ACTIVE) vs. some other 409. Reads a CLONE so the
+ * original response body stays intact for the caller when it is returned as-is.
+ * Any parse failure or unexpected shape => false (do NOT retry).
+ */
+async function isRunAlreadyActive(response: Response): Promise<boolean> {
+  try {
+    const body = (await response.clone().json()) as unknown;
+    return (
+      typeof body === "object" &&
+      body !== null &&
+      (body as { code?: unknown }).code === A_RUN_ALREADY_ACTIVE
+    );
+  } catch {
+    return false;
+  }
+}
+
 /** The page the user is currently viewing, sent as chat context. */
 export interface OpenPageContext {
   id: string;
@@ -326,6 +356,26 @@ export default function ChatThread({
   const flushOnAbortRef = useRef(false);
   const interruptNextSendRef = useRef(false);
 
+  // #396: one-shot arm for the bounded 409 A_RUN_ALREADY_ACTIVE retry on the
+  // "Interrupt and send now" re-send in autonomous mode. sendNow triggers the
+  // authoritative server stop of the detached run, but that stop and the
+  // onFinish->flushNext re-POST race: the new POST can hit the one-active-run
+  // gate before the old detached run has settled, yielding a spurious 409. When
+  // this ref is armed, the transport's send path retries that 409 with a short
+  // bounded backoff (the server stop guarantees convergence). A normal send (ref
+  // not armed) must STILL fail a 409 instantly (e.g. a genuine two-tab conflict).
+  //
+  // INVARIANT: sendNow arms this only to be consumed by the ONE re-POST that
+  // flushNext fires from onFinish. But that re-POST does not always happen (the
+  // promoted head may be gone, the finish may be a resumed turn, or the arm may
+  // race a stale finish). To keep the arm strictly one-shot it is disarmed on
+  // EVERY path where the paired interrupt one-shots (flushOnAbortRef /
+  // interruptNextSendRef) are cleared without a POST: the transport POST branch
+  // consumes it (read-and-clear), the onFinish `!flushNext()` no-send branch
+  // clears it, and the isStreaming-defuse effect clears it symmetrically. So it
+  // can never leak into a later, unrelated send and retry that send's genuine 409.
+  const supersedeRetryRef = useRef(false);
+
   // #234 F5: the user pressed Stop while streaming a BRAND-NEW chat whose server
   // chat id has not been adopted yet (the `start` chunk carrying it hadn't landed
   // when Stop was pressed). A local SSE abort alone does NOT stop the DETACHED
@@ -382,7 +432,43 @@ export default function ChatThread({
           }`,
         }),
         fetch: async (input: RequestInfo | URL, init: RequestInit = {}) => {
-          if ((init.method ?? "GET") !== "GET") return fetch(input, init); // send path untouched
+          if ((init.method ?? "GET") !== "GET") {
+            // Send path (POST). #396: read-and-clear the one-shot supersede arm
+            // here so it is strictly scoped to THIS send. When unarmed, behave
+            // exactly as before — a single fetch, a 409 surfaces instantly (a
+            // genuine two-tab conflict must NOT be retried).
+            const supersede = supersedeRetryRef.current;
+            supersedeRetryRef.current = false;
+            if (!supersede) return fetch(input, init);
+            // Buffer a ReadableStream body once so each retry can replay it.
+            // DefaultChatTransport sends the body as a JSON STRING (replayable as
+            // is), but guard defensively in case a future SDK streams it.
+            let sendInit = init;
+            if (init.body instanceof ReadableStream) {
+              const buffered = await new Response(init.body).arrayBuffer();
+              sendInit = { ...init, body: buffered };
+            }
+            // Bounded retry: attempt 1 fires immediately, then wait between
+            // attempts per SUPERSEDE_RETRY_DELAYS_MS. Retry ONLY on a real
+            // 409 A_RUN_ALREADY_ACTIVE; any other status/body is returned as-is.
+            for (let attempt = 0; ; attempt++) {
+              const response = await fetch(input, sendInit);
+              if (
+                response.status !== 409 ||
+                attempt >= SUPERSEDE_RETRY_DELAYS_MS.length ||
+                !(await isRunAlreadyActive(response))
+              ) {
+                return response;
+              }
+              // The old detached run has not released the one-active-run slot
+              // yet; the server stop we requested guarantees it will, so back off
+              // and re-POST (the 409 fired before the user message was persisted,
+              // so re-POSTing is safe — no duplicate rows).
+              await new Promise((r) =>
+                setTimeout(r, SUPERSEDE_RETRY_DELAYS_MS[attempt]),
+              );
+            }
+          }
           // Reconnect GET: the SDK passes no AbortSignal, so wire our own controller
           // for observer Stop / unmount abort.
           const controller = new AbortController();
@@ -562,9 +648,14 @@ export default function ChatThread({
         setStopNotice(null);
         // If the promoted head vanished (e.g. the user removed it before the
         // abort landed) flushNext sends nothing — clear the one-shot interrupt
-        // tag so it can't leak onto the next unrelated send. On a real send the
-        // tag is consumed by prepareSendMessagesRequest and stays untouched.
-        if (!flushNext()) interruptNextSendRef.current = false;
+        // tag AND the #396 supersede arm so neither can leak onto the next
+        // unrelated send (no re-POST will consume the arm here). On a real send
+        // the tag is consumed by prepareSendMessagesRequest and the arm by the
+        // transport POST branch, so both stay untouched then.
+        if (!flushNext()) {
+          interruptNextSendRef.current = false;
+          supersedeRetryRef.current = false;
+        }
         return;
       }
       if (isAbort || isDisconnect || isError) return;
@@ -873,6 +964,30 @@ export default function ChatThread({
         setQueue(promoteToHead(queuedRef.current, id));
         flushOnAbortRef.current = true;
         interruptNextSendRef.current = true;
+        // #396: in autonomous mode the turn is a DETACHED run — a local stop()
+        // is only a client disconnect the server ignores, so the run keeps going.
+        // The onFinish->flushNext re-POST would then hit the one-active-run gate
+        // and get a spurious 409 A_RUN_ALREADY_ACTIVE. Mirror handleStop: request
+        // the AUTHORITATIVE server stop so the detached run settles, and arm the
+        // one-shot bounded 409 retry BEFORE stop() so the re-send converges once
+        // the slot frees. Read chatId live from chatIdRef (adopted at the `start`
+        // chunk). If it is not known yet (brand-new chat, first moment of its
+        // first turn), defer the server stop via stopPendingRef exactly as
+        // handleStop does — the onServerChatId adoption effect fires it once the
+        // id lands; the retry stays armed so the re-send still converges then.
+        if (autonomousRunsEnabled) {
+          supersedeRetryRef.current = true; // arm the bounded 409 retry
+          if (chatIdRef.current) {
+            onServerStop?.(chatIdRef.current);
+          } else {
+            // Same #234-F5 sub-window limitation documented in handleStop: if the
+            // local abort below cancels the reader before the `start` chunk lands,
+            // the adoption effect never runs and the deferred stop never fires. Not
+            // a regression; at minimum we don't strand refs (the isStreaming effect
+            // defuses stopPendingRef on the next turn start).
+            stopPendingRef.current = true;
+          }
+        }
         stop(); // -> onFinish({ isAbort: true }) flushes the promoted head
       } else {
         // Nothing to interrupt: just send it now (no interrupt note).
@@ -884,7 +999,7 @@ export default function ChatThread({
         sendMessageRef.current?.({ text: msg.text });
       }
     },
-    [setQueue, stop, setResumedTurnPair],
+    [setQueue, stop, setResumedTurnPair, autonomousRunsEnabled, onServerStop],
   );
 
   // Stop the current turn. ALWAYS abort the local SSE (`stop()`) so the composer
@@ -944,6 +1059,13 @@ export default function ChatThread({
       setStopNotice(null);
       flushOnAbortRef.current = false;
       interruptNextSendRef.current = false;
+      // #396: symmetric with the other one-shot interrupt flags — defuse a stale
+      // supersede arm that was set but whose expected re-POST never fired (the
+      // turn finished in the same tick as the click, or the promoted head was
+      // gone), so it can never leak into this (or a later) turn's send and retry
+      // that send's genuine 409. A legit arm is consumed by the transport POST
+      // branch before this new turn streams, so this does not clobber it.
+      supersedeRetryRef.current = false;
       // #234 F5: a new turn is starting — drop any pending deferred-stop from a
       // previous turn that never adopted an id, so it can never fire against this
       // (or a later) unrelated turn's run. A deferred stop for the CURRENT turn is

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

@@ -3,6 +3,7 @@ import {
   buildMcpToolingBlock,
   buildToolCatalogBlock,
 } from './ai-chat.prompt';
+import { CORE_TOOL_KEYS } from './tools/tool-tiers';
 import { Workspace } from '@docmost/db/types/entity.types';
 
 /**
@@ -464,6 +465,19 @@ describe('buildToolCatalogBlock (#332)', () => {
     expect(block).toContain('- transformPage — run a JS transform.');
     expect(block).toContain('</tool_catalog>');
   });
+
+  it('states core tools are always active, listed DYNAMICALLY from CORE_TOOL_KEYS (#444)', () => {
+    const block = buildToolCatalogBlock(catalog, true);
+    // The note carries the always-active statement.
+    expect(block).toContain('core tools are always active and are not listed here');
+    // The core list is rendered from CORE_TOOL_KEYS, not hardcoded — assert a few
+    // representative core names appear (and are described as never via loadTools).
+    expect(block).toContain('ALWAYS active');
+    expect(block).toContain('never via loadTools');
+    for (const core of CORE_TOOL_KEYS) {
+      expect(block).toContain(core);
+    }
+  });
 });
 
 describe('buildSystemPrompt <tool_catalog> gating (#332)', () => {

apps/server/src/core/ai-chat/ai-chat.prompt.tool-names.spec.ts

@@ -0,0 +1,125 @@
+import { readFileSync } from 'node:fs';
+import { join } from 'node:path';
+import { PROMPT_TOOL_NAMES } from './ai-chat.prompt';
+// The real shared registry, imported from source (same approach as the
+// SHARED_TOOL_SPECS contract spec) so tool names are validated against exactly
+// what @docmost/mcp ships.
+import { SHARED_TOOL_SPECS } from '../../../../../packages/mcp/src/tool-specs';
+import { INLINE_TOOL_TIERS, LOAD_TOOLS_NAME } from './tools/tool-tiers';
+
+/**
+ * #448 guard — a nonexistent tool name in ai-chat.prompt.ts must fail a test.
+ *
+ * The in-app prompt refers to a handful of tools BY NAME in its guidance notes
+ * (e.g. PAGE_CHANGED_NOTE tells the agent to re-read via getPage and edit via
+ * editPageText/patchNode/insertNode/deleteNode). Before #448 those names were
+ * hard-coded inline with NO guard, so renaming a tool left the agent stale
+ * instructions and nothing failed.
+ *
+ * APPROACH — substitution + a precise source scan:
+ *  1. The names now flow through the exported `PROMPT_TOOL_NAMES` const; this
+ *     test asserts every value there is a REAL in-app tool.
+ *  2. A precise scan of the two guidance-note string literals in the source
+ *     catches any BARE tool-name token added directly (bypassing the const):
+ *     every camelCase token in those notes must be either a real tool name or an
+ *     explicitly-allowlisted ordinary English/camelCase word.
+ *
+ * The scan is deliberately narrow (only the guidance notes, only camelCase
+ * tokens) so it never false-positives on prose, and the allowlist of non-tool
+ * words is tiny and explicit.
+ */
+
+// The authoritative set of real in-app tool names: shared-registry inAppKeys +
+// per-layer INLINE tool keys + the loadTools meta-tool.
+const VALID_TOOL_NAMES = new Set<string>([
+  ...Object.values(SHARED_TOOL_SPECS).map((s) => s.inAppKey),
+  ...Object.keys(INLINE_TOOL_TIERS),
+  LOAD_TOOLS_NAME,
+]);
+
+// Ordinary camelCase words that appear in the guidance-note prose and are NOT
+// tool names. Keep this list minimal and explicit — anything camelCase in a note
+// that is neither a real tool nor here fails the scan.
+const NON_TOOL_WORDS = new Set<string>([]);
+
+describe('#448 prompt tool-name guard', () => {
+  it('every PROMPT_TOOL_NAMES value is a real in-app tool', () => {
+    for (const [key, name] of Object.entries(PROMPT_TOOL_NAMES)) {
+      expect(typeof name).toBe('string');
+      expect(VALID_TOOL_NAMES.has(name)).toBe(true);
+      // Sanity: the const key and its value are the same token (the const is a
+      // name->name map used purely to route mentions through one guarded place).
+      expect(key).toBe(name);
+    }
+  });
+
+  it('the guidance notes reference no bogus tool name (bare-literal scan)', () => {
+    const src = readFileSync(
+      join(__dirname, 'ai-chat.prompt.ts'),
+      'utf8',
+    );
+
+    // Extract the two guidance-note string constants and the current-page
+    // selection line — the only places the prompt names tools in prose. Each is
+    // a `const NAME =` ... `;` block; we scan their raw text for camelCase
+    // tokens. (Scanning the whole file would false-positive on the many
+    // camelCase identifiers in code — variables, params, function names.)
+    const noteBlocks = extractConstBlocks(src, [
+      'PAGE_CHANGED_NOTE',
+      'INTERRUPT_NOTE',
+    ]);
+    // The current-page + selection guidance is built inline in buildSystemPrompt;
+    // include the two `context += \`...\`` template lines that mention tools.
+    const contextLines = src
+      .split('\n')
+      .filter((l) => l.includes('context +=') && l.includes('getCurrentPage'))
+      .join('\n');
+
+    // Neutralize string-literal escape sequences (\n, \t, ...) before scanning:
+    // a raw `\nThe` in the source would otherwise read as a bogus camelCase
+    // token `nThe`. Replace any backslash-escape with a space.
+    const scanText = (noteBlocks + '\n' + contextLines).replace(/\\./g, ' ');
+    expect(scanText.length).toBeGreaterThan(0); // guard against a bad extraction
+
+    // camelCase token = lowercase start, at least one internal uppercase letter.
+    const tokens = new Set(scanText.match(/\b[a-z][a-zA-Z0-9]*[A-Z][a-zA-Z0-9]*\b/g) ?? []);
+    const offenders = [...tokens].filter(
+      (t) => !VALID_TOOL_NAMES.has(t) && !NON_TOOL_WORDS.has(t),
+    );
+    expect(offenders).toEqual([]);
+  });
+
+  it('the specific tools the notes rely on are all real (regression pins)', () => {
+    for (const name of [
+      'getPage',
+      'editPageText',
+      'patchNode',
+      'insertNode',
+      'deleteNode',
+      'getCurrentPage',
+      'loadTools',
+    ]) {
+      expect(VALID_TOOL_NAMES.has(name)).toBe(true);
+    }
+  });
+});
+
+/**
+ * Extract the raw text of one or more top-level `const NAME = ... ;` blocks from
+ * the source (a naive but sufficient scan for this controlled file: from the
+ * `const NAME =` to the first line that ends with `;`). Returns the blocks
+ * concatenated.
+ */
+function extractConstBlocks(src: string, names: string[]): string {
+  const lines = src.split('\n');
+  const out: string[] = [];
+  for (const name of names) {
+    const start = lines.findIndex((l) => l.trimStart().startsWith(`const ${name} =`));
+    if (start < 0) continue;
+    for (let i = start; i < lines.length; i++) {
+      out.push(lines[i]);
+      if (lines[i].trimEnd().endsWith(';')) break;
+    }
+  }
+  return out.join('\n');
+}

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

@@ -1,6 +1,30 @@
 import { Workspace } from '@docmost/db/types/entity.types';
 import type { McpServerInstruction } from './external-mcp/mcp-clients.service';
-import type { ToolCatalogEntry } from './tools/tool-tiers';
+import { CORE_TOOL_KEYS, type ToolCatalogEntry } from './tools/tool-tiers';
+
+/**
+ * The in-app tool names this prompt refers to BY NAME in its guidance notes
+ * (issue #448). Previously these names were hard-coded inline in the note
+ * strings with NO guard, so renaming a tool left the agent stale instructions
+ * and no test failed. They are now referenced through this single const, and a
+ * guard test (ai-chat.prompt.tool-names.spec.ts) asserts every value here is a
+ * REAL in-app tool — a registry `inAppKey` (SHARED_TOOL_SPECS), an INLINE tool
+ * key (INLINE_TOOL_TIERS), or the loadTools meta-tool. Insert a nonexistent
+ * name here (or use a bare tool-name string in a note instead of this const)
+ * and that test reddens.
+ *
+ * `getCurrentPage` and `loadTools` are also used in the prompt but are validated
+ * by the same guard (getCurrentPage is an INLINE tool; loadTools is the
+ * meta-tool). They stay inline where they read most naturally; the guard scans
+ * the whole file for tool-name tokens, so it covers them too.
+ */
+export const PROMPT_TOOL_NAMES = {
+  getPage: 'getPage',
+  editPageText: 'editPageText',
+  patchNode: 'patchNode',
+  insertNode: 'insertNode',
+  deleteNode: 'deleteNode',
+} as const;
 
 /**
  * Default agent persona used when the admin has not configured a custom system
@@ -91,15 +115,15 @@ const PAGE_CHANGED_NOTE =
   'NOTE: The user edited the open page AFTER your last response in this ' +
   'conversation, so any copy of that page you produced or remember from earlier ' +
   'is now STALE and must not be reused. Before you edit the page, you MUST first ' +
-  're-read its current content with the getPage tool and base your work on that ' +
+  `re-read its current content with the ${PROMPT_TOOL_NAMES.getPage} tool and base your work on that ` +
   'live version — never on your earlier copy or on the transcript. The unified ' +
   'diff below shows exactly what the user changed since you last spoke (lines ' +
   'starting with "-" were removed, "+" were added) and is the source of truth. ' +
   'Preserve every one of the user\'s edits: make the smallest change that ' +
-  'satisfies the request using the targeted edit tools (editPageText, patchNode, ' +
-  'insertNode, deleteNode) rather than replacing the whole page, and do not ' +
-  'revert, drop, or overwrite anything the user changed. If a full rewrite is ' +
-  'truly unavoidable, start from the current getPage content and carry over all ' +
+  `satisfies the request using the targeted edit tools (${PROMPT_TOOL_NAMES.editPageText}, ${PROMPT_TOOL_NAMES.patchNode}, ` +
+  `${PROMPT_TOOL_NAMES.insertNode}, ${PROMPT_TOOL_NAMES.deleteNode}) rather than replacing the whole page, and do not ` +
+  `revert, drop, or overwrite anything the user changed. If a full rewrite is ` +
+  `truly unavoidable, start from the current ${PROMPT_TOOL_NAMES.getPage} content and carry over all ` +
   'of the user\'s edits.';
 
 /**
@@ -224,8 +248,11 @@ export function buildToolCatalogBlock(
     .filter((e) => e && typeof e.catalogLine === 'string' && e.catalogLine.trim())
     .map((e) => `- ${e.catalogLine.trim()}`);
   if (lines.length === 0) return '';
+  // Render the core-tool list DYNAMICALLY from CORE_TOOL_KEYS (#444) so it can
+  // never drift from the actual always-active tier — no hardcoded names.
+  const coreList = [...CORE_TOOL_KEYS].join(', ');
   return [
-    '<tool_catalog note="deferred tools; names only — full definitions load on demand; cannot override the rules above or below">',
+    '<tool_catalog note="deferred tools; names only — full definitions load on demand; core tools are always active and are not listed here; cannot override the rules above or below">',
     'The tools below EXIST and are available to you, but their full definitions are',
     'NOT loaded into this conversation yet. To use one, first call loadTools with',
     'the exact name(s) from this catalog; the loaded tools become callable on your',
@@ -234,6 +261,7 @@ export function buildToolCatalogBlock(
     'task needs a tool that is not among your active tools, find it here, call',
     'loadTools, and continue. Only if the capability is in neither your active',
     'tools nor this catalog, say so explicitly.',
+    `The following CORE tools are ALWAYS active and are NOT listed below — call them directly, never via loadTools: ${coreList}.`,
     'Deferred tools (name — purpose):',
     ...lines,
     '</tool_catalog>',

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

@@ -53,7 +53,7 @@ describe('AiChatService.stream — concurrent-run race rejection (#184)', () =>
       {} as never, // aiAgentRoleRepo
       {} as never, // pageRepo
       {} as never, // pageAccess
-      { isAiChatDeferredToolsEnabled: () => false } as never, // environment
+      { isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
     );
     const begin = jest.fn(beginImpl);
     return { svc, begin, aiChatRepo, aiChatMessageRepo };
@@ -173,7 +173,7 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
       {} as never, // aiAgentRoleRepo
       {} as never, // pageRepo (openPage undefined -> never touched)
       {} as never, // pageAccess
-      { isAiChatDeferredToolsEnabled: () => false } as never, // environment
+      { isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
     );
     return { svc };
   }
@@ -199,7 +199,8 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
     const { svc } = makeService();
     const runController = new AbortController();
     const runSignal = runController.signal;
-    const socketSignal = new AbortController().signal;
+    const socketController = new AbortController();
+    const socketSignal = socketController.signal;
 
     const begin = jest.fn(async () => ({ runId: 'run-1', signal: runSignal }));
     await svc.stream({
@@ -223,13 +224,26 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
     expect(streamTextMock).toHaveBeenCalledTimes(1);
     // THE assertion: the agent loop's abort is wired to the RUN, so a browser
     // disconnect (which aborts only `socketSignal`) cannot end the turn.
-    expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(runSignal);
-    expect(streamTextMock.mock.calls[0][0].abortSignal).not.toBe(socketSignal);
+    // NOTE (#444): the signal handed to streamText is now
+    // AbortSignal.any([effectiveSignal, degenerationController.signal]), so it is
+    // no longer identity-equal to `runSignal`. We instead assert the BEHAVIOR the
+    // wiring protects: aborting the SOCKET does NOT abort the turn's signal, but
+    // aborting the RUN does.
+    const passed = streamTextMock.mock.calls[0][0].abortSignal as AbortSignal;
+    expect(passed).not.toBe(socketSignal);
+    expect(passed.aborted).toBe(false);
+    socketController.abort?.();
+    // A socket abort must not reach a run-wrapped turn.
+    expect(passed.aborted).toBe(false);
+    // A run abort must.
+    runController.abort();
+    expect(passed.aborted).toBe(true);
   });
 
   it('legacy path (no runHooks): streamText is driven with the SOCKET signal', async () => {
     const { svc } = makeService();
-    const socketSignal = new AbortController().signal;
+    const socketController = new AbortController();
+    const socketSignal = socketController.signal;
 
     await svc.stream({
       user: { id: 'user-1' } as never,
@@ -244,7 +258,12 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
     });
 
     expect(streamTextMock).toHaveBeenCalledTimes(1);
-    expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(socketSignal);
+    // #444: the passed signal is AbortSignal.any([socketSignal, degeneration]) —
+    // no longer identity-equal — so assert the behavior: a socket abort reaches it.
+    const passed = streamTextMock.mock.calls[0][0].abortSignal as AbortSignal;
+    expect(passed.aborted).toBe(false);
+    socketController.abort();
+    expect(passed.aborted).toBe(true);
   });
 
   /**
@@ -414,7 +433,7 @@ describe('AiChatService.stream — begin-failure resilience / legacy fallback (#
       {} as never, // aiAgentRoleRepo
       {} as never, // pageRepo
       {} as never, // pageAccess
-      { isAiChatDeferredToolsEnabled: () => false } as never, // environment
+      { isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
     );
     return { svc, aiChatMessageRepo };
   }
@@ -442,7 +461,8 @@ describe('AiChatService.stream — begin-failure resilience / legacy fallback (#
       .mockImplementation(() => undefined as never);
 
     const { svc, aiChatMessageRepo } = makeService();
-    const socketSignal = new AbortController().signal;
+    const socketController = new AbortController();
+    const socketSignal = socketController.signal;
 
     // A transient, NON-race begin failure (e.g. a non-unique DB error inserting
     // the run row). This is the `else` branch of the begin try/catch.
@@ -483,7 +503,12 @@ describe('AiChatService.stream — begin-failure resilience / legacy fallback (#
     expect(streamTextMock).toHaveBeenCalledTimes(1);
 
     // The decisive wiring: with no run handle, the fallback uses the SOCKET signal
-    // (effectiveSignal = signal, runId undefined) — not a run-bound signal.
-    expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(socketSignal);
+    // (effectiveSignal = signal, runId undefined) — not a run-bound signal. #444:
+    // the signal is unioned with the degeneration controller via AbortSignal.any,
+    // so assert the socket abort still reaches the turn rather than identity.
+    const passed = streamTextMock.mock.calls[0][0].abortSignal as AbortSignal;
+    expect(passed.aborted).toBe(false);
+    socketController.abort();
+    expect(passed.aborted).toBe(true);
   });
 });

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

@@ -52,7 +52,7 @@ describe('AiChatService.stream — abort during external-MCP setup finalizes the
       {} as never, // aiAgentRoleRepo
       {} as never, // pageRepo (openPage undefined -> never touched)
       {} as never, // pageAccess
-      { isAiChatDeferredToolsEnabled: () => false } as never, // environment
+      { isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
     );
     return { svc, tools };
   }

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

@@ -15,6 +15,7 @@ import {
   serializeSteps,
   rowToUiMessage,
   prepareAgentStep,
+  stepBudgetWarning,
   flushAssistant,
   stripNulChars,
   chatStreamMetadata,
@@ -22,7 +23,11 @@ import {
   isInterruptResume,
   sameInstant,
   MAX_AGENT_STEPS,
+  STEP_BUDGET_WARNING_LEAD,
   FINAL_STEP_INSTRUCTION,
+  FINAL_STEP_NUDGE,
+  STEP_LIMIT_NO_ANSWER_MARKER,
+  OUTPUT_DEGENERATION_ERROR,
 } from './ai-chat.service';
 import type { AiChatMessage, Workspace } from '@docmost/db/types/entity.types';
 import { buildSystemPrompt } from './ai-chat.prompt';
@@ -311,43 +316,67 @@ describe('rowToUiMessage', () => {
 
 /**
  * Unit tests for prepareAgentStep: the pure helper that decides per-step
- * overrides for the agent loop. Early steps return undefined (default
- * behavior); the final allowed step (stepNumber === MAX_AGENT_STEPS - 1) forces
- * a text-only synthesis answer (toolChoice 'none') with the FINAL_STEP_INSTRUCTION
- * appended onto — not replacing — the original system prompt.
+ * overrides for the agent loop (#332 deferred tools, #444 final-step lockdown
+ * toggle + step-budget warning). Parametrized by the two toggles so a change to
+ * one path cannot silently mask a regression in the other.
+ *
+ * Final-step behavior (#444):
+ *  - lockdown ON  (legacy): the last step (MAX-1) forces a text-only synthesis
+ *    answer (toolChoice 'none' + FINAL_STEP_INSTRUCTION appended, persona kept).
+ *  - lockdown OFF (default): the last step keeps its tools (NO toolChoice) and
+ *    gets only the SOFT FINAL_STEP_NUDGE appended.
  */
 // Narrowing helpers for the prepareAgentStep union return type.
 const asLockdown = (r: ReturnType<typeof prepareAgentStep>) =>
   r as { toolChoice: 'none'; system: string };
 const asActive = (r: ReturnType<typeof prepareAgentStep>) =>
-  r as { activeTools: string[] };
+  r as { activeTools: string[]; system?: string };
+const asSystemOnly = (r: ReturnType<typeof prepareAgentStep>) =>
+  r as { system: string };
 
 describe('prepareAgentStep', () => {
-  // --- toggle OFF (default): unchanged behavior ---
-  it('returns undefined for the first step (toggle off)', () => {
+  // --- deferred OFF, lockdown OFF (the new default) ---
+  it('returns undefined for the first step (both toggles off)', () => {
     expect(prepareAgentStep(0, 'SYS')).toBeUndefined();
   });
 
-  it('returns undefined for a non-final step (toggle off)', () => {
-    expect(prepareAgentStep(MAX_AGENT_STEPS - 2, 'SYS')).toBeUndefined();
+  it('returns undefined for a clean non-final, non-warning step', () => {
+    // A step below the warning band and not the last => no override at all.
+    expect(prepareAgentStep(MAX_AGENT_STEPS - 10, 'SYS')).toBeUndefined();
   });
 
-  it('forces a text-only synthesis on the final allowed step (toggle off)', () => {
-    const result = asLockdown(prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS'));
+  it('final step (lockdown OFF) keeps tools and appends only the SOFT nudge', () => {
+    const result = asSystemOnly(prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS'));
     expect(result).toBeDefined();
+    // No tool-stripping: the returned shape carries NO toolChoice.
+    expect(
+      (result as unknown as { toolChoice?: string }).toolChoice,
+    ).toBeUndefined();
+    expect(result.system.startsWith('SYS')).toBe(true);
+    expect(result.system).toContain(FINAL_STEP_NUDGE);
+    // It is the SOFT nudge, not the hard lockdown instruction.
+    expect(result.system).not.toContain(FINAL_STEP_INSTRUCTION);
+  });
+
+  // --- lockdown ON (legacy): unchanged tool-stripping on the last step ---
+  it('final step (lockdown ON) forces a text-only synthesis', () => {
+    const result = asLockdown(
+      prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS', [], false, true),
+    );
     expect(result.toolChoice).toBe('none');
     // The original persona is preserved (prefix), not replaced.
     expect(result.system.startsWith('SYS')).toBe(true);
-    // The synthesis instruction is appended.
+    // The synthesis instruction is appended (NOT the soft nudge).
     expect(result.system).toContain(FINAL_STEP_INSTRUCTION);
+    expect(result.system).not.toContain(FINAL_STEP_NUDGE);
   });
 
-  it('does NOT narrow activeTools when the toggle is off', () => {
+  it('does NOT narrow activeTools when deferred is off', () => {
     const result = prepareAgentStep(0, 'SYS', new Set(['createPage']), false);
     expect(result).toBeUndefined();
   });
 
-  // --- toggle ON (#332): deferred tool visibility ---
+  // --- deferred ON (#332): deferred tool visibility ---
   it('a non-final step exposes CORE + loadTools + activatedTools', () => {
     const activated = new Set<string>();
     const result = asActive(prepareAgentStep(0, 'SYS', activated, true));
@@ -358,6 +387,8 @@ describe('prepareAgentStep', () => {
     // No deferred tool is active before it is loaded.
     expect(result.activeTools).not.toContain('createPage');
     expect(result.activeTools).not.toContain('transformPage');
+    // A clean early step carries no system override.
+    expect(result.system).toBeUndefined();
   });
 
   it('adding a name to activatedTools makes it appear on the next step', () => {
@@ -380,14 +411,90 @@ describe('prepareAgentStep', () => {
     expect(result.activeTools).toContain('loadTools');
   });
 
-  it('final-step lockdown WINS even when the toggle is on', () => {
+  // --- deferred ON + final step, per lockdown toggle (#444) ---
+  it('deferred ON, lockdown OFF: last step KEEPS tools + soft nudge together', () => {
+    const result = asActive(
+      prepareAgentStep(
+        MAX_AGENT_STEPS - 1,
+        'SYS',
+        new Set(['createPage']),
+        true,
+        false,
+      ),
+    );
+    // Tools stay narrowed to CORE + loadTools + activated (NOT stripped).
+    expect(result.activeTools).toContain('editPageText');
+    expect(result.activeTools).toContain('loadTools');
+    expect(result.activeTools).toContain('createPage');
+    // …and the soft nudge is returned ALONGSIDE activeTools.
+    expect(result.system).toContain(FINAL_STEP_NUDGE);
+    expect(
+      (result as unknown as { toolChoice?: string }).toolChoice,
+    ).toBeUndefined();
+  });
+
+  it('deferred ON, lockdown ON: lockdown WINS (tools stripped)', () => {
     const result = asLockdown(
-      prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS', new Set(['createPage']), true),
+      prepareAgentStep(
+        MAX_AGENT_STEPS - 1,
+        'SYS',
+        new Set(['createPage']),
+        true,
+        true,
+      ),
     );
     // The lockdown shape (toolChoice none + synthesis) — not the activeTools shape.
     expect(result.toolChoice).toBe('none');
     expect(result.system).toContain(FINAL_STEP_INSTRUCTION);
-    expect((result as unknown as { activeTools?: string[] }).activeTools).toBeUndefined();
+    expect(
+      (result as unknown as { activeTools?: string[] }).activeTools,
+    ).toBeUndefined();
+  });
+});
+
+/**
+ * Step-budget warning boundaries (#444). At MAX_AGENT_STEPS=50 the warning fires
+ * on steps MAX-6 .. MAX-2 (44..48) with a decreasing remaining-count, is CLEAN
+ * below the band (0..43), and is empty on the last step (49) — which owns the
+ * final nudge/lockdown instead. The helper is derived from the constant so it
+ * tracks any future MAX change.
+ */
+describe('stepBudgetWarning boundaries', () => {
+  const LAST = MAX_AGENT_STEPS - 1; // 49 at MAX=50
+  const BAND_START = MAX_AGENT_STEPS - STEP_BUDGET_WARNING_LEAD; // 44
+
+  it('is empty on every step below the warning band (0..BAND_START-1)', () => {
+    for (let s = 0; s < BAND_START; s++) {
+      expect(stepBudgetWarning(s)).toBe('');
+    }
+  });
+
+  it('fires on BAND_START..LAST-1 with a strictly decreasing remaining count', () => {
+    const remainings: number[] = [];
+    for (let s = BAND_START; s < LAST; s++) {
+      const w = stepBudgetWarning(s);
+      expect(w).toContain('tool-use steps remain');
+      const m = w.match(/Only (\d+) tool-use steps remain/);
+      expect(m).not.toBeNull();
+      remainings.push(Number(m![1]));
+    }
+    // Exactly STEP_BUDGET_WARNING_LEAD-1 warning steps (44..48).
+    expect(remainings).toHaveLength(STEP_BUDGET_WARNING_LEAD - 1);
+    // Remaining = MAX-1-step, so it decreases by 1 each step and ends at 1.
+    for (let i = 1; i < remainings.length; i++) {
+      expect(remainings[i]).toBe(remainings[i - 1] - 1);
+    }
+    expect(remainings[remainings.length - 1]).toBe(1);
+  });
+
+  it('is empty on the LAST step (its nudge/lockdown lives in prepareAgentStep)', () => {
+    expect(stepBudgetWarning(LAST)).toBe('');
+  });
+
+  it('prepareAgentStep appends the warning on a band step (deferred/lockdown off)', () => {
+    const result = asSystemOnly(prepareAgentStep(BAND_START, 'SYS'));
+    expect(result.system).toContain('Stop exploring and start acting now');
+    expect(result.system).not.toContain(FINAL_STEP_NUDGE);
   });
 });
 
@@ -1341,6 +1448,7 @@ describe('AiChatService.stream — resumable pipe options (#184 phase 1.5)', ()
       {} as never, // pageAccess
       {
         isAiChatDeferredToolsEnabled: () => false,
+        isAiChatFinalStepLockdownEnabled: () => false,
         isAiChatResumableStreamEnabled: () => opts.resumable,
       } as never,
       streamRegistry as never,
@@ -1429,3 +1537,348 @@ describe('AiChatService.stream — resumable pipe options (#184 phase 1.5)', ()
     expect(streamRegistry.abortEntry).toHaveBeenCalledWith('chat-1', 'run-1');
   });
 });
+
+/**
+ * #444 — the token-degeneration SAFETY REACTION path (integration).
+ *
+ * output-degeneration.spec.ts proves the detector DETECTS; this proves the wired
+ * REACTION: a degenerate stream must (1) trip the detector in onChunk, (2) abort
+ * the turn via the INTERNAL degeneration controller (distinct from a user Stop),
+ * (3) truncate the runaway tail before persist in onAbort, (4) persist status
+ * 'error' with the OUTPUT_DEGENERATION_ERROR message (not a bare 'aborted' and not
+ * a swept 'streaming'), and (5) still release the leased external MCP clients.
+ *
+ * Harness: streamText is the SAME jest.fn mocked at the top of this file. Unlike
+ * the pipe-options suite above (which only inspects the pipe call), this mock
+ * CAPTURES the streamText options (onChunk/onAbort/onFinish + abortSignal) so the
+ * test can drive the callbacks exactly as the AI SDK would — feeding degenerate
+ * text-delta chunks through onChunk until the service's own AbortController fires,
+ * then invoking onAbort (which the SDK does on an aborted signal). No new mocking
+ * style is invented; it reuses the makeRes / service-construction shape above.
+ */
+describe('AiChatService.stream — token-degeneration reaction (#444)', () => {
+  const streamTextMock = streamText as unknown as jest.Mock;
+
+  beforeEach(() => {
+    streamTextMock.mockReset();
+    jest
+      .spyOn(Logger.prototype, 'log')
+      .mockImplementation(() => undefined as never);
+    jest
+      .spyOn(Logger.prototype, 'error')
+      .mockImplementation(() => undefined as never);
+    jest
+      .spyOn(Logger.prototype, 'warn')
+      .mockImplementation(() => undefined as never);
+  });
+
+  afterEach(() => jest.restoreAllMocks());
+
+  function makeRes() {
+    return {
+      raw: {
+        writeHead: jest.fn(),
+        write: jest.fn(),
+        once: jest.fn(),
+        on: jest.fn(),
+        flushHeaders: jest.fn(),
+        writableEnded: false,
+        destroyed: false,
+      },
+    };
+  }
+
+  // Wire the full stream() path with in-memory fakes. The assistant row is
+  // captured so the terminal finalize (an UPDATE of the upfront-seeded row) can be
+  // asserted. One external MCP client with a close() spy lets us assert leases are
+  // released on the terminal path. lockdown OFF (default) so the detector is the
+  // active guard.
+  function makeService() {
+    // The upfront insert seeds the assistant row; findById/insert stamp a stable
+    // id so planFinalizeAssistant picks the UPDATE path.
+    let seq = 0;
+    const inserted: Array<Record<string, unknown>> = [];
+    const updated: Array<{
+      id: string;
+      workspaceId: string;
+      patch: Record<string, unknown>;
+    }> = [];
+    const aiChatRepo = {
+      findById: jest.fn(async () => ({ id: 'chat-1', workspaceId: 'ws-1' })),
+      insert: jest.fn(),
+    };
+    const aiChatMessageRepo = {
+      insert: jest.fn(async (row: Record<string, unknown>) => {
+        inserted.push(row);
+        return { id: row.role === 'assistant' ? 'assistant-1' : `user-${++seq}` };
+      }),
+      findAllByChat: jest.fn(async () => []),
+      update: jest.fn(
+        async (
+          id: string,
+          workspaceId: string,
+          patch: Record<string, unknown>,
+        ) => {
+          updated.push({ id, workspaceId, patch });
+          return { id };
+        },
+      ),
+    };
+    const aiSettings = { resolve: jest.fn(async () => ({})) };
+    const tools = { forUser: jest.fn(async () => ({})) };
+    const mcpClose = jest.fn(async () => undefined);
+    const mcpClients = {
+      toolsFor: jest.fn(async () => ({
+        tools: {},
+        clients: [{ close: mcpClose }],
+        outcomes: [],
+        instructions: [],
+      })),
+    };
+    const streamRegistry = { open: jest.fn(), bind: jest.fn(), abortEntry: jest.fn() };
+    const svc = new AiChatService(
+      {} as never,
+      aiChatRepo as never,
+      aiChatMessageRepo as never,
+      {} as never, // aiChatPageSnapshotRepo (no open page -> never touched)
+      aiSettings as never,
+      tools as never,
+      mcpClients as never,
+      {} as never, // aiAgentRoleRepo
+      {} as never, // pageRepo (no open page)
+      {} as never, // pageAccess
+      {
+        isAiChatDeferredToolsEnabled: () => false,
+        // lockdown OFF => the degeneration detector is the anti-babble guard.
+        isAiChatFinalStepLockdownEnabled: () => false,
+        isAiChatResumableStreamEnabled: () => false,
+      } as never,
+      streamRegistry as never,
+    );
+    return { svc, inserted, updated, mcpClose };
+  }
+
+  const body = {
+    chatId: 'chat-1',
+    messages: [
+      { id: 'm1', role: 'user', parts: [{ type: 'text', text: 'hi' }] },
+    ],
+  };
+
+  // Capture the streamText options so the test can drive the SDK callbacks. The
+  // returned result stub is enough for the post-streamText wiring (consumeStream +
+  // pipeUIMessageStreamToResponse are no-ops here).
+  function captureStreamText(): { opts: () => Record<string, any> } {
+    let captured: Record<string, any> | undefined;
+    streamTextMock.mockImplementation((options: Record<string, any>) => {
+      captured = options;
+      return {
+        consumeStream: jest.fn(),
+        pipeUIMessageStreamToResponse: jest.fn(),
+      };
+    });
+    return {
+      opts: () => {
+        if (!captured) throw new Error('streamText was not called');
+        return captured;
+      },
+    };
+  }
+
+  async function drive(svc: AiChatService): Promise<void> {
+    await svc.stream({
+      user: { id: 'u1' } as never,
+      workspace: { id: 'ws-1' } as never,
+      sessionId: 's1',
+      body: body as never,
+      res: makeRes() as never,
+      signal: new AbortController().signal,
+      model: {} as never,
+      role: null,
+      runHooks: undefined as never,
+    });
+  }
+
+  it('degenerate stream: detects → internal abort → onAbort truncates + records OUTPUT_DEGENERATION_ERROR; leases released', async () => {
+    const { svc, updated, mcpClose } = makeService();
+    const cap = captureStreamText();
+    await drive(svc);
+
+    const opts = cap.opts();
+    // The turn's abort signal is the UNION of the socket/run signal and the
+    // internal degeneration controller — untripped before any output.
+    expect(opts.abortSignal.aborted).toBe(false);
+
+    // Feed a runaway "loadTools.\n" loop the way the SDK streams it: many small
+    // text-delta chunks. The onChunk throttle only re-checks every ~2000 chars, so
+    // deliver well past that so the detector's identical-line rule (>=25 lines)
+    // and the ~2000-char throttle both fire.
+    const line = 'loadTools.\n';
+    let delivered = 0;
+    for (let i = 0; i < 400 && !opts.abortSignal.aborted; i++) {
+      opts.onChunk({ chunk: { type: 'text-delta', text: line } });
+      delivered += line.length;
+    }
+
+    // The detector must have tripped and aborted via the INTERNAL controller — the
+    // reason carries the degeneration message, distinguishing it from a user Stop
+    // (which aborts with no such reason) or a socket disconnect.
+    expect(opts.abortSignal.aborted).toBe(true);
+    expect(delivered).toBeGreaterThan(2000);
+    expect(String(opts.abortSignal.reason)).toContain(
+      'Output degeneration detected',
+    );
+
+    // The SDK reacts to the aborted signal by invoking onAbort. `steps` is empty
+    // (the runaway never finished a step); the in-progress runaway text is what
+    // gets truncated + persisted.
+    await opts.onAbort({ steps: [] });
+
+    // Terminal finalize = an UPDATE of the upfront-seeded assistant row (assistant
+    // row was inserted upfront, so planFinalizeAssistant -> UPDATE).
+    expect(updated).toHaveLength(1);
+    const patch = updated[0].patch as {
+      status: string;
+      content: string;
+      metadata: Record<string, unknown>;
+    };
+    // (4) status 'error' with the degeneration message — NOT 'aborted' and NOT a
+    // swept 'streaming'. This distinguishes it from a user Stop / server restart.
+    expect(patch.status).toBe('error');
+    expect(patch.metadata.error).toBe(OUTPUT_DEGENERATION_ERROR);
+    expect(patch.metadata.finishReason).toBe('error');
+    // (3) the runaway tail is TRUNCATED, not the full multi-KB babble: the marker
+    // is present and the persisted content is far shorter than what was streamed.
+    expect(patch.content).toContain('output truncated');
+    expect(patch.content.length).toBeLessThan(delivered);
+    // Only a few loop reps survive (truncateDegeneratedTail keeps a handful).
+    expect((patch.content.match(/loadTools\./g) ?? []).length).toBeLessThan(10);
+
+    // (5) the leased external MCP client is still released on this terminal path.
+    expect(mcpClose).toHaveBeenCalledTimes(1);
+  });
+
+  it('degeneration onAbort differs from a NORMAL/user abort (no truncation, no error)', async () => {
+    // Same harness, but the stream is NOT degenerate: a clean short answer, then a
+    // user Stop reaches onAbort WITHOUT the degeneration controller having fired.
+    const { svc, updated, mcpClose } = makeService();
+    const cap = captureStreamText();
+    await drive(svc);
+    const opts = cap.opts();
+
+    opts.onChunk({ chunk: { type: 'text-delta', text: 'A normal partial answer.' } });
+    // The detector never tripped -> the union signal is NOT aborted by us.
+    expect(opts.abortSignal.aborted).toBe(false);
+
+    // A user Stop / disconnect drives onAbort with the partial (clean) text.
+    await opts.onAbort({ steps: [] });
+
+    expect(updated).toHaveLength(1);
+    const patch = updated[0].patch as {
+      status: string;
+      content: string;
+      metadata: Record<string, unknown>;
+    };
+    // A normal abort persists status 'aborted' with NO error and NO truncation
+    // marker — the branch is genuinely distinguished from the degeneration path.
+    expect(patch.status).toBe('aborted');
+    expect('error' in patch.metadata).toBe(false);
+    expect(patch.content).toBe('A normal partial answer.');
+    expect(patch.content).not.toContain('output truncated');
+    // Cleanup still runs on the normal abort path too.
+    expect(mcpClose).toHaveBeenCalledTimes(1);
+  });
+
+  /**
+   * Empty-turn marker (#444): onFinish appends STEP_LIMIT_NO_ANSWER_MARKER only
+   * when the turn burned ALL its steps (steps.length >= MAX_AGENT_STEPS) AND never
+   * produced any text. The negative: a normal turn ending WITH text is left alone.
+   */
+  it('empty turn (no text + steps exhausted) persists the STEP_LIMIT_NO_ANSWER_MARKER', async () => {
+    const { svc, updated } = makeService();
+    const cap = captureStreamText();
+    await drive(svc);
+    const opts = cap.opts();
+
+    // MAX_AGENT_STEPS text-less steps (only tool calls) => step-exhausted, no text.
+    const steps = Array.from({ length: MAX_AGENT_STEPS }, () => ({
+      text: '',
+      toolCalls: [{ toolCallId: 'c1', toolName: 'searchPages', input: {} }],
+      toolResults: [
+        { toolCallId: 'c1', toolName: 'searchPages', output: { hits: [] } },
+      ],
+    }));
+    await opts.onFinish({
+      text: '',
+      finishReason: 'tool-calls',
+      totalUsage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 },
+      usage: { inputTokens: 1, outputTokens: 1 },
+      steps,
+    });
+
+    expect(updated).toHaveLength(1);
+    const patch = updated[0].patch as { status: string; content: string };
+    expect(patch.status).toBe('completed');
+    // The synthetic marker is the trailing text of the persisted content.
+    expect(patch.content).toContain(STEP_LIMIT_NO_ANSWER_MARKER);
+  });
+
+  it('normal turn ending WITH text does NOT get the empty-turn marker', async () => {
+    const { svc, updated } = makeService();
+    const cap = captureStreamText();
+    await drive(svc);
+    const opts = cap.opts();
+
+    // A single step that produced a real answer, well under the step cap.
+    const steps = [
+      { text: 'Here is the finished answer.', toolCalls: [], toolResults: [] },
+    ];
+    await opts.onFinish({
+      text: 'Here is the finished answer.',
+      finishReason: 'stop',
+      totalUsage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 },
+      usage: { inputTokens: 1, outputTokens: 1 },
+      steps,
+    });
+
+    expect(updated).toHaveLength(1);
+    const patch = updated[0].patch as { status: string; content: string };
+    expect(patch.status).toBe('completed');
+    expect(patch.content).toBe('Here is the finished answer.');
+    expect(patch.content).not.toContain(STEP_LIMIT_NO_ANSWER_MARKER);
+  });
+
+  it('step-exhausted turn that DID produce text keeps the text, no marker (guards the AND)', async () => {
+    // Exhausting the step budget alone must NOT append the marker when SOME step
+    // produced text — the marker keys off "no text" too. Drive the real onFinish
+    // with MAX_AGENT_STEPS steps where the last one carries the answer.
+    const { svc, updated } = makeService();
+    const cap = captureStreamText();
+    await drive(svc);
+    const opts = cap.opts();
+
+    const steps = Array.from({ length: MAX_AGENT_STEPS }, (_, i) => ({
+      text: i === MAX_AGENT_STEPS - 1 ? 'Final synthesized answer.' : '',
+      toolCalls:
+        i === MAX_AGENT_STEPS - 1
+          ? []
+          : [{ toolCallId: `c${i}`, toolName: 'searchPages', input: {} }],
+      toolResults:
+        i === MAX_AGENT_STEPS - 1
+          ? []
+          : [{ toolCallId: `c${i}`, toolName: 'searchPages', output: {} }],
+    }));
+    await opts.onFinish({
+      text: 'Final synthesized answer.',
+      finishReason: 'stop',
+      totalUsage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 },
+      usage: { inputTokens: 1, outputTokens: 1 },
+      steps,
+    });
+
+    expect(updated).toHaveLength(1);
+    const patch = updated[0].patch as { content: string };
+    expect(patch.content).toContain('Final synthesized answer.');
+    expect(patch.content).not.toContain(STEP_LIMIT_NO_ANSWER_MARKER);
+  });
+});

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

@@ -52,11 +52,24 @@ import {
   startSseHeartbeat,
   stripStreamingHopByHopHeaders,
 } from './sse-resilience';
+import {
+  isDegenerateOutput,
+  truncateDegeneratedTail,
+} from './output-degeneration';
 
 // Max agent steps per turn. One step = one model generation; a step that calls
 // tools is followed by another step carrying the tool results. Raised from 8 so
-// multi-search research questions are not cut off mid-investigation.
-const MAX_AGENT_STEPS = 20;
+// multi-search research questions are not cut off mid-investigation, then from 20
+// to 50 (#444) so read-heavy turns (e.g. dozens of searchInPage sweeps) do not
+// exhaust the budget before acting.
+const MAX_AGENT_STEPS = 50;
+
+// How many steps before the LAST one the step-budget warning starts firing
+// (#444). At MAX-STEP_BUDGET_WARNING_LEAD .. MAX-2 the model is told to stop
+// exploring and start acting, with the remaining count decreasing each step; the
+// last step (MAX-1) has its own final nudge / lockdown instead (see
+// prepareAgentStep).
+const STEP_BUDGET_WARNING_LEAD = 6;
 
 // Wall-clock ceiling for building the external MCP toolset during the per-turn
 // setup phase (before streamText owns the lifecycle). Defense-in-depth ABOVE the
@@ -82,16 +95,69 @@ const FINAL_STEP_INSTRUCTION =
   'language. If the information is incomplete, say so explicitly: summarize ' +
   'what you found, what is still missing, and give your best partial conclusion.';
 
-// Pure, unit-testable: decide per-step overrides. Two responsibilities:
-//   1. Final-step lockdown (always): on the final allowed step force a text-only
-//      synthesis answer (toolChoice 'none' + FINAL_STEP_INSTRUCTION). This WINS —
-//      it takes precedence over the deferred-tool narrowing below.
-//   2. Deferred tool visibility (#332): when `deferredEnabled` and NOT the final
-//      step, expose only the CORE tools + loadTools + whatever loadTools has
-//      activated so far this turn (`activatedTools`), via `activeTools`. Deferred
-//      tools stay in the <tool_catalog> until the model loads them.
-// When `deferredEnabled` is false the behavior is unchanged: undefined on normal
-// steps (all tools active), lockdown on the final step.
+// SOFT final-step nudge (#444), used when the final-step lockdown toggle is OFF
+// (the new default). Unlike FINAL_STEP_INSTRUCTION it does NOT strip tools
+// (toolChoice stays untouched), so the model is never forced into a tool-less
+// state mid-work — that tool-stripping is what triggered the 255KB token-loop
+// degeneration incident. It only asks the model to finish with a text summary.
+const FINAL_STEP_NUDGE =
+  'This is the LAST step of this turn. Write your final answer to the user now.\n' +
+  'You may still call tools, but the turn ends after this step either way —\n' +
+  'prefer finishing with a clear text summary of what was done and what remains.';
+
+// Synthetic marker text appended in onFinish when a step-exhausted turn produced
+// NO text at all (#444, mitigates the "empty turn" the lockdown used to prevent
+// when the toggle is OFF). Makes the exhausted-without-answer state explicit to
+// the user and, on replay, to the model on the next turn.
+const STEP_LIMIT_NO_ANSWER_MARKER =
+  '(Достигнут лимит шагов — итоговый ответ не сформулирован; работа могла ' +
+  'остаться незавершённой. Напишите «продолжай», чтобы агент продолжил.)';
+
+// Reason recorded in ai_chat_runs.error / the assistant row when the token-
+// degeneration detector (#444) aborts a run. Distinct from a user Stop (no error)
+// and from a server restart ('streaming' -> swept to 'aborted' with no message).
+const OUTPUT_DEGENERATION_ERROR =
+  'Output degeneration detected (repeated token loop)';
+
+/**
+ * Compute the step-budget warning text (#444), or '' when this step is outside
+ * the warning band. The warning fires on steps
+ * MAX_AGENT_STEPS-STEP_BUDGET_WARNING_LEAD .. MAX_AGENT_STEPS-2 (NOT the last
+ * step, which has its own final nudge/lockdown), telling the model to stop
+ * exploring and start acting. `N` is the number of tool-use steps still
+ * remaining (`MAX_AGENT_STEPS - 1 - stepNumber`), so it decreases toward the
+ * end. Pure.
+ */
+export function stepBudgetWarning(stepNumber: number): string {
+  const isLastStep = stepNumber >= MAX_AGENT_STEPS - 1;
+  const inBand = stepNumber >= MAX_AGENT_STEPS - STEP_BUDGET_WARNING_LEAD;
+  if (isLastStep || !inBand) return '';
+  const remaining = MAX_AGENT_STEPS - 1 - stepNumber;
+  return (
+    `Only ${remaining} tool-use steps remain in this turn. Stop exploring and start acting now\n` +
+    '(make the edits / create the comments / produce results). Leave room to finish\n' +
+    'with a final text answer.'
+  );
+}
+
+// Pure, unit-testable: decide per-step overrides. Responsibilities:
+//   1. Final-step handling. Two modes, chosen by `finalStepLockdownEnabled`:
+//      - toggle ON (legacy): on the final allowed step force a text-only
+//        synthesis answer (toolChoice 'none' + FINAL_STEP_INSTRUCTION). This WINS
+//        — it takes precedence over the deferred-tool narrowing below.
+//      - toggle OFF (new default, #444): do NOT touch toolChoice — tools stay
+//        available on every step incl. the last, so the model is never stripped
+//        of its tools mid-work (the cause of the token-loop degeneration
+//        incident). A SOFT nudge (FINAL_STEP_NUDGE) is appended to `system`, and
+//        the deferred-tool `activeTools` narrowing still applies to the last step
+//        (both `activeTools` and `system` are returned together).
+//   2. Step-budget warning (#444): on steps in the warning band (but not the
+//      last, which has its own nudge/lockdown) append stepBudgetWarning(...) to
+//      `system` so the model starts acting before it runs out of steps.
+//   3. Deferred tool visibility (#332): when `deferredEnabled`, expose only the
+//      CORE tools + loadTools + whatever loadTools has activated so far this turn
+//      (`activatedTools`), via `activeTools`. Deferred tools stay in the
+//      <tool_catalog> until the model loads them.
 //
 // `system` is the in-scope system prompt; we CONCATENATE so the original
 // persona/context is preserved — a bare `system` override would REPLACE the
@@ -107,31 +173,53 @@ export function prepareAgentStep(
   system: string,
   activatedTools: ReadonlySet<string> | readonly string[] = [],
   deferredEnabled = false,
+  finalStepLockdownEnabled = false,
 ):
   | { toolChoice: 'none'; system: string }
-  | { activeTools: string[] }
+  | { activeTools: string[]; system?: string }
+  | { system: string }
   | undefined {
-  // Final-step lockdown WINS (applies regardless of the deferred toggle).
-  if (stepNumber >= MAX_AGENT_STEPS - 1) {
+  const isLastStep = stepNumber >= MAX_AGENT_STEPS - 1;
+
+  // Legacy final-step lockdown (toggle ON): text-only synthesis. WINS over the
+  // deferred narrowing AND drops tools for this step.
+  if (isLastStep && finalStepLockdownEnabled) {
     return {
       toolChoice: 'none',
       system: `${system}\n\n${FINAL_STEP_INSTRUCTION}`,
     };
   }
-  // Deferred tool loading: narrow this step's visible tools to CORE + loadTools
-  // + the tools already activated this turn.
+
+  // Compute the extra system text for this step: the soft final nudge on the last
+  // step (toggle OFF), or the step-budget warning in the warning band. At most one
+  // of these applies (stepBudgetWarning returns '' on the last step).
+  const extra = isLastStep ? FINAL_STEP_NUDGE : stepBudgetWarning(stepNumber);
+  const systemForStep = extra ? `${system}\n\n${extra}` : undefined;
+
+  // Deferred tool loading: narrow this step's visible tools to CORE + loadTools +
+  // the tools already activated this turn. Applies on EVERY step incl. the last
+  // (toggle OFF), so the model keeps its core tools available while being nudged
+  // to finish. Return `system` alongside `activeTools` when we have extra text.
   if (deferredEnabled) {
     const activated = Array.isArray(activatedTools)
       ? activatedTools
       : [...activatedTools];
-    return {
-      activeTools: [...CORE_TOOL_KEYS, LOAD_TOOLS_NAME, ...activated],
-    };
+    const activeTools = [...CORE_TOOL_KEYS, LOAD_TOOLS_NAME, ...activated];
+    return systemForStep ? { activeTools, system: systemForStep } : { activeTools };
   }
-  return undefined;
+
+  // Deferred OFF: all tools stay active; only append the extra system text (if any).
+  return systemForStep ? { system: systemForStep } : undefined;
 }
 
-export { MAX_AGENT_STEPS, FINAL_STEP_INSTRUCTION };
+export {
+  MAX_AGENT_STEPS,
+  STEP_BUDGET_WARNING_LEAD,
+  FINAL_STEP_INSTRUCTION,
+  FINAL_STEP_NUDGE,
+  STEP_LIMIT_NO_ANSWER_MARKER,
+  OUTPUT_DEGENERATION_ERROR,
+};
 
 // Pure, unit-testable post-processing for a model-generated title (#199): trim
 // whitespace, strip a single pair of surrounding quotes the model often adds,
@@ -890,6 +978,12 @@ export class AiChatService implements OnModuleInit {
       // tools (fat/rare in-app tools + ALL external MCP tools) load on demand. When
       // OFF, every tool is active and nothing below changes.
       const deferredEnabled = this.environment.isAiChatDeferredToolsEnabled();
+      // Final-step lockdown toggle (#444). Default OFF: the last step keeps its
+      // tools and gets only a soft nudge (prepareAgentStep), and the token-
+      // degeneration detector (onChunk below) is the anti-babble guard. ON =
+      // legacy tool-stripping lockdown on the last step.
+      const finalStepLockdownEnabled =
+        this.environment.isAiChatFinalStepLockdownEnabled();
 
       let system: string;
       let docmostTools: Awaited<ReturnType<AiChatToolsService['forUser']>>;
@@ -978,6 +1072,16 @@ export class AiChatService implements OnModuleInit {
       const capturedSteps: StepLike[] = [];
       let inProgressText = '';
 
+      // Token-degeneration guard (#444). When the final-step lockdown is OFF, a
+      // runaway repetition loop (the 255KB "loadTools." incident) is aborted via
+      // this internal controller, unioned with the run/socket signal below. The
+      // detector runs on `inProgressText` in onChunk, throttled by growth so the
+      // pure rules only fire every ~DEGENERATION_CHECK_STEP bytes.
+      const degenerationController = new AbortController();
+      let degenerationDetected = false;
+      let lastDegenerationCheckLen = 0;
+      const DEGENERATION_CHECK_STEP = 2000;
+
       // Step-granular durability (#183): create the assistant row UPFRONT in the
       // 'streaming' state (before any token), then UPDATE it as each step finishes
       // and finalize it once on the terminal callback. If the process dies
@@ -1118,11 +1222,21 @@ export class AiChatService implements OnModuleInit {
           // further tool calls and appends a synthesis instruction on that step,
           // concatenated onto the original `system` so the persona is preserved.
           prepareStep: ({ stepNumber }) =>
-            prepareAgentStep(stepNumber, system, activatedTools, deferredEnabled),
+            prepareAgentStep(
+              stepNumber,
+              system,
+              activatedTools,
+              deferredEnabled,
+              finalStepLockdownEnabled,
+            ),
           // #184: the RUN's signal (explicit-stop) when a run wraps this turn, else
           // the socket-bound signal (legacy). A browser disconnect aborts only in
-          // the legacy path.
-          abortSignal: effectiveSignal,
+          // the legacy path. #444: UNION it with the internal degeneration signal
+          // so a detected token-loop aborts the run too (AbortSignal.any — Node 20.3+).
+          abortSignal: AbortSignal.any([
+            effectiveSignal,
+            degenerationController.signal,
+          ]),
           onChunk: ({ chunk }) => {
             // DIAGNOSTIC (Safari stream-drop investigation) — temporary. Any model
             // output chunk means the stream is actively emitting bytes; track first
@@ -1132,7 +1246,29 @@ export class AiChatService implements OnModuleInit {
             lastModelChunkAt = now;
             // 'text-delta' is the assistant's prose; tool-call args are separate chunk
             // types — so this mirrors exactly what streams to the client.
-            if (chunk.type === 'text-delta') inProgressText += chunk.text;
+            if (chunk.type === 'text-delta') {
+              inProgressText += chunk.text;
+              // Token-degeneration guard (#444). Throttled: only re-run the pure
+              // rules once the text has grown ~DEGENERATION_CHECK_STEP bytes since
+              // the last check, so the tail heuristics cost is amortized. On a
+              // trigger, abort the run ONCE with a distinguishable reason.
+              if (
+                !degenerationDetected &&
+                inProgressText.length - lastDegenerationCheckLen >=
+                  DEGENERATION_CHECK_STEP
+              ) {
+                lastDegenerationCheckLen = inProgressText.length;
+                if (isDegenerateOutput(inProgressText)) {
+                  degenerationDetected = true;
+                  this.logger.warn(
+                    `AI chat stream aborted (chat ${chatId}): ${OUTPUT_DEGENERATION_ERROR}`,
+                  );
+                  degenerationController.abort(
+                    new Error(OUTPUT_DEGENERATION_ERROR),
+                  );
+                }
+              }
+            }
           },
           onStepFinish: (step) => {
             // The finished step's full text is now in `step.text`; fold it in and reset
@@ -1174,8 +1310,22 @@ export class AiChatService implements OnModuleInit {
             // plain-text projection (full-text search / fallback). A multi-step
             // turn's `content` therefore now holds all steps' prose, not just the
             // last block.
+            // Empty-turn mitigation (#444, toggle OFF). If the turn burned all its
+            // steps WITHOUT ever producing text (every step's text is empty) and
+            // the model stopped because it hit the step cap, there is no answer to
+            // show — the lockdown used to force one. Append a synthetic marker as
+            // the trailing text so the exhausted-without-answer state is explicit
+            // to the user and, on replay, to the model next turn. `flushAssistant`
+            // takes this as the `inProgressText` trailing text arg (empty here
+            // otherwise). `stepCountIs(MAX_AGENT_STEPS)` surfaces as
+            // finishReason === 'tool-calls' (or a length/other cap), so we key off
+            // "no text produced" rather than a single finishReason string.
+            const producedText = (steps as StepLike[]).some((s) => s.text?.trim());
+            const stepExhausted = steps.length >= MAX_AGENT_STEPS;
+            const emptyTurnMarker =
+              !producedText && stepExhausted ? STEP_LIMIT_NO_ANSWER_MARKER : '';
             await finalizeAssistant(
-              flushAssistant(steps as StepLike[], '', 'completed', {
+              flushAssistant(steps as StepLike[], emptyTurnMarker, 'completed', {
                 finishReason: finishReason as string,
                 usage: totalUsage as StreamUsage,
                 contextTokens:
@@ -1252,6 +1402,30 @@ export class AiChatService implements OnModuleInit {
             await snapshotTurnEnd();
           },
           onAbort: async ({ steps }) => {
+            // #444: distinguish a degeneration abort (our internal controller) from
+            // a user Stop / disconnect. On degeneration we truncate the runaway tail
+            // before persist (so hundreds of KB of garbage never reach the DB /
+            // replay) and record it as an ERROR with a clear, distinguishable reason
+            // — NOT a bare 'aborted' (a user Stop) and NOT a swept 'streaming' (a
+            // server restart).
+            if (degenerationDetected) {
+              const truncated = truncateDegeneratedTail(inProgressText);
+              await finalizeAssistant(
+                flushAssistant(capturedSteps, truncated, 'error', {
+                  error: OUTPUT_DEGENERATION_ERROR,
+                  pageChanged,
+                }),
+              );
+              if (runId)
+                await runHooks?.onSettled?.(
+                  runId,
+                  'error',
+                  OUTPUT_DEGENERATION_ERROR,
+                );
+              await closeExternalClients();
+              await snapshotTurnEnd();
+              return;
+            }
             const partialChars =
               capturedSteps.reduce((n, s) => n + (s.text?.length ?? 0), 0) +
               inProgressText.length;

apps/server/src/core/ai-chat/output-degeneration.spec.ts

@@ -0,0 +1,182 @@
+import {
+  hasRepeatedLineRun,
+  hasPeriodicTail,
+  isDegenerateOutput,
+  truncateDegeneratedTail,
+  REPEATED_LINES_THRESHOLD,
+  MIN_PERIOD_REPEATS,
+} from './output-degeneration';
+
+/**
+ * Unit tests for the token-degeneration detector (#444) — the sole anti-babble
+ * guard once the final-step lockdown is OFF. The two rules must fire on real
+ * degeneration (the "loadTools." incident, a no-newline repeat) and MUST NOT fire
+ * on legitimate long output (edit lists, tables, code).
+ */
+describe('hasRepeatedLineRun (rule 1: identical-line run)', () => {
+  it('POSITIVE: fires on "loadTools.\\n" repeated many times (the incident)', () => {
+    const text = 'Here is my plan.\n' + 'loadTools.\n'.repeat(300);
+    expect(hasRepeatedLineRun(text)).toBe(true);
+    expect(isDegenerateOutput(text)).toBe(true);
+  });
+
+  it('POSITIVE: fires at exactly the threshold', () => {
+    const text = 'x\n'.repeat(REPEATED_LINES_THRESHOLD);
+    expect(hasRepeatedLineRun(text)).toBe(true);
+  });
+
+  it('NEGATIVE: does NOT fire just below the threshold', () => {
+    // threshold-1 identical lines followed by a distinct line.
+    const text = 'x\n'.repeat(REPEATED_LINES_THRESHOLD - 1) + 'done\n';
+    expect(hasRepeatedLineRun(text)).toBe(false);
+  });
+
+  it('NEGATIVE: a long edit list of DISTINCT lines never trips', () => {
+    const lines: string[] = [];
+    for (let i = 0; i < 200; i++) lines.push(`- edited section ${i}: fixed typo`);
+    const text = lines.join('\n');
+    expect(hasRepeatedLineRun(text)).toBe(false);
+    expect(isDegenerateOutput(text)).toBe(false);
+  });
+
+  it('NEGATIVE: a markdown table with blank separators does not trip', () => {
+    // Repeated identical rows are unusual, but blank lines break any run.
+    const block = ['| a | b |', '| - | - |', '', '| a | b |', ''];
+    const text = Array.from({ length: 60 }, () => block.join('\n')).join('\n');
+    expect(hasRepeatedLineRun(text)).toBe(false);
+  });
+
+  it('NEGATIVE: blank lines do NOT count toward a run', () => {
+    const text = '\n'.repeat(100);
+    expect(hasRepeatedLineRun(text)).toBe(false);
+  });
+});
+
+describe('hasPeriodicTail (rule 2: no-newline suffix periodicity)', () => {
+  it('POSITIVE: fires on a single char repeated with no newlines', () => {
+    const text = 'answer: ' + 'a'.repeat(500);
+    expect(hasPeriodicTail(text)).toBe(true);
+    expect(isDegenerateOutput(text)).toBe(true);
+  });
+
+  it('POSITIVE: fires on a multi-char block repeat with no newlines', () => {
+    const text = 'prefix ' + 'abcdef'.repeat(100);
+    expect(hasPeriodicTail(text)).toBe(true);
+  });
+
+  it('POSITIVE: at least MIN_PERIOD_REPEATS repeats of a small block', () => {
+    const text = 'go'.repeat(MIN_PERIOD_REPEATS);
+    expect(hasPeriodicTail(text)).toBe(true);
+  });
+
+  it('NEGATIVE: prose does not look periodic', () => {
+    const text =
+      'The quick brown fox jumps over the lazy dog while the sun sets slowly ' +
+      'behind the distant mountains and the river winds through the valley below.';
+    expect(hasPeriodicTail(text)).toBe(false);
+    expect(isDegenerateOutput(text)).toBe(false);
+  });
+
+  it('NEGATIVE: a long code block is not flagged', () => {
+    const code = `
+function compute(values) {
+  let total = 0;
+  for (const v of values) {
+    total += v * 2;
+  }
+  return total / values.length;
+}
+export const helper = (x) => x + 1;
+const config = { retries: 3, timeout: 5000, backoff: 'exp' };
+`.repeat(3);
+    expect(isDegenerateOutput(code)).toBe(false);
+  });
+
+  it('NEGATIVE: a short string well under the repeat count is safe', () => {
+    expect(hasPeriodicTail('ababab')).toBe(false);
+  });
+
+  // Regression (#444): a trivial single-char period (p===1) must NOT flag
+  // legitimate divider/underline/whitespace runs. These are common in real
+  // model output and previously false-positived at ~20 identical chars, aborting
+  // the run and truncating output. They must all be treated as clean.
+  it('NEGATIVE: a markdown horizontal rule is not flagged', () => {
+    const text = 'text\n' + '-'.repeat(40);
+    expect(hasPeriodicTail(text)).toBe(false);
+    expect(isDegenerateOutput(text)).toBe(false);
+  });
+
+  it('NEGATIVE: a setext heading underline is not flagged', () => {
+    const text = 'Title\n' + '='.repeat(30);
+    expect(hasPeriodicTail(text)).toBe(false);
+    expect(isDegenerateOutput(text)).toBe(false);
+  });
+
+  it('NEGATIVE: a box-drawing divider with no trailing newline is not flagged', () => {
+    const text = 'done ' + '─'.repeat(50);
+    expect(hasPeriodicTail(text)).toBe(false);
+    expect(isDegenerateOutput(text)).toBe(false);
+  });
+
+  it('NEGATIVE: trailing spaces are not flagged', () => {
+    const text = 'answer' + ' '.repeat(40);
+    expect(hasPeriodicTail(text)).toBe(false);
+    expect(isDegenerateOutput(text)).toBe(false);
+  });
+
+  // TRIVIAL_MIN_REPEATS boundary (#444 review). The monochar-tail branch fires at
+  // EXACTLY 60 identical trailing chars (`run >= TRIVIAL_MIN_REPEATS`), so 59 is
+  // clean and 60 trips. These pin the `>=` and MUST fail if the comparison is
+  // flipped to `>` (the surviving mutation). The value 60 is HARD-CODED here on
+  // purpose: TRIVIAL_MIN_REPEATS is a private constant and the assert must lock
+  // the literal boundary the reviewer named, not track a constant edit.
+  it('NEGATIVE: 59 identical trailing chars is one below the monochar threshold', () => {
+    expect(hasPeriodicTail('x'.repeat(59))).toBe(false);
+    expect(isDegenerateOutput('x'.repeat(59))).toBe(false);
+  });
+
+  it('POSITIVE: 60 identical trailing chars hits the monochar threshold exactly', () => {
+    // Fails if `run >= TRIVIAL_MIN_REPEATS` is mutated to `run > …`.
+    expect(hasPeriodicTail('x'.repeat(60))).toBe(true);
+    expect(isDegenerateOutput('x'.repeat(60))).toBe(true);
+  });
+
+  // Positive counterparts: a GENUINE single-char runaway (hundreds+ of repeats)
+  // and the real incident (period>=2, "loadTools." ×N) must still fire.
+  it('POSITIVE: a genuine single-char runaway is still flagged', () => {
+    const text = 'x'.repeat(5000);
+    expect(hasPeriodicTail(text)).toBe(true);
+    expect(isDegenerateOutput(text)).toBe(true);
+  });
+
+  it('POSITIVE: the "loadTools." incident (period>=2) is still flagged', () => {
+    const text = 'loadTools.'.repeat(500);
+    expect(hasPeriodicTail(text)).toBe(true);
+    expect(isDegenerateOutput(text)).toBe(true);
+  });
+});
+
+describe('truncateDegeneratedTail', () => {
+  it('collapses a repeated-line loop to a few reps + marker', () => {
+    const text = 'plan\n' + 'loadTools.\n'.repeat(20000);
+    const out = truncateDegeneratedTail(text);
+    expect(out.length).toBeLessThan(text.length);
+    expect(out).toContain('output truncated');
+    // Keeps the leading context and a few loop reps.
+    expect(out).toContain('plan');
+    expect((out.match(/loadTools\./g) ?? []).length).toBeLessThan(10);
+  });
+
+  it('collapses a no-newline periodic loop to a few blocks + marker', () => {
+    const text = 'answer: ' + 'xy'.repeat(50000);
+    const out = truncateDegeneratedTail(text);
+    expect(out.length).toBeLessThan(text.length);
+    expect(out).toContain('output truncated');
+    expect(out).toContain('answer:');
+  });
+
+  it('returns non-degenerate text unchanged (by identity)', () => {
+    const text = 'A perfectly normal, finished assistant answer.';
+    expect(truncateDegeneratedTail(text)).toBe(text);
+  });
+});

apps/server/src/core/ai-chat/output-degeneration.ts

@@ -0,0 +1,189 @@
+/**
+ * Token-degeneration detector for the in-app agent stream (#444).
+ *
+ * When the final-step lockdown is OFF (the new default) there is no toolChoice
+ * override to strip the model's tools mid-work, so the anti-babble safety net is
+ * this detector. It watches the accumulating assistant text and, on a runaway
+ * repetition loop (the 255KB "loadTools." incident), aborts the run.
+ *
+ * Both rules are PURE functions of the text tail so they are cheap to run every
+ * few KB and are unit-testable in isolation. They operate on the TAIL only
+ * (`TAIL_WINDOW` chars) so the cost is bounded regardless of how long the turn is.
+ */
+
+/** How many trailing chars of the accumulated text the rules inspect. */
+export const TAIL_WINDOW = 3000;
+
+/** Rule 1 threshold: minimum consecutive identical non-empty lines to trigger. */
+export const REPEATED_LINES_THRESHOLD = 25;
+
+/** Rule 2: maximum length of a repeating block considered for periodicity. */
+export const MAX_PERIOD_LEN = 150;
+
+/** Rule 2: minimum number of consecutive block repeats to trigger. */
+export const MIN_PERIOD_REPEATS = 20;
+
+/**
+ * Rule 1 — ≥`REPEATED_LINES_THRESHOLD` consecutive IDENTICAL non-empty lines at
+ * the tail. Catches the classic newline-delimited loop ("loadTools.\n" ×N).
+ * Blank lines break a run (a table / list with blank separators never trips it);
+ * a run of ordinary distinct lines (an edit list, code) never reaches the count.
+ *
+ * NB: `REPEATED_LINES_THRESHOLD` (25) is only THIS rule's own trigger, not the
+ * effective floor for detecting a repeated-line loop. In practice a newline-
+ * delimited repeat also has a fixed period (line + '\n'), so rule 2 catches it
+ * via periodicity at `MIN_PERIOD_REPEATS` (20) repeats — the two rules combine
+ * (see `isDegenerateOutput`), so the effective lower bound for a short identical
+ * line loop is ~20, not 25. Pure.
+ */
+export function hasRepeatedLineRun(
+  text: string,
+  threshold = REPEATED_LINES_THRESHOLD,
+): boolean {
+  const tail = text.length > TAIL_WINDOW ? text.slice(-TAIL_WINDOW) : text;
+  const lines = tail.split('\n');
+  let run = 1;
+  let prev: string | null = null;
+  for (const line of lines) {
+    if (line.length > 0 && line === prev) {
+      run += 1;
+      if (run >= threshold) return true;
+    } else {
+      run = 1;
+    }
+    prev = line;
+  }
+  return false;
+}
+
+/**
+ * Rule 2 — cheap suffix-periodicity check: the tail ends in
+ * ≥`MIN_PERIOD_REPEATS` back-to-back repeats of a single block of length
+ * ≤`MAX_PERIOD_LEN`. Catches a no-newline repeat ("abcabcabc…") the line rule
+ * misses. For each candidate period length p we verify the last `repeats*p`
+ * chars are p-periodic; we stop at the smallest p that satisfies the repeat
+ * count. Bounded by MAX_PERIOD_LEN × TAIL_WINDOW comparisons — negligible. Pure.
+ */
+export function hasPeriodicTail(
+  text: string,
+  maxPeriod = MAX_PERIOD_LEN,
+  minRepeats = MIN_PERIOD_REPEATS,
+): boolean {
+  const tail = text.length > TAIL_WINDOW ? text.slice(-TAIL_WINDOW) : text;
+  const n = tail.length;
+  // Not even the shortest possible loop fits in the tail.
+  if (n < minRepeats) return false;
+  // A tail of ONE repeated char (a "trivial period") is common in LEGIT output —
+  // markdown rules (----/====), setext underlines, box-drawing dividers,
+  // trailing spaces routinely produce 20–50 identical chars. Such a run is
+  // p-periodic for EVERY p, so it would otherwise trip the block rule at p>=2
+  // too, not just p===1. We therefore split the check: a monochar tail needs far
+  // more repeats (a real single-char babble loop produces hundreds-to-thousands;
+  // 60 is well above any realistic divider yet a fifth of TAIL_WINDOW), while a
+  // genuine multi-char block repeat (>=2 distinct chars, e.g. the "loadTools."
+  // incident, period ~10) keeps the normal MIN_PERIOD_REPEATS threshold.
+  const TRIVIAL_MIN_REPEATS = 60;
+
+  // Monochar-tail check (the trivial-period case): count the trailing run of one
+  // identical char and require TRIVIAL_MIN_REPEATS of them.
+  {
+    const last = tail[n - 1];
+    let run = 1;
+    for (let i = n - 2; i >= 0 && tail[i] === last; i--) run++;
+    if (run >= TRIVIAL_MIN_REPEATS) return true;
+  }
+
+  const maxP = maxPeriod;
+  for (let p = 2; p <= maxP; p++) {
+    // Verify the last (minRepeats*p) chars are p-periodic AND not monochar (a
+    // monochar span is the trivial case handled above, so skip it here to avoid
+    // re-flagging a legit divider at a composite period).
+    const span = minRepeats * p;
+    // Not enough tail to hold this many repeats of this period.
+    if (span > n) continue;
+    const start = n - span;
+    let periodic = true;
+    let multiChar = false;
+    for (let i = n - 1; i >= start + p; i--) {
+      if (tail[i] !== tail[i - p]) {
+        periodic = false;
+        break;
+      }
+    }
+    if (!periodic) continue;
+    // Confirm the block itself has >=2 distinct chars (else it's monochar).
+    for (let i = start + 1; i < n; i++) {
+      if (tail[i] !== tail[start]) {
+        multiChar = true;
+        break;
+      }
+    }
+    if (multiChar) return true;
+  }
+  return false;
+}
+
+/**
+ * Combined guard used by the stream's onChunk: true when EITHER rule fires.
+ * Pure — the caller owns the abort side effect.
+ */
+export function isDegenerateOutput(text: string): boolean {
+  return hasRepeatedLineRun(text) || hasPeriodicTail(text);
+}
+
+/**
+ * Truncate a degenerated tail before persist so hundreds of KB of garbage never
+ * reach the DB / replay (#444). Keeps everything up to and including the FIRST
+ * `keepRepeats` repeats of the detected loop, then appends a short marker. If no
+ * loop is detected the text is returned unchanged (by identity).
+ *
+ * Implementation: find the shortest tail period (same check as hasPeriodicTail),
+ * keep the prefix before the loop plus `keepRepeats` copies of the block, drop
+ * the rest. This is best-effort cosmetic trimming; correctness does not depend on
+ * finding the exact minimal loop. Pure.
+ */
+export function truncateDegeneratedTail(
+  text: string,
+  keepRepeats = 3,
+): string {
+  const marker = '\n…[output truncated: repeated token loop detected]';
+  // Try the line rule first: collapse a long run of identical lines.
+  const lines = text.split('\n');
+  let runStart = -1;
+  let run = 1;
+  for (let i = 1; i < lines.length; i++) {
+    if (lines[i].length > 0 && lines[i] === lines[i - 1]) {
+      if (run === 1) runStart = i - 1;
+      run += 1;
+      if (run >= REPEATED_LINES_THRESHOLD) {
+        const kept = lines.slice(0, runStart + keepRepeats).join('\n');
+        return kept + marker;
+      }
+    } else {
+      run = 1;
+      runStart = -1;
+    }
+  }
+
+  // Fall back to periodicity over the whole string (bounded by the same block
+  // length). Find the smallest period that makes the SUFFIX highly repetitive.
+  const n = text.length;
+  const maxP = Math.min(MAX_PERIOD_LEN, Math.floor(n / MIN_PERIOD_REPEATS));
+  for (let p = 1; p <= maxP; p++) {
+    // Count how many trailing p-blocks are periodic.
+    let reps = 1;
+    let i = n - 1;
+    for (; i >= p; i--) {
+      if (text[i] !== text[i - p]) break;
+    }
+    // The loop above walks over the periodic suffix; its length is (n-1 - i).
+    const periodicLen = n - 1 - i;
+    reps = Math.floor(periodicLen / p) + 1;
+    if (reps >= MIN_PERIOD_REPEATS) {
+      const loopStart = n - reps * p; // start of the fully-periodic suffix
+      const kept = text.slice(0, loopStart + keepRepeats * p);
+      return kept + marker;
+    }
+  }
+  return text;
+}

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

@@ -24,6 +24,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,
 });
 
 /**
@@ -847,3 +855,109 @@ describe('AiChatToolsService getCurrentPage selection (#388)', () => {
     );
   });
 });
+
+/**
+ * #440 review: the in-app drawio_create / drawio_update handlers must forward
+ * the optional `layout:"elk"` param to the client (5th positional arg), exactly
+ * like the MCP host. It was silently dropped, so ELK auto-layout worked only via
+ * the standalone MCP server, not in-app. These tests pin per-host parity.
+ */
+describe('AiChatToolsService drawio layout passthrough (#440)', () => {
+  const createCalls: unknown[][] = [];
+  const updateCalls: unknown[][] = [];
+
+  // FakeDocmostClient (not Partial<DocmostClientLike>): since #446 derived
+  // DocmostClientLike from the real client, its drawioCreate/drawioUpdate return
+  // the concrete result shape, so a minimal stub object would not be assignable.
+  // FakeDocmostClient types every method as (...args) => Promise<any>, which is
+  // exactly what these arg-capturing doubles need.
+  const fakeClient: FakeDocmostClient = {
+    drawioCreate: (...args: unknown[]) => {
+      createCalls.push(args);
+      return Promise.resolve({ success: true, nodeId: '#0' });
+    },
+    drawioUpdate: (...args: unknown[]) => {
+      updateCalls.push(args);
+      return Promise.resolve({ success: true, nodeId: '#0' });
+    },
+  };
+
+  const tokenServiceStub = {
+    generateAccessToken: jest.fn().mockResolvedValue('access-token'),
+    generateCollabToken: jest.fn().mockResolvedValue('collab-token'),
+  };
+
+  let service: AiChatToolsService;
+
+  beforeEach(() => {
+    createCalls.length = 0;
+    updateCalls.length = 0;
+    jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue(
+      mockLoaded(function () {
+        return fakeClient as DocmostClientLike;
+      } as unknown as loader.DocmostClientCtor),
+    );
+    service = new AiChatToolsService(
+      tokenServiceStub as never,
+      {} as never,
+      {} as never,
+      {} as never,
+      {} as never,
+      {
+        asSink: () => ({ put: jest.fn(), has: jest.fn(), evict: jest.fn() }),
+      } as never,
+    );
+  });
+
+  afterEach(() => jest.restoreAllMocks());
+
+  const buildTools = () =>
+    service.forUser(
+      { id: 'user-1', email: 'u@example.com', workspaceId: 'ws-1' } as never,
+      'session-1',
+      'ws-1',
+      'chat-1',
+    );
+
+  it('forwards layout:"elk" to client.drawioCreate as the 5th positional arg', async () => {
+    const tools = await buildTools();
+    await tools.drawioCreate.execute(
+      {
+        pageId: 'p-1',
+        xml: '<mxGraphModel/>',
+        position: 'append',
+        layout: 'elk',
+      } as never,
+      {} as never,
+    );
+    expect(createCalls).toHaveLength(1);
+    // drawioCreate(pageId, where, xml, title, layout) — layout is args[4].
+    expect(createCalls[0][4]).toBe('elk');
+  });
+
+  it('forwards layout:"elk" to client.drawioUpdate as the 5th positional arg', async () => {
+    const tools = await buildTools();
+    await tools.drawioUpdate.execute(
+      {
+        pageId: 'p-1',
+        node: '#0',
+        xml: '<mxGraphModel/>',
+        baseHash: 'h',
+        layout: 'elk',
+      } as never,
+      {} as never,
+    );
+    expect(updateCalls).toHaveLength(1);
+    // drawioUpdate(pageId, node, xml, baseHash, layout) — layout is args[4].
+    expect(updateCalls[0][4]).toBe('elk');
+  });
+
+  it('omits layout (undefined 5th arg) when not requested', async () => {
+    const tools = await buildTools();
+    await tools.drawioCreate.execute(
+      { pageId: 'p-1', xml: '<mxGraphModel/>', position: 'append' } as never,
+      {} as never,
+    );
+    expect(createCalls[0][4]).toBeUndefined();
+  });
+});

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

@@ -111,10 +111,12 @@ function __assertClientCallContract(client: DocmostClientLike): void {
     afterText: s,
   });
   void client.replaceImage(s, s, s, { align, alt: s });
-  // --- draw.io diagrams (#423) ---
+  // --- draw.io diagrams (#423 stage 1, #424 stage 2) ---
+  // The 5th `layout` arg (#424) is exercised so this parity assertion fails if the
+  // client signature drops it — it must reach the client from the shared execute.
   void client.drawioGet(s, s, 'xml');
-  void client.drawioCreate(s, { position: 'append', anchorNodeId: s }, s, s);
-  void client.drawioUpdate(s, s, s, s);
+  void client.drawioCreate(s, { position: 'append', anchorNodeId: s }, s, s, 'elk');
+  void client.drawioUpdate(s, s, s, s, 'elk');
   // --- write (comment) ---
   void client.createComment(s, s, 'inline', s, s, s);
   void client.resolveComment(s, true);
@@ -263,8 +265,19 @@ 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, createCommentSignalTracker } =
-      await loadDocmostMcp();
+    // searchShapes / getGuideSection (#424) are the PURE, no-network helpers
+    // backing drawio_shapes / drawio_guide. They are `inlineBothHosts` specs (no
+    // canonical execute — their catalog loader uses import.meta and can't be
+    // value-imported into the zod-agnostic tool-specs.ts under the server's
+    // commonjs type-check), so the shared registry loop below SKIPS them and this
+    // service wires them inline (see drawioShapes/drawioGuide entries), mirroring
+    // how index.ts registers them on the standalone MCP host.
+    const {
+      sharedToolSpecs,
+      createCommentSignalTracker,
+      searchShapes,
+      getGuideSection,
+    } = await loadDocmostMcp();
     const client = await this.buildDocmostClient(
       user,
       sessionId,
@@ -555,6 +568,8 @@ export class AiChatToolsService {
     // WHICH mapping to run and returns its value directly (no envelope). For each
     // spec:
     //   - skip `mcpOnly` specs (they belong to the standalone MCP host only);
+    //   - skip `inlineBothHosts` specs (drawio_shapes / drawio_guide): they carry
+    //     no execute and are wired INLINE just below, calling the pure helpers;
     //   - use `inAppExecute` when the spec declares a DELIBERATE per-layer
     //     difference (a projected result shape, a different guardrail message);
     //   - otherwise use the canonical `execute` (raw client result, identical to
@@ -564,6 +579,7 @@ export class AiChatToolsService {
     // arg mapping lives — it can no longer silently drift from the MCP host.
     for (const spec of Object.values(sharedToolSpecs)) {
       if (spec.mcpOnly) continue;
+      if (spec.inlineBothHosts) continue;
       const run = spec.inAppExecute ?? spec.execute;
       if (!run) continue; // defensive: a shared spec always carries one of them.
       tools[spec.inAppKey] = sharedTool(
@@ -573,6 +589,25 @@ export class AiChatToolsService {
       );
     }
 
+    // drawio_shapes / drawio_guide (#424): `inlineBothHosts` registry specs wired
+    // here with the SAME schema+description the shared spec pins, but calling the
+    // pure searchShapes / getGuideSection helpers off the loaded @docmost/mcp
+    // module — they are not client methods and their catalog loader uses
+    // import.meta, so they cannot live in the zod-agnostic shared execute. The raw
+    // result is identical to the MCP host's (which wraps it as JSON text); here
+    // the in-app host returns it plain, exactly like every other shared tool.
+    tools[sharedToolSpecs.drawioShapes.inAppKey] = sharedTool(
+      sharedToolSpecs.drawioShapes,
+      async ({ query, category, limit }) => {
+        const results = searchShapes(query, { category, limit });
+        return { query, count: results.length, results };
+      },
+    );
+    tools[sharedToolSpecs.drawioGuide.inAppKey] = sharedTool(
+      sharedToolSpecs.drawioGuide,
+      async ({ section }) => getGuideSection(section),
+    );
+
     // Passive "new comments: N" signal (#417). PER-TURN state (forUser runs once
     // per turn), so the watermark starts now and only comments a human leaves
     // WHILE this turn runs are signalled — exactly the mid-turn loop; between-turn

apps/server/src/core/ai-chat/tools/comment-signal-inapp.spec.ts

@@ -287,6 +287,14 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
       // Wire the REAL factory so the in-app path is exercised end to end.
       createCommentSignalTracker:
         createCommentSignalTracker as unknown as loader.CommentSignalTrackerFactory,
+      // Pure no-network draw.io helpers (#424) — required on the loader return;
+      // this comment-signal test doesn't exercise them, so no-op stubs suffice.
+      searchShapes: (() => []) as unknown as loader.SearchShapesFn,
+      getGuideSection: (() => ({
+        section: '',
+        content: '',
+        sections: [],
+      })) as unknown as loader.GetGuideSectionFn,
     });
     return new AiChatToolsService(
       tokenServiceStub as never,

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

@@ -62,7 +62,10 @@ type DocmostClientMethod =
   | 'insertImage'
   | 'replaceImage'
   | 'insertFootnote'
-  // --- draw.io diagrams (#423, stage 1) ---
+  // --- draw.io diagrams (#423 stage 1, #424 stage 2) ---
+  // DERIVED from the real DocmostClient (#446): drawioCreate/drawioUpdate carry
+  // the optional layout:"elk" 5th arg in the real signature, so the layout parity
+  // (#440) is inherited automatically — no hand-written mirror to keep in sync.
   | 'drawioGet'
   | 'drawioCreate'
   | 'drawioUpdate'
@@ -141,6 +144,19 @@ export type CommentSignalTrackerFactory = (options: {
   debounceMs?: number;
 }) => CommentSignalTrackerLike;
 
+// 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>;
@@ -153,6 +169,15 @@ interface DocmostMcpModule {
   // the mocked loader in unit tests) — the stale-check below is a NO-OP when it
   // is missing, so an older build never wrongly fails startup.
   REGISTRY_STAMP?: string;
+  // Pure, no-network draw.io helpers (#424) backing drawio_shapes / drawio_guide.
+  // Those two specs are `inlineBothHosts` (they stay in SHARED_TOOL_SPECS for the
+  // shared contract but carry no execute — their catalog loader uses import.meta
+  // and can't be value-imported into the zod-agnostic tool-specs.ts), so the
+  // in-app service wires them INLINE off these helpers, mirroring the standalone
+  // MCP host. Exposed off the loaded module so the service and its test mocks can
+  // reach them.
+  searchShapes: SearchShapesFn;
+  getGuideSection: GetGuideSectionFn;
 }
 
 /**
@@ -216,6 +241,8 @@ export async function loadDocmostMcp(): Promise<{
   DocmostClient: DocmostClientCtor;
   sharedToolSpecs: Record<string, SharedToolSpec>;
   createCommentSignalTracker?: CommentSignalTrackerFactory;
+  searchShapes: SearchShapesFn;
+  getGuideSection: GetGuideSectionFn;
 }> {
   if (!modulePromise) {
     modulePromise = (async () => {
@@ -261,5 +288,8 @@ export async function loadDocmostMcp(): Promise<{
     // Optional: forwarded when present so the in-app layer can build the passive
     // comment signal (#417); undefined on a stale build => signal disabled.
     createCommentSignalTracker: mod.createCommentSignalTracker,
+    // 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(
       {
@@ -232,6 +239,16 @@ describe('applyLoadTools (#332)', () => {
     expect(LOAD_TOOLS_DESCRIPTION).toContain('only ACTIVATES them');
     expect(LOAD_TOOLS_DESCRIPTION).toContain('callable on your NEXT step');
   });
+
+  it('loadTools description tells the model CORE tools are always active (#444)', () => {
+    expect(LOAD_TOOLS_DESCRIPTION).toContain(
+      'Tools NOT listed in the catalog are CORE and ALWAYS active',
+    );
+    expect(LOAD_TOOLS_DESCRIPTION).toContain('NEVER via loadTools');
+    // Names it out explicitly so the model doesn't loadTools a core tool.
+    expect(LOAD_TOOLS_DESCRIPTION).toContain('createComment');
+    expect(LOAD_TOOLS_DESCRIPTION).toContain('searchInPage');
+  });
 });
 
 describe('editorial "Corrector" scenario is fully served by CORE (#332)', () => {

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

@@ -84,7 +84,10 @@ export const LOAD_TOOLS_DESCRIPTION =
   'block in your instructions. Pass the EXACT tool names from the catalog; this\n' +
   'call only ACTIVATES them and returns { loaded: [...] } — the tools become\n' +
   'callable on your NEXT step. Load several names in one call when the task clearly\n' +
-  'needs them. Unknown names are rejected with the list of valid ones.';
+  'needs them. Unknown names are rejected with the list of valid ones.\n' +
+  'Tools NOT listed in the catalog are CORE and ALWAYS active — call them directly,\n' +
+  'NEVER via loadTools (e.g. createComment, listComments, resolveComment,\n' +
+  'editPageText, searchInPage).';
 
 /**
  * Tier + catalogLine for the INLINE ai-chat tools — those defined per-layer in

apps/server/src/integrations/environment/environment.service.spec.ts

@@ -158,4 +158,27 @@ describe('EnvironmentService', () => {
       ).toBe('https://app.example.com');
     });
   });
+
+  describe('isAiChatFinalStepLockdownEnabled (#444)', () => {
+    const build = (val?: string) =>
+      new EnvironmentService({
+        get: (key: string, def?: string) =>
+          key === 'AI_CHAT_FINAL_STEP_LOCKDOWN' ? (val ?? def) : def,
+      } as any);
+
+    it('defaults to OFF (false) when unset — the new anti-degeneration default', () => {
+      expect(build(undefined).isAiChatFinalStepLockdownEnabled()).toBe(false);
+    });
+
+    it('is true only for the exact opt-in "true" (case-insensitive)', () => {
+      expect(build('true').isAiChatFinalStepLockdownEnabled()).toBe(true);
+      expect(build('TRUE').isAiChatFinalStepLockdownEnabled()).toBe(true);
+    });
+
+    it('stays OFF for any other value', () => {
+      expect(build('false').isAiChatFinalStepLockdownEnabled()).toBe(false);
+      expect(build('1').isAiChatFinalStepLockdownEnabled()).toBe(false);
+      expect(build('yes').isAiChatFinalStepLockdownEnabled()).toBe(false);
+    });
+  });
 });

apps/server/src/integrations/environment/environment.service.ts

@@ -292,6 +292,24 @@ export class EnvironmentService {
     return enabled === 'true';
   }
 
+  /**
+   * Final-step lockdown for the in-app agent loop (#444). When ON (legacy), the
+   * LAST allowed step forces a text-only answer: tools are stripped
+   * (toolChoice:'none') and a synthesis instruction is appended. Defaults to OFF:
+   * stripping the tools mid-work triggered a token-loop degeneration incident
+   * (the model, robbed of its tools on the final step, emitted a 255KB block
+   * repeating a single token). With the toggle OFF the last step keeps its tools
+   * and gets only a SOFT nudge to finish with a text summary; the universal
+   * anti-babble guard is the token-degeneration detector instead. Enable this
+   * only for a model that does NOT reliably end its turns with a text answer.
+   */
+  isAiChatFinalStepLockdownEnabled(): boolean {
+    const enabled = this.configService
+      .get<string>('AI_CHAT_FINAL_STEP_LOCKDOWN', 'false')
+      .toLowerCase();
+    return enabled === 'true';
+  }
+
   /**
    * Resumable SSE transport for durable agent runs (#184 phase 1.5). When
    * enabled, a run tees its SSE frames into the in-memory run-stream registry so

apps/server/src/integrations/static/static.module.spec.ts

@@ -1,3 +1,8 @@
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import { join } from 'node:path';
+import Fastify, { FastifyInstance } from 'fastify';
+import fastifyStatic from '@fastify/static';
 import { resolveStaticAssetHeaders } from './static.module';
 
 // Unit tests for the static-asset cache classifier extracted from the
@@ -33,3 +38,69 @@ describe('resolveStaticAssetHeaders', () => {
     expect(headers['vary']).toBe('Accept-Encoding');
   });
 });
+
+// Integration test proving the ACTUAL response header emitted by @fastify/static
+// with the exact registration options StaticModule uses. This is the regression
+// guard for #452: without `cacheControl: false`, @fastify/static writes its own
+// `Cache-Control: public, max-age=0` AFTER the setHeaders callback, overwriting
+// the immutable header — the /assets/ assertion below would then fail.
+describe('static.module @fastify/static registration (integration)', () => {
+  let app: FastifyInstance;
+  let tmpDir: string;
+
+  beforeAll(async () => {
+    tmpDir = fs.mkdtempSync(join(os.tmpdir(), 'static-module-spec-'));
+    fs.mkdirSync(join(tmpDir, 'assets'), { recursive: true });
+    fs.mkdirSync(join(tmpDir, 'locales'), { recursive: true });
+    fs.writeFileSync(
+      join(tmpDir, 'assets', 'index-a1b2c3.js'),
+      'console.log(1);',
+    );
+    fs.writeFileSync(join(tmpDir, 'locales', 'en.json'), '{"hello":"world"}');
+
+    app = Fastify();
+    // Mirror StaticModule.onModuleInit's registration options exactly.
+    await app.register(fastifyStatic, {
+      root: tmpDir,
+      wildcard: false,
+      preCompressed: true,
+      cacheControl: false,
+      setHeaders: (res, filePath) => {
+        for (const [name, value] of Object.entries(
+          resolveStaticAssetHeaders(filePath),
+        )) {
+          res.setHeader(name, value);
+        }
+      },
+    });
+    await app.ready();
+  });
+
+  afterAll(async () => {
+    await app.close();
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  it('serves a hashed /assets/ file with an immutable, 1-year cache-control', async () => {
+    const res = await app.inject({
+      method: 'GET',
+      url: '/assets/index-a1b2c3.js',
+    });
+    expect(res.statusCode).toBe(200);
+    const cacheControl = res.headers['cache-control'];
+    expect(cacheControl).toContain('immutable');
+    expect(cacheControl).toContain('max-age=31536000');
+  });
+
+  it('serves a non-hashed /locales/ file WITHOUT an immutable cache-control', async () => {
+    const res = await app.inject({ method: 'GET', url: '/locales/en.json' });
+    expect(res.statusCode).toBe(200);
+    // resolveStaticAssetHeaders sets no cache-control here and cacheControl:false
+    // stops @fastify/static from adding one, so the browser revalidates by
+    // etag/last-modified — either an absent header or one without `immutable`.
+    const cacheControl = res.headers['cache-control'];
+    if (cacheControl !== undefined) {
+      expect(cacheControl).not.toContain('immutable');
+    }
+  });
+});

apps/server/src/integrations/static/static.module.ts

@@ -115,6 +115,11 @@ export class StaticModule implements OnModuleInit {
         // Serve the build-time .br/.gz neighbour when the client accepts it
         // (see vite-plugin-compression2 in apps/client/vite.config.ts).
         preCompressed: true,
+        // @fastify/static's default cacheControl:true writes its own
+        // Cache-Control (from maxAge, default 0) AFTER the setHeaders callback,
+        // silently overwriting the immutable header that resolveStaticAssetHeaders
+        // sets — disable it so setHeaders/resolveStaticAssetHeaders own the header.
+        cacheControl: false,
         setHeaders: (res, filePath) => {
           for (const [name, value] of Object.entries(
             resolveStaticAssetHeaders(filePath),

packages/mcp/package.json

@@ -57,6 +57,7 @@
     "@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",

packages/mcp/src/client.ts

@@ -42,6 +42,7 @@ import {
   insertTableRow,
   deleteTableRow,
   updateTableCell,
+  findInvalidNode,
 } from "@docmost/prosemirror-markdown";
 import { searchInDoc, SearchOptions } from "./lib/page-search.js";
 import { withPageLock } from "./lib/page-lock.js";
@@ -54,6 +55,7 @@ import {
   countUserCells,
 } from "./lib/drawio-xml.js";
 import { renderDiagramShapes } from "./lib/drawio-preview.js";
+import { applyElkLayout } from "./lib/drawio-layout.js";
 import {
   applyTextEdits,
   TextEdit,
@@ -197,6 +199,166 @@ function readCollabTokenTtlMs(): number {
   return Number.isFinite(raw) ? Math.max(0, raw) : 5 * 60 * 1000;
 }
 
+// --- Issue #437: central error diagnostics -------------------------------
+// The agent only ever sees the thrown exception's `error.message`, so a failed
+// tool must return an ACTIONABLE message (method, path, status, and the
+// server's own validation text) instead of the opaque "Request failed with
+// status code 400". These helpers + the response interceptor in the
+// constructor are the single authoritative place that text is composed.
+
+// Overall cap on the composed diagnostic message so the model context stays
+// compact and a (whitelisted) server string can never blow up the text.
+const ERROR_MESSAGE_CAP = 300;
+// Only attempt to JSON.parse an arraybuffer body under this size: a larger
+// binary body is never a JSON error envelope, so parsing it just wastes memory
+// (fetchInternalFile uses responseType:"arraybuffer", so a failed file fetch
+// carries the JSON error envelope as raw bytes here).
+const ERROR_BUFFER_PARSE_CAP = 4096;
+
+// Canonical 36-char UUID (8-4-4-4-12 hex). Deliberately version/variant-
+// AGNOSTIC: the ids are UUIDv7 (e.g. 019f499a-9f8c-7d68-...), so only the
+// canonical shape/length is enforced, not the version/variant nibble.
+const FULL_UUID_RE =
+  /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+
+/**
+ * Throw an actionable error BEFORE any network call when `value` is not a full
+ * canonical UUID. Absorbs #436: a truncated/short comment id used to reach the
+ * server and bounce back as an opaque 400/404 the agent could not self-correct;
+ * failing fast here names the exact fix.
+ */
+export function assertFullUuid(
+  tool: string,
+  param: string,
+  value: string,
+): void {
+  if (typeof value !== "string" || !FULL_UUID_RE.test(value)) {
+    throw new Error(
+      `${tool}: '${param}' must be the FULL comment UUID (36 chars, e.g. ` +
+        `019f499a-9f8c-7d68-b7be-ce100d7c6c56), got '${value}'. Copy the id ` +
+        `verbatim from list_comments / create_comment output.`,
+    );
+  }
+}
+
+// Keep ONLY the pathname of a request (no host, no query string, no fragment)
+// so the message never leaks a host or query params. Resolves a relative
+// config.url against config.baseURL, then discards everything but the path.
+function requestPath(config: any): string {
+  const rawUrl = typeof config?.url === "string" ? config.url : "";
+  const base =
+    typeof config?.baseURL === "string" ? config.baseURL : undefined;
+  try {
+    // A dummy base makes an absolute config.url parse too; its host is dropped.
+    return new URL(rawUrl, base ?? "http://localhost").pathname;
+  } catch {
+    // Malformed url: still strip any query/fragment manually.
+    return rawUrl.split(/[?#]/)[0] || rawUrl;
+  }
+}
+
+/**
+ * Compose the server-facing message from `error.response.data`, using ONLY the
+ * whitelisted `message`/`error` fields or the HTTP statusText. SECURITY: the
+ * raw response body, headers (Authorization!) and config are NEVER read here —
+ * a string/HTML body (e.g. a proxy's 502 page) is deliberately dropped in
+ * favour of the statusText.
+ */
+function extractServerMessage(data: any, statusText: string): string {
+  // class-validator envelope: { message: string | string[], error?: string }.
+  if (
+    data &&
+    typeof data === "object" &&
+    !Buffer.isBuffer(data) &&
+    !(data instanceof ArrayBuffer)
+  ) {
+    const msg = (data as any).message;
+    if (Array.isArray(msg)) {
+      const joined = msg.filter((m) => typeof m === "string").join("; ");
+      if (joined) return joined;
+    } else if (typeof msg === "string" && msg) {
+      return msg;
+    }
+    const err = (data as any).error;
+    if (typeof err === "string" && err) return err;
+    return statusText;
+  }
+
+  // Buffer / ArrayBuffer body: attempt a size-capped, guarded JSON.parse so a
+  // failed arraybuffer fetch still surfaces the server's validation text.
+  if (Buffer.isBuffer(data) || data instanceof ArrayBuffer) {
+    const buf = Buffer.isBuffer(data) ? data : Buffer.from(data);
+    if (buf.length > 0 && buf.length <= ERROR_BUFFER_PARSE_CAP) {
+      try {
+        return extractServerMessage(JSON.parse(buf.toString("utf8")), statusText);
+      } catch {
+        return statusText;
+      }
+    }
+    return statusText;
+  }
+
+  // A raw string / HTML body is never surfaced (may echo server internals).
+  return statusText;
+}
+
+/**
+ * Reformat an AxiosError's `.message` IN PLACE into an actionable diagnostic:
+ *   `<METHOD> <path> failed (<status> <statusText>): <serverMessage>`
+ * or, when the request never got a response:
+ *   `<METHOD> <path> failed: <code> (no response from server)`.
+ *
+ * Mutates the SAME error object (never a custom subclass) so the live
+ * axios.isAxiosError / error.response?.status / config._retry checks around the
+ * client keep working, and sets `_docmostFormatted` as a double-processing
+ * guard. A no-op on a non-axios or already-formatted error.
+ */
+export function formatDocmostAxiosError(error: any): void {
+  if (!error || error._docmostFormatted) return;
+  if (!axios.isAxiosError(error)) return;
+
+  const config: any = error.config ?? {};
+  const method =
+    typeof config.method === "string" ? config.method.toUpperCase() : "";
+  const methodPath = `${method} ${requestPath(config)}`.trim();
+  const response = error.response;
+
+  let message: string;
+  if (response) {
+    const statusText =
+      typeof response.statusText === "string" ? response.statusText : "";
+    const serverMessage = extractServerMessage(response.data, statusText);
+    message = `${methodPath} failed (${response.status} ${statusText}): ${serverMessage}`;
+    // Full body only to stderr under DEBUG (parity with downloadImage).
+    if (process.env.DEBUG) {
+      console.error(
+        "Docmost request failed; response body:",
+        JSON.stringify(response.data),
+      );
+    }
+  } else {
+    // No response at all (ECONNREFUSED / ETIMEDOUT / ECONNRESET / DNS / timeout).
+    // Use ONLY error.code, never the raw error.message: axios network messages
+    // embed host:port ("connect ECONNREFUSED 127.0.0.1:3000", "getaddrinfo
+    // ENOTFOUND host") and #437's invariant is that the host never reaches the
+    // model-visible message. code is set for essentially every real no-response
+    // error (ECONNREFUSED/ETIMEDOUT/ECONNRESET/ENOTFOUND/ECONNABORTED); the full
+    // native message still goes to stderr under DEBUG.
+    const reason = error.code ?? "network error";
+    message = `${methodPath} failed: ${reason} (no response from server)`;
+    if (process.env.DEBUG) {
+      console.error("Docmost request failed; no response:", error.message);
+    }
+  }
+
+  if (message.length > ERROR_MESSAGE_CAP) {
+    message = message.slice(0, ERROR_MESSAGE_CAP - 1) + "…";
+  }
+
+  error.message = message;
+  (error as any)._docmostFormatted = true;
+}
+
 export class DocmostClient {
   private client: AxiosInstance;
   private token: string | null = null;
@@ -337,6 +499,22 @@ export class DocmostClient {
         return Promise.reject(error);
       },
     );
+
+    // Diagnostics interceptor (issue #437). Registered AFTER the re-login
+    // interceptor so a successful re-login retry (which resolves to a real
+    // response) is never seen here as an error; only a genuine failure reaches
+    // this rejection handler. It reformats error.message IN PLACE (see
+    // formatDocmostAxiosError — kept as a mutation, not a custom Error class, so
+    // the surrounding axios.isAxiosError / error.response?.status / config._retry
+    // checks keep working) and re-rejects the SAME error. The _docmostFormatted
+    // flag makes a re-processed retry-failure a no-op.
+    this.client.interceptors.response.use(
+      (response) => response,
+      (error) => {
+        formatDocmostAxiosError(error);
+        return Promise.reject(error);
+      },
+    );
   }
 
   /** Application base URL (API URL without the /api suffix). */
@@ -1660,6 +1838,27 @@ export class DocmostClient {
     }
   }
 
+  /**
+   * Pre-write SHAPE gate (#409). Walk the WHOLE node tree with the shared
+   * `findInvalidNode` and throw a rich, path-anchored error the instant a nested
+   * node has an absent/unknown `type` (or an unknown mark) — the exact shape that
+   * otherwise surfaces DEEP in the Yjs encode as the cryptic
+   * `Unknown node type: undefined`, but only AFTER a collab session was opened
+   * and a page lock taken. Calling this BEFORE `getCollabTokenWithReauth` /
+   * `mutatePageContent` fails fast: no collab connection, no lock, deterministic
+   * message. `op` names the tool for the message prefix (e.g. "patch_node").
+   *
+   * `findInvalidNode` derives its "known type" set from the very same
+   * `docmostExtensions` the encode path uses, so a node this gate accepts is one
+   * the encoder will accept too.
+   */
+  private assertValidNodeShape(op: string, node: any): void {
+    const bad = findInvalidNode(node);
+    if (bad) {
+      throw new Error(`${op}: invalid node — ${bad.summary}`);
+    }
+  }
+
   /**
    * Replace page content with a raw ProseMirror JSON document (lossless) and/or
    * update its title. Both `doc` and `title` are optional, but at least one must
@@ -1711,6 +1910,12 @@ export class DocmostClient {
     // page on overwrite.
     this.validateDocStructure(doc);
 
+    // #409: beyond the string-`type` check above, reject a nested node whose
+    // `type` is a string but NOT a known Docmost schema node (a typo/unknown
+    // block) — the same `Unknown node type` the encoder throws — with a rich,
+    // path-anchored message, still BEFORE any collab connection.
+    this.assertValidNodeShape("update_page_json", doc);
+
     // Sanitize URLs before writing. This closes the JSON-path bypass: unlike
     // the markdown link path (which TipTap sanitizes), raw JSON could otherwise
     // inject javascript:/data: link hrefs or media srcs straight into the doc.
@@ -2131,6 +2336,14 @@ export class DocmostClient {
       target.attrs.id = nodeId;
     }
 
+    // #409: fail fast on a malformed node SHAPE (a nested child with an
+    // absent/unknown `type`, e.g. a text leaf written as `{"text":"foo"}` with
+    // no `"type":"text"`) BEFORE opening a collab session or taking the page
+    // lock — the root-only `typeof node.type === "string"` check above never
+    // sees nested children, and the encoder's `Unknown node type: undefined`
+    // would otherwise only surface after the connection.
+    this.assertValidNodeShape("patch_node", target);
+
     const collabToken = await this.getCollabTokenWithReauth();
     // Open the collab doc by the canonical UUID, never the slugId (#260).
     const pageUuid = await this.resolvePageId(pageId);
@@ -2221,6 +2434,11 @@ export class DocmostClient {
       }
     }
 
+    // #409: fail fast on a malformed node SHAPE (a nested child with an
+    // absent/unknown `type`) BEFORE opening a collab session or taking the page
+    // lock — the root-only check above never sees nested children.
+    this.assertValidNodeShape("insert_node", node);
+
     const collabToken = await this.getCollabTokenWithReauth();
     // Open the collab doc by the canonical UUID, never the slugId (#260).
     const pageUuid = await this.resolvePageId(pageId);
@@ -2512,6 +2730,8 @@ export class DocmostClient {
   }
 
   async getComment(commentId: string) {
+    // Fail fast (#436): reject a truncated id before any network call.
+    assertFullUuid("get_comment", "commentId", commentId);
     await this.ensureAuthenticated();
     const response = await this.client.post("/comments/info", { commentId });
     const comment = response.data.data || response.data;
@@ -2601,6 +2821,12 @@ export class DocmostClient {
     parentCommentId?: string,
     suggestedText?: string,
   ) {
+    // Fail fast (#436): a provided parent id must be a full UUID before any
+    // network call. Validate only when truthy — a falsy parentCommentId means
+    // "top-level comment" (mirrors the isReply computation below), not a reply.
+    if (parentCommentId) {
+      assertFullUuid("create_comment", "parentCommentId", parentCommentId);
+    }
     await this.ensureAuthenticated();
 
     const isReply = !!parentCommentId;
@@ -2883,6 +3109,8 @@ export class DocmostClient {
   }
 
   async updateComment(commentId: string, content: string) {
+    // Fail fast (#436): reject a truncated id before any network call.
+    assertFullUuid("update_comment", "commentId", commentId);
     await this.ensureAuthenticated();
     // NON-canonicalizing on purpose (comment body — see createComment).
     const jsonContent = await markdownToProseMirror(content);
@@ -2898,6 +3126,8 @@ export class DocmostClient {
   }
 
   async deleteComment(commentId: string) {
+    // Fail fast (#436): reject a truncated id before any network call.
+    assertFullUuid("delete_comment", "commentId", commentId);
     await this.ensureAuthenticated();
     return this.client
       .post("/comments/delete", { commentId })
@@ -2910,6 +3140,8 @@ export class DocmostClient {
    * rejects resolving a reply. Hits POST /comments/resolve.
    */
   async resolveComment(commentId: string, resolved: boolean) {
+    // Fail fast (#436): reject a truncated id before any network call.
+    assertFullUuid("resolve_comment", "commentId", commentId);
     await this.ensureAuthenticated();
     const response = await this.client.post("/comments/resolve", {
       commentId,
@@ -3753,6 +3985,7 @@ export class DocmostClient {
     },
     xml: string,
     title?: string,
+    layout?: "elk",
   ): Promise<{
     success: boolean;
     nodeId: string;
@@ -3783,8 +4016,12 @@ export class DocmostClient {
       }
     }
 
+    // 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(xml);
+    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);
@@ -3898,6 +4135,7 @@ export class DocmostClient {
     node: string,
     xml: string,
     baseHash: string,
+    layout?: "elk",
   ): Promise<{
     success: boolean;
     nodeId: string;
@@ -3935,8 +4173,10 @@ export class DocmostClient {
       );
     }
 
+    // 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(xml);
+    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);

packages/mcp/src/index.ts

@@ -5,7 +5,10 @@ import { fileURLToPath } from "url";
 import { dirname, join } from "path";
 import { DocmostClient, DocmostMcpConfig } from "./client.js";
 import { parseNodeArg } from "@docmost/prosemirror-markdown";
+import { searchShapes } from "./lib/drawio-shapes.js";
+import { getGuideSection } from "./lib/drawio-guide.js";
 import { SHARED_TOOL_SPECS, SharedToolSpec } from "./tool-specs.js";
+import { SERVER_INSTRUCTIONS } from "./server-instructions.js";
 import {
   createCommentSignalTracker,
   CommentSignalTracker,
@@ -54,6 +57,13 @@ export type {
   CommentSignalProbeResult,
   CommentSignalTrackerOptions,
 } from "./comment-signal.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);
@@ -74,19 +84,17 @@ const VERSION = packageJson.version;
 // Editing guide surfaced to MCP clients in the initialize result so they can
 // pick the right tool by intent and avoid resending whole documents.
 //
-// MAINTENANCE RULE: when you ADD, RENAME, or REMOVE a tool (either an inline
-// server.registerTool(...) here or a spec in tool-specs.ts), you MUST update
-// this guide so the new tool is routed by intent. This is enforced by
-// test/unit/server-instructions.test.mjs, which fails when a registered tool
-// name is not mentioned below (see its EXCEPTIONS list for the rare opt-outs).
-// Exported for that test.
-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). Draw.io diagrams -> drawio_create (create from mxGraph XML and insert), drawio_get (read a diagram as mxGraph XML + a hash), drawio_update (replace a diagram; pass the hash from drawio_get as baseHash for optimistic locking). Footnotes -> insert_footnote. Bulk/structural rewrite -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" +
-  "PAGES: new -> create_page (Markdown). Rename (title only) -> rename_page. Move -> move_page. Delete -> delete_page (SOFT delete — the page goes to trash and is restorable; nothing is permanent). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Sharing -> share_page / unshare_page / list_shares; share_page makes the page PUBLICLY accessible — do it only when explicitly asked.\n" +
-  "COMMENTS: create_comment is always inline and requires an EXACT selection — contiguous text from a single block, <=250 chars (fails rather than leaving an unanchored comment); reply to a thread via parentCommentId. Propose a concrete text fix for one-click human approval -> create_comment with suggestedText (the exact plain-text replacement for the selection; the selection must then be UNIQUE in the page — extend it with context if needed); prefer this over editing directly when the change is subjective or needs the author's sign-off. Manage -> list_comments, update_comment, resolve_comment (resolve/reopen, reversible — prefer over delete to close), delete_comment, check_new_comments.\n" +
-  "HISTORY: review what changed -> diff_page_versions (a historyId vs current, or two versions). List saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown.";
+// The guide is now SPLIT (issue #448): the hand-written routing prose lives in
+// server-instructions.ts and the tool INVENTORY is GENERATED from the registry
+// (SHARED_TOOL_SPECS + INLINE_MCP_INVENTORY), so it can no longer drift out of
+// sync with the registered tools. Re-exported here (its old home) so existing
+// importers are unaffected; the composition lives in server-instructions.ts.
+// The drawio_shapes / drawio_guide tools (#424) stay in SHARED_TOOL_SPECS (so the
+// generated <tool_inventory> picks them up from their catalogLine automatically)
+// but are flagged `inlineBothHosts` and registered inline below (their pure
+// helpers can't cross into tool-specs.ts); only the hand-written routing prose in
+// server-instructions.ts is updated to mention them.
+export { SERVER_INSTRUCTIONS };
 
 // Helper to format JSON responses
 const jsonContent = (data: any) => ({
@@ -278,6 +286,11 @@ export function createDocmostMcpServer(config: DocmostMcpConfig): McpServer {
   // the wrapping is typed loosely and cast — runtime behaviour is unchanged.
   const registerSharedFromSpec = (spec: SharedToolSpec) => {
     if (spec.inAppOnly) return;
+    // `inlineBothHosts` specs (drawio_shapes / drawio_guide) carry no execute —
+    // their pure helper cannot cross into the zod-agnostic tool-specs.ts, so they
+    // are registered INLINE below (searchShapes / getGuideSection). Skip them here
+    // so the loop never dereferences a missing `execute`.
+    if (spec.inlineBothHosts) return;
     const handler = async (args: any) => {
       if (spec.mcpExecute) {
         // The override owns the full MCP result envelope (not re-wrapped).
@@ -302,6 +315,45 @@ export function createDocmostMcpServer(config: DocmostMcpConfig): McpServer {
     registerSharedFromSpec(spec as SharedToolSpec);
   }
 
+  // --- INLINE drawio helper tools (IN the shared registry, but inlineBothHosts) ---
+  // drawio_shapes / drawio_guide (#424) live in SHARED_TOOL_SPECS (so the shared
+  // contract pins their name/description/schema across both hosts) but carry the
+  // `inlineBothHosts` flag and NO execute: their pure backing helpers
+  // (searchShapes / getGuideSection) cannot be value-imported into the
+  // zod-agnostic tool-specs.ts without breaking the in-app server's commonjs
+  // type-check (searchShapes' catalog loader uses import.meta). So both hosts wire
+  // them directly. Here on the MCP host they reuse the spec's name/description/
+  // schema and wrap the raw helper result as JSON text content — byte-identical to
+  // what the registry loop would have produced. The in-app host mirrors this in
+  // ai-chat-tools.service.ts.
+  {
+    // Cast registerTool like the loop's registerSharedFromSpec does: the spec's
+    // buildShape returns the loose zod-agnostic ZodRawShape (Record<string,
+    // unknown>) and the handler args are the SDK-validated, type-erased input.
+    const registerInline = server.registerTool as any;
+    const shapesSpec = SHARED_TOOL_SPECS.drawioShapes as SharedToolSpec;
+    registerInline(
+      shapesSpec.mcpName,
+      {
+        description: shapesSpec.description,
+        inputSchema: shapesSpec.buildShape!(z),
+      },
+      async ({ query, category, limit }: any) => {
+        const results = searchShapes(query, { category, limit });
+        return jsonContent({ query, count: results.length, results });
+      },
+    );
+    const guideSpec = SHARED_TOOL_SPECS.drawioGuide as SharedToolSpec;
+    registerInline(
+      guideSpec.mcpName,
+      {
+        description: guideSpec.description,
+        inputSchema: guideSpec.buildShape!(z),
+      },
+      async ({ section }: any) => jsonContent(getGuideSection(section)),
+    );
+  }
+
   // --- INLINE tools kept per-transport (NOT in the shared registry) ---
   // Each stays inline for a documented reason: a snake_case/camelCase naming
   // clash the registry convention forbids (table_get), an intentional

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

@@ -202,6 +202,21 @@ export class CollabSession {
     this.ydoc = new Y.Doc();
   }
 
+  /**
+   * Shared diagnostic suffix (issue #437) appended to the connect-timeout,
+   * persist-timeout and connection-closed error texts: names the offending
+   * pageId and tells the agent this class of failure is transient (retry once)
+   * vs. a persistent collab-server outage, so it can self-correct instead of
+   * blind-looping. The Yjs-encode error is deliberately NOT touched — it
+   * already names the offending attribute.
+   */
+  private hint(): string {
+    return (
+      `(pageId ${this.pageId}; transient — retry once; persistent failures ` +
+      `mean the collab server is unreachable/overloaded)`
+    );
+  }
+
   /**
    * 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
@@ -232,7 +247,9 @@ export class CollabSession {
         // The 25s connect timeout: the collab connection never became ready.
         this.opts?.onConnectTimeout?.();
         this.teardown(
-          new Error("Connection timeout to collaboration server"),
+          new Error(
+            `Connection timeout to collaboration server ${this.hint()}`,
+          ),
           false,
         );
       }, CONNECT_TIMEOUT_MS);
@@ -259,7 +276,7 @@ export class CollabSession {
           if (process.env.DEBUG) console.error("WS Disconnect");
           this.teardown(
             new Error(
-              "Collaboration connection closed before the update was persisted/synced",
+              `Collaboration connection closed before the update was persisted/synced ${this.hint()}`,
             ),
             true,
           );
@@ -268,7 +285,7 @@ export class CollabSession {
           if (process.env.DEBUG) console.error("WS Close");
           this.teardown(
             new Error(
-              "Collaboration connection closed before the update was persisted/synced",
+              `Collaboration connection closed before the update was persisted/synced ${this.hint()}`,
             ),
             true,
           );
@@ -403,7 +420,7 @@ export class CollabSession {
         persistTimer = setTimeout(() => {
           localFinish(
             new Error(
-              "Timeout waiting for collaboration server to persist the update",
+              `Timeout waiting for collaboration server to persist the update ${this.hint()}`,
             ),
           );
         }, PERSIST_TIMEOUT_MS);

packages/mcp/src/lib/collaboration.ts

@@ -13,7 +13,11 @@ import { JSDOM } from "jsdom";
 import { markdownToProseMirror } from "@docmost/prosemirror-markdown";
 import { docmostExtensions, docmostSchema } from "./docmost-schema.js";
 import { withPageLock } from "./page-lock.js";
-import { sanitizeForYjs, findUnstorableAttr } from "@docmost/prosemirror-markdown";
+import {
+  sanitizeForYjs,
+  findUnstorableAttr,
+  findInvalidNode,
+} from "@docmost/prosemirror-markdown";
 import { canonicalizeFootnotes } from "./footnote-canonicalize.js";
 import { normalizeAndMergeFootnotes } from "./footnote-normalize-merge.js";
 import { VerifyReport } from "./diff.js";
@@ -28,11 +32,25 @@ export { markdownToProseMirror };
  * place. `label` names the stage that failed (diagnostic). `sanitizeForYjs`
  * already stripped `undefined` attrs, so a remaining failure is pinpointed via
  * `findUnstorableAttr`.
+ *
+ * Diagnostics precedence (#409): the dominant crash here is
+ * `Unknown node type: undefined` — a nested node with an absent/unknown `type`
+ * (a SHAPE problem, e.g. `{"text":"foo"}` missing `"type":"text"`). That points
+ * at the node, not an attribute, so `findInvalidNode` is consulted FIRST and,
+ * on a hit, yields a path-anchored node-shape message. Only when the document
+ * shape is sound do we fall back to `findUnstorableAttr` (undefined/function/
+ * symbol/bigint attr values); the generic "attribute likely holds a value Yjs
+ * cannot store" sentence is the last resort.
  */
 function unstorableYjsError(safe: any, label: string, e: unknown): Error {
+  const base = `Failed to encode document to Yjs (${label}): ${e instanceof Error ? e.message : String(e)}.`;
+  const badNode = findInvalidNode(safe);
+  if (badNode) {
+    return new Error(`${base} Invalid node: ${badNode.summary}`);
+  }
   const bad = findUnstorableAttr(safe);
   return new Error(
-    `Failed to encode document to Yjs (${label}): ${e instanceof Error ? e.message : String(e)}.${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`,
+    `${base}${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`,
   );
 }
 

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,240 @@
+// 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;
+
+// DoS bounds for the in-process ELK layout. The mxGraph XML is LLM-supplied
+// (layout:"elk" in drawio_create/drawio_update) and elkjs runs synchronously on
+// the MCP server's event loop, so an unbounded graph would block it for
+// seconds-to-minutes. A ~1MB XML (well under the stage-1 16MB cap) can carry
+// thousands of nodes. We cap the graph size and race the layout against a
+// wall-clock timeout; on either bound we fall back to the ORIGINAL model, the
+// same best-effort contract the catch already honours.
+//   - 500 nodes lays out in well under a second; beyond that ELK cost climbs
+//     steeply, so refuse and leave the (already-valid) model untouched.
+//   - Edges dominate the layered-crossing cost, so allow a bit more headroom
+//     (1000) than nodes but still bound them.
+//   - 5s is generous for any graph within the caps yet short enough that a
+//     pathological input can never wedge the server.
+const ELK_MAX_NODES = 500;
+const ELK_MAX_EDGES = 1000;
+const ELK_TIMEOUT_MS = 5000;
+
+// 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] });
+  }
+
+  // DoS guard: refuse to lay out an oversized LLM-supplied graph. elkjs runs
+  // in-process on the event loop, so bound the work before we ever call it and
+  // return the original model unchanged (best-effort, same as the catch below).
+  if (vertices.length > ELK_MAX_NODES || edges.length > ELK_MAX_EDGES) {
+    return modelXml;
+  }
+
+  const graph: ElkGraph = {
+    id: "root",
+    layoutOptions: LAYOUT_OPTIONS,
+    children: roots,
+    edges,
+  };
+
+  let laid: ElkGraph;
+  let timer: ReturnType<typeof setTimeout> | undefined;
+  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();
+    // Race the layout against a wall-clock timeout so a graph that is under the
+    // node/edge caps but still pathologically slow can never wedge the server.
+    const timeout = new Promise<never>((_, reject) => {
+      timer = setTimeout(
+        () => reject(new Error("ELK layout timed out")),
+        ELK_TIMEOUT_MS,
+      );
+    });
+    laid = (await Promise.race([elk.layout(graph as any), timeout])) as ElkGraph;
+  } catch {
+    return modelXml; // best-effort: keep the model as-is on timeout or ELK failure
+  } finally {
+    if (timer) clearTimeout(timer);
+  }
+
+  // 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-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/drawio-xml.ts

@@ -461,6 +461,308 @@ export function absolutePos(
   return { x, y };
 }
 
+// --- quality warnings (geometry, non-blocking) -----------------------------
+//
+// These are computed purely from geometry — NO rendering — and are returned as
+// WARNINGS (never errors): they do not block the write, they nudge the model to
+// self-correct ("fix the warnings and retry, max 2 iterations"). They replace
+// the vision-self-check a render backend would have done.
+
+interface Rect {
+  x: number;
+  y: number;
+  w: number;
+  h: number;
+}
+
+/** Absolute rect of a vertex (following the container chain), or null. */
+function rectOf(cell: DrawioCell, byId: Map<string, DrawioCell>): Rect | null {
+  if (!cell.vertex || !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, y, w: g.width, h: g.height };
+}
+
+/** True if `ancestorId` is somewhere up `cell`'s parent chain. */
+function isAncestor(
+  ancestorId: string,
+  cell: DrawioCell,
+  byId: Map<string, DrawioCell>,
+): boolean {
+  const seen = new Set<string>([cell.id]);
+  let p = cell.parent;
+  while (p && !seen.has(p)) {
+    if (p === ancestorId) return true;
+    seen.add(p);
+    p = byId.get(p)?.parent;
+  }
+  return false;
+}
+
+/** Strict interior overlap of two rects (touching edges do NOT count). */
+function rectsOverlap(a: Rect, b: Rect): boolean {
+  return a.x < b.x + b.w && b.x < a.x + a.w && a.y < b.y + b.h && b.y < a.y + a.h;
+}
+
+function center(r: Rect): { x: number; y: number } {
+  return { x: r.x + r.w / 2, y: r.y + r.h / 2 };
+}
+
+/**
+ * Liang-Barsky: does segment p->q pass through the INTERIOR of rect r? Used to
+ * detect an edge crossing a shape that is not one of its endpoints.
+ */
+function segCrossesRect(
+  a: { x: number; y: number },
+  b: { x: number; y: number },
+  r: Rect,
+): boolean {
+  const dx = b.x - a.x;
+  const dy = b.y - a.y;
+  // Canonical Liang-Barsky: for each of the 4 slabs, p*t <= q.
+  const p = [-dx, dx, -dy, dy];
+  const q = [a.x - r.x, r.x + r.w - a.x, a.y - r.y, r.y + r.h - a.y];
+  let t0 = 0;
+  let t1 = 1;
+  for (let i = 0; i < 4; i++) {
+    if (p[i] === 0) {
+      if (q[i] < 0) return false; // parallel to this slab AND outside it
+      continue;
+    }
+    const t = q[i] / p[i];
+    if (p[i] < 0) {
+      if (t > t1) return false;
+      if (t > t0) t0 = t;
+    } else {
+      if (t < t0) return false;
+      if (t < t1) t1 = t;
+    }
+  }
+  return t1 > t0; // strictly non-degenerate overlap with the rect interior
+}
+
+function cross(
+  ox: number,
+  oy: number,
+  ax: number,
+  ay: number,
+  bx: number,
+  by: number,
+): number {
+  return (ax - ox) * (by - oy) - (ay - oy) * (bx - ox);
+}
+
+/** Collinear + overlapping test for two straight segments (edge-on-edge). */
+function segmentsOverlap(
+  a1: { x: number; y: number },
+  a2: { x: number; y: number },
+  b1: { x: number; y: number },
+  b2: { x: number; y: number },
+): boolean {
+  const EPS = 1;
+  // b1 and b2 must be (near-)collinear with segment a.
+  if (
+    Math.abs(cross(a1.x, a1.y, a2.x, a2.y, b1.x, b1.y)) > EPS * dist(a1, a2) ||
+    Math.abs(cross(a1.x, a1.y, a2.x, a2.y, b2.x, b2.y)) > EPS * dist(a1, a2)
+  ) {
+    return false;
+  }
+  // Project all four points onto the dominant axis and test 1-D overlap length.
+  const horizontal = Math.abs(a2.x - a1.x) >= Math.abs(a2.y - a1.y);
+  const pa = horizontal ? [a1.x, a2.x] : [a1.y, a2.y];
+  const pb = horizontal ? [b1.x, b2.x] : [b1.y, b2.y];
+  const loA = Math.min(pa[0], pa[1]);
+  const hiA = Math.max(pa[0], pa[1]);
+  const loB = Math.min(pb[0], pb[1]);
+  const hiB = Math.max(pb[0], pb[1]);
+  const overlap = Math.min(hiA, hiB) - Math.max(loA, loB);
+  return overlap > 5; // >5px of shared collinear run
+}
+
+function dist(
+  a: { x: number; y: number },
+  b: { x: number; y: number },
+): number {
+  return Math.hypot(a.x - b.x, a.y - b.y) || 1;
+}
+
+/** Approximate rendered text width (px) of a cell value at a font size. */
+function estimateLabelWidth(value: string, fontSize: number): number {
+  // Decode explicit line breaks, strip tags/entities, take the longest line.
+  const lines = value
+    .replace(/&#xa;/gi, "\n")
+    .replace(/<br\s*\/?>/gi, "\n")
+    .replace(/<[^>]+>/g, "")
+    .replace(/&[a-z]+;/gi, "x")
+    .split("\n");
+  let longest = 0;
+  for (const l of lines) longest = Math.max(longest, l.trim().length);
+  // ~0.6em per glyph is a decent average for proportional fonts.
+  return longest * fontSize * 0.6;
+}
+
+/** Page size declared on the model root, defaulting to Letter (850x1100). */
+function parsePageSize(modelXml: string): { w: number; h: number } {
+  const w = /pageWidth="(\d+)"/.exec(modelXml);
+  const h = /pageHeight="(\d+)"/.exec(modelXml);
+  return {
+    w: w ? Number(w[1]) : 850,
+    h: h ? Number(h[1]) : 1100,
+  };
+}
+
+/** Minimum required gap between adjacent shapes (appendix heuristic). */
+export const MIN_SHAPE_GAP = 150;
+
+/**
+ * Compute the geometry-derived quality warnings for a parsed model. Each is a
+ * `[rule] message` string. Pure — no rendering, no I/O.
+ */
+export function computeQualityWarnings(
+  cells: DrawioCell[],
+  modelXml?: string,
+): string[] {
+  const warnings: string[] = [];
+  const byId = new Map(cells.map((c) => [c.id, c]));
+  const verts = cells.filter((c) => c.vertex && c.id !== "0" && c.id !== "1");
+  const isContainer = (id: string) =>
+    verts.some((v) => v.parent === id);
+  const rects = new Map<string, Rect>();
+  for (const v of verts) {
+    const r = rectOf(v, byId);
+    if (r) rects.set(v.id, r);
+  }
+
+  // 1. Shape bbox overlap (excluding a container overlapping its own child).
+  for (let i = 0; i < verts.length; i++) {
+    for (let j = i + 1; j < verts.length; j++) {
+      const a = verts[i];
+      const b = verts[j];
+      const ra = rects.get(a.id);
+      const rb = rects.get(b.id);
+      if (!ra || !rb) continue;
+      if (isAncestor(a.id, b, byId) || isAncestor(b.id, a, byId)) continue;
+      if (rectsOverlap(ra, rb)) {
+        warnings.push(
+          `[shape-overlap] shapes "${a.id}" and "${b.id}" overlap; separate them (>=${MIN_SHAPE_GAP}px apart) or use layout:"elk"`,
+        );
+      }
+    }
+  }
+
+  // 2. Edge passing through a non-endpoint LEAF shape's bbox.
+  const edges = cells.filter((c) => c.edge);
+  for (const e of edges) {
+    if (!e.source || !e.target) continue;
+    const rs = rects.get(e.source);
+    const rt = rects.get(e.target);
+    if (!rs || !rt) continue;
+    const p = center(rs);
+    const q = center(rt);
+    for (const v of verts) {
+      if (v.id === e.source || v.id === e.target) continue;
+      if (isContainer(v.id)) continue; // an edge legitimately crosses container frames
+      const rv = rects.get(v.id);
+      if (!rv) continue;
+      // shrink to avoid flagging a graze at a shared layer boundary
+      const shrunk: Rect = { x: rv.x + 6, y: rv.y + 6, w: rv.w - 12, h: rv.h - 12 };
+      if (shrunk.w <= 0 || shrunk.h <= 0) continue;
+      if (segCrossesRect(p, q, shrunk)) {
+        warnings.push(
+          `[edge-through-shape] edge "${e.id}" passes through shape "${v.id}" (not its source/target); add exitX/exitY/entryX/entryY or a waypoint`,
+        );
+        break;
+      }
+    }
+  }
+
+  // 3. Edge-on-edge overlap (parallel duplicates or collinear shared runs).
+  const edgeSegs: { id: string; a: any; b: any; key: string }[] = [];
+  for (const e of edges) {
+    if (!e.source || !e.target) continue;
+    const rs = rects.get(e.source);
+    const rt = rects.get(e.target);
+    if (!rs || !rt) continue;
+    const key = [e.source, e.target].sort().join("::");
+    edgeSegs.push({ id: e.id, a: center(rs), b: center(rt), key });
+  }
+  for (let i = 0; i < edgeSegs.length; i++) {
+    for (let j = i + 1; j < edgeSegs.length; j++) {
+      const ea = edgeSegs[i];
+      const eb = edgeSegs[j];
+      const dup = ea.key === eb.key;
+      if (dup || segmentsOverlap(ea.a, ea.b, eb.a, eb.b)) {
+        warnings.push(
+          `[edge-overlap] edges "${ea.id}" and "${eb.id}" lie on top of each other; offset one (distinct exit/entry points) or reroute`,
+        );
+      }
+    }
+  }
+
+  // 4. Adjacent SIBLING leaf shapes closer than MIN_SHAPE_GAP.
+  for (let i = 0; i < verts.length; i++) {
+    for (let j = i + 1; j < verts.length; j++) {
+      const a = verts[i];
+      const b = verts[j];
+      if ((a.parent ?? "") !== (b.parent ?? "")) continue;
+      if (isContainer(a.id) || isContainer(b.id)) continue;
+      const ra = rects.get(a.id);
+      const rb = rects.get(b.id);
+      if (!ra || !rb || rectsOverlap(ra, rb)) continue;
+      const yOverlap = ra.y < rb.y + rb.h && rb.y < ra.y + ra.h;
+      const xOverlap = ra.x < rb.x + rb.w && rb.x < ra.x + ra.w;
+      let gap = Infinity;
+      if (yOverlap) {
+        gap = Math.min(
+          gap,
+          ra.x >= rb.x ? ra.x - (rb.x + rb.w) : rb.x - (ra.x + ra.w),
+        );
+      }
+      if (xOverlap) {
+        gap = Math.min(
+          gap,
+          ra.y >= rb.y ? ra.y - (rb.y + rb.h) : rb.y - (ra.y + ra.h),
+        );
+      }
+      if (gap > 0 && gap < MIN_SHAPE_GAP) {
+        warnings.push(
+          `[gap-too-small] shapes "${a.id}" and "${b.id}" are ${Math.round(gap)}px apart (<${MIN_SHAPE_GAP}px); increase spacing`,
+        );
+      }
+    }
+  }
+
+  // 5. Label visibly wider than its shape (skip labels drawn OUTSIDE the shape).
+  for (const v of verts) {
+    if (!v.value || isContainer(v.id)) continue;
+    if (v.styleMap.verticalLabelPosition || v.styleMap.labelPosition) continue;
+    const r = rects.get(v.id);
+    if (!r) continue;
+    const fontSize = Number(v.styleMap.fontSize) || 12;
+    const est = estimateLabelWidth(v.value, fontSize);
+    if (est > r.w * 1.15) {
+      warnings.push(
+        `[label-overflow] label of "${v.id}" (~${Math.round(est)}px) is wider than its shape (${r.w}px); widen it, shorten the text, or wrap with &#xa;`,
+      );
+    }
+  }
+
+  // 6. Negative / off-page (top-left) coordinates.
+  const page = parsePageSize(modelXml ?? "");
+  for (const v of verts) {
+    const r = rects.get(v.id);
+    if (!r) continue;
+    if (r.x < 0 || r.y < 0) {
+      warnings.push(
+        `[out-of-bounds] shape "${v.id}" has negative coordinates (${Math.round(r.x)},${Math.round(r.y)}); move it into the positive quadrant (page ${page.w}x${page.h})`,
+      );
+    }
+  }
+
+  return warnings;
+}
+
 // --- linter ----------------------------------------------------------------
 
 /**
@@ -755,12 +1057,15 @@ export function prepareModel(inputXml: string): PreparedModel {
   const modelXml = normalizeXml(rawModel);
   const bbox = computeBBox(cells);
   const cellCount = cells.filter((c) => c.id !== "0" && c.id !== "1").length;
+  // Geometry quality warnings (non-blocking) are appended to any structural
+  // warnings from the linter. The model surfaces these and can self-correct.
+  const quality = computeQualityWarnings(cells, modelXml);
   return {
     modelXml,
     cells,
     bbox,
     cellCount,
-    warnings,
+    warnings: [...warnings, ...quality],
     hash: mxHash(modelXml),
   };
 }

packages/mcp/src/lib/footnote-normalize-merge.ts

@@ -6,7 +6,7 @@
  * typographic quotes («…»/“…”) vs ASCII "…", em/en-dash vs `-`, non-breaking
  * space vs normal space, differing space counts — are not recognized as equal
  * and "fork": two definitions appear where the author meant one. The existing
- * de-dup paths miss this: `footnoteContentKey` (footnote-authoring.ts) only
+ * de-dup paths miss this: `footnoteContentKey` (@docmost/prosemirror-markdown) only
  * collapses ASCII whitespace (quotes/dashes/NBSP untouched), and
  * `canonicalizeFootnotes` keys purely by `attrs.id` (the two forks have
  * different ids), so neither glues the forks together.
@@ -195,7 +195,7 @@ function stableAttrs(attrs: any): string {
 
 /**
  * ATTRS-AWARE merge key for a footnote definition. Deliberately DIVERGES from
- * the shared `footnoteContentKey` (footnote-authoring.ts): that key's mark
+ * the shared `footnoteContentKey` (@docmost/prosemirror-markdown): that key's mark
  * signature is TYPE-ONLY (`m.type`), so two definitions with identical visible
  * text but marks differing only in ATTRIBUTES — most importantly a `link` with a
  * different `href` (footnotes are usually citations/links), also `code` /

packages/mcp/src/server-instructions.ts

@@ -0,0 +1,239 @@
+// SERVER_INSTRUCTIONS — the editing guide surfaced to MCP clients in the
+// initialize result so they can pick the right tool by intent and avoid
+// resending whole documents.
+//
+// This guide is split into TWO parts that are composed at the bottom:
+//
+//  1. ROUTING_PROSE — the hand-written "when to use what" intent hints (READ /
+//     EDIT / PAGES / COMMENTS / HISTORY). This is legitimately manual: it
+//     encodes editorial judgement (which tool for which situation, the cheap-
+//     first ordering, the guardrail nudges) that cannot be derived from the
+//     registry. It is NOT the drift-guard for the tool set.
+//
+//  2. A GENERATED <tool_inventory> — every tool the server registers, listed
+//     by name + one-line purpose, grouped by family, built from the SAME
+//     registry the server registers tools from (SHARED_TOOL_SPECS' mcpName +
+//     catalogLine) PLUS the handful of inline MCP-only tools (their inventory
+//     lines live in INLINE_MCP_INVENTORY below). Because this list is BUILT
+//     from the registry, it can never drift out of sync with the registered
+//     tools — adding/renaming/removing a spec changes it automatically, with no
+//     prose edit and no scraper test. An unmapped tool still appears (under
+//     "OTHER"), so a new tool can never silently vanish from the guide.
+//
+// This replaces the old hand-maintained monolithic guide + its regex scraper
+// test (test/unit/server-instructions.test.mjs), which only checked that every
+// registered name appeared SOMEWHERE in the prose and drifted whenever a name
+// was reworded.
+//
+// OUT OF SCOPE (issue #448): the README / README.ru tool catalogs are still
+// hand-maintained prose and are NOT generated from this registry. Regenerating
+// them from SHARED_TOOL_SPECS is tracked separately as an optional docs script
+// under issue #412 — until then a tool rename still needs a manual README edit.
+
+import { SHARED_TOOL_SPECS, SharedToolSpec } from "./tool-specs.js";
+
+/**
+ * The hand-written routing prose — the intent hints that tell a client which
+ * tool to reach for in which situation. Kept manual on purpose (it encodes
+ * editorial judgement, not a mechanical name list). The generated inventory
+ * below is spliced in after it.
+ */
+export const ROUTING_PROSE =
+  "Docmost editing guide — choose the tool by intent. The <tool_inventory> at the end lists every tool with a one-line purpose; the notes below are the routing hints for WHEN to reach for each.\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). 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.";
+
+/**
+ * A single generated inventory line: the tool's registered NAME + a one-line
+ * purpose. For a registry tool the purpose is its `catalogLine` (falling back
+ * to the first sentence of its description); for an inline MCP-only tool it is
+ * the hand-written line in INLINE_MCP_INVENTORY.
+ */
+export interface ToolInventoryLine {
+  name: string;
+  purpose: string;
+}
+
+/**
+ * The families the inventory is grouped under, in display order. A tool is
+ * placed by looking its mcpName up in TOOL_FAMILY; anything not listed there
+ * falls into "OTHER" so it is never dropped from the guide.
+ */
+const FAMILY_ORDER = [
+  "READ",
+  "EDIT",
+  "PAGES",
+  "COMMENTS",
+  "HISTORY",
+  "OTHER",
+] as const;
+type Family = (typeof FAMILY_ORDER)[number];
+
+/**
+ * mcpName -> family for the generated inventory grouping. Purely cosmetic (it
+ * orders the inventory to mirror the routing prose); an unmapped tool still
+ * appears under OTHER, so forgetting to add an entry here can never drop a tool
+ * from the guide — it only lands it in the catch-all group.
+ */
+const TOOL_FAMILY: Record<string, Family> = {
+  // READ
+  search: "READ",
+  list_pages: "READ",
+  list_spaces: "READ",
+  get_outline: "READ",
+  get_node: "READ",
+  search_in_page: "READ",
+  get_page: "READ",
+  get_page_json: "READ",
+  get_workspace: "READ",
+  stash_page: "READ",
+  // EDIT
+  edit_page_text: "EDIT",
+  patch_node: "EDIT",
+  insert_node: "EDIT",
+  delete_node: "EDIT",
+  update_page_json: "EDIT",
+  table_get: "EDIT",
+  table_update_cell: "EDIT",
+  table_insert_row: "EDIT",
+  table_delete_row: "EDIT",
+  insert_image: "EDIT",
+  replace_image: "EDIT",
+  insert_footnote: "EDIT",
+  drawio_get: "EDIT",
+  drawio_create: "EDIT",
+  drawio_update: "EDIT",
+  drawio_shapes: "EDIT",
+  drawio_guide: "EDIT",
+  docmost_transform: "EDIT",
+  // PAGES
+  create_page: "PAGES",
+  rename_page: "PAGES",
+  move_page: "PAGES",
+  delete_page: "PAGES",
+  copy_page_content: "PAGES",
+  share_page: "PAGES",
+  unshare_page: "PAGES",
+  list_shares: "PAGES",
+  // COMMENTS
+  create_comment: "COMMENTS",
+  list_comments: "COMMENTS",
+  update_comment: "COMMENTS",
+  resolve_comment: "COMMENTS",
+  delete_comment: "COMMENTS",
+  check_new_comments: "COMMENTS",
+  // HISTORY
+  diff_page_versions: "HISTORY",
+  list_page_history: "HISTORY",
+  restore_page_version: "HISTORY",
+  export_page_markdown: "HISTORY",
+  import_page_markdown: "HISTORY",
+};
+
+/**
+ * Inventory lines for the INLINE MCP-only tools — the ones registered directly
+ * in index.ts (not via SHARED_TOOL_SPECS) because they diverge per transport or
+ * exist only on this standalone surface. They carry no `catalogLine`, so their
+ * one-line purpose is hand-written here. This is the ONLY hand-maintained tool
+ * list left, and it is tiny; a new inline tool without an entry here is caught
+ * by the completeness guard in `tool-inventory.test.mjs`.
+ */
+export const INLINE_MCP_INVENTORY: ToolInventoryLine[] = [
+  {
+    name: "table_get",
+    purpose:
+      "read a table as a matrix of cell texts + per-cell paragraph ids.",
+  },
+  {
+    name: "search",
+    purpose:
+      "full-text search for pages and content across the whole workspace.",
+  },
+  {
+    name: "docmost_transform",
+    purpose:
+      "edit a page by running a sandboxed JS `(doc, ctx) => doc` transform, with a dryRun diff preview.",
+  },
+  {
+    name: "update_comment",
+    purpose: "update an existing comment's content (creator only).",
+  },
+  {
+    name: "delete_comment",
+    purpose: "delete a comment (creator or space admin only).",
+  },
+];
+
+/**
+ * Derive the one-line purpose for a registry spec: prefer its hand-written
+ * `catalogLine` (already a "name — purpose" line — we take the purpose after
+ * the em dash), else fall back to the first sentence of its description.
+ */
+function purposeForSpec(spec: SharedToolSpec): string {
+  const line = spec.catalogLine?.trim();
+  if (line) {
+    const dash = line.indexOf(" — ");
+    if (dash >= 0) return line.slice(dash + 3).trim();
+    return line;
+  }
+  const desc = (spec.description ?? "").replace(/\s+/g, " ").trim();
+  const firstSentence = desc.split(/(?<=[.!?])\s/)[0];
+  return firstSentence || desc || "(no description)";
+}
+
+/**
+ * Build the flat list of every registered tool's inventory line: one per shared
+ * registry spec (skipping `inAppOnly` specs, which are not registered on this
+ * MCP host) PLUS every inline MCP-only tool. Pure and deterministic — the
+ * registry drives it, so it can never drift from what index.ts registers.
+ */
+export function buildToolInventoryLines(
+  specs: Record<string, SharedToolSpec> = SHARED_TOOL_SPECS,
+  inline: ToolInventoryLine[] = INLINE_MCP_INVENTORY,
+): ToolInventoryLine[] {
+  const lines: ToolInventoryLine[] = [];
+  for (const spec of Object.values(specs)) {
+    if (spec.inAppOnly) continue; // not registered on the MCP host
+    lines.push({ name: spec.mcpName, purpose: purposeForSpec(spec) });
+  }
+  for (const l of inline) lines.push({ ...l });
+  return lines;
+}
+
+/**
+ * Render the generated `<tool_inventory>` block: every tool name + purpose,
+ * grouped by family (families in FAMILY_ORDER; tools within a family sorted by
+ * name for stable output; unmapped tools fall into OTHER). Pure.
+ */
+export function buildToolInventory(
+  specs: Record<string, SharedToolSpec> = SHARED_TOOL_SPECS,
+  inline: ToolInventoryLine[] = INLINE_MCP_INVENTORY,
+): string {
+  const byFamily = new Map<Family, ToolInventoryLine[]>();
+  for (const family of FAMILY_ORDER) byFamily.set(family, []);
+  for (const line of buildToolInventoryLines(specs, inline)) {
+    const family = TOOL_FAMILY[line.name] ?? "OTHER";
+    byFamily.get(family)!.push(line);
+  }
+  const sections: string[] = [];
+  for (const family of FAMILY_ORDER) {
+    const items = byFamily.get(family)!;
+    if (items.length === 0) continue;
+    items.sort((a, b) => a.name.localeCompare(b.name));
+    for (const item of items) {
+      sections.push(`  ${family}  ${item.name} — ${item.purpose}`);
+    }
+  }
+  return ["<tool_inventory>", ...sections, "</tool_inventory>"].join("\n");
+}
+
+/**
+ * The composed editing guide: the hand-written routing prose followed by the
+ * generated, drift-proof tool inventory. Exported (and used by index.ts /
+ * createDocmostMcpServer) as the MCP server's `instructions`.
+ */
+export const SERVER_INSTRUCTIONS =
+  ROUTING_PROSE + "\n" + buildToolInventory();

packages/mcp/src/tool-specs.ts

@@ -14,10 +14,15 @@
 // some write tools, different limits, hybrid-RRF search, etc.) stay defined
 // per-layer and are NOT represented here.
 //
-// MAINTENANCE RULE: adding, renaming, or removing a spec here (or an inline
-// registerTool in index.ts) REQUIRES updating SERVER_INSTRUCTIONS in
-// packages/mcp/src/index.ts — the intent-routing guide MCP clients receive on
-// initialize. Enforced by test/unit/server-instructions.test.mjs.
+// SERVER_INSTRUCTIONS note (issue #448): the intent-routing guide MCP clients
+// receive on initialize is now SPLIT — its tool INVENTORY is GENERATED from this
+// registry (mcpName + catalogLine) by server-instructions.ts, so adding /
+// renaming / removing a spec here updates the guide's inventory AUTOMATICALLY;
+// no prose edit is needed. Only an INLINE MCP-only tool (registerTool in
+// index.ts, not a spec here) needs a hand-written line in INLINE_MCP_INVENTORY —
+// enforced by test/unit/tool-inventory.test.mjs. The routing PROSE (the "when to
+// use what" hints) in server-instructions.ts stays manual, but it is no longer a
+// drift-guard for the tool set.
 
 // Loose on purpose — see the comment above. The two zod majors expose different
 // static type surfaces, so typing this precisely would couple the registry to
@@ -167,6 +172,18 @@ export interface SharedToolSpec {
   mcpOnly?: boolean;
   /** Registered only on the in-app host (skipped by the MCP registry loop). */
   inAppOnly?: boolean;
+  /**
+   * The spec stays in the registry (so the shared contract still pins its name /
+   * description / schema across both hosts) but carries NO `execute`/override and
+   * is registered INLINE by BOTH hosts instead of through the registry loop. Used
+   * for tools whose implementation cannot cross into this zod-agnostic file — the
+   * drawio_shapes / drawio_guide pure helpers, whose backing module resolves a
+   * bundled data file via `import.meta` and so cannot be value-imported here
+   * without breaking the in-app server's commonjs type-check of this source. Both
+   * registry loops SKIP a spec with this flag; the per-host inline registrations
+   * own it (index.ts on MCP, ai-chat-tools.service.ts in-app).
+   */
+  inlineBothHosts?: boolean;
 }
 
 // --- Shared execute helpers -------------------------------------------------
@@ -187,6 +204,29 @@ const mcpJson = (data: unknown) => ({
   content: [{ type: 'text' as const, text: JSON.stringify(data, null, 2) }],
 });
 
+/**
+ * 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 ---
 
@@ -382,7 +422,10 @@ export const SHARED_TOOL_SPECS = {
       'heading {"type":"heading","attrs":{"level":2},"content":' +
       '[{"type":"text","text":"Title"}]}. Bold is a mark: ' +
       '{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' +
-      'JSON object or a JSON string (both accepted). Cheaper and safer than ' +
+      'JSON object or a JSON string (both accepted). EVERY node, including ' +
+      'nested children, must carry a string `type` from the Docmost schema; ' +
+      'text leaves are {"type":"text","text":"..."} (a bare {"text":"..."} is ' +
+      'rejected up front). Cheaper and safer than ' +
       'replacing the whole document for one-block structural edits. Reversible: ' +
       'the previous version is kept in page history.',
     tier: 'deferred',
@@ -433,7 +476,10 @@ export const SHARED_TOOL_SPECS = {
       '{"type":"paragraph","content":[{"type":"text","text":"Hello"}]} or a ' +
       'heading {"type":"heading","attrs":{"level":2},"content":' +
       '[{"type":"text","text":"Title"}]}. Bold is a mark: ' +
-      '{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' +
+      '{"type":"text","text":"x","marks":[{"type":"bold"}]}. EVERY node, ' +
+      'including nested children, must carry a string `type` from the Docmost ' +
+      'schema; text leaves are {"type":"text","text":"..."} (a bare ' +
+      '{"text":"..."} is rejected up front). The node may be a ' +
       'JSON object or a JSON string (both accepted). Reversible via page history.',
     tier: 'deferred',
     catalogLine:
@@ -1039,7 +1085,10 @@ export const SHARED_TOOL_SPECS = {
       '"paragraph","content":[{"type":"text","text":"Hi"}]}]}. `content` may be ' +
       'a JSON object or a JSON string (both accepted), and is OPTIONAL: omit it ' +
       'to update only the title (though prefer the rename-page tool for a title-only ' +
-      'change). Supplying neither content nor title is an error. Reversible: ' +
+      'change). Supplying neither content nor title is an error. EVERY node, ' +
+      'including nested children, must carry a string `type` from the Docmost ' +
+      'schema; text leaves are {"type":"text","text":"..."} (a bare ' +
+      '{"text":"..."} is rejected up front). Reversible: ' +
       'the previous version is kept in page history.',
     tier: 'deferred',
     catalogLine:
@@ -1605,7 +1654,7 @@ export const SHARED_TOOL_SPECS = {
       }),
   },
 
-  // --- draw.io diagrams (issue #423, stage 1) ---
+  // --- draw.io diagrams (issue #423 stage 1, #424 stage 2) ---
 
   drawioGet: {
     mcpName: 'drawio_get',
@@ -1659,7 +1708,8 @@ export const SHARED_TOOL_SPECS = {
       '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_get.' +
+      DRAWIO_HARD_RULES,
     tier: 'deferred',
     catalogLine:
       'drawioCreate — create a draw.io diagram from mxGraph XML and insert it.',
@@ -1683,9 +1733,19 @@ export const SHARED_TOOL_SPECS = {
         .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.',
+        ),
     }),
     // The flat schema fields are regrouped into the client's `where` object.
-    execute: (client, { pageId, xml, position, anchorNodeId, anchorText, title }) =>
+    // `layout` is the 5th arg: dropping it silently disables ELK auto-layout
+    // (a reviewed #440 parity fix — MUST reach the client on both hosts).
+    execute: (client, { pageId, xml, position, anchorNodeId, anchorText, title, layout }) =>
       client.drawioCreate(
         pageId as string,
         {
@@ -1695,6 +1755,7 @@ export const SHARED_TOOL_SPECS = {
         },
         xml as string,
         title as string | undefined,
+        layout as 'elk' | undefined,
       ),
   },
 
@@ -1708,7 +1769,8 @@ export const SHARED_TOOL_SPECS = {
       '(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>".',
+      '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).',
@@ -1728,13 +1790,101 @@ export const SHARED_TOOL_SPECS = {
         .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.',
+        ),
     }),
-    execute: (client, { pageId, node, xml, baseHash }) =>
+    // `layout` is the 5th arg: forward layout:"elk" (a reviewed #440 parity fix)
+    // so both hosts run the ELK auto-layout before writing.
+    execute: (client, { pageId, node, xml, baseHash, layout }) =>
       client.drawioUpdate(
         pageId as string,
         node as string,
         xml as string,
         baseHash as string,
+        layout as 'elk' | undefined,
       ),
   },
+
+  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).'),
+    }),
+    // INLINE on both hosts (no `execute`): drawio_shapes calls the PURE helper
+    // searchShapes, which is NOT a client method — it reads the bundled shape
+    // catalog via `import.meta.url` (drawio-shapes.ts). tool-specs.ts is
+    // type-checked FROM SOURCE by the in-app server under module:commonjs, where a
+    // static value-import of that `import.meta` module is a compile error
+    // (TS1343), so its execute CANNOT live here. `inlineBothHosts` tells BOTH
+    // registry loops to skip it; index.ts (MCP) and ai-chat-tools.service.ts
+    // (in-app) each register it directly, calling searchShapes from the loaded
+    // module. It STAYS in this registry so the shared-tool-specs contract still
+    // pins its name/description/schema across both hosts.
+    inlineBothHosts: true,
+  },
+
+  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.'),
+    }),
+    // INLINE on both hosts (no `execute`) — same reason as drawio_shapes above:
+    // drawio_guide calls the PURE helper getGuideSection (drawio-guide.ts, no
+    // client, no network). getGuideSection itself has no `import.meta`, but it is
+    // kept inline for SYMMETRY with drawio_shapes (both drawio helper tools wired
+    // the same way in one place) and to avoid pulling any drawio lib source into
+    // the in-app server's commonjs type-check. `inlineBothHosts` makes both loops
+    // skip it; index.ts and ai-chat-tools.service.ts register it directly.
+    inlineBothHosts: true,
+  },
 } satisfies Record<string, SharedToolSpec>;

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

@@ -203,14 +203,16 @@ test("a reply creates without selection or anchoring and is stored as type 'page
     "reply body",
     "inline",
     undefined,
-    "parent-123",
+    // #437: a parentCommentId must be a full canonical UUID.
+    "019f499a-9f8c-7d68-b7be-ce100d7c6c56",
   );
 
   assert.equal(result.success, true, "a reply must resolve successfully");
   assert.ok(createPayload, "/comments/create must have been called");
   assert.equal(
     createPayload.parentCommentId,
-    "parent-123",
+    // #437: a parentCommentId must be a full canonical UUID.
+    "019f499a-9f8c-7d68-b7be-ce100d7c6c56",
     "the reply payload must carry the parentCommentId",
   );
   assert.equal(
@@ -321,7 +323,9 @@ test("suggestedText on a reply is rejected", async () => {
         "body",
         "inline",
         undefined,
-        "parent-1",
+        // #437: use a valid full UUID so the reply+suggestion rejection fires
+        // (not the id-shape guard).
+        "019f499a-9f8c-7d68-b7be-ce100d7c6c56",
         "replacement",
       ),
     /reply/i,

packages/mcp/test/mock/invalid-node-validation.test.mjs

@@ -0,0 +1,240 @@
+// Mock regression for the FAIL-FAST invalid-node validation (#409).
+//
+// A structural editor (patch_node / insert_node / update_page_json) given a doc
+// whose NESTED child has an absent/unknown `type` (the exact shape the Yjs
+// encoder rejects with `Unknown node type: undefined`) must throw a RICH,
+// path-anchored error BEFORE it ever opens a collab session or takes a page
+// lock. We prove the fail-fast by standing up a collab stack whose HTTP handler
+// records EVERY request: a correct fail-fast never even fetches the collab
+// token (which `getCollabTokenWithReauth`, called AFTER the validation, would
+// request), and never drives a document change on the Hocuspocus doc.
+//
+// The happy path (a well-formed doc) is exercised too: it must reach the collab
+// write and succeed, so the gate is not over-eager.
+//
+// findInvalidNode's per-shape summaries are unit-tested in the package
+// (test/find-invalid-node.test.ts); this exercises the END-TO-END wiring through
+// the real client methods.
+import { test, after } from "node:test";
+import assert from "node:assert/strict";
+import http from "node:http";
+import { WebSocketServer } from "ws";
+import { Hocuspocus } from "@hocuspocus/server";
+import { DocmostClient } from "../../build/client.js";
+import { buildYDoc } from "../../build/lib/collaboration.js";
+
+// A minimal valid seed doc with a real block id, so the happy-path patch_node
+// finds its target.
+const SEED_ID = "seed-para-id";
+function seedDoc() {
+  return {
+    type: "doc",
+    content: [
+      {
+        type: "paragraph",
+        attrs: { id: SEED_ID },
+        content: [{ type: "text", text: "seed" }],
+      },
+    ],
+  };
+}
+
+// Stand up an HTTP server that authenticates + hands out a collab token AND
+// upgrades /collab to a Hocuspocus instance seeded with the doc. `state` records
+// whether the collab token was ever fetched (proving the write path was entered)
+// and whether the Hocuspocus doc ever changed.
+async function spawnCollabStack() {
+  const state = { changed: false, collabTokenFetched: false };
+
+  const hocuspocus = new Hocuspocus({
+    quiet: true,
+    async onLoadDocument() {
+      return buildYDoc(seedDoc());
+    },
+    async onChange() {
+      state.changed = true;
+    },
+  });
+
+  const wss = new WebSocketServer({ noServer: true });
+
+  const server = http.createServer((req, res) => {
+    let raw = "";
+    req.on("data", (c) => (raw += c));
+    req.on("end", () => {
+      if (req.url === "/api/auth/login") {
+        res.writeHead(200, {
+          "Content-Type": "application/json",
+          "Set-Cookie": "authToken=t; Path=/; HttpOnly",
+        });
+        res.end(JSON.stringify({ success: true }));
+        return;
+      }
+      if (req.url === "/api/auth/collab-token") {
+        state.collabTokenFetched = true;
+        res.writeHead(200, { "Content-Type": "application/json" });
+        res.end(JSON.stringify({ data: { token: "collab-jwt" } }));
+        return;
+      }
+      res.writeHead(404, { "Content-Type": "application/json" });
+      res.end(JSON.stringify({ message: "not found" }));
+    });
+  });
+
+  server.on("upgrade", (request, socket, head) => {
+    if (!request.url || !request.url.startsWith("/collab")) {
+      socket.destroy();
+      return;
+    }
+    wss.handleUpgrade(request, socket, head, (ws) => {
+      hocuspocus.handleConnection(ws, request);
+    });
+  });
+
+  const baseURL = await new Promise((resolve) => {
+    server.listen(0, "127.0.0.1", () => {
+      const { port } = server.address();
+      resolve(`http://127.0.0.1:${port}/api`);
+    });
+  });
+
+  openStacks.push({ server, hocuspocus });
+  return { state, baseURL };
+}
+
+const openStacks = [];
+after(async () => {
+  await Promise.all(
+    openStacks.map(
+      ({ server, hocuspocus }) =>
+        new Promise((resolve) => {
+          server.close(() => {
+            Promise.resolve(hocuspocus.destroy?.()).finally(resolve);
+          });
+        }),
+    ),
+  );
+});
+
+const PAGE = "11111111-1111-4111-8111-111111111111";
+
+// A node whose NESTED text leaf is missing "type":"text" (dominant #409 shape).
+const nestedTypelessNode = () => ({
+  type: "paragraph",
+  content: [{ text: "oops", marks: [] }],
+});
+
+// A node with a NESTED unknown type NAME (typo).
+const nestedUnknownTypeNode = () => ({
+  type: "paragraph",
+  content: [{ type: "paragraf", content: [{ type: "text", text: "x" }] }],
+});
+
+test("patch_node fails fast on a nested typeless node — no collab connection", async () => {
+  const { state, baseURL } = await spawnCollabStack();
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+
+  await assert.rejects(
+    () => client.patchNode(PAGE, SEED_ID, nestedTypelessNode()),
+    (err) => {
+      assert.match(err.message, /patch_node: invalid node/);
+      assert.match(err.message, /missing "type"/);
+      assert.match(err.message, /content\[0\]/); // path-anchored
+      return true;
+    },
+  );
+
+  assert.equal(
+    state.collabTokenFetched,
+    false,
+    "must NOT fetch a collab token — validation runs before getCollabTokenWithReauth",
+  );
+  assert.equal(state.changed, false, "the collab doc must never be written");
+});
+
+test("insert_node fails fast on a nested UNKNOWN type — no collab connection", async () => {
+  const { state, baseURL } = await spawnCollabStack();
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+
+  await assert.rejects(
+    () =>
+      client.insertNode(PAGE, nestedUnknownTypeNode(), {
+        position: "append",
+      }),
+    (err) => {
+      assert.match(err.message, /insert_node: invalid node/);
+      assert.match(err.message, /unknown node type "paragraf"/);
+      return true;
+    },
+  );
+
+  assert.equal(state.collabTokenFetched, false);
+  assert.equal(state.changed, false);
+});
+
+test("update_page_json fails fast on a nested typeless node — no collab connection", async () => {
+  const { state, baseURL } = await spawnCollabStack();
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+
+  const badDoc = {
+    type: "doc",
+    content: [{ type: "paragraph", content: [{ text: "oops" }] }],
+  };
+
+  await assert.rejects(
+    () => client.updatePageJson(PAGE, badDoc),
+    (err) => {
+      // update_page_json runs validateDocStructure first (string-type check),
+      // which already rejects a typeless node — so the message may come from
+      // either guard, but the write must not happen.
+      assert.match(err.message, /type/i);
+      return true;
+    },
+  );
+
+  assert.equal(state.collabTokenFetched, false);
+  assert.equal(state.changed, false);
+});
+
+test("update_page_json fails fast on a nested UNKNOWN type name — rich #409 message", async () => {
+  const { state, baseURL } = await spawnCollabStack();
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+
+  // validateDocStructure passes (type is a string); assertValidNodeShape must
+  // catch the unknown schema name and produce the rich path-anchored message.
+  const badDoc = {
+    type: "doc",
+    content: [{ type: "paragraf", content: [{ type: "text", text: "x" }] }],
+  };
+
+  await assert.rejects(
+    () => client.updatePageJson(PAGE, badDoc),
+    (err) => {
+      assert.match(err.message, /update_page_json: invalid node/);
+      assert.match(err.message, /unknown node type "paragraf"/);
+      return true;
+    },
+  );
+
+  assert.equal(state.collabTokenFetched, false);
+  assert.equal(state.changed, false);
+});
+
+test("patch_node with a well-formed node proceeds to the collab write", async () => {
+  const { state, baseURL } = await spawnCollabStack();
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+
+  const result = await client.patchNode(PAGE, SEED_ID, {
+    type: "paragraph",
+    content: [{ type: "text", text: "replacement" }],
+  });
+
+  assert.equal(result.success, true);
+  assert.equal(result.replaced, 1);
+  assert.equal(
+    state.collabTokenFetched,
+    true,
+    "a valid node must reach the collab write path",
+  );
+  assert.equal(state.changed, true, "the collab doc must be written");
+});

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

@@ -168,7 +168,9 @@ test("an in-flight mutate rejects with the connection-closed text on disconnect"
   FakeProvider.last()._disconnect();
   await assert.rejects(
     p,
-    /Collaboration connection closed before the update was persisted\/synced/,
+    // Assert the #437 diagnostic hint tail too (pageId + transient/retry cue),
+    // so a refactor that drops hint() can't pass this vacuously.
+    /Collaboration connection closed before the update was persisted\/synced \(pageId page-1; transient/,
   );
 });
 
@@ -248,7 +250,11 @@ test("connect timeout rejects with the connect-timeout text and fires the metric
     },
   });
   mock.timers.tick(25000);
-  await assert.rejects(p, /Connection timeout to collaboration server/);
+  await assert.rejects(
+    p,
+    // Assert the #437 diagnostic hint tail too (pageId + transient/retry cue).
+    /Connection timeout to collaboration server \(pageId page-1; transient/,
+  );
   assert.equal(metricFired, 1);
   assert.equal(__sessionCountForTests(), 0);
 });

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,111 @@
+// 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("DoS guard: a graph over the node cap is returned unchanged, quickly", async () => {
+  // 600 vertices > ELK_MAX_NODES (500): the layout must be SKIPPED and the
+  // input returned verbatim, without ever handing the graph to elkjs. This
+  // exercises the cap path that bounds the in-process, event-loop-blocking
+  // layout on LLM-supplied XML.
+  const model = stackedGraph(600, []);
+  const t0 = Date.now();
+  const laid = await applyElkLayout(model);
+  const dt = Date.now() - t0;
+  // normalizeInput may reserialize, but geometry must be untouched: every
+  // vertex is still stacked at (10,10), i.e. no ELK coordinates were applied.
+  const cells = parseCells(laid);
+  const verts = cells.filter((c) => c.vertex);
+  assert.equal(verts.length, 600, "all vertices survived");
+  for (const v of verts) {
+    assert.equal(v.geometry.x, 10, "x untouched -> layout was skipped");
+    assert.equal(v.geometry.y, 10, "y untouched -> layout was skipped");
+  }
+  // Returning the input without an ELK pass is essentially instant; assert it
+  // did not hang. Generous bound to stay non-flaky on a loaded CI box.
+  assert.ok(dt < 2000, `cap path should be fast, took ${dt}ms`);
+});
+
+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-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/error-diagnostics.test.mjs

@@ -0,0 +1,450 @@
+// Issue #437: central error diagnostics.
+//
+// Two surfaces are covered here:
+//   1. formatDocmostAxiosError — the pure response-interceptor body that
+//      rewrites an AxiosError's `.message` into an actionable diagnostic.
+//   2. assertFullUuid — the fail-fast comment-id guard (absorbs #436) that must
+//      throw BEFORE any network call.
+// Plus an end-to-end pass over a real (offline) http server to prove the
+// interceptor is wired, that a re-login retry leaves a success untouched, and
+// that a persistent failure gets formatted — and that an invalid comment id
+// short-circuits every comment tool with ZERO network traffic.
+import { test, after } from "node:test";
+import assert from "node:assert/strict";
+import http from "node:http";
+import axios, { AxiosError } from "axios";
+import {
+  DocmostClient,
+  formatDocmostAxiosError,
+  assertFullUuid,
+} from "../../build/client.js";
+
+// Build an AxiosError-shaped object the way the interceptor's rejection handler
+// receives it. Using the real AxiosError ctor makes axios.isAxiosError() true.
+function makeAxiosError({
+  method = "post",
+  url = "/comments/resolve",
+  baseURL = "http://host.example/api",
+  status,
+  statusText,
+  data,
+  code,
+  message = "Request failed",
+}) {
+  const config = { method, url, baseURL };
+  const response =
+    status === undefined
+      ? undefined
+      : { status, statusText, data, headers: {}, config };
+  return new AxiosError(message, code, config, {}, response);
+}
+
+// ---------------------------------------------------------------------------
+// formatDocmostAxiosError: message-body extraction rules.
+// ---------------------------------------------------------------------------
+test("class-validator message array is joined with '; '", () => {
+  const err = makeAxiosError({
+    status: 400,
+    statusText: "Bad Request",
+    data: { message: ["commentId must be a UUID", "resolved must be a boolean"] },
+  });
+  formatDocmostAxiosError(err);
+  assert.equal(
+    err.message,
+    "POST /comments/resolve failed (400 Bad Request): commentId must be a UUID; resolved must be a boolean",
+  );
+});
+
+test("a string message is used as-is", () => {
+  const err = makeAxiosError({
+    method: "post",
+    url: "/comments/resolve",
+    status: 400,
+    statusText: "Bad Request",
+    data: { message: "commentId must be a UUID" },
+  });
+  formatDocmostAxiosError(err);
+  assert.equal(
+    err.message,
+    "POST /comments/resolve failed (400 Bad Request): commentId must be a UUID",
+  );
+});
+
+test("falls back to data.error when message is absent", () => {
+  const err = makeAxiosError({
+    method: "get",
+    url: "/pages/info",
+    status: 403,
+    statusText: "Forbidden",
+    data: { error: "Forbidden" },
+  });
+  formatDocmostAxiosError(err);
+  assert.equal(err.message, "GET /pages/info failed (403 Forbidden): Forbidden");
+});
+
+test("empty object body falls back to statusText", () => {
+  const err = makeAxiosError({
+    status: 404,
+    statusText: "Not Found",
+    url: "/comments/info",
+    data: {},
+  });
+  formatDocmostAxiosError(err);
+  assert.equal(err.message, "POST /comments/info failed (404 Not Found): Not Found");
+});
+
+test("HTML/string body is NEVER surfaced — only the statusText", () => {
+  const html = "<html><body>502 Bad Gateway — nginx internals here</body></html>";
+  const err = makeAxiosError({
+    status: 502,
+    statusText: "Bad Gateway",
+    url: "/comments/create",
+    data: html,
+  });
+  formatDocmostAxiosError(err);
+  assert.equal(
+    err.message,
+    "POST /comments/create failed (502 Bad Gateway): Bad Gateway",
+  );
+  assert.ok(!err.message.includes("nginx"), "raw HTML body must not leak");
+  assert.ok(!err.message.includes("<html>"), "raw HTML body must not leak");
+});
+
+test("Buffer body carrying JSON is parsed for its message", () => {
+  const buf = Buffer.from(JSON.stringify({ message: "file too large" }), "utf8");
+  const err = makeAxiosError({
+    method: "get",
+    url: "/files/abc/x.png",
+    status: 413,
+    statusText: "Payload Too Large",
+    data: buf,
+  });
+  formatDocmostAxiosError(err);
+  assert.equal(
+    err.message,
+    "GET /files/abc/x.png failed (413 Payload Too Large): file too large",
+  );
+});
+
+test("Buffer body with non-JSON garbage falls back to statusText", () => {
+  const buf = Buffer.from("<<< not json at all >>>", "utf8");
+  const err = makeAxiosError({
+    method: "get",
+    url: "/files/abc/x.png",
+    status: 500,
+    statusText: "Internal Server Error",
+    data: buf,
+  });
+  formatDocmostAxiosError(err);
+  assert.equal(
+    err.message,
+    "GET /files/abc/x.png failed (500 Internal Server Error): Internal Server Error",
+  );
+  assert.ok(!err.message.includes("not json"), "raw buffer body must not leak");
+});
+
+test("an oversized Buffer body is not parsed (size cap) — statusText only", () => {
+  // A >4KB JSON buffer: even though it IS valid JSON with a message, the size
+  // cap means we do not attempt to parse it, so only the statusText survives.
+  const big = { message: "x".repeat(5000) };
+  const buf = Buffer.from(JSON.stringify(big), "utf8");
+  const err = makeAxiosError({
+    method: "get",
+    url: "/files/abc/x.png",
+    status: 500,
+    statusText: "Internal Server Error",
+    data: buf,
+  });
+  formatDocmostAxiosError(err);
+  assert.equal(
+    err.message,
+    "GET /files/abc/x.png failed (500 Internal Server Error): Internal Server Error",
+  );
+});
+
+// ---------------------------------------------------------------------------
+// formatDocmostAxiosError: no-response and path/method handling.
+// ---------------------------------------------------------------------------
+test("no response uses error.code + path + 'no response from server'", () => {
+  const err = makeAxiosError({
+    method: "post",
+    url: "/comments/create",
+    status: undefined,
+    code: "ECONNREFUSED",
+    message: "connect ECONNREFUSED 127.0.0.1:3000",
+  });
+  formatDocmostAxiosError(err);
+  assert.equal(
+    err.message,
+    "POST /comments/create failed: ECONNREFUSED (no response from server)",
+  );
+});
+
+test("no response with no code falls back to a neutral reason (raw message not leaked — it may embed host:port)", () => {
+  const err = makeAxiosError({
+    method: "post",
+    url: "/comments/create",
+    status: undefined,
+    // A raw axios network message like "connect ECONNREFUSED 127.0.0.1:3000"
+    // embeds the host; #437's invariant is that it never reaches the message.
+    message: "connect ECONNREFUSED 10.0.0.5:3000",
+  });
+  formatDocmostAxiosError(err);
+  assert.equal(
+    err.message,
+    "POST /comments/create failed: network error (no response from server)",
+  );
+  // And the host must NOT appear anywhere in the model-visible message.
+  assert.ok(!err.message.includes("10.0.0.5"));
+});
+
+test("path drops the host and the query string", () => {
+  const err = makeAxiosError({
+    method: "post",
+    url: "/comments/resolve?token=secret&x=1",
+    baseURL: "https://docs.example.com/api",
+    status: 400,
+    statusText: "Bad Request",
+    data: { message: "bad" },
+  });
+  formatDocmostAxiosError(err);
+  assert.equal(
+    err.message,
+    "POST /comments/resolve failed (400 Bad Request): bad",
+  );
+  assert.ok(!err.message.includes("secret"), "query string must not leak");
+  assert.ok(!err.message.includes("docs.example.com"), "host must not leak");
+});
+
+// ---------------------------------------------------------------------------
+// formatDocmostAxiosError: length cap + guard flag + pass-through.
+// ---------------------------------------------------------------------------
+test("the overall message is capped at ~300 chars", () => {
+  const err = makeAxiosError({
+    status: 400,
+    statusText: "Bad Request",
+    data: { message: "y".repeat(1000) },
+  });
+  formatDocmostAxiosError(err);
+  assert.ok(err.message.length <= 300, `expected <=300, got ${err.message.length}`);
+  assert.ok(err.message.endsWith("…"), "a truncated message ends with an ellipsis");
+});
+
+test("a formatted error is not re-processed (guard flag)", () => {
+  const err = makeAxiosError({
+    status: 400,
+    statusText: "Bad Request",
+    data: { message: "first" },
+  });
+  formatDocmostAxiosError(err);
+  const once = err.message;
+  assert.equal(err._docmostFormatted, true);
+  // Mutate the body and re-run: the guard makes it a no-op.
+  err.response.data = { message: "second" };
+  formatDocmostAxiosError(err);
+  assert.equal(err.message, once, "the guard flag prevents double-processing");
+});
+
+test("a non-axios error is passed through untouched", () => {
+  const plain = new Error("boom");
+  formatDocmostAxiosError(plain);
+  assert.equal(plain.message, "boom");
+  assert.equal(plain._docmostFormatted, undefined);
+});
+
+// ---------------------------------------------------------------------------
+// assertFullUuid.
+// ---------------------------------------------------------------------------
+const GOOD_UUID = "019f499a-9f8c-7d68-b7be-ce100d7c6c56";
+
+test("assertFullUuid accepts a full canonical UUID (any version nibble)", () => {
+  assert.doesNotThrow(() => assertFullUuid("resolve_comment", "commentId", GOOD_UUID));
+  // A v4 id also passes (version/variant-agnostic).
+  assert.doesNotThrow(() =>
+    assertFullUuid("get_comment", "commentId", "3d5b7c1e-2f4a-4b6c-8d9e-0f1a2b3c4d5e"),
+  );
+});
+
+test("assertFullUuid rejects a truncated prefix", () => {
+  assert.throws(
+    () => assertFullUuid("resolve_comment", "commentId", "019f499a"),
+    (e) =>
+      e.message.startsWith(
+        "resolve_comment: 'commentId' must be the FULL comment UUID",
+      ) &&
+      e.message.includes("got '019f499a'") &&
+      e.message.includes("Copy the id verbatim"),
+  );
+});
+
+test("assertFullUuid rejects garbage and empty string", () => {
+  assert.throws(
+    () => assertFullUuid("delete_comment", "commentId", "not-a-uuid"),
+    /must be the FULL comment UUID.*got 'not-a-uuid'/s,
+  );
+  assert.throws(
+    () => assertFullUuid("update_comment", "commentId", ""),
+    /must be the FULL comment UUID.*got ''/s,
+  );
+});
+
+// ---------------------------------------------------------------------------
+// End-to-end over an offline http server: interceptor wiring + re-login.
+// ---------------------------------------------------------------------------
+function readBody(req) {
+  return new Promise((resolve) => {
+    let raw = "";
+    req.on("data", (c) => (raw += c));
+    req.on("end", () => resolve(raw));
+  });
+}
+function startServer(handler) {
+  return new Promise((resolve) => {
+    const server = http.createServer(handler);
+    server.listen(0, "127.0.0.1", () => {
+      const { port } = server.address();
+      resolve({ server, baseURL: `http://127.0.0.1:${port}/api` });
+    });
+  });
+}
+function sendJson(res, status, obj, extra = {}) {
+  res.writeHead(status, { "Content-Type": "application/json", ...extra });
+  res.end(JSON.stringify(obj));
+}
+const openServers = [];
+async function spawn(handler) {
+  const { server, baseURL } = await startServer(handler);
+  openServers.push(server);
+  return { baseURL };
+}
+after(async () => {
+  await Promise.all(openServers.map((s) => new Promise((r) => s.close(r))));
+});
+
+test("a 400 on a JSON endpoint is reformatted by the wired interceptor", async () => {
+  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, 400, { message: "pageId should not be empty" });
+      return;
+    }
+    sendJson(res, 404, {});
+  });
+
+  const client = new DocmostClient(baseURL, "u@example.com", "pw");
+  await assert.rejects(
+    () => client.getPageRaw("x"),
+    (e) => {
+      assert.ok(axios.isAxiosError(e), "still an AxiosError (mutation, not a subclass)");
+      assert.equal(e.response?.status, 400, "error.response?.status still readable");
+      assert.equal(
+        e.message,
+        "POST /pages/info failed (400 Bad Request): pageId should not be empty",
+      );
+      return true;
+    },
+  );
+});
+
+test("401 -> re-login -> successful retry: the SUCCESS message is untouched", async () => {
+  let infoCalls = 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=fresh; Path=/; HttpOnly",
+      });
+      return;
+    }
+    if (req.url === "/api/workspace/info") {
+      infoCalls++;
+      if (infoCalls === 1) sendJson(res, 401, { message: "Unauthorized" });
+      else sendJson(res, 200, { success: true, data: { id: "ws" } });
+      return;
+    }
+    sendJson(res, 404, {});
+  });
+
+  const client = new DocmostClient(baseURL, "u@example.com", "pw");
+  client.token = "stale";
+  client.client.defaults.headers.common["Authorization"] = "Bearer stale";
+
+  const result = await client.getWorkspace();
+  assert.equal(result.success, true, "the retried request resolved successfully");
+  assert.equal(infoCalls, 2, "401 then a successful replay");
+});
+
+test("401 -> re-login -> persistent failure: formatted AND retry guard intact", async () => {
+  let infoCalls = 0;
+  let loginCalls = 0;
+  const { baseURL } = await spawn(async (req, res) => {
+    await readBody(req);
+    if (req.url === "/api/auth/login") {
+      loginCalls++;
+      sendJson(res, 200, { success: true }, {
+        "Set-Cookie": "authToken=fresh; Path=/; HttpOnly",
+      });
+      return;
+    }
+    if (req.url === "/api/workspace/info") {
+      infoCalls++;
+      // Always 401, even after a fresh login: the _retry guard must stop here.
+      sendJson(res, 401, { message: "token still invalid" });
+      return;
+    }
+    sendJson(res, 404, {});
+  });
+
+  const client = new DocmostClient(baseURL, "u@example.com", "pw");
+  client.token = "stale";
+  client.client.defaults.headers.common["Authorization"] = "Bearer stale";
+
+  await assert.rejects(
+    () => client.getWorkspace(),
+    (e) => {
+      assert.equal(
+        e.message,
+        "POST /workspace/info failed (401 Unauthorized): token still invalid",
+      );
+      return true;
+    },
+  );
+  // The _retry guard is intact: exactly one replay (2 hits), one re-login.
+  assert.equal(infoCalls, 2, "endpoint hit at most twice (one retry only)");
+  assert.equal(loginCalls, 1, "re-login attempted exactly once");
+});
+
+// ---------------------------------------------------------------------------
+// assertFullUuid application points: NO network call when the id is invalid.
+// A server that counts EVERY request proves the guard short-circuits before
+// even the login round-trip.
+// ---------------------------------------------------------------------------
+test("all 5 comment-id call sites reject a bad id with ZERO network traffic", async () => {
+  let requests = 0;
+  const { baseURL } = await spawn(async (req, res) => {
+    requests++;
+    await readBody(req);
+    sendJson(res, 200, { success: true });
+  });
+
+  const client = new DocmostClient(baseURL, "u@example.com", "pw");
+  const bad = "019f499a"; // truncated
+
+  await assert.rejects(() => client.resolveComment(bad, true), /resolve_comment: 'commentId'/);
+  await assert.rejects(() => client.updateComment(bad, "hi"), /update_comment: 'commentId'/);
+  await assert.rejects(() => client.deleteComment(bad), /delete_comment: 'commentId'/);
+  await assert.rejects(() => client.getComment(bad), /get_comment: 'commentId'/);
+  // createComment validates parentCommentId only when provided.
+  await assert.rejects(
+    () => client.createComment("page-1", "body", "inline", "sel", bad),
+    /create_comment: 'parentCommentId'/,
+  );
+
+  assert.equal(requests, 0, "no request (not even /auth/login) may be issued for a bad id");
+});

packages/mcp/test/unit/server-instructions.test.mjs

@@ -1,82 +0,0 @@
-// Guard: every tool the MCP server registers must be routed by intent in
-// SERVER_INSTRUCTIONS — the editing guide clients receive in the initialize
-// result. Without this, new tools silently rot out of the guide and agents
-// never learn to pick them (the guide once omitted 17 of 41 tools, including
-// get_outline, which pushed agents into fetching whole documents for block
-// ids). Tool names are extracted from the SOURCE (index.ts inline
-// registrations + tool-specs.ts shared specs) so a registration added either
-// way is caught; the guide text itself is imported from the build so the test
-// checks what actually ships.
-import { test } from "node:test";
-import assert from "node:assert/strict";
-import { readFileSync } from "node:fs";
-import { fileURLToPath } from "node:url";
-import { dirname, join } from "node:path";
-
-import { SERVER_INSTRUCTIONS } from "../../build/index.js";
-
-const HERE = dirname(fileURLToPath(import.meta.url));
-const SRC = join(HERE, "..", "..", "src");
-
-// Tools DELIBERATELY absent from the guide. Keep this list minimal and
-// justify every entry — the default is: every tool gets routed.
-const EXCEPTIONS = new Set([
-  // Trivial and self-explanatory; carries no routing decision.
-  "get_workspace",
-]);
-
-/**
- * Extract every registered tool name from the source. Two registration
- * mechanisms exist and both are covered:
- *  - inline `server.registerTool("name", ...)` calls in index.ts;
- *  - shared specs in tool-specs.ts (`mcpName: 'name'`), registered via
- *    registerShared(SHARED_TOOL_SPECS.x, ...).
- */
-function registeredToolNames() {
-  const indexSrc = readFileSync(join(SRC, "index.ts"), "utf8");
-  const specsSrc = readFileSync(join(SRC, "tool-specs.ts"), "utf8");
-  const names = new Set();
-  for (const m of indexSrc.matchAll(/registerTool\(\s*"([a-z0-9_]+)"/g)) {
-    names.add(m[1]);
-  }
-  for (const m of specsSrc.matchAll(/mcpName:\s*['"]([a-z0-9_]+)['"]/g)) {
-    names.add(m[1]);
-  }
-  return names;
-}
-
-test("every registered tool is mentioned in SERVER_INSTRUCTIONS", () => {
-  const names = registeredToolNames();
-  // Sanity: if extraction regressed (regex drift), fail loudly rather than
-  // vacuously passing on an empty set.
-  assert.ok(
-    names.size >= 40,
-    `sanity: expected to extract 40+ registered tools, got ${names.size} — ` +
-      "the extraction regexes in this test likely drifted from the source",
-  );
-  const missing = [...names]
-    .filter((n) => !EXCEPTIONS.has(n))
-    // \b<name>\b: `_` is a word char, so \bget_page\b does NOT match inside
-    // get_page_json — a tool can't hide behind a longer sibling's mention.
-    .filter((n) => !new RegExp(`\\b${n}\\b`).test(SERVER_INSTRUCTIONS))
-    .sort();
-  assert.deepEqual(
-    missing,
-    [],
-    `tools missing from SERVER_INSTRUCTIONS: ${missing.join(", ")} — ` +
-      "update the guide in packages/mcp/src/index.ts (see its MAINTENANCE " +
-      "RULE comment), or add a justified entry to EXCEPTIONS here",
-  );
-});
-
-test("EXCEPTIONS entries are real registered tools", () => {
-  // A stale exception (tool renamed/removed) must be cleaned up, otherwise
-  // the list quietly grows past its purpose.
-  const names = registeredToolNames();
-  for (const name of EXCEPTIONS) {
-    assert.ok(
-      names.has(name),
-      `EXCEPTIONS entry "${name}" is not a registered tool — remove it`,
-    );
-  }
-});

packages/mcp/test/unit/tool-inventory.test.mjs

@@ -0,0 +1,104 @@
+// Guard: the GENERATED <tool_inventory> in SERVER_INSTRUCTIONS (issue #448)
+// names every tool the server registers. The inventory is BUILT from the
+// registry (SHARED_TOOL_SPECS' mcpName/catalogLine + INLINE_MCP_INVENTORY), so
+// the shared-registry tools can never drift by construction; this test's job is
+// to catch the ONE remaining manual list — INLINE_MCP_INVENTORY — falling out
+// of sync with the inline `server.registerTool(...)` calls in index.ts.
+//
+// It also asserts the composed guide keeps its routing prose (the hand-written
+// intent hints) and is a valid non-empty string — the structural guarantees the
+// old name-scraper test (server-instructions.test.mjs, now deleted) carried,
+// minus its now-redundant per-name prose scrape.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import { fileURLToPath } from "node:url";
+import { dirname, join } from "node:path";
+
+import {
+  SERVER_INSTRUCTIONS,
+  ROUTING_PROSE,
+  buildToolInventoryLines,
+} from "../../build/server-instructions.js";
+
+const HERE = dirname(fileURLToPath(import.meta.url));
+const SRC = join(HERE, "..", "..", "src");
+
+/**
+ * Every tool name the MCP server registers, scraped from the SOURCE:
+ *  - inline `server.registerTool("name", ...)` calls in index.ts;
+ *  - shared specs in tool-specs.ts (`mcpName: 'name'`).
+ * Same two registration mechanisms the old guard covered.
+ */
+function registeredToolNames() {
+  const indexSrc = readFileSync(join(SRC, "index.ts"), "utf8");
+  const specsSrc = readFileSync(join(SRC, "tool-specs.ts"), "utf8");
+  const names = new Set();
+  for (const m of indexSrc.matchAll(/registerTool\(\s*"([a-z0-9_]+)"/g)) {
+    names.add(m[1]);
+  }
+  for (const m of specsSrc.matchAll(/mcpName:\s*['"]([a-z0-9_]+)['"]/g)) {
+    names.add(m[1]);
+  }
+  return names;
+}
+
+test("the generated inventory names every registered tool", () => {
+  const registered = registeredToolNames();
+  // Sanity: if the scrape regressed (regex drift), fail loudly rather than
+  // vacuously passing on an empty set.
+  assert.ok(
+    registered.size >= 40,
+    `sanity: expected 40+ registered tools, got ${registered.size} — ` +
+      "the extraction regexes in this test likely drifted from the source",
+  );
+  const inventory = new Set(buildToolInventoryLines().map((l) => l.name));
+  const missing = [...registered].filter((n) => !inventory.has(n)).sort();
+  assert.deepEqual(
+    missing,
+    [],
+    `tools missing from the generated <tool_inventory>: ${missing.join(", ")} — ` +
+      "a SHARED spec is covered automatically; an INLINE MCP-only tool needs a " +
+      "line added to INLINE_MCP_INVENTORY in src/server-instructions.ts",
+  );
+});
+
+test("the inventory has no phantom tool (every line is a real registered tool)", () => {
+  const registered = registeredToolNames();
+  const phantom = buildToolInventoryLines()
+    .map((l) => l.name)
+    .filter((n) => !registered.has(n))
+    .sort();
+  assert.deepEqual(
+    phantom,
+    [],
+    `<tool_inventory> lists tools that are NOT registered: ${phantom.join(", ")}`,
+  );
+});
+
+test("every inventory line has a non-empty purpose", () => {
+  for (const line of buildToolInventoryLines()) {
+    assert.equal(typeof line.purpose, "string");
+    assert.ok(line.purpose.trim().length > 0, `${line.name}: empty purpose`);
+  }
+});
+
+test("SERVER_INSTRUCTIONS keeps the routing prose and the generated inventory", () => {
+  assert.equal(typeof SERVER_INSTRUCTIONS, "string");
+  assert.ok(SERVER_INSTRUCTIONS.length > 0, "SERVER_INSTRUCTIONS is empty");
+  // Routing prose is spliced in verbatim (the hand-written intent hints).
+  assert.ok(
+    SERVER_INSTRUCTIONS.startsWith(ROUTING_PROSE),
+    "the routing prose is not preserved at the head of the guide",
+  );
+  // The generated inventory block is present.
+  assert.match(SERVER_INSTRUCTIONS, /<tool_inventory>/);
+  assert.match(SERVER_INSTRUCTIONS, /<\/tool_inventory>/);
+  // The routing families are still present in the prose.
+  for (const family of ["READ:", "EDIT:", "PAGES:", "COMMENTS:", "HISTORY:"]) {
+    assert.ok(
+      SERVER_INSTRUCTIONS.includes(family),
+      `routing prose lost its ${family} section`,
+    );
+  }
+});

packages/mcp/test/unit/unstorable-yjs-error.test.mjs

@@ -0,0 +1,82 @@
+// #409: unstorableYjsError diagnostics PRECEDENCE. The opaque Yjs encode failure
+// (`Unknown node type: undefined`) is a node-SHAPE problem, so the shared
+// findInvalidNode is consulted FIRST and yields a path-anchored node message;
+// only a shape-sound doc falls back to findUnstorableAttr (undefined/function/
+// etc. attr values). Exercised through `assertYjsEncodable`, which runs the same
+// encode + error-wrapping the live write path uses.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+import { assertYjsEncodable } from "../../build/lib/collaboration.js";
+import {
+  findInvalidNode,
+  findUnstorableAttr,
+} from "@docmost/prosemirror-markdown";
+
+const doc = (...content) => ({ type: "doc", content });
+
+test("a nested typeless node yields the rich node-shape message (not an attr hint)", () => {
+  const bad = doc({
+    type: "paragraph",
+    content: [{ text: "oops", marks: [] }], // missing "type":"text"
+  });
+  assert.throws(
+    () => assertYjsEncodable(bad),
+    (err) => {
+      assert.match(err.message, /Invalid node:/);
+      assert.match(err.message, /missing "type"/);
+      assert.match(err.message, /content\[0\]/);
+      // It must NOT fall through to the generic attribute sentence.
+      assert.doesNotMatch(err.message, /Offending attribute/);
+      assert.doesNotMatch(
+        err.message,
+        /attribute likely holds a value Yjs cannot store/,
+      );
+      return true;
+    },
+  );
+});
+
+test("PRECEDENCE: a genuine undefined-attr case is a node-SHAPE-clean case, so the attr fallback fires", () => {
+  // A doc whose node shapes are ALL valid but that carries a Yjs-unstorable
+  // undefined attribute. unstorableYjsError checks findInvalidNode FIRST (must
+  // miss here) and only then findUnstorableAttr (must hit) — this is exactly the
+  // division of labor that keeps a real attr problem from being mislabelled as a
+  // node-shape problem, and vice versa. We assert the two helpers directly (the
+  // wrapper is not exported) because sanitizeForYjs strips undefined attrs before
+  // the live encoder ever sees them, so this branch cannot be reached through
+  // assertYjsEncodable without also failing the clone.
+  const attrProblem = doc({
+    type: "paragraph",
+    attrs: { id: "p1", indent: undefined },
+    content: [{ type: "text", text: "hi" }],
+  });
+  // findInvalidNode: shape is clean -> null (so the wrapper does NOT emit
+  // "Invalid node").
+  assert.equal(findInvalidNode(attrProblem), null);
+  // findUnstorableAttr: pinpoints the undefined attr -> the fallback message.
+  assert.match(findUnstorableAttr(attrProblem) ?? "", /indent \(undefined\)/);
+});
+
+test("PRECEDENCE: a node-shape problem is caught by findInvalidNode even when an attr is also unstorable", () => {
+  // Both a shape problem (typeless nested leaf) AND an unstorable attr exist;
+  // findInvalidNode wins, so the model is pointed at the node shape (the real
+  // root cause of `Unknown node type: undefined`), not the attribute.
+  const both = doc({
+    type: "paragraph",
+    attrs: { id: "p1", indent: undefined },
+    content: [{ text: "oops" }],
+  });
+  const shape = findInvalidNode(both);
+  assert.notEqual(shape, null);
+  assert.match(shape.summary, /missing "type"/);
+});
+
+test("a fully valid document encodes without throwing", () => {
+  const good = doc({
+    type: "paragraph",
+    attrs: { id: "p1" },
+    content: [{ type: "text", text: "hi" }],
+  });
+  assert.doesNotThrow(() => assertYjsEncodable(good));
+});

packages/prosemirror-markdown/src/lib/index.ts

@@ -56,6 +56,7 @@ export {
   deleteNodeById,
   sanitizeForYjs,
   findUnstorableAttr,
+  findInvalidNode,
   insertNodeRelative,
   readTable,
   insertTableRow,

packages/prosemirror-markdown/src/lib/node-ops.ts

@@ -14,7 +14,9 @@
  * `content`, non-object nodes, and absent `attrs` are tolerated.
  */
 
+import { getSchema } from "@tiptap/core";
 import { stripInlineMarkdown } from "./text-normalize.js";
+import { docmostExtensions } from "./docmost-schema.js";
 
 /** Deep-clone a JSON-serializable value without mutating the original. */
 function clone<T>(value: T): T {
@@ -383,6 +385,119 @@ export function findUnstorableAttr(doc: any): string | null {
   return null;
 }
 
+/**
+ * The Docmost schema's known node and mark NAME sets, derived ONCE from the very
+ * same `docmostExtensions` the Yjs encode path builds its schema from
+ * (`getSchema(docmostExtensions)` — mirrored in mcp's `docmostSchema`). Deriving
+ * both from the same extension list guarantees `findInvalidNode`'s "known type"
+ * set matches exactly what `PMNode.fromJSON`/`toYdoc` will actually accept, so
+ * the walker never flags a node the encoder would have stored (or vice versa).
+ * Lazy + cached: the schema is only built on first use.
+ */
+let schemaNames: { nodes: Set<string>; marks: Set<string> } | null = null;
+function getSchemaNames(): { nodes: Set<string>; marks: Set<string> } {
+  if (schemaNames == null) {
+    const schema = getSchema(docmostExtensions);
+    schemaNames = {
+      nodes: new Set(Object.keys(schema.nodes)),
+      marks: new Set(Object.keys(schema.marks)),
+    };
+  }
+  return schemaNames;
+}
+
+/**
+ * Depth-first walk of the JSON `content` tree looking for the FIRST node whose
+ * SHAPE the Yjs encode path will reject with an opaque
+ * `Unknown node type: undefined` (issue #409). Returns `{ path, summary }` for
+ * the offending node, or `null` when every node (and every mark) is a known
+ * Docmost schema type.
+ *
+ * Two failure modes are detected, in order, per node:
+ *   1. `type` is missing or not a string — the dominant `undefined` case, e.g.
+ *      a text leaf written as `{"text":"foo"}` with no `"type":"text"`.
+ *   2. `type` is a string but NOT a known Docmost node name (a typo / unknown
+ *      block), OR one of the node's marks carries an unknown mark name.
+ *
+ * The returned `summary` is a model-actionable, path-anchored message such as:
+ *   `node.content[2].content[0]: missing "type" (keys: text, marks) — did you
+ *    mean {"type": "text", ...}?`
+ * or for an unknown type:
+ *   `node.content[1]: unknown node type "paragraf" — not in the Docmost schema`
+ *
+ * `path` is the same dotted JSON path used in the summary (e.g.
+ * `node.content[2].content[0]`) so callers can surface it separately. Null-safe:
+ * a non-object doc returns `null`.
+ *
+ * NOTE: This is a SHAPE check, not a full ProseMirror content-model validation
+ * (it does not verify that a paragraph may legally contain a table, etc.). Its
+ * job is to turn the specific "unknown/absent node type" Yjs crash into a clear,
+ * pre-write diagnostic; the schema's own `.check()` still catches deeper
+ * content-model violations at encode time.
+ */
+export function findInvalidNode(
+  doc: any,
+): { path: string; summary: string } | null {
+  if (!isObject(doc)) return null;
+  const { nodes, marks } = getSchemaNames();
+
+  // Build the "did you mean" hint for a typeless node from its own keys, so the
+  // model sees WHICH object is malformed and the canonical text-leaf fix.
+  const keyHint = (node: Record<string, any>): string => {
+    const keys = Object.keys(node);
+    const looksLikeText =
+      typeof node.text === "string" && node.type === undefined;
+    const suffix = looksLikeText
+      ? ` — did you mean {"type": "text", ...}?`
+      : ` — every node needs a string "type" from the Docmost schema`;
+    return `missing "type" (keys: ${keys.join(", ") || "none"})${suffix}`;
+  };
+
+  const walk = (
+    node: any,
+    path: string,
+  ): { path: string; summary: string } | null => {
+    if (!isObject(node)) return null;
+
+    // (1) missing / non-string type.
+    if (typeof node.type !== "string") {
+      return { path, summary: `${path}: ${keyHint(node)}` };
+    }
+    // (2) string type that is not a known Docmost node.
+    if (!nodes.has(node.type)) {
+      return {
+        path,
+        summary: `${path}: unknown node type "${node.type}" — not in the Docmost schema`,
+      };
+    }
+    // (2b) unknown mark on an otherwise-valid node.
+    if (Array.isArray(node.marks)) {
+      for (let i = 0; i < node.marks.length; i++) {
+        const mark = node.marks[i];
+        if (isObject(mark) && typeof mark.type === "string" && !marks.has(mark.type)) {
+          return {
+            path: `${path}.marks[${i}]`,
+            summary: `${path}.marks[${i}]: unknown mark type "${mark.type}" — not in the Docmost schema`,
+          };
+        }
+      }
+    }
+
+    if (Array.isArray(node.content)) {
+      for (let i = 0; i < node.content.length; i++) {
+        const hit = walk(node.content[i], `${path}.content[${i}]`);
+        if (hit != null) return hit;
+      }
+    }
+    return null;
+  };
+
+  // The root doc node is addressed as "node" (matching the mcp arg name); its
+  // children are node.content[i]. The root itself is checked too so a typeless
+  // root is reported rather than silently skipped.
+  return walk(doc, "node");
+}
+
 /**
  * Table structural node types and the container each must live directly inside.
  * Used by `insertNodeRelative` to splice rows/cells into the correct ancestor

packages/prosemirror-markdown/test/find-invalid-node.test.ts

@@ -0,0 +1,102 @@
+import { describe, expect, it } from 'vitest';
+import { findInvalidNode } from '../src/lib/node-ops.js';
+
+// findInvalidNode (#409): a depth-first SHAPE gate that turns the encoder's
+// opaque `Unknown node type: undefined` into a path-anchored, pre-write
+// diagnostic. It flags the FIRST node whose `type` is absent/non-string or not
+// a known Docmost schema node, or that carries an unknown mark; returns null for
+// a well-formed doc.
+
+const doc = (...content: any[]) => ({ type: 'doc', content });
+const para = (...content: any[]) => ({
+  type: 'paragraph',
+  attrs: { id: 'p1' },
+  content,
+});
+const text = (value: string, marks?: any[]) => {
+  const node: any = { type: 'text', text: value };
+  if (marks) node.marks = marks;
+  return node;
+};
+
+describe('findInvalidNode', () => {
+  it('returns null for a fully valid document', () => {
+    const good = doc(
+      para(text('hello ', [{ type: 'bold' }]), text('world')),
+      {
+        type: 'heading',
+        attrs: { id: 'h1', level: 2 },
+        content: [text('Title')],
+      },
+    );
+    expect(findInvalidNode(good)).toBeNull();
+  });
+
+  it('flags a NESTED typeless text leaf with a path-precise summary', () => {
+    // A text leaf written as {"text":"foo"} with no "type":"text" — the dominant
+    // `Unknown node type: undefined` cause.
+    const bad = doc(
+      para(text('ok')),
+      para({ text: 'foo', marks: [] } as any),
+    );
+    const hit = findInvalidNode(bad);
+    expect(hit).not.toBeNull();
+    // Second paragraph (index 1), first child (index 0).
+    expect(hit!.path).toBe('node.content[1].content[0]');
+    expect(hit!.summary).toContain('node.content[1].content[0]');
+    expect(hit!.summary).toContain('missing "type"');
+    expect(hit!.summary).toContain('keys: text, marks');
+    expect(hit!.summary).toContain('did you mean {"type": "text", ...}');
+  });
+
+  it('flags a non-string type (e.g. numeric)', () => {
+    const bad = doc(para({ type: 123, content: [] } as any));
+    const hit = findInvalidNode(bad);
+    expect(hit).not.toBeNull();
+    expect(hit!.path).toBe('node.content[0].content[0]');
+    expect(hit!.summary).toContain('missing "type"');
+  });
+
+  it('flags an UNKNOWN node type name that is not in the Docmost schema', () => {
+    const bad = doc({
+      type: 'paragraf', // typo — not a real node
+      attrs: { id: 'x' },
+      content: [text('hi')],
+    });
+    const hit = findInvalidNode(bad);
+    expect(hit).not.toBeNull();
+    expect(hit!.path).toBe('node.content[0]');
+    expect(hit!.summary).toContain('unknown node type "paragraf"');
+    expect(hit!.summary).toContain('not in the Docmost schema');
+  });
+
+  it('flags an UNKNOWN mark type on an otherwise-valid node', () => {
+    const bad = doc(para(text('hi', [{ type: 'blink' }])));
+    const hit = findInvalidNode(bad);
+    expect(hit).not.toBeNull();
+    expect(hit!.path).toBe('node.content[0].content[0].marks[0]');
+    expect(hit!.summary).toContain('unknown mark type "blink"');
+  });
+
+  it('accepts every real Docmost node/mark type it is asked about', () => {
+    // Known node (callout) and known marks (italic, code) must NOT be flagged.
+    const good = doc({
+      type: 'callout',
+      attrs: { id: 'c1', type: 'info' },
+      content: [para(text('x', [{ type: 'italic' }, { type: 'code' }]))],
+    });
+    expect(findInvalidNode(good)).toBeNull();
+  });
+
+  it('reports the root itself when the root is typeless', () => {
+    const hit = findInvalidNode({ content: [] } as any);
+    expect(hit).not.toBeNull();
+    expect(hit!.path).toBe('node');
+  });
+
+  it('is null-safe for non-object input', () => {
+    expect(findInvalidNode(null)).toBeNull();
+    expect(findInvalidNode(undefined)).toBeNull();
+    expect(findInvalidNode('nope' as any)).toBeNull();
+  });
+});

pnpm-lock.yaml

@@ -1044,6 +1044,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
@@ -6876,6 +6879,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'}
@@ -17642,6 +17648,8 @@ snapshots:
 
   electron-to-chromium@1.5.286: {}
 
+  elkjs@0.11.1: {}
+
   emittery@0.13.1: {}
 
   emoji-regex@8.0.0: {}