From c884731eb1d2f50f096e15831aaf21cc4091586e Mon Sep 17 00:00:00 2001 From: Forge Date: Sun, 8 Mar 2026 15:16:00 -0700 Subject: [PATCH 1/7] =?UTF-8?q?feat(#71):=20tool-native=20skill=20sync=20?= =?UTF-8?q?=E2=80=94=20Gemini=20dir-symlink,=20Windsurf=20injection,=20Cop?= =?UTF-8?q?ilot=20per-file,=20apc=20unsync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Gemini CLI - Add SKILL_DIR = ~/.gemini/skills/ — Gemini natively reads /SKILL.md subdirs - Dir-symlink on apc sync; new installs appear immediately ## Windsurf - No native skills dir; inject ## APC Skills section into global_rules.md - sync_skills_dir(): writes managed block ( markers) - apply_installed_skill(): regenerates block when new skill installed - unsync_skills(): removes the managed block cleanly ## GitHub Copilot - sync_skills_dir(): creates per-file symlinks in ~/.github/instructions/ .instructions.md → ~/.apc/skills//SKILL.md - apply_installed_skill(): creates one symlink for the new skill - unsync_skills(): removes all apc-managed .instructions.md symlinks ## BaseApplier - apply_installed_skill(name): default no-op for dir-symlink tools - unsync_skills(): default removes dir symlink, recreates empty dir ## manifest.py - record_tool_sync(sync_method): records non-dir-symlink syncs - sync_method property: returns sync method from stored dir_sync data ## install.py - _propagate_to_synced_tools(): after save_skill_file(), finds all synced tools not in explicit targets and calls apply_installed_skill() ## unsync.py (new) - apc unsync [TOOL...] --all --yes - Calls unsync_skills() per tool, clears dir_sync from manifest --- src/appliers/base.py | 25 +++++++++++++++ src/appliers/copilot.py | 64 +++++++++++++++++++++++++++++++++++++ src/appliers/gemini.py | 9 ++++++ src/appliers/manifest.py | 13 ++++++++ src/appliers/windsurf.py | 68 ++++++++++++++++++++++++++++++++++++++++ src/install.py | 37 ++++++++++++---------- src/main.py | 2 ++ src/unsync.py | 67 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 268 insertions(+), 17 deletions(-) create mode 100644 src/unsync.py diff --git a/src/appliers/base.py b/src/appliers/base.py index 302e2ef..e711b47 100644 --- a/src/appliers/base.py +++ b/src/appliers/base.py @@ -175,6 +175,31 @@ def sync_skills_dir(self) -> bool: os.symlink(skills_source, skill_dir) return True + def apply_installed_skill(self, name: str) -> bool: + """Propagate a newly installed skill to this tool (called by apc install). + + Dir-symlink tools: no-op — the symlink already makes the skill live. + Override in tools that need per-skill injection (Windsurf, Copilot). + Returns True if an action was taken, False if no-op. + """ + return False # dir-symlink tools need no action + + def unsync_skills(self) -> bool: + """Undo the skill sync for this tool. + + Dir-symlink tools: remove the symlink, recreate an empty dir. + Override in tools that use injection or per-file symlinks. + Returns True if anything was undone. + """ + skill_dir = self.SKILL_DIR + if skill_dir is None: + return False + if skill_dir.is_symlink(): + skill_dir.unlink() + skill_dir.mkdir(parents=True, exist_ok=True) + return True + return False + @abstractmethod def apply_mcp_servers( self, diff --git a/src/appliers/copilot.py b/src/appliers/copilot.py index d39e1f3..7b4e4c9 100644 --- a/src/appliers/copilot.py +++ b/src/appliers/copilot.py @@ -18,6 +18,11 @@ def _copilot_instructions_dir() -> Path: return Path.cwd() / ".github" / "instructions" +def _copilot_global_instructions_dir() -> Path: + """User-global instructions dir — applies across all projects via VS Code.""" + return Path.home() / ".github" / "instructions" + + def _vscode_mcp_json() -> Path: return Path.cwd() / ".vscode" / "mcp.json" @@ -96,6 +101,65 @@ def MEMORY_ALLOWED_BASE(self) -> "Path": # noqa: N802 # calling process later changes directory (#42). return Path.cwd().resolve() + def _global_instructions_dir(self) -> Path: + return _copilot_global_instructions_dir() + + def sync_skills_dir(self) -> bool: # type: ignore[override] + """Create per-skill .instructions.md symlinks in ~/.github/instructions/. + + Copilot reads each .instructions.md in the instructions dir. + We symlink: ~/.github/instructions/.instructions.md → + ~/.apc/skills//SKILL.md + so each skill's content is served as a Copilot instruction. + """ + from skills import get_skills_dir + + instr_dir = self._global_instructions_dir() + instr_dir.mkdir(parents=True, exist_ok=True) + skills_dir = get_skills_dir() + + if not skills_dir.exists(): + return True # nothing to link yet; will populate on first apc install + + for skill_path in skills_dir.iterdir(): + skill_md = skill_path / "SKILL.md" + if not skill_md.exists(): + continue + self._link_skill(skill_path.name, skill_md, instr_dir) + + return True + + def apply_installed_skill(self, name: str) -> bool: # type: ignore[override] + """Create a symlink for a newly installed skill.""" + from skills import get_skills_dir + + skill_md = get_skills_dir() / name / "SKILL.md" + if not skill_md.exists(): + return False + instr_dir = self._global_instructions_dir() + instr_dir.mkdir(parents=True, exist_ok=True) + self._link_skill(name, skill_md, instr_dir) + return True + + def unsync_skills(self) -> bool: # type: ignore[override] + """Remove all apc-managed .instructions.md symlinks from ~/.github/instructions/.""" + instr_dir = self._global_instructions_dir() + if not instr_dir.exists(): + return False + removed = 0 + for link in instr_dir.glob("*.instructions.md"): + if link.is_symlink(): + link.unlink() + removed += 1 + return removed > 0 + + @staticmethod + def _link_skill(name: str, skill_md: Path, instr_dir: Path) -> None: + link_path = instr_dir / f"{name}.instructions.md" + if link_path.is_symlink() or link_path.exists(): + link_path.unlink() + os.symlink(skill_md.resolve(), link_path) + def apply_skills(self, skills: List[Dict], manifest: ToolManifest) -> int: count = 0 instructions = _copilot_instructions() diff --git a/src/appliers/gemini.py b/src/appliers/gemini.py index 9cff29e..62875dd 100644 --- a/src/appliers/gemini.py +++ b/src/appliers/gemini.py @@ -65,6 +65,10 @@ def _gemini_dir() -> Path: return Path.home() / ".gemini" +def _gemini_skills_dir() -> Path: + return Path.home() / ".gemini" / "skills" + + def _gemini_settings() -> Path: return Path.home() / ".gemini/settings.json" @@ -75,6 +79,11 @@ def _gemini_md() -> Path: class GeminiApplier(BaseApplier): TOOL_NAME = "gemini-cli" + + @property + def SKILL_DIR(self) -> Path: # type: ignore[override] + return _gemini_skills_dir() + MEMORY_SCHEMA = GEMINI_MEMORY_SCHEMA @property # type: ignore[override] diff --git a/src/appliers/manifest.py b/src/appliers/manifest.py index 674e87d..0da7f0e 100644 --- a/src/appliers/manifest.py +++ b/src/appliers/manifest.py @@ -67,9 +67,22 @@ def record_dir_sync(self, skill_dir: str, target: str) -> None: self._data["dir_sync"] = { "skill_dir": skill_dir, "target": target, + "sync_method": "dir-symlink", "synced_at": _now_iso(), } + def record_tool_sync(self, sync_method: str) -> None: + """Record a tool-specific sync (injection or per-file symlinks).""" + self._data["dir_sync"] = { + "sync_method": sync_method, + "synced_at": _now_iso(), + } + + @property + def sync_method(self) -> str | None: + """Return the sync method recorded for this tool, or None if never synced.""" + return (self._data.get("dir_sync") or {}).get("sync_method") + @property def is_first_sync(self) -> bool: """True when no manifest existed on disk before this run.""" diff --git a/src/appliers/windsurf.py b/src/appliers/windsurf.py index f6b2154..c6d728c 100644 --- a/src/appliers/windsurf.py +++ b/src/appliers/windsurf.py @@ -97,6 +97,74 @@ class WindsurfApplier(BaseApplier): def MEMORY_ALLOWED_BASE(self) -> "Path": # noqa: N802 return _windsurf_dir() + _APC_SKILLS_HEADER = "" + _APC_SKILLS_FOOTER = "" + + def _skills_section(self) -> str: + """Build the APC-managed skills block for global_rules.md.""" + from skills import get_skills_dir + + skills_dir = get_skills_dir() + lines = [ + self._APC_SKILLS_HEADER, + "", + "## APC Skills", + "", + "The following skills are managed by apc. Each provides specialised", + "instructions — refer to the skill name when you need that capability.", + "", + ] + if skills_dir.exists(): + for skill_path in sorted(skills_dir.iterdir()): + skill_md = skill_path / "SKILL.md" + if skill_md.exists(): + lines.append(f"- **{skill_path.name}**: {skill_md}") + lines += ["", self._APC_SKILLS_FOOTER] + return "\n".join(lines) + + def _write_skills_to_global_rules(self) -> None: + """Inject (or update) the APC skills block in global_rules.md.""" + rules_path = _windsurf_global_rules() + rules_path.parent.mkdir(parents=True, exist_ok=True) + existing = rules_path.read_text(encoding="utf-8") if rules_path.exists() else "" + + block = self._skills_section() + + if self._APC_SKILLS_HEADER in existing: + # Replace existing block + start = existing.index(self._APC_SKILLS_HEADER) + end = existing.index(self._APC_SKILLS_FOOTER) + len(self._APC_SKILLS_FOOTER) + updated = existing[:start] + block + existing[end:] + else: + # Append block + updated = existing.rstrip("\n") + "\n\n" + block + "\n" + + rules_path.write_text(updated, encoding="utf-8") + + def sync_skills_dir(self) -> bool: # type: ignore[override] + """Inject the APC skills section into global_rules.md (no dir symlink).""" + self._write_skills_to_global_rules() + return True + + def apply_installed_skill(self, name: str) -> bool: # type: ignore[override] + """Regenerate the APC skills block when a new skill is installed.""" + self._write_skills_to_global_rules() + return True + + def unsync_skills(self) -> bool: # type: ignore[override] + """Remove the APC skills section from global_rules.md.""" + rules_path = _windsurf_global_rules() + if not rules_path.exists(): + return False + content = rules_path.read_text(encoding="utf-8") + if self._APC_SKILLS_HEADER not in content: + return False + start = content.index(self._APC_SKILLS_HEADER) + end = content.index(self._APC_SKILLS_FOOTER) + len(self._APC_SKILLS_FOOTER) + updated = (content[:start] + content[end:]).strip("\n") + "\n" + rules_path.write_text(updated, encoding="utf-8") + return True + def apply_skills(self, skills: List[Dict], manifest: ToolManifest) -> int: return 0 diff --git a/src/install.py b/src/install.py index 0a1bcd3..4b3daaf 100644 --- a/src/install.py +++ b/src/install.py @@ -83,27 +83,30 @@ def _resolve_targets(target_args: tuple, yes: bool) -> List[str]: return targets -def _note_per_skill_tools(target_list: list) -> None: - """Warn if any synced target uses per-skill symlinks (not dir-level). +def _propagate_to_synced_tools(skill_name: str, explicit_targets: list) -> None: + """Push a newly installed skill to all tools that have been synced. - For tools with SKILL_DIR set, the dir symlink is already live after - apc sync — new skills in ~/.apc/skills/ appear immediately. - For Copilot (no SKILL_DIR), re-run `apc sync` to pick up new skills. + - Dir-symlink tools: no-op (symlink already makes the skill live). + - Windsurf: regenerates global_rules.md skills section. + - Copilot: creates ~/.github/instructions/.instructions.md symlink. + + Only runs for tools that are already synced (have a manifest with last_sync_at). + Skips tools that were explicitly targeted by this install (already handled). """ - needs_sync = [] - for target_name in target_list: + from appliers.manifest import ToolManifest + from extractors import detect_installed_tools + + for tool_name in detect_installed_tools(): + if tool_name in explicit_targets: + continue + manifest = ToolManifest(tool_name) + if manifest.is_first_sync: + continue # not yet synced — skip try: - applier = get_applier(target_name) - if applier.SKILL_DIR is None: - needs_sync.append(target_name) + applier = get_applier(tool_name) + applier.apply_installed_skill(skill_name) except Exception: pass - if needs_sync: - click.echo( - f" ℹ {', '.join(needs_sync)}: run `apc sync` to link this skill " - "(per-skill symlinks — dir-level not supported for mixed skill dirs)", - err=True, - ) @click.command() @@ -239,7 +242,7 @@ def install(repo, skills, install_all, targets, branch, list_only, yes): save_skill_file(skill["name"], raw_content) # Apply directly to each target target - _note_per_skill_tools(target_list) + _propagate_to_synced_tools(skill["name"], target_list) # Save metadata to local cache existing = load_skills() diff --git a/src/main.py b/src/main.py index ed5b450..450c841 100644 --- a/src/main.py +++ b/src/main.py @@ -18,6 +18,7 @@ info, warning, ) +from unsync import unsync @click.group() @@ -56,6 +57,7 @@ def cli(): # Install cli.add_command(install) +cli.add_command(unsync) # MCP cli.add_command(mcp) diff --git a/src/unsync.py b/src/unsync.py new file mode 100644 index 0000000..f994d1d --- /dev/null +++ b/src/unsync.py @@ -0,0 +1,67 @@ +"""apc unsync command — remove skill sync for one or all tools.""" + +import click + +from appliers import get_applier +from appliers.manifest import ToolManifest +from extractors import detect_installed_tools +from ui import error, header, info, success, warning + + +@click.command() +@click.argument("tools", nargs=-1, metavar="[TOOL]...") +@click.option("--all", "all_tools", is_flag=True, help="Unsync all synced tools.") +@click.option("--yes", "-y", is_flag=True, help="Skip confirmation prompt.") +def unsync(tools: tuple, all_tools: bool, yes: bool) -> None: + """Remove skill sync for one or more tools. + + Undoes the effect of `apc sync` for the given TOOL(s): + + \b + - Dir-symlink tools (Claude Code, OpenClaw, Gemini CLI, Cursor): + Removes the symlink and recreates an empty directory. + - Windsurf: + Removes the APC Skills section from global_rules.md. + - GitHub Copilot: + Removes per-skill .instructions.md symlinks from ~/.github/instructions/. + + Run `apc sync` again to re-establish sync for a tool. + """ + if all_tools: + target_list = detect_installed_tools() + elif tools: + target_list = list(tools) + else: + click.echo("Specify a tool name or --all. Run `apc unsync --help` for usage.", err=True) + raise SystemExit(1) + + # Filter to only synced tools + synced = [t for t in target_list if not ToolManifest(t).is_first_sync] + if not synced: + info("No synced tools found. Nothing to do.") + return + + header("Unsync") + info(f"Tools to unsync: {', '.join(synced)}") + + if not yes: + if not click.confirm("\nProceed?"): + info("Cancelled.") + return + + for tool_name in synced: + try: + applier = get_applier(tool_name) + undone = applier.unsync_skills() + manifest = applier.get_manifest() + # Clear dir_sync record from manifest + manifest._data.pop("dir_sync", None) + manifest.save() + if undone: + success(f"{tool_name}: unsynced") + else: + info(f"{tool_name}: nothing to undo (already clean)") + except Exception as e: + error(f"{tool_name}: {e}") + + warning("\nSkill sync removed. Run `apc sync` to re-establish sync for a tool.") From 3778bb84b21b9b7919cab05eee83eda6c0b89d5a Mon Sep 17 00:00:00 2001 From: Forge Date: Sun, 8 Mar 2026 15:25:06 -0700 Subject: [PATCH 2/7] fix: remove --target/-t from apc install; propagate to ALL synced tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The --target flag was a legacy concept from when skills were copied into each tool's directory. With dir-symlink + sync registry, skills always land in ~/.apc/skills/ and propagate via sync — the target is irrelevant. Changes: - Drop --target/-t option from entirely - _propagate_to_synced_tools() no longer takes explicit_targets arg — calls apply_installed_skill() for every synced tool unconditionally - Dir-symlink tools: still a no-op (symlink already propagates) - Windsurf/Copilot: correctly get apply_installed_skill() even if they were previously the only flag passed to --target - Success message updated: 'to ~/.apc/skills/' instead of tool list - Tests: replaced -t/--target usage with plain -y; updated target-all test to test_install_saves_to_apc_skills --- src/install.py | 79 +++++++------------------------- tests/test_docker_integration.py | 29 +++++------- 2 files changed, 28 insertions(+), 80 deletions(-) diff --git a/src/install.py b/src/install.py index 4b3daaf..cb82012 100644 --- a/src/install.py +++ b/src/install.py @@ -4,7 +4,6 @@ """ import re -from typing import List import click @@ -52,56 +51,23 @@ def _validate_branch(branch: str) -> None: _AGENTS = ["claude-code", "cursor", "gemini-cli", "github-copilot", "openclaw", "windsurf"] -def _resolve_targets(target_args: tuple, yes: bool) -> List[str]: - """Resolve target targets from -a flags, '*', or interactive selection.""" - if not target_args: - detected = detect_installed_tools() - if not detected: - click.echo("No AI tools detected on this machine.", err=True) - return [] - if yes: - return detected - click.echo("\nDetected tools:") - for i, t in enumerate(detected, 1): - click.echo(f" {i}. {t}") - raw = click.prompt("Install to (e.g. 1,3 or 'all')", default="all") - if raw.strip().lower() == "all": - return detected - indices = [] - for part in raw.split(","): - part = part.strip() - if "-" in part: - a, b = part.split("-", 1) - indices.extend(range(int(a) - 1, int(b))) - elif part.isdigit(): - indices.append(int(part) - 1) - return [detected[i] for i in indices if 0 <= i < len(detected)] - - targets = list(target_args) - if "*" in targets: - return detect_installed_tools() - return targets - - -def _propagate_to_synced_tools(skill_name: str, explicit_targets: list) -> None: - """Push a newly installed skill to all tools that have been synced. - - - Dir-symlink tools: no-op (symlink already makes the skill live). - - Windsurf: regenerates global_rules.md skills section. - - Copilot: creates ~/.github/instructions/.instructions.md symlink. +def _propagate_to_synced_tools(skill_name: str) -> None: + """Push a newly installed skill to every tool that has been synced. + + Skills always land in ~/.apc/skills/ regardless of --tool flags. + This function ensures all synced tools see the new skill: - Only runs for tools that are already synced (have a manifest with last_sync_at). - Skips tools that were explicitly targeted by this install (already handled). + - Dir-symlink tools (Claude Code, OpenClaw, Gemini, Cursor): no-op — + the dir symlink already makes the new skill live immediately. + - Windsurf: regenerates the ## APC Skills block in global_rules.md. + - Copilot: creates ~/.github/instructions/.instructions.md symlink. """ from appliers.manifest import ToolManifest - from extractors import detect_installed_tools for tool_name in detect_installed_tools(): - if tool_name in explicit_targets: - continue manifest = ToolManifest(tool_name) if manifest.is_first_sync: - continue # not yet synced — skip + continue # never synced — skip try: applier = get_applier(tool_name) applier.apply_installed_skill(skill_name) @@ -115,13 +81,6 @@ def _propagate_to_synced_tools(skill_name: str, explicit_targets: list) -> None: "--skill", "-s", "skills", multiple=True, help="Skill name(s) to install. Use '*' for all." ) @click.option("--all", "install_all", is_flag=True, help="Install all skills from the repo.") -@click.option( - "--target", - "-t", - "targets", - multiple=True, - help="Target tool(s) to install to. Use '*' for all detected.", -) @click.option("--branch", default="main", show_default=True, help="Git branch to fetch from.") @click.option( "--list", @@ -130,9 +89,12 @@ def _propagate_to_synced_tools(skill_name: str, explicit_targets: list) -> None: help="List available skills in the repo without installing.", ) @click.option("-y", "--yes", is_flag=True, help="Non-interactive: skip all confirmation prompts.") -def install(repo, skills, install_all, targets, branch, list_only, yes): +def install(repo, skills, install_all, branch, list_only, yes): """Install skills from a GitHub repository. + Skills are saved to ~/.apc/skills/ and automatically propagated to every + tool that has been synced via `apc sync`. + \b Examples: apc install owner/repo --list @@ -140,8 +102,7 @@ def install(repo, skills, install_all, targets, branch, list_only, yes): apc install owner/repo --skill frontend-design --skill skill-creator apc install owner/repo --skill '*' apc install owner/repo --all - apc install owner/repo --skill frontend-design -t claude-code -t cursor - apc install owner/repo --all -t claude-code -y + apc install owner/repo --all -y """ # Validate: repo must look like owner/repo with safe characters if repo.startswith("http"): @@ -202,16 +163,10 @@ def install(repo, skills, install_all, targets, branch, list_only, yes): click.echo("No skills selected.", err=True) return - # Resolve target targets - target_list = _resolve_targets(targets, yes) - if not target_list: - return - # Confirm plan if not yes: click.echo(f"\nInstall {len(skill_names)} skill(s) from {repo}") click.echo(f" Skills: {', '.join(skill_names)}") - click.echo(f" To: {', '.join(target_list)}") if not click.confirm("\nProceed?", default=True): click.echo("Cancelled.") return @@ -242,7 +197,7 @@ def install(repo, skills, install_all, targets, branch, list_only, yes): save_skill_file(skill["name"], raw_content) # Apply directly to each target target - _propagate_to_synced_tools(skill["name"], target_list) + _propagate_to_synced_tools(skill["name"]) # Save metadata to local cache existing = load_skills() @@ -253,6 +208,6 @@ def install(repo, skills, install_all, targets, branch, list_only, yes): click.echo(" ✓") if installed_skills: - click.echo(f"\n✓ Installed {len(installed_skills)} skill(s) to {', '.join(target_list)}") + click.echo(f"\n✓ Installed {len(installed_skills)} skill(s) to ~/.apc/skills/") else: click.echo("\nNo skills were installed.") diff --git a/tests/test_docker_integration.py b/tests/test_docker_integration.py index 1a4cb98..b6b01cb 100644 --- a/tests/test_docker_integration.py +++ b/tests/test_docker_integration.py @@ -923,7 +923,7 @@ def test_install_single_skill(self, runner, cli, tmp_path, monkeypatch): monkeypatch.setenv("HOME", str(tmp_path)) result = runner.invoke( cli, - ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-t", "cursor", "-y"], + ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-y"], ) assert result.exit_code == 0, result.output assert "✓" in result.output @@ -943,8 +943,6 @@ def test_install_multiple_skills(self, runner, cli, tmp_path, monkeypatch): "pdf", "--skill", "skill-creator", - "-t", - "cursor", "-y", ], ) @@ -963,8 +961,6 @@ def test_install_nonexistent_skill(self, runner, cli, tmp_path, monkeypatch): self.TEST_REPO, "--skill", "totally-nonexistent-xyz", - "-t", - "cursor", "-y", ], ) @@ -977,7 +973,7 @@ def test_install_nonexistent_skill(self, runner, cli, tmp_path, monkeypatch): def test_install_all(self, runner, cli, tmp_path, monkeypatch): """--all installs every skill from the repo.""" monkeypatch.setenv("HOME", str(tmp_path)) - result = runner.invoke(cli, ["install", self.TEST_REPO, "--all", "-t", "cursor", "-y"]) + result = runner.invoke(cli, ["install", self.TEST_REPO, "--all", "-y"]) assert result.exit_code == 0, result.output assert "✓" in result.output skills_dir = tmp_path / ".apc" / "skills" @@ -989,7 +985,7 @@ def test_install_yes_skips_confirmation(self, runner, cli, tmp_path, monkeypatch monkeypatch.setenv("HOME", str(tmp_path)) result = runner.invoke( cli, - ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-t", "cursor", "-y"], + ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-y"], ) assert result.exit_code == 0 assert "Proceed?" not in result.output @@ -997,18 +993,17 @@ def test_install_yes_skips_confirmation(self, runner, cli, tmp_path, monkeypatch assert skill_md.exists(), "SKILL.md not written even with -y" assert len(skill_md.read_text()) > 0 - def test_install_target_all_agents(self, runner, cli, tmp_path, monkeypatch): - """--target '*' installs to all detected tools.""" + def test_install_saves_to_apc_skills(self, runner, cli, tmp_path, monkeypatch): + """apc install saves skills to ~/.apc/skills/ (no --target needed).""" monkeypatch.setenv("HOME", str(tmp_path)) - (tmp_path / ".cursor").mkdir() result = runner.invoke( cli, - ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "--target", "*", "-y"], + ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-y"], ) assert result.exit_code == 0, result.output assert "✓" in result.output skill_md = tmp_path / ".apc" / "skills" / self.KNOWN_SKILL / "SKILL.md" - assert skill_md.exists(), "SKILL.md not written when targeting all agents" + assert skill_md.exists(), "SKILL.md not written to ~/.apc/skills/" # --------------------------------------------------------------------------- @@ -1033,7 +1028,7 @@ def test_install_then_sync_symlinks_skill_to_tool(self, runner, cli, tmp_path, m (tmp_path / ".cursor" / "mcp.json").write_text("{}") r1 = runner.invoke( - cli, ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-t", "cursor", "-y"] + cli, ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-y"] ) assert r1.exit_code == 0, r1.output @@ -1052,7 +1047,7 @@ def test_installed_skill_appears_in_skill_list(self, runner, cli, tmp_path, monk monkeypatch.setenv("HOME", str(tmp_path)) runner.invoke( - cli, ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-t", "cursor", "-y"] + cli, ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-y"] ) result = runner.invoke(cli, ["skill", "list"]) @@ -1078,8 +1073,6 @@ def test_install_multiple_then_sync_all_land_in_tool(self, runner, cli, tmp_path skills[0], "--skill", skills[1], - "-t", - "cursor", "-y", ], ) @@ -1103,7 +1096,7 @@ def test_install_all_then_sync_dry_run(self, runner, cli, tmp_path, monkeypatch) (tmp_path / ".cursor").mkdir() (tmp_path / ".cursor" / "mcp.json").write_text("{}") - r_install = runner.invoke(cli, ["install", self.TEST_REPO, "--all", "-t", "cursor", "-y"]) + r_install = runner.invoke(cli, ["install", self.TEST_REPO, "--all", "-y"]) assert r_install.exit_code == 0, r_install.output skills_dir = tmp_path / ".apc" / "skills" @@ -1124,7 +1117,7 @@ def test_status_synced_after_install_and_sync(self, runner, cli, tmp_path, monke (tmp_path / ".cursor" / "mcp.json").write_text("{}") runner.invoke( - cli, ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-t", "cursor", "-y"] + cli, ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-y"] ) runner.invoke(cli, ["sync", "--tools", "cursor", "--yes"]) From 2e28d49d27f7588b89f47ee4d565a215618f689a Mon Sep 17 00:00:00 2001 From: Forge Date: Sun, 8 Mar 2026 15:26:09 -0700 Subject: [PATCH 3/7] fix: ruff format test_docker_integration.py (lint CI) --- tests/test_docker_integration.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/test_docker_integration.py b/tests/test_docker_integration.py index b6b01cb..7f54cb6 100644 --- a/tests/test_docker_integration.py +++ b/tests/test_docker_integration.py @@ -1027,9 +1027,7 @@ def test_install_then_sync_symlinks_skill_to_tool(self, runner, cli, tmp_path, m (tmp_path / ".cursor").mkdir() (tmp_path / ".cursor" / "mcp.json").write_text("{}") - r1 = runner.invoke( - cli, ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-y"] - ) + r1 = runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-y"]) assert r1.exit_code == 0, r1.output r2 = runner.invoke(cli, ["sync", "--tools", "cursor", "--yes"]) @@ -1046,9 +1044,7 @@ def test_installed_skill_appears_in_skill_list(self, runner, cli, tmp_path, monk """Installed skill appears in apc skill list immediately after install.""" monkeypatch.setenv("HOME", str(tmp_path)) - runner.invoke( - cli, ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-y"] - ) + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-y"]) result = runner.invoke(cli, ["skill", "list"]) assert result.exit_code == 0 @@ -1116,9 +1112,7 @@ def test_status_synced_after_install_and_sync(self, runner, cli, tmp_path, monke (tmp_path / ".cursor").mkdir() (tmp_path / ".cursor" / "mcp.json").write_text("{}") - runner.invoke( - cli, ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-y"] - ) + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.KNOWN_SKILL, "-y"]) runner.invoke(cli, ["sync", "--tools", "cursor", "--yes"]) r_status = runner.invoke(cli, ["status"]) From 80557f74e71e584b28f31c6428fa278f35a720a6 Mon Sep 17 00:00:00 2001 From: Forge Date: Sun, 8 Mar 2026 15:29:10 -0700 Subject: [PATCH 4/7] test: 42 unit tests for tool-native sync (Gemini, Windsurf, Copilot, apc unsync) --- tests/test_tool_native_sync.py | 758 +++++++++++++++++++++++++++++++++ 1 file changed, 758 insertions(+) create mode 100644 tests/test_tool_native_sync.py diff --git a/tests/test_tool_native_sync.py b/tests/test_tool_native_sync.py new file mode 100644 index 0000000..3a85583 --- /dev/null +++ b/tests/test_tool_native_sync.py @@ -0,0 +1,758 @@ +"""Unit tests for tool-native skill sync (issue #71). + +Covers: +- GeminiApplier.SKILL_DIR → ~/.gemini/skills/ +- WindsurfApplier: sync_skills_dir(), apply_installed_skill(), unsync_skills() +- CopilotApplier: sync_skills_dir(), apply_installed_skill(), unsync_skills() +- BaseApplier: apply_installed_skill() no-op, unsync_skills() dir-symlink removal +- manifest.py: record_tool_sync(), sync_method property +- install._propagate_to_synced_tools(): hits all synced tools unconditionally +- apc unsync command: single tool, --all, --yes +""" + +import os +import sys +import unittest +from pathlib import Path +from unittest.mock import MagicMock, call, patch + +import pytest + +# Ensure src/ is importable +sys.path.insert(0, str(Path(__file__).parent.parent / "src")) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_skills_dir(tmp_path: Path, *skill_names: str) -> Path: + """Create ~/.apc/skills//SKILL.md for each skill_name.""" + skills_dir = tmp_path / ".apc" / "skills" + for name in skill_names: + skill_dir = skills_dir / name + skill_dir.mkdir(parents=True, exist_ok=True) + (skill_dir / "SKILL.md").write_text(f"# {name}\nSkill content.", encoding="utf-8") + return skills_dir + + +# --------------------------------------------------------------------------- +# Gemini: SKILL_DIR +# --------------------------------------------------------------------------- + + +class TestGeminiSkillDir(unittest.TestCase): + def test_skill_dir_is_gemini_skills(self, tmp_path=None): + from appliers.gemini import GeminiApplier + + applier = GeminiApplier() + assert applier.SKILL_DIR == Path.home() / ".gemini" / "skills" + + def test_skill_dir_monkeypatched_home(self): + """SKILL_DIR resolves relative to HOME at call time.""" + import importlib + + import appliers.gemini as gemini_mod + + with patch.dict(os.environ, {"HOME": "/fakehome"}): + importlib.reload(gemini_mod) + applier = gemini_mod.GeminiApplier() + assert applier.SKILL_DIR == Path("/fakehome/.gemini/skills") + + importlib.reload(gemini_mod) # restore + + +# --------------------------------------------------------------------------- +# BaseApplier: apply_installed_skill & unsync_skills +# --------------------------------------------------------------------------- + + +class TestBaseApplierDefaults(unittest.TestCase): + """Test default implementations on a concrete dir-symlink applier (Claude).""" + + def test_apply_installed_skill_returns_false(self): + """Dir-symlink tools return False — symlink already propagates.""" + from appliers.claude import ClaudeApplier + + applier = ClaudeApplier() + result = applier.apply_installed_skill("some-skill") + assert result is False + + def test_unsync_skills_removes_symlink_and_recreates_dir(self, tmp_path=None): + import tempfile + + from appliers.claude import ClaudeApplier + + with tempfile.TemporaryDirectory() as td: + td = Path(td) + skill_dir = td / ".claude" / "skills" + apc_skills = td / ".apc" / "skills" + apc_skills.mkdir(parents=True) + + # Create the symlink as sync would + skill_dir.parent.mkdir(parents=True) + os.symlink(apc_skills, skill_dir) + assert skill_dir.is_symlink() + + with patch( + "appliers.claude.ClaudeApplier.SKILL_DIR", + new_callable=lambda: property(lambda self: skill_dir), + ): + applier = ClaudeApplier() + result = applier.unsync_skills() + + assert result is True + assert not skill_dir.is_symlink() + assert skill_dir.is_dir() + + def test_unsync_skills_returns_false_when_no_symlink(self, tmp_path=None): + import tempfile + + from appliers.claude import ClaudeApplier + + with tempfile.TemporaryDirectory() as td: + td = Path(td) + skill_dir = td / ".claude" / "skills" + skill_dir.mkdir(parents=True) # real dir, not a symlink + + with patch( + "appliers.claude.ClaudeApplier.SKILL_DIR", + new_callable=lambda: property(lambda self: skill_dir), + ): + applier = ClaudeApplier() + result = applier.unsync_skills() + + assert result is False + + def test_unsync_skills_returns_false_when_skill_dir_is_none(self): + from appliers.windsurf import WindsurfApplier + + applier = WindsurfApplier() + # Windsurf overrides unsync_skills, but test None guard via copilot-like stub + from appliers.base import BaseApplier + + class NullSkillDirApplier(BaseApplier): + TOOL_NAME = "null-tool" + SKILL_DIR = None + MEMORY_SCHEMA = "" + + @property + def MEMORY_ALLOWED_BASE(self): + return Path("/tmp") + + def apply_skills(self, skills, manifest): + return 0 + + def apply_mcp_servers(self, servers, secrets, manifest, override=False): + return 0 + + applier = NullSkillDirApplier() + assert applier.unsync_skills() is False + + +# --------------------------------------------------------------------------- +# manifest: record_tool_sync & sync_method +# --------------------------------------------------------------------------- + + +class TestManifestToolSync(unittest.TestCase): + def setUp(self): + import tempfile + + self._tmp = tempfile.TemporaryDirectory() + self._home = Path(self._tmp.name) + self._patcher = patch("appliers.manifest.Path.home", return_value=self._home) + self._patcher.start() + + def tearDown(self): + self._patcher.stop() + self._tmp.cleanup() + + def test_sync_method_is_none_before_record(self): + from appliers.manifest import ToolManifest + + m = ToolManifest("test-tool") + assert m.sync_method is None + + def test_record_dir_symlink_sets_sync_method(self): + from appliers.manifest import ToolManifest + + m = ToolManifest("test-tool") + m.record_dir_sync("/fake/skill_dir", "/fake/target") + assert m.sync_method == "dir-symlink" + + def test_record_tool_sync_injection(self): + from appliers.manifest import ToolManifest + + m = ToolManifest("windsurf") + m.record_tool_sync("injection") + assert m.sync_method == "injection" + + def test_record_tool_sync_per_file_symlink(self): + from appliers.manifest import ToolManifest + + m = ToolManifest("github-copilot") + m.record_tool_sync("per-file-symlink") + assert m.sync_method == "per-file-symlink" + + def test_sync_method_persists_after_save_reload(self): + from appliers.manifest import ToolManifest + + m = ToolManifest("github-copilot") + m.record_tool_sync("per-file-symlink") + m.save() + + m2 = ToolManifest("github-copilot") + assert m2.sync_method == "per-file-symlink" + + +# --------------------------------------------------------------------------- +# WindsurfApplier: injection into global_rules.md +# --------------------------------------------------------------------------- + + +class TestWindsurfSyncSkillsDir(unittest.TestCase): + def setUp(self): + import tempfile + + self._tmp = tempfile.TemporaryDirectory() + self.home = Path(self._tmp.name) + + def tearDown(self): + self._tmp.cleanup() + + def _make_applier(self): + from appliers.windsurf import WindsurfApplier + + return WindsurfApplier() + + def _rules_path(self): + return self.home / ".codeium" / "windsurf" / "memories" / "global_rules.md" + + def test_sync_creates_global_rules_with_apc_block(self): + _make_skills_dir(self.home, "pdf", "frontend-design") + + with ( + patch("appliers.windsurf._windsurf_global_rules", return_value=self._rules_path()), + patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"), + ): + applier = self._make_applier() + result = applier.sync_skills_dir() + + assert result is True + content = self._rules_path().read_text() + assert "" in content + assert "" in content + assert "## APC Skills" in content + assert "pdf" in content + assert "frontend-design" in content + + def test_sync_appends_to_existing_rules(self): + self._rules_path().parent.mkdir(parents=True, exist_ok=True) + self._rules_path().write_text("# My Global Rules\n\nBe concise.", encoding="utf-8") + _make_skills_dir(self.home, "pdf") + + with ( + patch("appliers.windsurf._windsurf_global_rules", return_value=self._rules_path()), + patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"), + ): + applier = self._make_applier() + applier.sync_skills_dir() + + content = self._rules_path().read_text() + assert "# My Global Rules" in content + assert "Be concise." in content + assert "" in content + + def test_sync_replaces_existing_apc_block(self): + _make_skills_dir(self.home, "pdf") + self._rules_path().parent.mkdir(parents=True, exist_ok=True) + self._rules_path().write_text( + "# Rules\n\n\n## APC Skills\n- **old-skill**\n\n", + encoding="utf-8", + ) + + with ( + patch("appliers.windsurf._windsurf_global_rules", return_value=self._rules_path()), + patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"), + ): + applier = self._make_applier() + applier.sync_skills_dir() + + content = self._rules_path().read_text() + assert "old-skill" not in content + assert "pdf" in content + # Only one start marker + assert content.count("") == 1 + + def test_apply_installed_skill_regenerates_block(self): + _make_skills_dir(self.home, "pdf") + + with ( + patch("appliers.windsurf._windsurf_global_rules", return_value=self._rules_path()), + patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"), + ): + applier = self._make_applier() + applier.sync_skills_dir() # initial sync + # Add a second skill after initial sync + _make_skills_dir(self.home, "frontend-design") + result = applier.apply_installed_skill("frontend-design") + + assert result is True + content = self._rules_path().read_text() + assert "pdf" in content + assert "frontend-design" in content + + def test_unsync_removes_apc_block(self): + _make_skills_dir(self.home, "pdf") + + with ( + patch("appliers.windsurf._windsurf_global_rules", return_value=self._rules_path()), + patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"), + ): + applier = self._make_applier() + applier.sync_skills_dir() + result = applier.unsync_skills() + + assert result is True + content = self._rules_path().read_text() + assert "" not in content + assert "" not in content + assert "## APC Skills" not in content + + def test_unsync_preserves_surrounding_content(self): + self._rules_path().parent.mkdir(parents=True, exist_ok=True) + self._rules_path().write_text("# Rules\n\nBe concise.", encoding="utf-8") + _make_skills_dir(self.home, "pdf") + + with ( + patch("appliers.windsurf._windsurf_global_rules", return_value=self._rules_path()), + patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"), + ): + applier = self._make_applier() + applier.sync_skills_dir() + applier.unsync_skills() + + content = self._rules_path().read_text() + assert "# Rules" in content + assert "Be concise." in content + + def test_unsync_returns_false_when_no_block(self): + self._rules_path().parent.mkdir(parents=True, exist_ok=True) + self._rules_path().write_text("# Rules\n\nBe concise.", encoding="utf-8") + + with patch("appliers.windsurf._windsurf_global_rules", return_value=self._rules_path()): + applier = self._make_applier() + result = applier.unsync_skills() + + assert result is False + + def test_unsync_returns_false_when_no_file(self): + with patch("appliers.windsurf._windsurf_global_rules", return_value=self._rules_path()): + applier = self._make_applier() + result = applier.unsync_skills() + + assert result is False + + def test_sync_empty_skills_dir(self): + """Sync with no skills still writes a valid (empty) block.""" + skills_dir = self.home / ".apc" / "skills" + skills_dir.mkdir(parents=True) + + with ( + patch("appliers.windsurf._windsurf_global_rules", return_value=self._rules_path()), + patch("skills.get_skills_dir", return_value=skills_dir), + ): + applier = self._make_applier() + result = applier.sync_skills_dir() + + assert result is True + content = self._rules_path().read_text() + assert "" in content + + +# --------------------------------------------------------------------------- +# CopilotApplier: per-file symlinks +# --------------------------------------------------------------------------- + + +class TestCopilotSyncSkillsDir(unittest.TestCase): + def setUp(self): + import tempfile + + self._tmp = tempfile.TemporaryDirectory() + self.home = Path(self._tmp.name) + + def tearDown(self): + self._tmp.cleanup() + + def _make_applier(self): + from appliers.copilot import CopilotApplier + + applier = CopilotApplier() + # Override the global instructions dir to use tmp home + applier._global_instructions_dir = lambda: self.home / ".github" / "instructions" + return applier + + def test_sync_creates_per_skill_symlinks(self): + _make_skills_dir(self.home, "pdf", "frontend-design") + instr_dir = self.home / ".github" / "instructions" + + with patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"): + applier = self._make_applier() + result = applier.sync_skills_dir() + + assert result is True + assert (instr_dir / "pdf.instructions.md").is_symlink() + assert (instr_dir / "frontend-design.instructions.md").is_symlink() + + def test_sync_symlink_points_to_skill_md(self): + _make_skills_dir(self.home, "pdf") + instr_dir = self.home / ".github" / "instructions" + + with patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"): + applier = self._make_applier() + applier.sync_skills_dir() + + link = instr_dir / "pdf.instructions.md" + target = Path(os.readlink(link)) + assert target == (self.home / ".apc" / "skills" / "pdf" / "SKILL.md").resolve() + + def test_sync_empty_skills_dir(self): + """Sync with no skills creates an empty instructions dir, returns True.""" + skills_dir = self.home / ".apc" / "skills" + skills_dir.mkdir(parents=True) + instr_dir = self.home / ".github" / "instructions" + + with patch("skills.get_skills_dir", return_value=skills_dir): + applier = self._make_applier() + result = applier.sync_skills_dir() + + assert result is True + assert instr_dir.is_dir() + + def test_sync_missing_skill_md_skipped(self): + """A skill dir without SKILL.md is silently skipped.""" + skills_dir = self.home / ".apc" / "skills" + (skills_dir / "incomplete-skill").mkdir(parents=True) + # No SKILL.md inside + + instr_dir = self.home / ".github" / "instructions" + with patch("skills.get_skills_dir", return_value=skills_dir): + applier = self._make_applier() + applier.sync_skills_dir() + + assert not list(instr_dir.glob("*.instructions.md")) + + def test_apply_installed_skill_creates_one_symlink(self): + _make_skills_dir(self.home, "pdf") + + with patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"): + applier = self._make_applier() + result = applier.apply_installed_skill("pdf") + + instr_dir = self.home / ".github" / "instructions" + assert result is True + assert (instr_dir / "pdf.instructions.md").is_symlink() + + def test_apply_installed_skill_returns_false_when_skill_missing(self): + skills_dir = self.home / ".apc" / "skills" + skills_dir.mkdir(parents=True) + + with patch("skills.get_skills_dir", return_value=skills_dir): + applier = self._make_applier() + result = applier.apply_installed_skill("nonexistent") + + assert result is False + + def test_apply_installed_skill_overwrites_existing_link(self): + """Re-applying an existing skill replaces the symlink cleanly.""" + _make_skills_dir(self.home, "pdf") + instr_dir = self.home / ".github" / "instructions" + instr_dir.mkdir(parents=True) + stale = instr_dir / "pdf.instructions.md" + stale.write_text("stale", encoding="utf-8") + + with patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"): + applier = self._make_applier() + applier.apply_installed_skill("pdf") + + assert stale.is_symlink() + + def test_unsync_removes_all_instruction_symlinks(self): + _make_skills_dir(self.home, "pdf", "frontend-design") + + with patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"): + applier = self._make_applier() + applier.sync_skills_dir() + result = applier.unsync_skills() + + instr_dir = self.home / ".github" / "instructions" + assert result is True + assert list(instr_dir.glob("*.instructions.md")) == [] + + def test_unsync_only_removes_symlinks_not_real_files(self): + """unsync_skills only removes symlinks; real .instructions.md files are untouched.""" + instr_dir = self.home / ".github" / "instructions" + instr_dir.mkdir(parents=True) + real_file = instr_dir / "manual.instructions.md" + real_file.write_text("# Manual instructions", encoding="utf-8") + + _make_skills_dir(self.home, "pdf") + with patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"): + applier = self._make_applier() + applier.sync_skills_dir() + applier.unsync_skills() + + assert real_file.exists() + assert not real_file.is_symlink() + + def test_unsync_returns_false_when_nothing_to_remove(self): + instr_dir = self.home / ".github" / "instructions" + instr_dir.mkdir(parents=True) + + applier = self._make_applier() + result = applier.unsync_skills() + + assert result is False + + def test_unsync_returns_false_when_dir_missing(self): + applier = self._make_applier() + result = applier.unsync_skills() + + assert result is False + + +# --------------------------------------------------------------------------- +# install._propagate_to_synced_tools: hits ALL synced tools +# --------------------------------------------------------------------------- + + +class TestPropagateToSyncedTools(unittest.TestCase): + def test_calls_apply_installed_skill_for_all_synced(self): + """_propagate_to_synced_tools calls apply_installed_skill for each synced tool.""" + from install import _propagate_to_synced_tools + + mock_windsurf = MagicMock() + mock_copilot = MagicMock() + mock_cursor = MagicMock() + + not_synced_manifest = MagicMock() + not_synced_manifest.is_first_sync = True + + synced_manifest = MagicMock() + synced_manifest.is_first_sync = False + + def fake_manifest(name): + return not_synced_manifest if name == "claude-code" else synced_manifest + + def fake_get_applier(name): + return { + "windsurf": mock_windsurf, + "github-copilot": mock_copilot, + "cursor": mock_cursor, + }[name] + + # ToolManifest is lazily imported inside _propagate_to_synced_tools — + # patch at the source module, not on install. + with ( + patch( + "install.detect_installed_tools", + return_value=["windsurf", "github-copilot", "cursor", "claude-code"], + ), + patch("appliers.manifest.ToolManifest", side_effect=fake_manifest), + patch("install.get_applier", side_effect=fake_get_applier), + ): + _propagate_to_synced_tools("new-skill") + + mock_windsurf.apply_installed_skill.assert_called_once_with("new-skill") + mock_copilot.apply_installed_skill.assert_called_once_with("new-skill") + mock_cursor.apply_installed_skill.assert_called_once_with("new-skill") + + def test_skips_unsynced_tools(self): + """Tools with is_first_sync=True are skipped entirely.""" + from install import _propagate_to_synced_tools + + not_synced = MagicMock() + not_synced.is_first_sync = True + + with ( + patch("install.detect_installed_tools", return_value=["cursor"]), + patch("appliers.manifest.ToolManifest", return_value=not_synced), + patch("install.get_applier") as mock_get, + ): + _propagate_to_synced_tools("some-skill") + + mock_get.assert_not_called() + + def test_exception_in_applier_does_not_abort(self): + """An error in one applier does not prevent others from running.""" + from install import _propagate_to_synced_tools + + mock_ok = MagicMock() + mock_bad = MagicMock() + mock_bad.apply_installed_skill.side_effect = RuntimeError("boom") + + synced = MagicMock() + synced.is_first_sync = False + + def fake_get_applier(name): + return mock_bad if name == "windsurf" else mock_ok + + with ( + patch("install.detect_installed_tools", return_value=["windsurf", "cursor"]), + patch("appliers.manifest.ToolManifest", return_value=synced), + patch("install.get_applier", side_effect=fake_get_applier), + ): + # Should not raise + _propagate_to_synced_tools("some-skill") + + mock_ok.apply_installed_skill.assert_called_once_with("some-skill") + + +# --------------------------------------------------------------------------- +# apc unsync command (CLI) +# --------------------------------------------------------------------------- + + +class TestUnsyncCommand(unittest.TestCase): + def setUp(self): + import tempfile + + self._tmp = tempfile.TemporaryDirectory() + self.home = Path(self._tmp.name) + + def tearDown(self): + self._tmp.cleanup() + + def _runner(self): + from click.testing import CliRunner + + return CliRunner() + + def _cli(self): + from unsync import unsync + + return unsync + + def test_unsync_single_tool(self): + mock_applier = MagicMock() + mock_applier.unsync_skills.return_value = True + mock_applier.get_manifest.return_value = MagicMock(_data={}, save=MagicMock()) + + synced_manifest = MagicMock() + synced_manifest.is_first_sync = False + + with ( + patch("unsync.detect_installed_tools", return_value=["cursor", "windsurf"]), + patch("unsync.ToolManifest", return_value=synced_manifest), + patch("unsync.get_applier", return_value=mock_applier), + ): + result = self._runner().invoke(self._cli(), ["cursor", "--yes"]) + + assert result.exit_code == 0, result.output + assert "unsynced" in result.output or "nothing to undo" in result.output + + def test_unsync_all(self): + mock_applier = MagicMock() + mock_applier.unsync_skills.return_value = True + mock_applier.get_manifest.return_value = MagicMock(_data={}, save=MagicMock()) + + synced = MagicMock() + synced.is_first_sync = False + + with ( + patch("unsync.detect_installed_tools", return_value=["cursor", "windsurf"]), + patch("unsync.ToolManifest", return_value=synced), + patch("unsync.get_applier", return_value=mock_applier), + ): + result = self._runner().invoke(self._cli(), ["--all", "--yes"]) + + assert result.exit_code == 0, result.output + + def test_unsync_skips_unsynced_tools(self): + not_synced = MagicMock() + not_synced.is_first_sync = True + + with ( + patch("unsync.detect_installed_tools", return_value=["cursor"]), + patch("unsync.ToolManifest", return_value=not_synced), + patch("unsync.get_applier") as mock_get, + ): + result = self._runner().invoke(self._cli(), ["cursor", "--yes"]) + + assert result.exit_code == 0 + mock_get.assert_not_called() + + def test_unsync_no_args_exits_nonzero(self): + result = self._runner().invoke(self._cli(), []) + assert result.exit_code != 0 + + def test_unsync_nothing_to_undo_message(self): + mock_applier = MagicMock() + mock_applier.unsync_skills.return_value = False # nothing undone + mock_applier.get_manifest.return_value = MagicMock(_data={}, save=MagicMock()) + + synced = MagicMock() + synced.is_first_sync = False + + with ( + patch("unsync.detect_installed_tools", return_value=["cursor"]), + patch("unsync.ToolManifest", return_value=synced), + patch("unsync.get_applier", return_value=mock_applier), + ): + result = self._runner().invoke(self._cli(), ["cursor", "--yes"]) + + assert result.exit_code == 0 + assert "nothing to undo" in result.output + + def test_unsync_clears_dir_sync_from_manifest(self): + mock_applier = MagicMock() + mock_applier.unsync_skills.return_value = True + manifest_data = {"dir_sync": {"sync_method": "dir-symlink", "synced_at": "2026-03-08"}} + mock_manifest = MagicMock(_data=manifest_data, save=MagicMock()) + mock_applier.get_manifest.return_value = mock_manifest + + synced = MagicMock() + synced.is_first_sync = False + + with ( + patch("unsync.detect_installed_tools", return_value=["cursor"]), + patch("unsync.ToolManifest", return_value=synced), + patch("unsync.get_applier", return_value=mock_applier), + ): + self._runner().invoke(self._cli(), ["cursor", "--yes"]) + + assert "dir_sync" not in manifest_data + mock_manifest.save.assert_called_once() + + +# --------------------------------------------------------------------------- +# apc install: --target flag removed +# --------------------------------------------------------------------------- + + +class TestInstallNoTargetFlag(unittest.TestCase): + def _runner(self): + from click.testing import CliRunner + + return CliRunner() + + def _cli(self): + from install import install + + return install + + def test_target_flag_rejected(self): + result = self._runner().invoke(self._cli(), ["owner/repo", "--target", "cursor"]) + assert result.exit_code != 0 + assert "no such option" in result.output.lower() or "Error" in result.output + + def test_short_t_flag_rejected(self): + result = self._runner().invoke(self._cli(), ["owner/repo", "-t", "cursor"]) + assert result.exit_code != 0 + + +if __name__ == "__main__": + unittest.main() From 72c58a10cf4753406ef39e27aa102f1018b08637 Mon Sep 17 00:00:00 2001 From: Forge Date: Sun, 8 Mar 2026 15:32:18 -0700 Subject: [PATCH 5/7] test: 15 docker integration tests for Windsurf injection + Copilot per-file sync Also fixes sync_skills() to correctly record sync method for non-dir tools: - record_dir_sync() only called when SKILL_DIR is set - record_tool_sync(SYNC_METHOD) called for Windsurf (injection) and Copilot (per-file-symlink) - SYNC_METHOD class attr added to BaseApplier (default: dir-symlink) - WindsurfApplier.SYNC_METHOD = 'injection' - CopilotApplier.SYNC_METHOD = 'per-file-symlink' Windsurf tests (8): - sync creates injection block in global_rules.md - global_rules.md is a real file, not a symlink - existing rules preserved when block appended - apc install propagates to synced Windsurf (regenerates block) - resync replaces stale block without duplicates - unsync removes APC block cleanly - unsync preserves surrounding rule content - status reflects windsurf as synced Copilot tests (7): - sync creates .instructions.md symlinks in ~/.github/instructions/ - symlinks resolve to correct ~/.apc/skills//SKILL.md target - apc install propagates to synced Copilot (creates new symlink) - instructions dir is a real dir, not a dir-level symlink - unsync removes all .instructions.md symlinks - unsync preserves manually created (non-symlink) .instructions.md files - resync after new installs refreshes all symlinks --- src/appliers/base.py | 1 + src/appliers/copilot.py | 1 + src/appliers/windsurf.py | 1 + src/sync_helpers.py | 22 ++- tests/test_docker_integration.py | 273 +++++++++++++++++++++++++++++++ 5 files changed, 290 insertions(+), 8 deletions(-) diff --git a/src/appliers/base.py b/src/appliers/base.py index e711b47..aca70bf 100644 --- a/src/appliers/base.py +++ b/src/appliers/base.py @@ -64,6 +64,7 @@ class BaseApplier(ABC): # Subclasses that support skills should set this to their skill directory # and the target name used in frontmatter filtering. SKILL_DIR: Optional[Path] = None + SYNC_METHOD: str = "dir-symlink" # override in injection/per-file tools TOOL_NAME: str = "" # Subclasses that support LLM-based memory sync should override this diff --git a/src/appliers/copilot.py b/src/appliers/copilot.py index 7b4e4c9..4687061 100644 --- a/src/appliers/copilot.py +++ b/src/appliers/copilot.py @@ -92,6 +92,7 @@ def _vscode_mcp_json() -> Path: class CopilotApplier(BaseApplier): TOOL_NAME = "github-copilot" + SYNC_METHOD = "per-file-symlink" MEMORY_SCHEMA = COPILOT_MEMORY_SCHEMA @property # type: ignore[override] diff --git a/src/appliers/windsurf.py b/src/appliers/windsurf.py index c6d728c..c61fd46 100644 --- a/src/appliers/windsurf.py +++ b/src/appliers/windsurf.py @@ -91,6 +91,7 @@ def _windsurf_global_rules() -> Path: class WindsurfApplier(BaseApplier): TOOL_NAME = "windsurf" + SYNC_METHOD = "injection" MEMORY_SCHEMA = WINDSURF_MEMORY_SCHEMA @property # type: ignore[override] diff --git a/src/sync_helpers.py b/src/sync_helpers.py index 8a79ab8..3435154 100644 --- a/src/sync_helpers.py +++ b/src/sync_helpers.py @@ -71,12 +71,12 @@ def sync_skills(tool_list: List[str]) -> Tuple[int, int]: """Establish skill links for all tools. Returns (dir_linked_count, skill_linked_count). ~/.apc/skills/ is the single source of truth for all skills (installed and collected). - Two strategies depending on whether the tool's skills dir is exclusively apc-managed: + Three strategies depending on the tool: - - SKILL_DIR_EXCLUSIVE=True (OpenClaw, Claude Code): replace the entire skills dir - with a single symlink → ~/.apc/skills/. Future installs are live immediately. - - SKILL_DIR_EXCLUSIVE=False (Cursor, Copilot): create per-skill symlinks inside - the tool's mixed dir. Re-run sync after new installs to pick them up. + - Dir-symlink (OpenClaw, Claude Code, Gemini, Cursor): replace the tool's skills + dir with a single symlink → ~/.apc/skills/. Future installs are live immediately. + - Injection (Windsurf): maintain an APC Skills block in global_rules.md. + - Per-file symlinks (Copilot): create .instructions.md → SKILL.md symlinks. """ skills_dir = get_skills_dir() @@ -88,12 +88,18 @@ def sync_skills(tool_list: List[str]) -> Tuple[int, int]: manifest = applier.get_manifest() if applier.sync_skills_dir(): - manifest.record_dir_sync(str(applier.SKILL_DIR), str(skills_dir)) + if applier.SKILL_DIR is not None: + # Dir-symlink tools: record the symlink target + manifest.record_dir_sync(str(applier.SKILL_DIR), str(skills_dir)) + success(f"{tool_name}: skills dir symlinked → ~/.apc/skills/") + else: + # Tool-specific sync (injection or per-file symlinks) + manifest.record_tool_sync(applier.SYNC_METHOD) + success(f"{tool_name}: skills synced ({applier.SYNC_METHOD})") manifest.save() total_dir += 1 - success(f"{tool_name}: skills dir symlinked → ~/.apc/skills/") else: - success(f"{tool_name}: no skills dir (SKILL_DIR=None) — skipping") + success(f"{tool_name}: no skills dir configured — skipping") except Exception as e: error(f"Failed to sync skills to {tool_name}: {e}") diff --git a/tests/test_docker_integration.py b/tests/test_docker_integration.py index 7f54cb6..f73bfa6 100644 --- a/tests/test_docker_integration.py +++ b/tests/test_docker_integration.py @@ -1449,3 +1449,276 @@ def test_status_after_round_trip(self, runner, cli, export_path): r = runner.invoke(cli, ["status"]) assert r.exit_code == 0 + + +# --------------------------------------------------------------------------- +# Phase 13: Windsurf injection sync (non-symlink) +# --------------------------------------------------------------------------- + + +class TestWindsurfInjectionSync: + """End-to-end tests for Windsurf global_rules.md injection. + + Windsurf has no dedicated skills dir; apc maintains an APC Skills block + inside ~/.codeium/windsurf/memories/global_rules.md. + """ + + TEST_REPO = "anthropics/skills" + SKILL_A = "pdf" + SKILL_B = "skill-creator" + + def _setup_windsurf(self, tmp_path: Path) -> Path: + """Create the Windsurf memories dir so the tool is detectable.""" + ws_dir = tmp_path / ".codeium" / "windsurf" / "memories" + ws_dir.mkdir(parents=True, exist_ok=True) + return ws_dir + + def _global_rules(self, tmp_path: Path) -> Path: + return tmp_path / ".codeium" / "windsurf" / "memories" / "global_rules.md" + + def test_sync_windsurf_creates_injection_block(self, runner, cli, tmp_path, monkeypatch): + """apc sync writes the APC Skills block into global_rules.md.""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_windsurf(tmp_path) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + r = runner.invoke(cli, ["sync", "--tools", "windsurf", "--yes"]) + assert r.exit_code == 0, r.output + + rules = self._global_rules(tmp_path) + assert rules.exists(), "global_rules.md was not created by apc sync" + content = rules.read_text() + assert "" in content + assert "" in content + assert "## APC Skills" in content + assert self.SKILL_A in content + + def test_sync_windsurf_not_a_symlink(self, runner, cli, tmp_path, monkeypatch): + """global_rules.md must be a real file, not a symlink.""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_windsurf(tmp_path) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "windsurf", "--yes"]) + + rules = self._global_rules(tmp_path) + assert not rules.is_symlink(), "global_rules.md must not be a symlink" + + def test_sync_windsurf_preserves_existing_rules(self, runner, cli, tmp_path, monkeypatch): + """apc sync appends the APC block without erasing existing global rules.""" + monkeypatch.setenv("HOME", str(tmp_path)) + ws_dir = self._setup_windsurf(tmp_path) + rules = ws_dir / "global_rules.md" + rules.write_text("# My Rules\n\nAlways use type hints.\n", encoding="utf-8") + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "windsurf", "--yes"]) + + content = rules.read_text() + assert "# My Rules" in content + assert "Always use type hints." in content + assert "" in content + + def test_install_propagates_to_synced_windsurf(self, runner, cli, tmp_path, monkeypatch): + """After apc sync windsurf, a subsequent apc install updates global_rules.md.""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_windsurf(tmp_path) + + # Sync with one skill + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "windsurf", "--yes"]) + + # Install a second skill — should auto-propagate to Windsurf + r = runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_B, "-y"]) + assert r.exit_code == 0, r.output + + content = self._global_rules(tmp_path).read_text() + assert self.SKILL_A in content + assert self.SKILL_B in content + + def test_sync_windsurf_replaces_stale_block_on_resync(self, runner, cli, tmp_path, monkeypatch): + """Running apc sync again replaces the old block with a fresh one (no duplicates).""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_windsurf(tmp_path) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "windsurf", "--yes"]) + # Install second skill and re-sync + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_B, "-y"]) + runner.invoke(cli, ["sync", "--tools", "windsurf", "--yes"]) + + content = self._global_rules(tmp_path).read_text() + assert content.count("") == 1, "Duplicate APC blocks found" + assert self.SKILL_A in content + assert self.SKILL_B in content + + def test_unsync_windsurf_removes_block(self, runner, cli, tmp_path, monkeypatch): + """apc unsync windsurf removes the APC Skills block from global_rules.md.""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_windsurf(tmp_path) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "windsurf", "--yes"]) + + r = runner.invoke(cli, ["unsync", "windsurf", "--yes"]) + assert r.exit_code == 0, r.output + + content = self._global_rules(tmp_path).read_text() + assert "" not in content + assert "## APC Skills" not in content + + def test_unsync_windsurf_preserves_other_rules(self, runner, cli, tmp_path, monkeypatch): + """apc unsync windsurf keeps existing rule content outside the APC block.""" + monkeypatch.setenv("HOME", str(tmp_path)) + ws_dir = self._setup_windsurf(tmp_path) + rules = ws_dir / "global_rules.md" + rules.write_text("# My Rules\n\nAlways use type hints.\n", encoding="utf-8") + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "windsurf", "--yes"]) + runner.invoke(cli, ["unsync", "windsurf", "--yes"]) + + content = rules.read_text() + assert "# My Rules" in content + assert "Always use type hints." in content + + def test_status_shows_windsurf_synced(self, runner, cli, tmp_path, monkeypatch): + """apc status reflects windsurf as synced after sync.""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_windsurf(tmp_path) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "windsurf", "--yes"]) + + r = runner.invoke(cli, ["status"]) + assert r.exit_code == 0 + + +# --------------------------------------------------------------------------- +# Phase 14: Copilot per-file symlink sync (non-dir-symlink) +# --------------------------------------------------------------------------- + + +class TestCopilotPerFileSync: + """End-to-end tests for Copilot per-file .instructions.md symlinks. + + Copilot reads instructions from ~/.github/instructions/.instructions.md. + apc creates per-skill symlinks pointing into ~/.apc/skills//SKILL.md. + """ + + TEST_REPO = "anthropics/skills" + SKILL_A = "pdf" + SKILL_B = "skill-creator" + + def _setup_copilot(self, tmp_path: Path) -> None: + """Create a minimal Copilot detection marker.""" + (tmp_path / ".copilot").mkdir(parents=True, exist_ok=True) + + def _instr_dir(self, tmp_path: Path) -> Path: + return tmp_path / ".github" / "instructions" + + def test_sync_copilot_creates_instructions_symlinks(self, runner, cli, tmp_path, monkeypatch): + """apc sync creates .instructions.md symlinks in ~/.github/instructions/.""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_copilot(tmp_path) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + r = runner.invoke(cli, ["sync", "--tools", "github-copilot", "--yes"]) + assert r.exit_code == 0, r.output + + instr_dir = self._instr_dir(tmp_path) + link = instr_dir / f"{self.SKILL_A}.instructions.md" + assert link.exists(), f"{link} not created by apc sync" + assert link.is_symlink(), f"{link} should be a symlink" + + def test_sync_copilot_symlink_points_to_skill_md(self, runner, cli, tmp_path, monkeypatch): + """Symlink target resolves to ~/.apc/skills//SKILL.md.""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_copilot(tmp_path) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "github-copilot", "--yes"]) + + link = self._instr_dir(tmp_path) / f"{self.SKILL_A}.instructions.md" + target = link.resolve() + expected = (tmp_path / ".apc" / "skills" / self.SKILL_A / "SKILL.md").resolve() + assert target == expected, f"Symlink points to {target}, expected {expected}" + + def test_install_propagates_to_synced_copilot(self, runner, cli, tmp_path, monkeypatch): + """After apc sync copilot, a subsequent apc install creates a new symlink.""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_copilot(tmp_path) + + # Sync with one skill + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "github-copilot", "--yes"]) + + # Install second skill — should auto-create its symlink + r = runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_B, "-y"]) + assert r.exit_code == 0, r.output + + instr_dir = self._instr_dir(tmp_path) + assert (instr_dir / f"{self.SKILL_A}.instructions.md").is_symlink() + assert (instr_dir / f"{self.SKILL_B}.instructions.md").is_symlink() + + def test_sync_copilot_not_a_dir_symlink(self, runner, cli, tmp_path, monkeypatch): + """~/.github/instructions/ must be a real directory, not a dir-level symlink.""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_copilot(tmp_path) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "github-copilot", "--yes"]) + + instr_dir = self._instr_dir(tmp_path) + assert instr_dir.is_dir() + assert not instr_dir.is_symlink(), "instructions dir should not be a dir-level symlink" + + def test_unsync_copilot_removes_symlinks(self, runner, cli, tmp_path, monkeypatch): + """apc unsync github-copilot removes all .instructions.md symlinks.""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_copilot(tmp_path) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_B, "-y"]) + runner.invoke(cli, ["sync", "--tools", "github-copilot", "--yes"]) + + r = runner.invoke(cli, ["unsync", "github-copilot", "--yes"]) + assert r.exit_code == 0, r.output + + instr_dir = self._instr_dir(tmp_path) + symlinks = list(instr_dir.glob("*.instructions.md")) + assert symlinks == [], f"Expected no symlinks after unsync, got: {symlinks}" + + def test_unsync_copilot_keeps_real_instruction_files(self, runner, cli, tmp_path, monkeypatch): + """apc unsync does not remove manually created .instructions.md files.""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_copilot(tmp_path) + + # Create a manual instruction file before syncing + instr_dir = self._instr_dir(tmp_path) + instr_dir.mkdir(parents=True, exist_ok=True) + manual = instr_dir / "manual.instructions.md" + manual.write_text("# Manual instructions\n", encoding="utf-8") + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "github-copilot", "--yes"]) + runner.invoke(cli, ["unsync", "github-copilot", "--yes"]) + + assert manual.exists(), "Manually created instruction file was deleted by unsync" + assert not manual.is_symlink() + + def test_resync_copilot_refreshes_symlinks(self, runner, cli, tmp_path, monkeypatch): + """Running apc sync again after new installs refreshes all symlinks correctly.""" + monkeypatch.setenv("HOME", str(tmp_path)) + self._setup_copilot(tmp_path) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "github-copilot", "--yes"]) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_B, "-y"]) + r = runner.invoke(cli, ["sync", "--tools", "github-copilot", "--yes"]) + assert r.exit_code == 0, r.output + + instr_dir = self._instr_dir(tmp_path) + assert (instr_dir / f"{self.SKILL_A}.instructions.md").is_symlink() + assert (instr_dir / f"{self.SKILL_B}.instructions.md").is_symlink() From 20c76ef23b1c8a657a616963f2a3468d8721508a Mon Sep 17 00:00:00 2001 From: Forge Date: Sun, 8 Mar 2026 15:33:16 -0700 Subject: [PATCH 6/7] fix: remove unused imports call, pytest (lint) --- tests/test_tool_native_sync.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/test_tool_native_sync.py b/tests/test_tool_native_sync.py index 3a85583..358a47e 100644 --- a/tests/test_tool_native_sync.py +++ b/tests/test_tool_native_sync.py @@ -14,9 +14,7 @@ import sys import unittest from pathlib import Path -from unittest.mock import MagicMock, call, patch - -import pytest +from unittest.mock import MagicMock, patch # Ensure src/ is importable sys.path.insert(0, str(Path(__file__).parent.parent / "src")) @@ -268,10 +266,14 @@ def test_sync_appends_to_existing_rules(self): def test_sync_replaces_existing_apc_block(self): _make_skills_dir(self.home, "pdf") self._rules_path().parent.mkdir(parents=True, exist_ok=True) - self._rules_path().write_text( - "# Rules\n\n\n## APC Skills\n- **old-skill**\n\n", - encoding="utf-8", + stale_block = ( + "# Rules\n\n" + "\n" + "## APC Skills\n" + "- **old-skill**\n" + "\n" ) + self._rules_path().write_text(stale_block, encoding="utf-8") with ( patch("appliers.windsurf._windsurf_global_rules", return_value=self._rules_path()), From 35442653b47932b043f0659adc5f01f02f7d0a0f Mon Sep 17 00:00:00 2001 From: Forge Date: Sun, 8 Mar 2026 15:36:42 -0700 Subject: [PATCH 7/7] =?UTF-8?q?feat:=20apc=20skill=20remove=20=E2=80=94=20?= =?UTF-8?q?propagates=20deletion=20to=20Windsurf=20+=20Copilot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a skill is removed from ~/.apc/skills/: - Dir-symlink tools (Claude Code, OpenClaw, Gemini, Cursor): automatic — the skill dir vanishes from the symlink with no extra action. - Windsurf: regenerates global_rules.md block; removed skill omitted. - Copilot: removes the now-dangling .instructions.md symlink. New: - skills.delete_skill_file(name): removes ~/.apc/skills// tree - BaseApplier.remove_installed_skill(name): default no-op - WindsurfApplier.remove_installed_skill(name): regenerate block - CopilotApplier.remove_installed_skill(name): delete symlink - install.propagate_remove_to_synced_tools(name): calls remove_installed_skill for all synced tools (mirrors _propagate_to_synced_tools pattern) - apc skill remove NAME [NAME...] [--yes]: new CLI command Tests (18 new): - 10 unit tests: BaseApplier no-op, Windsurf regenerate, Copilot symlink removal, propagate_remove_to_synced_tools (all synced, skip unsynced, exception isolation) - 8 docker integration tests: Windsurf block updated after remove, block kept with remaining skills, empty block after last skill, Copilot symlink deleted, other symlinks kept, ~/.apc/skills/ deleted, unknown skill warning, no-sync path safe --- src/appliers/base.py | 9 ++ src/appliers/copilot.py | 8 + src/appliers/windsurf.py | 9 ++ src/install.py | 21 +++ src/skill.py | 58 +++++++- src/skills.py | 12 ++ tests/test_docker_integration.py | 116 +++++++++++++++ tests/test_tool_native_sync.py | 244 +++++++++++++++++++++++++++++++ 8 files changed, 475 insertions(+), 2 deletions(-) diff --git a/src/appliers/base.py b/src/appliers/base.py index aca70bf..96b28c2 100644 --- a/src/appliers/base.py +++ b/src/appliers/base.py @@ -185,6 +185,15 @@ def apply_installed_skill(self, name: str) -> bool: """ return False # dir-symlink tools need no action + def remove_installed_skill(self, name: str) -> bool: + """Clean up after a skill is uninstalled from ~/.apc/skills/. + + Dir-symlink tools: no-op — the skill dir vanishes automatically. + Override in tools that maintain per-skill state (Windsurf, Copilot). + Returns True if an action was taken, False if no-op. + """ + return False # dir-symlink tools need no cleanup + def unsync_skills(self) -> bool: """Undo the skill sync for this tool. diff --git a/src/appliers/copilot.py b/src/appliers/copilot.py index 4687061..64284ea 100644 --- a/src/appliers/copilot.py +++ b/src/appliers/copilot.py @@ -142,6 +142,14 @@ def apply_installed_skill(self, name: str) -> bool: # type: ignore[override] self._link_skill(name, skill_md, instr_dir) return True + def remove_installed_skill(self, name: str) -> bool: # type: ignore[override] + """Remove the dangling .instructions.md symlink for an uninstalled skill.""" + link = self._global_instructions_dir() / f"{name}.instructions.md" + if link.is_symlink(): + link.unlink() + return True + return False + def unsync_skills(self) -> bool: # type: ignore[override] """Remove all apc-managed .instructions.md symlinks from ~/.github/instructions/.""" instr_dir = self._global_instructions_dir() diff --git a/src/appliers/windsurf.py b/src/appliers/windsurf.py index c61fd46..69a327a 100644 --- a/src/appliers/windsurf.py +++ b/src/appliers/windsurf.py @@ -152,6 +152,15 @@ def apply_installed_skill(self, name: str) -> bool: # type: ignore[override] self._write_skills_to_global_rules() return True + def remove_installed_skill(self, name: str) -> bool: # type: ignore[override] + """Regenerate the APC skills block after a skill is uninstalled. + + The deleted skill is already gone from ~/.apc/skills/ at this point, + so _write_skills_to_global_rules() will naturally omit it. + """ + self._write_skills_to_global_rules() + return True + def unsync_skills(self) -> bool: # type: ignore[override] """Remove the APC skills section from global_rules.md.""" rules_path = _windsurf_global_rules() diff --git a/src/install.py b/src/install.py index cb82012..b2c8279 100644 --- a/src/install.py +++ b/src/install.py @@ -51,6 +51,27 @@ def _validate_branch(branch: str) -> None: _AGENTS = ["claude-code", "cursor", "gemini-cli", "github-copilot", "openclaw", "windsurf"] +def propagate_remove_to_synced_tools(skill_name: str) -> None: + """Notify all synced tools that a skill has been uninstalled. + + - Dir-symlink tools: no-op — the skill dir is already gone from the symlink. + - Windsurf: regenerates global_rules.md block (deleted skill won't appear). + - Copilot: removes the now-dangling .instructions.md symlink. + """ + from appliers.manifest import ToolManifest + from extractors import detect_installed_tools + + for tool_name in detect_installed_tools(): + manifest = ToolManifest(tool_name) + if manifest.is_first_sync: + continue + try: + applier = get_applier(tool_name) + applier.remove_installed_skill(skill_name) + except Exception: + pass + + def _propagate_to_synced_tools(skill_name: str) -> None: """Push a newly installed skill to every tool that has been synced. diff --git a/src/skill.py b/src/skill.py index dc77a46..0677d64 100644 --- a/src/skill.py +++ b/src/skill.py @@ -5,9 +5,11 @@ import click -from cache import load_skills +from cache import load_skills, save_skills +from install import propagate_remove_to_synced_tools +from skills import delete_skill_file, get_skills_dir from sync_helpers import resolve_target_tools, sync_skills -from ui import dim, header, paged_print, skill_detail, skills_list +from ui import dim, error, header, paged_print, skill_detail, skills_list, success, warning @click.group() @@ -63,3 +65,55 @@ def skill_sync(tools, apply_all, yes): return sync_skills(tool_list) + + +@skill.command("remove") +@click.argument("names", nargs=-1, required=True, metavar="NAME [NAME...]") +@click.option("--yes", "-y", is_flag=True, help="Skip confirmation prompt.") +def remove(names: tuple, yes: bool) -> None: + """Remove one or more installed skills. + + Deletes the skill from ~/.apc/skills/ and cleans up any tool-specific + state left behind: + + \b + - Dir-symlink tools (Claude Code, OpenClaw, Gemini, Cursor): automatic — + the skill dir disappears from the symlink immediately. + - Windsurf: regenerates global_rules.md to drop the removed skill. + - Copilot: removes the now-dangling .instructions.md symlink. + + \b + Examples: + apc skill remove pdf + apc skill remove pdf skill-creator -y + """ + skills_dir = get_skills_dir() + existing = [n for n in names if (skills_dir / n).exists()] + missing = [n for n in names if n not in existing] + + if missing: + for m in missing: + warning(f"Skill '{m}' not found in ~/.apc/skills/ — skipping.") + + if not existing: + dim("Nothing to remove.") + return + + if not yes: + click.echo(f"\nRemove skill(s): {', '.join(existing)}") + if not click.confirm("Proceed?"): + click.echo("Cancelled.") + return + + removed = [] + for name in existing: + if delete_skill_file(name): + # Update the metadata cache + cached = [s for s in load_skills() if s.get("name") != name] + save_skills(cached) + # Notify synced tools + propagate_remove_to_synced_tools(name) + success(f"Removed: {name}") + removed.append(name) + else: + error(f"Failed to remove: {name}") diff --git a/src/skills.py b/src/skills.py index 8b86660..d3c5725 100644 --- a/src/skills.py +++ b/src/skills.py @@ -63,6 +63,18 @@ def save_skill_file(skill_name: str, raw_content: str) -> Path: return path +def delete_skill_file(skill_name: str) -> bool: + """Remove ~/.apc/skills// and its contents. Returns True if deleted.""" + skill_name = sanitize_skill_name(skill_name) + skill_dir = get_skills_dir() / skill_name + if not skill_dir.exists(): + return False + import shutil + + shutil.rmtree(skill_dir) + return True + + # --------------------------------------------------------------------------- # GitHub helpers # --------------------------------------------------------------------------- diff --git a/tests/test_docker_integration.py b/tests/test_docker_integration.py index f73bfa6..1c185b7 100644 --- a/tests/test_docker_integration.py +++ b/tests/test_docker_integration.py @@ -1722,3 +1722,119 @@ def test_resync_copilot_refreshes_symlinks(self, runner, cli, tmp_path, monkeypa instr_dir = self._instr_dir(tmp_path) assert (instr_dir / f"{self.SKILL_A}.instructions.md").is_symlink() assert (instr_dir / f"{self.SKILL_B}.instructions.md").is_symlink() + + +# --------------------------------------------------------------------------- +# Phase 15: apc skill remove — non-symlink tool cleanup +# --------------------------------------------------------------------------- + + +class TestSkillRemoveNonSymlinkTools: + """End-to-end: apc skill remove cleans up Windsurf injection and Copilot symlinks.""" + + TEST_REPO = "anthropics/skills" + SKILL_A = "pdf" + SKILL_B = "skill-creator" + + # -- Windsurf ------------------------------------------------------- + + def test_remove_updates_windsurf_injection(self, runner, cli, tmp_path, monkeypatch): + """After skill remove, Windsurf global_rules.md no longer lists the skill.""" + monkeypatch.setenv("HOME", str(tmp_path)) + ws_dir = tmp_path / ".codeium" / "windsurf" / "memories" + ws_dir.mkdir(parents=True, exist_ok=True) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_B, "-y"]) + runner.invoke(cli, ["sync", "--tools", "windsurf", "--yes"]) + + r = runner.invoke(cli, ["skill", "remove", self.SKILL_A, "--yes"]) + assert r.exit_code == 0, r.output + + content = (ws_dir / "global_rules.md").read_text() + assert self.SKILL_A not in content + assert self.SKILL_B in content + + def test_remove_keeps_windsurf_block_with_remaining(self, runner, cli, tmp_path, monkeypatch): + """APC Skills block stays intact; only the removed skill is omitted.""" + monkeypatch.setenv("HOME", str(tmp_path)) + ws_dir = tmp_path / ".codeium" / "windsurf" / "memories" + ws_dir.mkdir(parents=True, exist_ok=True) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_B, "-y"]) + runner.invoke(cli, ["sync", "--tools", "windsurf", "--yes"]) + runner.invoke(cli, ["skill", "remove", self.SKILL_A, "--yes"]) + + content = (ws_dir / "global_rules.md").read_text() + assert "" in content + assert "" in content + + def test_remove_last_skill_leaves_empty_block(self, runner, cli, tmp_path, monkeypatch): + """Removing the last skill leaves a valid (empty) APC Skills block.""" + monkeypatch.setenv("HOME", str(tmp_path)) + ws_dir = tmp_path / ".codeium" / "windsurf" / "memories" + ws_dir.mkdir(parents=True, exist_ok=True) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "windsurf", "--yes"]) + runner.invoke(cli, ["skill", "remove", self.SKILL_A, "--yes"]) + + content = (ws_dir / "global_rules.md").read_text() + assert "" in content + assert self.SKILL_A not in content + + # -- Copilot -------------------------------------------------------- + + def test_remove_deletes_copilot_symlink(self, runner, cli, tmp_path, monkeypatch): + """After skill remove, the .instructions.md symlink is gone.""" + monkeypatch.setenv("HOME", str(tmp_path)) + (tmp_path / ".copilot").mkdir(parents=True, exist_ok=True) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["sync", "--tools", "github-copilot", "--yes"]) + + r = runner.invoke(cli, ["skill", "remove", self.SKILL_A, "--yes"]) + assert r.exit_code == 0, r.output + + instr_dir = tmp_path / ".github" / "instructions" + assert not (instr_dir / f"{self.SKILL_A}.instructions.md").exists() + + def test_remove_keeps_other_copilot_symlinks(self, runner, cli, tmp_path, monkeypatch): + """Only the removed skill's symlink is deleted; others remain.""" + monkeypatch.setenv("HOME", str(tmp_path)) + (tmp_path / ".copilot").mkdir(parents=True, exist_ok=True) + + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_B, "-y"]) + runner.invoke(cli, ["sync", "--tools", "github-copilot", "--yes"]) + runner.invoke(cli, ["skill", "remove", self.SKILL_A, "--yes"]) + + instr_dir = tmp_path / ".github" / "instructions" + assert not (instr_dir / f"{self.SKILL_A}.instructions.md").exists() + assert (instr_dir / f"{self.SKILL_B}.instructions.md").is_symlink() + + def test_remove_skill_also_deletes_from_apc_skills(self, runner, cli, tmp_path, monkeypatch): + """Skill dir in ~/.apc/skills/ is deleted regardless of sync state.""" + monkeypatch.setenv("HOME", str(tmp_path)) + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + + skill_md = tmp_path / ".apc" / "skills" / self.SKILL_A / "SKILL.md" + assert skill_md.exists() + + runner.invoke(cli, ["skill", "remove", self.SKILL_A, "--yes"]) + assert not skill_md.exists() + + def test_remove_unknown_skill_warns_gracefully(self, runner, cli, tmp_path, monkeypatch): + """Removing a skill that doesn't exist prints a warning and exits cleanly.""" + monkeypatch.setenv("HOME", str(tmp_path)) + r = runner.invoke(cli, ["skill", "remove", "nonexistent-skill-xyz", "--yes"]) + assert r.exit_code == 0 + assert "not found" in r.output.lower() or "nothing" in r.output.lower() + + def test_remove_no_sync_does_not_crash(self, runner, cli, tmp_path, monkeypatch): + """Removing a skill when no tools are synced completes without error.""" + monkeypatch.setenv("HOME", str(tmp_path)) + runner.invoke(cli, ["install", self.TEST_REPO, "--skill", self.SKILL_A, "-y"]) + r = runner.invoke(cli, ["skill", "remove", self.SKILL_A, "--yes"]) + assert r.exit_code == 0 diff --git a/tests/test_tool_native_sync.py b/tests/test_tool_native_sync.py index 358a47e..28e791e 100644 --- a/tests/test_tool_native_sync.py +++ b/tests/test_tool_native_sync.py @@ -756,5 +756,249 @@ def test_short_t_flag_rejected(self): assert result.exit_code != 0 +# --------------------------------------------------------------------------- +# BaseApplier: remove_installed_skill default (no-op) +# --------------------------------------------------------------------------- + + +class TestBaseApplierRemoveSkill(unittest.TestCase): + def test_remove_installed_skill_returns_false(self): + """Dir-symlink tools return False — symlink makes the deletion automatic.""" + from appliers.claude import ClaudeApplier + + applier = ClaudeApplier() + result = applier.remove_installed_skill("pdf") + assert result is False + + +# --------------------------------------------------------------------------- +# WindsurfApplier: remove_installed_skill +# --------------------------------------------------------------------------- + + +class TestWindsurfRemoveSkill(unittest.TestCase): + def setUp(self): + import tempfile + + self._tmp = tempfile.TemporaryDirectory() + self.home = Path(self._tmp.name) + + def tearDown(self): + self._tmp.cleanup() + + def _rules_path(self): + return self.home / ".codeium" / "windsurf" / "memories" / "global_rules.md" + + def _make_applier(self): + from appliers.windsurf import WindsurfApplier + + return WindsurfApplier() + + def test_remove_regenerates_block_without_deleted_skill(self): + """After uninstall, global_rules.md block no longer lists the removed skill.""" + _make_skills_dir(self.home, "pdf", "frontend-design") + + with ( + patch("appliers.windsurf._windsurf_global_rules", return_value=self._rules_path()), + patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"), + ): + applier = self._make_applier() + applier.sync_skills_dir() + + # Delete the skill dir (simulate apc uninstall) + import shutil + + shutil.rmtree(self.home / ".apc" / "skills" / "pdf") + + result = applier.remove_installed_skill("pdf") + + assert result is True + content = self._rules_path().read_text() + assert "pdf" not in content + assert "frontend-design" in content + + def test_remove_returns_true_even_when_no_other_skills(self): + """Regeneration succeeds even if all skills are gone (produces empty block).""" + _make_skills_dir(self.home, "pdf") + + with ( + patch("appliers.windsurf._windsurf_global_rules", return_value=self._rules_path()), + patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"), + ): + applier = self._make_applier() + applier.sync_skills_dir() + + import shutil + + shutil.rmtree(self.home / ".apc" / "skills" / "pdf") + result = applier.remove_installed_skill("pdf") + + assert result is True + content = self._rules_path().read_text() + assert "" in content + assert "pdf" not in content + + +# --------------------------------------------------------------------------- +# CopilotApplier: remove_installed_skill +# --------------------------------------------------------------------------- + + +class TestCopilotRemoveSkill(unittest.TestCase): + def setUp(self): + import tempfile + + self._tmp = tempfile.TemporaryDirectory() + self.home = Path(self._tmp.name) + + def tearDown(self): + self._tmp.cleanup() + + def _make_applier(self): + from appliers.copilot import CopilotApplier + + applier = CopilotApplier() + applier._global_instructions_dir = lambda: self.home / ".github" / "instructions" + return applier + + def test_remove_deletes_dangling_symlink(self): + """remove_installed_skill removes the .instructions.md symlink for the skill.""" + _make_skills_dir(self.home, "pdf") + + with patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"): + applier = self._make_applier() + applier.apply_installed_skill("pdf") # create symlink + + # Simulate skill deletion (symlink is now dangling) + import shutil + + shutil.rmtree(self.home / ".apc" / "skills" / "pdf") + + result = applier.remove_installed_skill("pdf") + + instr_dir = self.home / ".github" / "instructions" + assert result is True + assert not (instr_dir / "pdf.instructions.md").exists() + + def test_remove_keeps_other_skill_symlinks(self): + """Only the targeted skill's symlink is removed.""" + _make_skills_dir(self.home, "pdf", "frontend-design") + + with patch("skills.get_skills_dir", return_value=self.home / ".apc" / "skills"): + applier = self._make_applier() + applier.sync_skills_dir() + + import shutil + + shutil.rmtree(self.home / ".apc" / "skills" / "pdf") + applier.remove_installed_skill("pdf") + + instr_dir = self.home / ".github" / "instructions" + assert not (instr_dir / "pdf.instructions.md").exists() + assert (instr_dir / "frontend-design.instructions.md").is_symlink() + + def test_remove_returns_false_when_no_symlink(self): + """Returns False when the symlink doesn't exist (already clean).""" + applier = self._make_applier() + result = applier.remove_installed_skill("nonexistent") + assert result is False + + def test_remove_does_not_touch_real_files(self): + """remove_installed_skill ignores real (non-symlink) instruction files.""" + instr_dir = self.home / ".github" / "instructions" + instr_dir.mkdir(parents=True, exist_ok=True) + real = instr_dir / "pdf.instructions.md" + real.write_text("# Manual", encoding="utf-8") + + applier = self._make_applier() + result = applier.remove_installed_skill("pdf") + + assert result is False # not a symlink — left untouched + assert real.exists() + + +# --------------------------------------------------------------------------- +# propagate_remove_to_synced_tools +# --------------------------------------------------------------------------- + + +class TestPropagateRemoveToSyncedTools(unittest.TestCase): + def test_calls_remove_for_all_synced_tools(self): + from install import propagate_remove_to_synced_tools + + mock_windsurf = MagicMock() + mock_copilot = MagicMock() + mock_cursor = MagicMock() + + not_synced = MagicMock() + not_synced.is_first_sync = True + synced = MagicMock() + synced.is_first_sync = False + + def fake_manifest(name): + return not_synced if name == "claude-code" else synced + + appliers = { + "windsurf": mock_windsurf, + "github-copilot": mock_copilot, + "cursor": mock_cursor, + } + + def fake_get_applier(name): + return appliers[name] + + with ( + patch( + "install.detect_installed_tools", + return_value=["windsurf", "github-copilot", "cursor", "claude-code"], + ), + patch("appliers.manifest.ToolManifest", side_effect=fake_manifest), + patch("install.get_applier", side_effect=fake_get_applier), + ): + propagate_remove_to_synced_tools("pdf") + + mock_windsurf.remove_installed_skill.assert_called_once_with("pdf") + mock_copilot.remove_installed_skill.assert_called_once_with("pdf") + mock_cursor.remove_installed_skill.assert_called_once_with("pdf") + + def test_skips_unsynced_tools(self): + from install import propagate_remove_to_synced_tools + + not_synced = MagicMock() + not_synced.is_first_sync = True + + with ( + patch("install.detect_installed_tools", return_value=["cursor"]), + patch("appliers.manifest.ToolManifest", return_value=not_synced), + patch("install.get_applier") as mock_get, + ): + propagate_remove_to_synced_tools("pdf") + + mock_get.assert_not_called() + + def test_exception_does_not_abort(self): + from install import propagate_remove_to_synced_tools + + mock_ok = MagicMock() + mock_bad = MagicMock() + mock_bad.remove_installed_skill.side_effect = RuntimeError("boom") + + synced = MagicMock() + synced.is_first_sync = False + + with ( + patch("install.detect_installed_tools", return_value=["windsurf", "cursor"]), + patch("appliers.manifest.ToolManifest", return_value=synced), + patch( + "install.get_applier", + side_effect=lambda n: mock_bad if n == "windsurf" else mock_ok, + ), + ): + propagate_remove_to_synced_tools("pdf") + + mock_ok.remove_installed_skill.assert_called_with("pdf") + assert mock_ok.remove_installed_skill.call_count >= 1 + + if __name__ == "__main__": unittest.main()