Skip to content

clean up retired skills on the claude target - #15

Merged
chiruu12 merged 3 commits into
mainfrom
claude-orphans
Aug 24, 2026
Merged

clean up retired skills on the claude target#15
chiruu12 merged 3 commits into
mainfrom
claude-orphans

Conversation

@chiruu12

Copy link
Copy Markdown
Owner

Closes #14

The claude target installs oss-<skill>/SKILL.md, one level deeper than reset_managed handles, so it recorded nothing and cleaned up nothing. A skill retired or renamed upstream stayed in ~/.claude/skills and kept being offered. The other eight targets already do this correctly.

Adds reset_managed_skills and record_managed_skill, which apply the same two proofs one directory deeper: the name has to be in the oss-<skill>/SKILL.md namespace setup owns, and the file has to still be byte for byte what setup wrote. A third check rejects a symlinked oss-<skill>/, which would otherwise put the delete outside the skills directory. The existing functions are untouched, so the eight working targets are not in the blast radius.

An install made before this change carries no manifest. The first run after upgrading adopts it and deletes nothing, so a skill retired before the upgrade stays until you remove it by hand.

The new CI step covers each guard with a case that fails when the guard is removed:

  • reset call absent: the retired skill is left behind
  • checksum proof gone: a retired skill you had edited gets deleted
  • name proof gone: a planted manifest entry deletes a file setup never wrote
  • symlink proof gone: the delete escapes to the symlink target

Verified by mutating setup four ways, one per guard, each failing with its own message, and the clean tree passing. Also checked that your own skills, your own oss- named directories, and your files inside a managed skill directory all survive. All 24 steps of the install job pass locally.

Copilot AI lite review requested due to automatic review settings August 23, 2026 03:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds manifest-based cleanup for retired Claude skills while preserving edited and user-owned files.

  • Restricts managed skill entries to the single-level oss-<skill>/SKILL.md layout.
  • Rejects traversal and symlink escape paths before deletion.
  • Adds CI coverage for retired skills, checksum preservation, namespace confinement, and direct or nested symlink escapes.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the nested manifest path from the previous thread is rejected before its intermediate symlink can reach deletion.

Important Files Changed

Filename Overview
setup Adds Claude-specific managed-skill cleanup and fully addresses the previously reported nested-path symlink bypass.
.github/workflows/setup.yml Adds focused lifecycle and deletion-boundary tests, including a regression test for the previously reported nested symlink escape.

Reviews (2): Last reviewed commit: "reject nested manifest paths" | Re-trigger Greptile

Comment thread setup
@chiruu12
chiruu12 merged commit ade4b2c into main Aug 24, 2026
3 checks passed
@chiruu12
chiruu12 deleted the claude-orphans branch August 24, 2026 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

claude target never removes a retired skill

2 participants