test(mcp): ассертить collab-hint (pageId + transient/retry) в reject-текстах (ревью #441)

Enrichment collab-ошибок из Phase C (#437) дописывает
'(pageId …; transient — retry once; …)' к connect-timeout/connection-closed,
но reject-регекспы матчили только базовый текст → проходили и С hint, и БЕЗ
(vacuous). Ужесточил два ассерта (connection-closed, connect-timeout) до
'<база> (pageId page-1; transient' — теперь рефактор, убравший hint(), их
роняет. Мутационно: hint()->'' → ровно эти 2 теста краснеют (18->16).

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

.github/workflows/test.yml

@@ -1,13 +1,5 @@
 name: Test
 
-# NO `paths:` filter on purpose (issue #447). The tool-spec REGISTRY is split
-# across two packages that MUST stay in sync: the specs live in `packages/mcp`
-# but the parity/tier guard tests that read them live in the `apps/server` jest
-# suite. A PR touching only `packages/mcp/**` must therefore still run the SERVER
-# suite (and vice-versa), or an in-app wiring break slips through green and only
-# surfaces on develop after merge. The `test` job below runs BOTH suites via
-# `pnpm -r test` on every PR; the dedicated `mcp-server-parity` job makes that
-# cross-package gate explicit and fast. Do not add a `paths:` filter here.
 on:
   pull_request:
   workflow_call:
@@ -140,53 +132,3 @@ jobs:
       # isolated `docmost_test` DB and migrates it to latest.
       - name: Run server integration tests
         run: pnpm --filter server test:int
-
-  # Cross-package tool-spec parity gate (issue #447). The tool-spec registry lives
-  # in `packages/mcp` but its parity/tier guard tests live in the `apps/server`
-  # jest suite, so a PR touching ONLY one of the two packages must still run BOTH
-  # sides — otherwise an in-app wiring break (e.g. PR #434 drawio) passes the mcp
-  # suite green and only surfaces on develop after merge. The `test` job already
-  # runs everything via `pnpm -r test`; this job is a fast, explicitly-named guard
-  # that runs the mcp `node --test` suite AND the server tool-guard jest specs
-  # together, so the coupling is visible and can never be accidentally split by a
-  # path filter. No Postgres/Redis needed: these specs mock the DB/loader.
-  mcp-server-parity:
-    runs-on: ubuntu-latest
-    timeout-minutes: 15
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v4
-
-      - name: Set up pnpm
-        uses: pnpm/action-setup@v4
-
-      - name: Set up Node
-        uses: actions/setup-node@v4
-        with:
-          node-version: 22
-          cache: pnpm
-
-      - name: Install dependencies
-        run: pnpm install --frozen-lockfile
-
-      # Shared deps first (build/ dirs are gitignored; see test.yml build order).
-      - name: Build editor-ext
-        run: pnpm --filter @docmost/editor-ext build
-
-      - name: Build prosemirror-markdown
-        run: pnpm --filter @docmost/prosemirror-markdown build
-
-      # Build the mcp package so build/ carries a FRESH REGISTRY_STAMP (#447): the
-      # build runs gen-registry-stamp.mjs before tsc, so a build/ vs src/ skew
-      # cannot slip into the tests that exercise the loader's stale-check.
-      - name: Build mcp (regenerates REGISTRY_STAMP)
-        run: pnpm --filter @docmost/mcp build
-
-      # mcp side: the standalone MCP server's own tool-spec / instructions guards.
-      - name: Run mcp tool-spec suite
-        run: pnpm --filter @docmost/mcp test
-
-      # server side: the parity + tier guards that read packages/mcp/src/tool-specs
-      # and assert the in-app AI-chat wiring matches it.
-      - name: Run server tool-spec guard specs
-        run: pnpm --filter server exec jest shared-tool-specs.contract tool-tiers ai-chat-tools.service --runInBand

.gitignore

@@ -19,11 +19,6 @@ packages/prosemirror-markdown/build/
 # markdown convention; the package is private and rebuilt at deploy.
 packages/mcp/build/
 
-# mcp REGISTRY_STAMP codegen output (issue #447). Regenerated into src/ by
-# scripts/gen-registry-stamp.mjs on every `build`/`pretest` (before tsc), so it
-# is a build artifact like build/ — never committed, always fresh.
-packages/mcp/src/registry-stamp.generated.ts
-
 # Logs
 logs
 *.log

AGENTS.md

@@ -248,22 +248,6 @@ pnpm collab:dev              # run the collaboration server process standalone (
 > that order). Reach for it whenever you run a consumer package's checks on their
 > own rather than through the full `pnpm build`.
 
-> **Editing an MCP tool spec requires a rebuild (issue #447).** The running
-> server loads the **compiled** `packages/mcp/build/` of `@docmost/mcp` (via the
-> runtime loader in `apps/server/src/core/ai-chat/tools/docmost-client.loader.ts`),
-> but the parity/tier guard tests read `packages/mcp/src/tool-specs.ts`. So if you
-> edit `tool-specs.ts` (any tool name, description, tier, catalog line, or input
-> schema) **without rebuilding**, `build/` and `src/` silently diverge — the tests
-> stay green while the server serves the OLD tools. To close that gap, the build
-> emits a `REGISTRY_STAMP` (a deterministic hash of the tool-specs content, via
-> `scripts/gen-registry-stamp.mjs` before `tsc`); on dev/test startup the loader
-> recomputes it from `src/` and **refuses to start with a "@docmost/mcp build is
-> stale …" error** on a mismatch (a pure no-op in prod, where only `build/` ships).
-> After editing tool specs, rebuild:
-> ```bash
-> pnpm --filter @docmost/mcp build     # or: pnpm --filter @docmost/mcp watch
-> ```
-
 **Lint** (per package — there is no root lint script):
 ```bash
 pnpm --filter server lint    # eslint --fix on server .ts

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

@@ -1,17 +1,6 @@
 import { AiChatToolsService } from './ai-chat-tools.service';
 import * as loader from './docmost-client.loader';
 import type { DocmostClientLike } from './docmost-client.loader';
-
-// Test-double type for the loopback client. `DocmostClientLike` is now DERIVED
-// from the real `DocmostClient` (issue #446), so its method RETURN types are the
-// concrete client shapes. These stubs deliberately return minimal recording
-// shapes (e.g. `{ ok: true }`), which no longer satisfy those concrete returns —
-// so the doubles are typed with the same method NAMES but loose async returns.
-// Each is still cast to `DocmostClientLike` at the (return-erased) mock site, so
-// the positional-call type-safety on the PRODUCTION client is unaffected.
-type FakeDocmostClient = Partial<
-  Record<keyof DocmostClientLike, (...args: any[]) => Promise<any>>
->;
 // The real zod-agnostic shared tool-spec registry. It has no runtime deps, so
 // importing the TS source directly keeps these mocks honest: the service builds
 // the shared tools from exactly the specs the package ships, not a hand-stub.
@@ -42,7 +31,7 @@ describe('AiChatToolsService deletePage guardrail (H4)', () => {
 
   // Minimal fake DocmostClient: only the write methods the tools touch need to
   // exist; deletePage records its args. No network, no ESM import.
-  const fakeClient: FakeDocmostClient = {
+  const fakeClient: Partial<DocmostClientLike> = {
     deletePage: (...args: unknown[]) => {
       deletePageCalls.push(args);
       return Promise.resolve({ success: true });
@@ -171,7 +160,7 @@ describe('AiChatToolsService deletePage guardrail (H4)', () => {
 describe('AiChatToolsService expanded toolset guardrails', () => {
   // No client method is invoked here — every assertion is on tool presence /
   // input schema — so an empty fake client is sufficient.
-  const fakeClient: FakeDocmostClient = {};
+  const fakeClient: Partial<DocmostClientLike> = {};
 
   const tokenServiceStub = {
     generateAccessToken: jest.fn().mockResolvedValue('access-token'),
@@ -276,7 +265,7 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => {
   const insertNodeCalls: unknown[][] = [];
   const updatePageJsonCalls: unknown[][] = [];
 
-  const fakeClient: FakeDocmostClient = {
+  const fakeClient: Partial<DocmostClientLike> = {
     patchNode: (...args: unknown[]) => {
       patchNodeCalls.push(args);
       return Promise.resolve({ ok: true });
@@ -450,7 +439,7 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => {
  * getOutline) are exercised here end-to-end through forUser().
  */
 describe('AiChatToolsService model-friendly input validation (#190)', () => {
-  const fakeClient: FakeDocmostClient = {};
+  const fakeClient: Partial<DocmostClientLike> = {};
   const tokenServiceStub = {
     generateAccessToken: jest.fn().mockResolvedValue('access-token'),
     generateCollabToken: jest.fn().mockResolvedValue('collab-token'),
@@ -568,7 +557,7 @@ describe('AiChatToolsService #294 changed execute wirings', () => {
     tableDeleteRow: [],
     tableUpdateCell: [],
   };
-  const fakeClient: FakeDocmostClient = {
+  const fakeClient: Partial<DocmostClientLike> = {
     movePage: (...args: unknown[]) => {
       calls.movePage.push(args);
       return Promise.resolve({ success: true });
@@ -677,7 +666,7 @@ describe('AiChatToolsService #410 footnote + image tools', () => {
     insertImage: [],
     replaceImage: [],
   };
-  const fakeClient: FakeDocmostClient = {
+  const fakeClient: Partial<DocmostClientLike> = {
     insertFootnote: (...args: unknown[]) => {
       calls.insertFootnote.push(args);
       return Promise.resolve({ success: true, footnoteId: 'fn1', reused: false });

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

@@ -26,100 +26,6 @@ import {
   type ToolCatalogEntry,
 } from './tool-tiers';
 
-/**
- * Compile-time contract (issue #446): the in-app tool `execute` closures below
- * call the loopback `DocmostClient` POSITIONALLY (e.g.
- * `client.drawioGet(pageId, node, format ?? 'xml')`). Those closures receive an
- * AI-SDK-erased (`any`) input, so a positional call inside them is NOT checked
- * against the real signature — a parameter reorder/type-change in
- * `packages/mcp/src/client.ts` would otherwise reach production as a runtime
- * "wrong argument" tool failure with zero compile signal (the restored #294
- * debt). This never-called function reproduces every positional call with
- * correctly-typed placeholder arguments against the DERIVED `DocmostClientLike`
- * (a `Pick` of the real `DocmostClient`), so any such reorder/rename becomes a
- * SERVER COMPILE ERROR here. It emits nothing (types only) and is never invoked;
- * keep each call in lockstep with the matching `execute` body below.
- */
-// eslint-disable-next-line @typescript-eslint/no-unused-vars
-function __assertClientCallContract(client: DocmostClientLike): void {
-  // Placeholders standing in for the AI-SDK-erased execute inputs. Their types
-  // are deliberately concrete so the positional calls are checked end-to-end.
-  const s = '' as string;
-  const n = 0 as number;
-  const node: unknown = null;
-  const edits: Array<{ find: string; replace: string; replaceAll?: boolean }> =
-    [];
-  const cells: string[] = [];
-  const align = undefined as 'left' | 'center' | 'right' | undefined;
-
-  // --- read ---
-  void client.search(s, undefined, n);
-  void client.getPage(s);
-  void client.getPageRaw(s);
-  void client.getWorkspace();
-  void client.getSpaces();
-  void client.listPages(s, n, true);
-  void client.listSidebarPages(s, s);
-  void client.getOutline(s);
-  void client.getPageJson(s);
-  void client.getNode(s, s);
-  void client.searchInPage(s, s, {
-    regex: true,
-    caseSensitive: true,
-    limit: n,
-  });
-  void client.getTable(s, s);
-  void client.listComments(s, true);
-  void client.getComment(s);
-  void client.checkNewComments(s, s, s);
-  void client.listShares();
-  void client.listPageHistory(s, s);
-  void client.getPageHistory(s);
-  void client.diffPageVersions(s, s, s);
-  void client.exportPageMarkdown(s);
-  // --- write (page) ---
-  void client.createPage(s, s, s, s);
-  void client.updatePage(s, s, s);
-  void client.renamePage(s, s);
-  void client.movePage(s, s, s);
-  void client.deletePage(s);
-  void client.editPageText(s, edits);
-  void client.patchNode(s, s, node);
-  void client.insertNode(s, node, {
-    position: 'append',
-    anchorNodeId: s,
-    anchorText: s,
-  });
-  void client.deleteNode(s, s);
-  void client.updatePageJson(s, node, s);
-  void client.tableInsertRow(s, s, cells, n);
-  void client.tableDeleteRow(s, s, n);
-  void client.tableUpdateCell(s, s, n, n, s);
-  void client.copyPageContent(s, s);
-  void client.importPageMarkdown(s, s);
-  void client.sharePage(s, true);
-  void client.unsharePage(s);
-  void client.restorePageVersion(s);
-  void client.transformPage(s, s, { dryRun: true });
-  void client.stashPage(s);
-  // --- write (image / footnote), in-app since #410 ---
-  void client.insertFootnote(s, s, s);
-  void client.insertImage(s, s, {
-    align,
-    alt: s,
-    replaceText: s,
-    afterText: s,
-  });
-  void client.replaceImage(s, s, s, { align, alt: s });
-  // --- draw.io diagrams (#423) ---
-  void client.drawioGet(s, s, 'xml');
-  void client.drawioCreate(s, { position: 'append', anchorNodeId: s }, s, s);
-  void client.drawioUpdate(s, s, s, s);
-  // --- write (comment) ---
-  void client.createComment(s, s, 'inline', s, s, s);
-  void client.resolveComment(s, true);
-}
-
 /**
  * Per-user, per-request adapter that exposes Docmost READ operations to the
  * agent as AI SDK tools (STAGE A = read only).
@@ -292,17 +198,6 @@ export class AiChatToolsService {
         execute,
       });
 
-    // The in-app toolset. It starts with the tools kept INLINE here for a
-    // documented per-layer reason: an intentional behaviour/schema divergence from
-    // the standalone MCP surface (searchPages' hybrid RRF, updatePageContent's
-    // Markdown write, transformPage's guardrailed shorter schema), a
-    // snake_case/camelCase naming clash the shared registry forbids (getTable vs
-    // the MCP `table_get`), per-request state the registry loop cannot provide
-    // (getCurrentPage reads the resolved openedPage; searchPages closes over the
-    // per-request user/embedding deps), or a tool with no MCP twin
-    // (listSidebarPages/getComment/getPageHistory). Every SHARED tool is then added
-    // by the registry loop below (see it), so there is exactly one arg-mapping per
-    // shared tool and it can never drift from the MCP host again (#445).
     const tools: Record<string, Tool> = {
       // INTENTIONAL per-transport divergence (not in the shared registry): this
       // in-app search runs a semantic + keyword hybrid (RRF) with in-process
@@ -437,8 +332,44 @@ export class AiChatToolsService {
         execute: async () => resolveCurrentPageResult(openedPage),
       }),
 
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      // The execute body keeps this layer's { title, markdown } projection.
+      getPage: sharedTool(sharedToolSpecs.getPage, async ({ pageId }) => {
+        // getPage(pageId) -> { data: filterPage(page, markdown), success }.
+        const result = await client.getPage(pageId);
+        const data = (result?.data ?? {}) as {
+          title?: string;
+          content?: string;
+        };
+        return {
+          title: data.title ?? '',
+          markdown: typeof data.content === 'string' ? data.content : '',
+        };
+      }),
+
       // --- WRITE tools (all reversible — history/trash; §6.5 / D3) ---
 
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      createPage: sharedTool(
+        sharedToolSpecs.createPage,
+        async ({ title, content, spaceId, parentPageId }) => {
+          // createPage(title, content, spaceId, parentPageId?) ->
+          // { data: filterPage(page, markdown), success }.
+          const result = await client.createPage(
+            title,
+            content ?? '',
+            spaceId,
+            parentPageId,
+          );
+          const data = (result?.data ?? {}) as {
+            id?: string;
+            slugId?: string;
+            title?: string;
+          };
+          return { id: data.id ?? data.slugId, title: data.title ?? title };
+        },
+      ),
+
       updatePageContent: tool({
         description:
           "Replace a page's body with new Markdown content (and optionally its " +
@@ -461,6 +392,121 @@ export class AiChatToolsService {
         },
       }),
 
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      renamePage: sharedTool(
+        sharedToolSpecs.renamePage,
+        async ({ pageId, title }) => {
+          // renamePage(pageId, title) -> { success, pageId, title }.
+          await client.renamePage(pageId, title);
+          return { pageId, title };
+        },
+      ),
+
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      // The shared schema adds the optional `position` field this layer lacked
+      // before; the execute now forwards it (the client already accepted it).
+      movePage: sharedTool(
+        sharedToolSpecs.movePage,
+        async ({ pageId, parentPageId, position }) => {
+          // movePage(pageId, parentPageId, position?) -> raw move response.
+          await client.movePage(pageId, parentPageId ?? null, position);
+          return { pageId, parentPageId: parentPageId ?? null, moved: true };
+        },
+      ),
+
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      // GUARDRAIL (§14 H4) preserved: the shared schema exposes ONLY pageId, so
+      // permanentlyDelete/forceDelete are never part of the input and can never
+      // be forwarded — the agent physically cannot permanently delete a page.
+      deletePage: sharedTool(sharedToolSpecs.deletePage, async ({ pageId }) => {
+        // deletePage(pageId) hits POST /pages/delete with { pageId } only,
+        // which is the soft-delete (trash) path on the server.
+        await client.deletePage(pageId);
+        return { pageId, trashed: true };
+      }),
+
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      // This layer keeps only its own execute-side guards (require a selection
+      // for a top-level comment; reject suggestedText on a reply / without a
+      // selection) — the schema+description are shared.
+      createComment: sharedTool(
+        sharedToolSpecs.createComment,
+        async ({
+          pageId,
+          content,
+          selection,
+          parentCommentId,
+          suggestedText,
+        }) => {
+          // createComment(pageId, content, type, selection?, parentCommentId?,
+          // suggestedText?). Top-level comments are inline and must carry a
+          // selection to anchor on; replies inherit the parent's anchor (no
+          // selection). Throwing here surfaces a tool error to the model (Vercel
+          // `ai` SDK) so the agent retries with a better selection — do not
+          // catch/suppress it.
+          if (!parentCommentId && (!selection || !selection.trim())) {
+            throw new Error(
+              "createComment requires a 'selection' (exact text to anchor on) for a new top-level comment.",
+            );
+          }
+          if (suggestedText !== undefined) {
+            if (parentCommentId) {
+              throw new Error(
+                "createComment: 'suggestedText' cannot be attached to a reply; it applies only to a top-level inline comment.",
+              );
+            }
+            if (!selection || !selection.trim()) {
+              throw new Error(
+                "createComment: 'suggestedText' requires a 'selection' to anchor and rewrite.",
+              );
+            }
+          }
+          const result = await client.createComment(
+            pageId,
+            content,
+            'inline',
+            selection,
+            parentCommentId,
+            suggestedText,
+          );
+          const data = (result?.data ?? {}) as { id?: string };
+          return { commentId: data.id, pageId };
+        },
+      ),
+
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      resolveComment: sharedTool(
+        sharedToolSpecs.resolveComment,
+        async ({ commentId, resolved }) => {
+          // resolveComment(commentId, resolved) -> { success, commentId, resolved }.
+          await client.resolveComment(commentId, resolved);
+          return { commentId, resolved };
+        },
+      ),
+
+      // --- READ tools (added) ---
+
+      getWorkspace: sharedTool(
+        sharedToolSpecs.getWorkspace,
+        async () => await client.getWorkspace(),
+      ),
+
+      listSpaces: sharedTool(
+        sharedToolSpecs.listSpaces,
+        async () => await client.getSpaces(),
+      ),
+
+      // INTENTIONAL per-transport divergence (not shared): keeps the `tree:true`
+      // hierarchy mode but is worded for the in-app agent; the standalone MCP
+      // `list_pages` carries its own wording. Kept per-layer so each side tunes
+      // its own guidance.
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      listPages: sharedTool(
+        sharedToolSpecs.listPages,
+        async ({ spaceId, limit, tree }) =>
+          await client.listPages(spaceId, limit, tree),
+      ),
+
       listSidebarPages: tool({
         description:
           'List sidebar pages for a space. With no pageId, returns the ' +
@@ -479,6 +525,31 @@ export class AiChatToolsService {
           await client.listSidebarPages(spaceId, pageId),
       }),
 
+      getOutline: sharedTool(
+        sharedToolSpecs.getOutline,
+        async ({ pageId }) => await client.getOutline(pageId),
+      ),
+
+      getPageJson: sharedTool(
+        sharedToolSpecs.getPageJson,
+        async ({ pageId }) => await client.getPageJson(pageId),
+      ),
+
+      getNode: sharedTool(
+        sharedToolSpecs.getNode,
+        async ({ pageId, nodeId }) => await client.getNode(pageId, nodeId),
+      ),
+
+      searchInPage: sharedTool(
+        sharedToolSpecs.searchInPage,
+        async ({ pageId, query, regex, caseSensitive, limit }) =>
+          await client.searchInPage(pageId, query, {
+            regex,
+            caseSensitive,
+            limit,
+          }),
+      ),
+
       // NOT shared (kept inline): the MCP tool name `table_get` is noun-first
       // while this key is `getTable` (verb-first), breaking the
       // snake_case(inAppKey) convention the shared registry enforces. Its
@@ -501,6 +572,13 @@ export class AiChatToolsService {
           await client.getTable(pageId, table),
       }),
 
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      listComments: sharedTool(
+        sharedToolSpecs.listComments,
+        async ({ pageId, includeResolved }) =>
+          await client.listComments(pageId, includeResolved),
+      ),
+
       getComment: tool({
         description: 'Fetch a single comment by id (content as Markdown).',
         inputSchema: modelFriendlyInput({
@@ -509,6 +587,24 @@ export class AiChatToolsService {
         execute: async ({ commentId }) => await client.getComment(commentId),
       }),
 
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      checkNewComments: sharedTool(
+        sharedToolSpecs.checkNewComments,
+        async ({ spaceId, since, parentPageId }) =>
+          await client.checkNewComments(spaceId, since, parentPageId),
+      ),
+
+      listShares: sharedTool(
+        sharedToolSpecs.listShares,
+        async () => await client.listShares(),
+      ),
+
+      listPageHistory: sharedTool(
+        sharedToolSpecs.listPageHistory,
+        async ({ pageId, cursor }) =>
+          await client.listPageHistory(pageId, cursor),
+      ),
+
       getPageHistory: tool({
         description:
           'Fetch a single page-history version including its lossless ' +
@@ -520,8 +616,203 @@ export class AiChatToolsService {
           await client.getPageHistory(historyId),
       }),
 
+      diffPageVersions: sharedTool(
+        sharedToolSpecs.diffPageVersions,
+        async ({ pageId, from, to }) =>
+          await client.diffPageVersions(pageId, from, to),
+      ),
+
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      exportPageMarkdown: sharedTool(
+        sharedToolSpecs.exportPageMarkdown,
+        async ({ pageId }) => {
+          const markdown = await client.exportPageMarkdown(pageId);
+          return { markdown };
+        },
+      ),
+
       // --- WRITE tools (added; reversible via page history/trash) ---
 
+      editPageText: sharedTool(
+        sharedToolSpecs.editPageText,
+        async ({ pageId, edits }) => await client.editPageText(pageId, edits),
+      ),
+
+      // Returns ONLY the short link object — never the document body — so a
+      // large page can be handed to an external consumer without bloating
+      // context.
+      stashPage: sharedTool(
+        sharedToolSpecs.stashPage,
+        async ({ pageId }) => await client.stashPage(pageId),
+      ),
+
+      // Schema + description from the shared registry (identical across both
+      // transports). The execute body keeps its OWN parseNodeArg normalization:
+      // the model sometimes serializes the node as a JSON string, and we parse it
+      // before the client's typeof-object guard rejects it (parity with the
+      // standalone MCP server, index.ts patch_node).
+      patchNode: sharedTool(
+        sharedToolSpecs.patchNode,
+        async ({ pageId, nodeId, node }) => {
+          const parsedNode = parseNodeArg(node);
+          return await client.patchNode(pageId, nodeId, parsedNode);
+        },
+      ),
+
+      // Shared registry schema + description; execute retains parseNodeArg on the
+      // incoming node (parity with the standalone MCP server, index.ts
+      // insert_node).
+      insertNode: sharedTool(
+        sharedToolSpecs.insertNode,
+        async ({ pageId, node, position, anchorNodeId, anchorText }) => {
+          const parsedNode = parseNodeArg(node);
+          return await client.insertNode(pageId, parsedNode, {
+            position,
+            anchorNodeId,
+            anchorText,
+          });
+        },
+      ),
+
+      deleteNode: sharedTool(
+        sharedToolSpecs.deleteNode,
+        async ({ pageId, nodeId }) => await client.deleteNode(pageId, nodeId),
+      ),
+
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      // The execute body keeps this layer's content normalization (parity with
+      // the standalone MCP server, index.ts update_page_json).
+      updatePageJson: sharedTool(
+        sharedToolSpecs.updatePageJson,
+        async ({ pageId, content, title }) => {
+          // undefined/null pass through as undefined (title-only / no-op); any
+          // string is JSON.parsed (so an empty string "" throws, matching the
+          // MCP server); an object is passed through unchanged.
+          let doc;
+          if (content === undefined || content === null) {
+            doc = undefined;
+          } else {
+            // String -> JSON.parse (throwing on invalid); object passes through.
+            doc = parseNodeArg(content, 'content was a string but not valid JSON');
+          }
+          return await client.updatePageJson(pageId, doc, title);
+        },
+      ),
+
+      // Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#410).
+      // Promoted from MCP-only so the in-app agent can attach a REAL footnote to
+      // already-written text instead of leaving a literal `^[...]` string.
+      insertFootnote: sharedTool(
+        sharedToolSpecs.insertFootnote,
+        async ({ pageId, anchorText, text }) =>
+          await client.insertFootnote(pageId, anchorText, text),
+      ),
+
+      // Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#410).
+      // The schema field is `imageUrl`; the client method takes it positionally.
+      insertImage: sharedTool(
+        sharedToolSpecs.insertImage,
+        async ({ pageId, imageUrl, align, alt, replaceText, afterText }) =>
+          await client.insertImage(pageId, imageUrl, {
+            align,
+            alt,
+            replaceText,
+            afterText,
+          }),
+      ),
+
+      // Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#410).
+      replaceImage: sharedTool(
+        sharedToolSpecs.replaceImage,
+        async ({ pageId, attachmentId, imageUrl, align, alt }) =>
+          await client.replaceImage(pageId, attachmentId, imageUrl, {
+            align,
+            alt,
+          }),
+      ),
+
+      // Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#423).
+      // meta.hash in the result is the baseHash drawioUpdate requires.
+      drawioGet: sharedTool(
+        sharedToolSpecs.drawioGet,
+        async ({ pageId, node, format }) =>
+          await client.drawioGet(pageId, node, format ?? 'xml'),
+      ),
+
+      // Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#423).
+      // The flat schema fields are regrouped into the client's `where` object.
+      drawioCreate: sharedTool(
+        sharedToolSpecs.drawioCreate,
+        async ({ pageId, xml, position, anchorNodeId, anchorText, title }) =>
+          await client.drawioCreate(
+            pageId,
+            { position, anchorNodeId, anchorText },
+            xml,
+            title,
+          ),
+      ),
+
+      // Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#423).
+      // baseHash is the optimistic lock: mismatch => structured conflict error.
+      drawioUpdate: sharedTool(
+        sharedToolSpecs.drawioUpdate,
+        async ({ pageId, node, xml, baseHash }) =>
+          await client.drawioUpdate(pageId, node, xml, baseHash),
+      ),
+
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      // The table reference parameter was unified to `table` (was `tableRef`).
+      tableInsertRow: sharedTool(
+        sharedToolSpecs.tableInsertRow,
+        async ({ pageId, table, cells, index }) =>
+          await client.tableInsertRow(pageId, table, cells, index),
+      ),
+
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      tableDeleteRow: sharedTool(
+        sharedToolSpecs.tableDeleteRow,
+        async ({ pageId, table, index }) =>
+          await client.tableDeleteRow(pageId, table, index),
+      ),
+
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      tableUpdateCell: sharedTool(
+        sharedToolSpecs.tableUpdateCell,
+        async ({ pageId, table, row, col, text }) =>
+          await client.tableUpdateCell(pageId, table, row, col, text),
+      ),
+
+      copyPageContent: sharedTool(
+        sharedToolSpecs.copyPageContent,
+        async ({ sourcePageId, targetPageId }) =>
+          await client.copyPageContent(sourcePageId, targetPageId),
+      ),
+
+      importPageMarkdown: sharedTool(
+        sharedToolSpecs.importPageMarkdown,
+        async ({ pageId, markdown }) =>
+          await client.importPageMarkdown(pageId, markdown),
+      ),
+
+      // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
+      // Both layers already carried the security-confirmation framing, so there
+      // was no real divergence to preserve — only wording drift.
+      sharePage: sharedTool(
+        sharedToolSpecs.sharePage,
+        async ({ pageId, searchIndexing }) =>
+          await client.sharePage(pageId, searchIndexing),
+      ),
+
+      unsharePage: sharedTool(
+        sharedToolSpecs.unsharePage,
+        async ({ pageId }) => await client.unsharePage(pageId),
+      ),
+
+      restorePageVersion: sharedTool(
+        sharedToolSpecs.restorePageVersion,
+        async ({ historyId }) => await client.restorePageVersion(historyId),
+      ),
+
       // INTENTIONAL per-transport divergence (not shared): deliberately omits the
       // `deleteComments` schema field (comment-deletion guardrail) and carries a
       // much shorter description; the standalone MCP `docmost_transform` exposes
@@ -550,29 +841,6 @@ export class AiChatToolsService {
       }),
     };
 
-    // Add EVERY shared tool from the zod-agnostic registry in one loop (#445).
-    // The spec owns the canonical arg->client mapping; this host only decides
-    // 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);
-    //   - 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
-    //     the MCP host's before it wraps it as JSON).
-    // The execute receives the AI-SDK-validated, type-erased input; the spec reads
-    // the same fields its buildShape declares. This is the SINGLE place the in-app
-    // arg mapping lives — it can no longer silently drift from the MCP host.
-    for (const spec of Object.values(sharedToolSpecs)) {
-      if (spec.mcpOnly) continue;
-      const run = spec.inAppExecute ?? spec.execute;
-      if (!run) continue; // defensive: a shared spec always carries one of them.
-      tools[spec.inAppKey] = sharedTool(
-        spec,
-        (async (args) =>
-          run(client, args as Record<string, unknown>)) as Tool['execute'],
-      );
-    }
-
     // 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

@@ -7,16 +7,6 @@ import type {
   DocmostClientLike,
   CommentSignalTrackerLike,
 } from './docmost-client.loader';
-
-// Test-double type for the loopback client. `DocmostClientLike` is now DERIVED
-// from the real `DocmostClient` (issue #446), so its method RETURN types are the
-// concrete client shapes. These probe stubs deliberately return minimal shapes
-// (e.g. `getPageRaw` yielding only `{ title }`), so the doubles use the same
-// method NAMES but loose async returns; each is cast to `DocmostClientLike` at
-// the (return-erased) mock site, leaving production positional-call safety intact.
-type FakeDocmostClient = Partial<
-  Record<keyof DocmostClientLike, (...args: any[]) => Promise<any>>
->;
 import { SHARED_TOOL_SPECS } from '../../../../../../packages/mcp/src/tool-specs';
 // The REAL shared tracker factory, imported from source (same cross-boundary
 // approach the tool-specs spec uses) so the in-app wiring is exercised against
@@ -278,7 +268,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
   // seeded at forUser time).
   const future = new Date(Date.now() + 3_600_000).toISOString();
 
-  function buildService(fakeClient: FakeDocmostClient) {
+  function buildService(fakeClient: Partial<DocmostClientLike>) {
     jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue({
       DocmostClient: function () {
         return fakeClient as DocmostClientLike;
@@ -327,7 +317,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
   afterEach(() => jest.restoreAllMocks());
 
   it('emits the signal (model-only) on a non-comment tool when a new comment exists', async () => {
-    const fakeClient: FakeDocmostClient = {
+    const fakeClient: Partial<DocmostClientLike> = {
       getPage: async () => ({
         data: { title: 'Иранские языки', content: 'body' },
         success: true,
@@ -352,7 +342,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
   });
 
   it('does NOT add the signal to the listComments tool itself (tautological)', async () => {
-    const fakeClient: FakeDocmostClient = {
+    const fakeClient: Partial<DocmostClientLike> = {
       listComments: async () => ({
         items: [{ createdAt: future }],
         resolvedThreadsHidden: 0,
@@ -366,7 +356,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
   });
 
   it('no new comments => tool output is byte-identical AND the model sees no signal', async () => {
-    const fakeClient: FakeDocmostClient = {
+    const fakeClient: Partial<DocmostClientLike> = {
       getPage: async () => ({
         data: { title: 'T', content: 'body' },
         success: true,
@@ -382,7 +372,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
   });
 
   it('injection-safety: a malicious page title cannot forge a second signal', async () => {
-    const fakeClient: FakeDocmostClient = {
+    const fakeClient: Partial<DocmostClientLike> = {
       getPage: async () => ({
         data: { title: 'body-title', content: 'body' },
         success: true,

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

@@ -1,173 +0,0 @@
-import { createHash } from 'node:crypto';
-import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
-import { tmpdir } from 'node:os';
-import { join } from 'node:path';
-
-import { computeSrcRegistryStamp } from './docmost-client.loader';
-
-// The exact message the loader throws on a build/src skew (issue #447). Kept as a
-// literal here so a reworded prod message reddens this test (the message is a
-// developer-facing contract: it tells them how to fix it).
-const STALE_BUILD_MESSAGE =
-  '@docmost/mcp build is stale (tool-specs changed since last build) — run: pnpm --filter @docmost/mcp build';
-
-// Replica of the loader's inline stale-check predicate + throw from
-// `loadDocmostMcp`. That guard is not independently exported (it lives inside the
-// dynamic-import IIFE, wired to a fixed `require.resolve('@docmost/mcp')`), so we
-// exercise the exact same three-condition logic against a stamp produced by the
-// REAL `computeSrcRegistryStamp`. This documents and locks the throw/no-throw
-// behaviour; if the prod predicate changes, this replica must change with it.
-function assertStaleGuard(
-  srcStamp: string | null,
-  registryStamp: string | undefined,
-): void {
-  if (
-    srcStamp !== null &&
-    typeof registryStamp === 'string' &&
-    srcStamp !== registryStamp
-  ) {
-    throw new Error(STALE_BUILD_MESSAGE);
-  }
-}
-
-// Build a throwaway `<pkg>/build/index.js` + optional `<pkg>/src/tool-specs.ts`
-// layout so `computeSrcRegistryStamp(<pkg>/build/index.js)` resolves src the same
-// way the loader does (dirname(dirname(entry))/src/tool-specs.ts).
-function makeFakePackage(toolSpecsSource: string | null): {
-  entry: string;
-  cleanup: () => void;
-} {
-  const root = mkdtempSync(join(tmpdir(), 'mcp-stamp-'));
-  const buildDir = join(root, 'build');
-  mkdirSync(buildDir, { recursive: true });
-  const entry = join(buildDir, 'index.js');
-  writeFileSync(entry, '// fake @docmost/mcp build entry\n', 'utf8');
-  if (toolSpecsSource !== null) {
-    const srcDir = join(root, 'src');
-    mkdirSync(srcDir, { recursive: true });
-    writeFileSync(join(srcDir, 'tool-specs.ts'), toolSpecsSource, 'utf8');
-  }
-  return { entry, cleanup: () => rmSync(root, { recursive: true, force: true }) };
-}
-
-describe('computeSrcRegistryStamp (#447 stale-build guard)', () => {
-  it('returns null when src/tool-specs.ts is absent (prod no-op path)', () => {
-    // A prod image ships only build/, no src/ — the guard must be a silent no-op.
-    const { entry, cleanup } = makeFakePackage(null);
-    try {
-      expect(computeSrcRegistryStamp(entry)).toBeNull();
-    } finally {
-      cleanup();
-    }
-  });
-
-  it('returns null for a bogus package entry (swallowed error path)', () => {
-    // A resolution/read hiccup must NEVER break startup — it resolves to null.
-    expect(
-      computeSrcRegistryStamp('/no/such/pkg/build/index.js'),
-    ).toBeNull();
-  });
-
-  it('computes a 64-char sha256 hex when src/tool-specs.ts exists', () => {
-    const { entry, cleanup } = makeFakePackage('export const specs = 1;\n');
-    try {
-      const stamp = computeSrcRegistryStamp(entry);
-      expect(stamp).toMatch(/^[0-9a-f]{64}$/);
-    } finally {
-      cleanup();
-    }
-  });
-
-  it('normalizes CRLF->LF and strips a single trailing newline', () => {
-    // A CRLF+trailing-newline variant of the same content hashes identically to
-    // the bare-LF form — the guard must not fire on a checkout-style difference.
-    const bare = makeFakePackage('alpha\nbeta');
-    const crlfTrailing = makeFakePackage('alpha\r\nbeta\r\n');
-    try {
-      expect(computeSrcRegistryStamp(crlfTrailing.entry)).toBe(
-        computeSrcRegistryStamp(bare.entry),
-      );
-    } finally {
-      bare.cleanup();
-      crlfTrailing.cleanup();
-    }
-  });
-
-  // CROSS-IMPL EQUALITY (covers reviewer suggestion 2). The SAME fixed input and
-  // EXPECTED hash are asserted in the mcp-side node test
-  // (packages/mcp/test/unit/registry-stamp.test.mjs) against the codegen's
-  // `computeRegistryStamp`. Asserting the SAME pair here against the loader's
-  // `computeSrcRegistryStamp` proves both implementations normalize+hash
-  // identically; a divergence in EITHER side reddens one of the two tests.
-  it('matches the documented cross-impl hash for a fixed input', () => {
-    const FIXED_INPUT = 'line1\r\nline2\n';
-    const EXPECTED =
-      '683376e290829b482c2655745caffa7a1dccfa10afaa62dac2b42dd6c68d0f83';
-    const { entry, cleanup } = makeFakePackage(FIXED_INPUT);
-    try {
-      expect(computeSrcRegistryStamp(entry)).toBe(EXPECTED);
-    } finally {
-      cleanup();
-    }
-  });
-
-  it('the documented EXPECTED is the normalize+sha256 of the fixed input', () => {
-    // Proves EXPECTED is not a magic constant but the documented computation.
-    const FIXED_INPUT = 'line1\r\nline2\n';
-    const normalized = FIXED_INPUT.replace(/\r\n/g, '\n').replace(/\n$/, '');
-    const expected = createHash('sha256')
-      .update(normalized, 'utf8')
-      .digest('hex');
-    const { entry, cleanup } = makeFakePackage(FIXED_INPUT);
-    try {
-      expect(computeSrcRegistryStamp(entry)).toBe(expected);
-    } finally {
-      cleanup();
-    }
-  });
-});
-
-describe('loadDocmostMcp stale-check predicate (#447)', () => {
-  it('THROWS the exact stale message when src stamp != built REGISTRY_STAMP', () => {
-    const { entry, cleanup } = makeFakePackage('export const specs = 1;\n');
-    try {
-      const srcStamp = computeSrcRegistryStamp(entry);
-      expect(srcStamp).not.toBeNull();
-      // Simulate a stale build: build/ carries a DIFFERENT stamp than src.
-      expect(() => assertStaleGuard(srcStamp, 'a'.repeat(64))).toThrow(
-        STALE_BUILD_MESSAGE,
-      );
-    } finally {
-      cleanup();
-    }
-  });
-
-  it('does NOT throw when src stamp equals the built REGISTRY_STAMP', () => {
-    const { entry, cleanup } = makeFakePackage('export const specs = 1;\n');
-    try {
-      const srcStamp = computeSrcRegistryStamp(entry);
-      // Fresh build: build/ stamp == src stamp -> guard is a no-op.
-      expect(() => assertStaleGuard(srcStamp, srcStamp as string)).not.toThrow();
-    } finally {
-      cleanup();
-    }
-  });
-
-  it('does NOT throw when src is absent (prod: srcStamp === null)', () => {
-    // Even against a present-but-mismatched REGISTRY_STAMP, a null src stamp
-    // (prod image with build/ only) must skip the check entirely.
-    expect(() => assertStaleGuard(null, 'a'.repeat(64))).not.toThrow();
-  });
-
-  it('does NOT throw when REGISTRY_STAMP is absent (pre-#447 build)', () => {
-    // An older @docmost/mcp build has no REGISTRY_STAMP export; the guard must be
-    // a no-op so an out-of-date build never wrongly blocks startup.
-    const { entry, cleanup } = makeFakePackage('export const specs = 1;\n');
-    try {
-      const srcStamp = computeSrcRegistryStamp(entry);
-      expect(() => assertStaleGuard(srcStamp, undefined)).not.toThrow();
-    } finally {
-      cleanup();
-    }
-  });
-});

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

@@ -1,93 +1,264 @@
-import { createHash } from 'node:crypto';
-import { existsSync, readFileSync } from 'node:fs';
-import { dirname, join } from 'node:path';
 import { pathToFileURL } from 'node:url';
-import type { DocmostClient, SharedToolSpec } from '@docmost/mcp';
-
-// Re-export SharedToolSpec so downstream server modules keep a single import
-// path (they import it from this loader). The shape is DERIVED from the package
-// entry, not re-declared here — see the import above (issue #446).
-export type { SharedToolSpec } from '@docmost/mcp';
 
 /**
- * The exact set of `DocmostClient` methods the per-user in-app tool adapter
- * consumes. This is the AUTHORITATIVE list of the client surface the server
- * depends on; the adapter calls these methods POSITIONALLY, so this set is what
- * the derived type below type-checks against the real class (issue #446).
+ * Minimal structural type for the `DocmostClient` class we consume from the
+ * ESM-only `@docmost/mcp` package. We only need the constructor + the read/write
+ * methods used by the per-user tool adapter; the full client surface lives in
+ * `packages/mcp/src/client.ts`. Signatures here mirror that file exactly.
+ *
+ * DRIFT GUARD: the method NAMES below are runtime-checked against the real
+ * `DocmostClient` by `packages/mcp/test/unit/client-host-contract.test.mjs`
+ * (which can import the ESM class directly). If you rename/remove a method here
+ * or in client.ts, that test fails — so a stale mirror cannot silently ship a
+ * runtime "x is not a function" into an agent tool call. Keep the two in sync.
+ *
+ * STAGED PLAN — full derivation `DocmostClientLike = <real DocmostClient type>`
+ * (issue #193, layer 3) is intentionally NOT done; it stays a hand-mirror for
+ * now because of two verified blockers across the ESM(mcp)/CJS(server) boundary:
+ *   1. `@docmost/mcp` emits NO declaration files (its tsconfig has no
+ *      `declaration`, package.json has no `types`/types-export) and the server
+ *      tsconfig has no path mapping for it — the server only loads it via the
+ *      runtime `import()` trick below, so there is no type to import today.
+ *   2. The real client methods have inferred, CONCRETE return types; the in-app
+ *      tool adapter reads results through loose `Record<string,unknown>` returns
+ *      + `as` casts (e.g. `(result?.data ?? {}) as { title?: string }`).
+ *      Deriving the exact type would make those casts non-overlapping ("may be a
+ *      mistake") and break the build, and `Partial<DocmostClientLike>` test stubs
+ *      would have to satisfy the full concrete surface.
+ * To do it safely later (incrementally): (a) turn on `declaration: true` in
+ * packages/mcp/tsconfig.json + add a `types` export condition and commit the
+ * emitted `.d.ts`; (b) `import type { DocmostClient } from '@docmost/mcp'` here
+ * and replace this interface with a `Pick<DocmostClient, ...>` of the consumed
+ * methods; (c) audit every `as` cast in ai-chat-tools.service.ts against the now
+ * concrete return types (double-cast through `unknown` only where genuinely
+ * needed); (d) keep the runtime guard test as a belt-and-braces check. Until
+ * then the guard test above is the cheap, behaviour-neutral protection.
  */
-type DocmostClientMethod =
+export interface DocmostClientLike {
   // --- read ---
-  | 'search'
-  | 'getPage'
-  | 'getPageRaw'
-  | 'getWorkspace'
-  | 'getSpaces'
-  | 'listPages'
-  | 'listSidebarPages'
-  | 'getOutline'
-  | 'getPageJson'
-  | 'getNode'
-  | 'searchInPage'
-  | 'getTable'
-  | 'listComments'
-  | 'getComment'
-  | 'checkNewComments'
-  | 'listShares'
-  | 'listPageHistory'
-  | 'getPageHistory'
-  | 'diffPageVersions'
-  | 'exportPageMarkdown'
+  search(
+    query: string,
+    spaceId?: string,
+    limit?: number,
+  ): Promise<{ items: unknown[]; success: boolean }>;
+  getPage(
+    pageId: string,
+  ): Promise<{ data: Record<string, unknown>; success: boolean }>;
+  // Light raw page info (`/pages/info`): title + slugId + ProseMirror content,
+  // WITHOUT the Markdown render / subpage expansion getPage does. Used by the
+  // comment-signal probe to read just the page title on a hit.
+  getPageRaw(pageId: string): Promise<Record<string, unknown> | null>;
+  getWorkspace(): Promise<{ data: Record<string, unknown>; success: boolean }>;
+  getSpaces(): Promise<unknown[]>;
+  listPages(
+    spaceId?: string,
+    limit?: number,
+    tree?: boolean,
+  ): Promise<unknown[]>;
+  listSidebarPages(spaceId: string, pageId?: string): Promise<unknown[]>;
+  getOutline(pageId: string): Promise<Record<string, unknown>>;
+  getPageJson(pageId: string): Promise<Record<string, unknown>>;
+  getNode(pageId: string, nodeId: string): Promise<Record<string, unknown>>;
+  searchInPage(
+    pageId: string,
+    query: string,
+    opts?: { regex?: boolean; caseSensitive?: boolean; limit?: number },
+  ): Promise<Record<string, unknown>>;
+  getTable(pageId: string, tableRef: string): Promise<Record<string, unknown>>;
+  // Returns `{ items, resolvedThreadsHidden }`. DEFAULT (includeResolved unset/
+  // false) hides resolved threads wholesale; pass true for the full feed.
+  listComments(
+    pageId: string,
+    includeResolved?: boolean,
+  ): Promise<{ items: unknown[]; resolvedThreadsHidden: number }>;
+  getComment(
+    commentId: string,
+  ): Promise<{ data: Record<string, unknown>; success: boolean }>;
+  checkNewComments(
+    spaceId: string,
+    since: string,
+    parentPageId?: string,
+  ): Promise<unknown>;
+  listShares(): Promise<unknown[]>;
+  listPageHistory(
+    pageId: string,
+    cursor?: string,
+  ): Promise<{ items: unknown[]; nextCursor: string | null }>;
+  getPageHistory(historyId: string): Promise<Record<string, unknown>>;
+  diffPageVersions(
+    pageId: string,
+    from?: string,
+    to?: string,
+  ): Promise<Record<string, unknown>>;
+  exportPageMarkdown(pageId: string): Promise<string>;
   // --- write (page) ---
-  | 'createPage'
-  | 'updatePage'
-  | 'renamePage'
-  | 'movePage'
-  | 'deletePage'
-  | 'editPageText'
-  | 'patchNode'
-  | 'insertNode'
-  | 'deleteNode'
-  | 'updatePageJson'
-  | 'tableInsertRow'
-  | 'tableDeleteRow'
-  | 'tableUpdateCell'
-  | 'copyPageContent'
-  | 'importPageMarkdown'
-  | 'sharePage'
-  | 'unsharePage'
-  | 'restorePageVersion'
-  | 'transformPage'
-  | 'stashPage'
-  // --- write (image / footnote), in-app since #410 ---
-  | 'insertImage'
-  | 'replaceImage'
-  | 'insertFootnote'
+  createPage(
+    title: string,
+    content: string,
+    spaceId: string,
+    parentPageId?: string,
+  ): Promise<{ data: Record<string, unknown>; success: boolean }>;
+  // Markdown content update via the collab path (carries provenance via the
+  // collab-token provider). Optionally also updates the title.
+  updatePage(
+    pageId: string,
+    content: string,
+    title?: string,
+  ): Promise<Record<string, unknown>>;
+  // Title-only rename via REST.
+  renamePage(
+    pageId: string,
+    title: string,
+  ): Promise<Record<string, unknown>>;
+  // Move via REST. parentPageId null => move to space root.
+  movePage(
+    pageId: string,
+    parentPageId: string | null,
+    position?: string,
+  ): Promise<unknown>;
+  // SOFT delete only (POST /pages/delete with { pageId }). NEVER permanent.
+  deletePage(pageId: string): Promise<unknown>;
+  editPageText(
+    pageId: string,
+    edits: Array<{ find: string; replace: string; replaceAll?: boolean }>,
+  ): Promise<Record<string, unknown>>;
+  patchNode(
+    pageId: string,
+    nodeId: string,
+    node: unknown,
+  ): Promise<Record<string, unknown>>;
+  insertNode(
+    pageId: string,
+    node: unknown,
+    opts: {
+      position: 'before' | 'after' | 'append';
+      anchorNodeId?: string;
+      anchorText?: string;
+    },
+  ): Promise<Record<string, unknown>>;
+  deleteNode(
+    pageId: string,
+    nodeId: string,
+  ): Promise<Record<string, unknown>>;
+  updatePageJson(
+    pageId: string,
+    doc?: unknown,
+    title?: string,
+  ): Promise<Record<string, unknown>>;
+  // Attach an author-inline footnote after the first occurrence of anchorText;
+  // numbering + the footnotes list are derived server-side.
+  insertFootnote(
+    pageId: string,
+    anchorText: string,
+    text: string,
+  ): Promise<Record<string, unknown>>;
+  // Download a web image and insert it into the page (append, or replace/after a
+  // text anchor). `url` is the image http(s) URL.
+  insertImage(
+    pageId: string,
+    url: string,
+    opts?: {
+      align?: 'left' | 'center' | 'right';
+      alt?: string;
+      replaceText?: string;
+      afterText?: string;
+    },
+  ): Promise<Record<string, unknown>>;
+  // Swap an existing image (by its attachmentId) for a new one fetched from a web
+  // URL, repointing every reference in the live document.
+  replaceImage(
+    pageId: string,
+    oldAttachmentId: string,
+    url: string,
+    opts?: { align?: 'left' | 'center' | 'right'; alt?: string },
+  ): Promise<Record<string, unknown>>;
   // --- draw.io diagrams (#423, stage 1) ---
-  | 'drawioGet'
-  | 'drawioCreate'
-  | 'drawioUpdate'
+  // Read a diagram as decoded mxGraph XML (default) or the raw .drawio.svg.
+  // meta.hash is the optimistic-lock key drawioUpdate expects as baseHash.
+  drawioGet(
+    pageId: string,
+    node: string,
+    format?: 'xml' | 'svg',
+  ): Promise<Record<string, unknown>>;
+  // Lint mxGraph XML, build the .drawio.svg attachment and insert a drawio node.
+  drawioCreate(
+    pageId: string,
+    where: {
+      position: 'before' | 'after' | 'append';
+      anchorNodeId?: string;
+      anchorText?: string;
+    },
+    xml: string,
+    title?: string,
+  ): Promise<Record<string, unknown>>;
+  // Optimistic-locked full replacement of a diagram (baseHash from drawioGet).
+  drawioUpdate(
+    pageId: string,
+    node: string,
+    xml: string,
+    baseHash: string,
+  ): Promise<Record<string, unknown>>;
+  tableInsertRow(
+    pageId: string,
+    tableRef: string,
+    cells: string[],
+    index?: number,
+  ): Promise<Record<string, unknown>>;
+  tableDeleteRow(
+    pageId: string,
+    tableRef: string,
+    index: number,
+  ): Promise<Record<string, unknown>>;
+  tableUpdateCell(
+    pageId: string,
+    tableRef: string,
+    row: number,
+    col: number,
+    text: string,
+  ): Promise<Record<string, unknown>>;
+  copyPageContent(
+    sourcePageId: string,
+    targetPageId: string,
+  ): Promise<Record<string, unknown>>;
+  importPageMarkdown(
+    pageId: string,
+    fullMarkdown: string,
+  ): Promise<Record<string, unknown>>;
+  sharePage(
+    pageId: string,
+    searchIndexing?: boolean,
+  ): Promise<Record<string, unknown>>;
+  unsharePage(pageId: string): Promise<Record<string, unknown>>;
+  restorePageVersion(historyId: string): Promise<Record<string, unknown>>;
+  // The opts type declares deleteComments? to match the real client signature,
+  // but the agent tool NEVER sets it (comment deletion stays unreachable).
+  transformPage(
+    pageId: string,
+    transformJs: string,
+    opts?: { dryRun?: boolean; deleteComments?: boolean },
+  ): Promise<Record<string, unknown>>;
   // --- write (comment) ---
-  | 'createComment'
-  | 'resolveComment';
-
-/**
- * The client surface the per-user tool adapter consumes, DERIVED from the real
- * `DocmostClient` type in `@docmost/mcp` (issue #446, restored #294 debt). This
- * replaces the former hand-mirror of ~45 method signatures.
- *
- * `import type` (above) is fully ERASED at compile time, so nothing is actually
- * imported from the ESM-only package at runtime — the server still loads the
- * class through the dynamic `import()` trick in `loadDocmostMcp` below; this is
- * purely a compile-time type. Deriving via `Pick` means a parameter reorder or a
- * type change to any of these methods in `client.ts` now becomes a SERVER
- * COMPILE ERROR at the positional call sites in ai-chat-tools.service.ts,
- * instead of a silent runtime "wrong argument" failure inside an agent tool.
- *
- * This made the old name-only drift-guard test
- * (packages/mcp/test/unit/client-host-contract.test.mjs) redundant — tsc now
- * enforces both names AND signatures — so that test was removed.
- */
-export type DocmostClientLike = Pick<DocmostClient, DocmostClientMethod>;
+  createComment(
+    pageId: string,
+    content: string,
+    type?: 'page' | 'inline',
+    selection?: string,
+    parentCommentId?: string,
+    suggestedText?: string,
+  ): Promise<{ data: Record<string, unknown>; success: boolean }>;
+  resolveComment(
+    commentId: string,
+    resolved: boolean,
+  ): Promise<Record<string, unknown>>;
+  // Serialize a page + mirror its internal images into the blob sandbox; returns
+  // ONLY a short anonymous URL (the body never enters the model context).
+  stashPage(pageId: string): Promise<{
+    uri: string;
+    sha256: string;
+    size: number;
+    images: { mirrored: number; failed: number };
+  }>;
+}
 
 export type DocmostClientConfig = {
   apiUrl: string;
@@ -109,7 +280,32 @@ export type DocmostClientConfig = {
 };
 
 export interface DocmostClientCtor {
-  new (config: DocmostClientConfig): DocmostClient;
+  new (config: DocmostClientConfig): DocmostClientLike;
+}
+
+/**
+ * Local hand-mirror of the `SharedToolSpec` shape exported from
+ * `@docmost/mcp` (packages/mcp/src/tool-specs.ts). Same approach as
+ * `DocmostClientLike`: we do not import the ESM package's types directly across
+ * the CJS/ESM boundary. The registry itself has no runtime deps, but keeping the
+ * type local avoids coupling the server build to the package's type surface.
+ *
+ * `buildShape` is intentionally zod-agnostic: it returns a plain ZodRawShape
+ * built with whatever zod namespace the caller passes (the server passes its own
+ * zod v4; the MCP package passes its zod v3). See the registry module comment.
+ */
+export interface SharedToolSpec {
+  mcpName: string;
+  inAppKey: string;
+  description: string;
+  // Deferred-tool metadata (#332). Optional in this mirror so an older/stale
+  // @docmost/mcp build (pre-#332) still type-checks; the in-app catalog builder
+  // reads them defensively. The external /mcp server ignores both fields.
+  tier?: 'core' | 'deferred';
+  catalogLine?: string;
+  // Loose `z` on purpose: the registry is zod-agnostic so the server can pass
+  // its own zod (v4) and the MCP package its own (v3) into the same builder.
+  buildShape?: (z: any) => Record<string, unknown>;
 }
 
 /**
@@ -148,50 +344,6 @@ interface DocmostMcpModule {
   // loader in unit tests. The in-app layer treats an absent factory as "signal
   // disabled" — a pure no-op that leaves tool results byte-identical.
   createCommentSignalTracker?: CommentSignalTrackerFactory;
-  // Optional (#447): a deterministic hash of the tool-specs registry content,
-  // generated into build/ by the package's build. Absent on a pre-#447 build (or
-  // 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;
-}
-
-/**
- * Recompute the REGISTRY_STAMP (#447) from the @docmost/mcp source tree, if it is
- * present. Returns the stamp string, or `null` when the source is absent (a prod
- * image ships only build/, no src/). MUST stay byte-for-byte identical to
- * packages/mcp/scripts/gen-registry-stamp.mjs's `computeRegistryStamp` so the
- * build-time and src-time hashes agree: same input file (src/tool-specs.ts), same
- * normalization (CRLF -> LF, strip a single trailing newline), same sha256.
- *
- * DEV vs PROD detection is by FILE EXISTENCE, not NODE_ENV: we resolve the
- * package's own directory from `require.resolve('@docmost/mcp')` (which points at
- * build/index.js) and look for ../src/tool-specs.ts next to it. In a dev/test
- * worktree that file exists; in a prod image (build/ only, src/ stripped) it does
- * not, so this returns null and the caller skips the check. Any error (ENOENT, a
- * bad resolve) is swallowed to null — the stale-check must NEVER break startup.
- *
- * Exported for unit testing (docmost-client.loader.spec.ts): the export keyword
- * is behaviourally a no-op — the module-internal caller `loadDocmostMcp` is
- * unaffected. The test drives the null (no-src) path and asserts this
- * normalize+sha256 stays identical to the codegen's `computeRegistryStamp`.
- */
-export function computeSrcRegistryStamp(packageEntry: string): string | null {
-  try {
-    // packageEntry is <pkg>/build/index.js; the source lives at <pkg>/src/.
-    const toolSpecsPath = join(
-      dirname(dirname(packageEntry)),
-      'src',
-      'tool-specs.ts',
-    );
-    if (!existsSync(toolSpecsPath)) return null; // prod: no src tree -> skip.
-    const source = readFileSync(toolSpecsPath, 'utf8');
-    const normalized = source.replace(/\r\n/g, '\n').replace(/\n$/, '');
-    return createHash('sha256').update(normalized, 'utf8').digest('hex');
-  } catch {
-    // Never let a resolution/read hiccup break server startup — treat as "no
-    // src available" and skip the check (identical to the prod no-op path).
-    return null;
-  }
 }
 
 // TS with module:commonjs downlevels a literal `import()` to `require()`, which
@@ -223,23 +375,6 @@ export async function loadDocmostMcp(): Promise<{
       const mod = (await esmImport(
         pathToFileURL(entry).href,
       )) as DocmostMcpModule;
-      // #447 stale-build guard (dev/test only). The server loads the COMPILED
-      // build/ of @docmost/mcp, but the parity/tier guard tests read src/. If a
-      // tool spec is edited in src without rebuilding the package, build/ and src/
-      // silently diverge and the running server serves the OLD tools. Here we
-      // recompute the stamp from src/tool-specs.ts and compare it to the stamp
-      // baked into build/. In PROD the src tree is absent (image ships build/
-      // only), so computeSrcRegistryStamp returns null and this is a pure no-op.
-      const srcStamp = computeSrcRegistryStamp(entry);
-      if (
-        srcStamp !== null &&
-        typeof mod.REGISTRY_STAMP === 'string' &&
-        srcStamp !== mod.REGISTRY_STAMP
-      ) {
-        throw new Error(
-          '@docmost/mcp build is stale (tool-specs changed since last build) — run: pnpm --filter @docmost/mcp build',
-        );
-      }
       return mod;
     })().catch((err) => {
       // Do not cache a rejected import — allow the next call to retry.

packages/mcp/package.json

@@ -5,26 +5,18 @@
   "private": true,
   "type": "module",
   "main": "./build/index.js",
-  "types": "./build/index.d.ts",
   "exports": {
-    ".": {
-      "types": "./build/index.d.ts",
-      "default": "./build/index.js"
-    },
-    "./http": {
-      "types": "./build/http.d.ts",
-      "default": "./build/http.js"
-    }
+    ".": "./build/index.js",
+    "./http": "./build/http.js"
   },
   "bin": {
     "docmost-mcp": "./build/stdio.js"
   },
   "scripts": {
-    "gen:stamp": "node scripts/gen-registry-stamp.mjs",
-    "build": "node scripts/gen-registry-stamp.mjs && tsc",
+    "build": "tsc",
     "start": "node build/stdio.js",
-    "watch": "node scripts/gen-registry-stamp.mjs && tsc --watch",
-    "pretest": "node scripts/gen-registry-stamp.mjs && tsc",
+    "watch": "tsc --watch",
+    "pretest": "tsc",
     "test": "node --test \"test/unit/*.test.mjs\" \"test/mock/*.test.mjs\"",
     "test:unit": "node --test \"test/unit/*.test.mjs\"",
     "test:mock": "node --test \"test/mock/*.test.mjs\"",

packages/mcp/scripts/gen-registry-stamp.mjs

@@ -1,67 +0,0 @@
-// Codegen: emit src/registry-stamp.generated.ts with a REGISTRY_STAMP hash of
-// the tool-specs REGISTRY CONTENT, so a build/ vs src/ skew (issue #447) is
-// detectable at runtime.
-//
-// WHY hash the raw source text (not extracted structured data):
-// SHARED_TOOL_SPECS carries `buildShape` functions (the input SCHEMAS) which are
-// NOT serializable. The input schema is exactly one of the things that MUST stay
-// in sync between build/ and src/, so we cannot drop it from the hash. Rather
-// than probe zod with a fragile shim to reconstruct the schema shape, we hash the
-// STABLE, deterministic source TEXT of tool-specs.ts. That text fully captures
-// every field that must stay in sync — mcpName, inAppKey, description, tier,
-// catalogLine AND the buildShape bodies (input schemas) — with zero probing
-// fragility. Any edit to a spec (a renamed tool, a reworded description, a
-// changed schema field) changes the text and therefore the stamp.
-//
-// DETERMINISM: the hash is computed over the file bytes with line endings
-// normalized to LF and a single trailing newline stripped, so a CRLF checkout or
-// an editor's trailing-newline habit cannot make build/ and src/ disagree. No
-// Date.now / randomness. The loader's dev-only stale-check (docmost-client.loader.ts)
-// re-runs THIS SAME normalization + sha256 over src/tool-specs.ts and compares to
-// the built REGISTRY_STAMP; the two must compute identically.
-//
-// This script runs from the `build` and `pretest` npm scripts BEFORE tsc, so
-// build/ always carries a stamp derived from the tool-specs.ts that was compiled.
-
-import { createHash } from 'node:crypto';
-import { readFileSync, writeFileSync } from 'node:fs';
-import { fileURLToPath } from 'node:url';
-import { dirname, join } from 'node:path';
-
-const __dirname = dirname(fileURLToPath(import.meta.url));
-const SRC_DIR = join(__dirname, '..', 'src');
-const TOOL_SPECS_PATH = join(SRC_DIR, 'tool-specs.ts');
-const OUT_PATH = join(SRC_DIR, 'registry-stamp.generated.ts');
-
-/**
- * Deterministic stamp of the tool-specs registry content. Kept as a plain
- * function (exported) so the algorithm has a single home; the loader duplicates
- * only the tiny normalize+sha256 steps because it lives in the CJS server build
- * and cannot import this ESM script. If you change the normalization here, mirror
- * it in apps/server/src/core/ai-chat/tools/docmost-client.loader.ts.
- */
-export function computeRegistryStamp(toolSpecsSource) {
-  const normalized = toolSpecsSource.replace(/\r\n/g, '\n').replace(/\n$/, '');
-  return createHash('sha256').update(normalized, 'utf8').digest('hex');
-}
-
-function main() {
-  const source = readFileSync(TOOL_SPECS_PATH, 'utf8');
-  const stamp = computeRegistryStamp(source);
-  const out =
-    '// AUTO-GENERATED by scripts/gen-registry-stamp.mjs — DO NOT EDIT BY HAND.\n' +
-    '// A deterministic hash of src/tool-specs.ts content (tool names, descriptions,\n' +
-    '// tiers, catalog lines and input schemas). Regenerated on every build/pretest\n' +
-    '// so build/ always matches the compiled src. The in-app loader recomputes this\n' +
-    '// from src and refuses to run on a mismatch (issue #447). This file is\n' +
-    '// gitignored and produced by the build — see .gitignore.\n' +
-    `export const REGISTRY_STAMP = ${JSON.stringify(stamp)};\n`;
-  writeFileSync(OUT_PATH, out, 'utf8');
-  // eslint-disable-next-line no-console
-  console.log(`gen-registry-stamp: wrote ${OUT_PATH} (${stamp.slice(0, 12)}…)`);
-}
-
-// Only run when invoked directly (not when imported for computeRegistryStamp).
-if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) {
-  main();
-}

packages/mcp/src/client.ts

@@ -197,6 +197,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 +497,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). */
@@ -553,18 +729,16 @@ export class DocmostClient {
     // forever and accumulate duplicates).
     const MAX_PAGES = 50;
 
-    let cursor: string | undefined;
+    let page = 1;
     let allItems: T[] = [];
-    let truncated = false;
+    let hasNextPage = true;
 
-    for (let page = 0; page < MAX_PAGES; page++) {
-      const payload: Record<string, any> = {
+    while (hasNextPage && page <= MAX_PAGES) {
+      const response = await this.client.post(endpoint, {
         ...basePayload,
         limit: clampedLimit,
-      };
-      if (cursor) payload.cursor = cursor;
-
-      const response = await this.client.post(endpoint, payload);
+        page,
+      });
 
       const data = response.data;
       const items = data.data?.items || data.items || [];
@@ -572,28 +746,22 @@ export class DocmostClient {
 
       allItems = allItems.concat(items);
 
-      // Advance strictly via the server-issued cursor. A missing nextCursor (or
-      // hasNextPage false) means we reached the end. A cursor identical to the
-      // one we just sent means the server did not understand our pagination
-      // param — stop instead of re-fetching page one forever and duplicating.
-      const next = meta?.hasNextPage ? meta?.nextCursor : null;
-      if (!next || next === cursor) {
-        // If the server still reports more pages but stopped issuing a usable
-        // cursor at the ceiling, flag the result as truncated below.
-        if (page === MAX_PAGES - 1 && meta?.hasNextPage) truncated = true;
+      // Stop if the page is empty or shorter than the requested size: a full
+      // page worth of items is the only situation where another page can exist,
+      // so this defends against a stuck hasNextPage flag in addition to it.
+      if (items.length === 0 || items.length < clampedLimit) {
         break;
       }
-      cursor = next;
 
-      // Reaching the ceiling with more pages still available means the result
-      // set is truncated.
-      if (page === MAX_PAGES - 1) truncated = true;
+      hasNextPage = meta?.hasNextPage || false;
+      page++;
     }
 
     // If the loop stopped because it hit the MAX_PAGES ceiling while the server
-    // still reported more results, the result set is truncated — warn so the
-    // caller is not silently handed an incomplete list.
-    if (truncated) {
+    // still reported more results (hasNextPage true and the last page was
+    // full), the result set is truncated — warn so the caller is not silently
+    // handed an incomplete list.
+    if (hasNextPage && page > MAX_PAGES) {
       console.warn(
         `paginateAll: results from "${endpoint}" truncated at the ${MAX_PAGES}-page cap; more pages exist on the server`,
       );
@@ -627,10 +795,9 @@ export class DocmostClient {
    * Tree (`tree` true): the space's FULL page hierarchy as a nested tree (each
    * node has a `children` array). This mode REQUIRES `spaceId` (a page tree is
    * scoped to one space) and IGNORES `limit` — the whole hierarchy is returned.
-   * It fetches the tree via `enumerateSpacePages`, which on the fork server
-   * resolves to a single `/pages/tree` request returning the whole
-   * permission-filtered flat page set (soft-deleted pages excluded
-   * server-side).
+   * It walks the sidebar tree via `enumerateSpacePages`, which performs N
+   * sidebar requests and is bounded by that method's 10000-node cap (and skips
+   * soft-deleted pages server-side).
    */
   async listPages(spaceId?: string, limit: number = 50, tree: boolean = false) {
     await this.ensureAuthenticated();
@@ -641,8 +808,8 @@ export class DocmostClient {
           "list_pages: tree mode requires a spaceId (a page tree is scoped to one space). Pass spaceId, or omit tree to get the recent-pages list.",
         );
       }
-      const { pages } = await this.enumerateSpacePages(spaceId);
-      return buildPageTree(pages);
+      const nodes = await this.enumerateSpacePages(spaceId);
+      return buildPageTree(nodes);
     }
 
     const clampedLimit = Math.max(1, Math.min(100, limit));
@@ -664,123 +831,57 @@ export class DocmostClient {
   async listSidebarPages(spaceId: string, pageId?: string) {
     await this.ensureAuthenticated();
 
-    // Paginate via the server-issued cursor. The server switched from OFFSET
-    // (`page`) to CURSOR (`cursor`/`nextCursor`) pagination, and the global
-    // ValidationPipe(whitelist:true) SILENTLY STRIPS the obsolete `page` field
-    // — so the old offset loop got the SAME first page every time (with
-    // hasNextPage stuck true) and dropped every child beyond the first page.
+    // Paginate: the endpoint returns server-paged children, so posting only
+    // { page: 1 } silently dropped every child beyond the first page. Loop on
+    // meta.hasNextPage (with a MAX_PAGES ceiling like paginateAll, guarding
+    // against a stuck hasNextPage flag) and accumulate all children.
     const MAX_PAGES = 50;
-    let cursor: string | undefined;
+    let page = 1;
     let allItems: any[] = [];
-    let truncated = false;
+    let hasNextPage = true;
 
-    for (let i = 0; i < MAX_PAGES; i++) {
-      // limit: 100 is the server-side Max; cuts request count 5x vs the default 20.
-      const payload: Record<string, any> = { spaceId, limit: 100 };
+    while (hasNextPage && page <= MAX_PAGES) {
       // Only send pageId when scoping to a page's children; omit it for roots.
+      const payload: Record<string, any> = { spaceId, page };
       if (pageId) payload.pageId = pageId;
-      if (cursor) payload.cursor = cursor;
 
-      const data = (await this.client.post("/pages/sidebar-pages", payload)).data
-        ?.data;
-      allItems = allItems.concat(data?.items ?? []);
+      const response = await this.client.post("/pages/sidebar-pages", payload);
+      const data = response.data?.data ?? response.data;
+      const items = data?.items || [];
+      allItems = allItems.concat(items);
 
-      // Advance strictly via the server-issued cursor; a missing/repeated cursor
-      // means the protocol drifted again — stop instead of looping on page one.
-      const next = data?.meta?.hasNextPage ? data?.meta?.nextCursor : null;
-      if (!next || next === cursor) break;
-      cursor = next;
-
-      // Reaching the ceiling with more pages still available means the child
-      // list is truncated (mirrors paginateAll).
-      if (i === MAX_PAGES - 1) truncated = true;
-    }
-
-    // Warn on real truncation (ceiling hit while the server still had pages) so
-    // the caller is not silently handed an incomplete child list.
-    if (truncated) {
-      console.warn(
-        `listSidebarPages: children of "${pageId ?? spaceId}" truncated at the ${MAX_PAGES}-page cap; more pages exist on the server`,
-      );
+      hasNextPage = data?.meta?.hasNextPage || false;
+      page++;
     }
 
     return allItems;
   }
 
   /**
-   * Enumerate EVERY page in a space (or in a subtree, when rootPageId is given).
+   * Enumerate EVERY page in a space (or in a subtree, when rootPageId is given)
+   * by walking the sidebar-pages tree.
    *
-   * Primary path (fork server): a SINGLE `POST /pages/tree` returns the whole
-   * space (or a subtree) as a flat, permission-filtered list in one request, in
-   * the exact node shape buildPageTree consumes. This replaces the old
-   * per-node BFS, which issued N sidebar requests and — after the server moved
-   * to cursor pagination — silently lost every child past the first sidebar
-   * page (the obsolete `page` param was stripped by ValidationPipe).
+   * Starting set: the children of rootPageId when provided, otherwise the
+   * space root pages. From there it does an iterative breadth-first walk: each
+   * node is collected, and when node.hasChildren is true its direct children
+   * are fetched via listSidebarPages(spaceId, node.id) and enqueued.
    *
-   * The subtree variant (rootPageId given) INCLUDES the root node itself
-   * (getPageAndDescendants seeds with id = rootPageId), unlike the old BFS
-   * which started from the root's children.
+   * This replaces the old "/pages/recent" enumeration, which is a bounded
+   * recent-activity feed (~5000 cap) and therefore misses comments on older
+   * pages that were never recently touched.
    *
-   * Fallback path (stdio mode may target STOCK upstream Docmost, which lacks
-   * `/pages/tree`): on a 404/405 it falls back to the cursor-based BFS below,
-   * walking direct children via the fixed cursor listSidebarPages. Safeguards:
-   * a `visited` Set of page ids prevents re-processing a node (cycles /
-   * duplicate references), and a hard node cap bounds pathological trees so the
-   * walk always terminates.
-   *
-   * Returns `{ pages, truncated }`. `truncated` is true ONLY when the fallback
-   * BFS stopped at its MAX_NODES cap — the primary /pages/tree path is uncapped
-   * and always returns the complete set, so it never reports truncation.
+   * Safeguards: a `visited` Set of page ids prevents re-processing a node
+   * (cycles / duplicate references), and a hard node cap bounds pathological
+   * trees so the walk always terminates.
    */
   private async enumerateSpacePages(
     spaceId: string,
     rootPageId?: string,
-  ): Promise<{ pages: any[]; truncated: boolean }> {
-    await this.ensureAuthenticated();
-
-    // Single request replaces the whole BFS: /pages/tree returns the full
-    // permission-filtered flat page set of a space (or a subtree) at once. This
-    // path is uncapped, so it is never truncated.
-    const payload = rootPageId ? { pageId: rootPageId } : { spaceId };
-    try {
-      const response = await this.client.post("/pages/tree", payload);
-      const pages = (response.data?.data ?? response.data)?.items ?? [];
-      return { pages, truncated: false };
-    } catch (e: any) {
-      // Only fall back when the endpoint is absent (stock upstream Docmost);
-      // any other error is a genuine failure and must propagate.
-      if (
-        !axios.isAxiosError(e) ||
-        (e.response?.status !== 404 && e.response?.status !== 405)
-      ) {
-        throw e;
-      }
-    }
-
-    // Fallback: cursor-based breadth-first walk via listSidebarPages.
+  ): Promise<any[]> {
     const MAX_NODES = 10000;
     const result: any[] = [];
     const visited = new Set<string>();
 
-    // Seed with the root node itself when scoping to a subtree, so its own
-    // comments aren't dropped: the primary /pages/tree seeds
-    // getPageAndDescendants with id = rootPageId (root included), but
-    // listSidebarPages(spaceId, rootPageId) returns only the root's CHILDREN.
-    // The `visited` set below prevents a double-add if the root also appears
-    // among the children. getPageRaw returns a page whose id/title/spaceId are
-    // exactly what buildPageTree and check_new_comments consume.
-    if (rootPageId) {
-      try {
-        const root = await this.getPageRaw(rootPageId);
-        if (root?.id) {
-          result.push(root);
-          visited.add(root.id);
-        }
-      } catch {
-        // Non-fatal: if the root can't be read, fall through to children-only.
-      }
-    }
-
     // Seed the queue with the starting level (subtree children or roots).
     const queue: any[] = await this.listSidebarPages(spaceId, rootPageId);
 
@@ -805,12 +906,7 @@ export class DocmostClient {
       }
     }
 
-    // Truncated only when the cap was hit with the queue still non-empty (real
-    // truncation, not a natural end at exactly MAX_NODES).
-    return {
-      pages: result,
-      truncated: result.length >= MAX_NODES && queue.length > 0,
-    };
+    return result;
   }
 
   /** Raw page info including the ProseMirror JSON content and slugId. */
@@ -2444,13 +2540,7 @@ export class DocmostClient {
     let allComments: any[] = [];
     let cursor: string | null = null;
 
-    // Hard ceiling + immovable-cursor guard (mirrors paginateAll): if /comments
-    // ever stops advancing the cursor (the exact #442 drift scenario) this loop
-    // would otherwise spin forever accumulating duplicates.
-    const MAX_PAGES = 50;
-    let truncated = false;
-
-    for (let page = 0; page < MAX_PAGES; page++) {
+    do {
       const payload: Record<string, any> = { pageId, limit: 100 };
       if (cursor) payload.cursor = cursor;
 
@@ -2458,23 +2548,8 @@ export class DocmostClient {
       const data = response.data.data || response.data;
       const items = data.items || [];
       allComments = allComments.concat(items);
-
-      // Advance strictly via the server-issued cursor. A missing nextCursor or a
-      // cursor identical to the one we just sent means the end (or a server that
-      // ignores our pagination param) — stop instead of re-fetching page one.
-      const next: string | null = data.meta?.nextCursor || null;
-      if (!next || next === cursor) break;
-      cursor = next;
-
-      // Reaching the ceiling with a still-advancing cursor means truncation.
-      if (page === MAX_PAGES - 1) truncated = true;
-    }
-
-    if (truncated) {
-      console.warn(
-        `listComments: comments for "${pageId}" truncated at the ${MAX_PAGES}-page cap; more pages exist on the server`,
-      );
-    }
+      cursor = data.meta?.nextCursor || null;
+    } while (cursor);
 
     const mapped = allComments.map((comment: any) => {
       const markdown = comment.content
@@ -2512,6 +2587,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 +2678,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 +2966,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 +2983,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 +2997,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,
@@ -2947,27 +3036,36 @@ export class DocmostClient {
       );
     }
 
-    // 1. Enumerate the FULL set of pages in scope via the page tree (a complete
-    // page index), NOT the bounded "/pages/recent" feed which caps at ~5000
-    // recent items and silently misses comments on older pages.
+    // 1. Enumerate the FULL set of pages in scope by walking the sidebar-pages
+    // tree (a complete page index), NOT the bounded "/pages/recent" feed which
+    // caps at ~5000 recent items and silently misses comments on older pages.
     //
     // Subtree scope: when parentPageId is given, the scope is that page ITSELF
-    // plus every descendant. Otherwise the scope is the whole space (all roots
-    // and their descendants).
+    // plus every descendant (enumerateSpacePages walks its children). Otherwise
+    // the scope is the whole space (all roots and their descendants).
     //
     // NOTE: do NOT pre-filter by page.updatedAt — creating a comment does not
     // bump it (verified on a live server), so such a filter silently misses
     // comments on pages that were not otherwise edited. The complete tree walk
     // already restricts the scope correctly, so no recent-feed allow-list is
     // needed any more.
-    //
-    // The subtree scope (parentPageId given) already INCLUDES the root node
-    // itself: /pages/tree seeds getPageAndDescendants with id = parentPageId, so
-    // no separate getPageRaw fetch for the parent is needed.
-    const { pages: pagesInScope, truncated } = await this.enumerateSpacePages(
-      spaceId,
-      parentPageId,
-    );
+    let pagesInScope: any[];
+    if (parentPageId) {
+      const subtree = await this.enumerateSpacePages(spaceId, parentPageId);
+      // Include the parent page node itself alongside its descendants. Fetch it
+      // so its title/id are available even though it is not returned by its own
+      // children listing.
+      let parentNode: any = { id: parentPageId };
+      try {
+        parentNode = await this.getPageRaw(parentPageId);
+      } catch (e: any) {
+        // Fall back to a minimal node if the parent can't be fetched; its
+        // comments are still attempted below (the fetch there is non-fatal).
+      }
+      pagesInScope = [parentNode, ...subtree];
+    } else {
+      pagesInScope = await this.enumerateSpacePages(spaceId);
+    }
 
     // 2. Fetch comments for each page, keep ones created after since
     const results: any[] = [];
@@ -2996,9 +3094,10 @@ export class DocmostClient {
       0,
     );
 
-    // `truncated` is reported by enumerateSpacePages: it is true ONLY when the
-    // stdio fallback BFS hit its node cap. The primary /pages/tree path is
-    // uncapped, so a space with legitimately many pages is not falsely flagged.
+    // enumerateSpacePages caps traversal at 10000 nodes; flag when that cap was
+    // hit so the caller knows the scan may be incomplete (some pages skipped).
+    const truncated = pagesInScope.length >= 10000;
+
     return {
       since,
       scope: parentPageId ? `subtree of ${parentPageId}` : `space ${spaceId}`,

packages/mcp/src/index.ts

@@ -29,14 +29,6 @@ export { destroyAllSessions } from "./lib/collab-session.js";
 export { SHARED_TOOL_SPECS } from "./tool-specs.js";
 export type { SharedToolSpec } 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
-// by scripts/gen-registry-stamp.mjs BEFORE tsc, so it lands in build/. The in-app
-// loader recomputes the same hash from src/tool-specs.ts (dev/test only) and
-// refuses to run on a mismatch, catching a build/ vs src/ skew (a spec edited in
-// src without rebuilding the package the server actually loads from build/).
-export { REGISTRY_STAMP } from "./registry-stamp.generated.js";
-
 // Re-export the shared "new comments: N" signal helper (#417) so the in-app
 // layer reads the SAME watermark/debounce/injection-safe line builder off the
 // loaded module (same pattern as SHARED_TOOL_SPECS). Both surfaces then differ
@@ -202,10 +194,10 @@ export function createDocmostMcpServer(config: DocmostMcpConfig): McpServer {
     { instructions: SERVER_INSTRUCTIONS },
   );
 
-  // Single choke point for MCP tool timing. Both `registerSharedFromSpec` (below)
-  // and the inline `server.registerTool(...)` calls funnel through this one
-  // method, so monkeypatching it HERE — before any tool is registered and before
-  // the registry loop captures a reference to it — times every tool with no
+  // Single choke point for MCP tool timing. Both `registerShared` (below) and
+  // the inline `server.registerTool(...)` calls funnel through this one method,
+  // so monkeypatching it HERE — before any tool is registered and before
+  // `registerShared` captures a reference to it — times every tool with no
   // per-tool boilerplate. The wrapped handler records wall-clock duration and,
   // in a `finally`, feeds the host's dependency-neutral sink
   // `config.onMetric("mcp_tool_duration_seconds", seconds, { tool })`. The tool
@@ -261,56 +253,92 @@ export function createDocmostMcpServer(config: DocmostMcpConfig): McpServer {
     return originalRegisterTool(...args.slice(0, -1), signalledHandler);
   };
 
-  // Register EVERY shared tool from the zod-agnostic registry in one loop (#445).
-  // The spec owns the canonical name + description + (optional) schema builder AND
-  // the canonical execute mapping; the host only supplies the RESULT ENVELOPE. For
-  // each spec:
-  //   - skip `inAppOnly` specs (they belong to the in-app host only);
-  //   - if the spec has an `mcpExecute` override (a deliberate per-layer
-  //     difference — a guardrail, an omitted param, or a non-JSON envelope like a
-  //     resource_link/bare success line), the override OWNS the full MCP content
-  //     result and is used VERBATIM;
-  //   - otherwise the canonical `execute` returns RAW data and this host wraps it
-  //     in the standard JSON text envelope (jsonContent), exactly as the old inline
-  //     bodies did.
-  // buildShape is invoked with THIS package's zod (v3); the in-app layer passes its
-  // own zod (v4). The registry's execute returns `unknown` (it is zod-agnostic), so
-  // the wrapping is typed loosely and cast — runtime behaviour is unchanged.
-  const registerSharedFromSpec = (spec: SharedToolSpec) => {
-    if (spec.inAppOnly) return;
-    const handler = async (args: any) => {
-      if (spec.mcpExecute) {
-        // The override owns the full MCP result envelope (not re-wrapped).
-        return (await spec.mcpExecute(docmostClient, args)) as {
-          content: { type: "text"; text: string }[];
-        };
-      }
-      // Canonical execute returns raw data; wrap it as JSON text content.
-      const raw = await spec.execute!(docmostClient, args);
-      return jsonContent(raw);
-    };
-    return (server.registerTool as any)(
+  // Register a tool from the shared, zod-agnostic spec registry. The spec owns
+  // the canonical name + model-facing description + (optional) schema builder;
+  // only the execute body is supplied per call. buildShape is invoked with THIS
+  // package's zod (v3); the in-app layer passes its own zod (v4).
+  //
+  // The spec's schema builder returns a plain ZodRawShape (Record<string,
+  // unknown> in the shared module since it must stay zod-agnostic), so the
+  // McpServer.registerTool overloads cannot infer the execute arg's shape from
+  // it. We type `execute` loosely and cast the call through `any`; runtime
+  // behaviour is unchanged — each execute body destructures the same fields the
+  // builder declares.
+  const registerShared = (
+    spec: SharedToolSpec,
+    execute: (args: any) => Promise<{ content: { type: "text"; text: string }[] }>,
+  ) =>
+    (server.registerTool as any)(
       spec.mcpName,
       spec.buildShape
         ? { description: spec.description, inputSchema: spec.buildShape(z) }
         : { description: spec.description },
-      handler,
+      execute,
     );
-  };
 
-  for (const spec of Object.values(SHARED_TOOL_SPECS)) {
-    registerSharedFromSpec(spec as SharedToolSpec);
-  }
+  // Tool: get_workspace
+  registerShared(SHARED_TOOL_SPECS.getWorkspace, async () => {
+    const workspace = await docmostClient.getWorkspace();
+    return jsonContent(workspace);
+  });
 
-  // --- 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
-  // per-transport behaviour/schema divergence (search, docmost_transform), or a
-  // tool that exists ONLY on this standalone MCP surface (update_comment,
-  // delete_comment — the in-app agent deliberately exposes no hard comment
-  // edit/delete tool).
+  // Tool: list_spaces
+  registerShared(SHARED_TOOL_SPECS.listSpaces, async () => {
+    const spaces = await docmostClient.getSpaces();
+    return jsonContent(spaces);
+  });
 
-  // Tool: table_get
+// Tool: list_pages
+// INTENTIONAL per-transport divergence (not in the shared registry): this
+// transport exposes a `tree:true` mode that returns the full nested hierarchy;
+// the in-app copy keeps the same tree option but is worded for the in-app agent.
+// Kept per-layer so each side can tune its own guidance.
+// Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294). This
+// transport keeps applying its own defaults (limit=50, tree=false) in execute.
+registerShared(SHARED_TOOL_SPECS.listPages, async ({ spaceId, limit, tree }) => {
+  const result = await docmostClient.listPages(spaceId, limit ?? 50, tree ?? false);
+  return jsonContent(result);
+});
+
+// Tool: get_page
+// Schema + description now live in the shared registry (#294).
+registerShared(SHARED_TOOL_SPECS.getPage, async ({ pageId }) => {
+  const page = await docmostClient.getPage(pageId);
+  return jsonContent(page);
+});
+
+// Tool: get_page_json
+registerShared(SHARED_TOOL_SPECS.getPageJson, async ({ pageId }) => {
+  const page = await docmostClient.getPageJson(pageId);
+  return jsonContent(page);
+});
+
+// Tool: get_outline
+registerShared(SHARED_TOOL_SPECS.getOutline, async ({ pageId }) => {
+  const result = await docmostClient.getOutline(pageId);
+  return jsonContent(result);
+});
+
+// Tool: get_node
+registerShared(SHARED_TOOL_SPECS.getNode, async ({ pageId, nodeId }) => {
+  const result = await docmostClient.getNode(pageId, nodeId);
+  return jsonContent(result);
+});
+
+// Tool: search_in_page
+registerShared(
+  SHARED_TOOL_SPECS.searchInPage,
+  async ({ pageId, query, regex, caseSensitive, limit }) => {
+    const result = await docmostClient.searchInPage(pageId, query, {
+      regex,
+      caseSensitive,
+      limit,
+    });
+    return jsonContent(result);
+  },
+);
+
+// Tool: table_get
 // NOT in the shared registry: the MCP tool name `table_get` is noun-first while
 // the in-app key is `getTable` (verb-first), breaking the snake_case(inAppKey)
 // convention the shared registry enforces (shared-tool-specs.contract.spec.ts).
@@ -336,6 +364,383 @@ server.registerTool(
   },
 );
 
+// Tool: table_insert_row
+// Schema + description now live in the shared registry (#294); the `table`
+// parameter name is the canonical one (the in-app layer was unified to it).
+registerShared(
+  SHARED_TOOL_SPECS.tableInsertRow,
+  async ({ pageId, table, cells, index }) => {
+    const result = await docmostClient.tableInsertRow(
+      pageId,
+      table,
+      cells,
+      index,
+    );
+    return jsonContent(result);
+  },
+);
+
+// Tool: table_delete_row
+// Schema + description now live in the shared registry (#294).
+registerShared(
+  SHARED_TOOL_SPECS.tableDeleteRow,
+  async ({ pageId, table, index }) => {
+    const result = await docmostClient.tableDeleteRow(pageId, table, index);
+    return jsonContent(result);
+  },
+);
+
+// Tool: table_update_cell
+// Schema + description now live in the shared registry (#294).
+registerShared(
+  SHARED_TOOL_SPECS.tableUpdateCell,
+  async ({ pageId, table, row, col, text }) => {
+    const result = await docmostClient.tableUpdateCell(
+      pageId,
+      table,
+      row,
+      col,
+      text,
+    );
+    return jsonContent(result);
+  },
+);
+
+// Tool: create_page
+// Schema + description now live in the shared registry (#294).
+registerShared(
+  SHARED_TOOL_SPECS.createPage,
+  async ({ title, content, spaceId, parentPageId }) => {
+    const result = await docmostClient.createPage(
+      title,
+      content,
+      spaceId,
+      parentPageId,
+    );
+    return jsonContent(result);
+  },
+);
+
+// Tool: update_page_json
+// Schema + description now live in the shared registry (#294). The execute body
+// keeps this transport's content normalization (parse a JSON-string content,
+// pass undefined/null through for a title-only/no-op update).
+registerShared(
+  SHARED_TOOL_SPECS.updatePageJson,
+  async ({ pageId, content, title }) => {
+    // Only parse/validate the document when it was actually supplied; when it
+    // is omitted, pass it straight through so the client performs a title-only
+    // (or no-op) update.
+    let doc;
+    if (content === undefined || content === null) {
+      doc = undefined;
+    } else {
+      // String -> JSON.parse (throwing on invalid); object passes through.
+      doc = parseNodeArg(content, "content was a string but not valid JSON");
+    }
+    const result = await docmostClient.updatePageJson(pageId, doc, title);
+    return jsonContent(result);
+  },
+);
+
+// Tool: export_page_markdown
+// Schema + description now live in the shared registry (#294).
+registerShared(SHARED_TOOL_SPECS.exportPageMarkdown, async ({ pageId }) => {
+  const md = await docmostClient.exportPageMarkdown(pageId);
+  return { content: [{ type: "text" as const, text: md }] };
+});
+
+// Tool: import_page_markdown
+registerShared(
+  SHARED_TOOL_SPECS.importPageMarkdown,
+  async ({ pageId, markdown }) => {
+    const res = await docmostClient.importPageMarkdown(pageId, markdown);
+    return jsonContent(res);
+  },
+);
+
+// Tool: copy_page_content
+registerShared(
+  SHARED_TOOL_SPECS.copyPageContent,
+  async ({ sourcePageId, targetPageId }) => {
+    const result = await docmostClient.copyPageContent(
+      sourcePageId,
+      targetPageId,
+    );
+    return jsonContent(result);
+  },
+);
+
+// Tool: rename_page
+// Schema + description now live in the shared registry (#294).
+registerShared(SHARED_TOOL_SPECS.renamePage, async ({ pageId, title }) => {
+  const result = await docmostClient.renamePage(pageId, title);
+  return jsonContent(result);
+});
+
+// Tool: edit_page_text
+registerShared(SHARED_TOOL_SPECS.editPageText, async ({ pageId, edits }) => {
+  const result = await docmostClient.editPageText(pageId, edits);
+  return jsonContent(result);
+});
+
+// Tool: stash_page — returns a resource_link (NOT embedded text) so the doc
+// body never enters the model context. Registered directly (not via
+// registerShared) because that helper only emits text content. Also returns
+// `structuredContent` carrying the full documented `{uri, sha256, size, images}`
+// shape alongside the resource_link, so MCP clients receive the blob's sha256
+// (its ETag, for integrity) and mirror counts, not just the link.
+server.registerTool(
+  SHARED_TOOL_SPECS.stashPage.mcpName,
+  {
+    description: SHARED_TOOL_SPECS.stashPage.description,
+    inputSchema: SHARED_TOOL_SPECS.stashPage.buildShape!(z),
+  },
+  async ({ pageId }: { pageId: string }) => {
+    const result = await docmostClient.stashPage(pageId);
+    return {
+      content: [
+        {
+          type: "resource_link" as const,
+          uri: result.uri,
+          name: "page.json",
+          mimeType: "application/json",
+          size: result.size,
+        },
+      ],
+      // Mirror the full documented result shape ({ uri, size, sha256, images })
+      // as structuredContent so MCP clients get the blob's sha256 (its ETag, for
+      // integrity) and the mirror counts, not just the resource_link.
+      structuredContent: {
+        uri: result.uri,
+        sha256: result.sha256,
+        size: result.size,
+        images: result.images,
+      },
+    };
+  },
+);
+
+// Tool: patch_node — schema + description from the shared registry (identical
+// across both transports). The execute body keeps its own parseNodeArg
+// normalization (the model sometimes serializes `node` as a JSON string).
+registerShared(
+  SHARED_TOOL_SPECS.patchNode,
+  async ({ pageId, nodeId, node }) => {
+    const parsedNode = parseNodeArg(node);
+    const result = await docmostClient.patchNode(pageId, nodeId, parsedNode);
+    return jsonContent(result);
+  },
+);
+
+// Tool: insert_node — schema + description from the shared registry. As with
+// patch_node, the execute body retains parseNodeArg on the incoming node.
+registerShared(
+  SHARED_TOOL_SPECS.insertNode,
+  async ({ pageId, node, position, anchorNodeId, anchorText }) => {
+    const parsedNode = parseNodeArg(node);
+    const result = await docmostClient.insertNode(pageId, parsedNode, {
+      position,
+      anchorNodeId,
+      anchorText,
+    });
+    return jsonContent(result);
+  },
+);
+
+// Tool: delete_node
+registerShared(SHARED_TOOL_SPECS.deleteNode, async ({ pageId, nodeId }) => {
+  const result = await docmostClient.deleteNode(pageId, nodeId);
+  return jsonContent(result);
+});
+
+// Tool: insert_image
+// Schema + description now live in the shared registry (#410) so BOTH this MCP
+// server and the in-app AI-chat agent expose it. The execute body is unchanged.
+registerShared(
+  SHARED_TOOL_SPECS.insertImage,
+  async ({ pageId, imageUrl, align, alt, replaceText, afterText }) => {
+    const result = await docmostClient.insertImage(pageId, imageUrl, {
+      align,
+      alt,
+      replaceText,
+      afterText,
+    });
+    return jsonContent(result);
+  },
+);
+
+// Tool: replace_image
+// Schema + description now live in the shared registry (#410).
+registerShared(
+  SHARED_TOOL_SPECS.replaceImage,
+  async ({ pageId, attachmentId, imageUrl, align, alt }) => {
+    const result = await docmostClient.replaceImage(
+      pageId,
+      attachmentId,
+      imageUrl,
+      {
+        align,
+        alt,
+      },
+    );
+    return jsonContent(result);
+  },
+);
+
+// Tool: drawio_get — read a draw.io diagram as mxGraph XML (or the raw SVG).
+registerShared(
+  SHARED_TOOL_SPECS.drawioGet,
+  async ({ pageId, node, format }) => {
+    const result = await docmostClient.drawioGet(pageId, node, format ?? "xml");
+    return jsonContent(result);
+  },
+);
+
+// Tool: drawio_create — lint mxGraph XML, build the .drawio.svg, insert a node.
+registerShared(
+  SHARED_TOOL_SPECS.drawioCreate,
+  async ({ pageId, xml, position, anchorNodeId, anchorText, title }) => {
+    const result = await docmostClient.drawioCreate(
+      pageId,
+      { position, anchorNodeId, anchorText },
+      xml,
+      title,
+    );
+    return jsonContent(result);
+  },
+);
+
+// Tool: drawio_update — optimistic-locked full replacement of a diagram.
+registerShared(
+  SHARED_TOOL_SPECS.drawioUpdate,
+  async ({ pageId, node, xml, baseHash }) => {
+    const result = await docmostClient.drawioUpdate(pageId, node, xml, baseHash);
+    return jsonContent(result);
+  },
+);
+
+// Tool: share_page
+// Schema + description now live in the shared registry (#294). The execute body
+// keeps this transport's own `searchIndexing ?? true` default.
+registerShared(
+  SHARED_TOOL_SPECS.sharePage,
+  async ({ pageId, searchIndexing }) => {
+    const result = await docmostClient.sharePage(pageId, searchIndexing ?? true);
+    return jsonContent(result);
+  },
+);
+
+// Tool: unshare_page
+registerShared(SHARED_TOOL_SPECS.unsharePage, async ({ pageId }) => {
+  const result = await docmostClient.unsharePage(pageId);
+  return jsonContent(result);
+});
+
+// Tool: list_shares
+registerShared(SHARED_TOOL_SPECS.listShares, async () => {
+  const result = await docmostClient.listShares();
+  return jsonContent(result);
+});
+
+// Tool: move_page
+// Schema + description now live in the shared registry (#294). The execute body
+// keeps this transport's cycle guard, its 'null'/'' -> null string coercion, and
+// its positive-confirmation check on the move response.
+registerShared(
+  SHARED_TOOL_SPECS.movePage,
+  async ({ pageId, parentPageId, position }) => {
+    const finalParentId =
+      parentPageId === "" || parentPageId === "null" ? null : parentPageId;
+
+    // Cheap cycle guard: a page cannot be moved directly under itself.
+    // (Deeper descendant-cycle detection is intentionally out of scope.)
+    if (finalParentId !== null && finalParentId === pageId) {
+      throw new Error("cannot move a page under itself");
+    }
+
+    const result = await docmostClient.movePage(
+      pageId,
+      finalParentId || null,
+      position,
+    );
+
+    // Require POSITIVE confirmation: the live /pages/move success shape is
+    // exactly { success: true, status: 200 }. An empty body, a 204, or any odd
+    // shape lacking success === true must NOT be reported as a successful move,
+    // so we surface the raw API result instead of declaring success.
+    if (!(result && typeof result === "object" && result.success === true)) {
+      throw new Error(
+        `Failed to move page ${pageId}: ${JSON.stringify(result)}`,
+      );
+    }
+
+    return jsonContent({
+      message: `Successfully moved page ${pageId} to parent ${finalParentId || "root"}`,
+      result,
+    });
+  },
+);
+
+// Tool: delete_page
+// Schema + description now live in the shared registry (#294). The shared schema
+// exposes ONLY pageId, so no permanent/force-delete flag can reach the client.
+registerShared(SHARED_TOOL_SPECS.deletePage, async ({ pageId }) => {
+  await docmostClient.deletePage(pageId);
+  return {
+    content: [
+      { type: "text" as const, text: `Successfully deleted page ${pageId}` },
+    ],
+  };
+});
+
+// --- Comment tools (ported from upstream PR #3 by Max Nikitin) ---
+
+// Tool: list_comments
+registerShared(
+  SHARED_TOOL_SPECS.listComments,
+  async ({ pageId, includeResolved }) => {
+    const comments = await docmostClient.listComments(pageId, includeResolved);
+    return jsonContent(comments);
+  },
+);
+
+// Tool: create_comment
+// Schema + description now live in the shared registry (#294). The execute body
+// keeps this transport's own guards (require a selection for a top-level
+// comment; reject suggestedText on a reply / without a selection).
+registerShared(
+  SHARED_TOOL_SPECS.createComment,
+  async ({ pageId, content, selection, parentCommentId, suggestedText }) => {
+    if (!parentCommentId && (!selection || !selection.trim())) {
+      throw new Error(
+        "create_comment: a 'selection' (exact text to anchor on) is required for a top-level comment; omit it only when replying via parentCommentId.",
+      );
+    }
+    if (suggestedText !== undefined) {
+      if (parentCommentId) {
+        throw new Error(
+          "create_comment: 'suggestedText' cannot be attached to a reply; it applies only to a top-level inline comment.",
+        );
+      }
+      if (!selection || !selection.trim()) {
+        throw new Error(
+          "create_comment: 'suggestedText' requires a 'selection' to anchor and rewrite.",
+        );
+      }
+    }
+    const result = await docmostClient.createComment(
+      pageId,
+      content,
+      "inline",
+      selection,
+      parentCommentId,
+      suggestedText,
+    );
+    return jsonContent(result);
+  },
+);
+
 // Tool: update_comment
 server.registerTool(
   "update_comment",
@@ -380,6 +785,39 @@ server.registerTool(
   },
 );
 
+// Tool: resolve_comment
+// Schema + description now live in the shared registry (#294).
+registerShared(
+  SHARED_TOOL_SPECS.resolveComment,
+  async ({ commentId, resolved }) => {
+    const result = await docmostClient.resolveComment(commentId, resolved);
+    return jsonContent(result);
+  },
+);
+
+// Tool: check_new_comments
+// Schema + description now live in the shared registry (#294). The execute body
+// keeps this transport's own guard rejecting an unparseable `since` timestamp.
+registerShared(
+  SHARED_TOOL_SPECS.checkNewComments,
+  async ({ spaceId, since, parentPageId }) => {
+    // Reject an unparseable timestamp up front: otherwise the comparison
+    // against NaN silently treats every comment as "not new" and the tool
+    // returns zero results without signalling the bad input.
+    if (Number.isNaN(Date.parse(since))) {
+      throw new Error(
+        `Invalid 'since' timestamp: ${JSON.stringify(since)} — expected an ISO 8601 date (e.g. '2026-03-10T00:00:00Z')`,
+      );
+    }
+    const result = await docmostClient.checkNewComments(
+      spaceId,
+      since,
+      parentPageId,
+    );
+    return jsonContent(result);
+  },
+);
+
 // Tool: search
 // INTENTIONAL per-transport divergence (not shared): the in-app `searchPages`
 // runs a semantic + keyword hybrid (RRF) with in-process access control and a
@@ -489,5 +927,43 @@ server.registerTool(
   },
 );
 
+// Tool: insert_footnote
+// Schema + description now live in the shared registry (#410) so the in-app
+// AI-chat agent exposes it too. The execute body is unchanged.
+registerShared(
+  SHARED_TOOL_SPECS.insertFootnote,
+  async ({ pageId, anchorText, text }) => {
+    const result = await docmostClient.insertFootnote(pageId, anchorText, text);
+    return jsonContent(result);
+  },
+);
+
+// Tool: diff_page_versions
+registerShared(
+  SHARED_TOOL_SPECS.diffPageVersions,
+  async ({ pageId, from, to }) => {
+    const result = await docmostClient.diffPageVersions(pageId, from, to);
+    return jsonContent(result);
+  },
+);
+
+// Tool: list_page_history
+registerShared(
+  SHARED_TOOL_SPECS.listPageHistory,
+  async ({ pageId, cursor }) => {
+    const result = await docmostClient.listPageHistory(pageId, cursor);
+    return jsonContent(result);
+  },
+);
+
+// Tool: restore_page_version
+registerShared(
+  SHARED_TOOL_SPECS.restorePageVersion,
+  async ({ historyId }) => {
+    const result = await docmostClient.restorePageVersion(historyId);
+    return jsonContent(result);
+  },
+);
+
   return server;
 }

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/tool-specs.ts

@@ -24,92 +24,6 @@
 // one of them. Each builder uses only the common, stable subset of the API.
 type ZodLike = any;
 
-// The `node` normalizer shared by BOTH hosts (patch_node / insert_node /
-// update_page_json): the model sometimes serializes a ProseMirror node arg as a
-// JSON string, so we parse a string to an object (throwing a documented message
-// on invalid JSON) and pass an object through. It lives in the converter package
-// (#414) so it is the ONE copy both the MCP server and the in-app server import;
-// putting it in a shared execute here keeps that single normalization in one
-// place instead of hand-mirrored per host. Pure — safe across the zod boundary.
-import { parseNodeArg } from '@docmost/prosemirror-markdown';
-// Type-only import (erased at compile) of the real client so `DocmostClientLike`
-// is DERIVED from it (issue #446), not hand-mirrored. The loosest correct client
-// surface both hosts satisfy: the in-app host passes its own DERIVED
-// `DocmostClientLike` (a Pick of the same class) and the MCP host passes the real
-// `DocmostClient`, so both are structurally assignable to this shared alias.
-import type { DocmostClient } from './client.js';
-
-/**
- * The client surface a shared `execute` may call — the LOOSEST correct type both
- * hosts satisfy: a `Pick` of the real `DocmostClient` methods the executes below
- * use. DERIVED from the real class (issue #446) so a signature change to any
- * consumed method surfaces as a compile error in the execute bodies here, not a
- * silent runtime "wrong argument". Kept as a Pick (not the whole class) so the
- * standalone MCP host's full `DocmostClient` AND the in-app host's OWN narrower
- * `DocmostClientLike` (also a Pick of the same class, a superset of these methods)
- * are both structurally assignable to it. `import type` is fully erased, so
- * tool-specs.ts pulls in no runtime dependency on the client and still crosses the
- * zod-major boundary freely. When you add a client call to an execute below, add
- * its method name here too (a compile error will point you at it).
- */
-export type DocmostClientLike = Pick<
-  DocmostClient,
-  | 'getWorkspace'
-  | 'getSpaces'
-  | 'listShares'
-  | 'listPages'
-  | 'getPage'
-  | 'getPageJson'
-  | 'getOutline'
-  | 'getNode'
-  | 'searchInPage'
-  | 'listComments'
-  | 'checkNewComments'
-  | 'listPageHistory'
-  | 'diffPageVersions'
-  | 'exportPageMarkdown'
-  | 'createPage'
-  | 'renamePage'
-  | 'movePage'
-  | 'deletePage'
-  | 'editPageText'
-  | 'patchNode'
-  | 'insertNode'
-  | 'deleteNode'
-  | 'updatePageJson'
-  | 'tableInsertRow'
-  | 'tableDeleteRow'
-  | 'tableUpdateCell'
-  | 'copyPageContent'
-  | 'importPageMarkdown'
-  | 'sharePage'
-  | 'unsharePage'
-  | 'restorePageVersion'
-  | 'stashPage'
-  | 'insertFootnote'
-  | 'insertImage'
-  | 'replaceImage'
-  | 'drawioGet'
-  | 'drawioCreate'
-  | 'drawioUpdate'
-  | 'createComment'
-  | 'resolveComment'
->;
-
-/**
- * A shared tool `execute`: the single canonical mapping from validated schema
- * args to the client call. Plain JS — it crosses the zod-major boundary (v3 in
- * the MCP package, v4 on the server) freely, receiving the already-validated,
- * type-erased args from whichever host invoked it. It returns RAW data; the host
- * applies its own result envelope (the MCP transport wraps it as JSON text
- * content, the in-app AI-SDK host returns it as-is). Host-specific overrides
- * (`mcpExecute`/`inAppExecute`) return a value the host uses instead of wrapping.
- */
-export type SharedToolExecute = (
-  client: DocmostClientLike,
-  args: Record<string, unknown>,
-) => Promise<unknown>;
-
 export interface SharedToolSpec {
   /** snake_case tool name passed to McpServer.registerTool. */
   mcpName: string;
@@ -140,53 +54,8 @@ export interface SharedToolSpec {
    * in-app side uses z.object({})).
    */
   buildShape?: (z: ZodLike) => Record<string, unknown>;
-  /**
-   * Single canonical mapping from validated schema args to the client call,
-   * shared by BOTH hosts. Returns RAW data — the MCP host wraps it as JSON text
-   * content (jsonContent), the in-app host returns it as-is. Present on tools
-   * whose mapping AND raw result are identical across the two layers. When a host
-   * needs a genuinely different mapping or result shape, it supplies an override
-   * (below) and the host uses that INSTEAD of `execute`.
-   */
-  execute?: SharedToolExecute;
-  /**
-   * MCP-host override for a DELIBERATE per-layer difference (a guardrail, an
-   * omitted param, or a non-JSON result envelope like a resource_link / a bare
-   * success line). When present, the MCP host calls this and uses its return
-   * value VERBATIM (it is NOT re-wrapped in jsonContent), so this override owns
-   * the full MCP content envelope.
-   */
-  mcpExecute?: SharedToolExecute;
-  /**
-   * In-app-host override for a DELIBERATE per-layer difference (a projected
-   * result shape, a different guardrail message). When present, the in-app host
-   * calls this and returns its value as the tool result (no wrapping).
-   */
-  inAppExecute?: SharedToolExecute;
-  /** Registered only on the MCP host (skipped by the in-app registry loop). */
-  mcpOnly?: boolean;
-  /** Registered only on the in-app host (skipped by the MCP registry loop). */
-  inAppOnly?: boolean;
 }
 
-// --- Shared execute helpers -------------------------------------------------
-//
-// Each helper is the ONE canonical arg->client mapping for a tool (or a host
-// override where the two layers deliberately differ). They are attached to their
-// spec below. Kept as named functions (not inline) so the spec table stays
-// readable and each mapping is individually greppable/testable.
-//
-// The `args` are the host's already-validated, zod-erased input; we read the
-// same fields the tool's buildShape declares. Return RAW data unless the name is
-// an mcp*/inApp* override that owns the host's full result shape.
-
-/** Format a JSON payload as the MCP transport's text-content envelope. Mirrors
- *  the private `jsonContent` in index.ts so an mcpExecute override that must NOT
- *  be re-wrapped can still emit the standard envelope for the data part. */
-const mcpJson = (data: unknown) => ({
-  content: [{ type: 'text' as const, text: JSON.stringify(data, null, 2) }],
-});
-
 export const SHARED_TOOL_SPECS = {
   // --- no-argument read tools ---
 
@@ -196,7 +65,6 @@ export const SHARED_TOOL_SPECS = {
     description: 'Fetch metadata about the current workspace (name, settings).',
     tier: 'core',
     catalogLine: 'getWorkspace — fetch current workspace metadata (name, settings).',
-    execute: (client) => client.getWorkspace(),
   },
 
   listSpaces: {
@@ -207,7 +75,6 @@ export const SHARED_TOOL_SPECS = {
       'spaces (id, name, slug, ...).',
     tier: 'core',
     catalogLine: 'listSpaces — list the spaces the user can access (id, name, slug).',
-    execute: (client) => client.getSpaces(),
   },
 
   listShares: {
@@ -217,7 +84,6 @@ export const SHARED_TOOL_SPECS = {
       'List all public shares in the workspace with page titles and public URLs.',
     tier: 'deferred',
     catalogLine: 'listShares — list all public shares in the workspace with their URLs.',
-    execute: (client) => client.listShares(),
   },
 
   // --- single-pageId read tools ---
@@ -236,7 +102,6 @@ export const SHARED_TOOL_SPECS = {
     buildShape: (z) => ({
       pageId: z.string().min(1),
     }),
-    execute: (client, { pageId }) => client.getPageJson(pageId as string),
   },
 
   getOutline: {
@@ -254,7 +119,6 @@ export const SHARED_TOOL_SPECS = {
     buildShape: (z) => ({
       pageId: z.string().min(1),
     }),
-    execute: (client, { pageId }) => client.getOutline(pageId as string),
   },
 
   // --- two-id read tool ---
@@ -275,8 +139,6 @@ export const SHARED_TOOL_SPECS = {
       pageId: z.string().min(1),
       nodeId: z.string().min(1),
     }),
-    execute: (client, { pageId, nodeId }) =>
-      client.getNode(pageId as string, nodeId as string),
   },
 
   // --- in-page occurrence search (client-side, over ProseMirror plain text) ---
@@ -334,12 +196,6 @@ export const SHARED_TOOL_SPECS = {
         .optional()
         .describe('Max matches to RETURN (default 50, max 200); total is always reported.'),
     }),
-    execute: (client, { pageId, query, regex, caseSensitive, limit }) =>
-      client.searchInPage(pageId as string, query as string, {
-        regex: regex as boolean | undefined,
-        caseSensitive: caseSensitive as boolean | undefined,
-        limit: limit as number | undefined,
-      }),
   },
 
   // --- node delete ---
@@ -356,8 +212,6 @@ export const SHARED_TOOL_SPECS = {
       pageId: z.string().min(1),
       nodeId: z.string().min(1),
     }),
-    execute: (client, { pageId, nodeId }) =>
-      client.deleteNode(pageId as string, nodeId as string),
   },
 
   // --- single-block structural write (patch / insert) ---
@@ -405,11 +259,6 @@ export const SHARED_TOOL_SPECS = {
             'JSON object or JSON string both accepted.',
         ),
     }),
-    // parseNodeArg normalizes a JSON-string node into an object (the model
-    // sometimes serializes it as a string) before the client's typeof-object
-    // guard rejects it — identical on both hosts.
-    execute: (client, { pageId, nodeId, node }) =>
-      client.patchNode(pageId as string, nodeId as string, parseNodeArg(node)),
   },
 
   insertNode: {
@@ -463,12 +312,6 @@ export const SHARED_TOOL_SPECS = {
             'are tolerated as a fallback; prefer plain text or anchorNodeId.',
         ),
     }),
-    execute: (client, { pageId, node, position, anchorNodeId, anchorText }) =>
-      client.insertNode(pageId as string, parseNodeArg(node), {
-        position: position as 'before' | 'after' | 'append',
-        anchorNodeId: anchorNodeId as string | undefined,
-        anchorText: anchorText as string | undefined,
-      }),
   },
 
   // --- share management ---
@@ -499,11 +342,6 @@ export const SHARED_TOOL_SPECS = {
         .optional()
         .describe('Allow public search engines to index it (default true).'),
     }),
-    // `searchIndexing ?? true` is a no-op default: the client method already
-    // defaults searchIndexing to true, so passing `undefined` (the in-app form)
-    // and `?? true` (the old MCP form) are byte-identical — one canonical mapping.
-    execute: (client, { pageId, searchIndexing }) =>
-      client.sharePage(pageId as string, (searchIndexing as boolean | undefined) ?? true),
   },
 
   unsharePage: {
@@ -515,7 +353,6 @@ export const SHARED_TOOL_SPECS = {
     buildShape: (z) => ({
       pageId: z.string().min(1).describe('ID of the page to unshare'),
     }),
-    execute: (client, { pageId }) => client.unsharePage(pageId as string),
   },
 
   // --- version history ---
@@ -544,12 +381,6 @@ export const SHARED_TOOL_SPECS = {
         .optional()
         .describe("historyId, or 'current'/omit for current content"),
     }),
-    execute: (client, { pageId, from, to }) =>
-      client.diffPageVersions(
-        pageId as string,
-        from as string | undefined,
-        to as string | undefined,
-      ),
   },
 
   listPageHistory: {
@@ -569,8 +400,6 @@ export const SHARED_TOOL_SPECS = {
         .optional()
         .describe('Pagination cursor from a previous nextCursor'),
     }),
-    execute: (client, { pageId, cursor }) =>
-      client.listPageHistory(pageId as string, cursor as string | undefined),
   },
 
   restorePageVersion: {
@@ -587,8 +416,6 @@ export const SHARED_TOOL_SPECS = {
     buildShape: (z) => ({
       historyId: z.string().min(1),
     }),
-    execute: (client, { historyId }) =>
-      client.restorePageVersion(historyId as string),
   },
 
   // --- markdown round-trip ---
@@ -610,8 +437,6 @@ export const SHARED_TOOL_SPECS = {
       pageId: z.string().min(1),
       markdown: z.string().min(1),
     }),
-    execute: (client, { pageId, markdown }) =>
-      client.importPageMarkdown(pageId as string, markdown as string),
   },
 
   // --- server-side content copy ---
@@ -634,8 +459,6 @@ export const SHARED_TOOL_SPECS = {
         .min(1)
         .describe('Page whose content is REPLACED (title/slug kept)'),
     }),
-    execute: (client, { sourcePageId, targetPageId }) =>
-      client.copyPageContent(sourcePageId as string, targetPageId as string),
   },
 
   // --- surgical text edit (folds in the documented drift-bug fix) ---
@@ -685,11 +508,6 @@ export const SHARED_TOOL_SPECS = {
         .min(1)
         .describe('List of find/replace operations, applied in order'),
     }),
-    execute: (client, { pageId, edits }) =>
-      client.editPageText(
-        pageId as string,
-        edits as Parameters<DocmostClientLike['editPageText']>[1],
-      ),
   },
 
   // --- hand a large page to an external consumer without bloating context ---
@@ -718,33 +536,6 @@ export const SHARED_TOOL_SPECS = {
     buildShape: (z) => ({
       pageId: z.string().min(1),
     }),
-    // In-app returns the full documented `{ uri, size, sha256, images }` object
-    // as-is (the canonical execute).
-    execute: (client, { pageId }) => client.stashPage(pageId as string),
-    // The MCP transport must deliver the body as a resource_link (so it never
-    // enters the model context) PLUS a structuredContent mirror of the documented
-    // shape (sha256 = the blob's ETag, mirror counts). Owns its full envelope, so
-    // it is NOT wrapped in jsonContent.
-    mcpExecute: async (client, { pageId }) => {
-      const result = await client.stashPage(pageId as string);
-      return {
-        content: [
-          {
-            type: 'resource_link' as const,
-            uri: result.uri,
-            name: 'page.json',
-            mimeType: 'application/json',
-            size: result.size,
-          },
-        ],
-        structuredContent: {
-          uri: result.uri,
-          sha256: result.sha256,
-          size: result.size,
-          images: result.images,
-        },
-      };
-    },
   },
 
   // --- page tools (unified from the per-layer inline definitions, #294) ---
@@ -771,20 +562,6 @@ export const SHARED_TOOL_SPECS = {
     buildShape: (z) => ({
       pageId: z.string().min(1).describe('The id (or slugId) of the page.'),
     }),
-    // MCP wraps the raw `{ data, success }` as JSON. The in-app host instead
-    // projects a token-efficient `{ title, markdown }` (its long-standing shape).
-    execute: (client, { pageId }) => client.getPage(pageId as string),
-    inAppExecute: async (client, { pageId }) => {
-      // getPage(pageId) -> { data: filterPage(page, markdown), success }.
-      const result = (await client.getPage(pageId as string)) as {
-        data?: { title?: string; content?: string };
-      };
-      const data = result?.data ?? {};
-      return {
-        title: data.title ?? '',
-        markdown: typeof data.content === 'string' ? data.content : '',
-      };
-    },
   },
 
   listPages: {
@@ -819,15 +596,6 @@ export const SHARED_TOOL_SPECS = {
             'Requires spaceId; ignores limit.',
         ),
     }),
-    // `limit ?? 50` / `tree ?? false` are no-op defaults: the client method
-    // already defaults limit=50, tree=false, so the old MCP explicit-default form
-    // and the in-app pass-through form are byte-identical — one canonical mapping.
-    execute: (client, { spaceId, limit, tree }) =>
-      client.listPages(
-        spaceId as string | undefined,
-        (limit as number | undefined) ?? 50,
-        (tree as boolean | undefined) ?? false,
-      ),
   },
 
   createPage: {
@@ -856,28 +624,6 @@ export const SHARED_TOOL_SPECS = {
         .optional()
         .describe('Optional parent page id to nest the new page under.'),
     }),
-    // MCP wraps the raw create response as JSON. In-app projects `{ id, title }`
-    // and defensively coerces a missing body to '' (the schema makes content a
-    // required string, so `?? ''` only guards an absent field — preserved).
-    execute: (client, { title, content, spaceId, parentPageId }) =>
-      client.createPage(
-        title as string,
-        content as string,
-        spaceId as string,
-        parentPageId as string | undefined,
-      ),
-    inAppExecute: async (client, { title, content, spaceId, parentPageId }) => {
-      // createPage(title, content, spaceId, parentPageId?) ->
-      // { data: filterPage(page, markdown), success }.
-      const result = (await client.createPage(
-        title as string,
-        (content as string | undefined) ?? '',
-        spaceId as string,
-        parentPageId as string | undefined,
-      )) as { data?: { id?: string; slugId?: string; title?: string } };
-      const data = result?.data ?? {};
-      return { id: data.id ?? data.slugId, title: data.title ?? (title as string) };
-    },
   },
 
   movePage: {
@@ -915,62 +661,6 @@ export const SHARED_TOOL_SPECS = {
             'append at the end.',
         ),
     }),
-    // The MCP host keeps its robustness guards (coerce 'null'/'' -> null, a cheap
-    // self-cycle guard, and a POSITIVE { success: true } confirmation) and its
-    // human-readable success envelope — owns its full result, so it is NOT
-    // wrapped in jsonContent.
-    mcpExecute: async (client, { pageId, parentPageId, position }) => {
-      const finalParentId =
-        parentPageId === '' || parentPageId === 'null'
-          ? null
-          : (parentPageId as string | null | undefined);
-
-      // Cheap cycle guard: a page cannot be moved directly under itself.
-      // (Deeper descendant-cycle detection is intentionally out of scope.)
-      if (finalParentId !== null && finalParentId === pageId) {
-        throw new Error('cannot move a page under itself');
-      }
-
-      const result = await client.movePage(
-        pageId as string,
-        finalParentId || null,
-        position as string | undefined,
-      );
-
-      // Require POSITIVE confirmation: the live /pages/move success shape is
-      // exactly { success: true, status: 200 }. An empty body, a 204, or any odd
-      // shape lacking success === true must NOT be reported as a successful move.
-      if (
-        !(
-          result &&
-          typeof result === 'object' &&
-          (result as { success?: unknown }).success === true
-        )
-      ) {
-        throw new Error(
-          `Failed to move page ${pageId}: ${JSON.stringify(result)}`,
-        );
-      }
-
-      return mcpJson({
-        message: `Successfully moved page ${pageId} to parent ${finalParentId || 'root'}`,
-        result,
-      });
-    },
-    // The in-app host has no guards; it forwards `parentPageId ?? null` + the
-    // optional position and projects `{ pageId, parentPageId, moved }`.
-    inAppExecute: async (client, { pageId, parentPageId, position }) => {
-      await client.movePage(
-        pageId as string,
-        (parentPageId as string | null | undefined) ?? null,
-        position as string | undefined,
-      );
-      return {
-        pageId,
-        parentPageId: (parentPageId as string | null | undefined) ?? null,
-        moved: true,
-      };
-    },
   },
 
   renamePage: {
@@ -985,13 +675,6 @@ export const SHARED_TOOL_SPECS = {
       pageId: z.string().min(1).describe('The id of the page to rename.'),
       title: z.string().min(1).describe('The new title.'),
     }),
-    // MCP wraps the raw rename response; in-app projects `{ pageId, title }`.
-    execute: (client, { pageId, title }) =>
-      client.renamePage(pageId as string, title as string),
-    inAppExecute: async (client, { pageId, title }) => {
-      await client.renamePage(pageId as string, title as string);
-      return { pageId, title };
-    },
   },
 
   deletePage: {
@@ -1008,23 +691,6 @@ export const SHARED_TOOL_SPECS = {
     buildShape: (z) => ({
       pageId: z.string().min(1).describe('The id of the page to move to trash.'),
     }),
-    // deletePage(pageId) hits POST /pages/delete with { pageId } only — the
-    // soft-delete (trash) path. GUARDRAIL: the schema exposes ONLY pageId, so no
-    // permanent/force-delete flag can reach the client on either host (asserted by
-    // ai-chat-tools.service.spec.ts). MCP emits a bare success line (owns its full
-    // envelope); in-app projects `{ pageId, trashed }`.
-    mcpExecute: async (client, { pageId }) => {
-      await client.deletePage(pageId as string);
-      return {
-        content: [
-          { type: 'text' as const, text: `Successfully deleted page ${pageId}` },
-        ],
-      };
-    },
-    inAppExecute: async (client, { pageId }) => {
-      await client.deletePage(pageId as string);
-      return { pageId, trashed: true };
-    },
   },
 
   updatePageJson: {
@@ -1055,19 +721,6 @@ export const SHARED_TOOL_SPECS = {
         ),
       title: z.string().optional().describe('Optional new title'),
     }),
-    // Content normalization is identical on both hosts: only parse/validate the
-    // document when actually supplied; undefined/null passes straight through so
-    // the client performs a title-only (or no-op) update. A string is JSON.parsed
-    // (an empty string "" therefore throws), an object passes through unchanged.
-    execute: (client, { pageId, content, title }) => {
-      let doc: unknown;
-      if (content === undefined || content === null) {
-        doc = undefined;
-      } else {
-        doc = parseNodeArg(content, 'content was a string but not valid JSON');
-      }
-      return client.updatePageJson(pageId as string, doc, title as string | undefined);
-    },
   },
 
   exportPageMarkdown: {
@@ -1088,17 +741,6 @@ export const SHARED_TOOL_SPECS = {
     buildShape: (z) => ({
       pageId: z.string().min(1).describe('The id of the page to export.'),
     }),
-    // The markdown is a bare string. MCP returns it as a single text-content
-    // element (NOT jsonContent — that would JSON-quote the whole document);
-    // in-app projects `{ markdown }`.
-    mcpExecute: async (client, { pageId }) => {
-      const md = await client.exportPageMarkdown(pageId as string);
-      return { content: [{ type: 'text' as const, text: md }] };
-    },
-    inAppExecute: async (client, { pageId }) => {
-      const markdown = await client.exportPageMarkdown(pageId as string);
-      return { markdown };
-    },
   },
 
   // --- comment tools (unified from the per-layer inline definitions, #294) ---
@@ -1181,69 +823,6 @@ export const SHARED_TOOL_SPECS = {
             'refused.',
         ),
     }),
-    // Both hosts enforce the SAME guardrails (a top-level comment requires a
-    // selection; suggestedText is forbidden on a reply / without a selection) but
-    // with per-layer error wording (snake_case 'create_comment:' on the MCP
-    // surface, camelCase 'createComment' in-app) and different result shapes (MCP
-    // jsonContent, in-app projects `{ commentId, pageId }`). Preserved byte-for-
-    // byte via the two overrides.
-    mcpExecute: async (client, { pageId, content, selection, parentCommentId, suggestedText }) => {
-      if (!parentCommentId && (!selection || !(selection as string).trim())) {
-        throw new Error(
-          "create_comment: a 'selection' (exact text to anchor on) is required for a top-level comment; omit it only when replying via parentCommentId.",
-        );
-      }
-      if (suggestedText !== undefined) {
-        if (parentCommentId) {
-          throw new Error(
-            "create_comment: 'suggestedText' cannot be attached to a reply; it applies only to a top-level inline comment.",
-          );
-        }
-        if (!selection || !(selection as string).trim()) {
-          throw new Error(
-            "create_comment: 'suggestedText' requires a 'selection' to anchor and rewrite.",
-          );
-        }
-      }
-      const result = await client.createComment(
-        pageId as string,
-        content as string,
-        'inline',
-        selection as string | undefined,
-        parentCommentId as string | undefined,
-        suggestedText as string | undefined,
-      );
-      return mcpJson(result);
-    },
-    inAppExecute: async (client, { pageId, content, selection, parentCommentId, suggestedText }) => {
-      if (!parentCommentId && (!selection || !(selection as string).trim())) {
-        throw new Error(
-          "createComment requires a 'selection' (exact text to anchor on) for a new top-level comment.",
-        );
-      }
-      if (suggestedText !== undefined) {
-        if (parentCommentId) {
-          throw new Error(
-            "createComment: 'suggestedText' cannot be attached to a reply; it applies only to a top-level inline comment.",
-          );
-        }
-        if (!selection || !(selection as string).trim()) {
-          throw new Error(
-            "createComment: 'suggestedText' requires a 'selection' to anchor and rewrite.",
-          );
-        }
-      }
-      const result = (await client.createComment(
-        pageId as string,
-        content as string,
-        'inline',
-        selection as string | undefined,
-        parentCommentId as string | undefined,
-        suggestedText as string | undefined,
-      )) as { data?: { id?: string } };
-      const data = result?.data ?? {};
-      return { commentId: data.id, pageId };
-    },
   },
 
   listComments: {
@@ -1269,8 +848,6 @@ export const SHARED_TOOL_SPECS = {
         .optional()
         .describe('default only active threads; true — include resolved'),
     }),
-    execute: (client, { pageId, includeResolved }) =>
-      client.listComments(pageId as string, includeResolved as boolean | undefined),
   },
 
   resolveComment: {
@@ -1304,13 +881,6 @@ export const SHARED_TOOL_SPECS = {
           'true (default) marks the thread resolved/closed; false reopens it',
         ),
     }),
-    // MCP wraps the raw resolve response; in-app projects `{ commentId, resolved }`.
-    execute: (client, { commentId, resolved }) =>
-      client.resolveComment(commentId as string, resolved as boolean),
-    inAppExecute: async (client, { commentId, resolved }) => {
-      await client.resolveComment(commentId as string, resolved as boolean);
-      return { commentId, resolved };
-    },
   },
 
   checkNewComments: {
@@ -1346,30 +916,6 @@ export const SHARED_TOOL_SPECS = {
             'Only pages under this parent will be checked.',
         ),
     }),
-    // The in-app host has NO `since` guard (the canonical execute, raw). The MCP
-    // host additionally rejects an unparseable `since` up front — otherwise the
-    // NaN comparison silently treats every comment as "not new" and returns zero
-    // without signalling the bad input. This guard is a DELIBERATE per-layer
-    // difference (the in-app surface never had it), preserved via mcpExecute.
-    execute: (client, { spaceId, since, parentPageId }) =>
-      client.checkNewComments(
-        spaceId as string,
-        since as string,
-        parentPageId as string | undefined,
-      ),
-    mcpExecute: async (client, { spaceId, since, parentPageId }) => {
-      if (Number.isNaN(Date.parse(since as string))) {
-        throw new Error(
-          `Invalid 'since' timestamp: ${JSON.stringify(since)} — expected an ISO 8601 date (e.g. '2026-03-10T00:00:00Z')`,
-        );
-      }
-      const result = await client.checkNewComments(
-        spaceId as string,
-        since as string,
-        parentPageId as string | undefined,
-      );
-      return mcpJson(result);
-    },
   },
 
   // --- table tools (unified from the per-layer inline definitions, #294) ---
@@ -1416,13 +962,6 @@ export const SHARED_TOOL_SPECS = {
         .optional()
         .describe('0-based insert position (0 inserts before the header); omit to append.'),
     }),
-    execute: (client, { pageId, table, cells, index }) =>
-      client.tableInsertRow(
-        pageId as string,
-        table as string,
-        cells as string[],
-        index as number | undefined,
-      ),
   },
 
   tableDeleteRow: {
@@ -1444,8 +983,6 @@ export const SHARED_TOOL_SPECS = {
         .describe('"#<index>" from the page outline, or a block id in the table.'),
       index: z.number().int().describe('0-based row index to delete.'),
     }),
-    execute: (client, { pageId, table, index }) =>
-      client.tableDeleteRow(pageId as string, table as string, index as number),
   },
 
   tableUpdateCell: {
@@ -1469,14 +1006,6 @@ export const SHARED_TOOL_SPECS = {
       col: z.number().int().describe('0-based column index.'),
       text: z.string().describe('The new cell text.'),
     }),
-    execute: (client, { pageId, table, row, col, text }) =>
-      client.tableUpdateCell(
-        pageId as string,
-        table as string,
-        row as number,
-        col as number,
-        text as string,
-      ),
   },
 
   // --- footnote + image write tools (promoted from inline MCP-only, #410) ---
@@ -1521,8 +1050,6 @@ export const SHARED_TOOL_SPECS = {
         .min(1)
         .describe('The footnote content as markdown (becomes the definition).'),
     }),
-    execute: (client, { pageId, anchorText, text }) =>
-      client.insertFootnote(pageId as string, anchorText as string, text as string),
   },
 
   insertImage: {
@@ -1560,13 +1087,6 @@ export const SHARED_TOOL_SPECS = {
           'Insert the image right after the first top-level block whose text contains this string',
         ),
     }),
-    execute: (client, { pageId, imageUrl, align, alt, replaceText, afterText }) =>
-      client.insertImage(pageId as string, imageUrl as string, {
-        align: align as 'left' | 'center' | 'right' | undefined,
-        alt: alt as string | undefined,
-        replaceText: replaceText as string | undefined,
-        afterText: afterText as string | undefined,
-      }),
   },
 
   replaceImage: {
@@ -1598,11 +1118,6 @@ export const SHARED_TOOL_SPECS = {
       align: z.enum(['left', 'center', 'right']).optional(),
       alt: z.string().optional(),
     }),
-    execute: (client, { pageId, attachmentId, imageUrl, align, alt }) =>
-      client.replaceImage(pageId as string, attachmentId as string, imageUrl as string, {
-        align: align as 'left' | 'center' | 'right' | undefined,
-        alt: alt as string | undefined,
-      }),
   },
 
   // --- draw.io diagrams (issue #423, stage 1) ---
@@ -1632,12 +1147,6 @@ export const SHARED_TOOL_SPECS = {
         .optional()
         .describe('"xml" (default) for mxGraph XML, or "svg" for the raw .drawio.svg.'),
     }),
-    execute: (client, { pageId, node, format }) =>
-      client.drawioGet(
-        pageId as string,
-        node as string,
-        (format as 'xml' | 'svg' | undefined) ?? 'xml',
-      ),
   },
 
   drawioCreate: {
@@ -1684,18 +1193,6 @@ export const SHARED_TOOL_SPECS = {
         .describe('Anchor text fragment (for before/after).'),
       title: z.string().optional().describe('Optional diagram title.'),
     }),
-    // The flat schema fields are regrouped into the client's `where` object.
-    execute: (client, { pageId, xml, position, anchorNodeId, anchorText, title }) =>
-      client.drawioCreate(
-        pageId as string,
-        {
-          position: position as 'before' | 'after' | 'append',
-          anchorNodeId: anchorNodeId as string | undefined,
-          anchorText: anchorText as string | undefined,
-        },
-        xml as string,
-        title as string | undefined,
-      ),
   },
 
   drawioUpdate: {
@@ -1729,12 +1226,5 @@ export const SHARED_TOOL_SPECS = {
         .min(1)
         .describe('The meta.hash from the drawio_get this edit is based on.'),
     }),
-    execute: (client, { pageId, node, xml, baseHash }) =>
-      client.drawioUpdate(
-        pageId as string,
-        node as string,
-        xml as string,
-        baseHash as string,
-      ),
   },
 } 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/pagination-cursor.test.mjs

@@ -1,440 +0,0 @@
-// Mock-HTTP tests for the cursor-pagination migration in DocmostClient (#442).
-//
-// The server switched its list endpoints from OFFSET (`page`) to CURSOR
-// (`cursor`/`nextCursor`) pagination, and the global ValidationPipe silently
-// strips the obsolete `page` field — so the old offset loops re-fetched page
-// one forever (hasNextPage stuck true), dropping every item past the first
-// page. These tests pin the new cursor behaviour and the immovable-cursor
-// guard that prevents a silent spin/duplication if the protocol drifts again.
-//
-// A local http.createServer stands in for Docmost so everything stays
-// deterministic and offline (same harness style as reauth.test.mjs).
-import { test, after } from "node:test";
-import assert from "node:assert/strict";
-import http from "node:http";
-import { DocmostClient } from "../../build/client.js";
-
-function readBody(req) {
-  return new Promise((resolve) => {
-    let raw = "";
-    req.on("data", (chunk) => {
-      raw += chunk;
-    });
-    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 closeServer(server) {
-  return new Promise((resolve) => server.close(resolve));
-}
-
-function sendJson(res, status, obj, extraHeaders = {}) {
-  res.writeHead(status, { "Content-Type": "application/json", ...extraHeaders });
-  res.end(JSON.stringify(obj));
-}
-
-const openServers = [];
-async function spawn(handler) {
-  const { server, baseURL } = await startServer(handler);
-  openServers.push(server);
-  return { server, baseURL };
-}
-
-after(async () => {
-  await Promise.all(openServers.map((s) => closeServer(s)));
-});
-
-// A login handler shared by every server below.
-function handleLogin(req, res) {
-  if (req.url === "/api/auth/login") {
-    sendJson(res, 200, { success: true }, {
-      "Set-Cookie": "authToken=t; Path=/; HttpOnly",
-    });
-    return true;
-  }
-  return false;
-}
-
-// -----------------------------------------------------------------------------
-// 1) listSidebarPages: collects every cursor page; #requests == #pages.
-// -----------------------------------------------------------------------------
-test("listSidebarPages walks all cursor pages and collects every item", async () => {
-  // Three pages keyed by the cursor the client sends back.
-  const PAGES = {
-    "": { items: [{ id: "a" }, { id: "b" }], nextCursor: "c1" },
-    c1: { items: [{ id: "c" }, { id: "d" }], nextCursor: "c2" },
-    c2: { items: [{ id: "e" }], nextCursor: null },
-  };
-  let requests = 0;
-  const sentLimits = [];
-
-  const { baseURL } = await spawn(async (req, res) => {
-    const raw = await readBody(req);
-    if (handleLogin(req, res)) return;
-    if (req.url === "/api/pages/sidebar-pages") {
-      requests++;
-      const body = JSON.parse(raw || "{}");
-      sentLimits.push(body.limit);
-      const page = PAGES[body.cursor ?? ""] ?? { items: [], nextCursor: null };
-      sendJson(res, 200, {
-        success: true,
-        data: {
-          items: page.items,
-          meta: {
-            hasNextPage: page.nextCursor != null,
-            nextCursor: page.nextCursor,
-          },
-        },
-      });
-      return;
-    }
-    sendJson(res, 404, {});
-  });
-
-  const client = new DocmostClient(baseURL, "user@example.com", "pw");
-  const all = await client.listSidebarPages("space-1");
-
-  assert.equal(requests, 3, "one request per cursor page");
-  assert.deepEqual(
-    all.map((p) => p.id),
-    ["a", "b", "c", "d", "e"],
-    "all items across all pages collected in order",
-  );
-  assert.ok(
-    sentLimits.every((l) => l === 100),
-    "requests limit:100 (server-side max)",
-  );
-});
-
-// -----------------------------------------------------------------------------
-// 2) REGRESSION on the bug class: server IGNORES the cursor param and always
-//    returns page one with hasNextPage:true -> the immovable-cursor guard must
-//    terminate the loop with no duplicates, NOT spin to MAX_PAGES.
-// -----------------------------------------------------------------------------
-test("listSidebarPages terminates (no dups) when the server ignores the cursor", async () => {
-  let requests = 0;
-
-  const { baseURL } = await spawn(async (req, res) => {
-    await readBody(req);
-    if (handleLogin(req, res)) return;
-    if (req.url === "/api/pages/sidebar-pages") {
-      requests++;
-      // Always the SAME first page with hasNextPage:true and the SAME cursor,
-      // exactly as a server that no longer understands our pagination param.
-      sendJson(res, 200, {
-        success: true,
-        data: {
-          items: [{ id: "x1" }, { id: "x2" }],
-          meta: { hasNextPage: true, nextCursor: "stuck" },
-        },
-      });
-      return;
-    }
-    sendJson(res, 404, {});
-  });
-
-  const client = new DocmostClient(baseURL, "user@example.com", "pw");
-  const all = await client.listSidebarPages("space-1");
-
-  // Request 1 (no cursor) gets "stuck"; request 2 (cursor "stuck") gets "stuck"
-  // again -> guard trips. Far below the MAX_PAGES=50 ceiling; no runaway dups.
-  assert.equal(requests, 2, "stops as soon as the cursor stops moving");
-  assert.equal(all.length, 4, "no runaway accumulation / duplication");
-});
-
-// -----------------------------------------------------------------------------
-// 3a) enumerateSpacePages happy path: a SINGLE /pages/tree request.
-// -----------------------------------------------------------------------------
-test("enumerateSpacePages (via list_pages tree) uses one /pages/tree request", async () => {
-  let treeRequests = 0;
-  let sidebarRequests = 0;
-  let treeBody = null;
-
-  const NODES = [
-    { id: "r1", slugId: "r1s", title: "Root 1", parentPageId: null, hasChildren: true, spaceId: "space-1", position: "a", icon: null, canEdit: true },
-    { id: "c1", slugId: "c1s", title: "Child 1", parentPageId: "r1", hasChildren: false, spaceId: "space-1", position: "a", icon: null, canEdit: true },
-    { id: "r2", slugId: "r2s", title: "Root 2", parentPageId: null, hasChildren: false, spaceId: "space-1", position: "b", icon: null, canEdit: true },
-  ];
-
-  const { baseURL } = await spawn(async (req, res) => {
-    const raw = await readBody(req);
-    if (handleLogin(req, res)) return;
-    if (req.url === "/api/pages/tree") {
-      treeRequests++;
-      treeBody = JSON.parse(raw || "{}");
-      sendJson(res, 200, { success: true, data: { items: NODES } });
-      return;
-    }
-    if (req.url === "/api/pages/sidebar-pages") {
-      sidebarRequests++;
-      sendJson(res, 200, { success: true, data: { items: [], meta: {} } });
-      return;
-    }
-    sendJson(res, 404, {});
-  });
-
-  const client = new DocmostClient(baseURL, "user@example.com", "pw");
-  // list_pages tree:true -> enumerateSpacePages(spaceId) -> buildPageTree.
-  const tree = await client.listPages("space-1", 50, true);
-
-  assert.equal(treeRequests, 1, "exactly one /pages/tree request for the space");
-  assert.equal(sidebarRequests, 0, "no per-node sidebar BFS requests");
-  assert.deepEqual(treeBody, { spaceId: "space-1" }, "space scope posts spaceId only");
-  // buildPageTree nests c1 under r1; two roots at the top level.
-  assert.equal(tree.length, 2, "two root nodes");
-  const r1 = tree.find((n) => n.id === "r1");
-  assert.equal(r1.children.length, 1, "child nested under its root");
-  assert.equal(r1.children[0].id, "c1");
-});
-
-// -----------------------------------------------------------------------------
-// 3b) enumerateSpacePages fallback: /pages/tree 404 -> cursor BFS via sidebar.
-// -----------------------------------------------------------------------------
-test("enumerateSpacePages falls back to the cursor BFS on /pages/tree 404", async () => {
-  let treeRequests = 0;
-  const sidebarCalls = [];
-
-  // Root level: one root with children. Child level (pageId=r1): one leaf.
-  const { baseURL } = await spawn(async (req, res) => {
-    const raw = await readBody(req);
-    if (handleLogin(req, res)) return;
-    if (req.url === "/api/pages/tree") {
-      treeRequests++;
-      // Stock upstream Docmost has no /pages/tree.
-      sendJson(res, 404, { message: "Not Found" });
-      return;
-    }
-    if (req.url === "/api/pages/sidebar-pages") {
-      const body = JSON.parse(raw || "{}");
-      sidebarCalls.push(body.pageId ?? "<root>");
-      if (!body.pageId) {
-        sendJson(res, 200, {
-          success: true,
-          data: {
-            items: [
-              { id: "r1", title: "Root", parentPageId: null, hasChildren: true },
-            ],
-            meta: { hasNextPage: false, nextCursor: null },
-          },
-        });
-      } else if (body.pageId === "r1") {
-        sendJson(res, 200, {
-          success: true,
-          data: {
-            items: [
-              { id: "c1", title: "Leaf", parentPageId: "r1", hasChildren: false },
-            ],
-            meta: { hasNextPage: false, nextCursor: null },
-          },
-        });
-      } else {
-        sendJson(res, 200, {
-          success: true,
-          data: { items: [], meta: { hasNextPage: false, nextCursor: null } },
-        });
-      }
-      return;
-    }
-    sendJson(res, 404, {});
-  });
-
-  const client = new DocmostClient(baseURL, "user@example.com", "pw");
-  const tree = await client.listPages("space-1", 50, true);
-
-  assert.ok(treeRequests >= 1, "the tree endpoint was attempted first");
-  assert.deepEqual(
-    sidebarCalls,
-    ["<root>", "r1"],
-    "fell back to the sidebar BFS: roots then the root's children",
-  );
-  assert.equal(tree.length, 1, "one root in the built tree");
-  assert.equal(tree[0].children[0].id, "c1", "leaf nested via the BFS");
-});
-
-// -----------------------------------------------------------------------------
-// 3c) enumerateSpacePages fallback SUBTREE: /pages/tree 404 + a rootPageId ->
-//     the ROOT page itself must be seeded (via getPageRaw) so its own comments
-//     aren't dropped. listSidebarPages(spaceId, root) returns only the root's
-//     CHILDREN, so without the seed the root would be absent. (Finding 1.)
-// -----------------------------------------------------------------------------
-test("enumerateSpacePages fallback subtree seeds the ROOT page itself", async () => {
-  const sidebarCalls = [];
-  let infoRequests = 0;
-  const commentedPages = [];
-
-  const { baseURL } = await spawn(async (req, res) => {
-    const raw = await readBody(req);
-    if (handleLogin(req, res)) return;
-    if (req.url === "/api/pages/tree") {
-      // Stock upstream Docmost -> fall back to the BFS.
-      sendJson(res, 404, { message: "Not Found" });
-      return;
-    }
-    if (req.url === "/api/pages/info") {
-      // getPageRaw for the root seed. Shape mirrors a real page-info response.
-      infoRequests++;
-      sendJson(res, 200, {
-        success: true,
-        data: { id: "root", title: "Root", spaceId: "space-1", hasChildren: true },
-      });
-      return;
-    }
-    if (req.url === "/api/pages/sidebar-pages") {
-      const body = JSON.parse(raw || "{}");
-      sidebarCalls.push(body.pageId ?? "<root>");
-      // Children of the root: one leaf. (Root itself is NOT in this list.)
-      const items =
-        body.pageId === "root"
-          ? [{ id: "leaf", title: "Leaf", parentPageId: "root", hasChildren: false }]
-          : [];
-      sendJson(res, 200, {
-        success: true,
-        data: { items, meta: { hasNextPage: false, nextCursor: null } },
-      });
-      return;
-    }
-    if (req.url === "/api/comments") {
-      const body = JSON.parse(raw || "{}");
-      commentedPages.push(body.pageId);
-      const items =
-        body.pageId === "root"
-          ? [{ id: "cm1", createdAt: "2030-01-01T00:00:00.000Z", content: null }]
-          : [];
-      sendJson(res, 200, {
-        success: true,
-        data: { items, meta: { nextCursor: null } },
-      });
-      return;
-    }
-    sendJson(res, 404, {});
-  });
-
-  const client = new DocmostClient(baseURL, "user@example.com", "pw");
-  // checkNewComments(space, since, parentPageId) exercises the subtree fallback.
-  const result = await client.checkNewComments(
-    "space-1",
-    "2020-01-01T00:00:00.000Z",
-    "root",
-  );
-
-  assert.equal(infoRequests, 1, "root was seeded via one getPageRaw");
-  assert.equal(sidebarCalls[0], "root", "BFS walked the root's children");
-  assert.ok(
-    commentedPages.includes("root"),
-    "the ROOT page is in scope (its comments were fetched) — not dropped",
-  );
-  assert.ok(commentedPages.includes("leaf"), "the descendant is in scope too");
-  assert.equal(result.checkedPages, 2, "root + one descendant scanned");
-  assert.equal(result.totalNewComments, 1, "the root's fresh comment found");
-});
-
-// -----------------------------------------------------------------------------
-// 5) listComments immovable-cursor guard: the server IGNORES the cursor and
-//    keeps returning the same nextCursor -> the loop must terminate (no
-//    infinite loop, no duplicates), not spin forever. (Finding 4.)
-// -----------------------------------------------------------------------------
-test("listComments terminates (no dups) when the server ignores the cursor", async () => {
-  let requests = 0;
-
-  const { baseURL } = await spawn(async (req, res) => {
-    await readBody(req);
-    if (handleLogin(req, res)) return;
-    if (req.url === "/api/comments") {
-      requests++;
-      // Always the SAME page with the SAME nextCursor, as a server that no
-      // longer advances the cursor would.
-      sendJson(res, 200, {
-        success: true,
-        data: {
-          items: [{ id: "cm1", createdAt: "2030-01-01T00:00:00.000Z", content: null }],
-          meta: { nextCursor: "stuck" },
-        },
-      });
-      return;
-    }
-    sendJson(res, 404, {});
-  });
-
-  const client = new DocmostClient(baseURL, "user@example.com", "pw");
-  const { items } = await client.listComments("page-1", true);
-
-  // Request 1 (no cursor) gets "stuck"; request 2 (cursor "stuck") gets "stuck"
-  // again -> guard trips. Bounded far below MAX_PAGES=50, no runaway dups.
-  assert.equal(requests, 2, "stops as soon as the cursor stops moving");
-  assert.equal(items.length, 2, "no runaway accumulation / duplication");
-});
-
-// -----------------------------------------------------------------------------
-// 4) check_new_comments subtree: the root is included in scope WITHOUT a
-//    separate getPageRaw (/pages/info) request for the parent.
-// -----------------------------------------------------------------------------
-test("checkNewComments subtree includes the root without a separate getPageRaw", async () => {
-  let pageInfoRequests = 0;
-  let treeBody = null;
-  const commentedPages = [];
-
-  // /pages/tree (subtree) returns the parent itself plus a descendant, exactly
-  // as getPageAndDescendants seeds with id = parentPageId.
-  const NODES = [
-    { id: "parent", title: "Parent", parentPageId: null, hasChildren: true },
-    { id: "kid", title: "Kid", parentPageId: "parent", hasChildren: false },
-  ];
-
-  const { baseURL } = await spawn(async (req, res) => {
-    const raw = await readBody(req);
-    if (handleLogin(req, res)) return;
-    if (req.url === "/api/pages/tree") {
-      treeBody = JSON.parse(raw || "{}");
-      sendJson(res, 200, { success: true, data: { items: NODES } });
-      return;
-    }
-    if (req.url === "/api/pages/info") {
-      // If checkNewComments still fetched the parent separately this would fire.
-      pageInfoRequests++;
-      sendJson(res, 200, { success: true, data: { id: "parent" } });
-      return;
-    }
-    if (req.url === "/api/comments") {
-      const body = JSON.parse(raw || "{}");
-      commentedPages.push(body.pageId);
-      // One fresh comment on the parent, none elsewhere.
-      const items =
-        body.pageId === "parent"
-          ? [{ id: "cm1", createdAt: "2030-01-01T00:00:00.000Z", content: null }]
-          : [];
-      sendJson(res, 200, {
-        success: true,
-        data: { items, meta: { nextCursor: null } },
-      });
-      return;
-    }
-    sendJson(res, 404, {});
-  });
-
-  const client = new DocmostClient(baseURL, "user@example.com", "pw");
-  const result = await client.checkNewComments(
-    "space-1",
-    "2020-01-01T00:00:00.000Z",
-    "parent",
-  );
-
-  assert.equal(pageInfoRequests, 0, "no separate getPageRaw for the root");
-  assert.deepEqual(treeBody, { pageId: "parent" }, "subtree scope posts pageId");
-  assert.ok(
-    commentedPages.includes("parent"),
-    "the root itself is in scope (comments fetched for it)",
-  );
-  assert.ok(commentedPages.includes("kid"), "descendants are in scope too");
-  assert.equal(result.checkedPages, 2, "root + one descendant scanned");
-  assert.equal(result.totalNewComments, 1, "the root's fresh comment found");
-});

packages/mcp/test/mock/reauth.test.mjs

@@ -297,12 +297,12 @@ test("a response with ONLY authTokenRefresh (no authToken) rejects login", async
 // -----------------------------------------------------------------------------
 // 5) paginateAll loop guards.
 // -----------------------------------------------------------------------------
-test("paginateAll stops at the MAX_PAGES cap when the server always issues a fresh cursor", async () => {
+test("paginateAll stops at the MAX_PAGES cap when hasNextPage is always true", async () => {
   let pageRequests = 0;
   const LIMIT = 100;
 
   const { baseURL } = await spawn(async (req, res) => {
-    const body = JSON.parse((await readBody(req)) || "{}");
+    await readBody(req);
     if (req.url === "/api/auth/login") {
       sendJson(res, 200, { success: true }, {
         "Set-Cookie": "authToken=t; Path=/; HttpOnly",
@@ -311,18 +311,15 @@ test("paginateAll stops at the MAX_PAGES cap when the server always issues a fre
     }
     if (req.url === "/api/spaces") {
       pageRequests++;
-      // Always return a FULL page AND hasNextPage:true with a FRESH nextCursor
-      // that differs from the one the client just sent, so the immovable-cursor
-      // guard never trips — only the MAX_PAGES ceiling can stop the loop.
+      // Always return a FULL page (== requested limit) AND hasNextPage:true.
+      // Both the page-length check and the hasNextPage flag say "keep going",
+      // so only the MAX_PAGES ceiling can stop the loop.
       const items = Array.from({ length: LIMIT }, (_, i) => ({
         id: `s-${pageRequests}-${i}`,
       }));
       sendJson(res, 200, {
         success: true,
-        data: {
-          items,
-          meta: { hasNextPage: true, nextCursor: `cursor-${pageRequests}` },
-        },
+        data: { items, meta: { hasNextPage: true } },
       });
       return;
     }
@@ -341,7 +338,7 @@ test("paginateAll stops at the MAX_PAGES cap when the server always issues a fre
   assert.equal(all.length, 50 * LIMIT, "accumulates one full page per request");
 });
 
-test("paginateAll stops on the immovable-cursor guard when the server ignores the cursor param", async () => {
+test("paginateAll stops early on a short page even if hasNextPage is true", async () => {
   let pageRequests = 0;
   const LIMIT = 100;
 
@@ -355,17 +352,15 @@ test("paginateAll stops on the immovable-cursor guard when the server ignores th
     }
     if (req.url === "/api/spaces") {
       pageRequests++;
-      // The bug class: the server IGNORES the pagination param and keeps
-      // returning page one with hasNextPage:true and the SAME nextCursor. The
-      // immovable-cursor guard must stop the loop instead of spinning to
-      // MAX_PAGES and duplicating items.
-      const items = Array.from({ length: LIMIT }, (_, i) => ({ id: `s-${i}` }));
+      // First page is full; second page is SHORT (fewer than limit). The short
+      // page must stop the loop immediately even though hasNextPage stays true.
+      const count = pageRequests === 1 ? LIMIT : 3;
+      const items = Array.from({ length: count }, (_, i) => ({
+        id: `s-${pageRequests}-${i}`,
+      }));
       sendJson(res, 200, {
         success: true,
-        data: {
-          items,
-          meta: { hasNextPage: true, nextCursor: "stuck" },
-        },
+        data: { items, meta: { hasNextPage: true } },
       });
       return;
     }
@@ -375,10 +370,8 @@ test("paginateAll stops on the immovable-cursor guard when the server ignores th
   const client = new DocmostClient(baseURL, "user@example.com", "pw");
   const all = await client.paginateAll("/spaces", {}, LIMIT);
 
-  // Request 1 sends no cursor and receives "stuck"; request 2 sends "stuck" and
-  // receives "stuck" again -> guard trips after exactly two requests, no dups.
-  assert.equal(pageRequests, 2, "stops once the cursor stops moving");
-  assert.equal(all.length, 2 * LIMIT, "no runaway accumulation past the guard");
+  assert.equal(pageRequests, 2, "stops right after the first short page");
+  assert.equal(all.length, LIMIT + 3, "full page + short page accumulated");
 });
 
 test("paginateAll handles both {data:{items,meta}} and {items,meta} envelopes", async () => {
@@ -394,16 +387,16 @@ test("paginateAll handles both {data:{items,meta}} and {items,meta} envelopes",
     }
     if (req.url === "/api/groups") {
       bareRequests.push(1);
-      // Page 1: hasNextPage true with a next cursor. Page 2: no next -> stop.
+      // Page 1: full page, hasNextPage true. Page 2: short page -> stop.
       if (bareRequests.length === 1) {
         sendJson(res, 200, {
           items: Array.from({ length: 100 }, (_, i) => ({ id: `g${i}` })),
-          meta: { hasNextPage: true, nextCursor: "c2" },
+          meta: { hasNextPage: true },
         });
       } else {
         sendJson(res, 200, {
           items: [{ id: "tail" }],
-          meta: { hasNextPage: false, nextCursor: null },
+          meta: { hasNextPage: false },
         });
       }
       return;

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

@@ -0,0 +1,224 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import { fileURLToPath } from "node:url";
+import { dirname, resolve } from "node:path";
+
+import { DocmostClient } from "../../build/index.js";
+
+// Drift guard for the THIRD hand-written layer of the AI tool set (issue #193,
+// layer 3): the in-app server hand-mirrors the DocmostClient method signatures
+// it consumes as the `DocmostClientLike` interface in
+// apps/server/src/core/ai-chat/tools/docmost-client.loader.ts ("Signatures here
+// mirror that file exactly"). That mirror lives across the ESM(mcp)/CJS(server)
+// boundary and the package ships NO .d.ts, so the server typecheck cannot verify
+// the names against the real class — a rename/removal in client.ts would surface
+// only as a runtime "x is not a function" inside an agent tool call.
+//
+// SCOPE: this guard checks the method-NAME set only, not signatures. It pins the
+// contract from the mcp side (ESM, where the real class is directly importable):
+// every method the embedding host depends on MUST exist as a function on a real
+// DocmostClient instance. If you rename/remove a client method, this fails here
+// AND you must update DocmostClientLike to match. It does NOT verify parameter or
+// return-type parity — signature drift between the hand-mirror and client.ts can
+// still ship silently; full signature/type parity is the deferred staged-plan
+// item below.
+//
+// Keep the HOST_CONTRACT_METHODS NAME list aligned with the method NAMES declared
+// in the server's DocmostClientLike interface (the in-app per-user tool adapter
+// only — it is a SUBSET of the DocmostClient surface — covers only what the in-app adapter
+// consumes; the standalone MCP transport (packages/mcp/src/index.ts) calls additional
+// client methods (deleteComment/updateComment) that this guard does NOT track — the
+// MCP transport's own typecheck covers those. insertImage/replaceImage/insertFootnote
+// were MCP-only but are now in-app-consumed too (#410), so they ARE tracked below. Full type-derivation
+// of DocmostClientLike from this class is deferred (see the staged plan in
+// docmost-client.loader.ts): the package emits no declarations and the real
+// (inferred, concrete) return types conflict with the host's loose
+// `Record<string,unknown>` + `as`-cast result handling.
+const HOST_CONTRACT_METHODS = [
+  // read
+  "search",
+  "getPage",
+  "getPageRaw",
+  "getWorkspace",
+  "getSpaces",
+  "listPages",
+  "listSidebarPages",
+  "getOutline",
+  "getPageJson",
+  "getNode",
+  "searchInPage",
+  "getTable",
+  "listComments",
+  "getComment",
+  "checkNewComments",
+  "listShares",
+  "listPageHistory",
+  "getPageHistory",
+  "diffPageVersions",
+  "exportPageMarkdown",
+  // write (page)
+  "createPage",
+  "updatePage",
+  "renamePage",
+  "movePage",
+  "deletePage",
+  "editPageText",
+  "patchNode",
+  "insertNode",
+  "deleteNode",
+  "updatePageJson",
+  "tableInsertRow",
+  "tableDeleteRow",
+  "tableUpdateCell",
+  "copyPageContent",
+  "importPageMarkdown",
+  "sharePage",
+  "unsharePage",
+  "restorePageVersion",
+  "transformPage",
+  "stashPage",
+  // write (image / footnote) — MCP-only until #410 promoted them to in-app tools
+  "insertImage",
+  "replaceImage",
+  "insertFootnote",
+  // draw.io diagrams (#423, stage 1) — read + create + optimistic-locked update
+  "drawioGet",
+  "drawioCreate",
+  "drawioUpdate",
+  // write (comment)
+  "createComment",
+  "resolveComment",
+];
+
+test("DocmostClient implements every method the in-app DocmostClientLike mirror declares", () => {
+  // The constructor is side-effect-free (no network/login on construction): it
+  // only stores config and creates an axios instance, so it is safe to build a
+  // throwaway instance here with a dummy token provider.
+  const client = new DocmostClient({
+    apiUrl: "http://127.0.0.1:1/api",
+    getToken: async () => "test-token",
+  });
+
+  const missing = HOST_CONTRACT_METHODS.filter(
+    (name) => typeof client[name] !== "function",
+  );
+
+  assert.deepEqual(
+    missing,
+    [],
+    `DocmostClient is missing host-contract method(s): ${missing.join(", ")}. ` +
+      `Update packages/mcp/src/client.ts and/or the server's DocmostClientLike ` +
+      `interface (apps/server/src/core/ai-chat/tools/docmost-client.loader.ts) ` +
+      `so the hand-mirrored method NAMES stay aligned (this guards names only, ` +
+      `not signatures).`,
+  );
+});
+
+test("HOST_CONTRACT_METHODS has no duplicates", () => {
+  assert.equal(
+    new Set(HOST_CONTRACT_METHODS).size,
+    HOST_CONTRACT_METHODS.length,
+  );
+});
+
+// Parse the method names declared in the server's `DocmostClientLike` interface
+// body. We read the .ts source as plain text (no TS compiler dep, and the file
+// lives in the CJS server tree across the ESM boundary): scan from the
+// `export interface DocmostClientLike {` line to its closing brace at column 0,
+// matching member-signature lines like `  methodName(`. Nested param-object
+// braces (`opts: { ... }`) are indented, so only the interface's own closing
+// `}` (column 0) ends the scan.
+function parseDocmostClientLikeMethods() {
+  const here = dirname(fileURLToPath(import.meta.url));
+  // packages/mcp/test/unit -> repo root is four levels up.
+  const loaderPath = resolve(
+    here,
+    "../../../../apps/server/src/core/ai-chat/tools/docmost-client.loader.ts",
+  );
+  let source;
+  try {
+    source = readFileSync(loaderPath, "utf8");
+  } catch (err) {
+    if (err && err.code === "ENOENT") {
+      throw new Error(
+        `Expected monorepo layout; server tree at ${loaderPath} not found. ` +
+          `This drift-guard reads the server's DocmostClientLike interface via a ` +
+          `fixed relative path and must run from inside the monorepo checkout.`,
+      );
+    }
+    throw err;
+  }
+  const lines = source.split(/\r?\n/);
+
+  const startIdx = lines.findIndex((l) =>
+    /^export interface DocmostClientLike\s*\{/.test(l),
+  );
+  assert.notEqual(
+    startIdx,
+    -1,
+    `Could not find "export interface DocmostClientLike {" in ${loaderPath}. ` +
+      `If the interface was renamed/moved, update this drift-guard test.`,
+  );
+
+  const methods = [];
+  let closed = false;
+  // Track whether we are inside a `/* ... */` block comment. Inner lines of a
+  // block comment need NOT start with `*`, so a `name(` line inside one would be
+  // falsely parsed as an interface method without this. (`//` line comments can
+  // never match the method regex below since they start with `/`.)
+  let inBlockComment = false;
+  for (let i = startIdx + 1; i < lines.length; i++) {
+    const line = lines[i];
+    if (inBlockComment) {
+      // Stay in the block until we see its closing `*/`.
+      if (line.includes("*/")) inBlockComment = false;
+      continue;
+    }
+    // Enter a block comment only when it opens without closing on the same line;
+    // a self-contained `/* ... */` on one line cannot precede a method name we
+    // care about (such lines start with `/`, so the method regex won't match).
+    if (line.includes("/*") && !line.includes("*/")) {
+      inBlockComment = true;
+      continue;
+    }
+    if (/^\}/.test(line)) {
+      closed = true;
+      break;
+    }
+    // Method-name match: a TS identifier (letters/digits/`_`/`$`, not starting
+    // with a digit) optionally followed by a generic clause (`method<T>(`), then
+    // the opening paren of the signature.
+    const m = /^\s*([A-Za-z_$][A-Za-z0-9_$]*)\s*(?:<[^>]*>)?\(/.exec(line);
+    if (m) methods.push(m[1]);
+  }
+  assert.ok(
+    closed,
+    `Did not find the closing brace of DocmostClientLike in ${loaderPath}.`,
+  );
+  assert.ok(
+    methods.length > 0,
+    `Parsed zero methods from DocmostClientLike in ${loaderPath} — the parser ` +
+      `is likely out of date with the interface formatting.`,
+  );
+  return methods;
+}
+
+// The point of the guard is to protect the DocmostClientLike mirror <-> client.ts
+// link, but HOST_CONTRACT_METHODS is itself a HAND-COPY of that interface kept in
+// sync manually. The list<->interface link must be tested too: a method consumed
+// by the adapter and added to DocmostClientLike but forgotten here (or removed
+// from the interface but left here) would otherwise escape both the server
+// typecheck (pkg emits no .d.ts) and the first test above (name not in the list).
+// Assert the two agree BOTH ways.
+test("HOST_CONTRACT_METHODS exactly mirrors the server's DocmostClientLike interface", () => {
+  const interfaceMethods = parseDocmostClientLikeMethods();
+  assert.deepEqual(
+    [...HOST_CONTRACT_METHODS].sort(),
+    [...interfaceMethods].sort(),
+    `HOST_CONTRACT_METHODS has drifted from the DocmostClientLike interface in ` +
+      `apps/server/src/core/ai-chat/tools/docmost-client.loader.ts. Add/remove ` +
+      `method names in HOST_CONTRACT_METHODS so it lists EXACTLY the methods ` +
+      `declared in that interface (both directions are checked).`,
+  );
+});

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/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/registry-stamp.test.mjs

@@ -1,101 +0,0 @@
-import { test } from "node:test";
-import assert from "node:assert/strict";
-import { createHash } from "node:crypto";
-import { readFileSync } from "node:fs";
-import { fileURLToPath } from "node:url";
-import { dirname, join } from "node:path";
-
-import { computeRegistryStamp } from "../../scripts/gen-registry-stamp.mjs";
-import { REGISTRY_STAMP } from "../../build/index.js";
-
-// Guard tests for the build/src-skew stamp (issue #447). The codegen script
-// exports `computeRegistryStamp(sourceText)` — a sha256 over normalized source
-// text (CRLF->LF, single trailing newline stripped). The in-app loader
-// (apps/server/.../docmost-client.loader.ts) DUPLICATES that normalize+sha256 to
-// recompute the stamp from src and refuse a stale build. These tests pin the
-// algorithm's behaviour AND assert the built stamp matches the current src, so a
-// stale generated file OR a normalize divergence reddens.
-
-const __dirname = dirname(fileURLToPath(import.meta.url));
-const TOOL_SPECS_PATH = join(__dirname, "..", "..", "src", "tool-specs.ts");
-
-test("computeRegistryStamp is deterministic: same input -> same hash", () => {
-  const input = "export const X = 1;\nexport const Y = 2;\n";
-  assert.equal(computeRegistryStamp(input), computeRegistryStamp(input));
-});
-
-test("computeRegistryStamp returns a 64-char lowercase hex sha256", () => {
-  const stamp = computeRegistryStamp("anything");
-  assert.match(stamp, /^[0-9a-f]{64}$/);
-});
-
-test("normalizes CRLF vs LF: the same content hashes equal", () => {
-  const lf = "line1\nline2\nline3";
-  const crlf = "line1\r\nline2\r\nline3";
-  assert.equal(computeRegistryStamp(crlf), computeRegistryStamp(lf));
-});
-
-test("normalizes a trailing newline: with/without a final \\n hashes equal", () => {
-  const noTrailing = "alpha\nbeta";
-  const trailing = "alpha\nbeta\n";
-  assert.equal(computeRegistryStamp(trailing), computeRegistryStamp(noTrailing));
-});
-
-test("a CRLF checkout WITH a trailing CRLF still hashes equal to bare LF", () => {
-  // A worst-case Windows checkout: CRLF line endings + a trailing CRLF. Both the
-  // \r\n->\n replace and the trailing-newline strip must apply for parity.
-  const bare = "alpha\nbeta";
-  const crlfTrailing = "alpha\r\nbeta\r\n";
-  assert.equal(
-    computeRegistryStamp(crlfTrailing),
-    computeRegistryStamp(bare),
-  );
-});
-
-test("a real content change hashes differently", () => {
-  const before = "export const description = 'search a page';\n";
-  const after = "export const description = 'search a PAGE';\n";
-  assert.notEqual(computeRegistryStamp(before), computeRegistryStamp(after));
-});
-
-// Only a SINGLE trailing newline is stripped — a second blank line is content and
-// must change the hash. This pins the exact `/\n$/` semantics the loader mirrors.
-test("only ONE trailing newline is stripped (two differ from one)", () => {
-  assert.notEqual(
-    computeRegistryStamp("x\n"),
-    computeRegistryStamp("x\n\n"),
-  );
-});
-
-// Cross-impl equality against a fixed, documented input. The SAME literal input
-// and expected hash are asserted in the server-side jest test
-// (docmost-client.loader.spec.ts). If either side's normalize+sha256 ever
-// diverges, one of the two tests reddens. Input exercises BOTH normalize steps.
-test("fixed-input hash matches the documented cross-impl value", () => {
-  const FIXED_INPUT = "line1\r\nline2\n";
-  const EXPECTED =
-    "683376e290829b482c2655745caffa7a1dccfa10afaa62dac2b42dd6c68d0f83";
-  assert.equal(computeRegistryStamp(FIXED_INPUT), EXPECTED);
-});
-
-// DESYNC GUARD (covers reviewer suggestion 2). Recompute the stamp from the
-// actual src/tool-specs.ts and assert it equals the REGISTRY_STAMP baked into the
-// freshly-built build/index.js. This reddens if the generated file is stale OR if
-// the codegen normalize ever diverges from what produced the built stamp.
-test("built REGISTRY_STAMP equals the stamp recomputed from src/tool-specs.ts", () => {
-  const source = readFileSync(TOOL_SPECS_PATH, "utf8");
-  assert.equal(computeRegistryStamp(source), REGISTRY_STAMP);
-});
-
-// Sanity: the fixed-input helper computes the SAME way the codegen does, proving
-// the EXPECTED constant above is not an arbitrary magic value but the documented
-// normalize+sha256 of FIXED_INPUT. Belt-and-braces so a bad EXPECTED can't hide a
-// real regression.
-test("the documented EXPECTED constant is the normalize+sha256 of FIXED_INPUT", () => {
-  const FIXED_INPUT = "line1\r\nline2\n";
-  const normalized = FIXED_INPUT.replace(/\r\n/g, "\n").replace(/\n$/, "");
-  const expected = createHash("sha256")
-    .update(normalized, "utf8")
-    .digest("hex");
-  assert.equal(computeRegistryStamp(FIXED_INPUT), expected);
-});

packages/mcp/tsconfig.json

@@ -5,8 +5,6 @@
     "moduleResolution": "Node16",
     "outDir": "./build",
     "rootDir": "./src",
-    "declaration": true,
-    "declarationMap": true,
     "strict": true,
     "esModuleInterop": true,
     "skipLibCheck": true,