fix: scope stale cleanup to files this sync installed (data loss in ~/.claude/skills) - #902
Open
S-Keilani wants to merge 1 commit into
Open
Conversation
The deferred stale cleanup walks the whole destination and unlinks every file
absent from the current source set. Destinations are shared: ~/.claude/skills
also holds skills installed by plugins, marketplaces and other repos. Those
are absent from this repo's source set, so the first sync deletes them.
Reproduction (before this change):
repo/ skills/tk-skill/SKILL.md
~/.claude/ skills/cloudflare/SKILL.md <- installed elsewhere
skills/cloudflare/references/api.md
After one sync both cloudflare files are gone.
On the machine where this was found it removed 412 files across 11 skill
directories — cloudflare (321), turnstile-spin, agents-sdk, use-railway,
durable-objects, wrangler, workers-best-practices, cloudflare-email-service,
sandbox-sdk, geo-audit, web-perf. All were live, in-use skills. The existing
_resolves_inside guard does not help: foreign files do not resolve inside the
repo, so they fall straight through to unlink().
Scope pruning by an ownership manifest at ~/.claude/.sync-manifest.json. A
file is stale only if a previous run of this sync installed it and the source
no longer provides it. Files this sync never installed are never touched.
With no manifest present nothing is pruned and the manifest is seeded, so
genuine stale cleanup resumes from the second run. The _resolves_inside guard
is preserved.
Verified against the fixture above: unpatched deletes both foreign files on
run 1; patched preserves them on runs 1 and 2 while still pruning a renamed
toolkit skill on run 2.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The deferred stale cleanup in
hooks/sync-to-user-claude.pywalks the whole destination and unlinks every file absent from the current source set:Destinations are shared.
~/.claude/skillsalso holds skills installed by plugins, marketplaces and other repos. Those are absent from this repo's source set, so the first sync deletes them.The existing
_resolves_insideguard doesn't help — foreign files don't resolve inside the repo, so they fall straight through tounlink().Reproduction
One sync, and both
cloudflarefiles are gone.On the machine where I hit this, a first sync would have removed 412 files across 11 skill directories:
cloudflare(321 files),turnstile-spin,agents-sdk,use-railway,durable-objects,wrangler,workers-best-practices,cloudflare-email-service,sandbox-sdk,geo-audit,web-perf. All were live, in-use skills. I caught it before running the sync, not after.Fix
Scope pruning by an ownership manifest at
~/.claude/.sync-manifest.json. A file is stale only if a previous run of this sync installed it and the source no longer provides it. Files this sync never installed are never touched.With no manifest present, nothing is pruned and the manifest is seeded — so genuine stale cleanup (the phantom-hook protection this feature exists for) resumes from the second run. That one-run delay is the tradeoff for not being able to distinguish ours-from-theirs on a first pass.
_resolves_insideis preserved.Verification
Against the fixture above, on this branch:
Run 2 logs the prune, so the feature still visibly works.
Notes
Found while auditing this toolkit on a Windows install. Two other issues I ran into turned out to be already fixed or obsolete upstream, so this is the only one I'm sending:
lib/stdin_timeout.pySIGALRM— you've since added aplatform.system() == "Windows"branch. Worth noting that branch drops the timeout guard entirely on Windows; a daemon-thread reader would keep it on both platforms. Happy to send that separately if you want it.install.shhook-block shape — that script has been rewritten since; not applicable.