From df2b09433f9acb2312d64155e9307c62a3fa8bbc Mon Sep 17 00:00:00 2001 From: Lanre Shittu <136805224+Shizoqua@users.noreply.github.com> Date: Wed, 14 Oct 2026 19:56:48 +0000 Subject: [PATCH] fix(agent): keep saved skill frontmatter name in sync with its slug Signed-off-by: Lanre Shittu <136805224+Shizoqua@users.noreply.github.com> --- agent/src/agent/skills.py | 3 +- agent/src/tools/skill_writer_tool.py | 38 +++++++++++++++++++++++++- agent/tests/test_skill_writer_tools.py | 19 +++++++++++++ agent/tests/test_skills.py | 21 ++++++++++++++ 4 files changed, 79 insertions(+), 2 deletions(-) diff --git a/agent/src/agent/skills.py b/agent/src/agent/skills.py index 60cd6032..e7bced39 100644 --- a/agent/src/agent/skills.py +++ b/agent/src/agent/skills.py @@ -182,7 +182,8 @@ class SkillsLoader: if self._user_skills_dir: skill = _load_skill_dir(self._user_skills_dir / name) if skill: - self.skills.append(skill) + if not any(existing.name == skill.name for existing in self.skills): + self.skills.append(skill) return f'\n{skill.body}\n' available = ", ".join(s.name for s in self.skills) diff --git a/agent/src/tools/skill_writer_tool.py b/agent/src/tools/skill_writer_tool.py index 1bfc39e5..68b59753 100644 --- a/agent/src/tools/skill_writer_tool.py +++ b/agent/src/tools/skill_writer_tool.py @@ -35,6 +35,11 @@ _NON_LATIN_SCRIPT_RANGES = ( ) _SKILL_SLUG_DISALLOWED_RE = re.compile(rf"[^a-z0-9\-{_NON_LATIN_SCRIPT_RANGES}]") +_FRONTMATTER_BLOCK_RE = re.compile( + r"^---[ \t]*\r?\n(?:(.*?)\r?\n)?---[ \t]*(?:\r?\n(.*))?$", + re.DOTALL, +) + def _sanitize_skill_name(name: str) -> str: """Sanitize skill name to a safe directory slug.""" @@ -45,6 +50,34 @@ def _sanitize_skill_name(name: str) -> str: return hashlib.sha256(name.strip().encode("utf-8")).hexdigest()[:8] return slug + +def _with_forced_frontmatter_name(content: str, slug: str) -> str: + """Force the frontmatter ``name:`` field to match the directory slug. + + SkillsLoader identifies a skill by its frontmatter ``name``, not by its + directory slug. If the caller supplies its own frontmatter with a + different name, the skill becomes unreachable under the slug this tool + just advertised, and every lookup under that slug falls through to a + disk-fallback path that appends a fresh duplicate entry with no + de-duplication. + """ + match = _FRONTMATTER_BLOCK_RE.match(content) + if not match: + return content + header_lines = (match.group(1) or "").split("\n") if match.group(1) else [] + body = match.group(2) or "" + new_lines = [] + replaced = False + for line in header_lines: + if line.strip().startswith("name:"): + new_lines.append(f"name: {slug}") + replaced = True + else: + new_lines.append(line) + if not replaced: + new_lines.insert(0, f"name: {slug}") + return f"---\n{chr(10).join(new_lines)}\n---\n{body}" + class SaveSkillTool(BaseTool): """Save a successful workflow as a reusable skill.""" @@ -95,13 +128,16 @@ class SaveSkillTool(BaseTool): skill_dir.mkdir(parents=True, exist_ok=True) skill_path = skill_dir / "SKILL.md" - # Ensure frontmatter exists + # Ensure frontmatter exists, and that its name always matches the + # directory slug this tool is about to advertise as load_skill(slug). if not content.strip().startswith("---"): content = ( f"---\nname: {slug}\n" f"description: User-created skill\n" f"category: {category}\n---\n\n{content}" ) + else: + content = _with_forced_frontmatter_name(content, slug) skill_path.write_text(content, encoding="utf-8") return json.dumps({ diff --git a/agent/tests/test_skill_writer_tools.py b/agent/tests/test_skill_writer_tools.py index 38131d1a..bcab4eeb 100644 --- a/agent/tests/test_skill_writer_tools.py +++ b/agent/tests/test_skill_writer_tools.py @@ -87,6 +87,25 @@ class TestSaveSkillTool: text = (user_dir / "custom" / "SKILL.md").read_text(encoding="utf-8") assert "description: Custom skill" in text + def test_save_forces_frontmatter_name_to_match_slug(self, tool: SaveSkillTool, user_dir: Path) -> None: + """A frontmatter name that disagrees with the slug this tool advertises + would make the skill unreachable under load_skill(slug) and cause + duplicate entries on every lookup; the saved name must always match. + """ + content = "---\nname: MySkill\ndescription: mismatched name\ncategory: user\n---\n\nBody" + result = json.loads(tool.execute(name="My Skill", content=content)) + assert result["status"] == "ok" + text = (user_dir / "my-skill" / "SKILL.md").read_text(encoding="utf-8") + assert "name: my-skill" in text + assert "name: MySkill" not in text + + def test_save_inserts_name_when_frontmatter_lacks_it(self, tool: SaveSkillTool, user_dir: Path) -> None: + content = "---\ndescription: no name key\ncategory: user\n---\n\nBody" + result = json.loads(tool.execute(name="headless", content=content)) + assert result["status"] == "ok" + text = (user_dir / "headless" / "SKILL.md").read_text(encoding="utf-8") + assert "name: headless" in text + def test_save_missing_name(self, tool: SaveSkillTool) -> None: result = json.loads(tool.execute(content="body")) assert result["status"] == "error" diff --git a/agent/tests/test_skills.py b/agent/tests/test_skills.py index e201ce02..5cc71990 100644 --- a/agent/tests/test_skills.py +++ b/agent/tests/test_skills.py @@ -168,6 +168,27 @@ class TestSkillsLoader: assert "Error" in content assert "nonexistent" in content + def test_get_content_fallback_does_not_duplicate_entries( + self, tmp_path: Path, empty_user_dir: Path + ) -> None: + """A mid-session skill whose frontmatter name differs from its + directory slug must not grow loader.skills on every repeated lookup. + """ + loader = SkillsLoader(tmp_path, user_skills_dir=empty_user_dir) + assert loader.skills == [] + + mismatched_dir = empty_user_dir / "my-skill" + mismatched_dir.mkdir() + (mismatched_dir / "SKILL.md").write_text( + "---\nname: MySkill\ncategory: user\ndescription: mismatched\n---\nBody", + encoding="utf-8", + ) + + for _ in range(5): + loader.get_content("my-skill") + + assert len(loader.skills) == 1 + def test_empty_dir(self, tmp_path: Path, empty_user_dir: Path) -> None: loader = SkillsLoader(tmp_path, user_skills_dir=empty_user_dir) assert loader.skills == []