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
45 changes: 39 additions & 6 deletions src/specify_cli/presets/_manager_skills.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ def _reconcile_skills(
# directory-independent, so it's computed once and then applied to
# every affected directory below.
preset_cmds: Dict[str, List[str]] = {}
composed_winners: Dict[str, str] = {}
non_preset_skills: List[tuple] = []
managed_skill_names: set = set()
reconciled_skill_commands: set[str] = set()
Expand Down Expand Up @@ -195,6 +196,8 @@ def _reconcile_skills(
pack_dir = self.presets_dir / pack_id
if top_path.is_relative_to(pack_dir):
preset_cmds.setdefault(pack_id, []).append(cmd_name)
if layers[0]["strategy"] != "replace":
composed_winners[cmd_name] = pack_id
found_preset = True
break
if not found_preset:
Expand Down Expand Up @@ -394,16 +397,21 @@ def apply_to_dir(
extra_dirs = extra_skills_dirs or {}
if active_skills_dir:
active_provenance = extra_dirs.get(active_skills_dir)
if extra_skills_dirs is None or active_provenance:
active_names: Optional[Set[str]] = None
if extra_skills_dirs is not None:
# Removal restores only skills the removed preset still owned,
# but a surviving preset composing over a removed lower layer
# owns its skill: recompose it while it keeps that marker.
active_names = set(active_provenance[1]) if active_provenance else set()
active_names |= self._skills_owned_by_preset(
active_skills_dir, composed_winners
)
if active_names is None or active_names:
apply_to_dir(
active_skills_dir,
active_ai,
is_active=True,
managed_names=(
set(active_provenance[1])
if active_provenance
else None
),
managed_names=active_names,
)

for extra_dir, (extra_agent, extra_names) in extra_dirs.items():
Expand All @@ -418,6 +426,31 @@ def apply_to_dir(

return reconciled_skill_commands

def _skills_owned_by_preset(
self, skills_dir: Path, owners: Dict[str, str]
) -> Set[str]:
"""Return skills in ``skills_dir`` whose source marker names their preset.

``owners`` maps command names to the expected owning preset id.
"""
registrar = self._command_registrar()
owned: Set[str] = set()
for cmd_name, pack_id in owners.items():
for skill_name in self._skill_names_for_command(cmd_name):
skill_file = skills_dir / skill_name / "SKILL.md"
if not self._validate_skill_subdir(
skill_file.parent, create=False, skills_root=skills_dir
) or not skill_file.is_file():
continue
try:
content = skill_file.read_text(encoding="utf-8")
except (OSError, UnicodeDecodeError):
continue
metadata = registrar.parse_frontmatter(content)[0].get("metadata")
if isinstance(metadata, dict) and metadata.get("source") == f"preset:{pack_id}":
owned.add(skill_name)
return owned

def _resolve_agent_skills_dir(self, agent_name: str) -> Path:
"""Resolve the real skill output directory for an integration."""
from .. import _get_skills_dir as _project_skills_dir
Expand Down
91 changes: 91 additions & 0 deletions tests/specify_cli/presets/test_manager_skills.py
Original file line number Diff line number Diff line change
Expand Up @@ -1897,6 +1897,97 @@ def test_remove_preserves_non_owned_skill_during_reconciliation(
"---\nname: speckit-specify\n---\n\nUser-owned body\n"
)

def _install_composed_specify_stack(self, project_dir, temp_dir, layers):
"""Install ``(preset_id, body, strategy, priority)`` layers for speckit.specify."""
self._write_init_options(project_dir, ai="claude", ai_skills=True)
self._create_skill(project_dir / ".claude" / "skills", "speckit-specify")
(project_dir / ".specify" / "templates" / "commands" / "specify.md").write_text(
"---\ndescription: Core specify\n---\n\nCore specify body\n",
encoding="utf-8",
)
manager = PresetManager(project_dir)
for preset_id, body, strategy, priority in layers:
preset_dir = self._create_command_preset(
temp_dir, preset_id, "speckit.specify", preset_id, body
)
if strategy != "replace":
manifest_path = preset_dir / "preset.yml"
manifest = yaml.safe_load(manifest_path.read_text())
manifest["provides"]["templates"][0]["strategy"] = strategy
manifest_path.write_text(yaml.dump(manifest))
manager.install_from_directory(preset_dir, "0.1.5", priority=priority)
return manager, project_dir / ".claude" / "skills" / "speckit-specify" / "SKILL.md"

def test_remove_base_preset_recomposes_surviving_composed_skill(
self, project_dir, temp_dir
):
"""Removing a composition's base layer must recompose the surviving skill.

The appending preset owns the composed SKILL.md, so removing the
replace preset beneath it restored nothing in the active skills
directory and reconciliation skipped it: the skill kept the removed
preset's body while command-file agents were recomposed onto core.
"""
manager, skill_file = self._install_composed_specify_stack(
project_dir,
temp_dir,
[
("base-preset", "Base preset body", "replace", 10),
("append-preset", "Appended body", "append", 5),
],
)
content = skill_file.read_text(encoding="utf-8")
assert "Base preset body" in content and "Appended body" in content

assert manager.remove("base-preset") is True

content = skill_file.read_text(encoding="utf-8")
assert "Base preset body" not in content
assert "Core specify body" in content
assert "Appended body" in content
assert "preset:append-preset" in content

def test_remove_middle_preset_drops_its_layer_from_composed_skill(
self, project_dir, temp_dir
):
"""Removing a middle composing layer must drop its fragment from the skill."""
manager, skill_file = self._install_composed_specify_stack(
project_dir,
temp_dir,
[
("base-preset", "Base preset body", "replace", 10),
("middle-preset", "Middle body", "append", 7),
("top-preset", "Top body", "append", 5),
],
)
assert "Middle body" in skill_file.read_text(encoding="utf-8")

assert manager.remove("middle-preset") is True

content = skill_file.read_text(encoding="utf-8")
assert "Middle body" not in content
assert "Base preset body" in content
assert "Top body" in content

def test_remove_base_preset_preserves_hand_edited_composed_skill(
self, project_dir, temp_dir
):
"""Recomposition after removal must not overwrite a hand-edited skill."""
manager, skill_file = self._install_composed_specify_stack(
project_dir,
temp_dir,
[
("base-preset", "Base preset body", "replace", 10),
("append-preset", "Appended body", "append", 5),
],
)
edited = "---\nname: speckit-specify\n---\n\nUser-owned body\n"
skill_file.write_text(edited, encoding="utf-8")

assert manager.remove("base-preset") is True

assert skill_file.read_text(encoding="utf-8") == edited

def test_shared_skills_dir_restored_once_using_active_agent(
self, project_dir, temp_dir
):
Expand Down