From d908209a85729fe8e98de06a20c02707ebc9cf31 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Fri, 19 Jun 2026 02:50:33 -0700 Subject: [PATCH] [eric] skills: multi-file folder skills (SKILL.md + supporting files) w/ legacy flat compat; inject folder path for on-demand provider-agnostic reads --- .../agents/manager/prompt/prompt_context.py | 32 ++++- backend/apps/skills/models.py | 5 + backend/apps/skills/skills.py | 115 ++++++++++++------ backend/tests/test_skills_folders.py | 105 ++++++++++++++++ 4 files changed, 220 insertions(+), 37 deletions(-) create mode 100644 backend/tests/test_skills_folders.py diff --git a/backend/apps/agents/manager/prompt/prompt_context.py b/backend/apps/agents/manager/prompt/prompt_context.py index 4c6236cb..063e0e3a 100644 --- a/backend/apps/agents/manager/prompt/prompt_context.py +++ b/backend/apps/agents/manager/prompt/prompt_context.py @@ -437,13 +437,39 @@ def _resolve_forced_tools(forced_tools: list[str] | None) -> str: def _resolve_attached_skills(attached_skills: list | None) -> str: - """Build a context block injecting attached skill content into the prompt.""" + """Build a context block injecting attached skill content into the prompt. + + For a multi-file (folder) skill we inject the SKILL.md body as text AND point + the agent at the folder so it can read supporting files (scripts, templates) + on demand with the normal Read/Glob/Bash tools. That keeps skills fully + provider-agnostic: plain prompt text plus universal file tools, identical on + Claude, OpenAI, Gemini, or any custom model routed through 9router. The + folder lookup is resolved backend-side from the skill id so the frontend + send payload stays a simple {id, name, content}.""" if not attached_skills: return "" + folder_by_id: dict[str, str] = {} + try: + from backend.apps.skills.skills import _sync_skills + for s in _sync_skills(): + if s.dir_path and s.has_supporting_files: + folder_by_id[s.id] = s.dir_path + except Exception: + folder_by_id = {} + sections = [] for skill in attached_skills: name = skill.get("name", "Unknown") content = skill.get("content", "") - if content: - sections.append(f"[Using skill: {name}]\n\n{content}") + if not content: + continue + block = f"[Using skill: {name}]\n\n{content}" + folder = folder_by_id.get(skill.get("id", "")) + if folder: + block += ( + f"\n\nThis skill bundles supporting files in {folder}. " + "Read them with your normal file tools (Read / Glob / Bash) when " + "the steps above call for one; don't guess their contents." + ) + sections.append(block) return "\n\n".join(sections) diff --git a/backend/apps/skills/models.py b/backend/apps/skills/models.py index 1d0ce84d..e6f701d9 100644 --- a/backend/apps/skills/models.py +++ b/backend/apps/skills/models.py @@ -12,6 +12,11 @@ class Skill(BaseModel): command: str = "" # Platform-shipped skills (e.g. App Builder): UI hides delete and DELETE returns 409, but content stays editable so users can tune them. built_in: bool = False + # Multi-file skills live in ~/.claude/skills// with a SKILL.md plus supporting files (scripts, templates). + # dir_path is set for those; empty for a legacy flat .md skill. has_supporting_files flags extra files + # beyond SKILL.md so the prompt layer knows to point the agent at the folder for on-demand reading. + dir_path: str = "" + has_supporting_files: bool = False class SkillCreate(BaseModel): diff --git a/backend/apps/skills/skills.py b/backend/apps/skills/skills.py index 6fc4334e..851d8993 100644 --- a/backend/apps/skills/skills.py +++ b/backend/apps/skills/skills.py @@ -122,30 +122,79 @@ async def skills_lifespan(): skills = SubApp("skills", skills_lifespan) +def _skill_md_path(skill_id: str) -> tuple[str | None, str]: + """Resolve where a skill's markdown lives: (path, kind). + + A skill is either a folder (~/.claude/skills//SKILL.md, multi-file) or a + legacy flat file (~/.claude/skills/.md). Folder wins if both exist. The + one place that knows the layout, so get/update/delete never re-guess it.""" + folder_md = os.path.join(SKILLS_DIR, skill_id, "SKILL.md") + if os.path.isfile(folder_md): + return folder_md, "folder" + flat_md = os.path.join(SKILLS_DIR, f"{skill_id}.md") + if os.path.isfile(flat_md): + return flat_md, "flat" + return None, "flat" + + +def _has_supporting_files(skill_dir: str) -> bool: + """True if a skill folder ships anything beyond its SKILL.md (scripts, templates).""" + try: + return any(e != "SKILL.md" and not e.startswith(".") for e in os.listdir(skill_dir)) + except OSError: + return False + + +def _build_skill(skill_id: str, content: str, md_path: str, kind: str, index: dict) -> Skill: + """Assemble a Skill from disk + index, falling back to SKILL.md frontmatter + for a folder skill the index hasn't catalogued (e.g. hand-dropped).""" + meta = dict(index.get(skill_id, {})) + if kind == "folder" and ("name" not in meta or "description" not in meta): + fm = _parse_skill_frontmatter(content) + meta.setdefault("name", fm.get("name", "")) + meta.setdefault("description", fm.get("description", "")) + pretty = skill_id.replace("-", " ").replace("_", " ").title() + skill_dir = os.path.join(SKILLS_DIR, skill_id) + return Skill( + id=skill_id, + name=meta.get("name") or pretty, + description=meta.get("description", ""), + content=content, + file_path=md_path, + command=meta.get("command", skill_id), + built_in=bool(meta.get("built_in", False)), + dir_path=skill_dir if kind == "folder" else "", + has_supporting_files=(kind == "folder" and _has_supporting_files(skill_dir)), + ) + + def _sync_skills() -> list[Skill]: - """Sync skills from the filesystem, updating the index.""" + """Sync skills from the filesystem, updating the index. Reads both layouts: + legacy flat .md files and multi-file /SKILL.md folders.""" index = _load_index() result = [] + seen: set[str] = set() - if os.path.exists(SKILLS_DIR): - for fname in os.listdir(SKILLS_DIR): - if fname.endswith(".md"): - fpath = os.path.join(SKILLS_DIR, fname) - with open(fpath) as f: - content = f.read() + if not os.path.exists(SKILLS_DIR): + return result - skill_id = fname.replace(".md", "") - meta = index.get(skill_id, {}) - skill = Skill( - id=skill_id, - name=meta.get("name", fname.replace(".md", "").replace("-", " ").replace("_", " ").title()), - description=meta.get("description", ""), - content=content, - file_path=fpath, - command=meta.get("command", fname.replace(".md", "")), - built_in=bool(meta.get("built_in", False)), - ) - result.append(skill) + for entry in os.listdir(SKILLS_DIR): + full = os.path.join(SKILLS_DIR, entry) + if os.path.isdir(full): + skill_id = entry + elif entry.endswith(".md"): + skill_id = entry[: -len(".md")] + else: + continue + if skill_id in seen: + continue + md_path, kind = _skill_md_path(skill_id) + if not md_path: + continue + with open(md_path, encoding="utf-8") as f: + content = f.read() + seen.add(skill_id) + result.append(_build_skill(skill_id, content, md_path, kind, index)) return result @@ -254,12 +303,12 @@ async def create_skill(body: SkillCreate): @skills.router.put("/{skill_id}") async def update_skill(skill_id: str, body: SkillUpdate): - fpath = os.path.join(SKILLS_DIR, f"{skill_id}.md") - if not os.path.exists(fpath): + md_path, kind = _skill_md_path(skill_id) + if not md_path: raise HTTPException(status_code=404, detail="Skill not found") if body.content is not None: - with open(fpath, "w") as f: + with open(md_path, "w", encoding="utf-8") as f: f.write(body.content) index = _load_index() @@ -273,17 +322,10 @@ async def update_skill(skill_id: str, body: SkillUpdate): index[skill_id] = meta _save_index(index) - with open(fpath) as f: + with open(md_path, encoding="utf-8") as f: content = f.read() - skill = Skill( - id=skill_id, - name=meta.get("name", skill_id), - description=meta.get("description", ""), - content=content, - file_path=fpath, - command=meta.get("command", skill_id), - ) + skill = _build_skill(skill_id, content, md_path, kind, index) return {"ok": True, "skill": skill.model_dump()} @@ -299,9 +341,14 @@ async def delete_skill(skill_id: str): "the next agent turn)." ), ) - fpath = os.path.join(SKILLS_DIR, f"{skill_id}.md") - if os.path.exists(fpath): - os.remove(fpath) + # Remove whichever layout exists: the whole folder, or the flat file. + import shutil + skill_dir = os.path.join(SKILLS_DIR, skill_id) + flat = os.path.join(SKILLS_DIR, f"{skill_id}.md") + if os.path.isdir(skill_dir): + shutil.rmtree(skill_dir, ignore_errors=True) + if os.path.isfile(flat): + os.remove(flat) index.pop(skill_id, None) _save_index(index) return {"ok": True} diff --git a/backend/tests/test_skills_folders.py b/backend/tests/test_skills_folders.py new file mode 100644 index 00000000..ab46a3f3 --- /dev/null +++ b/backend/tests/test_skills_folders.py @@ -0,0 +1,105 @@ +"""Multi-file (folder) skills, plus backward compatibility with legacy flat skills. + +A skill is now either ~/.claude/skills//SKILL.md (with optional supporting +files) or a legacy ~/.claude/skills/.md. Both must list, read, and delete +correctly, and a folder skill with supporting files must get its folder path +appended to the prompt so the agent can read those files on demand. +""" + +from __future__ import annotations + +import os +import json + +import pytest + +import backend.apps.skills.skills as skills_mod +from backend.apps.agents.manager.prompt.prompt_context import _resolve_attached_skills + + +@pytest.fixture +def skills_dir(tmp_path, monkeypatch): + d = tmp_path / "skills" + d.mkdir() + monkeypatch.setattr(skills_mod, "SKILLS_DIR", str(d)) + monkeypatch.setattr(skills_mod, "INDEX_PATH", str(d / ".skills_index.json")) + return d + + +def _write(path, text): + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, "w", encoding="utf-8") as f: + f.write(text) + + +def test_flat_skill_still_syncs(skills_dir): + _write(str(skills_dir / "my-flat.md"), "do the flat thing") + skills = {s.id: s for s in skills_mod._sync_skills()} + assert "my-flat" in skills + s = skills["my-flat"] + assert s.content == "do the flat thing" + assert s.dir_path == "" + assert s.has_supporting_files is False + + +def test_folder_skill_syncs_with_supporting_files(skills_dir): + base = skills_dir / "remotion" + _write(str(base / "SKILL.md"), "---\nname: Remotion\ndescription: make videos\n---\nrender stuff") + _write(str(base / "helper.py"), "print('hi')") + skills = {s.id: s for s in skills_mod._sync_skills()} + assert "remotion" in skills + s = skills["remotion"] + assert "render stuff" in s.content + assert s.dir_path == str(base) + assert s.has_supporting_files is True + # Frontmatter fills name/description when the index hasn't catalogued it. + assert s.name == "Remotion" + assert s.description == "make videos" + + +def test_folder_skill_without_extra_files_flags_false(skills_dir): + base = skills_dir / "solo" + _write(str(base / "SKILL.md"), "just one file") + s = {x.id: x for x in skills_mod._sync_skills()}["solo"] + assert s.dir_path == str(base) + assert s.has_supporting_files is False + + +@pytest.mark.asyncio +async def test_delete_removes_folder(skills_dir): + base = skills_dir / "doomed" + _write(str(base / "SKILL.md"), "x") + _write(str(base / "data.txt"), "y") + assert base.is_dir() + await skills_mod.delete_skill("doomed") + assert not base.exists() + + +@pytest.mark.asyncio +async def test_update_writes_folder_skill_md(skills_dir): + base = skills_dir / "editable" + _write(str(base / "SKILL.md"), "old body") + from backend.apps.skills.models import SkillUpdate + res = await skills_mod.update_skill("editable", SkillUpdate(content="new body", description="d")) + assert res["ok"] + with open(base / "SKILL.md", encoding="utf-8") as f: + assert f.read() == "new body" + assert res["skill"]["dir_path"] == str(base) + + +def test_injection_points_at_folder_for_supporting_files(skills_dir): + base = skills_dir / "withfiles" + _write(str(base / "SKILL.md"), "use the template") + _write(str(base / "template.html"), "") + + block = _resolve_attached_skills([{"id": "withfiles", "name": "WithFiles", "content": "use the template"}]) + assert "[Using skill: WithFiles]" in block + assert str(base) in block + assert "Read" in block # tells the agent to read supporting files + + +def test_injection_no_folder_note_for_flat_skill(skills_dir): + _write(str(skills_dir / "plain.md"), "plain content") + block = _resolve_attached_skills([{"id": "plain", "name": "Plain", "content": "plain content"}]) + assert "[Using skill: Plain]" in block + assert "supporting files" not in block.lower()