Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion skillopt_sleep/cycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -627,7 +638,7 @@ def _skill_proposals_from_groups(
)
continue
candidate = SkillProposal(
name,
proposal_name,
new_skill,
live_path,
live_sha256=live_sha256,
Expand Down
52 changes: 50 additions & 2 deletions skillopt_sleep/skill_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/<marketplace>/<plugin>/<version>/skills`` and a
legacy unversioned shape ``.../plugins/cache/<marketplace>/<plugin>/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)
Expand Down
50 changes: 50 additions & 0 deletions tests/test_sleep_adopt_skill_subset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 53 additions & 0 deletions tests/test_sleep_skill_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down