Skip to content
Open
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
3 changes: 3 additions & 0 deletions .changes/unreleased/Added-20260512-101000.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kind: Added
body: 'ci: Add ''gs ci merge-guard'' to block out-of-order stacked PR merges'
time: 2026-05-12T10:10:00.762024-04:00
2 changes: 1 addition & 1 deletion branch_restack.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@ func (cmd *branchRestackCmd) AfterApply(ctx context.Context, wt *git.Worktree) e
}

func (cmd *branchRestackCmd) Run(ctx context.Context, handler RestackHandler) error {
return handler.RestackBranch(ctx, cmd.Branch)
return handler.RestackBranch(ctx, cmd.Branch, nil)
}
6 changes: 6 additions & 0 deletions ci.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package main

// ciCmd groups subcommands for CI/CD integration.
type ciCmd struct {
MergeGuard ciMergeGuardCmd `cmd:"merge-guard" help:"Block merging a PR whose base is not trunk"`
}
165 changes: 165 additions & 0 deletions ci_merge_guard.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
package main

import (
"context"
"fmt"

"go.abhg.dev/gs/internal/forge"
"go.abhg.dev/gs/internal/handler/submit"
"go.abhg.dev/gs/internal/silog"
"go.abhg.dev/gs/internal/text"
)

type ciMergeGuardCmd struct {
Number int `arg:"" help:"Change request number to check"`
Trunk string `help:"Override trunk branch name"`
All bool `help:"Block all non-trunk-based PRs, not just git-spice managed ones"`
}

func (*ciMergeGuardCmd) Help() string {
return text.Dedent(`
Checks whether a change request is safe to merge
by verifying its base branch is trunk.

Use this in forge CI/CD pipelines to prevent
out-of-order merges in a stacked PR workflow.

By default, only git-spice managed PRs are checked.
Unmanaged PRs are allowed through.
Use --all to block any PR whose base is not trunk.

The trunk branch is detected from the git-spice
navigation comment on the PR.
Use --trunk to override this detection.

Exit codes:
0 PR is safe to merge (base is trunk, or unmanaged)
1 PR should not be merged yet
`)
}

func (cmd *ciMergeGuardCmd) Run(
ctx context.Context,
log *silog.Logger,
repo forge.Repository,
) error {
changeID, err := cmd.resolveChangeID(repo)
if err != nil {
return err
}

change, err := repo.FindChangeByID(ctx, changeID)
if err != nil {
return fmt.Errorf("find change #%d: %w", cmd.Number, err)
}

trunk, managed, err := cmd.detectTrunk(ctx, log, repo, changeID)
if err != nil {
return err
}

return cmd.evaluate(log, change, trunk, managed)
}

func (cmd *ciMergeGuardCmd) resolveChangeID(
repo forge.Repository,
) (forge.ChangeID, error) {
raw := fmt.Appendf(nil, "%d", cmd.Number)
id, err := repo.Forge().UnmarshalChangeID(raw)
if err != nil {
return nil, fmt.Errorf(
"construct change ID for #%d: %w", cmd.Number, err,
)
}
return id, nil
}

// detectTrunk determines the trunk branch name and whether
// the PR is managed by git-spice.
// Returns (trunk, managed, error).
func (cmd *ciMergeGuardCmd) detectTrunk(
ctx context.Context,
log *silog.Logger,
repo forge.Repository,
changeID forge.ChangeID,
) (trunk string, managed bool, _ error) {
if cmd.Trunk != "" {
return cmd.Trunk, true, nil
}

trunk, managed = cmd.trunkFromNavComment(ctx, log, repo, changeID)
if trunk != "" {
return trunk, managed, nil
}

if !managed {
return "", false, nil
}

return "", true, fmt.Errorf(
"could not determine trunk for #%d: "+
"use --trunk to specify it explicitly",
cmd.Number,
)
}

