diff --git a/cmd/entire/cli/clean.go b/cmd/entire/cli/clean.go index c8d726530..1d35363d9 100644 --- a/cmd/entire/cli/clean.go +++ b/cmd/entire/cli/clean.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" + "github.com/charmbracelet/huh" "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/strategy" @@ -40,12 +41,12 @@ This command finds and removes orphaned data from any strategy: Cached transcripts and other temporary data. Safe to delete when no active sessions are using them. -Default: shows a preview of items that would be deleted. -With --force, actually deletes the orphaned items. +Default: shows a preview and asks for confirmation before deleting. +With --force, deletes without prompting. The entire/checkpoints/v1 branch itself is never deleted.`, RunE: func(cmd *cobra.Command, _ []string) error { - return runClean(cmd.Context(), cmd.OutOrStdout(), forceFlag) + return runClean(cmd.Context(), cmd, forceFlag) }, } @@ -54,7 +55,9 @@ The entire/checkpoints/v1 branch itself is never deleted.`, return cmd } -func runClean(ctx context.Context, w io.Writer, force bool) error { +func runClean(ctx context.Context, cmd *cobra.Command, force bool) error { + w := cmd.OutOrStdout() + // Initialize logging so structured logs go to .entire/logs/ instead of stderr. // Error is non-fatal: if logging init fails, logs go to stderr (acceptable fallback). logging.SetLogLevelGetter(GetLogLevel) @@ -75,7 +78,44 @@ func runClean(ctx context.Context, w io.Writer, force bool) error { fmt.Fprintf(w, "Warning: failed to list temp files: %v\n", err) } - return runCleanWithItems(ctx, w, force, items, tempFiles) + // Force mode: skip preview and confirmation + if force { + return runCleanWithItems(ctx, w, true, items, tempFiles) + } + + // Show preview + if err := runCleanWithItems(ctx, w, false, items, tempFiles); err != nil { + return err + } + + // If nothing to clean, we're done (preview already printed the message) + totalItems := len(items) + len(tempFiles) + if totalItems == 0 { + return nil + } + + // Interactive confirmation + var confirmed bool + form := NewAccessibleForm( + huh.NewGroup( + huh.NewConfirm(). + Title("Delete these items?"). + Affirmative("Yes, delete"). + Negative("Cancel"). + Value(&confirmed), + ), + ) + + if err := form.Run(); err != nil { + return fmt.Errorf("confirmation cancelled: %w", err) + } + + if !confirmed { + fmt.Fprintln(w, "Clean cancelled.") + return nil + } + + return runCleanWithItems(ctx, w, true, items, tempFiles) } // listTempFiles returns files in .entire/tmp/ that are safe to delete, @@ -172,7 +212,7 @@ func runCleanWithItems(ctx context.Context, w io.Writer, force bool, items []str // Preview mode (default) if !force { totalItems := len(items) + len(tempFiles) - fmt.Fprintf(w, "Found %d items to clean:\n\n", totalItems) + fmt.Fprintf(w, "Found %d %s to clean:\n\n", totalItems, itemWord(totalItems)) if len(branches) > 0 { fmt.Fprintf(w, "Shadow branches (%d):\n", len(branches)) @@ -206,7 +246,6 @@ func runCleanWithItems(ctx context.Context, w io.Writer, force bool, items []str fmt.Fprintln(w) } - fmt.Fprintln(w, "Run with --force to delete these items.") return nil } @@ -224,70 +263,78 @@ func runCleanWithItems(ctx context.Context, w io.Writer, force bool, items []str totalFailed := len(result.FailedBranches) + len(result.FailedStates) + len(result.FailedCheckpoints) + len(failedTempFiles) if totalDeleted > 0 { - fmt.Fprintf(w, "Deleted %d items:\n", totalDeleted) + fmt.Fprintf(w, "✓ Deleted %d %s:\n", totalDeleted, itemWord(totalDeleted)) if len(result.ShadowBranches) > 0 { - fmt.Fprintf(w, "\n Shadow branches (%d):\n", len(result.ShadowBranches)) + fmt.Fprintf(w, "\nShadow branches (%d):\n", len(result.ShadowBranches)) for _, branch := range result.ShadowBranches { - fmt.Fprintf(w, " %s\n", branch) + fmt.Fprintf(w, " %s\n", branch) } } if len(result.SessionStates) > 0 { - fmt.Fprintf(w, "\n Session states (%d):\n", len(result.SessionStates)) + fmt.Fprintf(w, "\nSession states (%d):\n", len(result.SessionStates)) for _, state := range result.SessionStates { - fmt.Fprintf(w, " %s\n", state) + fmt.Fprintf(w, " %s\n", state) } } if len(result.Checkpoints) > 0 { - fmt.Fprintf(w, "\n Checkpoints (%d):\n", len(result.Checkpoints)) + fmt.Fprintf(w, "\nCheckpoints (%d):\n", len(result.Checkpoints)) for _, cp := range result.Checkpoints { - fmt.Fprintf(w, " %s\n", cp) + fmt.Fprintf(w, " %s\n", cp) } } if len(deletedTempFiles) > 0 { - fmt.Fprintf(w, "\n Temp files (%d):\n", len(deletedTempFiles)) + fmt.Fprintf(w, "\nTemp files (%d):\n", len(deletedTempFiles)) for _, file := range deletedTempFiles { - fmt.Fprintf(w, " %s\n", file) + fmt.Fprintf(w, " %s\n", file) } } } if totalFailed > 0 { - fmt.Fprintf(w, "\nFailed to delete %d items:\n", totalFailed) + fmt.Fprintf(w, "\nFailed to delete %d %s:\n", totalFailed, itemWord(totalFailed)) if len(result.FailedBranches) > 0 { - fmt.Fprintf(w, "\n Shadow branches:\n") + fmt.Fprintf(w, "\nShadow branches:\n") for _, branch := range result.FailedBranches { - fmt.Fprintf(w, " %s\n", branch) + fmt.Fprintf(w, " %s\n", branch) } } if len(result.FailedStates) > 0 { - fmt.Fprintf(w, "\n Session states:\n") + fmt.Fprintf(w, "\nSession states:\n") for _, state := range result.FailedStates { - fmt.Fprintf(w, " %s\n", state) + fmt.Fprintf(w, " %s\n", state) } } if len(result.FailedCheckpoints) > 0 { - fmt.Fprintf(w, "\n Checkpoints:\n") + fmt.Fprintf(w, "\nCheckpoints:\n") for _, cp := range result.FailedCheckpoints { - fmt.Fprintf(w, " %s\n", cp) + fmt.Fprintf(w, " %s\n", cp) } } if len(failedTempFiles) > 0 { - fmt.Fprintf(w, "\n Temp files:\n") + fmt.Fprintf(w, "\nTemp files:\n") for _, fe := range failedTempFiles { - fmt.Fprintf(w, " %s: %v\n", fe.File, fe.Err) + fmt.Fprintf(w, " %s: %v\n", fe.File, fe.Err) } } - return fmt.Errorf("failed to delete %d items", totalFailed) + return fmt.Errorf("failed to delete %d %s", totalFailed, itemWord(totalFailed)) } return nil } + +// itemWord returns "item" or "items" based on count. +func itemWord(n int) string { + if n == 1 { + return "item" + } + return "items" +} diff --git a/cmd/entire/cli/clean_test.go b/cmd/entire/cli/clean_test.go index af739e613..b520a389d 100644 --- a/cmd/entire/cli/clean_test.go +++ b/cmd/entire/cli/clean_test.go @@ -12,8 +12,19 @@ import ( "github.com/go-git/go-git/v6" "github.com/go-git/go-git/v6/plumbing" "github.com/go-git/go-git/v6/plumbing/object" + "github.com/spf13/cobra" ) +// newTestCleanCmd creates a cobra.Command with captured stdout for testing runClean. +func newTestCleanCmd(t *testing.T) (*cobra.Command, *bytes.Buffer) { + t.Helper() + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + var stdout bytes.Buffer + cmd.SetOut(&stdout) + return cmd, &stdout +} + func setupCleanTestRepo(t *testing.T) (*git.Repository, plumbing.Hash) { t.Helper() @@ -70,9 +81,9 @@ func TestRunClean_NoOrphanedItems(t *testing.T) { setupCleanTestRepo(t) var stdout bytes.Buffer - err := runClean(context.Background(), &stdout, false) + err := runCleanWithItems(context.Background(), &stdout, false, []strategy.CleanupItem{}, nil) if err != nil { - t.Fatalf("runClean() error = %v", err) + t.Fatalf("runCleanWithItems() error = %v", err) } output := stdout.String() @@ -99,17 +110,24 @@ func TestRunClean_PreviewMode(t *testing.T) { t.Fatalf("failed to create %s: %v", paths.MetadataBranchName, err) } + // Use runClean with force=true would need TTY for confirmation, + // so test preview output via runCleanWithItems directly. + items, err := strategy.ListAllCleanupItems(context.Background()) + if err != nil { + t.Fatalf("ListAllCleanupItems() error = %v", err) + } + var stdout bytes.Buffer - err := runClean(context.Background(), &stdout, false) // force=false + err = runCleanWithItems(context.Background(), &stdout, false, items, nil) if err != nil { - t.Fatalf("runClean() error = %v", err) + t.Fatalf("runCleanWithItems() error = %v", err) } output := stdout.String() // Should show preview header - if !strings.Contains(output, "items to clean") { - t.Errorf("Expected 'items to clean' in output, got: %s", output) + if !strings.Contains(output, "to clean") { + t.Errorf("Expected 'to clean' in output, got: %s", output) } // Should list the shadow branches @@ -125,9 +143,9 @@ func TestRunClean_PreviewMode(t *testing.T) { t.Errorf("Should not list '%s', got: %s", paths.MetadataBranchName, output) } - // Should prompt to use --force - if !strings.Contains(output, "--force") { - t.Errorf("Expected '--force' prompt in output, got: %s", output) + // Should NOT contain stale --force message (interactive prompt handles confirmation) + if strings.Contains(output, "--force") { + t.Errorf("Should not contain '--force' message in interactive mode, got: %s", output) } // Branches should still exist (preview mode doesn't delete) @@ -151,17 +169,17 @@ func TestRunClean_ForceMode(t *testing.T) { } } - var stdout bytes.Buffer - err := runClean(context.Background(), &stdout, true) // force=true + cmd, stdout := newTestCleanCmd(t) + err := runClean(cmd.Context(), cmd, true) // force=true skips confirmation if err != nil { t.Fatalf("runClean() error = %v", err) } output := stdout.String() - // Should show deletion confirmation - if !strings.Contains(output, "Deleted") { - t.Errorf("Expected 'Deleted' in output, got: %s", output) + // Should show deletion confirmation with ✓ prefix + if !strings.Contains(output, "✓ Deleted") { + t.Errorf("Expected '✓ Deleted' in output, got: %s", output) } // Branches should be deleted @@ -187,8 +205,8 @@ func TestRunClean_SessionsBranchPreserved(t *testing.T) { t.Fatalf("failed to create entire/checkpoints/v1: %v", err) } - var stdout bytes.Buffer - err := runClean(context.Background(), &stdout, true) // force=true + cmd, _ := newTestCleanCmd(t) + err := runClean(cmd.Context(), cmd, true) // force=true if err != nil { t.Fatalf("runClean() error = %v", err) } @@ -211,8 +229,8 @@ func TestRunClean_NotGitRepository(t *testing.T) { t.Chdir(dir) paths.ClearWorktreeRootCache() - var stdout bytes.Buffer - err := runClean(context.Background(), &stdout, false) + cmd, _ := newTestCleanCmd(t) + err := runClean(cmd.Context(), cmd, true) // force=true to skip TTY prompt // Should return error for non-git directory if err == nil { @@ -243,10 +261,16 @@ func TestRunClean_Subdirectory(t *testing.T) { t.Chdir(subDir) paths.ClearWorktreeRootCache() + // Use ListAllCleanupItems + runCleanWithItems to test preview without TTY + items, err := strategy.ListAllCleanupItems(context.Background()) + if err != nil { + t.Fatalf("ListAllCleanupItems() from subdirectory error = %v", err) + } + var stdout bytes.Buffer - err = runClean(context.Background(), &stdout, false) + err = runCleanWithItems(context.Background(), &stdout, false, items, nil) if err != nil { - t.Fatalf("runClean() from subdirectory error = %v", err) + t.Fatalf("runCleanWithItems() from subdirectory error = %v", err) } output := stdout.String() @@ -282,20 +306,24 @@ func TestRunCleanWithItems_PartialFailure(t *testing.T) { t.Fatal("runCleanWithItems() should return error when items fail to delete") } - // Error message should indicate the failure - if !strings.Contains(err.Error(), "failed to delete") { - t.Errorf("Error should mention 'failed to delete', got: %v", err) + // Error message should indicate the failure with correct grammar + if !strings.Contains(err.Error(), "failed to delete 1 item") { + t.Errorf("Error should mention 'failed to delete 1 item', got: %v", err) + } + // Verify singular (not "1 items") + if strings.Contains(err.Error(), "1 items") { + t.Errorf("Error should use singular 'item' for count 1, got: %v", err) } - // Output should show the successful deletion + // Output should show the successful deletion with ✓ and singular grammar output := stdout.String() - if !strings.Contains(output, "Deleted 1 items") { - t.Errorf("Output should show successful deletion, got: %s", output) + if !strings.Contains(output, "✓ Deleted 1 item:") { + t.Errorf("Output should show '✓ Deleted 1 item:', got: %s", output) } - // Output should also show the failures - if !strings.Contains(output, "Failed to delete 1 items") { - t.Errorf("Output should show failures, got: %s", output) + // Output should also show the failure with singular grammar + if !strings.Contains(output, "Failed to delete 1 item:") { + t.Errorf("Output should show 'Failed to delete 1 item:', got: %s", output) } } @@ -318,20 +346,20 @@ func TestRunCleanWithItems_AllFailures(t *testing.T) { t.Fatal("runCleanWithItems() should return error when items fail to delete") } - // Error message should indicate 2 failures + // Error message should indicate 2 failures with plural grammar if !strings.Contains(err.Error(), "failed to delete 2 items") { t.Errorf("Error should mention 'failed to delete 2 items', got: %v", err) } - // Output should NOT show any successful deletions + // Output should NOT show any successful deletions (no ✓ Deleted line) output := stdout.String() - if strings.Contains(output, "Deleted") { + if strings.Contains(output, "✓ Deleted") { t.Errorf("Output should not show successful deletions, got: %s", output) } - // Output should show the failures - if !strings.Contains(output, "Failed to delete 2 items") { - t.Errorf("Output should show failures, got: %s", output) + // Output should show the failures with plural grammar + if !strings.Contains(output, "Failed to delete 2 items:") { + t.Errorf("Output should show 'Failed to delete 2 items:', got: %s", output) } }