fix(memory): add pre-commit guard to validate skill formatting (#1482)
Co-authored-by: Letta Code <noreply@letta.com>
This commit is contained in:
@@ -165,6 +165,12 @@ PROTECTED_KEYS="read_only"
|
|||||||
ALL_KNOWN_KEYS="description limit read_only"
|
ALL_KNOWN_KEYS="description limit read_only"
|
||||||
errors=""
|
errors=""
|
||||||
|
|
||||||
|
# Skills must always be directories: skills/<name>/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/<name>/SKILL.md"
|
||||||
|
done
|
||||||
|
|
||||||
# Helper: extract a frontmatter value from content
|
# Helper: extract a frontmatter value from content
|
||||||
get_fm_value() {
|
get_fm_value() {
|
||||||
local content="$1" key="$2"
|
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/ *$//'
|
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")
|
staged=$(git show ":$file")
|
||||||
|
|
||||||
# Frontmatter is required
|
# Frontmatter is required
|
||||||
|
|||||||
@@ -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/<name>/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/<name>/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", () => {
|
describe("pre-commit hook: non-memory files", () => {
|
||||||
test("ignores non-memory files", () => {
|
test("ignores non-memory files", () => {
|
||||||
writeAndStage("README.md", "---\nbogus: true\n---\n\nThis is fine.\n");
|
writeAndStage("README.md", "---\nbogus: true\n---\n\nThis is fine.\n");
|
||||||
|
|||||||
Reference in New Issue
Block a user