diff --git a/src/cli/App.tsx b/src/cli/App.tsx index 7f864ee..7d854e7 100644 --- a/src/cli/App.tsx +++ b/src/cli/App.tsx @@ -233,6 +233,7 @@ import { import { isFileEditTool, isFileWriteTool, + isMemoryTool, isPatchTool, isShellTool, } from "./helpers/toolNameMapping"; @@ -2693,8 +2694,7 @@ export default function App({ for (const line of buffersRef.current.byId.values()) { if (line.kind !== "tool_call") continue; if (!line.toolCallId || !line.name) continue; - if (line.name !== "memory" && line.name !== "memory_apply_patch") - continue; + if (!isMemoryTool(line.name)) continue; if (memorySyncProcessedToolCallsRef.current.has(line.toolCallId)) continue; newToolCallIds.push(line.toolCallId); diff --git a/src/cli/helpers/toolNameMapping.ts b/src/cli/helpers/toolNameMapping.ts index bfb5ac4..0bbf79a 100644 --- a/src/cli/helpers/toolNameMapping.ts +++ b/src/cli/helpers/toolNameMapping.ts @@ -4,6 +4,7 @@ */ import { isInteractiveApprovalTool } from "../../tools/interactivePolicy"; +import { MEMORY_TOOL_NAMES } from "../../tools/toolset"; /** * Maps internal tool names to user-friendly display names. @@ -141,7 +142,7 @@ export function alwaysRequiresUserInput(name: string): boolean { * Checks if a tool is a memory tool (server-side memory management) */ export function isMemoryTool(name: string): boolean { - return name === "memory" || name === "memory_apply_patch"; + return MEMORY_TOOL_NAMES.has(name); } /** diff --git a/src/tests/cli/toolNameMapping.test.ts b/src/tests/cli/toolNameMapping.test.ts new file mode 100644 index 0000000..014462a --- /dev/null +++ b/src/tests/cli/toolNameMapping.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, test } from "bun:test"; +import { isMemoryTool } from "../../cli/helpers/toolNameMapping"; + +describe("toolNameMapping.isMemoryTool", () => { + test("recognizes all supported memory tool names", () => { + expect(isMemoryTool("memory")).toBe(true); + expect(isMemoryTool("memory_apply_patch")).toBe(true); + expect(isMemoryTool("memory_insert")).toBe(true); + expect(isMemoryTool("memory_replace")).toBe(true); + expect(isMemoryTool("memory_rethink")).toBe(true); + }); + + test("returns false for non-memory tools", () => { + expect(isMemoryTool("bash")).toBe(false); + expect(isMemoryTool("web_search")).toBe(false); + }); +}); diff --git a/src/tests/tools/toolset-memfs-detach.test.ts b/src/tests/tools/toolset-memfs-detach.test.ts new file mode 100644 index 0000000..fc13427 --- /dev/null +++ b/src/tests/tools/toolset-memfs-detach.test.ts @@ -0,0 +1,75 @@ +// Tests for detaching server-side memory tools when enabling memfs + +import { beforeEach, describe, expect, mock, test } from "bun:test"; + +// Mock getClient before importing the module under test + +const detachMock = mock((_toolId: string, _opts: { agent_id: string }) => + Promise.resolve({}), +); +const retrieveMock = mock((_agentId: string, _opts?: Record) => + Promise.resolve({ + tools: [ + { name: "memory", id: "tool-memory" }, + { name: "memory_apply_patch", id: "tool-memory-apply" }, + { name: "memory_insert", id: "tool-memory-insert" }, + { name: "memory_replace", id: "tool-memory-replace" }, + { name: "memory_rethink", id: "tool-memory-rethink" }, + { name: "web_search", id: "tool-web-search" }, + // No id should be ignored + { name: "memory_replace" }, + ], + }), +); + +const mockGetClient = mock(() => + Promise.resolve({ + agents: { + retrieve: retrieveMock, + tools: { + detach: detachMock, + }, + }, + }), +); + +mock.module("../../agent/client", () => ({ + getClient: mockGetClient, +})); + +const { detachMemoryTools } = await import("../../tools/toolset"); + +describe("detachMemoryTools", () => { + beforeEach(() => { + detachMock.mockClear(); + retrieveMock.mockClear(); + mockGetClient.mockClear(); + }); + + test("detaches all known memory tool variants", async () => { + const detached = await detachMemoryTools("agent-123"); + expect(detached).toBe(true); + + const detachedToolIds = detachMock.mock.calls.map((call) => call[0]); + expect(detachedToolIds).toEqual([ + "tool-memory", + "tool-memory-apply", + "tool-memory-insert", + "tool-memory-replace", + "tool-memory-rethink", + ]); + + // Ensure we did not detach unrelated tools + expect(detachedToolIds.includes("tool-web-search")).toBe(false); + }); + + test("returns false when no memory tools are attached", async () => { + retrieveMock.mockResolvedValueOnce({ + tools: [{ name: "web_search", id: "tool-web-search" }], + }); + + const detached = await detachMemoryTools("agent-123"); + expect(detached).toBe(false); + expect(detachMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/tools/toolset.ts b/src/tools/toolset.ts index 7f72330..33591cd 100644 --- a/src/tools/toolset.ts +++ b/src/tools/toolset.ts @@ -15,6 +15,16 @@ import { const CODEX_TOOLS = OPENAI_PASCAL_TOOLS; const GEMINI_TOOLS = GEMINI_PASCAL_TOOLS; +// Server-side memory tool names that can mutate memory blocks. +// When memfs is enabled, we detach ALL of these from the agent. +export const MEMORY_TOOL_NAMES = new Set([ + "memory", + "memory_apply_patch", + "memory_insert", + "memory_replace", + "memory_rethink", +]); + // Toolset type including snake_case variants export type ToolsetName = | "codex" @@ -130,7 +140,7 @@ export async function detachMemoryTools(agentId: string): Promise { let detachedAny = false; for (const tool of currentTools) { - if (tool.name === "memory" || tool.name === "memory_apply_patch") { + if (tool.name && MEMORY_TOOL_NAMES.has(tool.name)) { if (tool.id) { await client.agents.tools.detach(tool.id, { agent_id: agentId }); detachedAny = true;