Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/adopting-existing-prs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
4 changes: 4 additions & 0 deletions docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
30 changes: 29 additions & 1 deletion internal/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Comment on lines +1375 to +1379

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject unsafe merge-base anchors for rebased parents

trackRestackAnchor currently accepts any git merge-base(parent, branch) result as a repairable anchor and stores it in LastParentHeadOID, but that is unsafe when the parent was rebased/force-pushed: the merge base may fall back to trunk, and runRestackPlan later calls git rebase --onto <parent> <anchor> <branch>, which replays old parent-only commits onto the child (not just child commits). This changes prior behavior from a hard stop on invalid anchors to silent history corruption in stale-adoption flows where the parent history was rewritten, so this fallback needs an additional safety check (or explicit refusal) before accepting the merge-base anchor.

Useful? React with 👍 / 👎.

}

func resolveOID(runtime *stackruntime.Runtime, ref string) string {
oid, err := runtime.Git.ResolveRef(runtime.Context, ref)
if err != nil {
Expand Down
73 changes: 73 additions & 0 deletions internal/cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
30 changes: 30 additions & 0 deletions internal/git/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading