fix(memory): split update_description command and tighten path handling (#1456)
Co-authored-by: Letta Code <noreply@letta.com>
This commit is contained in:
@@ -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<typeof memory>[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`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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.")
|
||||
|
||||
|
||||
@@ -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<MemoryResult> {
|
||||
"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<MemoryResult> {
|
||||
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<MemoryResult> {
|
||||
"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<MemoryResult> {
|
||||
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<MemoryResult> {
|
||||
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");
|
||||
|
||||
@@ -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"],
|
||||
|
||||
Reference in New Issue
Block a user