Skip to content

Add sdd init orphan cleanup of installed legacy skills - #7

Merged
hlubek merged 5 commits into
mainfrom
worktree-init-orphan-cleanup
Sep 5, 2026
Merged

Add sdd init orphan cleanup of installed legacy skills#7
hlubek merged 5 commits into
mainfrom
worktree-init-orphan-cleanup

Conversation

@hlubek

@hlubek hlubek commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends sdd init to discover and remove SDD-stamped installed skill files that are no longer present in the embedded bundle.

  • Adds orphan discovery and ownership/content-hash classification.
  • Preserves modified or future-version files unless forced.
  • Reuses the dropped-agent pruning path and reports orphan cleanup through a new callback and presenter.
  • Adds integration coverage for removal, modification preservation, version downgrades, and foreign skills.
  • The new recursive discovery can make initialization fail on an unreadable foreign Markdown file.

Confidence Score: 4/5

The PR should not merge until orphan discovery no longer aborts initialization because of unreadable files that SDD does not own.

The recursive scan introduces a concrete initialization failure path by reading all foreign Markdown files before ownership classification; the remaining cleanup, downgrade protection, and refactoring behavior appears sound.

Files Needing Attention: internal/finders/skill.go

Important Files Changed

Filename Overview
internal/finders/skill.go Adds recursive orphan discovery, but reads foreign Markdown files before excluding them and therefore propagates unrelated access failures.
internal/handlers/handler_init.go Adds the post-install orphan sweep and consolidates safe removal behavior with existing agent pruning.
internal/model/skill.go Adds stamp-based orphan classification and a semantic-version downgrade guard.
internal/handlers/handler_init_orphan_test.go Covers the principal cleanup and preservation cases, but not unreadable foreign files.
internal/presenters/init.go Adds orphan-removal output and extracts shared modified-file reporting.
internal/command/init.go Adds the orphan-pruning callback and a helper for determining whether a result should be reported.
Prompt To Fix All With AI
### Issue 1
internal/finders/skill.go:87-90
**Foreign files abort initialization**

The orphan scan reads every Markdown file under the skills directory before determining whether SDD owns it. If a user-owned `.md` file is unreadable because of permissions or an I/O error, that error propagates through `SkillStatus` and aborts `sdd init`, even though foreign files are explicitly outside the cleanup operation.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "sdd: summarize 20260905-112554-s-tac-3ja..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

hlubek and others added 4 commits September 4, 2026 14:41
SkillStatus walked bundle entries only, so a file left behind by a removed
bundle source was invisible to the whole install pipeline (s-tac-zaz). It now
also walks the install directory and reports what has no embedded counterpart,
and init sweeps those for every rendered agent after the install pass.

The stamp is the ownership marker: a file with no sdd-content-hash is a skill
of the user's own sharing the directory, so it is neither removed nor
reported. A stamped orphan still matching its stamp is removed; an edited one
is preserved and named, and goes only under --force — the same rule
pruneAgentSkills already applies to a dropped agent's render.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The orphan sweep arrived as a near-copy of pruneAgentSkills. Both now feed
removePrunable, which owns the shared rule — unmodified files go, edited ones
are preserved and named, --force takes those too, emptied directories are
cleaned up. Each caller keeps only what differs: which files it offers, and
that a dropped agent also takes its parent skills dir.

SkillStampIsAhead stops an older binary pruning what a newer one installed:
absence from the running bundle is not evidence a later sdd never shipped it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…se bundle source is ...

SDD-Mutation: entry-20260905-112554-s-tac-3ja
SDD-Mutation: summary-20260905-112554-s-tac-3ja-416463235f34b340
Comment thread internal/finders/skill.go
The sweep read every Markdown file under the install directory to decide
ownership, so one it could not read — a foreign file closed by permissions, a
directory it may not enter — failed SkillStatus and took the whole init down,
including the install pass that calls it first.

Ownership is what the read establishes, and establishing it is the only thing
that leads to deletion, so a path that cannot be read is passed over: never
deletes more, at worst leaves an orphan for a later run. Files already known to
be sdd's are read by the bundle-entry loop, where a failure still stops
everything.

Reported by Greptile on #7.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@hlubek
hlubek merged commit 7c8be4b into main Sep 5, 2026
3 checks passed
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.

1 participant