From de02a1f49f1e591d815c6d08a31679f9fd547f44 Mon Sep 17 00:00:00 2001 From: Bryan Wade Date: Wed, 10 Jun 2026 15:42:20 -0700 Subject: [PATCH] Make rewind honor checkpoint-deleted tracked files Checkpoint trees record deletions by omission, so rewind must reconcile tracked HEAD paths against the selected checkpoint before restoring files that are present. Without that pass, a file deleted by the checkpoint can remain in the working tree and silently produce a mixed state. Constraint: Rewind must avoid go-git hard reset/checkout paths that can delete ignored infrastructure directories Rejected: Preserve tracked files absent from the checkpoint | this keeps data but violates rewind's selected-checkpoint file-state contract Confidence: high Scope-risk: narrow Directive: Keep tracked-delete preview and restore behavior aligned; checkpoint omission is the deletion signal Tested: mise run lint Tested: go test ./cmd/entire/cli/strategy -run 'TestShadowStrategy_(PreviewRewind|Rewind_)' -count=1 Tested: go test ./cmd/entire/cli/strategy ./cmd/entire/cli/checkpoint Not-tested: mise run check currently fails in TestRunAuthStatus_RendersSessionsTable expecting 2026-01-01 while output renders 2026-01-31 Entire-Checkpoint: 73b83f41aa8f --- .../cli/strategy/manual_commit_rewind.go | 131 ++++++++++++------ cmd/entire/cli/strategy/rewind_test.go | 88 ++++++++++++ 2 files changed, 176 insertions(+), 43 deletions(-) diff --git a/cmd/entire/cli/strategy/manual_commit_rewind.go b/cmd/entire/cli/strategy/manual_commit_rewind.go index 84c32d27c4..520e487c20 100644 --- a/cmd/entire/cli/strategy/manual_commit_rewind.go +++ b/cmd/entire/cli/strategy/manual_commit_rewind.go @@ -354,15 +354,10 @@ func (s *ManualCommitStrategy) Rewind(ctx context.Context, w, errW io.Writer, po } // Build set of files tracked in HEAD - trackedFiles := make(map[string]bool) - //nolint:errcheck // Error is not critical for rewind - _ = headTree.Files().ForEach(func(f *object.File) error { - if err := ctx.Err(); err != nil { - return err //nolint:wrapcheck // Propagating context cancellation - } - trackedFiles[f.Name] = true - return nil - }) + trackedFiles, err := trackedFilesInTree(ctx, headTree) + if err != nil { + return fmt.Errorf("failed to list HEAD files: %w", err) + } // Get repository root to walk from there repoRoot, err := paths.WorktreeRoot(ctx) @@ -379,35 +374,16 @@ func (s *ManualCommitStrategy) Rewind(ctx context.Context, w, errW io.Writer, po } defer repoRootHandle.Close() - // Find and delete untracked files that aren't in the checkpoint. - // Uses git ls-files to only consider non-ignored files, avoiding walks through - // large ignored directories like node_modules/. + if err := deleteTrackedFilesMissingFromCheckpoint(w, repoRootHandle, trackedFiles, checkpointFiles); err != nil { + return err + } + untrackedNow, err := collectUntrackedFiles(ctx) if err != nil { // Non-fatal - continue with restoration fmt.Fprintf(errW, "Warning: error listing untracked files: %v\n", err) } - for _, relPath := range untrackedNow { - // If file is in checkpoint, it will be restored - if checkpointFiles[relPath] { - continue - } - - // If file is tracked in HEAD, don't delete (user's committed work) - if trackedFiles[relPath] { - continue - } - - // If file existed at session start, preserve it (untracked user files) - if preservedUntrackedFiles[relPath] { - continue - } - - // File is untracked and not in checkpoint - delete it via os.Root - if removeErr := osroot.Remove(repoRootHandle, relPath); removeErr == nil { - fmt.Fprintf(w, " Deleted: %s\n", relPath) - } - } + deleteUntrackedFilesMissingFromCheckpoint(w, repoRootHandle, untrackedNow, trackedFiles, checkpointFiles, preservedUntrackedFiles) // Restore files from checkpoint err = tree.Files().ForEach(func(f *object.File) error { @@ -461,6 +437,73 @@ func (s *ManualCommitStrategy) Rewind(ctx context.Context, w, errW io.Writer, po return nil } +func deleteTrackedFilesMissingFromCheckpoint( + w io.Writer, + repoRootHandle *os.Root, + trackedFiles, checkpointFiles map[string]bool, +) error { + // Checkpoint trees encode deletions by omitting paths. Rewind must remove + // tracked HEAD files absent from the checkpoint before restoring present files. + for relPath := range trackedFiles { + if checkpointFiles[relPath] || isProtectedPath(relPath) { + continue + } + if removeErr := osroot.Remove(repoRootHandle, relPath); removeErr == nil { + fmt.Fprintf(w, " Deleted: %s\n", relPath) + } else if !errors.Is(removeErr, os.ErrNotExist) { + return fmt.Errorf("failed to delete tracked file %s: %w", relPath, removeErr) + } + } + return nil +} + +func deleteUntrackedFilesMissingFromCheckpoint( + w io.Writer, + repoRootHandle *os.Root, + untrackedNow []string, + trackedFiles, checkpointFiles, preservedUntrackedFiles map[string]bool, +) { + // Uses git ls-files output, so this only considers non-ignored files and avoids + // walks through large ignored directories like node_modules/. + for _, relPath := range untrackedNow { + // If file is in checkpoint, it will be restored + if checkpointFiles[relPath] { + continue + } + + // If file is tracked in HEAD, it was already handled by the tracked-file + // reconciliation above. + if trackedFiles[relPath] { + continue + } + + // If file existed at session start, preserve it (untracked user files) + if preservedUntrackedFiles[relPath] { + continue + } + + // File is untracked and not in checkpoint - delete it via os.Root + if removeErr := osroot.Remove(repoRootHandle, relPath); removeErr == nil { + fmt.Fprintf(w, " Deleted: %s\n", relPath) + } + } +} + +func trackedFilesInTree(ctx context.Context, tree *object.Tree) (map[string]bool, error) { + trackedFiles := make(map[string]bool) + err := tree.Files().ForEach(func(f *object.File) error { + if err := ctx.Err(); err != nil { + return err //nolint:wrapcheck // Propagating context cancellation + } + trackedFiles[f.Name] = true + return nil + }) + if err != nil { + return nil, fmt.Errorf("failed to iterate tree files: %w", err) + } + return trackedFiles, nil +} + // resetShadowBranchToCheckpoint resets the shadow branch HEAD to the given checkpoint. // This ensures that when the user commits after rewinding, the next checkpoint will only // include prompts from the rewound point, not prompts from later checkpoints. @@ -573,19 +616,21 @@ func (s *ManualCommitStrategy) PreviewRewind(ctx context.Context, point RewindPo } // Build set of files tracked in HEAD - trackedFiles := make(map[string]bool) - //nolint:errcheck // Error is not critical for preview - _ = headTree.Files().ForEach(func(f *object.File) error { - if err := ctx.Err(); err != nil { - return err //nolint:wrapcheck // Propagating context cancellation - } - trackedFiles[f.Name] = true - return nil - }) + trackedFiles, err := trackedFilesInTree(ctx, headTree) + if err != nil { + return nil, fmt.Errorf("failed to list HEAD files: %w", err) + } - // Find untracked files that would be deleted. + // Find tracked and untracked files that would be deleted. // Uses git ls-files to only consider non-ignored files. var filesToDelete []string + for relPath := range trackedFiles { + if checkpointFiles[relPath] || isProtectedPath(relPath) { + continue + } + filesToDelete = append(filesToDelete, relPath) + } + untrackedNow, untrackedErr := collectUntrackedFiles(ctx) if untrackedErr != nil { fmt.Fprintf(os.Stderr, "Warning: could not list untracked files for preview: %v\n", untrackedErr) diff --git a/cmd/entire/cli/strategy/rewind_test.go b/cmd/entire/cli/strategy/rewind_test.go index a604fa3162..1ca220a386 100644 --- a/cmd/entire/cli/strategy/rewind_test.go +++ b/cmd/entire/cli/strategy/rewind_test.go @@ -593,6 +593,94 @@ func TestShadowStrategy_Rewind_FromRepoRoot(t *testing.T) { } } +func TestShadowStrategy_Rewind_RemovesTrackedFilesDeletedByCheckpoint(t *testing.T) { + dir := t.TempDir() + repo, err := git.PlainInit(dir, false) + if err != nil { + t.Fatalf("failed to init git repo: %v", err) + } + + t.Chdir(dir) + paths.ClearWorktreeRootCache() + + worktree, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + + author := &object.Signature{ + Name: "Test", + Email: "test@example.com", + When: time.Now(), + } + + if err := os.WriteFile(filepath.Join(dir, "keep.txt"), []byte("keep\n"), 0o644); err != nil { + t.Fatalf("failed to write keep.txt: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, "delete-me.txt"), []byte("delete\n"), 0o644); err != nil { + t.Fatalf("failed to write delete-me.txt: %v", err) + } + if _, err := worktree.Add("keep.txt"); err != nil { + t.Fatalf("failed to add keep.txt: %v", err) + } + if _, err := worktree.Add("delete-me.txt"); err != nil { + t.Fatalf("failed to add delete-me.txt: %v", err) + } + initialCommit, err := worktree.Commit("Initial commit", &git.CommitOptions{Author: author}) + if err != nil { + t.Fatalf("failed to create initial commit: %v", err) + } + + if err := os.Remove(filepath.Join(dir, "delete-me.txt")); err != nil { + t.Fatalf("failed to delete tracked file: %v", err) + } + if _, err := worktree.Remove("delete-me.txt"); err != nil { + t.Fatalf("failed to stage delete-me.txt removal: %v", err) + } + checkpointHash, err := worktree.Commit("Checkpoint deletes tracked file", &git.CommitOptions{Author: author}) + if err != nil { + t.Fatalf("failed to create checkpoint: %v", err) + } + + if err := worktree.Reset(&git.ResetOptions{ + Commit: initialCommit, + Mode: git.HardReset, + }); err != nil { + t.Fatalf("failed to reset to initial: %v", err) + } + if _, err := os.Stat(filepath.Join(dir, "delete-me.txt")); err != nil { + t.Fatalf("expected delete-me.txt to exist before rewind: %v", err) + } + + s := NewManualCommitStrategy() + point := RewindPoint{ + ID: checkpointHash.String(), + Message: "Checkpoint deletes tracked file", + Date: time.Now(), + } + + preview, err := s.PreviewRewind(context.Background(), point) + if err != nil { + t.Fatalf("PreviewRewind() error = %v", err) + } + require.Contains(t, preview.FilesToDelete, "delete-me.txt") + + if err := s.Rewind(context.Background(), io.Discard, io.Discard, point); err != nil { + t.Fatalf("Rewind() error = %v", err) + } + + if _, err := os.Stat(filepath.Join(dir, "delete-me.txt")); !os.IsNotExist(err) { + t.Fatalf("delete-me.txt should be removed by rewind, got err: %v", err) + } + content, err := os.ReadFile(filepath.Join(dir, "keep.txt")) + if err != nil { + t.Fatalf("expected keep.txt to remain: %v", err) + } + if string(content) != "keep\n" { + t.Fatalf("keep.txt content = %q, want %q", string(content), "keep\n") + } +} + func writeCommittedRewindCheckpoint( t *testing.T, repo *git.Repository,