Add sdd init orphan cleanup of installed legacy skills

.sdd/graph/2026/09/05-112554-s-tac-3ja.md

@@ -0,0 +1,32 @@
+---
+type: signal
+layer: tactical
+kind: done
+refs:
+    - id: 20260821-123832-d-tac-ip1
+      kind: addresses
+      desc: delivers its orphan-cleanup criterion; the removal, rename, surface sweep, parity verdict, kind-capture coverage and release notes stay open
+    - id: 20260614-182310-s-tac-wqq
+      kind: builds-on
+      desc: the dropped-agent prune whose content-hash safety rule this extends to orphans within a still-rendered agent
+closes:
+    - 20260507-174656-s-tac-zaz
+participants:
+    - Christopher
+confidence: high
+topics:
+    - implementation/cli
+summary: '`sdd init` now detects and removes installed skill files whose bundle source is gone, extending the content-hash safety rule—preserve edited files, remove stamped matches, `--force` overrides—to orphans within a still-rendered agent, with the install stamp as the ownership marker that keeps skills sdd never wrote out of the sweep and a version guard that spares files stamped by a later release. It delivers the orphan-cleanup criterion of the retirement plan (20260821-123832-d-tac-ip1), leaving that plan''s other criteria open, and folds its own sweep together with the dropped-agent prune into one path holding the shared rule (20260614-182310-s-tac-wqq). It closes the gap signal (20260507-174656-s-tac-zaz).'
+---
+
+`sdd init` now removes installed skill files whose bundle source is gone, so an upgrade no longer leaves stale copies behind (commits 50387893 and cbc94f52).
+
+The read side was the hole: `SkillStatus` walked bundle entries only, so a file on disk with no embedded counterpart was invisible to the whole install pipeline. It now also walks the install directory and reports what has no bundle source, and init sweeps those for every rendered agent after the install pass — by the same safety rule the dropped-agent prune already applies to a whole render (20260614-182310-s-tac-wqq): a file still matching its install stamp is removed, an edited one is preserved and named, and `--force` removes that too. The sweep first landed as a near-copy of that prune; both now run through one path holding the rule, each caller supplying only the files it offers and what else its own case takes.
+
+One rule the gap did not state had to be settled in the building. The install directory also holds skills sdd never wrote, so the stamp became the ownership marker: a file carrying none is not an orphan at all — left untouched, and not reported as something to resolve. A test holds that under `--force`, where getting it wrong would delete a user's own skill during an upgrade. Because the stamp is written into the installed file rather than read from the bundle, recognising sdd's own work needs no record of what older versions shipped — the question raised in dialogue when the sweep was weighed against a one-time list of retired paths. One direction still needed a guard: a stamp naming a release ahead of the running binary is left alone, since absence from an older bundle is no evidence a newer sdd never shipped the file.
+
+Beyond the tests, the built binary was run against scratch projects: a tracked orphan is removed, its deletion committed, and the working tree is clean afterwards.
+
+That run also surfaced a pre-existing condition the sweep inherits: an orphan that was never committed is removed, and the auto-commit then fails on it. Ruled out of scope in dialogue, since init commits what it installs and a genuine orphan is therefore tracked.
+
+This is the orphan-cleanup criterion of the retirement plan (20260821-123832-d-tac-ip1) and closes the gap that plan adopted its rule from. Its remaining criteria are untouched.

cmd/sdd/main.go

