diff --git a/README.md b/README.md index abf68731..b4b128d2 100644 --- a/README.md +++ b/README.md @@ -291,14 +291,34 @@ Add to your editor's MCP config: ### AI Tool Integration -Automatically detect and configure AI coding assistants with Deepgram skills. +Install the Deepgram skills as folders in each AI coding assistant's skills folder. ```bash dg skills status # Detect AI tools dg skills setup # Interactive setup wizard dg skills install --all # Install for all detected tools +dg skills update # Replace the folders deepctl installed +dg skills remove --all # Delete the folders deepctl installed ``` +| Tool | Skills folder | +|---|---| +| Claude Code | `~/.claude/skills` | +| OpenAI Codex | `~/.agents/skills` | +| Gemini CLI | `~/.gemini/skills` | +| Cursor | `~/.cursor/skills` | +| OpenCode | `~/.config/opencode/skills` | +| Cline | `~/.cline/skills` | +| Amazon Q Developer, Aider | none: deepctl prints where to get the skills | + +deepctl replaces or deletes a folder only if skills.json records it for that tool, it is a real folder directly in that tool's skills folder, it holds deepctl's `.deepctl-skill` marker, and its contents are exactly what deepctl installed. So folders from `npx skills add`, and symlinked skill folders, are left alone. + +These commands abort if the filesystem cannot provide locking or no-replace directory moves. Installed skill folders and skills.json stay as they were, though the tool's skills folder and `~/.deepctl/skills/skills.json.lock` may already have been created. While another `dg skills` command is changing skills, the next one waits up to 30 seconds for it to finish. + +If you edit a deepctl folder, or add a file or link to it, deepctl leaves it alone: `update` stops without changing anything and `remove` won't delete it. To get updates again, rename or move your edited copy, or delete it yourself. Opening a skill folder in Finder or Explorer can add `.DS_Store`, `Thumbs.db` or `desktop.ini`, which counts as an edit. + +Files from deepctl 0.3.x, such as `~/.claude/commands/deepgram/*.md` and the rules files like `~/.cursor/rules/deepctl.mdc`, are currently kept, and `dg skills remove` doesn't delete them. + ### Starter Apps Scaffold a new project from Deepgram templates. diff --git a/packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py b/packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py index 9db92d92..e564ad80 100644 --- a/packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py +++ b/packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py @@ -2,20 +2,28 @@ from __future__ import annotations +import contextlib import importlib.metadata -from datetime import datetime, timezone -from typing import Any +from pathlib import Path +from typing import TYPE_CHECKING, Any import click from deepctl_core.auth import AuthManager from deepctl_core.base_group_command import BaseGroupCommand from deepctl_core.client import DeepgramClient from deepctl_core.config import Config -from deepctl_core.output import print_info, print_success, print_warning +from deepctl_core.output import print_error, print_info, print_success, print_warning from rich.console import Console +from rich.markup import escape from rich.table import Table +if TYPE_CHECKING: + from collections.abc import Iterator + + from deepctl_core.skill_generator import SkillGenerator + console = Console() +_REF_OPTION = click.option("--ref", metavar="REF", help="deepgram/skills git ref") class SkillsCommand(BaseGroupCommand): @@ -31,10 +39,10 @@ class SkillsCommand(BaseGroupCommand): "dg skills remove --all", ] agent_help = ( - "Manage skill files that teach AI coding assistants (Claude Code, " - "Codex, Gemini CLI, etc.) how to use deepctl. Use 'skills status' to " - "detect which AI CLIs are installed, 'skills install' to generate " - "integration files, and 'skills update' to regenerate after plugin changes." + "Install the Deepgram skills as folders for AI coding assistants (Claude " + "Code, Codex, Gemini CLI, Cursor, OpenCode, Cline). 'skills status' shows " + "detected tools, 'skills install' adds the folders, 'skills update' " + "replaces them and 'skills remove' deletes only folders deepctl installed." ) def execute(self, ctx: click.Context, **kwargs: Any) -> None: @@ -110,7 +118,7 @@ def _create_install_command(self, context_wrapper: Any) -> click.Command: @click.command( name="install", - help="Detect AI CLIs and install skill files", + help="Detect AI CLIs and install the Deepgram skill folders", ) @click.option( "--all", @@ -123,6 +131,7 @@ def _create_install_command(self, context_wrapper: Any) -> click.Command: "cli_name", help="Install for a specific AI CLI only", ) + @_REF_OPTION def install_cmd(**kwargs: Any) -> None: pass @@ -136,13 +145,14 @@ def _create_update_command(self, context_wrapper: Any) -> click.Command: @click.command( name="update", - help="Regenerate all installed skill files from current metadata", + help="Replace the installed skill folders deepctl owns", ) + @_REF_OPTION def update_cmd(**kwargs: Any) -> None: pass update_cmd.callback = context_wrapper( - lambda config, auth_manager, client, **kw: self._handle_update() + lambda config, auth_manager, client, **kw: self._handle_update(**kw) ) return update_cmd @@ -151,18 +161,18 @@ def _create_remove_command(self, context_wrapper: Any) -> click.Command: @click.command( name="remove", - help="Remove installed skill files", + help="Remove the skill folders deepctl installed", ) @click.option( "--all", "remove_all", is_flag=True, - help="Remove all installed skill files", + help="Remove all installed skill folders", ) @click.option( "--cli", "cli_name", - help="Remove skill files for a specific AI CLI", + help="Remove skill folders for a specific AI CLI", ) def remove_cmd(**kwargs: Any) -> None: pass @@ -177,7 +187,7 @@ def _create_list_command(self, context_wrapper: Any) -> click.Command: @click.command( name="list", - help="Show installed skills with paths and versions", + help="Show installed skill folders and their source", ) def list_cmd(**kwargs: Any) -> None: pass @@ -200,6 +210,7 @@ def _create_setup_command(self, context_wrapper: Any) -> click.Command: is_flag=True, help="Install for all detected tools without prompting", ) + @_REF_OPTION def setup_cmd(**kwargs: Any) -> None: pass @@ -212,49 +223,101 @@ def setup_cmd(**kwargs: Any) -> None: # Handlers # ------------------------------------------------------------------ - def _handle_status(self) -> None: - """Show detected AI CLIs and whether skills are installed.""" - from deepctl_core.skill_generator import get_all_generators, get_skills_state + def _install(self, plan: list[tuple[SkillGenerator, str]]) -> None: + """Fetch, preflight every tool, then install tool by tool.""" + from deepctl_core import skill_bundle + from deepctl_core import skill_generator as sg + + for gen, _ in plan: + if gen.skills_root() is None: + print_warning(escape(sg._msg("E15", gen))) + plan = [(g, r) for g, r in plan if g.skills_root() is not None] + bundles = { + r: skill_bundle.fetch_skill_bundle(r) + for r in dict.fromkeys(r for _, r in plan) + } + unproven, edited = list[Path](), list[Path]() + for ref, skills in bundles.items(): + u, e = sg.install_conflicts([g for g, r in plan if r == ref], skills) + unproven, edited = unproven + u, edited + e + if unproven or edited: # Nothing is written for any tool. + raise sg.SkillOwnershipError(unproven, edited) + count, tools, v = 0, 0, _version() + for gen, ref in plan: + try: + paths, leftover = sg.install_tool(gen, bundles[ref], ref=ref, version=v) + except sg.SkillInstallError as exc: + if exc.leftover: + print_warning(escape(sg._msg("E12", staging=exc.leftover))) + raise + done = f"{gen.display_name}: installed {_n(len(paths), 'skill')} in {gen.skills_root()}." + print_success(escape(done)) + if leftover: # Only this run's staging (SF3). + print_warning(escape(sg._msg("E12", staging=leftover))) + count, tools = count + len(paths), tools + 1 + if count: + labels = ", ".join(dict.fromkeys(_label(r) for _, r in plan)) + done = f"Installed {_n(count, 'skill folder')} for {_n(tools, 'tool')} from deepgram/skills {labels}." + print_success(escape(done)) + if any(g.cli_name == "claude" for g, _ in plan): # Every tool installed. + print_info( + "In Claude Code, run /setup-mcp to configure the Deepgram MCP server." + ) + else: + print_info("No skills were installed.") - generators = get_all_generators() - state = get_skills_state() - installed = state.get("installed_skills", {}) + def _handle_status(self) -> None: + """Show detected AI CLIs and the skill folders deepctl installed.""" + from deepctl_core import skill_generator as sg + with _clean_errors(): + state = sg.get_skills_state() + recs, legacy = state.get("skill_folders", {}), state["installed_skills"] table = Table(title="AI Coding Assistant Status") - table.add_column("CLI", style="cyan", no_wrap=True) - table.add_column("Detected", style="white") - table.add_column("Skills Installed", style="white") - - for gen in generators: - detected = gen.detect() - has_skills = gen.cli_name in installed + for col in ("Tool", "Detected", "Skills folder", "Installed"): + table.add_column( + col, style="cyan" if col == "Tool" else "white", no_wrap=col == "Tool" + ) + notes, old = list[str](), list[str]() + detected = False + for gen in sg.get_all_generators(): + st, found = sg.tool_status(gen, state), gen.detect() + detected = detected or found table.add_row( gen.display_name, - "[green]Yes[/green]" if detected else "[dim]No[/dim]", - "[green]Yes[/green]" if has_skills else "[dim]No[/dim]", + "[green]Yes[/green]" if found else "[dim]No[/dim]", + escape(str(st.root or "none")), + str(len(st.kinds["ok"])) if st.root else "-", ) - + notes += [sg._msg("E14", dest=p) for p in st.kinds["unproven"]] + notes += [sg._msg("E24", dest=p) for p in st.kinds["edited"]] + notes += [sg._msg("E25", dest=p) for p in st.kinds["unreadable"]] + notes += [sg._msg("E16", path=p) for p in st.leftovers] + if found and st.root is None: + notes.append(sg._msg("E15", gen)) + if st.root and gen.cli_name in legacy and gen.cli_name not in recs: + old.append(gen.display_name) console.print(table) - - detected_count = sum(1 for g in generators if g.detect()) - if detected_count > 0 and not installed: - print_info( - "\nRun 'deepctl skills install' to set up AI assistant integrations." - ) + for note in dict.fromkeys(notes): + print_warning(escape(note)) + if old: + note = f"Files from deepctl 0.3.x are recorded for {', '.join(old)}; run 'dg skills update' to install the skill folders, and the old files stay until a later release." + print_info(escape(note)) + if detected and not recs and not legacy: + print_info("Run 'dg skills install' to set up AI assistant integrations.") def _handle_install( self, install_all: bool = False, cli_name: str | None = None, + ref: str | None = None, ) -> None: - """Detect AI CLIs, prompt user, generate & install skill files.""" + """Detect AI CLIs, prompt the user, and install the skill folders.""" + from deepctl_core.skill_bundle import resolve_skills_ref from deepctl_core.skill_generator import ( - _commands_hash, - collect_command_metadata, detect_ai_clis, get_all_generators, get_skills_state, - save_skills_state, ) # If a specific CLI was requested, filter @@ -265,8 +328,8 @@ def _handle_install( # exits 0, and the README documents 1 for a command that # fails. main.py prints the message and exits 1. raise click.ClickException( - f"Unknown AI CLI: {cli_name}. " - "Run 'deepctl skills status' to see supported CLIs." + f"Unknown AI CLI: {cli_name}; " + "run 'dg skills status' to see the supported CLIs." ) if not generators[0].detect(): print_warning( @@ -289,117 +352,56 @@ def _handle_install( print_info(f" - {g.display_name}") return - # Collect metadata - commands = collect_command_metadata() - try: - version = importlib.metadata.version("deepctl") - except importlib.metadata.PackageNotFoundError: - version = "0.0.0" - - state = get_skills_state() - total_written: list[str] = [] - - for gen in generators: - if ( - not install_all - and not cli_name - and not self.confirm( + with _clean_errors(): + get_skills_state() # A corrupt file fails before any prompt. + selected = [ + gen + for gen in generators + if install_all + or cli_name + or self.confirm( f"Install deepctl skills for {gen.display_name}?", default=True, ) - ): - continue - - paths = gen.install(commands, version) - cmd_hash = _commands_hash(commands) - state["installed_skills"][gen.cli_name] = { - "paths": [str(p) for p in paths], - "installed_at": datetime.now(timezone.utc).isoformat(), - "version": version, - "commands_hash": cmd_hash, - } - for p in paths: - total_written.append(str(p)) - print_success(f" Wrote {p}") - - save_skills_state(state) - - if total_written: - print_success(f"\nInstalled skills: {len(total_written)} file(s)") - print_info("Run /deepgram:setup-mcp to configure the Deepgram MCP server.") - else: - print_info("No skills were installed.") - - def _handle_update(self) -> None: - """Regenerate all installed skill files from current metadata.""" - from deepctl_core.skill_generator import ( - _commands_hash, - collect_command_metadata, - get_all_generators, - get_skills_state, - save_skills_state, - ) - - state = get_skills_state() - installed = state.get("installed_skills", {}) - - if not installed: - print_info("No skills installed. Run 'deepctl skills install' first.") - return - - commands = collect_command_metadata() - try: - version = importlib.metadata.version("deepctl") - except importlib.metadata.PackageNotFoundError: - version = "0.0.0" - - generators = {g.cli_name: g for g in get_all_generators()} - updated_count = 0 - - for cli_key in list(installed.keys()): - gen = generators.get(cli_key) - if gen is None: - print_warning(f"Unknown CLI '{cli_key}', skipping.") - continue - - paths = gen.install(commands, version) - cmd_hash = _commands_hash(commands) - state["installed_skills"][cli_key].update( - { - "paths": [str(p) for p in paths], - "version": version, - "commands_hash": cmd_hash, - } - ) - updated_count += 1 - for p in paths: - print_success(f" Updated {p}") - - save_skills_state(state) - print_success(f"Updated {updated_count} skill(s)") - if updated_count: - print_info("Run /deepgram:setup-mcp to configure the Deepgram MCP server.") + ] + ref = resolve_skills_ref(ref) + self._install([(g, ref) for g in selected]) + + def _handle_update(self, ref: str | None = None) -> None: + """Replace the recorded skill folders from --ref, the env var or each recorded ref.""" + from deepctl_core import skill_generator as sg + + with _clean_errors(): + state = sg.get_skills_state() + names = [*state.get("skill_folders", {}), *state["installed_skills"]] + gens = {g.cli_name: g for g in sg.get_all_generators()} + for name in dict.fromkeys(n for n in names if n not in gens): + print_warning(escape(f"Unknown CLI '{name}', skipping.")) + targets = [g for g in gens.values() if g.cli_name in names] + if not targets: + hint = "No skills are installed, so there is nothing to update; run 'dg skills install' first." + print_info(hint) + return + self._install([(g, sg._ref_for(g.cli_name, state, ref)) for g in targets]) def _handle_remove( self, remove_all: bool = False, cli_name: str | None = None, ) -> None: - """Remove installed skill files.""" - from deepctl_core.skill_generator import ( - get_all_generators, - get_skills_state, - save_skills_state, - ) + """Remove the skill folders deepctl installed and can still prove.""" + from deepctl_core import skill_generator as sg - state = get_skills_state() - installed = state.get("installed_skills", {}) + with _clean_errors(): + state = sg.get_skills_state() + recs, legacy = state.get("skill_folders", {}), state["installed_skills"] + installed = list(dict.fromkeys([*recs, *legacy])) if not installed: print_info("No skills are installed.") return - generators = {g.cli_name: g for g in get_all_generators()} + generators = {g.cli_name: g for g in sg.get_all_generators()} if cli_name: targets = [cli_name] if cli_name in installed else [] @@ -409,43 +411,78 @@ def _handle_remove( # which the README documents as exit 1, not 0. raise click.ClickException(f"No skills installed for '{cli_name}'.") elif remove_all: - targets = list(installed.keys()) + targets = installed else: print_info("Specify --all to remove all, or --cli NAME.") return - for cli_key in targets: - gen = generators.get(cli_key) - if gen: - removed = gen.remove() - for p in removed: - print_info(f" Removed {p}") - del state["installed_skills"][cli_key] - - save_skills_state(state) - print_success(f"Removed {len(targets)} skill(s).") + removed, tools, failed = 0, 0, False + with _clean_errors(), sg._state_lock(): # Once for all tools: no wait per tool. + for cli_key in targets: + gen = generators.get(cli_key) + if gen is None: + print_warning(escape(f"Unknown CLI '{cli_key}', skipping.")) + continue + try: + res = sg.remove_tool(gen) + except sg.SkillInstallError as exc: # E18, E21, E9c: go on (N10). + print_error(escape(str(exc))) + failed = True + continue + notes = [sg._msg("E26", dest=p) for p in res.left_alone] + notes += [sg._msg("E23", dest=p) for p in res.edited] + notes += [sg._msg("E4", dest=d, aside=a) for d, a in res.moved] + notes += [sg._msg("E29", dest=d, aside=a) for d, a in res.stranded] + notes += [sg._msg("E13", dest=d, reason=why) for d, why in res.kept] + notes += [sg._msg("E12", staging=res.leftover)] if res.leftover else [] + for note in notes: + print_warning(escape(note)) + paths = legacy.get(cli_key, {}).get("paths", []) + old = recs.get(cli_key, {}).get("v03") or any( + Path(p).parent != gen.skills_root() for p in paths + ) # Not 0.3.x if every path is one of our folders. + v03 = "files from deepctl 0.3.x stay until a later release." + c10 = f"For {gen.display_name}, {v03}" + if cli_key not in recs: + c10 = f"{gen.display_name} has no skill folders recorded, so nothing was removed{'; ' + v03 if old else '.'}" + if old or cli_key not in recs: + print_info(escape(c10)) + failed = failed or bool( + res.kept or res.moved or res.stranded or res.leftover + ) + removed, tools = removed + len(res.removed), tools + bool(res.removed) + if failed: + c11 = "Some skill folders were not fully removed; fix the problems listed above." + raise click.ClickException(c11) + done = f"Removed {_n(removed, 'skill folder')} from {_n(tools, 'tool')}." + print_success(done) def _handle_list(self) -> None: - """Show installed skills with paths and versions.""" - from deepctl_core.skill_generator import get_skills_state + """Show the recorded skill folders with their source ref.""" + from deepctl_core import skill_generator as sg - state = get_skills_state() - installed = state.get("installed_skills", {}) + with _clean_errors(): + state = sg.get_skills_state() + recs = state.get("skill_folders", {}) - if not installed: + if not recs: print_info( - "No skills installed. Run 'deepctl skills install' to get started." + "No skill folders are installed; run 'dg skills install' to get started." ) return table = Table(title="Installed Skills") - table.add_column("CLI", style="cyan", no_wrap=True) - table.add_column("Version", style="green") - table.add_column("Paths", style="white") - - for cli_key, info in installed.items(): - paths = "\n".join(info.get("paths", [])) - table.add_row(cli_key, info.get("version", "?"), paths) + table.add_column("Tool", style="cyan", no_wrap=True) + table.add_column("Ref", style="green") + table.add_column("Skills", style="white") + table.add_column("Folder", style="white") + + for gen in sg.get_all_generators(): + if gen.cli_name in recs: + st = sg.tool_status(gen, state) + ok = f"{len(st.kinds['ok'])}/{len(recs[gen.cli_name].get('folders', {}))}" + ref = escape(_label(st.skills_ref) if st.skills_ref else "-") + table.add_row(gen.display_name, ref, ok, escape(str(st.root))) console.print(table) @@ -455,23 +492,17 @@ def _handle_list(self) -> None: "[dim]Auto-update is enabled — skills regenerate on plugin changes.[/dim]" ) - def _handle_setup(self, install_all: bool = False) -> None: + def _handle_setup(self, install_all: bool = False, ref: str | None = None) -> None: """Interactive first-run setup: detect AI tools and install skills. - Downloads Deepgram skills from the deepgram/skills GitHub repo and - installs both the repo skills and the local deepctl command reference - for each selected AI coding tool. + Downloads the deepgram/skills bundle (the pin unless --ref or the env + var names another ref) and installs every skill as a folder in each + selected AI coding tool's skills directory. """ import sys - from deepctl_core.skill_generator import ( - _commands_hash, - collect_command_metadata, - detect_ai_clis, - get_all_generators, - get_skills_state, - save_skills_state, - ) + from deepctl_core.skill_bundle import resolve_skills_ref + from deepctl_core.skill_generator import detect_ai_clis, get_all_generators is_tty = sys.stdout.isatty() @@ -523,35 +554,38 @@ def _handle_setup(self, install_all: bool = False) -> None: # Non-TTY without --all: install for all detected selected = list(detected) - # 3. Collect command metadata and install for selected tools + # 3. Install the skill folders for the selected tools console.print("\n[blue]Installing Deepgram skills...[/blue]") - commands = collect_command_metadata() - try: - version = importlib.metadata.version("deepctl") - except importlib.metadata.PackageNotFoundError: - version = "0.0.0" - - state = get_skills_state() - total_written: list[str] = [] - - for gen in selected: - paths = gen.install(commands, version) - cmd_hash = _commands_hash(commands) - state["installed_skills"][gen.cli_name] = { - "paths": [str(p) for p in paths], - "installed_at": datetime.now(timezone.utc).isoformat(), - "version": version, - "commands_hash": cmd_hash, - } - for p in paths: - total_written.append(str(p)) - print_success(f" {gen.display_name} → {p}") - - save_skills_state(state) - - if total_written: - console.print() - print_success(f"Setup complete — {len(total_written)} file(s) installed") - print_info("Run /deepgram:setup-mcp to configure the Deepgram MCP server.") - else: - print_info("No skills were installed.") + with _clean_errors(): + ref = resolve_skills_ref(ref) + self._install([(g, ref) for g in selected]) + + +def _version() -> str: + try: + return importlib.metadata.version("deepctl") + except importlib.metadata.PackageNotFoundError: + return "0.0.0" + + +def _n(count: int, word: str) -> str: + return f"{count} {word}{'' if count == 1 else 's'}" + + +def _label(ref: str) -> str: + from deepctl_core import skill_bundle + + pinned = ref == skill_bundle.DEFAULT_SKILLS_COMMIT + return skill_bundle.DEFAULT_SKILLS_RELEASE if pinned else ref + + +@contextlib.contextmanager +def _clean_errors() -> Iterator[None]: + """Turn a skills error into a one-line ClickException (exit 1).""" + from deepctl_core.skill_bundle import SkillFetchError + from deepctl_core.skill_generator import SkillInstallError + + try: + yield + except (SkillInstallError, SkillFetchError) as exc: + raise click.ClickException(escape(str(exc))) from exc # main.py prints markup. diff --git a/packages/deepctl-cmd-skills/tests/unit/test_skills_command.py b/packages/deepctl-cmd-skills/tests/unit/test_skills_command.py index 8564186f..e0bdce52 100644 --- a/packages/deepctl-cmd-skills/tests/unit/test_skills_command.py +++ b/packages/deepctl-cmd-skills/tests/unit/test_skills_command.py @@ -1,10 +1,127 @@ """Unit tests for skills command.""" +import errno +import hashlib +import json +import os +import shutil +import sys +import time +from pathlib import Path from unittest.mock import MagicMock, patch import click import pytest +from deepctl_cmd_skills import command from deepctl_cmd_skills.command import SkillsCommand +from deepctl_core import output, skill_bundle +from deepctl_core import skill_generator as sg +from deepctl_core.skill_bundle import RepoSkill, SkillFetchError +from deepctl_core.skill_generator import _msg, get_all_generators + + +@pytest.fixture(autouse=True) +def _throwaway_home(tmp_path, monkeypatch): + home = tmp_path / "home" + home.mkdir() + monkeypatch.setattr(Path, "home", staticmethod(lambda: home)) + monkeypatch.setenv("HOME", str(home)) + monkeypatch.setenv("USERPROFILE", str(home)) + monkeypatch.setattr(sg, "_SKILLS_DIR", home / ".deepctl" / "skills") + monkeypatch.setattr(sg, "_STATE_FILE", home / ".deepctl" / "skills" / "skills.json") + monkeypatch.setenv("COLUMNS", "400") + # Rich fixes a console's width at construction only if COLUMNS was set + # then; pin _width on each so teardown restores the old value (often None). + for con in (output.console, output.stderr_console, command.console): + monkeypatch.setattr(con, "_width", 400) + monkeypatch.delenv(skill_bundle.REF_ENV_VAR, raising=False) + # Detection must not depend on what the test machine has on PATH. + monkeypatch.setattr(shutil, "which", lambda name: None) + return home + + +@pytest.fixture(autouse=True) +def _pinned_output(): + """S4: pin the agentic output mode so prefixes never depend on the env.""" + from deepctl_core import output + + saved = dict(output._output_config) + output._output_config.update(agentic=True, format="default", quiet=False) + yield + output._output_config.clear() + output._output_config.update(saved) + + +@pytest.fixture +def bundle(tmp_path, monkeypatch): + """Patch the one fetch point; return the list of refs fetched.""" + fetched = [] + + def fetch(ref=None): + fetched.append(ref) + base = tmp_path / "bundle" + skills = [] + for name in ("api", "docs"): + folder = base / "skills" / name + folder.mkdir(parents=True, exist_ok=True) + (folder / "SKILL.md").write_bytes( + f"---\nname: {name}\n---\n{ref}\n".encode() + ) + skills.append(RepoSkill(name, folder)) + return skills + + monkeypatch.setattr(skill_bundle, "fetch_skill_bundle", fetch) + return fetched + + +@pytest.fixture(params=["set", "unset"]) +def agent_env(request, monkeypatch): + if request.param == "set": + monkeypatch.setenv("CI", "1") + monkeypatch.setenv("CLAUDECODE", "1") + monkeypatch.setenv("TERM", "xterm") + tty = True + else: + for name in ("CI", "CLAUDECODE", "CLAUDE_CODE_ENTRYPOINT"): + monkeypatch.delenv(name, raising=False) + tty = False + monkeypatch.setattr(sys.stdin, "isatty", lambda: tty, raising=False) + monkeypatch.setattr(sys.stdout, "isatty", lambda: tty, raising=False) + return request.param + + +def err_text(capsys): + return " ".join(click.unstyle(capsys.readouterr().err).split()) + + +def gen(cli): + return next(g for g in get_all_generators() if g.cli_name == cli) + + +def detect(*clis): + for cli in clis: + Path.home().joinpath(*gen(cli).homes[0]).mkdir(parents=True, exist_ok=True) + + +def sha(path): + return hashlib.sha256(Path(path).read_bytes()).hexdigest() + + +def sha_tree(path): + return { + os.path.relpath(os.path.join(d, f), path): sha(os.path.join(d, f)) + for d, _, files in os.walk(path) + for f in files + } + + +def write_state(state): + sg._STATE_FILE.parent.mkdir(parents=True, exist_ok=True) + sg._STATE_FILE.write_text(json.dumps(state), encoding="utf-8") + + +def disk_state(): + return json.loads(sg._STATE_FILE.read_bytes()) class TestSkillsCommand: @@ -29,15 +146,14 @@ def test_agent_help(self): def test_setup_commands_returns_subcommands(self): cmd = SkillsCommand() - subcommands = cmd.setup_commands() - names = {c.name for c in subcommands} - assert "install" in names - assert "update" in names - assert "remove" in names - assert "list" in names - assert "status" in names - - def test_declining_install_anyway_aborts_instead_of_exiting_zero(self): + subcommands = {c.name: c for c in cmd.setup_commands()} + assert {"install", "update", "remove", "list", "status", "setup"} <= set( + subcommands + ) + for name in ("install", "update", "setup"): + assert "--ref" in [o for p in subcommands[name].params for o in p.opts] + + def test_declining_install_anyway_aborts_instead_of_exiting_zero(self, bundle): """Declining the prompt must exit 2, not 0. `_handle_install` is a plain click callback returning None, so there @@ -47,81 +163,661 @@ def test_declining_install_anyway_aborts_instead_of_exiting_zero(self): Abort is what main.py turns into 2. """ cmd = SkillsCommand() - generator = MagicMock() - generator.cli_name = "claude" - generator.display_name = "Claude Code" - generator.detect.return_value = False - - with ( - patch( - "deepctl_core.skill_generator.get_all_generators", - return_value=[generator], - ), - patch.object(cmd, "confirm", return_value=False), - ): + with patch.object(cmd, "confirm", return_value=False): with pytest.raises(click.Abort): cmd._handle_install(cli_name="claude") + assert bundle == [] + assert not gen("claude").skills_root().exists() - generator.install.assert_not_called() - - def test_accepting_install_anyway_does_not_abort(self): + def test_accepting_install_anyway_does_not_abort(self, bundle): """Positive control: confirming must proceed to the install.""" cmd = SkillsCommand() - generator = MagicMock() - generator.cli_name = "claude" - generator.display_name = "Claude Code" - generator.detect.return_value = False - generator.install.return_value = [] - - with ( - patch( - "deepctl_core.skill_generator.get_all_generators", - return_value=[generator], - ), - patch( - "deepctl_core.skill_generator.collect_command_metadata", - return_value={}, - ), - patch( - "deepctl_core.skill_generator.get_skills_state", - return_value={"installed_skills": {}}, - ), - patch("deepctl_core.skill_generator.save_skills_state"), - patch.object(cmd, "confirm", return_value=True), - ): + with patch.object(cmd, "confirm", return_value=True): cmd._handle_install(cli_name="claude") - - generator.install.assert_called_once() + assert (gen("claude").skills_root() / "api" / "SKILL.md").is_file() def test_unknown_cli_exits_one(self): """An unknown --cli must exit 1, not print an error and exit 0.""" cmd = SkillsCommand() - with patch( - "deepctl_core.skill_generator.get_all_generators", - return_value=[], - ): - with pytest.raises(click.ClickException) as exc: - cmd._handle_install(cli_name="nonexistent-cli") + with pytest.raises(click.ClickException) as exc: + cmd._handle_install(cli_name="nonexistent-cli") assert "Unknown AI CLI: nonexistent-cli" in str(exc.value) def test_removing_skills_that_are_not_installed_exits_one(self): """`skills remove --cli X` with nothing installed for X exits 1.""" + write_state({"installed_skills": {"claude": {"paths": []}}}) cmd = SkillsCommand() + with pytest.raises(click.ClickException) as exc: + cmd._handle_remove(cli_name="cursor", remove_all=False) + + assert "No skills installed for 'cursor'" in str(exc.value) + + +class TestAgentPrefixes: + """S4: the OK/INFO prefixes hold with CI, CLAUDECODE and a TTY set or unset.""" + + def test_install_success_line_has_ok_prefix(self, agent_env, bundle, capsys): + detect("claude") + SkillsCommand()._handle_install(install_all=True) + assert "OK: Claude Code: installed 2 skills in" in err_text(capsys) + + def test_list_hint_has_info_prefix(self, agent_env, capsys): + SkillsCommand()._handle_list() + assert ( + "INFO: No skill folders are installed; run 'dg skills install' to get started." + in err_text(capsys) + ) + + +class TestSkillsFlows: + def test_preflight_across_tools_writes_nothing_on_conflict(self, bundle): + detect("claude", "cursor") + mine = gen("cursor").skills_root() / "api" + mine.mkdir(parents=True) + (mine / "SKILL.md").write_bytes(b"mine") + with pytest.raises(click.ClickException) as exc: + SkillsCommand()._handle_install(install_all=True) + assert exc.value.exit_code == 1 + assert exc.value.message == _msg("E1", paths=str(mine)) + assert not gen("claude").skills_root().exists() + assert not sg._STATE_FILE.exists() + + def test_fetch_failure_writes_nothing(self, monkeypatch): + detect("claude") + + def fail(ref=None): + raise SkillFetchError("Could not download the bundle.") + + monkeypatch.setattr(skill_bundle, "fetch_skill_bundle", fail) + with pytest.raises(click.ClickException) as exc: + SkillsCommand()._handle_install(install_all=True) + assert exc.value.message == "Could not download the bundle." + assert not gen("claude").skills_root().exists() + assert not sg._STATE_FILE.exists() + + def test_hint_only_tool_prints_hint_exit_zero(self, bundle, capsys): + detect("amazonq") + SkillsCommand()._handle_install(cli_name="amazonq") + err = err_text(capsys) + assert _msg("E15", gen("amazonq")) in err + assert "No skills were installed." in err + assert bundle == [] + + def test_update_targets_records_and_03x_entries(self, bundle): + old = Path.home() / ".codex" / "instructions.md" + old.parent.mkdir() + old.write_bytes(b"user text\n\nold\n") + before = sha(old) + write_state( + {"installed_skills": {"codex": {"paths": [str(old)], "version": "0.3.2"}}} + ) + SkillsCommand()._handle_update() + assert (gen("codex").skills_root() / "api" / sg._MARKER).is_file() + assert sha(old) == before + assert set(disk_state()["skill_folders"]["codex"]["folders"]) == {"api", "docs"} + + @pytest.mark.parametrize("how", ["recorded", "--ref", "env"]) + def test_update_follows_recorded_ref_unless_overridden( + self, bundle, monkeypatch, how + ): + detect("claude") + SkillsCommand()._handle_install(install_all=True, ref="my-branch") + assert bundle == ["my-branch"] + if how == "env": + monkeypatch.setenv(skill_bundle.REF_ENV_VAR, "env-ref") + SkillsCommand()._handle_update(ref="other" if how == "--ref" else None) + assert ( + bundle[-1] + == {"recorded": "my-branch", "--ref": "other", "env": "env-ref"}[how] + ) + + def test_only_this_runs_staging_gets_e12(self, bundle, monkeypatch, capsys): + detect("claude") + root = gen("claude").skills_root() + seeded = [root / ".deepctl-staging-old1", root / ".deepctl-staging-mine"] + for d in seeded: + d.mkdir(parents=True) + (d / "f").write_bytes(b"x") + SkillsCommand()._handle_install(install_all=True) + assert "could not finish cleaning up" not in err_text(capsys) + SkillsCommand()._handle_status() + err = err_text(capsys) + for d in seeded: + assert _msg("E16", path=d) in err + real = shutil.rmtree + + def keep_this_runs_copies(path, *a, **k): + if Path(path).parent.name == "old": + return None + return real(path, *a, **k) + + monkeypatch.setattr(shutil, "rmtree", keep_this_runs_copies) + SkillsCommand()._handle_update() + err = err_text(capsys) + assert err.count("could not finish cleaning up") == 1 + new = [ + n + for n in os.listdir(root) + if n.startswith(".deepctl-staging-") and root / n not in seeded + ] + assert len(new) == 1 + assert _msg("E12", staging=root / new[0]) in err + assert all(str(d) not in err for d in seeded) + + def test_remove_exit_one_when_folder_kept(self, bundle, monkeypatch, capsys): + detect("claude") + SkillsCommand()._handle_install(install_all=True) + api = gen("claude").skills_root() / "api" + real = os.rename + + def deny(src, dst, *a, **k): + if Path(src) == api: + raise PermissionError(errno.EACCES, "Permission denied") + return real(src, dst, *a, **k) + + monkeypatch.setattr(os, "rename", deny) + capsys.readouterr() + with pytest.raises(click.ClickException) as exc: + SkillsCommand()._handle_remove(remove_all=True) + assert exc.value.exit_code == 1 + assert exc.value.message == ( + "Some skill folders were not fully removed; fix the problems listed above." + ) + assert _msg("E13", dest=api, reason="Permission denied") in err_text(capsys) + assert api.is_dir() + + def test_remove_leaves_an_empty_folder_at_a_recorded_name(self, bundle, capsys): + detect("claude") + SkillsCommand()._handle_install(install_all=True) + docs = gen("claude").skills_root() / "docs" + shutil.rmtree(docs) + docs.mkdir() # deepctl never makes one, so it is not deepctl's. + capsys.readouterr() + SkillsCommand()._handle_remove(remove_all=True) # Exit 0. + err = err_text(capsys) + assert _msg("E26", dest=docs) in err + assert "still recorded" not in err + assert "Removed 1 skill folder from 1 tool." in err + assert "claude" not in sg.get_skills_state().get("skill_folders", {}) + assert docs.is_dir() and os.listdir(docs) == [] + + def test_install_while_another_command_holds_the_lock_exits_one( + self, bundle, monkeypatch + ): + detect("claude") + monkeypatch.setattr(sg, "_LOCK_TIMEOUT", 0.3) + fd = sg._try_lock(sg._STATE_FILE.with_name("skills.json.lock")) + assert fd >= 0 # As another deepctl process would. + try: + with pytest.raises(click.ClickException) as exc: + SkillsCommand()._handle_install(install_all=True) + finally: + if sys.platform == "win32": + sg.msvcrt.locking(fd, sg.msvcrt.LK_UNLCK, 1) + os.close(fd) + assert (exc.value.exit_code, exc.value.message) == (1, _msg("E27")) + assert not sg._STATE_FILE.exists() + assert not gen("claude").skills_root().exists() + + def test_remove_waits_for_the_lock_once_for_all_tools( + self, bundle, monkeypatch, capsys + ): + detect("claude", "cursor") + SkillsCommand()._handle_install(install_all=True) + roots = [gen(c).skills_root() for c in ("claude", "cursor")] + before = sg._STATE_FILE.read_bytes(), [sha_tree(r) for r in roots] + capsys.readouterr() + monkeypatch.setattr(sg, "_LOCK_TIMEOUT", 0.5) + calls = [] + real_remove = sg.remove_tool + monkeypatch.setattr( + sg, "remove_tool", lambda g: calls.append(g) or real_remove(g) + ) + fd = sg._try_lock(sg._STATE_FILE.with_name("skills.json.lock")) + assert fd >= 0 # As another deepctl process would. + try: + start = time.monotonic() + with pytest.raises(click.ClickException) as exc: + SkillsCommand()._handle_remove(remove_all=True) + took = time.monotonic() - start + finally: + if sys.platform == "win32": + sg.msvcrt.locking(fd, sg.msvcrt.LK_UNLCK, 1) + os.close(fd) + assert (exc.value.exit_code, exc.value.message) == (1, _msg("E27")) + assert 0.5 <= took < 1.0 # One wait, not one per tool. + assert calls == [] and _msg("E27") not in err_text(capsys) + assert (sg._STATE_FILE.read_bytes(), [sha_tree(r) for r in roots]) == before + + def test_fetch_runs_without_the_lock(self, bundle, monkeypatch): + detect("claude") + real = skill_bundle.fetch_skill_bundle + + def fetch(ref=None): + assert getattr(sg._LOCAL, "fd", None) is None + return real(ref) + + monkeypatch.setattr(skill_bundle, "fetch_skill_bundle", fetch) + SkillsCommand()._handle_install(install_all=True) + assert bundle and (gen("claude").skills_root() / "api").is_dir() + + @pytest.mark.parametrize("cli", ["claude", "cursor"]) + def test_setup_mcp_hint_only_after_claude_install(self, bundle, capsys, cli): + detect(cli) + SkillsCommand()._handle_install(install_all=True) + err = err_text(capsys) + assert "Installed 2 skill folders for 1 tool from" in err + assert f"from deepgram/skills {skill_bundle.DEFAULT_SKILLS_RELEASE}." in err + hint = "In Claude Code, run /setup-mcp to configure the Deepgram MCP server." + assert (hint in err) == (cli == "claude") + assert "/deepgram:setup-mcp" not in err + + def test_failed_install_reports_its_staging(self, bundle, monkeypatch, capsys): + detect("claude") + staging = gen("claude").skills_root() / ".deepctl-staging-x" + + def fail(*a, **k): + exc = sg.SkillInstallError("boom") + exc.leftover = staging + raise exc + + monkeypatch.setattr(sg, "install_tool", fail) + with pytest.raises(click.ClickException) as exc: + SkillsCommand()._handle_install(install_all=True) + assert exc.value.message == "boom" + assert _msg("E12", staging=staging) in err_text(capsys) + + def test_remove_exit_one_when_staging_left(self, bundle, monkeypatch, capsys): + detect("claude") + SkillsCommand()._handle_install(install_all=True) + monkeypatch.setattr(shutil, "rmtree", lambda *a, **k: None) + capsys.readouterr() + with pytest.raises(click.ClickException) as exc: + SkillsCommand()._handle_remove(remove_all=True) + assert exc.value.exit_code == 1 + assert "could not finish cleaning up" in err_text(capsys) + + @pytest.mark.parametrize("old", [True, False]) + def test_remove_notes_03x_files_left_behind(self, bundle, capsys, old): + detect("claude") + if old: + write_state( + { + "installed_skills": { + "claude": { + "paths": [ + str( + Path.home() + / ".claude" + / "commands" + / "deepgram" + / "api.md" + ) + ] + } + } + } + ) + SkillsCommand()._handle_install(install_all=True) + SkillsCommand()._handle_update() # Its own folders are not 0.3.x files. + capsys.readouterr() + SkillsCommand()._handle_remove(remove_all=True) + err = err_text(capsys) + assert ("0.3.x" in err) == old + assert ( + "For Claude Code, files from deepctl 0.3.x stay until a later release." + in err + ) == old + assert "OK: Removed 2 skill folders from 1 tool." in err + + def test_remove_03x_only_tool_message_and_files_untouched(self, capsys): + old = Path.home() / ".cursor" / "rules" / "deepctl.mdc" + old.parent.mkdir(parents=True) + old.write_bytes(b"0.3.x rules") + before = sha(old) + write_state( + {"installed_skills": {"cursor": {"paths": [str(old)]}}, "auto_update": True} + ) + SkillsCommand()._handle_remove(remove_all=True) + err = err_text(capsys) + assert "Cursor has no skill folders recorded, so nothing was removed" in err + assert "OK: Removed 0 skill folders from 0 tools." in err + assert sha(old) == before + assert "cursor" not in disk_state()["installed_skills"] + + def test_remove_tool_with_no_records_or_paths_says_so(self, capsys): + write_state({"installed_skills": {"cursor": {"paths": []}}}) + SkillsCommand()._handle_remove(remove_all=True) + assert err_text(capsys) == ( + "INFO: Cursor has no skill folders recorded, so nothing was removed. " + "OK: Removed 0 skill folders from 0 tools." + ) + + def test_remove_tool_error_is_printed_and_exits_one(self, bundle, capsys): + detect("claude", "cursor") + SkillsCommand()._handle_install(install_all=True) + shutil.rmtree(gen("claude").skills_root()) + gen("claude").skills_root().write_bytes(b"not a folder") + capsys.readouterr() + with pytest.raises(click.ClickException) as exc: + SkillsCommand()._handle_remove(remove_all=True) + assert exc.value.exit_code == 1 + assert exc.value.message == ( + "Some skill folders were not fully removed; fix the problems listed above." + ) + assert f"ERROR: {_msg('E18', gen('claude'))}" in err_text(capsys) + assert not (gen("cursor").skills_root() / "api").exists() # It went on. + assert "claude" in disk_state()["skill_folders"] + + def test_remove_e21_is_printed_and_exits_one(self, bundle, monkeypatch, capsys): + detect("claude") + SkillsCommand()._handle_install(install_all=True) + saved = sg._STATE_FILE.read_bytes() + + def denied(*a, **k): + raise PermissionError(errno.EACCES, "Permission denied") + + monkeypatch.setattr(sg.tempfile, "mkdtemp", denied) + capsys.readouterr() + with pytest.raises(click.ClickException) as exc: + SkillsCommand()._handle_remove(remove_all=True) + assert exc.value.exit_code == 1 + why = _msg("E21", root=gen("claude").skills_root(), reason="Permission denied") + assert f"ERROR: {why}" in err_text(capsys) + assert sg._STATE_FILE.read_bytes() == saved + assert (gen("claude").skills_root() / "api").is_dir() + + def test_setup_passes_ref_through(self, bundle): + detect("claude") + SkillsCommand()._handle_setup(install_all=True, ref="my-branch") + assert bundle == ["my-branch"] + assert disk_state()["skill_folders"]["claude"]["skills_ref"] == "my-branch" + + def test_corrupt_state_fails_before_any_prompt(self, bundle): + detect("claude") + sg._STATE_FILE.parent.mkdir(parents=True) + sg._STATE_FILE.write_bytes(b"{") + cmd = SkillsCommand() + confirm = MagicMock(return_value=True) with ( - patch( - "deepctl_core.skill_generator.get_skills_state", - return_value={"installed_skills": {"claude": {"paths": []}}}, - ), - patch( - "deepctl_core.skill_generator.get_all_generators", - return_value=[], - ), + patch.object(cmd, "confirm", confirm), + pytest.raises(click.ClickException) as exc, ): - with pytest.raises(click.ClickException) as exc: - cmd._handle_remove(cli_name="cursor", remove_all=False) + cmd._handle_install() + assert exc.value.message == _msg("E7") + confirm.assert_not_called() + assert bundle == [] + + def test_update_with_nothing_installed_says_so(self, capsys): + SkillsCommand()._handle_update() + assert err_text(capsys) == ( + "INFO: No skills are installed, so there is nothing to update; " + "run 'dg skills install' first." + ) - assert "No skills installed for 'cursor'" in str(exc.value) + def test_status_counts_only_proven_folders(self, bundle, capsys): + detect("claude") + SkillsCommand()._handle_install(install_all=True) + with open(gen("claude").skills_root() / "docs" / "SKILL.md", "ab") as f: + f.write(b"mine\n") + capsys.readouterr() + SkillsCommand()._handle_status() + (row,) = [r for r in capsys.readouterr().out.splitlines() if "Claude Code" in r] + assert row.split("│")[-2].strip() == "1" + + def test_setup_mcp_hint_when_claude_is_not_first(self, bundle, capsys): + ref = skill_bundle.DEFAULT_SKILLS_COMMIT + SkillsCommand()._install([(gen("cursor"), ref), (gen("claude"), ref)]) + hint = "In Claude Code, run /setup-mcp to configure the Deepgram MCP server." + assert hint in err_text(capsys) + + def test_status_ignores_a_recorded_folder_the_user_deleted(self, bundle, capsys): + detect("claude") + SkillsCommand()._handle_install(install_all=True) + api = gen("claude").skills_root() / "api" + shutil.rmtree(api) + capsys.readouterr() + SkillsCommand()._handle_status() + err = err_text(capsys) + assert str(api) not in err + assert "cannot prove" not in err + + @pytest.mark.parametrize( + "sub", ["status", "install", "update", "remove", "list", "setup"] + ) + def test_corrupt_state_exits_one_file_unchanged(self, bundle, sub): + detect("claude") + write_state({}) + sg._STATE_FILE.write_bytes(b"{") + cmd = SkillsCommand() + call = { + "status": cmd._handle_status, + "install": lambda: cmd._handle_install(install_all=True), + "update": cmd._handle_update, + "remove": lambda: cmd._handle_remove(remove_all=True), + "list": cmd._handle_list, + "setup": lambda: cmd._handle_setup(install_all=True), + }[sub] + with pytest.raises(click.ClickException) as exc: + call() + assert exc.value.exit_code == 1 + assert exc.value.message == _msg("E7") + assert sg._STATE_FILE.read_bytes() == b"{" + assert not gen("claude").skills_root().exists() + + def test_status_and_list_report_unproven_leftovers_and_03x_note( + self, bundle, monkeypatch, capsys + ): + detect("claude", "amazonq") + SkillsCommand()._handle_install(cli_name="claude") + root = gen("claude").skills_root() + (root / "api" / sg._MARKER).unlink() + with open(root / "docs" / "SKILL.md", "ab") as f: + f.write(b"mine\n") + leftover = root / ".deepctl-staging-x" + leftover.mkdir() + state = disk_state() + state["installed_skills"]["codex"] = {"paths": ["/old/instructions.md"]} + state["installed_skills"]["amazonq"] = {"paths": ["/old/deepctl.md"]} + write_state(state) + capsys.readouterr() + SkillsCommand()._handle_status() + err = err_text(capsys) + assert _msg("E14", dest=root / "api") in err + assert _msg("E24", dest=root / "docs") in err + assert _msg("E16", path=leftover) in err + assert _msg("E15", gen("amazonq")) in err + assert _msg("E15", gen("aider")) not in err # Not detected. + assert "Files from deepctl 0.3.x are recorded for OpenAI Codex;" in err + SkillsCommand()._handle_list() + out = capsys.readouterr() + assert "0/2" in out.out + assert skill_bundle.DEFAULT_SKILLS_RELEASE in out.out + assert "0.3.x" not in " ".join(out.err.split()) + + target = os.fspath(root / "docs" / "SKILL.md") + real = os.open + + def deny(path, *a, **k): + if os.fspath(path) == target: + raise PermissionError(errno.EACCES, "Permission denied") + return real(path, *a, **k) + + monkeypatch.setattr(os, "open", deny) + SkillsCommand()._handle_status() + assert _msg("E25", dest=root / "docs") in err_text(capsys) + + def test_update_halts_every_tool_when_one_folder_is_edited(self, bundle): + detect("claude", "cursor") + SkillsCommand()._handle_install(install_all=True) + with open(gen("cursor").skills_root() / "api" / "SKILL.md", "ab") as f: + f.write(b"mine\n") + claude, saved = ( + sha_tree(gen("claude").skills_root()), + sg._STATE_FILE.read_bytes(), + ) + with pytest.raises(click.ClickException) as exc: + SkillsCommand()._handle_update() + assert exc.value.exit_code == 1 + assert sha_tree(gen("claude").skills_root()) == claude + assert sg._STATE_FILE.read_bytes() == saved + + def test_update_warns_about_unknown_recorded_cli(self, capsys): + write_state({"installed_skills": {"vim": {"paths": []}}}) + SkillsCommand()._handle_update() + assert "Unknown CLI 'vim', skipping." in err_text(capsys) + + def test_remove_moved_folder_reports_e4_and_exits_one(self, monkeypatch, capsys): + write_state({"installed_skills": {"claude": {"paths": []}}}) + api = gen("claude").skills_root() / "api" + aside = api.parent / ".deepctl-staging-x" / "old" / "api" + res = sg.RemoveResult(moved=[(api, aside)]) + monkeypatch.setattr(sg, "remove_tool", lambda g: res) + with pytest.raises(click.ClickException) as exc: + SkillsCommand()._handle_remove(remove_all=True) + assert exc.value.exit_code == 1 + assert _msg("E4", dest=api, aside=aside) in err_text(capsys) + + def test_remove_notes_03x_files_after_a_plugin_refresh(self, bundle, capsys): + detect("claude") + old = Path.home() / ".claude" / "commands" / "deepgram" / "api.md" + rule = Path.home() / ".amazonq" / "rules" / "deepctl.md" + write_state( + { + "installed_skills": { + "claude": {"paths": [str(old)]}, + "amazonq": {"paths": [str(rule)]}, + } + } + ) + state = sg.get_skills_state() # The plugin flow: shim, then a stale save. + for cli, info in state["installed_skills"].items(): + info.update(paths=[str(p) for p in gen(cli).install([], "x")]) + sg.save_skills_state(state) + SkillsCommand()._handle_update() # A later write keeps the flag. + capsys.readouterr() + SkillsCommand()._handle_remove(remove_all=True) + err, v03 = ( + err_text(capsys), + "files from deepctl 0.3.x stay until a later release.", + ) + assert f"For Claude Code, {v03}" in err + assert ( + f"Amazon Q Developer has no skill folders recorded, so nothing was removed; {v03}" + in err + ) + + def test_update_edited_folder_exits_one_with_rename_advice(self, bundle): + detect("claude") + SkillsCommand()._handle_install(install_all=True) + api = gen("claude").skills_root() / "api" + with open(api / "SKILL.md", "ab") as f: + f.write(b"mine\n") + with pytest.raises(click.ClickException) as exc: + SkillsCommand()._handle_update() + assert exc.value.exit_code == 1 + assert " ".join(click.unstyle(exc.value.format_message()).split()) == _msg( + "E22", dest=api + ) + + def test_remove_edited_folder_exits_zero_and_reports(self, bundle, capsys): + detect("claude") + SkillsCommand()._handle_install(install_all=True) + api = gen("claude").skills_root() / "api" + with open(api / "SKILL.md", "ab") as f: + f.write(b"mine\n") + capsys.readouterr() + SkillsCommand()._handle_remove(remove_all=True) + err = err_text(capsys) + assert _msg("E23", dest=api) in err + assert "OK: Removed 1 skill folder from 1 tool." in err + assert api.is_dir() + + def test_remove_reports_folder_without_marker(self, bundle, capsys): + detect("claude") + SkillsCommand()._handle_install(install_all=True) + api = gen("claude").skills_root() / "api" + (api / sg._MARKER).unlink() + SkillsCommand()._handle_remove(remove_all=True) + err = err_text(capsys) + assert _msg("E26", dest=api) in err + assert _msg("E14", dest=api) not in err + assert api.is_dir() + assert "claude" not in disk_state().get("skill_folders", {}) + + def test_upgrade_home_from_main_has_no_data_loss(self, bundle, capsys): + home = Path.home() + files = { + ".claude/commands/deepgram/api.md": b"api", + ".claude/commands/deepgram/docs.md": b"docs", + ".claude/commands/deepgram/setup-mcp.md": b"setup", + ".claude/commands/deepgram/starters.md": b"starters", + ".claude/commands/deepgram/mine.md": b"the user's own command", + ".codex/instructions.md": b"user text\nx\n", + ".gemini/GEMINI.md": b"x\n", + ".opencode/agents.md": b"x\n", + ".amazonq/rules/deepctl.md": b"q", + ".cursor/rules/deepctl.mdc": b"cursor", + ".cline/rules/deepctl.md": b"cline", + ".deepctl/skills/deepctl-conventions.md": b"aider", + ".aider.conf.yml": b"read: [x]\n", + ".deepctl/skills/repo_cache/api.md": b"cache", + ".deepctl/skills/repo_cache/.fetched": b"1", + } + for rel, data in files.items(): + path = home.joinpath(*rel.split("/")) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(data) + tools = ( + "claude", + "codex", + "gemini", + "amazonq", + "aider", + "opencode", + "cursor", + "cline", + ) + legacy = { + t: { + "paths": ["x"], + "installed_at": "2026-01-02T03:04:05+00:00", + "version": "0.3.2", + "commands_hash": "sha256:ab12", + } + for t in tools + } + write_state({"installed_skills": legacy, "auto_update": True}) + before = {rel: sha(home.joinpath(*rel.split("/"))) for rel in files} + + def unchanged(): + return {rel: sha(home.joinpath(*rel.split("/"))) for rel in files} == before + + def fingerprinted(): + for tool in disk_state()["skill_folders"].values(): + for rec in tool["folders"].values(): + assert rec["state"] == "installed" + assert sg._FP_PATTERN.fullmatch(rec["fingerprint"]) + assert "pending" not in rec + + cmd = SkillsCommand() + cmd._handle_status() + assert "Run 'dg skills install'" not in err_text(capsys) # 0.3.x recorded. + cmd._handle_install(install_all=True) + assert unchanged() + fingerprinted() + assert len(disk_state()["skill_folders"]) == 6 + assert disk_state()["installed_skills"] == legacy + cmd._handle_update() + assert unchanged() + fingerprinted() + cmd._handle_remove(remove_all=True) + assert unchanged() + assert disk_state().get("skill_folders", {}) == {} + for g in get_all_generators(): + if g.skills_root() is not None: + assert os.listdir(g.skills_root()) == [] class TestSkillsStartupCheck: @@ -168,7 +864,6 @@ def test_suppressed_when_quiet(self): ) def test_broken_pipe_swallowed(self, error): """A closed/broken stderr (e.g. `dg mcp` host disconnect) is tolerated.""" - import sys import threading from deepctl_cmd_skills import startup_check diff --git a/packages/deepctl-core/src/deepctl_core/skill_bundle.py b/packages/deepctl-core/src/deepctl_core/skill_bundle.py index 4fd6e2db..53406918 100644 --- a/packages/deepctl-core/src/deepctl_core/skill_bundle.py +++ b/packages/deepctl-core/src/deepctl_core/skill_bundle.py @@ -105,6 +105,12 @@ def _cache_name(ref: str) -> str: return "ref-" + ref.replace("/", "%2F") +def portable_name(name: str) -> bool: + """True if ``name`` is one plain folder name safe on every OS (as entries).""" + match = _ENTRY_PATTERN.fullmatch(f"skills/{name}") + return bool(match) and not _WINDOWS_UNSAFE.fullmatch(name) + + def validate_ref(ref: str) -> str: """Return ``ref`` if it is safe in a URL path and a cache name, else raise.""" if len(ref) > _MAX_REF_LENGTH: diff --git a/packages/deepctl-core/src/deepctl_core/skill_generator.py b/packages/deepctl-core/src/deepctl_core/skill_generator.py index d6ade6e2..3ef36b55 100644 --- a/packages/deepctl-core/src/deepctl_core/skill_generator.py +++ b/packages/deepctl-core/src/deepctl_core/skill_generator.py @@ -1,19 +1,47 @@ -"""Skill generator for AI coding assistant integration. +"""Install the Deepgram skills as folders in each AI coding tool's skills root. -Generates skill/instruction files that teach AI coding CLIs (Claude Code, -Codex, Gemini CLI, etc.) how to use deepctl. +deepctl replaces or deletes a skill folder only when skills.json records it for +that tool, it is a real folder directly in the tool's root, it holds the exact +``.deepctl-skill`` marker, and its contents still hash to what deepctl placed. """ from __future__ import annotations +import contextlib +import copy +import ctypes +import errno +import functools import hashlib import json +import os +import platform +import re import shutil -from abc import ABC, abstractmethod -from dataclasses import dataclass +import stat +import sys +import tempfile +import threading +import time +import uuid +from dataclasses import dataclass, field +from datetime import datetime, timezone from importlib import metadata from pathlib import Path -from typing import Any +from typing import TYPE_CHECKING, Any + +from deepctl_core import skill_bundle +from deepctl_core.skill_bundle import portable_name + +if sys.platform == "win32": + import msvcrt +else: + import fcntl + +if TYPE_CHECKING: + from collections.abc import Callable, Iterator, Sequence + + from deepctl_core.skill_bundle import RepoSkill # --------------------------------------------------------------------------- # Data model @@ -41,83 +69,243 @@ class CommandMetadata: # State management # --------------------------------------------------------------------------- +# Tests patch these two, so read them at call time, never in a default. _SKILLS_DIR = Path.home() / ".deepctl" / "skills" _STATE_FILE = _SKILLS_DIR / "skills.json" -_REPO_CACHE_DIR = _SKILLS_DIR / "repo_cache" -_SKILLS_REPO = "deepgram/skills" -_SKILLS_BRANCH = "main" +_RECORDS_KEY = "skill_folders" +_MARKER = ".deepctl-skill" +_STAGING_PREFIX = ".deepctl-staging-" +_STATES = ("installed", "installing") +_MAX_MARKER_BYTES = 512 +_MAX_TREE_BYTES = 64 << 20 # fingerprint read cap; a folder holding more is "edited" +_FP_PATTERN = re.compile(r"sha256:[0-9a-f]{64}") +_FP_DOMAIN = b"deepctl-skill-tree-v1\0" +_NO_REPLACE = (errno.EEXIST, errno.ENOTEMPTY, errno.ENOTDIR) +_WINDOWS = os.name == "nt" # patched by tests to exercise the Windows branch +_LOCK_TIMEOUT = 30.0 # seconds; tests patch it +_LOCAL = threading.local() # .fd: this thread's lock, so nested takes never wait +_BUSY = (errno.EAGAIN, errno.EWOULDBLOCK, errno.EACCES, errno.EDEADLK, errno.ENOENT) +_NO_EXCL = "this filesystem cannot move a folder without the risk of replacing one; deepctl needs this folder on a local disk" +_NO_EXCL_SYS = "this system cannot move a folder without the risk of replacing one; deepctl needs macOS, Windows, or Linux 3.15 or later on a supported architecture with a Python that matches the kernel's word size" +# renameat2 numbers by uname machine for [64-bit, 32-bit] Python. Linux calls them raw, so errno is the kernel's +# (glibc 2.28+ turns ENOSYS into EINVAL); other pairs, such as x32, need the renameat2 wrapper or fail closed. +_NR_RENAMEAT2 = ( + {"x86_64": 316, "aarch64": 276, "arm64": 276, "riscv64": 276, "s390x": 347} + | {"ppc64": 357, "ppc64le": 357}, + {"i386": 353, "i686": 353, "armv7l": 382, "armv6l": 382, "arm": 382}, +) + +# One sentence each. {file} is skills.json; {display} and {root} name the tool. +_MSG = { + "E1": "deepctl cannot prove it installed {paths}, so it will not replace anything there; move or rename what is there, then run the command again.", + "E2": "{dest} appeared while deepctl was installing, so it was left alone and {display} was not fully installed.", + "E3": "{dest} changed while deepctl was replacing it, so it was left in place and {display} was not fully installed.", + "E4": "{dest} changed while deepctl was replacing or removing it, so what was there is now in {aside}; move it back by hand if you need it.", + "E5": "Could not install the skills for {display} in {root}: {reason}.", + "E6": "Could not install {name} for {display}, and its previous copy could not be put back, so deepctl kept it in {aside}; move it back by hand.", + "E6b": "{name} was not updated for {display} because something appeared at its folder during the update, so its previous copy is back in place; check that folder, then run the command again.", + "E7": "{file} is not a skills record deepctl can read, so deepctl will not change it; fix or delete that file, then run the command again.", + "E8": "Could not read {file}: {reason}.", + "E9": "Could not save {file}: {reason}, so nothing was installed for {display}.", + "E9b": "The skills for {display} are installed, but {file} could not be saved: {reason}; the next 'dg skills' command still treats them as deepctl's.", + "E9c": "Could not save {file}: {reason}.", + "E10": "The skill {name} in the bundle contains a .deepctl-skill file, a link or a special file, or is larger than 64 MiB, so deepctl will not install it.", + "E11": "The skill name {name!r} is not a plain folder name, so deepctl will not install it.", + "E12": "deepctl could not finish cleaning up {staging}, so it was left in place; check it and delete it by hand.", + "E13": "Could not remove {dest}: {reason}; it is still recorded, so check its permissions or close any tool using it, then run 'dg skills remove' again.", + "E14": "{dest} is recorded but deepctl cannot prove it installed it, so it was left in place.", + "E15": "{display} does not load skill folders, so deepctl does not install skills for it; see https://github.com/deepgram/skills to add the skills by hand.", + "E16": "{path} looks like staging from an interrupted deepctl run; deepctl never deletes it, so check it and delete it by hand.", + "E18": "{root} exists but is not a folder, so deepctl changed nothing for {display}; move it away or point it at a folder, then run the command again.", + "E21": "Could not remove the skills from {root}: {reason}.", + "E22": "{dest} was edited since deepctl installed it, so deepctl left it alone and did not install over it; rename or move your edited folder, then run the command again.", + "E23": "{dest} was edited since deepctl installed it, so deepctl left it in place and no longer tracks it; delete it yourself if you don't need it.", + "E24": "{dest} was edited since deepctl installed it, so 'dg skills remove' leaves it alone and 'dg skills update' stops until you rename or move it to keep your edits, or delete it to get deepctl's copy back.", + "E25": "deepctl could not read {dest}, so it cannot tell whether that folder is still its own copy; check its permissions or close any tool using it, then run the command again.", + "E26": "deepctl cannot prove it installed {dest}, so it left it in place and no longer tracks it.", + "E27": "Another deepctl command is installing or removing skills, so this one waited 30 seconds and changed nothing; wait for it to finish, then run the command again.", + "E29": "An interrupted deepctl run left the previous copy of {dest} in {aside}; delete it, or move it out of the skills folder if you want to keep it, then run the command again.", + "E28": "Could not lock {lock}: {reason}, so deepctl changed nothing; check that you own that file and its folder and that they are on a local disk, then run the command again.", +} + + +def _msg(key: str, gen: SkillGenerator | None = None, **kw: Any) -> str: + if gen is not None: + kw.update(display=gen.display_name, root=gen.skills_root()) + return _MSG[key].format(file=_STATE_FILE, **kw) + + +def _err(key: str, gen: SkillGenerator | None = None, **kw: Any) -> SkillInstallError: + return SkillInstallError(_msg(key, gen, **kw)) + + +def _reason(exc: BaseException) -> str: + if isinstance(exc, shutil.Error) and exc.args and isinstance(exc.args[0], list): + exc = OSError(re.sub(r"^\[\w+ \d+\] |: '.*", "", str(exc.args[0][0][-1]))) + return str(getattr(exc, "strerror", None) or exc).rstrip(".") + + +class SkillInstallError(Exception): + """An install, remove or skills.json failure; the message is one sentence per problem.""" + + leftover: Path | None = None # This run's staging, if cleanup left it (SF3). + kept: Path | None = None # E4 or E6: where what was at dest was kept. + + +class SkillOwnershipError(SkillInstallError): + """deepctl cannot prove it owns ``paths``, or the user edited ``edited``.""" + + def __init__( + self, paths: list[Path], edited: Sequence[Path] = (), message: str | None = None + ) -> None: + self.paths, self.edited = list(paths), list(edited) + parts = [_msg("E1", paths=", ".join(map(str, paths)))] if paths else [] + parts += [_msg("E22", dest=p) for p in edited] + super().__init__(message or " ".join(parts)) + + +def _validated(raw: Any) -> dict[str, Any]: + """Return ``raw`` with defaults filled in, or raise E7 if it is not valid.""" + tools = raw.get(_RECORDS_KEY, {}) if isinstance(raw, dict) else None + legacy = raw.get("installed_skills", {}) if isinstance(raw, dict) else None + if not isinstance(tools, dict) or not isinstance(legacy, dict): + raise _err("E7") + for info in legacy.values(): + paths = info.get("paths", []) if isinstance(info, dict) else None + if not isinstance(paths, list) or not all(isinstance(p, str) for p in paths): + raise _err("E7") + for tool in tools.values(): + folders = tool.get("folders", {}) if isinstance(tool, dict) else None + if ( + not isinstance(folders, dict) + or not isinstance(tool.get("skills_ref", ""), str) + or not isinstance(tool.get("v03", False), bool) + ): + raise _err("E7") + for name, rec in folders.items(): + ok = portable_name(name) and isinstance(rec, dict) + fps = [rec[k] for k in ("fingerprint", "pending") if k in rec] if ok else [] + if ( + not ok + or rec.get("state") not in _STATES + or not all(isinstance(v, str) and _FP_PATTERN.fullmatch(v) for v in fps) + ): + raise _err("E7") + raw.setdefault("installed_skills", {}) + raw.setdefault("auto_update", True) + return raw # type: ignore[no-any-return] def get_skills_state() -> dict[str, Any]: - """Read the skills state file.""" + """Read the skills state file (E8 if unreadable, E7 if not valid).""" try: - result: dict[str, Any] = json.loads(_STATE_FILE.read_text()) - return result - except (FileNotFoundError, json.JSONDecodeError, OSError): + data = _STATE_FILE.read_bytes() + except FileNotFoundError: return {"installed_skills": {}, "auto_update": True} - - -def save_skills_state(state: dict[str, Any]) -> None: - """Persist the skills state file.""" - _SKILLS_DIR.mkdir(parents=True, exist_ok=True) - _STATE_FILE.write_text(json.dumps(state, indent=2)) - - -def fetch_repo_skills(force: bool = False) -> dict[str, str]: - """Download skill markdown files from the deepgram/skills GitHub repo. - - Returns a mapping of skill name -> markdown content. - Caches locally to avoid repeated network requests. + except OSError as exc: + raise _err("E8", reason=_reason(exc)) + try: + raw = json.loads(data.decode("utf-8")) + except ValueError: + raise _err("E7") + return _validated(raw) + + +def _write_state(state: dict[str, Any]) -> None: + """Validate, then atomically replace skills.json (a symlink stays one).""" + _validated(copy.deepcopy(state)) + path = Path(os.path.realpath(_STATE_FILE)) + path.parent.mkdir(parents=True, exist_ok=True) + fd, tmp = tempfile.mkstemp(prefix=".skills.json.", suffix=".tmp", dir=path.parent) + try: + # The ``with`` closes the handle first: Windows can't unlink an open file. + with os.fdopen(fd, "w", encoding="utf-8") as f: + json.dump(state, f, indent=2) + f.flush() + os.fsync(f.fileno()) + os.replace(tmp, path) + except BaseException: + with contextlib.suppress(OSError): + os.unlink(tmp) # Proof: mkstemp made it in this call. + raise + + +def _try_lock(lock: Path) -> int: # Lock without waiting: fd, -1 if busy, or E28. + fd = -1 + try: + lock.parent.mkdir(parents=True, exist_ok=True) + flags = os.O_RDWR | os.O_CREAT | getattr(os, "O_NOFOLLOW", 0) + fd = os.open(lock, flags, 0o600) # Never truncates, writes or deletes it. + if sys.platform == "win32": # Windows cannot delete or replace an open file. + msvcrt.locking(fd, msvcrt.LK_NBLCK, 1) + else: + fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + if not os.path.samestat(os.fstat(fd), os.lstat(lock)): # Deleted/replaced. + raise FileNotFoundError(errno.ENOENT, "replaced") # Lock the new one. + fd, got = -1, fd # Ours now: the finally must not close it. + return got + except OSError as exc: + if fd < 0 or exc.errno not in _BUSY: # Busy, or deleted or replaced. + raise _err("E28", lock=lock, reason=_reason(exc)) from exc + return -1 + finally: + if fd >= 0: + os.close(fd) # Busy, E28 or Ctrl-C: never keep this file open. + + +@contextlib.contextmanager +def _state_lock() -> Iterator[None]: + """Hold skills.json.lock against other deepctl processes and threads (B1). + + Re-entrant per thread. The OS drops it when its holder exits, even on a + kill, so the file is never deleted and a stale lock cannot exist. """ - import urllib.request - - cache_marker = _REPO_CACHE_DIR / ".fetched" - if not force and cache_marker.exists(): - # Check if cache is less than 1 hour old - import time - - try: - age = time.time() - cache_marker.stat().st_mtime - if age < 3600: # 1 hour - return _read_cached_skills() - except OSError: - pass - - base = f"https://raw.githubusercontent.com/{_SKILLS_REPO}/{_SKILLS_BRANCH}" - skill_names = ["api", "docs", "setup-mcp", "starters"] - skills: dict[str, str] = {} - - _REPO_CACHE_DIR.mkdir(parents=True, exist_ok=True) - - for name in skill_names: - url = f"{base}/skills/{name}/SKILL.md" - try: - with urllib.request.urlopen(url, timeout=10) as resp: - content = resp.read().decode("utf-8") - skills[name] = content - ( # Cache to disk - _REPO_CACHE_DIR / f"{name}.md" - ).write_text(content) - except Exception: - # Use cached version if available - cached = _REPO_CACHE_DIR / f"{name}.md" - if cached.exists(): - skills[name] = cached.read_text() + if getattr(_LOCAL, "fd", None) is not None: + yield # This thread already holds it. + return + lock, deadline = _STATE_FILE.with_name("skills.json.lock"), time.monotonic() + while (fd := _try_lock(lock)) < 0: + if time.monotonic() >= deadline + _LOCK_TIMEOUT: + raise _err("E27") + time.sleep(0.05) + try: + _LOCAL.fd = fd + yield + finally: + _LOCAL.fd = None + if sys.platform == "win32": + with contextlib.suppress(OSError): + msvcrt.locking(fd, msvcrt.LK_UNLCK, 1) + os.close(fd) # Releases the POSIX lock. + + +@_state_lock() +def _update_state( + mutate: Callable[[dict[str, Any]], None], + failure: str = "E9c", + gen: SkillGenerator | None = None, +) -> None: + """Read skills.json, apply ``mutate`` and write it: every write goes here.""" + # Re-entrant: install_tool and remove_tool hold it for the whole operation (B1). + state = get_skills_state() # E7 and E8 pass through unchanged (N5). + mutate(state) + try: + _write_state(state) + except (OSError, TypeError) as exc: + raise _err(failure, gen, reason=_reason(exc)) from exc - if skills: - cache_marker.write_text("1") - return skills +def save_skills_state(state: dict[str, Any]) -> None: + """Persist the skills state file, keeping ``skill_folders`` from disk.""" + def keep(disk: dict[str, Any]) -> None: + recs = disk.get(_RECORDS_KEY) + disk.clear() + disk.update({k: v for k, v in state.items() if k != _RECORDS_KEY}) + if recs is not None: + disk[_RECORDS_KEY] = recs -def _read_cached_skills() -> dict[str, str]: - """Read previously cached repo skills.""" - skills: dict[str, str] = {} - if not _REPO_CACHE_DIR.exists(): - return skills - for md_file in _REPO_CACHE_DIR.glob("*.md"): - skills[md_file.stem] = md_file.read_text() - return skills + _update_state(keep) def _commands_hash(commands: list[CommandMetadata]) -> str: @@ -671,462 +859,553 @@ def render_skill_content( return render_developer_guide(version, include_frontmatter=include_frontmatter) -# --------------------------------------------------------------------------- -# Generator base class -# --------------------------------------------------------------------------- - - -class SkillGenerator(ABC): - """Base class for AI CLI skill file generators.""" - - cli_name: str = "" - display_name: str = "" - - @abstractmethod - def detect(self) -> bool: - """Return True if this AI CLI is installed/available.""" - - @abstractmethod - def get_skill_paths(self) -> list[Path]: - """Return the file paths where skills will be written.""" - - @abstractmethod - def generate( - self, commands: list[CommandMetadata], version: str - ) -> dict[Path, str]: - """Generate skill file contents. - - Returns: - Mapping of file path -> content string - """ - - def install( - self, - commands: list[CommandMetadata], # noqa: ARG002 - version: str, # noqa: ARG002 - ) -> list[Path]: - """Fetch skills from deepgram/skills repo and install them.""" - repo_skills = fetch_repo_skills(force=True) - if not repo_skills: - return [] - return self._write_repo_skills(repo_skills) - - def _write_repo_skills(self, repo_skills: dict[str, str]) -> list[Path]: - """Write combined repo skill content to this tool's skill paths.""" - combined = "\n\n---\n\n".join(repo_skills.values()) - written: list[Path] = [] - for path in self.get_skill_paths(): - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(combined) - written.append(path) - return written - - def remove(self) -> list[Path]: - """Remove installed skill files. Returns paths removed.""" - removed: list[Path] = [] - for path in self.get_skill_paths(): - if path.exists(): - path.unlink() - removed.append(path) - return removed - - def is_installed(self) -> bool: - """Check if skill files exist.""" - return any(p.exists() for p in self.get_skill_paths()) - - -# --------------------------------------------------------------------------- -# Concrete generators -# --------------------------------------------------------------------------- - - -class ClaudeCodeGenerator(SkillGenerator): - """Generator for Claude Code (Anthropic).""" - - cli_name = "claude" - display_name = "Claude Code" - - @property - def _skill_dir(self) -> Path: - return Path.home() / ".claude" / "commands" / "deepgram" - - def detect(self) -> bool: - return ( - Path.home().joinpath(".claude").is_dir() - or shutil.which("claude") is not None - ) - - def get_skill_paths(self) -> list[Path]: - return [ - self._skill_dir / f"{name}.md" - for name in ["api", "docs", "setup-mcp", "starters"] - ] - - def generate( - self, commands: list[CommandMetadata], version: str - ) -> dict[Path, str]: - content = render_skill_content(commands, version, include_frontmatter=True) - return {self._skill_dir / "deepgram.md": content} - - def _write_repo_skills(self, repo_skills: dict[str, str]) -> list[Path]: - self._skill_dir.mkdir(parents=True, exist_ok=True) - written: list[Path] = [] - for name, content in repo_skills.items(): - path = self._skill_dir / f"{name}.md" - path.write_text(content) - written.append(path) - return written - - def remove(self) -> list[Path]: - removed: list[Path] = [] - if self._skill_dir.exists(): - for f in self._skill_dir.glob("*.md"): - f.unlink() - removed.append(f) - try: - self._skill_dir.rmdir() - except OSError: - pass - return removed - - -class CodexGenerator(SkillGenerator): - """Generator for OpenAI Codex CLI.""" - - cli_name = "codex" - display_name = "OpenAI Codex" - - _BEGIN = "" - _END = "" +def _folders(state: dict[str, Any], cli: str) -> dict[str, Any]: + folders: dict[str, Any] = ( + state.get(_RECORDS_KEY, {}).get(cli, {}).get("folders", {}) + ) + return folders - def detect(self) -> bool: - return ( - Path.home().joinpath(".codex").is_dir() or shutil.which("codex") is not None - ) - def get_skill_paths(self) -> list[Path]: - return [Path.home() / ".codex" / "instructions.md"] - - def generate( - self, commands: list[CommandMetadata], version: str - ) -> dict[Path, str]: - content = render_skill_content(commands, version) - wrapped = f"{self._BEGIN}\n{content}{self._END}\n" - path = self.get_skill_paths()[0] - return {path: self._merge(path, wrapped)} - - def _merge(self, path: Path, section: str) -> str: - """Merge delimited section into existing file content.""" - if not path.exists(): - return section - existing = path.read_text() - if self._BEGIN in existing: - before = existing[: existing.index(self._BEGIN)] - after_end = existing.find(self._END) - after = existing[after_end + len(self._END) :] if after_end != -1 else "" - return before + section + after.lstrip("\n") - return existing.rstrip("\n") + "\n\n" + section - - def _write_repo_skills(self, repo_skills: dict[str, str]) -> list[Path]: - combined = "\n\n---\n\n".join(repo_skills.values()) - wrapped = f"{self._BEGIN}\n{combined}\n{self._END}\n" - path = self.get_skill_paths()[0] - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(self._merge(path, wrapped)) - return [path] - - def remove(self) -> list[Path]: - path = self.get_skill_paths()[0] - if not path.exists(): - return [] - content = path.read_text() - if self._BEGIN not in content: - return [] - before = content[: content.index(self._BEGIN)] - after_end = content.find(self._END) - after = content[after_end + len(self._END) :] if after_end != -1 else "" - remaining = (before + after).strip() - if remaining: - path.write_text(remaining + "\n") - else: - path.unlink() - return [path] +def _is_link(st: os.stat_result) -> bool: + """True for a symlink, or any reparse point (a Windows junction included).""" + attrs = getattr(st, "st_file_attributes", 0) + return stat.S_ISLNK(st.st_mode) or bool(attrs & stat.FILE_ATTRIBUTE_REPARSE_POINT) -class GeminiGenerator(SkillGenerator): - """Generator for Google Gemini CLI.""" +def _marker_text(cli: str, name: str) -> str: + return f"deepctl installed this folder ({cli}/{name}); 'dg skills update' replaces it and 'dg skills remove' deletes it.\n" - cli_name = "gemini" - display_name = "Gemini CLI" - _BEGIN = "" - _END = "" +def _read_regular(path: str | Path, limit: int) -> bytes | None: + """Read one regular file of at most ``limit`` bytes, never via a link, else None.""" + lst = os.lstat(path) # The link check on Windows, which has no O_NOFOLLOW. + if _is_link(lst) or not stat.S_ISREG(lst.st_mode) or lst.st_size > limit: + return None + flags = os.O_RDONLY | getattr(os, "O_BINARY", 0) | getattr(os, "O_NOFOLLOW", 0) + fd = os.open(path, flags | getattr(os, "O_NONBLOCK", 0)) + try: + st, data = os.fstat(fd), bytearray() + if (st.st_dev, st.st_ino) != (lst.st_dev, lst.st_ino) or st.st_size > limit: + return None # Swapped between the lstat and the open. + while (chunk := os.read(fd, 65536)) and len(data) <= limit: + data += chunk + return bytes(data) if len(data) <= limit else None + finally: + os.close(fd) + + +def _marker_ok(path: Path, cli: str, name: str) -> bool: + """True if ``path`` is a real directory holding the exact marker for cli/name.""" + try: + st = os.lstat(path) + if _is_link(st) or not stat.S_ISDIR(st.st_mode): + return False + data = _read_regular(path / _MARKER, _MAX_MARKER_BYTES) + except (FileNotFoundError, NotADirectoryError): + return False # Missing: not ours. Any other OSError propagates. + return data == _marker_text(cli, name).encode() + + +def _fingerprint(path: str | Path) -> str | None: + """Hash each entry's name, kind and bytes; None for a link, special file or cap.""" + entries: list[tuple[bytes, bytes, str | None]] = [] + stack = [("", os.fspath(path))] + while stack: + rel, where = stack.pop() + with os.scandir(where) as it: + for e in it: + st, sub = e.stat(follow_symlinks=False), f"{rel}{e.name}" + if _is_link(st) or not ( + stat.S_ISDIR(st.st_mode) or stat.S_ISREG(st.st_mode) + ): + return None + if stat.S_ISDIR(st.st_mode): + entries.append((os.fsencode(sub), b"d", None)) + stack.append((sub + "/", e.path)) + else: + entries.append((os.fsencode(sub), b"f", e.path)) + h, total = hashlib.sha256(_FP_DOMAIN), 0 + for key, kind, file in sorted(entries, key=lambda x: x[0]): + h.update(kind + len(key).to_bytes(4, "big") + key) + if file is not None: + data = _read_regular(file, _MAX_TREE_BYTES - total) + if data is None: + return None + total += len(data) + h.update(hashlib.sha256(data).digest()) + return "sha256:" + h.hexdigest() + + +def _ownership(path: Path, cli: str, name: str, rec: dict[str, Any] | None) -> str: + """Return "ok", "edited", "unproven" or "unreadable" for ``path``.""" + want = {rec.get("fingerprint"), rec.get("pending")} - {None} if rec else set() + if not want: + return "unproven" + try: + if not _marker_ok(path, cli, name): + return "unproven" + fp = _fingerprint(path) + except OSError: + return "unreadable" # Never "edited", which would drop a record (SF4). + return "ok" if fp in want else "edited" + + +def _rename_excl(src: Path, dest: Path) -> None: + """Rename ``src`` to ``dest`` in one step that fails if anything is at ``dest``.""" + if _WINDOWS: + os.rename(src, dest) # Windows rename refuses any existing dest. + return + mac, libc = sys.platform == "darwin", ctypes.CDLL(None, use_errno=True) + fn = getattr(libc, "renamex_np" if mac else "renameat2", None) + nr = _NR_RENAMEAT2[sys.maxsize < 2**32].get(platform.machine()) + if nr and sys.platform == "linux" and hasattr(libc, "syscall"): # Every glibc. + fn = functools.partial(libc.syscall, ctypes.c_long(nr)) # The kernel's errno. + if fn is None: # No call to make on this OS, machine or Python. + raise OSError(errno.ENOSYS, _NO_EXCL_SYS, str(dest)) + a, b = os.fsencode(src), os.fsencode(dest) + if (fn(a, b, 4) if mac else fn(-100, a, -100, b, 1)) != 0: # RENAME_EXCL/NOREPLACE + e = ctypes.get_errno() or errno.EIO # Never "Success" for a failed call. + bad = e in (errno.EINVAL, errno.ENOTSUP, errno.EOPNOTSUPP) # The filesystem. + why = _NO_EXCL_SYS if e == errno.ENOSYS else _NO_EXCL if bad else None + raise OSError(e, why or os.strerror(e), str(dest)) # ENOSYS: kernel < 3.15. + + +def _place(src: Path, dest: Path) -> None: + """Move ``src`` to ``dest``; if anything is at ``dest``, the OS refuses atomically (B2).""" + _rename_excl(src, dest) + + +@dataclass(frozen=True) +class SkillGenerator: + """One AI coding tool and the skills root deepctl installs into.""" + + cli_name: str + display_name: str + root_parts: tuple[str, ...] | None # under Path.home(); None = hint-only + homes: tuple[tuple[str, ...], ...] # dirs whose presence means "detected" + binary: str | None + + def skills_root(self) -> Path | None: + return Path.home().joinpath(*self.root_parts) if self.root_parts else None def detect(self) -> bool: - return ( - Path.home().joinpath(".gemini").is_dir() - or shutil.which("gemini") is not None - ) - - def get_skill_paths(self) -> list[Path]: - return [Path.home() / ".gemini" / "GEMINI.md"] - - def generate( - self, commands: list[CommandMetadata], version: str - ) -> dict[Path, str]: - content = render_skill_content(commands, version) - wrapped = f"{self._BEGIN}\n{content}{self._END}\n" - path = self.get_skill_paths()[0] - return {path: self._merge(path, wrapped)} - - def _merge(self, path: Path, section: str) -> str: - if not path.exists(): - return section - existing = path.read_text() - if self._BEGIN in existing: - before = existing[: existing.index(self._BEGIN)] - after_end = existing.find(self._END) - after = existing[after_end + len(self._END) :] if after_end != -1 else "" - return before + section + after.lstrip("\n") - return existing.rstrip("\n") + "\n\n" + section - - def _write_repo_skills(self, repo_skills: dict[str, str]) -> list[Path]: - combined = "\n\n---\n\n".join(repo_skills.values()) - wrapped = f"{self._BEGIN}\n{combined}\n{self._END}\n" - path = self.get_skill_paths()[0] - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(self._merge(path, wrapped)) - return [path] - - def remove(self) -> list[Path]: - path = self.get_skill_paths()[0] - if not path.exists(): + found = any(Path.home().joinpath(*h).is_dir() for h in self.homes) + return found or bool(self.binary and shutil.which(self.binary)) + + def install(self, commands: list[CommandMetadata], version: str) -> list[Path]: # noqa: ARG002 + """Compatibility shim for login and plugin; goal 3 deletes it.""" + if self.skills_root() is None: # Hint-only: never fetches, never raises. + with contextlib.suppress(SkillInstallError, AttributeError, TypeError): + legacy = get_skills_state()["installed_skills"].get(self.cli_name, {}) + return [Path(p) for p in legacy.get("paths", [])] # Keeps 0.3.x. return [] - content = path.read_text() - if self._BEGIN not in content: - return [] - before = content[: content.index(self._BEGIN)] - after_end = content.find(self._END) - after = content[after_end + len(self._END) :] if after_end != -1 else "" - remaining = (before + after).strip() - if remaining: - path.write_text(remaining + "\n") - else: - path.unlink() - return [path] + ref = _ref_for(self.cli_name, get_skills_state()) # Keeps a --ref (SF5). + skills = skill_bundle.fetch_skill_bundle(ref) + return install_tool(self, skills, ref=ref, version=version)[0] + + +_GENERATORS = [ + SkillGenerator(*row) + for row in ( + ("claude", "Claude Code", (".claude", "skills"), ((".claude",),), "claude"), + ("codex", "OpenAI Codex", (".agents", "skills"), ((".codex",),), "codex"), + ("gemini", "Gemini CLI", (".gemini", "skills"), ((".gemini",),), "gemini"), + ("amazonq", "Amazon Q Developer", None, ((".amazonq",),), None), + ("aider", "Aider", None, (), "aider"), + ( + "opencode", + "OpenCode", + (".config", "opencode", "skills"), + ((".opencode",), (".config", "opencode")), + "opencode", + ), + ("cursor", "Cursor", (".cursor", "skills"), ((".cursor",),), "cursor"), + ("cline", "Cline", (".cline", "skills"), ((".cline",),), None), + ) +] -class AmazonQGenerator(SkillGenerator): - """Generator for Amazon Q Developer CLI.""" +def get_all_generators() -> list[SkillGenerator]: + """Return instances of all registered generators.""" + return list(_GENERATORS) - cli_name = "amazonq" - display_name = "Amazon Q Developer" - def detect(self) -> bool: - return Path.home().joinpath(".amazonq").is_dir() +def detect_ai_clis() -> list[SkillGenerator]: + """Return generators for detected AI CLIs.""" + return [g for g in get_all_generators() if g.detect()] - def get_skill_paths(self) -> list[Path]: - return [Path.home() / ".amazonq" / "rules" / "deepctl.md"] - def generate( - self, commands: list[CommandMetadata], version: str - ) -> dict[Path, str]: - content = render_skill_content(commands, version) - return {self.get_skill_paths()[0]: content} +def _ref_for(cli: str, state: dict[str, Any], explicit: str | None = None) -> str: + """``explicit``, then the env var, then the recorded ref, then the pin.""" + if explicit or os.environ.get(skill_bundle.REF_ENV_VAR, "").strip(): + return skill_bundle.resolve_skills_ref(explicit) + recorded = state.get(_RECORDS_KEY, {}).get(cli, {}).get("skills_ref") + if recorded: + return skill_bundle.validate_ref(recorded) + return skill_bundle.DEFAULT_SKILLS_COMMIT + + +def _conflicts( + gen: SkillGenerator, skills: Sequence[RepoSkill], recorded: dict[str, Any] +) -> tuple[list[Path], list[Path]]: + """Return the (unproven, edited) destinations; moves nothing, writes no state.""" + root = gen.skills_root() + assert root is not None + if os.path.lexists(root) and not os.path.isdir(root): + raise _err("E18", gen) + found: dict[str, list[Path]] = {"unproven": [], "edited": [], "ok": []} + for s in skills: + dest, rec = root / s.name, recorded.get(s.name) + if os.path.lexists(dest): + kind = _ownership(dest, gen.cli_name, s.name, rec) + if kind == "unreadable": + raise _err("E5", gen, reason=f"could not read {dest}") + found[kind].append(dest) + return found["unproven"], found["edited"] + + +def install_conflicts( + generators: Sequence[SkillGenerator], skills: Sequence[RepoSkill] +) -> tuple[list[Path], list[Path]]: + """Return (unproven, edited) across the folder tools; writes no state.""" + state, unproven, edited = get_skills_state(), list[Path](), list[Path]() + for gen in generators: + if gen.skills_root() is not None: + u, e = _conflicts(gen, skills, _folders(state, gen.cli_name)) + unproven, edited = unproven + u, edited + e + return unproven, edited + + +def _refusal(gen: SkillGenerator, dest: Path, kind: str, rec: Any) -> SkillInstallError: + if kind == "unreadable": + return _err("E5", gen, reason=f"could not read {dest}") + if kind == "edited": + return SkillOwnershipError([], [dest]) + msg = _msg("E3" if rec else "E2", gen, dest=dest) + return SkillOwnershipError([dest], message=msg) + + +def _stage( + gen: SkillGenerator, skills: Sequence[RepoSkill], staging: Path +) -> dict[str, str]: + """Copy each skill into staging/new, add its marker, and fingerprint the copy.""" + fps: dict[str, str] = {} + os.mkdir(staging / "new") + _place(staging / "new", staging / "old") # Fails closed before anything moves (B2). + os.mkdir(staging / "new") + for s in skills: + copy_ = staging / "new" / s.name + shutil.copytree(s.path, copy_) + try: + with open(copy_ / _MARKER, "x", encoding="utf-8", newline="\n") as f: + f.write(_marker_text(gen.cli_name, s.name)) + fp = _fingerprint(copy_) + except FileExistsError: + fp = None + if fp is None: + raise _err("E10", name=s.name) + fps[s.name] = fp + return fps + + +def _swap(gen: SkillGenerator, name: str, staging: Path, rec: Any) -> None: + """Move a proven old folder aside, then place the staged copy.""" + cli, dest, aside = gen.cli_name, staging.parent / name, staging / "old" / name + moved, refusal = False, None + if os.path.lexists(dest): + if (kind := _ownership(dest, cli, name, rec)) != "ok": # The pre-check. + raise _refusal(gen, dest, kind, rec) + try: + os.rename(dest, aside) + moved = True + except FileNotFoundError: + pass # Vanished: install as absent. + except OSError as exc: + raise _err("E5", gen, reason=_reason(exc)) + try: # Starts right after the move, so the copy always goes back (SF1). + if moved and (kind := _ownership(aside, cli, name, rec)) != "ok": + raise (refusal := _refusal(gen, dest, kind, rec)) # The real proof. + _place(staging / "new" / name, dest) + except BaseException as exc: + if os.path.lexists(aside): + try: + _place(aside, dest) # Put back whatever was moved; never overwrites. + except OSError: + if not isinstance(exc, Exception): + raise exc # Ctrl-C: cleanup's invariant decides the copy. + key = "E4" if exc is refusal else "E6" + err = _err(key, gen, dest=dest, aside=aside, name=name) + err.kept = aside # Cleanup may yet put an E6 copy back. + raise err from exc + if getattr(exc, "errno", None) in _NO_REPLACE: # Someone else's dest. + raise _refusal(gen, dest, "unproven", None) from exc + raise + + +def _cleanup(staging: Path, cli: str, before: dict[str, Any]) -> Path | None: + """Remove this run's staging; an old copy goes back only if its new one never left.""" + for a in _scan(staging / "old"): # Before staging/new, which is the proof. + if _ownership(a, cli, a.name, before.get(a.name)) != "ok": + continue # Not exactly the old folder: keep it for the user. + dest = staging.parent / a.name + with contextlib.suppress(OSError): + if os.path.lexists(staging / "new" / a.name): + _place(a, dest) # Stranded by a Ctrl-C. + elif not os.path.lexists(dest) or _marker_ok(dest, cli, a.name): + shutil.rmtree(a, ignore_errors=True) # Replaced (B1): never back. + for e in _scan(staging / "new"): + shutil.rmtree(e, ignore_errors=True) # Copied by this call. + for d in (staging / "old", staging / "new", staging): + with contextlib.suppress(OSError): + os.rmdir(d) # Removes only an empty dir. + return staging if os.path.lexists(staging) else None + + +def _real_dir(path: Path) -> bool: + """True for a real directory, never a link to one.""" + try: + st = os.lstat(path) + except OSError: + return False + return not _is_link(st) and stat.S_ISDIR(st.st_mode) -class AiderGenerator(SkillGenerator): - """Generator for Aider CLI.""" +def _old_copy(staging: Path, cli: str, name: str) -> Path | None: + """A marked old copy of cli/name in a real staging folder; anything else is ignored.""" + old = staging / "old" / name + try: + ok = _real_dir(staging) and _real_dir(old.parent) and _marker_ok(old, cli, name) + except OSError: + return None + return old if ok else None - cli_name = "aider" - display_name = "Aider" - _SKILL_FILE = Path.home() / ".deepctl" / "skills" / "deepctl-conventions.md" +def _scan(path: Path) -> list[Path]: + try: + return [Path(e.path) for e in os.scandir(path)] + except OSError: + return [] - def detect(self) -> bool: - return shutil.which("aider") is not None - - def get_skill_paths(self) -> list[Path]: - return [self._SKILL_FILE] - - def generate( - self, commands: list[CommandMetadata], version: str - ) -> dict[Path, str]: - content = render_skill_content(commands, version) - return {self._SKILL_FILE: content} - - def install(self, commands: list[CommandMetadata], version: str) -> list[Path]: - written = super().install(commands, version) - # Add read reference to aider config if not already present - self._ensure_config_ref() - return written - - def _ensure_config_ref(self) -> None: - """Add the skill file as a read reference in ~/.aider.conf.yml.""" - conf_path = Path.home() / ".aider.conf.yml" - ref = str(self._SKILL_FILE) - try: - import yaml - if conf_path.exists(): - data = yaml.safe_load(conf_path.read_text()) or {} +@_state_lock() +def install_tool( + gen: SkillGenerator, skills: Sequence[RepoSkill], *, ref: str, version: str +) -> tuple[list[Path], Path | None]: + """Install ``skills`` for ``gen``; return (placed folders, leftover staging).""" + root, cli = gen.skills_root(), gen.cli_name + if root is None: + return [], None + for s in skills: + if not portable_name(s.name): + raise _err("E11", name=s.name) + before = {n: dict(r) for n, r in _folders(get_skills_state(), cli).items()} + unproven, edited = _conflicts(gen, skills, before) + if unproven or edited: + raise SkillOwnershipError(unproven, edited) + fps: dict[str, str] = {} + placed: list[str] = [] + error: BaseException | None = None + run = uuid.uuid4().hex # Tags this run's "installing" records (S1). + + def mark(state: dict[str, Any]) -> None: + tool = state.setdefault(_RECORDS_KEY, {}).setdefault(cli, {}) + folders = tool.setdefault("folders", {}) + for n, fp in fps.items(): # Keeps the old fingerprint beside ``pending``. + folders[n] = {**folders.get(n, {}), "state": "installing", "pending": fp} + folders[n]["run"] = run # Retags a crashed run's record too. + + def settle(state: dict[str, Any]) -> None: # From proof on disk, not ``placed``. + tool = state.setdefault(_RECORDS_KEY, {}).setdefault(cli, {}) + folders = tool.setdefault("folders", {}) + for s in skills: + rec, dest = folders.get(s.name), root / s.name + if all(os.path.lexists(staging / d / s.name) for d in ("old", "new")): + continue # Cleanup may yet put the old copy back: keep its record. + # This run's staged copy is proof too: another run may drop the record (S1). + recs = (rec or {}, before.get(s.name, {}), {"pending": fps.get(s.name)}) + want = {r.get(k) for r in recs for k in ("fingerprint", "pending")} - {None} + try: + ok = _marker_ok(dest, cli, s.name) + fp = _fingerprint(dest) if ok else None + except OSError: + continue # Unreadable keeps its record exactly as it is (SF4). + if fp in want: + folders[s.name] = {"state": "installed", "fingerprint": fp} + elif rec and rec.get("run", run) != run: + continue # Another run settles its own. else: - data = {} - read_list = data.get("read", []) - if not isinstance(read_list, list): - read_list = [read_list] if read_list else [] - if ref not in read_list: - read_list.append(ref) - data["read"] = read_list - conf_path.write_text(yaml.dump(data, default_flow_style=False)) - except Exception: - pass + folders.pop(s.name, None) + legacy = state["installed_skills"].get(cli, {}).get("paths", []) + if any(Path(p).parent != root for p in legacy): # 0.3.x files remain. + tool["v03"] = True + now = datetime.now(timezone.utc).isoformat() + if placed: + tool.update(skills_ref=ref, version=version, installed_at=now) + if not folders: + del state[_RECORDS_KEY][cli] + elif cli not in state["installed_skills"]: # Login and startup key on it. + paths = [str(root / n) for n in folders] + mirror = {"paths": paths, "installed_at": now, "version": version} + state["installed_skills"][cli] = mirror - def remove(self) -> list[Path]: - removed = super().remove() - # Remove reference from aider config - conf_path = Path.home() / ".aider.conf.yml" - ref = str(self._SKILL_FILE) - try: - import yaml - - if conf_path.exists(): - data = yaml.safe_load(conf_path.read_text()) or {} - read_list = data.get("read", []) - if isinstance(read_list, list) and ref in read_list: - read_list.remove(ref) - data["read"] = read_list - conf_path.write_text(yaml.dump(data, default_flow_style=False)) - except Exception: - pass - return removed - - -class OpenCodeGenerator(SkillGenerator): - """Generator for OpenCode CLI.""" - - cli_name = "opencode" - display_name = "OpenCode" - - _BEGIN = "" - _END = "" + try: + root.mkdir(parents=True, exist_ok=True) + staging = Path(tempfile.mkdtemp(prefix=_STAGING_PREFIX, dir=root)) + except OSError as exc: + raise _err("E5", gen, reason=_reason(exc)) + try: # Right after mkdtemp, so cleanup runs after any failure or Ctrl-C (SF1). + fps.update(_stage(gen, skills, staging)) + _update_state(mark, "E9", gen) # Record before swap (B6): nothing moved. + for s in skills: + try: + _swap(gen, s.name, staging, before.get(s.name)) + except BaseException as exc: + error = exc + break + placed.append(s.name) + _update_state(settle, "E9b", gen) + except BaseException as exc: + error = error or exc + finally: + leftover = _cleanup(staging, cli, before) + if isinstance(error, OSError): + error = _err("E5", gen, reason=_reason(error)) + n = a.name if (a := getattr(error, "kept", None)) and not os.path.lexists(a) else "" + if n and _ownership(root / n, cli, n, before.get(n)) == "ok": + error = _err("E6b", gen, name=n) # E6, but cleanup put the old copy back. + if isinstance(error, SkillInstallError): + error.leftover = leftover + if error is not None: + raise error + return [root / n for n in placed], leftover - def detect(self) -> bool: - return ( - Path.home().joinpath(".opencode").is_dir() - or shutil.which("opencode") is not None - ) - def get_skill_paths(self) -> list[Path]: - return [Path.home() / ".opencode" / "agents.md"] - - def generate( - self, commands: list[CommandMetadata], version: str - ) -> dict[Path, str]: - content = render_skill_content(commands, version) - wrapped = f"{self._BEGIN}\n{content}{self._END}\n" - path = self.get_skill_paths()[0] - return {path: self._merge(path, wrapped)} - - def _merge(self, path: Path, section: str) -> str: - if not path.exists(): - return section - existing = path.read_text() - if self._BEGIN in existing: - before = existing[: existing.index(self._BEGIN)] - after_end = existing.find(self._END) - after = existing[after_end + len(self._END) :] if after_end != -1 else "" - return before + section + after.lstrip("\n") - return existing.rstrip("\n") + "\n\n" + section - - def _write_repo_skills(self, repo_skills: dict[str, str]) -> list[Path]: - combined = "\n\n---\n\n".join(repo_skills.values()) - wrapped = f"{self._BEGIN}\n{combined}\n{self._END}\n" - path = self.get_skill_paths()[0] - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(self._merge(path, wrapped)) - return [path] - - def remove(self) -> list[Path]: - path = self.get_skill_paths()[0] - if not path.exists(): - return [] - content = path.read_text() - if self._BEGIN not in content: - return [] - before = content[: content.index(self._BEGIN)] - after_end = content.find(self._END) - after = content[after_end + len(self._END) :] if after_end != -1 else "" - remaining = (before + after).strip() - if remaining: - path.write_text(remaining + "\n") +@dataclass +class RemoveResult: + """What ``remove_tool`` did with each recorded folder.""" + + removed: list[Path] = field(default_factory=list) + kept: list[tuple[Path, str]] = field(default_factory=list) + left_alone: list[Path] = field(default_factory=list) + edited: list[Path] = field(default_factory=list) + moved: list[tuple[Path, Path]] = field(default_factory=list) + stranded: list[tuple[Path, Path]] = field(default_factory=list) # E29 + leftover: Path | None = None + + def refused(self, dest: Path, kind: str) -> None: + if kind == "unreadable": + self.kept.append((dest, "deepctl could not read it")) else: - path.unlink() - return [path] + (self.edited if kind == "edited" else self.left_alone).append(dest) -class CursorGenerator(SkillGenerator): - """Generator for Cursor IDE CLI.""" +@_state_lock() +def remove_tool(gen: SkillGenerator) -> RemoveResult: + """Delete ``gen``'s recorded folders that prove ours; leave everything else.""" + cli, root, res, staging = gen.cli_name, gen.skills_root(), RemoveResult(), None + folders = dict(_folders(get_skills_state(), cli)) - cli_name = "cursor" - display_name = "Cursor" - - def detect(self) -> bool: - return ( - Path.home().joinpath(".cursor").is_dir() - or shutil.which("cursor") is not None + def settle(state: dict[str, Any]) -> None: # Keeps only what proves on disk. + fs, res.stranded = _folders(state, cli), [] + dirs = ( + [d for d in _scan(root) if d.name.startswith(_STAGING_PREFIX)] + if root + else [] ) - - def get_skill_paths(self) -> list[Path]: - return [Path.home() / ".cursor" / "rules" / "deepctl.mdc"] - - def generate( - self, commands: list[CommandMetadata], version: str - ) -> dict[Path, str]: - content = render_skill_content(commands, version) - return {self.get_skill_paths()[0]: content} - - -class ClineGenerator(SkillGenerator): - """Generator for Cline CLI.""" - - cli_name = "cline" - display_name = "Cline" - - def detect(self) -> bool: - return Path.home().joinpath(".cline").is_dir() - - def get_skill_paths(self) -> list[Path]: - return [Path.home() / ".cline" / "rules" / "deepctl.md"] - - def generate( - self, commands: list[CommandMetadata], version: str - ) -> dict[Path, str]: - content = render_skill_content(commands, version) - return {self.get_skill_paths()[0]: content} - - -# --------------------------------------------------------------------------- -# Registry of all generators -# --------------------------------------------------------------------------- - -_ALL_GENERATORS: list[type[SkillGenerator]] = [ - ClaudeCodeGenerator, - CodexGenerator, - GeminiGenerator, - AmazonQGenerator, - AiderGenerator, - OpenCodeGenerator, - CursorGenerator, - ClineGenerator, -] - - -def get_all_generators() -> list[SkillGenerator]: - """Return instances of all registered generators.""" - return [cls() for cls in _ALL_GENERATORS] - - -def detect_ai_clis() -> list[SkillGenerator]: - """Return generators for detected AI CLIs.""" - return [g for g in get_all_generators() if g.detect()] + for n in list(fs): + kind = _ownership(root / n, cli, n, fs[n]) if root else "unproven" + # An old copy in staging, even a killed run's, keeps its record (S1). + olds = [a for d in dirs if (a := _old_copy(d, cli, n))] + stuck = [a for a in olds if a.parents[2] / n not in res.removed] + if kind not in ("ok", "unreadable") and not stuck: + del fs[n] # Gone, edited or unproven: no longer deepctl's. + continue # Removed or replaced here: an old copy is only a leftover. + res.stranded += [ + (a.parents[2] / n, a) for a in stuck if a.parents[1] != staging + ] + if not fs: + state.get(_RECORDS_KEY, {}).pop(cli, None) + state["installed_skills"].pop(cli, None) + + if root is None or not folders: + _update_state(settle) + return res + if os.path.lexists(root) and not os.path.isdir(root): + raise _err("E18", gen) # The records stay: the folders may come back. + if any(os.path.lexists(root / n) for n in folders): + try: + staging = Path(tempfile.mkdtemp(prefix=_STAGING_PREFIX, dir=root)) + os.mkdir(staging / "new") + _place(staging / "new", staging / "old") # Fails closed first (B2). + except BaseException as exc: # Ctrl-C too: never leave this run's empty dirs. + if staging: + _cleanup(staging, cli, {}) + if not isinstance(exc, OSError): + raise + raise _err("E21", root=root, reason=_reason(exc)) + try: + for name, rec in folders.items(): + dest = root / name + if staging is None or not os.path.lexists(dest): + continue # Dropped at settle. + aside = staging / "old" / name + if (kind := _ownership(dest, cli, name, rec)) != "ok": # The pre-check. + res.refused(dest, kind) + continue + try: # Covers the move too, so a Ctrl-C right after it puts it back. + os.rename(dest, aside) + kind = _ownership(aside, cli, name, rec) # The real proof (SF1). + except FileNotFoundError: + continue + except OSError as exc: # Nothing moved: _ownership never raises it. + res.kept.append((dest, _reason(exc))) # Stays recorded (B2). + continue + except BaseException: + with contextlib.suppress(OSError): + _place(aside, dest) + raise + if kind == "ok": + shutil.rmtree(aside, ignore_errors=True) + res.removed.append(dest) + continue + try: + _place(aside, dest) + res.refused(dest, kind) + except OSError: + res.moved.append((dest, aside)) + except BaseException: + with contextlib.suppress(OSError): + _place(aside, dest) + raise + finally: + _update_state(settle) + if staging is not None: + for d in (staging / "old", staging): + with contextlib.suppress(OSError): + os.rmdir(d) + res.leftover = staging if os.path.lexists(staging) else None + return res + + +@dataclass(frozen=True) +class ToolStatus: + """Read-only view of one tool's recorded folders and staging leftovers.""" + + root: Path | None + kinds: dict[str, list[Path]] # "ok"/"unproven"/"edited"/"unreadable" + leftovers: list[Path] + skills_ref: str | None + + +def tool_status(gen: SkillGenerator, state: dict[str, Any]) -> ToolStatus: + """Classify ``gen``'s recorded folders on disk; changes nothing.""" + root, cli = gen.skills_root(), gen.cli_name + kinds = {k: list[Path]() for k in ("ok", "unproven", "edited", "unreadable")} + leftovers: list[Path] = [] + if root is not None: + for n, rec in _folders(state, cli).items(): + if os.path.lexists(root / n): + kinds[_ownership(root / n, cli, n, rec)].append(root / n) + leftovers = sorted(root.glob(_STAGING_PREFIX + "*")) if root.is_dir() else [] + ref = state.get(_RECORDS_KEY, {}).get(cli, {}).get("skills_ref") + return ToolStatus(root, kinds, leftovers, ref) diff --git a/packages/deepctl-core/tests/unit/test_skill_generator.py b/packages/deepctl-core/tests/unit/test_skill_generator.py index 8b1e574e..ff781800 100644 --- a/packages/deepctl-core/tests/unit/test_skill_generator.py +++ b/packages/deepctl-core/tests/unit/test_skill_generator.py @@ -1,27 +1,46 @@ """Unit tests for skill generator module.""" +import contextlib +import errno +import hashlib import json +import multiprocessing +import os +import shutil +import stat +import sys +import threading +import time from pathlib import Path -from unittest.mock import MagicMock, patch +from types import SimpleNamespace +from unittest.mock import patch import pytest +from deepctl_core import skill_bundle +from deepctl_core import skill_generator as sg +from deepctl_core.skill_bundle import RepoSkill from deepctl_core.skill_generator import ( - AmazonQGenerator, - ClaudeCodeGenerator, - ClineGenerator, - CodexGenerator, CommandMetadata, - CursorGenerator, - GeminiGenerator, + SkillInstallError, + SkillOwnershipError, _commands_hash, + _fingerprint, + _marker_ok, + _msg, + _ownership, + _place, collect_command_metadata, detect_ai_clis, get_all_generators, get_skills_state, + install_conflicts, + install_tool, + remove_tool, render_developer_guide, render_skill_content, save_skills_state, skills_need_update, + tool_status, ) @@ -54,7 +73,9 @@ def test_create(self): assert cmd.examples == ["dg test foo"] def test_create_with_parent_group(self): - cmd = _make_command(name="audio", parent_group="debug", full_command="deepctl debug audio") + cmd = _make_command( + name="audio", parent_group="debug", full_command="deepctl debug audio" + ) assert cmd.parent_group == "debug" assert cmd.full_command == "deepctl debug audio" @@ -63,7 +84,10 @@ class TestCommandsHash: """Test _commands_hash.""" def test_deterministic(self): - cmds = [_make_command(), _make_command(name="other", full_command="deepctl other")] + cmds = [ + _make_command(), + _make_command(name="other", full_command="deepctl other"), + ] h1 = _commands_hash(cmds) h2 = _commands_hash(cmds) assert h1 == h2 @@ -90,23 +114,26 @@ def test_get_skills_state_missing_file(self, tmp_path): def test_save_and_get_skills_state(self, tmp_path): state_file = tmp_path / "skills.json" - with patch("deepctl_core.skill_generator._STATE_FILE", state_file), \ - patch("deepctl_core.skill_generator._SKILLS_DIR", tmp_path): - save_skills_state({"installed_skills": {"claude": {}}, "auto_update": False}) + with ( + patch("deepctl_core.skill_generator._STATE_FILE", state_file), + patch("deepctl_core.skill_generator._SKILLS_DIR", tmp_path), + ): + save_skills_state( + {"installed_skills": {"claude": {}}, "auto_update": False} + ) result = get_skills_state() assert result["installed_skills"] == {"claude": {}} assert result["auto_update"] is False def test_skills_need_update_no_installed(self): - with patch("deepctl_core.skill_generator.get_skills_state", return_value={"installed_skills": {}}): + with patch( + "deepctl_core.skill_generator.get_skills_state", + return_value={"installed_skills": {}}, + ): assert skills_need_update([_make_command()]) is False def test_skills_need_update_stale_hash(self): - state = { - "installed_skills": { - "claude": {"commands_hash": "sha256:old"} - } - } + state = {"installed_skills": {"claude": {"commands_hash": "sha256:old"}}} with patch("deepctl_core.skill_generator.get_skills_state", return_value=state): assert skills_need_update([_make_command()]) is True @@ -199,165 +226,2830 @@ def test_render_skill_content_frontmatter(self): assert "description:" in content -class TestClaudeCodeGenerator: - """Test ClaudeCodeGenerator.""" +# --------------------------------------------------------------------------- +# Skill folder install: fixtures and helpers +# --------------------------------------------------------------------------- - def test_detect_with_dir(self, tmp_path): - gen = ClaudeCodeGenerator() - with patch.object(Path, "joinpath", return_value=tmp_path): - with patch.object(tmp_path.__class__, "is_dir", return_value=True): - assert gen.detect() is True +POSIX = pytest.mark.skipif(os.name == "nt", reason="POSIX-only filesystem behavior") +NT = pytest.mark.skipif(os.name != "nt", reason="Windows-only filesystem behavior") +FP_A = "sha256:" + "a" * 64 +FP_B = "sha256:" + "b" * 64 +REF = skill_bundle.DEFAULT_SKILLS_COMMIT - def test_skill_path(self): - gen = ClaudeCodeGenerator() - paths = gen.get_skill_paths() - assert len(paths) > 0 - assert all("deepgram" in str(p) for p in paths) - assert all("commands" in str(p) for p in paths) - assert all(p.suffix == ".md" for p in paths) - def test_generate_includes_frontmatter(self): - gen = ClaudeCodeGenerator() - cmds = [_make_command()] - result = gen.generate(cmds, "1.0.0") - assert len(result) == 1 - content = list(result.values())[0] - assert content.startswith("---\n") +@pytest.fixture(autouse=True) +def _throwaway_home(tmp_path, monkeypatch): + home = tmp_path / "home" + home.mkdir() + monkeypatch.setattr(Path, "home", staticmethod(lambda: home)) + monkeypatch.setenv("HOME", str(home)) + monkeypatch.setenv("USERPROFILE", str(home)) + monkeypatch.setattr(sg, "_SKILLS_DIR", home / ".deepctl" / "skills") + monkeypatch.setattr(sg, "_STATE_FILE", home / ".deepctl" / "skills" / "skills.json") + monkeypatch.setenv("COLUMNS", "400") + monkeypatch.delenv(skill_bundle.REF_ENV_VAR, raising=False) + return home - def test_install_writes_individual_skill_files(self, tmp_path): - from unittest.mock import PropertyMock - gen = ClaudeCodeGenerator() - skill_dir = tmp_path / "commands" / "deepgram" - with patch.object(type(gen), "_skill_dir", new_callable=PropertyMock, return_value=skill_dir): - with patch("deepctl_core.skill_generator.fetch_repo_skills", return_value={"api": "# API\n", "docs": "# Docs\n"}): - written = gen.install([_make_command()], "1.0.0") - assert len(written) == 2 - assert skill_dir / "api.md" in written - assert (skill_dir / "api.md").read_text() == "# API\n" - assert (skill_dir / "docs.md").read_text() == "# Docs\n" - - def test_install_returns_empty_when_no_repo_skills(self, tmp_path): - from unittest.mock import PropertyMock - gen = ClaudeCodeGenerator() - skill_dir = tmp_path / "commands" / "deepgram" - with patch.object(type(gen), "_skill_dir", new_callable=PropertyMock, return_value=skill_dir): - with patch("deepctl_core.skill_generator.fetch_repo_skills", return_value={}): - written = gen.install([_make_command()], "1.0.0") - assert written == [] - - def test_remove_deletes_skill_dir(self, tmp_path): - from unittest.mock import PropertyMock - gen = ClaudeCodeGenerator() - skill_dir = tmp_path / "deepgram" - skill_dir.mkdir() - (skill_dir / "api.md").write_text("hello") - with patch.object(type(gen), "_skill_dir", new_callable=PropertyMock, return_value=skill_dir): - removed = gen.remove() - assert len(removed) == 1 - assert not (skill_dir / "api.md").exists() - - def test_is_installed(self, tmp_path): - from unittest.mock import PropertyMock - gen = ClaudeCodeGenerator() - skill_dir = tmp_path / "deepgram" - with patch.object(type(gen), "_skill_dir", new_callable=PropertyMock, return_value=skill_dir): - assert gen.is_installed() is False - skill_dir.mkdir() - (skill_dir / "api.md").write_text("hello") - assert gen.is_installed() is True - - -class TestCodexGenerator: - """Test CodexGenerator (append-mode).""" - - def test_generate_wraps_in_delimiters(self): - gen = CodexGenerator() - cmds = [_make_command()] - result = gen.generate(cmds, "1.0.0") - path = gen.get_skill_paths()[0] - content = result[path] - assert "" in content - - def test_merge_into_existing(self, tmp_path): - gen = CodexGenerator() - target = tmp_path / "instructions.md" - target.write_text("# My instructions\n\nSome content\n") - - with patch.object(gen, "get_skill_paths", return_value=[target]): - result = gen.generate([_make_command()], "1.0.0") - content = result[target] - assert content.startswith("# My instructions\n") - assert "\n" - "old content\n" - "\n" - "after\n" + +@pytest.fixture(autouse=True) +def _pinned_output(): + """S4: pin the agentic output mode so prefixes never depend on the env.""" + from deepctl_core import output + + saved = dict(output._output_config) + output._output_config.update(agentic=True, format="default", quiet=False) + yield + output._output_config.clear() + output._output_config.update(saved) + + +def make_bundle(tmp, names=("api", "docs"), body="v1"): + """A fake validated bundle: each skill has SKILL.md and references/r.md.""" + base = Path(tmp) / f"bundle-{body}" + skills = [] + for name in names: + folder = base / "skills" / name + (folder / "references").mkdir(parents=True, exist_ok=True) + (folder / "SKILL.md").write_bytes(f"---\nname: {name}\n---\n{body}\n".encode()) + (folder / "references" / "r.md").write_bytes(f"ref {name} {body}\n".encode()) + skills.append(RepoSkill(name, folder)) + return skills + + +def gen(cli): + return next(g for g in get_all_generators() if g.cli_name == cli) + + +def root(cli="claude"): + return gen(cli).skills_root() + + +def symlink_or_skip(link, target, *, is_dir): + try: + Path(link).symlink_to(target, target_is_directory=is_dir) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"cannot create a symlink here: {exc}") + + +def rel(target, link): + """A relative symlink target, so no test ever compares a \\\\?\\ prefix.""" + return os.path.relpath(target, Path(link).parent) + + +def sha_tree(path): + """Map every entry under ``path`` (or the path itself) to a content digest.""" + path = Path(path) + st = os.lstat(path) + if sg._is_link(st): + return {"": "link"} + if not stat.S_ISDIR(st.st_mode): + return {"": hashlib.sha256(path.read_bytes()).hexdigest()} + out = {} + stack = [(path, "")] + while stack: + where, prefix = stack.pop() + for e in os.scandir(where): + est = e.stat(follow_symlinks=False) + key = prefix + e.name + if sg._is_link(est): + out[key] = "link" + elif stat.S_ISDIR(est.st_mode): + out[key] = "dir" + stack.append((Path(e.path), key + "/")) + elif stat.S_ISREG(est.st_mode): + out[key] = hashlib.sha256(Path(e.path).read_bytes()).hexdigest() + else: + out[key] = "special" + return out + + +def edit(path, text="mine\n"): + with open(Path(path) / "SKILL.md", "ab") as f: + f.write(text.encode()) + + +def state_bytes(): + try: + return sg._STATE_FILE.read_bytes() + except FileNotFoundError: + return None + + +def disk_state(): + return json.loads(sg._STATE_FILE.read_bytes()) + + +def records(cli="claude"): + return disk_state().get("skill_folders", {}).get(cli, {}).get("folders", {}) + + +def write_state(state): + sg._STATE_FILE.parent.mkdir(parents=True, exist_ok=True) + sg._STATE_FILE.write_text(json.dumps(state), encoding="utf-8") + + +def staging_dirs(where): + if not os.path.isdir(where): + return [] + return [n for n in os.listdir(where) if n.startswith(sg._STAGING_PREFIX)] + + +def install(skills, cli="claude", ref=REF): + return install_tool(gen(cli), skills, ref=ref, version="9.9.9") + + +def installed_copy(tmp, monkeypatch, cli, name): + """Install ``name`` for ``cli`` in a scratch home; return the folder and its fingerprint.""" + scratch = Path(tmp) / f"scratch-{cli}-{name}" + scratch.mkdir() + with monkeypatch.context() as m: + m.setattr(Path, "home", staticmethod(lambda: scratch)) + m.setattr(sg, "_STATE_FILE", scratch / "skills.json") + install(make_bundle(tmp, (name,), body=f"copy-{cli}"), cli) + folder = gen(cli).skills_root() / name + return folder, _fingerprint(folder) + + +def wrap(monkeypatch, owner, name, before=None): + """Patch ``owner.name`` with a wrapper that runs ``before(*args)`` first.""" + real = getattr(owner, name) + + def wrapper(*args, **kwargs): + if before is not None: + result = before(*args, **kwargs) + if result is not None: + return result + return real(*args, **kwargs) + + monkeypatch.setattr(owner, name, wrapper) + return real + + +def is_aside(src, dst): + """True for the move-aside rename: dest -> /old/.""" + d = Path(dst) + return d.parent.name == "old" and d.parent.parent.name.startswith( + sg._STAGING_PREFIX + ) + + +# --------------------------------------------------------------------------- +# B5: a destination that appears after the check is never taken over +# --------------------------------------------------------------------------- + + +class TestB5: + def test_dest_created_after_install_conflicts_survives(self, tmp_path): + skills = make_bundle(tmp_path) + assert install_conflicts([gen("claude")], skills) == ([], []) + api = root() / "api" + api.mkdir(parents=True) + (api / "user.txt").write_bytes(b"mine") + with pytest.raises(SkillOwnershipError) as exc: + install(skills) + assert str(exc.value) == _msg("E1", paths=str(api)) + assert (api / "user.txt").read_bytes() == b"mine" + assert not (api / sg._MARKER).exists() + assert state_bytes() is None + + def test_dest_created_during_staging_survives(self, tmp_path, monkeypatch): + skills = make_bundle(tmp_path) + api = root() / "api" + + def racer(src, dst, *a, **k): + if Path(dst).name == "api": + api.mkdir(parents=True) + (api / "user.txt").write_bytes(b"mine") + + wrap(monkeypatch, shutil, "copytree", racer) + with pytest.raises(SkillOwnershipError) as exc: + install(skills) + assert str(exc.value) == _msg("E2", gen("claude"), dest=api) + assert sha_tree(api) == {"user.txt": hashlib.sha256(b"mine").hexdigest()} + assert "api" not in records() + assert staging_dirs(root()) == [] + + @pytest.mark.parametrize("racer", ["made", "remade"]) + def test_empty_dir_made_right_before_the_move_survives( + self, tmp_path, monkeypatch, racer + ): + """Greg r1 B2: the real no-replace move refuses another process's empty dir.""" + skills = make_bundle(tmp_path) + api, made = root() / "api", [] + + def race(src, dest): + if Path(dest) == api and not made: + api.mkdir() + if racer == "remade": # Greg's case: removed and made again. + api.rmdir() + api.mkdir() + made.append(os.lstat(api).st_ino) + + wrap(monkeypatch, sg, "_rename_excl", race) + with pytest.raises(SkillOwnershipError) as exc: + install(skills) + assert str(exc.value) == _msg("E2", gen("claude"), dest=api) + assert os.lstat(api).st_ino == made[0] and os.listdir(api) == [] + assert "api" not in disk_state().get("skill_folders", {}).get("claude", {}) + assert staging_dirs(root()) == [] + + def test_empty_dir_made_right_before_the_update_move_survives( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + api, docs, made = root() / "api", root() / "docs", [] + old, docs_rec = sha_tree(api), records()["docs"] + + def race(src, dest): + if Path(dest) == api and Path(src).parent.name == "new": + api.mkdir() # After the move aside, before the place. + made.append(os.lstat(api).st_ino) + + wrap(monkeypatch, sg, "_rename_excl", race) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, body="v2")) + aside = exc.value.leftover / "old" / "api" + assert str(exc.value) == _msg("E6", gen("claude"), name="api", aside=aside) + assert os.lstat(api).st_ino == made[0] and os.listdir(api) == [] + assert sha_tree(aside) == old + assert records()["docs"] == docs_rec + assert _ownership(docs, "claude", "docs", docs_rec) == "ok" + + def test_old_copy_put_back_after_the_racer_leaves_keeps_its_record( + self, tmp_path, monkeypatch + ): + """The racer's empty dir is gone by cleanup, so the old copy goes back proven.""" + install(make_bundle(tmp_path)) + docs, real_upd = root() / "docs", sg._update_state + old = sha_tree(docs) + + def race(src, dest): + if Path(dest) == docs and Path(src).parent.name == "new": + docs.mkdir() + + def settle_then_leave(mutate, *a, **k): + real_upd(mutate, *a, **k) + if mutate.__name__ == "settle" and os.path.isdir(docs): + docs.rmdir() # After the settle write, before cleanup. + + with monkeypatch.context() as m: + wrap(m, sg, "_rename_excl", race) + m.setattr(sg, "_update_state", settle_then_leave) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E6b", gen("claude"), name="docs") + assert str(exc.value) == ( + "docs was not updated for Claude Code because something appeared at" + " its folder during the update, so its previous copy is back in place;" + " check that folder, then run the command again." ) + assert exc.value.leftover is None and staging_dirs(root()) == [] + assert sha_tree(docs) == old + assert _ownership(docs, "claude", "docs", records()["docs"]) == "ok" + install(make_bundle(tmp_path, body="v3")) + assert b"v3" in (docs / "SKILL.md").read_bytes() + + @pytest.mark.parametrize("other", ["takes the old copy", "copies it to dest"]) + def test_old_copy_not_put_back_by_cleanup_still_gives_e6( + self, tmp_path, monkeypatch, other + ): + install(make_bundle(tmp_path)) + docs, real_cleanup = root() / "docs", sg._cleanup + + def race(src, dest): + if Path(dest) == docs and Path(src).parent.name == "new": + docs.mkdir() + + def cleanup(staging, *a): + old = staging / "old" / "docs" + if other == "takes the old copy": + shutil.move(old, tmp_path / "taken") # Gone, but not back at dest. + else: + docs.rmdir() + shutil.copytree(old, docs, symlinks=True) # Proves, but not moved. + return real_cleanup(staging, *a) - with patch.object(gen, "get_skill_paths", return_value=[target]): - result = gen.generate([_make_command()], "1.0.0") - content = result[target] - assert "old content" not in content - assert "before\n" in content - assert "after\n" in content - - def test_remove_section(self, tmp_path): - gen = CodexGenerator() - target = tmp_path / "instructions.md" - target.write_text( - "before\n" - "\n" - "content\n" - "\n" - "after\n" + wrap(monkeypatch, sg, "_rename_excl", race) + monkeypatch.setattr(sg, "_cleanup", cleanup) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value).startswith("Could not install docs for Claude Code,") + if exc.value.leftover: + assert os.path.isdir(exc.value.leftover / "old" / "docs") + + @pytest.mark.parametrize( + "kind", ["empty dir", "dir", "file", "dir link", "dangling link"] + ) + def test_place_never_replaces_anything_at_dest(self, tmp_path, kind): + src, dest, target = tmp_path / "src", tmp_path / "dest", tmp_path / "t" + src.mkdir() + (src / "f").write_bytes(b"ours") + target.mkdir() + if kind in ("empty dir", "dir"): + dest.mkdir() + if kind == "dir": + (dest / "f").write_bytes(b"theirs") + elif kind == "file": + dest.write_bytes(b"theirs") + else: + gone = target if kind == "dir link" else tmp_path / "gone" + symlink_or_skip(dest, rel(gone, dest), is_dir=True) + before, ino = sha_tree(dest), os.lstat(dest).st_ino + with pytest.raises(FileExistsError): + _place(src, dest) + assert (sha_tree(dest), os.lstat(dest).st_ino) == (before, ino) + assert sha_tree(src) == {"f": hashlib.sha256(b"ours").hexdigest()} + assert os.listdir(target) == [] + + @pytest.mark.parametrize("kind", ["file", "dangling link", "fifo"]) + def test_place_moves_a_non_dir_whole(self, tmp_path, kind): + src, dest = tmp_path / "src", tmp_path / "dest" + if kind == "file": + src.write_bytes(b"mine") + elif kind == "fifo": + if not hasattr(os, "mkfifo"): + pytest.skip("no FIFOs here") + os.mkfifo(src) + else: + symlink_or_skip(src, "nowhere", is_dir=False) + before = os.lstat(src) + _place(src, dest) + after = os.lstat(dest) + assert not os.path.lexists(src) + assert (after.st_ino, after.st_mode) == (before.st_ino, before.st_mode) + if kind == "dangling link": + assert os.readlink(dest) == "nowhere" + src.write_bytes(b"again") + with pytest.raises(FileExistsError): + _place(src, dest) + assert os.lstat(dest).st_ino == before.st_ino + assert src.read_bytes() == b"again" + + def test_windows_branch_uses_one_plain_rename(self, tmp_path, monkeypatch): + src, dest = tmp_path / "src", tmp_path / "dest" + src.mkdir() + calls = [] + wrap(monkeypatch, os, "rename", lambda a, b, *x, **k: calls.append((a, b))) + monkeypatch.setattr(sg, "_WINDOWS", True) + monkeypatch.setattr(sg.ctypes, "CDLL", None) # Never reached on Windows. + _place(src, dest) + assert [(Path(a), Path(b)) for a, b in calls] == [(src, dest)] + assert dest.is_dir() + + @pytest.mark.parametrize( + ("code", "why"), + [ + (errno.EINVAL, sg._NO_EXCL), + (errno.ENOSYS, sg._NO_EXCL_SYS), # The kernel, not the filesystem. + (errno.EXDEV, os.strerror(errno.EXDEV)), + ] + + [(e, sg._NO_EXCL) for e in sorted({errno.ENOTSUP, errno.EOPNOTSUPP})], + ) + def test_no_replace_call_errors(self, tmp_path, monkeypatch, code, why): + src, dest = tmp_path / "src", tmp_path / "dest" + src.mkdir() + monkeypatch.setattr(sg, "_WINDOWS", False) + fake = SimpleNamespace(renameat2=lambda *a: -1, renamex_np=lambda *a: -1) + monkeypatch.setattr(sg.ctypes, "CDLL", lambda *a, **k: fake) + monkeypatch.setattr(sg.ctypes, "get_errno", lambda: code) + with pytest.raises(OSError) as exc: + _place(src, dest) + assert (exc.value.errno, exc.value.strerror) == (code, why) + monkeypatch.setattr(sg.ctypes, "CDLL", lambda *a, **k: SimpleNamespace()) + with pytest.raises(OSError) as exc: # The call itself is missing. + _place(src, dest) + assert (exc.value.errno, exc.value.strerror) == (errno.ENOSYS, sg._NO_EXCL_SYS) + assert src.is_dir() and not os.path.lexists(dest) + + @staticmethod + def _old_glibc(monkeypatch, machine, *, bits64=True, plat="linux", ret=0, **libc): + """A libc with ``syscall`` (plus any ``libc`` names); record syscall args.""" + calls = [] + + def syscall(*args): + calls.append(args) + return ret + + monkeypatch.setattr(sg, "_WINDOWS", False) + monkeypatch.setattr( + sg, + "sys", + SimpleNamespace(platform=plat, maxsize=2**63 - 1 if bits64 else 2**31 - 1), + ) + monkeypatch.setattr(sg, "platform", SimpleNamespace(machine=lambda: machine)) + monkeypatch.setattr( + sg.ctypes, + "CDLL", + lambda *a, **k: SimpleNamespace(syscall=syscall, **libc), + ) + return calls + + @pytest.mark.parametrize( + ("machine", "bits64", "raw"), + [ + ("x86_64", True, True), + ("armv7l", False, True), + ("mips64", True, False), # No number: the wrapper. + ("x86_64", False, False), # x32 or 32-bit Python: the wrapper. + ], + ) + def test_listed_linux_machines_skip_the_glibc_wrapper( + self, tmp_path, monkeypatch, machine, bits64, raw + ): + """glibc 2.28+ turns ENOSYS into EINVAL, so listed machines call the kernel.""" + src, dest = tmp_path / "src", tmp_path / "dest" + wrapped = [] + calls = self._old_glibc( + monkeypatch, + machine, + bits64=bits64, + renameat2=lambda *a: wrapped.append(a) or 0, ) + _place(src, dest) + assert (len(calls), len(wrapped)) == ((1, 0) if raw else (0, 1)) + args = [-100, os.fsencode(src), -100, os.fsencode(dest), 1] + assert list((calls or wrapped)[0][raw:]) == args + + _NR = sg._NR_RENAMEAT2[sys.maxsize < 2**32].get(sg.platform.machine()) + + @pytest.mark.skipif( + sys.platform != "linux" or _NR is None, reason="Linux on a listed machine" + ) + def test_real_linux_move_goes_through_the_raw_syscall(self, tmp_path, monkeypatch): + real, used = sg.ctypes.CDLL(None, use_errno=True), [] + + def syscall(*args): + used.append(args[0].value) + return real.syscall(*args) + + monkeypatch.setattr( + sg.ctypes, "CDLL", lambda *a, **k: SimpleNamespace(syscall=syscall) + ) + src, empty, dest = tmp_path / "src", tmp_path / "empty", tmp_path / "dest" + src.mkdir() + empty.mkdir() + ino = os.lstat(empty).st_ino + with pytest.raises(FileExistsError): + _place(src, empty) + assert os.lstat(empty).st_ino == ino and os.listdir(empty) == [] + _place(src, dest) + assert dest.is_dir() and not os.path.lexists(src) + assert used == [self._NR, self._NR] + + @pytest.mark.parametrize( + ("machine", "bits64", "nr"), + [ + ("x86_64", True, 316), + ("aarch64", True, 276), + ("arm64", True, 276), + ("riscv64", True, 276), + ("ppc64", True, 357), + ("ppc64le", True, 357), + ("s390x", True, 347), + ("i386", False, 353), + ("i686", False, 353), + ("armv7l", False, 382), + ("armv6l", False, 382), + ("arm", False, 382), + ], + ) + def test_old_glibc_calls_the_renameat2_syscall_by_number( + self, tmp_path, monkeypatch, machine, bits64, nr + ): + src, dest = tmp_path / "src", tmp_path / "dest" + calls = self._old_glibc(monkeypatch, machine, bits64=bits64) + _place(src, dest) + [(num, *rest)] = calls + assert isinstance(num, sg.ctypes.c_long) and num.value == nr + assert rest == [-100, os.fsencode(src), -100, os.fsencode(dest), 1] + + @pytest.mark.parametrize( + ("machine", "bits64", "plat"), + [ + ("mips64", True, "linux"), + ("armv8l", False, "linux"), + ("", True, "linux"), + # 32-bit Python on a 64-bit kernel, or x32: fail closed, never guess. + ("x86_64", False, "linux"), + ("aarch64", False, "linux"), + # A 64-bit Python under the linux32 personality. + ("i686", True, "linux"), + ("armv7l", True, "linux"), + # Only Linux has these numbers. + ("x86_64", True, "freebsd14"), + ("arm64", True, "darwin"), + ], + ) + def test_old_glibc_without_a_known_number_fails_closed( + self, tmp_path, monkeypatch, machine, bits64, plat + ): + src, dest = tmp_path / "src", tmp_path / "dest" + src.mkdir() + calls = self._old_glibc(monkeypatch, machine, bits64=bits64, plat=plat) + with pytest.raises(OSError) as exc: + _place(src, dest) + assert (exc.value.errno, exc.value.strerror) == (errno.ENOSYS, sg._NO_EXCL_SYS) + assert calls == [] and src.is_dir() and not os.path.lexists(dest) + + def test_old_glibc_without_syscall_fails_closed(self, tmp_path, monkeypatch): + src, dest = tmp_path / "src", tmp_path / "dest" + self._old_glibc(monkeypatch, "x86_64") + monkeypatch.setattr(sg.ctypes, "CDLL", lambda *a, **k: SimpleNamespace()) + with pytest.raises(OSError) as exc: + _place(src, dest) + assert (exc.value.errno, exc.value.strerror) == (errno.ENOSYS, sg._NO_EXCL_SYS) + + @pytest.mark.parametrize( + ("code", "want", "why"), + [ + (errno.ENOSYS, errno.ENOSYS, sg._NO_EXCL_SYS), # A kernel older than 3.15. + (errno.EINVAL, errno.EINVAL, sg._NO_EXCL), + (errno.ENOTSUP, errno.ENOTSUP, sg._NO_EXCL), + (errno.EOPNOTSUPP, errno.EOPNOTSUPP, sg._NO_EXCL), + (errno.EEXIST, errno.EEXIST, os.strerror(errno.EEXIST)), + (0, errno.EIO, os.strerror(errno.EIO)), # Never "Success". + ], + ) + def test_raw_syscall_errors(self, tmp_path, monkeypatch, code, want, why): + src, dest = tmp_path / "src", tmp_path / "dest" + unused = lambda *a: pytest.fail("the glibc wrapper hides ENOSYS") # noqa: E731 + self._old_glibc(monkeypatch, "aarch64", ret=-1, renameat2=unused) + monkeypatch.setattr(sg.ctypes, "get_errno", lambda: code) + with pytest.raises(OSError) as exc: + _place(src, dest) + assert (exc.value.errno, exc.value.strerror) == (want, why) + + def test_a_failed_call_that_leaves_errno_zero_never_reads_success( + self, tmp_path, monkeypatch + ): + src, dest = tmp_path / "src", tmp_path / "dest" + src.mkdir() + monkeypatch.setattr(sg, "_WINDOWS", False) + fake = SimpleNamespace(renameat2=lambda *a: -1, renamex_np=lambda *a: -1) + monkeypatch.setattr(sg.ctypes, "CDLL", lambda *a, **k: fake) + monkeypatch.setattr(sg.ctypes, "get_errno", lambda: 0) + with pytest.raises(OSError) as exc: + _place(src, dest) + assert (exc.value.errno, exc.value.strerror) == ( + errno.EIO, + os.strerror(errno.EIO), + ) + assert src.is_dir() and not os.path.lexists(dest) + + def test_ctrl_c_at_the_remove_probe_leaves_no_staging(self, tmp_path, monkeypatch): + install(make_bundle(tmp_path)) + saved, tree = state_bytes(), sha_tree(root()) + + def interrupt(src, dest): + if Path(dest).name == "old": + raise KeyboardInterrupt + + wrap(monkeypatch, sg, "_rename_excl", interrupt) + with pytest.raises(KeyboardInterrupt): + remove_tool(gen("claude")) + assert staging_dirs(root()) == [] + assert (state_bytes(), sha_tree(root())) == (saved, tree) + + @pytest.mark.parametrize("op", ["install", "update", "remove"]) + def test_unsupported_no_replace_move_changes_nothing( + self, tmp_path, monkeypatch, op + ): + if op != "install": + install(make_bundle(tmp_path)) + root().mkdir(parents=True, exist_ok=True) + saved, tree = state_bytes(), sha_tree(root()) + + def unsupported(src, dest): + raise OSError(errno.EINVAL, sg._NO_EXCL, str(dest)) + + monkeypatch.setattr(sg, "_rename_excl", unsupported) + with pytest.raises(SkillInstallError) as exc: + if op == "remove": + remove_tool(gen("claude")) + else: + install(make_bundle(tmp_path, body="v2")) + if op == "remove": + assert str(exc.value) == _msg("E21", root=root(), reason=sg._NO_EXCL) + else: + assert str(exc.value) == _msg("E5", gen("claude"), reason=sg._NO_EXCL) + assert (state_bytes(), sha_tree(root())) == (saved, tree) + + @NT + def test_windows_rename_refuses_dest_created_just_before( + self, tmp_path, monkeypatch + ): + skills = make_bundle(tmp_path) + api = root() / "api" + + def racer(a, b, *x, **k): + if Path(b) == api: + api.mkdir() + (api / "user.txt").write_bytes(b"mine") + + wrap(monkeypatch, os, "rename", racer) + with pytest.raises(SkillOwnershipError) as exc: + install(skills) + assert str(exc.value) == _msg("E2", gen("claude"), dest=api) + assert sha_tree(api) == {"user.txt": hashlib.sha256(b"mine").hexdigest()} + + +# --------------------------------------------------------------------------- +# B6: the record is saved before anything moves +# --------------------------------------------------------------------------- + + +class TestB6: + def test_record_before_swap_failure_moves_nothing(self, tmp_path, monkeypatch): + """_write_state is the write both save_skills_state() and the installer use.""" + install(make_bundle(tmp_path)) + v1, saved = sha_tree(root()), state_bytes() + calls = [] + + def fail(state): + calls.append(1) + if len(calls) == 1: + raise OSError(errno.ENOSPC, os.strerror(errno.ENOSPC)) + + wrap(monkeypatch, sg, "_write_state", fail) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, ("api", "docs", "new"), body="v2")) + reason = os.strerror(errno.ENOSPC).rstrip(".") + assert str(exc.value) == _msg("E9", gen("claude"), reason=reason) + assert sha_tree(root()) == v1 + assert not (root() / "new").exists() + assert staging_dirs(root()) == [] + assert state_bytes() == saved + + def test_mark_save_read_error_passes_through_unwrapped(self, tmp_path, monkeypatch): + install(make_bundle(tmp_path)) + v1 = sha_tree(root()) + reads = [] + real = Path.read_bytes + + def read_bytes(self): + if self == sg._STATE_FILE: + reads.append(1) + if len(reads) == 2: # The mark's re-read. + raise PermissionError(errno.EACCES, "Permission denied") + return real(self) + + monkeypatch.setattr(Path, "read_bytes", read_bytes) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E8", reason="Permission denied") + assert sha_tree(root()) == v1 + assert staging_dirs(root()) == [] + + def test_save_skills_state_failure_after_shim_install_keeps_records( + self, tmp_path, monkeypatch + ): + skills = make_bundle(tmp_path) + monkeypatch.setattr(skill_bundle, "fetch_skill_bundle", lambda ref=None: skills) + state = get_skills_state() + paths = gen("claude").install([], "x") + state["installed_skills"]["claude"] = {"paths": [str(p) for p in paths]} + wrap( + monkeypatch, + sg, + "_write_state", + lambda s: (_ for _ in ()).throw(OSError(errno.EIO, "I/O error")), + ) + with pytest.raises(SkillInstallError) as exc: + save_skills_state(state) + assert str(exc.value) == _msg("E9c", reason="I/O error") + assert set(records()) == {"api", "docs"} + + def test_final_save_failure_leaves_installing_records_that_prove_ownership( + self, tmp_path, monkeypatch + ): + skills = make_bundle(tmp_path) + calls = [] + + def fail_second(state): + calls.append(1) + if len(calls) == 2: + raise OSError(errno.ENOSPC, "No space left on device") + + with monkeypatch.context() as m: + wrap(m, sg, "_write_state", fail_second) + with pytest.raises(SkillInstallError) as exc: + install(skills) + assert str(exc.value) == _msg( + "E9b", gen("claude"), reason="No space left on device" + ) + for name in ("api", "docs"): + folder = root() / name + assert _marker_ok(folder, "claude", name) + rec = records()[name] + assert rec["state"] == "installing" + assert rec["pending"] == _fingerprint(folder) + assert install_conflicts([gen("claude")], skills) == ([], []) + install(skills) + assert records() == { + n: {"state": "installed", "fingerprint": _fingerprint(root() / n)} + for n in ("api", "docs") + } + assert sorted(remove_tool(gen("claude")).removed) == [ + root() / "api", + root() / "docs", + ] + assert os.listdir(root()) == [] + + +# --------------------------------------------------------------------------- +# B7: nothing is deleted by name +# --------------------------------------------------------------------------- + + +class TestB7: + @pytest.mark.parametrize( + "name", [".api.tmp-123", ".api.old-1-2", ".deepctl-staging-mine"] + ) + def test_user_dot_names_survive_install_update_remove(self, tmp_path, name): + mine = root() / name + mine.mkdir(parents=True) + (mine / "keep.txt").write_bytes(b"keep") + _, leftover = install(make_bundle(tmp_path)) + assert leftover is None + _, leftover = install(make_bundle(tmp_path, body="v2")) + assert leftover is None + if name.startswith(sg._STAGING_PREFIX): + assert tool_status(gen("claude"), get_skills_state()).leftovers == [mine] + res = remove_tool(gen("claude")) + assert res.leftover is None + assert len(res.removed) == 2 + assert (mine / "keep.txt").read_bytes() == b"keep" + + def test_state_dir_temp_lookalike_survives(self, tmp_path): + lookalike = sg._STATE_FILE.parent / ".skills.json.x.tmp" + lookalike.parent.mkdir(parents=True) + lookalike.write_bytes(b"mine") + install(make_bundle(tmp_path)) + save_skills_state(get_skills_state()) + remove_tool(gen("claude")) + assert lookalike.read_bytes() == b"mine" + + +# --------------------------------------------------------------------------- +# Ownership +# --------------------------------------------------------------------------- + + +class TestOwnership: + def test_unrelated_user_skill_survives_install_and_remove(self, tmp_path): + mine = root() / "my-skill" + mine.mkdir(parents=True) + (mine / "SKILL.md").write_bytes(b"mine") + install(make_bundle(tmp_path)) + remove_tool(gen("claude")) + assert sha_tree(mine) == {"SKILL.md": hashlib.sha256(b"mine").hexdigest()} + + def test_same_name_unrecorded_folder_makes_install_refuse(self, tmp_path): + api = root() / "api" + api.mkdir(parents=True) + (api / "SKILL.md").write_bytes(b"mine") + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path)) + assert str(exc.value) == _msg("E1", paths=str(api)) + assert sha_tree(api) == {"SKILL.md": hashlib.sha256(b"mine").hexdigest()} + + @pytest.mark.parametrize( + "kind", + [ + "dir", + "empty dir", + "file", + "live dir link", + "live file link", + "dangling dir link", + "dangling file link", + ], + ) + def test_unrecorded_dest_kinds_refuse(self, tmp_path, kind): + api = root() / "api" + root().mkdir(parents=True) + target = tmp_path / "target" + if kind == "dir": + api.mkdir() + (api / "x").write_bytes(b"x") + elif kind == "empty dir": + api.mkdir() + elif kind == "file": + api.write_bytes(b"x") + elif kind == "live dir link": + target.mkdir() + (target / "x").write_bytes(b"x") + symlink_or_skip(api, rel(target, api), is_dir=True) + elif kind == "live file link": + target.write_bytes(b"x") + symlink_or_skip(api, rel(target, api), is_dir=False) + elif kind == "dangling dir link": + symlink_or_skip(api, rel(tmp_path / "gone", api), is_dir=True) + else: + symlink_or_skip(api, rel(tmp_path / "gone", api), is_dir=False) + before = sha_tree(api) + target_before = sha_tree(target) if os.path.lexists(target) else None + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path)) + assert str(exc.value) == _msg("E1", paths=str(api)) + assert state_bytes() is None # No "installing" write happened (SF7). + assert sha_tree(api) == before + if target_before is not None: + assert sha_tree(target) == target_before + + def test_empty_dir_at_a_recorded_name_is_left_alone(self, tmp_path): + """deepctl never makes an empty folder at a skill name, so one is never its own.""" + install(make_bundle(tmp_path)) + docs = root() / "docs" + shutil.rmtree(docs) + docs.mkdir() + ino = os.lstat(docs).st_ino + st = tool_status(gen("claude"), get_skills_state()) + assert [p.name for p in st.kinds["unproven"]] == ["docs"] + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E1", paths=str(docs)) + res = remove_tool(gen("claude")) + assert [p.name for p in res.removed] == ["api"] + assert (res.left_alone, res.kept) == ([docs], []) + assert "skill_folders" not in disk_state() or records() == {} + assert os.lstat(docs).st_ino == ino and os.listdir(docs) == [] + crashed = {"state": "installing", "pending": FP_A, "run": "f" * 32} + write_state({"skill_folders": {"claude": {"folders": {"docs": crashed}}}}) + with pytest.raises(SkillOwnershipError) as exc: # A crashed run's record too. + install(make_bundle(tmp_path)) + assert str(exc.value) == _msg("E1", paths=str(docs)) + assert os.lstat(docs).st_ino == ino and os.listdir(docs) == [] + + def test_recorded_name_now_symlink_is_never_written_through(self, tmp_path): + install(make_bundle(tmp_path)) + api, mine = root() / "api", Path.home() / "mine" / "api" + mine.parent.mkdir() + os.rename(api, mine) + symlink_or_skip(api, rel(mine, api), is_dir=True) + assert ( + _fingerprint(mine) == records()["api"]["fingerprint"] + ) # A real install (SF7). + before, saved = sha_tree(mine), state_bytes() + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E1", paths=str(api)) + assert state_bytes() == saved + assert api.is_symlink() + assert sha_tree(mine) == before + + def test_recorded_folder_without_marker_is_left_alone(self, tmp_path): + install(make_bundle(tmp_path)) + api = root() / "api" + (api / sg._MARKER).unlink() + before = sha_tree(api) + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E1", paths=str(api)) + res = remove_tool(gen("claude")) + assert res.left_alone == [api] + assert sha_tree(api) == before + + def test_recorded_folder_losing_marker_after_precheck_gives_e3( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + api = root() / "api" + + def drop_marker(src, dst, *a, **k): + if Path(dst).name == "api": + (api / sg._MARKER).unlink() + + wrap(monkeypatch, shutil, "copytree", drop_marker) + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E3", gen("claude"), dest=api) + assert not (api / sg._MARKER).exists() + assert b"v1" in (api / "SKILL.md").read_bytes() + assert staging_dirs(root()) == [] + + def test_marker_is_bound_to_the_tool(self, tmp_path, monkeypatch): + folder, fp = installed_copy(tmp_path, monkeypatch, "claude", "api") + api = root("cursor") / "api" + shutil.copytree(folder, api) + write_state( + { + "skill_folders": { + "cursor": { + "folders": {"api": {"state": "installed", "fingerprint": fp}} + } + } + } + ) + assert _fingerprint(api) == fp + before, saved = sha_tree(api), state_bytes() + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path), "cursor") + assert str(exc.value) == _msg("E1", paths=str(api)) + assert sha_tree(api) == before + assert state_bytes() == saved + + @pytest.mark.parametrize("kind", ["dir", "symlink", "fifo", "big"]) + def test_marker_must_be_a_small_regular_file(self, tmp_path, kind): + folder = tmp_path / "api" + folder.mkdir() + marker, text = folder / sg._MARKER, sg._marker_text("claude", "api").encode() + if kind == "dir": + marker.mkdir() + elif kind == "symlink": + (tmp_path / "real").write_bytes(text) + symlink_or_skip(marker, rel(tmp_path / "real", marker), is_dir=False) + elif kind == "fifo": + if not hasattr(os, "mkfifo"): + pytest.skip("no FIFOs on this OS") + os.mkfifo(marker) + else: + marker.write_bytes(text + b"x" * 1024) + assert _marker_ok(folder, "claude", "api") is False + + def test_marker_symlink_refused_without_o_nofollow(self, tmp_path, monkeypatch): + folder = tmp_path / "api" + folder.mkdir() + marker, text = folder / sg._MARKER, sg._marker_text("claude", "api").encode() + marker.write_bytes(text) + assert _marker_ok(folder, "claude", "api") is True + monkeypatch.delattr(os, "O_NOFOLLOW", raising=False) + other = tmp_path / "other" + other.write_bytes(text) # Exact marker bytes, but a different file. + swapped = [] + + def swap_after_lstat(path, *a, **k): + if Path(path) == marker and not swapped: + swapped.append(1) + os.replace(other, marker) + + with monkeypatch.context() as m: + wrap(m, os, "open", swap_after_lstat) + assert _marker_ok(folder, "claude", "api") is False + assert swapped + marker.unlink() + (tmp_path / "real").write_bytes(text) + symlink_or_skip(marker, rel(tmp_path / "real", marker), is_dir=False) + assert _marker_ok(folder, "claude", "api") is False + + def test_skills_cli_symlink_farm_is_refused(self, tmp_path): + agents_api = root("codex") / "api" + agents_api.mkdir(parents=True) + (agents_api / "SKILL.md").write_bytes(b"npx") + claude_api = root("claude") / "api" + root("claude").mkdir(parents=True) + symlink_or_skip(claude_api, rel(agents_api, claude_api), is_dir=True) + before = sha_tree(agents_api) + for cli, dest in (("claude", claude_api), ("codex", agents_api)): + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path), cli) + assert str(exc.value) == _msg("E1", paths=str(dest)) + assert claude_api.is_symlink() + assert sha_tree(agents_api) == before + assert state_bytes() is None + + def test_symlinked_root_is_followed(self, tmp_path): + real = Path.home() / "dotfiles" / "skills" + real.mkdir(parents=True) + (Path.home() / ".claude").mkdir() + symlink_or_skip(root(), rel(real, root()), is_dir=True) + install(make_bundle(tmp_path)) + assert root().is_symlink() + assert _marker_ok(real / "api", "claude", "api") + + @NT + def test_junction_at_dest_is_a_conflict(self, tmp_path): + import _winapi + + target = tmp_path / "target" + target.mkdir() + root().mkdir(parents=True) + _winapi.CreateJunction(str(target), str(root() / "api")) + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path)) + assert str(exc.value) == _msg("E1", paths=str(root() / "api")) + assert target.is_dir() + + def test_reparse_attribute_counts_as_link(self): + fake = SimpleNamespace(st_mode=stat.S_IFDIR | 0o755, st_file_attributes=0x400) + assert sg._is_link(fake) is True + assert sg._is_link(SimpleNamespace(st_mode=stat.S_IFDIR | 0o755)) is False + + +# --------------------------------------------------------------------------- +# Edits: the content fingerprint +# --------------------------------------------------------------------------- + + +class TestEdits: + def test_edited_folder_survives_update(self, tmp_path): + install(make_bundle(tmp_path)) + api, docs = root() / "api", root() / "docs" + edit(api) + edited, docs_before, saved = sha_tree(api), sha_tree(docs), state_bytes() + v2 = make_bundle(tmp_path, body="v2") + assert install_conflicts([gen("claude")], v2) == ([], [api]) + with pytest.raises(SkillOwnershipError) as exc: + install(v2) + assert str(exc.value) == _msg("E22", dest=api) + assert sha_tree(api) == edited + assert sha_tree(docs) == docs_before + assert staging_dirs(root()) == [] + assert state_bytes() == saved + + def test_edited_folder_survives_remove(self, tmp_path): + install(make_bundle(tmp_path)) + api = root() / "api" + edit(api) + edited = sha_tree(api) + res = remove_tool(gen("claude")) + assert res.edited == [api] + assert res.removed == [root() / "docs"] + assert sha_tree(api) == edited + assert not (root() / "docs").exists() + state = disk_state() + assert "claude" not in state.get("skill_folders", {}) + assert "claude" not in state["installed_skills"] + again = remove_tool(gen("claude")) + assert again.removed == again.edited == again.left_alone == [] + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E1", paths=str(api)) + + @pytest.mark.parametrize("op", ["update", "remove"]) + def test_edit_racing_move_is_caught_after_move_and_put_back( + self, tmp_path, monkeypatch, op + ): + install(make_bundle(tmp_path)) + api = root() / "api" + + def racer(src, dst, *a, **k): + if Path(src) == api and is_aside(src, dst): + edit(api) # Lands after the pre-check, before the move. + + wrap(monkeypatch, os, "rename", racer) + expected = sha_tree(api) + expected["SKILL.md"] = hashlib.sha256( + (api / "SKILL.md").read_bytes() + b"mine\n" + ).hexdigest() + if op == "update": + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E22", dest=api) + else: + assert remove_tool(gen("claude")).edited == [api] + assert sha_tree(api) == expected + assert staging_dirs(root()) == [] + + @pytest.mark.parametrize( + "how", ["added link", "folder swapped for a link to an identical copy"] + ) + def test_symlink_inside_deepctl_folder_blocks_replace_and_remove( + self, tmp_path, how + ): + install(make_bundle(tmp_path)) + api, docs = root() / "api", root() / "docs" + if how == "added link": + link = api / "link" + symlink_or_skip(link, os.path.join("..", "docs"), is_dir=True) + else: # Followed, the tree would hash exactly as installed. + link, copy_ = api / "references", tmp_path / "references-copy" + shutil.copytree(link, copy_) + shutil.rmtree(link) + symlink_or_skip(link, rel(copy_, link), is_dir=True) + before = sha_tree(api) + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E22", dest=api) + assert remove_tool(gen("claude")).edited == [api] + assert sha_tree(api) == before + assert link.is_symlink() + assert not docs.exists() # docs was ours and unedited, so remove took it. + + @POSIX + def test_special_file_inside_blocks_replace_and_remove(self, tmp_path): + install(make_bundle(tmp_path)) + api = root() / "api" + os.mkfifo(api / "pipe") + before = sha_tree(api) + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E22", dest=api) + assert remove_tool(gen("claude")).edited == [api] + assert sha_tree(api) == before + + @pytest.mark.parametrize("op", ["update", "remove"]) + def test_added_file_blocks_replace_and_remove(self, tmp_path, op): + install(make_bundle(tmp_path)) + api = root() / "api" + (api / "notes.md").write_bytes(b"my notes") + before = sha_tree(api) + if op == "update": + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E22", dest=api) + else: + assert remove_tool(gen("claude")).edited == [api] + assert sha_tree(api) == before + + def test_edit_inside_staging_before_cleanup_keeps_the_copy( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + + def tamper(mutate, *a, **k): + if mutate.__name__ == "settle": + (copy_,) = (root() / s / "old" / "api" for s in staging_dirs(root())) + edit(copy_) + + wrap(monkeypatch, sg, "_update_state", tamper) + _, leftover = install(make_bundle(tmp_path, body="v2")) + assert leftover is not None + assert b"mine" in (leftover / "old" / "api" / "SKILL.md").read_bytes() + assert b"v2" in (root() / "api" / "SKILL.md").read_bytes() + + @pytest.mark.parametrize("op", ["update", "remove", "status"]) + def test_unreadable_folder_is_never_called_edited(self, tmp_path, monkeypatch, op): + install(make_bundle(tmp_path)) + api = root() / "api" + before, rec = sha_tree(api), records()["api"] + target = os.fspath(api / "SKILL.md") + + def deny(path, *a, **k): + if os.fspath(path) == target: + raise PermissionError(errno.EACCES, "Permission denied", target) + + with monkeypatch.context() as m: + wrap(m, os, "open", deny) + if op == "update": + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg( + "E5", gen("claude"), reason=f"could not read {api}" + ) + assert staging_dirs(root()) == [] + elif op == "remove": + res = remove_tool(gen("claude")) + assert res.kept == [(api, "deepctl could not read it")] + assert res.edited == [] + assert res.removed == [root() / "docs"] + assert records()["api"] == rec + else: + st = tool_status(gen("claude"), get_skills_state()) + assert st.kinds["unreadable"] == [api] + assert st.kinds["edited"] == [] + assert sha_tree(api) == before + if op == "remove": + assert remove_tool(gen("claude")).removed == [api] + + def test_settle_keeps_an_unreadable_record_exactly(self, tmp_path, monkeypatch): + install(make_bundle(tmp_path)) + api, seen = root() / "api", {} + + def deny(path): + raise PermissionError(errno.EACCES, "Permission denied", str(path)) + + def unreadable_at_settle(mutate, *a, **k): + if mutate.__name__ == "settle": # Settle only: staging read fine. + seen["rec"] = records()["api"] # As mark left it. + monkeypatch.setattr(sg, "_fingerprint", deny) + + wrap(monkeypatch, sg, "_update_state", unreadable_at_settle) + install(make_bundle(tmp_path, body="v2")) + assert seen["rec"]["state"] == "installing" + assert records()["api"] == seen["rec"] + assert b"v2" in (api / "SKILL.md").read_bytes() + + def test_fingerprint_definition(self, tmp_path, monkeypatch): + def tree(where, order): + where.mkdir() + for name in order: + if name == "sub": + (where / "sub").mkdir() + (where / "sub" / "b.md").write_bytes(b"b") + else: + (where / name).write_bytes(name.encode()) + return where + + a = tree(tmp_path / "a", ["x.md", "sub", sg._MARKER]) + b = tree(tmp_path / "b", [sg._MARKER, "sub", "x.md"]) + base = _fingerprint(a) + assert base == _fingerprint(a) == _fingerprint(b) + assert base and sg._FP_PATTERN.fullmatch(base) + + def changed(mutate): + mutate() + fp = _fingerprint(b) + shutil.rmtree(b) + tree(b, ["x.md", "sub", sg._MARKER]) + return fp != base + + assert changed(lambda: (b / "x.md").write_bytes(b"y")) + assert changed(lambda: (b / "new.md").write_bytes(b"")) + assert changed(lambda: (b / "x.md").unlink()) + assert changed(lambda: os.rename(b / "x.md", b / "z.md")) + assert changed(lambda: (b / "empty").mkdir()) + assert changed(lambda: (b / sg._MARKER).write_bytes(b"other marker")) + os.utime(b / "x.md", (1, 1)) + os.chmod(b / "x.md", 0o444) # Still readable; on Windows, the read-only flag. + assert _fingerprint(b) == base + os.chmod(b / "x.md", 0o644) + + fake = SimpleNamespace(name="r", path=str(a / "x.md")) + fake.stat = lambda follow_symlinks=True: SimpleNamespace( + st_mode=stat.S_IFREG | 0o644, st_file_attributes=0x400 + ) + + class Scan: + def __enter__(self): + return iter([fake]) + + def __exit__(self, *exc): + return False + + with monkeypatch.context() as m: + m.setattr(os, "scandir", lambda p: Scan()) + assert _fingerprint(a) is None + with monkeypatch.context() as m: + m.setattr( + os, + "scandir", + lambda p: (_ for _ in ()).throw( + PermissionError(errno.EACCES, "denied") + ), + ) + with pytest.raises(OSError): + _fingerprint(a) + if hasattr(os, "mkfifo"): + os.mkfifo(b / "pipe") + assert _fingerprint(b) is None + os.unlink(b / "pipe") + symlink_or_skip(b / "link", "x.md", is_dir=False) + assert _fingerprint(b) is None + + def test_fingerprint_matches_a_reference_digest(self, tmp_path): + top = tmp_path / "t" + (top / "references" / "deep").mkdir(parents=True) + (top / "SKILL.md").write_bytes(b"skill") + (top / "references" / "r.md").write_bytes(b"ref") + (top / "references" / "deep" / "empty").mkdir() + (top / sg._MARKER).write_bytes(sg._marker_text("claude", "t").encode()) + entries = [] + for dirpath, dirs, files in os.walk(top): + prefix = Path(dirpath).relative_to(top).as_posix() + prefix = "" if prefix == "." else prefix + "/" + entries += [(os.fsencode(prefix + d), b"d", None) for d in dirs] + entries += [ + (os.fsencode(prefix + f), b"f", Path(dirpath) / f) for f in files + ] + h = hashlib.sha256(b"deepctl-skill-tree-v1\0") + for key, kind, file in sorted(entries, key=lambda e: e[0]): + h.update(kind + len(key).to_bytes(4, "big") + key) + if file: + h.update(hashlib.sha256(file.read_bytes()).digest()) + assert _fingerprint(top) == "sha256:" + h.hexdigest() + + def test_fingerprint_size_cap(self, tmp_path, monkeypatch): + monkeypatch.setattr(sg, "_MAX_TREE_BYTES", 10) + top = tmp_path / "t" + top.mkdir() + (top / "a").write_bytes(b"12345") + (top / "b").write_bytes(b"12345") + assert _fingerprint(top) is not None + (top / "c").write_bytes(b"1") + assert _fingerprint(top) is None + + def test_record_without_fingerprint_is_not_ours(self, tmp_path): + install(make_bundle(tmp_path)) + state = disk_state() + state["skill_folders"]["claude"]["folders"]["api"] = {"state": "installed"} + write_state(state) + api = root() / "api" + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E1", paths=str(api)) + res = remove_tool(gen("claude")) + assert res.left_alone == [api] + assert api.is_dir() + assert "claude" not in disk_state().get("skill_folders", {}) + + @pytest.mark.parametrize("crash", ["before swap", "after swap", "neither"]) + def test_crashed_installing_record_proves_by_old_or_pending(self, tmp_path, crash): + install(make_bundle(tmp_path)) + api = root() / "api" + fp = records()["api"]["fingerprint"] + rec = { + "before swap": {"state": "installing", "fingerprint": fp, "pending": FP_A}, + "after swap": {"state": "installing", "pending": fp}, + "neither": {"state": "installing", "fingerprint": FP_A, "pending": FP_B}, + }[crash] + state = disk_state() + state["skill_folders"]["claude"]["folders"]["api"] = rec + write_state(state) + if crash == "neither": + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E22", dest=api) + return + install(make_bundle(tmp_path, body="v2")) + assert b"v2" in (api / "SKILL.md").read_bytes() + assert records()["api"] == { + "state": "installed", + "fingerprint": _fingerprint(api), + } + assert api in remove_tool(gen("claude")).removed + + +# --------------------------------------------------------------------------- +# Update +# --------------------------------------------------------------------------- + + +class TestUpdate: + def test_update_replaces_recorded_folder(self, tmp_path): + install(make_bundle(tmp_path)) + placed, leftover = install(make_bundle(tmp_path, body="v2")) + api = root() / "api" + assert placed == [api, root() / "docs"] + assert leftover is None + assert b"v2" in (api / "SKILL.md").read_bytes() + assert records()["api"] == { + "state": "installed", + "fingerprint": _fingerprint(api), + } + assert disk_state()["skill_folders"]["claude"]["version"] == "9.9.9" + + def test_update_restores_old_when_new_place_fails(self, tmp_path, monkeypatch): + install(make_bundle(tmp_path)) + api = root() / "api" + old = sha_tree(api) + + def fail(src, dest): + if Path(src).parent.name == "new" and Path(src).name == "api": + raise OSError(errno.EIO, "I/O error") + + wrap(monkeypatch, sg, "_place", fail) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E5", gen("claude"), reason="I/O error") + assert sha_tree(api) == old + assert staging_dirs(root()) == [] + assert records()["api"]["state"] == "installed" + + def test_restore_failure_keeps_previous_copy_where_e6_says( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + api = root() / "api" + old = sha_tree(api) + + def racer(src, dest): + if Path(src).parent.name == "new" and Path(src).name == "api": + api.mkdir() + (api / "user.txt").write_bytes(b"theirs") + + wrap(monkeypatch, sg, "_place", racer) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, body="v2")) + aside = exc.value.leftover / "old" / "api" + assert exc.value.leftover.parent == root() + assert str(exc.value) == _msg("E6", gen("claude"), name="api", aside=aside) + assert sha_tree(aside) == old + assert sha_tree(api) == {"user.txt": hashlib.sha256(b"theirs").hexdigest()} + + def test_old_copy_stays_at_e6_path_when_staged_copy_and_dest_change( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + api, stash = root() / "api", tmp_path / "stash" + old = sha_tree(api) + + def outsider(src, dest): + if Path(src).parent.name == "new" and Path(src).name == "api": + os.rename(src, stash) # Moved away by something outside deepctl. + api.mkdir() # And a user recreates the destination. + (api / "user.txt").write_bytes(b"theirs") + + wrap(monkeypatch, sg, "_place", outsider) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, body="v2")) + aside = exc.value.leftover / "old" / "api" + assert str(exc.value) == _msg("E6", gen("claude"), name="api", aside=aside) + assert sha_tree(aside) == old + assert sha_tree(api) == {"user.txt": hashlib.sha256(b"theirs").hexdigest()} + assert b"v2" in (stash / "SKILL.md").read_bytes() + + def test_update_moved_aside_dir_failing_reproof_is_put_back( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + api, stash = root() / "api", tmp_path / "stash" + + def swap_in_user_dir(src, dst, *a, **k): + if Path(src) == api and is_aside(src, dst): + os.rename(api, stash) + api.mkdir() + (api / "user.txt").write_bytes(b"mine") + + wrap(monkeypatch, os, "rename", swap_in_user_dir) + with pytest.raises(SkillOwnershipError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E3", gen("claude"), dest=api) + assert sha_tree(api) == {"user.txt": hashlib.sha256(b"mine").hexdigest()} + + def test_update_moved_aside_file_kept_in_staging(self, tmp_path, monkeypatch): + install(make_bundle(tmp_path)) + api, stash = root() / "api", tmp_path / "stash" + state = {"moved": False} + real_rename = os.rename + + def swap_in_file(src, dst, *a, **k): + if Path(src) == api and is_aside(src, dst) and not state["moved"]: + state["moved"] = True + real_rename(api, stash) + api.write_bytes(b"user file") + real_rename(src, dst) + api.mkdir() # And a racer takes dest, so the put-back fails. + (api / "racer.txt").write_bytes(b"racer") + return True + + wrap(monkeypatch, os, "rename", swap_in_file) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, body="v2")) + aside = exc.value.leftover / "old" / "api" + assert str(exc.value) == _msg("E4", dest=api, aside=aside) + assert aside.read_bytes() == b"user file" + assert (api / "racer.txt").read_bytes() == b"racer" + + def test_update_ctrl_c_during_place_restores_old(self, tmp_path, monkeypatch): + install(make_bundle(tmp_path)) + api = root() / "api" + old = sha_tree(api) + + def interrupt(src, dest): + if Path(src).parent.name == "new" and Path(src).name == "api": + raise KeyboardInterrupt + + wrap(monkeypatch, sg, "_place", interrupt) + with pytest.raises(KeyboardInterrupt): + install(make_bundle(tmp_path, body="v2")) + assert sha_tree(api) == old + assert staging_dirs(root()) == [] + + def test_update_ctrl_c_during_aside_reproof_puts_old_back( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + api = root() / "api" + old = sha_tree(api) + fired = [] + + def interrupt(path): # Every time: cleanup must never need to re-prove it. + p = Path(path) + if p.name == "api" and p.parent.name == "old": + fired.append(1) + raise KeyboardInterrupt + + wrap(monkeypatch, sg, "_fingerprint", interrupt) + with pytest.raises(KeyboardInterrupt): + install(make_bundle(tmp_path, body="v2")) + assert fired + assert sha_tree(api) == old + assert _ownership(api, "claude", "api", records()["api"]) == "ok" + assert staging_dirs(root()) == [] + + def test_update_ctrl_c_during_mark_save_leaves_no_staging( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + before, saved = sha_tree(root()), state_bytes() + + def interrupt(state): + raise KeyboardInterrupt + + wrap(monkeypatch, sg, "_write_state", interrupt) + with pytest.raises(KeyboardInterrupt): + install(make_bundle(tmp_path, body="v2")) + assert sha_tree(root()) == before + assert state_bytes() == saved + assert staging_dirs(root()) == [] + + def test_cleanup_deletes_old_only_when_new_is_in_place(self, tmp_path, monkeypatch): + install(make_bundle(tmp_path)) + api, docs = root() / "api", root() / "docs" + + def interrupt_settle(mutate, *a, **k): + if mutate.__name__ == "settle": + raise KeyboardInterrupt + + with monkeypatch.context() as m: + wrap(m, sg, "_update_state", interrupt_settle) + with pytest.raises(KeyboardInterrupt): + install(make_bundle(tmp_path, body="v2")) + for folder in (api, docs): + assert b"v2" in (folder / "SKILL.md").read_bytes() + rec = records()[folder.name] + assert rec["state"] == "installing" + assert rec["pending"] == _fingerprint(folder) + assert staging_dirs(root()) == [] + + install(make_bundle(tmp_path, body="v3")) + docs_v3 = sha_tree(docs) + calls = [] + + def interrupt_docs(src, dest): + if Path(src).parent.name == "new" and Path(src).name == "docs": + calls.append("place") + raise KeyboardInterrupt + if Path(src).parent.name == "old" and calls == ["place"]: + calls.append("put-back") + raise OSError(errno.EIO, "I/O error") + + wrap(monkeypatch, sg, "_place", interrupt_docs) + with pytest.raises(KeyboardInterrupt): + install(make_bundle(tmp_path, body="v4")) + assert calls == ["place", "put-back"] + assert sha_tree(docs) == docs_v3 # Cleanup's invariant put it back. + assert b"v4" in (api / "SKILL.md").read_bytes() + assert staging_dirs(root()) == [] + + def test_ctrl_c_between_move_aside_and_place_keeps_the_record( + self, tmp_path, monkeypatch + ): + """The put-back fails too, so cleanup restores the copy after settle.""" + install(make_bundle(tmp_path)) + api = root() / "api" + old = sha_tree(api) + calls = [] + + def interrupt(src, dest): + if Path(src).parent.name == "new" and Path(src).name == "api": + calls.append("place") + raise KeyboardInterrupt + if Path(src).parent.name == "old" and calls == ["place"]: + calls.append("put-back") + raise OSError(errno.EIO, "I/O error") + + with monkeypatch.context() as m: + wrap(m, sg, "_place", interrupt) + with pytest.raises(KeyboardInterrupt): + install(make_bundle(tmp_path, body="v2")) + assert calls == ["place", "put-back"] + assert sha_tree(api) == old + assert _ownership(api, "claude", "api", records()["api"]) == "ok" + assert staging_dirs(root()) == [] + assert install_conflicts([gen("claude")], make_bundle(tmp_path, body="v3")) == ( + [], + [], + ) + install(make_bundle(tmp_path, body="v3")) + assert b"v3" in (api / "SKILL.md").read_bytes() + assert records()["api"] == { + "state": "installed", + "fingerprint": _fingerprint(api), + } + + def test_update_move_aside_permission_error_changes_nothing( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + api = root() / "api" + old = sha_tree(api) + + def deny(src, dst, *a, **k): + if Path(src) == api and is_aside(src, dst): + raise PermissionError(errno.EACCES, "Permission denied") + + wrap(monkeypatch, os, "rename", deny) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg("E5", gen("claude"), reason="Permission denied") + assert sha_tree(api) == old + assert records()["api"]["state"] == "installed" + + def test_failed_update_keeps_the_recorded_ref(self, tmp_path, monkeypatch): + install(make_bundle(tmp_path)) + + def fail(*a, **k): + raise SkillInstallError("boom") + + monkeypatch.setattr(sg, "_swap", fail) + with pytest.raises(SkillInstallError): + install(make_bundle(tmp_path, body="v2"), ref="f" * 40) + assert disk_state()["skill_folders"]["claude"]["skills_ref"] == REF + + def test_update_leaves_names_not_in_bundle_alone(self, tmp_path): + install(make_bundle(tmp_path, ("api", "docs", "extra"))) + extra, rec = sha_tree(root() / "extra"), records()["extra"] + install(make_bundle(tmp_path, body="v2")) + assert sha_tree(root() / "extra") == extra + assert records()["extra"] == rec + + +# --------------------------------------------------------------------------- +# Remove +# --------------------------------------------------------------------------- + + +class TestRemove: + def test_a_user_made_staging_lookalike_without_a_marker_is_ignored(self, tmp_path): + install(make_bundle(tmp_path)) + notes = root() / ".deepctl-staging-x" / "old" / "api" / "notes.txt" + notes.parent.mkdir(parents=True) + notes.write_text("user data\n") + shutil.rmtree(root() / "api") + res = remove_tool(gen("claude")) + assert res.removed == [root() / "docs"] + assert (res.stranded, res.kept, res.moved, res.leftover) == ([], [], [], None) + assert records() == {} and notes.read_text() == "user data\n" + + def test_a_symlinked_staging_folder_is_ignored(self, tmp_path): + install(make_bundle(tmp_path)) + outside = tmp_path / "outside" / "old" / "api" + outside.mkdir(parents=True) # A marked copy: ignored only for the link. + (outside / sg._MARKER).write_bytes(sg._marker_text("claude", "api").encode()) + before = sha_tree(outside) + link = root() / ".deepctl-staging-lnk" + symlink_or_skip(link, rel(tmp_path / "outside", link), is_dir=True) + shutil.rmtree(root() / "api") + res = remove_tool(gen("claude")) + assert res.removed == [root() / "docs"] + assert (res.stranded, res.kept, res.moved, res.leftover) == ([], [], [], None) + assert records() == {} and sha_tree(outside) == before + + def test_remove_deletes_only_proven_recorded_folders(self, tmp_path): + install(make_bundle(tmp_path)) + mine = root() / "my-skill" + mine.mkdir() + (mine / "SKILL.md").write_bytes(b"mine") + res = remove_tool(gen("claude")) + assert sorted(res.removed) == [root() / "api", root() / "docs"] + assert os.listdir(root()) == ["my-skill"] + + def test_remove_failed_move_aside_stays_recorded_and_retries( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + api = root() / "api" + with monkeypatch.context() as m: + + def deny(src, dst, *a, **k): + if Path(src) == api: + raise PermissionError(errno.EACCES, "Permission denied") + + wrap(m, os, "rename", deny) + res = remove_tool(gen("claude")) + assert res.kept == [(api, "Permission denied")] + assert res.removed == [root() / "docs"] + assert set(records()) == {"api"} + assert remove_tool(gen("claude")).removed == [api] + assert not api.exists() + + def test_remove_reproves_after_move_aside(self, tmp_path, monkeypatch): + install(make_bundle(tmp_path)) + api, stash = root() / "api", tmp_path / "stash" + + def swap_in_user_dir(src, dst, *a, **k): + if Path(src) == api and is_aside(src, dst): + os.rename(api, stash) + api.mkdir() + (api / "user.txt").write_bytes(b"mine") + + wrap(monkeypatch, os, "rename", swap_in_user_dir) + res = remove_tool(gen("claude")) + assert res.left_alone == [api] + assert sha_tree(api) == {"user.txt": hashlib.sha256(b"mine").hexdigest()} + + def test_remove_ctrl_c_during_aside_reproof_puts_it_back( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + api = root() / "api" + old = sha_tree(api) + fired = [] + + def interrupt(path): # Every time: cleanup must never need to re-prove it. + p = Path(path) + if p.name == "api" and p.parent.name == "old": + fired.append(1) + raise KeyboardInterrupt + + wrap(monkeypatch, sg, "_fingerprint", interrupt) + with pytest.raises(KeyboardInterrupt): + remove_tool(gen("claude")) + assert sha_tree(api) == old + assert "api" in records() + assert staging_dirs(root()) == [] + + def test_remove_puts_back_a_file_swapped_in_before_the_move( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + api, stash = root() / "api", tmp_path / "stash" + + def swap_in_file(src, dst, *a, **k): + if Path(src) == api and is_aside(src, dst): + os.rename(api, stash) + api.write_bytes(b"user file") + + wrap(monkeypatch, os, "rename", swap_in_file) + res = remove_tool(gen("claude")) + assert res.left_alone == [api] + assert api.read_bytes() == b"user file" + assert "api" not in records() + assert (res.moved, res.leftover) == ([], None) + + @pytest.mark.parametrize("times", [1, 2]) + def test_remove_ctrl_c_during_put_back(self, tmp_path, monkeypatch, times): + install(make_bundle(tmp_path, ("api",))) + api = root() / "api" + real_rename, real_place, fired = os.rename, sg._place, [] + + def rename(src, dst, *a, **k): + real_rename(src, dst, *a, **k) + if Path(src) == api and is_aside(src, dst): + edit(dst, "raced the move\n") # The edit lands in the moved copy. + return True + + def place(src, dest): + if Path(src).parent.name == "old" and len(fired) < times: + fired.append(1) + raise KeyboardInterrupt + return real_place(src, dest) + + wrap(monkeypatch, os, "rename", rename) + monkeypatch.setattr(sg, "_place", place) + with pytest.raises(KeyboardInterrupt): + remove_tool(gen("claude")) + if times == 1: # The retry puts the edited folder back. + assert (api / "SKILL.md").read_bytes().endswith(b"raced the move\n") + assert staging_dirs(root()) == [] + else: # Still in staging, so its record stays. + assert not os.path.lexists(api) + (left,) = staging_dirs(root()) + assert ( + (root() / left / "old" / "api" / "SKILL.md") + .read_bytes() + .endswith(b"raced the move\n") + ) + assert "api" in records() + + def test_remove_keeps_a_moved_copy_it_cannot_put_back(self, tmp_path, monkeypatch): + install(make_bundle(tmp_path, ("api",))) + api, moved = root() / "api", [] + + def race(src, dst, *a, **k): + if Path(src) == api and is_aside(src, dst): + real(src, dst) + edit(dst, "raced the move\n") # The edit lands in the moved copy, + api.mkdir() # and something else takes the name. + moved.append(Path(dst)) + return True + return None + + real = wrap(monkeypatch, os, "rename", race) + res = remove_tool(gen("claude")) + assert res.moved == [(api, moved[0])] + assert (moved[0] / "SKILL.md").read_bytes().endswith(b"raced the move\n") + assert res.leftover == moved[0].parent.parent + assert "api" in records() + + @pytest.mark.parametrize("put_back", ["works", "fails"]) + def test_remove_ctrl_c_right_after_move_aside( + self, tmp_path, monkeypatch, put_back + ): + install(make_bundle(tmp_path, ("api",))) + api = root() / "api" + old, moved = sha_tree(api), [] + + def interrupt(src, dst, *a, **k): + if Path(src) == api and is_aside(src, dst): + real(src, dst) + moved.append(Path(dst)) + raise KeyboardInterrupt + + def place(src, dest): + if put_back == "fails" and Path(src).parent.name == "old": + raise OSError(errno.EACCES, "Permission denied") + return real_place(src, dest) + + real = wrap(monkeypatch, os, "rename", interrupt) + real_place = sg._place + monkeypatch.setattr(sg, "_place", place) + with pytest.raises(KeyboardInterrupt): + remove_tool(gen("claude")) + assert "api" in records() + if put_back == "works": + assert sha_tree(api) == old + assert staging_dirs(root()) == [] + else: # Stuck in staging, so its record stays. + assert not os.path.lexists(api) + assert sha_tree(moved[0]) == old + + def test_remove_rmtree_failure_reports_staging(self, tmp_path, monkeypatch): + install(make_bundle(tmp_path)) + wrap(monkeypatch, shutil, "rmtree", lambda path, *a, **k: True) + res = remove_tool(gen("claude")) + assert res.leftover is not None + assert res.leftover.parent == root() + assert sorted(os.listdir(res.leftover / "old")) == ["api", "docs"] + assert records() == {} # Proven ours and gone from dest: E12 covers it. + + def test_remove_dest_vanishing_before_the_move_drops_its_record( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + api = root() / "api" + + def vanish(src, dst, *a, **k): + if Path(src) == api and is_aside(src, dst): + shutil.rmtree(api) + + wrap(monkeypatch, os, "rename", vanish) + res = remove_tool(gen("claude")) + assert res.removed == [root() / "docs"] + assert records() == {} + + def test_remove_drops_tool_record_and_mirror_when_empty(self, tmp_path): + install(make_bundle(tmp_path)) + assert "claude" in disk_state()["installed_skills"] + remove_tool(gen("claude")) + state = disk_state() + assert "claude" not in state.get("skill_folders", {}) + assert "claude" not in state["installed_skills"] + + def test_remove_missing_root_drops_records(self, tmp_path): + install(make_bundle(tmp_path)) + shutil.rmtree(root()) + res = remove_tool(gen("claude")) + assert res.removed == [] + assert "claude" not in disk_state().get("skill_folders", {}) + + @pytest.mark.parametrize("kind", ["file", "dangling link"]) + def test_remove_unreachable_root_refuses_and_keeps_records(self, tmp_path, kind): + install(make_bundle(tmp_path)) + shutil.rmtree(root()) + if kind == "file": + root().write_bytes(b"not a folder") + else: + symlink_or_skip(root(), rel(tmp_path / "gone", root()), is_dir=True) + saved = state_bytes() + with pytest.raises(SkillInstallError) as exc: + remove_tool(gen("claude")) + assert str(exc.value) == _msg("E18", gen("claude")) + assert state_bytes() == saved + + def test_remove_ctrl_c_keeps_unprocessed_names_recorded( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + docs = root() / "docs" + old = sha_tree(docs) + + def interrupt(path, *a, **k): + raise KeyboardInterrupt + + wrap(monkeypatch, shutil, "rmtree", interrupt) + with pytest.raises(KeyboardInterrupt): + remove_tool(gen("claude")) + assert "docs" in records() + assert sha_tree(docs) == old + + +# --------------------------------------------------------------------------- +# Staging, paths and the bundle +# --------------------------------------------------------------------------- + + +class TestStaging: + def test_crash_leftover_staging_is_reported_not_deleted(self, tmp_path): + crash = root() / ".deepctl-staging-old1" + crash.mkdir(parents=True) + (crash / "f").write_bytes(b"x") + _, leftover = install(make_bundle(tmp_path)) + assert leftover is None + assert (crash / "f").read_bytes() == b"x" + assert tool_status(gen("claude"), get_skills_state()).leftovers == [crash] + + @pytest.mark.parametrize("code", [errno.ENOSPC, errno.EACCES, errno.ENAMETOOLONG]) + def test_copy_failure_changes_nothing(self, tmp_path, monkeypatch, code): + def fail(*a, **k): + raise OSError(code, os.strerror(code)) + + wrap(monkeypatch, shutil, "copytree", fail) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path)) + assert str(exc.value) == _msg( + "E5", gen("claude"), reason=os.strerror(code).rstrip(".") + ) + assert os.listdir(root()) == [] + assert state_bytes() is None + + def test_bundle_marker_collision_refuses(self, tmp_path): + skills = make_bundle(tmp_path) + (skills[0].path / sg._MARKER).write_bytes(b"bundle's own") + with pytest.raises(SkillInstallError) as exc: + install(skills) + assert str(exc.value) == _msg("E10", name="api") + assert os.listdir(root()) == [] + assert state_bytes() is None + + +# --------------------------------------------------------------------------- +# skills.json +# --------------------------------------------------------------------------- + + +class TestState: + @pytest.mark.parametrize( + "content", + [ + b"{", + b"[]", + b"\xff\xfe{}", + b'{"skill_folders": []}', + b'{"skill_folders": {"claude": {"folders": {"../x": {"state": "installed"}}}}}', + b'{"skill_folders": {"claude": {"folders": {"CON": {"state": "installed"}}}}}', + b'{"skill_folders": {"claude": {"folders": {"api": {"state": "weird"}}}}}', + b'{"skill_folders": {"claude": {"folders": {"api": "installed"}}}}', + b'{"skill_folders": {"claude": {"folders": {"api": {"state": "installed", "fingerprint": "md5:abc"}}}}}', + b'{"skill_folders": {"claude": {"skills_ref": 5, "folders": {}}}}', + b'{"skill_folders": {"claude": {"v03": "yes", "folders": {}}}}', + b'{"installed_skills": {"claude": "x"}}', + b'{"installed_skills": {"claude": {"paths": "/x/a.md"}}}', + b'{"installed_skills": {"claude": {"paths": [5]}}}', + ], + ) + def test_corrupt_state_is_refused_and_left_byte_identical(self, tmp_path, content): + sg._STATE_FILE.parent.mkdir(parents=True) + sg._STATE_FILE.write_bytes(content) + expected = _msg("E7") + for call in ( + get_skills_state, + lambda: save_skills_state({"installed_skills": {}}), + lambda: install(make_bundle(tmp_path)), + ): + with pytest.raises(SkillInstallError) as exc: + call() + assert str(exc.value) == expected + assert sg._STATE_FILE.read_bytes() == content + assert not root().exists() + + def test_save_failure_leaves_no_temp(self, monkeypatch): + def fail(fd): + raise OSError(errno.ENOSPC, "No space left on device") + + monkeypatch.setattr(os, "fsync", fail) + with pytest.raises(SkillInstallError) as exc: + save_skills_state({"installed_skills": {}}) + assert str(exc.value) == _msg("E9c", reason="No space left on device") + assert [ + n for n in os.listdir(sg._STATE_FILE.parent) if n.endswith(".tmp") + ] == [] + + def test_save_ctrl_c_removes_temp(self, monkeypatch): + def interrupt(fd): + raise KeyboardInterrupt + + monkeypatch.setattr(os, "fsync", interrupt) + with pytest.raises(KeyboardInterrupt): + save_skills_state({"installed_skills": {}}) + assert os.listdir(sg._STATE_FILE.parent) == ["skills.json.lock"] + + def test_symlinked_state_file_stays_a_symlink(self, tmp_path): + real = tmp_path / "dotfiles" / "skills.json" + real.parent.mkdir() + real.write_text('{"installed_skills": {}}', encoding="utf-8") + sg._STATE_FILE.parent.mkdir(parents=True) + symlink_or_skip(sg._STATE_FILE, rel(real, sg._STATE_FILE), is_dir=False) + save_skills_state({"installed_skills": {"x": {}}, "auto_update": False}) + assert sg._STATE_FILE.is_symlink() + assert json.loads(real.read_text(encoding="utf-8"))["auto_update"] is False + + def test_public_save_cannot_write_records(self): + forged = { + "claude": {"folders": {"api": {"state": "installed", "fingerprint": FP_A}}} + } + save_skills_state({"installed_skills": {}, "skill_folders": forged}) + assert "skill_folders" not in disk_state() + + +# --------------------------------------------------------------------------- +# Compatibility shim for login and plugin +# --------------------------------------------------------------------------- + + +class TestSurvivors: + """Each pins one guard a mutation sweep could otherwise drop unnoticed.""" + + def test_save_validates_before_writing(self): + write_state({"installed_skills": {}, "auto_update": True}) + saved = state_bytes() + with pytest.raises(SkillInstallError) as exc: + save_skills_state({"installed_skills": [], "auto_update": True}) + assert str(exc.value) == _msg("E7") + assert state_bytes() == saved + + def test_recorded_ref_is_validated(self): + state = { + "installed_skills": {}, + "skill_folders": {"claude": {"skills_ref": "../x"}}, + } + with pytest.raises(skill_bundle.SkillRefInvalidError): + sg._ref_for("claude", state) + + def test_non_portable_skill_name_is_refused_before_any_write(self, tmp_path): + (skill,) = make_bundle(tmp_path, ("api",)) + with pytest.raises(SkillInstallError) as exc: + install([RepoSkill("CON", skill.path)]) + assert str(exc.value) == _msg("E11", name="CON") + assert not root().exists() + assert state_bytes() is None + + def test_install_into_a_root_that_is_a_file_gives_e18(self, tmp_path): + root().parent.mkdir(parents=True) + root().write_bytes(b"not a folder") + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path)) + assert str(exc.value) == _msg("E18", gen("claude")) + assert state_bytes() is None + + def test_failed_first_install_leaves_no_empty_tool_record( + self, tmp_path, monkeypatch + ): + def fail(*a, **k): + raise OSError(errno.EIO, "I/O error") + + monkeypatch.setattr(sg, "_swap", fail) + with pytest.raises(SkillInstallError): + install(make_bundle(tmp_path)) + state = disk_state() + assert "claude" not in state.get("skill_folders", {}) + assert "claude" not in state["installed_skills"] + + def test_unreadable_copy_after_move_aside_gives_e5_and_goes_back( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + api = root() / "api" + before = sha_tree(api) + + def deny(path): + if Path(path).parent.name == "old": + raise PermissionError(errno.EACCES, "Permission denied", str(path)) + + wrap(monkeypatch, sg, "_fingerprint", deny) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, body="v2")) + assert str(exc.value) == _msg( + "E5", gen("claude"), reason=f"could not read {api}" + ) + assert sha_tree(api) == before + + @pytest.mark.parametrize("when", ["before", "after"]) + def test_ctrl_c_at_the_move_never_blocks_the_next_run( + self, tmp_path, monkeypatch, when + ): + skills = make_bundle(tmp_path) + docs, real, fired = root() / "docs", sg._rename_excl, [] + + def interrupt(src, dest): + if Path(dest) == docs and not fired: + fired.append(1) + if when == "after": + real(src, dest) + raise KeyboardInterrupt + return real(src, dest) + + with monkeypatch.context() as m: + m.setattr(sg, "_rename_excl", interrupt) + with pytest.raises(KeyboardInterrupt): + install(skills) + assert fired and os.path.lexists(docs) == (when == "after") + assert install_conflicts([gen("claude")], skills) == ([], []) + install(skills) + for n in ("api", "docs"): + assert records()[n]["state"] == "installed" + assert _ownership(root() / n, "claude", n, records()[n]) == "ok" + assert staging_dirs(root()) == [] + + def test_copy_failure_reason_is_one_line(self, tmp_path, monkeypatch): + many = [ + ( + f"/b/{i}", + f"/s/{i}", + f"[Errno 28] No space left on device: '/b/{i}' -> '/s/{i}'", + ) + for i in range(3) + ] + + def full(*a, **k): + raise shutil.Error(many) + + monkeypatch.setattr(shutil, "copytree", full) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path)) + assert str(exc.value) == _msg( + "E5", gen("claude"), reason="No space left on device" + ) + win = "[WinError 112] There is not enough space on the disk: 'C:\\b'" + many[:] = [("C:\\b", "C:\\s", win)] + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path)) + assert str(exc.value) == _msg( + "E5", gen("claude"), reason="There is not enough space on the disk" + ) + + def test_reason_drops_a_trailing_period(self): + assert sg._reason(OSError(errno.EACCES, "Permission denied.")) == ( + "Permission denied" + ) + + +class TestShim: + def test_stale_save_shim_login_flow_keeps_records(self, tmp_path, monkeypatch): + skills = make_bundle(tmp_path) + monkeypatch.setattr(skill_bundle, "fetch_skill_bundle", lambda ref=None: skills) + state = get_skills_state() + paths = gen("claude").install([], "x") + state["installed_skills"]["claude"] = { + "paths": [str(p) for p in paths], + "version": "x", + } + save_skills_state(state) + assert set(records()) == {"api", "docs"} + assert install_conflicts([gen("claude")], skills) == ([], []) + + def test_stale_save_shim_plugin_flow_keeps_records(self, tmp_path, monkeypatch): + skills = make_bundle(tmp_path) + monkeypatch.setattr(skill_bundle, "fetch_skill_bundle", lambda ref=None: skills) + write_state( + {"installed_skills": {"claude": {"paths": ["old"]}, "aider": {"paths": []}}} + ) + state = get_skills_state() + gens = {g.cli_name: g for g in get_all_generators()} + for cli, info in state["installed_skills"].items(): + info.update(paths=[str(p) for p in gens[cli].install([], "x")]) + save_skills_state(state) + assert set(records()) == {"api", "docs"} + assert install_conflicts([gen("claude")], skills) == ([], []) + + def test_shim_follows_recorded_ref(self, tmp_path, monkeypatch): + skills = make_bundle(tmp_path) + fetched = [] + monkeypatch.setattr( + skill_bundle, + "fetch_skill_bundle", + lambda ref=None: fetched.append(ref) or skills, + ) + install(skills, ref="my-branch") + gen("claude").install([], "x") + assert fetched == ["my-branch"] + assert disk_state()["skill_folders"]["claude"]["skills_ref"] == "my-branch" + monkeypatch.setenv(skill_bundle.REF_ENV_VAR, "env-ref") + gen("claude").install([], "x") + assert fetched[-1] == "env-ref" + monkeypatch.delenv(skill_bundle.REF_ENV_VAR) + gen("cursor").install([], "x") + assert fetched[-1] == REF + + def test_hint_only_install_returns_empty_and_never_raises(self, monkeypatch): + def boom(ref=None): + raise AssertionError("fetched") + + monkeypatch.setattr(skill_bundle, "fetch_skill_bundle", boom) + assert gen("amazonq").install([], "x") == [] + assert gen("aider").install([], "x") == [] + rule = Path.home() / ".amazonq" / "rules" / "deepctl.md" + write_state({"installed_skills": {"amazonq": {"paths": [str(rule)]}}}) + assert gen("amazonq").install([], "x") == [rule] # Keeps 0.3.x paths. + for bad in ({"installed_skills": {"amazonq": "x"}}, {"skill_folders": 1}): + write_state(bad) # Corrupt: [] so plugin's loop goes on. + assert gen("amazonq").install([], "x") == [] + + def test_remove_e21_keeps_records(self, tmp_path, monkeypatch): + install(make_bundle(tmp_path)) + saved = state_bytes() + + def denied(*a, **k): + raise PermissionError(errno.EACCES, "Permission denied") + + monkeypatch.setattr(sg.tempfile, "mkdtemp", denied) + with pytest.raises(SkillInstallError) as exc: + remove_tool(gen("claude")) + assert str(exc.value) == _msg("E21", root=root(), reason="Permission denied") + assert state_bytes() == saved + assert (root() / "api").is_dir() + + def test_shim_surface_exists(self): + for name in ( + "detect_ai_clis", + "get_all_generators", + "get_skills_state", + "save_skills_state", + "collect_command_metadata", + "_commands_hash", + ): + assert callable(getattr(sg, name)) + for g in get_all_generators(): + assert g.cli_name and g.display_name and callable(g.install) + assert [g.cli_name for g in get_all_generators()] == [ + "claude", + "codex", + "gemini", + "amazonq", + "aider", + "opencode", + "cursor", + "cline", + ] + + @pytest.mark.parametrize( + ("cli", "parts", "homes", "binary"), + [ + ("claude", (".claude", "skills"), [(".claude",)], "claude"), + ("codex", (".agents", "skills"), [(".codex",)], "codex"), + ("gemini", (".gemini", "skills"), [(".gemini",)], "gemini"), + ("amazonq", None, [(".amazonq",)], None), + ("aider", None, [], "aider"), + ( + "opencode", + (".config", "opencode", "skills"), + [(".opencode",), (".config", "opencode")], + "opencode", + ), + ("cursor", (".cursor", "skills"), [(".cursor",)], "cursor"), + ("cline", (".cline", "skills"), [(".cline",)], None), + ], + ) + def test_tool_table_roots_and_detection( + self, monkeypatch, cli, parts, homes, binary + ): + g = gen(cli) + assert g.skills_root() == (Path.home().joinpath(*parts) if parts else None) + monkeypatch.setattr(shutil, "which", lambda name: None) + assert g.detect() is False + for home in homes: + Path.home().joinpath(*home).mkdir(parents=True) + assert g.detect() is True + shutil.rmtree(Path.home().joinpath(*home)) + monkeypatch.setattr( + shutil, "which", lambda name: "/bin/x" if name == binary else None + ) + assert g.detect() is (binary is not None) + + +# --------------------------------------------------------------------------- +# Concurrency +# --------------------------------------------------------------------------- + + +class TestConcurrency: + def test_concurrent_install_fails_cleanly_and_keeps_records( + self, tmp_path, monkeypatch + ): + skills = make_bundle(tmp_path) + nested = [] + + def other_run_first(src, dest): + if Path(src).parent.name == "new" and not nested: + nested.append(1) + install(skills) + + wrap(monkeypatch, sg, "_place", other_run_first) + with pytest.raises(SkillOwnershipError) as exc: + install(skills) + assert str(exc.value) == _msg("E2", gen("claude"), dest=root() / "api") + assert exc.value.leftover is None + assert {n: r["state"] for n, r in records().items()} == { + "api": "installed", + "docs": "installed", + } + for n in ("api", "docs"): + assert _ownership(root() / n, "claude", n, records()[n]) == "ok" + assert staging_dirs(root()) == [] + + def test_login_save_during_install_keeps_records(self, tmp_path, monkeypatch): + stale = get_skills_state() + + def login_save(*a, **k): + save_skills_state(stale) + + wrap(monkeypatch, sg, "_swap", login_save) + install(make_bundle(tmp_path)) + assert set(records()) == {"api", "docs"} + assert all(r["state"] == "installed" for r in records().values()) + + def test_root_alias_second_tool_refuses_without_writing(self, tmp_path): + root("claude").mkdir(parents=True) + (Path.home() / ".cursor").mkdir() + symlink_or_skip( + root("cursor"), rel(root("claude"), root("cursor")), is_dir=True + ) + skills = make_bundle(tmp_path) + assert install_conflicts([gen("claude"), gen("cursor")], skills) == ([], []) + install(skills) + claude = sha_tree(root("claude")) + with pytest.raises(SkillOwnershipError) as exc: + install(skills, "cursor") + assert str(exc.value) == _msg( + "E1", paths=f"{root('cursor') / 'api'}, {root('cursor') / 'docs'}" + ) + assert sha_tree(root("claude")) == claude + assert "cursor" not in disk_state()["skill_folders"] + + def test_concurrent_runs_never_drop_each_others_records( + self, tmp_path, monkeypatch + ): + """B holds the lock in _stage; A waits for it, then installs over B (B1).""" + skills = make_bundle(tmp_path, ("api", "docs", "starters")) + b_staging, a_waiting = threading.Event(), threading.Event() + real_stage, real_try, results = sg._stage, sg._try_lock, {} + + def stage(g, s, staging): + if threading.current_thread().name == "B": + b_staging.set() + assert a_waiting.wait(10) # A is blocked on the lock B holds. + return real_stage(g, s, staging) + + def try_lock(lock): + got = real_try(lock) + if threading.current_thread().name == "A" and got < 0: + a_waiting.set() + return got + + def run(tag): + try: + results[tag] = [p.name for p in install(skills)[0]] + except SkillInstallError as exc: + results[tag] = str(exc) + + monkeypatch.setattr(sg, "_stage", stage) + monkeypatch.setattr(sg, "_try_lock", try_lock) + b = threading.Thread(target=run, args=("B",), name="B") + b.start() + assert b_staging.wait(10) + a = threading.Thread(target=run, args=("A",), name="A") + a.start() + a.join(20) + b.join(20) + assert results == {"A": ["api", "docs", "starters"], "B": results["A"]} + for n in ("api", "docs", "starters"): + assert records()[n]["state"] == "installed" + assert _ownership(root() / n, "claude", n, records()[n]) == "ok" + assert staging_dirs(root()) == [] + + def test_another_runs_pending_record_is_kept_and_a_crashed_one_settles( + self, tmp_path, monkeypatch + ): + skills = make_bundle(tmp_path) + other = {"state": "installing", "pending": FP_A, "run": "f" * 32} + + def other_run_marks_docs(g, name, staging, rec): + if name == "api": + sg._update_state( + lambda st: st["skill_folders"]["claude"]["folders"].update( + docs=dict(other) + ) + ) + else: + raise OSError(errno.EIO, "I/O error") + + with monkeypatch.context() as m: + wrap(m, sg, "_swap", other_run_marks_docs) + with pytest.raises(SkillInstallError): + install(skills) + assert records()["docs"] == other # Absent, but not this run's to drop. + assert records()["api"]["state"] == "installed" + install(skills) # The other run never came back: this run settles it. + for n in ("api", "docs"): + assert records()[n]["state"] == "installed" + assert _ownership(root() / n, "claude", n, records()[n]) == "ok" + + def test_remove_during_install_never_puts_the_old_copy_back( + self, tmp_path, monkeypatch + ): + """Remove deletes the new api; cleanup must not restore the old one (S1).""" + install(make_bundle(tmp_path)) + removed = [] + + def remove_after_api(g, name, staging, rec): + real(g, name, staging, rec) + if name == "api": + removed.append(remove_tool(gen("claude"))) + return True + + real = wrap(monkeypatch, sg, "_swap", remove_after_api) + install(make_bundle(tmp_path, body="v2")) + assert [p.name for p in removed[0].removed] == ["api", "docs"] + assert not os.path.lexists(root() / "api") + recs = records() + for n in os.listdir(root()): # No marked folder is left without a record. + assert _ownership(root() / n, "claude", n, recs.get(n)) == "ok", n + assert staging_dirs(root()) == [] + + def test_a_later_runs_marks_survive_this_runs_settle(self, tmp_path, monkeypatch): + skills = make_bundle(tmp_path) + real_upd, nested = sg._update_state, [] + + def later_run_marks_then_dies(g, name, staging, rec): + if not nested: + nested.append(1) + + def killed_after_mark(mutate, *a, **k): + real_upd(mutate, *a, **k) + raise KeyboardInterrupt + + with monkeypatch.context() as m: + m.setattr(sg, "_update_state", killed_after_mark) + with pytest.raises(KeyboardInterrupt): + install(skills) + raise OSError(errno.EIO, "I/O error") + + with monkeypatch.context() as m: + m.setattr(sg, "_swap", later_run_marks_then_dies) + with pytest.raises(SkillInstallError): + install(skills) + recs = records() + assert {n: r["state"] for n, r in recs.items()} == { + "api": "installing", + "docs": "installing", + } # The later run tagged them, so this run's settle left them. + assert len({r["run"] for r in recs.values()}) == 1 + install(skills) + for n in ("api", "docs"): + assert _ownership(root() / n, "claude", n, records()[n]) == "ok" + + def test_a_crashed_runs_record_is_retagged_and_dropped(self, tmp_path, monkeypatch): + skills = make_bundle(tmp_path) + crashed = {"state": "installing", "pending": FP_A, "run": "f" * 32} + write_state({"skill_folders": {"claude": {"folders": {"docs": crashed}}}}) + + def fail_docs(g, name, staging, rec): + if name == "docs": + raise OSError(errno.EIO, "I/O error") + + with monkeypatch.context() as m: + wrap(m, sg, "_swap", fail_docs) + with pytest.raises(SkillInstallError): + install(skills) + assert not os.path.lexists(root() / "docs") + assert set(records()) == {"api"} # This run's tag: absent, so dropped. + + +# --------------------------------------------------------------------------- +# B1: one skills.json lock across each tool's whole install and remove +# --------------------------------------------------------------------------- + + +def _hold_lock_child(home, mode, held, release, base=""): + """Spawned child: hold the lock, or park inside an install that holds it. + + Module level so a spawned interpreter can import it by name. The parent's + monkeypatches do not cross the process boundary, so the home comes in. + """ + home = Path(home) + Path.home = staticmethod(lambda: home) # type: ignore[method-assign] + sg._STATE_FILE = home / ".deepctl" / "skills" / "skills.json" + + def park(*a, **k): + held.set() + release.wait(60) + + if mode == "hold": + with sg._state_lock(): + park() + return + real_swap, real_place = sg._swap, sg._place + + def swap(*a, **k): + sg._swap = real_swap + park() + return real_swap(*a, **k) + + def place(src, dest): # Parks after _swap moved the old copy aside (S1). + if Path(src).parent.name == "new": + park() + return real_place(src, dest) + + if mode == "aside": + sg._place = place + else: + sg._swap = swap + skills = [RepoSkill(n, Path(base) / n) for n in ("api", "docs")] + install_tool(gen("claude"), skills, ref="child-ref", version="1") + + +@contextlib.contextmanager +def lock_child(monkeypatch, mode="hold", base=""): + """A real second interpreter holding the lock until the block ends.""" + # pytest's importlib mode does not put the test root on sys.path; a + # spawned child gets the parent's sys.path, so add it to import this file. + depth = len(__name__.split(".")) + monkeypatch.syspath_prepend(str(Path(__file__).resolve().parents[depth - 1])) + ctx = multiprocessing.get_context("spawn") + held, release = ctx.Event(), ctx.Event() + args = (str(Path.home()), mode, held, release, base) + child = ctx.Process(target=_hold_lock_child, args=args, daemon=True) + child.start() + try: + deadline = time.monotonic() + 60 + while not held.wait(0.05): # Fails fast if the child dies on startup. + assert child.is_alive(), f"the child exited ({child.exitcode})" + assert time.monotonic() < deadline, "the child never took the lock" + yield child, release + finally: + if child.is_alive(): # A killed sleeper would deadlock Event.set(). + release.set() + child.join(30) + if child.is_alive(): + child.kill() + + +@contextlib.contextmanager +def held_elsewhere(): + """Hold the lock on a second open file, as another process would.""" + lock = sg._STATE_FILE.with_name("skills.json.lock") + fd = sg._try_lock(lock) + assert fd >= 0 + try: + yield lock + finally: + if sys.platform == "win32": + sg.msvcrt.locking(fd, sg.msvcrt.LK_UNLCK, 1) + os.close(fd) + + +class TestStateLock: + def test_lock_held_by_another_process_gives_e27_and_changes_nothing( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + saved, tree = state_bytes(), sha_tree(root()) + monkeypatch.setattr(sg, "_LOCK_TIMEOUT", 0.3) + with lock_child(monkeypatch) as (child, _): + for op in ( + lambda: install(make_bundle(tmp_path, body="v2")), + lambda: remove_tool(gen("claude")), + lambda: save_skills_state(get_skills_state()), + ): + with pytest.raises(SkillInstallError) as exc: + op() + assert str(exc.value) == _msg("E27") + assert (state_bytes(), sha_tree(root())) == (saved, tree) + assert child.exitcode == 0 + with sg._state_lock(): # Released when the child let go. + pass + + def test_second_process_waits_then_installs_over_the_first( + self, tmp_path, monkeypatch + ): + v1, v2 = make_bundle(tmp_path), make_bundle(tmp_path, body="v2") + done = {} + with lock_child(monkeypatch, "install", str(v1[0].path.parent)) as (c, go): + t = threading.Thread( + target=lambda: done.update(r=install(v2, ref="parent-ref")) + ) + t.start() + t.join(0.5) + assert t.is_alive() # Waiting: the child is mid-install. + go.set() + t.join(30) + c.join(30) + assert c.exitcode == 0 + assert [p.name for p in done["r"][0]] == ["api", "docs"] + for n in ("api", "docs"): + assert records()[n]["state"] == "installed" + assert _ownership(root() / n, "claude", n, records()[n]) == "ok" + assert b"v2" in (root() / n / "SKILL.md").read_bytes() + assert disk_state()["skill_folders"]["claude"]["skills_ref"] == "parent-ref" + assert staging_dirs(root()) == [] + + def test_a_killed_holder_never_leaves_a_stale_lock(self, monkeypatch): + with lock_child(monkeypatch) as (child, _): + child.kill() # SIGKILL on POSIX, TerminateProcess on Windows. + child.join(30) + monkeypatch.setattr(sg, "_LOCK_TIMEOUT", 1.0) + start = time.monotonic() + with sg._state_lock(): + pass + assert time.monotonic() - start < 1.0 + assert sg._STATE_FILE.with_name("skills.json.lock").exists() + + def test_a_killed_replacement_keeps_its_record_and_old_copy( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + old = sha_tree(root() / "api") + v2 = make_bundle(tmp_path, body="v2") + with lock_child(monkeypatch, "aside", str(v2[0].path.parent)) as (child, _): + child.kill() # Old api is in the child's staging/old; new not placed. + child.join(30) + [aside] = [root() / d / "old" / "api" for d in staging_dirs(root())] + assert not os.path.lexists(root() / "api") and sha_tree(aside) == old + res = remove_tool(gen("claude")) + assert res.removed == [root() / "docs"] + assert res.stranded == [(root() / "api", aside)] + assert set(records()) == {"api"} and sha_tree(aside) == old + st = tool_status(gen("claude"), get_skills_state()) + assert st.leftovers == [aside.parents[1]] + install(v2) # Replaces api: its record is this run's now. + res = remove_tool(gen("claude")) + assert set(res.removed) == {root() / "api", root() / "docs"} + assert (res.stranded, res.kept, res.moved, res.leftover) == ([], [], [], None) + assert records() == {} and sha_tree(aside) == old # Left for the user. + + def test_a_killed_update_then_install_then_remove_does_not_fail( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + old = sha_tree(root() / "api") + v2 = make_bundle(tmp_path, body="v2") + with lock_child(monkeypatch, "aside", str(v2[0].path.parent)) as (child, _): + child.kill() + child.join(30) + [aside] = [root() / d / "old" / "api" for d in staging_dirs(root())] + install(v2) + assert records()["api"]["state"] == "installed" + res = remove_tool(gen("claude")) + assert set(res.removed) == {root() / "api", root() / "docs"} + assert (res.stranded, res.kept, res.moved, res.leftover) == ([], [], [], None) + assert records() == {} and sha_tree(aside) == old + + def test_lock_is_reentrant_in_one_thread_only(self, tmp_path, monkeypatch): + monkeypatch.setattr(sg, "_LOCK_TIMEOUT", 0.3) + seen = [] + + def other(): + try: + with sg._state_lock(): + seen.append("took it") + except SkillInstallError as exc: + seen.append(str(exc)) + + with sg._state_lock(), sg._state_lock(): # Nested: never waits. + install(make_bundle(tmp_path)) # Takes it again, and _update_state too. + t = threading.Thread(target=other) + t.start() + t.join(10) + assert seen == [_msg("E27")] + assert getattr(sg._LOCAL, "fd", None) is None + t = threading.Thread(target=other) + t.start() + t.join(10) + assert seen[1:] == ["took it"] + + def test_lock_file_sits_next_to_skills_json_and_is_never_rewritten(self, tmp_path): + lock = sg._STATE_FILE.with_name("skills.json.lock") + install(make_bundle(tmp_path)) + assert lock.is_file() + if os.name != "nt": + assert stat.S_IMODE(os.lstat(lock).st_mode) == 0o600 + lock.write_bytes(b"keep") + install(make_bundle(tmp_path, body="v2")) + remove_tool(gen("claude")) + assert lock.read_bytes() == b"keep" + + @POSIX + def test_symlinked_lock_or_read_only_folder_gives_e28(self, tmp_path): + if os.geteuid() == 0: + pytest.skip("root ignores file permissions") + install(make_bundle(tmp_path)) + lock, target = sg._STATE_FILE.with_name("skills.json.lock"), tmp_path / "t" + target.write_bytes(b"theirs") + lock.unlink() + lock.symlink_to(target) + saved, tree = state_bytes(), sha_tree(root()) + with pytest.raises(SkillInstallError) as exc: + install(make_bundle(tmp_path, body="v2")) + reason = os.strerror(errno.ELOOP) + assert str(exc.value) == _msg("E28", lock=lock, reason=reason) + assert target.read_bytes() == b"theirs" and lock.is_symlink() + lock.unlink() + lock.parent.chmod(0o500) + try: + with pytest.raises(SkillInstallError) as exc: + remove_tool(gen("claude")) + reason = os.strerror(errno.EACCES) + assert str(exc.value) == _msg("E28", lock=lock, reason=reason) + st = tool_status(gen("claude"), get_skills_state()) # Status needs none. + assert [p.name for p in st.kinds["ok"]] == ["api", "docs"] + assert install_conflicts([gen("claude")], make_bundle(tmp_path)) == ([], []) + finally: + lock.parent.chmod(0o700) + assert (state_bytes(), sha_tree(root())) == (saved, tree) + + @pytest.mark.parametrize( + ("code", "key"), [(errno.ENOLCK, "E28"), (errno.EAGAIN, "E27")] + ) + def test_only_a_busy_lock_is_waited_for(self, tmp_path, monkeypatch, code, key): + def fail(*a): + raise OSError(code, os.strerror(code)) + + owner = sg.msvcrt if sys.platform == "win32" else sg.fcntl + monkeypatch.setattr( + owner, "locking" if sys.platform == "win32" else "flock", fail + ) + wait = 5.0 if key == "E28" else 0.5 # E28 must return long before 5 s. + monkeypatch.setattr(sg, "_LOCK_TIMEOUT", wait) + skills = make_bundle(tmp_path) + start = time.monotonic() + with pytest.raises(SkillInstallError) as exc: + install(skills) + took = time.monotonic() - start + lock = sg._STATE_FILE.with_name("skills.json.lock") + assert str(exc.value) == _msg(key, lock=lock, reason=os.strerror(code)) + assert took < 2.0 if key == "E28" else took >= 0.5 + assert state_bytes() is None and not root().exists() + + def test_ctrl_c_while_waiting_closes_the_lock_and_writes_nothing( + self, tmp_path, monkeypatch + ): + install(make_bundle(tmp_path)) + saved, tree, opened, closed = state_bytes(), sha_tree(root()), [], [] + real_open = os.open + + def open_(path, *a, **k): + fd = real_open(path, *a, **k) + if Path(path).name == "skills.json.lock": + opened.append(fd) + return fd + + def interrupt(seconds): + raise KeyboardInterrupt + + with held_elsewhere(), monkeypatch.context() as m: + m.setattr(os, "open", open_) + wrap(m, os, "close", closed.append) + m.setattr(sg.time, "sleep", interrupt) + with pytest.raises(KeyboardInterrupt): + install(make_bundle(tmp_path, body="v2")) + assert len(opened) == 1 and opened[0] in closed + assert getattr(sg._LOCAL, "fd", None) is None + assert (state_bytes(), sha_tree(root())) == (saved, tree) + assert staging_dirs(root()) == [] + + def test_fetch_runs_without_the_lock(self, tmp_path, monkeypatch): + skills = make_bundle(tmp_path) + + def fetch(ref=None): + assert getattr(sg._LOCAL, "fd", None) is None + return skills + + monkeypatch.setattr(skill_bundle, "fetch_skill_bundle", fetch) + gen("claude").install([], "x") + assert set(records()) == {"api", "docs"} + + def test_lock_messages_end_with_the_retry_phrase(self): + for key in ("E27", "E28"): + text = _msg(key, lock=Path("x"), reason="r") + assert text.endswith(", then run the command again.") + assert "on a local disk" in _msg("E28", lock=Path("x"), reason="r") + assert sg._NO_EXCL.endswith("deepctl needs this folder on a local disk") + assert "Linux 3.15 or later" in sg._NO_EXCL_SYS + + def test_ctrl_c_right_after_the_lock_is_taken_never_keeps_it(self, monkeypatch): + win = sys.platform == "win32" + owner, name = (sg.msvcrt, "locking") if win else (sg.fcntl, "flock") + real = getattr(owner, name) + + def locked_then_interrupted(*a): + real(*a) + raise KeyboardInterrupt + + with monkeypatch.context() as m: + m.setattr(owner, name, locked_then_interrupted) + with pytest.raises(KeyboardInterrupt), sg._state_lock(): + pass + monkeypatch.setattr(sg, "_LOCK_TIMEOUT", 5.0) + start = time.monotonic() + with sg._state_lock(): # The interrupted take did not keep the file locked. + pass + assert time.monotonic() - start < (5.0 if win else 1.0) + assert getattr(sg._LOCAL, "fd", None) is None + + @POSIX + def test_lock_file_replaced_before_the_lock_is_never_entered(self, monkeypatch): + """Another run deletes and remakes the file between our open and our lock.""" + lock, real, other = ( + sg._STATE_FILE.with_name("skills.json.lock"), + sg.fcntl.flock, + [], + ) + + def flock(fd, op): + if not other: + other.append(-1) + os.unlink(lock) + other[0] = sg._try_lock(lock) # Holds the new file. + return real(fd, op) - with patch.object(gen, "get_skill_paths", return_value=[target]): - removed = gen.remove() - assert target in removed - text = target.read_text() - assert "BEGIN deepctl" not in text - assert "before" in text - assert "after" in text - - -class TestGetAllGenerators: - """Test get_all_generators.""" - - def test_returns_all_generators(self): - generators = get_all_generators() - names = {g.cli_name for g in generators} - assert "claude" in names - assert "codex" in names - assert "gemini" in names - assert "amazonq" in names - assert "aider" in names - assert "opencode" in names - assert "cursor" in names - assert "cline" in names - - def test_generators_have_display_names(self): - for gen in get_all_generators(): - assert gen.display_name, f"{gen.cli_name} missing display_name" - - -class TestDetectAiClis: - """Test detect_ai_clis.""" - - def test_returns_only_detected(self): - with patch.object(ClaudeCodeGenerator, "detect", return_value=True), \ - patch.object(CodexGenerator, "detect", return_value=False), \ - patch.object(GeminiGenerator, "detect", return_value=False), \ - patch.object(AmazonQGenerator, "detect", return_value=False), \ - patch.object(CursorGenerator, "detect", return_value=False), \ - patch.object(ClineGenerator, "detect", return_value=False): - detected = detect_ai_clis() - claude = [g for g in detected if g.cli_name == "claude"] - assert len(claude) >= 1 + monkeypatch.setattr(sg.fcntl, "flock", flock) + monkeypatch.setattr(sg, "_LOCK_TIMEOUT", 0.3) + try: + with pytest.raises(SkillInstallError) as exc, sg._state_lock(): + pass # Never runs: the file it locked is no longer the lock. + finally: + os.close(other[0]) + assert other[0] >= 0 and str(exc.value) == _msg("E27") + assert getattr(sg._LOCAL, "fd", None) is None