From 608c380c5ab5a8c8610daa13b60af4e048278e5a Mon Sep 17 00:00:00 2001 From: abhinav7x94 Date: Sun, 16 Aug 2026 07:46:43 +0530 Subject: [PATCH 01/11] fix(tools): isolate OpenAI middleware clients --- packages/tools/README.md | 2 +- packages/tools/src/openai/index.ts | 2 +- packages/tools/src/openai/middleware.ts | 75 ++++++++++++++++++++----- 3 files changed, 62 insertions(+), 17 deletions(-) diff --git a/packages/tools/README.md b/packages/tools/README.md index f65967466..1bc677d22 100644 --- a/packages/tools/README.md +++ b/packages/tools/README.md @@ -288,7 +288,7 @@ The `MemoryPromptData` object provides: #### OpenAI Middleware with Supermemory -The `withSupermemory` function creates an OpenAI client with SuperMemory middleware automatically injected: +The `withSupermemory` function creates an isolated middleware facade around an OpenAI client. It does not mutate the supplied client, so a shared base client can safely be wrapped with different user or conversation options: ```typescript import { withSupermemory } from "@supermemory/tools/openai" diff --git a/packages/tools/src/openai/index.ts b/packages/tools/src/openai/index.ts index 1523bf73f..d229a4edf 100644 --- a/packages/tools/src/openai/index.ts +++ b/packages/tools/src/openai/index.ts @@ -23,7 +23,7 @@ import { * @param options.addMemory - Optional mode for memory addition: "always" (default), "never" * @param options.apiKey - Optional Supermemory API key; falls back to SUPERMEMORY_API_KEY * - * @returns An OpenAI client with SuperMemory middleware injected for both Chat Completions and Responses APIs + * @returns A distinct OpenAI middleware facade for Chat Completions and Responses APIs. The supplied client is not mutated. * * @example * ```typescript diff --git a/packages/tools/src/openai/middleware.ts b/packages/tools/src/openai/middleware.ts index 82326102c..5c76a78ee 100644 --- a/packages/tools/src/openai/middleware.ts +++ b/packages/tools/src/openai/middleware.ts @@ -19,6 +19,34 @@ import { deduplicateMemoriesForMode } from "../tools-shared" import { createLogger, type Logger } from "../vercel/logger" import { convertProfileToMarkdown } from "../vercel/util" +// Keep canonicalization stable across duplicate package copies and hot reloads. +const BASE_CLIENT_SYMBOL = Symbol.for("@supermemory/tools/openai/base-client") +const baseClientByWrapper = new WeakMap() + +const getBaseClient = (client: OpenAI) => + (Reflect.get(client, BASE_CLIENT_SYMBOL) as OpenAI | undefined) ?? + baseClientByWrapper.get(client) ?? + client + +const cloneWithOverrides = ( + source: T, + overrides: Partial, +): T => { + const descriptors = Object.getOwnPropertyDescriptors(source) + + for (const key of Reflect.ownKeys(overrides) as Array) { + const current = Object.getOwnPropertyDescriptor(source, key) + Reflect.set(descriptors, key, { + configurable: current?.configurable ?? true, + enumerable: current?.enumerable ?? false, + value: overrides[key], + writable: current && "writable" in current ? current.writable : true, + }) + } + + return Object.create(Object.getPrototypeOf(source), descriptors) as T +} + const normalizeBaseUrl = (url?: string): string => { const defaultUrl = "https://api.supermemory.ai" return url?.trim().replace(/\/+$/, "") || defaultUrl @@ -658,6 +686,11 @@ export function createOpenAIMiddleware( namespace: string, options?: OpenAIMiddlewareOptions, ) { + const baseClient = getBaseClient(openaiClient) + const baseChat = baseClient.chat + const baseCompletions = baseChat.completions + const baseResponses = baseClient.responses + const logger = createLogger(options?.verbose ?? false) const apiKey = options?.apiKey?.trim() || process.env.SUPERMEMORY_API_KEY?.trim() || "" @@ -676,8 +709,8 @@ export function createOpenAIMiddleware( const mode = options?.mode ?? "profile" const addMemory = options?.addMemory ?? "always" - const originalCreate = openaiClient.chat.completions.create - const originalResponsesCreate = openaiClient.responses?.create + const originalCreate = baseCompletions.create + const originalResponsesCreate = baseResponses?.create /** * Searches for memories and formats them for injection into API calls. @@ -818,7 +851,7 @@ export function createOpenAIMiddleware( } return { request: originalResponsesCreate.call( - openaiClient.responses, + baseResponses, cleanedParams, requestOptions, ), @@ -864,7 +897,7 @@ export function createOpenAIMiddleware( return { request: originalResponsesCreate.call( - openaiClient.responses, + baseResponses, { ...params, input: cleanedInput, @@ -913,7 +946,7 @@ export function createOpenAIMiddleware( logger.debug("No textual user message found, skipping memory search") return { request: originalCreate.call( - openaiClient.chat.completions, + baseCompletions, { ...params, messages: updateChatMemoryContexts(messages), @@ -968,7 +1001,7 @@ export function createOpenAIMiddleware( return { request: originalCreate.call( - openaiClient.chat.completions, + baseCompletions, { ...params, messages: enhancedMessages, @@ -983,14 +1016,26 @@ export function createOpenAIMiddleware( requestOptions?: OpenAI.RequestOptions, ) => deferAPIPromise(() => prepareCreateWithMemory(params, requestOptions)) - openaiClient.chat.completions.create = - createWithMemory as typeof originalCreate - - // Wrap Responses API if available - if (originalResponsesCreate) { - openaiClient.responses.create = - createResponsesWithMemory as typeof originalResponsesCreate - } + const wrappedCompletions = cloneWithOverrides(baseCompletions, { + create: createWithMemory as typeof originalCreate, + }) + const wrappedChat = cloneWithOverrides(baseChat, { + completions: wrappedCompletions, + }) + const wrappedResponses = + baseResponses && originalResponsesCreate + ? cloneWithOverrides(baseResponses, { + create: createResponsesWithMemory as typeof originalResponsesCreate, + }) + : undefined + const wrappedClient = cloneWithOverrides(baseClient, { + chat: wrappedChat, + ...(wrappedResponses ? { responses: wrappedResponses } : {}), + }) - return openaiClient + Object.defineProperty(wrappedClient, BASE_CLIENT_SYMBOL, { + value: baseClient, + }) + baseClientByWrapper.set(wrappedClient, baseClient) + return wrappedClient } From 432a761a48bf60196786296c3f64b4640a441a13 Mon Sep 17 00:00:00 2001 From: Rohit <71192000+rohitsux@users.noreply.github.com> Date: Sat, 18 Jul 2026 17:56:12 +0530 Subject: [PATCH 02/11] fix(tools): align claude-memory insert with memory_20250818 line semantics Anthropic's memory_20250818 spec defines insert as: insert_text is inserted AFTER line insert_line, 0 inserts at the beginning of the file, and the valid range is [0, n_lines]. The implementation treated insert_line as a 1-based insert-BEFORE index with range [1, n_lines + 1]. Since the caller of this tool is Claude itself, which is trained on the spec semantics, every model-driven insert landed one line earlier than intended, insert_line: 0 (insert at top of file) was rejected as invalid, and insert_line: n_lines (append) inserted before the last line instead of after it. Fix the validation range to [0, n_lines], splice at insert_line directly (0-based insert-after), and update the error and success messages to match. One existing tool-operations test encoded the old insert-before behavior; its insert_line is adjusted so its expected output is unchanged under spec semantics. Adds four regression tests covering top-of-file, middle, append, and both out-of-range directions. --- packages/tools/src/claude-memory.test.ts | 74 ++++++++++++++++++++++ packages/tools/src/claude-memory.ts | 14 ++-- packages/tools/src/tool-operations.test.ts | 2 +- 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/packages/tools/src/claude-memory.test.ts b/packages/tools/src/claude-memory.test.ts index 97bfeda5c..5c4831236 100644 --- a/packages/tools/src/claude-memory.test.ts +++ b/packages/tools/src/claude-memory.test.ts @@ -188,6 +188,80 @@ describe("ClaudeMemoryTool path collisions", () => { }) }) +describe("ClaudeMemoryTool insert line semantics", () => { + let tool: ClaudeMemoryTool + + beforeEach(() => { + searchExecute.mockReset() + addMock.mockReset() + mockDocument(FILE_CONTENT) + tool = new ClaudeMemoryTool("test-api-key") + }) + + // The memory_20250818 spec: insert_text is inserted AFTER line insert_line, + // 0 inserts at the beginning of the file, and the valid range is [0, n_lines]. + + it("insert_line: 0 inserts at the beginning of the file", async () => { + const result = await tool.handleCommand({ + command: "insert", + path: FILE_PATH, + insert_line: 0, + insert_text: "header", + }) + + expect(result.success).toBe(true) + const stored = addMock.mock.calls[0]?.[0]?.content as string + expect(stored).toBe("header\nline1\nline2\nline3\nline4\nline5") + }) + + it("inserts AFTER the given line, not before it", async () => { + const result = await tool.handleCommand({ + command: "insert", + path: FILE_PATH, + insert_line: 2, + insert_text: "after2", + }) + + expect(result.success).toBe(true) + const stored = addMock.mock.calls[0]?.[0]?.content as string + // Regression guard: the old 1-based insert-BEFORE landed this one line early. + expect(stored).toBe("line1\nline2\nafter2\nline3\nline4\nline5") + }) + + it("insert_line: n_lines appends at the end of the file", async () => { + const result = await tool.handleCommand({ + command: "insert", + path: FILE_PATH, + insert_line: 5, + insert_text: "tail", + }) + + expect(result.success).toBe(true) + const stored = addMock.mock.calls[0]?.[0]?.content as string + expect(stored).toBe("line1\nline2\nline3\nline4\nline5\ntail") + }) + + it("rejects insert_line outside [0, n_lines] without writing", async () => { + const below = await tool.handleCommand({ + command: "insert", + path: FILE_PATH, + insert_line: -1, + insert_text: "x", + }) + expect(below.success).toBe(false) + expect(below.error).toContain("[0, 5]") + + const above = await tool.handleCommand({ + command: "insert", + path: FILE_PATH, + insert_line: 6, + insert_text: "x", + }) + expect(above.success).toBe(false) + expect(addMock).not.toHaveBeenCalled() + }) +}) + describe("ClaudeMemoryTool str_replace replacement literalness", () => { let tool: ClaudeMemoryTool diff --git a/packages/tools/src/claude-memory.ts b/packages/tools/src/claude-memory.ts index 7b5d1d11b..477862551 100644 --- a/packages/tools/src/claude-memory.ts +++ b/packages/tools/src/claude-memory.ts @@ -414,16 +414,18 @@ export class ClaudeMemoryTool { const originalContent = readResult.document.content const lines = originalContent.split("\n") - // Validate line number - if (insertLine < 1 || insertLine > lines.length + 1) { + // Validate line number. Per the memory_20250818 spec the valid range + // is [0, n_lines]: text is inserted AFTER `insert_line`, and 0 means + // the beginning of the file. + if (insertLine < 0 || insertLine > lines.length) { return { success: false, - error: `Invalid line number: ${insertLine}. File has ${lines.length} lines.`, + error: `Invalid insert_line parameter: ${insertLine}. It should be within the range of lines of the file: [0, ${lines.length}]`, } } - // Insert the text (insertLine is 1-based) - lines.splice(insertLine - 1, 0, insertText) + // Insert the text after line `insertLine` (0-based insert-after) + lines.splice(insertLine, 0, insertText) const newContent = lines.join("\n") await this.writeFile(readResult.document.documentId, newContent, { @@ -434,7 +436,7 @@ export class ClaudeMemoryTool { return { success: true, - content: `Text inserted at line ${insertLine} in file: ${filePath}`, + content: `Text inserted after line ${insertLine} in file: ${filePath}`, } } catch (error) { return { diff --git a/packages/tools/src/tool-operations.test.ts b/packages/tools/src/tool-operations.test.ts index cc1dd1e0a..1ec5c0ea4 100644 --- a/packages/tools/src/tool-operations.test.ts +++ b/packages/tools/src/tool-operations.test.ts @@ -438,7 +438,7 @@ describe("ClaudeMemoryTool", () => { const result = await tool.handleCommand({ command: "insert", path: FILE_PATH, - insert_line: 2, + insert_line: 1, insert_text: "", }) From 3237107e12dfdfb69d7c849926c73a7c538befcd Mon Sep 17 00:00:00 2001 From: Aniruddha Adak Date: Tue, 8 Sep 2026 12:01:29 +0530 Subject: [PATCH 03/11] fix(tools): reject parent-directory segments in Claude memory paths --- packages/tools/src/claude-memory.test.ts | 26 ++++++++++++++++++++++++ packages/tools/src/claude-memory.ts | 14 ++++++++----- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/packages/tools/src/claude-memory.test.ts b/packages/tools/src/claude-memory.test.ts index 5c4831236..856d8a45b 100644 --- a/packages/tools/src/claude-memory.test.ts +++ b/packages/tools/src/claude-memory.test.ts @@ -291,3 +291,29 @@ describe("ClaudeMemoryTool str_replace replacement literalness", () => { expect(stored).toContain(`price is ${dollarSequence} today`) }) }) + +describe("ClaudeMemoryTool path traversal", () => { + let tool: ClaudeMemoryTool + + beforeEach(() => { + documentsListMock.mockReset() + documentsGetMock.mockReset() + addMock.mockReset() + mockDocument(FILE_CONTENT) + 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, + }) + + expect(result.success).toBe(false) + expect(result.error).toContain("Invalid path") + expect(documentsListMock).not.toHaveBeenCalled() + }, + ) +}) diff --git a/packages/tools/src/claude-memory.ts b/packages/tools/src/claude-memory.ts index 477862551..16ad3d587 100644 --- a/packages/tools/src/claude-memory.ts +++ b/packages/tools/src/claude-memory.ts @@ -620,11 +620,15 @@ export class ClaudeMemoryTool { * Validate that path starts with /memories for security */ private isValidPath(path: string): boolean { - return ( - (path.startsWith("/memories/") || path === "/memories") && - !path.includes("../") && - !path.includes("..\\") - ) + if (!(path.startsWith("/memories/") || path === "/memories")) { + return false + } + if (path.includes("..\\")) { + return false + } + // Reject any parent-directory segment, including trailing "/.." which + // the previous "../" substring check missed (e.g. "/memories/.."). + return !path.split("/").some((segment) => segment === "..") } } From fd837c7e432da4ac0f6672960c6fc03c8d1b8860 Mon Sep 17 00:00:00 2001 From: Aditya kumar singh <143548997+Adityakk9031@users.noreply.github.com> Date: Thu, 17 Sep 2026 16:27:11 +0530 Subject: [PATCH 04/11] fix(tools): prevent customId collisions in claude memory tool (#1547) --- packages/tools/src/claude-memory.ts | 71 +++++++++++++++++------------ 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/packages/tools/src/claude-memory.ts b/packages/tools/src/claude-memory.ts index 16ad3d587..c25f04f45 100644 --- a/packages/tools/src/claude-memory.ts +++ b/packages/tools/src/claude-memory.ts @@ -53,8 +53,17 @@ export class ClaudeMemoryTool { private client: Supermemory private namespace: string - /** Converts /memories/file.txt -> memories_file_txt */ + /** Reversible encoding so distinct paths never share an ID: /memories/a_b.txt -> memories_s_a__b_d_txt */ private normalizePathToId(path: string): string { + return path + .replace(/^\//, "") + .replace(/_/g, "__") + .replace(/\//g, "_s_") + .replace(/\./g, "_d_") + } + + /** Pre-3.0.1 IDs, where / and . both became _ */ + private legacyNormalizePathToId(path: string): string { return path.replace(/^\//, "").replace(/\//g, "_").replace(/\./g, "_") } @@ -561,39 +570,45 @@ export class ClaudeMemoryTool { error?: string }> { try { - const id = this.normalizePathToId(filePath) - let document: Awaited> - try { - document = await this.client.documents.get(this.namespace, id) - } catch (error) { - if (isNotFoundError(error)) { - return { success: false, error: `File not found: ${filePath}` } + // Files written before 3.0.1 still live under the legacy ID. + const ids = [ + this.normalizePathToId(filePath), + this.legacyNormalizePathToId(filePath), + ] + for (const id of ids) { + let document: Awaited> + try { + document = await this.client.documents.get(this.namespace, id) + } catch (error) { + if (isNotFoundError(error)) continue + throw error + } + + // Different paths can share a legacy ID; only the exact file counts. + if ( + document.metadata?.source !== CLAUDE_MEMORY_SOURCE || + this.getDocumentFilePath(document) !== filePath + ) { + continue + } + if (typeof document.content !== "string") { + return { + success: false, + error: `File content unavailable: ${filePath}`, + } } - throw error - } - // Different paths can normalize to the same ID; only the exact file counts. - if ( - document.metadata?.source !== CLAUDE_MEMORY_SOURCE || - this.getDocumentFilePath(document) !== filePath - ) { - return { success: false, error: `File not found: ${filePath}` } - } - if (typeof document.content !== "string") { return { - success: false, - error: `File content unavailable: ${filePath}`, + success: true, + document: { + documentId: id, + content: document.content, + metadata: document.metadata as ClaudeFileMetadata, + }, } } - return { - success: true, - document: { - documentId: id, - content: document.content, - metadata: document.metadata as ClaudeFileMetadata, - }, - } + return { success: false, error: `File not found: ${filePath}` } } catch (error) { return { success: false, From 0efc7b9d33ab590428ebaffe592305ac63cad5a2 Mon Sep 17 00:00:00 2001 From: Rikinshah787 Date: Tue, 25 Aug 2026 16:07:14 -0700 Subject: [PATCH 05/11] 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 | 48 +++--- .../tools/test/claude-memory-commands.test.ts | 162 ++++++++++++++++++ 3 files changed, 191 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 39ecacd55..3bee1fc0a 100644 --- a/apps/docs/integrations/claude-memory.mdx +++ b/apps/docs/integrations/claude-memory.mdx @@ -182,7 +182,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 c25f04f45..a59bffa43 100644 --- a/packages/tools/src/claude-memory.ts +++ b/packages/tools/src/claude-memory.ts @@ -12,7 +12,8 @@ export const CLAUDE_MEMORY_SOURCE = "claude-memory" 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 @@ -23,7 +24,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 } @@ -77,17 +80,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 { @@ -95,21 +105,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). @@ -124,12 +134,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 { @@ -137,7 +147,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, @@ -423,9 +433,8 @@ export class ClaudeMemoryTool { const originalContent = readResult.document.content const lines = originalContent.split("\n") - // Validate line number. Per the memory_20250818 spec the valid range - // is [0, n_lines]: text is inserted AFTER `insert_line`, and 0 means - // the beginning of the file. + // 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, @@ -433,7 +442,6 @@ export class ClaudeMemoryTool { } } - // Insert the text after line `insertLine` (0-based insert-after) lines.splice(insertLine, 0, insertText) const newContent = lines.join("\n") 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" }), + ) + }) + }) +}) From 2604c45653528d0d1f6e337f5c41e3e7536550af Mon Sep 17 00:00:00 2001 From: Agnik47 <140933190+Agnik47@users.noreply.github.com> Date: Fri, 28 Aug 2026 22:25:19 +0530 Subject: [PATCH 06/11] fix(tools): accept zero-argument OpenAI tool calls and reject non-object args `getProfile`, `documentList` and `memoryForget` all declare `required: []`, so a model may legitimately call them with no arguments at all. The OpenAI API serialises that as `arguments: ""`, and `parseToolArguments` handed the empty string straight to `JSON.parse`, so every no-argument call came back as {"success":false,"error":"Invalid JSON arguments for getProfile"} Those three tools were unreachable in their documented no-argument form. The same gate also lets non-object JSON through. `"null"` parses cleanly and then rejects in the destructuring parameter of every tool function -- `TypeError: Cannot destructure property 'containerTag' of 'object null'` -- which escapes `executeToolCall`, since it has no catch, and fails the whole request. That is precisely the throw #1488 added this gate to contain. `"5"` and `"\"text\""` are quieter but worse: they destructure to `undefined` and call the API with no container tag at all. Treat blank arguments as `{}`, and require the parsed value to be a non-null, non-array object. Malformed JSON still returns the tool error #1488 added. Adds eight regression tests. Six of them fail against the current implementation -- two on the blank-argument path and four on the non-object path, one carrying the raw TypeError. The two guard tests, malformed JSON and an ordinary well-formed call, pass both before and after, so the behaviour --- packages/tools/src/openai/tools.ts | 12 +++- packages/tools/src/tool-operations.test.ts | 81 ++++++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/packages/tools/src/openai/tools.ts b/packages/tools/src/openai/tools.ts index 5069a6151..9bc593b0e 100644 --- a/packages/tools/src/openai/tools.ts +++ b/packages/tools/src/openai/tools.ts @@ -458,8 +458,18 @@ export function getToolDefinitions(): OpenAI.Chat.Completions.ChatCompletionTool } function parseToolArguments(argumentsJson: string) { + // OpenAI sends "" for tools called with no arguments. + const source = argumentsJson?.trim() || "{}" + try { - return { success: true as const, value: JSON.parse(argumentsJson) } + const value = JSON.parse(source) + + // "null", "5" and "[]" parse but would throw when destructured. + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return { success: false as const } + } + + return { success: true as const, value } } catch { return { success: false as const } } diff --git a/packages/tools/src/tool-operations.test.ts b/packages/tools/src/tool-operations.test.ts index 1ec5c0ea4..0283fcc35 100644 --- a/packages/tools/src/tool-operations.test.ts +++ b/packages/tools/src/tool-operations.test.ts @@ -341,6 +341,87 @@ describe("memoryForget", () => { }) }) +describe("openai executeToolCall argument parsing", () => { + type ExecutorToolCall = Parameters< + ReturnType + >[0] + + function toolCall(name: string, args: string) { + return { + id: "call_1", + type: "function", + function: { name, arguments: args }, + } as ExecutorToolCall + } + + // getProfile, documentList and memoryForget declare `required: []`, so the model + // is allowed to call them with no arguments. OpenAI serialises that as "". + it.each([ + "", + " ", + ])("runs a zero-argument tool when arguments are %p", async (args) => { + const execute = openAi.createToolCallExecutor(API_KEY, { + namespace: "user_1", + }) + + const result = JSON.parse(await execute(toolCall("getProfile", args))) + + expect(result.success).toBe(true) + expect(sdk.profile).toHaveBeenCalledTimes(1) + expect(sdk.profile.mock.calls[0]?.[0]).toBe("user_1") + }) + + // These parse cleanly, so the JSON guard lets them through to a destructuring + // parameter that rejects — the throw the guard exists to contain. + it.each([ + "null", + "5", + "[]", + '"text"', + ])("rejects non-object arguments %p as a tool result rather than throwing", async (args) => { + const execute = openAi.createToolCallExecutor(API_KEY) + + const result = JSON.parse(await execute(toolCall("getProfile", args))) + + expect(result.success).toBe(false) + expect(result.error).toMatch(/Invalid JSON arguments for getProfile/) + expect(sdk.profile).not.toHaveBeenCalled() + }) + + it("still reports malformed JSON as a tool error", async () => { + const execute = openAi.createToolCallExecutor(API_KEY) + + const result = JSON.parse( + await execute(toolCall("searchMemories", "{not json")), + ) + + expect(result.success).toBe(false) + expect(result.error).toMatch(/Invalid JSON arguments for searchMemories/) + expect(sdk.search).not.toHaveBeenCalled() + }) + + it("still passes well-formed arguments through", async () => { + const execute = openAi.createToolCallExecutor(API_KEY, { + namespace: "user_1", + }) + + const result = JSON.parse( + await execute( + toolCall( + "searchMemories", + JSON.stringify({ informationToGet: "tea", limit: 3 }), + ), + ), + ) + + expect(result.success).toBe(true) + expect(sdk.search).toHaveBeenCalledWith( + "user_1", + expect.objectContaining({ query: "tea", limit: 3 }), + ) + }) +}) + describe("ClaudeMemoryTool", () => { const FILE_PATH = "/memories/prefs.txt" const FILE_ID = "memories_prefs_txt" From a93d85adeea8b79b0c86f9f704a937dc0ff7a5f5 Mon Sep 17 00:00:00 2001 From: Aditya kumar singh <143548997+Adityakk9031@users.noreply.github.com> Date: Thu, 17 Sep 2026 17:07:40 +0530 Subject: [PATCH 07/11] fix(tools): escape delimiters in makeTurnKey to prevent cache collision --- packages/tools/src/shared/cache.ts | 7 ++++- packages/tools/src/tools-shared.test.ts | 37 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/packages/tools/src/shared/cache.ts b/packages/tools/src/shared/cache.ts index e27407b21..d382bb1a8 100644 --- a/packages/tools/src/shared/cache.ts +++ b/packages/tools/src/shared/cache.ts @@ -5,6 +5,9 @@ import type { MemoryMode } from "./types" * Generic memory cache for storing per-turn memories to avoid redundant API calls. * Used to cache memory retrieval results during tool-call loops within the same turn. */ +const escapeKeySegment = (segment: string): string => + segment.replace(/%/g, "%25").replace(/:/g, "%3A") + export class MemoryCache { private cache: LRUCache = new LRUCache({ max: 100 }) @@ -25,7 +28,9 @@ export class MemoryCache { message: string, ): string { const normalizedMessage = message.trim().replace(/\s+/g, " ") - return `${namespace}:${threadId || ""}:${mode}:${normalizedMessage}` + const safeNamespace = escapeKeySegment(namespace) + const safeThreadId = escapeKeySegment(threadId ?? "") + return `${safeNamespace}:${safeThreadId}:${mode}:${normalizedMessage}` } /** diff --git a/packages/tools/src/tools-shared.test.ts b/packages/tools/src/tools-shared.test.ts index 638749c5a..756cec7e1 100644 --- a/packages/tools/src/tools-shared.test.ts +++ b/packages/tools/src/tools-shared.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest" +import { makeTurnKey } from "./shared/cache" import { DEFAULT_VALUES, SEARCH_LIMIT_BOUNDS, @@ -120,3 +121,39 @@ describe("deduplicateMemoriesForMode", () => { expect(deduplicated.searchResults).toEqual([]) }) }) + +describe("makeTurnKey", () => { + it("generates predictable turn key for standard inputs", () => { + expect(makeTurnKey("user-123", "thread-456", "full", "hello world")).toBe( + "user-123:thread-456:full:hello world", + ) + }) + + it("normalizes and collapses whitespace in message", () => { + expect( + makeTurnKey("user-123", "thread-456", "full", " hello world \n "), + ).toBe("user-123:thread-456:full:hello world") + }) + + it("handles undefined threadId cleanly", () => { + expect(makeTurnKey("user-123", undefined, "profile", "test")).toBe( + "user-123::profile:test", + ) + }) + + it("escapes colons to prevent cache key collisions between tag and threadId", () => { + const keyA = makeTurnKey("user:123", "456", "profile", "hi") + const keyB = makeTurnKey("user", "123:456", "profile", "hi") + expect(keyA).toBe("user%3A123:456:profile:hi") + expect(keyB).toBe("user:123%3A456:profile:hi") + expect(keyA).not.toBe(keyB) + }) + + it("escapes percent signs to avoid ambiguity with encoded sequences", () => { + const keyA = makeTurnKey("user%3A123", "456", "profile", "hi") + const keyB = makeTurnKey("user:123", "456", "profile", "hi") + expect(keyA).toBe("user%253A123:456:profile:hi") + expect(keyB).toBe("user%3A123:456:profile:hi") + expect(keyA).not.toBe(keyB) + }) +}) From 85ddb64bfd1f359d630fe1f6da7a908add040db2 Mon Sep 17 00:00:00 2001 From: Aditya kumar singh <143548997+Adityakk9031@users.noreply.github.com> Date: Thu, 17 Sep 2026 17:05:29 +0530 Subject: [PATCH 08/11] fix(tools): handle trailing slashes and whitespace in normalizeBaseUrl and addConversation --- packages/tools/src/shared/context.ts | 4 ++-- packages/tools/src/tools-shared.test.ts | 27 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/tools/src/shared/context.ts b/packages/tools/src/shared/context.ts index f98bd2658..8521eb5a7 100644 --- a/packages/tools/src/shared/context.ts +++ b/packages/tools/src/shared/context.ts @@ -8,8 +8,8 @@ import { Supermemory } from "supermemory" */ export const normalizeBaseUrl = (url?: string): string => { const defaultUrl = "https://api.supermemory.ai" - if (!url) return defaultUrl - return url.endsWith("/") ? url.slice(0, -1) : url + const trimmed = url?.trim().replace(/\/+$/, "") + return trimmed || defaultUrl } /** diff --git a/packages/tools/src/tools-shared.test.ts b/packages/tools/src/tools-shared.test.ts index 756cec7e1..f963b896a 100644 --- a/packages/tools/src/tools-shared.test.ts +++ b/packages/tools/src/tools-shared.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest" import { makeTurnKey } from "./shared/cache" +import { normalizeBaseUrl } from "./shared/context" import { DEFAULT_VALUES, SEARCH_LIMIT_BOUNDS, @@ -157,3 +158,29 @@ describe("makeTurnKey", () => { expect(keyA).not.toBe(keyB) }) }) + +describe("normalizeBaseUrl", () => { + it("returns default URL when input is missing or empty", () => { + expect(normalizeBaseUrl()).toBe("https://api.supermemory.ai") + expect(normalizeBaseUrl("")).toBe("https://api.supermemory.ai") + expect(normalizeBaseUrl(" ")).toBe("https://api.supermemory.ai") + }) + + it("collapses single and multiple trailing slashes", () => { + expect(normalizeBaseUrl("http://localhost:6768/")).toBe( + "http://localhost:6768", + ) + expect(normalizeBaseUrl("http://localhost:6768///")).toBe( + "http://localhost:6768", + ) + expect(normalizeBaseUrl("https://api.supermemory.ai/")).toBe( + "https://api.supermemory.ai", + ) + }) + + it("trims whitespace around URLs", () => { + expect(normalizeBaseUrl(" http://localhost:6768/ ")).toBe( + "http://localhost:6768", + ) + }) +}) From 8d12b4bee24bd03aa73398186334284551a39947 Mon Sep 17 00:00:00 2001 From: Mahesh Sanikommu Date: Mon, 12 Oct 2026 14:04:31 -0700 Subject: [PATCH 09/11] fix(tools): adapt community fixes to v5 and release 3.1.2 Ports the #1676 collision fix to v5 with a reversible path ID and a legacy-ID fallback, so files written by 3.1.1 still open and get overwritten in place (also while their content is still processing). Keeps new_str required for str_replace and moves the PR tests onto the v5 mocks. --- packages/tools/package.json | 2 +- packages/tools/src/claude-memory.test.ts | 120 +++++++++-- packages/tools/src/claude-memory.ts | 29 +-- packages/tools/src/tool-operations.test.ts | 4 +- .../tools/test/claude-memory-commands.test.ts | 192 ++++++------------ 5 files changed, 177 insertions(+), 170 deletions(-) diff --git a/packages/tools/package.json b/packages/tools/package.json index 8e9fb55c9..63d6de35c 100644 --- a/packages/tools/package.json +++ b/packages/tools/package.json @@ -1,7 +1,7 @@ { "name": "@supermemory/tools", "type": "module", - "version": "3.1.1", + "version": "3.1.2", "description": "Memory tools for AI SDK, OpenAI, Voltagent and Mastra with supermemory", "scripts": { "build": "tsdown", diff --git a/packages/tools/src/claude-memory.test.ts b/packages/tools/src/claude-memory.test.ts index 856d8a45b..ea9b59998 100644 --- a/packages/tools/src/claude-memory.test.ts +++ b/packages/tools/src/claude-memory.test.ts @@ -192,8 +192,8 @@ describe("ClaudeMemoryTool insert line semantics", () => { let tool: ClaudeMemoryTool beforeEach(() => { - searchExecute.mockReset() - addMock.mockReset() + documentsGetMock.mockReset() + updateMock.mockReset().mockResolvedValue({ id: "doc", status: "queued" }) mockDocument(FILE_CONTENT) tool = new ClaudeMemoryTool("test-api-key") }) @@ -210,7 +210,7 @@ describe("ClaudeMemoryTool insert line semantics", () => { }) expect(result.success).toBe(true) - const stored = addMock.mock.calls[0]?.[0]?.content as string + const stored = updateMock.mock.calls[0]?.[2]?.content as string expect(stored).toBe("header\nline1\nline2\nline3\nline4\nline5") }) @@ -223,7 +223,7 @@ describe("ClaudeMemoryTool insert line semantics", () => { }) expect(result.success).toBe(true) - const stored = addMock.mock.calls[0]?.[0]?.content as string + const stored = updateMock.mock.calls[0]?.[2]?.content as string // Regression guard: the old 1-based insert-BEFORE landed this one line early. expect(stored).toBe("line1\nline2\nafter2\nline3\nline4\nline5") }) @@ -237,7 +237,7 @@ describe("ClaudeMemoryTool insert line semantics", () => { }) expect(result.success).toBe(true) - const stored = addMock.mock.calls[0]?.[0]?.content as string + const stored = updateMock.mock.calls[0]?.[2]?.content as string expect(stored).toBe("line1\nline2\nline3\nline4\nline5\ntail") }) @@ -258,7 +258,7 @@ describe("ClaudeMemoryTool insert line semantics", () => { insert_text: "x", }) expect(above.success).toBe(false) - expect(addMock).not.toHaveBeenCalled() + expect(updateMock).not.toHaveBeenCalled() }) }) @@ -296,24 +296,102 @@ describe("ClaudeMemoryTool path traversal", () => { let tool: ClaudeMemoryTool beforeEach(() => { - documentsListMock.mockReset() documentsGetMock.mockReset() - addMock.mockReset() mockDocument(FILE_CONTENT) 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, - }) - - expect(result.success).toBe(false) - expect(result.error).toContain("Invalid path") - expect(documentsListMock).not.toHaveBeenCalled() - }, - ) + 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(documentsGetMock).not.toHaveBeenCalled() + }) +}) + +describe("ClaudeMemoryTool path IDs", () => { + let tool: ClaudeMemoryTool + + beforeEach(() => { + documentsGetMock.mockReset() + updateMock.mockReset().mockResolvedValue({ id: "doc", status: "queued" }) + mockDocuments([]) + tool = new ClaudeMemoryTool("test-api-key") + }) + + it("gives paths that used to collide distinct IDs", async () => { + const paths = [ + "/memories/notes.txt", + "/memories/notes_txt", + "/memories/notes/txt", + ] + for (const path of paths) { + await tool.handleCommand({ command: "create", path, file_text: "x" }) + } + + const ids = updateMock.mock.calls.map((call) => call[1]) + expect(new Set(ids).size).toBe(paths.length) + }) + + it("reads and overwrites a file stored under its legacy ID", async () => { + mockDocuments([ + { id: "memories_notes_txt", filePath: FILE_PATH, content: "legacy" }, + ]) + + const view = await tool.handleCommand({ command: "view", path: FILE_PATH }) + expect(view.success).toBe(true) + expect(view.content).toContain("legacy") + + await tool.handleCommand({ + command: "create", + path: FILE_PATH, + file_text: "fresh", + }) + expect(updateMock).toHaveBeenCalledWith( + expect.anything(), + "memories_notes_txt", + expect.objectContaining({ content: "fresh" }), + ) + }) + + it("overwrites a legacy file whose content is still processing", async () => { + mockDocuments([ + { + id: "memories_notes_txt", + filePath: FILE_PATH, + content: null as unknown as string, + }, + ]) + + await tool.handleCommand({ + command: "create", + path: FILE_PATH, + file_text: "fresh", + }) + expect(updateMock).toHaveBeenCalledWith( + expect.anything(), + "memories_notes_txt", + expect.objectContaining({ content: "fresh" }), + ) + }) + + it("ignores a legacy ID that belongs to a different path", async () => { + mockDocuments([ + { id: "memories_notes_txt", filePath: FILE_PATH, content: "other" }, + ]) + + const result = await tool.handleCommand({ + command: "view", + path: "/memories/notes_txt", + }) + expect(result.success).toBe(false) + }) }) diff --git a/packages/tools/src/claude-memory.ts b/packages/tools/src/claude-memory.ts index a59bffa43..f227eb1ee 100644 --- a/packages/tools/src/claude-memory.ts +++ b/packages/tools/src/claude-memory.ts @@ -65,7 +65,7 @@ export class ClaudeMemoryTool { .replace(/\./g, "_d_") } - /** Pre-3.0.1 IDs, where / and . both became _ */ + /** Pre-3.1.2 IDs, where / and . both became _ */ private legacyNormalizePathToId(path: string): string { return path.replace(/^\//, "").replace(/\//g, "_").replace(/\./g, "_") } @@ -107,20 +107,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 ( @@ -340,7 +334,13 @@ export class ClaudeMemoryTool { fileText: string, ): Promise { try { - await this.writeFile(this.normalizePathToId(filePath), fileText, { + // Overwrite a legacy-ID file in place instead of leaving a stale duplicate. + const existing = await this.getFileDocument(filePath) + const id = + existing.document?.documentId ?? + existing.documentId ?? + this.normalizePathToId(filePath) + await this.writeFile(id, fileText, { source: CLAUDE_MEMORY_SOURCE, claude_memory_type: "file", file_path: filePath, @@ -575,10 +575,12 @@ export class ClaudeMemoryTool { private async getFileDocument(filePath: string): Promise<{ success: boolean document?: ClaudeFileDocument + /** Set when the file exists, even if its content is still processing. */ + documentId?: string error?: string }> { try { - // Files written before 3.0.1 still live under the legacy ID. + // Files written before 3.1.2 still live under the legacy ID. const ids = [ this.normalizePathToId(filePath), this.legacyNormalizePathToId(filePath), @@ -602,6 +604,7 @@ export class ClaudeMemoryTool { if (typeof document.content !== "string") { return { success: false, + documentId: id, error: `File content unavailable: ${filePath}`, } } diff --git a/packages/tools/src/tool-operations.test.ts b/packages/tools/src/tool-operations.test.ts index 0283fcc35..8059aa09a 100644 --- a/packages/tools/src/tool-operations.test.ts +++ b/packages/tools/src/tool-operations.test.ts @@ -424,7 +424,7 @@ describe("openai executeToolCall argument parsing", () => { describe("ClaudeMemoryTool", () => { const FILE_PATH = "/memories/prefs.txt" - const FILE_ID = "memories_prefs_txt" + const FILE_ID = "memories_s_prefs_d_txt" function mockFileDocument(content: string) { sdk.documentsGet.mockResolvedValue({ @@ -578,7 +578,7 @@ describe("ClaudeMemoryTool", () => { expect(result.success).toBe(true) expect(sdk.add).toHaveBeenCalledWith( DEFAULT_NS, - expect.objectContaining({ id: "memories_renamed_txt" }), + expect.objectContaining({ id: "memories_s_renamed_d_txt" }), ) expect(sdk.documentsDelete).toHaveBeenCalledWith(DEFAULT_NS, { ids: [FILE_ID], diff --git a/packages/tools/test/claude-memory-commands.test.ts b/packages/tools/test/claude-memory-commands.test.ts index fd72a83ce..8764cd805 100644 --- a/packages/tools/test/claude-memory-commands.test.ts +++ b/packages/tools/test/claude-memory-commands.test.ts @@ -1,162 +1,88 @@ 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(), -})) +const sdk = vi.hoisted(() => { + class NotFoundError extends Error { + statusCode = 404 + } + return { + NotFoundError, + get: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + add: vi.fn(), + } +}) -vi.mock("supermemory", () => ({ - default: class MockSupermemory { - add = addMock - search = { execute: executeMock } - documents = { delete: deleteMock } - }, -})) +vi.mock("supermemory", () => { + class Supermemory { + add = sdk.add + list = vi.fn() + documents = { get: sdk.get, update: sdk.update, delete: sdk.delete } + } + return { default: Supermemory, Supermemory, NotFoundError: sdk.NotFoundError } +}) 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 }, - }, - ], - }) + sdk.get.mockImplementation(async () => ({ + id: "doc_src", + metadata: { + source: "claude-memory", + claude_memory_type: "file", + file_path: path, + }, + content, + })) } -describe("ClaudeMemoryTool command handling", () => { +describe("ClaudeMemoryTool rename", () => { let tool: ReturnType beforeEach(() => { vi.clearAllMocks() - addMock.mockResolvedValue({ id: "doc_1" }) - deleteMock.mockResolvedValue({}) - executeMock.mockResolvedValue({ results: [] }) + sdk.update.mockResolvedValue({ id: "doc", status: "queued" }) + sdk.delete.mockResolvedValue({ count: 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") - - const result = await tool.handleCommand({ - command: "rename", - old_path: "/memories/draft.txt", - new_path: "/memories/final.txt", - }) + it("handles the old_path/new_path shape Claude actually sends", async () => { + stubFile("/memories/draft.txt", "file body") - 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(sdk.update).toHaveBeenCalledWith( + expect.anything(), + expect.any(String), + expect.objectContaining({ content: "file body" }), + ) }) - 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, - }) - } + it("still accepts path as the source for older callers", async () => { + stubFile("/memories/draft.txt", "file body") - 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") }) }) From 8fdb2d47799a7fa07fcd8ddd0d36606777de9ae2 Mon Sep 17 00:00:00 2001 From: Mahesh Sanikommu Date: Mon, 12 Oct 2026 15:05:34 -0700 Subject: [PATCH 10/11] fix(tools): close legacy-ID and blank-argument gaps in 3.1.2 - New claude-memory IDs use a cm_ prefix so they can never match a legacy ID, and a write refuses an ID held by another document. - rename overwrites an existing destination (including a legacy ID) instead of leaving a stale copy. - Non-404 lookup failures return an error instead of allowing a write; delete works while content is still processing. - OpenAI executor rejects blank or missing required arguments, and documentDelete requires a documentId. - MemoryCommand.path is required again for every command except rename. --- packages/tools/src/claude-memory.test.ts | 93 ++++++++++++++++++- packages/tools/src/claude-memory.ts | 73 ++++++++++----- packages/tools/src/openai/tools.ts | 17 ++++ packages/tools/src/tool-operations.test.ts | 45 ++++++--- .../tools/test/claude-memory-commands.test.ts | 22 +++-- 5 files changed, 207 insertions(+), 43 deletions(-) diff --git a/packages/tools/src/claude-memory.test.ts b/packages/tools/src/claude-memory.test.ts index ea9b59998..477b04816 100644 --- a/packages/tools/src/claude-memory.test.ts +++ b/packages/tools/src/claude-memory.test.ts @@ -26,7 +26,7 @@ vi.mock("supermemory", () => { return { default: Supermemory, Supermemory, NotFoundError: sdk.NotFoundError } }) -import { ClaudeMemoryTool } from "./claude-memory" +import { ClaudeMemoryTool, type MemoryCommand } from "./claude-memory" const FILE_PATH = "/memories/notes.txt" // 5 distinct lines so an off-by-one at either end is observable. @@ -383,6 +383,97 @@ describe("ClaudeMemoryTool path IDs", () => { ) }) + it("never writes /memories/a into the legacy ID of /memories/s/a", async () => { + mockDocuments([ + { id: "memories_s_a", filePath: "/memories/s/a", content: "legacy" }, + ]) + + const result = await tool.handleCommand({ + command: "create", + path: "/memories/a", + file_text: "new", + }) + + expect(result.success).toBe(true) + expect(updateMock).toHaveBeenCalledTimes(1) + expect(updateMock.mock.calls[0]?.[1]).toBe("cm_memories_s_a") + }) + + it("refuses to overwrite a foreign document holding the file's ID", async () => { + mockDocuments([ + { id: "cm_memories_s_a", filePath: "/memories/other", content: "x" }, + ]) + + const result = await tool.handleCommand({ + command: "create", + path: "/memories/a", + file_text: "new", + }) + + expect(result.success).toBe(false) + expect(updateMock).not.toHaveBeenCalled() + }) + + it("renames onto a legacy destination in place", async () => { + mockDocuments([ + { + id: "cm_memories_s_src_d_txt", + filePath: "/memories/src.txt", + content: "moved", + }, + { + id: "memories_dst_txt", + filePath: "/memories/dst.txt", + content: "stale", + }, + ]) + sdk.delete.mockResolvedValue({ count: 1 }) + + const result = await tool.handleCommand({ + command: "rename", + old_path: "/memories/src.txt", + new_path: "/memories/dst.txt", + }) + + expect(result.success).toBe(true) + expect(updateMock).toHaveBeenCalledWith( + expect.anything(), + "memories_dst_txt", + expect.objectContaining({ content: "moved" }), + ) + expect(sdk.delete).toHaveBeenCalledWith(expect.anything(), { + ids: ["cm_memories_s_src_d_txt"], + }) + }) + + it("does not write when the lookup fails with a non-404 error", async () => { + documentsGetMock.mockRejectedValue( + Object.assign(new Error("upstream down"), { status: 500 }), + ) + + const result = await tool.handleCommand({ + command: "create", + path: FILE_PATH, + file_text: "fresh", + }) + + expect(result.success).toBe(false) + expect(result.error).toContain("upstream down") + expect(updateMock).not.toHaveBeenCalled() + expect(sdk.add).not.toHaveBeenCalled() + }) + + it("keeps path required for every command except rename", () => { + // @ts-expect-error view needs a path + const view: MemoryCommand = { command: "view" } + const rename: MemoryCommand = { + command: "rename", + old_path: "/memories/a", + new_path: "/memories/b", + } + expect([view, rename]).toHaveLength(2) + }) + it("ignores a legacy ID that belongs to a different path", async () => { mockDocuments([ { id: "memories_notes_txt", filePath: FILE_PATH, content: "other" }, diff --git a/packages/tools/src/claude-memory.ts b/packages/tools/src/claude-memory.ts index f227eb1ee..6ea73f6ec 100644 --- a/packages/tools/src/claude-memory.ts +++ b/packages/tools/src/claude-memory.ts @@ -10,10 +10,7 @@ export type ClaudeMemoryConfig = SupermemoryToolsConfig export const CLAUDE_MEMORY_SOURCE = "claude-memory" -export interface MemoryCommand { - command: "view" | "create" | "str_replace" | "insert" | "delete" | "rename" - // every command except rename addresses the file via path - path?: string +interface MemoryCommandFields { // view specific view_range?: [number, number] // create specific @@ -30,6 +27,13 @@ export interface MemoryCommand { new_path?: string } +export type MemoryCommand = + | (MemoryCommandFields & { + command: "view" | "create" | "str_replace" | "insert" | "delete" + path: string + }) + | (MemoryCommandFields & { command: "rename"; path?: string }) + export interface MemoryResponse { success: boolean content?: string @@ -56,13 +60,13 @@ export class ClaudeMemoryTool { private client: Supermemory private namespace: string - /** Reversible encoding so distinct paths never share an ID: /memories/a_b.txt -> memories_s_a__b_d_txt */ + /** Reversible encoding; the `cm_` prefix keeps it disjoint from legacy IDs, which always start with `memories`: /memories/a_b.txt -> cm_memories_s_a__b_d_txt */ private normalizePathToId(path: string): string { - return path + return `cm_${path .replace(/^\//, "") .replace(/_/g, "__") .replace(/\//g, "_s_") - .replace(/\./g, "_d_") + .replace(/\./g, "_d_")}` } /** Pre-3.1.2 IDs, where / and . both became _ */ @@ -334,13 +338,9 @@ export class ClaudeMemoryTool { fileText: string, ): Promise { try { - // Overwrite a legacy-ID file in place instead of leaving a stale duplicate. - const existing = await this.getFileDocument(filePath) - const id = - existing.document?.documentId ?? - existing.documentId ?? - this.normalizePathToId(filePath) - await this.writeFile(id, fileText, { + const target = await this.resolveWriteId(filePath) + if ("error" in target) return { success: false, error: target.error } + await this.writeFile(target.id, fileText, { source: CLAUDE_MEMORY_SOURCE, claude_memory_type: "file", file_path: filePath, @@ -470,14 +470,16 @@ export class ClaudeMemoryTool { try { // Find the document first const readResult = await this.getFileDocument(filePath) - if (!readResult.success || !readResult.document) { + const documentId = + readResult.document?.documentId ?? readResult.documentId + if (!documentId) { return { success: false, error: readResult.error || "File not found", } } - await this.deleteFile(readResult.document.documentId) + await this.deleteFile(documentId) return { success: true, @@ -516,17 +518,17 @@ export class ClaudeMemoryTool { } } - const originalContent = readResult.document.content - const newNormalizedId = this.normalizePathToId(newPath) + // Overwrite the destination's existing document (possibly a legacy ID) so no stale copy survives. + const target = await this.resolveWriteId(newPath) + if ("error" in target) return { success: false, error: target.error } - await this.writeFile(newNormalizedId, originalContent, { + await this.writeFile(target.id, readResult.document.content, { ...readResult.document.metadata, file_path: newPath, last_modified: new Date().toISOString(), }) - // Same normalized ID means the write above already replaced the file in place. - if (readResult.document.documentId !== newNormalizedId) { + if (readResult.document.documentId !== target.id) { await this.deleteFile(readResult.document.documentId) } @@ -577,6 +579,10 @@ export class ClaudeMemoryTool { document?: ClaudeFileDocument /** Set when the file exists, even if its content is still processing. */ documentId?: string + /** Only true for a confirmed 404, never for a failed lookup. */ + notFound?: boolean + /** The file's own ID is held by a document that isn't this file. */ + occupied?: boolean error?: string }> { try { @@ -585,6 +591,7 @@ export class ClaudeMemoryTool { this.normalizePathToId(filePath), this.legacyNormalizePathToId(filePath), ] + let occupied = false for (const id of ids) { let document: Awaited> try { @@ -599,6 +606,7 @@ export class ClaudeMemoryTool { document.metadata?.source !== CLAUDE_MEMORY_SOURCE || this.getDocumentFilePath(document) !== filePath ) { + if (id === ids[0]) occupied = true continue } if (typeof document.content !== "string") { @@ -619,7 +627,12 @@ export class ClaudeMemoryTool { } } - return { success: false, error: `File not found: ${filePath}` } + return { + success: false, + notFound: true, + occupied, + error: `File not found: ${filePath}`, + } } catch (error) { return { success: false, @@ -628,6 +641,22 @@ export class ClaudeMemoryTool { } } + /** The ID a write to filePath must use; errors rather than guessing when the lookup fails. */ + private async resolveWriteId( + filePath: string, + ): Promise<{ id: string } | { error: string }> { + const existing = await this.getFileDocument(filePath) + const existingId = existing.document?.documentId ?? existing.documentId + if (existingId) return { id: existingId } + if (!existing.notFound) { + return { error: existing.error ?? `Failed to look up ${filePath}` } + } + if (existing.occupied) { + return { error: `Another document already uses the ID for ${filePath}` } + } + return { id: this.normalizePathToId(filePath) } + } + private getDocumentFilePath(document: { metadata: unknown }): string | undefined { diff --git a/packages/tools/src/openai/tools.ts b/packages/tools/src/openai/tools.ts index 9bc593b0e..9655550bc 100644 --- a/packages/tools/src/openai/tools.ts +++ b/packages/tools/src/openai/tools.ts @@ -332,6 +332,9 @@ export function createDocumentDeleteFunction( }: { documentId: string }): Promise { + if (!documentId?.trim()) { + return { success: false, error: "documentId is required" } + } try { await deleteDocument(client, namespace, documentId) @@ -497,6 +500,20 @@ export function createToolCallExecutor( } const args = parsed.value + // Blank arguments are only valid for tools with no required fields. + const schema = + memoryToolSchemas[functionName as keyof typeof memoryToolSchemas] + const missing = schema?.parameters.required.filter( + (field: string) => + typeof args[field] !== "string" || args[field].trim() === "", + ) + if (missing?.length) { + return JSON.stringify({ + success: false, + error: `Missing required arguments for ${functionName}: ${missing.join(", ")}`, + }) + } + switch (functionName) { case "searchMemories": return JSON.stringify(await tools.searchMemories(args)) diff --git a/packages/tools/src/tool-operations.test.ts b/packages/tools/src/tool-operations.test.ts index 8059aa09a..ca48d547e 100644 --- a/packages/tools/src/tool-operations.test.ts +++ b/packages/tools/src/tool-operations.test.ts @@ -388,6 +388,22 @@ describe("openai executeToolCall argument parsing", () => { expect(sdk.profile).not.toHaveBeenCalled() }) + it.each([ + ["documentDelete", "documentId"], + ["searchMemories", "informationToGet"], + ["addMemory", "memory"], + ])("rejects blank arguments for %s, which requires %s", async (name, field) => { + const execute = openAi.createToolCallExecutor(API_KEY) + + const result = JSON.parse(await execute(toolCall(name, ""))) + + expect(result.success).toBe(false) + expect(result.error).toContain(field) + expect(sdk.documentsDelete).not.toHaveBeenCalled() + expect(sdk.search).not.toHaveBeenCalled() + expect(sdk.add).not.toHaveBeenCalled() + }) + it("still reports malformed JSON as a tool error", async () => { const execute = openAi.createToolCallExecutor(API_KEY) @@ -424,18 +440,25 @@ describe("openai executeToolCall argument parsing", () => { describe("ClaudeMemoryTool", () => { const FILE_PATH = "/memories/prefs.txt" - const FILE_ID = "memories_s_prefs_d_txt" + const FILE_ID = "cm_memories_s_prefs_d_txt" + + beforeEach(() => { + sdk.documentsGet.mockRejectedValue(new sdk.NotFoundError("missing")) + }) function mockFileDocument(content: string) { - sdk.documentsGet.mockResolvedValue({ - id: "doc_file_1", - content, - metadata: { - source: "claude-memory", - claude_memory_type: "file", - file_path: FILE_PATH, - }, - system: { status: "done" }, + sdk.documentsGet.mockImplementation(async (_ns: string, id: string) => { + if (id !== FILE_ID) throw new sdk.NotFoundError("missing") + return { + id: "doc_file_1", + content, + metadata: { + source: "claude-memory", + claude_memory_type: "file", + file_path: FILE_PATH, + }, + system: { status: "done" }, + } }) } @@ -578,7 +601,7 @@ describe("ClaudeMemoryTool", () => { expect(result.success).toBe(true) expect(sdk.add).toHaveBeenCalledWith( DEFAULT_NS, - expect.objectContaining({ id: "memories_s_renamed_d_txt" }), + expect.objectContaining({ id: "cm_memories_s_renamed_d_txt" }), ) expect(sdk.documentsDelete).toHaveBeenCalledWith(DEFAULT_NS, { ids: [FILE_ID], diff --git a/packages/tools/test/claude-memory-commands.test.ts b/packages/tools/test/claude-memory-commands.test.ts index 8764cd805..953a2bc38 100644 --- a/packages/tools/test/claude-memory-commands.test.ts +++ b/packages/tools/test/claude-memory-commands.test.ts @@ -25,15 +25,19 @@ vi.mock("supermemory", () => { import { createClaudeMemoryTool } from "../src/claude-memory" function stubFile(path: string, content: string) { - sdk.get.mockImplementation(async () => ({ - id: "doc_src", - metadata: { - source: "claude-memory", - claude_memory_type: "file", - file_path: path, - }, - content, - })) + const fileId = `cm_${path.slice(1).replace(/_/g, "__").replace(/\//g, "_s_").replace(/\./g, "_d_")}` + sdk.get.mockImplementation(async (_ns: string, id: string) => { + if (id !== fileId) throw new sdk.NotFoundError("missing") + return { + id: "doc_src", + metadata: { + source: "claude-memory", + claude_memory_type: "file", + file_path: path, + }, + content, + } + }) } describe("ClaudeMemoryTool rename", () => { From 92508cc6d0f1950b45ee3302d4e1df0b075c4e4c Mon Sep 17 00:00:00 2001 From: Mahesh Sanikommu Date: Mon, 12 Oct 2026 16:01:33 -0700 Subject: [PATCH 11/11] fix(tools): treat inherited names as unknown tools in the OpenAI executor Names like constructor, toString and __proto__ resolved Object.prototype members as schemas and threw on parameters.required, failing the whole tool batch. The schema lookup now uses Object.hasOwn, so they return the usual unknown-function error. --- packages/tools/src/openai/tools.ts | 6 ++++-- packages/tools/src/tool-operations.test.ts | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/tools/src/openai/tools.ts b/packages/tools/src/openai/tools.ts index 9655550bc..fec1536fe 100644 --- a/packages/tools/src/openai/tools.ts +++ b/packages/tools/src/openai/tools.ts @@ -501,8 +501,10 @@ export function createToolCallExecutor( const args = parsed.value // Blank arguments are only valid for tools with no required fields. - const schema = - memoryToolSchemas[functionName as keyof typeof memoryToolSchemas] + // hasOwn so names like "constructor" fall through to the unknown-function error. + const schema = Object.hasOwn(memoryToolSchemas, functionName) + ? memoryToolSchemas[functionName as keyof typeof memoryToolSchemas] + : undefined const missing = schema?.parameters.required.filter( (field: string) => typeof args[field] !== "string" || args[field].trim() === "", diff --git a/packages/tools/src/tool-operations.test.ts b/packages/tools/src/tool-operations.test.ts index ca48d547e..d0b46eb18 100644 --- a/packages/tools/src/tool-operations.test.ts +++ b/packages/tools/src/tool-operations.test.ts @@ -404,6 +404,21 @@ describe("openai executeToolCall argument parsing", () => { expect(sdk.add).not.toHaveBeenCalled() }) + it.each([ + "constructor", + "toString", + "__proto__", + ])("reports inherited name %p as an unknown function", async (name) => { + const execute = openAi.createToolCallExecutor(API_KEY) + + const result = JSON.parse(await execute(toolCall(name, "{}"))) + + expect(result).toEqual({ + success: false, + error: `Unknown function: ${name}`, + }) + }) + it("still reports malformed JSON as a tool error", async () => { const execute = openAi.createToolCallExecutor(API_KEY)