feat: add --prune to remove orphaned skills on update

[EYH0602]

Problem

When an upstream tap renames or deprecates a skill, the local installed copy is left stale. skillshub tap update only printed an advisory ("run `skillshub uninstall` manually"), and skillshub update skipped the skill with "(not in registry)" — neither removed anything. A rename is the worst case: the old skill stays installed + linked while its replacement is never installed, so the user silently keeps a deprecated skill.

Change

Add an opt-in --prune flag to both update commands:

• skillshub tap update --prune — after refreshing the tap, uninstall installed skills no longer present upstream (removed_installed).
• skillshub update --prune — uninstall installed skills that are gone upstream (missing from the registry, or from a gist source).

Pruning removes the install dir, the db.installed entry, and the now-empty tap dir. Default behavior is unchanged; without --prune the advisory now also hints at the flag.

Key implementation detail

uninstall_skill opens its own DB and saves it. The update flows hold their own in-memory Database and save once at the end, so calling uninstall_skill from inside them would let that final save_db resurrect the removed entry. To avoid this, the file/entry removal is extracted into remove_installed_skill_files(db, install_dir, skill_id) — an in-memory helper that mutates the caller's db without init/save. Both uninstall_skill and the prune paths use it, so persistence happens exactly once per command.

Scope

• Symlinks: prune reuses existing uninstall semantics (files + db entry). Dangling agent symlinks left by both uninstall and --prune are tracked as a follow-up in Prune/uninstall leave dangling agent symlinks behind #76.
• No tap manifest declares renames (skills are discovered by walking SKILL.md), so prune treats a rename as delete-old + add-new rather than following it.

Tests & verification

• Two new tests in src/registry/tap.rs: prune removes only the targeted orphan (siblings intact), and prune cleans up an emptied tap dir.
• Full suite green (196 lib + integration), cargo clippy --all-targets clean, --prune verified in both --help outputs.

Docs

• README.md, docs/cli-reference.md, CLAUDE.md updated with the new flag.

🤖 Generated with Claude Code

[fenfenai]

Review Summary

Reviewed 8 files (+340/-20) across 6 review aspects (bug detection, error handling, type design, test coverage, comment quality, guidelines compliance). Core mechanics are sound — the helper extraction correctly avoids the db-resurrection trap, and iterating over cloned key Vecs while mutating db.installed is safe. Two functional defects in the prune logic and one workflow violation need attention before merge.

Critical Issues (confidence 90-100)

None.

Important Issues (confidence 75-89)

1. src/registry/tap.rs:296-310 — the tap update --prune re-run path silently no-ops (confidence: 85). removed_installed is a diff against the pre-update cache, which the same run overwrites; the advisory's own suggestion ("re-run with --prune") then finds an empty diff.
2. src/registry/skill.rs:579-611 — update --prune decides against a cache-only registry (confidence: 80). Stale cache → misses real removals (falls through to a confusing "Skill path not found in local clone" copy error) or can prune a skill that currently exists upstream.
3. plans/prune-orphaned-skills.md — plan shipped as-is instead of rewritten into docs/ (confidence: 88). Explicit AGENTS.md workflow rule; the merged plan also still presents already-resolved decisions as open questions.
4. Test gap: prune flag wiring untested (confidence: 78). Only the extracted helper is tested; the prune branches in update_tap/update_skill (tap, registry, and gist paths) — where findings 1 and 2 live — have no coverage, though the plan promised exactly those tests.
5. src/cli.rs:41 — update --prune help text omits the gist case (confidence: 76). The code prunes gist-sourced skills too; the doc says only "in their tap".

Positive Observations

• The remove_installed_skill_files extraction is the right fix for the db-resurrection hazard, and its doc comment explains the caller-owns-persistence contract precisely.
• Both new tests are high quality: TestHomeGuard + #[serial] + TempDir, deterministic, offline, behavior-focused.
• Default behavior is unchanged; --prune is strictly opt-in, matching the plan's non-destructive-by-default design.
• README, CLAUDE.md, and cli-reference docs all updated per repo rules.

Verdict: Request changes — findings 1 and 2 undermine the feature's core promise in exactly the scenarios it targets.

[fenfenai]

Re-Review: All previous findings addressed

Resolved (5 threads)

• src/registry/tap.rs — baseline-diff no-op ✓ Fixed. Prune set now comes from orphaned_installed_skills (membership against current_skills from the fresh registry), independent of the diff baseline; the "re-run with --prune" hint now works after a non-prune update. Covered by test_orphaned_installed_skills_detected_after_cache_refresh.
• src/registry/skill.rs — cache-only prune decision ✓ Fixed. Clone-backed taps decide membership against the freshly-pulled clone via discover_skills_from_local after pull_or_reclone (never the stale cache); default/gist taps decide from their authoritative caches; the cache is refreshed after each scan. Both failure directions (missed removal → cryptic copy error; pruning a skill that exists upstream) are eliminated.
• plans/prune-orphaned-skills.md ✓ Fixed. Plan removed from plans/; rewritten as docs/prune-orphaned-skills.md per the repo workflow.
• Test coverage ✓ Addressed. Four orphan-membership tests plus test_prune_or_report_missing_{prunes_when_flag_set,keeps_when_flag_unset} cover the previously flagged defect scenarios.
• src/cli.rs help text ✓ Fixed. Now reads "no longer exist upstream (tap or gist)".

Verification

• Checked out PR head (4a46a5c) in a worktree and ran the full suite: 290 passed, 0 failed (202 bin + 88 integration).
• Read the reworked update_tap/update_skill/update_single_tap/orphaned_installed_skills/prune_or_report_missing paths; no new findings at reportable confidence. One sub-threshold nit (not blocking): after update --prune prunes a clone-backed skill, the tap's cached registry is only refreshed on the skill-found path, so list can show a stale available entry until the next tap update.

Approving.