diff --git a/.github/workflows/homebrew-formula.yml b/.github/workflows/homebrew-formula.yml index 3b12598..44a68cc 100644 --- a/.github/workflows/homebrew-formula.yml +++ b/.github/workflows/homebrew-formula.yml @@ -86,11 +86,21 @@ jobs: --tag "${RELEASE_TAG}" \ --check-release-tag - - name: Report tap workflow handoff + - name: Trigger tap formula update env: RELEASE_TAG: ${{ steps.release.outputs.tag }} RELEASE_REVISION: ${{ steps.metadata.outputs.revision }} + GH_TOKEN: ${{ secrets.HOMEBREW_TAP_DISPATCH_TOKEN }} run: | set -euo pipefail echo "Release ${RELEASE_TAG}@${RELEASE_REVISION} is valid." - echo "Update faustodavid/homebrew-tap with its Update Smith Formula workflow." + if [[ -z "${GH_TOKEN}" ]]; then + echo "HOMEBREW_TAP_DISPATCH_TOKEN is not configured." + echo "Update faustodavid/homebrew-tap manually with its Update Smith Formula workflow." + exit 0 + fi + gh workflow run update-smith-formula.yml \ + --repo faustodavid/homebrew-tap \ + -f tag="${RELEASE_TAG}" \ + -f revision="${RELEASE_REVISION}" + echo "Dispatched faustodavid/homebrew-tap Update Smith Formula workflow for ${RELEASE_TAG}." diff --git a/README.md b/README.md index 41e50e3..e1acfc3 100644 --- a/README.md +++ b/README.md @@ -73,8 +73,9 @@ irm https://raw.githubusercontent.com/faustodavid/smith/main/scripts/install.py ``` `smith config init` syncs the Smith agent skill to `~/.agents/skills/smith`. -The standalone installer does this during install too. Refresh the skill later -with: +The standalone installer does this during install too. The skill stays current +on its own after upgrades (set `SMITH_SKILL_CHECK=0` to opt out); to refresh it +manually run: ```bash smith skill sync diff --git a/pyproject.toml b/pyproject.toml index df7793d..c90a684 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "smith" -version = "0.1.2" +version = "0.1.3" description = "Read-only Azure DevOps and GitHub investigation CLI" readme = "README.md" requires-python = ">=3.12" diff --git a/scripts/install.py b/scripts/install.py index 4a09157..c92ea11 100644 --- a/scripts/install.py +++ b/scripts/install.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 """Cross-platform installer for Smith. Works on macOS and Windows.""" +import os import shutil import subprocess import sys @@ -28,8 +29,37 @@ def require_tool(name: str, install_hint: str) -> None: sys.exit(1) +def find_smith_executable() -> str | None: + """Locate the smith CLI, including fresh installs where uv's bin dir is not on PATH yet.""" + found = shutil.which("smith") + if found: + return found + try: + result = subprocess.run(["uv", "tool", "dir", "--bin"], capture_output=True, text=True) + except OSError: + return None + if result.returncode != 0: + return None + name = "smith.exe" if sys.platform == "win32" else "smith" + candidate = Path(result.stdout.strip()) / name + if candidate.exists(): + return str(candidate) + return None + + +def sync_skill_via_cli(source: Path) -> bool: + """Sync the skill with the installed CLI so all install paths share one implementation.""" + smith_bin = find_smith_executable() + if not smith_bin: + return False + env = dict(os.environ) + env["SMITH_SKILL_SOURCE_DIR"] = str(source) + result = subprocess.run([smith_bin, "skill", "sync"], env=env) + return result.returncode == 0 + + def sync_skill(source: Path, target: Path) -> None: - """Copy skill directory to target.""" + """Copy skill directory to target. Fallback for when the smith CLI is not on PATH.""" target.parent.mkdir(parents=True, exist_ok=True) temp_root = Path(tempfile.mkdtemp(prefix=f".{target.name}.tmp-", dir=target.parent)) staged = temp_root / "staged" @@ -82,15 +112,17 @@ def main() -> None: print(f"Error: skill directory not found after install: {SKILL_SOURCE}", file=sys.stderr) sys.exit(1) - print("==> Syncing skill") - sync_skill(SKILL_SOURCE, TARGET_SKILL_DIR) - print("==> Installing smith CLI globally with uv") run(["uv", "tool", "install", "-e", str(REPO_DIR), "--force"]) print("==> Ensuring smith is on PATH") run(["uv", "tool", "update-shell"]) + print("==> Syncing skill") + if not sync_skill_via_cli(SKILL_SOURCE): + target = Path(os.environ.get("SMITH_SKILL_DIR") or TARGET_SKILL_DIR).expanduser() + sync_skill(SKILL_SOURCE, target) + print() print("Smith installed successfully!") print() diff --git a/scripts/update_homebrew_formula.py b/scripts/update_homebrew_formula.py index 5a3f9ae..9005c03 100644 --- a/scripts/update_homebrew_formula.py +++ b/scripts/update_homebrew_formula.py @@ -16,8 +16,31 @@ r'(?P,\n\s+revision:\s+)"(?P[0-9a-f]{40})"', re.MULTILINE, ) +TARBALL_PIN_RE = re.compile( + r'url "https://github\.com/faustodavid/smith/archive/refs/tags/(?P[^"/]+)\.tar\.gz"\n' + r'(?P[ \t]+)sha256 "(?P[0-9a-f]{64})"' +) SHA_RE = re.compile(r"^[0-9a-f]{40}$") +SHA256_RE = re.compile(r"^[0-9a-f]{64}$") TAG_RE = re.compile(r"^v[0-9A-Za-z][0-9A-Za-z._-]*$") +# Build backends needed to install the vendored sdist resources without build +# isolation. Single source of truth for the formula's bootstrap_resources list +# and for `brew update-python-resources --extra-packages` in the tap workflow. +BOOTSTRAP_RESOURCES = ( + "setuptools", + "flit-core", + "packaging", + "wheel", + "cython", + "pathspec", + "pluggy", + "trove-classifiers", + "hatchling", + "vcs-versioning", + "setuptools-scm", + "hatch-vcs", +) +BOOTSTRAP_RE = re.compile(r"(?P[ \t]+)bootstrap_resources = %w\[\n(?:[ \t]*[^\s\]]+\n)+[ \t]*\]") CAVEATS_RE = re.compile(r"\n+\s+def caveats\n\s+<<~EOS\n.*?\n\s+EOS\n\s+end", re.DOTALL) CAVEATS_METHOD = """\ def caveats @@ -96,15 +119,51 @@ def validate_tag(tag: str) -> None: raise FormulaUpdateError(f"tag must start with v and contain only letters, numbers, dots, underscores, and hyphens, got {tag!r}") -def parse_formula_pin(text: str) -> tuple[str, str]: +def validate_sha256(sha256: str) -> None: + if not SHA256_RE.fullmatch(sha256): + raise FormulaUpdateError(f"sha256 must be a 64-character lowercase hex digest, got {sha256!r}") + + +@dataclass(frozen=True) +class FormulaPin: + tag: str + revision: str | None + sha256: str | None + + @property + def checksum(self) -> str: + return self.sha256 or self.revision or "" + + +def parse_formula_pin(text: str) -> FormulaPin: match = PIN_RE.search(text) - if not match: - raise FormulaUpdateError("could not find the Smith formula url tag/revision pin") - return match.group("tag"), match.group("revision") + if match: + return FormulaPin(tag=match.group("tag"), revision=match.group("revision"), sha256=None) + tarball = TARBALL_PIN_RE.search(text) + if tarball: + return FormulaPin(tag=tarball.group("tag"), revision=None, sha256=tarball.group("sha256")) + raise FormulaUpdateError("could not find the Smith formula url pin") + +def _tarball_pin_block(tag: str, sha256: str) -> str: + return f'url "https://github.com/faustodavid/smith/archive/refs/tags/{tag}.tar.gz"\n sha256 "{sha256}"' -def update_formula_pin_text(text: str, tag: str, revision: str) -> str: + +def update_formula_pin_text(text: str, tag: str, revision: str | None, sha256: str | None = None) -> str: validate_tag(tag) + + if sha256 is not None: + validate_sha256(sha256) + replacement = _tarball_pin_block(tag, sha256) + updated, count = TARBALL_PIN_RE.subn(replacement, text, count=1) + if count != 1: + updated, count = PIN_RE.subn(replacement, text, count=1) + if count != 1: + raise FormulaUpdateError("could not find the Smith formula url pin") + return updated + + if revision is None: + raise FormulaUpdateError("a revision is required to update the git pin; pass --sha256 to pin a release tarball instead") validate_revision(revision) def replace(match: re.Match[str]) -> str: @@ -112,12 +171,14 @@ def replace(match: re.Match[str]) -> str: updated, count = PIN_RE.subn(replace, text, count=1) if count != 1: + if TARBALL_PIN_RE.search(text): + raise FormulaUpdateError("formula pins a release tarball; pass --sha256 to update it") raise FormulaUpdateError("could not find the Smith formula url tag/revision pin") return updated -def prepare_formula_update(text: str, tag: str, revision: str) -> FormulaUpdate: - pin_updated = update_formula_pin_text(text, tag, revision) +def prepare_formula_update(text: str, tag: str, revision: str | None, sha256: str | None = None) -> FormulaUpdate: + pin_updated = update_formula_pin_text(text, tag, revision, sha256) caveats_updated = ensure_formula_caveats(pin_updated) return FormulaUpdate( text=caveats_updated, @@ -126,8 +187,37 @@ def prepare_formula_update(text: str, tag: str, revision: str) -> FormulaUpdate: ) -def update_formula_text(text: str, tag: str, revision: str) -> str: - return prepare_formula_update(text, tag, revision).text +def update_formula_text(text: str, tag: str, revision: str | None, sha256: str | None = None) -> str: + return prepare_formula_update(text, tag, revision, sha256).text + + +def render_bootstrap_block(indent: str) -> str: + entries = "\n".join(f"{indent} {name}" for name in BOOTSTRAP_RESOURCES) + return f"{indent}bootstrap_resources = %w[\n{entries}\n{indent}]" + + +def ensure_bootstrap_resources(text: str) -> str: + match = BOOTSTRAP_RE.search(text) + if not match: + raise FormulaUpdateError("could not find the bootstrap_resources list in the formula") + missing = [name for name in BOOTSTRAP_RESOURCES if f'resource "{name}" do' not in text] + if missing: + raise FormulaUpdateError( + f"bootstrap resources have no matching resource stanza: {', '.join(missing)}. " + "Run `brew update-python-resources` with the --extra-packages list from --print-extra-packages first." + ) + return text[: match.start()] + render_bootstrap_block(match.group("indent")) + text[match.end() :] + + +def sync_bootstrap_resources(path: Path, *, check: bool) -> bool: + text = path.read_text(encoding="utf-8") + updated = ensure_bootstrap_resources(text) + changed = updated != text + if check and changed: + raise FormulaUpdateError(f"{path} bootstrap_resources list is stale. Rerun this script with --sync-bootstrap-resources.") + if changed: + path.write_text(updated, encoding="utf-8") + return changed def ensure_formula_caveats(text: str) -> str: @@ -149,14 +239,14 @@ def _join_with_caveats(prefix: str, suffix: str) -> str: return f"{prefix.rstrip()}\n\n{CAVEATS_METHOD}{suffix}" -def update_formula(path: Path, tag: str, revision: str, *, check: bool) -> bool: +def update_formula(path: Path, tag: str, revision: str | None, *, sha256: str | None = None, check: bool) -> bool: text = path.read_text(encoding="utf-8") - update = prepare_formula_update(text, tag, revision) + update = prepare_formula_update(text, tag, revision, sha256) if check and update.changed: - current_tag, current_revision = parse_formula_pin(text) + current = parse_formula_pin(text) problems: list[str] = [] if update.pin_changed: - problems.append(f"pins {current_tag}@{current_revision}; expected {tag}@{revision}") + problems.append(f"pins {current.tag}@{current.checksum}; expected {tag}@{sha256 or revision}") if update.caveats_changed: problems.append("Homebrew caveats are missing or stale") raise FormulaUpdateError(f"{path} {'; '.join(problems)}. Run scripts/update_homebrew_formula.py --formula to refresh it.") @@ -172,12 +262,23 @@ def build_parser() -> argparse.ArgumentParser: parser.add_argument("--version", help="Project version to convert to a v-prefixed tag. Defaults to pyproject.toml.") parser.add_argument("--tag", help="Release tag to pin. Defaults to v{project.version}.") parser.add_argument("--revision", help="Release commit SHA. Defaults to resolving the selected tag with git.") + parser.add_argument("--sha256", help="Release tarball sha256. When set, the formula pins the GitHub tag tarball, not a git revision.") parser.add_argument("--check", action="store_true", help="Fail if the formula is not already up to date.") parser.add_argument( "--check-release-tag", action="store_true", help="Validate that the selected release tag matches project.version, then exit without reading a formula.", ) + parser.add_argument( + "--sync-bootstrap-resources", + action="store_true", + help="Rewrite the formula's bootstrap_resources list from the canonical list and exit. Run after `brew update-python-resources`.", + ) + parser.add_argument( + "--print-extra-packages", + action="store_true", + help="Print the canonical bootstrap package list for `brew update-python-resources --extra-packages`, then exit.", + ) return parser @@ -185,6 +286,17 @@ def main(argv: list[str] | None = None) -> int: parser = build_parser() args = parser.parse_args(argv) + if args.print_extra_packages: + print(",".join(BOOTSTRAP_RESOURCES)) + return 0 + + if args.sync_bootstrap_resources: + if args.formula is None: + parser.error("--formula is required with --sync-bootstrap-resources") + changed = sync_bootstrap_resources(args.formula, check=args.check) + print(f"{args.formula}: bootstrap resources {'updated' if changed else 'already current'}") + return 0 + version = args.version or load_project_version(args.pyproject) tag = args.tag or expected_tag(version) validate_tag(tag) @@ -197,10 +309,10 @@ def main(argv: list[str] | None = None) -> int: if args.formula is None: parser.error("--formula is required unless --check-release-tag is set") - revision = args.revision or resolve_tag_revision(tag) - changed = update_formula(args.formula, tag, revision, check=args.check) + revision = None if args.sha256 else (args.revision or resolve_tag_revision(tag)) + changed = update_formula(args.formula, tag, revision, sha256=args.sha256, check=args.check) status = "updated" if changed else "already current" - print(f"{args.formula}: {status} at {tag}@{revision}") + print(f"{args.formula}: {status} at {tag}@{args.sha256 or revision}") return 0 diff --git a/src/smith/cli/handlers.py b/src/smith/cli/handlers.py index 84ab11f..a292262 100644 --- a/src/smith/cli/handlers.py +++ b/src/smith/cli/handlers.py @@ -283,6 +283,8 @@ def handle_config_init(client: SmithClient | None, args: argparse.Namespace) -> 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) + elif skill_result.ok and skill_result.mode == "copy": + print("Smith will refresh this copy automatically after upgrades.", file=stream) print(file=stream) manual_init = bool(getattr(args, "manual", False) or args.output_format == "json") diff --git a/src/smith/cli/main.py b/src/smith/cli/main.py index 7ec9ad3..2233368 100644 --- a/src/smith/cli/main.py +++ b/src/smith/cli/main.py @@ -14,6 +14,7 @@ ) from smith.cli.parser import build_parser from smith.errors import SmithApiError, SmithAuthError +from smith.skill import ensure_skill_fresh _SMITH_CLI_HANDLER_ATTR = "_smith_cli_handler" @@ -51,6 +52,7 @@ def main(argv: list[str] | None = None) -> int: return EXIT_INVALID_ARGS command = getattr(args, "command_id", "unknown") + ensure_skill_fresh(command) try: validate_args_for_remote(args) diff --git a/src/smith/skill.py b/src/smith/skill.py index 308a06d..84cd855 100644 --- a/src/smith/skill.py +++ b/src/smith/skill.py @@ -1,14 +1,23 @@ from __future__ import annotations import filecmp +import json import os import shutil import sys +import tempfile +import time from dataclasses import dataclass +from importlib.metadata import PackageNotFoundError, version from pathlib import Path DEFAULT_SKILL_TARGET = Path.home() / ".agents" / "skills" / "smith" _SKILL_RELATIVE_PATH = Path("share") / "smith" / "skills" / "smith" +SKILL_MARKER_NAME = ".smith-skill-meta.json" +_FRESHNESS_STAMP_NAME = "skill-freshness-stamp" +_FRESHNESS_INTERVAL_SECONDS = 24 * 60 * 60 +_FRESHNESS_EXEMPT_COMMANDS = {"skill.sync", "skill.status", "config.init"} +_STALE_SKILL_HINT = "run 'smith skill sync' to refresh." @dataclass(frozen=True) @@ -69,11 +78,9 @@ def _homebrew_skill_sources() -> list[Path]: 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 + # The opt/smith symlink always points at the active install (including + # HEAD installs), so it is the only candidate that survives upgrades. + return [prefix / "opt" / "smith" / _SKILL_RELATIVE_PATH for prefix in prefixes] def _repo_skill_source() -> Path: @@ -91,13 +98,6 @@ def resolve_skill_source_dir() -> Path | None: 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())) @@ -112,12 +112,37 @@ def _symlink_destination(target: Path) -> Path | None: return _absolute_path_without_resolving(target.parent / destination) +def _installed_version() -> str | None: + try: + return version("smith") + except PackageNotFoundError: + return None + + +def _write_sync_marker(target: Path, source: Path) -> None: + marker = { + "source": str(source), + "version": _installed_version(), + "synced_at": time.strftime("%Y-%m-%dT%H:%M:%S%z"), + } + (target / SKILL_MARKER_NAME).write_text(json.dumps(marker, indent=2) + "\n", encoding="utf-8") + + +def _read_sync_marker(target: Path) -> dict[str, object] | None: + try: + data = json.loads((target / SKILL_MARKER_NAME).read_text(encoding="utf-8")) + except (OSError, ValueError): + return None + return data if isinstance(data, dict) else None + + 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("*")} + marker = Path(SKILL_MARKER_NAME) + target_entries = {path.relative_to(target) for path in target.rglob("*")} - {marker} + source_entries = {path.relative_to(source) for path in source.rglob("*")} - {marker} if target_entries != source_entries: return False @@ -187,25 +212,19 @@ def sync_skill( 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) + mode = _stage_and_swap(source, target, prefer_symlink=prefer_symlink) + if mode == "symlink": + return SkillSyncResult( + ok=True, + status="linked", + target=target, + source=source, + mode="symlink", + message=f"Smith skill linked to: {target}", + ) return SkillSyncResult( ok=True, status="copied", @@ -214,3 +233,117 @@ def sync_skill( mode="copy", message=f"Smith skill copied to: {target}", ) + + +def _stage_and_swap(source: Path, target: Path, *, prefer_symlink: bool) -> str: + """Build the new skill next to the target, then swap it in. The previous + install is only displaced once the replacement is complete, and is restored + if the swap fails.""" + temp_root = Path(tempfile.mkdtemp(prefix=f".{target.name}.tmp-", dir=target.parent)) + staged = temp_root / "staged" + backup = temp_root / "backup" + mode: str | None = None + try: + if prefer_symlink: + try: + staged.symlink_to(source, target_is_directory=True) + mode = "symlink" + except OSError: + pass + if mode is None: + shutil.copytree(source, staged) + _write_sync_marker(staged, source) + mode = "copy" + if target.exists() or target.is_symlink(): + target.replace(backup) + staged.replace(target) + except Exception: + if not (target.exists() or target.is_symlink()) and (backup.exists() or backup.is_symlink()): + backup.replace(target) + raise + finally: + shutil.rmtree(temp_root, ignore_errors=True) + return mode + + +def _freshness_stamp_path() -> Path: + cache_root = _path_from_env("XDG_CACHE_HOME") or Path.home() / ".cache" + return cache_root / "smith" / _FRESHNESS_STAMP_NAME + + +def _freshness_check_due(stamp: Path) -> bool: + try: + age = time.time() - stamp.stat().st_mtime + except OSError: + return True + return age >= _FRESHNESS_INTERVAL_SECONDS or age < 0 + + +def _refresh_or_hint(target: Path, source: Path | None) -> None: + result = None + if source is not None: + try: + result = sync_skill(source_dir=source, target_dir=target) + except Exception: + result = None + if result is not None and result.ok: + print(f"smith: refreshed agent skill at {target}", file=sys.stderr) + else: + print(f"smith: agent skill at {target} may be out of date; {_STALE_SKILL_HINT}", file=sys.stderr) + + +def ensure_skill_fresh(command_id: str | None = None) -> None: + """Refresh a stale skill install at most once a day. Must never break the CLI.""" + try: + _ensure_skill_fresh(command_id) + except Exception: # pragma: no cover - defensive guard around CLI startup + pass + + +def _ensure_skill_fresh(command_id: str | None) -> None: + if os.getenv("SMITH_SKILL_CHECK", "").strip() == "0": + return + if command_id in _FRESHNESS_EXEMPT_COMMANDS: + return + + target = default_skill_target_dir() + # Only refresh installs that already exist; installing the skill is an + # explicit choice made via `smith skill sync` or `smith config init`. + if not target.exists() and not target.is_symlink(): + return + + stamp = _freshness_stamp_path() + if not _freshness_check_due(stamp): + return + stamp.parent.mkdir(parents=True, exist_ok=True) + stamp.touch() + + if target.is_symlink(): + # A working symlink is left alone even when it does not point at the + # resolvable source: developers deliberately link to a repo checkout, + # and Homebrew links track upgrades via the opt path on their own. + if target.exists(): + return + _refresh_or_hint(target, resolve_skill_source_dir()) + return + + marker = _read_sync_marker(target) + source = resolve_skill_source_dir() + if source is None and marker is not None: + recorded = Path(str(marker.get("source") or "")).expanduser() + if str(recorded) != "." and recorded.exists(): + source = recorded + + if source is not None: + if skill_target_points_to_source(target, source): + return + if marker is None: + # Without the sync marker the directory is not known to be + # smith-managed; never delete content smith did not write. + print(f"smith: agent skill at {target} may be out of date; {_STALE_SKILL_HINT}", file=sys.stderr) + return + _refresh_or_hint(target, source) + return + + if marker is not None and marker.get("version") not in (None, _installed_version()): + print(f"smith: agent skill at {target} may be out of date; {_STALE_SKILL_HINT}", file=sys.stderr) diff --git a/tests/conftest.py b/tests/conftest.py index 63b2882..7a49a2b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -46,6 +46,14 @@ def pytest_collection_modifyitems(config: pytest.Config, items: list[pytest.Item item.add_marker(skip_integration) +@pytest.fixture(autouse=True) +def _disable_skill_freshness_check(monkeypatch: pytest.MonkeyPatch, request: pytest.FixtureRequest) -> None: + """Keep CLI invocations hermetic: never touch the developer's real skill install.""" + if request.node.get_closest_marker("integration"): + return + monkeypatch.setenv("SMITH_SKILL_CHECK", "0") + + @pytest.fixture def runtime_config(): return make_runtime_config() diff --git a/tests/unit/test_install_script.py b/tests/unit/test_install_script.py index c7888e9..53978f3 100644 --- a/tests/unit/test_install_script.py +++ b/tests/unit/test_install_script.py @@ -70,6 +70,34 @@ def _failing_copytree(src: Path, dst: Path) -> None: assert existing.read_text(encoding="utf-8") == "existing" +def test_sync_skill_via_cli_returns_false_when_smith_missing(monkeypatch: Any, tmp_path: Path) -> None: + install = _load_install_module() + monkeypatch.setattr(install.shutil, "which", lambda name: None) + + assert install.sync_skill_via_cli(tmp_path / "skills" / "smith") is False + + +def test_sync_skill_via_cli_runs_skill_sync_with_source_env(monkeypatch: Any, tmp_path: Path) -> None: + install = _load_install_module() + source = tmp_path / "skills" / "smith" + calls: list[tuple[list[str], dict[str, str]]] = [] + + monkeypatch.setattr(install.shutil, "which", lambda name: "/usr/local/bin/smith") + + class _Result: + returncode = 0 + + def _fake_run(cmd: list[str], **kwargs: Any) -> _Result: + calls.append((cmd, kwargs["env"])) + return _Result() + + monkeypatch.setattr(install.subprocess, "run", _fake_run) + + assert install.sync_skill_via_cli(source) is True + assert calls == [(["/usr/local/bin/smith", "skill", "sync"], calls[0][1])] + assert calls[0][1]["SMITH_SKILL_SOURCE_DIR"] == str(source) + + def test_install_refuses_existing_non_git_directory(monkeypatch: Any, tmp_path: Path, capsys: Any) -> None: install = _load_install_module() repo_dir = tmp_path / "smith" diff --git a/tests/unit/test_skill.py b/tests/unit/test_skill.py index 7bdf0a9..f9526d6 100644 --- a/tests/unit/test_skill.py +++ b/tests/unit/test_skill.py @@ -1,6 +1,8 @@ from __future__ import annotations +import json import os +import shutil from pathlib import Path from smith import skill @@ -140,3 +142,220 @@ def test_resolve_skill_source_prefers_homebrew_opt_path(monkeypatch, tmp_path: P monkeypatch.delenv("SMITH_SKILL_SOURCE_DIR", raising=False) assert skill.resolve_skill_source_dir() == source + + +def test_sync_skill_copy_writes_marker_with_source_and_version(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, prefer_symlink=False) + + assert result.status == "copied" + marker = skill._read_sync_marker(target) + assert marker is not None + assert marker["source"] == str(source) + assert marker["version"] == skill._installed_version() + + +def test_sync_skill_symlink_does_not_write_marker(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.mode == "symlink" + assert not (source / skill.SKILL_MARKER_NAME).exists() + + +def _freshness_env(monkeypatch, tmp_path: Path, target: Path, source: Path | None) -> None: + monkeypatch.setenv("SMITH_SKILL_DIR", str(target)) + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache")) + monkeypatch.delenv("SMITH_SKILL_CHECK", raising=False) + monkeypatch.delenv("SMITH_HOMEBREW_PREFIX", raising=False) + if source is not None: + monkeypatch.setenv("SMITH_SKILL_SOURCE_DIR", str(source)) + else: + monkeypatch.setenv("SMITH_SKILL_SOURCE_DIR", str(tmp_path / "missing-source")) + + +def test_ensure_skill_fresh_refreshes_stale_copy(monkeypatch, capsys, 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") + _freshness_env(monkeypatch, tmp_path, target, source) + + skill.ensure_skill_fresh("repos.list") + + assert "refreshed agent skill" in capsys.readouterr().err + assert skill.skill_target_points_to_source(target, source) + + +def test_ensure_skill_fresh_repairs_broken_symlink(monkeypatch, capsys, tmp_path: Path) -> None: + source = _make_skill_source(tmp_path) + target = tmp_path / ".agents" / "skills" / "smith" + target.parent.mkdir(parents=True) + target.symlink_to(tmp_path / "gone", target_is_directory=True) + _freshness_env(monkeypatch, tmp_path, target, source) + + skill.ensure_skill_fresh("repos.list") + + assert "refreshed agent skill" in capsys.readouterr().err + assert skill.skill_target_points_to_source(target, source) + + +def test_ensure_skill_fresh_noop_for_current_symlink(monkeypatch, capsys, 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) + _freshness_env(monkeypatch, tmp_path, target, source) + + skill.ensure_skill_fresh("repos.list") + + assert capsys.readouterr().err == "" + + +def test_ensure_skill_fresh_hints_on_version_mismatch_without_source(monkeypatch, capsys, 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) + shutil.rmtree(source) + marker_path = target / skill.SKILL_MARKER_NAME + marker = json.loads(marker_path.read_text(encoding="utf-8")) + marker["version"] = "0.0.0" + marker_path.write_text(json.dumps(marker), encoding="utf-8") + _freshness_env(monkeypatch, tmp_path, target, None) + + skill.ensure_skill_fresh("repos.list") + + assert "may be out of date" in capsys.readouterr().err + assert not target.is_symlink() + + +def test_ensure_skill_fresh_uses_marker_source_when_unresolvable(monkeypatch, capsys, 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") + _freshness_env(monkeypatch, tmp_path, target, None) + + skill.ensure_skill_fresh("repos.list") + + assert "refreshed agent skill" in capsys.readouterr().err + assert skill.skill_target_points_to_source(target, source) + + +def test_ensure_skill_fresh_throttled_by_stamp(monkeypatch, capsys, 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") + _freshness_env(monkeypatch, tmp_path, target, source) + stamp = tmp_path / "cache" / "smith" / "skill-freshness-stamp" + stamp.parent.mkdir(parents=True) + stamp.touch() + + skill.ensure_skill_fresh("repos.list") + + assert capsys.readouterr().err == "" + assert not skill.skill_target_points_to_source(target, source) + + +def test_ensure_skill_fresh_respects_kill_switch(monkeypatch, capsys, 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") + _freshness_env(monkeypatch, tmp_path, target, source) + monkeypatch.setenv("SMITH_SKILL_CHECK", "0") + + skill.ensure_skill_fresh("repos.list") + + assert capsys.readouterr().err == "" + + +def test_ensure_skill_fresh_skips_exempt_commands(monkeypatch, capsys, 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") + _freshness_env(monkeypatch, tmp_path, target, source) + + skill.ensure_skill_fresh("skill.sync") + + assert capsys.readouterr().err == "" + assert not skill.skill_target_points_to_source(target, source) + + +def test_ensure_skill_fresh_skips_missing_target(monkeypatch, capsys, tmp_path: Path) -> None: + source = _make_skill_source(tmp_path) + target = tmp_path / ".agents" / "skills" / "smith" + _freshness_env(monkeypatch, tmp_path, target, source) + + skill.ensure_skill_fresh("repos.list") + + assert capsys.readouterr().err == "" + assert not target.exists() + + +def test_ensure_skill_fresh_silent_for_current_copy(monkeypatch, capsys, 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) + _freshness_env(monkeypatch, tmp_path, target, source) + + skill.ensure_skill_fresh("repos.list") + + assert capsys.readouterr().err == "" + assert not target.is_symlink() + + +def test_ensure_skill_fresh_never_deletes_unmanaged_directory(monkeypatch, capsys, tmp_path: Path) -> None: + source = _make_skill_source(tmp_path) + target = tmp_path / ".agents" / "skills" / "smith" + target.mkdir(parents=True) + (target / "SKILL.md").write_text("hand-rolled", encoding="utf-8") + (target / "my-notes.txt").write_text("keep me", encoding="utf-8") + _freshness_env(monkeypatch, tmp_path, target, source) + + skill.ensure_skill_fresh("repos.list") + + assert "may be out of date" in capsys.readouterr().err + assert (target / "my-notes.txt").read_text(encoding="utf-8") == "keep me" + assert (target / "SKILL.md").read_text(encoding="utf-8") == "hand-rolled" + + +def test_ensure_skill_fresh_leaves_working_symlink_to_other_source(monkeypatch, capsys, tmp_path: Path) -> None: + source = _make_skill_source(tmp_path) + dev_checkout = tmp_path / "dev" / "skills" / "smith" + dev_checkout.mkdir(parents=True) + (dev_checkout / "SKILL.md").write_text("dev copy", encoding="utf-8") + target = tmp_path / ".agents" / "skills" / "smith" + target.parent.mkdir(parents=True) + target.symlink_to(dev_checkout, target_is_directory=True) + _freshness_env(monkeypatch, tmp_path, target, source) + + skill.ensure_skill_fresh("repos.list") + + assert capsys.readouterr().err == "" + assert Path(os.readlink(target)) == dev_checkout + + +def test_sync_skill_restores_previous_copy_when_staging_fails(monkeypatch, 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) + original = (target / "SKILL.md").read_text(encoding="utf-8") + (source / "SKILL.md").write_text("---\nname: smith\nupdated: true\n---\n", encoding="utf-8") + + def _failing_copytree(src, dst, **kwargs): + raise OSError("disk full") + + monkeypatch.setattr(skill.shutil, "copytree", _failing_copytree) + + try: + skill.sync_skill(source_dir=source, target_dir=target, prefer_symlink=False) + except OSError: + pass + + assert (target / "SKILL.md").read_text(encoding="utf-8") == original diff --git a/tests/unit/test_update_homebrew_formula.py b/tests/unit/test_update_homebrew_formula.py index 006caf5..1b53aca 100644 --- a/tests/unit/test_update_homebrew_formula.py +++ b/tests/unit/test_update_homebrew_formula.py @@ -187,3 +187,124 @@ def test_update_formula_rejects_unsafe_tag() -> None: with pytest.raises(updater.FormulaUpdateError, match="tag must start with v"): updater.update_formula_text(FORMULA, 'v1.2.3"; echo nope', "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") + + +SHA256 = "c" * 64 + +FORMULA_WITH_TARBALL = """# frozen_string_literal: true + +class Smith < Formula + url "https://github.com/faustodavid/smith/archive/refs/tags/v0.1.0.tar.gz" + sha256 "{sha256}" +end +""".format(sha256="d" * 64) + +FORMULA_WITH_BOOTSTRAP = """# frozen_string_literal: true + +class Smith < Formula + url "https://github.com/faustodavid/smith.git", + tag: "v0.1.0", + revision: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + resource "setuptools" do + end + + def install + bootstrap_resources = %w[ + setuptools + stale-entry + ] + end +end +""" + + +def test_update_formula_text_converts_git_pin_to_tarball() -> None: + updater = _load_formula_module() + + updated = updater.update_formula_text(FORMULA, "v0.1.1", None, SHA256) + + assert 'url "https://github.com/faustodavid/smith/archive/refs/tags/v0.1.1.tar.gz"' in updated + assert f'sha256 "{SHA256}"' in updated + assert "revision:" not in updated + + +def test_update_formula_text_updates_existing_tarball_pin() -> None: + updater = _load_formula_module() + + updated = updater.update_formula_text(FORMULA_WITH_TARBALL, "v0.1.1", None, SHA256) + + assert "refs/tags/v0.1.1.tar.gz" in updated + assert f'sha256 "{SHA256}"' in updated + assert "d" * 64 not in updated + + +def test_update_formula_text_requires_sha256_for_tarball_pin() -> None: + updater = _load_formula_module() + + with pytest.raises(updater.FormulaUpdateError, match="pass --sha256"): + updater.update_formula_text(FORMULA_WITH_TARBALL, "v0.1.1", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") + + +def test_update_formula_rejects_invalid_sha256() -> None: + updater = _load_formula_module() + + with pytest.raises(updater.FormulaUpdateError, match="sha256 must be"): + updater.update_formula_text(FORMULA, "v0.1.1", None, "nothex") + + +def test_parse_formula_pin_reads_both_forms() -> None: + updater = _load_formula_module() + + git_pin = updater.parse_formula_pin(FORMULA) + tarball_pin = updater.parse_formula_pin(FORMULA_WITH_TARBALL) + + assert (git_pin.tag, git_pin.revision, git_pin.sha256) == ("v0.1.0", "a" * 40, None) + assert (tarball_pin.tag, tarball_pin.revision, tarball_pin.sha256) == ("v0.1.0", None, "d" * 64) + + +def test_ensure_bootstrap_resources_rewrites_canonical_list() -> None: + updater = _load_formula_module() + formula = FORMULA_WITH_BOOTSTRAP.replace( + ' resource "setuptools" do\n end\n', + "".join(f' resource "{name}" do\n end\n' for name in updater.BOOTSTRAP_RESOURCES), + ) + + updated = updater.ensure_bootstrap_resources(formula) + + assert "stale-entry" not in updated + for name in updater.BOOTSTRAP_RESOURCES: + assert f" {name}\n" in updated + + +def test_ensure_bootstrap_resources_fails_on_missing_stanza() -> None: + updater = _load_formula_module() + + with pytest.raises(updater.FormulaUpdateError, match="no matching resource stanza"): + updater.ensure_bootstrap_resources(FORMULA_WITH_BOOTSTRAP) + + +def test_main_print_extra_packages(capsys: pytest.CaptureFixture[str]) -> None: + updater = _load_formula_module() + + assert updater.main(["--print-extra-packages"]) == 0 + + out = capsys.readouterr().out.strip() + assert out == ",".join(updater.BOOTSTRAP_RESOURCES) + + +def test_main_sync_bootstrap_resources_updates_formula(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + updater = _load_formula_module() + formula = tmp_path / "smith.rb" + formula.write_text( + FORMULA_WITH_BOOTSTRAP.replace( + ' resource "setuptools" do\n end\n', + "".join(f' resource "{name}" do\n end\n' for name in updater.BOOTSTRAP_RESOURCES), + ), + encoding="utf-8", + ) + + assert updater.main(["--formula", str(formula), "--sync-bootstrap-resources"]) == 0 + + assert "bootstrap resources updated" in capsys.readouterr().out + assert "stale-entry" not in formula.read_text(encoding="utf-8")