// trunkFromNavComment searches for a git-spice navigation comment
// on the given change and extracts the trunk branch name.
// Returns ("", false) if no navigation comment is found.
func (cmd *ciMergeGuardCmd) trunkFromNavComment(
ctx context.Context,
log *silog.Logger,
repo forge.Repository,
changeID forge.ChangeID,
) (trunk string, managed bool) {
opts := &forge.ListChangeCommentsOptions{
BodyMatchesAll: submit.NavCommentRegexes,
}

for comment, err := range repo.ListChangeComments(
ctx, changeID, opts,
) {
if err != nil {
log.Warn("Error listing comments", "error", err)
return "", false
}

trunk := submit.ExtractTrunkFromComment(comment.Body)
return trunk, true
}

return "", false
}

// evaluate decides whether the PR is safe to merge
// based on the change's base branch and the detected trunk.
func (cmd *ciMergeGuardCmd) evaluate(
log *silog.Logger,
change *forge.FindChangeItem,
trunk string,
managed bool,
) error {
// Unmanaged PR: allow unless --all is set.
if !managed {
if cmd.All {
return fmt.Errorf(
"#%d: base %q is not trunk (unmanaged PR blocked by --all)",
cmd.Number, change.BaseName,
)
}
log.Infof("#%d: not managed by git-spice, allowing", cmd.Number)
return nil
}

if change.BaseName == trunk {
log.Infof("#%d: base is %q (trunk), safe to merge",
cmd.Number, trunk)
return nil
}

return fmt.Errorf(
"#%d: base is %q, expected trunk %q. "+
"Merge the downstack PR first or retarget to trunk",
cmd.Number, change.BaseName, trunk,
)
}
37 changes: 37 additions & 0 deletions doc/includes/cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -1415,6 +1415,43 @@ going back to the state before the rebase.
The command can be used in place of 'git rebase --abort'
even if a git-spice operation is not currently in progress.

## CI

### git-spice ci merge-guard {#gs-ci-merge-guard}

```
gs ci merge-guard <number> [flags]
```

Block merging a PR whose base is not trunk

Checks whether a change request is safe to merge
by verifying its base branch is trunk.

Use this in forge CI/CD pipelines to prevent
out-of-order merges in a stacked PR workflow.

By default, only git-spice managed PRs are checked.
Unmanaged PRs are allowed through.
Use --all to block any PR whose base is not trunk.

The trunk branch is detected from the git-spice
navigation comment on the PR.
Use --trunk to override this detection.

Exit codes:
0 PR is safe to merge (base is trunk, or unmanaged)
1 PR should not be merged yet

**Arguments**

* `number`: Change request number to check

**Flags**

* `--trunk=STRING`: Override trunk branch name
* `--all`: Block all non-trunk-based PRs, not just git-spice managed ones

## Navigation

### git-spice up {#gs-up}
Expand Down
2 changes: 1 addition & 1 deletion downstack_restack.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ func (cmd *downstackRestackCmd) Run(
return errors.New("nothing to restack below trunk")
}

return handler.RestackDownstack(ctx, cmd.Branch)
return handler.RestackDownstack(ctx, cmd.Branch, nil)
}
5 changes: 3 additions & 2 deletions internal/handler/merge/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strings"
"time"

"go.abhg.dev/gs/internal/handler/restack"
"go.abhg.dev/gs/internal/handler/submit"
"go.abhg.dev/gs/internal/handler/sync"

Expand All @@ -36,7 +37,7 @@ type Service interface {

// RestackHandler restacks branches after their bases are merged.
type RestackHandler interface {
RestackBranch(context.Context, string) error
RestackBranch(ctx context.Context, branch string, opts *restack.Options) error
}

// SubmitHandler updates change requests after branch restacks.
Expand Down Expand Up @@ -599,7 +600,7 @@ func (e *mergePlanExecutor) prepareForMerge(
return fmt.Errorf("verify restacked: %w", err)
}

if err := e.Restack.RestackBranch(ctx, item.branch); err != nil {
if err := e.Restack.RestackBranch(ctx, item.branch, nil); err != nil {
return fmt.Errorf("restack branch: %w", err)
}

Expand Down
6 changes: 3 additions & 3 deletions internal/handler/merge/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,10 +317,10 @@ func TestExecutePlan_retargets(t *testing.T) {

mockRestack := NewMockRestackHandler(ctrl)
mockRestack.EXPECT().
RestackBranch(gomock.Any(), "feat2").
RestackBranch(gomock.Any(), "feat2", gomock.Nil()).
Return(nil)
mockRestack.EXPECT().
RestackBranch(gomock.Any(), "feat3").
RestackBranch(gomock.Any(), "feat3", gomock.Nil()).
Return(nil)

mockSubmit := NewMockSubmitHandler(ctrl)
Expand Down Expand Up @@ -396,7 +396,7 @@ func TestExecutePlan_waitsForPreparedChangeHeadBeforeChecks(t *testing.T) {

mockRestack := NewMockRestackHandler(ctrl)
mockRestack.EXPECT().
RestackBranch(gomock.Any(), "feat2").
RestackBranch(gomock.Any(), "feat2", gomock.Nil()).
Return(nil)

mockSubmit := NewMockSubmitHandler(ctrl)
Expand Down
13 changes: 7 additions & 6 deletions internal/handler/merge/mocks_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion internal/handler/restack/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ package restack
import "context"

// RestackBranch restacks the given branch onto its base.
func (h *Handler) RestackBranch(ctx context.Context, branch string) error {
func (h *Handler) RestackBranch(
ctx context.Context, branch string, opts *Options,
) error {
_, err := h.Restack(ctx, &Request{
Branch: branch,
ContinueCommand: []string{"branch", "restack"},
AutoResolve: opts.autoResolvePtr(),
})
return err
}
10 changes: 5 additions & 5 deletions internal/handler/restack/branch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestHandler_RestackBranch(t *testing.T) {
Store: statetest.NewMemoryStore(t, "main", "", log),
Service: mockService,
}
require.NoError(t, handler.RestackBranch(t.Context(), "feature"))
require.NoError(t, handler.RestackBranch(t.Context(), "feature", nil))
assert.Contains(t, logBuffer.String(), "feature: restacked on main")
})

Expand Down Expand Up @@ -82,7 +82,7 @@ func TestHandler_RestackBranch(t *testing.T) {
Service: mockService,
}

require.NoError(t, handler.RestackBranch(t.Context(), "feature"))
require.NoError(t, handler.RestackBranch(t.Context(), "feature", nil))
})

t.Run("UntrackedBranch", func(t *testing.T) {
Expand Down Expand Up @@ -113,7 +113,7 @@ func TestHandler_RestackBranch(t *testing.T) {
Service: mockService,
}

err := handler.RestackBranch(t.Context(), "untracked")
err := handler.RestackBranch(t.Context(), "untracked", nil)
require.Error(t, err)
assert.ErrorContains(t, err, "untracked branch")
assert.Contains(t, logBuffer.String(), "untracked: branch not tracked: run '"+cli.Name()+" branch track")
Expand Down Expand Up @@ -146,7 +146,7 @@ func TestHandler_RestackBranch(t *testing.T) {
Store: statetest.NewMemoryStore(t, "main", "", log),
Service: mockService,
}
require.NoError(t, handler.RestackBranch(t.Context(), "already-restacked"))
require.NoError(t, handler.RestackBranch(t.Context(), "already-restacked", nil))
assert.Contains(t, logBuffer.String(), "already-restacked: branch does not need to be restacked.")
})

Expand Down Expand Up @@ -177,7 +177,7 @@ func TestHandler_RestackBranch(t *testing.T) {
Store: statetest.NewMemoryStore(t, "main", "", log),
Service: mockService,
}
err := handler.RestackBranch(t.Context(), "feature")
err := handler.RestackBranch(t.Context(), "feature", nil)
require.Error(t, err)
assert.ErrorContains(t, err, "restack branch")
assert.ErrorIs(t, err, unexpectedErr)
Expand Down
Loading
Loading