@@ -1617,6 +1617,9 @@ func initCmd() *cli.Command {
 				OnAgentSkillsPruned: func(result command.AgentPruneResult) {
 					presenters.RenderInitPrune(os.Stdout, result)
 				},
+				OnSkillOrphansPruned: func(result command.AgentPruneResult) {
+					presenters.RenderInitOrphans(os.Stdout, result)
+				},
 				OnIndexMigrated: func(legacyDir, storeDir string, moved bool) {
 					if moved {
 						fmt.Printf("  index migrated: %s → %s\n", legacyDir, storeDir)

internal/command/init.go

@@ -159,23 +159,36 @@ type InitCmd struct {
 	// files removed and any user-modified files preserved.
 	OnAgentSkillsPruned func(result AgentPruneResult)
 
+	// OnSkillOrphansPruned fires once per rendered agent whose install
+	// directory held files the bundle no longer carries, reporting what was
+	// removed and which user-modified copies were preserved. Does not fire
+	// when the sweep found nothing.
+	OnSkillOrphansPruned func(result AgentPruneResult)
+
 	// OnMCPRegistered fires for each project-scope config file written to
 	// register the SDD MCP server for an agent (a fresh file or an
 	// add-if-missing merge). Does not fire when an existing sdd entry is
 	// left untouched.
 	OnMCPRegistered func(target model.AgentTarget, path string)
 }
 
-// AgentPruneResult reports the outcome of pruning a dropped agent's rendered
-// skills: which files were removed and which user-modified files were kept
-// (removable only under --force).
+// AgentPruneResult reports the outcome of a prune pass over one agent's
+// install directory — a dropped agent's whole render, or the orphans a
+// still-rendered agent's bundle no longer carries: which files were removed
+// and which user-modified files were kept (removable only under --force).
 type AgentPruneResult struct {
 	Target       model.AgentTarget
 	InstallDir   string
 	Removed      []string
 	KeptModified []string
 }
 
+// TouchedAnything reports whether the pass found something worth telling the
+// user about — a prune that matched nothing stays silent.
+func (r AgentPruneResult) TouchedAnything() bool {
+	return len(r.Removed) > 0 || len(r.KeptModified) > 0
+}
+
 // Validate checks required fields.
 func (c *InitCmd) Validate() error {
 	if c.RepoRoot == "" {

internal/finders/skill.go

@@ -7,6 +7,7 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/networkteam/sdd/internal/bundledskills"
 	"github.com/networkteam/sdd/internal/model"
@@ -41,8 +42,10 @@ func (f *Finder) SkillStatus(ctx context.Context, q query.SkillStatusQuery) (*qu
 		InstallDir: installDir,
 		Entries:    make([]query.SkillStatusEntry, 0, len(bundle.Entries)),
 	}
+	fromBundle := make(map[string]bool, len(bundle.Entries))
 	for _, e := range bundle.Entries {
 		abs := filepath.Join(installDir, e.Skill, e.RelPath)
+		fromBundle[abs] = true
 		installed, err := readSkillFile(abs)
 		if err != nil {
 			return nil, fmt.Errorf("reading installed skill %s: %w", abs, err)
@@ -57,9 +60,70 @@ func (f *Finder) SkillStatus(ctx context.Context, q query.SkillStatusQuery) (*qu
 			Installed: installed,
 		})
 	}
+
+	orphans, err := findSkillOrphans(installDir, fromBundle)
+	if err != nil {
+		return nil, err
+	}
+	result.Orphans = orphans
 	return result, nil
 }
 
+// findSkillOrphans walks the install directory for skill files the bundle no
+// longer carries. Files sdd never wrote carry no stamp and are dropped here,
+// so they never reach a caller that removes things.
+//
+// The directory holds other people's files, so a path this walk cannot read is
+// a path whose ownership cannot be established — and establishing ownership is
+// the only thing that leads to deletion. Such a path is passed over rather than
+// failing the run, which never deletes more and at worst leaves an orphan for a
+// later init. Files already known to be sdd's are read by the bundle-entry loop
+// above, where a read failure still stops everything.
+func findSkillOrphans(installDir string, fromBundle map[string]bool) ([]query.SkillOrphanEntry, error) {
+	var orphans []query.SkillOrphanEntry
+	err := filepath.WalkDir(installDir, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			if path == installDir {
+				// Nothing installed for this target yet, or the directory is
+				// closed to us: there are no orphans to find either way.
+				return fs.SkipAll
+			}
+			if d != nil && d.IsDir() {
+				return fs.SkipDir
+			}
+			return nil
+		}
+		if d.IsDir() || filepath.Ext(path) != ".md" || fromBundle[path] {
+			return nil
+		}
+		installed, err := readSkillFile(path)
+		if err != nil {
+			return nil
+		}
+		class := model.ClassifySkillOrphan(installed)
+		if class == model.SkillOrphanForeign {
+			return nil
+		}
+		rel, err := filepath.Rel(installDir, path)
+		if err != nil {
+			return fmt.Errorf("resolving %s against %s: %w", path, installDir, err)
+		}
+		skill, relPath, _ := strings.Cut(filepath.ToSlash(rel), "/")
+		orphans = append(orphans, query.SkillOrphanEntry{
+			Skill:     skill,
+			RelPath:   relPath,
+			AbsPath:   path,
+			Class:     class,
+			Installed: installed,
+		})
+		return nil
+	})
+	if err != nil {
+		return nil, fmt.Errorf("scanning %s for orphaned skill files: %w", installDir, err)
+	}
+	return orphans, nil
+}
+
 // readSkillFile returns a parsed SkillFile for path, or nil if the file does
 // not exist. Any other error is returned so callers can distinguish real
 // read failures from missing files.

internal/handlers/handler_init.go

@@ -398,6 +398,12 @@ func (h *Handler) Init(ctx context.Context, cmd *command.InitCmd) error {
 		}
 	}
 
+	// Sweep out what the bundle stopped shipping, after the install pass so
+	// this run's own writes are on disk and stamped (s-tac-zaz).
+	if err := h.pruneSkillOrphans(ctx, effectiveAgents, effectiveScope, cmd, &touched); err != nil {
+		return err
+	}
+
 	// Transition messaging (d-tac-o2v): reach users who upgrade without
 	// invoking the skill.
 	log.Info("the /sdd skill family is deprecated — work in /sdd-engine; v0.18.0 will remove the legacy skills and rename /sdd-engine to /sdd")
@@ -615,43 +621,110 @@ func (h *Handler) pruneAgentSkills(ctx context.Context, dropped []model.AgentTar
 			return fmt.Errorf("classifying %s skills for prune: %w", target, err)
 		}
 
-		var removed, keptModified []string
-		skillDirs := map[string]bool{}
+		var files []prunable
 		for _, e := range status.Entries {
 			if e.Status == model.SkillStatusMissing {
 				continue
 			}
-			if e.Status == model.SkillStatusModified && !cmd.Force {
-				keptModified = append(keptModified, e.AbsPath)
-				continue
-			}
-			// Current, Pristine, or (Modified under --force): sdd-owned.
-			if err := os.Remove(e.AbsPath); err != nil && !errors.Is(err, fs.ErrNotExist) {
-				return fmt.Errorf("removing %s: %w", e.AbsPath, err)
-			}
-			removed = append(removed, e.AbsPath)
-			skillDirs[filepath.Join(status.InstallDir, e.Skill)] = true
+			files = append(files, prunable{
+				absPath:  e.AbsPath,
+				skill:    e.Skill,
+				modified: e.Status == model.SkillStatusModified,
+			})
 		}
 
-		// Remove emptied skill subdirs (and their empty descendants), then the
-		// parent skills dir if nothing remains. A dir still holding a user's
-		// non-sdd skill is left in place.
-		for dir := range skillDirs {
-			pruneEmptyDirs(dir)
+		result, err := removePrunable(files, target, status.InstallDir, cmd.Force, touched)
+		if err != nil {
+			return err
 		}
+		// A dropped agent leaves the whole tree behind, so the parent skills
+		// dir goes too when nothing remains in it. A dir still holding a
+		// user's non-sdd skill is left in place.
 		if entries, err := os.ReadDir(status.InstallDir); err == nil && len(entries) == 0 {
 			_ = os.Remove(status.InstallDir)
 		}
 
-		*touched = append(*touched, removed...)
-		if cmd.OnAgentSkillsPruned != nil && (len(removed) > 0 || len(keptModified) > 0) {
-			cmd.OnAgentSkillsPruned(command.AgentPruneResult{
-				Target:       target,
-				InstallDir:   status.InstallDir,
-				Removed:      removed,
-				KeptModified: keptModified,
+		if cmd.OnAgentSkillsPruned != nil && result.TouchedAnything() {
+			cmd.OnAgentSkillsPruned(result)
+		}
+	}
+	return nil
+}
+
+// prunable is one installed file a prune pass may remove, paired with whether
+// the user has edited it since sdd wrote it.
+type prunable struct {
+	absPath  string
+	skill    string
+	modified bool
+}
+
+// removePrunable is the one rule both prune passes apply — a dropped agent's
+// whole render, and the orphans a still-rendered agent's bundle no longer
+// carries: unmodified files go, edited ones are preserved and named, --force
+// takes those too, and directories a removal emptied are cleaned up. Removed
+// paths join touched so the commit records the deletions.
+func removePrunable(files []prunable, target model.AgentTarget, installDir string, force bool, touched *[]string) (command.AgentPruneResult, error) {
+	result := command.AgentPruneResult{Target: target, InstallDir: installDir}
+	skillDirs := map[string]bool{}
+	for _, f := range files {
+		if f.modified && !force {
+			result.KeptModified = append(result.KeptModified, f.absPath)
+			continue
+		}
+		if err := os.Remove(f.absPath); err != nil && !errors.Is(err, fs.ErrNotExist) {
+			return result, fmt.Errorf("removing %s: %w", f.absPath, err)
+		}
+		result.Removed = append(result.Removed, f.absPath)
+		skillDirs[filepath.Join(installDir, f.skill)] = true
+	}
+	for dir := range skillDirs {
+		pruneEmptyDirs(dir)
+	}
+	*touched = append(*touched, result.Removed...)
+	return result, nil
+}
+
+// pruneSkillOrphans removes installed files whose bundle source is gone, for
+// agents that are still rendered — the upgrade path pruneAgentSkills does not
+// cover, since nothing was dropped from supported_agents. Ownership and safety
+// follow the same rule: only unmodified sdd-written files go, user-modified
+// copies are preserved and named, and --force removes those too. Files sdd
+// never wrote carry no stamp and never reach here.
+func (h *Handler) pruneSkillOrphans(ctx context.Context, targets []model.AgentTarget, scope model.Scope, cmd *command.InitCmd, touched *[]string) error {
+	for _, target := range targets {
+		status, err := h.reader.SkillStatus(ctx, query.SkillStatusQuery{
+			Target:   target,
+			Scope:    scope,
+			RepoRoot: cmd.RepoRoot,
+			UserHome: cmd.UserHome,
+		})
+		if err != nil {
+			return fmt.Errorf("classifying %s skills for orphan prune: %w", target, err)
+		}
+
+		var files []prunable
+		for _, o := range status.Orphans {
+			// A file stamped by a later sdd is not an orphan, it is the
+			// future: an older binary running here must not delete what a
+			// newer one installed just because its own bundle lacks it.
+			if model.SkillStampIsAhead(o.Installed.StoredVersion, cmd.BinaryVersion) {
+				continue
+			}
+			files = append(files, prunable{
+				absPath:  o.AbsPath,
+				skill:    o.Skill,
+				modified: o.Class == model.SkillOrphanModified,
 			})
 		}
+
+		result, err := removePrunable(files, target, status.InstallDir, cmd.Force, touched)
+		if err != nil {
+			return err
+		}
+		if cmd.OnSkillOrphansPruned != nil && result.TouchedAnything() {
+			cmd.OnSkillOrphansPruned(result)
+		}
 	}
 	return nil
 }