From 9c677d444e675cd317daf451bba5ca5d3c7950b0 Mon Sep 17 00:00:00 2001 From: Sarah Wooders Date: Sat, 21 Mar 2026 14:15:02 -0700 Subject: [PATCH] fix(memory): add pre-commit guard to validate skill formatting (#1482) Co-authored-by: Letta Code --- src/agent/memoryGit.ts | 10 +++- src/tests/agent/memoryGit.precommit.test.ts | 59 +++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/agent/memoryGit.ts b/src/agent/memoryGit.ts index 31cf063..8bcdb34 100644 --- a/src/agent/memoryGit.ts +++ b/src/agent/memoryGit.ts @@ -165,6 +165,12 @@ PROTECTED_KEYS="read_only" ALL_KNOWN_KEYS="description limit read_only" errors="" +# Skills must always be directories: skills//SKILL.md +# Reject legacy flat skill files (both current and legacy repo layouts). +for file in $(git diff --cached --name-only --diff-filter=ACMR | grep -E '^(memory/)?skills/[^/]+\\.md$' || true); do + errors="$errors\\n $file: invalid skill path (skills must be folders). Use skills//SKILL.md" +done + # Helper: extract a frontmatter value from content get_fm_value() { local content="$1" key="$2" @@ -174,7 +180,9 @@ get_fm_value() { echo "$content" | tail -n +2 | head -n $((closing_line - 1)) | grep "^$key:" | cut -d: -f2- | sed 's/^ *//;s/ *$//' } -for file in $(git diff --cached --name-only --diff-filter=ACM | grep '^memory/.*\\.md$'); do +# Match .md files under system/ or reference/ (with optional memory/ prefix). +# Skip skill SKILL.md files — they use a different frontmatter format. +for file in $(git diff --cached --name-only --diff-filter=ACM | grep -E '^(memory/)?(system|reference)/.*\\.md$'); do staged=$(git show ":$file") # Frontmatter is required diff --git a/src/tests/agent/memoryGit.precommit.test.ts b/src/tests/agent/memoryGit.precommit.test.ts index 38c5f57..db2587d 100644 --- a/src/tests/agent/memoryGit.precommit.test.ts +++ b/src/tests/agent/memoryGit.precommit.test.ts @@ -290,6 +290,65 @@ describe("pre-commit hook: read_only protection", () => { }); }); +describe("pre-commit hook: skill path guard", () => { + test("rejects legacy flat skill file in nested memory layout", () => { + writeAndStage( + "memory/skills/slack-search.md", + `${VALID_FM}Legacy flat skill file.\n`, + ); + const result = tryCommit(); + expect(result.success).toBe(false); + expect(result.output).toContain("Use skills//SKILL.md"); + }); + + test("rejects legacy flat skill file in top-level layout", () => { + writeAndStage("skills/slack-search.md", "Legacy flat skill file.\n"); + const result = tryCommit(); + expect(result.success).toBe(false); + expect(result.output).toContain("Use skills//SKILL.md"); + }); + + test("allows canonical directory-based skill path", () => { + writeAndStage("skills/slack-search/SKILL.md", "# Slack Search\n"); + const result = tryCommit(); + expect(result.success).toBe(true); + }); +}); + +describe("pre-commit hook: top-level layout (no memory/ prefix)", () => { + test("validates frontmatter for system files without memory/ prefix", () => { + writeAndStage( + "system/human.md", + "Just plain content\nno frontmatter here\n", + ); + const result = tryCommit(); + expect(result.success).toBe(false); + expect(result.output).toContain("missing frontmatter"); + }); + + test("allows valid frontmatter in system files without memory/ prefix", () => { + writeAndStage("system/human.md", `${VALID_FM}Block content here.\n`); + const result = tryCommit(); + expect(result.success).toBe(true); + }); + + test("validates frontmatter for reference files without memory/ prefix", () => { + writeAndStage("reference/notes.md", "No frontmatter\n"); + const result = tryCommit(); + expect(result.success).toBe(false); + expect(result.output).toContain("missing frontmatter"); + }); + + test("skips SKILL.md files inside skill directories", () => { + writeAndStage( + "skills/my-skill/SKILL.md", + "# My Skill\nNo frontmatter needed.\n", + ); + const result = tryCommit(); + expect(result.success).toBe(true); + }); +}); + describe("pre-commit hook: non-memory files", () => { test("ignores non-memory files", () => { writeAndStage("README.md", "---\nbogus: true\n---\n\nThis is fine.\n");