From c7b4d53578ff4640bb2af3924ab04a1f03942d31 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Wed, 18 Mar 2026 09:55:36 +0000 Subject: [PATCH 1/4] Deprecate reset command The reset and clean commands had an overlap which at times made it harder for users to understand the difference between them. This change deprecates reset in favour of a new flag in the clean command. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Changes cmd/entire/cli/clean.go — Unified command - New entire clean defaults to cleaning current HEAD sessions (old reset behavior) - --all flag for repo-wide orphan cleanup (old clean behavior) - --session flag for single-session cleanup - --dry-run flag for preview without action - --force flag to skip confirmation and active session guard - --all and --session are mutually exclusive - Moved activeSessionsOnCurrentHead() here from reset.go - Added previewCurrentHead() for dry-run support - Renamed internal functions: runCleanAll, runCleanAllWithItems, runCleanSession cmd/entire/cli/reset.go — Deprecated wrapper - Cobra Deprecated field set — auto-prints deprecation warning when used - Delegates to the same underlying strategy methods - Keeps --force and --session flags for backward compatibility cmd/entire/cli/clean_test.go — Comprehensive tests - Default mode: nothing to clean, force, dry-run, dry-run empty, sessions without shadow branch, multiple sessions, not git repo - --all mode: no orphans, preview, dry-run, force, sessions branch preserved, not git repo, subdirectory - runCleanAllWithItems unit tests: partial failure, all failures, no items, mixed types preview - Flag validation: mutual exclusion of --all and --session cmd/entire/cli/reset_test.go — Deprecated wrapper tests - TestResetCmd_IsDeprecated — verifies deprecation field - Retained core functional tests (nothing to reset, force, not git repo) ### Documentation - README.md — updated command table and troubleshooting section - docs/architecture/checkpoint-scenarios.md — updated cleanup examples Assisted-by: Claude Opus 4.6 Signed-off-by: Paulo Gomes Entire-Checkpoint: b79b35cd956d --- README.md | 14 +- cmd/entire/cli/clean.go | 406 +++++++++++++++----- cmd/entire/cli/clean_test.go | 448 ++++++++++++++++++---- cmd/entire/cli/reset.go | 118 +----- cmd/entire/cli/reset_test.go | 159 +------- docs/architecture/checkpoint-scenarios.md | 6 +- 6 files changed, 732 insertions(+), 419 deletions(-) diff --git a/README.md b/README.md index 7d104a051..6289ffce9 100644 --- a/README.md +++ b/README.md @@ -164,12 +164,11 @@ Multiple AI sessions can run on the same commit. If you start a second session w | Command | Description | | ---------------- | ------------------------------------------------------------------------------------------------- | -| `entire clean` | Clean up orphaned Entire data | +| `entire clean` | Clean up session data and orphaned Entire data (use `--all` for repo-wide cleanup) | | `entire disable` | Remove Entire hooks from repository | | `entire doctor` | Fix or clean up stuck sessions | | `entire enable` | Enable Entire in your repository | | `entire explain` | Explain a session or commit | -| `entire reset` | Delete the shadow branch and session state for the current HEAD commit | | `entire resume` | Switch to a branch, restore latest checkpointed session metadata, and show command(s) to continue | | `entire rewind` | Rewind to a previous checkpoint | | `entire status` | Show current session info | @@ -396,7 +395,7 @@ Entire automatically redacts detected secrets (API keys, tokens, credentials) wh | "Not a git repository" | Navigate to a Git repository first | | "Entire is disabled" | Run `entire enable` | | "No rewind points found" | Work with your configured agent and commit your changes | -| "shadow branch conflict" | Run `entire reset --force` | +| "shadow branch conflict" | Run `entire clean --force` | ### SSH Authentication Errors @@ -425,11 +424,14 @@ ENTIRE_LOG_LEVEL=debug entire status } ``` -### Resetting State +### Cleaning Up State ``` -# Reset shadow branch for current commit -entire reset --force +# Clean session data for current commit +entire clean --force + +# Clean all orphaned data across the repository +entire clean --all --force # Disable and re-enable entire disable && entire enable --force diff --git a/cmd/entire/cli/clean.go b/cmd/entire/cli/clean.go index c8d726530..502f60679 100644 --- a/cmd/entire/cli/clean.go +++ b/cmd/entire/cli/clean.go @@ -2,154 +2,278 @@ package cli import ( "context" + "errors" "fmt" "io" "os" "path/filepath" "strings" + "github.com/charmbracelet/huh" + "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" + "github.com/entireio/cli/cmd/entire/cli/session" "github.com/entireio/cli/cmd/entire/cli/strategy" + "github.com/go-git/go-git/v6/plumbing" "github.com/spf13/cobra" ) func newCleanCmd() *cobra.Command { var forceFlag bool + var allFlag bool + var dryRunFlag bool + var sessionFlag string cmd := &cobra.Command{ Use: "clean", - Short: "Clean up orphaned Entire data", - Long: `Remove orphaned Entire data (session state, shadow branches, checkpoint metadata, temp files) that wasn't cleaned up automatically. + Short: "Clean up session data and orphaned Entire data", + Long: `Clean up Entire session data for the current HEAD commit. -This command finds and removes orphaned data from any strategy: +By default, cleans session state and shadow branches for the current HEAD: + - Session state files (.git/entire-sessions/.json) + - Shadow branch (entire/-) - Shadow branches (entire/) - Normally auto-cleaned when sessions - are condensed during commits. +Use --all to clean all orphaned Entire data across the repository: + - Orphaned shadow branches + - Orphaned session state files + - Temporary files (.entire/tmp/) + - Checkpoint metadata is never deleted - Session state files (.git/entire-sessions/) - Track active sessions. Orphaned when no checkpoints or shadow branches - reference them. +Use --session to clean a specific session only. - Checkpoint metadata (entire/checkpoints/v1 branch) - Checkpoints are permanent (condensed session history) and are - never considered orphaned. +Without --force, prompts for confirmation before deleting. +Use --dry-run to preview what would be deleted without prompting.`, + RunE: func(cmd *cobra.Command, _ []string) error { + ctx := cmd.Context() - Temporary files (.entire/tmp/) - Cached transcripts and other temporary data. Safe to delete when no - active sessions are using them. + // Validate mutually exclusive flags + if allFlag && sessionFlag != "" { + return errors.New("--all and --session cannot be used together") + } -Default: shows a preview of items that would be deleted. -With --force, actually deletes the orphaned items. + // Initialize logging + logging.SetLogLevelGetter(GetLogLevel) + if err := logging.Init(ctx, ""); err == nil { + defer logging.Close() + } -The entire/checkpoints/v1 branch itself is never deleted.`, - RunE: func(cmd *cobra.Command, _ []string) error { - return runClean(cmd.Context(), cmd.OutOrStdout(), forceFlag) + if allFlag { + return runCleanAll(ctx, cmd.OutOrStdout(), forceFlag, dryRunFlag) + } + + // Check if in git repository + if _, err := paths.WorktreeRoot(ctx); err != nil { + return errors.New("not a git repository") + } + + if sessionFlag != "" { + strat := GetStrategy(ctx) + return runCleanSession(ctx, cmd, strat, sessionFlag, forceFlag, dryRunFlag) + } + + return runCleanCurrentHead(ctx, cmd, forceFlag, dryRunFlag) }, } - cmd.Flags().BoolVarP(&forceFlag, "force", "f", false, "Actually delete items (default: dry run)") + cmd.Flags().BoolVarP(&forceFlag, "force", "f", false, "Skip confirmation prompt and override active session guard") + cmd.Flags().BoolVarP(&allFlag, "all", "a", false, "Clean all orphaned data across the repository") + cmd.Flags().BoolVarP(&dryRunFlag, "dry-run", "d", false, "Preview what would be deleted without deleting") + cmd.Flags().StringVar(&sessionFlag, "session", "", "Clean a specific session by ID") return cmd } -func runClean(ctx context.Context, w io.Writer, force bool) error { - // 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) - if err := logging.Init(ctx, ""); err == nil { - defer logging.Close() +// runCleanCurrentHead cleans session data for the current HEAD commit. +func runCleanCurrentHead(ctx context.Context, cmd *cobra.Command, force, dryRun bool) error { + strat := GetStrategy(ctx) + w := cmd.OutOrStdout() + + // Dry-run: show what would be cleaned + if dryRun { + return previewCurrentHead(ctx, w) } - // List all cleanup items - items, err := strategy.ListAllCleanupItems(ctx) - if err != nil { - return fmt.Errorf("failed to list orphaned items: %w", err) + // Check for active sessions before cleaning + if !force { + activeSessions, err := activeSessionsOnCurrentHead(ctx) + if err != nil { + fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not check for active sessions: %v\n", err) + fmt.Fprintln(cmd.ErrOrStderr(), "Use --force to override.") + return nil + } + if len(activeSessions) > 0 { + fmt.Fprintln(cmd.ErrOrStderr(), "Active sessions detected on current HEAD:") + for _, s := range activeSessions { + fmt.Fprintf(cmd.ErrOrStderr(), " %s (phase: %s)\n", s.SessionID, s.Phase) + } + fmt.Fprintln(cmd.ErrOrStderr(), "Use --force to override or wait for sessions to finish.") + return nil + } } - // List temp files - tempFiles, err := listTempFiles(ctx) - if err != nil { - // Non-fatal: continue with other cleanup items - fmt.Fprintf(w, "Warning: failed to list temp files: %v\n", err) + // Prompt for confirmation + if !force { + var confirmed bool + + form := NewAccessibleForm( + huh.NewGroup( + huh.NewConfirm(). + Title("Clean session data for current HEAD?"). + Value(&confirmed), + ), + ) + + if err := form.Run(); err != nil { + if errors.Is(err, huh.ErrUserAborted) { + return nil + } + return fmt.Errorf("failed to get confirmation: %w", err) + } + + if !confirmed { + return nil + } } - return runCleanWithItems(ctx, w, force, items, tempFiles) + if err := strat.Reset(ctx); err != nil { + return fmt.Errorf("clean failed: %w", err) + } + + return nil } -// listTempFiles returns files in .entire/tmp/ that are safe to delete, -// excluding files belonging to active sessions. -func listTempFiles(ctx context.Context) ([]string, error) { - tmpDir, err := paths.AbsPath(ctx, paths.EntireTmpDir) +// previewCurrentHead shows what would be cleaned for the current HEAD. +func previewCurrentHead(ctx context.Context, w io.Writer) error { + repo, err := openRepository(ctx) if err != nil { - return nil, fmt.Errorf("failed to get temp dir path: %w", err) + return err } - entries, err := os.ReadDir(tmpDir) - if os.IsNotExist(err) { - return nil, nil + head, err := repo.Head() + if err != nil { + return fmt.Errorf("failed to get HEAD: %w", err) } + + worktreePath, err := paths.WorktreeRoot(ctx) if err != nil { - return nil, fmt.Errorf("failed to read temp dir: %w", err) + return fmt.Errorf("failed to get worktree path: %w", err) + } + worktreeID, err := paths.GetWorktreeID(worktreePath) + if err != nil { + return fmt.Errorf("failed to get worktree ID: %w", err) } - // Build set of active session IDs to protect their temp files - activeSessionIDs := make(map[string]bool) - if states, listErr := strategy.ListSessionStates(ctx); listErr == nil { - for _, state := range states { - activeSessionIDs[state.SessionID] = true + shadowBranchName := checkpoint.ShadowBranchNameForCommit(head.Hash().String(), worktreeID) + + // Check if shadow branch exists + refName := plumbing.NewBranchReferenceName(shadowBranchName) + _, refErr := repo.Reference(refName, true) + hasShadowBranch := refErr == nil + + // Find sessions for this commit + strat := GetStrategy(ctx) + sessions, err := strat.FindSessionsForCommit(ctx, head.Hash().String()) + if err != nil { + sessions = nil + } + + if !hasShadowBranch && len(sessions) == 0 { + fmt.Fprintln(w, "Nothing to clean for current HEAD.") + return nil + } + + fmt.Fprint(w, "Would clean the following items:\n\n") + + if len(sessions) > 0 { + fmt.Fprintf(w, "Session states (%d):\n", len(sessions)) + for _, s := range sessions { + fmt.Fprintf(w, " %s (checkpoints: %d)\n", s.SessionID, s.StepCount) } + fmt.Fprintln(w) } - var files []string - for _, entry := range entries { - if entry.IsDir() { - continue + if hasShadowBranch { + fmt.Fprintf(w, "Shadow branch:\n %s\n\n", shadowBranchName) + } + + fmt.Fprintln(w, "Run without --dry-run to clean these items.") + return nil +} + +// runCleanSession handles the --session flag: clean a single session. +func runCleanSession(ctx context.Context, cmd *cobra.Command, strat *strategy.ManualCommitStrategy, sessionID string, force, dryRun bool) error { + // Verify the session exists + state, err := strategy.LoadSessionState(ctx, sessionID) + if err != nil { + return fmt.Errorf("failed to load session: %w", err) + } + if state == nil { + return fmt.Errorf("session not found: %s", sessionID) + } + + if dryRun { + w := cmd.OutOrStdout() + fmt.Fprintf(w, "Would clean session %s (phase: %s, checkpoints: %d)\n", sessionID, state.Phase, state.StepCount) + return nil + } + + if !force { + var confirmed bool + + title := fmt.Sprintf("Clean session %s?", sessionID) + description := fmt.Sprintf("Phase: %s, Checkpoints: %d", state.Phase, state.StepCount) + + form := NewAccessibleForm( + huh.NewGroup( + huh.NewConfirm(). + Title(title). + Description(description). + Value(&confirmed), + ), + ) + + if err := form.Run(); err != nil { + if errors.Is(err, huh.ErrUserAborted) { + return nil + } + return fmt.Errorf("failed to get confirmation: %w", err) } - // Skip temp files belonging to active sessions (e.g., "session-id.json") - name := entry.Name() - sessionID := strings.TrimSuffix(name, ".json") - if sessionID != name && activeSessionIDs[sessionID] { - continue + + if !confirmed { + return nil } - files = append(files, name) } - return files, nil -} -// TempFileDeleteError contains a file name and the error that occurred during deletion. -type TempFileDeleteError struct { - File string - Err error + if err := strat.ResetSession(ctx, sessionID); err != nil { + return fmt.Errorf("clean session failed: %w", err) + } + + fmt.Fprintf(cmd.OutOrStdout(), "Session %s has been cleaned. File changes remain in the working directory.\n", sessionID) + return nil } -// deleteTempFiles removes all files in .entire/tmp/. -// Returns successfully deleted files and any failures with their error reasons. -func deleteTempFiles(ctx context.Context, files []string) (deleted []string, failed []TempFileDeleteError) { - tmpDir, err := paths.AbsPath(ctx, paths.EntireTmpDir) +// runCleanAll cleans all orphaned data across the repository (old `entire clean` behavior). +func runCleanAll(ctx context.Context, w io.Writer, force, dryRun bool) error { + // List all cleanup items + items, err := strategy.ListAllCleanupItems(ctx) if err != nil { - // Can't get path - mark all as failed with the same error - for _, file := range files { - failed = append(failed, TempFileDeleteError{File: file, Err: err}) - } - return nil, failed + return fmt.Errorf("failed to list orphaned items: %w", err) } - for _, file := range files { - path := filepath.Join(tmpDir, file) - if err := os.Remove(path); err != nil { - failed = append(failed, TempFileDeleteError{File: file, Err: err}) - } else { - deleted = append(deleted, file) - } + // List temp files + tempFiles, err := listTempFiles(ctx) + if err != nil { + // Non-fatal: continue with other cleanup items + fmt.Fprintf(w, "Warning: failed to list temp files: %v\n", err) } - return deleted, failed + + return runCleanAllWithItems(ctx, w, force, dryRun, items, tempFiles) } -// runCleanWithItems is the core logic for cleaning orphaned items. +// runCleanAllWithItems is the core logic for cleaning all orphaned items. // Separated for testability. -func runCleanWithItems(ctx context.Context, w io.Writer, force bool, items []strategy.CleanupItem, tempFiles []string) error { +func runCleanAllWithItems(ctx context.Context, w io.Writer, force, dryRun bool, items []strategy.CleanupItem, tempFiles []string) error { // Handle no items case if len(items) == 0 && len(tempFiles) == 0 { fmt.Fprintln(w, "No orphaned items to clean up.") @@ -169,8 +293,8 @@ func runCleanWithItems(ctx context.Context, w io.Writer, force bool, items []str } } - // Preview mode (default) - if !force { + // Dry-run or non-force: show preview + if dryRun || !force { totalItems := len(items) + len(tempFiles) fmt.Fprintf(w, "Found %d items to clean:\n\n", totalItems) @@ -206,6 +330,11 @@ func runCleanWithItems(ctx context.Context, w io.Writer, force bool, items []str fmt.Fprintln(w) } + if dryRun { + fmt.Fprintln(w, "Run without --dry-run to delete these items.") + return nil + } + fmt.Fprintln(w, "Run with --force to delete these items.") return nil } @@ -291,3 +420,104 @@ func runCleanWithItems(ctx context.Context, w io.Writer, force bool, items []str return nil } + +// listTempFiles returns files in .entire/tmp/ that are safe to delete, +// excluding files belonging to active sessions. +func listTempFiles(ctx context.Context) ([]string, error) { + tmpDir, err := paths.AbsPath(ctx, paths.EntireTmpDir) + if err != nil { + return nil, fmt.Errorf("failed to get temp dir path: %w", err) + } + + entries, err := os.ReadDir(tmpDir) + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("failed to read temp dir: %w", err) + } + + // Build set of active session IDs to protect their temp files + activeSessionIDs := make(map[string]bool) + if states, listErr := strategy.ListSessionStates(ctx); listErr == nil { + for _, state := range states { + activeSessionIDs[state.SessionID] = true + } + } + + var files []string + for _, entry := range entries { + if entry.IsDir() { + continue + } + // Skip temp files belonging to active sessions (e.g., "session-id.json") + name := entry.Name() + sessionID := strings.TrimSuffix(name, ".json") + if sessionID != name && activeSessionIDs[sessionID] { + continue + } + files = append(files, name) + } + return files, nil +} + +// TempFileDeleteError contains a file name and the error that occurred during deletion. +type TempFileDeleteError struct { + File string + Err error +} + +// deleteTempFiles removes all files in .entire/tmp/. +// Returns successfully deleted files and any failures with their error reasons. +func deleteTempFiles(ctx context.Context, files []string) (deleted []string, failed []TempFileDeleteError) { + tmpDir, err := paths.AbsPath(ctx, paths.EntireTmpDir) + if err != nil { + // Can't get path - mark all as failed with the same error + for _, file := range files { + failed = append(failed, TempFileDeleteError{File: file, Err: err}) + } + return nil, failed + } + + for _, file := range files { + path := filepath.Join(tmpDir, file) + if err := os.Remove(path); err != nil { + failed = append(failed, TempFileDeleteError{File: file, Err: err}) + } else { + deleted = append(deleted, file) + } + } + return deleted, failed +} + +// activeSessionsOnCurrentHead returns sessions on the current HEAD +// that are in an active phase (ACTIVE). +func activeSessionsOnCurrentHead(ctx context.Context) ([]*session.State, error) { + repo, err := openRepository(ctx) + if err != nil { + return nil, err + } + + head, err := repo.Head() + if err != nil { + return nil, fmt.Errorf("failed to get HEAD: %w", err) + } + currentHead := head.Hash().String() + + states, err := strategy.ListSessionStates(ctx) + if err != nil { + return nil, fmt.Errorf("failed to list session states: %w", err) + } + + var active []*session.State + for _, state := range states { + if state.BaseCommit != currentHead { + continue + } + if state.Phase.IsActive() { + active = append(active, state) + } + } + + return active, nil +} diff --git a/cmd/entire/cli/clean_test.go b/cmd/entire/cli/clean_test.go index af739e613..964c22903 100644 --- a/cmd/entire/cli/clean_test.go +++ b/cmd/entire/cli/clean_test.go @@ -3,10 +3,14 @@ package cli import ( "bytes" "context" + "encoding/json" + "os" "path/filepath" "strings" "testing" + "time" + "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/strategy" "github.com/go-git/go-git/v6" @@ -66,13 +70,281 @@ func setupCleanTestRepo(t *testing.T) (*git.Repository, plumbing.Hash) { return repo, commitHash } -func TestRunClean_NoOrphanedItems(t *testing.T) { +// createSessionStateFile creates a session state JSON file in .git/entire-sessions/. +func createSessionStateFile(t *testing.T, repoRoot string, sessionID string, commitHash plumbing.Hash) string { + t.Helper() + + sessionStateDir := filepath.Join(repoRoot, ".git", "entire-sessions") + if err := os.MkdirAll(sessionStateDir, 0o755); err != nil { + t.Fatalf("failed to create session state dir: %v", err) + } + + sessionFile := filepath.Join(sessionStateDir, sessionID+".json") + sessionState := map[string]any{ + "session_id": sessionID, + "base_commit": commitHash.String(), + "checkpoint_count": 1, + "started_at": time.Now().Format(time.RFC3339), + } + sessionData, err := json.Marshal(sessionState) + if err != nil { + t.Fatalf("failed to marshal session state: %v", err) + } + if err := os.WriteFile(sessionFile, sessionData, 0o600); err != nil { + t.Fatalf("failed to write session state file: %v", err) + } + return sessionFile +} + +// --- Default mode tests (current HEAD cleanup) --- + +func TestCleanCmd_DefaultMode_NothingToClean(t *testing.T) { + setupCleanTestRepo(t) + + cmd := newCleanCmd() + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs([]string{"--force"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("clean command error = %v", err) + } +} + +func TestCleanCmd_DefaultMode_WithForce(t *testing.T) { + repo, commitHash := setupCleanTestRepo(t) + + wt, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + worktreePath := wt.Filesystem.Root() + worktreeID, err := paths.GetWorktreeID(worktreePath) + if err != nil { + t.Fatalf("failed to get worktree ID: %v", err) + } + + // Create shadow branch + shadowBranch := checkpoint.ShadowBranchNameForCommit(commitHash.String(), worktreeID) + shadowRef := plumbing.NewHashReference(plumbing.NewBranchReferenceName(shadowBranch), commitHash) + if err := repo.Storer.SetReference(shadowRef); err != nil { + t.Fatalf("failed to create shadow branch: %v", err) + } + + // Create session state file + sessionFile := createSessionStateFile(t, worktreePath, "2026-02-02-test123", commitHash) + + cmd := newCleanCmd() + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs([]string{"--force"}) + + err = cmd.Execute() + if err != nil { + t.Fatalf("clean command error = %v", err) + } + + // Verify shadow branch deleted + refName := plumbing.NewBranchReferenceName(shadowBranch) + if _, err := repo.Reference(refName, true); err == nil { + t.Error("shadow branch should be deleted") + } + + // Verify session state file deleted + if _, err := os.Stat(sessionFile); !os.IsNotExist(err) { + t.Error("session state file should be deleted") + } +} + +func TestCleanCmd_DefaultMode_DryRun(t *testing.T) { + repo, commitHash := setupCleanTestRepo(t) + + wt, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + worktreePath := wt.Filesystem.Root() + worktreeID, err := paths.GetWorktreeID(worktreePath) + if err != nil { + t.Fatalf("failed to get worktree ID: %v", err) + } + + // Create shadow branch + shadowBranch := checkpoint.ShadowBranchNameForCommit(commitHash.String(), worktreeID) + shadowRef := plumbing.NewHashReference(plumbing.NewBranchReferenceName(shadowBranch), commitHash) + if err := repo.Storer.SetReference(shadowRef); err != nil { + t.Fatalf("failed to create shadow branch: %v", err) + } + + // Create session state file + sessionFile := createSessionStateFile(t, worktreePath, "2026-02-02-test123", commitHash) + + cmd := newCleanCmd() + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs([]string{"--dry-run"}) + + err = cmd.Execute() + if err != nil { + t.Fatalf("clean command error = %v", err) + } + + output := stdout.String() + if !strings.Contains(output, "Would clean") { + t.Errorf("Expected 'Would clean' in output, got: %s", output) + } + if !strings.Contains(output, shadowBranch) { + t.Errorf("Expected shadow branch name in output, got: %s", output) + } + if !strings.Contains(output, "2026-02-02-test123") { + t.Errorf("Expected session ID in output, got: %s", output) + } + + // Verify nothing was deleted + refName := plumbing.NewBranchReferenceName(shadowBranch) + if _, err := repo.Reference(refName, true); err != nil { + t.Error("shadow branch should still exist after dry-run") + } + if _, err := os.Stat(sessionFile); os.IsNotExist(err) { + t.Error("session state file should still exist after dry-run") + } +} + +func TestCleanCmd_DefaultMode_DryRun_NothingToClean(t *testing.T) { setupCleanTestRepo(t) + cmd := newCleanCmd() + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs([]string{"--dry-run"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("clean command error = %v", err) + } + + output := stdout.String() + if !strings.Contains(output, "Nothing to clean") { + t.Errorf("Expected 'Nothing to clean' message, got: %s", output) + } +} + +func TestCleanCmd_DefaultMode_SessionsWithoutShadowBranch(t *testing.T) { + repo, commitHash := setupCleanTestRepo(t) + + wt, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + worktreePath := wt.Filesystem.Root() + + // Create session state files WITHOUT a shadow branch + sessionFile := createSessionStateFile(t, worktreePath, "2026-02-02-orphaned", commitHash) + + cmd := newCleanCmd() + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs([]string{"--force"}) + + err = cmd.Execute() + if err != nil { + t.Fatalf("clean command error = %v", err) + } + + // Verify session state file deleted + if _, err := os.Stat(sessionFile); !os.IsNotExist(err) { + t.Error("session state file should be deleted even without shadow branch") + } +} + +func TestCleanCmd_DefaultMode_MultipleSessions(t *testing.T) { + repo, commitHash := setupCleanTestRepo(t) + + wt, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + worktreePath := wt.Filesystem.Root() + worktreeID, err := paths.GetWorktreeID(worktreePath) + if err != nil { + t.Fatalf("failed to get worktree ID: %v", err) + } + + // Create shadow branch + shadowBranch := checkpoint.ShadowBranchNameForCommit(commitHash.String(), worktreeID) + shadowRef := plumbing.NewHashReference(plumbing.NewBranchReferenceName(shadowBranch), commitHash) + if err := repo.Storer.SetReference(shadowRef); err != nil { + t.Fatalf("failed to create shadow branch: %v", err) + } + + // Create multiple session state files + session1File := createSessionStateFile(t, worktreePath, "2026-02-02-session1", commitHash) + session2File := createSessionStateFile(t, worktreePath, "2026-02-02-session2", commitHash) + + cmd := newCleanCmd() + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs([]string{"--force"}) + + err = cmd.Execute() + if err != nil { + t.Fatalf("clean command error = %v", err) + } + + // Verify both session files deleted + if _, err := os.Stat(session1File); !os.IsNotExist(err) { + t.Error("session1 file should be deleted") + } + if _, err := os.Stat(session2File); !os.IsNotExist(err) { + t.Error("session2 file should be deleted") + } + + // Verify shadow branch deleted + refName := plumbing.NewBranchReferenceName(shadowBranch) + if _, err := repo.Reference(refName, true); err == nil { + t.Error("shadow branch should be deleted") + } +} + +func TestCleanCmd_DefaultMode_NotGitRepo(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + paths.ClearWorktreeRootCache() + + cmd := newCleanCmd() + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + + err := cmd.Execute() + if err == nil { + t.Fatal("clean command should return error for non-git directory") + } + if !strings.Contains(err.Error(), "not a git repository") { + t.Errorf("Expected 'not a git repository' error, got: %v", err) + } +} + +// --- --all mode tests (repo-wide orphan cleanup) --- + +func TestCleanCmd_All_NoOrphanedItems(t *testing.T) { + setupCleanTestRepo(t) + + cmd := newCleanCmd() var stdout bytes.Buffer - err := runClean(context.Background(), &stdout, false) + cmd.SetOut(&stdout) + cmd.SetArgs([]string{"--all"}) + + err := cmd.Execute() if err != nil { - t.Fatalf("runClean() error = %v", err) + t.Fatalf("clean --all error = %v", err) } output := stdout.String() @@ -81,7 +353,7 @@ func TestRunClean_NoOrphanedItems(t *testing.T) { } } -func TestRunClean_PreviewMode(t *testing.T) { +func TestCleanCmd_All_PreviewMode(t *testing.T) { repo, commitHash := setupCleanTestRepo(t) // Create shadow branches @@ -99,33 +371,30 @@ func TestRunClean_PreviewMode(t *testing.T) { t.Fatalf("failed to create %s: %v", paths.MetadataBranchName, err) } + cmd := newCleanCmd() var stdout bytes.Buffer - err := runClean(context.Background(), &stdout, false) // force=false + cmd.SetOut(&stdout) + cmd.SetArgs([]string{"--all"}) // no --force, shows preview + + err := cmd.Execute() if err != nil { - t.Fatalf("runClean() error = %v", err) + t.Fatalf("clean --all 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) } - - // Should list the shadow branches if !strings.Contains(output, "entire/abc1234") { t.Errorf("Expected 'entire/abc1234' in output, got: %s", output) } if !strings.Contains(output, "entire/def5678") { t.Errorf("Expected 'entire/def5678' in output, got: %s", output) } - - // Should NOT list entire/checkpoints/v1 if strings.Contains(output, paths.MetadataBranchName) { 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) } @@ -139,11 +408,10 @@ func TestRunClean_PreviewMode(t *testing.T) { } } -func TestRunClean_ForceMode(t *testing.T) { +func TestCleanCmd_All_DryRun(t *testing.T) { repo, commitHash := setupCleanTestRepo(t) - // Create shadow branches - shadowBranches := []string{"entire/abc1234", "entire/def5678"} + shadowBranches := []string{"entire/abc1234"} for _, b := range shadowBranches { ref := plumbing.NewHashReference(plumbing.NewBranchReferenceName(b), commitHash) if err := repo.Storer.SetReference(ref); err != nil { @@ -151,15 +419,55 @@ func TestRunClean_ForceMode(t *testing.T) { } } + cmd := newCleanCmd() var stdout bytes.Buffer - err := runClean(context.Background(), &stdout, true) // force=true + cmd.SetOut(&stdout) + cmd.SetArgs([]string{"--all", "--dry-run"}) + + err := cmd.Execute() if err != nil { - t.Fatalf("runClean() error = %v", err) + t.Fatalf("clean --all --dry-run error = %v", err) } output := stdout.String() + if !strings.Contains(output, "items to clean") { + t.Errorf("Expected 'items to clean' in output, got: %s", output) + } + if !strings.Contains(output, "without --dry-run") { + t.Errorf("Expected '--dry-run' hint in output, got: %s", output) + } + + // Branches should still exist + for _, b := range shadowBranches { + refName := plumbing.NewBranchReferenceName(b) + if _, err := repo.Reference(refName, true); err != nil { + t.Errorf("Branch %s should still exist after dry-run", b) + } + } +} + +func TestCleanCmd_All_ForceMode(t *testing.T) { + repo, commitHash := setupCleanTestRepo(t) + + shadowBranches := []string{"entire/abc1234", "entire/def5678"} + for _, b := range shadowBranches { + ref := plumbing.NewHashReference(plumbing.NewBranchReferenceName(b), commitHash) + if err := repo.Storer.SetReference(ref); err != nil { + t.Fatalf("failed to create branch %s: %v", b, err) + } + } + + cmd := newCleanCmd() + var stdout bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetArgs([]string{"--all", "--force"}) - // Should show deletion confirmation + err := cmd.Execute() + if err != nil { + t.Fatalf("clean --all --force error = %v", err) + } + + output := stdout.String() if !strings.Contains(output, "Deleted") { t.Errorf("Expected 'Deleted' in output, got: %s", output) } @@ -173,10 +481,9 @@ func TestRunClean_ForceMode(t *testing.T) { } } -func TestRunClean_SessionsBranchPreserved(t *testing.T) { +func TestCleanCmd_All_SessionsBranchPreserved(t *testing.T) { repo, commitHash := setupCleanTestRepo(t) - // Create shadow branch and sessions branch shadowRef := plumbing.NewHashReference(plumbing.NewBranchReferenceName("entire/abc1234"), commitHash) if err := repo.Storer.SetReference(shadowRef); err != nil { t.Fatalf("failed to create shadow branch: %v", err) @@ -187,10 +494,14 @@ func TestRunClean_SessionsBranchPreserved(t *testing.T) { t.Fatalf("failed to create entire/checkpoints/v1: %v", err) } + cmd := newCleanCmd() var stdout bytes.Buffer - err := runClean(context.Background(), &stdout, true) // force=true + cmd.SetOut(&stdout) + cmd.SetArgs([]string{"--all", "--force"}) + + err := cmd.Execute() if err != nil { - t.Fatalf("runClean() error = %v", err) + t.Fatalf("clean --all --force error = %v", err) } // Shadow branch should be deleted @@ -206,30 +517,31 @@ func TestRunClean_SessionsBranchPreserved(t *testing.T) { } } -func TestRunClean_NotGitRepository(t *testing.T) { +func TestCleanCmd_All_NotGitRepository(t *testing.T) { dir := t.TempDir() t.Chdir(dir) paths.ClearWorktreeRootCache() + cmd := newCleanCmd() var stdout bytes.Buffer - err := runClean(context.Background(), &stdout, false) + cmd.SetOut(&stdout) + cmd.SetArgs([]string{"--all"}) + err := cmd.Execute() // Should return error for non-git directory if err == nil { - t.Error("runClean() should return error for non-git directory") + t.Error("clean --all should return error for non-git directory") } } -func TestRunClean_Subdirectory(t *testing.T) { +func TestCleanCmd_All_Subdirectory(t *testing.T) { repo, commitHash := setupCleanTestRepo(t) - // Create shadow branch shadowRef := plumbing.NewHashReference(plumbing.NewBranchReferenceName("entire/abc1234"), commitHash) if err := repo.Storer.SetReference(shadowRef); err != nil { t.Fatalf("failed to create shadow branch: %v", err) } - // Create and cd into subdirectory within the repo wt, err := repo.Worktree() if err != nil { t.Fatalf("failed to get worktree: %v", err) @@ -243,10 +555,14 @@ func TestRunClean_Subdirectory(t *testing.T) { t.Chdir(subDir) paths.ClearWorktreeRootCache() + cmd := newCleanCmd() var stdout bytes.Buffer - err = runClean(context.Background(), &stdout, false) + cmd.SetOut(&stdout) + cmd.SetArgs([]string{"--all"}) + + err = cmd.Execute() if err != nil { - t.Fatalf("runClean() from subdirectory error = %v", err) + t.Fatalf("clean --all from subdirectory error = %v", err) } output := stdout.String() @@ -255,93 +571,74 @@ func TestRunClean_Subdirectory(t *testing.T) { } } -func TestRunCleanWithItems_PartialFailure(t *testing.T) { - // This test verifies that runCleanWithItems returns an error when some - // deletions fail. We use runCleanWithItems to inject a list - // containing both existing and non-existing items. +// --- runCleanAllWithItems unit tests --- +func TestRunCleanAllWithItems_PartialFailure(t *testing.T) { repo, commitHash := setupCleanTestRepo(t) - // Create one shadow branch shadowRef := plumbing.NewHashReference(plumbing.NewBranchReferenceName("entire/abc1234"), commitHash) if err := repo.Storer.SetReference(shadowRef); err != nil { t.Fatalf("failed to create shadow branch: %v", err) } - // Call runCleanWithItems with a mix of existing and non-existing branches items := []strategy.CleanupItem{ {Type: strategy.CleanupTypeShadowBranch, ID: "entire/abc1234", Reason: "test"}, {Type: strategy.CleanupTypeShadowBranch, ID: "entire/nonexistent1234567", Reason: "test"}, } var stdout bytes.Buffer - err := runCleanWithItems(context.Background(), &stdout, true, items, nil) // force=true + err := runCleanAllWithItems(context.Background(), &stdout, true, false, items, nil) - // Should return an error because one branch failed to delete if err == nil { - t.Fatal("runCleanWithItems() should return error when items fail to delete") + t.Fatal("runCleanAllWithItems() 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) } - // Output should show the successful deletion output := stdout.String() if !strings.Contains(output, "Deleted 1 items") { t.Errorf("Output should show successful deletion, 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) } } -func TestRunCleanWithItems_AllFailures(t *testing.T) { - // Test that error is returned when ALL items fail to delete - +func TestRunCleanAllWithItems_AllFailures(t *testing.T) { setupCleanTestRepo(t) - // Call runCleanWithItems with only non-existing branches items := []strategy.CleanupItem{ {Type: strategy.CleanupTypeShadowBranch, ID: "entire/nonexistent1234567", Reason: "test"}, {Type: strategy.CleanupTypeShadowBranch, ID: "entire/alsononexistent", Reason: "test"}, } var stdout bytes.Buffer - err := runCleanWithItems(context.Background(), &stdout, true, items, nil) // force=true + err := runCleanAllWithItems(context.Background(), &stdout, true, false, items, nil) - // Should return an error because all items failed to delete if err == nil { - t.Fatal("runCleanWithItems() should return error when items fail to delete") + t.Fatal("runCleanAllWithItems() should return error when items fail to delete") } - - // Error message should indicate 2 failures 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 := stdout.String() 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) } } -func TestRunCleanWithItems_NoItems(t *testing.T) { +func TestRunCleanAllWithItems_NoItems(t *testing.T) { setupCleanTestRepo(t) var stdout bytes.Buffer - err := runCleanWithItems(context.Background(), &stdout, false, []strategy.CleanupItem{}, nil) + err := runCleanAllWithItems(context.Background(), &stdout, false, false, []strategy.CleanupItem{}, nil) if err != nil { - t.Fatalf("runCleanWithItems() error = %v", err) + t.Fatalf("runCleanAllWithItems() error = %v", err) } output := stdout.String() @@ -350,10 +647,9 @@ func TestRunCleanWithItems_NoItems(t *testing.T) { } } -func TestRunCleanWithItems_MixedTypes_Preview(t *testing.T) { +func TestRunCleanAllWithItems_MixedTypes_Preview(t *testing.T) { setupCleanTestRepo(t) - // Test preview mode with different cleanup types items := []strategy.CleanupItem{ {Type: strategy.CleanupTypeShadowBranch, ID: "entire/abc1234", Reason: "test"}, {Type: strategy.CleanupTypeSessionState, ID: "session-123", Reason: "no checkpoints"}, @@ -361,14 +657,12 @@ func TestRunCleanWithItems_MixedTypes_Preview(t *testing.T) { } var stdout bytes.Buffer - err := runCleanWithItems(context.Background(), &stdout, false, items, nil) // preview mode + err := runCleanAllWithItems(context.Background(), &stdout, false, false, items, nil) if err != nil { - t.Fatalf("runCleanWithItems() error = %v", err) + t.Fatalf("runCleanAllWithItems() error = %v", err) } output := stdout.String() - - // Should show all types if !strings.Contains(output, "Shadow branches") { t.Errorf("Expected 'Shadow branches' section, got: %s", output) } @@ -378,9 +672,27 @@ func TestRunCleanWithItems_MixedTypes_Preview(t *testing.T) { if !strings.Contains(output, "Checkpoint metadata") { t.Errorf("Expected 'Checkpoint metadata' section, got: %s", output) } - - // Should show total count if !strings.Contains(output, "Found 3 items to clean") { t.Errorf("Expected 'Found 3 items to clean', got: %s", output) } } + +// --- Flag validation tests --- + +func TestCleanCmd_MutuallyExclusiveFlags(t *testing.T) { + setupCleanTestRepo(t) + + cmd := newCleanCmd() + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs([]string{"--all", "--session", "test-session"}) + + err := cmd.Execute() + if err == nil { + t.Fatal("--all and --session should be mutually exclusive") + } + if !strings.Contains(err.Error(), "cannot be used together") { + t.Errorf("Expected mutual exclusion error, got: %v", err) + } +} diff --git a/cmd/entire/cli/reset.go b/cmd/entire/cli/reset.go index f6217ccf0..0e45867ac 100644 --- a/cmd/entire/cli/reset.go +++ b/cmd/entire/cli/reset.go @@ -1,14 +1,12 @@ package cli import ( - "context" "errors" "fmt" "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/session" - "github.com/entireio/cli/cmd/entire/cli/strategy" "github.com/spf13/cobra" ) @@ -17,39 +15,28 @@ func newResetCmd() *cobra.Command { var sessionFlag string cmd := &cobra.Command{ - Use: "reset", - Short: "Reset the shadow branch and session state for current HEAD", - Long: `Reset deletes the shadow branch and session state for the current HEAD commit. - -This allows starting fresh without existing checkpoints on your current commit. - - -The command will: - - Find all sessions where base_commit matches the current HEAD - - Delete each session state file (.git/entire-sessions/.json) - - Delete the shadow branch (entire/-) - -Use --session to reset a single session instead of all sessions. - -Example: If HEAD is at commit abc1234567890, the command will: - 1. Find all .json files in .git/entire-sessions/ with "base_commit": "abc1234567890" - 2. Delete those session files (e.g., 2026-02-02-xyz123.json, 2026-02-02-abc456.json) - 3. Delete the shadow branch entire/abc1234-fd5432 - -Without --force, prompts for confirmation before deleting.`, + Use: "reset", + Short: "Reset the shadow branch and session state for current HEAD", + Deprecated: "use 'entire clean' instead (or 'entire clean --all' for repo-wide cleanup)", RunE: func(cmd *cobra.Command, _ []string) error { ctx := cmd.Context() + + // Initialize logging + logging.SetLogLevelGetter(GetLogLevel) + if err := logging.Init(ctx, ""); err == nil { + defer logging.Close() + } + // Check if in git repository if _, err := paths.WorktreeRoot(ctx); err != nil { return errors.New("not a git repository") } - // Get current strategy strat := GetStrategy(ctx) - // Handle --session flag: reset a single session + // Handle --session flag: delegate to clean's session logic if sessionFlag != "" { - return runResetSession(ctx, cmd, strat, sessionFlag, forceFlag) + return runCleanSession(ctx, cmd, strat, sessionFlag, forceFlag, false) } // Check for active sessions before bulk reset @@ -93,7 +80,6 @@ Without --force, prompts for confirmation before deleting.`, } } - // Call strategy's Reset method if err := strat.Reset(ctx); err != nil { return fmt.Errorf("reset failed: %w", err) } @@ -107,81 +93,3 @@ Without --force, prompts for confirmation before deleting.`, return cmd } - -// runResetSession handles the --session flag: reset a single session. -func runResetSession(ctx context.Context, cmd *cobra.Command, strat *strategy.ManualCommitStrategy, sessionID string, force bool) error { - // Verify the session exists - state, err := strategy.LoadSessionState(ctx, sessionID) - if err != nil { - return fmt.Errorf("failed to load session: %w", err) - } - if state == nil { - return fmt.Errorf("session not found: %s", sessionID) - } - - if !force { - var confirmed bool - - title := fmt.Sprintf("Reset session %s?", sessionID) - description := fmt.Sprintf("Phase: %s, Checkpoints: %d", state.Phase, state.StepCount) - - form := NewAccessibleForm( - huh.NewGroup( - huh.NewConfirm(). - Title(title). - Description(description). - Value(&confirmed), - ), - ) - - if err := form.Run(); err != nil { - if errors.Is(err, huh.ErrUserAborted) { - return nil - } - return fmt.Errorf("failed to get confirmation: %w", err) - } - - if !confirmed { - return nil - } - } - - if err := strat.ResetSession(ctx, sessionID); err != nil { - return fmt.Errorf("reset session failed: %w", err) - } - - fmt.Fprintf(cmd.OutOrStdout(), "Session %s has been reset. File changes remain in the working directory.\n", sessionID) - return nil -} - -// activeSessionsOnCurrentHead returns sessions on the current HEAD -// that are in an active phase (ACTIVE). -func activeSessionsOnCurrentHead(ctx context.Context) ([]*session.State, error) { - repo, err := openRepository(ctx) - if err != nil { - return nil, err - } - - head, err := repo.Head() - if err != nil { - return nil, fmt.Errorf("failed to get HEAD: %w", err) - } - currentHead := head.Hash().String() - - states, err := strategy.ListSessionStates(ctx) - if err != nil { - return nil, fmt.Errorf("failed to list session states: %w", err) - } - - var active []*session.State - for _, state := range states { - if state.BaseCommit != currentHead { - continue - } - if state.Phase.IsActive() { - active = append(active, state) - } - } - - return active, nil -} diff --git a/cmd/entire/cli/reset_test.go b/cmd/entire/cli/reset_test.go index f384741cf..4bf46a8c4 100644 --- a/cmd/entire/cli/reset_test.go +++ b/cmd/entire/cli/reset_test.go @@ -71,10 +71,19 @@ func setupResetTestRepo(t *testing.T) (*git.Repository, plumbing.Hash) { return repo, commitHash } +func TestResetCmd_IsDeprecated(t *testing.T) { + cmd := newResetCmd() + if cmd.Deprecated == "" { + t.Error("reset command should have Deprecated field set") + } + if !strings.Contains(cmd.Deprecated, "entire clean") { + t.Errorf("Deprecated message should mention 'entire clean', got: %s", cmd.Deprecated) + } +} + func TestResetCmd_NothingToReset(t *testing.T) { setupResetTestRepo(t) - // No shadow branch and no sessions - should report nothing to reset cmd := newResetCmd() var stdout, stderr bytes.Buffer cmd.SetOut(&stdout) @@ -85,14 +94,11 @@ func TestResetCmd_NothingToReset(t *testing.T) { if err != nil { t.Fatalf("reset command error = %v", err) } - - // Command should succeed without deleting anything } func TestResetCmd_WithForce(t *testing.T) { repo, commitHash := setupResetTestRepo(t) - // Get worktree path and ID for shadow branch naming wt, err := repo.Worktree() if err != nil { t.Fatalf("failed to get worktree: %v", err) @@ -155,66 +161,11 @@ func TestResetCmd_WithForce(t *testing.T) { } } -func TestResetCmd_SessionsWithoutShadowBranch(t *testing.T) { - repo, commitHash := setupResetTestRepo(t) - - // Create session state files WITHOUT a shadow branch - wt, err := repo.Worktree() - if err != nil { - t.Fatalf("failed to get worktree: %v", err) - } - repoRoot := wt.Filesystem.Root() - sessionStateDir := filepath.Join(repoRoot, ".git", "entire-sessions") - if err := os.MkdirAll(sessionStateDir, 0o755); err != nil { - t.Fatalf("failed to create session state dir: %v", err) - } - - sessionFile := filepath.Join(sessionStateDir, "2026-02-02-orphaned.json") - sessionState := map[string]any{ - "session_id": "2026-02-02-orphaned", - "base_commit": commitHash.String(), - "checkpoint_count": 1, - } - sessionData, err := json.Marshal(sessionState) - if err != nil { - t.Fatalf("failed to marshal session state: %v", err) - } - if err := os.WriteFile(sessionFile, sessionData, 0o600); err != nil { - t.Fatalf("failed to write session state file: %v", err) - } - - // Run reset command with force - cmd := newResetCmd() - var stdout, stderr bytes.Buffer - cmd.SetOut(&stdout) - cmd.SetErr(&stderr) - cmd.SetArgs([]string{"--force"}) - - err = cmd.Execute() - if err != nil { - t.Fatalf("reset command error = %v", err) - } - - // Verify session state file deleted (even without shadow branch) - if _, err := os.Stat(sessionFile); !os.IsNotExist(err) { - t.Error("session state file should be deleted even without shadow branch") - } - - // Verify no shadow branch was created or exists - shadowBranch := "entire/" + commitHash.String()[:7] - refName := plumbing.NewBranchReferenceName(shadowBranch) - if _, err := repo.Reference(refName, true); err == nil { - t.Error("shadow branch should not exist") - } -} - func TestResetCmd_NotGitRepo(t *testing.T) { - // Create temp dir (not git repo) dir := t.TempDir() t.Chdir(dir) paths.ClearWorktreeRootCache() - // Run reset cmd := newResetCmd() var stdout, stderr bytes.Buffer cmd.SetOut(&stdout) @@ -224,94 +175,4 @@ func TestResetCmd_NotGitRepo(t *testing.T) { if err == nil { t.Fatal("reset command should return error for non-git directory") } - - // Verify error message - output := stderr.String() - if !strings.Contains(output, "not a git repository") { - t.Errorf("Expected 'not a git repository' message, got: %s", output) - } -} - -func TestResetCmd_MultipleSessions(t *testing.T) { - repo, commitHash := setupResetTestRepo(t) - - // Get worktree path and ID for shadow branch naming - wt, err := repo.Worktree() - if err != nil { - t.Fatalf("failed to get worktree: %v", err) - } - worktreePath := wt.Filesystem.Root() - worktreeID, err := paths.GetWorktreeID(worktreePath) - if err != nil { - t.Fatalf("failed to get worktree ID: %v", err) - } - - // Create shadow branch with correct naming format - shadowBranch := checkpoint.ShadowBranchNameForCommit(commitHash.String(), worktreeID) - shadowRef := plumbing.NewHashReference(plumbing.NewBranchReferenceName(shadowBranch), commitHash) - if err := repo.Storer.SetReference(shadowRef); err != nil { - t.Fatalf("failed to create shadow branch: %v", err) - } - - // Create multiple session state files - repoRoot := worktreePath - sessionStateDir := filepath.Join(repoRoot, ".git", "entire-sessions") - if err := os.MkdirAll(sessionStateDir, 0o755); err != nil { - t.Fatalf("failed to create session state dir: %v", err) - } - - session1File := filepath.Join(sessionStateDir, "2026-02-02-session1.json") - session1State := map[string]any{ - "session_id": "2026-02-02-session1", - "base_commit": commitHash.String(), - "checkpoint_count": 1, - } - session1Data, err := json.Marshal(session1State) - if err != nil { - t.Fatalf("failed to marshal session1 state: %v", err) - } - if err := os.WriteFile(session1File, session1Data, 0o600); err != nil { - t.Fatalf("failed to write session1 state file: %v", err) - } - - session2File := filepath.Join(sessionStateDir, "2026-02-02-session2.json") - session2State := map[string]any{ - "session_id": "2026-02-02-session2", - "base_commit": commitHash.String(), - "checkpoint_count": 2, - } - session2Data, err := json.Marshal(session2State) - if err != nil { - t.Fatalf("failed to marshal session2 state: %v", err) - } - if err := os.WriteFile(session2File, session2Data, 0o600); err != nil { - t.Fatalf("failed to write session2 state file: %v", err) - } - - // Run reset with force - cmd := newResetCmd() - var stdout, stderr bytes.Buffer - cmd.SetOut(&stdout) - cmd.SetErr(&stderr) - cmd.SetArgs([]string{"--force"}) - - err = cmd.Execute() - if err != nil { - t.Fatalf("reset command error = %v", err) - } - - // Verify both session files deleted - if _, err := os.Stat(session1File); !os.IsNotExist(err) { - t.Error("session1 file should be deleted") - } - - if _, err := os.Stat(session2File); !os.IsNotExist(err) { - t.Error("session2 file should be deleted") - } - - // Verify shadow branch deleted - refName := plumbing.NewBranchReferenceName(shadowBranch) - if _, err := repo.Reference(refName, true); err == nil { - t.Error("shadow branch should be deleted") - } } diff --git a/docs/architecture/checkpoint-scenarios.md b/docs/architecture/checkpoint-scenarios.md index 570ce03e9..023eec5c2 100644 --- a/docs/architecture/checkpoint-scenarios.md +++ b/docs/architecture/checkpoint-scenarios.md @@ -603,9 +603,9 @@ Most orphaned data is cleaned up automatically: - **Shadow branches**: Deleted after condensation if no other sessions reference them - **Session states**: Cleaned up during session listing when shadow branch no longer exists (and session is not ACTIVE, has no `LastCheckpointID`) -For anything that slips through, run `entire clean` manually: +For anything that slips through, run `entire clean --all` manually: ```bash -entire clean # Preview orphaned items -entire clean --force # Delete orphaned items +entire clean --all # Preview orphaned items +entire clean --all --force # Delete orphaned items ``` From aba6ef176b57514f407694a414948de475b62dc8 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Wed, 18 Mar 2026 12:04:24 +0000 Subject: [PATCH 2/4] Fix clean/reset output consistency and defensive logging order - Update clean --all help text to reflect that orphaned checkpoint entries can be removed (the branch itself is preserved) - Accept io.Writer in Reset/ResetSession instead of hardcoding os.Stderr, so callers use cmd.ErrOrStderr() and output goes through Cobra streams - Check for git repository before initializing logging in reset command to avoid creating .entire/logs in arbitrary directories - Parameterize runCleanSession with action verb so reset uses "Reset" wording and clean uses "Clean" wording in user-facing messages Signed-off-by: Paulo Gomes Assisted-by: Claude Opus 4.6 Entire-Checkpoint: abdf248e9af4 --- cmd/entire/cli/clean.go | 23 +++++++++++-------- cmd/entire/cli/reset.go | 15 ++++++------ .../cli/strategy/manual_commit_reset.go | 23 +++++++++++-------- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/cmd/entire/cli/clean.go b/cmd/entire/cli/clean.go index 502f60679..e566c8ea9 100644 --- a/cmd/entire/cli/clean.go +++ b/cmd/entire/cli/clean.go @@ -37,8 +37,9 @@ By default, cleans session state and shadow branches for the current HEAD: Use --all to clean all orphaned Entire data across the repository: - Orphaned shadow branches - Orphaned session state files + - Orphaned checkpoint entries on entire/checkpoints/v1 - Temporary files (.entire/tmp/) - - Checkpoint metadata is never deleted +The entire/checkpoints/v1 branch itself is preserved. Use --session to clean a specific session only. @@ -69,7 +70,7 @@ Use --dry-run to preview what would be deleted without prompting.`, if sessionFlag != "" { strat := GetStrategy(ctx) - return runCleanSession(ctx, cmd, strat, sessionFlag, forceFlag, dryRunFlag) + return runCleanSession(ctx, cmd, strat, sessionFlag, forceFlag, dryRunFlag, "Clean", "cleaned") } return runCleanCurrentHead(ctx, cmd, forceFlag, dryRunFlag) @@ -136,7 +137,7 @@ func runCleanCurrentHead(ctx context.Context, cmd *cobra.Command, force, dryRun } } - if err := strat.Reset(ctx); err != nil { + if err := strat.Reset(ctx, cmd.ErrOrStderr()); err != nil { return fmt.Errorf("clean failed: %w", err) } @@ -201,8 +202,10 @@ func previewCurrentHead(ctx context.Context, w io.Writer) error { return nil } -// runCleanSession handles the --session flag: clean a single session. -func runCleanSession(ctx context.Context, cmd *cobra.Command, strat *strategy.ManualCommitStrategy, sessionID string, force, dryRun bool) error { +// runCleanSession handles the --session flag: clean/reset a single session. +// actionVerb is the capitalized verb (e.g., "Clean" or "Reset") and pastVerb +// is the past tense (e.g., "cleaned" or "reset") used in user-facing messages. +func runCleanSession(ctx context.Context, cmd *cobra.Command, strat *strategy.ManualCommitStrategy, sessionID string, force, dryRun bool, actionVerb, pastVerb string) error { // Verify the session exists state, err := strategy.LoadSessionState(ctx, sessionID) if err != nil { @@ -214,14 +217,14 @@ func runCleanSession(ctx context.Context, cmd *cobra.Command, strat *strategy.Ma if dryRun { w := cmd.OutOrStdout() - fmt.Fprintf(w, "Would clean session %s (phase: %s, checkpoints: %d)\n", sessionID, state.Phase, state.StepCount) + fmt.Fprintf(w, "Would %s session %s (phase: %s, checkpoints: %d)\n", strings.ToLower(actionVerb), sessionID, state.Phase, state.StepCount) return nil } if !force { var confirmed bool - title := fmt.Sprintf("Clean session %s?", sessionID) + title := fmt.Sprintf("%s session %s?", actionVerb, sessionID) description := fmt.Sprintf("Phase: %s, Checkpoints: %d", state.Phase, state.StepCount) form := NewAccessibleForm( @@ -245,11 +248,11 @@ func runCleanSession(ctx context.Context, cmd *cobra.Command, strat *strategy.Ma } } - if err := strat.ResetSession(ctx, sessionID); err != nil { - return fmt.Errorf("clean session failed: %w", err) + if err := strat.ResetSession(ctx, cmd.ErrOrStderr(), sessionID); err != nil { + return fmt.Errorf("%s session failed: %w", strings.ToLower(actionVerb), err) } - fmt.Fprintf(cmd.OutOrStdout(), "Session %s has been cleaned. File changes remain in the working directory.\n", sessionID) + fmt.Fprintf(cmd.OutOrStdout(), "Session %s has been %s. File changes remain in the working directory.\n", sessionID, pastVerb) return nil } diff --git a/cmd/entire/cli/reset.go b/cmd/entire/cli/reset.go index 0e45867ac..b80f7624d 100644 --- a/cmd/entire/cli/reset.go +++ b/cmd/entire/cli/reset.go @@ -21,22 +21,23 @@ func newResetCmd() *cobra.Command { RunE: func(cmd *cobra.Command, _ []string) error { ctx := cmd.Context() + // Check if in git repository before initializing logging, + // to avoid creating .entire/logs in arbitrary directories. + if _, err := paths.WorktreeRoot(ctx); err != nil { + return errors.New("not a git repository") + } + // Initialize logging logging.SetLogLevelGetter(GetLogLevel) if err := logging.Init(ctx, ""); err == nil { defer logging.Close() } - // Check if in git repository - if _, err := paths.WorktreeRoot(ctx); err != nil { - return errors.New("not a git repository") - } - strat := GetStrategy(ctx) // Handle --session flag: delegate to clean's session logic if sessionFlag != "" { - return runCleanSession(ctx, cmd, strat, sessionFlag, forceFlag, false) + return runCleanSession(ctx, cmd, strat, sessionFlag, forceFlag, false, "Reset", "reset") } // Check for active sessions before bulk reset @@ -80,7 +81,7 @@ func newResetCmd() *cobra.Command { } } - if err := strat.Reset(ctx); err != nil { + if err := strat.Reset(ctx, cmd.ErrOrStderr()); err != nil { return fmt.Errorf("reset failed: %w", err) } diff --git a/cmd/entire/cli/strategy/manual_commit_reset.go b/cmd/entire/cli/strategy/manual_commit_reset.go index 21501a3e3..c4d4b513c 100644 --- a/cmd/entire/cli/strategy/manual_commit_reset.go +++ b/cmd/entire/cli/strategy/manual_commit_reset.go @@ -3,6 +3,7 @@ package strategy import ( "context" "fmt" + "io" "os" "github.com/entireio/cli/cmd/entire/cli/paths" @@ -18,7 +19,8 @@ func isAccessibleMode() bool { // Reset deletes the shadow branch and session state for the current HEAD. // This allows starting fresh without existing checkpoints. -func (s *ManualCommitStrategy) Reset(ctx context.Context) error { +// Output is written to w; pass cmd.ErrOrStderr() from the caller. +func (s *ManualCommitStrategy) Reset(ctx context.Context, w io.Writer) error { repo, err := OpenRepository(ctx) if err != nil { return fmt.Errorf("failed to open git repository: %w", err) @@ -54,9 +56,9 @@ func (s *ManualCommitStrategy) Reset(ctx context.Context) error { sessions = nil // Ignore error, treat as no sessions } - // If nothing to reset, return early + // If nothing to clean, return early if !hasShadowBranch && len(sessions) == 0 { - fmt.Fprintf(os.Stderr, "Nothing to reset for %s\n", shadowBranchName) + fmt.Fprintf(w, "Nothing to clean for %s\n", shadowBranchName) return nil } @@ -64,7 +66,7 @@ func (s *ManualCommitStrategy) Reset(ctx context.Context) error { clearedSessions := make([]string, 0) for _, state := range sessions { if err := s.clearSessionState(ctx, state.SessionID); err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to clear session state for %s: %v\n", state.SessionID, err) + fmt.Fprintf(w, "Warning: failed to clear session state for %s: %v\n", state.SessionID, err) } else { clearedSessions = append(clearedSessions, state.SessionID) } @@ -73,7 +75,7 @@ func (s *ManualCommitStrategy) Reset(ctx context.Context) error { // Report cleared session states with session IDs if len(clearedSessions) > 0 { for _, sessionID := range clearedSessions { - fmt.Fprintf(os.Stderr, "Cleared session state for %s\n", sessionID) + fmt.Fprintf(w, "Cleared session state for %s\n", sessionID) } } @@ -82,7 +84,7 @@ func (s *ManualCommitStrategy) Reset(ctx context.Context) error { if err := DeleteBranchCLI(ctx, shadowBranchName); err != nil { return fmt.Errorf("failed to delete shadow branch: %w", err) } - fmt.Fprintf(os.Stderr, "Deleted shadow branch %s\n", shadowBranchName) + fmt.Fprintf(w, "Deleted shadow branch %s\n", shadowBranchName) } return nil @@ -90,7 +92,8 @@ func (s *ManualCommitStrategy) Reset(ctx context.Context) error { // ResetSession clears a single session's state and removes the shadow branch // if no other sessions reference it. File changes remain in the working directory. -func (s *ManualCommitStrategy) ResetSession(ctx context.Context, sessionID string) error { +// Output is written to w; pass cmd.ErrOrStderr() from the caller. +func (s *ManualCommitStrategy) ResetSession(ctx context.Context, w io.Writer, sessionID string) error { // Load the session state state, err := s.loadSessionState(ctx, sessionID) if err != nil { @@ -104,7 +107,7 @@ func (s *ManualCommitStrategy) ResetSession(ctx context.Context, sessionID strin if err := s.clearSessionState(ctx, sessionID); err != nil { return fmt.Errorf("failed to clear session state: %w", err) } - fmt.Fprintf(os.Stderr, "Cleared session state for %s\n", sessionID) + fmt.Fprintf(w, "Cleared session state for %s\n", sessionID) // Determine the shadow branch for this session shadowBranchName := getShadowBranchNameForCommit(state.BaseCommit, state.WorktreeID) @@ -117,12 +120,12 @@ func (s *ManualCommitStrategy) ResetSession(ctx context.Context, sessionID strin // Clean up shadow branch if no other sessions need it if err := s.cleanupShadowBranchIfUnused(ctx, repo, shadowBranchName, sessionID); err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to clean up shadow branch %s: %v\n", shadowBranchName, err) + fmt.Fprintf(w, "Warning: failed to clean up shadow branch %s: %v\n", shadowBranchName, err) } else { // Check if it was actually deleted via git CLI (go-git's cache // may be stale after CLI-based deletion with packed refs) if err := branchExistsCLI(ctx, shadowBranchName); err != nil { - fmt.Fprintf(os.Stderr, "Deleted shadow branch %s\n", shadowBranchName) + fmt.Fprintf(w, "Deleted shadow branch %s\n", shadowBranchName) } } From 1848ba8d9afbcf30d53d068b8ab26b9d88694e78 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Wed, 18 Mar 2026 12:37:49 +0000 Subject: [PATCH 3/4] Use os.Root in deleteTempFiles to confine deletions to .entire/tmp/ Replaces filepath.Join + os.Remove with os.OpenRoot + root.Remove, so path traversal in file names cannot escape the temp directory. Signed-off-by: Paulo Gomes Assisted-by: Claude Opus 4.6 Entire-Checkpoint: bc3ca6a2ea9a --- cmd/entire/cli/clean.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/cmd/entire/cli/clean.go b/cmd/entire/cli/clean.go index e566c8ea9..71e0e2966 100644 --- a/cmd/entire/cli/clean.go +++ b/cmd/entire/cli/clean.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "os" - "path/filepath" "strings" "github.com/charmbracelet/huh" @@ -471,20 +470,20 @@ type TempFileDeleteError struct { } // deleteTempFiles removes all files in .entire/tmp/. +// Uses os.Root to ensure deletions are confined to the temp directory. // Returns successfully deleted files and any failures with their error reasons. -func deleteTempFiles(ctx context.Context, files []string) (deleted []string, failed []TempFileDeleteError) { - tmpDir, err := paths.AbsPath(ctx, paths.EntireTmpDir) +func deleteTempFiles(_ context.Context, files []string) (deleted []string, failed []TempFileDeleteError) { + root, err := os.OpenRoot(paths.EntireTmpDir) if err != nil { - // Can't get path - mark all as failed with the same error for _, file := range files { failed = append(failed, TempFileDeleteError{File: file, Err: err}) } return nil, failed } + defer root.Close() for _, file := range files { - path := filepath.Join(tmpDir, file) - if err := os.Remove(path); err != nil { + if err := root.Remove(file); err != nil { failed = append(failed, TempFileDeleteError{File: file, Err: err}) } else { deleted = append(deleted, file) From b0a1ca3c95cdb6fd469349c3c5a6470831e434cd Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Wed, 18 Mar 2026 12:56:38 +0000 Subject: [PATCH 4/4] Use os.DirFS + fs.WalkDir in listTempFiles to confine listing to .entire/tmp/ Replaces os.ReadDir with os.DirFS + fs.WalkDir so directory listing is scoped to the temp directory, consistent with deleteTempFiles. Signed-off-by: Paulo Gomes Assisted-by: Claude Opus 4.6 Entire-Checkpoint: fd7de86795d7 --- cmd/entire/cli/clean.go | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/cmd/entire/cli/clean.go b/cmd/entire/cli/clean.go index 71e0e2966..2ecdcd888 100644 --- a/cmd/entire/cli/clean.go +++ b/cmd/entire/cli/clean.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "io/fs" "os" "strings" @@ -425,19 +426,13 @@ func runCleanAllWithItems(ctx context.Context, w io.Writer, force, dryRun bool, // listTempFiles returns files in .entire/tmp/ that are safe to delete, // excluding files belonging to active sessions. +// Uses os.DirFS + fs.WalkDir to confine listing to the temp directory. func listTempFiles(ctx context.Context) ([]string, error) { - tmpDir, err := paths.AbsPath(ctx, paths.EntireTmpDir) - if err != nil { - return nil, fmt.Errorf("failed to get temp dir path: %w", err) - } - - entries, err := os.ReadDir(tmpDir) - if os.IsNotExist(err) { - return nil, nil - } + root, err := os.OpenRoot(paths.EntireTmpDir) if err != nil { - return nil, fmt.Errorf("failed to read temp dir: %w", err) + return nil, fmt.Errorf("failed to open root: %w", err) } + defer root.Close() // Build set of active session IDs to protect their temp files activeSessionIDs := make(map[string]bool) @@ -448,17 +443,27 @@ func listTempFiles(ctx context.Context) ([]string, error) { } var files []string - for _, entry := range entries { - if entry.IsDir() { - continue + err = fs.WalkDir(root.FS(), ".", func(_ string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + return nil } // Skip temp files belonging to active sessions (e.g., "session-id.json") - name := entry.Name() + name := d.Name() sessionID := strings.TrimSuffix(name, ".json") if sessionID != name && activeSessionIDs[sessionID] { - continue + return nil } files = append(files, name) + return nil + }) + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("failed to list temp dir: %w", err) } return files, nil }