refactor(tools): генерировать инвентарь SERVER_INSTRUCTIONS из реестра + guard имён тулов в промпте (#448)
Финальный линк Фазы 1б. Инвентарь тулов жил в 4 рукописных прозаических копиях (SERVER_INSTRUCTIONS под regex-тестом; <tool_catalog>; имена в ai-chat.prompt.ts без гарда; README) — роадмап #416 планировал 4 последовательных ручных правки этого текста (#411/#412/#413/#415). - SERVER_INSTRUCTIONS разбит (новый модуль server-instructions.ts): ROUTING_ PROSE (рукописные intent-подсказки «когда что» — осмысленно ручные, перенесены ДОСЛОВНО со всеми предостережениями: <=250 у create_comment, soft-delete у delete_page, baseHash у drawio_update, PUBLIC у share_page) + buildToolInventory() — генерирует <tool_inventory> из реестра (mcpName + purpose из catalogLine, группировка по TOOL_FAMILY, бакет OTHER ловит незамаппленное → тул нельзя тихо потерять) + 5 inline MCP-only (INLINE_MCP_INVENTORY). Детерминирован (семейства FAMILY_ORDER, имена localeCompare). regex-тест server-instructions удалён; структурные гарантии — в новом tool-inventory.test.mjs (точное членство множества сильнее старого \b-скрейпа). - Имена тулов в ai-chat.prompt.ts → через экспорт PROMPT_TOOL_NAMES; новый гард ai-chat.prompt.tool-names.spec.ts: каждое имя — реальный тул реестра, скан guidance-нот на camelCase-токены падает на несуществующем (escape- нейтрализация против ложных nThe-токенов). - INLINE_TOOL_TIERS уже содержал ровно 8 genuinely-inline тулов (после #445) — сжатие не потребовалось. Критерий: добавление/переименование спека меняет инвентарь БЕЗ правки прозы. Внутреннее ревью: APPROVE — фактическим прогоном подтверждено, что НИ ОДИН тул из старого SERVER_INSTRUCTIONS не выпал (диф старый-vs-новый пуст; добавился get_workspace, раньше прятавшийся в EXCEPTIONS); проза дословна; инвентарь полон/детерминирован/без фантомов; гард краснеет на обеих ветках провала. 613 node + 289 jest зелёные. Стоит на #445 — мержить последним в стопке 1б. README-каталоги вне обязательного скоупа (docs-скрипт) — в чек-лист #412. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,125 @@
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { PROMPT_TOOL_NAMES } from './ai-chat.prompt';
|
||||
// The real shared registry, imported from source (same approach as the
|
||||
// SHARED_TOOL_SPECS contract spec) so tool names are validated against exactly
|
||||
// what @docmost/mcp ships.
|
||||
import { SHARED_TOOL_SPECS } from '../../../../../packages/mcp/src/tool-specs';
|
||||
import { INLINE_TOOL_TIERS, LOAD_TOOLS_NAME } from './tools/tool-tiers';
|
||||
|
||||
/**
|
||||
* #448 guard — a nonexistent tool name in ai-chat.prompt.ts must fail a test.
|
||||
*
|
||||
* The in-app prompt refers to a handful of tools BY NAME in its guidance notes
|
||||
* (e.g. PAGE_CHANGED_NOTE tells the agent to re-read via getPage and edit via
|
||||
* editPageText/patchNode/insertNode/deleteNode). Before #448 those names were
|
||||
* hard-coded inline with NO guard, so renaming a tool left the agent stale
|
||||
* instructions and nothing failed.
|
||||
*
|
||||
* APPROACH — substitution + a precise source scan:
|
||||
* 1. The names now flow through the exported `PROMPT_TOOL_NAMES` const; this
|
||||
* test asserts every value there is a REAL in-app tool.
|
||||
* 2. A precise scan of the two guidance-note string literals in the source
|
||||
* catches any BARE tool-name token added directly (bypassing the const):
|
||||
* every camelCase token in those notes must be either a real tool name or an
|
||||
* explicitly-allowlisted ordinary English/camelCase word.
|
||||
*
|
||||
* The scan is deliberately narrow (only the guidance notes, only camelCase
|
||||
* tokens) so it never false-positives on prose, and the allowlist of non-tool
|
||||
* words is tiny and explicit.
|
||||
*/
|
||||
|
||||
// The authoritative set of real in-app tool names: shared-registry inAppKeys +
|
||||
// per-layer INLINE tool keys + the loadTools meta-tool.
|
||||
const VALID_TOOL_NAMES = new Set<string>([
|
||||
...Object.values(SHARED_TOOL_SPECS).map((s) => s.inAppKey),
|
||||
...Object.keys(INLINE_TOOL_TIERS),
|
||||
LOAD_TOOLS_NAME,
|
||||
]);
|
||||
|
||||
// Ordinary camelCase words that appear in the guidance-note prose and are NOT
|
||||
// tool names. Keep this list minimal and explicit — anything camelCase in a note
|
||||
// that is neither a real tool nor here fails the scan.
|
||||
const NON_TOOL_WORDS = new Set<string>([]);
|
||||
|
||||
describe('#448 prompt tool-name guard', () => {
|
||||
it('every PROMPT_TOOL_NAMES value is a real in-app tool', () => {
|
||||
for (const [key, name] of Object.entries(PROMPT_TOOL_NAMES)) {
|
||||
expect(typeof name).toBe('string');
|
||||
expect(VALID_TOOL_NAMES.has(name)).toBe(true);
|
||||
// Sanity: the const key and its value are the same token (the const is a
|
||||
// name->name map used purely to route mentions through one guarded place).
|
||||
expect(key).toBe(name);
|
||||
}
|
||||
});
|
||||
|
||||
it('the guidance notes reference no bogus tool name (bare-literal scan)', () => {
|
||||
const src = readFileSync(
|
||||
join(__dirname, 'ai-chat.prompt.ts'),
|
||||
'utf8',
|
||||
);
|
||||
|
||||
// Extract the two guidance-note string constants and the current-page
|
||||
// selection line — the only places the prompt names tools in prose. Each is
|
||||
// a `const NAME =` ... `;` block; we scan their raw text for camelCase
|
||||
// tokens. (Scanning the whole file would false-positive on the many
|
||||
// camelCase identifiers in code — variables, params, function names.)
|
||||
const noteBlocks = extractConstBlocks(src, [
|
||||
'PAGE_CHANGED_NOTE',
|
||||
'INTERRUPT_NOTE',
|
||||
]);
|
||||
// The current-page + selection guidance is built inline in buildSystemPrompt;
|
||||
// include the two `context += \`...\`` template lines that mention tools.
|
||||
const contextLines = src
|
||||
.split('\n')
|
||||
.filter((l) => l.includes('context +=') && l.includes('getCurrentPage'))
|
||||
.join('\n');
|
||||
|
||||
// Neutralize string-literal escape sequences (\n, \t, ...) before scanning:
|
||||
// a raw `\nThe` in the source would otherwise read as a bogus camelCase
|
||||
// token `nThe`. Replace any backslash-escape with a space.
|
||||
const scanText = (noteBlocks + '\n' + contextLines).replace(/\\./g, ' ');
|
||||
expect(scanText.length).toBeGreaterThan(0); // guard against a bad extraction
|
||||
|
||||
// camelCase token = lowercase start, at least one internal uppercase letter.
|
||||
const tokens = new Set(scanText.match(/\b[a-z][a-zA-Z0-9]*[A-Z][a-zA-Z0-9]*\b/g) ?? []);
|
||||
const offenders = [...tokens].filter(
|
||||
(t) => !VALID_TOOL_NAMES.has(t) && !NON_TOOL_WORDS.has(t),
|
||||
);
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
|
||||
it('the specific tools the notes rely on are all real (regression pins)', () => {
|
||||
for (const name of [
|
||||
'getPage',
|
||||
'editPageText',
|
||||
'patchNode',
|
||||
'insertNode',
|
||||
'deleteNode',
|
||||
'getCurrentPage',
|
||||
'loadTools',
|
||||
]) {
|
||||
expect(VALID_TOOL_NAMES.has(name)).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Extract the raw text of one or more top-level `const NAME = ... ;` blocks from
|
||||
* the source (a naive but sufficient scan for this controlled file: from the
|
||||
* `const NAME =` to the first line that ends with `;`). Returns the blocks
|
||||
* concatenated.
|
||||
*/
|
||||
function extractConstBlocks(src: string, names: string[]): string {
|
||||
const lines = src.split('\n');
|
||||
const out: string[] = [];
|
||||
for (const name of names) {
|
||||
const start = lines.findIndex((l) => l.trimStart().startsWith(`const ${name} =`));
|
||||
if (start < 0) continue;
|
||||
for (let i = start; i < lines.length; i++) {
|
||||
out.push(lines[i]);
|
||||
if (lines[i].trimEnd().endsWith(';')) break;
|
||||
}
|
||||
}
|
||||
return out.join('\n');
|
||||
}
|
||||
@@ -2,6 +2,30 @@ import { Workspace } from '@docmost/db/types/entity.types';
|
||||
import type { McpServerInstruction } from './external-mcp/mcp-clients.service';
|
||||
import type { ToolCatalogEntry } from './tools/tool-tiers';
|
||||
|
||||
/**
|
||||
* The in-app tool names this prompt refers to BY NAME in its guidance notes
|
||||
* (issue #448). Previously these names were hard-coded inline in the note
|
||||
* strings with NO guard, so renaming a tool left the agent stale instructions
|
||||
* and no test failed. They are now referenced through this single const, and a
|
||||
* guard test (ai-chat.prompt.tool-names.spec.ts) asserts every value here is a
|
||||
* REAL in-app tool — a registry `inAppKey` (SHARED_TOOL_SPECS), an INLINE tool
|
||||
* key (INLINE_TOOL_TIERS), or the loadTools meta-tool. Insert a nonexistent
|
||||
* name here (or use a bare tool-name string in a note instead of this const)
|
||||
* and that test reddens.
|
||||
*
|
||||
* `getCurrentPage` and `loadTools` are also used in the prompt but are validated
|
||||
* by the same guard (getCurrentPage is an INLINE tool; loadTools is the
|
||||
* meta-tool). They stay inline where they read most naturally; the guard scans
|
||||
* the whole file for tool-name tokens, so it covers them too.
|
||||
*/
|
||||
export const PROMPT_TOOL_NAMES = {
|
||||
getPage: 'getPage',
|
||||
editPageText: 'editPageText',
|
||||
patchNode: 'patchNode',
|
||||
insertNode: 'insertNode',
|
||||
deleteNode: 'deleteNode',
|
||||
} as const;
|
||||
|
||||
/**
|
||||
* Default agent persona used when the admin has not configured a custom system
|
||||
* prompt (`settings.ai.provider.systemPrompt`).
|
||||
@@ -91,15 +115,15 @@ const PAGE_CHANGED_NOTE =
|
||||
'NOTE: The user edited the open page AFTER your last response in this ' +
|
||||
'conversation, so any copy of that page you produced or remember from earlier ' +
|
||||
'is now STALE and must not be reused. Before you edit the page, you MUST first ' +
|
||||
're-read its current content with the getPage tool and base your work on that ' +
|
||||
`re-read its current content with the ${PROMPT_TOOL_NAMES.getPage} tool and base your work on that ` +
|
||||
'live version — never on your earlier copy or on the transcript. The unified ' +
|
||||
'diff below shows exactly what the user changed since you last spoke (lines ' +
|
||||
'starting with "-" were removed, "+" were added) and is the source of truth. ' +
|
||||
'Preserve every one of the user\'s edits: make the smallest change that ' +
|
||||
'satisfies the request using the targeted edit tools (editPageText, patchNode, ' +
|
||||
'insertNode, deleteNode) rather than replacing the whole page, and do not ' +
|
||||
'revert, drop, or overwrite anything the user changed. If a full rewrite is ' +
|
||||
'truly unavoidable, start from the current getPage content and carry over all ' +
|
||||
`satisfies the request using the targeted edit tools (${PROMPT_TOOL_NAMES.editPageText}, ${PROMPT_TOOL_NAMES.patchNode}, ` +
|
||||
`${PROMPT_TOOL_NAMES.insertNode}, ${PROMPT_TOOL_NAMES.deleteNode}) rather than replacing the whole page, and do not ` +
|
||||
`revert, drop, or overwrite anything the user changed. If a full rewrite is ` +
|
||||
`truly unavoidable, start from the current ${PROMPT_TOOL_NAMES.getPage} content and carry over all ` +
|
||||
'of the user\'s edits.';
|
||||
|
||||
/**
|
||||
|
||||
@@ -6,6 +6,7 @@ import { dirname, join } from "path";
|
||||
import { DocmostClient, DocmostMcpConfig } from "./client.js";
|
||||
import { parseNodeArg } from "@docmost/prosemirror-markdown";
|
||||
import { SHARED_TOOL_SPECS, SharedToolSpec } from "./tool-specs.js";
|
||||
import { SERVER_INSTRUCTIONS } from "./server-instructions.js";
|
||||
import {
|
||||
createCommentSignalTracker,
|
||||
CommentSignalTracker,
|
||||
@@ -74,19 +75,12 @@ 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.
|
||||
//
|
||||
// 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). Find every occurrence of a string/regex ON a page (and where each is) -> search_in_page, NOT block-by-block get_node — it returns each hit's node ref + block index + context for a targeted comment. 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). Draw.io diagrams -> drawio_create (create from mxGraph XML and insert), drawio_get (read a diagram as mxGraph XML + a hash), drawio_update (replace a diagram; pass the hash from drawio_get as baseHash for optimistic locking). 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.";
|
||||
// The guide is now SPLIT (issue #448): the hand-written routing prose lives in
|
||||
// server-instructions.ts and the tool INVENTORY is GENERATED from the registry
|
||||
// (SHARED_TOOL_SPECS + INLINE_MCP_INVENTORY), so it can no longer drift out of
|
||||
// sync with the registered tools. Re-exported here (its old home) so existing
|
||||
// importers are unaffected; the composition lives in server-instructions.ts.
|
||||
export { SERVER_INSTRUCTIONS };
|
||||
|
||||
// Helper to format JSON responses
|
||||
const jsonContent = (data: any) => ({
|
||||
|
||||
@@ -0,0 +1,237 @@
|
||||
// SERVER_INSTRUCTIONS — the editing guide surfaced to MCP clients in the
|
||||
// initialize result so they can pick the right tool by intent and avoid
|
||||
// resending whole documents.
|
||||
//
|
||||
// This guide is split into TWO parts that are composed at the bottom:
|
||||
//
|
||||
// 1. ROUTING_PROSE — the hand-written "when to use what" intent hints (READ /
|
||||
// EDIT / PAGES / COMMENTS / HISTORY). This is legitimately manual: it
|
||||
// encodes editorial judgement (which tool for which situation, the cheap-
|
||||
// first ordering, the guardrail nudges) that cannot be derived from the
|
||||
// registry. It is NOT the drift-guard for the tool set.
|
||||
//
|
||||
// 2. A GENERATED <tool_inventory> — every tool the server registers, listed
|
||||
// by name + one-line purpose, grouped by family, built from the SAME
|
||||
// registry the server registers tools from (SHARED_TOOL_SPECS' mcpName +
|
||||
// catalogLine) PLUS the handful of inline MCP-only tools (their inventory
|
||||
// lines live in INLINE_MCP_INVENTORY below). Because this list is BUILT
|
||||
// from the registry, it can never drift out of sync with the registered
|
||||
// tools — adding/renaming/removing a spec changes it automatically, with no
|
||||
// prose edit and no scraper test. An unmapped tool still appears (under
|
||||
// "OTHER"), so a new tool can never silently vanish from the guide.
|
||||
//
|
||||
// This replaces the old hand-maintained monolithic guide + its regex scraper
|
||||
// test (test/unit/server-instructions.test.mjs), which only checked that every
|
||||
// registered name appeared SOMEWHERE in the prose and drifted whenever a name
|
||||
// was reworded.
|
||||
//
|
||||
// OUT OF SCOPE (issue #448): the README / README.ru tool catalogs are still
|
||||
// hand-maintained prose and are NOT generated from this registry. Regenerating
|
||||
// them from SHARED_TOOL_SPECS is tracked separately as an optional docs script
|
||||
// under issue #412 — until then a tool rename still needs a manual README edit.
|
||||
|
||||
import { SHARED_TOOL_SPECS, SharedToolSpec } from "./tool-specs.js";
|
||||
|
||||
/**
|
||||
* The hand-written routing prose — the intent hints that tell a client which
|
||||
* tool to reach for in which situation. Kept manual on purpose (it encodes
|
||||
* editorial judgement, not a mechanical name list). The generated inventory
|
||||
* below is spliced in after it.
|
||||
*/
|
||||
export const ROUTING_PROSE =
|
||||
"Docmost editing guide — choose the tool by intent. The <tool_inventory> at the end lists every tool with a one-line purpose; the notes below are the routing hints for WHEN to reach for each.\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). Find every occurrence of a string/regex ON a page (and where each is) -> search_in_page, NOT block-by-block get_node — it returns each hit's node ref + block index + context for a targeted comment. 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). Draw.io diagrams -> drawio_create (create from mxGraph XML and insert), drawio_get (read a diagram as mxGraph XML + a hash), drawio_update (replace a diagram; pass the hash from drawio_get as baseHash for optimistic locking). 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.";
|
||||
|
||||
/**
|
||||
* A single generated inventory line: the tool's registered NAME + a one-line
|
||||
* purpose. For a registry tool the purpose is its `catalogLine` (falling back
|
||||
* to the first sentence of its description); for an inline MCP-only tool it is
|
||||
* the hand-written line in INLINE_MCP_INVENTORY.
|
||||
*/
|
||||
export interface ToolInventoryLine {
|
||||
name: string;
|
||||
purpose: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* The families the inventory is grouped under, in display order. A tool is
|
||||
* placed by looking its mcpName up in TOOL_FAMILY; anything not listed there
|
||||
* falls into "OTHER" so it is never dropped from the guide.
|
||||
*/
|
||||
const FAMILY_ORDER = [
|
||||
"READ",
|
||||
"EDIT",
|
||||
"PAGES",
|
||||
"COMMENTS",
|
||||
"HISTORY",
|
||||
"OTHER",
|
||||
] as const;
|
||||
type Family = (typeof FAMILY_ORDER)[number];
|
||||
|
||||
/**
|
||||
* mcpName -> family for the generated inventory grouping. Purely cosmetic (it
|
||||
* orders the inventory to mirror the routing prose); an unmapped tool still
|
||||
* appears under OTHER, so forgetting to add an entry here can never drop a tool
|
||||
* from the guide — it only lands it in the catch-all group.
|
||||
*/
|
||||
const TOOL_FAMILY: Record<string, Family> = {
|
||||
// READ
|
||||
search: "READ",
|
||||
list_pages: "READ",
|
||||
list_spaces: "READ",
|
||||
get_outline: "READ",
|
||||
get_node: "READ",
|
||||
search_in_page: "READ",
|
||||
get_page: "READ",
|
||||
get_page_json: "READ",
|
||||
get_workspace: "READ",
|
||||
stash_page: "READ",
|
||||
// EDIT
|
||||
edit_page_text: "EDIT",
|
||||
patch_node: "EDIT",
|
||||
insert_node: "EDIT",
|
||||
delete_node: "EDIT",
|
||||
update_page_json: "EDIT",
|
||||
table_get: "EDIT",
|
||||
table_update_cell: "EDIT",
|
||||
table_insert_row: "EDIT",
|
||||
table_delete_row: "EDIT",
|
||||
insert_image: "EDIT",
|
||||
replace_image: "EDIT",
|
||||
insert_footnote: "EDIT",
|
||||
drawio_get: "EDIT",
|
||||
drawio_create: "EDIT",
|
||||
drawio_update: "EDIT",
|
||||
docmost_transform: "EDIT",
|
||||
// PAGES
|
||||
create_page: "PAGES",
|
||||
rename_page: "PAGES",
|
||||
move_page: "PAGES",
|
||||
delete_page: "PAGES",
|
||||
copy_page_content: "PAGES",
|
||||
share_page: "PAGES",
|
||||
unshare_page: "PAGES",
|
||||
list_shares: "PAGES",
|
||||
// COMMENTS
|
||||
create_comment: "COMMENTS",
|
||||
list_comments: "COMMENTS",
|
||||
update_comment: "COMMENTS",
|
||||
resolve_comment: "COMMENTS",
|
||||
delete_comment: "COMMENTS",
|
||||
check_new_comments: "COMMENTS",
|
||||
// HISTORY
|
||||
diff_page_versions: "HISTORY",
|
||||
list_page_history: "HISTORY",
|
||||
restore_page_version: "HISTORY",
|
||||
export_page_markdown: "HISTORY",
|
||||
import_page_markdown: "HISTORY",
|
||||
};
|
||||
|
||||
/**
|
||||
* Inventory lines for the INLINE MCP-only tools — the ones registered directly
|
||||
* in index.ts (not via SHARED_TOOL_SPECS) because they diverge per transport or
|
||||
* exist only on this standalone surface. They carry no `catalogLine`, so their
|
||||
* one-line purpose is hand-written here. This is the ONLY hand-maintained tool
|
||||
* list left, and it is tiny; a new inline tool without an entry here still
|
||||
* surfaces via the completeness guard in tool-specs.test.mjs's sibling test.
|
||||
*/
|
||||
export const INLINE_MCP_INVENTORY: ToolInventoryLine[] = [
|
||||
{
|
||||
name: "table_get",
|
||||
purpose:
|
||||
"read a table as a matrix of cell texts + per-cell paragraph ids.",
|
||||
},
|
||||
{
|
||||
name: "search",
|
||||
purpose:
|
||||
"full-text search for pages and content across the whole workspace.",
|
||||
},
|
||||
{
|
||||
name: "docmost_transform",
|
||||
purpose:
|
||||
"edit a page by running a sandboxed JS `(doc, ctx) => doc` transform, with a dryRun diff preview.",
|
||||
},
|
||||
{
|
||||
name: "update_comment",
|
||||
purpose: "update an existing comment's content (creator only).",
|
||||
},
|
||||
{
|
||||
name: "delete_comment",
|
||||
purpose: "delete a comment (creator or space admin only).",
|
||||
},
|
||||
];
|
||||
|
||||
/**
|
||||
* Derive the one-line purpose for a registry spec: prefer its hand-written
|
||||
* `catalogLine` (already a "name — purpose" line — we take the purpose after
|
||||
* the em dash), else fall back to the first sentence of its description.
|
||||
*/
|
||||
function purposeForSpec(spec: SharedToolSpec): string {
|
||||
const line = spec.catalogLine?.trim();
|
||||
if (line) {
|
||||
const dash = line.indexOf(" — ");
|
||||
if (dash >= 0) return line.slice(dash + 3).trim();
|
||||
return line;
|
||||
}
|
||||
const desc = (spec.description ?? "").replace(/\s+/g, " ").trim();
|
||||
const firstSentence = desc.split(/(?<=[.!?])\s/)[0];
|
||||
return firstSentence || desc || "(no description)";
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the flat list of every registered tool's inventory line: one per shared
|
||||
* registry spec (skipping `inAppOnly` specs, which are not registered on this
|
||||
* MCP host) PLUS every inline MCP-only tool. Pure and deterministic — the
|
||||
* registry drives it, so it can never drift from what index.ts registers.
|
||||
*/
|
||||
export function buildToolInventoryLines(
|
||||
specs: Record<string, SharedToolSpec> = SHARED_TOOL_SPECS,
|
||||
inline: ToolInventoryLine[] = INLINE_MCP_INVENTORY,
|
||||
): ToolInventoryLine[] {
|
||||
const lines: ToolInventoryLine[] = [];
|
||||
for (const spec of Object.values(specs)) {
|
||||
if (spec.inAppOnly) continue; // not registered on the MCP host
|
||||
lines.push({ name: spec.mcpName, purpose: purposeForSpec(spec) });
|
||||
}
|
||||
for (const l of inline) lines.push({ ...l });
|
||||
return lines;
|
||||
}
|
||||
|
||||
/**
|
||||
* Render the generated `<tool_inventory>` block: every tool name + purpose,
|
||||
* grouped by family (families in FAMILY_ORDER; tools within a family sorted by
|
||||
* name for stable output; unmapped tools fall into OTHER). Pure.
|
||||
*/
|
||||
export function buildToolInventory(
|
||||
specs: Record<string, SharedToolSpec> = SHARED_TOOL_SPECS,
|
||||
inline: ToolInventoryLine[] = INLINE_MCP_INVENTORY,
|
||||
): string {
|
||||
const byFamily = new Map<Family, ToolInventoryLine[]>();
|
||||
for (const family of FAMILY_ORDER) byFamily.set(family, []);
|
||||
for (const line of buildToolInventoryLines(specs, inline)) {
|
||||
const family = TOOL_FAMILY[line.name] ?? "OTHER";
|
||||
byFamily.get(family)!.push(line);
|
||||
}
|
||||
const sections: string[] = [];
|
||||
for (const family of FAMILY_ORDER) {
|
||||
const items = byFamily.get(family)!;
|
||||
if (items.length === 0) continue;
|
||||
items.sort((a, b) => a.name.localeCompare(b.name));
|
||||
for (const item of items) {
|
||||
sections.push(` ${family} ${item.name} — ${item.purpose}`);
|
||||
}
|
||||
}
|
||||
return ["<tool_inventory>", ...sections, "</tool_inventory>"].join("\n");
|
||||
}
|
||||
|
||||
/**
|
||||
* The composed editing guide: the hand-written routing prose followed by the
|
||||
* generated, drift-proof tool inventory. Exported (and used by index.ts /
|
||||
* createDocmostMcpServer) as the MCP server's `instructions`.
|
||||
*/
|
||||
export const SERVER_INSTRUCTIONS =
|
||||
ROUTING_PROSE + "\n" + buildToolInventory();
|
||||
@@ -14,10 +14,15 @@
|
||||
// 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.
|
||||
// SERVER_INSTRUCTIONS note (issue #448): the intent-routing guide MCP clients
|
||||
// receive on initialize is now SPLIT — its tool INVENTORY is GENERATED from this
|
||||
// registry (mcpName + catalogLine) by server-instructions.ts, so adding /
|
||||
// renaming / removing a spec here updates the guide's inventory AUTOMATICALLY;
|
||||
// no prose edit is needed. Only an INLINE MCP-only tool (registerTool in
|
||||
// index.ts, not a spec here) needs a hand-written line in INLINE_MCP_INVENTORY —
|
||||
// enforced by test/unit/tool-inventory.test.mjs. The routing PROSE (the "when to
|
||||
// use what" hints) in server-instructions.ts stays manual, but it is no longer a
|
||||
// drift-guard for the tool set.
|
||||
|
||||
// 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
|
||||
|
||||
@@ -1,82 +0,0 @@
|
||||
// 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`,
|
||||
);
|
||||
}
|
||||
});
|
||||
@@ -0,0 +1,104 @@
|
||||
// Guard: the GENERATED <tool_inventory> in SERVER_INSTRUCTIONS (issue #448)
|
||||
// names every tool the server registers. The inventory is BUILT from the
|
||||
// registry (SHARED_TOOL_SPECS' mcpName/catalogLine + INLINE_MCP_INVENTORY), so
|
||||
// the shared-registry tools can never drift by construction; this test's job is
|
||||
// to catch the ONE remaining manual list — INLINE_MCP_INVENTORY — falling out
|
||||
// of sync with the inline `server.registerTool(...)` calls in index.ts.
|
||||
//
|
||||
// It also asserts the composed guide keeps its routing prose (the hand-written
|
||||
// intent hints) and is a valid non-empty string — the structural guarantees the
|
||||
// old name-scraper test (server-instructions.test.mjs, now deleted) carried,
|
||||
// minus its now-redundant per-name prose scrape.
|
||||
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,
|
||||
ROUTING_PROSE,
|
||||
buildToolInventoryLines,
|
||||
} from "../../build/server-instructions.js";
|
||||
|
||||
const HERE = dirname(fileURLToPath(import.meta.url));
|
||||
const SRC = join(HERE, "..", "..", "src");
|
||||
|
||||
/**
|
||||
* Every tool name the MCP server registers, scraped from the SOURCE:
|
||||
* - inline `server.registerTool("name", ...)` calls in index.ts;
|
||||
* - shared specs in tool-specs.ts (`mcpName: 'name'`).
|
||||
* Same two registration mechanisms the old guard covered.
|
||||
*/
|
||||
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("the generated inventory names every registered tool", () => {
|
||||
const registered = registeredToolNames();
|
||||
// Sanity: if the scrape regressed (regex drift), fail loudly rather than
|
||||
// vacuously passing on an empty set.
|
||||
assert.ok(
|
||||
registered.size >= 40,
|
||||
`sanity: expected 40+ registered tools, got ${registered.size} — ` +
|
||||
"the extraction regexes in this test likely drifted from the source",
|
||||
);
|
||||
const inventory = new Set(buildToolInventoryLines().map((l) => l.name));
|
||||
const missing = [...registered].filter((n) => !inventory.has(n)).sort();
|
||||
assert.deepEqual(
|
||||
missing,
|
||||
[],
|
||||
`tools missing from the generated <tool_inventory>: ${missing.join(", ")} — ` +
|
||||
"a SHARED spec is covered automatically; an INLINE MCP-only tool needs a " +
|
||||
"line added to INLINE_MCP_INVENTORY in src/server-instructions.ts",
|
||||
);
|
||||
});
|
||||
|
||||
test("the inventory has no phantom tool (every line is a real registered tool)", () => {
|
||||
const registered = registeredToolNames();
|
||||
const phantom = buildToolInventoryLines()
|
||||
.map((l) => l.name)
|
||||
.filter((n) => !registered.has(n))
|
||||
.sort();
|
||||
assert.deepEqual(
|
||||
phantom,
|
||||
[],
|
||||
`<tool_inventory> lists tools that are NOT registered: ${phantom.join(", ")}`,
|
||||
);
|
||||
});
|
||||
|
||||
test("every inventory line has a non-empty purpose", () => {
|
||||
for (const line of buildToolInventoryLines()) {
|
||||
assert.equal(typeof line.purpose, "string");
|
||||
assert.ok(line.purpose.trim().length > 0, `${line.name}: empty purpose`);
|
||||
}
|
||||
});
|
||||
|
||||
test("SERVER_INSTRUCTIONS keeps the routing prose and the generated inventory", () => {
|
||||
assert.equal(typeof SERVER_INSTRUCTIONS, "string");
|
||||
assert.ok(SERVER_INSTRUCTIONS.length > 0, "SERVER_INSTRUCTIONS is empty");
|
||||
// Routing prose is spliced in verbatim (the hand-written intent hints).
|
||||
assert.ok(
|
||||
SERVER_INSTRUCTIONS.startsWith(ROUTING_PROSE),
|
||||
"the routing prose is not preserved at the head of the guide",
|
||||
);
|
||||
// The generated inventory block is present.
|
||||
assert.match(SERVER_INSTRUCTIONS, /<tool_inventory>/);
|
||||
assert.match(SERVER_INSTRUCTIONS, /<\/tool_inventory>/);
|
||||
// The routing families are still present in the prose.
|
||||
for (const family of ["READ:", "EDIT:", "PAGES:", "COMMENTS:", "HISTORY:"]) {
|
||||
assert.ok(
|
||||
SERVER_INSTRUCTIONS.includes(family),
|
||||
`routing prose lost its ${family} section`,
|
||||
);
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user