Skip to content

Commit 9728b38

Browse files
skarimCopilot
andcommitted
skip stale merged PRs when syncing branches with reused names
When a branch name was previously used for a merged PR, syncStackPRs would adopt the old merged PR for the new branch, falsely showing it as merged in gh stack view. Now, branches with no recorded PR only adopt open PRs from the API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9c58a54 commit 9728b38

2 files changed

Lines changed: 110 additions & 0 deletions

File tree

cmd/utils.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,13 @@ func syncStackPRs(cfg *config.Config, s *stack.Stack) {
252252
continue
253253
}
254254

255+
// If this branch has no recorded PR yet and the API returned a
256+
// merged or closed PR, skip it — it's likely from a prior use of
257+
// the same branch name and doesn't belong to this stack.
258+
if b.PullRequest == nil && (pr.Merged || pr.State == "CLOSED") {
259+
continue
260+
}
261+
255262
b.PullRequest = &stack.PullRequestRef{
256263
Number: pr.Number,
257264
ID: pr.ID,

cmd/utils_test.go

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@ package cmd
33
import (
44
"errors"
55
"fmt"
6+
"io"
67
"strings"
78
"testing"
89

910
"github.com/AlecAivazis/survey/v2/terminal"
1011
"github.com/github/gh-stack/internal/config"
1112
"github.com/github/gh-stack/internal/git"
13+
"github.com/github/gh-stack/internal/github"
1214
"github.com/github/gh-stack/internal/stack"
1315
"github.com/stretchr/testify/assert"
1416
)
@@ -271,3 +273,104 @@ func TestParsePRURL(t *testing.T) {
271273
})
272274
}
273275
}
276+
277+
func TestSyncStackPRs_SkipsStalesMergedPRForReusedBranch(t *testing.T) {
278+
s := &stack.Stack{
279+
Trunk: stack.BranchRef{Branch: "main"},
280+
Branches: []stack.BranchRef{
281+
{Branch: "feat-a"}, // No PullRequest recorded yet
282+
},
283+
}
284+
285+
cfg, _, errR := config.NewTestConfig()
286+
cfg.GitHubClientOverride = &github.MockClient{
287+
FindAnyPRForBranchFn: func(branch string) (*github.PullRequest, error) {
288+
// Simulate a previously merged PR with the same branch name.
289+
return &github.PullRequest{
290+
Number: 20,
291+
ID: "PR_20",
292+
URL: "https://github.com/o/r/pull/20",
293+
State: "MERGED",
294+
Merged: true,
295+
}, nil
296+
},
297+
}
298+
299+
syncStackPRs(cfg, s)
300+
301+
cfg.Err.Close()
302+
_, _ = io.ReadAll(errR)
303+
304+
// The merged PR from a prior use of the branch name should NOT be adopted.
305+
assert.Nil(t, s.Branches[0].PullRequest, "should not adopt a merged PR for a branch with no recorded PR")
306+
assert.False(t, s.Branches[0].IsMerged(), "branch should not appear merged")
307+
}
308+
309+
func TestSyncStackPRs_AdoptsOpenPRForNewBranch(t *testing.T) {
310+
s := &stack.Stack{
311+
Trunk: stack.BranchRef{Branch: "main"},
312+
Branches: []stack.BranchRef{
313+
{Branch: "feat-b"}, // No PullRequest recorded yet
314+
},
315+
}
316+
317+
cfg, _, errR := config.NewTestConfig()
318+
cfg.GitHubClientOverride = &github.MockClient{
319+
FindAnyPRForBranchFn: func(branch string) (*github.PullRequest, error) {
320+
return &github.PullRequest{
321+
Number: 42,
322+
ID: "PR_42",
323+
URL: "https://github.com/o/r/pull/42",
324+
State: "OPEN",
325+
Merged: false,
326+
}, nil
327+
},
328+
}
329+
330+
syncStackPRs(cfg, s)
331+
332+
cfg.Err.Close()
333+
_, _ = io.ReadAll(errR)
334+
335+
// An open PR should be adopted normally.
336+
assert.NotNil(t, s.Branches[0].PullRequest, "should adopt an open PR")
337+
assert.Equal(t, 42, s.Branches[0].PullRequest.Number)
338+
assert.False(t, s.Branches[0].IsMerged())
339+
}
340+
341+
func TestSyncStackPRs_UpdatesExistingPRMergedStatus(t *testing.T) {
342+
s := &stack.Stack{
343+
Trunk: stack.BranchRef{Branch: "main"},
344+
Branches: []stack.BranchRef{
345+
{
346+
Branch: "feat-c",
347+
PullRequest: &stack.PullRequestRef{
348+
Number: 55,
349+
ID: "PR_55",
350+
},
351+
},
352+
},
353+
}
354+
355+
cfg, _, errR := config.NewTestConfig()
356+
cfg.GitHubClientOverride = &github.MockClient{
357+
FindAnyPRForBranchFn: func(branch string) (*github.PullRequest, error) {
358+
return &github.PullRequest{
359+
Number: 55,
360+
ID: "PR_55",
361+
URL: "https://github.com/o/r/pull/55",
362+
State: "MERGED",
363+
Merged: true,
364+
}, nil
365+
},
366+
}
367+
368+
syncStackPRs(cfg, s)
369+
370+
cfg.Err.Close()
371+
_, _ = io.ReadAll(errR)
372+
373+
// When a PR is already recorded, merged status should be synced.
374+
assert.NotNil(t, s.Branches[0].PullRequest)
375+
assert.True(t, s.Branches[0].IsMerged(), "should update merged status for already-recorded PR")
376+
}

0 commit comments

Comments
 (0)