fix(#33): apc install symlinks skills instead of copying them

- Replace _apply_skill_to_targets() (apply_skills) with _link_skill_to_targets()
  (link_skills) in install.py — skill is saved to ~/.apc/skills/ then symlinked
  into each tool's skills dir, never copied
- Revert openclaw.py symlink-unlink workaround — no longer needed since
  apply_skills() is only called for collected skills (no pre-existing symlink)
- Update test: apply_skills now asserts real dir created (collected skills path);
  add test confirming no symlinks from apply_skills

Author: forge-fz2000

src/appliers/openclaw.py

@@ -85,11 +85,7 @@ def apply_skills(self, skills: List[Dict], manifest: ToolManifest) -> int:
 
             # OpenClaw uses directory-based skills: <name>/SKILL.md
             skill_dir = _openclaw_skills_dir() / name
-
-            # If the dir exists (real or symlink → dir from apc install), skip mkdir
-            # so we write through the existing symlink rather than replacing it.
-            if not skill_dir.exists():
-                skill_dir.mkdir(parents=True, exist_ok=True)
+            skill_dir.mkdir(parents=True, exist_ok=True)
             path = skill_dir / "SKILL.md"
             path.write_text(content, encoding="utf-8")
             manifest.record_skill(name, file_path=str(path), content=content)

src/install.py

@@ -83,17 +83,22 @@ def _resolve_targets(target_args: tuple, yes: bool) -> List[str]:
     return targets
 
 
-def _apply_skill_to_targets(skill: dict, target_list: list) -> int:
-    """Write a skill directly to each target's skill directory. Returns applied count."""
+def _link_skill_to_targets(skill: dict, target_list: list) -> int:
+    """Symlink a skill from ~/.apc/skills/ into each target's skill directory.
 
+    Uses link_skills() so the tool's skill dir always points back to the
+    canonical source — never a stale copy.
+    Returns number of targets linked.
+    """
+    skills_dir = get_skills_dir()
     count = 0
     for target_name in target_list:
         try:
             applier = get_applier(target_name)
             manifest = applier.get_manifest()
-            applied = applier.apply_skills([skill], manifest)
+            linked = applier.link_skills([skill], skills_dir, manifest)
             manifest.save()
-            count += applied
+            count += linked
         except Exception as e:
             click.echo(f"  ! {target_name}: {e}", err=True)
     return count
@@ -232,7 +237,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
-        _apply_skill_to_targets(skill, target_list)
+        _link_skill_to_targets(skill, target_list)
 
         # Save metadata to local cache
         existing = load_skills()

tests/test_appliers.py

@@ -453,35 +453,22 @@ def test_apply_skills_clean_dir(self):
         self.assertTrue(skill_md.exists())
         self.assertIn("test-skill", manifest.managed_skill_names())
 
-    def test_apply_skills_replaces_symlink(self):
-        """apply_skills must not crash when skill dir is a pre-existing symlink (apc install)."""
-        # Simulate what `apc install` does: create a symlink in skills_dir
-        real_source = self.tmpdir / "source" / "test-skill"
-        real_source.mkdir(parents=True)
-        (real_source / "SKILL.md").write_text("# Original", encoding="utf-8")
-
-        symlink_path = self.skills_dir / "test-skill"
-        symlink_path.symlink_to(real_source)
-        self.assertTrue(symlink_path.is_symlink(), "precondition: symlink must exist")
-
-        # Now run apply_skills — must NOT raise FileExistsError (errno 17)
+    def test_apply_skills_does_not_create_symlinks(self):
+        """apply_skills (collected skills path) always writes real directories, never symlinks.
+
+        Installed skills are linked via link_skills(). apply_skills() is only called
+        for collected skills which have no source directory — so it must create a real dir.
+        """
         manifest = self._manifest()
         with patch("appliers.openclaw._openclaw_skills_dir", return_value=self.skills_dir):
             from appliers.openclaw import OpenClawApplier
 
             applier = OpenClawApplier()
-            try:
-                count = applier.apply_skills([self._skill()], manifest)
-            except FileExistsError as exc:
-                self.fail(f"apply_skills raised FileExistsError on symlink: {exc}")
+            applier.apply_skills([self._skill()], manifest)
 
-        self.assertEqual(count, 1)
-        skill_md = self.skills_dir / "test-skill" / "SKILL.md"
-        self.assertTrue(skill_md.exists(), "SKILL.md should be written through the symlink")
-        self.assertTrue(
-            symlink_path.is_symlink(),
-            "apc install symlink must be preserved — apply_skills writes through it, not over it",
-        )
+        skill_dir = self.skills_dir / "test-skill"
+        self.assertTrue(skill_dir.is_dir())
+        self.assertFalse(skill_dir.is_symlink(), "apply_skills must create a real dir, not a symlink")
 
     def test_apply_skills_multiple_skills(self):
         """apply_skills handles multiple skills in one call."""