fix(tools): clean up legacy customId documents during mutations to prevent path ambiguity

This commit is contained in:
Aditya kumar singh 2026-10-02 02:54:24 +05:30 • committed by Mahesh Sanikommu
parent d110df8ad2
commit 4929603cb4
2 changed files with 137 additions and 2 deletions

View file

@ -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", () => {
@ -312,6 +314,13 @@ describe("ClaudeMemoryTool path traversal", () => {
})
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 = [
@ -348,4 +357,93 @@ describe("ClaudeMemoryTool path normalization collision resistance", () => {
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"],
})
})
})

View file

@ -41,6 +41,7 @@ type ClaudeFileMetadata = Record<string, string | number | boolean | string[]>
interface ClaudeFileDocument {
documentId: string
customId?: string
content: string
metadata: ClaudeFileMetadata
}
@ -393,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({
@ -408,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}`,
@ -466,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}`,
@ -526,6 +549,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 after line ${insertLine} in file: ${filePath}`,
@ -756,7 +788,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 {