From 00b3ba886f5caa1b186d1b2b8cd06d77197bd79e Mon Sep 17 00:00:00 2001 From: Rikinshah787 Date: Tue, 25 Aug 2026 16:07:14 -0700 Subject: [PATCH] fix(tools): handle the memory tool commands Claude actually sends Three places where ClaudeMemoryTool diverges from the documented memory_20250818 wire format: - rename sends old_path/new_path, not path. handleCommand validated command.path, so every rename coming from a real model died with "Cannot read properties of undefined (reading 'startsWith')". path is still accepted as the source for existing callers. - insert_line means "insert after this line" (0 = top of file), but we spliced at insertLine - 1 and rejected 0, so every insert landed one line above where Claude asked and inserting at the top was impossible. - str_replace with new_str omitted is a deletion per the spec; we rejected it. The new tests mock the supermemory client so they run without an API key. Also fixed the rename example in the docs, which showed the same path shape the code expected. --- apps/docs/integrations/claude-memory.mdx | 2 +- packages/tools/src/claude-memory.ts | 50 +++--- .../tools/test/claude-memory-commands.test.ts | 162 ++++++++++++++++++ 3 files changed, 193 insertions(+), 21 deletions(-) create mode 100644 packages/tools/test/claude-memory-commands.test.ts diff --git a/apps/docs/integrations/claude-memory.mdx b/apps/docs/integrations/claude-memory.mdx index ba7fb90c0..577969f3a 100644 --- a/apps/docs/integrations/claude-memory.mdx +++ b/apps/docs/integrations/claude-memory.mdx @@ -183,7 +183,7 @@ Paths are normalized for storage: `/memories/preferences` is stored as `--memori ```typescript { command: "rename", - path: "/memories/old-name.txt", + old_path: "/memories/old-name.txt", new_path: "/memories/new-name.txt" } ``` diff --git a/packages/tools/src/claude-memory.ts b/packages/tools/src/claude-memory.ts index 8c665701a..963152749 100644 --- a/packages/tools/src/claude-memory.ts +++ b/packages/tools/src/claude-memory.ts @@ -9,7 +9,8 @@ export interface ClaudeMemoryConfig extends SupermemoryToolsConfig { export interface MemoryCommand { command: "view" | "create" | "str_replace" | "insert" | "delete" | "rename" - path: string + // every command except rename addresses the file via path + path?: string // view specific view_range?: [number, number] // create specific @@ -20,7 +21,9 @@ export interface MemoryCommand { // insert specific insert_line?: number insert_text?: string - // rename specific + // rename specific: Claude sends old_path/new_path (path is accepted too + // for backwards compatibility with earlier callers) + old_path?: string new_path?: string } @@ -76,17 +79,24 @@ export class ClaudeMemoryTool { */ async handleCommand(command: MemoryCommand): Promise { try { + // rename is the one command that doesn't use `path`: Claude sends + // old_path/new_path. Fall back to `path` so older callers keep working. + const path = + command.command === "rename" + ? (command.old_path ?? command.path) + : command.path + // Validate path security - if (!this.isValidPath(command.path)) { + if (path === undefined || !this.isValidPath(path)) { return { success: false, - error: `Invalid path: ${command.path}. All paths must start with /memories/`, + error: `Invalid path: ${path}. All paths must start with /memories/`, } } switch (command.command) { case "view": - return await this.view(command.path, command.view_range) + return await this.view(path, command.view_range) case "create": if (!command.file_text) { return { @@ -94,21 +104,21 @@ export class ClaudeMemoryTool { error: "file_text is required for create command", } } - return await this.create(command.path, command.file_text) + return await this.create(path, command.file_text) case "str_replace": - // new_str may legitimately be "" (deleting text), so only reject - // when it is missing entirely. old_str must be non-empty — replacing - // the empty string would prepend instead of replacing. - if (!command.old_str || command.new_str === undefined) { + // 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) { return { success: false, - error: "old_str and new_str are required for str_replace command", + error: "old_str is required for str_replace command", } } return await this.strReplace( - command.path, + path, command.old_str, - command.new_str, + command.new_str ?? "", ) case "insert": // insert_text may be "" (inserting a blank line). @@ -123,12 +133,12 @@ export class ClaudeMemoryTool { } } return await this.insert( - command.path, + path, command.insert_line, command.insert_text, ) case "delete": - return await this.delete(command.path) + return await this.delete(path) case "rename": if (!command.new_path) { return { @@ -136,7 +146,7 @@ export class ClaudeMemoryTool { error: "new_path is required for rename command", } } - return await this.rename(command.path, command.new_path) + return await this.rename(path, command.new_path) default: return { success: false, @@ -437,16 +447,16 @@ export class ClaudeMemoryTool { readResult.document.raw || readResult.document.content || "" const lines = originalContent.split("\n") - // Validate line number - if (insertLine < 1 || insertLine > lines.length + 1) { + // insert_line is the line the text goes after: 0 means the beginning + // of the file and lines.length appends at the end. + if (insertLine < 0 || insertLine > lines.length) { return { success: false, error: `Invalid line number: ${insertLine}. File has ${lines.length} lines.`, } } - // Insert the text (insertLine is 1-based) - lines.splice(insertLine - 1, 0, insertText) + lines.splice(insertLine, 0, insertText) const newContent = lines.join("\n") // Update the document diff --git a/packages/tools/test/claude-memory-commands.test.ts b/packages/tools/test/claude-memory-commands.test.ts new file mode 100644 index 000000000..fd72a83ce --- /dev/null +++ b/packages/tools/test/claude-memory-commands.test.ts @@ -0,0 +1,162 @@ +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(() => ({ + addMock: vi.fn(), + deleteMock: vi.fn(), + executeMock: vi.fn(), +})) + +vi.mock("supermemory", () => ({ + default: class MockSupermemory { + add = addMock + search = { execute: executeMock } + documents = { delete: deleteMock } + }, +})) + +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: [ + { + documentId: customIdFor(path), + raw: content, + metadata: { file_path: path }, + }, + ], + }) +} + +describe("ClaudeMemoryTool command handling", () => { + let tool: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + addMock.mockResolvedValue({ id: "doc_1" }) + deleteMock.mockResolvedValue({}) + executeMock.mockResolvedValue({ results: [] }) + 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") + + 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"), + ) + }) + + 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") + }) + }) + + describe("insert", () => { + const path = "/memories/notes.txt" + + 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") + }) + + 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") + }) + }) + + 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" }), + ) + }) + }) +})