Skip to content

Commit a67b5cc

Browse files
committed
fix: enforce stack parent invariants and deterministic PR lookup
1 parent 5471af3 commit a67b5cc

8 files changed

Lines changed: 312 additions & 5 deletions

File tree

README.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ base, head, and remote state, then hands that PR to GitHub auto-merge or merge
4242
queue. After the merge lands, `stack sync` helps the rest of the stack catch up
4343
without guessing through ambiguous cases.
4444

45+
That handoff uses GitHub's own auto-merge path via `gh pr merge --auto`, so the
46+
repository must have auto-merge enabled. If the repo also uses merge queue,
47+
GitHub decides whether the PR goes straight to auto-merge or enters the queue.
48+
4549
## How it differs from Graphite and similar tools
4650

4751
`stack` is closest in spirit to tools that keep explicit local stack metadata,
@@ -75,6 +79,9 @@ stack submit --all
7579
stack queue feature/base
7680
```
7781

82+
Before using `stack queue`, make sure the GitHub repository has auto-merge
83+
enabled.
84+
7885
For the full daily workflow, start with [docs/usage.md](docs/usage.md).
7986

8087
## Starting from existing PRs

docs/how-it-works.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ branch, and PR head all match, then hands that PR to GitHub auto-merge or merge
4545
queue. After the merge lands, `stack sync` helps advance the remaining stack to
4646
the next safe state.
4747

48+
Because `stack` delegates that final step to GitHub, the repository must have
49+
auto-merge enabled. On repos with merge queue configured, GitHub applies queue
50+
policy after the handoff.
51+
4852
## How this differs from Graphite and similar tools
4953

5054
The important difference is workflow shape: `stack` keeps branches and PRs

docs/troubleshooting.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ Then choose a repair path deliberately.
7272
Check:
7373

7474
- `gh auth status`
75+
- GitHub repository auto-merge is enabled
7576
- the branch is pushed to the expected remote
7677
- the tracked PR is open and on the expected base
7778
- the local head still matches the pushed head
@@ -83,6 +84,10 @@ stack submit <branch>
8384
stack queue <branch>
8485
```
8586

87+
If the CLI reports multiple open PRs for one head branch, it is refusing to
88+
guess which live PR owns that branch. Close or retarget the duplicate until one
89+
open PR remains for that head name, then rerun `stack submit`.
90+
8691
## Release automation does not update the tap
8792

8893
The release workflow needs:

docs/usage.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ and cached PR state.
4545
4. Run `stack queue <branch>` only when the bottom branch targets trunk and is healthy.
4646
5. Run `stack sync` after merges or GitHub-side base changes.
4747

48+
For `stack queue`, GitHub repository auto-merge must be enabled. `stack` hands
49+
off through `gh pr merge --auto`, then GitHub applies the repo's normal
50+
auto-merge or merge-queue policy.
51+
4852
## Repair loop
4953

5054
Use `stack sync` first when local metadata and GitHub disagree.
@@ -68,3 +72,4 @@ clean recovery point.
6872
- `move`, `restack`, `submit`, and `queue` preview before destructive work unless you pass `--yes`
6973
- `sync` stops on ambiguous merged-parent cases instead of guessing
7074
- `queue` is only for a healthy bottom-of-stack PR
75+
- `queue` requires GitHub repository auto-merge to be enabled

