From 305b4dd6fff52be1c576b14c9c6930eda83e4369 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Fri, 28 Aug 2026 11:29:16 +0800 Subject: [PATCH] fix: one name for the memory MCP server; drop "Claude" from a shared notice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both surfaced in a live qwen session and both are backend-leakage bugs. 1. The memory MCP server had two names. `MEMORY_MCP_SERVER_NAME` is "codeoid_memory" and every mount key and `isSafeTool` prefix derives from it, but `buildMemoryMcpServer` hard-coded the INSTANCE name as "codeoid-memory". Backends disagree about which of the two they namespace mounted tools by: the Claude SDK uses the mcpServers map KEY, qwen-code uses the instance's own name. So on the qwen backend the recall tools were exposed as `mcp__codeoid-memory__*`, which matched neither the provider's `allowedTools` grant nor any `isSafeTool` prefix — and the read-only memory tools raised an approval prompt on every single call, when the whole point is that recall is transparently available. Claude was unaffected, which is why nothing caught it. The instance now uses the constant. One name removes the class of mismatch rather than teaching each backend about both spellings. The test that asserted the literal is the reason this survived, so it now asserts against the constant and additionally checks the resulting tool names are auto-approvable. 2. A shared code path named Claude. The mid-turn queue notice is reached by any keep-warm backend that supports injection, but read "Claude is re-integrating with new context". In a Qwen session that renders between the user's question and Qwen's answer, which reads like the wrong backend replied — the observation that started this investigation. Co-Authored-By: Claude Opus 5 (1M context) --- src/daemon/memory/mcp.test.ts | 25 +++++++++++++++++++++++-- src/daemon/memory/mcp.ts | 11 ++++++++++- src/daemon/session.ts | 6 +++++- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/daemon/memory/mcp.test.ts b/src/daemon/memory/mcp.test.ts index f3ab13dd..7c2f3d32 100644 --- a/src/daemon/memory/mcp.test.ts +++ b/src/daemon/memory/mcp.test.ts @@ -2,6 +2,9 @@ import { describe, test, expect } from "bun:test"; import { SqliteEpisodeStore } from "./store"; import { MemoryEngine } from "./engine"; import { buildMemoryMcpServer } from "./mcp"; +import { MEMORY_MCP_SERVER_NAME } from "./mcp-http"; +import { MEMORY_TOOL_NAMES } from "./tools"; +import { isSafeTool } from "../providers/tool-safety"; import type { Embedder } from "./embedder"; class FakeEmbedder implements Embedder { @@ -15,7 +18,7 @@ class FakeEmbedder implements Embedder { } describe("buildMemoryMcpServer (thin adapter over the registry)", () => { - test("builds an in-process SDK MCP server named codeoid-memory from the shared defs", async () => { + test("builds an in-process SDK MCP server from the shared defs", async () => { const engine = new MemoryEngine({ store: new SqliteEpisodeStore(":memory:"), embedder: new FakeEmbedder() }); await engine.init(); const server = buildMemoryMcpServer(engine, { workspaceId: "ws", sessionId: "s1" }); @@ -23,7 +26,25 @@ describe("buildMemoryMcpServer (thin adapter over the registry)", () => { // asserting the config shape exercises the whole adapter path without // depending on SDK internals. expect(server.type).toBe("sdk"); - expect(server.name).toBe("codeoid-memory"); expect(server.instance).toBeDefined(); }); + + test("names the server with the canonical constant, not a literal", async () => { + // Regression: this asserted the literal "codeoid-memory" while every mount + // key and `isSafeTool` prefix derives from MEMORY_MCP_SERVER_NAME + // ("codeoid_memory"). Backends disagree about which of the two they + // namespace tools by — Claude uses the map key, qwen-code the instance + // name — so the divergence silently produced `mcp__codeoid-memory__*` on + // qwen, matching neither the allowedTools grant nor the safe-tool prefixes, + // and the read-only recall tools prompted on every call. Asserting against + // the constant (not a literal) is what keeps them from drifting apart again. + const engine = new MemoryEngine({ store: new SqliteEpisodeStore(":memory:"), embedder: new FakeEmbedder() }); + await engine.init(); + const server = buildMemoryMcpServer(engine, { workspaceId: "ws", sessionId: "s1" }); + expect(server.name).toBe(MEMORY_MCP_SERVER_NAME); + // And the tools that name produces must be auto-approvable. + for (const t of MEMORY_TOOL_NAMES) { + expect(isSafeTool(`mcp__${server.name}__${t}`)).toBe(true); + } + }); }); diff --git a/src/daemon/memory/mcp.ts b/src/daemon/memory/mcp.ts index 60d49630..62c1d310 100644 --- a/src/daemon/memory/mcp.ts +++ b/src/daemon/memory/mcp.ts @@ -15,6 +15,7 @@ import { type McpSdkServerConfigWithInstance, } from "@anthropic-ai/claude-agent-sdk"; import type { MemoryEngine } from "./engine.js"; +import { MEMORY_MCP_SERVER_NAME } from "./mcp-http.js"; import { memoryToolDefs, type MemoryToolContext } from "./tools.js"; export interface MemoryMcpBinding { @@ -38,7 +39,15 @@ export function buildMemoryMcpServer( })), ); return createSdkMcpServer({ - name: "codeoid-memory", + // MUST equal MEMORY_MCP_SERVER_NAME. Backends disagree about which name + // they namespace mounted tools by: the Claude SDK uses the mcpServers map + // KEY (always this constant), while qwen-code uses the server INSTANCE's + // own name. While these differed (`codeoid_memory` vs `codeoid-memory`) + // the qwen backend exposed `mcp__codeoid-memory__*`, which matched neither + // the provider's `allowedTools` grant nor `isSafeTool`'s prefixes — so the + // read-only recall tools prompted for approval on every single call. + // One name removes the whole class of mismatch. + name: MEMORY_MCP_SERVER_NAME, version: "0.1.0", tools, }); diff --git a/src/daemon/session.ts b/src/daemon/session.ts index de65a6a8..be7893f9 100644 --- a/src/daemon/session.ts +++ b/src/daemon/session.ts @@ -1781,7 +1781,11 @@ export class Session { if (wasWorking && this.#activeRun?.pushMidTurn) { const hint = effectivePriority === "now" - ? "⎆ Queued mid-turn — Claude is re-integrating with new context" + ? // Provider-agnostic: this path is reached by any keep-warm backend + // that supports mid-turn injection, so naming Claude here surfaced + // "Claude is re-integrating" in the middle of a Qwen conversation — + // which reads like the wrong backend answered. + "⎆ Queued mid-turn — the agent is re-integrating with new context" : effectivePriority === "next" ? "⎆ Queued — will be picked up after current turn completes" : "⎆ Queued";