From 6ec55273864ce0cdefdf4b526205104372294cf0 Mon Sep 17 00:00:00 2001
From: nesquena-hermes <agent@nesquena-hermes>
Date: Fri, 12 Jun 2026 20:28:46 +0000
Subject: [PATCH] harden(#4057): pass local skills dir to
 _skill_category_from_path explicitly (Opus SHOULD-FIX)

_active_skill_search_dirs filters to existing dirs, so on a host with no local
skills dir but configured external dirs the local root is dropped from the list
and the position-based skills_dirs[0]==local assumption misclassified the first
external root as local (its flat skills silently lost their category label).
Pass the local dir explicitly (backward-compatible optional param defaulting to
the old skills_dirs[0] behavior) + regression test for the absent-local-dir case.
---
 api/routes.py                                 | 23 ++++++++++++----
 ...est_issue4053_external_skill_categories.py | 26 +++++++++++++++++++
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/api/routes.py b/api/routes.py
index 00968879..6e09c5e6 100644
--- a/api/routes.py
+++ b/api/routes.py
@@ -226,13 +226,24 @@ def _skill_path_within(base_dir: Path, candidate: Path) -> bool:
         return False
 
 
-def _skill_category_from_path(skill_md: Path, skills_dirs: list[Path]) -> str | None:
+def _skill_category_from_path(
+    skill_md: Path,
+    skills_dirs: list[Path],
+    local_skills_dir: Path | None = None,
+) -> str | None:
     """Return the UI category for a discovered skill path.
 
-    ``skills_dirs[0]`` must be the active local skills root so flat local skills
-    stay uncategorized while flat external roots can use their directory name.
+    Flat skills directly under the active *local* skills root stay uncategorized,
+    while flat skills under an *external* root use that root's directory name as
+    their category. ``local_skills_dir`` identifies the local root explicitly; if
+    omitted it falls back to ``skills_dirs[0]`` for backward compatibility, but
+    callers should pass it directly because the local root can be filtered out of
+    ``skills_dirs`` (e.g. when it does not exist yet on a host with only external
+    skills configured), which would otherwise misclassify the first external root
+    as local.
     """
-    local_skills_dir = skills_dirs[0] if skills_dirs else None
+    if local_skills_dir is None:
+        local_skills_dir = skills_dirs[0] if skills_dirs else None
     for skills_dir in skills_dirs:
         try:
             rel_path = skill_md.relative_to(skills_dir)
@@ -399,7 +410,9 @@ def _skills_list_from_dir(skills_dir: Path, category: str | None = None) -> dict
                     {
                         "name": name,
                         "description": description,
-                        "category": _skill_category_from_path(skill_md, search_dirs),
+                        "category": _skill_category_from_path(
+                            skill_md, search_dirs, local_skills_dir=skills_dir
+                        ),
                         "disabled": name in disabled,
                     }
                 )
diff --git a/tests/test_issue4053_external_skill_categories.py b/tests/test_issue4053_external_skill_categories.py
index ee339356..a506dd80 100644
--- a/tests/test_issue4053_external_skill_categories.py
+++ b/tests/test_issue4053_external_skill_categories.py
@@ -39,3 +39,29 @@ def test_external_skill_categories_keep_local_flat_and_label_external_roots(tmp_
         == external_flat_root.name
     )
     assert routes._skill_category_from_path(nested_external_skill, search_dirs) == "ops"
+
+
+def test_external_skill_category_when_local_dir_absent_from_search_dirs(tmp_path):
+    """When the local skills dir does not exist it is filtered out of the search
+    list (``_active_skill_search_dirs`` keeps only existing dirs). Passing the
+    local dir explicitly must keep its flat skills uncategorized AND still let a
+    flat external root use its directory name — instead of misidentifying the
+    first surviving (external) root as local."""
+    from api import routes
+
+    local_root = tmp_path / "skills"  # intentionally never created
+    external_flat_root = tmp_path / "partner-skills"
+    flat_external_skill = _write_skill(external_flat_root, "external-flat-skill")
+
+    # _active_skill_search_dirs would drop the missing local_root; simulate that.
+    search_dirs = [external_flat_root]
+
+    # Without local_skills_dir, the old position-based logic would treat the
+    # external root as local and return None. With it passed explicitly, the
+    # external root is correctly labeled.
+    assert (
+        routes._skill_category_from_path(
+            flat_external_skill, search_dirs, local_skills_dir=local_root
+        )
+        == external_flat_root.name
+    )