diff --git a/AGENTS.md b/AGENTS.md index ff8a0be9..1a13dea6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -293,6 +293,7 @@ Vite SPA. Code is organized by feature under `apps/client/src/features/*` (mirro - The version string shown in the UI comes from `APP_VERSION` (CI/Docker) or `git describe --tags --always` (local), resolved in `vite.config.ts` — not from `package.json`. - Server TS config is permissive (`noImplicitAny: false`, `strictNullChecks: false`, `no-explicit-any` lint disabled). Follow the existing relaxed style rather than tightening types broadly. - Dependency versions are heavily pinned via `pnpm.overrides` and `pnpm.patchedDependencies` (`scimmy`, `yjs`) in the root `package.json`. Don't bump pinned/patched deps casually; the patches and overrides exist for compatibility/security reasons. +- **Adding/renaming/removing an MCP tool requires updating `SERVER_INSTRUCTIONS`** in `packages/mcp/src/index.ts` — the intent-routing guide MCP clients receive on initialize. This applies both to inline `server.registerTool(...)` calls in `index.ts` and to specs in `packages/mcp/src/tool-specs.ts`. Enforced by `packages/mcp/test/unit/server-instructions.test.mjs`, which fails when a registered tool is not mentioned in the guide (deliberate opt-outs go into its `EXCEPTIONS` list). Remember `packages/mcp/build/` is committed — rebuild after editing. ## CI / release diff --git a/packages/mcp/build/index.js b/packages/mcp/build/index.js index aa5fe567..aca8fc64 100644 --- a/packages/mcp/build/index.js +++ b/packages/mcp/build/index.js @@ -27,10 +27,19 @@ const VERSION = packageJson.version; // --- Modern McpServer Implementation --- // Editing guide surfaced to MCP clients in the initialize result so they can // pick the right tool by intent and avoid resending whole documents. -const SERVER_INSTRUCTIONS = "Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (always inline; requires an EXACT selection — the contiguous text to anchor/highlight on; fails rather than leaving an unanchored comment), list_comments, update_comment, resolve_comment (resolve/reopen a thread, reversible — prefer over delete to close), delete_comment, check_new_comments. Propose a concrete text fix for one-click human approval -> create_comment with suggestedText (the exact plain-text replacement for the selection; the selection must then be UNIQUE in the page — extend it with context if needed); prefer this over editing directly when the change is subjective or needs the author's sign-off. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " + - "Complex/scripted rewrite (multiple coordinated edits, footnotes, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes. " + - "Review what changed -> diff_page_versions (compare a historyId to current, or two history versions). See a page's saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). " + - "Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown."; +// +// MAINTENANCE RULE: when you ADD, RENAME, or REMOVE a tool (either an inline +// server.registerTool(...) here or a spec in tool-specs.ts), you MUST update +// this guide so the new tool is routed by intent. This is enforced by +// test/unit/server-instructions.test.mjs, which fails when a registered tool +// name is not mentioned below (see its EXCEPTIONS list for the rare opt-outs). +// Exported for that test. +export const SERVER_INSTRUCTIONS = "Docmost editing guide — choose the tool by intent.\n" + + "READ: find a page -> search (workspace-wide full-text); list -> list_pages / list_spaces. Locate blocks and their ids CHEAPLY -> get_outline (compact top-level map; start here, not get_page_json). One block's subtree -> get_node (by attrs.id, or \"#\" for tables, which carry no id). Whole page -> get_page (Markdown, lossy; inline tags are comment anchors — markup, not text) or get_page_json (lossless ProseMirror with block ids). Hand a huge page (with images) to an external consumer without pulling it through the model context -> stash_page (returns a short-lived anonymous URL).\n" + + "EDIT: fix wording/typos/numbers -> edit_page_text (find/replace inside blocks, no node id needed). Change ONE block (paragraph/heading/callout/etc.) structurally -> patch_node (by attrs.id from get_outline). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Tables -> table_get / table_update_cell / table_insert_row / table_delete_row (address by \"#\" from get_outline; table nodes have no attrs.id). Images -> insert_image (add from a web URL) / replace_image (swap an existing image). Footnotes -> insert_footnote. Bulk/structural rewrite -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" + + "PAGES: new -> create_page (Markdown). Rename (title only) -> rename_page. Move -> move_page. Delete -> delete_page (SOFT delete — the page goes to trash and is restorable; nothing is permanent). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Sharing -> share_page / unshare_page / list_shares; share_page makes the page PUBLICLY accessible — do it only when explicitly asked.\n" + + "COMMENTS: create_comment is always inline and requires an EXACT selection — contiguous text from a single block, <=250 chars (fails rather than leaving an unanchored comment); reply to a thread via parentCommentId. Propose a concrete text fix for one-click human approval -> create_comment with suggestedText (the exact plain-text replacement for the selection; the selection must then be UNIQUE in the page — extend it with context if needed); prefer this over editing directly when the change is subjective or needs the author's sign-off. Manage -> list_comments, update_comment, resolve_comment (resolve/reopen, reversible — prefer over delete to close), delete_comment, check_new_comments.\n" + + "HISTORY: review what changed -> diff_page_versions (a historyId vs current, or two versions). List saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown."; // Helper to format JSON responses const jsonContent = (data) => ({ content: [{ type: "text", text: JSON.stringify(data, null, 2) }], @@ -486,7 +495,8 @@ export function createDocmostMcpServer(config) { }); // Tool: delete_page server.registerTool("delete_page", { - description: "Delete a single page by ID.", + description: "Delete a single page by ID. SOFT delete only: the page is moved to " + + "trash and can be restored; nothing is permanently deleted.", inputSchema: { pageId: z.string().min(1), }, diff --git a/packages/mcp/build/tool-specs.js b/packages/mcp/build/tool-specs.js index 501e30ad..f47e6149 100644 --- a/packages/mcp/build/tool-specs.js +++ b/packages/mcp/build/tool-specs.js @@ -13,6 +13,11 @@ // diverge on purpose (security guardrails, tuned UX, "Reversible" framing on // some write tools, different limits, hybrid-RRF search, etc.) stay defined // per-layer and are NOT represented here. +// +// MAINTENANCE RULE: adding, renaming, or removing a spec here (or an inline +// registerTool in index.ts) REQUIRES updating SERVER_INSTRUCTIONS in +// packages/mcp/src/index.ts — the intent-routing guide MCP clients receive on +// initialize. Enforced by test/unit/server-instructions.test.mjs. export const SHARED_TOOL_SPECS = { // --- no-argument read tools --- getWorkspace: { diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index 92d45124..22fab8b2 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -37,11 +37,20 @@ const VERSION = packageJson.version; // Editing guide surfaced to MCP clients in the initialize result so they can // pick the right tool by intent and avoid resending whole documents. -const SERVER_INSTRUCTIONS = - "Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (always inline; requires an EXACT selection — the contiguous text to anchor/highlight on; fails rather than leaving an unanchored comment), list_comments, update_comment, resolve_comment (resolve/reopen a thread, reversible — prefer over delete to close), delete_comment, check_new_comments. Propose a concrete text fix for one-click human approval -> create_comment with suggestedText (the exact plain-text replacement for the selection; the selection must then be UNIQUE in the page — extend it with context if needed); prefer this over editing directly when the change is subjective or needs the author's sign-off. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " + - "Complex/scripted rewrite (multiple coordinated edits, footnotes, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes. " + - "Review what changed -> diff_page_versions (compare a historyId to current, or two history versions). See a page's saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). " + - "Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown."; +// +// MAINTENANCE RULE: when you ADD, RENAME, or REMOVE a tool (either an inline +// server.registerTool(...) here or a spec in tool-specs.ts), you MUST update +// this guide so the new tool is routed by intent. This is enforced by +// test/unit/server-instructions.test.mjs, which fails when a registered tool +// name is not mentioned below (see its EXCEPTIONS list for the rare opt-outs). +// Exported for that test. +export const SERVER_INSTRUCTIONS = + "Docmost editing guide — choose the tool by intent.\n" + + "READ: find a page -> search (workspace-wide full-text); list -> list_pages / list_spaces. Locate blocks and their ids CHEAPLY -> get_outline (compact top-level map; start here, not get_page_json). One block's subtree -> get_node (by attrs.id, or \"#\" for tables, which carry no id). Whole page -> get_page (Markdown, lossy; inline tags are comment anchors — markup, not text) or get_page_json (lossless ProseMirror with block ids). Hand a huge page (with images) to an external consumer without pulling it through the model context -> stash_page (returns a short-lived anonymous URL).\n" + + "EDIT: fix wording/typos/numbers -> edit_page_text (find/replace inside blocks, no node id needed). Change ONE block (paragraph/heading/callout/etc.) structurally -> patch_node (by attrs.id from get_outline). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Tables -> table_get / table_update_cell / table_insert_row / table_delete_row (address by \"#\" from get_outline; table nodes have no attrs.id). Images -> insert_image (add from a web URL) / replace_image (swap an existing image). Footnotes -> insert_footnote. Bulk/structural rewrite -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" + + "PAGES: new -> create_page (Markdown). Rename (title only) -> rename_page. Move -> move_page. Delete -> delete_page (SOFT delete — the page goes to trash and is restorable; nothing is permanent). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Sharing -> share_page / unshare_page / list_shares; share_page makes the page PUBLICLY accessible — do it only when explicitly asked.\n" + + "COMMENTS: create_comment is always inline and requires an EXACT selection — contiguous text from a single block, <=250 chars (fails rather than leaving an unanchored comment); reply to a thread via parentCommentId. Propose a concrete text fix for one-click human approval -> create_comment with suggestedText (the exact plain-text replacement for the selection; the selection must then be UNIQUE in the page — extend it with context if needed); prefer this over editing directly when the change is subjective or needs the author's sign-off. Manage -> list_comments, update_comment, resolve_comment (resolve/reopen, reversible — prefer over delete to close), delete_comment, check_new_comments.\n" + + "HISTORY: review what changed -> diff_page_versions (a historyId vs current, or two versions). List saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown."; // Helper to format JSON responses const jsonContent = (data: any) => ({ @@ -675,7 +684,9 @@ server.registerTool( server.registerTool( "delete_page", { - description: "Delete a single page by ID.", + description: + "Delete a single page by ID. SOFT delete only: the page is moved to " + + "trash and can be restored; nothing is permanently deleted.", inputSchema: { pageId: z.string().min(1), }, diff --git a/packages/mcp/src/tool-specs.ts b/packages/mcp/src/tool-specs.ts index ae7b58ff..30efd3b8 100644 --- a/packages/mcp/src/tool-specs.ts +++ b/packages/mcp/src/tool-specs.ts @@ -13,6 +13,11 @@ // diverge on purpose (security guardrails, tuned UX, "Reversible" framing on // some write tools, different limits, hybrid-RRF search, etc.) stay defined // per-layer and are NOT represented here. +// +// MAINTENANCE RULE: adding, renaming, or removing a spec here (or an inline +// registerTool in index.ts) REQUIRES updating SERVER_INSTRUCTIONS in +// packages/mcp/src/index.ts — the intent-routing guide MCP clients receive on +// initialize. Enforced by test/unit/server-instructions.test.mjs. // Loose on purpose — see the comment above. The two zod majors expose different // static type surfaces, so typing this precisely would couple the registry to diff --git a/packages/mcp/test/unit/server-instructions.test.mjs b/packages/mcp/test/unit/server-instructions.test.mjs new file mode 100644 index 00000000..ffd9f5b6 --- /dev/null +++ b/packages/mcp/test/unit/server-instructions.test.mjs @@ -0,0 +1,82 @@ +// Guard: every tool the MCP server registers must be routed by intent in +// SERVER_INSTRUCTIONS — the editing guide clients receive in the initialize +// result. Without this, new tools silently rot out of the guide and agents +// never learn to pick them (the guide once omitted 17 of 41 tools, including +// get_outline, which pushed agents into fetching whole documents for block +// ids). Tool names are extracted from the SOURCE (index.ts inline +// registrations + tool-specs.ts shared specs) so a registration added either +// way is caught; the guide text itself is imported from the build so the test +// checks what actually ships. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, join } from "node:path"; + +import { SERVER_INSTRUCTIONS } from "../../build/index.js"; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const SRC = join(HERE, "..", "..", "src"); + +// Tools DELIBERATELY absent from the guide. Keep this list minimal and +// justify every entry — the default is: every tool gets routed. +const EXCEPTIONS = new Set([ + // Trivial and self-explanatory; carries no routing decision. + "get_workspace", +]); + +/** + * Extract every registered tool name from the source. Two registration + * mechanisms exist and both are covered: + * - inline `server.registerTool("name", ...)` calls in index.ts; + * - shared specs in tool-specs.ts (`mcpName: 'name'`), registered via + * registerShared(SHARED_TOOL_SPECS.x, ...). + */ +function registeredToolNames() { + const indexSrc = readFileSync(join(SRC, "index.ts"), "utf8"); + const specsSrc = readFileSync(join(SRC, "tool-specs.ts"), "utf8"); + const names = new Set(); + for (const m of indexSrc.matchAll(/registerTool\(\s*"([a-z0-9_]+)"/g)) { + names.add(m[1]); + } + for (const m of specsSrc.matchAll(/mcpName:\s*['"]([a-z0-9_]+)['"]/g)) { + names.add(m[1]); + } + return names; +} + +test("every registered tool is mentioned in SERVER_INSTRUCTIONS", () => { + const names = registeredToolNames(); + // Sanity: if extraction regressed (regex drift), fail loudly rather than + // vacuously passing on an empty set. + assert.ok( + names.size >= 40, + `sanity: expected to extract 40+ registered tools, got ${names.size} — ` + + "the extraction regexes in this test likely drifted from the source", + ); + const missing = [...names] + .filter((n) => !EXCEPTIONS.has(n)) + // \b\b: `_` is a word char, so \bget_page\b does NOT match inside + // get_page_json — a tool can't hide behind a longer sibling's mention. + .filter((n) => !new RegExp(`\\b${n}\\b`).test(SERVER_INSTRUCTIONS)) + .sort(); + assert.deepEqual( + missing, + [], + `tools missing from SERVER_INSTRUCTIONS: ${missing.join(", ")} — ` + + "update the guide in packages/mcp/src/index.ts (see its MAINTENANCE " + + "RULE comment), or add a justified entry to EXCEPTIONS here", + ); +}); + +test("EXCEPTIONS entries are real registered tools", () => { + // A stale exception (tool renamed/removed) must be cleaned up, otherwise + // the list quietly grows past its purpose. + const names = registeredToolNames(); + for (const name of EXCEPTIONS) { + assert.ok( + names.has(name), + `EXCEPTIONS entry "${name}" is not a registered tool — remove it`, + ); + } +});