Fimeg/letta-code
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

refactor(memory): remove frontmatter limit requirement (#1484)

Browse Source 
Co-authored-by: Letta Code <noreply@letta.com>
This commit is contained in:
Sarah Wooders
2026-03-22 12:47:51 -07:00
committed by GitHub
parent 7871ddc5c2
commit d557d47ead
3 changed files with 22 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
src/agent/memoryGit.ts
View File

@@ -150,19 +150,20 @@ async function configureLocalCredentialHelper(
* Rules:
* - Frontmatter is REQUIRED (must start with ---)
* - Must be properly closed with ---
* - Required fields: description (non-empty string), limit (positive integer)
* - Required fields: description (non-empty string)
* - read_only is a PROTECTED field: agent cannot add, remove, or change it.
* Files where HEAD has read_only: true cannot be modified at all.
* - Only allowed agent-editable keys: description, limit
* - Only allowed agent-editable key: description
* - Legacy key 'limit' is tolerated for backward compatibility
* - read_only may exist (from server) but agent must not change it
*/
export const PRE_COMMIT_HOOK_SCRIPT = `#!/usr/bin/env bash
# Validate frontmatter in staged memory .md files
# Installed by Letta Code CLI
AGENT_EDITABLE_KEYS="description limit"
AGENT_EDITABLE_KEYS="description"
PROTECTED_KEYS="read_only"
ALL_KNOWN_KEYS="description limit read_only"
ALL_KNOWN_KEYS="description read_only limit"
errors=""
# Skills must always be directories: skills/<name>/SKILL.md
@@ -214,7 +215,6 @@ for file in $(git diff --cached --name-only --diff-filter=ACM | grep -E '^(memor
# Track required fields
has_description=false
has_limit=false
# Validate each line
while IFS= read -r line; do
@@ -255,10 +255,7 @@ for file in $(git diff --cached --name-only --diff-filter=ACM | grep -E '^(memor
# Validate value types
case "$key" in
limit)
has_limit=true
if ! echo "$value" | grep -qE '^[0-9]+$' || [ "$value" = "0" ]; then
errors="$errors\\n $file: 'limit' must be a positive integer, got '$value'"
fi
# Legacy field accepted for backward compatibility.
;;
description)
has_description=true
@@ -273,9 +270,6 @@ for file in $(git diff --cached --name-only --diff-filter=ACM | grep -E '^(memor
if [ "$has_description" = "false" ]; then
errors="$errors\\n $file: missing required field 'description'"
fi
if [ "$has_limit" = "false" ]; then
errors="$errors\\n $file: missing required field 'limit'"
fi
# Check if protected keys were removed (existed in HEAD but not in staged)
if [ -n "$head_content" ]; then

83
src/tests/agent/memoryGit.precommit.test.ts
View File

@@ -53,7 +53,7 @@ function tryCommit(): { success: boolean; output: string } {
}
/** Valid frontmatter for convenience */
const VALID_FM = "---\ndescription: Test block\nlimit: 20000\n---\n\n";
const VALID_FM = "---\ndescription: Test block\n---\n\n";
beforeEach(() => {
tempDir = mkdtempSync(join(tmpdir(), "memgit-test-"));
@@ -92,7 +92,7 @@ describe("pre-commit hook: frontmatter required", () => {
test("rejects unclosed frontmatter", () => {
writeAndStage(
"memory/system/broken.md",
"---\ndescription: oops\nlimit: 20000\n\nContent without closing ---\n",
"---\ndescription: oops\n\nContent without closing ---\n",
);
const result = tryCommit();
expect(result.success).toBe(false);
@@ -102,29 +102,16 @@ describe("pre-commit hook: frontmatter required", () => {
describe("pre-commit hook: required fields", () => {
test("rejects missing description", () => {
writeAndStage(
"memory/system/bad.md",
"---\nlimit: 20000\n---\n\nContent.\n",
);
writeAndStage("memory/system/bad.md", "---\n---\n\nContent.\n");
const result = tryCommit();
expect(result.success).toBe(false);
expect(result.output).toContain("missing required field 'description'");
});
test("rejects missing limit", () => {
writeAndStage(
"memory/system/bad.md",
"---\ndescription: A block\n---\n\nContent.\n",
);
const result = tryCommit();
expect(result.success).toBe(false);
expect(result.output).toContain("missing required field 'limit'");
});
test("rejects empty description", () => {
writeAndStage(
"memory/system/bad.md",
"---\ndescription:\nlimit: 20000\n---\n\nContent.\n",
"---\ndescription:\n---\n\nContent.\n",
);
const result = tryCommit();
expect(result.success).toBe(false);
@@ -133,50 +120,10 @@ describe("pre-commit hook: required fields", () => {
});
describe("pre-commit hook: field validation", () => {
test("rejects non-integer limit", () => {
writeAndStage(
"memory/system/bad.md",
"---\ndescription: valid\nlimit: abc\n---\n\nContent.\n",
);
const result = tryCommit();
expect(result.success).toBe(false);
expect(result.output).toContain("positive integer");
});
test("rejects zero limit", () => {
writeAndStage(
"memory/system/bad.md",
"---\ndescription: valid\nlimit: 0\n---\n\nContent.\n",
);
const result = tryCommit();
expect(result.success).toBe(false);
expect(result.output).toContain("positive integer");
});
test("rejects negative limit", () => {
writeAndStage(
"memory/system/bad.md",
"---\ndescription: valid\nlimit: -5\n---\n\nContent.\n",
);
const result = tryCommit();
expect(result.success).toBe(false);
expect(result.output).toContain("positive integer");
});
test("rejects float limit", () => {
writeAndStage(
"memory/system/bad.md",
"---\ndescription: valid\nlimit: 20.5\n---\n\nContent.\n",
);
const result = tryCommit();
expect(result.success).toBe(false);
expect(result.output).toContain("positive integer");
});
test("allows limit with trailing whitespace", () => {
test("allows legacy limit key for backward compatibility", () => {
writeAndStage(
"memory/system/ok.md",
"---\ndescription: test\nlimit: 20000 \n---\n\nContent.\n",
"---\ndescription: test\nlimit: legacy\n---\n\nContent.\n",
);
const result = tryCommit();
expect(result.success).toBe(true);
@@ -200,7 +147,7 @@ describe("pre-commit hook: read_only protection", () => {
rmSync(hookPath);
writeAndStage(
"memory/system/skills.md",
"---\ndescription: Skills\nlimit: 20000\nread_only: true\n---\n\nOriginal.\n",
"---\ndescription: Skills\nread_only: true\n---\n\nOriginal.\n",
);
tryCommit();
writeFileSync(hookPath, PRE_COMMIT_HOOK_SCRIPT, { mode: 0o755 });
@@ -208,7 +155,7 @@ describe("pre-commit hook: read_only protection", () => {
// Second commit: try to modify it
writeAndStage(
"memory/system/skills.md",
"---\ndescription: Skills\nlimit: 20000\nread_only: true\n---\n\nModified.\n",
"---\ndescription: Skills\nread_only: true\n---\n\nModified.\n",
);
const result = tryCommit();
expect(result.success).toBe(false);
@@ -218,7 +165,7 @@ describe("pre-commit hook: read_only protection", () => {
test("rejects agent adding read_only to new file", () => {
writeAndStage(
"memory/system/new.md",
"---\ndescription: New block\nlimit: 20000\nread_only: false\n---\n\nContent.\n",
"---\ndescription: New block\nread_only: false\n---\n\nContent.\n",
);
const result = tryCommit();
expect(result.success).toBe(false);
@@ -232,7 +179,7 @@ describe("pre-commit hook: read_only protection", () => {
rmSync(hookPath);
writeAndStage(
"memory/system/block.md",
"---\ndescription: A block\nlimit: 20000\nread_only: false\n---\n\nContent.\n",
"---\ndescription: A block\nread_only: false\n---\n\nContent.\n",
);
tryCommit();
// Re-install hook
@@ -241,7 +188,7 @@ describe("pre-commit hook: read_only protection", () => {
// Now try to change read_only
writeAndStage(
"memory/system/block.md",
"---\ndescription: A block\nlimit: 20000\nread_only: true\n---\n\nContent.\n",
"---\ndescription: A block\nread_only: true\n---\n\nContent.\n",
);
const result = tryCommit();
expect(result.success).toBe(false);
@@ -254,7 +201,7 @@ describe("pre-commit hook: read_only protection", () => {
rmSync(hookPath);
writeAndStage(
"memory/system/block.md",
"---\ndescription: A block\nlimit: 20000\nread_only: false\n---\n\nOriginal.\n",
"---\ndescription: A block\nread_only: false\n---\n\nOriginal.\n",
);
tryCommit();
writeFileSync(hookPath, PRE_COMMIT_HOOK_SCRIPT, { mode: 0o755 });
@@ -262,7 +209,7 @@ describe("pre-commit hook: read_only protection", () => {
// Modify content but keep read_only the same
writeAndStage(
"memory/system/block.md",
"---\ndescription: A block\nlimit: 20000\nread_only: false\n---\n\nUpdated.\n",
"---\ndescription: A block\nread_only: false\n---\n\nUpdated.\n",
);
const result = tryCommit();
expect(result.success).toBe(true);
@@ -274,7 +221,7 @@ describe("pre-commit hook: read_only protection", () => {
rmSync(hookPath);
writeAndStage(
"memory/system/block.md",
"---\ndescription: A block\nlimit: 20000\nread_only: false\n---\n\nContent.\n",
"---\ndescription: A block\nread_only: false\n---\n\nContent.\n",
);
tryCommit();
writeFileSync(hookPath, PRE_COMMIT_HOOK_SCRIPT, { mode: 0o755 });
@@ -282,7 +229,7 @@ describe("pre-commit hook: read_only protection", () => {
// Remove read_only from frontmatter
writeAndStage(
"memory/system/block.md",
"---\ndescription: A block\nlimit: 20000\n---\n\nContent.\n",
"---\ndescription: A block\n---\n\nContent.\n",
);
const result = tryCommit();
expect(result.success).toBe(false);

24
src/tools/impl/Memory.ts
View File

@@ -85,14 +85,11 @@ interface MemoryResult {
interface ParsedMemoryFile {
frontmatter: {
description: string;
limit: number;
read_only?: string;
};
body: string;
}
const DEFAULT_LIMIT = 2000;
export async function memory(args: MemoryArgs): Promise<MemoryResult> {
validateRequiredParams(args, ["command", "reason"], "memory");
@@ -126,7 +123,6 @@ export async function memory(args: MemoryArgs): Promise<MemoryResult> {
const rendered = renderMemoryFile(
{
description,
limit: DEFAULT_LIMIT,
},
body,
);
@@ -451,7 +447,6 @@ function parseMemoryFile(content: string): ParsedMemoryFile {
const body = match[2] ?? "";
let description: string | undefined;
let limit: number | undefined;
let readOnly: string | undefined;
for (const line of frontmatterText.split(/\r?\n/)) {
@@ -462,11 +457,6 @@ function parseMemoryFile(content: string): ParsedMemoryFile {
if (key === "description") {
description = value;
} else if (key === "limit") {
const parsedLimit = Number.parseInt(value, 10);
if (!Number.isNaN(parsedLimit)) {
limit = parsedLimit;
}
} else if (key === "read_only") {
readOnly = value;
}
@@ -475,16 +465,9 @@ function parseMemoryFile(content: string): ParsedMemoryFile {
if (!description || !description.trim()) {
throw new Error("memory: target file frontmatter is missing 'description'");
}
if (!limit || !Number.isInteger(limit) || limit <= 0) {
throw new Error(
"memory: target file frontmatter is missing a valid positive 'limit'",
);
}
return {
frontmatter: {
description,
limit,
...(readOnly !== undefined ? { read_only: readOnly } : {}),
},
body,
@@ -492,21 +475,16 @@ function parseMemoryFile(content: string): ParsedMemoryFile {
}
function renderMemoryFile(
frontmatter: { description: string; limit: number; read_only?: string },
frontmatter: { description: string; read_only?: string },
body: string,
): string {
const description = frontmatter.description.trim();
if (!description) {
throw new Error("memory: 'description' must not be empty");
}
if (!Number.isInteger(frontmatter.limit) || frontmatter.limit <= 0) {
throw new Error("memory: 'limit' must be a positive integer");
}
const lines = [
"---",
`description: ${sanitizeFrontmatterValue(description)}`,
`limit: ${frontmatter.limit}`,
];
if (frontmatter.read_only !== undefined) {