From 913f4978b729bcaa2731da698259b070c09d9785 Mon Sep 17 00:00:00 2001 From: Daniel Vydra Date: Mon, 2 Mar 2026 17:33:18 +1100 Subject: [PATCH 1/2] refactor: unify tree modification to use ApplyTreeChanges Three call sites modified git trees using FlattenTree + BuildTreeFromEntries (flatten entire tree, mutate map, rebuild from scratch). A fourth site (WriteCommitted) already used the surgical ApplyTreeChanges primitive. Add DiffTrees helper that computes []TreeChange from two flat entry maps, then convert all three flatten+rebuild sites to use DiffTrees + ApplyTreeChanges: - cherryPickOnto (metadata_reconcile.go): eliminates tip tree flatten entirely - fetchAndMergeSessionsCommon (push_common.go): separate flattens + surgical apply - DeleteOrphanedCheckpoints (cleanup.go): flatten to discover paths + surgical delete All sites now use the same tree-modification primitive, reducing cognitive overhead from having two different approaches for the same operation. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 0e7526b4fd6b --- cmd/entire/cli/checkpoint/parse_tree.go | 30 ++ cmd/entire/cli/checkpoint/parse_tree_test.go | 183 ++++++++++++ cmd/entire/cli/strategy/cleanup.go | 35 +-- cmd/entire/cli/strategy/metadata_reconcile.go | 272 ++++++++++++++++++ cmd/entire/cli/strategy/push_common.go | 20 +- 5 files changed, 515 insertions(+), 25 deletions(-) create mode 100644 cmd/entire/cli/strategy/metadata_reconcile.go diff --git a/cmd/entire/cli/checkpoint/parse_tree.go b/cmd/entire/cli/checkpoint/parse_tree.go index 6bdec5bde..ae31b7e4d 100644 --- a/cmd/entire/cli/checkpoint/parse_tree.go +++ b/cmd/entire/cli/checkpoint/parse_tree.go @@ -282,6 +282,36 @@ func ApplyTreeChanges( return storeTree(repo, result) } +// DiffTrees computes the []TreeChange needed to transform a tree with base +// entries into a tree with target entries. Both maps should be flat +// (path → TreeEntry) as produced by FlattenTree. +// +// - Entries in target but absent/different in base: add/modify +// - Entries in base but absent in target: delete (nil Entry) +func DiffTrees(base, target map[string]object.TreeEntry) []TreeChange { + changes := make([]TreeChange, 0, len(target)) + + // Adds and modifications + for path, entry := range target { + baseEntry, exists := base[path] + if !exists || baseEntry.Hash != entry.Hash { + changes = append(changes, TreeChange{ + Path: path, + Entry: &object.TreeEntry{Name: entry.Name, Mode: entry.Mode, Hash: entry.Hash}, + }) + } + } + + // Deletions + for path := range base { + if _, exists := target[path]; !exists { + changes = append(changes, TreeChange{Path: path, Entry: nil}) + } + } + + return changes +} + // splitFirstSegment splits "a/b/c" into ("a", "b/c"), and "file.txt" into ("file.txt", ""). func splitFirstSegment(path string) (first, rest string) { parts := strings.SplitN(path, "/", 2) diff --git a/cmd/entire/cli/checkpoint/parse_tree_test.go b/cmd/entire/cli/checkpoint/parse_tree_test.go index 943962b3e..20b2681dd 100644 --- a/cmd/entire/cli/checkpoint/parse_tree_test.go +++ b/cmd/entire/cli/checkpoint/parse_tree_test.go @@ -683,6 +683,189 @@ func TestApplyTreeChanges_MixedOperations(t *testing.T) { } } +func TestDiffTrees_BothEmpty(t *testing.T) { + t.Parallel() + changes := DiffTrees( + map[string]object.TreeEntry{}, + map[string]object.TreeEntry{}, + ) + if len(changes) != 0 { + t.Errorf("expected 0 changes, got %d", len(changes)) + } +} + +func TestDiffTrees_Identical(t *testing.T) { + t.Parallel() + hash := plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + entries := map[string]object.TreeEntry{ + "unchanged.txt": {Name: "unchanged.txt", Mode: filemode.Regular, Hash: hash}, + } + changes := DiffTrees(entries, entries) + if len(changes) != 0 { + t.Errorf("expected 0 changes for identical trees, got %d", len(changes)) + } +} + +func TestDiffTrees_AddsOnly(t *testing.T) { + t.Parallel() + hash := plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + base := map[string]object.TreeEntry{} + target := map[string]object.TreeEntry{ + "new.txt": {Name: "new.txt", Mode: filemode.Regular, Hash: hash}, + } + changes := DiffTrees(base, target) + if len(changes) != 1 { + t.Fatalf("expected 1 change, got %d", len(changes)) + } + if changes[0].Path != "new.txt" || changes[0].Entry == nil { + t.Error("expected add for new.txt") + } +} + +func TestDiffTrees_DeletesOnly(t *testing.T) { + t.Parallel() + hash := plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + base := map[string]object.TreeEntry{ + "old.txt": {Name: "old.txt", Mode: filemode.Regular, Hash: hash}, + } + target := map[string]object.TreeEntry{} + changes := DiffTrees(base, target) + if len(changes) != 1 { + t.Fatalf("expected 1 change, got %d", len(changes)) + } + if changes[0].Path != "old.txt" || changes[0].Entry != nil { + t.Error("expected delete for old.txt") + } +} + +func TestDiffTrees_ModificationsOnly(t *testing.T) { + t.Parallel() + hashOld := plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + hashNew := plumbing.NewHash("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") + base := map[string]object.TreeEntry{ + "modified.txt": {Name: "modified.txt", Mode: filemode.Regular, Hash: hashOld}, + } + target := map[string]object.TreeEntry{ + "modified.txt": {Name: "modified.txt", Mode: filemode.Regular, Hash: hashNew}, + } + changes := DiffTrees(base, target) + if len(changes) != 1 { + t.Fatalf("expected 1 change, got %d", len(changes)) + } + if changes[0].Path != "modified.txt" || changes[0].Entry == nil { + t.Error("expected modify for modified.txt") + } + if changes[0].Entry.Hash != hashNew { + t.Errorf("expected new hash %s, got %s", hashNew, changes[0].Entry.Hash) + } +} + +func TestDiffTrees_Mixed(t *testing.T) { + t.Parallel() + hashA := plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + hashB := plumbing.NewHash("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") + hashC := plumbing.NewHash("cccccccccccccccccccccccccccccccccccccccc") + base := map[string]object.TreeEntry{ + "keep.txt": {Name: "keep.txt", Mode: filemode.Regular, Hash: hashA}, + "modify.txt": {Name: "modify.txt", Mode: filemode.Regular, Hash: hashA}, + "delete.txt": {Name: "delete.txt", Mode: filemode.Regular, Hash: hashA}, + } + target := map[string]object.TreeEntry{ + "keep.txt": {Name: "keep.txt", Mode: filemode.Regular, Hash: hashA}, + "modify.txt": {Name: "modify.txt", Mode: filemode.Regular, Hash: hashB}, + "add.txt": {Name: "add.txt", Mode: filemode.Regular, Hash: hashC}, + } + changes := DiffTrees(base, target) + // Should have: modify (modify.txt), add (add.txt), delete (delete.txt) = 3 changes + if len(changes) != 3 { + t.Fatalf("expected 3 changes, got %d", len(changes)) + } + + changeMap := make(map[string]*TreeChange, len(changes)) + for i := range changes { + changeMap[changes[i].Path] = &changes[i] + } + + if c, ok := changeMap["modify.txt"]; !ok || c.Entry == nil || c.Entry.Hash != hashB { + t.Error("expected modify for modify.txt with new hash") + } + if c, ok := changeMap["add.txt"]; !ok || c.Entry == nil || c.Entry.Hash != hashC { + t.Error("expected add for add.txt") + } + if c, ok := changeMap["delete.txt"]; !ok || c.Entry != nil { + t.Error("expected delete for delete.txt") + } + if _, ok := changeMap["keep.txt"]; ok { + t.Error("keep.txt should not appear in changes") + } +} + +// TestDiffTrees_EquivalenceWithApplyTreeChanges verifies that computing a diff +// and applying it via ApplyTreeChanges produces the same file content as +// FlattenTree + BuildTreeFromEntries. +func TestDiffTrees_EquivalenceWithApplyTreeChanges(t *testing.T) { + t.Parallel() + repo := mustInitBareRepo(t) + + blobA := storeBlob(t, repo, "content-a") + blobB := storeBlob(t, repo, "content-b") + blobC := storeBlob(t, repo, "content-c") + blobNew := storeBlob(t, repo, "new-content") + + // Build base tree: a3/ckpt1/meta.json, a3/ckpt2/meta.json, b7/ckpt3/meta.json + ckpt1 := mustStoreTree(t, repo, []object.TreeEntry{{Name: "meta.json", Mode: filemode.Regular, Hash: blobA}}) + ckpt2 := mustStoreTree(t, repo, []object.TreeEntry{{Name: "meta.json", Mode: filemode.Regular, Hash: blobB}}) + ckpt3 := mustStoreTree(t, repo, []object.TreeEntry{{Name: "meta.json", Mode: filemode.Regular, Hash: blobC}}) + shardA3 := mustStoreTree(t, repo, []object.TreeEntry{ + {Name: "ckpt1", Mode: filemode.Dir, Hash: ckpt1}, + {Name: "ckpt2", Mode: filemode.Dir, Hash: ckpt2}, + }) + shardB7 := mustStoreTree(t, repo, []object.TreeEntry{ + {Name: "ckpt3", Mode: filemode.Dir, Hash: ckpt3}, + }) + baseTree := mustStoreTree(t, repo, []object.TreeEntry{ + {Name: "a3", Mode: filemode.Dir, Hash: shardA3}, + {Name: "b7", Mode: filemode.Dir, Hash: shardB7}, + }) + + // Target: modify a3/ckpt1, add a3/ckpt4, keep a3/ckpt2, keep b7/ckpt3 + // (no full-directory deletions — ApplyTreeChanges leaves empty dirs on delete) + ckpt1mod := mustStoreTree(t, repo, []object.TreeEntry{{Name: "meta.json", Mode: filemode.Regular, Hash: blobNew}}) + ckpt4 := mustStoreTree(t, repo, []object.TreeEntry{{Name: "meta.json", Mode: filemode.Regular, Hash: blobNew}}) + shardA3Target := mustStoreTree(t, repo, []object.TreeEntry{ + {Name: "ckpt1", Mode: filemode.Dir, Hash: ckpt1mod}, + {Name: "ckpt2", Mode: filemode.Dir, Hash: ckpt2}, + {Name: "ckpt4", Mode: filemode.Dir, Hash: ckpt4}, + }) + targetTree := mustStoreTree(t, repo, []object.TreeEntry{ + {Name: "a3", Mode: filemode.Dir, Hash: shardA3Target}, + {Name: "b7", Mode: filemode.Dir, Hash: shardB7}, + }) + + // Approach 1: Flatten both, DiffTrees, ApplyTreeChanges + baseEntries := make(map[string]object.TreeEntry) + if err := FlattenTree(repo, mustTreeObject(t, repo, baseTree), "", baseEntries); err != nil { + t.Fatalf("FlattenTree base: %v", err) + } + targetEntries := make(map[string]object.TreeEntry) + if err := FlattenTree(repo, mustTreeObject(t, repo, targetTree), "", targetEntries); err != nil { + t.Fatalf("FlattenTree target: %v", err) + } + changes := DiffTrees(baseEntries, targetEntries) + surgeryResult, err := ApplyTreeChanges(repo, baseTree, changes) + if err != nil { + t.Fatalf("ApplyTreeChanges() error = %v", err) + } + + // Both should produce the same tree hash (no full-directory deletions involved) + if surgeryResult != targetTree { + surgeryFiles := flattenTreeHelper(t, repo, surgeryResult, "") + targetFiles := flattenTreeHelper(t, repo, targetTree, "") + t.Errorf("DiffTrees+ApplyTreeChanges (%s) != target tree (%s)\nsurgery: %v\ntarget: %v", + surgeryResult, targetTree, surgeryFiles, targetFiles) + } +} + // TestUpdateSubtree_EquivalenceWithFlattenRebuild verifies that UpdateSubtree // produces the same result as the old FlattenTree + modify + BuildTreeFromEntries approach. func TestUpdateSubtree_EquivalenceWithFlattenRebuild(t *testing.T) { diff --git a/cmd/entire/cli/strategy/cleanup.go b/cmd/entire/cli/strategy/cleanup.go index 713981f6e..0299cc256 100644 --- a/cmd/entire/cli/strategy/cleanup.go +++ b/cmd/entire/cli/strategy/cleanup.go @@ -266,36 +266,37 @@ func DeleteOrphanedCheckpoints(ctx context.Context, checkpointIDs []string) (del return nil, nil, fmt.Errorf("failed to get tree: %w", err) } - // Flatten tree to entries + // Flatten tree to discover file paths, then build deletion changes entries := make(map[string]object.TreeEntry) if err := checkpoint.FlattenTree(repo, baseTree, "", entries); err != nil { return nil, nil, fmt.Errorf("failed to flatten tree: %w", err) } - // Remove entries for each checkpoint - checkpointSet := make(map[string]bool) - for _, id := range checkpointIDs { - checkpointSet[id] = true + // Build set of checkpoint paths to delete + checkpointPaths := make(map[string]bool) + for _, idStr := range checkpointIDs { + cpID, parseErr := id.NewCheckpointID(idStr) + if parseErr != nil { + continue // Skip invalid checkpoint IDs + } + checkpointPaths[cpID.Path()+"/"] = true } - // Find and remove entries matching checkpoint paths + // Collect deletion changes for matching entries + var changes []checkpoint.TreeChange for path := range entries { - for checkpointIDStr := range checkpointSet { - cpID, err := id.NewCheckpointID(checkpointIDStr) - if err != nil { - continue // Skip invalid checkpoint IDs - } - cpPath := cpID.Path() - if strings.HasPrefix(path, cpPath+"/") { - delete(entries, path) + for cpPrefix := range checkpointPaths { + if strings.HasPrefix(path, cpPrefix) { + changes = append(changes, checkpoint.TreeChange{Path: path, Entry: nil}) + break } } } - // Build new tree - newTreeHash, err := checkpoint.BuildTreeFromEntries(repo, entries) + // Apply deletions surgically + newTreeHash, err := checkpoint.ApplyTreeChanges(repo, parentCommit.TreeHash, changes) if err != nil { - return nil, nil, fmt.Errorf("failed to build tree: %w", err) + return nil, nil, fmt.Errorf("failed to apply tree changes: %w", err) } // Create commit diff --git a/cmd/entire/cli/strategy/metadata_reconcile.go b/cmd/entire/cli/strategy/metadata_reconcile.go new file mode 100644 index 000000000..ee7f78235 --- /dev/null +++ b/cmd/entire/cli/strategy/metadata_reconcile.go @@ -0,0 +1,272 @@ +package strategy + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "time" + + "github.com/entireio/cli/cmd/entire/cli/checkpoint" + "github.com/entireio/cli/cmd/entire/cli/paths" + + "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/object" +) + +// ReconcileDisconnectedMetadataBranch detects and repairs disconnected local/remote +// entire/checkpoints/v1 branches. Disconnected means no common ancestor, which +// only happens due to the empty-orphan bug. Diverged (shared ancestor) is normal +// and handled by the push path's tree merge. +// +// Repair strategy: cherry-pick local commits onto remote tip, preserving all data. +// Checkpoint shards use unique paths (//), so cherry-picks always +// apply cleanly. +func ReconcileDisconnectedMetadataBranch(repo *git.Repository) error { + refName := plumbing.NewBranchReferenceName(paths.MetadataBranchName) + + // Check local branch + localRef, err := repo.Reference(refName, true) + if errors.Is(err, plumbing.ErrReferenceNotFound) { + return nil // No local branch — nothing to reconcile + } + if err != nil { + return fmt.Errorf("failed to check local metadata branch: %w", err) + } + + // Check remote-tracking branch + remoteRefName := plumbing.NewRemoteReferenceName("origin", paths.MetadataBranchName) + remoteRef, err := repo.Reference(remoteRefName, true) + if errors.Is(err, plumbing.ErrReferenceNotFound) { + return nil // No remote branch — nothing to reconcile + } + if err != nil { + return fmt.Errorf("failed to check remote metadata branch: %w", err) + } + + localHash := localRef.Hash() + remoteHash := remoteRef.Hash() + + // Same hash — nothing to do + if localHash == remoteHash { + return nil + } + + // Check if disconnected using git merge-base + repoPath, err := getRepoPath(repo) + if err != nil { + return err + } + + disconnected, err := isDisconnected(repoPath, localHash.String(), remoteHash.String()) + if err != nil { + return fmt.Errorf("failed to check metadata branch ancestry: %w", err) + } + if !disconnected { + // Shared ancestry (diverged or ancestor) — not our problem + return nil + } + + // Disconnected — cherry-pick local commits onto remote tip + fmt.Fprintln(os.Stderr, "[entire] Detected disconnected session metadata (local and remote share no common ancestor)") + + // Collect local commits oldest-first + localCommits, err := collectCommitChain(repo, localHash) + if err != nil { + return fmt.Errorf("failed to collect local commits: %w", err) + } + + // Filter out empty-tree commits (the orphan bug commit) + var dataCommits []*object.Commit + for _, c := range localCommits { + tree, treeErr := c.Tree() + if treeErr != nil { + return fmt.Errorf("failed to read tree for commit %s: %w", c.Hash.String()[:7], treeErr) + } + if len(tree.Entries) > 0 { + dataCommits = append(dataCommits, c) + } + } + + if len(dataCommits) == 0 { + // Local only had empty orphan — just point to remote + ref := plumbing.NewHashReference(refName, remoteHash) + if err := repo.Storer.SetReference(ref); err != nil { + return fmt.Errorf("failed to reset metadata branch to remote: %w", err) + } + fmt.Fprintln(os.Stderr, "[entire] Done — local had no checkpoint data, reset to remote") + return nil + } + + fmt.Fprintf(os.Stderr, "[entire] Cherry-picking %d local checkpoint(s) onto remote...\n", len(dataCommits)) + + newTip, err := cherryPickOnto(repo, remoteHash, dataCommits) + if err != nil { + return fmt.Errorf("failed to cherry-pick local commits onto remote: %w", err) + } + + // Update local branch ref + ref := plumbing.NewHashReference(refName, newTip) + if err := repo.Storer.SetReference(ref); err != nil { + return fmt.Errorf("failed to update metadata branch: %w", err) + } + + fmt.Fprintln(os.Stderr, "[entire] Done — all local and remote checkpoints preserved") + return nil +} + +// isDisconnected checks if two commits have no common ancestor using git merge-base. +// Returns (true, nil) if disconnected, (false, nil) if they share ancestry, +// or (false, error) if git merge-base failed for another reason. +// +// git merge-base exit codes: +// - 0: common ancestor found (shared ancestry) +// - 1: no common ancestor (disconnected) +// - 128+: error (corrupt repo, invalid hash, etc.) +func isDisconnected(repoPath, hashA, hashB string) (bool, error) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "git", "merge-base", hashA, hashB) + cmd.Dir = repoPath + if err := cmd.Run(); err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { + return true, nil // No common ancestor — disconnected + } + return false, fmt.Errorf("git merge-base failed: %w", err) + } + return false, nil // Shared ancestry +} + +// collectCommitChain walks from tip to root following first parent, returns oldest-first. +func collectCommitChain(repo *git.Repository, tip plumbing.Hash) ([]*object.Commit, error) { + var chain []*object.Commit + current := tip + + reachedRoot := false + for range MaxCommitTraversalDepth { + commit, err := repo.CommitObject(current) + if err != nil { + return nil, fmt.Errorf("failed to get commit %s: %w", current, err) + } + chain = append(chain, commit) + + if len(commit.ParentHashes) == 0 { + reachedRoot = true + break + } + current = commit.ParentHashes[0] + } + + if !reachedRoot { + return nil, fmt.Errorf("commit chain exceeded %d commits without reaching root; aborting reconciliation", MaxCommitTraversalDepth) + } + + // Reverse to oldest-first + for i, j := 0, len(chain)-1; i < j; i, j = i+1, j-1 { + chain[i], chain[j] = chain[j], chain[i] + } + + return chain, nil +} + +// cherryPickOnto applies each commit's delta onto base, building a linear chain. +// For each commit, it computes the diff from its parent using DiffTrees, then +// applies that delta onto the current tip's tree using ApplyTreeChanges. +func cherryPickOnto(repo *git.Repository, base plumbing.Hash, commits []*object.Commit) (plumbing.Hash, error) { + currentTip := base + + for _, commit := range commits { + // Flatten the commit's tree and its parent's tree to compute the delta + commitTree, err := commit.Tree() + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("failed to get tree for commit %s: %w", commit.Hash, err) + } + commitEntries := make(map[string]object.TreeEntry) + if err := checkpoint.FlattenTree(repo, commitTree, "", commitEntries); err != nil { + return plumbing.ZeroHash, fmt.Errorf("failed to flatten commit tree: %w", err) + } + + parentEntries := make(map[string]object.TreeEntry) + if len(commit.ParentHashes) > 0 { + parentCommit, pErr := repo.CommitObject(commit.ParentHashes[0]) + if pErr != nil { + return plumbing.ZeroHash, fmt.Errorf("failed to get parent commit %s: %w", commit.ParentHashes[0], pErr) + } + parentTree, ptErr := parentCommit.Tree() + if ptErr != nil { + return plumbing.ZeroHash, fmt.Errorf("failed to get parent tree for commit %s: %w", commit.ParentHashes[0], ptErr) + } + if err := checkpoint.FlattenTree(repo, parentTree, "", parentEntries); err != nil { + return plumbing.ZeroHash, fmt.Errorf("failed to flatten parent tree for commit %s: %w", commit.ParentHashes[0], err) + } + } + + // Compute delta and apply surgically to the tip tree + changes := checkpoint.DiffTrees(parentEntries, commitEntries) + if len(changes) == 0 { + continue // Skip no-op commits + } + + tipCommit, err := repo.CommitObject(currentTip) + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("failed to get tip commit: %w", err) + } + + mergedTreeHash, err := checkpoint.ApplyTreeChanges(repo, tipCommit.TreeHash, changes) + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("failed to apply tree changes: %w", err) + } + + newHash, err := createCherryPickCommit(repo, mergedTreeHash, currentTip, commit) + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("failed to create cherry-pick commit: %w", err) + } + + currentTip = newHash + } + + return currentTip, nil +} + +// createCherryPickCommit creates a new commit on top of parent, preserving the +// original commit's message and author. +func createCherryPickCommit(repo *git.Repository, treeHash, parent plumbing.Hash, original *object.Commit) (plumbing.Hash, error) { + authorName, authorEmail := GetGitAuthorFromRepo(repo) + now := time.Now() + + commit := &object.Commit{ + TreeHash: treeHash, + ParentHashes: []plumbing.Hash{parent}, + Author: original.Author, + Committer: object.Signature{ + Name: authorName, + Email: authorEmail, + When: now, + }, + Message: original.Message, + } + + obj := repo.Storer.NewEncodedObject() + if err := commit.Encode(obj); err != nil { + return plumbing.ZeroHash, fmt.Errorf("failed to encode commit: %w", err) + } + + hash, err := repo.Storer.SetEncodedObject(obj) + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("failed to store commit: %w", err) + } + + return hash, nil +} + +// getRepoPath returns the filesystem path for the repository's worktree. +func getRepoPath(repo *git.Repository) (string, error) { + wt, err := repo.Worktree() + if err != nil { + return "", fmt.Errorf("failed to get worktree: %w", err) + } + return wt.Filesystem.Root(), nil +} diff --git a/cmd/entire/cli/strategy/push_common.go b/cmd/entire/cli/strategy/push_common.go index 90337d2c3..aa4957428 100644 --- a/cmd/entire/cli/strategy/push_common.go +++ b/cmd/entire/cli/strategy/push_common.go @@ -168,20 +168,24 @@ func fetchAndMergeSessionsCommon(ctx context.Context, remote, branchName string) return fmt.Errorf("failed to get remote tree: %w", err) } - // Flatten both trees and combine entries - // Session logs have unique cond-* directories, so no conflicts expected - entries := make(map[string]object.TreeEntry) - if err := checkpoint.FlattenTree(repo, localTree, "", entries); err != nil { + // Compute what the remote has that differs from local, then apply surgically. + // Session logs have unique cond-* directories, so no conflicts expected. + localEntries := make(map[string]object.TreeEntry) + if err := checkpoint.FlattenTree(repo, localTree, "", localEntries); err != nil { return fmt.Errorf("failed to flatten local tree: %w", err) } - if err := checkpoint.FlattenTree(repo, remoteTree, "", entries); err != nil { + remoteEntries := make(map[string]object.TreeEntry) + if err := checkpoint.FlattenTree(repo, remoteTree, "", remoteEntries); err != nil { return fmt.Errorf("failed to flatten remote tree: %w", err) } - // Build merged tree - mergedTreeHash, err := checkpoint.BuildTreeFromEntries(repo, entries) + // DiffTrees computes changes to transform local into remote (remote wins on collision) + changes := checkpoint.DiffTrees(localEntries, remoteEntries) + + // Apply changes surgically to the local tree + mergedTreeHash, err := checkpoint.ApplyTreeChanges(repo, localCommit.TreeHash, changes) if err != nil { - return fmt.Errorf("failed to build merged tree: %w", err) + return fmt.Errorf("failed to apply tree changes: %w", err) } // Create merge commit with both parents From ba48780017c6ca9b9e158adcc310c176b91fb226 Mon Sep 17 00:00:00 2001 From: Daniel Vydra Date: Mon, 2 Mar 2026 17:38:09 +1100 Subject: [PATCH 2/2] fix: remove metadata_reconcile.go (belongs to PR #533, not yet merged) Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 2df5313f9ab9 --- cmd/entire/cli/strategy/metadata_reconcile.go | 272 ------------------ 1 file changed, 272 deletions(-) delete mode 100644 cmd/entire/cli/strategy/metadata_reconcile.go diff --git a/cmd/entire/cli/strategy/metadata_reconcile.go b/cmd/entire/cli/strategy/metadata_reconcile.go deleted file mode 100644 index ee7f78235..000000000 --- a/cmd/entire/cli/strategy/metadata_reconcile.go +++ /dev/null @@ -1,272 +0,0 @@ -package strategy - -import ( - "context" - "errors" - "fmt" - "os" - "os/exec" - "time" - - "github.com/entireio/cli/cmd/entire/cli/checkpoint" - "github.com/entireio/cli/cmd/entire/cli/paths" - - "github.com/go-git/go-git/v5" - "github.com/go-git/go-git/v5/plumbing" - "github.com/go-git/go-git/v5/plumbing/object" -) - -// ReconcileDisconnectedMetadataBranch detects and repairs disconnected local/remote -// entire/checkpoints/v1 branches. Disconnected means no common ancestor, which -// only happens due to the empty-orphan bug. Diverged (shared ancestor) is normal -// and handled by the push path's tree merge. -// -// Repair strategy: cherry-pick local commits onto remote tip, preserving all data. -// Checkpoint shards use unique paths (//), so cherry-picks always -// apply cleanly. -func ReconcileDisconnectedMetadataBranch(repo *git.Repository) error { - refName := plumbing.NewBranchReferenceName(paths.MetadataBranchName) - - // Check local branch - localRef, err := repo.Reference(refName, true) - if errors.Is(err, plumbing.ErrReferenceNotFound) { - return nil // No local branch — nothing to reconcile - } - if err != nil { - return fmt.Errorf("failed to check local metadata branch: %w", err) - } - - // Check remote-tracking branch - remoteRefName := plumbing.NewRemoteReferenceName("origin", paths.MetadataBranchName) - remoteRef, err := repo.Reference(remoteRefName, true) - if errors.Is(err, plumbing.ErrReferenceNotFound) { - return nil // No remote branch — nothing to reconcile - } - if err != nil { - return fmt.Errorf("failed to check remote metadata branch: %w", err) - } - - localHash := localRef.Hash() - remoteHash := remoteRef.Hash() - - // Same hash — nothing to do - if localHash == remoteHash { - return nil - } - - // Check if disconnected using git merge-base - repoPath, err := getRepoPath(repo) - if err != nil { - return err - } - - disconnected, err := isDisconnected(repoPath, localHash.String(), remoteHash.String()) - if err != nil { - return fmt.Errorf("failed to check metadata branch ancestry: %w", err) - } - if !disconnected { - // Shared ancestry (diverged or ancestor) — not our problem - return nil - } - - // Disconnected — cherry-pick local commits onto remote tip - fmt.Fprintln(os.Stderr, "[entire] Detected disconnected session metadata (local and remote share no common ancestor)") - - // Collect local commits oldest-first - localCommits, err := collectCommitChain(repo, localHash) - if err != nil { - return fmt.Errorf("failed to collect local commits: %w", err) - } - - // Filter out empty-tree commits (the orphan bug commit) - var dataCommits []*object.Commit - for _, c := range localCommits { - tree, treeErr := c.Tree() - if treeErr != nil { - return fmt.Errorf("failed to read tree for commit %s: %w", c.Hash.String()[:7], treeErr) - } - if len(tree.Entries) > 0 { - dataCommits = append(dataCommits, c) - } - } - - if len(dataCommits) == 0 { - // Local only had empty orphan — just point to remote - ref := plumbing.NewHashReference(refName, remoteHash) - if err := repo.Storer.SetReference(ref); err != nil { - return fmt.Errorf("failed to reset metadata branch to remote: %w", err) - } - fmt.Fprintln(os.Stderr, "[entire] Done — local had no checkpoint data, reset to remote") - return nil - } - - fmt.Fprintf(os.Stderr, "[entire] Cherry-picking %d local checkpoint(s) onto remote...\n", len(dataCommits)) - - newTip, err := cherryPickOnto(repo, remoteHash, dataCommits) - if err != nil { - return fmt.Errorf("failed to cherry-pick local commits onto remote: %w", err) - } - - // Update local branch ref - ref := plumbing.NewHashReference(refName, newTip) - if err := repo.Storer.SetReference(ref); err != nil { - return fmt.Errorf("failed to update metadata branch: %w", err) - } - - fmt.Fprintln(os.Stderr, "[entire] Done — all local and remote checkpoints preserved") - return nil -} - -// isDisconnected checks if two commits have no common ancestor using git merge-base. -// Returns (true, nil) if disconnected, (false, nil) if they share ancestry, -// or (false, error) if git merge-base failed for another reason. -// -// git merge-base exit codes: -// - 0: common ancestor found (shared ancestry) -// - 1: no common ancestor (disconnected) -// - 128+: error (corrupt repo, invalid hash, etc.) -func isDisconnected(repoPath, hashA, hashB string) (bool, error) { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - cmd := exec.CommandContext(ctx, "git", "merge-base", hashA, hashB) - cmd.Dir = repoPath - if err := cmd.Run(); err != nil { - var exitErr *exec.ExitError - if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { - return true, nil // No common ancestor — disconnected - } - return false, fmt.Errorf("git merge-base failed: %w", err) - } - return false, nil // Shared ancestry -} - -// collectCommitChain walks from tip to root following first parent, returns oldest-first. -func collectCommitChain(repo *git.Repository, tip plumbing.Hash) ([]*object.Commit, error) { - var chain []*object.Commit - current := tip - - reachedRoot := false - for range MaxCommitTraversalDepth { - commit, err := repo.CommitObject(current) - if err != nil { - return nil, fmt.Errorf("failed to get commit %s: %w", current, err) - } - chain = append(chain, commit) - - if len(commit.ParentHashes) == 0 { - reachedRoot = true - break - } - current = commit.ParentHashes[0] - } - - if !reachedRoot { - return nil, fmt.Errorf("commit chain exceeded %d commits without reaching root; aborting reconciliation", MaxCommitTraversalDepth) - } - - // Reverse to oldest-first - for i, j := 0, len(chain)-1; i < j; i, j = i+1, j-1 { - chain[i], chain[j] = chain[j], chain[i] - } - - return chain, nil -} - -// cherryPickOnto applies each commit's delta onto base, building a linear chain. -// For each commit, it computes the diff from its parent using DiffTrees, then -// applies that delta onto the current tip's tree using ApplyTreeChanges. -func cherryPickOnto(repo *git.Repository, base plumbing.Hash, commits []*object.Commit) (plumbing.Hash, error) { - currentTip := base - - for _, commit := range commits { - // Flatten the commit's tree and its parent's tree to compute the delta - commitTree, err := commit.Tree() - if err != nil { - return plumbing.ZeroHash, fmt.Errorf("failed to get tree for commit %s: %w", commit.Hash, err) - } - commitEntries := make(map[string]object.TreeEntry) - if err := checkpoint.FlattenTree(repo, commitTree, "", commitEntries); err != nil { - return plumbing.ZeroHash, fmt.Errorf("failed to flatten commit tree: %w", err) - } - - parentEntries := make(map[string]object.TreeEntry) - if len(commit.ParentHashes) > 0 { - parentCommit, pErr := repo.CommitObject(commit.ParentHashes[0]) - if pErr != nil { - return plumbing.ZeroHash, fmt.Errorf("failed to get parent commit %s: %w", commit.ParentHashes[0], pErr) - } - parentTree, ptErr := parentCommit.Tree() - if ptErr != nil { - return plumbing.ZeroHash, fmt.Errorf("failed to get parent tree for commit %s: %w", commit.ParentHashes[0], ptErr) - } - if err := checkpoint.FlattenTree(repo, parentTree, "", parentEntries); err != nil { - return plumbing.ZeroHash, fmt.Errorf("failed to flatten parent tree for commit %s: %w", commit.ParentHashes[0], err) - } - } - - // Compute delta and apply surgically to the tip tree - changes := checkpoint.DiffTrees(parentEntries, commitEntries) - if len(changes) == 0 { - continue // Skip no-op commits - } - - tipCommit, err := repo.CommitObject(currentTip) - if err != nil { - return plumbing.ZeroHash, fmt.Errorf("failed to get tip commit: %w", err) - } - - mergedTreeHash, err := checkpoint.ApplyTreeChanges(repo, tipCommit.TreeHash, changes) - if err != nil { - return plumbing.ZeroHash, fmt.Errorf("failed to apply tree changes: %w", err) - } - - newHash, err := createCherryPickCommit(repo, mergedTreeHash, currentTip, commit) - if err != nil { - return plumbing.ZeroHash, fmt.Errorf("failed to create cherry-pick commit: %w", err) - } - - currentTip = newHash - } - - return currentTip, nil -} - -// createCherryPickCommit creates a new commit on top of parent, preserving the -// original commit's message and author. -func createCherryPickCommit(repo *git.Repository, treeHash, parent plumbing.Hash, original *object.Commit) (plumbing.Hash, error) { - authorName, authorEmail := GetGitAuthorFromRepo(repo) - now := time.Now() - - commit := &object.Commit{ - TreeHash: treeHash, - ParentHashes: []plumbing.Hash{parent}, - Author: original.Author, - Committer: object.Signature{ - Name: authorName, - Email: authorEmail, - When: now, - }, - Message: original.Message, - } - - obj := repo.Storer.NewEncodedObject() - if err := commit.Encode(obj); err != nil { - return plumbing.ZeroHash, fmt.Errorf("failed to encode commit: %w", err) - } - - hash, err := repo.Storer.SetEncodedObject(obj) - if err != nil { - return plumbing.ZeroHash, fmt.Errorf("failed to store commit: %w", err) - } - - return hash, nil -} - -// getRepoPath returns the filesystem path for the repository's worktree. -func getRepoPath(repo *git.Repository) (string, error) { - wt, err := repo.Worktree() - if err != nil { - return "", fmt.Errorf("failed to get worktree: %w", err) - } - return wt.Filesystem.Root(), nil -}