Skip to content

Commit 6dcf9f0

Browse files
CopilotCopilot
authored
Respect repository default branch in gh stack link (#265)
* Initial plan * Respect repository default branch in gh stack link --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent a365459 commit 6dcf9f0

5 files changed

Lines changed: 261 additions & 6 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ If the PRs are not yet in a stack, a new stack is created. If some of the PRs ar
416416

417417
| Flag | Description |
418418
|------|-------------|
419-
| `--base <branch>` | Base branch for the bottom of the stack (default: `main`) |
419+
| `--base <branch>` | Base branch for the bottom of the stack (defaults to the repository's default branch) |
420420
| `--open` | Mark new and existing PRs as ready for review |
421421
| `--remote <name>` | Remote to push to (defaults to auto-detected remote) |
422422

‎cmd/link.go‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ only when it matches an existing stack.`,
7777
},
7878
}
7979

80-
cmd.Flags().StringVar(&opts.base, "base", "main", "Base branch for the bottom of the stack")
80+
cmd.Flags().StringVar(&opts.base, "base", "", "Base branch for the bottom of the stack (defaults to the repository default branch)")
8181
cmd.Flags().BoolVar(&opts.open, "open", false, "Mark new and existing PRs as ready for review")
8282
cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to push to (defaults to auto-detected remote)")
8383

@@ -209,6 +209,18 @@ func runLinkCreateOrUpdate(cfg *config.Config, client github.ClientOps, opts *li
209209
}
210210
}
211211

212+
// Resolve the base branch for the bottom of the stack. When --base isn't
213+
// given, default to the repository's default branch (like `gh stack init`).
214+
bottomBase := opts.base
215+
if bottomBase == "" {
216+
base, err := git.DefaultBranch()
217+
if err != nil {
218+
cfg.Errorf("unable to determine default branch\nUse --base to specify the base branch")
219+
return ErrSilent
220+
}
221+
bottomBase = base
222+
}
223+
212224
// Create PRs for branches that don't have one yet.
213225
needsCreation := 0
214226
for _, r := range found {
@@ -219,13 +231,13 @@ func runLinkCreateOrUpdate(cfg *config.Config, client github.ClientOps, opts *li
219231
if needsCreation > 0 {
220232
cfg.Printf("Creating %d %s...", needsCreation, plural(needsCreation, "PR", "PRs"))
221233
}
222-
resolved, err := createMissingPRs(cfg, client, opts, prArgs, found, templateContent, opts.base)
234+
resolved, err := createMissingPRs(cfg, client, opts, prArgs, found, templateContent, bottomBase)
223235
if err != nil {
224236
return err
225237
}
226238

227239
// Fix base branches for existing PRs with wrong bases.
228-
fixBaseBranches(cfg, client, opts, resolved, opts.base)
240+
fixBaseBranches(cfg, client, opts, resolved, bottomBase)
229241

230242
// Upsert the stack (reuse the stacks fetched above).
231243
prNumbers := make([]int, len(resolved))

‎cmd/link_test.go‎

Lines changed: 243 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ func newLinkGitMock(branches ...string) *git.MockOps {
3333
// --- PR-number tests ---
3434

3535
func TestLink_PRNumbers_CreateNewStack(t *testing.T) {
36+
restore := git.SetOps(newLinkGitMock())
37+
defer restore()
38+
3639
var createdPRs []int
3740
cfg, _, errR := config.NewTestConfig()
3841
cfg.GitHubClientOverride = &github.MockClient{
@@ -69,6 +72,9 @@ func TestLink_PRNumbers_CreateNewStack(t *testing.T) {
6972
}
7073

7174
func TestLink_PRNumbers_UpdateExistingStack(t *testing.T) {
75+
restore := git.SetOps(newLinkGitMock())
76+
defer restore()
77+
7278
var updatedNumber int
7379
var updatedPRs []int
7480
cfg, _, errR := config.NewTestConfig()
@@ -110,6 +116,9 @@ func TestLink_PRNumbers_UpdateExistingStack(t *testing.T) {
110116
}
111117

112118
func TestLink_PRNumbers_ExactMatch_NoOp(t *testing.T) {
119+
restore := git.SetOps(newLinkGitMock())
120+
defer restore()
121+
113122
cfg, _, errR := config.NewTestConfig()
114123
cfg.GitHubClientOverride = &github.MockClient{
115124
FindPRByNumberFn: func(n int) (*github.PullRequest, error) {
@@ -269,6 +278,9 @@ func TestLink_StacksUnavailable(t *testing.T) {
269278
}
270279

271280
func TestLink_Create422(t *testing.T) {
281+
restore := git.SetOps(newLinkGitMock())
282+
defer restore()
283+
272284
cfg, _, errR := config.NewTestConfig()
273285
cfg.GitHubClientOverride = &github.MockClient{
274286
FindPRByNumberFn: func(n int) (*github.PullRequest, error) {
@@ -561,6 +573,9 @@ func TestLink_ReportsMultipleIneligiblePRs(t *testing.T) {
561573
// Regression test to ensure a queued PR that is already a member of
562574
// the target stack does not block adding new PRs to that same stack.
563575
func TestLink_AllowsQueuedPRAlreadyInStack(t *testing.T) {
576+
restore := git.SetOps(newLinkGitMock())
577+
defer restore()
578+
564579
var updatedNumber int
565580
var updatedPRs []int
566581
cfg, _, errR := config.NewTestConfig()
@@ -614,6 +629,9 @@ func TestLink_AllowsQueuedPRAlreadyInStack(t *testing.T) {
614629
// TestLink_AllowsMergedPRAlreadyInStack verifies the exemption also covers
615630
// state-based ineligibility (merged/closed) for PRs already in the stack.
616631
func TestLink_AllowsMergedPRAlreadyInStack(t *testing.T) {
632+
restore := git.SetOps(newLinkGitMock())
633+
defer restore()
634+
617635
var updatedPRs []int
618636
cfg, _, errR := config.NewTestConfig()
619637
cfg.GitHubClientOverride = &github.MockClient{
@@ -660,6 +678,9 @@ func TestLink_AllowsMergedPRAlreadyInStack(t *testing.T) {
660678
// TestLink_AllowsAutoMergePRAlreadyInStack verifies the exemption also covers
661679
// auto-merge-enabled PRs already in the stack.
662680
func TestLink_AllowsAutoMergePRAlreadyInStack(t *testing.T) {
681+
restore := git.SetOps(newLinkGitMock())
682+
defer restore()
683+
663684
var updatedPRs []int
664685
cfg, _, errR := config.NewTestConfig()
665686
cfg.GitHubClientOverride = &github.MockClient{
@@ -1293,6 +1314,216 @@ func TestLink_FixesBaseBranches(t *testing.T) {
12931314
assert.Contains(t, output, "Updated base branch")
12941315
}
12951316

1317+
// TestLink_DefaultBase_RetargetsBottomPRToDefaultBranch is a regression test
1318+
// for #260: when --base is omitted, the bottom of the stack must be based on
1319+
// the repository's default branch (resolved via git.DefaultBranch), not a
1320+
// hardcoded "main". Here the default branch is "develop", so the bottom PR —
1321+
// which currently targets "main" — should be retargeted to "develop".
1322+
func TestLink_DefaultBase_RetargetsBottomPRToDefaultBranch(t *testing.T) {
1323+
defaultBranchCalled := false
1324+
restore := git.SetOps(&git.MockOps{
1325+
BranchExistsFn: func(string) bool { return false },
1326+
DefaultBranchFn: func() (string, error) {
1327+
defaultBranchCalled = true
1328+
return "develop", nil
1329+
},
1330+
})
1331+
defer restore()
1332+
1333+
var baseUpdates []struct {
1334+
number int
1335+
base string
1336+
}
1337+
1338+
cfg, _, errR := config.NewTestConfig()
1339+
cfg.GitHubClientOverride = &github.MockClient{
1340+
FindPRByNumberFn: func(n int) (*github.PullRequest, error) {
1341+
switch n {
1342+
case 10:
1343+
// Bottom PR currently targets "main"; it should be retargeted
1344+
// to the repository default branch ("develop").
1345+
return &github.PullRequest{
1346+
Number: 10, HeadRefName: "feat-a", BaseRefName: "main",
1347+
URL: "https://github.com/o/r/pull/10",
1348+
}, nil
1349+
case 20:
1350+
return &github.PullRequest{
1351+
Number: 20, HeadRefName: "feat-b", BaseRefName: "feat-a",
1352+
URL: "https://github.com/o/r/pull/20",
1353+
}, nil
1354+
}
1355+
return nil, nil
1356+
},
1357+
UpdatePRBaseFn: func(number int, base string) error {
1358+
baseUpdates = append(baseUpdates, struct {
1359+
number int
1360+
base string
1361+
}{number, base})
1362+
return nil
1363+
},
1364+
ListStacksFn: func() ([]github.RemoteStack, error) { return []github.RemoteStack{}, nil },
1365+
CreateStackFn: func([]int) (*github.RemoteStack, error) { return &github.RemoteStack{ID: 1, Number: 1}, nil },
1366+
}
1367+
1368+
cmd := LinkCmd(cfg)
1369+
cmd.SetArgs([]string{"10", "20"})
1370+
cmd.SetOut(io.Discard)
1371+
cmd.SetErr(io.Discard)
1372+
err := cmd.Execute()
1373+
1374+
cfg.Err.Close()
1375+
_, _ = io.ReadAll(errR)
1376+
1377+
assert.NoError(t, err)
1378+
assert.True(t, defaultBranchCalled, "git.DefaultBranch should be consulted when --base is omitted")
1379+
// The bottom PR (#10) should be retargeted to the default branch, not "main".
1380+
require.Len(t, baseUpdates, 1)
1381+
assert.Equal(t, 10, baseUpdates[0].number)
1382+
assert.Equal(t, "develop", baseUpdates[0].base)
1383+
}
1384+
1385+
// TestLink_DefaultBase_CreatesBottomPROnDefaultBranch verifies that a newly
1386+
// created bottom PR is based on the repository default branch when --base is
1387+
// omitted, rather than a hardcoded "main".
1388+
func TestLink_DefaultBase_CreatesBottomPROnDefaultBranch(t *testing.T) {
1389+
restore := git.SetOps(&git.MockOps{
1390+
BranchExistsFn: func(name string) bool { return name == "feat-a" || name == "feat-b" },
1391+
PushFn: func(string, []string, bool, bool) error { return nil },
1392+
ResolveRemoteFn: func(string) (string, error) { return "origin", nil },
1393+
DefaultBranchFn: func() (string, error) { return "develop", nil },
1394+
})
1395+
defer restore()
1396+
1397+
var createdPRs []struct{ base, head string }
1398+
cfg, _, errR := config.NewTestConfig()
1399+
cfg.GitHubClientOverride = &github.MockClient{
1400+
FindPRForBranchFn: func(string) (*github.PullRequest, error) { return nil, nil },
1401+
CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) {
1402+
createdPRs = append(createdPRs, struct{ base, head string }{base, head})
1403+
n := len(createdPRs)
1404+
return &github.PullRequest{
1405+
Number: n, HeadRefName: head, BaseRefName: base,
1406+
URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n),
1407+
}, nil
1408+
},
1409+
ListStacksFn: func() ([]github.RemoteStack, error) { return []github.RemoteStack{}, nil },
1410+
CreateStackFn: func([]int) (*github.RemoteStack, error) { return &github.RemoteStack{ID: 1, Number: 1}, nil },
1411+
}
1412+
1413+
cmd := LinkCmd(cfg)
1414+
cmd.SetArgs([]string{"feat-a", "feat-b"})
1415+
cmd.SetOut(io.Discard)
1416+
cmd.SetErr(io.Discard)
1417+
err := cmd.Execute()
1418+
1419+
cfg.Err.Close()
1420+
_, _ = io.ReadAll(errR)
1421+
1422+
assert.NoError(t, err)
1423+
require.Len(t, createdPRs, 2)
1424+
// Bottom PR based on the repository default branch, not "main".
1425+
assert.Equal(t, "develop", createdPRs[0].base)
1426+
// Second PR chains off the previous branch.
1427+
assert.Equal(t, "feat-a", createdPRs[1].base)
1428+
}
1429+
1430+
// TestLink_DefaultBase_ErrorWhenUnresolvable verifies that link fails with a
1431+
// helpful message when --base is omitted and the default branch cannot be
1432+
// determined.
1433+
func TestLink_DefaultBase_ErrorWhenUnresolvable(t *testing.T) {
1434+
restore := git.SetOps(&git.MockOps{
1435+
BranchExistsFn: func(string) bool { return false },
1436+
DefaultBranchFn: func() (string, error) { return "", fmt.Errorf("no default branch") },
1437+
})
1438+
defer restore()
1439+
1440+
cfg, _, errR := config.NewTestConfig()
1441+
cfg.GitHubClientOverride = &github.MockClient{
1442+
FindPRByNumberFn: func(n int) (*github.PullRequest, error) {
1443+
return &github.PullRequest{
1444+
Number: n, HeadRefName: fmt.Sprintf("b%d", n), BaseRefName: "main",
1445+
}, nil
1446+
},
1447+
ListStacksFn: func() ([]github.RemoteStack, error) { return []github.RemoteStack{}, nil },
1448+
}
1449+
1450+
cmd := LinkCmd(cfg)
1451+
cmd.SetArgs([]string{"10", "20"})
1452+
cmd.SetOut(io.Discard)
1453+
cmd.SetErr(io.Discard)
1454+
err := cmd.Execute()
1455+
1456+
cfg.Err.Close()
1457+
errOut, _ := io.ReadAll(errR)
1458+
output := string(errOut)
1459+
1460+
assert.ErrorIs(t, err, ErrSilent)
1461+
assert.Contains(t, output, "unable to determine default branch")
1462+
}
1463+
1464+
// TestLink_ExplicitBase_SkipsDefaultBranchResolution verifies that passing
1465+
// --base bypasses default branch resolution and is used as the bottom base.
1466+
func TestLink_ExplicitBase_SkipsDefaultBranchResolution(t *testing.T) {
1467+
defaultBranchCalled := false
1468+
restore := git.SetOps(&git.MockOps{
1469+
BranchExistsFn: func(string) bool { return false },
1470+
DefaultBranchFn: func() (string, error) {
1471+
defaultBranchCalled = true
1472+
return "develop", nil
1473+
},
1474+
})
1475+
defer restore()
1476+
1477+
var baseUpdates []struct {
1478+
number int
1479+
base string
1480+
}
1481+
1482+
cfg, _, errR := config.NewTestConfig()
1483+
cfg.GitHubClientOverride = &github.MockClient{
1484+
FindPRByNumberFn: func(n int) (*github.PullRequest, error) {
1485+
switch n {
1486+
case 10:
1487+
return &github.PullRequest{
1488+
Number: 10, HeadRefName: "feat-a", BaseRefName: "main",
1489+
URL: "https://github.com/o/r/pull/10",
1490+
}, nil
1491+
case 20:
1492+
return &github.PullRequest{
1493+
Number: 20, HeadRefName: "feat-b", BaseRefName: "feat-a",
1494+
URL: "https://github.com/o/r/pull/20",
1495+
}, nil
1496+
}
1497+
return nil, nil
1498+
},
1499+
UpdatePRBaseFn: func(number int, base string) error {
1500+
baseUpdates = append(baseUpdates, struct {
1501+
number int
1502+
base string
1503+
}{number, base})
1504+
return nil
1505+
},
1506+
ListStacksFn: func() ([]github.RemoteStack, error) { return []github.RemoteStack{}, nil },
1507+
CreateStackFn: func([]int) (*github.RemoteStack, error) { return &github.RemoteStack{ID: 1, Number: 1}, nil },
1508+
}
1509+
1510+
cmd := LinkCmd(cfg)
1511+
cmd.SetArgs([]string{"--base", "release", "10", "20"})
1512+
cmd.SetOut(io.Discard)
1513+
cmd.SetErr(io.Discard)
1514+
err := cmd.Execute()
1515+
1516+
cfg.Err.Close()
1517+
_, _ = io.ReadAll(errR)
1518+
1519+
assert.NoError(t, err)
1520+
assert.False(t, defaultBranchCalled, "git.DefaultBranch must not be consulted when --base is given")
1521+
// The bottom PR (#10) should be retargeted to the explicit base, not "main".
1522+
require.Len(t, baseUpdates, 1)
1523+
assert.Equal(t, 10, baseUpdates[0].number)
1524+
assert.Equal(t, "release", baseUpdates[0].base)
1525+
}
1526+
12961527
func TestLink_DuplicateBranchResolvesToSamePR(t *testing.T) {
12971528
cfg, _, errR := config.NewTestConfig()
12981529
cfg.GitHubClientOverride = &github.MockClient{
@@ -1319,6 +1550,9 @@ func TestLink_DuplicateBranchResolvesToSamePR(t *testing.T) {
13191550
}
13201551

13211552
func TestLink_UpdateDeletedStack_FallsBackToCreate(t *testing.T) {
1553+
restore := git.SetOps(newLinkGitMock())
1554+
defer restore()
1555+
13221556
var created bool
13231557
cfg, _, errR := config.NewTestConfig()
13241558
cfg.GitHubClientOverride = &github.MockClient{
@@ -1859,6 +2093,9 @@ func TestLink_PRNumbers_NoTemplateUsesFooter(t *testing.T) {
18592093
// --- PR URL tests ---
18602094

18612095
func TestLink_PRURLs_CreateNewStack(t *testing.T) {
2096+
restore := git.SetOps(newLinkGitMock())
2097+
defer restore()
2098+
18622099
var createdPRs []int
18632100
cfg, _, errR := config.NewTestConfig()
18642101
cfg.GitHubClientOverride = &github.MockClient{
@@ -1924,6 +2161,9 @@ func TestLink_PRURLs_NotFound(t *testing.T) {
19242161
}
19252162

19262163
func TestLink_MixedURLsAndNumbers(t *testing.T) {
2164+
restore := git.SetOps(newLinkGitMock())
2165+
defer restore()
2166+
19272167
var createdPRs []int
19282168
cfg, _, errR := config.NewTestConfig()
19292169
cfg.GitHubClientOverride = &github.MockClient{
@@ -2270,6 +2510,9 @@ func TestLink_AddMode_ExemptsIneligibleExistingMember(t *testing.T) {
22702510
}
22712511

22722512
func TestLink_NumericFirstArgNotAStack_UsesCreateMode(t *testing.T) {
2513+
restore := git.SetOps(newLinkGitMock())
2514+
defer restore()
2515+
22732516
var createdPRs []int
22742517
cfg, _, errR := config.NewTestConfig()
22752518
cfg.GitHubClientOverride = &github.MockClient{

‎docs/src/content/docs/reference/cli.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ To grow an existing stack without re-listing its PRs, pass a stack number (the n
431431

432432
| Flag | Description |
433433
|------|-------------|
434-
| `--base <branch>` | Base branch for the bottom of the stack (default: `main`); ignored when adding to an existing stack |
434+
| `--base <branch>` | Base branch for the bottom of the stack (defaults to the repository's default branch); ignored when adding to an existing stack |
435435
| `--open` | Mark new and existing PRs as ready for review |
436436
| `--remote <name>` | Remote to push to (defaults to auto-detected remote) |
437437

‎skills/gh-stack/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ When the first argument is a stack number, the remaining arguments are appended
591591

592592
| Flag | Description |
593593
|------|---------|
594-
| `--base <branch>` | Base branch for the bottom of the stack (default: `main`) |
594+
| `--base <branch>` | Base branch for the bottom of the stack (defaults to the repository's default branch) |
595595
| `--open` | Mark new and existing PRs as ready for review |
596596
| `--remote <name>` | Remote to push to (use if multiple remotes exist) |
597597

0 commit comments

Comments
 (0)