diff --git a/AGENTS.md b/AGENTS.md index ead439f1b..0f9a8f332 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,7 +12,7 @@ agentmemory is a persistent memory system for AI coding agents, built on iii-eng ## Consistency Rules **When adding or removing MCP tools, you MUST update ALL of the following:** -1. `src/mcp/tools-registry.ts` — tool definition + `getAllTools()` array +1. `src/mcp/tools-registry.ts` — tool definition + `getAllTools()` array (every tool must include `annotations` with `readOnlyHint`/`destructiveHint`: read-only tools get `readOnlyHint: true`, destructive tools get `destructiveHint: true`) 2. `src/mcp/server.ts` — handler case in the `mcp::tools::call` switch 3. `src/triggers/api.ts` — REST endpoint registration 4. `src/index.ts` — function registration + endpoint count in the log line diff --git a/src/mcp/tools-registry.ts b/src/mcp/tools-registry.ts index c4df3499c..f787edefb 100644 --- a/src/mcp/tools-registry.ts +++ b/src/mcp/tools-registry.ts @@ -1,3 +1,11 @@ +export type McpToolAnnotations = { + readOnlyHint?: boolean; + destructiveHint?: boolean; + idempotentHint?: boolean; + openWorldHint?: boolean; + title?: string; +}; + export type McpToolDef = { name: string; description: string; @@ -6,6 +14,7 @@ export type McpToolDef = { properties: Record; required?: string[]; }; + annotations?: McpToolAnnotations; }; export const CORE_TOOLS: McpToolDef[] = [ @@ -35,6 +44,7 @@ export const CORE_TOOLS: McpToolDef[] = [ }, required: ["query"], }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_compress_file", @@ -50,6 +60,7 @@ export const CORE_TOOLS: McpToolDef[] = [ }, required: ["filePath"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_save", @@ -86,6 +97,7 @@ export const CORE_TOOLS: McpToolDef[] = [ }, required: ["content"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_file_history", @@ -101,6 +113,7 @@ export const CORE_TOOLS: McpToolDef[] = [ }, required: ["files"], }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_patterns", @@ -111,12 +124,14 @@ export const CORE_TOOLS: McpToolDef[] = [ project: { type: "string", description: "Project path to analyze" }, }, }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_sessions", description: "List recent sessions with their status and observation counts.", inputSchema: { type: "object", properties: {} }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_smart_search", @@ -133,6 +148,7 @@ export const CORE_TOOLS: McpToolDef[] = [ }, required: ["query"], }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_vision_search", @@ -148,6 +164,7 @@ export const CORE_TOOLS: McpToolDef[] = [ sessionId: { type: "string", description: "Filter to a single session" }, }, }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_timeline", @@ -171,6 +188,7 @@ export const CORE_TOOLS: McpToolDef[] = [ }, required: ["anchor"], }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_profile", @@ -186,11 +204,13 @@ export const CORE_TOOLS: McpToolDef[] = [ }, required: ["project"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_export", description: "Export all memory data as JSON.", inputSchema: { type: "object", properties: {} }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_relations", @@ -213,6 +233,7 @@ export const CORE_TOOLS: McpToolDef[] = [ }, required: ["memoryId"], }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_commit_lookup", @@ -225,6 +246,7 @@ export const CORE_TOOLS: McpToolDef[] = [ }, required: ["sha"], }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_commits", @@ -238,6 +260,7 @@ export const CORE_TOOLS: McpToolDef[] = [ limit: { type: "number", description: "Max results (default 100, max 500)" }, }, }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, ]; @@ -257,6 +280,7 @@ export const V040_TOOLS: McpToolDef[] = [ }, required: ["direction"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_graph_query", @@ -276,6 +300,7 @@ export const V040_TOOLS: McpToolDef[] = [ query: { type: "string", description: "Search nodes by name" }, }, }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_consolidate", @@ -290,6 +315,7 @@ export const V040_TOOLS: McpToolDef[] = [ }, }, }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_team_share", @@ -308,6 +334,7 @@ export const V040_TOOLS: McpToolDef[] = [ }, required: ["itemId", "itemType"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_team_feed", @@ -318,6 +345,7 @@ export const V040_TOOLS: McpToolDef[] = [ limit: { type: "number", description: "Max items (default 20)" }, }, }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_audit", @@ -329,6 +357,7 @@ export const V040_TOOLS: McpToolDef[] = [ limit: { type: "number", description: "Max entries (default 50)" }, }, }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_governance_delete", @@ -344,6 +373,7 @@ export const V040_TOOLS: McpToolDef[] = [ }, required: ["memoryIds"], }, + annotations: { readOnlyHint: false, destructiveHint: true }, }, { name: "memory_snapshot_create", @@ -354,6 +384,7 @@ export const V040_TOOLS: McpToolDef[] = [ message: { type: "string", description: "Snapshot description" }, }, }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, ]; @@ -391,6 +422,7 @@ export const V050_TOOLS: McpToolDef[] = [ }, required: ["title"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_action_update", @@ -412,6 +444,7 @@ export const V050_TOOLS: McpToolDef[] = [ }, required: ["actionId"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_frontier", @@ -428,6 +461,7 @@ export const V050_TOOLS: McpToolDef[] = [ limit: { type: "number", description: "Max results (default 20)" }, }, }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_next", @@ -440,6 +474,7 @@ export const V050_TOOLS: McpToolDef[] = [ agentId: { type: "string", description: "Current agent ID" }, }, }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_lease", @@ -465,6 +500,7 @@ export const V050_TOOLS: McpToolDef[] = [ }, required: ["actionId", "agentId", "operation"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_routine_run", @@ -479,6 +515,7 @@ export const V050_TOOLS: McpToolDef[] = [ }, required: ["routineId"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_signal_send", @@ -504,6 +541,7 @@ export const V050_TOOLS: McpToolDef[] = [ }, required: ["from", "content"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_signal_read", @@ -525,6 +563,7 @@ export const V050_TOOLS: McpToolDef[] = [ }, required: ["agentId"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_checkpoint", @@ -558,6 +597,7 @@ export const V050_TOOLS: McpToolDef[] = [ }, required: ["operation"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_mesh_sync", @@ -576,6 +616,7 @@ export const V050_TOOLS: McpToolDef[] = [ }, }, }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, ]; @@ -604,6 +645,7 @@ export const V051_TOOLS: McpToolDef[] = [ }, required: ["name", "type"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_sentinel_trigger", @@ -617,6 +659,7 @@ export const V051_TOOLS: McpToolDef[] = [ }, required: ["sentinelId"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_sketch_create", @@ -632,6 +675,7 @@ export const V051_TOOLS: McpToolDef[] = [ }, required: ["title"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_sketch_promote", @@ -645,6 +689,7 @@ export const V051_TOOLS: McpToolDef[] = [ }, required: ["sketchId"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_crystallize", @@ -662,6 +707,7 @@ export const V051_TOOLS: McpToolDef[] = [ }, required: ["actionIds"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_diagnose", @@ -676,6 +722,7 @@ export const V051_TOOLS: McpToolDef[] = [ }, }, }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_heal", @@ -694,6 +741,7 @@ export const V051_TOOLS: McpToolDef[] = [ }, }, }, + annotations: { readOnlyHint: false, destructiveHint: true }, }, { name: "memory_facet_tag", @@ -712,6 +760,7 @@ export const V051_TOOLS: McpToolDef[] = [ }, required: ["targetId", "targetType", "dimension", "value"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_facet_query", @@ -734,6 +783,7 @@ export const V051_TOOLS: McpToolDef[] = [ }, }, }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, ]; @@ -752,6 +802,7 @@ export const V061_TOOLS: McpToolDef[] = [ }, required: ["id"], }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, ]; @@ -780,6 +831,7 @@ export const V070_TOOLS: McpToolDef[] = [ }, required: ["content"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_lesson_recall", @@ -798,6 +850,7 @@ export const V070_TOOLS: McpToolDef[] = [ }, required: ["query"], }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_obsidian_export", @@ -816,6 +869,7 @@ export const V070_TOOLS: McpToolDef[] = [ }, }, }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, ]; @@ -834,6 +888,7 @@ export const V073_TOOLS: McpToolDef[] = [ }, }, }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_insight_list", @@ -850,6 +905,7 @@ export const V073_TOOLS: McpToolDef[] = [ limit: { type: "number", description: "Max results (default 50)" }, }, }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, ]; @@ -859,6 +915,7 @@ export const V010_SLOTS_TOOLS: McpToolDef[] = [ description: "List all memory slots (pinned + project + global). Slots are editable, size-limited memory units the agent can read and modify across sessions.", inputSchema: { type: "object", properties: {} }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_slot_get", @@ -870,6 +927,7 @@ export const V010_SLOTS_TOOLS: McpToolDef[] = [ }, required: ["label"], }, + annotations: { readOnlyHint: true, destructiveHint: false }, }, { name: "memory_slot_create", @@ -886,6 +944,7 @@ export const V010_SLOTS_TOOLS: McpToolDef[] = [ }, required: ["label"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_slot_append", @@ -899,6 +958,7 @@ export const V010_SLOTS_TOOLS: McpToolDef[] = [ }, required: ["label", "text"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_slot_replace", @@ -911,6 +971,7 @@ export const V010_SLOTS_TOOLS: McpToolDef[] = [ }, required: ["label", "content"], }, + annotations: { readOnlyHint: false, destructiveHint: false }, }, { name: "memory_slot_delete", @@ -922,6 +983,7 @@ export const V010_SLOTS_TOOLS: McpToolDef[] = [ }, required: ["label"], }, + annotations: { readOnlyHint: false, destructiveHint: true }, }, ]; diff --git a/test/mcp-tool-annotations.test.ts b/test/mcp-tool-annotations.test.ts new file mode 100644 index 000000000..d8a778f75 --- /dev/null +++ b/test/mcp-tool-annotations.test.ts @@ -0,0 +1,199 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; + +vi.mock("../src/logger.js", () => ({ + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +vi.mock("node:fs", () => ({ + existsSync: vi.fn().mockReturnValue(false), + readFileSync: vi.fn(), + writeFileSync: vi.fn(), + mkdirSync: vi.fn(), +})); + +vi.mock("../src/mcp/transport.js", () => ({ + createStdioTransport: vi.fn(() => ({ start: vi.fn(), stop: vi.fn() })), +})); + +vi.mock("../src/config.js", () => ({ + getStandalonePersistPath: vi.fn(() => "/tmp/test-annotations.json"), +})); + +import { + getAllTools, + getVisibleTools, + ESSENTIAL_TOOLS, + type McpToolDef, +} from "../src/mcp/tools-registry.js"; +import { handleToolsList } from "../src/mcp/standalone.js"; +import { + resetHandleForTests, + setLivezProbe, +} from "../src/mcp/rest-proxy.js"; + +const READ_ONLY_TOOLS = new Set([ + "memory_recall", + "memory_file_history", + "memory_patterns", + "memory_sessions", + "memory_smart_search", + "memory_vision_search", + "memory_timeline", + "memory_export", + "memory_relations", + "memory_commit_lookup", + "memory_commits", + "memory_graph_query", + "memory_team_feed", + "memory_audit", + "memory_frontier", + "memory_next", + "memory_diagnose", + "memory_facet_query", + "memory_verify", + "memory_lesson_recall", + "memory_insight_list", + "memory_slot_list", + "memory_slot_get", +]); + +const DESTRUCTIVE_TOOLS = new Set([ + "memory_governance_delete", + "memory_heal", + "memory_slot_delete", +]); + +const TOTAL = getAllTools().length; + +const instantLocalFallbackProbe = vi.fn(async () => ({ + ok: false, + status: 0, + statusText: "stubbed: forced local fallback", +})); + +const fetchTrap = vi.fn(async (url: unknown) => { + throw new Error( + `unexpected real fetch() in mcp-tool-annotations.test.ts: ${String(url)}`, + ); +}); + +function classification(tool: McpToolDef): string { + const ro = tool.annotations?.readOnlyHint === true; + const de = tool.annotations?.destructiveHint === true; + if (ro && !de) return "read-only"; + if (!ro && de) return "destructive"; + if (!ro && !de) return "state-changing"; + return "invalid"; +} + +describe("MCP tool risk annotations", () => { + it("every tool carries an annotations object", () => { + for (const tool of getAllTools()) { + expect(tool.annotations, `tool ${tool.name} missing annotations`).toBeDefined(); + expect(typeof tool.annotations).toBe("object"); + } + }); + + it("no tool is both read-only and destructive", () => { + for (const tool of getAllTools()) { + const a = tool.annotations; + if (a?.readOnlyHint && a?.destructiveHint) { + throw new Error(`tool ${tool.name} is both readOnly and destructive`); + } + } + }); + + it("read-only set is exactly 23 tools with readOnlyHint true + destructiveHint false", () => { + const tools = getAllTools(); + const ro = tools.filter((t) => classification(t) === "read-only"); + expect(new Set(ro.map((t) => t.name))).toEqual(READ_ONLY_TOOLS); + expect(ro.length).toBe(23); + for (const t of ro) { + expect(t.annotations?.readOnlyHint).toBe(true); + expect(t.annotations?.destructiveHint).toBe(false); + } + }); + + it("destructive set is exactly 3 tools with destructiveHint true + readOnlyHint false", () => { + const tools = getAllTools(); + const de = tools.filter((t) => classification(t) === "destructive"); + expect(new Set(de.map((t) => t.name))).toEqual(DESTRUCTIVE_TOOLS); + expect(de.length).toBe(3); + for (const t of de) { + expect(t.annotations?.destructiveHint).toBe(true); + expect(t.annotations?.readOnlyHint).toBe(false); + } + }); + + it("remaining 27 tools are state-changing (readOnlyHint false, destructiveHint false)", () => { + const tools = getAllTools(); + const sc = tools.filter((t) => classification(t) === "state-changing"); + expect(sc.length).toBe(27); + for (const t of sc) { + expect(t.annotations?.readOnlyHint).toBe(false); + expect(t.annotations?.destructiveHint).toBe(false); + expect(READ_ONLY_TOOLS.has(t.name)).toBe(false); + expect(DESTRUCTIVE_TOOLS.has(t.name)).toBe(false); + } + }); + + it("every tool falls into exactly one classification (23 + 3 + 27 covers total)", () => { + const tools = getAllTools(); + expect(tools.length).toBe(TOTAL); + const counts = { "read-only": 0, destructive: 0, "state-changing": 0, invalid: 0 }; + for (const t of tools) counts[classification(t) as keyof typeof counts]++; + expect(counts["read-only"]).toBe(23); + expect(counts.destructive).toBe(3); + expect(counts["state-changing"]).toBe(27); + expect(counts.invalid).toBe(0); + expect(counts["read-only"] + counts.destructive + counts["state-changing"]).toBe(TOTAL); + }); + + it("getVisibleTools preserves annotations in every visibility mode", () => { + const prev = process.env["AGENTMEMORY_TOOLS"]; + try { + for (const mode of ["all", "core"]) { + process.env["AGENTMEMORY_TOOLS"] = mode; + const visible = getVisibleTools(); + if (mode === "all") expect(visible.length).toBe(TOTAL); + if (mode === "core") expect(visible.length).toBe(ESSENTIAL_TOOLS.size); + for (const tool of visible) { + expect(tool.annotations, `tool ${tool.name} lost annotations in ${mode} mode`).toBeDefined(); + } + } + } finally { + if (prev === undefined) delete process.env["AGENTMEMORY_TOOLS"]; + else process.env["AGENTMEMORY_TOOLS"] = prev; + } + }); +}); + +describe("MCP tools/list wire response carries annotations", () => { + const originalFetch = globalThis.fetch; + + beforeEach(() => { + instantLocalFallbackProbe.mockClear(); + fetchTrap.mockClear(); + resetHandleForTests(); + setLivezProbe(instantLocalFallbackProbe); + (globalThis as { fetch: typeof fetch }).fetch = + fetchTrap as unknown as typeof fetch; + }); + + afterEach(() => { + (globalThis as { fetch: typeof fetch }).fetch = originalFetch; + resetHandleForTests(); + }); + + it("handleToolsList returns tools whose annotations match the registry", async () => { + const registry = new Map(getAllTools().map((t) => [t.name, t])); + const res = await handleToolsList(); + expect(Array.isArray(res.tools)).toBe(true); + expect(res.tools.length).toBeGreaterThan(0); + for (const wire of res.tools as McpToolDef[]) { + const reg = registry.get(wire.name); + expect(reg, `wire tool ${wire.name} not in registry`).toBeDefined(); + expect(wire.annotations).toEqual(reg?.annotations); + } + }); +});