fix(tools): stop memory file paths from colliding on one customId

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

@@ -24,9 +24,9 @@ const FILE_CONTENT = "line1\nline2\nline3\nline4\nline5"
 
 function mockDocument(content: string) {
 	// `readFile` matches by `documentId === normalizePathToCustomId(path)`.
-	// normalizePathToCustomId("/memories/notes.txt") -> "memories_notes_txt"
+	// normalizePathToCustomId("/memories/notes.txt") -> "memories_snotes_dtxt"
 	searchExecute.mockResolvedValue({
-		results: [{ documentId: "memories_notes_txt", content }],
+		results: [{ documentId: "memories_snotes_dtxt", content }],
 	})
 }
 
@@ -97,8 +97,8 @@ describe("ClaudeMemoryTool exact-file matching", () => {
 	it("view finds the exact file even when a neighbour ranks first", async () => {
 		searchExecute.mockResolvedValue({
 			results: [
-				{ documentId: "memories_notes_backup_txt", content: "backup stuff" },
-				{ documentId: "memories_notes_txt", content: FILE_CONTENT },
+				{ documentId: "memories_snotes__backup_dtxt", content: "backup stuff" },
+				{ documentId: "memories_snotes_dtxt", content: FILE_CONTENT },
 			],
 		})
 
@@ -117,7 +117,7 @@ describe("ClaudeMemoryTool exact-file matching", () => {
 		// be served as the requested one.
 		searchExecute.mockResolvedValue({
 			results: [
-				{ documentId: "memories_notes_backup_txt", content: "backup stuff" },
+				{ documentId: "memories_snotes__backup_dtxt", content: "backup stuff" },
 			],
 		})
 
@@ -133,7 +133,7 @@ describe("ClaudeMemoryTool exact-file matching", () => {
 	it("str_replace refuses to modify a different file than requested", async () => {
 		searchExecute.mockResolvedValue({
 			results: [
-				{ documentId: "memories_notes_backup_txt", content: "backup stuff" },
+				{ documentId: "memories_snotes__backup_dtxt", content: "backup stuff" },
 			],
 		})
 
@@ -156,7 +156,7 @@ describe("ClaudeMemoryTool str_replace replacement literalness", () => {
 		searchExecute.mockReset()
 		addMock.mockReset()
 		searchExecute.mockResolvedValue({
-			results: [{ documentId: "memories_notes_txt", content: FILE_CONTENT }],
+			results: [{ documentId: "memories_snotes_dtxt", content: FILE_CONTENT }],
 		})
 		tool = new ClaudeMemoryTool("test-api-key")
 	})
@@ -181,3 +181,80 @@ describe("ClaudeMemoryTool str_replace replacement literalness", () => {
 		expect(stored).not.toContain("line3")
 	})
 })
+
+describe("ClaudeMemoryTool customId encoding", () => {
+	let tool: ClaudeMemoryTool
+
+	beforeEach(() => {
+		searchExecute.mockReset()
+		addMock.mockReset()
+		tool = new ClaudeMemoryTool("test-api-key")
+	})
+
+	async function customIdFor(path: string) {
+		addMock.mockReset()
+		const result = await tool.handleCommand({
+			command: "create",
+			path,
+			file_text: "contents",
+		})
+		expect(result.success).toBe(true)
+		return addMock.mock.calls[0]?.[0]?.customId as string
+	}
+
+	it("gives paths that differ only in their separators distinct customIds", async () => {
+		// All three collapsed to `memories_notes_txt` under the old encoding,
+		// so creating one silently overwrote the others.
+		const ids = [
+			await customIdFor("/memories/notes.txt"),
+			await customIdFor("/memories/notes_txt"),
+			await customIdFor("/memories/notes/txt"),
+			await customIdFor("/memories/project/a.md"),
+			await customIdFor("/memories/project_a.md"),
+		]
+
+		expect(new Set(ids).size).toBe(ids.length)
+	})
+
+	it("still finds documents written under the legacy customId", async () => {
+		searchExecute.mockResolvedValue({
+			results: [
+				{
+					documentId: "memories_notes_txt",
+					content: FILE_CONTENT,
+					metadata: { file_path: FILE_PATH },
+				},
+			],
+		})
+
+		const result = await tool.handleCommand({
+			command: "view",
+			path: FILE_PATH,
+		})
+
+		expect(result.success).toBe(true)
+		expect(result.content).toContain("line1")
+	})
+
+	it("does not serve a legacy document whose stored path differs", async () => {
+		// `memories_notes_txt` is ambiguous: it could be /memories/notes.txt,
+		// /memories/notes_txt or /memories/notes/txt. Only the stored path decides.
+		searchExecute.mockResolvedValue({
+			results: [
+				{
+					documentId: "memories_notes_txt",
+					content: "someone else's file",
+					metadata: { file_path: "/memories/notes/txt" },
+				},
+			],
+		})
+
+		const result = await tool.handleCommand({
+			command: "view",
+			path: FILE_PATH,
+		})
+
+		expect(result.success).toBe(false)
+		expect(result.error).toContain("File not found")
+	})
+})

packages/tools/src/claude-memory.ts

@@ -47,14 +47,32 @@
 	private memoryContainerPrefix: string
 
 	/**
-	 * Normalize file path to be used as customId
-	 * Converts /memories/file.txt -> memories_file_txt
+	 * Normalize file path to be used as customId.
+	 * Converts /memories/file.txt -> memories_sfile_dtxt
+	 *
+	 * The encoding is reversible: `_` is the escape character, so every `_` in
+	 * the output opens a two-character sequence (`__` = literal `_`, `_s` = `/`,
+	 * `_d` = `.`). Distinct paths therefore always get distinct customIds. The
+	 * previous scheme collapsed `/`, `.` and `_` all to `_`, so
+	 * /memories/notes.txt, /memories/notes_txt and /memories/notes/txt shared
+	 * one customId and silently overwrote each other.
 	 */
 	private normalizePathToCustomId(path: string): string {
 		return path
 			.replace(/^\//, "") // Remove leading slash
-			.replace(/\//g, "_") // Replace / with _
-			.replace(/\./g, "_") // Replace . with _
+			.replace(/_/g, "__") // Escape literal _ first
+			.replace(/\//g, "_s") // / -> _s
+			.replace(/\./g, "_d") // . -> _d
+	}
+
+	/**
+	 * The pre-collision-fix customId for a path. Documents written before that
+	 * fix still carry these ids, so reads fall back to them — guarded by an
+	 * exact `file_path` match, since legacy ids are the ambiguous ones. The
+	 * next write to the path promotes the document to the new customId.
+	 */
+	private legacyPathToCustomId(path: string): string {
+		return path.replace(/^\//, "").replace(/\//g, "_").replace(/\./g, "_")
 	}
 
 	constructor(apiKey: string, config?: ClaudeMemoryConfig) {
@@ -140,7 +158,7 @@
 				default:
 					return {
 						success: false,
 						error: `Unknown command: ${(command as any).command}`,
 					}
 			}
 		} catch (error) {
@@ -571,14 +589,17 @@
 	 */
 	private async getFileDocument(filePath: string): Promise<{
 		success: boolean
 		document?: any
 		error?: string
 	}> {
 		try {
 			const normalizedId = this.normalizePathToCustomId(filePath)
+			const legacyId = this.legacyPathToCustomId(filePath)
 
+			// Query with the legacy id: it keeps the path words intact, so search
+			// relevance is unchanged, and it is what pre-fix documents are keyed on.
 			const response = await this.client.search.execute({
-				q: normalizedId,
+				q: legacyId,
 				containerTags: this.containerTags,
 				limit: 5,
 				includeFullDocs: true,
@@ -587,9 +608,15 @@
 			// Only accept the exact customId match. Falling back to the top
 			// semantic hit would let callers read — and worse, modify or
 			// delete — a different file than the one they asked for.
-			const document = response.results?.find(
-				(r) => r.documentId === normalizedId,
-			)
+			const document =
+				response.results?.find((r) => r.documentId === normalizedId) ??
+				// Documents written before the customId encoding was made
+				// reversible. Their ids are ambiguous, so require the stored
+				// path to match exactly.
+				response.results?.find(
+					(r) =>
+						r.documentId === legacyId && r.metadata?.file_path === filePath,
+				)
 
 			if (!document) {
 				return {

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

@@ -176,7 +176,7 @@ describe("memoryForget", () => {
 
 describe("ClaudeMemoryTool", () => {
 	const FILE_PATH = "/memories/prefs.txt"
-	const CUSTOM_ID = "memories_prefs_txt"
+	const CUSTOM_ID = "memories_sprefs_dtxt"
 
 	function mockFileDocument(content: string) {
 		searchExecute.mockResolvedValue({
@@ -262,7 +262,7 @@ describe("ClaudeMemoryTool", () => {
 
 		expect(result.success).toBe(true)
 		expect(clientAdd).toHaveBeenCalledWith(
-			expect.objectContaining({ customId: "memories_renamed_txt" }),
+			expect.objectContaining({ customId: "memories_srenamed_dtxt" }),
 		)
 		expect(documentsDelete).toHaveBeenCalledWith(CUSTOM_ID)
 	})