mirror of
https://github.com/supermemoryai/supermemory.git
synced 2026-10-26 06:04:08 +00:00
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.
This commit is contained in:
parent
85ddb64bfd
commit
8d12b4bee2
5 changed files with 176 additions and 169 deletions
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
})
|
||||
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()
|
||||
},
|
||||
)
|
||||
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)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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<MemoryResponse> {
|
||||
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}`,
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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],
|
||||
|
|
|
|||
|
|
@ -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 sdk = vi.hoisted(() => {
|
||||
class NotFoundError extends Error {
|
||||
statusCode = 404
|
||||
}
|
||||
return {
|
||||
NotFoundError,
|
||||
get: vi.fn(),
|
||||
update: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
add: vi.fn(),
|
||||
}
|
||||
})
|
||||
|
||||
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 }
|
||||
},
|
||||
}))
|
||||
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<typeof createClaudeMemoryTool>
|
||||
|
||||
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")
|
||||
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"),
|
||||
)
|
||||
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"
|
||||
it("still accepts path as the source for older callers", async () => {
|
||||
stubFile("/memories/draft.txt", "file body")
|
||||
|
||||
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")
|
||||
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")
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue