feat(skills): discover Claude and project skills
This commit is contained in:
@@ -541,7 +541,31 @@ Available Skills:
|
||||
- ... 40+ more
|
||||
```
|
||||
|
||||
**Compatible with [anthropics/skills](https://github.com/anthropics/skills)** — just copy `.md` files to `~/.openharness/skills/`.
|
||||
Skills can live in bundled, user, ohmo, project, or plugin locations. User-level skills are loaded from:
|
||||
|
||||
```text
|
||||
~/.openharness/skills/<skill>/SKILL.md
|
||||
~/.claude/skills/<skill>/SKILL.md
|
||||
~/.agents/skills/<skill>/SKILL.md
|
||||
```
|
||||
|
||||
Project-level skills are enabled by default and are discovered from the current working directory up to the git root:
|
||||
|
||||
```text
|
||||
<project>/.openharness/skills/<skill>/SKILL.md
|
||||
<project>/.agents/skills/<skill>/SKILL.md
|
||||
<project>/.claude/skills/<skill>/SKILL.md
|
||||
```
|
||||
|
||||
Disable project skills for untrusted repositories with:
|
||||
|
||||
```bash
|
||||
oh config set allow_project_skills false
|
||||
```
|
||||
|
||||
Use `/skills` to list loaded skills with their source and path. User-invocable skills can be run directly as slash commands, for example `/deploy staging`.
|
||||
|
||||
**Compatible with [anthropics/skills](https://github.com/anthropics/skills)** — use the `SKILL.md` directory layout above.
|
||||
|
||||
### 🔌 Plugin System
|
||||
|
||||
|
||||
@@ -914,10 +914,11 @@ def create_default_command_registry(
|
||||
lines = ["Available skills:"]
|
||||
for skill in skills:
|
||||
source = f" [{skill.source}]"
|
||||
path = f" {skill.path}" if skill.path else ""
|
||||
command_name = _skill_command_name(skill)
|
||||
slash = f" /{command_name}" if skill.user_invocable and _is_valid_skill_command_name(command_name) else ""
|
||||
display = f" ({skill.display_name})" if skill.display_name else ""
|
||||
lines.append(f"- {command_name}{display}{source}{slash}: {skill.description}")
|
||||
lines.append(f"- {command_name}{display}{source}{path}{slash}: {skill.description}")
|
||||
return CommandResult(message="\n".join(lines))
|
||||
|
||||
async def _config_handler(args: str, context: CommandContext) -> CommandResult:
|
||||
|
||||
@@ -518,6 +518,10 @@ class Settings(BaseModel):
|
||||
sandbox: SandboxSettings = Field(default_factory=SandboxSettings)
|
||||
enabled_plugins: dict[str, bool] = Field(default_factory=dict)
|
||||
allow_project_plugins: bool = False
|
||||
allow_project_skills: bool = True
|
||||
project_skill_dirs: list[str] = Field(
|
||||
default_factory=lambda: [".openharness/skills", ".agents/skills", ".claude/skills"]
|
||||
)
|
||||
mcp_servers: dict[str, McpServerConfig] = Field(default_factory=dict)
|
||||
|
||||
# UI
|
||||
|
||||
@@ -8,14 +8,28 @@ if TYPE_CHECKING: # pragma: no cover
|
||||
from openharness.skills.registry import SkillRegistry
|
||||
from openharness.skills.types import SkillDefinition
|
||||
|
||||
__all__ = ["SkillDefinition", "SkillRegistry", "get_user_skills_dir", "load_skill_registry"]
|
||||
__all__ = [
|
||||
"SkillDefinition",
|
||||
"SkillRegistry",
|
||||
"discover_project_skill_dirs",
|
||||
"get_user_skill_dirs",
|
||||
"get_user_skills_dir",
|
||||
"load_skill_registry",
|
||||
]
|
||||
|
||||
|
||||
def __getattr__(name: str):
|
||||
if name in {"get_user_skills_dir", "load_skill_registry"}:
|
||||
from openharness.skills.loader import get_user_skills_dir, load_skill_registry
|
||||
if name in {"discover_project_skill_dirs", "get_user_skill_dirs", "get_user_skills_dir", "load_skill_registry"}:
|
||||
from openharness.skills.loader import (
|
||||
discover_project_skill_dirs,
|
||||
get_user_skill_dirs,
|
||||
get_user_skills_dir,
|
||||
load_skill_registry,
|
||||
)
|
||||
|
||||
return {
|
||||
"discover_project_skill_dirs": discover_project_skill_dirs,
|
||||
"get_user_skill_dirs": get_user_skill_dirs,
|
||||
"get_user_skills_dir": get_user_skills_dir,
|
||||
"load_skill_registry": load_skill_registry,
|
||||
}[name]
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Skill loading from bundled and user directories."""
|
||||
"""Skill loading from bundled, user, compatibility, and project directories."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -20,14 +20,25 @@ from openharness.skills.types import SkillDefinition
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_USER_COMPAT_SKILL_DIRS = (
|
||||
(".claude", "skills"),
|
||||
(".agents", "skills"),
|
||||
)
|
||||
_DEFAULT_PROJECT_SKILL_DIRS = (".openharness/skills", ".agents/skills", ".claude/skills")
|
||||
|
||||
|
||||
def get_user_skills_dir() -> Path:
|
||||
"""Return the user skills directory."""
|
||||
"""Return the OpenHarness user skills directory."""
|
||||
path = get_config_dir() / "skills"
|
||||
path.mkdir(parents=True, exist_ok=True)
|
||||
return path
|
||||
|
||||
|
||||
def get_user_skill_dirs() -> list[Path]:
|
||||
"""Return user-level skill directories loaded by default."""
|
||||
return [get_user_skills_dir(), *(Path.home().joinpath(*parts) for parts in _USER_COMPAT_SKILL_DIRS)]
|
||||
|
||||
|
||||
def load_skill_registry(
|
||||
cwd: str | Path | None = None,
|
||||
*,
|
||||
@@ -35,18 +46,27 @@ def load_skill_registry(
|
||||
extra_plugin_roots: Iterable[str | Path] | None = None,
|
||||
settings=None,
|
||||
) -> SkillRegistry:
|
||||
"""Load bundled and user-defined skills."""
|
||||
"""Load bundled, user-defined, project, and plugin skills."""
|
||||
registry = SkillRegistry()
|
||||
for skill in get_bundled_skills():
|
||||
registry.register(skill)
|
||||
for skill in load_user_skills():
|
||||
registry.register(skill)
|
||||
for skill in load_skills_from_dirs(extra_skill_dirs):
|
||||
for skill in load_skills_from_dirs(extra_skill_dirs, source="user"):
|
||||
registry.register(skill)
|
||||
|
||||
resolved_settings = settings or load_settings()
|
||||
if cwd is not None and getattr(resolved_settings, "allow_project_skills", True):
|
||||
project_dirs = discover_project_skill_dirs(
|
||||
cwd,
|
||||
getattr(resolved_settings, "project_skill_dirs", list(_DEFAULT_PROJECT_SKILL_DIRS)),
|
||||
)
|
||||
for skill in load_skills_from_dirs(project_dirs, source="project", create_missing=False):
|
||||
registry.register(skill)
|
||||
|
||||
if cwd is not None:
|
||||
from openharness.plugins.loader import load_plugins
|
||||
|
||||
resolved_settings = settings or load_settings()
|
||||
for plugin in load_plugins(resolved_settings, cwd, extra_roots=extra_plugin_roots):
|
||||
if not plugin.enabled:
|
||||
continue
|
||||
@@ -56,14 +76,85 @@ def load_skill_registry(
|
||||
|
||||
|
||||
def load_user_skills() -> list[SkillDefinition]:
|
||||
"""Load markdown skills from the user config directory."""
|
||||
return load_skills_from_dirs([get_user_skills_dir()], source="user")
|
||||
"""Load markdown skills from user-level OpenHarness and compatibility directories."""
|
||||
return load_skills_from_dirs(get_user_skill_dirs(), source="user")
|
||||
|
||||
|
||||
def discover_project_skill_dirs(
|
||||
cwd: str | Path,
|
||||
project_skill_dirs: Iterable[str] | None = None,
|
||||
) -> list[Path]:
|
||||
"""Return existing project skill directories from cwd up to the git root.
|
||||
|
||||
Directories are ordered from least-specific to most-specific so later registry
|
||||
entries can override broader project or user skills deterministically.
|
||||
"""
|
||||
start = Path(cwd).expanduser().resolve()
|
||||
if not start.exists():
|
||||
start = start.parent
|
||||
if start.is_file():
|
||||
start = start.parent
|
||||
|
||||
relative_dirs = _valid_project_skill_dirs(project_skill_dirs or _DEFAULT_PROJECT_SKILL_DIRS)
|
||||
git_root = _find_git_root(start)
|
||||
home = Path.home().resolve()
|
||||
current = start
|
||||
levels: list[Path] = []
|
||||
while True:
|
||||
levels.append(current)
|
||||
if git_root is not None and current == git_root:
|
||||
break
|
||||
if git_root is None and current == home:
|
||||
break
|
||||
parent = current.parent
|
||||
if parent == current:
|
||||
break
|
||||
current = parent
|
||||
|
||||
roots: list[Path] = []
|
||||
seen: set[Path] = set()
|
||||
for base in reversed(levels):
|
||||
for rel in relative_dirs:
|
||||
candidate = (base / rel).resolve()
|
||||
if candidate in seen or not candidate.is_dir():
|
||||
continue
|
||||
seen.add(candidate)
|
||||
roots.append(candidate)
|
||||
return roots
|
||||
|
||||
|
||||
def _valid_project_skill_dirs(project_skill_dirs: Iterable[str]) -> list[Path]:
|
||||
"""Return safe relative project skill paths."""
|
||||
paths: list[Path] = []
|
||||
for raw in project_skill_dirs:
|
||||
value = str(raw).strip()
|
||||
if not value:
|
||||
continue
|
||||
rel = Path(value)
|
||||
if rel.is_absolute() or ".." in rel.parts:
|
||||
logger.warning("Ignoring unsafe project skill dir: %s", raw)
|
||||
continue
|
||||
paths.append(rel)
|
||||
return paths
|
||||
|
||||
|
||||
def _find_git_root(start: Path) -> Path | None:
|
||||
"""Find the nearest git root containing start, if any."""
|
||||
current = start
|
||||
while True:
|
||||
if (current / ".git").exists():
|
||||
return current
|
||||
parent = current.parent
|
||||
if parent == current:
|
||||
return None
|
||||
current = parent
|
||||
|
||||
|
||||
def load_skills_from_dirs(
|
||||
directories: Iterable[str | Path] | None,
|
||||
*,
|
||||
source: str = "user",
|
||||
create_missing: bool = True,
|
||||
) -> list[SkillDefinition]:
|
||||
"""Load markdown skills from one or more directories.
|
||||
|
||||
@@ -76,7 +167,10 @@ def load_skills_from_dirs(
|
||||
seen: set[Path] = set()
|
||||
for directory in directories:
|
||||
root = Path(directory).expanduser().resolve()
|
||||
root.mkdir(parents=True, exist_ok=True)
|
||||
if create_missing:
|
||||
root.mkdir(parents=True, exist_ok=True)
|
||||
elif not root.is_dir():
|
||||
continue
|
||||
candidates: list[Path] = []
|
||||
for child in sorted(root.iterdir()):
|
||||
if child.is_dir():
|
||||
|
||||
@@ -18,7 +18,7 @@ class SkillTool(BaseTool):
|
||||
"""Return the content of a loaded skill."""
|
||||
|
||||
name = "skill"
|
||||
description = "Read a bundled, user, or plugin skill by name."
|
||||
description = "Read a bundled, user, project, or plugin skill by name."
|
||||
input_model = SkillToolInput
|
||||
|
||||
def is_read_only(self, arguments: SkillToolInput) -> bool:
|
||||
|
||||
@@ -697,6 +697,51 @@ async def test_user_invocable_false_skill_is_not_slash_resolved(tmp_path: Path,
|
||||
assert lookup_skill_slash_command("/hidden", context) is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_project_skill_registers_as_context_slash_command(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.setenv("OPENHARNESS_CONFIG_DIR", str(tmp_path / "config"))
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
skill_dir = repo / ".claude" / "skills" / "shipit"
|
||||
skill_dir.mkdir(parents=True)
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"---\n"
|
||||
"description: Project ship workflow.\n"
|
||||
"---\n\n"
|
||||
"# Shipit\n\nShip this repo.\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
context = _make_context(repo)
|
||||
|
||||
parsed = lookup_skill_slash_command("/shipit now", context)
|
||||
assert parsed is not None
|
||||
command, args = parsed
|
||||
result = await command.handler(args, context)
|
||||
|
||||
assert command.name == "shipit"
|
||||
assert result.submit_prompt is not None
|
||||
assert f"Base directory for this skill: {skill_dir.resolve()}" in result.submit_prompt
|
||||
assert "Arguments: now" in result.submit_prompt
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_skills_command_lists_project_skill_path(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.setenv("OPENHARNESS_CONFIG_DIR", str(tmp_path / "config"))
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
skill_dir = repo / ".agents" / "skills" / "triage"
|
||||
skill_dir.mkdir(parents=True)
|
||||
(skill_dir / "SKILL.md").write_text("# Triage\nTriage workflow.\n", encoding="utf-8")
|
||||
registry = create_default_command_registry()
|
||||
command, args = registry.lookup("/skills")
|
||||
assert command is not None
|
||||
|
||||
result = await command.handler(args, _make_context(repo))
|
||||
|
||||
assert "triage (Triage) [project]" in result.message
|
||||
assert str((skill_dir / "SKILL.md").resolve()) in result.message
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_disable_model_invocation_skill_still_allows_user_slash(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.setenv("OPENHARNESS_CONFIG_DIR", str(tmp_path / "config"))
|
||||
|
||||
@@ -5,7 +5,9 @@ from __future__ import annotations
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
|
||||
from openharness.config.settings import Settings
|
||||
from openharness.skills import get_user_skills_dir, load_skill_registry
|
||||
from openharness.skills.loader import discover_project_skill_dirs, get_user_skill_dirs
|
||||
from openharness.skills.bundled import _parse_frontmatter as parse_bundled_frontmatter
|
||||
from openharness.skills.loader import _parse_skill_markdown as parse_skill_markdown
|
||||
|
||||
@@ -25,6 +27,14 @@ def test_load_skill_registry_includes_bundled(tmp_path: Path, monkeypatch):
|
||||
assert "Create, improve, and verify OpenHarness skills" in skill_creator.description
|
||||
|
||||
|
||||
def _write_skill(root: Path, name: str, body: str | None = None) -> Path:
|
||||
skill_dir = root / name
|
||||
skill_dir.mkdir(parents=True, exist_ok=True)
|
||||
skill_file = skill_dir / "SKILL.md"
|
||||
skill_file.write_text(body or f"# {name}\n{name} guidance\n", encoding="utf-8")
|
||||
return skill_file
|
||||
|
||||
|
||||
def test_load_skill_registry_includes_user_skills(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.setenv("OPENHARNESS_CONFIG_DIR", str(tmp_path / "config"))
|
||||
skills_dir = get_user_skills_dir()
|
||||
@@ -40,6 +50,33 @@ def test_load_skill_registry_includes_user_skills(tmp_path: Path, monkeypatch):
|
||||
assert "Deployment workflow guidance" in deploy.content
|
||||
|
||||
|
||||
def test_load_skill_registry_includes_user_compat_skill_dirs(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.setenv("OPENHARNESS_CONFIG_DIR", str(tmp_path / "config"))
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path / "home")
|
||||
claude_skill = _write_skill(tmp_path / "home" / ".claude" / "skills", "claude-review")
|
||||
agents_skill = _write_skill(tmp_path / "home" / ".agents" / "skills", "agents-plan")
|
||||
|
||||
registry = load_skill_registry()
|
||||
|
||||
assert registry.get("claude-review") is not None
|
||||
assert registry.get("agents-plan") is not None
|
||||
assert registry.get("claude-review").source == "user" # type: ignore[union-attr]
|
||||
assert registry.get("agents-plan").source == "user" # type: ignore[union-attr]
|
||||
assert str(claude_skill) in (registry.get("claude-review").path or "") # type: ignore[union-attr]
|
||||
assert str(agents_skill) in (registry.get("agents-plan").path or "") # type: ignore[union-attr]
|
||||
|
||||
|
||||
def test_get_user_skill_dirs_includes_openharness_claude_and_agents(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.setenv("OPENHARNESS_CONFIG_DIR", str(tmp_path / "config"))
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path / "home")
|
||||
|
||||
dirs = get_user_skill_dirs()
|
||||
|
||||
assert tmp_path / "config" / "skills" in dirs
|
||||
assert tmp_path / "home" / ".claude" / "skills" in dirs
|
||||
assert tmp_path / "home" / ".agents" / "skills" in dirs
|
||||
|
||||
|
||||
def test_user_skill_metadata_tracks_command_name_and_frontmatter_flags(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.setenv("OPENHARNESS_CONFIG_DIR", str(tmp_path / "config"))
|
||||
skills_dir = get_user_skills_dir()
|
||||
@@ -76,6 +113,85 @@ def test_user_skill_metadata_tracks_command_name_and_frontmatter_flags(tmp_path:
|
||||
assert by_command.argument_hint == "ENV"
|
||||
|
||||
|
||||
def test_project_skills_load_by_default_from_supported_dirs(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.setenv("OPENHARNESS_CONFIG_DIR", str(tmp_path / "config"))
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path / "home")
|
||||
repo = tmp_path / "repo"
|
||||
(repo / ".git").mkdir(parents=True)
|
||||
_write_skill(repo / ".openharness" / "skills", "oh-project")
|
||||
_write_skill(repo / ".agents" / "skills", "agents-project")
|
||||
_write_skill(repo / ".claude" / "skills", "claude-project")
|
||||
|
||||
registry = load_skill_registry(repo, settings=Settings())
|
||||
|
||||
assert registry.get("oh-project").source == "project" # type: ignore[union-attr]
|
||||
assert registry.get("agents-project").source == "project" # type: ignore[union-attr]
|
||||
assert registry.get("claude-project").source == "project" # type: ignore[union-attr]
|
||||
|
||||
|
||||
def test_project_skills_can_be_disabled(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.setenv("OPENHARNESS_CONFIG_DIR", str(tmp_path / "config"))
|
||||
repo = tmp_path / "repo"
|
||||
(repo / ".git").mkdir(parents=True)
|
||||
_write_skill(repo / ".claude" / "skills", "project-only")
|
||||
|
||||
registry = load_skill_registry(repo, settings=Settings(allow_project_skills=False))
|
||||
|
||||
assert registry.get("project-only") is None
|
||||
|
||||
|
||||
def test_project_skill_discovery_walks_up_to_git_root(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path / "home")
|
||||
repo = tmp_path / "repo"
|
||||
cwd = repo / "packages" / "api" / "src"
|
||||
cwd.mkdir(parents=True)
|
||||
(repo / ".git").mkdir()
|
||||
root_skill_dir = repo / ".claude" / "skills"
|
||||
package_skill_dir = repo / "packages" / ".agents" / "skills"
|
||||
outside_skill_dir = tmp_path / ".claude" / "skills"
|
||||
root_skill_dir.mkdir(parents=True)
|
||||
package_skill_dir.mkdir(parents=True)
|
||||
outside_skill_dir.mkdir(parents=True)
|
||||
|
||||
dirs = discover_project_skill_dirs(cwd)
|
||||
|
||||
assert root_skill_dir.resolve() in dirs
|
||||
assert package_skill_dir.resolve() in dirs
|
||||
assert outside_skill_dir.resolve() not in dirs
|
||||
assert dirs.index(root_skill_dir.resolve()) < dirs.index(package_skill_dir.resolve())
|
||||
|
||||
|
||||
def test_project_skill_nearer_cwd_overrides_parent_and_user(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.setenv("OPENHARNESS_CONFIG_DIR", str(tmp_path / "config"))
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path / "home")
|
||||
_write_skill(tmp_path / "home" / ".claude" / "skills", "deploy", "# user deploy\nuser version\n")
|
||||
repo = tmp_path / "repo"
|
||||
cwd = repo / "services" / "api"
|
||||
cwd.mkdir(parents=True)
|
||||
(repo / ".git").mkdir()
|
||||
_write_skill(repo / ".claude" / "skills", "deploy", "# root deploy\nroot version\n")
|
||||
_write_skill(cwd / ".claude" / "skills", "deploy", "# api deploy\napi version\n")
|
||||
|
||||
registry = load_skill_registry(cwd, settings=Settings())
|
||||
skill = registry.get("deploy")
|
||||
|
||||
assert skill is not None
|
||||
assert skill.source == "project"
|
||||
assert "api version" in skill.content
|
||||
|
||||
|
||||
def test_unsafe_project_skill_dirs_are_ignored(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path / "home")
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
escaped = tmp_path / "escaped" / "skills"
|
||||
escaped.mkdir(parents=True)
|
||||
|
||||
dirs = discover_project_skill_dirs(repo, ["../escaped/skills", str(escaped), ".claude/skills"])
|
||||
|
||||
assert escaped.resolve() not in dirs
|
||||
|
||||
|
||||
# --- parse_skill_markdown unit tests ---
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user