internal/cmd/root.go

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,9 @@ func newCreateCommand(runtime *stackruntime.Runtime) *cobra.Command {
153153
if err != nil {
154154
return err
155155
}
156+
if err := ensureCreateParentAllowed(state, parent); err != nil {
157+
return err
158+
}
156159

157160
if err := runtime.Git.SwitchCreate(runtime.Context, args[0]); err != nil {
158161
return err
@@ -211,6 +214,9 @@ func newTrackCommand(runtime *stackruntime.Runtime) *cobra.Command {
211214
if parent != state.Trunk && !runtime.Git.BranchExists(runtime.Context, parent) {
212215
return fmt.Errorf("parent branch %q does not exist locally", parent)
213216
}
217+
if err := ensureTrackedParentAllowed(state, parent); err != nil {
218+
return err
219+
}
214220
if err := stack.EnsureBranchCanParent(state, branch, parent); err != nil {
215221
return err
216222
}
@@ -463,10 +469,8 @@ stack move feature/b --parent main --yes
463469
if !runtime.Git.BranchExists(runtime.Context, parent) && parent != state.Trunk {
464470
return fmt.Errorf("parent branch %q does not exist locally", parent)
465471
}
466-
if parent != state.Trunk {
467-
if _, ok := state.Branches[parent]; !ok {
468-
return fmt.Errorf("parent branch %q is not tracked in local metadata; track it first or move under %s", parent, state.Trunk)
469-
}
472+
if err := ensureTrackedParentAllowed(state, parent); err != nil {
473+
return err
470474
}
471475
if err := stack.EnsureBranchCanParent(state, branch, parent); err != nil {
472476
return err
@@ -1322,6 +1326,31 @@ func validateTrackedPR(branch string, record store.BranchRecord) error {
13221326
return nil
13231327
}
13241328

1329+
func ensureCreateParentAllowed(state store.RepoState, parent string) error {
1330+
parent = strings.TrimSpace(parent)
1331+
if parent == "" {
1332+
return fmt.Errorf("cannot create a tracked branch from detached HEAD; switch to %s or another tracked branch first", state.Trunk)
1333+
}
1334+
if parent == state.Trunk {
1335+
return nil
1336+
}
1337+
if _, ok := state.Branches[parent]; ok {
1338+
return nil
1339+
}
1340+
return fmt.Errorf("current branch %q is not tracked in local metadata; track it first or switch to %s", parent, state.Trunk)
1341+
}
1342+
1343+
func ensureTrackedParentAllowed(state store.RepoState, parent string) error {
1344+
parent = strings.TrimSpace(parent)
1345+
if parent == "" || parent == state.Trunk {
1346+
return nil
1347+
}
1348+
if _, ok := state.Branches[parent]; ok {
1349+
return nil
1350+
}
1351+
return fmt.Errorf("parent branch %q is not tracked in local metadata; track it first or move under %s", parent, state.Trunk)
1352+
}
1353+
13251354
func resolveOID(runtime *stackruntime.Runtime, ref string) string {
13261355
oid, err := runtime.Git.ResolveRef(runtime.Context, ref)
13271356
if err != nil {

internal/cmd/root_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -821,6 +821,103 @@ func TestSubmitCreatesAndTracksPR(t *testing.T) {
821821
}
822822
}
823823

824+
func TestCreateRejectsDetachedHEAD(t *testing.T) {
825+
repo := testutil.SetupGitRepo(t)
826+
testutil.Run(t, repo, "git", "checkout", "--detach")
827+
828+
runtime := newTestRuntime(repo)
829+
state := store.RepoState{
830+
Version: 1,
831+
Repo: "hack-dance/stack",
832+
DefaultRemote: "origin",
833+
Trunk: "main",
834+
Branches: map[string]store.BranchRecord{},
835+
}
836+
if err := runtime.Store.WriteState(runtime.Context, state); err != nil {
837+
t.Fatalf("write state: %v", err)
838+
}
839+
840+
err := executeCommandExpectError(runtime, "create", "feature/a")
841+
if err == nil || !strings.Contains(err.Error(), "detached HEAD") {
842+
t.Fatalf("expected detached HEAD error, got %v", err)
843+
}
844+
845+
state, err = runtime.Store.ReadState(runtime.Context)
846+
if err != nil {
847+
t.Fatalf("read state: %v", err)
848+
}
849+
if len(state.Branches) != 0 {
850+
t.Fatalf("expected no tracked branches, got %+v", state.Branches)
851+
}
852+
if runtime.Git.BranchExists(runtime.Context, "feature/a") {
853+
t.Fatalf("expected feature/a branch to not be created")
854+
}
855+
}
856+
857+
func TestCreateRejectsUntrackedCurrentParent(t *testing.T) {
858+
repo := testutil.SetupGitRepo(t)
859+
testutil.Run(t, repo, "git", "switch", "-c", "feature/base")
860+
861+
runtime := newTestRuntime(repo)
862+
state := store.RepoState{
863+
Version: 1,
864+
Repo: "hack-dance/stack",
865+
DefaultRemote: "origin",
866+
Trunk: "main",
867+
Branches: map[string]store.BranchRecord{},
868+
}
869+
if err := runtime.Store.WriteState(runtime.Context, state); err != nil {
870+
t.Fatalf("write state: %v", err)
871+
}
872+
873+
err := executeCommandExpectError(runtime, "create", "feature/child")
874+
if err == nil || !strings.Contains(err.Error(), "not tracked in local metadata") {
875+
t.Fatalf("expected untracked parent error, got %v", err)
876+
}
877+
878+
state, err = runtime.Store.ReadState(runtime.Context)
879+
if err != nil {
880+
t.Fatalf("read state: %v", err)
881+
}
882+
if len(state.Branches) != 0 {
883+
t.Fatalf("expected no tracked branches, got %+v", state.Branches)
884+
}
885+
if runtime.Git.BranchExists(runtime.Context, "feature/child") {
886+
t.Fatalf("expected feature/child branch to not be created")
887+
}
888+
}
889+
890+
func TestTrackRejectsUntrackedParent(t *testing.T) {
891+
repo := testutil.SetupGitRepo(t)
892+
testutil.Run(t, repo, "git", "switch", "-c", "feature/base")
893+
testutil.Run(t, repo, "git", "switch", "-c", "feature/child")
894+
895+
runtime := newTestRuntime(repo)
896+
state := store.RepoState{
897+
Version: 1,
898+
Repo: "hack-dance/stack",
899+
DefaultRemote: "origin",
900+
Trunk: "main",
901+
Branches: map[string]store.BranchRecord{},
902+
}
903+
if err := runtime.Store.WriteState(runtime.Context, state); err != nil {
904+
t.Fatalf("write state: %v", err)
905+
}
906+
907+
err := executeCommandExpectError(runtime, "track", "feature/child", "--parent", "feature/base")
908+
if err == nil || !strings.Contains(err.Error(), "parent branch \"feature/base\" is not tracked in local metadata") {
909+
t.Fatalf("expected untracked parent error, got %v", err)
910+
}
911+
912+
state, err = runtime.Store.ReadState(runtime.Context)
913+
if err != nil {
914+
t.Fatalf("read state: %v", err)
915+
}
916+
if len(state.Branches) != 0 {
917+
t.Fatalf("expected no tracked branches, got %+v", state.Branches)
918+
}
919+
}
920+
824921
func TestVersionCommandPrintsBuildInfo(t *testing.T) {
825922
repo := testutil.SetupGitRepo(t)
826923
runtime := newTestRuntime(repo)

internal/github/client.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,25 @@ func (c *Client) FindPRByHead(ctx context.Context, branch string) (store.PullReq
9494
return store.PullRequest{}, nil
9595
}
9696

97-
return payload[0].toStorePullRequest(), nil
97+
open := make([]pullRequestPayload, 0, len(payload))
98+
for _, pr := range payload {
99+
if pr.State == "OPEN" {
100+
open = append(open, pr)
101+
}
102+
}
103+
104+
switch len(open) {
105+
case 0:
106+
return store.PullRequest{}, nil
107+
case 1:
108+
return open[0].toStorePullRequest(), nil
109+
default:
110+
numbers := make([]string, 0, len(open))
111+
for _, pr := range open {
112+
numbers = append(numbers, fmt.Sprintf("#%d", pr.Number))
113+
}
114+
return store.PullRequest{}, fmt.Errorf("multiple open pull requests match head %q: %s", branch, strings.Join(numbers, ", "))
115+
}
98116
}
99117

100118
func (c *Client) CreatePR(ctx context.Context, base string, head string, title string, body string, draft bool) (store.PullRequest, error) {

0 commit comments

Comments
 (0)