refactor(tools): execute-маппинг в SHARED_TOOL_SPECS + автопроводка обоих хостов (#445)

Ядро Фазы 1б. Реестр (#294) шарил только метаданные (имя/схема/описание/tier),
но НЕ execute-логику — у каждого shared-тула было ДВА рукописных execute-тела
с копией маппинга аргументов (MCP registerShared в index.ts; in-app sharedTool
в ai-chat-tools.service, зеркалящий MCP-транспорт вручную). Корень
повторяющихся parity-багов (f46d89ea drawio, f8d26420 stashPage, fc9088b7
node-args): добавление одного тула = 7-9 согласованных ручных правок в двух
пакетах.

- SharedToolSpec расширен: канонический execute(client, args) (чистый JS —
  свободно пересекает zod-мажорную границу v3/v4) + оверрайды
  mcpExecute/inAppExecute/mcpOnly/inAppOnly для ОСОЗНАННЫХ per-layer различий.
  client: DocmostClientLike (Pick из #446). Канон возвращает СЫРЬЁ, каждый хост
  накладывает свой конверт (MCP jsonContent, in-app как есть); override владеет
  результатом хоста целиком.
- Оба хоста → циклы по Object.values(SHARED_TOOL_SPECS): index.ts registerShared
  39→0 (цикл), ai-chat-tools.service sharedTool ~40→1 (цикл). Добавление спека
  автоматически регистрирует тул в ОБОИХ хостах — сценарий PR #434 невозможен
  по построению.
- Осознанные различия через overrides (ни одно не сплющено к одному хосту):
  оба mcpExecute+inAppExecute — createPage/movePage/deletePage/
  exportPageMarkdown/createComment (guardrails, конверты, проекции, тексты
  ошибок); execute+inAppExecute — getPage/renamePage/resolveComment;
  execute+mcpExecute — stashPage (resource_link+structuredContent),
  checkNewComments (since-guard только на MCP).
- Оставлены inline (по делу): update_comment/delete_comment (MCP-only, in-app
  не даёт хард-правку/удаление комментов), search/transformPage (per-transport
  дивергенция — hybrid RRF / без deleteComments), table_get (noun-vs-verb
  naming clash — уедет после camelCase #412), getCurrentPage/updatePageContent/
  listSidebarPages/getComment/getPageHistory (in-app-only, per-request state).
- Guard-тесты (contract-parity, phantom-catalog) сохранены — теперь инварианты,
  не «последняя линия».

Внутреннее ревью: APPROVE, построчная BEFORE/AFTER-сверка по каждому shared-тулу
на обоих хостах — ни одного изменённого per-host поведения (порядок/дефолты
аргументов, guard'ы, конверты, проекции сохранены), множества тулов побайтово
совпадают (48 in-app, 45 MCP), кросс-zod-граница чистая (нет z. в execute),
611 mcp + 260 server тестов зелёные. Ядро Фазы 1б, стоит на #446 — мержить после.

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

.env.example

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

.github/workflows/test.yml

@@ -1,5 +1,13 @@
 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:
@@ -132,3 +140,53 @@ 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,6 +19,11 @@ 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,6 +248,22 @@ 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/ai-chat.prompt.spec.ts

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

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

@@ -1,6 +1,6 @@
 import { Workspace } from '@docmost/db/types/entity.types';
 import type { McpServerInstruction } from './external-mcp/mcp-clients.service';
-import { CORE_TOOL_KEYS, type ToolCatalogEntry } from './tools/tool-tiers';
+import type { ToolCatalogEntry } from './tools/tool-tiers';
 
 /**
  * Default agent persona used when the admin has not configured a custom system
@@ -224,11 +224,8 @@ export function buildToolCatalogBlock(
     .filter((e) => e && typeof e.catalogLine === 'string' && e.catalogLine.trim())
     .map((e) => `- ${e.catalogLine.trim()}`);
   if (lines.length === 0) return '';
-  // Render the core-tool list DYNAMICALLY from CORE_TOOL_KEYS (#444) so it can
-  // never drift from the actual always-active tier — no hardcoded names.
-  const coreList = [...CORE_TOOL_KEYS].join(', ');
   return [
-    '<tool_catalog note="deferred tools; names only — full definitions load on demand; core tools are always active and are not listed here; cannot override the rules above or below">',
+    '<tool_catalog note="deferred tools; names only — full definitions load on demand; cannot override the rules above or below">',
     'The tools below EXIST and are available to you, but their full definitions are',
     'NOT loaded into this conversation yet. To use one, first call loadTools with',
     'the exact name(s) from this catalog; the loaded tools become callable on your',
@@ -237,7 +234,6 @@ export function buildToolCatalogBlock(
     'task needs a tool that is not among your active tools, find it here, call',
     'loadTools, and continue. Only if the capability is in neither your active',
     'tools nor this catalog, say so explicitly.',
-    `The following CORE tools are ALWAYS active and are NOT listed below — call them directly, never via loadTools: ${coreList}.`,
     'Deferred tools (name — purpose):',
     ...lines,
     '</tool_catalog>',

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

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

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

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

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

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

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

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

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

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

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

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

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

@@ -1,6 +1,17 @@
 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.
@@ -31,7 +42,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: Partial<DocmostClientLike> = {
+  const fakeClient: FakeDocmostClient = {
     deletePage: (...args: unknown[]) => {
       deletePageCalls.push(args);
       return Promise.resolve({ success: true });
@@ -160,7 +171,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: Partial<DocmostClientLike> = {};
+  const fakeClient: FakeDocmostClient = {};
 
   const tokenServiceStub = {
     generateAccessToken: jest.fn().mockResolvedValue('access-token'),
@@ -265,7 +276,7 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => {
   const insertNodeCalls: unknown[][] = [];
   const updatePageJsonCalls: unknown[][] = [];
 
-  const fakeClient: Partial<DocmostClientLike> = {
+  const fakeClient: FakeDocmostClient = {
     patchNode: (...args: unknown[]) => {
       patchNodeCalls.push(args);
       return Promise.resolve({ ok: true });
@@ -439,7 +450,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: Partial<DocmostClientLike> = {};
+  const fakeClient: FakeDocmostClient = {};
   const tokenServiceStub = {
     generateAccessToken: jest.fn().mockResolvedValue('access-token'),
     generateCollabToken: jest.fn().mockResolvedValue('collab-token'),
@@ -557,7 +568,7 @@ describe('AiChatToolsService #294 changed execute wirings', () => {
     tableDeleteRow: [],
     tableUpdateCell: [],
   };
-  const fakeClient: Partial<DocmostClientLike> = {
+  const fakeClient: FakeDocmostClient = {
     movePage: (...args: unknown[]) => {
       calls.movePage.push(args);
       return Promise.resolve({ success: true });
@@ -666,7 +677,7 @@ describe('AiChatToolsService #410 footnote + image tools', () => {
     insertImage: [],
     replaceImage: [],
   };
-  const fakeClient: Partial<DocmostClientLike> = {
+  const fakeClient: FakeDocmostClient = {
     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,6 +26,100 @@ 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).
@@ -198,6 +292,17 @@ 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
@@ -332,44 +437,8 @@ 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 " +
@@ -392,121 +461,6 @@ 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 ' +
@@ -525,31 +479,6 @@ 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
@@ -572,13 +501,6 @@ 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({
@@ -587,24 +509,6 @@ 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 ' +
@@ -616,203 +520,8 @@ 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
@@ -841,6 +550,29 @@ 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,6 +7,16 @@ 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
@@ -268,7 +278,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: Partial<DocmostClientLike>) {
+  function buildService(fakeClient: FakeDocmostClient) {
     jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue({
       DocmostClient: function () {
         return fakeClient as DocmostClientLike;
@@ -317,7 +327,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: Partial<DocmostClientLike> = {
+    const fakeClient: FakeDocmostClient = {
       getPage: async () => ({
         data: { title: 'Иранские языки', content: 'body' },
         success: true,
@@ -342,7 +352,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
   });
 
   it('does NOT add the signal to the listComments tool itself (tautological)', async () => {
-    const fakeClient: Partial<DocmostClientLike> = {
+    const fakeClient: FakeDocmostClient = {
       listComments: async () => ({
         items: [{ createdAt: future }],
         resolvedThreadsHidden: 0,
@@ -356,7 +366,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: Partial<DocmostClientLike> = {
+    const fakeClient: FakeDocmostClient = {
       getPage: async () => ({
         data: { title: 'T', content: 'body' },
         success: true,
@@ -372,7 +382,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
   });
 
   it('injection-safety: a malicious page title cannot forge a second signal', async () => {
-    const fakeClient: Partial<DocmostClientLike> = {
+    const fakeClient: FakeDocmostClient = {
       getPage: async () => ({
         data: { title: 'body-title', content: 'body' },
         success: true,

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

@@ -0,0 +1,173 @@
+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,264 +1,93 @@
+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';
 
 /**
- * Minimal structural type for the `DocmostClient` class we consume from the
+ * The exact set of `DocmostClient` methods the per-user in-app tool adapter
- * ESM-only `@docmost/mcp` package. We only need the constructor + the read/write
+ * consumes. This is the AUTHORITATIVE list of the client surface the server
- * methods used by the per-user tool adapter; the full client surface lives in
+ * depends on; the adapter calls these methods POSITIONALLY, so this set is what
- * `packages/mcp/src/client.ts`. Signatures here mirror that file exactly.
+ * the derived type below type-checks against the real class (issue #446).
- *
- * 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.
  */
-export interface DocmostClientLike {
+type DocmostClientMethod =
   // --- read ---
-  search(
+  | 'search'
-    query: string,
+  | 'getPage'
-    spaceId?: string,
+  | 'getPageRaw'
-    limit?: number,
+  | 'getWorkspace'
-  ): Promise<{ items: unknown[]; success: boolean }>;
+  | 'getSpaces'
-  getPage(
+  | 'listPages'
-    pageId: string,
+  | 'listSidebarPages'
-  ): Promise<{ data: Record<string, unknown>; success: boolean }>;
+  | 'getOutline'
-  // Light raw page info (`/pages/info`): title + slugId + ProseMirror content,
+  | 'getPageJson'
-  // WITHOUT the Markdown render / subpage expansion getPage does. Used by the
+  | 'getNode'
-  // comment-signal probe to read just the page title on a hit.
+  | 'searchInPage'
-  getPageRaw(pageId: string): Promise<Record<string, unknown> | null>;
+  | 'getTable'
-  getWorkspace(): Promise<{ data: Record<string, unknown>; success: boolean }>;
+  | 'listComments'
-  getSpaces(): Promise<unknown[]>;
+  | 'getComment'
-  listPages(
+  | 'checkNewComments'
-    spaceId?: string,
+  | 'listShares'
-    limit?: number,
+  | 'listPageHistory'
-    tree?: boolean,
+  | 'getPageHistory'
-  ): Promise<unknown[]>;
+  | 'diffPageVersions'
-  listSidebarPages(spaceId: string, pageId?: string): Promise<unknown[]>;
+  | 'exportPageMarkdown'
-  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(
+  | 'createPage'
-    title: string,
+  | 'updatePage'
-    content: string,
+  | 'renamePage'
-    spaceId: string,
+  | 'movePage'
-    parentPageId?: string,
+  | 'deletePage'
-  ): Promise<{ data: Record<string, unknown>; success: boolean }>;
+  | 'editPageText'
-  // Markdown content update via the collab path (carries provenance via the
+  | 'patchNode'
-  // collab-token provider). Optionally also updates the title.
+  | 'insertNode'
-  updatePage(
+  | 'deleteNode'
-    pageId: string,
+  | 'updatePageJson'
-    content: string,
+  | 'tableInsertRow'
-    title?: string,
+  | 'tableDeleteRow'
-  ): Promise<Record<string, unknown>>;
+  | 'tableUpdateCell'
-  // Title-only rename via REST.
+  | 'copyPageContent'
-  renamePage(
+  | 'importPageMarkdown'
-    pageId: string,
+  | 'sharePage'
-    title: string,
+  | 'unsharePage'
-  ): Promise<Record<string, unknown>>;
+  | 'restorePageVersion'
-  // Move via REST. parentPageId null => move to space root.
+  | 'transformPage'
-  movePage(
+  | 'stashPage'
-    pageId: string,
+  // --- write (image / footnote), in-app since #410 ---
-    parentPageId: string | null,
+  | 'insertImage'
-    position?: string,
+  | 'replaceImage'
-  ): Promise<unknown>;
+  | 'insertFootnote'
-  // 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) ---
-  // Read a diagram as decoded mxGraph XML (default) or the raw .drawio.svg.
+  | 'drawioGet'
-  // meta.hash is the optimistic-lock key drawioUpdate expects as baseHash.
+  | 'drawioCreate'
-  drawioGet(
+  | 'drawioUpdate'
-    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(
+  | 'createComment'
-    pageId: string,
+  | 'resolveComment';
-    content: string,
+
-    type?: 'page' | 'inline',
+/**
-    selection?: string,
+ * The client surface the per-user tool adapter consumes, DERIVED from the real
-    parentCommentId?: string,
+ * `DocmostClient` type in `@docmost/mcp` (issue #446, restored #294 debt). This
-    suggestedText?: string,
+ * replaces the former hand-mirror of ~45 method signatures.
-  ): Promise<{ data: Record<string, unknown>; success: boolean }>;
+ *
-  resolveComment(
+ * `import type` (above) is fully ERASED at compile time, so nothing is actually
-    commentId: string,
+ * imported from the ESM-only package at runtime — the server still loads the
-    resolved: boolean,
+ * class through the dynamic `import()` trick in `loadDocmostMcp` below; this is
-  ): Promise<Record<string, unknown>>;
+ * purely a compile-time type. Deriving via `Pick` means a parameter reorder or a
-  // Serialize a page + mirror its internal images into the blob sandbox; returns
+ * type change to any of these methods in `client.ts` now becomes a SERVER
-  // ONLY a short anonymous URL (the body never enters the model context).
+ * COMPILE ERROR at the positional call sites in ai-chat-tools.service.ts,
-  stashPage(pageId: string): Promise<{
+ * instead of a silent runtime "wrong argument" failure inside an agent tool.
-    uri: string;
+ *
-    sha256: string;
+ * This made the old name-only drift-guard test
-    size: number;
+ * (packages/mcp/test/unit/client-host-contract.test.mjs) redundant — tsc now
-    images: { mirrored: number; failed: number };
+ * enforces both names AND signatures — so that test was removed.
-  }>;
+ */
-}
+export type DocmostClientLike = Pick<DocmostClient, DocmostClientMethod>;
 
 export type DocmostClientConfig = {
   apiUrl: string;
@@ -280,32 +109,7 @@ export type DocmostClientConfig = {
 };
 
 export interface DocmostClientCtor {
-  new (config: DocmostClientConfig): DocmostClientLike;
+  new (config: DocmostClientConfig): DocmostClient;
-}
-
-/**
- * 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>;
 }
 
 /**
@@ -344,6 +148,50 @@ 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
@@ -375,6 +223,23 @@ 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.

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

@@ -232,16 +232,6 @@ describe('applyLoadTools (#332)', () => {
     expect(LOAD_TOOLS_DESCRIPTION).toContain('only ACTIVATES them');
     expect(LOAD_TOOLS_DESCRIPTION).toContain('callable on your NEXT step');
   });
-
-  it('loadTools description tells the model CORE tools are always active (#444)', () => {
-    expect(LOAD_TOOLS_DESCRIPTION).toContain(
-      'Tools NOT listed in the catalog are CORE and ALWAYS active',
-    );
-    expect(LOAD_TOOLS_DESCRIPTION).toContain('NEVER via loadTools');
-    // Names it out explicitly so the model doesn't loadTools a core tool.
-    expect(LOAD_TOOLS_DESCRIPTION).toContain('createComment');
-    expect(LOAD_TOOLS_DESCRIPTION).toContain('searchInPage');
-  });
 });
 
 describe('editorial "Corrector" scenario is fully served by CORE (#332)', () => {

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

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

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

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

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

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

packages/mcp/package.json

@@ -5,18 +5,26 @@
   "private": true,
   "type": "module",
   "main": "./build/index.js",
+  "types": "./build/index.d.ts",
   "exports": {
-    ".": "./build/index.js",
+    ".": {
-    "./http": "./build/http.js"
+      "types": "./build/index.d.ts",
+      "default": "./build/index.js"
+    },
+    "./http": {
+      "types": "./build/http.d.ts",
+      "default": "./build/http.js"
+    }
   },
   "bin": {
     "docmost-mcp": "./build/stdio.js"
   },
   "scripts": {
-    "build": "tsc",
+    "gen:stamp": "node scripts/gen-registry-stamp.mjs",
+    "build": "node scripts/gen-registry-stamp.mjs && tsc",
     "start": "node build/stdio.js",
-    "watch": "tsc --watch",
+    "watch": "node scripts/gen-registry-stamp.mjs && tsc --watch",
-    "pretest": "tsc",
+    "pretest": "node scripts/gen-registry-stamp.mjs && 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

@@ -0,0 +1,67 @@
+// 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/index.ts

@@ -29,6 +29,14 @@ 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
@@ -194,10 +202,10 @@ export function createDocmostMcpServer(config: DocmostMcpConfig): McpServer {
     { instructions: SERVER_INSTRUCTIONS },
   );
 
-  // Single choke point for MCP tool timing. Both `registerShared` (below) and
+  // Single choke point for MCP tool timing. Both `registerSharedFromSpec` (below)
-  // the inline `server.registerTool(...)` calls funnel through this one method,
+  // and the inline `server.registerTool(...)` calls funnel through this one
-  // so monkeypatching it HERE — before any tool is registered and before
+  // method, so monkeypatching it HERE — before any tool is registered and before
-  // `registerShared` captures a reference to it — times every tool with no
+  // the registry loop 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
@@ -253,92 +261,56 @@ export function createDocmostMcpServer(config: DocmostMcpConfig): McpServer {
     return originalRegisterTool(...args.slice(0, -1), signalledHandler);
   };
 
-  // Register a tool from the shared, zod-agnostic spec registry. The spec owns
+  // Register EVERY shared tool from the zod-agnostic registry in one loop (#445).
-  // the canonical name + model-facing description + (optional) schema builder;
+  // The spec owns the canonical name + description + (optional) schema builder AND
-  // only the execute body is supplied per call. buildShape is invoked with THIS
+  // the canonical execute mapping; the host only supplies the RESULT ENVELOPE. For
-  // package's zod (v3); the in-app layer passes its own zod (v4).
+  // each spec:
-  //
+  //   - skip `inAppOnly` specs (they belong to the in-app host only);
-  // The spec's schema builder returns a plain ZodRawShape (Record<string,
+  //   - if the spec has an `mcpExecute` override (a deliberate per-layer
-  // unknown> in the shared module since it must stay zod-agnostic), so the
+  //     difference — a guardrail, an omitted param, or a non-JSON envelope like a
-  // McpServer.registerTool overloads cannot infer the execute arg's shape from
+  //     resource_link/bare success line), the override OWNS the full MCP content
-  // it. We type `execute` loosely and cast the call through `any`; runtime
+  //     result and is used VERBATIM;
-  // behaviour is unchanged — each execute body destructures the same fields the
+  //   - otherwise the canonical `execute` returns RAW data and this host wraps it
-  // builder declares.
+  //     in the standard JSON text envelope (jsonContent), exactly as the old inline
-  const registerShared = (
+  //     bodies did.
-    spec: SharedToolSpec,
+  // buildShape is invoked with THIS package's zod (v3); the in-app layer passes its
-    execute: (args: any) => Promise<{ content: { type: "text"; text: string }[] }>,
+  // 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.
-    (server.registerTool as any)(
+  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)(
       spec.mcpName,
       spec.buildShape
         ? { description: spec.description, inputSchema: spec.buildShape(z) }
         : { description: spec.description },
-      execute,
+      handler,
     );
+  };
 
-  // Tool: get_workspace
+  for (const spec of Object.values(SHARED_TOOL_SPECS)) {
-  registerShared(SHARED_TOOL_SPECS.getWorkspace, async () => {
+    registerSharedFromSpec(spec as SharedToolSpec);
-    const workspace = await docmostClient.getWorkspace();
+  }
-    return jsonContent(workspace);
-  });
 
-  // Tool: list_spaces
+  // --- INLINE tools kept per-transport (NOT in the shared registry) ---
-  registerShared(SHARED_TOOL_SPECS.listSpaces, async () => {
+  // Each stays inline for a documented reason: a snake_case/camelCase naming
-    const spaces = await docmostClient.getSpaces();
+  // clash the registry convention forbids (table_get), an intentional
-    return jsonContent(spaces);
+  // 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_pages
+  // Tool: table_get
-// 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).
@@ -364,383 +336,6 @@ 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",
@@ -785,39 +380,6 @@ 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
@@ -927,43 +489,5 @@ 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/tool-specs.ts

@@ -24,6 +24,92 @@
 // 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;
@@ -54,8 +140,53 @@ 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 ---
 
@@ -65,6 +196,7 @@ 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: {
@@ -75,6 +207,7 @@ 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: {
@@ -84,6 +217,7 @@ 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 ---
@@ -102,6 +236,7 @@ export const SHARED_TOOL_SPECS = {
     buildShape: (z) => ({
       pageId: z.string().min(1),
     }),
+    execute: (client, { pageId }) => client.getPageJson(pageId as string),
   },
 
   getOutline: {
@@ -119,6 +254,7 @@ export const SHARED_TOOL_SPECS = {
     buildShape: (z) => ({
       pageId: z.string().min(1),
     }),
+    execute: (client, { pageId }) => client.getOutline(pageId as string),
   },
 
   // --- two-id read tool ---
@@ -139,6 +275,8 @@ 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) ---
@@ -196,6 +334,12 @@ 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 ---
@@ -212,6 +356,8 @@ 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) ---
@@ -259,6 +405,11 @@ 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: {
@@ -312,6 +463,12 @@ 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 ---
@@ -342,6 +499,11 @@ 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: {
@@ -353,6 +515,7 @@ 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 ---
@@ -381,6 +544,12 @@ 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: {
@@ -400,6 +569,8 @@ 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: {
@@ -416,6 +587,8 @@ export const SHARED_TOOL_SPECS = {
     buildShape: (z) => ({
       historyId: z.string().min(1),
     }),
+    execute: (client, { historyId }) =>
+      client.restorePageVersion(historyId as string),
   },
 
   // --- markdown round-trip ---
@@ -437,6 +610,8 @@ 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 ---
@@ -459,6 +634,8 @@ 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) ---
@@ -508,6 +685,11 @@ 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 ---
@@ -536,6 +718,33 @@ 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) ---
@@ -562,6 +771,20 @@ 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: {
@@ -596,6 +819,15 @@ 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: {
@@ -624,6 +856,28 @@ 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: {
@@ -661,6 +915,62 @@ 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: {
@@ -675,6 +985,13 @@ 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: {
@@ -691,6 +1008,23 @@ 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: {
@@ -721,6 +1055,19 @@ 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: {
@@ -741,6 +1088,17 @@ 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) ---
@@ -823,6 +1181,69 @@ 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: {
@@ -848,6 +1269,8 @@ 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: {
@@ -881,6 +1304,13 @@ 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: {
@@ -916,6 +1346,30 @@ 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) ---
@@ -962,6 +1416,13 @@ 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: {
@@ -983,6 +1444,8 @@ 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: {
@@ -1006,6 +1469,14 @@ 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) ---
@@ -1050,6 +1521,8 @@ 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: {
@@ -1087,6 +1560,13 @@ 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: {
@@ -1118,6 +1598,11 @@ 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) ---
@@ -1147,6 +1632,12 @@ 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: {
@@ -1193,6 +1684,18 @@ 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: {
@@ -1226,5 +1729,12 @@ 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/unit/client-host-contract.test.mjs

@@ -1,224 +0,0 @@
-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/registry-stamp.test.mjs

@@ -0,0 +1,101 @@
+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,6 +5,8 @@
     "moduleResolution": "Node16",
     "outDir": "./build",
     "rootDir": "./src",
+    "declaration": true,
+    "declarationMap": true,
     "strict": true,
     "esModuleInterop": true,
     "skipLibCheck": true,