diff --git a/packages/tools/src/claude-memory.test.ts b/packages/tools/src/claude-memory.test.ts index 3509541c..efd993c0 100644 --- a/packages/tools/src/claude-memory.test.ts +++ b/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", () => { diff --git a/packages/tools/src/claude-memory.ts b/packages/tools/src/claude-memory.ts index 5c36981f..bd169879 100644 --- a/packages/tools/src/claude-memory.ts +++ b/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, }, diff --git a/packages/tools/src/conversations-client.ts b/packages/tools/src/conversations-client.ts index b83e725d..981d0bf6 100644 --- a/packages/tools/src/conversations-client.ts +++ b/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 diff --git a/packages/tools/src/tool-operations.test.ts b/packages/tools/src/tool-operations.test.ts index ddd68d8d..7723b8d2 100644 --- a/packages/tools/src/tool-operations.test.ts +++ b/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 }), ) }) diff --git a/packages/tools/src/tools-shared.test.ts b/packages/tools/src/tools-shared.test.ts index 034f3cce..81aab493 100644 --- a/packages/tools/src/tools-shared.test.ts +++ b/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() - }) -}) diff --git a/packages/tools/test/claude-memory-commands.test.ts b/packages/tools/test/claude-memory-commands.test.ts index fd72a83c..ef1d92ef 100644 --- a/packages/tools/test/claude-memory-commands.test.ts +++ b/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 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") }) })