diff --git a/cmd/entire/cli/explain.go b/cmd/entire/cli/explain.go index fc286557b1..0145a5c470 100644 --- a/cmd/entire/cli/explain.go +++ b/cmd/entire/cli/explain.go @@ -13,6 +13,7 @@ import ( "sort" "strconv" "strings" + "sync/atomic" "time" "github.com/entireio/cli/cmd/entire/cli/agent" @@ -1388,17 +1389,14 @@ func getAssociatedCommits(ctx context.Context, repo *git.Repository, checkpointI 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 - } - + // Branch-scoped walk of this branch's own history — first-parent spine + // plus any side branch merged into it — excluding the default branch's + // history. Follows merge commits' second parents so a checkpoint on a + // feature branch merged INTO this branch is still found (issue #931); a + // first-parent-only walk would miss it. + mainReach := computeReachableFromMain(ctx, repo) + + err = walkBranchOwnCommits(ctx, repo, head.Hash(), mainReach, commitScanLimit, func(c *object.Commit) error { cpID, found := trailers.ParseCheckpoint(c.Message) if found && cpID.String() == targetID { collectCommit(c) @@ -1950,16 +1948,75 @@ 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) +// mainSpineScanLimit bounds how many commits of the default branch's first-parent +// chain computeReachableFromMain scans. It caps the read-path cost: an unbounded +// scan is multiple seconds on a repo whose main has tens of thousands of commits, +// and this runs synchronously on every checkpoint list/explain. Beyond this many +// commits the shared-with-main test falls back to a commit-date frontier +// (see mainReachability.sharedWithMain), so main's older history is still excluded. +const mainSpineScanLimit = strategy.MaxCommitTraversalDepth + +// computeReachableFromMainCalls counts computeReachableFromMain invocations. Tests +// use it to assert the scan runs once per command (not once per checkpoint). +var computeReachableFromMainCalls atomic.Int64 + +// mainReachability answers "is this commit shared with the default branch's own +// (first-parent) history?" — the pruning test used by walkBranchOwnCommits. +// +// onSpine holds the default branch's first-parent commits up to mainSpineScanLimit. +// Only first-parent commits are included — commits from side branches merged into +// main are deliberately excluded, so a feature merged into main and then contained +// by another branch still shows its checkpoints there. +// +// When the scan hit the limit before reaching main's root (reachedRoot == false), +// onSpine is incomplete, so any commit at or before the oldest scanned main commit +// (frontierTime) is treated as shared. This is the fail-safe for the deep-history +// case: a former main tip merged into the branch beyond the scanned window is older +// than the frontier and is pruned, so main's older checkpoints never leak — at the +// cost of possibly hiding a branch-unique commit older than the frontier (the safe +// direction). It mirrors the commit-date boundary git's own rev-list uses to +// compute `main..HEAD`. +type mainReachability struct { + onSpine map[plumbing.Hash]bool + reachedRoot bool + frontierTime time.Time // committer time of the oldest scanned main commit; used only when !reachedRoot +} + +// sharedWithMain reports whether commit h belongs to the default branch's history +// and must therefore be excluded from the current branch's own commits. +func (m *mainReachability) sharedWithMain(repo *git.Repository, h plumbing.Hash) bool { + if m.onSpine[h] { + return true + } + if m.reachedRoot { + return false + } + // The first-parent scan was truncated at mainSpineScanLimit. Fall back to the + // commit-date frontier: treat anything at or before the oldest scanned main + // commit as main's history (fail-safe — exclude rather than risk leaking). + c, err := repo.CommitObject(h) + if err != nil { + return false // cannot classify; let the caller's own load surface the error + } + return !c.Committer.When.After(m.frontierTime) +} + +// computeReachableFromMain builds the shared-with-main test for the current branch. +// On the default branch it returns an empty reachability (nothing is shared). +// +// It scans the default branch's first-parent chain up to mainSpineScanLimit +// commits (not to the root). The bound caps the read-path cost; when it is hit +// before main's root, the returned mainReachability falls back to a commit-date +// frontier (see sharedWithMain) so main's older history is still excluded — closing +// the deep-history leak without an unbounded walk. +func computeReachableFromMain(ctx context.Context, repo *git.Repository) *mainReachability { + computeReachableFromMainCalls.Add(1) + + m := &mainReachability{onSpine: make(map[plumbing.Hash]bool), reachedRoot: true} isOnDefault, _ := strategy.IsOnDefaultBranch(repo) if isOnDefault { - return reachableFromMain // No filtering needed on default branch + return m // No filtering needed on default branch } // Resolve main branch hash @@ -1977,16 +2034,28 @@ func computeReachableFromMain(ctx context.Context, repo *git.Repository) map[plu mainBranchHash = strategy.GetMainBranchHash(repo) } if mainBranchHash == plumbing.ZeroHash { - return reachableFromMain + return m } - // 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 + // Scan main's first-parent chain up to the bound, recording the oldest commit + // seen so sharedWithMain can fall back to a commit-date frontier if truncated. + var last *object.Commit + scanned := 0 + _ = walkFirstParentCommits(ctx, repo, mainBranchHash, mainSpineScanLimit, func(c *object.Commit) error { //nolint:errcheck // Best-effort + m.onSpine[c.Hash] = true + last = c + scanned++ return nil }) - return reachableFromMain + // If we stopped because we hit the limit (not the root), the scan is + // incomplete: record the frontier for the commit-date fail-safe. + if last != nil && scanned >= mainSpineScanLimit && last.NumParents() > 0 { + m.reachedRoot = false + m.frontierTime = last.Committer.When + } + + return m } // walkFirstParentCommits walks the first-parent chain starting from `from`, @@ -2026,6 +2095,81 @@ func walkFirstParentCommits(ctx context.Context, repo *git.Repository, from plum return nil } +// walkBranchOwnCommits walks the commits that belong to the current branch's own +// history — the branch's first-parent spine PLUS any side branch merged into it +// (via a merge commit's second+ parents) — while excluding the default branch's +// own first-parent spine (see mainReachability; this is not the full set of +// commits reachable from the default branch's DAG). It calls fn for each such +// commit, visiting each at most once. +// +// A commit reach.sharedWithMain reports true for is treated as on main's spine: +// it is neither visited nor traversed through, so the walk stops at main's +// first-parent spine and never descends into it. This is the property that a +// full repo.Log() DAG walk lacked (it walked into main's entire history through +// merge commits and hit the scan limit before older checkpoints were found — +// see git history for getBranchCheckpoints). +// +// Unlike a first-parent-only walk, this follows every parent that is not shared +// with main, so checkpoints on a feature branch that was merged INTO this branch +// (living on a merge commit's second parent) are discovered. A first-parent-only +// walk misses them entirely, which caused merged session references to vanish +// from any non-default target branch (issue #931). +// +// The walk visits at most `limit` commits (0 = no limit). fn may return +// errStopIteration to end the walk early. +func walkBranchOwnCommits( + ctx context.Context, + repo *git.Repository, + from plumbing.Hash, + reach *mainReachability, + limit int, + fn func(*object.Commit) error, +) error { + // If HEAD itself is shared with main there is no branch-unique history. + if reach.sharedWithMain(repo, from) { + return nil + } + + visited := map[plumbing.Hash]struct{}{from: {}} + queue := []plumbing.Hash{from} + + for count := 0; len(queue) > 0 && (limit <= 0 || count < limit); count++ { + if err := ctx.Err(); err != nil { + return err //nolint:wrapcheck // Propagating context cancellation + } + + hash := queue[0] + queue = queue[1:] + + current, err := repo.CommitObject(hash) + if err != nil { + return fmt.Errorf("failed to get commit %s: %w", hash, err) + } + + if err := fn(current); err != nil { + if errors.Is(err, errStopIteration) { + return nil + } + return err + } + + // Enqueue every parent that is not already seen and not shared with + // main. Pruning at reach.sharedWithMain keeps the walk on this branch's + // own history and stops it at the merge base with the default branch. + for _, parent := range current.ParentHashes { + if _, seen := visited[parent]; seen { + continue + } + if reach.sharedWithMain(repo, parent) { + continue + } + visited[parent] = struct{}{} + queue = append(queue, parent) + } + } + return nil +} + // getBranchCheckpoints returns checkpoints relevant to the current branch. // This is strategy-agnostic - it queries checkpoints directly from the checkpoint store. // @@ -2132,16 +2276,15 @@ func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) 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 - } + // On non-default branches, walk this branch's own history — its + // first-parent spine plus any side branches merged into it — while + // excluding the default branch's history. A first-parent-only walk + // misses checkpoints from a feature branch merged INTO this branch (they + // live on a merge commit's second parent), which dropped merged session + // references from non-default target branches (issue #931). + mainReach := computeReachableFromMain(ctx, repo) + + err = walkBranchOwnCommits(ctx, repo, head.Hash(), mainReach, commitScanLimit, func(c *object.Commit) error { collectCheckpoint(c) return nil }) @@ -2279,6 +2422,15 @@ func getReachableTemporaryCheckpoints(ctx context.Context, repo *git.Repository, // 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. +// +// NOTE: This is a first-parent-only walk. It is the ephemeral/shadow counterpart of +// the committed-checkpoint read path and is NOT merge-aware: a shadow branch whose +// base entered history via a merge's second parent is not found, and (conversely) it +// does not exclude the default branch's own history. Making it merge-aware without +// regressing the common "checkout -b then start coding" workflow needs session-origin +// metadata that isn't persisted today — tracked as a follow-up (see #1730). This +// function is intentionally left at its pre-existing behavior; issue #931 is a +// committed-checkpoint read-path bug (see getBranchCheckpoints / walkBranchOwnCommits). 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 { diff --git a/cmd/entire/cli/explain_test.go b/cmd/entire/cli/explain_test.go index dd5af7de7c..a294c67836 100644 --- a/cmd/entire/cli/explain_test.go +++ b/cmd/entire/cli/explain_test.go @@ -5088,7 +5088,7 @@ func TestFormatCheckpointOutput_NoCommitsOnBranch(t *testing.T) { } } -func TestGetAssociatedCommits_SearchAllFindsMergedBranchCommits(t *testing.T) { +func TestGetAssociatedCommits_FindsMergedBranchCommits(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 @@ -5180,14 +5180,18 @@ 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 + // Without --search-all: the branch-scoped walk still follows merge commits' + // second parents (pruning only the default branch's own history), so the + // feature commit merged in here is found. This is the issue #931 fix — a + // first-parent-only walk used to miss it and drop the merged reference. + // The remaining difference from --search-all is the depth bound and the + // default-branch-history pruning, not merge awareness. commits, err := getAssociatedCommits(context.Background(), repo, checkpointID, false) 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.Errorf("expected 1 commit for merged feature checkpoint without --search-all, got %d", len(commits)) } // With --search-all (full DAG walk): SHOULD find the feature commit @@ -5316,6 +5320,644 @@ func TestGetBranchCheckpoints_DefaultBranchFindsMergedCheckpoints(t *testing.T) } } +func TestGetBranchCheckpoints_NonDefaultTargetFindsMergedCheckpoints(t *testing.T) { + // Regression test for issue #931: merging a feature branch into a NON-default + // target branch (e.g. "release") via a merge commit puts the feature's + // checkpoint commits on the merge's second parent. First-parent-only + // traversal — used for every branch that is not the repo default — missed + // them, so the merged session references vanished from the target branch. + // The default-branch DAG walk added earlier only covered the default branch. + // + // It also asserts the exclusion invariant still holds: a checkpoint that + // lives only on the default branch is NOT surfaced on the target branch. + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + testutil.InitRepo(t, tmpDir) + repo, err := git.PlainOpen(tmpDir) + if err != nil { + t.Fatalf("failed to open git repo: %v", err) + } + + w, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + + // Initial commit on master (the default branch — InitRepo uses master). + testFile := filepath.Join(tmpDir, "test.txt") + 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) + } + masterBase, err := w.Commit("initial commit", &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) + } + + // A checkpoint that lives only on master (must NOT leak onto the target). + cpMaster := id.MustCheckpointID("aa5700000001") + if err := os.WriteFile(testFile, []byte("master work"), 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) + } + masterTip, err := w.Commit(trailers.FormatCheckpoint("master: work", cpMaster), &git.CommitOptions{ + Author: &object.Signature{Name: "Main Dev", Email: "main@example.com", When: time.Now().Add(-4 * time.Hour)}, + }) + if err != nil { + t.Fatalf("failed to create master checkpoint commit: %v", err) + } + + // Feature branch off the base with its own checkpoint. + featureBranch := plumbing.NewBranchReferenceName("feature/x") + if err := w.Checkout(&git.CheckoutOptions{Hash: masterBase, Branch: featureBranch, Create: true}); err != nil { + t.Fatalf("failed to create feature branch: %v", err) + } + cpFeature := id.MustCheckpointID("fea700000002") + if err := os.WriteFile(testFile, []byte("feature work"), 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) + } + featureCommit, err := w.Commit(trailers.FormatCheckpoint("feat: add feature", cpFeature), &git.CommitOptions{ + Author: &object.Signature{Name: "Feature Dev", Email: "dev@example.com", When: time.Now().Add(-3 * time.Hour)}, + }) + if err != nil { + t.Fatalf("failed to create feature commit: %v", err) + } + featureObj, err := repo.CommitObject(featureCommit) + if err != nil { + t.Fatalf("failed to get feature commit: %v", err) + } + featureTree, err := featureObj.Tree() + if err != nil { + t.Fatalf("failed to get feature tree: %v", err) + } + + // Create the non-default target branch "release" at master's tip, then merge + // the feature branch into it: first parent = release tip (master), second + // parent = feature. Anchoring release at masterTip makes master's own + // checkpoint cpMaster a genuine first-parent ancestor of release, so the + // "must NOT leak" assertion below actually exercises the prune: without it, + // the walk would descend through masterTip and surface cpMaster. + mergeHash := createMergeCommit(t, repo, masterTip, featureCommit, featureTree.Hash, "Merge feature/x into release") + releaseBranch := plumbing.NewBranchReferenceName("release") + if err := repo.Storer.SetReference(plumbing.NewHashReference(releaseBranch, mergeHash)); err != nil { + t.Fatalf("failed to set release ref: %v", err) + } + if err := repo.Storer.SetReference(plumbing.NewSymbolicReference("HEAD", releaseBranch)); err != nil { + t.Fatalf("failed to point HEAD at release: %v", err) + } + + // Sanity: we must be on a non-default branch for this test to be meaningful. + if isOnDefault, cur := strategy.IsOnDefaultBranch(repo); isOnDefault { + t.Fatalf("test setup invalid: expected to be on a non-default branch, got default branch %q", cur) + } + + // Write committed checkpoint metadata for both checkpoints. + store := checkpoint.NewGitStore(repo, checkpoint.DefaultV1Refs()) + for _, cp := range []id.CheckpointID{cpFeature, cpMaster} { + if err := store.Write(context.Background(), checkpoint.Session{ + CheckpointID: cp, + SessionID: "test-session-" + cp.String(), + Strategy: "manual-commit", + FilesTouched: []string{"test.txt"}, + Prompts: []string{"work"}, + }); err != nil { + t.Fatalf("failed to write committed checkpoint %s: %v", cp, err) + } + } + + points, _, err := getBranchCheckpoints(context.Background(), repo, 100) + if err != nil { + t.Fatalf("getBranchCheckpoints error: %v", err) + } + + var foundFeature, foundMaster bool + for _, p := range points { + if p.CheckpointID == cpFeature { + foundFeature = true + } + if p.CheckpointID == cpMaster { + foundMaster = true + } + } + if !foundFeature { + t.Errorf("expected feature checkpoint %s (merged into non-default target) to be found, got %d points: %v", cpFeature, len(points), points) + } + if foundMaster { + t.Errorf("master-only checkpoint %s must not leak onto the non-default target branch", cpMaster) + } +} + +// createNaryMergeCommit builds a merge commit with an arbitrary number of +// parents (>= 2), used to model octopus merges in tests. The first parent is +// the target branch's own tip; the remaining parents are the merged-in heads. +func createNaryMergeCommit(t *testing.T, repo *git.Repository, treeHash plumbing.Hash, message string, parents ...plumbing.Hash) plumbing.Hash { + t.Helper() + if len(parents) < 2 { + t.Fatalf("createNaryMergeCommit needs >= 2 parents, got %d", len(parents)) + } + sig := object.Signature{Name: "Test", Email: "test@example.com", When: time.Now()} + commit := object.Commit{ + Author: sig, + Committer: sig, + Message: message, + TreeHash: treeHash, + ParentHashes: parents, + } + obj := repo.Storer.NewEncodedObject() + if err := commit.Encode(obj); err != nil { + t.Fatalf("failed to encode n-ary merge commit: %v", err) + } + hash, err := repo.Storer.SetEncodedObject(obj) + if err != nil { + t.Fatalf("failed to store n-ary merge commit: %v", err) + } + return hash +} + +// writeCommittedCheckpointsForTest writes minimal committed checkpoint metadata +// (entire/checkpoints/v1) for each id so getBranchCheckpoints can surface them. +// Every id passed here is a valid committed checkpoint; whether it appears in +// the result is therefore decided purely by commit-graph reachability, which is +// exactly what the traversal-scoping tests need to assert. +func writeCommittedCheckpointsForTest(t *testing.T, repo *git.Repository, ids ...id.CheckpointID) { + t.Helper() + store := checkpoint.NewGitStore(repo, checkpoint.DefaultV1Refs()) + for _, cp := range ids { + if err := store.Write(context.Background(), checkpoint.Session{ + CheckpointID: cp, + SessionID: "test-session-" + cp.String(), + Strategy: "manual-commit", + FilesTouched: []string{"test.txt"}, + Prompts: []string{"work"}, + }); err != nil { + t.Fatalf("failed to write committed checkpoint %s: %v", cp, err) + } + } +} + +// setBranchHead points branch at tip and checks out HEAD to it (symbolic). +func setBranchHead(t *testing.T, repo *git.Repository, branch string, tip plumbing.Hash) { + t.Helper() + ref := plumbing.NewBranchReferenceName(branch) + if err := repo.Storer.SetReference(plumbing.NewHashReference(ref, tip)); err != nil { + t.Fatalf("failed to set %s ref: %v", branch, err) + } + if err := repo.Storer.SetReference(plumbing.NewSymbolicReference("HEAD", ref)); err != nil { + t.Fatalf("failed to point HEAD at %s: %v", branch, err) + } +} + +// collectCheckpointIDs indexes rewind points by checkpoint ID for membership +// assertions. +func collectCheckpointIDs(points []strategy.RewindPoint) map[id.CheckpointID]bool { + found := make(map[id.CheckpointID]bool, len(points)) + for _, p := range points { + found[p.CheckpointID] = true + } + return found +} + +// newMasterWithCheckpoint creates the initial commit and a second commit that +// carries cpMaster, repoints refs/heads/master at the second commit, and returns +// the base commit, the master tip, and the shared base tree hash. Reusing a +// single tree across the whole graph is fine — checkpoint scanning only reads +// commit messages and the parent DAG, never tree contents. +func newMasterWithCheckpoint(t *testing.T, repo *git.Repository, w *git.Worktree, dir string, cpMaster id.CheckpointID) (base, masterTip, baseTree plumbing.Hash) { + t.Helper() + testFile := filepath.Join(dir, "test.txt") + 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) + } + base, err := w.Commit("initial commit", &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) + } + baseObj, err := repo.CommitObject(base) + if err != nil { + t.Fatalf("failed to get base commit: %v", err) + } + tree, err := baseObj.Tree() + if err != nil { + t.Fatalf("failed to get base tree: %v", err) + } + masterTip = createCommitWithTree(t, repo, tree.Hash, []plumbing.Hash{base}, trailers.FormatCheckpoint("master: work", cpMaster)) + if err := repo.Storer.SetReference(plumbing.NewHashReference(plumbing.NewBranchReferenceName("master"), masterTip)); err != nil { + t.Fatalf("failed to advance master ref: %v", err) + } + return base, masterTip, tree.Hash +} + +func TestGetBranchCheckpoints_MainMergedIntoFeatureExcludesMainCheckpoint(t *testing.T) { + // Invariant guard for the issue #931 fix: merging the DEFAULT branch INTO a + // feature branch must NOT surface the default branch's own checkpoints. Those + // commits live on the merge's second parent and become ancestors of the + // feature branch, so a naive "follow every parent" walk would leak them. + // walkBranchOwnCommits prunes everything reachable from the default branch, so + // cpMaster is excluded while the feature's own checkpoint is still found. + // (Deleting the prune makes cpMaster leak — the mutation this locks down.) + tmpDir := t.TempDir() + t.Chdir(tmpDir) + testutil.InitRepo(t, tmpDir) + repo, err := git.PlainOpen(tmpDir) + if err != nil { + t.Fatalf("failed to open git repo: %v", err) + } + w, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + + cpMaster := id.MustCheckpointID("aa5700000001") + cpFeature := id.MustCheckpointID("fea700000002") + masterBase, masterTip, baseTree := newMasterWithCheckpoint(t, repo, w, tmpDir, cpMaster) + + // Feature branches off the base with its own checkpoint, then merges master + // IN: first parent = feature tip, second parent = master tip. + featureCommit := createCommitWithTree(t, repo, baseTree, []plumbing.Hash{masterBase}, trailers.FormatCheckpoint("feat: work", cpFeature)) + mergeHash := createMergeCommit(t, repo, featureCommit, masterTip, baseTree, "Merge branch 'master' into feature/x") + setBranchHead(t, repo, "feature/x", mergeHash) + + if isOnDefault, cur := strategy.IsOnDefaultBranch(repo); isOnDefault { + t.Fatalf("test setup invalid: expected a non-default branch, got default branch %q", cur) + } + + writeCommittedCheckpointsForTest(t, repo, cpMaster, cpFeature) + + points, _, err := getBranchCheckpoints(context.Background(), repo, 100) + if err != nil { + t.Fatalf("getBranchCheckpoints error: %v", err) + } + found := collectCheckpointIDs(points) + if !found[cpFeature] { + t.Errorf("expected feature checkpoint %s to be found, got %d points: %v", cpFeature, len(points), points) + } + if found[cpMaster] { + t.Errorf("default-branch checkpoint %s must not leak onto a feature branch after merging the default branch in", cpMaster) + } +} + +func TestGetBranchCheckpoints_OctopusMergeIntoNonDefaultTarget(t *testing.T) { + // Adversarial graph: an octopus merge (three feature parents) into a + // non-default target branch. Every merged feature's checkpoint must be + // discovered — the walk follows all parents, not just the first two — while + // the default branch's own checkpoint stays excluded. + tmpDir := t.TempDir() + t.Chdir(tmpDir) + testutil.InitRepo(t, tmpDir) + repo, err := git.PlainOpen(tmpDir) + if err != nil { + t.Fatalf("failed to open git repo: %v", err) + } + w, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + + cpMaster := id.MustCheckpointID("aa5700000001") + cp1 := id.MustCheckpointID("f00100000001") + cp2 := id.MustCheckpointID("f00200000002") + cp3 := id.MustCheckpointID("f00300000003") + masterBase, masterTip, baseTree := newMasterWithCheckpoint(t, repo, w, tmpDir, cpMaster) + + f1 := createCommitWithTree(t, repo, baseTree, []plumbing.Hash{masterBase}, trailers.FormatCheckpoint("feat 1", cp1)) + f2 := createCommitWithTree(t, repo, baseTree, []plumbing.Hash{masterBase}, trailers.FormatCheckpoint("feat 2", cp2)) + f3 := createCommitWithTree(t, repo, baseTree, []plumbing.Hash{masterBase}, trailers.FormatCheckpoint("feat 3", cp3)) + + // Octopus merge into "release": first parent = release tip (master), then the + // three feature heads. + octopus := createNaryMergeCommit(t, repo, baseTree, "Octopus merge feat 1,2,3 into release", masterTip, f1, f2, f3) + setBranchHead(t, repo, "release", octopus) + + if isOnDefault, cur := strategy.IsOnDefaultBranch(repo); isOnDefault { + t.Fatalf("test setup invalid: expected a non-default branch, got default branch %q", cur) + } + + writeCommittedCheckpointsForTest(t, repo, cpMaster, cp1, cp2, cp3) + + points, _, err := getBranchCheckpoints(context.Background(), repo, 100) + if err != nil { + t.Fatalf("getBranchCheckpoints error: %v", err) + } + found := collectCheckpointIDs(points) + for _, cp := range []id.CheckpointID{cp1, cp2, cp3} { + if !found[cp] { + t.Errorf("expected octopus-merged checkpoint %s to be found, got %d points: %v", cp, len(points), points) + } + } + if found[cpMaster] { + t.Errorf("default-branch checkpoint %s must not leak onto the octopus target branch", cpMaster) + } +} + +func TestGetBranchCheckpoints_NestedMergeIntoNonDefaultTarget(t *testing.T) { + // Adversarial graph: nested merges. A sub-feature is merged into feature A, + // and feature A is then merged into a non-default "release" target. Both the + // feature-A checkpoint and the transitively-merged sub-feature checkpoint must + // be discovered (a first-parent-only walk would miss the sub-feature), while + // the default branch's own checkpoint stays excluded. + tmpDir := t.TempDir() + t.Chdir(tmpDir) + testutil.InitRepo(t, tmpDir) + repo, err := git.PlainOpen(tmpDir) + if err != nil { + t.Fatalf("failed to open git repo: %v", err) + } + w, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + + cpMaster := id.MustCheckpointID("aa5700000001") + cpFeatureA := id.MustCheckpointID("aaa000000002") + cpSub := id.MustCheckpointID("b0b000000003") + masterBase, masterTip, baseTree := newMasterWithCheckpoint(t, repo, w, tmpDir, cpMaster) + + // Sub-feature and feature A both branch off the base. + subFeature := createCommitWithTree(t, repo, baseTree, []plumbing.Hash{masterBase}, trailers.FormatCheckpoint("sub-feature", cpSub)) + featureA := createCommitWithTree(t, repo, baseTree, []plumbing.Hash{masterBase}, trailers.FormatCheckpoint("feature A", cpFeatureA)) + + // Feature A merges the sub-feature in (first parent = A, second parent = sub). + mergeAsub := createMergeCommit(t, repo, featureA, subFeature, baseTree, "Merge sub-feature into feature A") + // Release merges feature A in (first parent = release tip (master), second = + // feature A's merge commit). + releaseMerge := createMergeCommit(t, repo, masterTip, mergeAsub, baseTree, "Merge feature A into release") + setBranchHead(t, repo, "release", releaseMerge) + + if isOnDefault, cur := strategy.IsOnDefaultBranch(repo); isOnDefault { + t.Fatalf("test setup invalid: expected a non-default branch, got default branch %q", cur) + } + + writeCommittedCheckpointsForTest(t, repo, cpMaster, cpFeatureA, cpSub) + + points, _, err := getBranchCheckpoints(context.Background(), repo, 100) + if err != nil { + t.Fatalf("getBranchCheckpoints error: %v", err) + } + found := collectCheckpointIDs(points) + if !found[cpFeatureA] { + t.Errorf("expected feature-A checkpoint %s to be found, got %d points: %v", cpFeatureA, len(points), points) + } + if !found[cpSub] { + t.Errorf("expected transitively-merged sub-feature checkpoint %s to be found, got %d points: %v", cpSub, len(points), points) + } + if found[cpMaster] { + t.Errorf("default-branch checkpoint %s must not leak onto the nested-merge target branch", cpMaster) + } +} + +// createCommitWithTreeAt is createCommitWithTree with an explicit commit time. +// Git stores commit timestamps at 1-second resolution, so tests that exercise the +// commit-date frontier must space their commits seconds apart rather than relying +// on time.Now() (which ties for commits built in the same second). +func createCommitWithTreeAt(t *testing.T, repo *git.Repository, treeHash plumbing.Hash, parents []plumbing.Hash, message string, when time.Time) plumbing.Hash { + t.Helper() + sig := object.Signature{Name: "Test", Email: "test@example.com", When: when} + commit := object.Commit{Author: sig, Committer: sig, Message: message, TreeHash: treeHash, ParentHashes: parents} + obj := repo.Storer.NewEncodedObject() + if err := commit.Encode(obj); err != nil { + t.Fatalf("failed to encode commit: %v", err) + } + hash, err := repo.Storer.SetEncodedObject(obj) + if err != nil { + t.Fatalf("failed to store commit: %v", err) + } + return hash +} + +func TestGetBranchCheckpoints_DeepHistoryDoesNotLeakMainCheckpoint(t *testing.T) { + // The shared-with-main test must still exclude main's history when main's + // first-parent chain is longer than the scan bound. A branch that merged main + // in long ago — with main since advanced past the bound — has the merged-in + // former-main-tip fall outside the scanned set; the commit-date frontier + // fail-safe (it is older than the oldest scanned main commit) must still prune + // it, so main's older checkpoints do not leak onto the branch, while the + // branch's own (newer) checkpoint is still shown. + // + // Commit times are explicit and seconds apart because git stores timestamps at + // 1-second resolution: mainOld is old, the branch commit is recent, and main's + // filler commits sit between them so the scan frontier lands between the two. + tmpDir := t.TempDir() + t.Chdir(tmpDir) + testutil.InitRepo(t, tmpDir) + repo, err := git.PlainOpen(tmpDir) + if err != nil { + t.Fatalf("failed to open git repo: %v", err) + } + w, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + + base := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) + + testFile := filepath.Join(tmpDir, "test.txt") + 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) + } + masterBase, err := w.Commit("initial commit", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com", When: base}, + }) + if err != nil { + t.Fatalf("failed to create initial commit: %v", err) + } + baseObj, err := repo.CommitObject(masterBase) + if err != nil { + t.Fatalf("failed to get base commit: %v", err) + } + tree, err := baseObj.Tree() + if err != nil { + t.Fatalf("failed to get base tree: %v", err) + } + + cpMainOld := id.MustCheckpointID("aa5700000001") + cpFeature := id.MustCheckpointID("fea700000002") + + // An OLD main tip carrying a checkpoint (t = base + 1h). + mainOldTime := base.Add(1 * time.Hour) + mainOld := createCommitWithTreeAt(t, repo, tree.Hash, []plumbing.Hash{masterBase}, trailers.FormatCheckpoint("main: old work", cpMainOld), mainOldTime) + + // Advance master past the scan bound. Fillers sit in the MIDDLE (t = base+2h), + // so the scan frontier (some filler) is newer than mainOld and older than the + // branch commit. + fillerTime := base.Add(2 * time.Hour) + prev := mainOld + for range mainSpineScanLimit + 5 { + prev = createCommitWithTreeAt(t, repo, tree.Hash, []plumbing.Hash{prev}, "main: filler", fillerTime) + } + masterHead := prev + if err := repo.Storer.SetReference(plumbing.NewHashReference(plumbing.NewBranchReferenceName("master"), masterHead)); err != nil { + t.Fatalf("failed to advance master ref: %v", err) + } + + // Feature branches from the base with a RECENT checkpoint (t = base+3h), then + // merges the OLD main tip in (first parent = feature, second parent = mainOld). + featureTime := base.Add(3 * time.Hour) + featureCommit := createCommitWithTreeAt(t, repo, tree.Hash, []plumbing.Hash{masterBase}, trailers.FormatCheckpoint("feat: work", cpFeature), featureTime) + mergeHash := createCommitWithTreeAt(t, repo, tree.Hash, []plumbing.Hash{featureCommit, mainOld}, "Merge old main into feature", featureTime) + setBranchHead(t, repo, "feature/x", mergeHash) + if isOnDefault, cur := strategy.IsOnDefaultBranch(repo); isOnDefault { + t.Fatalf("test setup invalid: expected a non-default branch, got default branch %q", cur) + } + + writeCommittedCheckpointsForTest(t, repo, cpMainOld, cpFeature) + + points, _, err := getBranchCheckpoints(context.Background(), repo, 100) + if err != nil { + t.Fatalf("getBranchCheckpoints error: %v", err) + } + found := collectCheckpointIDs(points) + if !found[cpFeature] { + t.Errorf("expected feature checkpoint %s to be found, got %d points: %v", cpFeature, len(points), points) + } + if found[cpMainOld] { + t.Errorf("old default-branch checkpoint %s (merged in beyond the scan bound) must not leak onto the feature branch", cpMainOld) + } +} + +func TestComputeReachableFromMain_ScanIsBounded(t *testing.T) { + // The default-branch first-parent scan must be bounded by mainSpineScanLimit + // regardless of how long main is — otherwise checkpoint list/explain would do + // an unbounded O(main) traversal on every call on a repo with a long history. + tmpDir := t.TempDir() + t.Chdir(tmpDir) + testutil.InitRepo(t, tmpDir) + repo, err := git.PlainOpen(tmpDir) + if err != nil { + t.Fatalf("failed to open git repo: %v", err) + } + w, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + + base := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) + testFile := filepath.Join(tmpDir, "test.txt") + 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) + } + masterBase, err := w.Commit("initial commit", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com", When: base}, + }) + if err != nil { + t.Fatalf("failed to create initial commit: %v", err) + } + baseObj, err := repo.CommitObject(masterBase) + if err != nil { + t.Fatalf("failed to get base commit: %v", err) + } + tree, err := baseObj.Tree() + if err != nil { + t.Fatalf("failed to get base tree: %v", err) + } + + // Main much longer than the bound. + prev := masterBase + total := mainSpineScanLimit + 50 + for i := range total { + prev = createCommitWithTreeAt(t, repo, tree.Hash, []plumbing.Hash{prev}, "main: filler", base.Add(time.Duration(i)*time.Second)) + } + if err := repo.Storer.SetReference(plumbing.NewHashReference(plumbing.NewBranchReferenceName("master"), prev)); err != nil { + t.Fatalf("failed to advance master ref: %v", err) + } + // A feature branch so computeReachableFromMain does not short-circuit on default. + featureCommit := createCommitWithTreeAt(t, repo, tree.Hash, []plumbing.Hash{masterBase}, "feature work", base.Add(time.Duration(total+10)*time.Second)) + setBranchHead(t, repo, "feature/x", featureCommit) + + reach := computeReachableFromMain(context.Background(), repo) + if got := len(reach.onSpine); got > mainSpineScanLimit { + t.Errorf("scanned %d main commits, want at most mainSpineScanLimit=%d (scan must be bounded)", got, mainSpineScanLimit) + } + if reach.reachedRoot { + t.Errorf("expected reachedRoot=false when main (%d commits) exceeds the scan bound", total+1) + } +} + +func TestGetBranchCheckpoints_ComputesReachabilityOncePerCall(t *testing.T) { + // The main first-parent scan must run once per getBranchCheckpoints call, not + // once per checkpoint — otherwise the bounded-but-nontrivial scan would be + // multiplied by the number of checkpoints on the branch. + // + // Not parallel: it reads a process-global call counter. + tmpDir := t.TempDir() + t.Chdir(tmpDir) + testutil.InitRepo(t, tmpDir) + repo, err := git.PlainOpen(tmpDir) + if err != nil { + t.Fatalf("failed to open git repo: %v", err) + } + w, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + + base := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) + testFile := filepath.Join(tmpDir, "test.txt") + 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) + } + masterBase, err := w.Commit("initial commit", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com", When: base}, + }) + if err != nil { + t.Fatalf("failed to create initial commit: %v", err) + } + baseObj, err := repo.CommitObject(masterBase) + if err != nil { + t.Fatalf("failed to get base commit: %v", err) + } + tree, err := baseObj.Tree() + if err != nil { + t.Fatalf("failed to get base tree: %v", err) + } + + // A feature branch with several committed checkpoints. + featureBranch := plumbing.NewBranchReferenceName("feature/x") + if err := w.Checkout(&git.CheckoutOptions{Hash: masterBase, Branch: featureBranch, Create: true}); err != nil { + t.Fatalf("failed to create feature branch: %v", err) + } + var ids []id.CheckpointID + prev := masterBase + for i, s := range []string{"a11100000001", "b22200000002", "c33300000003"} { + cp := id.MustCheckpointID(s) + ids = append(ids, cp) + prev = createCommitWithTreeAt(t, repo, tree.Hash, []plumbing.Hash{prev}, trailers.FormatCheckpoint("feat", cp), base.Add(time.Duration(i+1)*time.Minute)) + } + setBranchHead(t, repo, "feature/x", prev) + writeCommittedCheckpointsForTest(t, repo, ids...) + + before := computeReachableFromMainCalls.Load() + if _, _, err := getBranchCheckpoints(context.Background(), repo, 100); err != nil { + t.Fatalf("getBranchCheckpoints error: %v", err) + } + if delta := computeReachableFromMainCalls.Load() - before; delta != 1 { + t.Errorf("computeReachableFromMain ran %d times for one getBranchCheckpoints call over %d checkpoints; want exactly 1", delta, len(ids)) + } +} + func TestGetBranchCheckpoints_ReadsPromptFromCommittedCheckpoint(t *testing.T) { // Verifies that getBranchCheckpoints populates RewindPoint.SessionPrompt // from prompt.txt on entire/checkpoints/v1 (committed checkpoint) without