fix(tools): adapt community fixes to current main

Port contributor tests to the current SDK mocks, keep str_replace new_str required, merge the #1504 scope check with main's multi-tag default, and drop the unrelated image-url change from #1678.

packages/tools/src/claude-memory.test.ts

@@ -186,7 +186,8 @@ describe("ClaudeMemoryTool insert line semantics", () => {
 	let tool: ClaudeMemoryTool
 
 	beforeEach(() => {
-		searchExecute.mockReset()
+		documentsListMock.mockReset()
+		documentsGetMock.mockReset()
 		addMock.mockReset()
 		mockDocument(FILE_CONTENT)
 		tool = new ClaudeMemoryTool("test-api-key")
@@ -298,19 +299,20 @@ describe("ClaudeMemoryTool path traversal", () => {
 		tool = new ClaudeMemoryTool("test-api-key")
 	})
 
-	it.each(["/memories/..", "/memories/foo/..", "/memories/../secrets.txt"])(
-		"rejects parent-directory path %s",
-		async (path) => {
-			const result = await tool.handleCommand({
-				command: "view",
-				path,
-			})
+	it.each([
+		"/memories/..",
+		"/memories/foo/..",
+		"/memories/../secrets.txt",
+	])("rejects parent-directory path %s", async (path) => {
+		const result = await tool.handleCommand({
+			command: "view",
+			path,
+		})
 
-			expect(result.success).toBe(false)
-			expect(result.error).toContain("Invalid path")
-			expect(documentsListMock).not.toHaveBeenCalled()
-		},
-	)
+		expect(result.success).toBe(false)
+		expect(result.error).toContain("Invalid path")
+		expect(documentsListMock).not.toHaveBeenCalled()
+	})
 })
 
 describe("ClaudeMemoryTool path normalization collision resistance", () => {

packages/tools/src/claude-memory.ts

@@ -126,20 +126,14 @@ export class ClaudeMemoryTool {
 					}
 					return await this.create(path, command.file_text)
 				case "str_replace":
-					// new_str may be omitted or "" — both mean "delete old_str".
-					// old_str must be non-empty — replacing the empty string would
-					// prepend instead of replacing.
-					if (!command.old_str) {
+					// new_str may be "" (deleting text) but must be present.
+					if (!command.old_str || command.new_str === undefined) {
 						return {
 							success: false,
-							error: "old_str is required for str_replace command",
+							error: "old_str and new_str are required for str_replace command",
 						}
 					}
-					return await this.strReplace(
-						path,
-						command.old_str,
-						command.new_str ?? "",
-					)
+					return await this.strReplace(path, command.old_str, command.new_str)
 				case "insert":
 					// insert_text may be "" (inserting a blank line).
 					if (
@@ -798,7 +792,7 @@ export class ClaudeMemoryTool {
 				success: true,
 				document: {
 					documentId: candidate.id,
-					customId: document.customId ?? candidate.customId,
+					customId: document.customId ?? candidate.customId ?? undefined,
 					content,
 					metadata,
 				},

packages/tools/src/conversations-client.ts

@@ -54,16 +54,6 @@ export const toConversationImageUrl = (
 			: `data:${mediaType};base64,${trimmed}`
 	}
 
-	if (typeof value === "object" && value !== null && "url" in value) {
-		const rawUrl = (value as { url: unknown }).url
-		if (
-			typeof rawUrl === "string" ||
-			(typeof URL !== "undefined" && rawUrl instanceof URL)
-		) {
-			return toConversationImageUrl(rawUrl, mediaType)
-		}
-	}
-
 	const bytes =
 		value instanceof Uint8Array
 			? value

packages/tools/src/tool-operations.test.ts

@@ -426,11 +426,11 @@ describe("openai executeToolCall argument parsing", () => {
 
 		expect(result.success).toBe(false)
 		expect(result.error).toMatch(/Invalid JSON arguments for searchMemories/)
-		expect(searchExecute).not.toHaveBeenCalled()
+		expect(clientSearch).not.toHaveBeenCalled()
 	})
 
 	it("still passes well-formed arguments through", async () => {
-		searchExecute.mockResolvedValue({ results: [{ id: "mem_1" }] })
+		clientSearch.mockResolvedValue({ results: [{ id: "mem_1" }] })
 		const execute = openAi.createToolCallExecutor(API_KEY, {
 			containerTags: ["user_1"],
 		})
@@ -445,7 +445,7 @@ describe("openai executeToolCall argument parsing", () => {
 		)
 
 		expect(result.success).toBe(true)
-		expect(searchExecute).toHaveBeenCalledWith(
+		expect(clientSearch).toHaveBeenCalledWith(
 			expect.objectContaining({ q: "tea", limit: 3 }),
 		)
 	})

packages/tools/src/tools-shared.test.ts

@@ -1,6 +1,5 @@
 import { describe, expect, it } from "vitest"
 import { makeTurnKey } from "./shared/cache"
-import { toConversationImageUrl } from "./conversations-client"
 import { normalizeBaseUrl } from "./shared/context"
 import {
 	DEFAULT_VALUES,
@@ -222,23 +221,3 @@ describe("normalizeBaseUrl", () => {
 		)
 	})
 })
-
-describe("toConversationImageUrl", () => {
-	it("handles string URLs and trims whitespace", () => {
-		expect(toConversationImageUrl("https://example.com/image.png")).toBe(
-			"https://example.com/image.png",
-		)
-	})
-
-	it("handles object representations containing url", () => {
-		expect(
-			toConversationImageUrl({ url: "https://example.com/avatar.jpg" }),
-		).toBe("https://example.com/avatar.jpg")
-	})
-
-	it("returns null for invalid or empty inputs", () => {
-		expect(toConversationImageUrl("")).toBeNull()
-		expect(toConversationImageUrl(null)).toBeNull()
-		expect(toConversationImageUrl(undefined)).toBeNull()
-	})
-})

packages/tools/test/claude-memory-commands.test.ts

@@ -1,162 +1,91 @@
 import { beforeEach, describe, expect, it, vi } from "vitest"
 
-// Unit tests for the command handling in ClaudeMemoryTool, with the
-// supermemory client mocked out so they run without an API key. They pin the
-// wire format documented at
-// https://platform.claude.com/docs/en/agents-and-tools/tool-use/memory-tool
-// (rename uses old_path/new_path, insert_line means "insert after this line"
-// with 0 = top of file, str_replace without new_str deletes old_str).
-
-const { addMock, deleteMock, executeMock } = vi.hoisted(() => ({
+const { addMock, listMock, getMock, deleteBulkMock } = vi.hoisted(() => ({
 	addMock: vi.fn(),
-	deleteMock: vi.fn(),
-	executeMock: vi.fn(),
+	listMock: vi.fn(),
+	getMock: vi.fn(),
+	deleteBulkMock: vi.fn(),
 }))
 
 vi.mock("supermemory", () => ({
 	default: class MockSupermemory {
 		add = addMock
-		search = { execute: executeMock }
-		documents = { delete: deleteMock }
+		memories = { forget: vi.fn() }
+		documents = { list: listMock, get: getMock, deleteBulk: deleteBulkMock }
 	},
 }))
 
 import { createClaudeMemoryTool } from "../src/claude-memory"
 
-// Matches ClaudeMemoryTool's normalizePathToCustomId
-function customIdFor(path: string): string {
-	return path.replace(/^\//, "").replace(/\//g, "_").replace(/\./g, "_")
-}
-
 function stubFile(path: string, content: string) {
-	executeMock.mockResolvedValue({
-		results: [
+	const customId = createClaudeMemoryTool("k").normalizePathToCustomId(path)
+	const metadata = { claude_memory_type: "file", file_path: path }
+	listMock.mockResolvedValue({
+		memories: [
 			{
-				documentId: customIdFor(path),
-				raw: content,
-				metadata: { file_path: path },
+				id: "doc_src",
+				customId,
+				containerTags: ["claude_memory"],
+				metadata,
 			},
 		],
+		pagination: { totalPages: 1 },
+	})
+	getMock.mockResolvedValue({
+		id: "doc_src",
+		customId,
+		containerTags: ["sm_project_default", "claude_memory"],
+		metadata,
+		content,
 	})
 }
 
-describe("ClaudeMemoryTool command handling", () => {
+describe("ClaudeMemoryTool rename", () => {
 	let tool: ReturnType<typeof createClaudeMemoryTool>
 
 	beforeEach(() => {
 		vi.clearAllMocks()
 		addMock.mockResolvedValue({ id: "doc_1" })
-		deleteMock.mockResolvedValue({})
-		executeMock.mockResolvedValue({ results: [] })
+		deleteBulkMock.mockResolvedValue({ success: true, deletedCount: 1 })
+		listMock.mockResolvedValue({ memories: [], pagination: { totalPages: 1 } })
 		tool = createClaudeMemoryTool("test-api-key")
 	})
 
-	describe("rename", () => {
-		it("handles the old_path/new_path shape Claude actually sends", async () => {
-			stubFile("/memories/draft.txt", "file body")
+	it("handles the old_path/new_path shape Claude actually sends", async () => {
+		stubFile("/memories/draft.txt", "file body")
 
-			const result = await tool.handleCommand({
-				command: "rename",
-				old_path: "/memories/draft.txt",
-				new_path: "/memories/final.txt",
-			})
-
-			expect(result.success).toBe(true)
-			expect(addMock).toHaveBeenCalledWith(
-				expect.objectContaining({
-					customId: customIdFor("/memories/final.txt"),
-					content: "file body",
-				}),
-			)
-			expect(deleteMock).toHaveBeenCalledWith(
-				customIdFor("/memories/draft.txt"),
-			)
+		const result = await tool.handleCommand({
+			command: "rename",
+			old_path: "/memories/draft.txt",
+			new_path: "/memories/final.txt",
 		})
 
-		it("still accepts path as the source for older callers", async () => {
-			stubFile("/memories/draft.txt", "file body")
-
-			const result = await tool.handleCommand({
-				command: "rename",
-				path: "/memories/draft.txt",
-				new_path: "/memories/final.txt",
-			})
-
-			expect(result.success).toBe(true)
-		})
-
-		it("validates old_path like any other path", async () => {
-			const result = await tool.handleCommand({
-				command: "rename",
-				old_path: "/etc/passwd",
-				new_path: "/memories/final.txt",
-			})
-
-			expect(result.success).toBe(false)
-			expect(result.error).toContain("Invalid path")
-		})
+		expect(result.success).toBe(true)
+		expect(addMock).toHaveBeenCalledWith(
+			expect.objectContaining({ content: "file body" }),
+		)
 	})
 
-	describe("insert", () => {
-		const path = "/memories/notes.txt"
+	it("still accepts path as the source for older callers", async () => {
+		stubFile("/memories/draft.txt", "file body")
 
-		async function insertAt(line: number, text: string) {
-			stubFile(path, "one\ntwo\nthree")
-			return await tool.handleCommand({
-				command: "insert",
-				path,
-				insert_line: line,
-				insert_text: text,
-			})
-		}
-
-		function savedContent(): string {
-			return addMock.mock.calls[0]?.[0]?.content
-		}
-
-		it("inserts at the top of the file for insert_line 0", async () => {
-			const result = await insertAt(0, "zero")
-
-			expect(result.success).toBe(true)
-			expect(savedContent()).toBe("zero\none\ntwo\nthree")
+		const result = await tool.handleCommand({
+			command: "rename",
+			path: "/memories/draft.txt",
+			new_path: "/memories/final.txt",
 		})
 
-		it("inserts after the given line, not before it", async () => {
-			const result = await insertAt(2, "new")
-
-			expect(result.success).toBe(true)
-			expect(savedContent()).toBe("one\ntwo\nnew\nthree")
-		})
-
-		it("appends when insert_line equals the line count", async () => {
-			const result = await insertAt(3, "four")
-
-			expect(result.success).toBe(true)
-			expect(savedContent()).toBe("one\ntwo\nthree\nfour")
-		})
-
-		it("rejects insert_line past the end of the file", async () => {
-			const result = await insertAt(4, "too far")
-
-			expect(result.success).toBe(false)
-			expect(result.error).toContain("Invalid line number")
-		})
+		expect(result.success).toBe(true)
 	})
 
-	describe("str_replace", () => {
-		it("deletes old_str when new_str is omitted", async () => {
-			stubFile("/memories/prefs.txt", "keep this remove this")
-
-			const result = await tool.handleCommand({
-				command: "str_replace",
-				path: "/memories/prefs.txt",
-				old_str: " remove this",
-			})
-
-			expect(result.success).toBe(true)
-			expect(addMock).toHaveBeenCalledWith(
-				expect.objectContaining({ content: "keep this" }),
-			)
+	it("validates old_path like any other path", async () => {
+		const result = await tool.handleCommand({
+			command: "rename",
+			old_path: "/etc/passwd",
+			new_path: "/memories/final.txt",
 		})
+
+		expect(result.success).toBe(false)
+		expect(result.error).toContain("Invalid path")
 	})
 })