diff --git a/agent/skill_utils.py b/agent/skill_utils.py index a76c5cc1d2..51ecc7044b 100644 --- a/agent/skill_utils.py +++ b/agent/skill_utils.py @@ -481,9 +481,10 @@ def pick_skill_candidate(candidates) -> Tuple[Optional[int], List[int]]: def is_disabled_entry(entry: Dict[str, Any], disabled: Set[str]) -> bool: - """``skills.disabled`` matches a resolved catalog entry by its declared name OR its exact relative - path — the identifier a same-tier duplicate's list/config/web row shows (``a/one``) and saves.""" - return bool(disabled) and not disabled.isdisjoint({str(entry["name"]), entry["relative_path"]}) + """``skills.disabled`` matches a resolved catalog entry by its declared name OR its ``load_name`` — + the exact path a same-tier duplicate's list/config/web row shows (``a/one``) and saves. A unique + copy elsewhere that merely sits at the same relative path is not matched.""" + return bool(disabled) and not disabled.isdisjoint({str(entry["name"]), entry.get("load_name")} - {None}) def resolve_skill_catalog(entries: List[Dict[str, Any]]) -> List[Dict[str, Any]]: diff --git a/tests/agent/test_skill_commands.py b/tests/agent/test_skill_commands.py index cf17816f6e..a799853aef 100644 --- a/tests/agent/test_skill_commands.py +++ b/tests/agent/test_skill_commands.py @@ -592,14 +592,24 @@ class TestDuplicateNamesAgreeAcrossSurfaces: assert ambiguous in parts[0] and "LOCAL XDUP" in "\n".join(parts) # Disabling a duplicate by the exact path its row shows (what `hermes skills` / the web toggle - # save) hides it from list and index and refuses the load. + # save) hides it from list and index and refuses the load — but not an unrelated, uniquely named + # skill in another tier that merely sits at the same relative path. + _make_skill(ext, "other", category="a", body="UNRELATED OTHER") + (ext / "a" / "other").rename(ext / "a" / "one") disabled = {"a/one"} monkeypatch.setattr(skill_utils, "get_disabled_skill_names", lambda *a, **k: disabled) monkeypatch.setattr(pb, "get_disabled_skill_names", lambda *a, **k: disabled) - monkeypatch.setattr(skills_tool_module, "_is_skill_disabled", lambda n, platform=None: n in disabled) - assert sorted(s["name"] for s in json.loads(skills_list())["skills"]) == ["b/two", "xdup"] - assert "a/one" not in pb.build_skills_system_prompt() - assert "disabled" in json.loads(skill_view("a/one"))["error"] + monkeypatch.setattr(skills_tool_module, "_is_skill_disabled", lambda *n, platform=None: not disabled.isdisjoint(n)) + monkeypatch.setattr(skills_tool_module, "_SKILLS_CACHE", {}) + pb.clear_skills_system_prompt_cache() + assert sorted(s["name"] for s in json.loads(skills_list())["skills"]) == ["b/two", "other", "xdup"] + index = [ln.strip() for ln in pb.build_skills_system_prompt().splitlines() if ln.startswith(" - ")] + assert index == ["- other: Description for other.", "- b/two: Description for dup-demo.", + "- xdup: Description for xdup."] + for ident in ("a/one", "one"): # refused by any alias, as list/index hide it + assert "disabled" in json.loads(skill_view(ident))["error"] + assert "UNRELATED OTHER" in json.loads(skill_view("other"))["content"] + assert build_preloaded_skills_prompt(["other"])[1:] == (["other"], []) class TestBuildSkillInvocationMessage: diff --git a/tools/skills_tool.py b/tools/skills_tool.py index 3cb4679539..572a859ebb 100644 --- a/tools/skills_tool.py +++ b/tools/skills_tool.py @@ -607,8 +607,11 @@ def skill_view( if not skill_matches_platform(frontmatter): return _fail(f"Skill '{name}' is not supported on this platform.", readiness_status=SkillReadinessStatus.UNSUPPORTED.value) resolved_name = frontmatter.get("name", skill_md.parent.name) - # Disabled by declared name or by the exact path its list row shows (same-tier duplicates). - if _is_skill_disabled(resolved_name) or _is_skill_disabled(_owned_relative(skill_dir, skill_md, all_dirs)): + # Disabled by declared name, or by the exact path that IS this copy's load name (a same-tier + # duplicate's row) — whatever alias reached it; an unrelated copy at that path elsewhere is not. + rel = _owned_relative(skill_dir, skill_md, all_dirs) + if _is_skill_disabled(resolved_name) or (_is_skill_disabled(rel) and any( + e["path"] == skill_md and e["load_name"] == rel for e in _skill_catalog(skip_disabled=True))): return _fail(f"Skill '{resolved_name}' is disabled. Enable it with `hermes skills` or inspect the files directly on disk.") if file_path and skill_dir: return _serve_skill_file(