Merge branch 'refactor/ai-tools-spec-registry' into develop

Shared zod-agnostic tool-spec registry for the 14 identical AI tools across
the standalone MCP server and the in-app AI-SDK chat (keeps execute/auth and
the ~17 intentionally-divergent guardrail tools per-layer), folds in the
edit_page_text drift-bug fix, and formalizes the integration-test db factory.

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

@@ -1,6 +1,19 @@
 import { AiChatToolsService } from './ai-chat-tools.service';
 import * as loader from './docmost-client.loader';
 import type { DocmostClientLike } from './docmost-client.loader';
+// 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.
+import { SHARED_TOOL_SPECS } from '../../../../../../packages/mcp/src/tool-specs';
+
+// loadDocmostMcp now resolves to { DocmostClient, sharedToolSpecs }. Every mock
+// below must supply sharedToolSpecs or the service throws while building the
+// shared tools. Factor the resolved-value shape so the three mock sites stay in
+// sync.
+const mockLoaded = (DocmostClient: loader.DocmostClientCtor) => ({
+  DocmostClient,
+  sharedToolSpecs: SHARED_TOOL_SPECS as Record<string, loader.SharedToolSpec>,
+});
 
 /**
  * Guardrail test (§14 [H4]): the adapter's `deletePage` write tool must be a
@@ -37,11 +50,11 @@ describe('AiChatToolsService deletePage guardrail (H4)', () => {
   beforeEach(() => {
     deletePageCalls.length = 0;
     // Intercept the ESM loader so `new DocmostClient(config)` returns our fake.
-    jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue({
-      DocmostClient: function () {
+    jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue(
+      mockLoaded(function () {
         return fakeClient as DocmostClientLike;
-      } as unknown as loader.DocmostClientCtor,
-    });
+      } as unknown as loader.DocmostClientCtor),
+    );
     // The new semanticSearch deps (aiService + repos) are not exercised by the
     // deletePage guardrail tests; pass stubs to satisfy the constructor arity.
     service = new AiChatToolsService(
@@ -144,11 +157,11 @@ describe('AiChatToolsService expanded toolset guardrails', () => {
   let service: AiChatToolsService;
 
   beforeEach(() => {
-    jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue({
-      DocmostClient: function () {
+    jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue(
+      mockLoaded(function () {
         return fakeClient as DocmostClientLike;
-      } as unknown as loader.DocmostClientCtor,
-    });
+      } as unknown as loader.DocmostClientCtor),
+    );
     service = new AiChatToolsService(
       tokenServiceStub as never,
       {} as never,
@@ -252,11 +265,11 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => {
     patchNodeCalls.length = 0;
     insertNodeCalls.length = 0;
     updatePageJsonCalls.length = 0;
-    jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue({
-      DocmostClient: function () {
+    jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue(
+      mockLoaded(function () {
         return fakeClient as DocmostClientLike;
-      } as unknown as loader.DocmostClientCtor,
-    });
+      } as unknown as loader.DocmostClientCtor),
+    );
     service = new AiChatToolsService(
       tokenServiceStub as never,
       {} as never,

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

@@ -11,6 +11,7 @@ import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo'
 import {
   loadDocmostMcp,
   type DocmostClientLike,
+  type SharedToolSpec,
 } from './docmost-client.loader';
 import { resolveCurrentPageResult } from './current-page.util';
 import { parseNodeArg } from './parse-node-arg';
@@ -84,13 +85,29 @@ export class AiChatToolsService {
         aiChatId,
       });
 
-    const { DocmostClient } = await loadDocmostMcp();
+    const { DocmostClient, sharedToolSpecs } = await loadDocmostMcp();
     const client: DocmostClientLike = new DocmostClient({
       apiUrl,
       getToken,
       getCollabToken,
     });
 
+    // Build an ai-SDK tool from a shared, zod-agnostic spec. The spec owns the
+    // canonical description + (optional) schema builder, which is invoked with
+    // THIS layer's zod (v4); only the execute body is supplied per call. No-arg
+    // specs (no buildShape) get an empty object schema.
+    const sharedTool = (
+      spec: SharedToolSpec,
+      execute: Tool['execute'],
+    ): Tool =>
+      tool({
+        description: spec.description,
+        inputSchema: spec.buildShape
+          ? z.object(spec.buildShape(z) as z.ZodRawShape)
+          : z.object({}),
+        execute,
+      });
+
     return {
       searchPages: tool({
         description:
@@ -416,20 +433,15 @@ export class AiChatToolsService {
 
       // --- READ tools (added) ---
 
-      getWorkspace: tool({
-        description:
-          'Fetch metadata about the current workspace (name, settings).',
-        inputSchema: z.object({}),
-        execute: async () => await client.getWorkspace(),
-      }),
+      getWorkspace: sharedTool(
+        sharedToolSpecs.getWorkspace,
+        async () => await client.getWorkspace(),
+      ),
 
-      listSpaces: tool({
-        description:
-          'List the spaces the current user can access. Returns the array ' +
-          'of spaces (id, name, slug, ...).',
-        inputSchema: z.object({}),
-        execute: async () => await client.getSpaces(),
-      }),
+      listSpaces: sharedTool(
+        sharedToolSpecs.listSpaces,
+        async () => await client.getSpaces(),
+      ),
 
       listPages: tool({
         description:
@@ -477,43 +489,20 @@ export class AiChatToolsService {
           await client.listSidebarPages(spaceId, pageId),
       }),
 
-      getOutline: tool({
-        description:
-          "Compact outline of a page's top-level blocks, with block ids. Use " +
-          'it to locate sections/tables and grab block ids before drilling in ' +
-          'with getNode / patchNode / insertNode.',
-        inputSchema: z.object({
-          pageId: z.string().describe('The id of the page.'),
-        }),
-        execute: async ({ pageId }) => await client.getOutline(pageId),
-      }),
+      getOutline: sharedTool(
+        sharedToolSpecs.getOutline,
+        async ({ pageId }) => await client.getOutline(pageId),
+      ),
 
-      getPageJson: tool({
-        description:
-          'Fetch a page as lossless ProseMirror JSON (preserves block ids and ' +
-          'marks). Use this when you need exact structure for node-level edits.',
-        inputSchema: z.object({
-          pageId: z.string().describe('The id of the page.'),
-        }),
-        execute: async ({ pageId }) => await client.getPageJson(pageId),
-      }),
+      getPageJson: sharedTool(
+        sharedToolSpecs.getPageJson,
+        async ({ pageId }) => await client.getPageJson(pageId),
+      ),
 
-      getNode: tool({
-        description:
-          "Fetch a single block's full ProseMirror subtree (lossless) by " +
-          'reference.',
-        inputSchema: z.object({
-          pageId: z.string().describe('The id of the page.'),
-          nodeId: z
-            .string()
-            .describe(
-              'A block id from getOutline, or "#<index>" to select a ' +
-                'top-level block by its outline index (e.g. a table).',
-            ),
-        }),
-        execute: async ({ pageId, nodeId }) =>
-          await client.getNode(pageId, nodeId),
-      }),
+      getNode: sharedTool(
+        sharedToolSpecs.getNode,
+        async ({ pageId, nodeId }) => await client.getNode(pageId, nodeId),
+      ),
 
       getTable: tool({
         description:
@@ -570,27 +559,16 @@ export class AiChatToolsService {
           await client.checkNewComments(spaceId, since, parentPageId),
       }),
 
-      listShares: tool({
-        description:
-          'List all public shares in the workspace, each with its public URL.',
-        inputSchema: z.object({}),
-        execute: async () => await client.listShares(),
-      }),
+      listShares: sharedTool(
+        sharedToolSpecs.listShares,
+        async () => await client.listShares(),
+      ),
 
-      listPageHistory: tool({
-        description:
-          'List the saved versions (history snapshots) of a page, newest ' +
-          'first. Returns one cursor-paginated page of results.',
-        inputSchema: z.object({
-          pageId: z.string().describe('The id of the page.'),
-          cursor: z
-            .string()
-            .optional()
-            .describe('Optional pagination cursor from a previous call.'),
-        }),
-        execute: async ({ pageId, cursor }) =>
+      listPageHistory: sharedTool(
+        sharedToolSpecs.listPageHistory,
+        async ({ pageId, cursor }) =>
           await client.listPageHistory(pageId, cursor),
-      }),
+      ),
 
       getPageHistory: tool({
         description:
@@ -603,24 +581,11 @@ export class AiChatToolsService {
           await client.getPageHistory(historyId),
       }),
 
-      diffPageVersions: tool({
-        description:
-          'Diff two versions of a page and return the change set. from/to ' +
-          "each accept a historyId or 'current' (or omit for current).",
-        inputSchema: z.object({
-          pageId: z.string().describe('The id of the page.'),
-          from: z
-            .string()
-            .optional()
-            .describe("A historyId, or 'current'/omit for current content."),
-          to: z
-            .string()
-            .optional()
-            .describe("A historyId, or 'current'/omit for current content."),
-        }),
-        execute: async ({ pageId, from, to }) =>
+      diffPageVersions: sharedTool(
+        sharedToolSpecs.diffPageVersions,
+        async ({ pageId, from, to }) =>
           await client.diffPageVersions(pageId, from, to),
-      }),
+      ),
 
       exportPageMarkdown: tool({
         description:
@@ -638,46 +603,10 @@ export class AiChatToolsService {
 
       // --- WRITE tools (added; reversible via page history/trash) ---
 
-      editPageText: tool({
-        description:
-          'Surgical find/replace inside a page\'s text, preserving all block ' +
-          'ids and marks. A find MAY cross bold/italic/link boundaries; the ' +
-          'replacement inherits marks from the unchanged common prefix/suffix ' +
-          '(so editing plain text next to a bold word keeps it bold, and ' +
-          'editing inside a bold word keeps the new text bold). Each find must ' +
-          'match exactly once unless replaceAll is set. The batch applies what ' +
-          'it can and returns applied[] + failed[] plus a verify change-report ' +
-          '(the text/marks/structure that ACTUALLY changed — read it to confirm ' +
-          'your edit landed; do not assume success); a fully-unmatched batch ' +
-          'writes nothing and errors. find and replace are LITERAL text, not ' +
-          'markdown. This tool edits plain text ONLY and CANNOT add or remove ' +
-          'formatting marks: a formatting change — find/replace that differ only ' +
-          'in markdown markers (e.g. find:"~~x~~", replace:"x"), or a replace ' +
-          'containing **bold**/~~strike~~/`code` wrappers — is REFUSED into ' +
-          'failed[]. To change bold/italic/strike/code/link, read the block with ' +
-          'getPageJson and use patchNode (or updatePageJson) to set its marks. ' +
-          'Examples: edits:[{find:"teh",replace:"the"}]; edits:[{find:"Hello ' +
-          'world",replace:"Hello there"}] (crosses a bold boundary). Reversible: ' +
-          'the previous version is kept in page history.',
-        inputSchema: z.object({
-          pageId: z.string().describe('The id of the page to edit.'),
-          edits: z
-            .array(
-              z.object({
-                find: z.string().describe('Exact text to find.'),
-                replace: z.string().describe('Replacement text.'),
-                replaceAll: z
-                  .boolean()
-                  .optional()
-                  .describe('Replace every occurrence (default: one match).'),
-              }),
-            )
-            .min(1)
-            .describe('One or more find/replace edits.'),
-        }),
-        execute: async ({ pageId, edits }) =>
-          await client.editPageText(pageId, edits),
-      }),
+      editPageText: sharedTool(
+        sharedToolSpecs.editPageText,
+        async ({ pageId, edits }) => await client.editPageText(pageId, edits),
+      ),
 
       patchNode: tool({
         description:
@@ -767,17 +696,10 @@ export class AiChatToolsService {
         },
       }),
 
