diff --git a/stapler-scripts/llm-sync/src/cli.py b/stapler-scripts/llm-sync/src/cli.py index 2ae59793..2ce911c2 100644 --- a/stapler-scripts/llm-sync/src/cli.py +++ b/stapler-scripts/llm-sync/src/cli.py @@ -113,7 +113,11 @@ def sync_to_target( def get_changed(items: List, item_type: str): changed = [] for item in items: - current_hash = item.get_hash() + current_hash = ( + target.get_sync_hash(item, item_type) + if hasattr(target, "get_sync_hash") + else item.get_hash() + ) last_hash = state_manager.get_hash('to-target', target_name, item_type, item.name) if change_detection is ChangeDetection.ALL or current_hash != last_hash: changed.append(item) @@ -140,7 +144,16 @@ def get_changed(items: List, item_type: str): s_saved = target.save_skills(changed_skills, dry_run=dry_run, force=True) counts.append(f"{s_saved} skills") if mode is not SyncMode.PREVIEW: - for s in changed_skills: state_manager.set_hash('to-target', target_name, 'skills', s.name, s.get_hash()) + for s in changed_skills: + state_manager.set_hash( + 'to-target', target_name, 'skills', s.name, + target.get_sync_hash(s, 'skills') if hasattr(target, "get_sync_hash") else s.get_hash(), + ) + + if hasattr(target, "cleanup_legacy_skill_layout"): + removed = target.cleanup_legacy_skill_layout(skills, dry_run=dry_run) + if removed: + counts.append(f"{removed} legacy skills removed") if changed_commands: c_saved = target.save_commands(changed_commands, dry_run=dry_run, force=True) diff --git a/stapler-scripts/llm-sync/src/targets/pi.py b/stapler-scripts/llm-sync/src/targets/pi.py index bc85e756..6c7f82d5 100644 --- a/stapler-scripts/llm-sync/src/targets/pi.py +++ b/stapler-scripts/llm-sync/src/targets/pi.py @@ -36,8 +36,24 @@ def _short_hash(value: str) -> str: return hashlib.sha256(value.encode("utf-8")).hexdigest()[:8] -def _normalize_skill_name(name: str) -> str: - """Convert a source name to a valid, stable Agent Skills identifier.""" +def _canonical_skill_name(name: str) -> str: + """Remove nested plugin layout directories from a Claude skill identity. + + Claude plugins conventionally store a skill at + ``/skills//SKILL.md``. ``ClaudeSource`` preserves that + on-disk path as (for example) ``sdd/skills/full``. The intermediate + ``skills`` segment is packaging structure, not part of the invocation + name; retaining it makes Pi advertise the less discoverable + ``sdd-skills-full`` instead of ``sdd-full``. + """ + parts = str(name).replace("\\", "/").split("/") + return "/".join( + part for index, part in enumerate(parts) if part != "skills" or index == 0 + ) + + +def _normalize_name(name: str) -> str: + """Convert a name to a valid, stable Agent Skills identifier.""" ascii_name = unicodedata.normalize("NFKD", str(name)).encode( "ascii", "ignore" ).decode("ascii") @@ -54,6 +70,16 @@ def _normalize_skill_name(name: str) -> str: return normalized +def _normalize_skill_name(name: str) -> str: + """Convert a source skill name to its Pi-facing identifier.""" + return _normalize_name(_canonical_skill_name(name)) + + +def _legacy_normalize_skill_name(name: str) -> str: + """Return the Pi identifier emitted before plugin-layout normalization.""" + return _normalize_name(name) + + def _flatten(name: str) -> str: """Flatten a namespaced command name for Pi's non-recursive prompts dir.""" return name.replace("/", "-") @@ -64,6 +90,10 @@ def _description_for_pi(description: str) -> str: class PiTarget(SyncTarget, SyncSource): + # Changing this invalidates only Pi skill state, forcing a safe one-time + # rewrite when the target's naming rules change. + _SKILL_NAMING_VERSION = "2" + def __init__(self, agent_dir: Optional[Path] = None): base = agent_dir or Path.home() / ".pi" / "agent" self.skills_dir = base / "skills" @@ -114,6 +144,13 @@ def _load_md_item(self, item_file: Path, base_dir: Path, cls): console.print(f"[red]Error reading Pi item {item_file}: {e}[/red]") return None + def get_sync_hash(self, item, item_type: str) -> str: + """Version Pi skill state when its output layout changes.""" + item_hash = item.get_hash() + if item_type == "skills": + return f"pi-skill-name-v{self._SKILL_NAMING_VERSION}:{item_hash}" + return item_hash + def _pi_names(self, skills: List[Skill]) -> dict[int, str]: """Resolve normalized names and deterministically disambiguate collisions.""" grouped = defaultdict(list) @@ -196,6 +233,52 @@ def save_skills( return saved_count + def cleanup_legacy_skill_layout( + self, skills: List[Skill], dry_run: bool = False + ) -> int: + """Remove prior generated names after a naming-format migration. + + Only remove a directory when its frontmatter proves it was generated + using the old name, and never when that name remains a current output. + """ + valid_skills = [ + skill for skill in skills if _description_for_pi(skill.description) + ] + pi_names = self._pi_names(valid_skills) + current_names = set(pi_names.values()) + removed = 0 + + for skill in valid_skills: + old_name = _legacy_normalize_skill_name(skill.name) + new_name = pi_names[id(skill)] + if old_name == new_name or old_name in current_names: + continue + + old_dir = self.skills_dir / old_name + old_file = old_dir / "SKILL.md" + new_file = self.skills_dir / new_name / "SKILL.md" + # State may have been updated by a separate --pi-dir run. Never + # delete a legacy output until the canonical replacement exists + # at this target location. + if not old_file.is_file() or not new_file.is_file(): + continue + try: + metadata = yaml.safe_load( + old_file.read_text(encoding="utf-8").split("---", 2)[1] + ) or {} + except (IndexError, OSError, yaml.YAMLError): + continue + if metadata.get("name") != old_name: + continue + + if dry_run: + console.print(f"[blue]Would remove legacy Pi skill {old_dir}[/blue]") + else: + shutil.rmtree(old_dir) + removed += 1 + + return removed + def save_commands( self, commands: List[Command], dry_run: bool = False, force: bool = False ) -> int: diff --git a/stapler-scripts/llm-sync/test_pi_sync.py b/stapler-scripts/llm-sync/test_pi_sync.py index 90751710..de162b4f 100644 --- a/stapler-scripts/llm-sync/test_pi_sync.py +++ b/stapler-scripts/llm-sync/test_pi_sync.py @@ -69,7 +69,7 @@ def test_pi_writes_agent_skills_compatible_names_and_descriptions(): paths = sorted((Path(tmp) / "skills").glob("*/SKILL.md")) assert {path.parent.name for path in paths} == { "academic-cv-builder", - "code-skills-fix-loop", + "code-fix-loop", } for path in paths: metadata = _frontmatter(path) @@ -79,6 +79,42 @@ def test_pi_writes_agent_skills_compatible_names_and_descriptions(): assert metadata["name"] == path.parent.name +def test_pi_omits_plugin_skills_layout_segment_from_names(): + with tempfile.TemporaryDirectory() as tmp: + target = PiTarget(agent_dir=Path(tmp)) + target.save_skills( + [ + Skill( + name="sdd/skills/full", + description="Runs the complete SDD workflow.", + content="# SDD full", + ) + ] + ) + + assert (Path(tmp) / "skills" / "sdd-full" / "SKILL.md").exists() + assert not (Path(tmp) / "skills" / "sdd-skills-full").exists() + + +def test_pi_removes_only_proven_legacy_plugin_layout_output(): + with tempfile.TemporaryDirectory() as tmp: + target = PiTarget(agent_dir=Path(tmp)) + skill = Skill( + name="sdd/skills/full", + description="Runs the complete SDD workflow.", + content="# SDD full", + ) + target.save_skills([skill]) + legacy = Path(tmp) / "skills" / "sdd-skills-full" + legacy.mkdir(parents=True) + (legacy / "SKILL.md").write_text( + "---\nname: sdd-skills-full\ndescription: Old output.\n---\n\n# Old", encoding="utf-8" + ) + + assert target.cleanup_legacy_skill_layout([skill]) == 1 + assert not legacy.exists() + + def test_pi_skips_skills_without_descriptions(): with tempfile.TemporaryDirectory() as tmp: target = PiTarget(agent_dir=Path(tmp))