From c09ac9c11028936d2416eac9321b61a229e12069 Mon Sep 17 00:00:00 2001 From: Aaravkhanal Date: Thu, 29 Oct 2026 17:51:35 +0530 Subject: [PATCH] fix(sleep): resolve Claude plugin-qualified skills --- skillopt_sleep/cycle.py | 13 ++++++- skillopt_sleep/skill_resolver.py | 52 ++++++++++++++++++++++++- tests/test_sleep_adopt_skill_subset.py | 50 ++++++++++++++++++++++++ tests/test_sleep_skill_resolver.py | 53 ++++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 3 deletions(-) diff --git a/skillopt_sleep/cycle.py b/skillopt_sleep/cycle.py index 42a663628..762607261 100644 --- a/skillopt_sleep/cycle.py +++ b/skillopt_sleep/cycle.py @@ -598,6 +598,16 @@ def _skill_proposals_from_groups( if not live_path: notes.append(_cycle_skip_note(name, "no resolved live baseline")) continue + # Plugin calls are recorded as ``plugin:skill`` but proposals + # target the actual skill directory and must use its portable + # basename (the colon is not a valid staged filename on Windows). + # Keep ordinary names unchanged so symlink aliases retain their + # existing refusal behavior during staging. + proposal_name = ( + os.path.basename(os.path.dirname(live_path)) + if ":" in name + else name + ) live_sha256 = ( resolved_hashes.get(name, "") if resolved_hashes is not None @@ -614,6 +624,7 @@ def _skill_proposals_from_groups( ) continue live_path = resolution.path + proposal_name = resolution.name live_sha256 = None live_key = unicodedata.normalize( "NFC", os.path.realpath(live_path) @@ -627,7 +638,7 @@ def _skill_proposals_from_groups( ) continue candidate = SkillProposal( - name, + proposal_name, new_skill, live_path, live_sha256=live_sha256, diff --git a/skillopt_sleep/skill_resolver.py b/skillopt_sleep/skill_resolver.py index e9cedd323..53590e83c 100644 --- a/skillopt_sleep/skill_resolver.py +++ b/skillopt_sleep/skill_resolver.py @@ -210,18 +210,66 @@ def _contained_skill_file(root: str, name: str) -> str: return skill_file +def _plugin_name_for_root(root: str) -> str: + """Return the Claude plugin name for an installed plugin skills root. + + Installed Claude plugin roots have the versioned shape + ``.../plugins/cache////skills`` and a + legacy unversioned shape ``.../plugins/cache///skills``. + Requiring one of those shapes prevents an explicit user skill root from + accidentally being treated as a plugin root just because its directory + happens to be named like one. + """ + parts = os.path.normpath(os.path.realpath(root)).split(os.sep) + if ( + len(parts) >= 8 + and parts[-1] == "skills" + and parts[-6:-4] == ["plugins", "cache"] + ): + return parts[-3] + if ( + len(parts) >= 7 + and parts[-1] == "skills" + and parts[-5:-3] == ["plugins", "cache"] + ): + return parts[-2] + return "" + + def resolve_skill(name: object, roots: Sequence[str]) -> SkillResolution: """Resolve ``name`` against ``roots`` without touching any skill content.""" - normalized = normalize_skill_name(name) + original = name.strip() if isinstance(name, str) else "" + plugin_name = "" + if ":" in original: + if original.count(":") != 1: + return SkillResolution( + name=original, + status=REJECTED, + reason="plugin-qualified skill name must contain one safe plugin and skill name", + ) + raw_plugin, raw_skill = original.split(":", 1) + plugin_name = normalize_skill_name(raw_plugin) + normalized = normalize_skill_name(raw_skill) + if not plugin_name or not normalized: + normalized = "" + else: + normalized = normalize_skill_name(name) + if not normalized: return SkillResolution( name=name if isinstance(name, str) else "", status=REJECTED, - reason="skill name is empty or not a single safe path segment", + reason=( + "plugin-qualified skill name must contain one safe plugin and skill name" + if plugin_name or ":" in original + else "skill name is empty or not a single safe path segment" + ), ) matches: List[str] = [] for root in roots: + if plugin_name and _plugin_name_for_root(root) != plugin_name: + continue found = _contained_skill_file(root, normalized) if found and found not in matches: matches.append(found) diff --git a/tests/test_sleep_adopt_skill_subset.py b/tests/test_sleep_adopt_skill_subset.py index f6c32af56..d95f8e94e 100644 --- a/tests/test_sleep_adopt_skill_subset.py +++ b/tests/test_sleep_adopt_skill_subset.py @@ -1203,6 +1203,56 @@ def test_skips_managed_catch_all_and_unresolved_names(self): self.assertTrue(any("ghost-skill" in note for note in notes)) self.assertFalse(any("skillopt-sleep-learned" in note for note in notes)) + def test_plugin_qualified_group_stages_under_the_live_directory_name(self): + from skillopt_sleep.config import load_config + from skillopt_sleep.cycle import _skill_proposals_from_groups + + with tempfile.TemporaryDirectory() as home: + claude_home = os.path.join(home, ".claude") + live = os.path.join( + claude_home, "plugins", "cache", "market", "toolbox", "0.6.4", + "skills", "doc-truth", "SKILL.md", + ) + _write(live, "# original\n") + cfg = load_config(claude_home=claude_home) + + proposals, notes = _skill_proposals_from_groups( + cfg, + {"toolbox:doc-truth": _accepted_group( + "toolbox:doc-truth", "# improved\n")}, + "skillopt-sleep-learned", + ) + + self.assertEqual(notes, []) + self.assertEqual(len(proposals), 1) + self.assertEqual(proposals[0].skill_name, "doc-truth") + self.assertEqual(proposals[0].live_skill_path, os.path.realpath(live)) + + def test_pinned_plugin_path_stages_under_directory_name(self): + from skillopt_sleep.config import load_config + from skillopt_sleep.cycle import _skill_proposals_from_groups + + with tempfile.TemporaryDirectory() as home: + claude_home = os.path.join(home, ".claude") + live = os.path.join( + claude_home, "plugins", "cache", "market", "toolbox", "0.6.4", + "skills", "doc-truth", "SKILL.md", + ) + _write(live, "# original\n") + cfg = load_config(claude_home=claude_home) + + proposals, notes = _skill_proposals_from_groups( + cfg, + {"toolbox:doc-truth": _accepted_group( + "toolbox:doc-truth", "# improved\n")}, + "skillopt-sleep-learned", + resolved_paths={"toolbox:doc-truth": os.path.realpath(live)}, + resolved_hashes={"toolbox:doc-truth": "known-hash"}, + ) + + self.assertEqual(notes, []) + self.assertEqual([proposal.skill_name for proposal in proposals], ["doc-truth"]) + def test_skips_a_second_skill_that_resolves_to_the_same_live_file(self): from skillopt_sleep.config import load_config from skillopt_sleep.cycle import _skill_proposals_from_groups diff --git a/tests/test_sleep_skill_resolver.py b/tests/test_sleep_skill_resolver.py index 423c50d8f..944b343c9 100644 --- a/tests/test_sleep_skill_resolver.py +++ b/tests/test_sleep_skill_resolver.py @@ -105,6 +105,59 @@ def test_traversal_is_rejected_without_touching_the_filesystem(self): self.assertEqual(res.status, REJECTED) self.assertEqual(res.path, "") + def test_plugin_qualified_name_only_resolves_in_that_plugins_root(self): + with tempfile.TemporaryDirectory() as tmp: + user = os.path.join(tmp, "claude", "skills") + wanted = os.path.join( + tmp, "claude", "plugins", "cache", "market", "toolbox", "0.6.4", "skills" + ) + other = os.path.join( + tmp, "claude", "plugins", "cache", "market", "other", "0.6.4", "skills" + ) + _write_skill(user, "doc-truth") + expected = _write_skill(wanted, "doc-truth") + _write_skill(other, "doc-truth") + + result = resolve_skill("toolbox:doc-truth", [user, wanted, other]) + + self.assertEqual(result.status, FOUND) + self.assertEqual(result.name, "doc-truth") + self.assertEqual(result.path, os.path.realpath(expected)) + + def test_legacy_plugin_root_resolves_qualified_name(self): + with tempfile.TemporaryDirectory() as tmp: + root = os.path.join( + tmp, "claude", "plugins", "cache", "market", "toolbox", "skills" + ) + expected = _write_skill(root, "doc-truth") + + result = resolve_skill("toolbox:doc-truth", [root]) + + self.assertEqual(result.status, FOUND) + self.assertEqual(result.path, os.path.realpath(expected)) + + def test_plugin_qualified_name_across_marketplaces_remains_ambiguous(self): + with tempfile.TemporaryDirectory() as tmp: + first = os.path.join( + tmp, "claude", "plugins", "cache", "market-a", "toolbox", "1", "skills" + ) + second = os.path.join( + tmp, "claude", "plugins", "cache", "market-b", "toolbox", "1", "skills" + ) + _write_skill(first, "doc-truth") + _write_skill(second, "doc-truth") + + result = resolve_skill("toolbox:doc-truth", [first, second]) + + self.assertEqual(result.status, AMBIGUOUS) + self.assertEqual(len(result.candidates), 2) + + def test_malformed_plugin_qualified_names_are_rejected(self): + with tempfile.TemporaryDirectory() as tmp: + for name in (":doc-truth", "toolbox:", "toolbox:../outside", "toolbox:doc:truth"): + with self.subTest(name=name): + self.assertEqual(resolve_skill(name, [tmp]).status, REJECTED) + def test_symlinked_skill_dir_escaping_the_root_is_refused(self): with tempfile.TemporaryDirectory() as tmp: root = os.path.join(tmp, "roots")