From ce70fab1dfde5c052c1e008eaa526f512939427f Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 5 Jul 2026 02:17:14 +0300 Subject: [PATCH] refactor(ai-chat): unify share_page into SHARED_TOOL_SPECS (#294, misc family) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrates share_page / sharePage into the transport-agnostic spec registry (schema + description declared once; each transport keeps only its execute/auth): - sharePage (deferred) -> SHARED_TOOL_SPECS; index.ts uses registerShared(), ai-chat uses sharedTool(); removed from INLINE_TOOL_TIERS. Drift reconciled (documented inline): both inline copies already carried the "only share when the user explicitly asked" security framing, so the old "per-transport divergence" note in BOTH layers was STALE — there was no real behavioral divergence, only wording drift. The canonical description merges the MCP copy's URL-format + idempotency detail with the in-app copy's reversibility note and keeps the shared security framing. pageId keeps the MCP copy's stricter .min(1). The MCP execute keeps its own `searchIndexing ?? true` default (per-layer, not part of the shared schema). Intentionally NOT migrated (kept inline — genuinely divergent, as their existing notes state): - search / searchPages: the in-app tool is a semantic+keyword hybrid (RRF) with in-process access control and a tuned schema (limit 1-20); the MCP `search` is a plain REST full-text search (limit up to 100). Different behavior AND schema. - docmost_transform / transformPage: the in-app tool deliberately omits the `deleteComments` schema field (a comment-deletion guardrail) and carries a shorter description. Different schema. Gate: mcp build 0 + node --test 458/458 (page-search excluded — hangs only under the local re2->RegExp type-shim, its source untouched), server jest 775 incl. tool-tiers catalog-partition + shared-spec contract parity, server tsc 0. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ai-chat/tools/ai-chat-tools.service.ts | 25 +++++------------ .../src/core/ai-chat/tools/tool-tiers.ts | 8 +++--- packages/mcp/src/index.ts | 24 +++------------- packages/mcp/src/tool-specs.ts | 28 +++++++++++++++++++ 4 files changed, 43 insertions(+), 42 deletions(-) diff --git a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts index d7548e15..d543390e 100644 --- a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts +++ b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts @@ -717,25 +717,14 @@ export class AiChatToolsService { await client.importPageMarkdown(pageId, markdown), ), - // INTENTIONAL per-transport divergence (not shared): adds a security - // confirmation framing ("Only share when the user explicitly asked, since - // this exposes the page to anyone with the link") for the in-app agent; the - // standalone MCP `share_page` keeps the plain public-URL wording. - sharePage: tool({ - description: - 'Make a page PUBLICLY accessible and return its public URL. ' + - 'Reversible via unsharePage. Only share when the user explicitly ' + - 'asked, since this exposes the page to anyone with the link.', - inputSchema: modelFriendlyInput({ - pageId: z.string().describe('The id of the page to share.'), - searchIndexing: z - .boolean() - .optional() - .describe('Allow public search engines to index it (default true).'), - }), - execute: async ({ pageId, searchIndexing }) => + // 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, diff --git a/apps/server/src/core/ai-chat/tools/tool-tiers.ts b/apps/server/src/core/ai-chat/tools/tool-tiers.ts index b0c13b2b..7d26498c 100644 --- a/apps/server/src/core/ai-chat/tools/tool-tiers.ts +++ b/apps/server/src/core/ai-chat/tools/tool-tiers.ts @@ -140,10 +140,10 @@ export const INLINE_TOOL_TIERS: Record< catalogLine: 'getPageHistory — fetch one page-history version with its ProseMirror content.', }, - sharePage: { - tier: 'deferred', - catalogLine: 'sharePage — make a page publicly accessible and return its URL.', - }, + // NOTE: sharePage moved to @docmost/mcp's SHARED_TOOL_SPECS (#294); it carries + // its own deferred tier + catalogLine there. transformPage stays inline (its + // schema deliberately diverges — it omits the deleteComments field the MCP + // docmost_transform exposes, a comment-deletion guardrail). transformPage: { tier: 'deferred', catalogLine: "transformPage — run a sandboxed JS transform over a page's document.", diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index fb373415..771c3e6f 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -163,7 +163,6 @@ registerShared( }, ); -// Tool: table_get // 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) @@ -473,25 +472,10 @@ server.registerTool( ); // Tool: share_page -// INTENTIONAL per-transport divergence (not shared): the in-app copy adds a -// security-confirmation framing ("only share when the user explicitly asked, -// since this exposes the page to anyone with the link") tuned for the in-app -// agent; this transport keeps the plain public-URL wording. -server.registerTool( - "share_page", - { - description: - "Make a page publicly accessible (idempotent) and return its public " + - "URL. The URL format is /share//p/. This exposes the " + - "page content to ANYONE with the URL — do it only when explicitly asked.", - inputSchema: { - pageId: z.string().min(1).describe("ID of the page to share"), - searchIndexing: z - .boolean() - .optional() - .describe("Allow search engines to index the page (default true)"), - }, - }, +// 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); diff --git a/packages/mcp/src/tool-specs.ts b/packages/mcp/src/tool-specs.ts index 43eee4fd..a52f7953 100644 --- a/packages/mcp/src/tool-specs.ts +++ b/packages/mcp/src/tool-specs.ts @@ -316,6 +316,34 @@ export const SHARED_TOOL_SPECS = { // --- share management --- + // Unified from the per-layer inline definitions (#294). Both layers already + // carried the "only share when explicitly asked" security framing (the + // "per-transport divergence" note on the old inline copies was stale), so + // there was no real behavioral divergence to preserve — only wording drift. + sharePage: { + mcpName: 'share_page', + inAppKey: 'sharePage', + // CANONICAL: merges the MCP copy's URL-format + idempotency detail with the + // in-app copy's reversibility note; keeps the security framing both had. + description: + 'Make a page PUBLICLY accessible (idempotent) and return its public URL ' + + '(format: /share//p/). This exposes the page content ' + + 'to ANYONE with the URL — only share when the user explicitly asked. ' + + 'Reversible: unshare it later to revoke the public URL.', + tier: 'deferred', + catalogLine: 'sharePage — make a page publicly accessible and return its URL.', + // Reconciled: MCP's stricter .min(1) on pageId kept; field descriptions from + // the in-app copy. The MCP execute keeps its own `searchIndexing ?? true` + // default (a per-layer concern, not part of the shared schema). + buildShape: (z) => ({ + pageId: z.string().min(1).describe('The id of the page to share.'), + searchIndexing: z + .boolean() + .optional() + .describe('Allow public search engines to index it (default true).'), + }), + }, + unsharePage: { mcpName: 'unshare_page', inAppKey: 'unsharePage',