fix(tools): prevent customId collisions in claude memory tool (#1547)

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

@@ -4,7 +4,9 @@ import { beforeEach, describe, expect, it, vi } from "vitest"
 // operations can be exercised deterministically without any network access.
 const documentsListMock = vi.fn()
 const documentsGetMock = vi.fn()
-const documentsDeleteBulkMock = vi.fn()
+const documentsDeleteBulkMock = vi
+	.fn()
+	.mockResolvedValue({ success: true, deletedCount: 1 })
 const addMock = vi.fn()
 
 vi.mock("supermemory", () => {
@@ -210,3 +212,138 @@ describe("ClaudeMemoryTool str_replace replacement literalness", () => {
 		expect(stored).toContain(`price is ${dollarSequence} today`)
 	})
 })
+
+describe("ClaudeMemoryTool path normalization collision resistance", () => {
+	beforeEach(() => {
+		documentsListMock.mockReset()
+		documentsGetMock.mockReset()
+		addMock.mockReset()
+		documentsDeleteBulkMock.mockReset()
+	})
+
+	it("produces distinct customIds for paths that previously collided", () => {
+		const tool = new ClaudeMemoryTool("test-api-key")
+		const paths = [
+			"/memories/notes.txt",
+			"/memories/notes_txt",
+			"/memories/notes/txt",
+			"/memories/project/a.md",
+			"/memories/project_a.md",
+		]
+
+		const ids = paths.map((path) => tool.normalizePathToCustomId(path))
+		const uniqueIds = new Set(ids)
+
+		expect(uniqueIds.size).toBe(paths.length)
+	})
+
+	it("resolves documents stored under legacy customId format", async () => {
+		// Mock a document saved with legacy normalization (memories_notes_txt)
+		mockDocuments([
+			{
+				id: "legacy-doc",
+				customId: "memories_notes_txt",
+				filePath: "/memories/notes.txt",
+				content: "legacy content",
+			},
+		])
+
+		const tool = new ClaudeMemoryTool("test-api-key")
+		const result = await tool.handleCommand({
+			command: "view",
+			path: "/memories/notes.txt",
+		})
+
+		expect(result.success).toBe(true)
+		expect(result.content).toContain("legacy content")
+	})
+
+	it("cleans up legacy-customId document when str_replace updates the file", async () => {
+		mockDocuments([
+			{
+				id: "legacy-doc",
+				customId: "memories_notes_txt",
+				filePath: "/memories/notes.txt",
+				content: "legacy content hello",
+			},
+		])
+		documentsDeleteBulkMock.mockResolvedValue({
+			success: true,
+			deletedCount: 1,
+		})
+
+		const tool = new ClaudeMemoryTool("test-api-key")
+		const result = await tool.handleCommand({
+			command: "str_replace",
+			path: "/memories/notes.txt",
+			old_str: "hello",
+			new_str: "world",
+		})
+
+		expect(result.success).toBe(true)
+		expect(addMock).toHaveBeenCalledTimes(1)
+		expect(addMock.mock.calls[0]?.[0]?.customId).toBe("memories_s_notes_d_txt")
+		expect(documentsDeleteBulkMock).toHaveBeenCalledWith({
+			ids: ["legacy-doc"],
+		})
+	})
+
+	it("cleans up legacy-customId document when insert updates the file", async () => {
+		mockDocuments([
+			{
+				id: "legacy-doc",
+				customId: "memories_notes_txt",
+				filePath: "/memories/notes.txt",
+				content: "line1\nline2",
+			},
+		])
+		documentsDeleteBulkMock.mockResolvedValue({
+			success: true,
+			deletedCount: 1,
+		})
+
+		const tool = new ClaudeMemoryTool("test-api-key")
+		const result = await tool.handleCommand({
+			command: "insert",
+			path: "/memories/notes.txt",
+			insert_line: 2,
+			insert_text: "inserted line",
+		})
+
+		expect(result.success).toBe(true)
+		expect(addMock).toHaveBeenCalledTimes(1)
+		expect(addMock.mock.calls[0]?.[0]?.customId).toBe("memories_s_notes_d_txt")
+		expect(documentsDeleteBulkMock).toHaveBeenCalledWith({
+			ids: ["legacy-doc"],
+		})
+	})
+
+	it("cleans up legacy-customId document when create overwrites an existing file", async () => {
+		mockDocuments([
+			{
+				id: "legacy-doc",
+				customId: "memories_notes_txt",
+				filePath: "/memories/notes.txt",
+				content: "legacy content",
+			},
+		])
+		documentsDeleteBulkMock.mockResolvedValue({
+			success: true,
+			deletedCount: 1,
+		})
+
+		const tool = new ClaudeMemoryTool("test-api-key")
+		const result = await tool.handleCommand({
+			command: "create",
+			path: "/memories/notes.txt",
+			file_text: "brand new content",
+		})
+
+		expect(result.success).toBe(true)
+		expect(addMock).toHaveBeenCalledTimes(1)
+		expect(addMock.mock.calls[0]?.[0]?.customId).toBe("memories_s_notes_d_txt")
+		expect(documentsDeleteBulkMock).toHaveBeenCalledWith({
+			ids: ["legacy-doc"],
+		})
+	})
+})

packages/tools/src/claude-memory.ts

@@ -41,6 +41,7 @@ type ClaudeFileMetadata = Record<string, string | number | boolean | string[]>
 
 interface ClaudeFileDocument {
 	documentId: string
+	customId?: string
 	content: string
 	metadata: ClaudeFileMetadata
 }
@@ -57,13 +58,22 @@ export class ClaudeMemoryTool {
 
 	/**
 	 * Normalize file path to be used as customId
-	 * Converts /memories/file.txt -> memories_file_txt
+	 * Reversibly encodes path components to prevent collisions between paths like
+	 * `/memories/notes.txt`, `/memories/notes_txt`, and `/memories/notes/txt`.
 	 */
-	private normalizePathToCustomId(path: string): string {
+	normalizePathToCustomId(path: string): string {
 		return path
 			.replace(/^\//, "") // Remove leading slash
-			.replace(/\//g, "_") // Replace / with _
-			.replace(/\./g, "_") // Replace . with _
+			.replace(/_/g, "__") // Escape underscores: _ -> __
+			.replace(/\//g, "_s_") // Encode slashes: / -> _s_
+			.replace(/\./g, "_d_") // Encode dots: . -> _d_
+	}
+
+	/**
+	 * Legacy normalization used in older versions (/ and . both flattened to _)
+	 */
+	private legacyNormalizePathToCustomId(path: string): string {
+		return path.replace(/^\//, "").replace(/\//g, "_").replace(/\./g, "_")
 	}
 
 	constructor(apiKey: string, config?: ClaudeMemoryConfig) {
@@ -384,6 +394,8 @@ export class ClaudeMemoryTool {
 		fileText: string,
 	): Promise<MemoryResponse> {
 		try {
+			const existing = await this.getFileDocument(filePath)
+
 			const normalizedId = this.normalizePathToCustomId(filePath)
 
 			const _response = await this.client.add({
@@ -399,6 +411,17 @@ export class ClaudeMemoryTool {
 				},
 			})
 
+			// If an existing document was stored under a legacy customId, clean it up
+			// so the file path does not collide or become ambiguous with multiple documents.
+			if (
+				existing.success &&
+				existing.document &&
+				existing.document.customId &&
+				existing.document.customId !== normalizedId
+			) {
+				await deleteDocumentById(this.client, existing.document.documentId)
+			}
+
 			return {
 				success: true,
 				content: `File created: ${filePath}`,
@@ -457,6 +480,15 @@ export class ClaudeMemoryTool {
 				},
 			})
 
+			// If the modified file was stored under a legacy customId, clean up the legacy
+			// document to prevent path ambiguity.
+			if (
+				readResult.document.customId &&
+				readResult.document.customId !== normalizedId
+			) {
+				await deleteDocumentById(this.client, readResult.document.documentId)
+			}
+
 			return {
 				success: true,
 				content: `String replaced in file: ${filePath}`,
@@ -515,6 +547,15 @@ export class ClaudeMemoryTool {
 				},
 			})
 
+			// If the modified file was stored under a legacy customId, clean up the legacy
+			// document to prevent path ambiguity.
+			if (
+				readResult.document.customId &&
+				readResult.document.customId !== normalizedId
+			) {
+				await deleteDocumentById(this.client, readResult.document.documentId)
+			}
+
 			return {
 				success: true,
 				content: `Text inserted at line ${insertLine} in file: ${filePath}`,
@@ -650,8 +691,12 @@ export class ClaudeMemoryTool {
 				})
 
 				for (const document of response.memories) {
+					const isMatchingCustomId =
+						document.customId === normalizedId ||
+						document.customId === this.legacyNormalizePathToCustomId(filePath)
+
 					if (
-						document.customId === normalizedId &&
+						isMatchingCustomId &&
 						this.getDocumentFilePath(document) === filePath &&
 						this.isDocumentInConfiguredScope(document)
 					) {
@@ -681,8 +726,12 @@ export class ClaudeMemoryTool {
 					hasUnverifiedCandidate = true
 					continue
 				}
+				const isMatchingCustomId =
+					document.customId === normalizedId ||
+					document.customId === this.legacyNormalizePathToCustomId(filePath)
+
 				if (
-					document.customId !== normalizedId ||
+					!isMatchingCustomId ||
 					this.getDocumentFilePath(document) !== filePath ||
 					!this.hasExactContainerTags(document.containerTags)
 				) {
@@ -737,7 +786,12 @@ export class ClaudeMemoryTool {
 
 			return {
 				success: true,
-				document: { documentId: candidate.id, content, metadata },
+				document: {
+					documentId: candidate.id,
+					customId: document.customId ?? candidate.customId,
+					content,
+					metadata,
+				},
 			}
 		} catch (error) {
 			return {

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

@@ -344,7 +344,9 @@ describe("ClaudeMemoryTool", () => {
 
 		expect(result.success).toBe(true)
 		expect(clientAdd).toHaveBeenCalledWith(
-			expect.objectContaining({ customId: "memories_renamed_txt" }),
+			expect.objectContaining({
+				customId: tool.normalizePathToCustomId("/memories/renamed.txt"),
+			}),
 		)
 		expect(documentsDeleteBulk).toHaveBeenCalledWith({ ids: [DOCUMENT_ID] })
 	})