fix(agent): keep saved skill frontmatter name in sync with its slug
Signed-off-by: Lanre Shittu <136805224+Shizoqua@users.noreply.github.com>
This commit is contained in:
1 parent
d7f1b9e22b
commit
df2b09433f
4 files changed
+79
-2
No files matched your search
@@ -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'<skill name="{name}">\n{skill.body}\n</skill>'
|
||||
|
||||
available = ", ".join(s.name for s in self.skills)
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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 == []
|
||||
|
||||
Reference in new issue
Block a user