fix(ai-chat): MCP-кэш — in-run восстановление, ретраи только для readOnly (#489)

Кэш внешних MCP-клиентов не делал health-check/reconnect-on-error: после
разрыва SSE-транспорта (bodyTimeout режет при тишине МЕЖДУ вызовами) отдавал
труп до конца TTL, а модель жгла шаги на ретраях ВНУТРИ текущего рана.

- Новое поле writeClass ('readOnly'|'write') на КАЖДОМ SHARED_TOOL_SPEC +
  registration-time assert (и compile-time через satisfies). Все 48 спеков
  расклассифицированы: чтения → readOnly, любая мутация страницы/коммента/шэра/
  диаграммы → write. Экспортированы SHARED_TOOL_WRITE_CLASS + isRetryableWriteClass.
- Per-run обёртка восстановления транспорта: при транспортной ошибке readOnly-тул
  реконнектит свой сервер и ретраит РОВНО 1 раз ВНУТРИ рана; write-тул НЕ
  авторетраится (indeterminate — «могло примениться, проверь», класс инцидента
  #435). CAS-своп байндинга по identity (проигравший конкурентный вызов ретраит
  на текущем клиенте, не минтит второй). Лизы не освобождаются mid-run — ран
  копит set (старая+новая) и релизит на turn-end.
- Проверка abortSignal ПЕРЕД ретраем И ПЕРЕД чеканкой свежего клиента; per-call
  cap покрывает оба attempt'а + connect.
- Классификация транспортной ошибки по РЕАЛЬНЫМ шейпам undici (SocketError/
  BodyTimeoutError, cause-цепочка), не по мок-ошибкам.
- Отдельный, поднятый bodyTimeout для SSE-транспорта MCP (тишина между вызовами
  легальна) — DEFAULT 10 мин, AI_MCP_SSE_BODY_TIMEOUT_MS.

write-class map грузится в mcp-clients лениво через dynamic import (пакет ESM),
type-only импорт — без static require ESM из commonjs.

Тесты на РЕАЛЬНЫХ error-шейпах: «повтор после обрыва ВНУТРИ рана получает живой
клиент», «write-тул не авторетраится», «ретрай после Stop не происходит»,
+ writeClass-контракт в mcp node --test.

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

apps/server/src/core/ai-chat/external-mcp/mcp-clients.recovery.spec.ts

@@ -0,0 +1,214 @@
+import { errors } from 'undici';
+import {
+  McpClientsService,
+  isRetryableConnectError,
+} from './mcp-clients.service';
+
+/**
+ * #489 — external-MCP in-run transport recovery.
+ *
+ * The transport-error classification + retry gate are exercised against the REAL
+ * undici error CLASSES prod throws (`errors.SocketError` / `errors.BodyTimeoutError`,
+ * carrying the true `UND_ERR_*` codes and class names), wrapped EXACTLY as undici's
+ * `fetch` wraps them — a `TypeError('fetch failed'|'terminated')` whose `.cause` is
+ * the undici error. These are the real classes, not hand-rolled `{code:'...'}`
+ * mocks: constructing the genuine class is what makes this a faithful test of the
+ * prod predicate (epic root-cause #4 — a mock-shaped predicate would leave the
+ * evict/retry path silently dead in production while CI stays green). We construct
+ * rather than drive a live fetch because Jest's environment degrades the live-fetch
+ * error to a generic `Error` cause (no undici code), which would NOT be the prod
+ * shape.
+ */
+
+/** A REAL undici socket reset, wrapped as fetch wraps it. */
+function realSocketResetError(): unknown {
+  const err = new TypeError('fetch failed');
+  (err as { cause?: unknown }).cause = new errors.SocketError('other side closed');
+  return err;
+}
+
+/** A REAL undici body timeout, wrapped as fetch wraps it. */
+function realBodyTimeoutError(): unknown {
+  const err = new TypeError('terminated');
+  (err as { cause?: unknown }).cause = new errors.BodyTimeoutError();
+  return err;
+}
+
+type FakeServer = {
+  id: string;
+  name: string;
+  transport: 'http' | 'sse';
+  url: string;
+  headersEnc: string | null;
+  toolAllowlist: string[] | null;
+  instructions: string | null;
+};
+
+const server = (over: Partial<FakeServer> = {}): FakeServer => ({
+  id: 's1',
+  name: 'srv',
+  transport: 'http',
+  url: 'http://example.test/mcp',
+  headersEnc: null,
+  toolAllowlist: null,
+  instructions: null,
+  ...over,
+});
+
+function buildService(servers: FakeServer[]) {
+  const repo = { listEnabled: jest.fn().mockResolvedValue(servers) };
+  const service = new McpClientsService(repo as never, {} as never);
+  // Seed a DETERMINISTIC write-class map so the retry gate is controlled here
+  // (the production map loads from @docmost/mcp via a dynamic ESM import). getPage
+  // is a read, patchNode is a write — the real classifications.
+  (
+    service as unknown as { writeClassMapPromise: Promise<unknown> }
+  ).writeClassMapPromise = Promise.resolve({
+    getPage: 'readOnly',
+    patchNode: 'write',
+  });
+  return { service, repo };
+}
+
+/** Spy the private `connect` so each call yields a controlled fake client whose
+ *  single tool's execute is the supplied function. Returns the connect spy. */
+function stubConnect(
+  service: McpClientsService,
+  toolName: string,
+  execs: Array<(...a: unknown[]) => Promise<unknown>>,
+) {
+  let n = 0;
+  return jest
+    .spyOn(
+      service as unknown as { connect: (s: FakeServer) => Promise<unknown> },
+      'connect',
+    )
+    .mockImplementation(async () => {
+      const exec = execs[Math.min(n, execs.length - 1)];
+      n += 1;
+      return {
+        tools: async () => ({ [toolName]: { description: 'x', execute: exec } }),
+        close: jest.fn().mockResolvedValue(undefined),
+      };
+    });
+}
+
+const opts = (abortSignal?: AbortSignal) =>
+  ({ toolCallId: 't', messages: [], abortSignal }) as never;
+
+describe('isRetryableConnectError (#489, REAL error shapes)', () => {
+  it('classifies a real undici socket reset and body timeout as retryable', async () => {
+    const socketErr = await realSocketResetError();
+    const bodyErr = await realBodyTimeoutError();
+    expect(isRetryableConnectError(socketErr)).toBe(true);
+    expect(isRetryableConnectError(bodyErr)).toBe(true);
+    // Unwraps a wrapped cause chain (e.g. an MCPClientError around the socket err).
+    const wrapped = new Error('mcp call failed');
+    (wrapped as { cause?: unknown }).cause = socketErr;
+    expect(isRetryableConnectError(wrapped)).toBe(true);
+  });
+
+  it('does NOT classify an application-level error as a transport break', () => {
+    expect(isRetryableConnectError(new Error('validation failed'))).toBe(false);
+    expect(isRetryableConnectError({ name: 'HttpError', status: 400 })).toBe(false);
+    expect(isRetryableConnectError(undefined)).toBe(false);
+    expect(isRetryableConnectError('boom')).toBe(false);
+  });
+});
+
+describe('McpClientsService in-run transport recovery (#489)', () => {
+  afterEach(() => jest.restoreAllMocks());
+
+  it('a readOnly tool whose transport breaks reconnects and retries WITHIN the same run', async () => {
+    const realErr = await realSocketResetError();
+    const { service } = buildService([server()]);
+    const first = jest.fn().mockRejectedValue(realErr);
+    const second = jest.fn().mockResolvedValue({ ok: true });
+    const connectSpy = stubConnect(service, 'getPage', [first, second]);
+
+    const toolset = await service.toolsFor('ws-1');
+    const tool = toolset.tools['srv_getPage'];
+    const result = await (tool.execute as (a: unknown, o: unknown) => Promise<unknown>)(
+      { pageId: 'p' },
+      opts(),
+    );
+
+    // The repeat call within the run got a LIVE client and succeeded.
+    expect(result).toEqual({ ok: true });
+    expect(first).toHaveBeenCalledTimes(1);
+    expect(second).toHaveBeenCalledTimes(1);
+    // Exactly one reconnect was minted (initial build connect + one recovery).
+    expect(connectSpy).toHaveBeenCalledTimes(2);
+    // The run accumulated BOTH leases (old + reconnected) — released together at end.
+    expect(toolset.clients).toHaveLength(2);
+    await Promise.all(toolset.clients.map((c) => c.close()));
+  });
+
+  it('a WRITE tool does NOT auto-retry on a transport error (indeterminate)', async () => {
+    const realErr = await realSocketResetError();
+    const { service } = buildService([server()]);
+    const exec = jest.fn().mockRejectedValue(realErr);
+    const connectSpy = stubConnect(service, 'patchNode', [exec]);
+
+    const toolset = await service.toolsFor('ws-2');
+    const tool = toolset.tools['srv_patchNode'];
+    await expect(
+      (tool.execute as (a: unknown, o: unknown) => Promise<unknown>)(
+        { pageId: 'p' },
+        opts(),
+      ),
+    ).rejects.toThrow(/MAY have already applied/);
+
+    // Called exactly once — NO blind retry (avoids double-apply, the #435 class).
+    expect(exec).toHaveBeenCalledTimes(1);
+    // No fresh connection was minted for a write.
+    expect(connectSpy).toHaveBeenCalledTimes(1);
+    await Promise.all(toolset.clients.map((c) => c.close()));
+  });
+
+  it('does NOT retry (or reconnect) after the run is aborted (Stop)', async () => {
+    const realErr = await realSocketResetError();
+    const { service } = buildService([server()]);
+    const controller = new AbortController();
+    // The transport error arrives, but the run was Stopped in the same tick.
+    const first = jest.fn().mockImplementation(async () => {
+      controller.abort();
+      throw realErr;
+    });
+    const second = jest.fn().mockResolvedValue({ ok: true });
+    const connectSpy = stubConnect(service, 'getPage', [first, second]);
+
+    const toolset = await service.toolsFor('ws-3');
+    const tool = toolset.tools['srv_getPage'];
+    await expect(
+      (tool.execute as (a: unknown, o: unknown) => Promise<unknown>)(
+        { pageId: 'p' },
+        opts(controller.signal),
+      ),
+    ).rejects.toBeDefined();
+
+    // getPage IS readOnly, but the Stop blocks the retry — no second call, no mint.
+    expect(second).not.toHaveBeenCalled();
+    expect(connectSpy).toHaveBeenCalledTimes(1);
+    await Promise.all(toolset.clients.map((c) => c.close()));
+  });
+
+  it('an app-level (non-transport) tool error is surfaced verbatim, never retried', async () => {
+    const { service } = buildService([server()]);
+    const appErr = new Error('tool says: bad input');
+    const exec = jest.fn().mockRejectedValue(appErr);
+    const connectSpy = stubConnect(service, 'getPage', [exec]);
+
+    const toolset = await service.toolsFor('ws-4');
+    const tool = toolset.tools['srv_getPage'];
+    await expect(
+      (tool.execute as (a: unknown, o: unknown) => Promise<unknown>)(
+        { pageId: 'p' },
+        opts(),
+      ),
+    ).rejects.toThrow('tool says: bad input');
+    expect(exec).toHaveBeenCalledTimes(1);
+    expect(connectSpy).toHaveBeenCalledTimes(1); // no reconnect for an app error
+    await Promise.all(toolset.clients.map((c) => c.close()));
+  });
+});

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

@@ -106,8 +106,11 @@ describe('McpClientsService.decryptHeaders', () => {
 
 describe('McpClientsService.guardedFetch (SSRF per-request guard)', () => {
   // The bound guardedFetch closure lives on the instance as a private field.
+  // #489 split it into per-transport HTTP/SSE bindings (they differ only in the
+  // dispatcher's bodyTimeout); the SSRF guard is identical, so testing the HTTP
+  // one is sufficient.
   const guardedFetchOf = (service: McpClientsService) =>
-    (service as unknown as { guardedFetch: typeof fetch }).guardedFetch;
+    (service as unknown as { guardedFetchHttp: typeof fetch }).guardedFetchHttp;
 
   let fetchSpy: jest.SpiedFunction<typeof fetch>;
 

apps/server/src/core/ai-chat/external-mcp/mcp-clients.service.ts

@@ -1,5 +1,6 @@
 import { isIP } from 'node:net';
 import { lookup as dnsLookup, type LookupAddress } from 'node:dns';
+import { pathToFileURL } from 'node:url';
 import { Injectable, Logger } from '@nestjs/common';
 import { type Tool, type ToolCallOptions } from 'ai';
 import { createMCPClient } from '@ai-sdk/mcp';
@@ -10,9 +11,29 @@ import {
   streamingDispatcherOptions,
   mcpStreamTimeoutMs,
   mcpCallTimeoutMs,
+  mcpSseBodyTimeoutMs,
 } from '../../../integrations/ai/ai-streaming-fetch';
 import { SecretBoxService } from '../../../integrations/crypto/secret-box';
 import { isUrlAllowed, isIpAllowed } from './ssrf-guard';
+// TYPE-ONLY (erased at compile): @docmost/mcp is ESM-only and cannot be a runtime
+// `require()` from this commonjs module (same constraint as docmost-client.loader).
+// The write-class MAP is loaded lazily via the dynamic-import trick below.
+import type { ToolWriteClass } from '@docmost/mcp';
+
+// TS(commonjs) downlevels a literal `import()` to `require()`, which cannot load
+// the ESM-only @docmost/mcp. Indirect through Function so the real dynamic
+// `import()` survives compilation (same trick as docmost-client.loader.ts).
+const esmImport = new Function(
+  'specifier',
+  'return import(specifier)',
+) as (specifier: string) => Promise<unknown>;
+
+/** Local read-only predicate — avoids a value import of the ESM-only package.
+ *  Only a pure read is retry-safe after a transport break (a write is
+ *  indeterminate). Kept in lockstep with @docmost/mcp's isRetryableWriteClass. */
+function isReadOnlyWriteClass(writeClass: ToolWriteClass | undefined): boolean {
+  return writeClass === 'readOnly';
+}
 
 /** A closable external MCP client handle. */
 export interface Closable {
@@ -81,12 +102,52 @@ const MAX_TOOL_NAME_LENGTH = 64;
  * close until the turn releases it, so a TTL expiry mid-turn never closes a
  * client a stream is still executing against.
  */
+/**
+ * Where a merged (namespaced) tool came from, so the per-run recovery wrapper
+ * (#489) can, on a transport error, reconnect THAT server and re-resolve the SAME
+ * underlying tool by its raw name. `writeClass` gates the single auto-retry (a
+ * read is retry-safe; a write is indeterminate). `serverIndex` indexes the
+ * entry's `servers` array (which server config to reconnect).
+ */
+interface ToolProvenance {
+  serverIndex: number;
+  rawName: string;
+  writeClass: ToolWriteClass | undefined;
+}
+
+/** A live reconnected server (its fresh client + raw call-timeout-wrapped tools). */
+interface RecoveredServerState {
+  client: McpClient;
+  tools: Record<string, Tool>;
+}
+
+/**
+ * Per-run, per-server recovery binding (#489). `current` is the server's LIVE
+ * target for this run: `null` means "use the ORIGINAL cached client/template";
+ * a non-null value is a reconnected throwaway client all this server's tools now
+ * call. `reconnecting` dedupes concurrent reconnects so only ONE fresh client is
+ * minted per death (a losing concurrent call awaits it and retries on the SAME
+ * new client — the CAS-by-identity rule).
+ */
+interface ServerBinding {
+  current: RecoveredServerState | null;
+  reconnecting?: Promise<RecoveredServerState>;
+}
+
 interface CacheEntry {
   tools: Record<string, Tool>;
   clients: McpClient[];
   outcomes: ServerOutcome[];
   /** Prompt guidance for qualifying servers (see McpServerInstruction). */
   instructions: McpServerInstruction[];
+  /**
+   * The enabled server configs used to build this entry (#489), so the per-run
+   * recovery wrapper can reconnect a specific server by index. Parallel to the
+   * indices referenced by {@link toolMeta}.
+   */
+  servers: AiMcpServer[];
+  /** merged-tool-key -> provenance (#489), for the per-run recovery wrapper. */
+  toolMeta: Record<string, ToolProvenance>;
   expiresAt: number;
   /** Active leases (turns currently using these clients). */
   refCount: number;
@@ -120,20 +181,65 @@ export class McpClientsService {
    */
   private readonly cache = new Map<string, Promise<CacheEntry>>();
   /**
-   * A single shared SSRF-pinned dispatcher for ALL outbound external-MCP fetches.
-   * Its custom connect.lookup runs per connection, so one instance safely guards
-   * every server's connections (we never connect to an unvalidated IP).
+   * SSRF-pinned dispatchers for outbound external-MCP fetches. Both use the SAME
+   * custom connect.lookup (so every connection is IP-validated), but carry a
+   * DIFFERENT `bodyTimeout` (#489): the HTTP (streamable) transport opens a fresh
+   * request per call, so it keeps the tight silence timeout; the SSE transport
+   * holds ONE long-lived body open across many calls, so a >1-min idle BETWEEN
+   * calls is LEGITIMATE and must not break the socket — it gets a much larger
+   * bodyTimeout. (headersTimeout stays tight on both.)
    */
-  private readonly dispatcher: Dispatcher = buildPinnedDispatcher();
-  /** guardedFetch bound to the pinned dispatcher; reused by every transport. */
-  private readonly guardedFetch: typeof fetch = (input, init) =>
-    guardedFetch(this.dispatcher, input, init);
+  private readonly dispatcherHttp: Dispatcher = buildPinnedDispatcher(
+    mcpStreamTimeoutMs(),
+  );
+  private readonly dispatcherSse: Dispatcher = buildPinnedDispatcher(
+    mcpSseBodyTimeoutMs(),
+  );
+  /** guardedFetch bound to each dispatcher; picked by transport type in connect(). */
+  private readonly guardedFetchHttp: typeof fetch = (input, init) =>
+    guardedFetch(this.dispatcherHttp, input, init);
+  private readonly guardedFetchSse: typeof fetch = (input, init) =>
+    guardedFetch(this.dispatcherSse, input, init);
+
+  /**
+   * Memoized write-class map (#489), loaded lazily from @docmost/mcp via the
+   * dynamic-import trick. Keyed by tool name (=== mcpName). A tool NOT in the map
+   * (any third-party external MCP tool) classifies as `undefined` -> treated as a
+   * write by the retry gate (the safe default: never blind-retry an unknown tool).
+   * On any load failure the map is `{}` (every tool -> no auto-retry), so a
+   * missing/older @docmost/mcp build only DISABLES retries, never mis-retries.
+   */
+  private writeClassMapPromise: Promise<Record<string, ToolWriteClass>> | null =
+    null;
 
   constructor(
     private readonly repo: AiMcpServerRepo,
     private readonly secretBox: SecretBoxService,
   ) {}
 
+  /** Lazily load + memoize the shared write-class map (see the field doc). */
+  private getWriteClassMap(): Promise<Record<string, ToolWriteClass>> {
+    if (!this.writeClassMapPromise) {
+      this.writeClassMapPromise = (async () => {
+        try {
+          const entry = require.resolve('@docmost/mcp');
+          const mod = (await esmImport(pathToFileURL(entry).href)) as {
+            SHARED_TOOL_WRITE_CLASS?: Record<string, ToolWriteClass>;
+          };
+          return mod.SHARED_TOOL_WRITE_CLASS ?? {};
+        } catch (err) {
+          this.logger.warn(
+            `Could not load MCP write-class map (auto-retry disabled): ${shortError(
+              err,
+            )}`,
+          );
+          return {};
+        }
+      })();
+    }
+    return this.writeClassMapPromise;
+  }
+
   /**
    * Build (or reuse a cached) external toolset for a workspace. Returns the
    * merged tools, the open client handles to release, and per-server outcomes.
@@ -162,11 +268,37 @@ export class McpClientsService {
         }
       },
     };
-    // One release handle drives the whole leased entry; closing it releases all
-    // underlying clients together (they share the same lease lifecycle).
+
+    // #489: the run accumulates a SET of leases — the primary cache lease PLUS any
+    // throwaway client minted by an in-run transport-recovery reconnect. They are
+    // NEVER released mid-run (releasing a swapped-out client while a concurrent
+    // in-flight call still holds it would INDUCE a second failure); the caller
+    // releases the WHOLE set together at turn-end. A recovery reconnect pushes its
+    // lease onto this live array, which the consumer closes over.
+    const leaseSet: Closable[] = [release];
+
+    // #489: per-RUN transport-recovery binding, one per server, SHARED by all of
+    // that server's tools so a swap by one call is seen by the next (CAS by
+    // identity). Kept per-run (here, not in the cached entry) because the binding
+    // + lease-set state is per-run.
+    const bindings = new Map<number, ServerBinding>();
+    const capMs = mcpCallTimeoutMs();
+
+    // Wrap each cached tool with the recovery layer. On a transport error a
+    // declared readOnly tool reconnects its server and retries ONCE; a write is
+    // never blind-retried (indeterminate — may have applied before the reset). A
+    // tool without provenance (a minimal stub entry in a test) passes through raw.
+    const tools: Record<string, Tool> = {};
+    for (const [key, tool] of Object.entries(entry.tools)) {
+      const meta = entry.toolMeta?.[key];
+      tools[key] = meta
+        ? this.wrapWithTransportRecovery(entry, meta, tool, leaseSet, bindings, capMs)
+        : tool;
+    }
+
     return {
-      tools: entry.tools,
-      clients: [release],
+      tools,
+      clients: leaseSet,
       outcomes: entry.outcomes,
       instructions: entry.instructions,
     };
@@ -254,6 +386,11 @@ export class McpClientsService {
     // Per-call total wall-clock cap, read once for this build (env-overridable).
     const callTimeoutMs = mcpCallTimeoutMs();
     const instructions: McpServerInstruction[] = [];
+    // merged-key -> provenance for the per-run recovery wrapper (#489).
+    const toolMeta: Record<string, ToolProvenance> = {};
+    // Shared write-class map (#489): classifies each merged tool by its raw name
+    // so the recovery wrapper knows which tools are retry-safe reads.
+    const writeClassMap = await this.getWriteClassMap();
 
     // Per-server connect+tools result, still tagged with its server so the merge
     // below can be applied in the SAME order as `servers` (see the parallel note).
@@ -332,6 +469,9 @@ export class McpClientsService {
         result.guarded,
         server.name,
         server.id,
+        toolMeta,
+        i,
+        writeClassMap,
       );
       outcomes.push({ name: server.name, ok: true });
       // Include this server's guidance ONLY when it actually contributed at
@@ -353,6 +493,8 @@ export class McpClientsService {
       clients,
       outcomes,
       instructions,
+      servers,
+      toolMeta,
       expiresAt: Date.now() + CACHE_TTL_MS,
       refCount: 0,
       evicted: false,
@@ -379,18 +521,30 @@ export class McpClientsService {
     picked: Record<string, Tool>,
     serverName: string,
     serverId: string,
+    toolMeta: Record<string, ToolProvenance>,
+    serverIndex: number,
+    writeClassMap: Record<string, ToolWriteClass>,
   ): { count: number; prefix: string } {
     let count = 0;
-    for (const [name, tool] of Object.entries(namespace(picked, serverName))) {
-      let key = name;
+    for (const { full, raw, tool } of namespace(picked, serverName)) {
+      let key = full;
       if (key in target) {
         const original = key;
-        key = disambiguate(name, serverId, (candidate) => candidate in target);
+        key = disambiguate(full, serverId, (candidate) => candidate in target);
         this.logger.debug(
           `External MCP tool name "${original}" collided; renamed to "${key}"`,
         );
       }
       target[key] = tool;
+      // Record provenance so the per-run recovery wrapper (#489) can reconnect
+      // this tool's server and re-resolve it by its raw name, and gate the retry
+      // on its write-class (unknown third-party tool -> undefined -> treated as a
+      // write, i.e. never auto-retried).
+      toolMeta[key] = {
+        serverIndex,
+        rawName: raw,
+        writeClass: writeClassMap[raw],
+      };
       count += 1;
     }
     return { count, prefix: namespacePrefix(serverName) };
@@ -424,7 +578,10 @@ export class McpClientsService {
         // Defense in depth: re-validate the actual request host on EVERY fetch
         // AND pin the socket to a validated IP via the dispatcher's connect
         // lookup, closing the DNS-rebinding TOCTOU between check and connect.
-        fetch: this.guardedFetch,
+        // #489: the SSE transport uses the raised-bodyTimeout dispatcher (idle
+        // between calls is legit); HTTP uses the tight one.
+        fetch:
+          transportType === 'sse' ? this.guardedFetchSse : this.guardedFetchHttp,
       },
     })) as unknown as McpClient;
     return client;
@@ -505,6 +662,168 @@ export class McpClientsService {
     }
   }
 
+  /**
+   * Wrap one merged external tool with the per-run transport-recovery layer (#489).
+   *
+   * attempt 1 runs on the server's CURRENT binding (the cached client, or a client
+   * a sibling tool already reconnected this run). On a REAL transport error
+   * (undici/@ai-sdk socket/body-timeout shapes — {@link isRetryableConnectError},
+   * NOT a mock) and ONLY for a declared readOnly tool, it reconnects the server
+   * and retries EXACTLY ONCE on the fresh client; a write is surfaced as an
+   * indeterminate error (it may have applied before the reset — never
+   * blind-retried). A single per-call cap bounds BOTH attempts + the reconnect,
+   * and the run's abort signal is checked before the retry AND before minting a
+   * fresh connection (no connection is opened for a stopped run).
+   */
+  private wrapWithTransportRecovery(
+    entry: CacheEntry,
+    meta: ToolProvenance,
+    template: Tool,
+    leaseSet: Closable[],
+    bindings: Map<number, ServerBinding>,
+    capMs: number,
+  ): Tool {
+    const original = template.execute;
+    if (typeof original !== 'function') return template;
+    const service = this;
+    const { serverIndex, rawName, writeClass } = meta;
+
+    let binding = bindings.get(serverIndex);
+    if (!binding) {
+      binding = { current: null };
+      bindings.set(serverIndex, binding);
+    }
+    const boundBinding = binding;
+
+    const execute = async (args: unknown, options: ToolCallOptions) => {
+      // The per-call cap governs the WHOLE sequence (attempt1 + reconnect +
+      // attempt2). Compose it with the run's abort signal so a Stop or the cap
+      // ends any awaited call — @ai-sdk/mcp does not settle on abort, so we RACE.
+      const capController = new AbortController();
+      const capTimer = setTimeout(() => {
+        capController.abort(new Error(`MCP tool call timed out after ${capMs}ms`));
+      }, capMs);
+      capTimer.unref?.();
+      const runSignal = options?.abortSignal;
+      const composed = runSignal
+        ? AbortSignal.any([runSignal, capController.signal])
+        : capController.signal;
+      const stopped = () => runSignal?.aborted === true || capController.signal.aborted;
+
+      const callOn = async (
+        exec: NonNullable<Tool['execute']>,
+      ): Promise<unknown> => {
+        const aborted = new Promise<never>((_, reject) => {
+          const fail = () => reject(abortReason(composed));
+          if (composed.aborted) fail();
+          else composed.addEventListener('abort', fail, { once: true });
+        });
+        return Promise.race([exec(args, { ...options, abortSignal: composed }), aborted]);
+      };
+
+      const execFor = (
+        state: RecoveredServerState | null,
+      ): NonNullable<Tool['execute']> | undefined =>
+        state ? (state.tools[rawName]?.execute as NonNullable<Tool['execute']>) : original;
+
+      try {
+        // Snapshot the target BEFORE the call so a swap by a concurrent call is
+        // detected by identity in the catch.
+        const attemptState = boundBinding.current;
+        const attemptExec = execFor(attemptState);
+        if (typeof attemptExec !== 'function') {
+          throw new Error(`external MCP tool "${rawName}" is not callable`);
+        }
+        try {
+          return await callOn(attemptExec);
+        } catch (err) {
+          // Never retry on a Stop or an exhausted cap.
+          if (stopped()) throw err;
+          // Only a genuine transport break is a recovery candidate.
+          if (!isRetryableConnectError(err)) throw err;
+          // A write tool is INDETERMINATE on a transport error (may have applied
+          // before the reset) — surface that; do NOT auto-retry (double-apply is
+          // the #435 incident class).
+          if (!isReadOnlyWriteClass(writeClass)) {
+            throw new Error(
+              `external MCP tool "${rawName}" hit a transport error and MAY have already ` +
+                `applied on the server — not retried automatically; verify state before ` +
+                `retrying. (${shortError(err)})`,
+            );
+          }
+          // Abort check BEFORE minting a fresh connection.
+          if (stopped()) throw err;
+          // CAS-swap by IDENTITY: mint+swap only if nobody swapped since this
+          // call's snapshot; a losing concurrent call awaits the same reconnect
+          // and retries on the SAME fresh client.
+          let target: RecoveredServerState;
+          if (boundBinding.current === attemptState) {
+            if (!boundBinding.reconnecting) {
+              boundBinding.reconnecting = (async () => {
+                const server = entry.servers[serverIndex];
+                const fresh = await service.reconnectServer(server, capMs);
+                leaseSet.push(fresh.lease); // accumulate; released at turn-end
+                boundBinding.current = fresh.state;
+                return fresh.state;
+              })();
+              // Clear the in-flight marker once it settles (success or failure) so
+              // a LATER death of the new client can reconnect again.
+              void boundBinding.reconnecting.then(
+                () => (boundBinding.reconnecting = undefined),
+                () => (boundBinding.reconnecting = undefined),
+              );
+            }
+            target = await boundBinding.reconnecting;
+          } else {
+            target = boundBinding.current as RecoveredServerState;
+          }
+          // Abort check BEFORE the retry.
+          if (stopped()) throw err;
+          const retryExec = execFor(target);
+          if (typeof retryExec !== 'function') throw err;
+          return await callOn(retryExec);
+        }
+      } finally {
+        clearTimeout(capTimer);
+      }
+    };
+    return { ...template, execute } as unknown as Tool;
+  }
+
+  /**
+   * Reconnect ONE server for an in-run recovery (#489): open a fresh client and
+   * list+wrap its tools. The throwaway client is NOT cached — it is owned by the
+   * RUN via the returned lease (closed at turn-end), independent of the shared
+   * cache entry (whose TTL rebuild heals future turns). On a failure the fresh
+   * client is closed so its socket never leaks.
+   */
+  private async reconnectServer(
+    server: AiMcpServer,
+    capMs: number,
+  ): Promise<{ state: RecoveredServerState; lease: Closable }> {
+    const client = await this.connectWithTimeout(server, CONNECT_TIMEOUT_MS);
+    let tools: Record<string, Tool>;
+    try {
+      const raw = await withTimeout(client.tools(), CONNECT_TIMEOUT_MS);
+      const allow = server.toolAllowlist;
+      const picked =
+        Array.isArray(allow) && allow.length > 0 ? pick(raw, allow) : raw;
+      tools = wrapToolsWithCallTimeout(picked, capMs);
+    } catch (err) {
+      void client.close().catch(() => undefined);
+      throw err;
+    }
+    let released = false;
+    const lease: Closable = {
+      close: async () => {
+        if (released) return;
+        released = true;
+        await client.close().catch(() => undefined);
+      },
+    };
+    return { state: { client, tools }, lease };
+  }
+
   /** Mark an entry evicted; close its clients now if nothing is leasing them. */
   private evict(entry: CacheEntry): void {
     clearTimeout(entry.timer);
@@ -554,22 +873,21 @@ export function validateResolvedAddresses(addrs: readonly LookupAddress[]): {
  * certificate validation still uses the real hostname (we never rewrite the URL
  * to an IP literal).
  */
-function buildPinnedDispatcher(): Agent {
-  // External-MCP traffic uses a DEDICATED, shorter silence timeout
+function buildPinnedDispatcher(bodyTimeoutMs: number): Agent {
+  // External-MCP traffic uses a DEDICATED, shorter HEADERS silence timeout
   // (`AI_MCP_STREAM_TIMEOUT_MS`, default 1 min) — deliberately tighter than the
   // chat provider's 15-min `streamTimeoutMs()` — so a byte-silent/hung MCP
   // upstream is broken in ~1 min instead of 15. We keep the keep-alive options
-  // from `streamingDispatcherOptions()` but OVERRIDE headers/body timeouts.
-  // Accepted trade-off: a legitimately long but byte-silent single tool call,
-  // and an SSE transport idling >1 min BETWEEN tool calls, are also cut here; the
-  // per-call total cap (wrapToolsWithCallTimeout, `AI_MCP_CALL_TIMEOUT_MS`) is the
-  // complementary guard for chatty-but-stuck calls that keep the socket warm yet
-  // never return.
-  const mcpSilenceMs = mcpStreamTimeoutMs();
+  // from `streamingDispatcherOptions()` but OVERRIDE the timeouts. `bodyTimeout`
+  // is passed in per-transport (#489): tight for HTTP (fresh request per call),
+  // raised for SSE (one long-lived body across calls — idle BETWEEN calls is
+  // legit). The per-call total cap (`AI_MCP_CALL_TIMEOUT_MS`) is the complementary
+  // guard for chatty-but-stuck calls that keep the socket warm yet never return.
+  const headersMs = mcpStreamTimeoutMs();
   return new Agent({
     ...streamingDispatcherOptions(),
-    headersTimeout: mcpSilenceMs,
-    bodyTimeout: mcpSilenceMs,
+    headersTimeout: headersMs,
+    bodyTimeout: bodyTimeoutMs,
     connect: {
       lookup: (hostname, _options, callback) => {
         // Always resolve ALL addresses ourselves; do not trust the caller's
@@ -669,18 +987,22 @@ function pick(
 function namespace(
   tools: Record<string, Tool>,
   serverName: string,
-): Record<string, Tool> {
+): Array<{ full: string; raw: string; tool: Tool }> {
   const prefix = namespacePrefix(serverName);
-  const out: Record<string, Tool> = {};
+  const out: Array<{ full: string; raw: string; tool: Tool }> = [];
+  const taken: Record<string, true> = {};
   for (const [name, t] of Object.entries(tools)) {
     const safe = sanitizeName(name);
     let full = capName(`${prefix}_${safe}`);
     // Duplicate names within ONE server can still collide after sanitize/
     // truncate — suffix-disambiguate so the second tool is not overwritten.
-    if (full in out) {
-      full = disambiguate(full, '', (candidate) => candidate in out);
+    if (full in taken) {
+      full = disambiguate(full, '', (candidate) => candidate in taken);
     }
-    out[full] = t;
+    taken[full] = true;
+    // Keep the RAW (un-namespaced) name alongside the merged key so the per-run
+    // recovery wrapper (#489) can re-resolve the same tool on a fresh client.
+    out.push({ full, raw: name, tool: t });
   }
   return out;
 }
@@ -804,6 +1126,69 @@ export function wrapToolWithCallTimeout(tool: Tool, ms: number): Tool {
   return { ...tool, execute } as unknown as Tool;
 }
 
+/**
+ * undici / Node network error CODES that mean the connection broke (not an
+ * application-level error) — a transient transport failure a readOnly call may
+ * safely retry after reconnecting. Matched against the REAL error shapes (#489):
+ * a socket reset surfaces as `TypeError: fetch failed` whose `.cause` is an
+ * undici `SocketError { code:'UND_ERR_SOCKET' }`; a body-timeout as
+ * `TypeError: terminated` whose `.cause` is `BodyTimeoutError`. Classifying by
+ * these real codes/names (not by mock errors) is essential — a mock-shaped
+ * predicate would leave eviction silently dead in production while CI is green.
+ */
+const RETRYABLE_TRANSPORT_ERROR_CODES: ReadonlySet<string> = new Set([
+  'ECONNRESET',
+  'ECONNREFUSED',
+  'ECONNABORTED',
+  'EPIPE',
+  'ETIMEDOUT',
+  'EAI_AGAIN',
+  'ENETUNREACH',
+  'EHOSTUNREACH',
+  'UND_ERR_SOCKET',
+  'UND_ERR_CONNECT_TIMEOUT',
+  'UND_ERR_HEADERS_TIMEOUT',
+  'UND_ERR_BODY_TIMEOUT',
+  'UND_ERR_CLOSED',
+  'UND_ERR_DESTROYED',
+]);
+
+/** undici error CLASS names for the same transport-break conditions. */
+const RETRYABLE_TRANSPORT_ERROR_NAMES: ReadonlySet<string> = new Set([
+  'SocketError',
+  'BodyTimeoutError',
+  'HeadersTimeoutError',
+  'ConnectTimeoutError',
+  'ClientClosedError',
+  'ClientDestroyedError',
+]);
+
+/**
+ * Whether `err` is a retryable TRANSPORT break (a broken socket / body timeout),
+ * classified by the REAL undici/@ai-sdk error shapes (#489). undici surfaces a
+ * reset as `TypeError('fetch failed'|'terminated')` with the real error in
+ * `.cause`, and @ai-sdk/mcp may wrap it again in an `MCPClientError` (cause
+ * chain), so we walk `.cause` (bounded depth) checking `.code` and `.name`. An
+ * app-level tool error (a 4xx, a validation failure) is NOT retryable and returns
+ * false — only a connection-level failure heals with a reconnect.
+ */
+export function isRetryableConnectError(err: unknown, depth = 0): boolean {
+  if (!err || typeof err !== 'object' || depth > 6) return false;
+  const e = err as {
+    code?: unknown;
+    name?: unknown;
+    cause?: unknown;
+  };
+  if (typeof e.code === 'string' && RETRYABLE_TRANSPORT_ERROR_CODES.has(e.code)) {
+    return true;
+  }
+  if (typeof e.name === 'string' && RETRYABLE_TRANSPORT_ERROR_NAMES.has(e.name)) {
+    return true;
+  }
+  if (e.cause != null) return isRetryableConnectError(e.cause, depth + 1);
+  return false;
+}
+
 /** The signal's reason as an Error (informative thrown value on abort/timeout). */
 function abortReason(signal: AbortSignal): Error {
   const r = signal.reason;

apps/server/src/integrations/ai/ai-streaming-fetch.ts

@@ -129,6 +129,12 @@ const DEFAULT_MCP_STREAM_TIMEOUT_MS = 60_000;
 /** Default total wall-clock cap for ONE external MCP tool call (2 min). */
 const DEFAULT_MCP_CALL_TIMEOUT_MS = 120_000;
 
+/**
+ * Default `bodyTimeout` for the EXTERNAL-MCP SSE transport (10 min) — #489.
+ * Deliberately much LARGER than {@link DEFAULT_MCP_STREAM_TIMEOUT_MS}.
+ */
+const DEFAULT_MCP_SSE_BODY_TIMEOUT_MS = 600_000;
+
 /**
  * SILENCE timeout (ms) for EXTERNAL-MCP transport ONLY. Override with
  * `AI_MCP_STREAM_TIMEOUT_MS`; a missing/invalid/non-positive value falls back to
@@ -164,6 +170,26 @@ export function mcpCallTimeoutMs(): number {
   return positiveEnv('AI_MCP_CALL_TIMEOUT_MS', DEFAULT_MCP_CALL_TIMEOUT_MS);
 }
 
+/**
+ * `bodyTimeout` (ms) for the EXTERNAL-MCP **SSE** transport ONLY — #489. Override
+ * with `AI_MCP_SSE_BODY_TIMEOUT_MS`; a missing/invalid/non-positive value falls
+ * back to {@link DEFAULT_MCP_SSE_BODY_TIMEOUT_MS} (10 min).
+ *
+ * The SSE transport holds ONE long-lived response body open across many tool
+ * calls, so undici's `bodyTimeout` (time between body bytes) counts the LEGITIMATE
+ * silence BETWEEN calls, not just a hung single call. At the tight HTTP silence
+ * timeout ({@link mcpStreamTimeoutMs}, 1 min) a normal >1-min gap between the
+ * model's tool calls would break the SSE socket, and the cache would then serve a
+ * dead client until TTL. So the SSE transport gets its OWN, RAISED bodyTimeout;
+ * the per-call total cap ({@link mcpCallTimeoutMs}) still bounds a single stuck
+ * call, and the app-level transport-error retry heals a socket that does break.
+ * The HTTP (streamable) transport keeps the tight timeout — it opens a fresh
+ * request per call, so idle-between-calls does not apply there.
+ */
+export function mcpSseBodyTimeoutMs(): number {
+  return positiveEnv('AI_MCP_SSE_BODY_TIMEOUT_MS', DEFAULT_MCP_SSE_BODY_TIMEOUT_MS);
+}
+
 /**
  * undici `Agent` options for streaming AI traffic — the (generous, finite)
  * silence timeouts plus the keep-alive recycle window. Shared by the chat

packages/mcp/src/index.ts

@@ -30,6 +30,13 @@ export { destroyAllSessions } from "./lib/collab-session.js";
 // internals directly; it goes through loadDocmostMcp()).
 export { SHARED_TOOL_SPECS } from "./tool-specs.js";
 export type { SharedToolSpec } from "./tool-specs.js";
+// #489 — write-class registry consumed by the in-app external-MCP retry gate.
+export {
+  SHARED_TOOL_WRITE_CLASS,
+  isRetryableWriteClass,
+  assertEverySpecDeclaresWriteClass,
+} from "./tool-specs.js";
+export type { ToolWriteClass } from "./tool-specs.js";
 
 // Re-export the build-time REGISTRY_STAMP (issue #447): a deterministic hash of
 // the tool-specs registry content, generated into src/registry-stamp.generated.ts

packages/mcp/src/tool-specs.ts

@@ -127,6 +127,18 @@ export interface SharedToolSpec {
   mcpName: string;
   /** camelCase key in the ai-SDK tools object (the in-app layer). */
   inAppKey: string;
+  /**
+   * Write-class of the tool (#489), declared on EVERY spec (a registration-time
+   * assert enforces completeness; `satisfies Record<string, SharedToolSpec>`
+   * makes it a compile error to omit). 'readOnly' = a pure read that mutates
+   * NOTHING durable, so it is safe to auto-retry once after a transport break.
+   * 'write' = anything that mutates a page/comment/share/diagram/etc — a
+   * transport error is INDETERMINATE (the server may have applied it before the
+   * connection reset), so it is NEVER blind-retried (a retry would double-apply,
+   * the #435 incident class). Consumed by the external-MCP retry path
+   * (mcp-clients.service.ts) to gate its single auto-retry.
+   */
+  writeClass: 'readOnly' | 'write';
   /** Single canonical model-facing description used by both layers. */
   description: string;
   /**
@@ -240,6 +252,7 @@ export const SHARED_TOOL_SPECS = {
   getWorkspace: {
     mcpName: 'getWorkspace',
     inAppKey: 'getWorkspace',
+    writeClass: 'readOnly',
     description: 'Fetch metadata about the current workspace (name, settings).',
     tier: 'core',
     catalogLine: 'getWorkspace — fetch current workspace metadata (name, settings).',
@@ -249,6 +262,7 @@ export const SHARED_TOOL_SPECS = {
   listSpaces: {
     mcpName: 'listSpaces',
     inAppKey: 'listSpaces',
+    writeClass: 'readOnly',
     description:
       'List the spaces the current user can access. Returns the array of ' +
       'spaces (id, name, slug, ...).',
@@ -260,6 +274,7 @@ export const SHARED_TOOL_SPECS = {
   listShares: {
     mcpName: 'listShares',
     inAppKey: 'listShares',
+    writeClass: 'readOnly',
     description:
       'List all public shares in the workspace with page titles and public URLs.',
     tier: 'deferred',
@@ -272,6 +287,7 @@ export const SHARED_TOOL_SPECS = {
   getPageJson: {
     mcpName: 'getPageJson',
     inAppKey: 'getPageJson',
+    writeClass: 'readOnly',
     description:
       'Get page details with the raw ProseMirror JSON content (lossless: ' +
       'includes block ids, callouts, tables, link/image attributes) plus the ' +
@@ -289,6 +305,7 @@ export const SHARED_TOOL_SPECS = {
   getOutline: {
     mcpName: 'getOutline',
     inAppKey: 'getOutline',
+    writeClass: 'readOnly',
     description:
       "Return a COMPACT outline of a page's top-level blocks ({index, type, " +
       'id, level, firstText}; tables add rows/cols/header; lists add item ' +
@@ -309,6 +326,7 @@ export const SHARED_TOOL_SPECS = {
   getNode: {
     mcpName: 'getNode',
     inAppKey: 'getNode',
+    writeClass: 'readOnly',
     description:
       "Fetch a single block for editing. `nodeId` is a block id from the page " +
       'outline or page-JSON view (works for headings/paragraphs/callouts/images), OR ' +
@@ -350,6 +368,7 @@ export const SHARED_TOOL_SPECS = {
   searchInPage: {
     mcpName: 'searchInPage',
     inAppKey: 'searchInPage',
+    writeClass: 'readOnly',
     description:
       'Find every occurrence of a string (or regex) INSIDE one page and get ' +
       'WHERE each is — instead of pulling blocks one-by-one with getNode. ' +
@@ -413,6 +432,7 @@ export const SHARED_TOOL_SPECS = {
   deleteNode: {
     mcpName: 'deleteNode',
     inAppKey: 'deleteNode',
+    writeClass: 'write',
     description:
       'Remove a single block by its attrs.id (from the page outline or ' +
       'page-JSON view) WITHOUT resending the whole document.',
@@ -438,6 +458,7 @@ export const SHARED_TOOL_SPECS = {
   patchNode: {
     mcpName: 'patchNode',
     inAppKey: 'patchNode',
+    writeClass: 'write',
     description:
       'Replace a single content block identified by its attrs.id, WITHOUT ' +
       'resending the whole document; the replacement keeps the same block id. ' +
@@ -505,6 +526,7 @@ export const SHARED_TOOL_SPECS = {
   insertNode: {
     mcpName: 'insertNode',
     inAppKey: 'insertNode',
+    writeClass: 'write',
     description:
       'Insert content before/after another block (by attrs.id or anchor text) ' +
       'or append it at the end (top level). For before/after you MUST provide ' +
@@ -597,6 +619,7 @@ export const SHARED_TOOL_SPECS = {
   sharePage: {
     mcpName: 'sharePage',
     inAppKey: 'sharePage',
+    writeClass: 'write',
     // CANONICAL: merges the MCP copy's URL-format + idempotency detail with the
     // in-app copy's reversibility note; keeps the security framing both had.
     description:
@@ -626,6 +649,7 @@ export const SHARED_TOOL_SPECS = {
   unsharePage: {
     mcpName: 'unsharePage',
     inAppKey: 'unsharePage',
+    writeClass: 'write',
     description: 'Remove the public share of a page (revokes the public URL).',
     tier: 'deferred',
     catalogLine: "unsharePage — revoke a page's public share (removes the public URL).",
@@ -640,6 +664,7 @@ export const SHARED_TOOL_SPECS = {
   diffPageVersions: {
     mcpName: 'diffPageVersions',
     inAppKey: 'diffPageVersions',
+    writeClass: 'readOnly',
     description:
       'Diff two versions of a page and return a Docmost-equivalent change set ' +
       '(inserted/deleted text, integrity counts for images/links/tables/' +
@@ -672,6 +697,7 @@ export const SHARED_TOOL_SPECS = {
   listPageHistory: {
     mcpName: 'listPageHistory',
     inAppKey: 'listPageHistory',
+    writeClass: 'readOnly',
     description:
       "List a page's saved versions (Docmost auto-snapshots on every save), " +
       'newest first, cursor-paginated. Returns { items, nextCursor }; each ' +
@@ -693,6 +719,7 @@ export const SHARED_TOOL_SPECS = {
   restorePageVersion: {
     mcpName: 'restorePageVersion',
     inAppKey: 'restorePageVersion',
+    writeClass: 'write',
     description:
       'Restore a page to a saved version: writes that version\'s content back ' +
       'as the page\'s current content (Docmost has no restore endpoint, so ' +
@@ -713,6 +740,7 @@ export const SHARED_TOOL_SPECS = {
   importPageMarkdown: {
     mcpName: 'importPageMarkdown',
     inAppKey: 'importPageMarkdown',
+    writeClass: 'write',
     // IN-APP ONLY (issue #411): the external /mcp surface no longer exposes
     // importPageMarkdown — the registry loop in index.ts skips inAppOnly specs,
     // so this stays available to the in-app agent (round-tripping an EXPORTED
@@ -742,6 +770,7 @@ export const SHARED_TOOL_SPECS = {
   copyPageContent: {
     mcpName: 'copyPageContent',
     inAppKey: 'copyPageContent',
+    writeClass: 'write',
     description:
       "Replace targetPageId's content with a copy of sourcePageId's content, " +
       'entirely server-side — the document is NOT sent through the model. The ' +
@@ -770,6 +799,7 @@ export const SHARED_TOOL_SPECS = {
   editPageText: {
     mcpName: 'editPageText',
     inAppKey: 'editPageText',
+    writeClass: 'write',
     description:
       "Surgical find/replace inside a page's text, preserving all block " +
       'ids and marks. A find MAY cross bold/italic/link boundaries; the ' +
@@ -819,6 +849,7 @@ export const SHARED_TOOL_SPECS = {
   stashPage: {
     mcpName: 'stashPage',
     inAppKey: 'stashPage',
+    writeClass: 'readOnly',
     description:
       'Serialize a whole page (the full ProseMirror JSON, as getPageJson ' +
       'returns) into an ephemeral in-memory blob and return ONLY a short ' +
@@ -880,6 +911,7 @@ export const SHARED_TOOL_SPECS = {
   getPage: {
     mcpName: 'getPage',
     inAppKey: 'getPage',
+    writeClass: 'readOnly',
     description:
       'Fetch a single page as Markdown by its id. Returns the page title and ' +
       'its Markdown content. The converter is canonical (round-trips text and ' +
@@ -919,6 +951,7 @@ export const SHARED_TOOL_SPECS = {
   listPages: {
     mcpName: 'listPages',
     inAppKey: 'listPages',
+    writeClass: 'readOnly',
     description:
       'List the most recent pages (ordered by updatedAt, descending), ' +
       'optionally scoped to a single space. Returns a bounded list (default ' +
@@ -965,6 +998,7 @@ export const SHARED_TOOL_SPECS = {
   getTree: {
     mcpName: 'getTree',
     inAppKey: 'getTree',
+    writeClass: 'readOnly',
     description:
       "Get a space's page hierarchy (or one subtree) as a nested tree in a " +
       'SINGLE request — completely and without loss. Each node is ' +
@@ -1009,6 +1043,7 @@ export const SHARED_TOOL_SPECS = {
   getPageContext: {
     mcpName: 'getPageContext',
     inAppKey: 'getPageContext',
+    writeClass: 'readOnly',
     description:
       'Given a pageId, get its LOCATION and immediate surroundings (metadata ' +
       'only, no page content) in one call — answers "where am I / what is ' +
@@ -1038,6 +1073,7 @@ export const SHARED_TOOL_SPECS = {
   createPage: {
     mcpName: 'createPage',
     inAppKey: 'createPage',
+    writeClass: 'write',
     description:
       'Create a new page with a Markdown body in a space, optionally under a ' +
       'parent page (omit parentPageId to create at the space root). Returns ' +
@@ -1088,6 +1124,7 @@ export const SHARED_TOOL_SPECS = {
   movePage: {
     mcpName: 'movePage',
     inAppKey: 'movePage',
+    writeClass: 'write',
     description:
       'Move a page under a new parent page, or to the space root when no ' +
       'parent is given. Reversible: move it back at any time.',
@@ -1181,6 +1218,7 @@ export const SHARED_TOOL_SPECS = {
   renamePage: {
     mcpName: 'renamePage',
     inAppKey: 'renamePage',
+    writeClass: 'write',
     description:
       'Rename a page (change its title only; the body is untouched, never ' +
       'resent). Reversible: rename back at any time.',
@@ -1202,6 +1240,7 @@ export const SHARED_TOOL_SPECS = {
   deletePage: {
     mcpName: 'deletePage',
     inAppKey: 'deletePage',
+    writeClass: 'write',
     description:
       'Move a page to the trash — SOFT delete only: the page can be restored ' +
       'from trash and nothing is ever permanently deleted.',
@@ -1235,6 +1274,7 @@ export const SHARED_TOOL_SPECS = {
   updatePageJson: {
     mcpName: 'updatePageJson',
     inAppKey: 'updatePageJson',
+    writeClass: 'write',
     description:
       "Replace a page's content with a raw ProseMirror JSON document (lossless " +
       'write: preserves the block ids, callouts, tables and attributes you pass ' +
@@ -1291,6 +1331,7 @@ export const SHARED_TOOL_SPECS = {
   updatePageMarkdown: {
     mcpName: 'updatePageMarkdown',
     inAppKey: 'updatePageMarkdown',
+    writeClass: 'write',
     description:
       "Replace a page's body with new Markdown content (and optionally its " +
       'title). The whole body is re-imported from the markdown (block ids ' +
@@ -1325,6 +1366,7 @@ export const SHARED_TOOL_SPECS = {
   exportPageMarkdown: {
     mcpName: 'exportPageMarkdown',
     inAppKey: 'exportPageMarkdown',
+    writeClass: 'readOnly',
     // CANONICAL: the MCP copy (a strict superset of the terse in-app wording).
     description:
       'Export a page to a single self-contained Docmost-flavoured Markdown ' +
@@ -1373,6 +1415,7 @@ export const SHARED_TOOL_SPECS = {
   createComment: {
     mcpName: 'createComment',
     inAppKey: 'createComment',
+    writeClass: 'write',
     // CANONICAL: the in-app copy (the more-maintained one). It keeps the same
     // rules as the MCP copy — inline-only, top-level requires a `selection`, no
     // page-level comments, replies inherit the anchor, suggestedText must be
@@ -1505,6 +1548,7 @@ export const SHARED_TOOL_SPECS = {
   listComments: {
     mcpName: 'listComments',
     inAppKey: 'listComments',
+    writeClass: 'readOnly',
     // CANONICAL: the two copies are near-identical; the MCP copy is the
     // superset (it keeps the "(pagination is handled internally)" note the
     // in-app copy dropped), so it is used verbatim.
@@ -1532,6 +1576,7 @@ export const SHARED_TOOL_SPECS = {
   resolveComment: {
     mcpName: 'resolveComment',
     inAppKey: 'resolveComment',
+    writeClass: 'write',
     // CANONICAL: the MCP copy's richer wording, minus its reference
     // to `deleteComment` (a sibling tool that does NOT exist in the in-app
     // layer) — rephrased transport-neutrally per the registry convention.
@@ -1572,6 +1617,7 @@ export const SHARED_TOOL_SPECS = {
   checkNewComments: {
     mcpName: 'checkNewComments',
     inAppKey: 'checkNewComments',
+    writeClass: 'readOnly',
     // CANONICAL: the MCP copy (the more detailed of the two). The MCP layer's
     // execute-side guard that rejects an unparseable `since` timestamp stays in
     // its execute body (per-layer logic), not in the shared schema.
@@ -1651,6 +1697,7 @@ export const SHARED_TOOL_SPECS = {
   tableInsertRow: {
     mcpName: 'tableInsertRow',
     inAppKey: 'tableInsertRow',
+    writeClass: 'write',
     description:
       'Insert a row of plain-text cells into a table. `table` is `#<index>` ' +
       'from the page outline, or a block id inside it. `cells` is the text per ' +
@@ -1684,6 +1731,7 @@ export const SHARED_TOOL_SPECS = {
   tableDeleteRow: {
     mcpName: 'tableDeleteRow',
     inAppKey: 'tableDeleteRow',
+    writeClass: 'write',
     description:
       'Delete the row at 0-based `index` from a table (`table` is `#<index>` ' +
       'from the page outline, or a block id inside it). Refuses to delete the ' +
@@ -1707,6 +1755,7 @@ export const SHARED_TOOL_SPECS = {
   tableUpdateCell: {
     mcpName: 'tableUpdateCell',
     inAppKey: 'tableUpdateCell',
+    writeClass: 'write',
     description:
       'Set the plain-text content of cell [row, col] (0-based) in a table ' +
       '(`table` is `#<index>` from the page outline, or a block id inside it). ' +
@@ -1747,6 +1796,7 @@ export const SHARED_TOOL_SPECS = {
   insertFootnote: {
     mcpName: 'insertFootnote',
     inAppKey: 'insertFootnote',
+    writeClass: 'write',
     description:
       'Insert an AUTHOR-INLINE footnote: you specify only WHERE (anchorText) ' +
       'and WHAT (text). The footnote marker is placed right after anchorText in ' +
@@ -1784,6 +1834,7 @@ export const SHARED_TOOL_SPECS = {
   insertImage: {
     mcpName: 'insertImage',
     inAppKey: 'insertImage',
+    writeClass: 'write',
     description:
       'Download an image from a web (http/https) URL and insert it into ' +
       'a page in one step. By default ' +
@@ -1828,6 +1879,7 @@ export const SHARED_TOOL_SPECS = {
   replaceImage: {
     mcpName: 'replaceImage',
     inAppKey: 'replaceImage',
+    writeClass: 'write',
     description:
       'Replace an existing image on a page with a new image fetched from a web ' +
       '(http/https) URL: uploads the new file as a NEW ' +
@@ -1866,6 +1918,7 @@ export const SHARED_TOOL_SPECS = {
   drawioGet: {
     mcpName: 'drawioGet',
     inAppKey: 'drawioGet',
+    writeClass: 'readOnly',
     description:
       'Read a draw.io diagram on a page as mxGraph XML (default) or as its raw ' +
       '`.drawio.svg`. `node` is the drawio node\'s attrs.id (from getOutline / ' +
@@ -1899,6 +1952,7 @@ export const SHARED_TOOL_SPECS = {
   drawioCreate: {
     mcpName: 'drawioCreate',
     inAppKey: 'drawioCreate',
+    writeClass: 'write',
     description:
       'Create a draw.io diagram from mxGraph XML and insert it as a diagram ' +
       'block. `xml` is a bare `<mxGraphModel>` OR a list of `<mxCell>` elements ' +
@@ -1969,6 +2023,7 @@ export const SHARED_TOOL_SPECS = {
   drawioUpdate: {
     mcpName: 'drawioUpdate',
     inAppKey: 'drawioUpdate',
+    writeClass: 'write',
     description:
       'Replace a draw.io diagram\'s content with new mxGraph XML (same lint ' +
       'pipeline as drawioCreate). `baseHash` is MANDATORY: pass the hash from ' +
@@ -2020,6 +2075,7 @@ export const SHARED_TOOL_SPECS = {
   drawioEditCells: {
     mcpName: 'drawioEditCells',
     inAppKey: 'drawioEditCells',
+    writeClass: 'write',
     description:
       'Make TARGETED, id-based edits to an existing draw.io diagram instead of ' +
       'resending the whole XML (a full-XML diff is fragile — draw.io reorders ' +
@@ -2078,6 +2134,7 @@ export const SHARED_TOOL_SPECS = {
   drawioFromGraph: {
     mcpName: 'drawioFromGraph',
     inAppKey: 'drawioFromGraph',
+    writeClass: 'write',
     description:
       'Build a draw.io diagram from a SEMANTIC graph — you describe nodes, groups ' +
       'and edges by MEANING and the server picks every coordinate, color and icon ' +
@@ -2202,6 +2259,7 @@ export const SHARED_TOOL_SPECS = {
   drawioFromMermaid: {
     mcpName: 'drawioFromMermaid',
     inAppKey: 'drawioFromMermaid',
+    writeClass: 'write',
     description:
       'Convert Mermaid `flowchart` text into an EDITABLE draw.io diagram (LLMs ' +
       'write Mermaid reliably). Best for STANDARD flowcharts/decision trees: ' +
@@ -2248,6 +2306,7 @@ export const SHARED_TOOL_SPECS = {
   drawioShapes: {
     mcpName: 'drawioShapes',
     inAppKey: 'drawioShapes',
+    writeClass: 'readOnly',
     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 ' +
@@ -2294,6 +2353,7 @@ export const SHARED_TOOL_SPECS = {
   drawioGuide: {
     mcpName: 'drawioGuide',
     inAppKey: 'drawioGuide',
+    writeClass: 'readOnly',
     description:
       'Progressive-disclosure draw.io authoring reference. Call with a `section` ' +
       'to pull one focused, <=4KB chapter instead of bloating context: ' +
@@ -2323,3 +2383,53 @@ export const SHARED_TOOL_SPECS = {
     inlineBothHosts: true,
   },
 } satisfies Record<string, SharedToolSpec>;
+
+// --- write-class registry (#489) ------------------------------------------
+
+/** A tool's retry-safety class. 'readOnly' may be auto-retried once after a
+ *  transport break; 'write' is indeterminate and must never be blind-retried. */
+export type ToolWriteClass = 'readOnly' | 'write';
+
+/**
+ * Name → write-class map for the shared registry, keyed by mcpName (=== inAppKey).
+ * The external-MCP retry path (mcp-clients.service.ts) looks a tool up here by its
+ * RAW (un-namespaced) name to decide whether a transport failure may be retried.
+ * A tool NOT in this map (a third-party external MCP tool) is treated as 'write'
+ * by the consumer — the safe default (never blind-retry an unknown tool).
+ */
+export const SHARED_TOOL_WRITE_CLASS: Record<string, ToolWriteClass> =
+  Object.fromEntries(
+    Object.values(SHARED_TOOL_SPECS).map((spec) => [spec.mcpName, spec.writeClass]),
+  );
+
+/** Whether a write-class permits a single automatic retry after a transport
+ *  break. Only a pure read is retry-safe; everything mutating is indeterminate. */
+export function isRetryableWriteClass(
+  writeClass: ToolWriteClass | undefined,
+): boolean {
+  return writeClass === 'readOnly';
+}
+
+/**
+ * Registration-time assert (#489): EVERY spec must declare a valid write-class.
+ * `satisfies Record<string, SharedToolSpec>` already makes an omission a compile
+ * error, but this guards a raw/cast construction path and documents the invariant
+ * at the point of use. Runs once on import — both hosts import this module, so
+ * both get the check. Throws (fails startup) rather than silently mis-gating a
+ * retry in production.
+ */
+export function assertEverySpecDeclaresWriteClass(): void {
+  for (const [key, spec] of Object.entries(SHARED_TOOL_SPECS)) {
+    const wc = (spec as SharedToolSpec).writeClass;
+    if (wc !== 'readOnly' && wc !== 'write') {
+      throw new Error(
+        `tool-specs: spec "${key}" must declare writeClass ('readOnly' | 'write'), got ${JSON.stringify(
+          wc,
+        )}`,
+      );
+    }
+  }
+}
+
+// Enforce at module load (registration time) on both hosts.
+assertEverySpecDeclaresWriteClass();

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

@@ -2,7 +2,12 @@ import { test } from "node:test";
 import assert from "node:assert/strict";
 import { z } from "zod";
 
-import { SHARED_TOOL_SPECS } from "../../build/tool-specs.js";
+import {
+  SHARED_TOOL_SPECS,
+  SHARED_TOOL_WRITE_CLASS,
+  isRetryableWriteClass,
+  assertEverySpecDeclaresWriteClass,
+} from "../../build/tool-specs.js";
 
 // The shared registry is consumed by BOTH the zod-v3 MCP server and the zod-v4
 // in-app AI-SDK service, so every spec must carry the cross-layer wiring
@@ -43,6 +48,41 @@ test("mcpName and inAppKey are each unique across the registry", () => {
   }
 });
 
+// #489 — every spec must declare its write-class so the external-MCP retry path
+// can gate a single auto-retry ONLY on a pure read (a blind retry of a write =
+// double-apply). The declaration is enforced at registration time.
+test("#489: every spec declares a valid writeClass ('readOnly' | 'write')", () => {
+  for (const [key, spec] of Object.entries(SHARED_TOOL_SPECS)) {
+    assert.ok(
+      spec.writeClass === "readOnly" || spec.writeClass === "write",
+      `${key}: missing/invalid writeClass: ${JSON.stringify(spec.writeClass)}`,
+    );
+  }
+  // The registration-time assert must not throw for the shipped registry.
+  assert.doesNotThrow(() => assertEverySpecDeclaresWriteClass());
+});
+
+test("#489: SHARED_TOOL_WRITE_CLASS maps every mcpName to its class; helper gates on readOnly", () => {
+  const specs = Object.values(SHARED_TOOL_SPECS);
+  assert.equal(Object.keys(SHARED_TOOL_WRITE_CLASS).length, specs.length);
+  for (const spec of specs) {
+    assert.equal(SHARED_TOOL_WRITE_CLASS[spec.mcpName], spec.writeClass);
+  }
+  // Only a readOnly tool is retry-eligible; a write tool and an unknown tool are not.
+  assert.equal(isRetryableWriteClass("readOnly"), true);
+  assert.equal(isRetryableWriteClass("write"), false);
+  assert.equal(isRetryableWriteClass(undefined), false);
+});
+
+test("#489: representative reads are readOnly and representative writes are write", () => {
+  for (const name of ["getPage", "getTree", "searchInPage", "listComments"]) {
+    assert.equal(SHARED_TOOL_SPECS[name].writeClass, "readOnly", `${name} should be readOnly`);
+  }
+  for (const name of ["patchNode", "createPage", "deletePage", "createComment", "drawioCreate"]) {
+    assert.equal(SHARED_TOOL_SPECS[name].writeClass, "write", `${name} should be write`);
+  }
+});
+
 test("buildShape (when present) returns a usable ZodRawShape with a real zod", () => {
   for (const [key, spec] of Object.entries(SHARED_TOOL_SPECS)) {
     if (!spec.buildShape) continue;