diff --git a/cmd/entire/cli/checkpoint_group.go b/cmd/entire/cli/checkpoint_group.go index 11643b8205..cfacde9067 100644 --- a/cmd/entire/cli/checkpoint_group.go +++ b/cmd/entire/cli/checkpoint_group.go @@ -49,22 +49,33 @@ func newCheckpointSearchCmd() *cobra.Command { return cmd } -// newCheckpointListCmd wraps the existing branch-default list view. +// newCheckpointListCmd wraps the branch list view, with optional revision-range scoping. func newCheckpointListCmd() *cobra.Command { var sessionFlag string var noPagerFlag bool cmd := &cobra.Command{ - Use: "list", - Short: "List checkpoints on the current branch", - Long: `List checkpoints on the current branch. + Use: "list []", + Short: "List checkpoints reachable from HEAD or a revision range", + Long: `List checkpoints reachable from HEAD, mirroring git log semantics +(checkpoints on merged feature branches are included). -Optionally filter by session ID with --session.`, - RunE: func(cmd *cobra.Command, _ []string) error { +Optionally restrict to a git-style revision range: + list main..HEAD — checkpoints on this branch but not in main + list main.. — same (HEAD is implicit) + list — full history of + +Filter further by session ID with --session.`, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { if checkDisabledGuard(cmd.Context(), cmd.OutOrStdout()) { return nil } - return runExplainBranchWithFilter(cmd.Context(), cmd.OutOrStdout(), noPagerFlag, sessionFlag) + var revRangeArg string + if len(args) > 0 { + revRangeArg = args[0] + } + return runExplainBranchWithFilter(cmd.Context(), cmd.OutOrStdout(), noPagerFlag, sessionFlag, revRangeArg) }, } diff --git a/cmd/entire/cli/explain.go b/cmd/entire/cli/explain.go index 8ef6ec85da..0016453ac8 100644 --- a/cmd/entire/cli/explain.go +++ b/cmd/entire/cli/explain.go @@ -365,7 +365,7 @@ func runExplain(ctx context.Context, w, errW io.Writer, sessionID, commitRef, ch } // Default or with session filter: show list view (optionally filtered by session) - return runExplainBranchWithFilter(ctx, w, noPager, sessionID) + return runExplainBranchWithFilter(ctx, w, noPager, sessionID, "") } // runExplainAuto resolves a positional target as either a checkpoint ID @@ -629,7 +629,7 @@ func runExplainCheckpointWithLookup(ctx context.Context, w, errW io.Writer, chec } // Find associated commits (git commits with matching Entire-Checkpoint trailer) - associatedCommits, _ := getAssociatedCommits(ctx, lookup.repo, fullCheckpointID, searchAll) //nolint:errcheck // Best-effort + associatedCommits, _ := getAssociatedCommits(ctx, lookup.repo, fullCheckpointID, searchAll, errW) //nolint:errcheck // Best-effort // Derive author from the first associated commit (the user who made the commit). // Fall back to GetCheckpointAuthor (walks entire/checkpoints/v1) for checkpoints @@ -1386,82 +1386,101 @@ func explainTemporaryCheckpoint(ctx context.Context, w, errW io.Writer, repo *gi return sb.String(), true, nil } -// getAssociatedCommits finds git commits that reference the given checkpoint ID. -// Searches commits on the current branch for Entire-Checkpoint trailer matches. -// When searchAll is true, uses full DAG walk with no depth limit (may be slow). -// This finds checkpoint commits on merged feature branches (second parents of merges). -func getAssociatedCommits(ctx context.Context, repo *git.Repository, checkpointID id.CheckpointID, searchAll bool) ([]associatedCommit, error) { +// getAssociatedCommits finds git commits whose Entire-Checkpoint trailer +// matches the given checkpoint ID, walking the full DAG from HEAD so +// checkpoints on merged feature branches are visible. +// +// Default behavior (searchAll=false): walk capped at commitScanLimit. If the +// cap is reached with zero matches, it auto-falls back to an uncapped walk +// and writes a one-line note to errW (when non-nil). +// +// When searchAll is true, the bounded pass is skipped and the uncapped walk +// runs immediately — useful when the caller already knows the commit is +// older than the cap. +func getAssociatedCommits(ctx context.Context, repo *git.Repository, checkpointID id.CheckpointID, searchAll bool, errW io.Writer) ([]associatedCommit, error) { + return getAssociatedCommitsWithLimit(ctx, repo, checkpointID, searchAll, errW, commitScanLimit) +} + +// getAssociatedCommitsWithLimit is the test seam for getAssociatedCommits. +// `limit` controls the bounded-pass cap; pass 0 to disable the cap entirely +// (equivalent to searchAll=true). +func getAssociatedCommitsWithLimit(ctx context.Context, repo *git.Repository, checkpointID id.CheckpointID, searchAll bool, errW io.Writer, limit int) ([]associatedCommit, error) { head, err := repo.Head() if err != nil { return nil, fmt.Errorf("failed to get HEAD: %w", err) } - - commits := []associatedCommit{} // Initialize as empty slice, not nil (nil means "not searched") targetID := checkpointID.String() - collectCommit := func(c *object.Commit) { - fullSHA := c.Hash.String() - shortSHA := fullSHA - if len(fullSHA) >= 7 { - shortSHA = fullSHA[:7] - } - commits = append(commits, associatedCommit{ - SHA: fullSHA, - ShortSHA: shortSHA, - Message: strings.Split(c.Message, "\n")[0], - Author: c.Author.Name, - Email: c.Author.Email, - Date: c.Author.When, - }) + if searchAll || limit <= 0 { + commits, _, err := walkDAGForCheckpointCommits(ctx, repo, head.Hash(), targetID, 0) + return commits, err } - if searchAll { - // Full DAG walk: follows all parents of merge commits, no depth limit. - // This finds checkpoint commits on merged feature branches. - iter, iterErr := repo.Log(&git.LogOptions{ - From: head.Hash(), - Order: git.LogOrderCommitterTime, - }) - if iterErr != nil { - return nil, fmt.Errorf("failed to get commit log: %w", iterErr) - } - defer iter.Close() + commits, capReached, err := walkDAGForCheckpointCommits(ctx, repo, head.Hash(), targetID, limit) + if err != nil { + return nil, err + } + if len(commits) > 0 || !capReached { + return commits, nil + } - err = iter.ForEach(func(c *object.Commit) error { - if err := ctx.Err(); err != nil { - return err //nolint:wrapcheck // Propagating context cancellation - } - cpID, found := trailers.ParseCheckpoint(c.Message) - if found && cpID.String() == targetID { - collectCommit(c) - } - return nil - }) - } else { - // First-parent walk with depth limit and branch filtering. - // Avoids walking into main's history through merge commit parents. - reachableFromMain := computeReachableFromMain(ctx, repo) - - err = walkFirstParentCommits(ctx, repo, head.Hash(), commitScanLimit, func(c *object.Commit) error { - // Once we hit a commit reachable from main on the first-parent chain, - // all earlier ancestors are also shared-with-main, so stop scanning. - if reachableFromMain[c.Hash] { - return errStopIteration - } + if errW != nil { + fmt.Fprintf(errW, "Searching full history (no match in last %d commits)…\n", limit) + } + commits, _, err = walkDAGForCheckpointCommits(ctx, repo, head.Hash(), targetID, 0) + return commits, err +} - cpID, found := trailers.ParseCheckpoint(c.Message) - if found && cpID.String() == targetID { - collectCommit(c) - } - return nil - }) +// walkDAGForCheckpointCommits walks the DAG from `head`, collecting commits +// whose Entire-Checkpoint trailer matches `targetID`. When `limit` > 0 the +// walk is capped at that many commits and capReached reports whether the +// cap was hit. When `limit` <= 0 the walk is unbounded and capReached is +// always false. +func walkDAGForCheckpointCommits(ctx context.Context, repo *git.Repository, head plumbing.Hash, targetID string, limit int) (commits []associatedCommit, capReached bool, err error) { + commits = []associatedCommit{} // empty (not nil) so callers can distinguish "searched, none" from "not searched" + + iter, iterErr := repo.Log(&git.LogOptions{ + From: head, + Order: git.LogOrderCommitterTime, + }) + if iterErr != nil { + return nil, false, fmt.Errorf("failed to get commit log: %w", iterErr) } + defer iter.Close() + + count := 0 + err = iter.ForEach(func(c *object.Commit) error { + if err := ctx.Err(); err != nil { + return err //nolint:wrapcheck // Propagating context cancellation + } + if limit > 0 && count >= limit { + capReached = true + return storer.ErrStop + } + count++ + cpID, found := trailers.ParseCheckpoint(c.Message) + if found && cpID.String() == targetID { + fullSHA := c.Hash.String() + shortSHA := fullSHA + if len(fullSHA) >= 7 { + shortSHA = fullSHA[:7] + } + commits = append(commits, associatedCommit{ + SHA: fullSHA, + ShortSHA: shortSHA, + Message: strings.Split(c.Message, "\n")[0], + Author: c.Author.Name, + Email: c.Author.Email, + Date: c.Author.When, + }) + } + return nil + }) if err != nil { - return nil, fmt.Errorf("error iterating commits: %w", err) + return nil, false, fmt.Errorf("error iterating commits: %w", err) } - - return commits, nil + return commits, capReached, nil } // scopeTranscriptForCheckpoint slices a transcript to include only the portion @@ -1975,9 +1994,6 @@ const branchCheckpointsLimit = 100 // commitScanLimit is how far back to scan git history for checkpoints const commitScanLimit = 500 -// errStopIteration is used to stop commit iteration early -var errStopIteration = errors.New("stop iteration") - // getCurrentWorktreeHash returns the hashed worktree ID for the current working directory. // This is used to filter shadow branches to only those belonging to this worktree. func getCurrentWorktreeHash(ctx context.Context) string { @@ -1992,93 +2008,155 @@ func getCurrentWorktreeHash(ctx context.Context) string { return checkpoint.HashWorktreeID(worktreeID) } -// computeReachableFromMain returns a set of commit hashes on the main/default branch's first-parent chain. -// On the default branch itself, returns an empty map (no filtering needed). -// Only first-parent commits are included — commits from side branches merged into main are excluded, -// since those could be feature branch commits that shouldn't be filtered out. -func computeReachableFromMain(ctx context.Context, repo *git.Repository) map[plumbing.Hash]bool { - reachableFromMain := make(map[plumbing.Hash]bool) - - isOnDefault, _ := strategy.IsOnDefaultBranch(repo) - if isOnDefault { - return reachableFromMain // No filtering needed on default branch +// buildReachableSet walks the full DAG from `from` and returns the set of +// visited commit hashes. `limit` <= 0 means no cap (the same convention as +// walkFirstParentCommits). When `from` is the zero hash, returns an empty +// (non-nil) set. Returns an error if `repo.Log` fails to start or the +// context is cancelled; iteration errors after the walk has begun yield a +// partial set. +func buildReachableSet(ctx context.Context, repo *git.Repository, from plumbing.Hash, limit int) (map[plumbing.Hash]bool, error) { + set := make(map[plumbing.Hash]bool) + if from == plumbing.ZeroHash { + return set, nil } - - // Resolve main branch hash - var mainBranchHash plumbing.Hash - if defaultBranchName := strategy.GetDefaultBranchName(repo); defaultBranchName != "" { - ref, refErr := repo.Reference(plumbing.ReferenceName("refs/heads/"+defaultBranchName), true) - if refErr != nil { - ref, refErr = repo.Reference(plumbing.ReferenceName("refs/remotes/origin/"+defaultBranchName), true) + iter, err := repo.Log(&git.LogOptions{From: from, Order: git.LogOrderCommitterTime}) + if err != nil { + return nil, fmt.Errorf("log from %s: %w", from, err) + } + defer iter.Close() + count := 0 + err = iter.ForEach(func(c *object.Commit) error { + if err := ctx.Err(); err != nil { + return err //nolint:wrapcheck // Propagating context cancellation } - if refErr == nil { - mainBranchHash = ref.Hash() + if limit > 0 && count >= limit { + return storer.ErrStop } - } - if mainBranchHash == plumbing.ZeroHash { - mainBranchHash = strategy.GetMainBranchHash(repo) - } - if mainBranchHash == plumbing.ZeroHash { - return reachableFromMain - } - - // Walk main's first-parent chain to build the set - _ = walkFirstParentCommits(ctx, repo, mainBranchHash, strategy.MaxCommitTraversalDepth, func(c *object.Commit) error { //nolint:errcheck // Best-effort - reachableFromMain[c.Hash] = true + count++ + set[c.Hash] = true return nil }) - - return reachableFromMain + if err != nil && !errors.Is(err, ctx.Err()) { + logging.Debug(ctx, "buildReachableSet: iteration error", slog.String("error", err.Error())) + return set, nil + } + if err := ctx.Err(); err != nil { + return nil, err //nolint:wrapcheck // Propagating context cancellation + } + return set, nil } -// walkFirstParentCommits walks the first-parent chain starting from `from`, -// calling fn for each commit. It stops after visiting `limit` commits (0 = no limit). -// This avoids the full DAG traversal that repo.Log() does, which follows ALL parents -// of merge commits and can walk into unrelated branch history (e.g., main's full -// history after merging main into a feature branch). -func walkFirstParentCommits(ctx context.Context, repo *git.Repository, from plumbing.Hash, limit int, fn func(*object.Commit) error) error { - current, err := repo.CommitObject(from) - if err != nil { - return fmt.Errorf("failed to get commit %s: %w", from, err) +// parseCheckpointRevRange parses a git-style revision range and resolves it +// against the repository. Supported forms: +// +// "" → from=HEAD, excludeFrom=zero (full reachable from HEAD) +// "" → from=, excludeFrom=zero +// "A..B" → from=B, excludeFrom=A +// "A.." → from=HEAD, excludeFrom=A +// "..B" → from=B, excludeFrom=zero +// +// Symmetric difference (A...B) is rejected as unsupported. +func parseCheckpointRevRange(repo *git.Repository, arg string) (checkpointWalkRange, error) { + var r checkpointWalkRange + resolveHEAD := func() (plumbing.Hash, error) { + head, err := repo.Head() + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("resolve HEAD: %w", err) + } + return head.Hash(), nil + } + resolveRev := func(ref string) (plumbing.Hash, error) { + h, err := repo.ResolveRevision(plumbing.Revision(ref)) + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("resolve %q: %w", ref, err) + } + return *h, nil } - for count := 0; limit <= 0 || count < limit; count++ { - if err := ctx.Err(); err != nil { - return err //nolint:wrapcheck // Propagating context cancellation + if arg == "" { + h, err := resolveHEAD() + if err != nil { + return r, err } - if err := fn(current); err != nil { - if errors.Is(err, errStopIteration) { - return nil - } - return err + r.from = h + return r, nil + } + + if strings.Contains(arg, "...") { + return r, fmt.Errorf("symmetric-difference range %q is not supported; use A..B", arg) + } + + if !strings.Contains(arg, "..") { + h, err := resolveRev(arg) + if err != nil { + return r, err } + r.from = h + return r, nil + } - // Follow first parent only (skip merge parents). - // When there are no parents or parent lookup fails, we've reached the - // end of the chain — this is a normal termination, not an error. - if current.NumParents() == 0 { - return nil + left, right, _ := strings.Cut(arg, "..") + if right == "" { + h, err := resolveHEAD() + if err != nil { + return r, err } - parentHash := current.Hash - current, err = current.Parent(0) + r.from = h + } else { + h, err := resolveRev(right) if err != nil { - return fmt.Errorf("failed to load first parent of commit %s: %w", parentHash, err) + return r, err } + r.from = h } - return nil + if left != "" { + h, err := resolveRev(left) + if err != nil { + return r, err + } + r.excludeFrom = h + } + return r, nil } -// getBranchCheckpoints returns checkpoints relevant to the current branch. -// This is strategy-agnostic - it queries checkpoints directly from the checkpoint store. -// -// Behavior: -// - On feature branches: only show checkpoints unique to this branch (not in main) -// - On default branch (main/master): show all checkpoints in history (up to limit) -// - Includes both committed checkpoints (entire/checkpoints/v1) and temporary checkpoints (shadow branches) +// checkpointWalkRange describes the slice of git history to scan for +// checkpoints. `from` is the right side (commits reachable from here are +// visited); `excludeFrom` is the left side (commits also reachable from +// here are skipped). When excludeFrom is the zero hash, no exclusion is +// applied. Mirrors `git log ..` semantics. +type checkpointWalkRange struct { + from plumbing.Hash + excludeFrom plumbing.Hash +} + +// getBranchCheckpoints returns checkpoints reachable from HEAD, mirroring +// `git log` semantics regardless of branch. See getBranchCheckpointsInRange +// for the underlying walk; this is a thin wrapper that defaults to HEAD +// with no exclusion. func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) ([]strategy.RewindPoint, error) { + head, err := repo.Head() + if err != nil { + if errors.Is(err, plumbing.ErrReferenceNotFound) { + return []strategy.RewindPoint{}, nil + } + return nil, fmt.Errorf("failed to get HEAD: %w", err) + } + return getBranchCheckpointsInRange(ctx, repo, checkpointWalkRange{from: head.Hash()}, limit) +} + +// getBranchCheckpointsInRange returns checkpoints in the given range, walking +// the full DAG (so checkpoints on merged feature branches are included) and +// capping at commitScanLimit. Includes both committed checkpoints +// (entire/checkpoints/v1) and temporary checkpoints from shadow branches +// whose base commit is in the visible reachable set. +func getBranchCheckpointsInRange(ctx context.Context, repo *git.Repository, walk checkpointWalkRange, limit int) ([]strategy.RewindPoint, error) { // Warn (once per process) if metadata branches are disconnected strategy.WarnIfMetadataDisconnected() + if walk.from == plumbing.ZeroHash { + return nil, errors.New("checkpoint walk range: from hash is zero") + } + v1Store := checkpoint.NewGitStore(repo) v2URL, err := remote.FetchURL(ctx) if err != nil { @@ -2104,25 +2182,23 @@ func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) } } - head, err := repo.Head() - if err != nil { - // Unborn HEAD (no commits yet) - return empty list instead of erroring - if errors.Is(err, plumbing.ErrReferenceNotFound) { - return []strategy.RewindPoint{}, nil - } - return nil, fmt.Errorf("failed to get HEAD: %w", err) - } - - // Check if we're on the default branch (needed for getReachableTemporaryCheckpoints) - isOnDefault, _ := strategy.IsOnDefaultBranch(repo) - // Fetch metadata trees for reading session prompts (cheap tree lookups). // Try v2 /main first, fall back to v1 metadata branch. v1MetadataTree, _ := strategy.GetMetadataBranchTree(repo) //nolint:errcheck // Best-effort v2MetadataTree, _ := strategy.GetV2MetadataBranchTree(repo) //nolint:errcheck // Best-effort promptTree := resolvePromptTree(v1MetadataTree, v2MetadataTree, preferCheckpointsV2) + // Build exclude set when a left side was given. + // Uncapped: a left-side history truncated at commitScanLimit would leak + // older left-side commits past the exclude into the visible set. Hashes + // are 20 bytes, so even 100k commits cost ~2MB. + excludeSet, err := buildReachableSet(ctx, repo, walk.excludeFrom, 0) + if err != nil { + return nil, fmt.Errorf("compute exclude set: %w", err) + } + var points []strategy.RewindPoint + reachable := make(map[plumbing.Hash]bool) collectCheckpoint := func(c *object.Commit) { cpID, found := trailers.ParseCheckpoint(c.Message) @@ -2156,52 +2232,44 @@ func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) points = append(points, point) } - if isOnDefault { - // On the default branch, use full DAG walk to find checkpoint commits - // on merged feature branches (second parents of merge commits). - iter, iterErr := repo.Log(&git.LogOptions{ - From: head.Hash(), - Order: git.LogOrderCommitterTime, - }) - if iterErr != nil { - return nil, fmt.Errorf("failed to get commit log: %w", iterErr) - } - defer iter.Close() + // Full DAG walk from `from`, capped at commitScanLimit. Follows merge + // parents so checkpoints on merged feature branches are included. + iter, iterErr := repo.Log(&git.LogOptions{ + From: walk.from, + Order: git.LogOrderCommitterTime, + }) + if iterErr != nil { + return nil, fmt.Errorf("failed to get commit log: %w", iterErr) + } + defer iter.Close() - count := 0 - err = iter.ForEach(func(c *object.Commit) error { - if err := ctx.Err(); err != nil { - return err //nolint:wrapcheck // Propagating context cancellation - } - if count >= commitScanLimit { - return storer.ErrStop - } - count++ - collectCheckpoint(c) - return nil - }) - } else { - // On feature branches, use first-parent walk with branch filtering. - // This avoids walking into main's full history through merge commit parents. - reachableFromMain := computeReachableFromMain(ctx, repo) - - err = walkFirstParentCommits(ctx, repo, head.Hash(), commitScanLimit, func(c *object.Commit) error { - // Once we hit a commit reachable from main on the first-parent chain, - // all earlier ancestors are also shared-with-main, so stop scanning. - if reachableFromMain[c.Hash] { - return errStopIteration - } - collectCheckpoint(c) + // Excluded commits don't consume commitScanLimit: a `main..HEAD` walk on + // a feature branch that merged main would otherwise eat the budget on + // interleaved main commits before reaching older feature checkpoints. + count := 0 + err = iter.ForEach(func(c *object.Commit) error { + if err := ctx.Err(); err != nil { + return err //nolint:wrapcheck // Propagating context cancellation + } + if excludeSet[c.Hash] { return nil - }) - } + } + if count >= commitScanLimit { + return storer.ErrStop + } + count++ + reachable[c.Hash] = true + collectCheckpoint(c) + return nil + }) if err != nil { return nil, fmt.Errorf("error iterating commits: %w", err) } - // Get temporary checkpoints from ALL shadow branches whose base commit is reachable from HEAD. - tempPoints := getReachableTemporaryCheckpoints(ctx, repo, v1Store, head.Hash(), isOnDefault, limit) + // Get temporary checkpoints from shadow branches whose base commit is in + // the reachable set we just built (no extra walk needed). + tempPoints := getReachableTemporaryCheckpoints(ctx, repo, v1Store, reachable, limit) points = append(points, tempPoints...) // Sort by date, most recent first @@ -2217,11 +2285,11 @@ func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) return points, nil } -// getReachableTemporaryCheckpoints returns temporary checkpoints from shadow branches -// whose base commit is reachable from the given HEAD hash and that belong to this worktree. -// For default branches, all shadow branches for this worktree are included. -// For feature branches, only shadow branches whose base commit is in HEAD's history are included. -func getReachableTemporaryCheckpoints(ctx context.Context, repo *git.Repository, store *checkpoint.GitStore, headHash plumbing.Hash, isOnDefault bool, limit int) []strategy.RewindPoint { +// getReachableTemporaryCheckpoints returns temporary checkpoints from shadow +// branches whose base commit is in `reachable` and that belong to this +// worktree. `reachable` is the precomputed full-DAG set built by the caller's +// commit walk. +func getReachableTemporaryCheckpoints(ctx context.Context, repo *git.Repository, store *checkpoint.GitStore, reachable map[plumbing.Hash]bool, limit int) []strategy.RewindPoint { var points []strategy.RewindPoint // Compute current worktree's hash for filtering shadow branches @@ -2237,8 +2305,9 @@ func getReachableTemporaryCheckpoints(ctx context.Context, repo *git.Repository, continue } - // Check if this shadow branch's base commit is reachable from current HEAD - if !isShadowBranchReachable(ctx, repo, sb.BaseCommit, headHash, isOnDefault) { + // Check if this shadow branch's base commit is reachable from HEAD via + // the DAG. sb.BaseCommit is a 7-char prefix from the branch name. + if !isBaseCommitInReachableSet(sb.BaseCommit, reachable) { continue } @@ -2255,26 +2324,19 @@ func getReachableTemporaryCheckpoints(ctx context.Context, repo *git.Repository, return points } -// isShadowBranchReachable checks if a shadow branch's base commit is reachable from HEAD. -// For default branches, all shadow branches are considered reachable. -// For feature branches, we check if any commit with the base commit prefix is in HEAD's history. -func isShadowBranchReachable(ctx context.Context, repo *git.Repository, baseCommit string, headHash plumbing.Hash, isOnDefault bool) bool { - // For default branch: all shadow branches are potentially relevant - if isOnDefault { - return true +// isBaseCommitInReachableSet reports whether any hash in `reachable` has the +// given prefix. Shadow branches store base commit as a 7-char prefix, so we +// can't use a direct map lookup. +func isBaseCommitInReachableSet(baseCommitPrefix string, reachable map[plumbing.Hash]bool) bool { + if baseCommitPrefix == "" { + return false } - - // Check if base commit hash prefix matches any commit in HEAD's first-parent chain - found := false - _ = walkFirstParentCommits(ctx, repo, headHash, commitScanLimit, func(c *object.Commit) error { //nolint:errcheck // Best-effort - if strings.HasPrefix(c.Hash.String(), baseCommit) { - found = true - return errStopIteration + for h := range reachable { + if strings.HasPrefix(h.String(), baseCommitPrefix) { + return true } - return nil - }) - - return found + } + return false } // convertTemporaryCheckpoint converts a TemporaryCheckpointInfo to a RewindPoint. @@ -2318,33 +2380,51 @@ func convertTemporaryCheckpoint(repo *git.Repository, tc checkpoint.TemporaryChe } } -// runExplainBranchWithFilter shows checkpoints on the current branch, optionally filtered by session. +// runExplainBranchWithFilter shows checkpoints reachable from HEAD (or a +// caller-supplied revision range), optionally filtered by session. +// `revRangeArg` is a git-style revision range string; see +// parseCheckpointRevRange for accepted forms. Empty string means "full +// reachable from HEAD". // This is strategy-agnostic - it queries checkpoints directly. -func runExplainBranchWithFilter(ctx context.Context, w io.Writer, noPager bool, sessionFilter string) error { +func runExplainBranchWithFilter(ctx context.Context, w io.Writer, noPager bool, sessionFilter, revRangeArg string) error { repo, err := openRepository(ctx) if err != nil { return fmt.Errorf("not a git repository: %w", err) } - // Get current branch name - branchName := strategy.GetCurrentBranchName(repo) - if branchName == "" { - // Detached HEAD state or unborn HEAD - try to use short commit hash if possible - head, headErr := repo.Head() - if headErr != nil { - // Unborn HEAD (no commits yet) - treat as empty history instead of erroring - if errors.Is(headErr, plumbing.ErrReferenceNotFound) { - branchName = "HEAD (no commits yet)" + // Compute the header label: branch name when no range was given, otherwise + // the range string itself (clearer than the current branch when scoped). + headerLabel := revRangeArg + if headerLabel == "" { + headerLabel = strategy.GetCurrentBranchName(repo) + if headerLabel == "" { + head, headErr := repo.Head() + if headErr != nil { + if errors.Is(headErr, plumbing.ErrReferenceNotFound) { + headerLabel = "HEAD (no commits yet)" + } else { + return fmt.Errorf("failed to get HEAD: %w", headErr) + } } else { - return fmt.Errorf("failed to get HEAD: %w", headErr) + headerLabel = "HEAD (" + head.Hash().String()[:7] + ")" } - } else { - branchName = "HEAD (" + head.Hash().String()[:7] + ")" } } - // Get checkpoints for this branch (strategy-agnostic) - points, err := getBranchCheckpoints(ctx, repo, branchCheckpointsLimit) + walk, err := parseCheckpointRevRange(repo, revRangeArg) + if err != nil { + // Unborn HEAD with empty arg yields a ReferenceNotFound — fall through + // to the empty-list path so users get a helpful message. + if revRangeArg == "" && errors.Is(err, plumbing.ErrReferenceNotFound) { + output := formatBranchCheckpoints(w, headerLabel, nil, sessionFilter) + outputExplainContent(w, output, noPager) + return nil + } + return fmt.Errorf("invalid revision range %q: %w", revRangeArg, err) + } + + // Get checkpoints for the requested range (strategy-agnostic) + points, err := getBranchCheckpointsInRange(ctx, repo, walk, branchCheckpointsLimit) if err != nil { // If context was cancelled (e.g. user hit Ctrl+C), exit silently if ctx.Err() != nil { @@ -2356,16 +2436,16 @@ func runExplainBranchWithFilter(ctx context.Context, w io.Writer, noPager bool, } // Format output - output := formatBranchCheckpoints(w, branchName, points, sessionFilter) + output := formatBranchCheckpoints(w, headerLabel, points, sessionFilter) outputExplainContent(w, output, noPager) return nil } -// runExplainBranchDefault shows all checkpoints on the current branch grouped by date. +// runExplainBranchDefault shows all checkpoints reachable from HEAD grouped by date. // This is a convenience wrapper that calls runExplainBranchWithFilter with no filter. func runExplainBranchDefault(ctx context.Context, w io.Writer, noPager bool) error { - return runExplainBranchWithFilter(ctx, w, noPager, "") + return runExplainBranchWithFilter(ctx, w, noPager, "", "") } // outputExplainContent outputs content with optional pager support. diff --git a/cmd/entire/cli/explain_test.go b/cmd/entire/cli/explain_test.go index e7a009f221..ea6bf51aed 100644 --- a/cmd/entire/cli/explain_test.go +++ b/cmd/entire/cli/explain_test.go @@ -4732,7 +4732,7 @@ func TestGetAssociatedCommits(t *testing.T) { } // Test: should find the one commit with matching checkpoint - commits, err := getAssociatedCommits(context.Background(), repo, checkpointID, false) + commits, err := getAssociatedCommits(context.Background(), repo, checkpointID, false, io.Discard) if err != nil { t.Fatalf("getAssociatedCommits error: %v", err) } @@ -4790,7 +4790,7 @@ func TestGetAssociatedCommits_NoMatches(t *testing.T) { // Search for a checkpoint ID that doesn't exist (valid format: 12 hex chars) checkpointID := id.MustCheckpointID("aaaa11112222") - commits, err := getAssociatedCommits(context.Background(), repo, checkpointID, false) + commits, err := getAssociatedCommits(context.Background(), repo, checkpointID, false, io.Discard) if err != nil { t.Fatalf("getAssociatedCommits error: %v", err) } @@ -4874,7 +4874,7 @@ func TestGetAssociatedCommits_MultipleMatches(t *testing.T) { } // Test: should find both commits with matching checkpoint - commits, err := getAssociatedCommits(context.Background(), repo, checkpointID, false) + commits, err := getAssociatedCommits(context.Background(), repo, checkpointID, false, io.Discard) if err != nil { t.Fatalf("getAssociatedCommits error: %v", err) } @@ -5110,7 +5110,7 @@ func TestGetBranchCheckpoints_WithMergeFromMain(t *testing.T) { // Test getAssociatedCommits - should find BOTH feature checkpoint commits // by walking first-parent chain (skipping the merge's second parent into main) - commits1, err := getAssociatedCommits(context.Background(), repo, cpID1, false) + commits1, err := getAssociatedCommits(context.Background(), repo, cpID1, false, io.Discard) if err != nil { t.Fatalf("getAssociatedCommits for cpID1 error: %v", err) } @@ -5118,7 +5118,7 @@ func TestGetBranchCheckpoints_WithMergeFromMain(t *testing.T) { t.Errorf("expected 1 commit for cpID1 (first feature checkpoint), got %d", len(commits1)) } - commits2, err := getAssociatedCommits(context.Background(), repo, cpID2, false) + commits2, err := getAssociatedCommits(context.Background(), repo, cpID2, false, io.Discard) if err != nil { t.Fatalf("getAssociatedCommits for cpID2 error: %v", err) } @@ -5235,7 +5235,7 @@ func TestGetBranchCheckpoints_MergeCommitAtHEAD(t *testing.T) { // HEAD is the merge commit itself. // getAssociatedCommits should walk: merge -> featureCommit -> initial // and find the checkpoint on featureCommit. - commits, err := getAssociatedCommits(context.Background(), repo, cpID, false) + commits, err := getAssociatedCommits(context.Background(), repo, cpID, false, io.Discard) if err != nil { t.Fatalf("getAssociatedCommits error: %v", err) } @@ -5247,124 +5247,6 @@ func TestGetBranchCheckpoints_MergeCommitAtHEAD(t *testing.T) { } } -func TestWalkFirstParentCommits_SkipsMergeParents(t *testing.T) { - // Verify that walkFirstParentCommits follows only first parents and doesn't - // enter the second parent (merge source) of merge commits. - tmpDir := t.TempDir() - t.Chdir(tmpDir) - - repo, err := git.PlainInit(tmpDir, false) - if err != nil { - t.Fatalf("failed to init git repo: %v", err) - } - - w, err := repo.Worktree() - if err != nil { - t.Fatalf("failed to get worktree: %v", err) - } - - // Create initial commit (shared ancestor) - testFile := filepath.Join(tmpDir, "test.txt") - if err := os.WriteFile(testFile, []byte("initial"), 0o644); err != nil { - t.Fatalf("failed to write test file: %v", err) - } - if _, err := w.Add("test.txt"); err != nil { - t.Fatalf("failed to add test file: %v", err) - } - initialCommit, err := w.Commit("A: initial", &git.CommitOptions{ - Author: &object.Signature{Name: "Test", Email: "test@example.com", When: time.Now().Add(-5 * time.Hour)}, - }) - if err != nil { - t.Fatalf("failed to create initial commit: %v", err) - } - - // Create feature branch with one commit - featureBranch := plumbing.NewBranchReferenceName("feature/walk-test") - if err := w.Checkout(&git.CheckoutOptions{ - Hash: initialCommit, - Branch: featureBranch, - Create: true, - }); err != nil { - t.Fatalf("failed to create feature branch: %v", err) - } - if err := os.WriteFile(testFile, []byte("feature"), 0o644); err != nil { - t.Fatalf("failed to write test file: %v", err) - } - if _, err := w.Add("test.txt"); err != nil { - t.Fatalf("failed to add test file: %v", err) - } - featureCommit, err := w.Commit("B: feature work", &git.CommitOptions{ - Author: &object.Signature{Name: "Test", Email: "test@example.com", When: time.Now().Add(-4 * time.Hour)}, - }) - if err != nil { - t.Fatalf("failed to create feature commit: %v", err) - } - - // Create main branch commit (will be merge source) - if err := w.Checkout(&git.CheckoutOptions{ - Branch: plumbing.NewBranchReferenceName("master"), - }); err != nil { - t.Fatalf("failed to checkout master: %v", err) - } - mainFile := filepath.Join(tmpDir, "main.txt") - if err := os.WriteFile(mainFile, []byte("main"), 0o644); err != nil { - t.Fatalf("failed to write main file: %v", err) - } - if _, err := w.Add("main.txt"); err != nil { - t.Fatalf("failed to add main file: %v", err) - } - mainCommit, err := w.Commit("C: main work", &git.CommitOptions{ - Author: &object.Signature{Name: "Test", Email: "test@example.com", When: time.Now().Add(-3 * time.Hour)}, - }) - if err != nil { - t.Fatalf("failed to create main commit: %v", err) - } - - // Switch to feature and create merge commit - if err := w.Checkout(&git.CheckoutOptions{ - Branch: featureBranch, - }); err != nil { - t.Fatalf("failed to checkout feature: %v", err) - } - featureCommitObj, commitObjErr := repo.CommitObject(featureCommit) - if commitObjErr != nil { - t.Fatalf("failed to get feature commit object: %v", commitObjErr) - } - featureTree, treeErr := featureCommitObj.Tree() - if treeErr != nil { - t.Fatalf("failed to get feature commit tree: %v", treeErr) - } - mergeHash := createMergeCommit(t, repo, featureCommit, mainCommit, featureTree.Hash, "M: merge main into feature") - - // Walk should visit: M (merge) -> B (feature) -> A (initial) - // It should NOT visit C (main work), because that's the second parent of the merge. - var visited []string - err = walkFirstParentCommits(context.Background(), repo, mergeHash, 0, func(c *object.Commit) error { - visited = append(visited, strings.Split(c.Message, "\n")[0]) - return nil - }) - if err != nil { - t.Fatalf("walkFirstParentCommits error: %v", err) - } - - expected := []string{"M: merge main into feature", "B: feature work", "A: initial"} - if len(visited) != len(expected) { - t.Fatalf("expected %d commits visited, got %d: %v", len(expected), len(visited), visited) - } - for i, msg := range expected { - if visited[i] != msg { - t.Errorf("commit %d: expected %q, got %q", i, msg, visited[i]) - } - } - - // Verify C was NOT visited - for _, msg := range visited { - if strings.Contains(msg, "C: main work") { - t.Error("walkFirstParentCommits visited main branch commit (second parent of merge) - should only follow first parents") - } - } -} - func TestFormatCheckpointOutput_NoCommitsOnBranch(t *testing.T) { summary := &checkpoint.CheckpointSummary{ CheckpointID: id.MustCheckpointID("abc123def456"), @@ -5393,11 +5275,11 @@ func TestFormatCheckpointOutput_NoCommitsOnBranch(t *testing.T) { } } -func TestGetAssociatedCommits_SearchAllFindsMergedBranchCommits(t *testing.T) { - // Regression test: --search-all should find checkpoint commits that live on - // a feature branch merged into main via a true merge commit. These commits - // are on the second parent of the merge, so first-parent-only traversal - // won't find them — but --search-all should use full DAG walk. +func TestGetAssociatedCommits_FindsMergedBranchCommits(t *testing.T) { + // Regression test: getAssociatedCommits walks the full DAG by default, so + // checkpoint commits that live on a feature branch merged into main via a + // true merge commit are found whether or not --search-all is set. The + // commit is on the second parent of the merge. tmpDir := t.TempDir() t.Chdir(tmpDir) @@ -5484,18 +5366,21 @@ func TestGetAssociatedCommits_SearchAllFindsMergedBranchCommits(t *testing.T) { t.Fatalf("failed to set HEAD: %v", err) } - // Without --search-all (first-parent only): should NOT find the feature commit - // because it's on the second parent of the merge - commits, err := getAssociatedCommits(context.Background(), repo, checkpointID, false) + // Default search (capped DAG walk): finds the feature commit because the + // walk follows merge parents. + commits, err := getAssociatedCommits(context.Background(), repo, checkpointID, false, io.Discard) if err != nil { t.Fatalf("getAssociatedCommits error: %v", err) } - if len(commits) != 0 { - t.Errorf("expected 0 commits without --search-all (first-parent only), got %d", len(commits)) + if len(commits) != 1 { + t.Fatalf("expected 1 commit by default (DAG walk), got %d", len(commits)) + } + if commits[0].Author != "Feature Dev" { + t.Errorf("expected author 'Feature Dev', got %q", commits[0].Author) } - // With --search-all (full DAG walk): SHOULD find the feature commit - commits, err = getAssociatedCommits(context.Background(), repo, checkpointID, true) + // --search-all (uncapped DAG walk): also finds it. + commits, err = getAssociatedCommits(context.Background(), repo, checkpointID, true, io.Discard) if err != nil { t.Fatalf("getAssociatedCommits --search-all error: %v", err) } @@ -5507,6 +5392,84 @@ func TestGetAssociatedCommits_SearchAllFindsMergedBranchCommits(t *testing.T) { } } +func TestGetAssociatedCommits_FallbackBeyondCap(t *testing.T) { + // When the matching checkpoint is older than the bounded cap, the default + // path should auto-fall back to an uncapped walk, write a note to errW, + // and still return the match. + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + testutil.InitRepo(t, tmpDir) + repo, err := git.PlainOpen(tmpDir) + require.NoError(t, err) + + w, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + + checkpointID := id.MustCheckpointID("aaa111bbb222") + testFile := filepath.Join(tmpDir, "test.txt") + + // First commit carries the checkpoint trailer; later commits bury it + // past the cap we'll set below. + if err := os.WriteFile(testFile, []byte("initial"), 0o644); err != nil { + t.Fatalf("failed to write file: %v", err) + } + if _, err := w.Add("test.txt"); err != nil { + t.Fatalf("failed to add file: %v", err) + } + _, err = w.Commit(trailers.FormatCheckpoint("feat: target", checkpointID), &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com", When: time.Now().Add(-10 * time.Hour)}, + }) + if err != nil { + t.Fatalf("failed to create target commit: %v", err) + } + + for i := range 5 { + if err := os.WriteFile(testFile, []byte(fmt.Sprintf("noise-%d", i)), 0o644); err != nil { + t.Fatalf("failed to write file: %v", err) + } + if _, err := w.Add("test.txt"); err != nil { + t.Fatalf("failed to add file: %v", err) + } + _, err = w.Commit(fmt.Sprintf("noise %d", i), &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com", When: time.Now().Add(-time.Duration(9-i) * time.Hour)}, + }) + if err != nil { + t.Fatalf("failed to create noise commit %d: %v", i, err) + } + } + + // Cap of 3 means the walk visits the 3 most-recent noise commits and + // stops before reaching the trailered commit at the bottom. + var errBuf bytes.Buffer + commits, err := getAssociatedCommitsWithLimit(context.Background(), repo, checkpointID, false, &errBuf, 3) + if err != nil { + t.Fatalf("getAssociatedCommitsWithLimit error: %v", err) + } + if len(commits) != 1 { + t.Fatalf("expected fallback to find 1 commit, got %d", len(commits)) + } + if !strings.Contains(errBuf.String(), "Searching full history") { + t.Errorf("expected fallback note on errW, got %q", errBuf.String()) + } + + // When the cap is large enough to include the target on the first pass, + // no fallback should fire. + errBuf.Reset() + commits, err = getAssociatedCommitsWithLimit(context.Background(), repo, checkpointID, false, &errBuf, 100) + if err != nil { + t.Fatalf("getAssociatedCommitsWithLimit error: %v", err) + } + if len(commits) != 1 { + t.Fatalf("expected 1 commit within cap, got %d", len(commits)) + } + if errBuf.Len() != 0 { + t.Errorf("expected no fallback note, got %q", errBuf.String()) + } +} + func TestGetBranchCheckpoints_DefaultBranchFindsMergedCheckpoints(t *testing.T) { // Regression test: on the default branch, getBranchCheckpoints should find // checkpoint commits that came in via merge commits (second parents). @@ -5619,6 +5582,170 @@ func TestGetBranchCheckpoints_DefaultBranchFindsMergedCheckpoints(t *testing.T) } } +func TestGetBranchCheckpointsInRange_ExcludesLeftSide(t *testing.T) { + // End-to-end check that getBranchCheckpointsInRange honors the exclude + // set: a range A..B drops checkpoints that are reachable from A while + // still collecting checkpoints reachable from B but not A. + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + testutil.InitRepo(t, tmpDir) + repo, err := git.PlainOpen(tmpDir) + require.NoError(t, err) + + w, err := repo.Worktree() + if err != nil { + t.Fatalf("worktree: %v", err) + } + + testFile := filepath.Join(tmpDir, "test.txt") + mustCommit := func(content, msg string, author time.Time) plumbing.Hash { + t.Helper() + require.NoError(t, os.WriteFile(testFile, []byte(content), 0o644)) + _, err := w.Add("test.txt") + require.NoError(t, err) + hash, err := w.Commit(msg, &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com", When: author}, + }) + require.NoError(t, err) + return hash + } + + now := time.Now() + // Linear chain: A (no trailer) <- B (cpB) <- C (cpC). + commitA := mustCommit("a", "initial", now.Add(-3*time.Hour)) + cpB := id.MustCheckpointID("bbbbbbbbbbbb") + commitB := mustCommit("b", trailers.FormatCheckpoint("feat: B", cpB), now.Add(-2*time.Hour)) + cpC := id.MustCheckpointID("cccccccccccc") + commitC := mustCommit("c", trailers.FormatCheckpoint("feat: C", cpC), now.Add(-1*time.Hour)) + + store := checkpoint.NewGitStore(repo) + for _, write := range []checkpoint.WriteCommittedOptions{ + {CheckpointID: cpB, SessionID: "sess-b", Strategy: "manual-commit", FilesTouched: []string{"test.txt"}, Prompts: []string{"add B"}}, + {CheckpointID: cpC, SessionID: "sess-c", Strategy: "manual-commit", FilesTouched: []string{"test.txt"}, Prompts: []string{"add C"}}, + } { + require.NoError(t, store.WriteCommitted(context.Background(), write)) + } + + hasCheckpoint := func(points []strategy.RewindPoint, cpID id.CheckpointID) bool { + for _, p := range points { + if p.CheckpointID == cpID { + return true + } + } + return false + } + + cases := []struct { + name string + walk checkpointWalkRange + wantContains []id.CheckpointID + wantNotContains []id.CheckpointID + }{ + { + name: "full reach from C: finds B and C", + walk: checkpointWalkRange{from: commitC}, + wantContains: []id.CheckpointID{cpB, cpC}, + }, + { + name: "C excluding A: A has no checkpoint, B and C still visible", + walk: checkpointWalkRange{from: commitC, excludeFrom: commitA}, + wantContains: []id.CheckpointID{cpB, cpC}, + }, + { + name: "C excluding B: drops B (and ancestors), keeps C", + walk: checkpointWalkRange{from: commitC, excludeFrom: commitB}, + wantContains: []id.CheckpointID{cpC}, + wantNotContains: []id.CheckpointID{cpB}, + }, + { + name: "B alone: C unreachable", + walk: checkpointWalkRange{from: commitB}, + wantContains: []id.CheckpointID{cpB}, + wantNotContains: []id.CheckpointID{cpC}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + points, err := getBranchCheckpointsInRange(context.Background(), repo, tc.walk, 100) + if err != nil { + t.Fatalf("getBranchCheckpointsInRange: %v", err) + } + for _, cp := range tc.wantContains { + if !hasCheckpoint(points, cp) { + t.Errorf("expected %s in result, got %v", cp, points) + } + } + for _, cp := range tc.wantNotContains { + if hasCheckpoint(points, cp) { + t.Errorf("did not expect %s in result, got %v", cp, points) + } + } + }) + } +} + +func TestGetBranchCheckpointsInRange_ExcludedCommitsDoNotConsumeBudget(t *testing.T) { + // Regression: excluded commits must not count toward commitScanLimit. We + // build a chain longer than the cap and put the trailered target on top — + // if the budget bug returns, the target gets skipped because the cap is + // exhausted by excluded ancestors. + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + testutil.InitRepo(t, tmpDir) + repo, err := git.PlainOpen(tmpDir) + require.NoError(t, err) + + // Use plumbing-level commit creation to avoid the cost of + // commitScanLimit+5 worktree checkouts. + w, err := repo.Worktree() + require.NoError(t, err) + testFile := filepath.Join(tmpDir, "test.txt") + require.NoError(t, os.WriteFile(testFile, []byte("seed"), 0o644)) + _, err = w.Add("test.txt") + require.NoError(t, err) + seedHash, err := w.Commit("seed", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com", When: time.Now()}, + }) + require.NoError(t, err) + seedCommit, err := repo.CommitObject(seedHash) + require.NoError(t, err) + seedTree, err := seedCommit.Tree() + require.NoError(t, err) + + leftTip := seedHash + for i := range commitScanLimit + 5 { + leftTip = createCommitWithTree(t, repo, seedTree.Hash, []plumbing.Hash{leftTip}, fmt.Sprintf("noise %d", i)) + } + cpID := id.MustCheckpointID("ddddeeeeffff") + rightTip := createCommitWithTree(t, repo, seedTree.Hash, []plumbing.Hash{leftTip}, trailers.FormatCheckpoint("feat: target", cpID)) + + store := checkpoint.NewGitStore(repo) + require.NoError(t, store.WriteCommitted(context.Background(), checkpoint.WriteCommittedOptions{ + CheckpointID: cpID, + SessionID: "sess-target", + Strategy: "manual-commit", + FilesTouched: []string{"test.txt"}, + Prompts: []string{"add target"}, + })) + + walk := checkpointWalkRange{from: rightTip, excludeFrom: leftTip} + points, err := getBranchCheckpointsInRange(context.Background(), repo, walk, 100) + require.NoError(t, err) + var found bool + for _, p := range points { + if p.CheckpointID == cpID { + found = true + break + } + } + if !found { + t.Errorf("expected to find target checkpoint past the exclude set, got %d points: %v", len(points), points) + } +} + func TestGetBranchCheckpoints_ReadsPromptFromCommittedCheckpoint(t *testing.T) { // Verifies that getBranchCheckpoints populates RewindPoint.SessionPrompt // from prompt.txt on entire/checkpoints/v1 (committed checkpoint) without @@ -6148,3 +6275,92 @@ func TestRenderExplainBody_NoColorReturnsRawMarkdown(t *testing.T) { t.Errorf("expected raw markdown when no color\n got: %q", got) } } + +func TestParseCheckpointRevRange(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + testutil.InitRepo(t, tmpDir) + repo, err := git.PlainOpen(tmpDir) + require.NoError(t, err) + + w, err := repo.Worktree() + if err != nil { + t.Fatalf("worktree: %v", err) + } + + // Two commits on the default branch ("master" by default for go-git). + first := filepath.Join(tmpDir, "first.txt") + if err := os.WriteFile(first, []byte("a"), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + if _, err := w.Add("first.txt"); err != nil { + t.Fatalf("add: %v", err) + } + commitA, err := w.Commit("A", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com", When: time.Now().Add(-2 * time.Hour)}, + }) + if err != nil { + t.Fatalf("commit A: %v", err) + } + if err := os.WriteFile(first, []byte("b"), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + if _, err := w.Add("first.txt"); err != nil { + t.Fatalf("add: %v", err) + } + commitB, err := w.Commit("B", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com", When: time.Now().Add(-1 * time.Hour)}, + }) + if err != nil { + t.Fatalf("commit B: %v", err) + } + + defaultBranch := strategy.GetCurrentBranchName(repo) + if defaultBranch == "" { + t.Fatalf("could not resolve current branch") + } + + cases := []struct { + name string + arg string + wantFrom plumbing.Hash + wantExclude plumbing.Hash + wantErr bool + errSubstring string + }{ + {name: "empty defaults to HEAD", arg: "", wantFrom: commitB}, + {name: "single ref", arg: defaultBranch, wantFrom: commitB}, + {name: "single hash", arg: commitA.String(), wantFrom: commitA}, + {name: "A..B", arg: commitA.String() + ".." + commitB.String(), wantFrom: commitB, wantExclude: commitA}, + {name: "A.. shortcut", arg: commitA.String() + "..", wantFrom: commitB, wantExclude: commitA}, + {name: "..B shortcut", arg: ".." + commitB.String(), wantFrom: commitB}, + {name: "symmetric difference rejected", arg: commitA.String() + "..." + commitB.String(), wantErr: true, errSubstring: "symmetric-difference"}, + {name: "unknown ref rejected", arg: "no-such-ref", wantErr: true, errSubstring: "resolve"}, + {name: "unknown left side rejected", arg: "no-such-ref..HEAD", wantErr: true, errSubstring: "resolve"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r, err := parseCheckpointRevRange(repo, tc.arg) + if tc.wantErr { + if err == nil { + t.Fatalf("expected error, got nil") + } + if tc.errSubstring != "" && !strings.Contains(err.Error(), tc.errSubstring) { + t.Errorf("error %q missing %q", err.Error(), tc.errSubstring) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if r.from != tc.wantFrom { + t.Errorf("from: got %s, want %s", r.from, tc.wantFrom) + } + if r.excludeFrom != tc.wantExclude { + t.Errorf("excludeFrom: got %s, want %s", r.excludeFrom, tc.wantExclude) + } + }) + } +}