diff --git a/Formula/smith.rb b/Formula/smith.rb index 292c29b..6baf974 100644 --- a/Formula/smith.rb +++ b/Formula/smith.rb @@ -180,18 +180,18 @@ def install rm -rf "$target_dir" mkdir -p "$(dirname "$target_dir")" - cp -R "$source_dir" "$target_dir" - echo "Smith skill installed to: $target_dir" + ln -s "$source_dir" "$target_dir" + echo "Smith skill linked to: $target_dir" BASH chmod 0555, bin/"smith-install-skill" end def caveats <<~EOS - To install or refresh the Smith skill: + To link or refresh the Smith skill: smith-install-skill - By default it writes to ~/.agents/skills/smith. + By default it links to ~/.agents/skills/smith. To choose another destination: SMITH_SKILL_DIR=/path/to/skills/smith smith-install-skill EOS @@ -203,8 +203,9 @@ def caveats skill_dir = testpath/"skills/smith" with_env("SMITH_SKILL_DIR" => skill_dir.to_s) do - system bin/"smith-install-skill" + assert_match "Smith skill linked to:", shell_output(bin/"smith-install-skill") end + assert_predicate skill_dir, :symlink? assert_path_exists skill_dir/"SKILL.md" end end diff --git a/README.md b/README.md index cba5601..c3898b3 100644 --- a/README.md +++ b/README.md @@ -153,10 +153,10 @@ smith github-public code grep my-repo "TODO" --format json ```bash brew install faustodavid/tap/smith -smith-install-skill +smith config init ``` -Homebrew installs the `smith` CLI and `ripgrep`. `smith-install-skill` installs the canonical Smith skill into `~/.agents/skills/smith`. +Homebrew installs the `smith` CLI and `ripgrep`. `smith config init` creates your config file and links the canonical Smith skill into `~/.agents/skills/smith`. ### Install with the standalone script @@ -184,9 +184,10 @@ The installer keeps a managed Smith repo checkout at `~/.local/share/smith`, mir ```bash brew upgrade faustodavid/tap/smith -smith-install-skill ``` +The Homebrew skill link points at Homebrew's stable `opt/smith` path, so it stays current after `brew upgrade`. If you already have a config and only need to refresh the skill link, run `smith skill sync`. + If you installed with the standalone script instead, rerun `python3 ~/.local/share/smith/scripts/install.py`. The standalone installer runs `uv tool update-shell` for you, but you may need to **restart your shell** (or open a new terminal) for PATH changes to take effect - especially on Windows, where `uv` writes the update to the user PATH in the registry. @@ -342,7 +343,7 @@ Smith was built for AI agents from the ground up. `skills/smith/SKILL.md` is a s - **Failure recovery** — specific handlers for 401 / 403, 429, truncation, empty results, and wrong-repo misses. - **Answer contract** — evidence-first format with exact path citations and a `Sources` section. -The Homebrew formula and standalone installer both mirror the canonical skill into `~/.agents/skills/smith`. The standalone installer also keeps a managed repo checkout at `~/.local/share/smith`. +`smith config init` links the canonical skill into `~/.agents/skills/smith`. Homebrew installs use a symlink to Homebrew's stable `opt/smith` path, so the skill stays current after `brew upgrade`. --- @@ -481,7 +482,7 @@ Provider MCPs expose a `get_file_contents`-style surface that downloads entire f ### Does Smith work with Claude Code, Cursor, Windsurf, GitHub Copilot, and Codex? -Yes. Smith ships a structured skill document (`skills/smith/SKILL.md`) that any LLM-powered editor can load as a rule / instruction / skill. The Homebrew formula and standalone installer mirror the canonical skill into `~/.agents/skills/smith`. See [Use with your AI editor](#use-with-your-ai-editor) for per-editor hints. +Yes. Smith ships a structured skill document (`skills/smith/SKILL.md`) that any LLM-powered editor can load as a rule / instruction / skill. `smith config init` links it into `~/.agents/skills/smith`. See [Use with your AI editor](#use-with-your-ai-editor) for per-editor hints. ### Is Smith read-only? Can an agent accidentally push or comment? diff --git a/src/smith/cli/handlers.py b/src/smith/cli/handlers.py index 65e05e5..8450be5 100644 --- a/src/smith/cli/handlers.py +++ b/src/smith/cli/handlers.py @@ -9,6 +9,13 @@ from smith.client import SmithClient from smith.config import RemoteConfig, SmithConfig, _default_config_path, load_config, save_config from smith.formatting import dumps_json, make_envelope, render_text +from smith.skill import ( + SkillSyncResult, + default_skill_target_dir, + resolve_skill_source_dir, + skill_target_points_to_source, + sync_skill, +) EXIT_OK = 0 EXIT_INVALID_ARGS = 2 @@ -263,13 +270,21 @@ def handle_config_init(client: SmithClient | None, args: argparse.Namespace) -> exit_code=EXIT_INVALID_ARGS, ) + skill_result = sync_skill() + if args.output_format != "json": + stream = sys.stdout if skill_result.ok else sys.stderr + print(skill_result.message, file=stream) + if skill_result.ok and skill_result.mode == "symlink": + print("The skill will stay current when Smith is upgraded.", file=stream) + print(file=stream) + if args.output_format == "json": config = SmithConfig(remotes={}, defaults={}) save_config(config, config_path=path) return _emit_success( args=args, command=args.command_id, - data={"path": str(path), "remotes_count": 0}, + data={"path": str(path), "remotes_count": 0, "skill": skill_result.to_dict()}, partial=False, ) @@ -294,20 +309,24 @@ def handle_config_init(client: SmithClient | None, args: argparse.Namespace) -> raise SystemExit(1) if not raw or raw == "1": config = run_interactive_init(config_path=path) + if args.output_format != "json": + return EXIT_OK return _emit_success( args=args, command=args.command_id, - data={"path": str(path), "remotes_count": len(config.remotes)}, + data={"path": str(path), "remotes_count": len(config.remotes), "skill": skill_result.to_dict()}, partial=False, ) if raw == "2": config = SmithConfig(remotes={}, defaults={}) save_config(config, config_path=path) _print_manual_setup_instructions(path) + if args.output_format != "json": + return EXIT_OK return _emit_success( args=args, command=args.command_id, - data={"path": str(path), "remotes_count": 0}, + data={"path": str(path), "remotes_count": 0, "skill": skill_result.to_dict()}, partial=False, ) print(" Enter 1 or 2.") @@ -340,6 +359,57 @@ def handle_config_path(client: SmithClient | None, args: argparse.Namespace) -> ) +def _skill_status_data(result: SkillSyncResult | None = None) -> dict[str, object]: + target = default_skill_target_dir() + source = resolve_skill_source_dir() + target_exists = target.exists() or target.is_symlink() + points_to_source = False + mode: str | None = None + if target_exists: + mode = "symlink" if target.is_symlink() else "directory" + if source is not None: + points_to_source = skill_target_points_to_source(target, source) + data: dict[str, object] = { + "target": str(target), + "source": str(source) if source is not None else None, + "exists": target_exists, + "mode": mode, + "current": bool(target_exists and points_to_source), + } + if result is not None: + data["sync"] = result.to_dict() + return data + + +def handle_skill_sync(client: SmithClient | None, args: argparse.Namespace) -> int: + del client + result = sync_skill() + if not result.ok: + return _emit_error( + args=args, + command=args.command_id, + code=result.status, + message=result.message, + exit_code=EXIT_INVALID_ARGS, + ) + return _emit_success( + args=args, + command=args.command_id, + data=_skill_status_data(result), + partial=False, + ) + + +def handle_skill_status(client: SmithClient | None, args: argparse.Namespace) -> int: + del client + return _emit_success( + args=args, + command=args.command_id, + data=_skill_status_data(), + partial=False, + ) + + def handle_config_enable(client: SmithClient | None, args: argparse.Namespace) -> int: del client config = load_config() diff --git a/src/smith/cli/onboarding.py b/src/smith/cli/onboarding.py index 745b8ca..b028ccf 100644 --- a/src/smith/cli/onboarding.py +++ b/src/smith/cli/onboarding.py @@ -5,7 +5,7 @@ from pathlib import Path from smith.config import ( - _RESERVED_REMOTE_NAMES, + _NEW_REMOTE_RESERVED_NAMES, RemoteConfig, SmithConfig, _compute_api_url_for_remote, @@ -116,8 +116,8 @@ def _prompt_yes_no(prompt: str, *, default: bool = True) -> bool: def _validate_remote_name(name: str) -> str | None: - if name.lower() in _RESERVED_REMOTE_NAMES: - reserved = ", ".join(sorted(_RESERVED_REMOTE_NAMES)) + if name.lower() in _NEW_REMOTE_RESERVED_NAMES: + reserved = ", ".join(sorted(_NEW_REMOTE_RESERVED_NAMES)) return f"'{name}' is reserved. Avoid: {reserved}" if not name.replace("-", "").replace("_", "").isalnum(): return "Name must contain only letters, numbers, hyphens, and underscores." diff --git a/src/smith/cli/parser.py b/src/smith/cli/parser.py index 63ee31e..67c4d07 100644 --- a/src/smith/cli/parser.py +++ b/src/smith/cli/parser.py @@ -27,6 +27,8 @@ handle_pr_list, handle_pr_search, handle_pr_threads, + handle_skill_status, + handle_skill_sync, handle_work_get, handle_work_mine, handle_work_search, @@ -341,6 +343,31 @@ def _add_config_group(root_subparsers: Any) -> None: ) +def _add_skill_group(root_subparsers: Any) -> None: + skill = _add_parser(root_subparsers, "skill", help_text="Manage the Smith agent skill") + skill_sub = skill.add_subparsers(dest="skill_action", required=True) + + skill_sync = _add_parser(skill_sub, "sync", help_text="Install or refresh the Smith agent skill") + _add_output_format(skill_sync) + _set_handler( + skill_sync, + handle_skill_sync, + "skill.sync", + primary_path="skill sync", + requires_client=False, + ) + + skill_status = _add_parser(skill_sub, "status", help_text="Show Smith agent skill sync status") + _add_output_format(skill_status) + _set_handler( + skill_status, + handle_skill_status, + "skill.status", + primary_path="skill status", + requires_client=False, + ) + + def _add_cache_group(root_subparsers: Any, *, remotes: list[RemoteConfig]) -> None: cache = _add_parser(root_subparsers, "cache", help_text="Manage local grep caches") cache_sub = cache.add_subparsers(dest="cache_action", required=True) @@ -738,6 +765,7 @@ def build_parser(*, smith_config: SmithConfig | None = None) -> argparse.Argumen _add_global_code_group(root_subparsers) _add_global_prs_group(root_subparsers) _add_config_group(root_subparsers) + _add_skill_group(root_subparsers) _add_cache_group(root_subparsers, remotes=remotes) for remote in remotes: _add_remote_command_tree(root_subparsers, remote=remote) diff --git a/src/smith/config.py b/src/smith/config.py index 8710cca..fa225c4 100644 --- a/src/smith/config.py +++ b/src/smith/config.py @@ -254,6 +254,7 @@ def _compute_api_url_for_remote(provider: str, host: str) -> str: _RESERVED_REMOTE_NAMES = {"all", "cache", "code", "config", "help", "prs", "search"} +_NEW_REMOTE_RESERVED_NAMES = _RESERVED_REMOTE_NAMES | {"skill"} def _normalize_config_api_url(raw_api_url: Any) -> str: diff --git a/src/smith/formatting.py b/src/smith/formatting.py index eb06198..f06128d 100755 --- a/src/smith/formatting.py +++ b/src/smith/formatting.py @@ -707,6 +707,32 @@ def _render_config_show(data: Any) -> str: return "\n".join(lines) +def _render_config_init(data: Any) -> str: + if not isinstance(data, dict): + return "" + lines = [f"Config saved to {data.get('path')}"] + skill = data.get("skill") + if isinstance(skill, dict) and skill.get("message"): + lines.append(str(skill["message"])) + return "\n".join(lines) + + +def _render_skill_status(data: Any) -> str: + if not isinstance(data, dict): + return "" + lines = [ + f"target: {data.get('target')}", + f"source: {data.get('source')}", + f"exists: {str(bool(data.get('exists'))).lower()}", + f"mode: {data.get('mode') or '-'}", + f"current: {str(bool(data.get('current'))).lower()}", + ] + sync = data.get("sync") + if isinstance(sync, dict) and sync.get("message"): + lines.append(str(sync["message"])) + return "\n".join(lines) + + def _render_pipelines_list(data: Any) -> str: pipelines = data.get("pipelines", []) if isinstance(data, dict) else [] lines: list[str] = [] @@ -885,6 +911,9 @@ def _render_needs(needs: Any, *, name_to_id: dict[str, Any]) -> str: "stories.mine": _render_story_table, "config.list": _render_config_list, "config.show": _render_config_show, + "config.init": _render_config_init, + "skill.sync": _render_skill_status, + "skill.status": _render_skill_status, } diff --git a/src/smith/skill.py b/src/smith/skill.py new file mode 100644 index 0000000..308a06d --- /dev/null +++ b/src/smith/skill.py @@ -0,0 +1,216 @@ +from __future__ import annotations + +import filecmp +import os +import shutil +import sys +from dataclasses import dataclass +from pathlib import Path + +DEFAULT_SKILL_TARGET = Path.home() / ".agents" / "skills" / "smith" +_SKILL_RELATIVE_PATH = Path("share") / "smith" / "skills" / "smith" + + +@dataclass(frozen=True) +class SkillSyncResult: + ok: bool + status: str + target: Path + source: Path | None + mode: str | None + message: str + + def to_dict(self) -> dict[str, object]: + return { + "ok": self.ok, + "status": self.status, + "target": str(self.target), + "source": str(self.source) if self.source is not None else None, + "mode": self.mode, + "message": self.message, + } + + +def default_skill_target_dir() -> Path: + configured = os.getenv("SMITH_SKILL_DIR", "").strip() + if configured: + return Path(configured).expanduser() + return DEFAULT_SKILL_TARGET + + +def _path_from_env(name: str) -> Path | None: + value = os.getenv(name, "").strip() + if not value: + return None + return Path(value).expanduser() + + +def _homebrew_prefix_from_path(path: Path) -> Path | None: + parts = path.resolve().parts + for index, part in enumerate(parts): + if part != "Cellar": + continue + if index + 1 < len(parts) and parts[index + 1] == "smith": + return Path(*parts[:index]) + return None + + +def _homebrew_skill_sources() -> list[Path]: + prefixes: list[Path] = [] + configured_prefix = _path_from_env("SMITH_HOMEBREW_PREFIX") + if configured_prefix is not None: + prefixes.append(configured_prefix) + + for raw_path in [sys.argv[0], sys.executable, __file__]: + try: + prefix = _homebrew_prefix_from_path(Path(raw_path)) + except (OSError, RuntimeError): + continue + if prefix is not None and prefix not in prefixes: + prefixes.append(prefix) + + candidates: list[Path] = [] + for prefix in prefixes: + candidates.append(prefix / "opt" / "smith" / _SKILL_RELATIVE_PATH) + candidates.append(prefix / "Cellar" / "smith" / "HEAD" / _SKILL_RELATIVE_PATH) + return candidates + + +def _repo_skill_source() -> Path: + return Path(__file__).resolve().parents[2] / "skills" / "smith" + + +def resolve_skill_source_dir() -> Path | None: + configured = _path_from_env("SMITH_SKILL_SOURCE_DIR") + if configured is not None: + return configured if configured.exists() else None + + for candidate in [*_homebrew_skill_sources(), _repo_skill_source()]: + if candidate.exists(): + return candidate + return None + + +def _remove_existing_target(target: Path) -> None: + if target.is_symlink() or target.is_file(): + target.unlink() + elif target.exists(): + shutil.rmtree(target) + + +def _absolute_path_without_resolving(path: Path) -> Path: + return Path(os.path.abspath(path.expanduser())) + + +def _symlink_destination(target: Path) -> Path | None: + try: + destination = Path(os.readlink(target)) + except OSError: + return None + if destination.is_absolute(): + return _absolute_path_without_resolving(destination) + return _absolute_path_without_resolving(target.parent / destination) + + +def _directory_contents_match(target: Path, source: Path) -> bool: + if not target.is_dir() or not source.is_dir(): + return False + + target_entries = {path.relative_to(target) for path in target.rglob("*")} + source_entries = {path.relative_to(source) for path in source.rglob("*")} + if target_entries != source_entries: + return False + + for relative_path in target_entries: + target_path = target / relative_path + source_path = source / relative_path + if target_path.is_symlink() or source_path.is_symlink(): + if not target_path.is_symlink() or not source_path.is_symlink(): + return False + if os.readlink(target_path) != os.readlink(source_path): + return False + elif target_path.is_dir() or source_path.is_dir(): + if not target_path.is_dir() or not source_path.is_dir(): + return False + elif target_path.is_file() or source_path.is_file(): + if not target_path.is_file() or not source_path.is_file(): + return False + if not filecmp.cmp(target_path, source_path, shallow=False): + return False + else: + return False + return True + + +def skill_target_points_to_source(target: Path, source: Path) -> bool: + source = _absolute_path_without_resolving(source) + try: + if target.is_symlink(): + return _symlink_destination(target) == source + if not target.exists(): + return False + return target.resolve() == source.resolve() or _directory_contents_match(target, source) + except OSError: + return False + + +def sync_skill( + *, + source_dir: Path | None = None, + target_dir: Path | None = None, + prefer_symlink: bool = True, +) -> SkillSyncResult: + target = target_dir or default_skill_target_dir() + source = source_dir or resolve_skill_source_dir() + + if source is None or not source.exists(): + return SkillSyncResult( + ok=False, + status="missing_source", + target=target, + source=source, + mode=None, + message="Smith skill source not found.", + ) + + source = _absolute_path_without_resolving(source) + target_parent = target.parent + + if target.exists() or target.is_symlink(): + if skill_target_points_to_source(target, source): + mode = "symlink" if target.is_symlink() else "directory" + return SkillSyncResult( + ok=True, + status="current", + target=target, + source=source, + mode=mode, + message=f"Smith skill already points to: {source}", + ) + _remove_existing_target(target) + + target_parent.mkdir(parents=True, exist_ok=True) + + if prefer_symlink: + try: + target.symlink_to(source, target_is_directory=True) + return SkillSyncResult( + ok=True, + status="linked", + target=target, + source=source, + mode="symlink", + message=f"Smith skill linked to: {target}", + ) + except OSError: + pass + + shutil.copytree(source, target) + return SkillSyncResult( + ok=True, + status="copied", + target=target, + source=source, + mode="copy", + message=f"Smith skill copied to: {target}", + ) diff --git a/tests/contract/test_cli_contracts.py b/tests/contract/test_cli_contracts.py index 6bfe40a..fbd52ec 100644 --- a/tests/contract/test_cli_contracts.py +++ b/tests/contract/test_cli_contracts.py @@ -2,6 +2,7 @@ import json from argparse import Namespace +from pathlib import Path from types import SimpleNamespace from typing import Any @@ -9,6 +10,7 @@ from smith.cli import handlers from smith.config import RemoteConfig, SmithConfig +from smith.skill import SkillSyncResult def _make_args(**overrides: Any) -> Namespace: @@ -118,6 +120,19 @@ def _make_remote_config( ) +def _make_homebrew_opt_skill(tmp_path: Path) -> tuple[Path, Path]: + cellar_prefix = tmp_path / "Cellar" / "smith" / "1.0" + cellar_source = cellar_prefix / "share" / "smith" / "skills" / "smith" + cellar_source.mkdir(parents=True) + (cellar_source / "SKILL.md").write_text("---\nname: smith\n---\n", encoding="utf-8") + + opt_dir = tmp_path / "opt" + opt_dir.mkdir() + (opt_dir / "smith").symlink_to(cellar_prefix, target_is_directory=True) + opt_source = opt_dir / "smith" / "share" / "smith" / "skills" / "smith" + return opt_source, cellar_source + + def test_csv_list_and_remote_helpers() -> None: args = _make_args(remote="github", remote_provider="github") @@ -531,33 +546,115 @@ def test_handle_config_show_returns_not_found_error(monkeypatch: Any, capsys: An def test_handle_config_init_creates_empty_config_file(monkeypatch: Any, tmp_path: Any, capsys: Any) -> None: path = tmp_path / "smith-config.yaml" + skill_target = tmp_path / "skills" / "smith" + skill_source = tmp_path / "source" / "skills" / "smith" + skill_result = SkillSyncResult( + ok=True, + status="linked", + target=skill_target, + source=skill_source, + mode="symlink", + message=f"Smith skill linked to: {skill_target}", + ) monkeypatch.setattr(handlers, "_default_config_path", lambda: path) + monkeypatch.setattr(handlers, "sync_skill", lambda: skill_result) args = _make_args(command_id="config.init", output_format="json") exit_code = handlers.handle_config_init(None, args) payload = json.loads(capsys.readouterr().out) assert exit_code == handlers.EXIT_OK - assert payload["data"] == {"path": str(path), "remotes_count": 0} + assert payload["data"] == { + "path": str(path), + "remotes_count": 0, + "skill": skill_result.to_dict(), + } assert handlers.load_config(config_path=path) == SmithConfig(remotes={}, defaults={}) def test_handle_config_init_rejects_existing_config_file(monkeypatch: Any, tmp_path: Any, capsys: Any) -> None: path = tmp_path / "smith-config.yaml" path.write_text("remotes: {}\ndefaults: {}\n", encoding="utf-8") + calls = 0 + + def _sync_skill() -> SkillSyncResult: + nonlocal calls + calls += 1 + return SkillSyncResult( + ok=True, + status="linked", + target=Path("/tmp/smith-skill"), + source=Path("/tmp/smith-source"), + mode="symlink", + message="Smith skill linked to: /tmp/smith-skill", + ) + monkeypatch.setattr(handlers, "_default_config_path", lambda: path) + monkeypatch.setattr(handlers, "sync_skill", _sync_skill) args = _make_args(command_id="config.init", output_format="json") exit_code = handlers.handle_config_init(None, args) payload = json.loads(capsys.readouterr().out) assert exit_code == handlers.EXIT_INVALID_ARGS + assert calls == 0 assert payload["error"] == { "code": "already_exists", "message": f"Config file already exists at {path}", } +def test_handle_skill_sync_reports_current_link(monkeypatch: Any, tmp_path: Any, capsys: Any) -> None: + target = tmp_path / ".agents" / "skills" / "smith" + source = tmp_path / "opt" / "smith" / "share" / "smith" / "skills" / "smith" + source.mkdir(parents=True) + target.parent.mkdir(parents=True) + target.symlink_to(source, target_is_directory=True) + result = SkillSyncResult( + ok=True, + status="current", + target=target, + source=source, + mode="symlink", + message=f"Smith skill already points to: {source}", + ) + monkeypatch.setattr(handlers, "sync_skill", lambda: result) + monkeypatch.setattr(handlers, "default_skill_target_dir", lambda: target) + monkeypatch.setattr(handlers, "resolve_skill_source_dir", lambda: source) + args = _make_args(command_id="skill.sync", output_format="json") + + exit_code = handlers.handle_skill_sync(None, args) + payload = json.loads(capsys.readouterr().out) + + assert exit_code == handlers.EXIT_OK + assert payload["data"]["current"] is True + assert payload["data"]["mode"] == "symlink" + assert payload["data"]["sync"] == result.to_dict() + + +def test_handle_skill_status_reports_cellar_link_stale( + monkeypatch: Any, + tmp_path: Any, + capsys: Any, +) -> None: + opt_source, cellar_source = _make_homebrew_opt_skill(tmp_path) + target = tmp_path / ".agents" / "skills" / "smith" + target.parent.mkdir(parents=True) + target.symlink_to(cellar_source, target_is_directory=True) + monkeypatch.setattr(handlers, "default_skill_target_dir", lambda: target) + monkeypatch.setattr(handlers, "resolve_skill_source_dir", lambda: opt_source) + args = _make_args(command_id="skill.status", output_format="json") + + exit_code = handlers.handle_skill_status(None, args) + payload = json.loads(capsys.readouterr().out) + + assert exit_code == handlers.EXIT_OK + assert payload["data"]["exists"] is True + assert payload["data"]["mode"] == "symlink" + assert payload["data"]["source"] == str(opt_source) + assert payload["data"]["current"] is False + + @pytest.mark.parametrize("exists", [False, True]) def test_handle_config_path_reports_default_path_and_existence( monkeypatch: Any, diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 8e6de4b..85e7057 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -591,6 +591,26 @@ def test_load_config_rejects_reserved_top_level_remote_names(tmp_path: Path, rem load_config(config_path=config_path) +def test_load_config_allows_legacy_skill_remote_name(tmp_path: Path) -> None: + config_path = tmp_path / "config.yaml" + config_path.write_text( + """ +remotes: + skill: + provider: github + org: octo-org + host: github.com +defaults: {} +""", + encoding="utf-8", + ) + + config = load_config(config_path=config_path) + + assert config.remotes["skill"].name == "skill" + assert config.remotes["skill"].provider == "github" + + def test_load_config_preserves_explicit_github_api_url_override(tmp_path: Path) -> None: config_path = tmp_path / "config.yaml" config_path.write_text( diff --git a/tests/unit/test_onboarding.py b/tests/unit/test_onboarding.py index a131cf4..6fae89a 100644 --- a/tests/unit/test_onboarding.py +++ b/tests/unit/test_onboarding.py @@ -24,6 +24,7 @@ def test_reserved_name_rejected(self) -> None: assert _validate_remote_name("all") is not None assert _validate_remote_name("cache") is not None assert _validate_remote_name("config") is not None + assert _validate_remote_name("skill") is not None def test_valid_name_accepted(self) -> None: assert _validate_remote_name("my-github") is None diff --git a/tests/unit/test_skill.py b/tests/unit/test_skill.py new file mode 100644 index 0000000..7bdf0a9 --- /dev/null +++ b/tests/unit/test_skill.py @@ -0,0 +1,142 @@ +from __future__ import annotations + +import os +from pathlib import Path + +from smith import skill + + +def _make_skill_source(tmp_path: Path) -> Path: + source = tmp_path / "source" / "skills" / "smith" + source.mkdir(parents=True) + (source / "SKILL.md").write_text("---\nname: smith\n---\n", encoding="utf-8") + return source + + +def _make_homebrew_opt_skill(tmp_path: Path) -> tuple[Path, Path]: + cellar_prefix = tmp_path / "Cellar" / "smith" / "1.0" + cellar_source = cellar_prefix / "share" / "smith" / "skills" / "smith" + cellar_source.mkdir(parents=True) + (cellar_source / "SKILL.md").write_text("---\nname: smith\n---\n", encoding="utf-8") + + opt_dir = tmp_path / "opt" + opt_dir.mkdir() + (opt_dir / "smith").symlink_to(cellar_prefix, target_is_directory=True) + opt_source = opt_dir / "smith" / "share" / "smith" / "skills" / "smith" + return opt_source, cellar_source + + +def test_sync_skill_links_target_to_source(tmp_path: Path) -> None: + source = _make_skill_source(tmp_path) + target = tmp_path / ".agents" / "skills" / "smith" + + result = skill.sync_skill(source_dir=source, target_dir=target) + + assert result.ok is True + assert result.status == "linked" + assert result.mode == "symlink" + assert target.is_symlink() + assert target.resolve() == source.resolve() + + +def test_sync_skill_replaces_stale_directory_with_symlink(tmp_path: Path) -> None: + source = _make_skill_source(tmp_path) + target = tmp_path / ".agents" / "skills" / "smith" + target.mkdir(parents=True) + (target / "old.md").write_text("old", encoding="utf-8") + + result = skill.sync_skill(source_dir=source, target_dir=target) + + assert result.ok is True + assert result.status == "linked" + assert target.is_symlink() + assert target.resolve() == source.resolve() + assert not (target / "old.md").exists() + + +def test_sync_skill_reports_current_when_copied_target_matches_source(tmp_path: Path) -> None: + source = _make_skill_source(tmp_path) + target = tmp_path / ".agents" / "skills" / "smith" + + copied = skill.sync_skill(source_dir=source, target_dir=target, prefer_symlink=False) + current = skill.sync_skill(source_dir=source, target_dir=target, prefer_symlink=False) + + assert copied.ok is True + assert copied.status == "copied" + assert current.ok is True + assert current.status == "current" + assert current.mode == "directory" + assert skill.skill_target_points_to_source(target, source) is True + + +def test_sync_skill_refreshes_stale_copied_target(tmp_path: Path) -> None: + source = _make_skill_source(tmp_path) + target = tmp_path / ".agents" / "skills" / "smith" + skill.sync_skill(source_dir=source, target_dir=target, prefer_symlink=False) + (source / "SKILL.md").write_text("---\nname: smith\nupdated: true\n---\n", encoding="utf-8") + + result = skill.sync_skill(source_dir=source, target_dir=target, prefer_symlink=False) + + assert result.ok is True + assert result.status == "copied" + assert (target / "SKILL.md").read_text(encoding="utf-8") == (source / "SKILL.md").read_text(encoding="utf-8") + + +def test_sync_skill_reports_current_when_target_already_points_to_source(tmp_path: Path) -> None: + source = _make_skill_source(tmp_path) + target = tmp_path / ".agents" / "skills" / "smith" + target.parent.mkdir(parents=True) + target.symlink_to(source, target_is_directory=True) + + result = skill.sync_skill(source_dir=source, target_dir=target) + + assert result.ok is True + assert result.status == "current" + assert result.mode == "symlink" + + +def test_sync_skill_preserves_homebrew_opt_symlink_target(tmp_path: Path) -> None: + opt_source, cellar_source = _make_homebrew_opt_skill(tmp_path) + target = tmp_path / ".agents" / "skills" / "smith" + + result = skill.sync_skill(source_dir=opt_source, target_dir=target) + + assert result.ok is True + assert result.status == "linked" + assert result.source == opt_source + assert target.is_symlink() + assert Path(os.readlink(target)) == opt_source + assert target.resolve() == cellar_source.resolve() + + +def test_sync_skill_replaces_cellar_link_with_opt_link(tmp_path: Path) -> None: + opt_source, cellar_source = _make_homebrew_opt_skill(tmp_path) + target = tmp_path / ".agents" / "skills" / "smith" + target.parent.mkdir(parents=True) + target.symlink_to(cellar_source, target_is_directory=True) + + result = skill.sync_skill(source_dir=opt_source, target_dir=target) + + assert result.ok is True + assert result.status == "linked" + assert result.source == opt_source + assert target.is_symlink() + assert Path(os.readlink(target)) == opt_source + assert target.resolve() == cellar_source.resolve() + + +def test_sync_skill_reports_missing_source(tmp_path: Path) -> None: + result = skill.sync_skill(source_dir=tmp_path / "missing", target_dir=tmp_path / "target") + + assert result.ok is False + assert result.status == "missing_source" + + +def test_resolve_skill_source_prefers_homebrew_opt_path(monkeypatch, tmp_path: Path) -> None: + source = tmp_path / "opt" / "smith" / "share" / "smith" / "skills" / "smith" + source.mkdir(parents=True) + (source / "SKILL.md").write_text("---\nname: smith\n---\n", encoding="utf-8") + monkeypatch.setenv("SMITH_HOMEBREW_PREFIX", str(tmp_path)) + monkeypatch.delenv("SMITH_SKILL_SOURCE_DIR", raising=False) + + assert skill.resolve_skill_source_dir() == source