fix(skills): resolve skill_manage targets through the symlink-following index walk

tests/tools/test_skill_manager_tool.py

@@ -242,6 +242,92 @@ def test_edit_invalid_content_rejected(self, tmp_path):
         content = (tmp_path / "my-skill" / "SKILL.md").read_text()
         assert "A test skill" in content
 
+# ---------------------------------------------------------------------------
+# Profiles that install skills as per-skill symlinks into a shared tree
+# ---------------------------------------------------------------------------
+
+
+@contextmanager
+def _symlinked_skill_lane(tmp_path):
+    """A profile whose skills root holds real category dirs and SYMLINKED skills.
+
+    The profile installs every skill as a per-skill symlink into the shared tree
+    (``profiles/<p>/skills/<cat>/<skill>`` -> ``<shared>/<cat>/<skill>``), so its
+    skills are links, not directories. Yields ``(lane_root, canonical_skill_dir)``
+    with both skill-dir lookups patched to the lane root.
+    """
+    canonical = tmp_path / "shared" / "ops" / "linked-skill"
+    canonical.mkdir(parents=True)
+    (canonical / "SKILL.md").write_text(VALID_SKILL_CONTENT)
+    lane_root = tmp_path / "lane" / "skills"
+    (lane_root / "ops").mkdir(parents=True)
+    (lane_root / "ops" / "linked-skill").symlink_to(canonical, target_is_directory=True)
+    with patch("tools.skill_manager_tool.SKILLS_DIR", lane_root), \
+         patch("agent.skill_utils.get_all_skills_dirs", return_value=[lane_root]):
+        yield lane_root, canonical
+
+
+class TestSymlinkedSkillDirs:
+    """skill_manage must resolve the same skills skill_view resolves.
+
+    ``Path.rglob`` does not descend into symlinked directories, so the write path
+    saw none of that profile's skills while the read path (``iter_skill_index_files``,
+    which walks with ``followlinks=True``) resolved all of them. Every write failed
+    with "not found in active profile", and the error even named the profiles that
+    DO "have" the skill, because a profile whose skills root IS a symlink works
+    with rglob.
+    """
+
+    def test_find_skill_resolves_per_skill_symlink(self, tmp_path):
+        with _symlinked_skill_lane(tmp_path) as (lane_root, _canonical):
+            found = _find_skill("linked-skill")
+        assert found is not None, "a lane's symlinked skill must be discoverable"
+        assert found["path"] == lane_root / "ops" / "linked-skill"
+
+    def test_find_skill_resolves_categorized_name_for_symlink(self, tmp_path):
+        with _symlinked_skill_lane(tmp_path) as (lane_root, _canonical):
+            found = _find_skill("ops/linked-skill")
+        assert found is not None, "categorized form must resolve through a symlink too"
+        assert found["path"] == lane_root / "ops" / "linked-skill"
+
+    def test_patch_edits_canonical_file_behind_symlink(self, tmp_path):
+        with _symlinked_skill_lane(tmp_path) as (_lane_root, canonical):
+            result = _patch_skill("linked-skill", "Do the thing.", "Do the new thing.")
+        assert result["success"] is True, result.get("error")
+        assert "Do the new thing." in (canonical / "SKILL.md").read_text()
+
+    def test_write_file_lands_in_canonical_dir_behind_symlink(self, tmp_path):
+        with _symlinked_skill_lane(tmp_path) as (lane_root, canonical):
+            result = _write_file("linked-skill", "references/notes.md", "lane note")
+            lane_side = lane_root / "ops" / "linked-skill" / "references" / "notes.md"
+        assert result["success"] is True, result.get("error")
+        assert (canonical / "references" / "notes.md").read_text() == "lane note"
+        assert lane_side.read_text() == "lane note", "same file through the link"
+
+    def test_dispatcher_patches_through_symlink(self, tmp_path):
+        """The model-facing entry point, not only the private helper."""
+        with _symlinked_skill_lane(tmp_path) as (_lane_root, canonical):
+            raw = skill_manage(action="patch", name="linked-skill",
+                               old_string="Do the thing.", new_string="Do the new thing.")
+        result = json.loads(raw)
+        assert result["success"] is True, result.get("error")
+        assert "Do the new thing." in (canonical / "SKILL.md").read_text()
+
+    def test_whole_root_symlink_still_resolves(self, tmp_path):
+        """No behaviour change for a profile whose skills root IS the symlink."""
+        shared = tmp_path / "shared-root"
+        (shared / "cat" / "rooted-skill").mkdir(parents=True)
+        (shared / "cat" / "rooted-skill" / "SKILL.md").write_text(VALID_SKILL_CONTENT)
+        root_link = tmp_path / "linked-root"
+        root_link.symlink_to(shared, target_is_directory=True)
+        with patch("tools.skill_manager_tool.SKILLS_DIR", root_link), \
+             patch("agent.skill_utils.get_all_skills_dirs", return_value=[root_link]):
+            bare = _find_skill("rooted-skill")
+            categorized = _find_skill("cat/rooted-skill")
+        assert bare is not None and bare["path"] == root_link / "cat" / "rooted-skill"
+        assert categorized is not None, "categorized form must keep resolving here"
+
+
 class TestPatchSkill:
     def test_patch_unique_match(self, tmp_path):
         with _skill_dir(tmp_path):

