Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ Pipeline: **Scan → Classify → Filter/Sort → Output/Clean**
- **AI agent friendly**: all interactions via CLI flags (`--yes`, `--json`), no interactive prompts required
- **Metadata enrichment**: scanners populate `ScanResult.Label` / `Recommendation` so users can decide without decoding paths (UUID → "iPhone 17 Pro · iOS 26.3", "superseded by newer build", "runtime unavailable"). First applied in xcode; reusable for any ecosystem with opaque identifiers
- **Vendor cleanup**: scanners may implement the optional `VendorCleaner` interface to register ecosystem-native cleanup commands (e.g. `xcrun simctl delete unavailable`). `devclean clean --vendor-cleanup` runs them alongside path-based deletion so vendor internal state stays consistent — natural fit for Docker (`system prune`), Homebrew (`cleanup`), Gradle, etc.
- **Deletion strategy**: `ScanResult.Delete` (`model.DeleteMethod`: kind path/command/api + display + Run closure) expresses non-path reclaims per item; nil = path removal. Cleaner applies protected/dry-run gates uniformly, then delegates to the method or falls back to trash/force. `VendorCleanup` embeds the same `DeleteMethod` — bulk (ecosystem-level) vs per-item are two uses of one execution contract

## Documentation Rules

Expand Down
26 changes: 23 additions & 3 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,25 @@ Scanners derive these from peer comparison (DeviceSupport build ages), vendor AP

Use this pattern when a single ecosystem produces directories whose names alone don't tell the user what they are.

### Deletion Strategy

Reclaiming is not always `os.RemoveAll` on a path — Docker images or simulator devices are reclaimed through vendor commands or API calls. Each `ScanResult` carries an optional `DeleteMethod` describing how it is reclaimed:

```go
type DeleteMethod struct {
Kind DeleteKind // "path" | "command" | "api"
Display string // what would run — dry-run / listings
Run func(ctx context.Context) error // executes the reclaim
}

// On ScanResult:
Delete *DeleteMethod `json:"delete,omitempty"` // nil = path removal
```

The cleaner applies its policy gates (protected refusal, dry-run) uniformly, then executes: `Delete.Run(ctx)` when a method is attached, otherwise path removal (trash or permanent). A method with a nil `Run` is refused rather than falling back to path removal — a misconfigured item must never delete a path its method didn't intend. Trash/permanent choice only applies to path removal; command/api items follow the vendor's own recovery semantics (surfacing that in the selection UI belongs to the first ecosystem that ships such items).

In JSON output, non-path items serialize as `"delete": {"kind": "command", "display": "..."}` (`Run` never serializes), so agents can tell strategies apart; absence of the key means path removal.

### Vendor Cleanup

