From 18fc6dea137ea9fab01ba8ab5b213a539e24ec9d Mon Sep 17 00:00:00 2001 From: tjb-tech <1193992557@qq.com> Date: Fri, 8 May 2026 03:49:02 +0000 Subject: [PATCH] feat(skills): discover Claude and project skills --- README.md | 26 +++++- src/openharness/commands/registry.py | 3 +- src/openharness/config/settings.py | 4 + src/openharness/skills/__init__.py | 20 ++++- src/openharness/skills/loader.py | 110 +++++++++++++++++++++++-- src/openharness/tools/skill_tool.py | 2 +- tests/test_commands/test_registry.py | 45 +++++++++++ tests/test_skills/test_loader.py | 116 +++++++++++++++++++++++++++ 8 files changed, 312 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index c2e80b6..3dfb5ad 100644 --- a/README.md +++ b/README.md @@ -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.md +~/.claude/skills//SKILL.md +~/.agents/skills//SKILL.md +``` + +Project-level skills are enabled by default and are discovered from the current working directory up to the git root: + +```text +/.openharness/skills//SKILL.md +/.agents/skills//SKILL.md +/.claude/skills//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 diff --git a/src/openharness/commands/registry.py b/src/openharness/commands/registry.py index d066196..67dc106 100644 --- a/src/openharness/commands/registry.py +++ b/src/openharness/commands/registry.py @@ -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: diff --git a/src/openharness/config/settings.py b/src/openharness/config/settings.py index 517867b..47b7284 100644 --- a/src/openharness/config/settings.py +++ b/src/openharness/config/settings.py @@ -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 diff --git a/src/openharness/skills/__init__.py b/src/openharness/skills/__init__.py index 8e36f0f..dd6665e 100644 --- a/src/openharness/skills/__init__.py +++ b/src/openharness/skills/__init__.py @@ -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] diff --git a/src/openharness/skills/loader.py b/src/openharness/skills/loader.py index d502b45..414b384 100644 --- a/src/openharness/skills/loader.py +++ b/src/openharness/skills/loader.py @@ -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(): diff --git a/src/openharness/tools/skill_tool.py b/src/openharness/tools/skill_tool.py index 6563e69..2d9c4dd 100644 --- a/src/openharness/tools/skill_tool.py +++ b/src/openharness/tools/skill_tool.py @@ -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: diff --git a/tests/test_commands/test_registry.py b/tests/test_commands/test_registry.py index 872ede6..da2e080 100644 --- a/tests/test_commands/test_registry.py +++ b/tests/test_commands/test_registry.py @@ -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")) diff --git a/tests/test_skills/test_loader.py b/tests/test_skills/test_loader.py index 856133a..e5ccaa9 100644 --- a/tests/test_skills/test_loader.py +++ b/tests/test_skills/test_loader.py @@ -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 ---