-      deleteNode: tool({
-        description:
-          'Remove a content BLOCK by its id (NOT a page). Reversible: the ' +
-          'previous version is kept in page history.',
-        inputSchema: z.object({
-          pageId: z.string().describe('The id of the page.'),
-          nodeId: z.string().describe('The block id to remove.'),
-        }),
-        execute: async ({ pageId, nodeId }) =>
-          await client.deleteNode(pageId, nodeId),
-      }),
+      deleteNode: sharedTool(
+        sharedToolSpecs.deleteNode,
+        async ({ pageId, nodeId }) => await client.deleteNode(pageId, nodeId),
+      ),
 
       updatePageJson: tool({
         description:
@@ -866,35 +788,17 @@ export class AiChatToolsService {
           await client.tableUpdateCell(pageId, tableRef, row, col, text),
       }),
 
-      copyPageContent: tool({
-        description:
-          "Replace the target page's BODY with the source page's body " +
-          '(title/slug are kept). Runs server-side — no document passes ' +
-          'through the model. Reversible: the target keeps page history.',
-        inputSchema: z.object({
-          sourcePageId: z.string().describe('The id of the source page.'),
-          targetPageId: z
-            .string()
-            .describe('The id of the target page to overwrite.'),
-        }),
-        execute: async ({ sourcePageId, targetPageId }) =>
+      copyPageContent: sharedTool(
+        sharedToolSpecs.copyPageContent,
+        async ({ sourcePageId, targetPageId }) =>
           await client.copyPageContent(sourcePageId, targetPageId),
-      }),
+      ),
 
-      importPageMarkdown: tool({
-        description:
-          "Replace a page's body from Docmost-flavoured Markdown (as produced " +
-          'by exportPageMarkdown). Reversible: the previous version is kept in ' +
-          'page history.',
-        inputSchema: z.object({
-          pageId: z.string().describe('The id of the page to overwrite.'),
-          markdown: z
-            .string()
-            .describe('Docmost-flavoured Markdown for the page body.'),
-        }),
-        execute: async ({ pageId, markdown }) =>
+      importPageMarkdown: sharedTool(
+        sharedToolSpecs.importPageMarkdown,
+        async ({ pageId, markdown }) =>
           await client.importPageMarkdown(pageId, markdown),
-      }),
+      ),
 
       sharePage: tool({
         description:
@@ -912,27 +816,15 @@ export class AiChatToolsService {
           await client.sharePage(pageId, searchIndexing),
       }),
 
-      unsharePage: tool({
-        description:
-          'Remove the public share of a page (reverses sharePage).',
-        inputSchema: z.object({
-          pageId: z.string().describe('The id of the page to unshare.'),
-        }),
-        execute: async ({ pageId }) => await client.unsharePage(pageId),
-      }),
+      unsharePage: sharedTool(
+        sharedToolSpecs.unsharePage,
+        async ({ pageId }) => await client.unsharePage(pageId),
+      ),
 
-      restorePageVersion: tool({
-        description:
-          'Restore a past version by writing its content back as the current ' +
-          'page content. Itself reversible: it creates a new history snapshot.',
-        inputSchema: z.object({
-          historyId: z
-            .string()
-            .describe('The id of the history version to restore.'),
-        }),
-        execute: async ({ historyId }) =>
-          await client.restorePageVersion(historyId),
-      }),
+      restorePageVersion: sharedTool(
+        sharedToolSpecs.restorePageVersion,
+        async ({ historyId }) => await client.restorePageVersion(historyId),
+      ),
 
       transformPage: tool({
         description:

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

@@ -167,8 +167,29 @@ 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;
+  // 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>;
+}
+
 interface DocmostMcpModule {
   DocmostClient: DocmostClientCtor;
+  SHARED_TOOL_SPECS: Record<string, SharedToolSpec>;
 }
 
 // TS with module:commonjs downlevels a literal `import()` to `require()`, which
@@ -191,6 +212,7 @@ let modulePromise: Promise<DocmostMcpModule> | null = null;
  */
 export async function loadDocmostMcp(): Promise<{
   DocmostClient: DocmostClientCtor;
+  sharedToolSpecs: Record<string, SharedToolSpec>;
 }> {
   if (!modulePromise) {
     modulePromise = (async () => {
@@ -206,5 +228,15 @@ export async function loadDocmostMcp(): Promise<{
     });
   }
   const mod = await modulePromise;
-  return { DocmostClient: mod.DocmostClient };
+  if (!mod.SHARED_TOOL_SPECS) {
+    // A stale @docmost/mcp build (missing the shared registry export) would
+    // otherwise surface as a confusing TypeError deep in the tools service.
+    throw new Error(
+      '@docmost/mcp is stale: SHARED_TOOL_SPECS missing — rebuild the package (pnpm --filter @docmost/mcp build).',
+    );
+  }
+  return {
+    DocmostClient: mod.DocmostClient,
+    sharedToolSpecs: mod.SHARED_TOOL_SPECS,
+  };
 }

apps/server/test/integration/db.ts

@@ -3,6 +3,31 @@ import { CamelCasePlugin, Kysely } from 'kysely';
 import { PostgresJSDialect } from 'kysely-postgres-js';
 import * as postgres from 'postgres';
 
+/**
+ * db.ts — THE canonical place to seed prerequisite rows for integration tests.
+ *
+ * Seeders here use minimal, explicit `insertInto(...).values(...)` calls and are
+ * DELIBERATELY decoupled from the app's repo `insert*` methods. Those repo
+ * methods carry side effects integration specs do not want — password hashing,
+ * validation, default/derived columns, event emission — so reproducing only the
+ * columns a test needs keeps the fixtures small, fast and predictable.
+ *
+ * CONVENTIONS:
+ *  - New entity seeders go HERE (a `createX(db, ...)` helper) rather than as raw
+ *    `insertInto` calls scattered across spec files, so the schema knowledge
+ *    lives in one place.
+ *  - Each seeder inserts only the NOT NULL / uniquely-constrained columns plus
+ *    whatever the consuming tests assert on; everything else is left to DB
+ *    defaults.
+ *  - Plain `randomUUID()` (v4) is fine for FK integrity; the app uses uuid v7,
+ *    but tests never depend on id ordering.
+ *
+ * TRADE-OFF: because the column/constraint knowledge below is mirrored from the
+ * Kysely schema rather than derived from it, a migration that changes a NOT NULL
+ * column or a unique constraint can make an insert here fail. When that happens
+ * the fix is to update the relevant seeder, not the spec that calls it.
+ */
+
 /**
  * Isolated test database connection string. The dev DB is `docmost`; tests run
  * against a dedicated `docmost_test` that global-setup drops + recreates +
@@ -58,21 +83,27 @@ export async function destroyTestDb(): Promise<void> {
 }
 
 // --- Seeding helpers ---------------------------------------------------------
-// Insert minimal valid rows (only the columns the tests need + NOT NULL ones).
-// Plain randomUUID() is fine for FK integrity in tests (the app uses uuid v7).
+// Each helper inserts a minimal valid row (only the columns the tests need plus
+// the NOT NULL / uniquely-constrained ones) and returns the generated id. See
+// the module doc comment above for why these bypass the app's repo layer.
+
+// Short, human-readable suffix derived from a row's uuid. Used to build unique
+// names/slugs/hostnames for seeded rows so unique constraints never collide.
+const shortId = (id: string): string => id.slice(0, 8);
 
 export async function createWorkspace(
   db: Kysely<any>,
   overrides: { settings?: unknown; name?: string } = {},
 ): Promise<{ id: string; settings: any }> {
   const id = randomUUID();
+  const suffix = shortId(id);
   const row = await db
     .insertInto('workspaces')
     .values({
       id,
-      name: overrides.name ?? `ws-${id.slice(0, 8)}`,
+      name: overrides.name ?? `ws-${suffix}`,
       // hostname is uniquely constrained; keep it unique per workspace.
-      hostname: `host-${id.slice(0, 8)}`,
+      hostname: `host-${suffix}`,
       settings: overrides.settings === undefined ? null : (overrides.settings as any),
     })
     .returning(['id', 'settings'])
@@ -86,12 +117,13 @@ export async function createUser(
   overrides: { email?: string; name?: string } = {},
 ): Promise<{ id: string }> {
   const id = randomUUID();
+  const suffix = shortId(id);
   const row = await db
     .insertInto('users')
     .values({
       id,
-      email: overrides.email ?? `user-${id.slice(0, 8)}@example.test`,
-      name: overrides.name ?? `user-${id.slice(0, 8)}`,
+      email: overrides.email ?? `user-${suffix}@example.test`,
+      name: overrides.name ?? `user-${suffix}`,
       workspaceId,
     })
     .returning(['id'])
@@ -105,13 +137,14 @@ export async function createSpace(
   overrides: { slug?: string; name?: string } = {},
 ): Promise<{ id: string }> {
   const id = randomUUID();
+  const suffix = shortId(id);
   const row = await db
     .insertInto('spaces')
     .values({
       id,
-      name: overrides.name ?? `space-${id.slice(0, 8)}`,
+      name: overrides.name ?? `space-${suffix}`,
       // slug is unique per workspace + NOT NULL.
-      slug: overrides.slug ?? `space-${id.slice(0, 8)}`,
+      slug: overrides.slug ?? `space-${suffix}`,
       workspaceId,
     })
     .returning(['id'])
@@ -124,13 +157,14 @@ export async function createPage(
   args: { workspaceId: string; spaceId: string; title?: string },
 ): Promise<{ id: string }> {
   const id = randomUUID();
+  const suffix = shortId(id);
   const row = await db
     .insertInto('pages')
     .values({
       id,
       // slug_id is NOT NULL + globally unique.
-      slugId: `slug-${id.slice(0, 8)}`,
-      title: args.title ?? `page-${id.slice(0, 8)}`,
+      slugId: `slug-${suffix}`,
+      title: args.title ?? `page-${suffix}`,
       spaceId: args.spaceId,
       workspaceId: args.workspaceId,
     })
@@ -186,7 +220,7 @@ export async function createChat(
       workspaceId: args.workspaceId,
       creatorId: args.creatorId,
       roleId: args.roleId ?? null,
-      title: args.title ?? `chat-${id.slice(0, 8)}`,
+      title: args.title ?? `chat-${shortId(id)}`,
     })
     .returning(['id'])
     .executeTakeFirstOrThrow();