Scanners may opt into an additional interface to register ecosystem-native cleanup commands:
Expand All @@ -111,11 +130,12 @@ type VendorCleaner interface {
type VendorCleanup struct {
ID string
Description string
Command string // for dry-run display
Run func(ctx context.Context) error // executes the command
model.DeleteMethod // Kind + Display (dry-run) + Run — shared execution contract
}
```

`VendorCleanup` is the ecosystem-level **bulk** counterpart of a per-item `ScanResult.Delete`: both share the `model.DeleteMethod` execution contract. Bulk actions (e.g. `brew cleanup`) register here; per-item non-path reclaims (e.g. `docker rmi <id>`) attach a `DeleteMethod` to their `ScanResult`.

`devclean clean --vendor-cleanup` collects cleanups from selected ecosystems and runs them alongside path-based deletion. Vendor commands keep the ecosystem's internal state consistent (e.g. `xcrun simctl delete unavailable` removes simulator devices and updates CoreSimulator's database in one step). Dry-run prints the command without executing.

Current implementations:
Expand Down Expand Up @@ -173,7 +193,7 @@ Table output groups results by: **ecosystem → project → sub-package → arti
- Quick select: `[a]` all, `[n]` none, `[s]` safe only, `[d]` dormant only
- Protected projects hidden with explanation
3. Choose: Move to Trash / Permanently delete / Cancel
4. Execute with per-artifact status output
4. Execute with per-artifact status output — items with an attached `DeleteMethod` run it instead of path removal (see Deletion Strategy)
5. Summary: items cleaned, space freed

Trash moves use `os.Rename` into the Trash dir on the home volume. When the artifact lives on a different filesystem (external drive, separate partition), `os.Rename` fails with `EXDEV`; the cleaner falls back to a recursive copy followed by removing the original. The copy recreates directories, regular files (contents + permission bits), and symlinks (as links, never followed). The original is removed only after the copy fully succeeds — a mid-copy failure leaves the original intact and discards the partial copy, so a cross-device move can never lose data.
20 changes: 16 additions & 4 deletions internal/cleaner/cleaner.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cleaner

import (
"context"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -29,8 +30,10 @@ func New(opts Options) *Cleaner {
return &Cleaner{opts: opts}
}

// Clean deletes or trashes a scan result item.
func (c *Cleaner) Clean(r model.ScanResult) error {
// Clean reclaims a scan result item: protection and dry-run gates apply to
// every strategy; execution then goes through the item's DeleteMethod when one
// is attached, or falls back to path removal (trash or permanent).
func (c *Cleaner) Clean(ctx context.Context, r model.ScanResult) error {
if r.Protected {
return fmt.Errorf("refusing to delete protected item: %s (%s)", r.Path, r.Reason)
}
Expand All @@ -39,6 +42,15 @@ func (c *Cleaner) Clean(r model.ScanResult) error {
return nil
}

if r.Delete != nil {
// A method without Run is a scanner bug; refuse rather than fall back
// to path removal, which could delete a path the method never meant to.
if r.Delete.Run == nil {
return fmt.Errorf("delete method for %s has no Run function", r.Path)
}
return r.Delete.Run(ctx)
}

if c.opts.Force {
return os.RemoveAll(r.Path)
}
Expand All @@ -53,10 +65,10 @@ type CleanResult struct {
}

// CleanAll cleans multiple results and returns per-item outcomes.
func (c *Cleaner) CleanAll(results []model.ScanResult) []CleanResult {
func (c *Cleaner) CleanAll(ctx context.Context, results []model.ScanResult) []CleanResult {
var out []CleanResult
for _, r := range results {
err := c.Clean(r)
err := c.Clean(ctx, r)
out = append(out, CleanResult{Item: r, Error: err})
}
return out
Expand Down
154 changes: 147 additions & 7 deletions internal/cleaner/cleaner_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package cleaner_test

import (
"context"
"errors"
"os"
"path/filepath"
"testing"
Expand Down Expand Up @@ -32,7 +34,7 @@ func TestForceDelete(t *testing.T) {
c := cleaner.New(cleaner.Options{Force: true})
result := model.ScanResult{Path: target, Size: 100, Safety: model.SafetySafe}

err := c.Clean(result)
err := c.Clean(t.Context(), result)
if err != nil {
t.Fatalf("Clean error: %v", err)
}
Expand All @@ -50,7 +52,7 @@ func TestDryRun(t *testing.T) {
c := cleaner.New(cleaner.Options{DryRun: true})
result := model.ScanResult{Path: target, Safety: model.SafetySafe}

err := c.Clean(result)
err := c.Clean(t.Context(), result)
if err != nil {
t.Fatalf("Clean error: %v", err)
}
Expand All @@ -68,7 +70,7 @@ func TestProtectedNotDeleted(t *testing.T) {
c := cleaner.New(cleaner.Options{Force: true})
result := model.ScanResult{Path: target, Protected: true, Safety: model.SafetyProtected}

err := c.Clean(result)
err := c.Clean(t.Context(), result)
if err == nil {
t.Error("expected error when cleaning protected item")
}
Expand All @@ -91,7 +93,7 @@ func TestTrashDelete(t *testing.T) {
c := cleaner.New(cleaner.Options{TrashDir: trashDir})
result := model.ScanResult{Path: target, Size: 100, Safety: model.SafetySafe}

err := c.Clean(result)
err := c.Clean(t.Context(), result)
if err != nil {
t.Fatalf("Clean error: %v", err)
}
Expand Down Expand Up @@ -120,7 +122,7 @@ func TestTrashDelete_NameConflict(t *testing.T) {

c := cleaner.New(cleaner.Options{TrashDir: trashDir})
result := model.ScanResult{Path: target, Safety: model.SafetySafe}
err := c.Clean(result)
err := c.Clean(t.Context(), result)
if err != nil {
t.Fatalf("Clean error: %v", err)
}
Expand All @@ -145,7 +147,7 @@ func TestCleanAll(t *testing.T) {
{Path: t2, Size: 200, Safety: model.SafetySafe},
}

cleanResults := c.CleanAll(results)
cleanResults := c.CleanAll(t.Context(), results)

if len(cleanResults) != 2 {
t.Fatalf("expected 2 results, got %d", len(cleanResults))
Expand Down Expand Up @@ -179,7 +181,7 @@ func TestCleanAllSkipsProtected(t *testing.T) {
{Path: protected, Size: 200, Safety: model.SafetyProtected, Protected: true},
}

cleanResults := c.CleanAll(results)
cleanResults := c.CleanAll(t.Context(), results)

// First should succeed
if cleanResults[0].Error != nil {
Expand All @@ -196,3 +198,141 @@ func TestCleanAllSkipsProtected(t *testing.T) {
t.Error("protected dir should still exist")
}
}

// deleteMethodItem builds a ScanResult whose reclaim goes through a
// DeleteMethod, backed by a real path so tests can prove path removal
// was NOT taken.
func deleteMethodItem(t *testing.T, ran *bool) model.ScanResult {
t.Helper()
dir := t.TempDir()
target := filepath.Join(dir, "vendor-managed")
mustMkdir(t, target)
return model.ScanResult{
Path: target,
Size: 100,
Safety: model.SafetySafe,
Delete: &model.DeleteMethod{
Kind: model.DeleteKindCommand,
Display: "vendor prune",
Run: func(context.Context) error {
*ran = true
return nil
},
},
}
}

func TestDeleteMethodRun(t *testing.T) {
var ran bool
result := deleteMethodItem(t, &ran)

c := cleaner.New(cleaner.Options{Force: true})
if err := c.Clean(t.Context(), result); err != nil {
t.Fatalf("Clean error: %v", err)
}

if !ran {
t.Error("expected DeleteMethod.Run to be called")
}
// Path removal must not have been taken, even with Force set.
if _, err := os.Stat(result.Path); err != nil {
t.Errorf("path must be untouched when a DeleteMethod is attached: %v", err)
}
}

func TestDeleteMethodDryRun(t *testing.T) {
var ran bool
result := deleteMethodItem(t, &ran)

c := cleaner.New(cleaner.Options{DryRun: true})
if err := c.Clean(t.Context(), result); err != nil {
t.Fatalf("Clean error: %v", err)
}

if ran {
t.Error("dry-run must not execute DeleteMethod.Run")
}
}

func TestDeleteMethodProtected(t *testing.T) {
var ran bool
result := deleteMethodItem(t, &ran)
result.Protected = true

c := cleaner.New(cleaner.Options{})
if err := c.Clean(t.Context(), result); err == nil {
t.Error("expected error for protected item")
}

if ran {
t.Error("protected item must not execute DeleteMethod.Run")
}
}

func TestDeleteMethodRunError(t *testing.T) {
wantErr := errors.New("vendor tool failed")
result := model.ScanResult{
Path: "/nonexistent",
Safety: model.SafetySafe,
Delete: &model.DeleteMethod{
Kind: model.DeleteKindCommand,
Run: func(context.Context) error { return wantErr },
},
}

c := cleaner.New(cleaner.Options{})
if err := c.Clean(t.Context(), result); !errors.Is(err, wantErr) {
t.Errorf("expected Run error to propagate, got %v", err)
}
}

func TestDeleteMethodNilRun(t *testing.T) {
dir := t.TempDir()
target := filepath.Join(dir, "misconfigured")
mustMkdir(t, target)
result := model.ScanResult{
Path: target,
Safety: model.SafetySafe,
Delete: &model.DeleteMethod{Kind: model.DeleteKindCommand},
}

c := cleaner.New(cleaner.Options{Force: true})
if err := c.Clean(t.Context(), result); err == nil {
t.Error("expected error for DeleteMethod without Run")
}

// No fallback to path removal for a misconfigured method.
if _, err := os.Stat(target); err != nil {
t.Errorf("path must survive a misconfigured DeleteMethod: %v", err)
}
}

func TestCleanAllMixedStrategies(t *testing.T) {
dir := t.TempDir()
pathItem := filepath.Join(dir, "by-path")
mustMkdir(t, pathItem)

var ran bool
methodItem := deleteMethodItem(t, &ran)

c := cleaner.New(cleaner.Options{Force: true})
cleanResults := c.CleanAll(t.Context(), []model.ScanResult{
{Path: pathItem, Safety: model.SafetySafe},
methodItem,
})

for _, cr := range cleanResults {
if cr.Error != nil {
t.Errorf("unexpected error for %s: %v", cr.Item.Path, cr.Error)
}
}
if _, err := os.Stat(pathItem); !os.IsNotExist(err) {
t.Error("path item should be removed via path strategy")
}
if !ran {
t.Error("method item should be reclaimed via its Run")
}
if _, err := os.Stat(methodItem.Path); err != nil {
t.Error("method item's path must be untouched")
}
}
4 changes: 2 additions & 2 deletions internal/cli/clean.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func newCleanCmd() *cobra.Command {
relPath = rel
}
}
err := c.Clean(r)
err := c.Clean(cmd.Context(), r)
if err != nil {
failed++
fmt.Printf(" %s %s — %v\n", ui.ErrStyle.Render("✗"), relPath, err)
Expand Down Expand Up @@ -280,7 +280,7 @@ func runVendorCleanups(ecosystems []model.Ecosystem, dryRun bool) {
fmt.Printf("\n%s\n", ui.ProjectStyle.Render("Vendor cleanup:"))
for _, p := range actions {
header := fmt.Sprintf(" [%s] %s", p.eco, p.action.Description)
fmt.Println(ui.DimStyle.Render(" " + p.action.Command))
fmt.Println(ui.DimStyle.Render(" " + p.action.Display))
if dryRun {
fmt.Printf("%s %s\n", header, ui.DimStyle.Render("(dry-run)"))
continue
Expand Down
2 changes: 1 addition & 1 deletion internal/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func TestIntegration_ScanClassifyClean(t *testing.T) {
// Clean (force, since TempDir won't map to Trash)
c := cleaner.New(cleaner.Options{Force: true})
for _, r := range results {
if err := c.Clean(r); err != nil {
if err := c.Clean(t.Context(), r); err != nil {
t.Errorf("clean error for %s: %v", r.Path, err)
}
}
Expand Down
Loading