tools/skill_manager_tool.py

@@ -193,10 +193,19 @@ def _resolve_skill_dir(name: str, category: str = None) -> Path:
 
 
 def _iter_skill_dirs(root: Path):
-    from agent.skill_utils import is_excluded_skill_path
-    for skill_md in root.rglob("SKILL.md"):
-        if not is_excluded_skill_path(skill_md):
-            yield skill_md.parent
+    """Skill dirs under ``root`` (every directory holding a ``SKILL.md``).
+
+    Delegates to the canonical index walk the READ path uses (skill_view /
+    skills_list). ``Path.rglob`` does not descend into symlinked directories, so a
+    profile that installs each skill as a per-skill symlink into a shared tree
+    (``profiles/<p>/skills/<cat>/<skill>`` -> ``<shared>/<cat>/<skill>``) had none of
+    its skills visible to the write path while every read resolved them: writes
+    failed with "not found in active profile". Keep this on ``iter_skill_index_files``
+    -- write resolution and read resolution disagreeing IS the bug.
+    """
+    from agent.skill_utils import iter_skill_index_files
+    for skill_md in iter_skill_index_files(root, "SKILL.md"):
+        yield skill_md.parent
 
 
 def _find_skill(name: str) -> Optional[Dict[str, Any]]:
@@ -206,25 +215,32 @@ def _find_skill(name: str) -> Optional[Dict[str, Any]]:
     categorized relative path (``mlops/axolotl``) — the two forms skill_view resolves. The
     categorized form matches RELATIVE to the local root only (relative_to raises for external dirs)."""
     from agent.skill_utils import get_all_skills_dirs
-    local_root = None
+    local_roots: List[Path] = []
     if "/" in name or "\\" in name:
+        # BOTH forms of the root: a profile that keeps its skills as per-skill symlinks
+        # into a shared tree would fail a resolved comparison (it points outside the root),
+        # while a profile whose skills root is itself a symlink resolves past the configured
+        # path. Lexical first, resolved as a fallback.
+        local_roots.append(_skills_dir())
         try:
-            local_root = _skills_dir().resolve()
+            resolved_root = _skills_dir().resolve()
         except OSError:
             logger.debug(
-                "skills dir resolve failed; categorized lookups fall back to the unresolved path",
+                "skills dir resolve failed; categorized lookups use the unresolved path only",
                 exc_info=True)
-            local_root = _skills_dir()
+        else:
+            if resolved_root != local_roots[0]:
+                local_roots.append(resolved_root)
     for skills_dir in get_all_skills_dirs():
         if not skills_dir.exists():
             continue
         for skill_dir in _iter_skill_dirs(skills_dir):
             if skill_dir.name == name:
                 return {"path": skill_dir}
-            if local_root is not None:
-                resolved = skill_dir.resolve()
-                if (resolved.is_relative_to(local_root)
-                        and resolved.relative_to(local_root).as_posix() == name):  # POSIX form
+            for local_root in local_roots:
+                if not skill_dir.is_relative_to(local_root):
+                    continue
+                if skill_dir.relative_to(local_root).as_posix() == name:  # POSIX form
                     return {"path": skill_dir}
     return None