Merge pull request 'refactor(mcp): вывести DocmostClientLike/SharedToolSpec из реального типа — убить ручные зеркала (#446)' (#458) from refactor/446-derive-client-types into develop

Reviewed-on: #458

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).

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

@@ -2,266 +2,92 @@ 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
- * ESM-only `@docmost/mcp` package. We only need the constructor + the read/write
- * methods used by the per-user tool adapter; the full client surface lives in
- * `packages/mcp/src/client.ts`. Signatures here mirror that file exactly.
- *
- * DRIFT GUARD: the method NAMES below are runtime-checked against the real
- * `DocmostClient` by `packages/mcp/test/unit/client-host-contract.test.mjs`
- * (which can import the ESM class directly). If you rename/remove a method here
- * or in client.ts, that test fails — so a stale mirror cannot silently ship a
- * runtime "x is not a function" into an agent tool call. Keep the two in sync.
- *
- * STAGED PLAN — full derivation `DocmostClientLike = <real DocmostClient type>`
- * (issue #193, layer 3) is intentionally NOT done; it stays a hand-mirror for
- * now because of two verified blockers across the ESM(mcp)/CJS(server) boundary:
- *   1. `@docmost/mcp` emits NO declaration files (its tsconfig has no
- *      `declaration`, package.json has no `types`/types-export) and the server
- *      tsconfig has no path mapping for it — the server only loads it via the
- *      runtime `import()` trick below, so there is no type to import today.
- *   2. The real client methods have inferred, CONCRETE return types; the in-app
- *      tool adapter reads results through loose `Record<string,unknown>` returns
- *      + `as` casts (e.g. `(result?.data ?? {}) as { title?: string }`).
- *      Deriving the exact type would make those casts non-overlapping ("may be a
- *      mistake") and break the build, and `Partial<DocmostClientLike>` test stubs
- *      would have to satisfy the full concrete surface.
- * To do it safely later (incrementally): (a) turn on `declaration: true` in
- * packages/mcp/tsconfig.json + add a `types` export condition and commit the
- * emitted `.d.ts`; (b) `import type { DocmostClient } from '@docmost/mcp'` here
- * and replace this interface with a `Pick<DocmostClient, ...>` of the consumed
- * methods; (c) audit every `as` cast in ai-chat-tools.service.ts against the now
- * concrete return types (double-cast through `unknown` only where genuinely
- * needed); (d) keep the runtime guard test as a belt-and-braces check. Until
- * then the guard test above is the cheap, behaviour-neutral protection.
+ * The exact set of `DocmostClient` methods the per-user in-app tool adapter
+ * consumes. This is the AUTHORITATIVE list of the client surface the server
+ * depends on; the adapter calls these methods POSITIONALLY, so this set is what
+ * the derived type below type-checks against the real class (issue #446).
  */
-export interface DocmostClientLike {
+type DocmostClientMethod =
   // --- read ---
-  search(
-    query: string,
-    spaceId?: string,
-    limit?: number,
-  ): Promise<{ items: unknown[]; success: boolean }>;
-  getPage(
-    pageId: string,
-  ): Promise<{ data: Record<string, unknown>; success: boolean }>;
-  // Light raw page info (`/pages/info`): title + slugId + ProseMirror content,
-  // WITHOUT the Markdown render / subpage expansion getPage does. Used by the
-  // comment-signal probe to read just the page title on a hit.
-  getPageRaw(pageId: string): Promise<Record<string, unknown> | null>;
-  getWorkspace(): Promise<{ data: Record<string, unknown>; success: boolean }>;
-  getSpaces(): Promise<unknown[]>;
-  listPages(
-    spaceId?: string,
-    limit?: number,
-    tree?: boolean,
-  ): Promise<unknown[]>;
-  listSidebarPages(spaceId: string, pageId?: string): Promise<unknown[]>;
-  getOutline(pageId: string): Promise<Record<string, unknown>>;
-  getPageJson(pageId: string): Promise<Record<string, unknown>>;
-  getNode(pageId: string, nodeId: string): Promise<Record<string, unknown>>;
-  searchInPage(
-    pageId: string,
-    query: string,
-    opts?: { regex?: boolean; caseSensitive?: boolean; limit?: number },
-  ): Promise<Record<string, unknown>>;
-  getTable(pageId: string, tableRef: string): Promise<Record<string, unknown>>;
-  // Returns `{ items, resolvedThreadsHidden }`. DEFAULT (includeResolved unset/
-  // false) hides resolved threads wholesale; pass true for the full feed.
-  listComments(
-    pageId: string,
-    includeResolved?: boolean,
-  ): Promise<{ items: unknown[]; resolvedThreadsHidden: number }>;
-  getComment(
-    commentId: string,
-  ): Promise<{ data: Record<string, unknown>; success: boolean }>;
-  checkNewComments(
-    spaceId: string,
-    since: string,
-    parentPageId?: string,
-  ): Promise<unknown>;
-  listShares(): Promise<unknown[]>;
-  listPageHistory(
-    pageId: string,
-    cursor?: string,
-  ): Promise<{ items: unknown[]; nextCursor: string | null }>;
-  getPageHistory(historyId: string): Promise<Record<string, unknown>>;
-  diffPageVersions(
-    pageId: string,
-    from?: string,
-    to?: string,
-  ): Promise<Record<string, unknown>>;
-  exportPageMarkdown(pageId: string): Promise<string>;
+  | 'search'
+  | 'getPage'
+  | 'getPageRaw'
+  | 'getWorkspace'
+  | 'getSpaces'
+  | 'listPages'
+  | 'listSidebarPages'
+  | 'getOutline'
+  | 'getPageJson'
+  | 'getNode'
+  | 'searchInPage'
+  | 'getTable'
+  | 'listComments'
+  | 'getComment'
+  | 'checkNewComments'
+  | 'listShares'
+  | 'listPageHistory'
+  | 'getPageHistory'
+  | 'diffPageVersions'
+  | 'exportPageMarkdown'
   // --- write (page) ---
-  createPage(
-    title: string,
-    content: string,
-    spaceId: string,
-    parentPageId?: string,
-  ): Promise<{ data: Record<string, unknown>; success: boolean }>;
-  // Markdown content update via the collab path (carries provenance via the
-  // collab-token provider). Optionally also updates the title.
-  updatePage(
-    pageId: string,
-    content: string,
-    title?: string,
-  ): Promise<Record<string, unknown>>;
-  // Title-only rename via REST.
-  renamePage(
-    pageId: string,
-    title: string,
-  ): Promise<Record<string, unknown>>;
-  // Move via REST. parentPageId null => move to space root.
-  movePage(
-    pageId: string,
-    parentPageId: string | null,
-    position?: string,
-  ): Promise<unknown>;
-  // SOFT delete only (POST /pages/delete with { pageId }). NEVER permanent.
-  deletePage(pageId: string): Promise<unknown>;
-  editPageText(
-    pageId: string,
-    edits: Array<{ find: string; replace: string; replaceAll?: boolean }>,
-  ): Promise<Record<string, unknown>>;
-  patchNode(
-    pageId: string,
-    nodeId: string,
-    node: unknown,
-  ): Promise<Record<string, unknown>>;
-  insertNode(
-    pageId: string,
-    node: unknown,
-    opts: {
-      position: 'before' | 'after' | 'append';
-      anchorNodeId?: string;
-      anchorText?: string;
-    },
-  ): Promise<Record<string, unknown>>;
-  deleteNode(
-    pageId: string,
-    nodeId: string,
-  ): Promise<Record<string, unknown>>;
-  updatePageJson(
-    pageId: string,
-    doc?: unknown,
-    title?: string,
-  ): Promise<Record<string, unknown>>;
-  // Attach an author-inline footnote after the first occurrence of anchorText;
-  // numbering + the footnotes list are derived server-side.
-  insertFootnote(
-    pageId: string,
-    anchorText: string,
-    text: string,
-  ): Promise<Record<string, unknown>>;
-  // Download a web image and insert it into the page (append, or replace/after a
-  // text anchor). `url` is the image http(s) URL.
-  insertImage(
-    pageId: string,
-    url: string,
-    opts?: {
-      align?: 'left' | 'center' | 'right';
-      alt?: string;
-      replaceText?: string;
-      afterText?: string;
-    },
-  ): Promise<Record<string, unknown>>;
-  // Swap an existing image (by its attachmentId) for a new one fetched from a web
-  // URL, repointing every reference in the live document.
-  replaceImage(
-    pageId: string,
-    oldAttachmentId: string,
-    url: string,
-    opts?: { align?: 'left' | 'center' | 'right'; alt?: string },
-  ): Promise<Record<string, unknown>>;
+  | 'createPage'
+  | 'updatePage'
+  | 'renamePage'
+  | 'movePage'
+  | 'deletePage'
+  | 'editPageText'
+  | 'patchNode'
+  | 'insertNode'
+  | 'deleteNode'
+  | 'updatePageJson'
+  | 'tableInsertRow'
+  | 'tableDeleteRow'
+  | 'tableUpdateCell'
+  | 'copyPageContent'
+  | 'importPageMarkdown'
+  | 'sharePage'
+  | 'unsharePage'
+  | 'restorePageVersion'
+  | 'transformPage'
+  | 'stashPage'
+  // --- write (image / footnote), in-app since #410 ---
+  | 'insertImage'
+  | 'replaceImage'
+  | 'insertFootnote'
   // --- draw.io diagrams (#423, stage 1) ---
-  // Read a diagram as decoded mxGraph XML (default) or the raw .drawio.svg.
-  // meta.hash is the optimistic-lock key drawioUpdate expects as baseHash.
-  drawioGet(
-    pageId: string,
-    node: string,
-    format?: 'xml' | 'svg',
-  ): Promise<Record<string, unknown>>;
-  // Lint mxGraph XML, build the .drawio.svg attachment and insert a drawio node.
-  drawioCreate(
-    pageId: string,
-    where: {
-      position: 'before' | 'after' | 'append';
-      anchorNodeId?: string;
-      anchorText?: string;
-    },
-    xml: string,
-    title?: string,
-  ): Promise<Record<string, unknown>>;
-  // Optimistic-locked full replacement of a diagram (baseHash from drawioGet).
-  drawioUpdate(
-    pageId: string,
-    node: string,
-    xml: string,
-    baseHash: string,
-  ): Promise<Record<string, unknown>>;
-  tableInsertRow(
-    pageId: string,
-    tableRef: string,
-    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>>;
+  | 'drawioGet'
+  | 'drawioCreate'
+  | 'drawioUpdate'
   // --- write (comment) ---
-  createComment(
-    pageId: string,
-    content: string,
-    type?: 'page' | 'inline',
-    selection?: string,
-    parentCommentId?: string,
-    suggestedText?: string,
-  ): Promise<{ data: Record<string, unknown>; success: boolean }>;
-  resolveComment(
-    commentId: string,
-    resolved: boolean,
-  ): Promise<Record<string, unknown>>;
-  // Serialize a page + mirror its internal images into the blob sandbox; returns
-  // ONLY a short anonymous URL (the body never enters the model context).
-  stashPage(pageId: string): Promise<{
-    uri: string;
-    sha256: string;
-    size: number;
-    images: { mirrored: number; failed: number };
-  }>;
-}
+  | 'createComment'
+  | 'resolveComment';
+
+/**
+ * The client surface the per-user tool adapter consumes, DERIVED from the real
+ * `DocmostClient` type in `@docmost/mcp` (issue #446, restored #294 debt). This
+ * replaces the former hand-mirror of ~45 method signatures.
+ *
+ * `import type` (above) is fully ERASED at compile time, so nothing is actually
+ * imported from the ESM-only package at runtime — the server still loads the
+ * class through the dynamic `import()` trick in `loadDocmostMcp` below; this is
+ * purely a compile-time type. Deriving via `Pick` means a parameter reorder or a
+ * type change to any of these methods in `client.ts` now becomes a SERVER
+ * COMPILE ERROR at the positional call sites in ai-chat-tools.service.ts,
+ * instead of a silent runtime "wrong argument" failure inside an agent tool.
+ *
+ * This made the old name-only drift-guard test
+ * (packages/mcp/test/unit/client-host-contract.test.mjs) redundant — tsc now
+ * enforces both names AND signatures — so that test was removed.
+ */
+export type DocmostClientLike = Pick<DocmostClient, DocmostClientMethod>;
 
 export type DocmostClientConfig = {
   apiUrl: string;
@@ -283,32 +109,7 @@ export type DocmostClientConfig = {
 };
 
 export interface DocmostClientCtor {
-  new (config: DocmostClientConfig): DocmostClientLike;
-}
-
-/**
- * Local hand-mirror of the `SharedToolSpec` shape exported from
- * `@docmost/mcp` (packages/mcp/src/tool-specs.ts). Same approach as
- * `DocmostClientLike`: we do not import the ESM package's types directly across
- * the CJS/ESM boundary. The registry itself has no runtime deps, but keeping the
- * type local avoids coupling the server build to the package's type surface.
- *
- * `buildShape` is intentionally zod-agnostic: it returns a plain ZodRawShape
- * built with whatever zod namespace the caller passes (the server passes its own
- * zod v4; the MCP package passes its zod v3). See the registry module comment.
- */
-export interface SharedToolSpec {
-  mcpName: string;
-  inAppKey: string;
-  description: string;
-  // Deferred-tool metadata (#332). Optional in this mirror so an older/stale
-  // @docmost/mcp build (pre-#332) still type-checks; the in-app catalog builder
-  // reads them defensively. The external /mcp server ignores both fields.
-  tier?: 'core' | 'deferred';
-  catalogLine?: string;
-  // Loose `z` on purpose: the registry is zod-agnostic so the server can pass
-  // its own zod (v4) and the MCP package its own (v3) into the same builder.
-  buildShape?: (z: any) => Record<string, unknown>;
+  new (config: DocmostClientConfig): DocmostClient;
 }
 
 /**

packages/mcp/package.json

@@ -5,9 +5,16 @@
   "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"

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/tsconfig.json

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