From c5d68c227cc2382ef1b1b4b41f8f0c0c85ec5abb Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 25 Mar 2026 22:17:52 -0400 Subject: [PATCH] fix: use merge-base anchors for stale branch adoption Fix HACK-615 by teaching stack track to record a repairable restack anchor when adopting a branch whose parent has already moved. If the current parent tip is not an ancestor, track now stores the merge-base instead of unusable metadata. --- docs/adopting-existing-prs.md | 5 +++ docs/usage.md | 4 ++ internal/cmd/root.go | 30 +++++++++++++- internal/cmd/root_test.go | 73 +++++++++++++++++++++++++++++++++++ internal/git/client.go | 30 ++++++++++++++ 5 files changed, 141 insertions(+), 1 deletion(-) diff --git a/docs/adopting-existing-prs.md b/docs/adopting-existing-prs.md index ede72db..bab5312 100644 --- a/docs/adopting-existing-prs.md +++ b/docs/adopting-existing-prs.md @@ -73,6 +73,11 @@ stack track feature/runtime --parent feature/parser stack track feature/ui --parent feature/runtime ``` +When the parent branch has moved since a child branch was originally cut, +`stack track` records a merge-base-style restack anchor instead of blindly +assuming the current parent tip. That makes stale existing PR heads adoptable +without breaking the first `stack restack`. + If the first parent chain is wrong, that is fine. Get a draft graph in place first. diff --git a/docs/usage.md b/docs/usage.md index 66f7e2d..198fb8b 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -21,6 +21,10 @@ Or adopt an existing branch and make the parent explicit: stack track feature/child --parent feature/base ``` +If the branch is stale and `feature/base` has moved since it was cut, `track` +records a repairable restack anchor from shared history instead of the current +parent tip. + If you already have a larger set of open PRs and want to turn them into an explicit stack after the fact, use [adopting-existing-prs.md](adopting-existing-prs.md). diff --git a/internal/cmd/root.go b/internal/cmd/root.go index f2cb4b9..ba83070 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -221,7 +221,10 @@ func newTrackCommand(runtime *stackruntime.Runtime) *cobra.Command { return err } - parentOID, _ := runtime.Git.ResolveRef(runtime.Context, parent) + parentOID, err := trackRestackAnchor(runtime, branch, parent) + if err != nil { + return err + } state.Branches[branch] = store.BranchRecord{ ParentBranch: parent, RemoteName: state.DefaultRemote, @@ -1351,6 +1354,31 @@ func ensureTrackedParentAllowed(state store.RepoState, parent string) error { return fmt.Errorf("parent branch %q is not tracked in local metadata; track it first or move under %s", parent, state.Trunk) } +func trackRestackAnchor(runtime *stackruntime.Runtime, branch string, parent string) (string, error) { + parentOID, err := runtime.Git.ResolveRef(runtime.Context, parent) + if err != nil { + return "", err + } + + validAnchor, err := runtime.Git.IsAncestor(runtime.Context, parentOID, branch) + if err != nil { + return "", err + } + if validAnchor { + return parentOID, nil + } + + mergeBase, ok, err := runtime.Git.MergeBase(runtime.Context, parent, branch) + if err != nil { + return "", err + } + if !ok || mergeBase == "" { + return "", fmt.Errorf("branch %q does not share a repairable merge base with parent %q; rebase it first or choose a different parent", branch, parent) + } + + return mergeBase, nil +} + func resolveOID(runtime *stackruntime.Runtime, ref string) string { oid, err := runtime.Git.ResolveRef(runtime.Context, ref) if err != nil { diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index db2b52e..45e910b 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -918,6 +918,79 @@ func TestTrackRejectsUntrackedParent(t *testing.T) { } } +func TestTrackUsesMergeBaseAnchorForStaleAdoption(t *testing.T) { + repo := testutil.SetupGitRepo(t) + + testutil.Run(t, repo, "git", "switch", "-c", "feature/base") + testutil.WriteFile(t, filepath.Join(repo, "feature-base.txt"), "feature base\n") + testutil.Run(t, repo, "git", "add", "feature-base.txt") + testutil.Run(t, repo, "git", "commit", "-m", "add feature base") + originalFeatureBaseHead := strings.TrimSpace(testutil.Run(t, repo, "git", "rev-parse", "HEAD")) + + testutil.Run(t, repo, "git", "switch", "-c", "feature/child") + testutil.WriteFile(t, filepath.Join(repo, "feature-child.txt"), "feature child\n") + testutil.Run(t, repo, "git", "add", "feature-child.txt") + testutil.Run(t, repo, "git", "commit", "-m", "add feature child") + originalFeatureChildHead := strings.TrimSpace(testutil.Run(t, repo, "git", "rev-parse", "HEAD")) + + testutil.Run(t, repo, "git", "switch", "feature/base") + testutil.WriteFile(t, filepath.Join(repo, "feature-base.txt"), "feature base advanced\n") + testutil.Run(t, repo, "git", "add", "feature-base.txt") + testutil.Run(t, repo, "git", "commit", "-m", "advance feature base") + advancedFeatureBaseHead := strings.TrimSpace(testutil.Run(t, repo, "git", "rev-parse", "HEAD")) + + runtime := newTestRuntime(repo) + mainHead, err := runtime.Git.ResolveRef(runtime.Context, "main") + if err != nil { + t.Fatalf("resolve main head: %v", err) + } + state := store.RepoState{ + Version: 1, + Repo: "hack-dance/stack", + DefaultRemote: "origin", + Trunk: "main", + Branches: map[string]store.BranchRecord{}, + } + if err := runtime.Store.WriteState(runtime.Context, state); err != nil { + t.Fatalf("write state: %v", err) + } + + executeCommand(t, runtime, "track", "feature/base", "--parent", "main") + executeCommand(t, runtime, "track", "feature/child", "--parent", "feature/base") + + state, err = runtime.Store.ReadState(runtime.Context) + if err != nil { + t.Fatalf("read state after track: %v", err) + } + if got := state.Branches["feature/base"].Restack.LastParentHeadOID; got != mainHead { + t.Fatalf("expected feature/base anchor %q, got %q", mainHead, got) + } + if got := state.Branches["feature/child"].Restack.LastParentHeadOID; got != originalFeatureBaseHead { + t.Fatalf("expected feature/child merge-base anchor %q, got %q", originalFeatureBaseHead, got) + } + if originalFeatureBaseHead == advancedFeatureBaseHead { + t.Fatalf("expected feature/base head to advance") + } + + executeCommand(t, runtime, "restack", "--all", "--yes") + + state, err = runtime.Store.ReadState(runtime.Context) + if err != nil { + t.Fatalf("read state after restack: %v", err) + } + if got := state.Branches["feature/child"].Restack.LastParentHeadOID; got != advancedFeatureBaseHead { + t.Fatalf("expected feature/child anchor to update to %q, got %q", advancedFeatureBaseHead, got) + } + if got := strings.TrimSpace(testutil.Run(t, repo, "git", "rev-parse", "feature/child")); got == originalFeatureChildHead { + t.Fatalf("expected feature/child head to change after restack") + } + + mergeBase := strings.TrimSpace(testutil.Run(t, repo, "git", "merge-base", "feature/base", "feature/child")) + if mergeBase != advancedFeatureBaseHead { + t.Fatalf("expected feature/child merge-base %q after restack, got %q", advancedFeatureBaseHead, mergeBase) + } +} + func TestVersionCommandPrintsBuildInfo(t *testing.T) { repo := testutil.SetupGitRepo(t) runtime := newTestRuntime(repo) diff --git a/internal/git/client.go b/internal/git/client.go index a3dfaa4..18b3f6f 100644 --- a/internal/git/client.go +++ b/internal/git/client.go @@ -114,6 +114,36 @@ func (c *Client) IsAncestor(ctx context.Context, ancestor string, descendant str return false, err } +func (c *Client) MergeBase(ctx context.Context, left string, right string) (string, bool, error) { + cmd := exec.CommandContext(ctx, "git", "merge-base", left, right) + cmd.Dir = c.cwd + + var stdout bytes.Buffer + var stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + err := cmd.Run() + if err == nil { + return strings.TrimSpace(stdout.String()), true, nil + } + + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { + return "", false, nil + } + + message := strings.TrimSpace(stderr.String()) + if message == "" { + message = strings.TrimSpace(stdout.String()) + } + if message == "" { + message = err.Error() + } + + return "", false, fmt.Errorf("git merge-base %s %s: %s", left, right, message) +} + func (c *Client) SwitchCreate(ctx context.Context, branch string) error { _, err := c.run(ctx, "switch", "-c", branch) return err