7cb3199d09
Один логический тул жил под двумя именами: внешний MCP snake_case (edit_page_text), in-app camelCase (editPageText) — дублирование доков, путаница при переносе промптов/скиллов, помеха шарингу спек (#294). Решение владельца: единый camelCase везде, включая внешний MCP. После этого mcpName === inAppKey. - tool-specs.ts: mcpName ВЫВЕДЕН из ключа спеки (mcpName == inAppKey) для всех 43 shared-спек — раньше divergent snake, теперь равен ключу (проверено: mcpName читается только структурно — цикл регистрации, генератор <tool_inventory>, TOOL_FAMILY). +5 inline-регистраций (tableGet/updateComment/deleteComment/ docmostTransform; search без изменений). Рантайм: 47 тулов, все camelCase, ноль подчёркиваний. - Контракт-конвенция ИНВЕРТИРОВАНА: shared-tool-specs.contract.spec `mcpName === toSnake(inAppKey)` → `mcpName === inAppKey`; tool-specs.test и tool-inventory.test обновлены. - ROUTING_PROSE/TOOL_FAMILY/INLINE_MCP_INVENTORY (server-instructions.ts) → camelCase (105 замен). ai-chat.prompt/guard уже на in-app camelCase-ключах — без изменений (guard прошёл). comment-signal EXCLUDED_TOOLS схлопнут с дублей snake+camel до camelCase. - Некоторое неочевидное: assertUnambiguousMatch(op: "patch_node"|"delete_node") в prosemirror-markdown/node-ops — op интерполируется в model-facing ошибку; литерал-юнион + call-sites → "patchNode"|"deleteNode". - Все snake-имена в описаниях/error-строках/комментах/тестах/доках → camelCase (whole-token, longest-match-first). CHANGELOG: BREAKING-таблица 46 строк + миграция (allowlists mcp__gitmost-*__get_node→__getNode, промпты/скиллы, .mcp.json, метрики по tool-label); релизится вместе с #411. Внутренние имена методов (PageService.updatePageContent и т.п.) НЕ тронуты — переименованы только ИМЕНА ТУЛОВ. Гейт: mcp node --test 677/677; tsc -p apps/server чисто; jest ai-chat-tools. service + shared-tool-specs.contract + tool-tiers + ai-chat.prompt + comment-signal-inapp → 323. Второй линк breaking-окна (#411→ЭТОТ→#413→#415). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
136 lines
5.5 KiB
JavaScript
136 lines
5.5 KiB
JavaScript
// 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'`), EXCEPT `inAppOnly`
|
|
* specs, which the registry loop in index.ts SKIPS on the MCP host (#411).
|
|
* 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-zA-Z0-9_]+)"/g)) {
|
|
names.add(m[1]);
|
|
}
|
|
// Each spec is one `{ ... }` block; scrape its mcpName but skip a block that
|
|
// carries `inAppOnly: true` (not registered on the external MCP host).
|
|
for (const block of specsSrc.split(/\n\s{2}\w+:\s*\{/)) {
|
|
const nameMatch = block.match(/mcpName:\s*['"]([a-zA-Z0-9_]+)['"]/);
|
|
if (!nameMatch) continue;
|
|
if (/inAppOnly:\s*true/.test(block)) continue;
|
|
names.add(nameMatch[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(", ")}`,
|
|
);
|
|
});
|
|
|
|
// #411: the external MCP surface gains updatePageMarkdown and LOSES
|
|
// importPageMarkdown (now inAppOnly). The in-app agent still keeps
|
|
// importPageMarkdown — asserted in the server-side contract spec. (#412 renamed
|
|
// both public MCP tool names to camelCase.)
|
|
test("updatePageMarkdown is on the MCP surface; importPageMarkdown is NOT", () => {
|
|
const inventory = new Set(buildToolInventoryLines().map((l) => l.name));
|
|
assert.ok(
|
|
inventory.has("updatePageMarkdown"),
|
|
"updatePageMarkdown should be registered on the external MCP surface",
|
|
);
|
|
assert.ok(
|
|
!inventory.has("importPageMarkdown"),
|
|
"importPageMarkdown must be dropped from the external MCP surface (#411)",
|
|
);
|
|
// And the routing prose no longer points MCP clients at it.
|
|
assert.ok(
|
|
!ROUTING_PROSE.includes("importPageMarkdown"),
|
|
"ROUTING_PROSE still mentions the removed importPageMarkdown",
|
|
);
|
|
assert.ok(
|
|
ROUTING_PROSE.includes("updatePageMarkdown"),
|
|
"ROUTING_PROSE should mention updatePageMarkdown",
|
|
);
|
|
});
|
|
|
|
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`,
|
|
);
|
|
}
|
|
});
|