prompt(mcp): rewrite SERVER_INSTRUCTIONS to cover all tools + guard test
The intent-routing guide had rotted: 17 of 41 registered tools were absent (get_outline, get_node, the whole table_* family, search, stash_page, sharing, page lifecycle), and two tips were actively harmful — 'read block ids via get_page_json' told agents to pull the whole ~100KB document when get_outline exists precisely to grab ids cheaply, and 'table cell -> patch_node by attrs.id' dead-ends because table nodes carry no attrs.id. - Rewrite SERVER_INSTRUCTIONS as intent clusters (READ / EDIT / PAGES / COMMENTS / HISTORY) covering every tool except get_workspace; add safety notes (share_page = PUBLIC, delete_page = soft) and a comment-anchor markup warning for get_page. - delete_page tool description: state SOFT delete / restorable explicitly. - MAINTENANCE RULE comments at both registration sites (index.ts, tool-specs.ts) + an AGENTS.md convention bullet: adding/renaming/removing a tool REQUIRES updating the guide. - New guard test (test/unit/server-instructions.test.mjs): extracts every registered tool name from source and fails when one is not mentioned in the shipped SERVER_INSTRUCTIONS (word-boundary match, so get_page can't hide behind get_page_json); EXCEPTIONS list is itself validated against the registry. SERVER_INSTRUCTIONS exported for the test. Tests: @docmost/mcp 450/450 (448 + 2 new). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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 \"#<index>\" for tables, which carry no id). Whole page -> get_page (Markdown, lossy; inline <span data-comment-id> 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 \"#<index>\" 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),
|
||||
},
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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 \"#<index>\" for tables, which carry no id). Whole page -> get_page (Markdown, lossy; inline <span data-comment-id> 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 \"#<index>\" 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),
|
||||
},
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<name>\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`,
|
||||
);
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user