From e77b282c93ac983c1e3cb0679159f805fbcf31c3 Mon Sep 17 00:00:00 2001 From: Sarah Wooders Date: Thu, 19 Mar 2026 20:33:15 -0700 Subject: [PATCH] fix(memory): split update_description command and tighten path handling (#1456) Co-authored-by: Letta Code --- src/tests/tools/memory-tool.test.ts | 93 ++++++++++++- src/tools/descriptions/Memory.md | 11 +- src/tools/impl/Memory.ts | 207 +++++++++++++++++----------- src/tools/schemas/Memory.json | 21 +-- 4 files changed, 235 insertions(+), 97 deletions(-) diff --git a/src/tests/tools/memory-tool.test.ts b/src/tests/tools/memory-tool.test.ts index 2d3e9d0..6ed035f 100644 --- a/src/tests/tools/memory-tool.test.ts +++ b/src/tests/tools/memory-tool.test.ts @@ -196,7 +196,94 @@ describe("memory tool", () => { expect(content).toContain("Sarah: +1-555-0100"); }); - test("rejects /memories-style paths", async () => { + test("accepts absolute file paths under MEMORY_DIR", async () => { + const absolutePath = join(memoryDir, "system", "contacts.md"); + + await memory({ + command: "create", + reason: "Create contacts via absolute path", + path: absolutePath, + description: "Contacts memory absolute", + file_text: "Timber: good dog", + }); + + const content = await runGit(memoryDir, [ + "show", + "HEAD:system/contacts.md", + ]); + expect(content).toContain("description: Contacts memory absolute"); + expect(content).toContain("Timber: good dog"); + }); + + test("updates frontmatter description via update_description command", async () => { + await memory({ + command: "create", + reason: "Create coding prefs", + path: "system/human/prefs/coding.md", + description: "Old description", + file_text: "keep body unchanged", + }); + + await memory({ + command: "update_description", + reason: "Update coding prefs description", + path: "system/human/prefs/coding.md", + description: "New description", + }); + + const content = await runGit(memoryDir, [ + "show", + "HEAD:system/human/prefs/coding.md", + ]); + expect(content).toContain("description: New description"); + expect(content).toContain("keep body unchanged"); + }); + + test("rename requires old_path and new_path", async () => { + await expect( + memory({ + command: "rename", + reason: "should fail", + path: "system/contacts.md", + description: "Should not update description via rename", + } as Parameters[0]), + ).rejects.toThrow(/memory rename: 'old_path' must be a non-empty string/i); + }); + + test("delete supports recursive directory removal", async () => { + await memory({ + command: "create", + reason: "Create draft note one", + path: "reference/history/draft-one.md", + description: "Draft one", + file_text: "one", + }); + + await memory({ + command: "create", + reason: "Create draft note two", + path: "reference/history/draft-two.md", + description: "Draft two", + file_text: "two", + }); + + await memory({ + command: "delete", + reason: "Delete history directory", + path: "reference/history", + }); + + const fileTree = await runGit(memoryDir, [ + "ls-tree", + "-r", + "--name-only", + "HEAD", + ]); + expect(fileTree).not.toContain("reference/history/draft-one.md"); + expect(fileTree).not.toContain("reference/history/draft-two.md"); + }); + + test("rejects absolute paths outside MEMORY_DIR", async () => { await expect( memory({ command: "create", @@ -204,6 +291,8 @@ describe("memory tool", () => { path: "/memories/contacts", description: "Contacts memory", }), - ).rejects.toThrow(/relative path like system\/contacts\.md/i); + ).rejects.toThrow( + `The memory tool can only be used to modify files in {${memoryDir}} or provided as a relative path`, + ); }); }); diff --git a/src/tools/descriptions/Memory.md b/src/tools/descriptions/Memory.md index a6bc3c1..0035e69 100644 --- a/src/tools/descriptions/Memory.md +++ b/src/tools/descriptions/Memory.md @@ -6,15 +6,17 @@ Files stored inside of `system/` eventually become part of the agent's system pr Supported operations on memory files: - `str_replace` - `insert` -- `delete` -- `rename` (path rename or description update mode) +- `delete` (files, or directories recursively) +- `rename` (path rename only) +- `update_description` - `create` More general operations can be performanced through directory modifying the files. Path formats accepted: - relative memory file paths (e.g. `system/contacts.md`, `reference/project/team.md`) +- absolute paths only when they are inside `$MEMORY_DIR` -Note: absolute paths and `/memories/...` paths are not supported by this client-side tool. +Note: absolute paths outside `$MEMORY_DIR` are rejected. Examples: @@ -31,6 +33,9 @@ memory(command="delete", reason="Remove stale notes", path="reference/history/ol # Rename a memory file memory(command="rename", reason="Promote temp notes", old_path="reference/history/temp.md", new_path="reference/history/permanent.md") +# Update a block description +memory(command="update_description", reason="Clarify coding prefs block", path="system/human/prefs/coding.md", description="Dr. Wooders' coding preferences.") + # Create a block with starting text memory(command="create", reason="Track coding preferences", path="system/human/prefs/coding.md", description="The user's coding preferences.", file_text="The user seems to add type hints to all of their Python code.") diff --git a/src/tools/impl/Memory.ts b/src/tools/impl/Memory.ts index 14475a6..f89c75d 100644 --- a/src/tools/impl/Memory.ts +++ b/src/tools/impl/Memory.ts @@ -1,6 +1,14 @@ import { execFile as execFileCb } from "node:child_process"; import { existsSync } from "node:fs"; -import { mkdir, readFile, rename, unlink, writeFile } from "node:fs/promises"; +import { + mkdir, + readFile, + rename, + rm, + stat, + unlink, + writeFile, +} from "node:fs/promises"; import { homedir } from "node:os"; import { dirname, isAbsolute, relative, resolve } from "node:path"; import { promisify } from "node:util"; @@ -10,7 +18,13 @@ import { validateRequiredParams } from "./validation"; const execFile = promisify(execFileCb); -type MemoryCommand = "str_replace" | "insert" | "delete" | "rename" | "create"; +type MemoryCommand = + | "str_replace" + | "insert" + | "delete" + | "rename" + | "update_description" + | "create"; interface MemoryArgs { command: MemoryCommand; @@ -24,7 +38,6 @@ interface MemoryArgs { insert_text?: string; description?: string; file_text?: string; - limit?: number; } async function getAgentIdentity(): Promise<{ @@ -101,7 +114,7 @@ export async function memory(args: MemoryArgs): Promise { "description", "create", ); - const label = normalizeMemoryLabel(pathArg, "path"); + const label = normalizeMemoryLabel(memoryDir, pathArg, "path"); const filePath = resolveMemoryFilePath(memoryDir, label); const relPath = toRepoRelative(memoryDir, filePath); @@ -109,16 +122,11 @@ export async function memory(args: MemoryArgs): Promise { throw new Error(`memory create: block already exists at ${pathArg}`); } - const limit = args.limit ?? DEFAULT_LIMIT; - if (!Number.isInteger(limit) || limit <= 0) { - throw new Error("memory create: 'limit' must be a positive integer"); - } - const body = args.file_text ?? ""; const rendered = renderMemoryFile( { description, - limit, + limit: DEFAULT_LIMIT, }, body, ); @@ -139,7 +147,7 @@ export async function memory(args: MemoryArgs): Promise { "str_replace", ); - const label = normalizeMemoryLabel(pathArg, "path"); + const label = normalizeMemoryLabel(memoryDir, pathArg, "path"); const filePath = resolveMemoryFilePath(memoryDir, label); const relPath = toRepoRelative(memoryDir, filePath); const file = await loadEditableMemoryFile(filePath, pathArg); @@ -166,7 +174,7 @@ export async function memory(args: MemoryArgs): Promise { throw new Error("memory insert: 'insert_line' must be a number"); } - const label = normalizeMemoryLabel(pathArg, "path"); + const label = normalizeMemoryLabel(memoryDir, pathArg, "path"); const filePath = resolveMemoryFilePath(memoryDir, label); const relPath = toRepoRelative(memoryDir, filePath); const file = await loadEditableMemoryFile(filePath, pathArg); @@ -187,68 +195,66 @@ export async function memory(args: MemoryArgs): Promise { affectedPaths = [relPath]; } else if (command === "delete") { const pathArg = requireString(args.path, "path", "delete"); - const label = normalizeMemoryLabel(pathArg, "path"); - const filePath = resolveMemoryFilePath(memoryDir, label); - const relPath = toRepoRelative(memoryDir, filePath); + const label = normalizeMemoryLabel(memoryDir, pathArg, "path"); + const targetPath = resolveMemoryPath(memoryDir, label); - await loadEditableMemoryFile(filePath, pathArg); - await unlink(filePath); - affectedPaths = [relPath]; - } else if (command === "rename") { - const hasDescriptionUpdate = - typeof args.path === "string" && - args.path.trim().length > 0 && - typeof args.description === "string" && - args.description.trim().length > 0 && - !args.old_path && - !args.new_path; - - if (hasDescriptionUpdate) { - const pathArg = requireString(args.path, "path", "rename"); - const newDescription = requireString( - args.description, - "description", - "rename description update", - ); - - const label = normalizeMemoryLabel(pathArg, "path"); - const filePath = resolveMemoryFilePath(memoryDir, label); - const relPath = toRepoRelative(memoryDir, filePath); - const file = await loadEditableMemoryFile(filePath, pathArg); - - const rendered = renderMemoryFile( - { - ...file.frontmatter, - description: newDescription, - }, - file.body, - ); - await writeFile(filePath, rendered, "utf8"); + if (existsSync(targetPath) && (await stat(targetPath)).isDirectory()) { + const relPath = toRepoRelative(memoryDir, targetPath); + await rm(targetPath, { recursive: true, force: false }); affectedPaths = [relPath]; } else { - const oldPathArg = requireString(args.old_path, "old_path", "rename"); - const newPathArg = requireString(args.new_path, "new_path", "rename"); + const filePath = resolveMemoryFilePath(memoryDir, label); + const relPath = toRepoRelative(memoryDir, filePath); - const oldLabel = normalizeMemoryLabel(oldPathArg, "old_path"); - const newLabel = normalizeMemoryLabel(newPathArg, "new_path"); - - const oldFilePath = resolveMemoryFilePath(memoryDir, oldLabel); - const newFilePath = resolveMemoryFilePath(memoryDir, newLabel); - - const oldRelPath = toRepoRelative(memoryDir, oldFilePath); - const newRelPath = toRepoRelative(memoryDir, newFilePath); - - if (existsSync(newFilePath)) { - throw new Error( - `memory rename: destination already exists at ${newPathArg}`, - ); - } - - await loadEditableMemoryFile(oldFilePath, oldPathArg); - await mkdir(dirname(newFilePath), { recursive: true }); - await rename(oldFilePath, newFilePath); - affectedPaths = [oldRelPath, newRelPath]; + await loadEditableMemoryFile(filePath, pathArg); + await unlink(filePath); + affectedPaths = [relPath]; } + } else if (command === "rename") { + const oldPathArg = requireString(args.old_path, "old_path", "rename"); + const newPathArg = requireString(args.new_path, "new_path", "rename"); + + const oldLabel = normalizeMemoryLabel(memoryDir, oldPathArg, "old_path"); + const newLabel = normalizeMemoryLabel(memoryDir, newPathArg, "new_path"); + + const oldFilePath = resolveMemoryFilePath(memoryDir, oldLabel); + const newFilePath = resolveMemoryFilePath(memoryDir, newLabel); + + const oldRelPath = toRepoRelative(memoryDir, oldFilePath); + const newRelPath = toRepoRelative(memoryDir, newFilePath); + + if (existsSync(newFilePath)) { + throw new Error( + `memory rename: destination already exists at ${newPathArg}`, + ); + } + + await loadEditableMemoryFile(oldFilePath, oldPathArg); + await mkdir(dirname(newFilePath), { recursive: true }); + await rename(oldFilePath, newFilePath); + affectedPaths = [oldRelPath, newRelPath]; + } else if (command === "update_description") { + const pathArg = requireString(args.path, "path", "update_description"); + const newDescription = requireString( + args.description, + "description", + "update_description", + ); + + const label = normalizeMemoryLabel(memoryDir, pathArg, "path"); + const filePath = resolveMemoryFilePath(memoryDir, label); + const relPath = toRepoRelative(memoryDir, filePath); + const file = await loadEditableMemoryFile(filePath, pathArg); + + const rendered = renderMemoryFile( + { + ...file.frontmatter, + description: newDescription, + }, + file.body, + ); + await writeFile(filePath, rendered, "utf8"); + affectedPaths = [relPath]; } else { throw new Error(`Unsupported memory command: ${command}`); } @@ -309,7 +315,45 @@ function ensureMemoryRepo(memoryDir: string): void { } } -function normalizeMemoryLabel(inputPath: string, fieldName: string): string { +function normalizeMemoryLabel( + memoryDir: string, + inputPath: string, + fieldName: string, +): string { + const raw = inputPath.trim(); + if (!raw) { + throw new Error(`memory: '${fieldName}' must be a non-empty string`); + } + + if (raw.startsWith("~/") || raw.startsWith("$HOME/")) { + throw new Error( + `memory: '${fieldName}' must be a memory-relative file path, not a home-relative filesystem path`, + ); + } + + const isWindowsAbsolute = /^[a-zA-Z]:[\\/]/.test(raw); + if (isAbsolute(raw) || isWindowsAbsolute) { + const absolutePath = resolve(raw); + const relToMemory = relative(memoryDir, absolutePath); + + if ( + relToMemory && + !relToMemory.startsWith("..") && + !isAbsolute(relToMemory) + ) { + return normalizeRelativeMemoryLabel(relToMemory, fieldName); + } + + throw new Error(memoryPrefixError(memoryDir)); + } + + return normalizeRelativeMemoryLabel(raw, fieldName); +} + +function normalizeRelativeMemoryLabel( + inputPath: string, + fieldName: string, +): string { const raw = inputPath.trim(); if (!raw) { throw new Error(`memory: '${fieldName}' must be a non-empty string`); @@ -317,18 +361,6 @@ function normalizeMemoryLabel(inputPath: string, fieldName: string): string { const normalized = raw.replace(/\\/g, "/"); - if (/^[a-zA-Z]:\//.test(normalized)) { - throw new Error( - `memory: '${fieldName}' must be a memory-relative file path, not an absolute host path`, - ); - } - - if (normalized.startsWith("~/") || normalized.startsWith("$HOME/")) { - throw new Error( - `memory: '${fieldName}' must be a memory-relative file path, not a home-relative filesystem path`, - ); - } - if (normalized.startsWith("/")) { throw new Error( `memory: '${fieldName}' must be a relative path like system/contacts.md`, @@ -365,8 +397,17 @@ function normalizeMemoryLabel(inputPath: string, fieldName: string): string { return segments.join("/"); } +function memoryPrefixError(memoryDir: string): string { + return `The memory tool can only be used to modify files in {${memoryDir}} or provided as a relative path`; +} + function resolveMemoryFilePath(memoryDir: string, label: string): string { - const absolute = resolve(memoryDir, `${label}.md`); + const absolute = resolveMemoryPath(memoryDir, `${label}.md`); + return absolute; +} + +function resolveMemoryPath(memoryDir: string, path: string): string { + const absolute = resolve(memoryDir, path); const rel = relative(memoryDir, absolute); if (rel.startsWith("..") || isAbsolute(rel)) { throw new Error("memory: resolved path escapes memory directory"); diff --git a/src/tools/schemas/Memory.json b/src/tools/schemas/Memory.json index 89e6fed..251b57d 100644 --- a/src/tools/schemas/Memory.json +++ b/src/tools/schemas/Memory.json @@ -4,7 +4,14 @@ "properties": { "command": { "type": "string", - "enum": ["str_replace", "insert", "delete", "rename", "create"], + "enum": [ + "str_replace", + "insert", + "delete", + "rename", + "update_description", + "create" + ], "description": "Memory operation to perform" }, "reason": { @@ -13,15 +20,15 @@ }, "path": { "type": "string", - "description": "Target memory file path relative to memory root (e.g. system/contacts.md)" + "description": "Target memory path (file or directory). Accepts relative paths like system/contacts.md or absolute paths under MEMORY_DIR" }, "old_path": { "type": "string", - "description": "Source memory file path for rename operations (e.g. system/temp.md)" + "description": "Source memory file path for rename operations (relative path or absolute path under MEMORY_DIR)" }, "new_path": { "type": "string", - "description": "Destination memory file path for rename operations (e.g. system/permanent.md)" + "description": "Destination memory file path for rename operations (relative path or absolute path under MEMORY_DIR)" }, "old_string": { "type": "string", @@ -41,15 +48,11 @@ }, "description": { "type": "string", - "description": "Block description (required for create, or used with rename + path to update description)" + "description": "Block description (required for create and update_description)" }, "file_text": { "type": "string", "description": "Initial block content for create" - }, - "limit": { - "type": "number", - "description": "Optional positive integer limit for create" } }, "required": ["command", "reason"],