fix(coding-agents): identify safe MCP knowledge reads

hindsight-integrations/coding-agents/src/core/knowledge-tools.ts

@@ -1,10 +1,12 @@
 /**
- * Knowledge-page MCP tool specs — SDK-free so this stays unit-testable without a real MCP host.
+ * Knowledge-page MCP tool specs — runtime SDK-free so this stays unit-testable without a real MCP
+ * host.
  *
- * `src/mcp-server.ts` is the only file that imports the MCP SDK; it wires the specs returned here
- * into an `McpServer`. Each tool wraps one `HindsightClient` knowledge-page/recall method: it never
- * throws — a thrown client error is caught and turned into an `isError:true` text result so the
- * calling LLM sees the failure instead of the process crashing.
+ * `src/mcp-server.ts` is the only file with a runtime MCP SDK import; this module uses only its
+ * `ToolAnnotations` type. The server wires the specs returned here into an `McpServer`. Each tool
+ * wraps one `HindsightClient` knowledge-page/recall method: it never throws — a thrown client error
+ * is caught and turned into an `isError:true` text result so the calling LLM sees the failure
+ * instead of the process crashing.
  *
  * The agent-facing surface is intentionally curated: grounding + capture only. Raw page CRUD
  * (create/update/delete) is deliberately NOT exposed — agents never author page structure; they
@@ -16,6 +18,7 @@ import { z } from "zod";
 import { existsSync } from "node:fs";
 import { homedir } from "node:os";
 import { join } from "node:path";
+import type { ToolAnnotations } from "@modelcontextprotocol/sdk/types.js";
 import type { ZodRawShape } from "zod";
 import type { HindsightClient } from "./hindsight";
 import { syncStatus } from "./status";
@@ -33,21 +36,39 @@ export interface ToolResult {
   isError?: boolean;
 }
 
+type ToolSafetyAnnotations = Required<
+  Pick<ToolAnnotations, "readOnlyHint" | "destructiveHint" | "idempotentHint" | "openWorldHint">
+>;
+
+const READ_ONLY_ANNOTATIONS: ToolSafetyAnnotations = {
+  readOnlyHint: true,
+  destructiveHint: false,
+  idempotentHint: true,
+  openWorldHint: false,
+};
+
+const NON_DESTRUCTIVE_WRITE_ANNOTATIONS: ToolSafetyAnnotations = {
+  readOnlyHint: false,
+  destructiveHint: false,
+  idempotentHint: false,
+  openWorldHint: false,
+};
+
 export interface ToolSpec {
   name: string;
   description: string;
   inputSchema: ZodRawShape;
   /**
-   * True when the tool only READS Hindsight — no document, page or initiative is written.
+   * Safety metadata published verbatim as the tool's MCP annotations (src/mcp-server.ts).
    *
-   * Surfaced to clients as MCP's standard `readOnlyHint` annotation (src/mcp-server.ts). It is not
-   * cosmetic: Dcode gates every MCP tool lacking a coherent read-only annotation behind an approval
-   * prompt, so in its headless (`dcode -n`) runtime an unannotated tool is REJECTED outright —
-   * "This MCP action requires approval, but the current headless runtime has no approval UI."
-   * Without this, recall and the knowledge-page tools were unusable in every non-interactive Dcode
-   * session. Writes stay unannotated on purpose: gating them there is the correct behaviour.
+   * Required, not optional: it is not cosmetic. Dcode gates every MCP tool lacking a coherent
+   * read-only annotation behind an approval prompt, so in its headless (`dcode -n`) runtime an
+   * unannotated tool is REJECTED outright — "This MCP action requires approval, but the current
+   * headless runtime has no approval UI." Codex Auto-review likewise treats an unannotated call as
+   * unverified external access. Reads get READ_ONLY_ANNOTATIONS; the two writes get
+   * NON_DESTRUCTIVE_WRITE_ANNOTATIONS so clients still gate them, but for the right reason.
    */
-  readOnly?: boolean;
+  annotations: ToolSafetyAnnotations;
   handler: (args: any) => Promise<ToolResult>;
 }
 
@@ -92,14 +113,14 @@ export function buildKnowledgeTools(
   return [
     {
       name: "hindsight_sync_status",
-      readOnly: true,
       description:
         "Report whether this repo's memory bank is in sync: gitlog seed present, how much recent " +
         "history has been deepened with full diffs, conversations ingested, knowledge pages " +
         "created, and extractions still running. `synced: true` means the seeded memory is fully " +
         "queryable. Ingestion is automatic and background — if not synced, it is in progress; " +
         "nothing to run.",
       inputSchema: {},
+      annotations: READ_ONLY_ANNOTATIONS,
       handler: async () => {
         try {
           return ok(await syncStatus(client, bankId, opts.repoDir ?? process.cwd()));
@@ -110,13 +131,13 @@ export function buildKnowledgeTools(
     },
     {
       name: "hindsight_diagnose",
-      readOnly: true,
       description:
         "Report safe Hindsight runtime diagnostics for this coding-agent session: resolved bank, " +
         "workspace, harness, config location, API endpoint, and non-secret environment overrides. " +
         "Use this when memory, hooks, MCP tools, or configuration appear not to work. Tokens and " +
         "other secret values are never returned.",
       inputSchema: {},
+      annotations: READ_ONLY_ANNOTATIONS,
       handler: async (_args: Record<string, never>) => {
         const configPath =
           process.env.HINDSIGHT_CONFIG || join(homedir(), ".hindsight", "coding-agent.json");
@@ -161,7 +182,6 @@ export function buildKnowledgeTools(
     },
     {
       name: "hindsight_search_knowledge_pages",
-      readOnly: true,
       description:
         "Search this repository's Hindsight knowledge pages for content relevant to a query — " +
         "hybrid full-text + semantic search, server-side. Call this when the user's question may " +
@@ -171,6 +191,7 @@ export function buildKnowledgeTools(
         "credit it visibly: start that part with a markdown blockquote header " +
         '"> 🧠 **From Hindsight memory (<page name>)** — <the facts you drew on>".',
       inputSchema: { query: z.string().describe("what to look for") },
+      annotations: READ_ONLY_ANNOTATIONS,
       handler: async (args: { query: string }) => {
         try {
           const hits = await client.searchKnowledgePages(args.query, 3);
@@ -189,7 +210,6 @@ export function buildKnowledgeTools(
     },
     {
       name: "hindsight_list_knowledge_pages",
-      readOnly: true,
       description:
         "List this repository's Hindsight knowledge pages — curated, continuously-updated " +
         "summaries of the project's durable knowledge (architecture, components, conventions, key " +
@@ -198,11 +218,11 @@ export function buildKnowledgeTools(
         "periodically in long sessions, to see what the project already knows before you read code " +
         "or ask the user. The list changes as work is captured, so re-check it occasionally.",
       inputSchema: {},
+      annotations: READ_ONLY_ANNOTATIONS,
       handler: guarded(async () => client.listPages()),
     },
     {
       name: "hindsight_read_knowledge_page",
-      readOnly: true,
       description:
         "Read the full content of one knowledge page by its id (from " +
         "hindsight_list_knowledge_pages). Call this whenever a listed page is relevant to what " +
@@ -211,11 +231,11 @@ export function buildKnowledgeTools(
         "contain [[page:<id>]] links to related pages; follow one by calling this tool again with " +
         "that id. Prefer reading a page over re-deriving the same understanding from source.",
       inputSchema: { page_id: z.string() },
+      annotations: READ_ONLY_ANNOTATIONS,
       handler: guarded(async ({ page_id }) => client.getPage(page_id)),
     },
     {
       name: "hindsight_reflect",
-      readOnly: true,
       description:
         "Deep memory reasoning: an agentic synthesis over this repository's FULL memory (git " +
         "decisions, past sessions, ingested knowledge) that answers WHY questions — the past " +
@@ -224,6 +244,7 @@ export function buildKnowledgeTools(
         "shallow and you need the root cause or the decided literals. When the answer informs " +
         'your reply, credit it visibly with a blockquote header: "> 🧠 **From Hindsight memory** — <summary>".',
       inputSchema: { query: z.string().describe("the question to reason over memory about") },
+      annotations: READ_ONLY_ANNOTATIONS,
       handler: guarded(async ({ query }: { query: string }) =>
         client.reflect(query, {
           budget: opts.reflectBudget ?? "high",
@@ -260,6 +281,7 @@ export function buildKnowledgeTools(
         summary: z.string(),
         relates_to_page_id: z.string().optional(),
       },
+      annotations: NON_DESTRUCTIVE_WRITE_ANNOTATIONS,
       handler: guarded(async ({ title, summary, relates_to_page_id }) =>
         client.captureInitiative({
           title,
@@ -281,6 +303,7 @@ export function buildKnowledgeTools(
         "'Correction: <topic>' stating what memory claimed, what is actually true, and the " +
         "evidence — the newer fact supersedes the stale one in future retrieval.",
       inputSchema: { title: z.string(), content: z.string() },
+      annotations: NON_DESTRUCTIVE_WRITE_ANNOTATIONS,
       handler: guarded(async ({ title, content }) => {
         const docId =
           title

hindsight-integrations/coding-agents/src/dsh.test.ts

@@ -153,6 +153,12 @@ describe("toDshParameters", () => {
     name: "hindsight_capture_initiative",
     description: "…",
     inputSchema,
+    annotations: {
+      readOnlyHint: false,
+      destructiveHint: false,
+      idempotentHint: false,
+      openWorldHint: false,
+    },
     handler: async () => ({ content: [] }),
   });
 

hindsight-integrations/coding-agents/src/mcp-server.test.ts

@@ -164,38 +164,45 @@ describe("buildMcpServer", () => {
 
   /**
    * Dcode computes an "is this tool coherently read-only" verdict from the MCP annotations and,
-   * in headless mode, REJECTS every call that fails it. So the annotation is a functional
-   * requirement, not documentation: without it `dcode -n` cannot search or read knowledge pages.
-   * Assert it over the wire (what a client actually sees), not on the specs.
+   * in headless mode, REJECTS every call that fails it; Codex Auto-review reads the same metadata
+   * to tell a safe knowledge read from a write. So the annotations are a functional requirement,
+   * not documentation. Assert them over the wire (what a client actually sees), not on the specs.
    */
-  it("advertises readOnlyHint on exactly the tools that only read Hindsight", async () => {
+  it("publishes explicit safety annotations for every enabled tool", async () => {
+    const readOnly = {
+      readOnlyHint: true,
+      destructiveHint: false,
+      idempotentHint: true,
+      openWorldHint: false,
+    };
+    // The writes are additive, never idempotent: clients should still gate them.
+    const nonDestructiveWrite = {
+      readOnlyHint: false,
+      destructiveHint: false,
+      idempotentHint: false,
+      openWorldHint: false,
+    };
+    const expected = {
+      hindsight_sync_status: readOnly,
+      hindsight_diagnose: readOnly,
+      hindsight_search_knowledge_pages: readOnly,
+      hindsight_list_knowledge_pages: readOnly,
+      hindsight_read_knowledge_page: readOnly,
+      hindsight_reflect: readOnly,
+      hindsight_capture_initiative: nonDestructiveWrite,
+      hindsight_ingest_document: nonDestructiveWrite,
+    };
     const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
     const server = buildMcpServer(selectTools(resolveConfig({}), stubClient, "b"));
     const client = new Client({ name: "test-client", version: "0.1.0" });
 
     await server.connect(serverTransport);
     await client.connect(clientTransport);
     try {
-      const { tools } = await client.listTools();
-      const readOnly = tools
-        .filter((t) => t.annotations?.readOnlyHint === true)
-        .map((t) => t.name)
-        .sort();
-      expect(readOnly).toEqual([
-        "hindsight_diagnose",
-        "hindsight_list_knowledge_pages",
-        "hindsight_read_knowledge_page",
-        "hindsight_reflect",
-        "hindsight_search_knowledge_pages",
-        "hindsight_sync_status",
-      ]);
-      // The writes must stay unannotated: gating them behind approval is correct.
-      const writes = tools.filter((t) => t.annotations?.readOnlyHint !== true).map((t) => t.name);
-      expect(writes.sort()).toEqual(["hindsight_capture_initiative", "hindsight_ingest_document"]);
-      // A read-only hint paired with a destructive one is INCOHERENT and gets gated anyway.
-      for (const tool of tools) {
-        if (tool.annotations?.readOnlyHint) expect(tool.annotations.destructiveHint).toBe(false);
-      }
+      const listed = await client.listTools();
+      expect(Object.fromEntries(listed.tools.map((tool) => [tool.name, tool.annotations]))).toEqual(
+        expected
+      );
     } finally {
       await client.close();
       await server.close();

hindsight-integrations/coding-agents/src/mcp-server.ts

@@ -85,14 +85,15 @@ export function buildMcpServer(tools: ToolSpec[]): McpServer {
   }
 
   for (const tool of tools) {
-    // registerTool (not the deprecated `tool()`) so the read-only annotation reaches the client:
-    // Dcode rejects unannotated MCP calls outright in headless mode. See ToolSpec.readOnly.
+    // registerTool (not the deprecated `tool()`) so the safety annotations reach the client:
+    // Dcode rejects unannotated MCP calls outright in headless mode, and Codex Auto-review treats
+    // them as unverified external access. See ToolSpec.annotations.
     server.registerTool(
       tool.name,
       {
         description: tool.description,
         inputSchema: tool.inputSchema,
-        ...(tool.readOnly ? { annotations: { readOnlyHint: true, destructiveHint: false } } : {}),
+        annotations: tool.annotations,
       },
       tool.handler
     );

hindsight-integrations/coding-agents/src/prime-agent.test.ts

@@ -108,6 +108,12 @@ describe("Prime Agent extension adapter", () => {
       name: "hindsight_search_knowledge_pages",
       description: "Search the knowledge pages",
       inputSchema: { query: z.string(), limit: z.number().optional() },
+      annotations: {
+        readOnlyHint: true,
+        destructiveHint: false,
+        idempotentHint: true,
+        openWorldHint: false,
+      },
       handler: async () => ({ content: [{ type: "text", text: "page A\npage B" }] }),
     };