diff --git a/.changes/unreleased/Added-20260512-101000.yaml b/.changes/unreleased/Added-20260512-101000.yaml new file mode 100644 index 00000000..07bd53d2 --- /dev/null +++ b/.changes/unreleased/Added-20260512-101000.yaml @@ -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 diff --git a/branch_restack.go b/branch_restack.go index 32b8dc59..f84b803b 100644 --- a/branch_restack.go +++ b/branch_restack.go @@ -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) } diff --git a/ci.go b/ci.go new file mode 100644 index 00000000..4e2a0246 --- /dev/null +++ b/ci.go @@ -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"` +} diff --git a/ci_merge_guard.go b/ci_merge_guard.go new file mode 100644 index 00000000..d8c7b92d --- /dev/null +++ b/ci_merge_guard.go @@ -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, + ) +} diff --git a/doc/includes/cli-reference.md b/doc/includes/cli-reference.md index 507c9d1c..4d598acd 100644 --- a/doc/includes/cli-reference.md +++ b/doc/includes/cli-reference.md @@ -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 [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} diff --git a/downstack_restack.go b/downstack_restack.go index e3f95a42..87a273f6 100644 --- a/downstack_restack.go +++ b/downstack_restack.go @@ -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) } diff --git a/internal/handler/merge/handler.go b/internal/handler/merge/handler.go index 8172b233..0f921e88 100644 --- a/internal/handler/merge/handler.go +++ b/internal/handler/merge/handler.go @@ -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" @@ -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. @@ -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) } diff --git a/internal/handler/merge/handler_test.go b/internal/handler/merge/handler_test.go index 317d2a05..f1d1f165 100644 --- a/internal/handler/merge/handler_test.go +++ b/internal/handler/merge/handler_test.go @@ -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) @@ -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) diff --git a/internal/handler/merge/mocks_test.go b/internal/handler/merge/mocks_test.go index 9d4523a5..c59410e4 100644 --- a/internal/handler/merge/mocks_test.go +++ b/internal/handler/merge/mocks_test.go @@ -13,6 +13,7 @@ import ( reflect "reflect" git "go.abhg.dev/gs/internal/git" + restack "go.abhg.dev/gs/internal/handler/restack" submit "go.abhg.dev/gs/internal/handler/submit" sync "go.abhg.dev/gs/internal/handler/sync" spice "go.abhg.dev/gs/internal/spice" @@ -207,17 +208,17 @@ func (m *MockRestackHandler) EXPECT() *MockRestackHandlerMockRecorder { } // RestackBranch mocks base method. -func (m *MockRestackHandler) RestackBranch(arg0 context.Context, arg1 string) error { +func (m *MockRestackHandler) RestackBranch(ctx context.Context, branch string, opts *restack.Options) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "RestackBranch", arg0, arg1) + ret := m.ctrl.Call(m, "RestackBranch", ctx, branch, opts) ret0, _ := ret[0].(error) return ret0 } // RestackBranch indicates an expected call of RestackBranch. -func (mr *MockRestackHandlerMockRecorder) RestackBranch(arg0, arg1 any) *MockRestackHandlerRestackBranchCall { +func (mr *MockRestackHandlerMockRecorder) RestackBranch(ctx, branch, opts any) *MockRestackHandlerRestackBranchCall { mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RestackBranch", reflect.TypeOf((*MockRestackHandler)(nil).RestackBranch), arg0, arg1) + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RestackBranch", reflect.TypeOf((*MockRestackHandler)(nil).RestackBranch), ctx, branch, opts) return &MockRestackHandlerRestackBranchCall{Call: call} } @@ -233,13 +234,13 @@ func (c *MockRestackHandlerRestackBranchCall) Return(arg0 error) *MockRestackHan } // Do rewrite *gomock.Call.Do -func (c *MockRestackHandlerRestackBranchCall) Do(f func(context.Context, string) error) *MockRestackHandlerRestackBranchCall { +func (c *MockRestackHandlerRestackBranchCall) Do(f func(context.Context, string, *restack.Options) error) *MockRestackHandlerRestackBranchCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockRestackHandlerRestackBranchCall) DoAndReturn(f func(context.Context, string) error) *MockRestackHandlerRestackBranchCall { +func (c *MockRestackHandlerRestackBranchCall) DoAndReturn(f func(context.Context, string, *restack.Options) error) *MockRestackHandlerRestackBranchCall { c.Call = c.Call.DoAndReturn(f) return c } diff --git a/internal/handler/restack/branch.go b/internal/handler/restack/branch.go index 8d880183..889d9a27 100644 --- a/internal/handler/restack/branch.go +++ b/internal/handler/restack/branch.go @@ -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 } diff --git a/internal/handler/restack/branch_test.go b/internal/handler/restack/branch_test.go index 95664e68..c7435831 100644 --- a/internal/handler/restack/branch_test.go +++ b/internal/handler/restack/branch_test.go @@ -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") }) @@ -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) { @@ -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") @@ -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.") }) @@ -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) diff --git a/internal/handler/restack/downstack.go b/internal/handler/restack/downstack.go index e4df6f7a..60c0ab42 100644 --- a/internal/handler/restack/downstack.go +++ b/internal/handler/restack/downstack.go @@ -4,11 +4,14 @@ import "context" // RestackDownstack restacks the downstack of the given branch. // This includes the branch itself. -func (h *Handler) RestackDownstack(ctx context.Context, branch string) error { +func (h *Handler) RestackDownstack( + ctx context.Context, branch string, opts *Options, +) error { _, err := h.Restack(ctx, &Request{ Branch: branch, Scope: ScopeDownstack, ContinueCommand: []string{"downstack", "restack"}, + AutoResolve: opts.autoResolvePtr(), }) return err } diff --git a/internal/handler/restack/downstack_test.go b/internal/handler/restack/downstack_test.go index fa633e06..a99ac339 100644 --- a/internal/handler/restack/downstack_test.go +++ b/internal/handler/restack/downstack_test.go @@ -54,7 +54,7 @@ func TestHandler_RestackDownstack(t *testing.T) { Service: mockService, } - require.NoError(t, handler.RestackDownstack(t.Context(), "feature3")) + require.NoError(t, handler.RestackDownstack(t.Context(), "feature3", nil)) assert.Contains(t, logBuffer.String(), "feature1: restacked on main") assert.Contains(t, logBuffer.String(), "feature2: restacked on feature1") assert.Contains(t, logBuffer.String(), "feature3: restacked on feature2") @@ -99,6 +99,6 @@ func TestHandler_RestackDownstack(t *testing.T) { Service: mockService, } - require.NoError(t, handler.RestackDownstack(t.Context(), "feature2")) + require.NoError(t, handler.RestackDownstack(t.Context(), "feature2", nil)) }) } diff --git a/internal/handler/restack/handler.go b/internal/handler/restack/handler.go index 7ff1f6d4..93d0dadc 100644 --- a/internal/handler/restack/handler.go +++ b/internal/handler/restack/handler.go @@ -93,6 +93,33 @@ type Request struct { // // Defaults to ScopeBranch. Scope Scope + + // AutoResolve, if non-nil, overrides + // [Handler.DefaultAutoResolve] for this invocation. A true + // value enables the configured resolver; a false value disables + // it even when configured. + AutoResolve *bool +} + +// Options are optional parameters shared by the high-level restack +// entry points ([Handler.RestackBranch], [Handler.RestackStack], +// [Handler.RestackDownstack]). +type Options struct { + // AutoResolve, if non-nil, overrides + // [Handler.DefaultAutoResolve] for this invocation. A true + // value enables the configured resolver; a false value disables + // it even when configured. + AutoResolve *bool +} + +// autoResolvePtr returns the AutoResolve pointer if opts is +// non-nil, or nil. Callers pass the result through to +// [Request.AutoResolve]. +func (o *Options) autoResolvePtr() *bool { + if o == nil { + return nil + } + return o.AutoResolve } // Restack restacks one or more branches according to the request. diff --git a/internal/handler/restack/stack.go b/internal/handler/restack/stack.go index f37ad358..592e99ea 100644 --- a/internal/handler/restack/stack.go +++ b/internal/handler/restack/stack.go @@ -5,11 +5,14 @@ import "context" // RestackStack restacks the stack of the given branch. // This includes all upstack and downtrack branches, // as well as the branch itself. -func (h *Handler) RestackStack(ctx context.Context, branch string) error { +func (h *Handler) RestackStack( + ctx context.Context, branch string, opts *Options, +) error { _, err := h.Restack(ctx, &Request{ Branch: branch, Scope: ScopeStack, ContinueCommand: []string{"stack", "restack"}, + AutoResolve: opts.autoResolvePtr(), }) return err } diff --git a/internal/handler/submit/handler.go b/internal/handler/submit/handler.go index f1587929..ddcc08f2 100644 --- a/internal/handler/submit/handler.go +++ b/internal/handler/submit/handler.go @@ -533,6 +533,7 @@ func (h *Handler) SubmitBatch(ctx context.Context, req *BatchRequest) error { opts.NavCommentSync, opts.NavCommentDownstack, opts.NavCommentMarker, + h.Store.Trunk(), branchesToComment, h.upstreamRepository, ) @@ -597,6 +598,7 @@ func (h *Handler) Submit(ctx context.Context, req *Request) error { opts.NavCommentSync, opts.NavCommentDownstack, opts.NavCommentMarker, + h.Store.Trunk(), []string{req.Branch}, h.upstreamRepository, ) @@ -739,7 +741,7 @@ func (h *Handler) submitBranch( // If we're importing an existing CR, // also check if there's a stack navigation comment to import. listCommentOpts := forge.ListChangeCommentsOptions{ - BodyMatchesAll: _navCommentRegexes, + BodyMatchesAll: NavCommentRegexes, CanUpdate: true, } diff --git a/internal/handler/submit/nav_comment.go b/internal/handler/submit/nav_comment.go index 9501f9c4..9dbf906d 100644 --- a/internal/handler/submit/nav_comment.go +++ b/internal/handler/submit/nav_comment.go @@ -91,6 +91,7 @@ func updateNavigationComments( navCommentSync NavCommentSync, navCommentDownstack NavCommentDownstack, navCommentMarker string, + trunk string, submittedBranches []string, getRemoteRepo func(context.Context) (forge.Repository, error), ) error { @@ -395,7 +396,7 @@ func updateNavigationComments( } info := infos[idx] - commentBody := generateStackNavigationComment(nodes, idx, navCommentMarker, remoteRepo.Forge()) + commentBody := generateStackNavigationComment(nodes, idx, navCommentMarker, trunk, remoteRepo.Forge()) if info.Meta.NavigationCommentID() == nil { postc <- &postComment{ Branch: info.Branch, @@ -462,23 +463,54 @@ const ( // Uses Markdown link definition syntax which is invisible when rendered. const _markdownCommentMarker = "[gs]: # (navigation comment)" -// Regular expressions that must ALL match a comment -// for it to be considered a navigation comment -// when detecting existing comments. -var _navCommentRegexes = []*regexp.Regexp{ +// Trunk metadata markers embed the trunk branch name +// in the navigation comment for CI merge-guard detection. +const ( + _trunkMarkerHTML = "" + _trunkMarkerMarkdown = "[gs:trunk]: # (%s)" +) + +// NavCommentRegexes are regular expressions +// that must ALL match a comment body +// for it to be considered a git-spice navigation comment. +var NavCommentRegexes = []*regexp.Regexp{ regexp.MustCompile(`(?m)^\Q` + _commentHeader + `\E$`), // Match either standard HTML comment or Markdown link definition marker. regexp.MustCompile(`(?m)^(\Q` + _commentMarker + `\E|\Q` + _markdownCommentMarker + `\E)$`), } +// _trunkMetadataRegex extracts the trunk branch name +// from either HTML or Markdown trunk metadata markers. +var _trunkMetadataRegex = regexp.MustCompile( + `(?m)^(?:|\[gs:trunk\]: # \((\S+)\))$`, +) + +// ExtractTrunkFromComment extracts the trunk branch name +// from a git-spice navigation comment body. +// Returns "" if not found. +func ExtractTrunkFromComment(body string) string { + m := _trunkMetadataRegex.FindStringSubmatch(body) + if m == nil { + return "" + } + + // Group 1 = HTML format, group 2 = Markdown format. + if m[1] != "" { + return m[1] + } + return m[2] +} + func generateStackNavigationComment( nodes []*stackedChange, current int, marker string, + trunk string, f forge.Forge, ) string { footer := _commentFooter commentMarker := _commentMarker + trunkMarker := _trunkMarkerHTML if fc, ok := f.(forge.WithCommentFormat); ok { format := fc.CommentFormat() if format.Footer != "" { @@ -486,6 +518,7 @@ func generateStackNavigationComment( } if format.Marker != "" { commentMarker = format.Marker + trunkMarker = _trunkMarkerMarkdown } } @@ -501,9 +534,12 @@ func generateStackNavigationComment( sb.WriteString("\n") sb.WriteString(footer) - sb.WriteString("\n") sb.WriteString(commentMarker) + if trunk != "" { + sb.WriteString("\n") + fmt.Fprintf(&sb, trunkMarker, trunk) + } sb.WriteString("\n") return sb.String() } diff --git a/internal/handler/submit/nav_comment_test.go b/internal/handler/submit/nav_comment_test.go index 9eb347d2..eba2fed9 100644 --- a/internal/handler/submit/nav_comment_test.go +++ b/internal/handler/submit/nav_comment_test.go @@ -4,6 +4,7 @@ import ( "cmp" "context" "encoding/json" + "fmt" "strings" "sync" "sync/atomic" @@ -450,6 +451,7 @@ func TestUpdateNavigationComments(t *testing.T) { tt.sync, tt.downstack, "", + "main", tt.submit, func(context.Context) (forge.Repository, error) { return mockRemoteRepo, nil @@ -462,9 +464,12 @@ func TestUpdateNavigationComments(t *testing.T) { if assert.Len(t, commentIDs, 1, "change %v doesn't have exactly one comment", changeID) { body, ok := comments[commentIDs[0]] if assert.True(t, ok, "comment %v on change %v has no body", commentIDs[0], changeID) { - // Strip header, footer, and marker to get just the navigation content + // Strip header, footer, marker, and trunk metadata + // to get just the navigation content. stripped := strings.TrimPrefix(body, _commentHeader+"\n\n") - stripped = strings.TrimSuffix(stripped, "\n"+_commentFooter+"\n"+_commentMarker+"\n") + stripped = strings.TrimSuffix(stripped, + "\n"+_commentFooter+"\n"+_commentMarker+ + "\n"+fmt.Sprintf(_trunkMarkerHTML, "main")+"\n") gotComments[int(changeID)] = stripped } } @@ -562,6 +567,7 @@ func TestUpdateNavigationComments_deletedExternally(t *testing.T) { NavCommentSyncBranch, NavCommentDownstackAll, "", + "main", []string{"feat1"}, func(context.Context) (forge.Repository, error) { return mockRemoteRepo, nil @@ -677,6 +683,7 @@ func TestUpdateNavigationComments_deletedExternally(t *testing.T) { NavCommentSyncDownstack, NavCommentDownstackAll, "", + "main", []string{"feat3"}, func(context.Context) (forge.Repository, error) { return mockRemoteRepo, nil @@ -784,13 +791,13 @@ func TestGenerateStackNavigationComment(t *testing.T) { tt.want + "\n" + _commentFooter + "\n" + _commentMarker + "\n" - got := generateStackNavigationComment(tt.graph, tt.current, "", nil) + got := generateStackNavigationComment(tt.graph, tt.current, "", "", nil) assert.Equal(t, want, got) // Sanity check: All generated comments must match // these regular expressions. t.Run("Regexp", func(t *testing.T) { - for _, re := range _navCommentRegexes { + for _, re := range NavCommentRegexes { assert.True(t, re.MatchString(got), "regexp %q failed", re) } }) @@ -804,7 +811,7 @@ func TestGenerateStackNavigationComment(t *testing.T) { } graph[0].Aboves = []int{1} - got := generateStackNavigationComment(graph, 1, "<-- you are here", nil) + got := generateStackNavigationComment(graph, 1, "<-- you are here", "", nil) want := _commentHeader + "\n\n" + joinLines( "- #123", @@ -827,7 +834,7 @@ func TestGenerateStackNavigationComment(t *testing.T) { } graph[0].Aboves = []int{1} - got := generateStackNavigationComment(graph, 1, "", nil) + got := generateStackNavigationComment(graph, 1, "", "", nil) want := _commentHeader + "\n\n" + joinLines( "- #123+", @@ -962,6 +969,33 @@ func TestNavCommentDownstack_UnmarshalText(t *testing.T) { }) } +func TestExtractTrunkFromComment(t *testing.T) { + t.Run("HTML", func(t *testing.T) { + body := "some text\n\n" + assert.Equal(t, "main", ExtractTrunkFromComment(body)) + }) + + t.Run("Markdown", func(t *testing.T) { + body := "some text\n[gs:trunk]: # (develop)\n" + assert.Equal(t, "develop", ExtractTrunkFromComment(body)) + }) + + t.Run("Missing", func(t *testing.T) { + body := "no trunk metadata here\n" + assert.Equal(t, "", ExtractTrunkFromComment(body)) + }) + + t.Run("FullComment", func(t *testing.T) { + body := generateStackNavigationComment( + []*stackedChange{ + {Change: _changeID("1"), Base: -1}, + }, + 0, "", "main", nil, + ) + assert.Equal(t, "main", ExtractTrunkFromComment(body)) + }) +} + type _changeID string func (s _changeID) String() string { diff --git a/internal/handler/sync/handler.go b/internal/handler/sync/handler.go index 4aa7da79..af116412 100644 --- a/internal/handler/sync/handler.go +++ b/internal/handler/sync/handler.go @@ -78,7 +78,7 @@ type DeleteHandler interface { // RestackHandler allows restacking branches after sync // has removed their downstack branches. type RestackHandler interface { - RestackBranch(ctx context.Context, branch string) error + RestackBranch(ctx context.Context, branch string, opts *restack.Options) error RestackUpstack(ctx context.Context, branch string, opts *restack.UpstackOptions) error } @@ -983,7 +983,7 @@ func (h *Handler) restackDeletedBranchUpstack( return fmt.Errorf("restack upstack %q: %w", target, err) } case mode.Includes(spice.RestackAboves): - if err := h.Restack.RestackBranch(ctx, target); err != nil { + if err := h.Restack.RestackBranch(ctx, target, nil); err != nil { return fmt.Errorf("restack branch %q: %w", target, err) } case mode.Includes(spice.RestackNone): diff --git a/internal/handler/sync/handler_test.go b/internal/handler/sync/handler_test.go index b6450f28..415897d2 100644 --- a/internal/handler/sync/handler_test.go +++ b/internal/handler/sync/handler_test.go @@ -368,7 +368,7 @@ func TestHandler_SyncTrunk_restackDeletedUpstacks(t *testing.T) { mockRestack := NewMockRestackHandler(ctrl) mockRestack.EXPECT(). - RestackBranch(gomock.Any(), "child"). + RestackBranch(gomock.Any(), "child", gomock.Nil()). Return(nil) mockAutostash := NewMockAutostashHandler(ctrl) diff --git a/internal/handler/sync/mocks_test.go b/internal/handler/sync/mocks_test.go index 51c7adcf..de0af290 100644 --- a/internal/handler/sync/mocks_test.go +++ b/internal/handler/sync/mocks_test.go @@ -782,17 +782,17 @@ func (m *MockRestackHandler) EXPECT() *MockRestackHandlerMockRecorder { } // RestackBranch mocks base method. -func (m *MockRestackHandler) RestackBranch(ctx context.Context, branch string) error { +func (m *MockRestackHandler) RestackBranch(ctx context.Context, branch string, opts *restack.Options) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "RestackBranch", ctx, branch) + ret := m.ctrl.Call(m, "RestackBranch", ctx, branch, opts) ret0, _ := ret[0].(error) return ret0 } // RestackBranch indicates an expected call of RestackBranch. -func (mr *MockRestackHandlerMockRecorder) RestackBranch(ctx, branch any) *MockRestackHandlerRestackBranchCall { +func (mr *MockRestackHandlerMockRecorder) RestackBranch(ctx, branch, opts any) *MockRestackHandlerRestackBranchCall { mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RestackBranch", reflect.TypeOf((*MockRestackHandler)(nil).RestackBranch), ctx, branch) + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RestackBranch", reflect.TypeOf((*MockRestackHandler)(nil).RestackBranch), ctx, branch, opts) return &MockRestackHandlerRestackBranchCall{Call: call} } @@ -808,13 +808,13 @@ func (c *MockRestackHandlerRestackBranchCall) Return(arg0 error) *MockRestackHan } // Do rewrite *gomock.Call.Do -func (c *MockRestackHandlerRestackBranchCall) Do(f func(context.Context, string) error) *MockRestackHandlerRestackBranchCall { +func (c *MockRestackHandlerRestackBranchCall) Do(f func(context.Context, string, *restack.Options) error) *MockRestackHandlerRestackBranchCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockRestackHandlerRestackBranchCall) DoAndReturn(f func(context.Context, string) error) *MockRestackHandlerRestackBranchCall { +func (c *MockRestackHandlerRestackBranchCall) DoAndReturn(f func(context.Context, string, *restack.Options) error) *MockRestackHandlerRestackBranchCall { c.Call = c.Call.DoAndReturn(f) return c } diff --git a/main.go b/main.go index d020ea4c..50eb0451 100644 --- a/main.go +++ b/main.go @@ -314,6 +314,8 @@ type mainCmd struct { Rebase rebaseCmd `cmd:"" aliases:"rb" group:"Rebase"` + CI ciCmd `cmd:"" group:"CI" help:"CI/CD integration commands"` + // Navigation Up upCmd `cmd:"" aliases:"u" group:"Navigation" help:"Move up one branch"` Down downCmd `cmd:"" aliases:"d" group:"Navigation" help:"Move down one branch"` diff --git a/stack_restack.go b/stack_restack.go index 88ef562b..7d7b4d68 100644 --- a/stack_restack.go +++ b/stack_restack.go @@ -88,5 +88,5 @@ func (cmd *stackRestackCmd) Run( return err } - return handler.RestackStack(ctx, cmd.Branch) + return handler.RestackStack(ctx, cmd.Branch, nil) } diff --git a/testdata/help/ci_merge-guard.txt b/testdata/help/ci_merge-guard.txt new file mode 100644 index 00000000..823186af --- /dev/null +++ b/testdata/help/ci_merge-guard.txt @@ -0,0 +1,35 @@ +Usage: gs ci merge-guard [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: + 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 + +Global Flags: + -h, --help Show help for the command + --version Print version information and quit + -v, --verbose Enable verbose output ($GIT_SPICE_VERBOSE) + -C, --dir=DIR Change to DIR before doing anything + --[no-]prompt Whether to prompt for missing information diff --git a/testdata/help/gs.txt b/testdata/help/gs.txt index 43406fde..51e82ea4 100644 --- a/testdata/help/gs.txt +++ b/testdata/help/gs.txt @@ -73,6 +73,9 @@ Rebase rebase (rb) continue (c) Continue an interrupted operation rebase (rb) abort (a) Abort an operation +CI + ci merge-guard Block merging a PR whose base is not trunk + Navigation up (u) Move up one branch down (d) Move down one branch diff --git a/testdata/script/branch_create_below_with_downstack_history.txt b/testdata/script/branch_create_below_with_downstack_history.txt index 4ac2630c..196a392b 100644 --- a/testdata/script/branch_create_below_with_downstack_history.txt +++ b/testdata/script/branch_create_below_with_downstack_history.txt @@ -81,6 +81,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -91,6 +92,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/final-comments.txt -- - change: 2 body: | @@ -103,6 +105,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -114,6 +117,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 4 body: | This change is part of the following stack: @@ -125,3 +129,4 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/branch_onto_two_stacks_with_downstack_history.txt b/testdata/script/branch_onto_two_stacks_with_downstack_history.txt index 0cea91ca..dc1a17d0 100644 --- a/testdata/script/branch_onto_two_stacks_with_downstack_history.txt +++ b/testdata/script/branch_onto_two_stacks_with_downstack_history.txt @@ -152,6 +152,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -162,6 +163,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -172,6 +174,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/submit-second-stack-comments.txt -- - change: 4 body: | @@ -182,6 +185,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 5 body: | This change is part of the following stack: @@ -191,6 +195,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/bottom-prs-merged-comments.txt -- - change: 2 body: | @@ -202,6 +207,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -212,6 +218,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 5 body: | This change is part of the following stack: @@ -221,6 +228,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/branch-onto-comments.txt -- - change: 2 body: | @@ -233,6 +241,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -243,6 +252,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 5 body: | This change is part of the following stack: @@ -253,3 +263,4 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/branch_submit_by_name.txt b/testdata/script/branch_submit_by_name.txt index 73b25bbc..dcfe1787 100644 --- a/testdata/script/branch_submit_by_name.txt +++ b/testdata/script/branch_submit_by_name.txt @@ -71,3 +71,4 @@ Contents of feature1 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/branch_submit_detect_existing.txt b/testdata/script/branch_submit_detect_existing.txt index b905a7c7..246f144b 100644 --- a/testdata/script/branch_submit_detect_existing.txt +++ b/testdata/script/branch_submit_detect_existing.txt @@ -120,6 +120,7 @@ feature 2 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/pulls-updated.json -- [ { @@ -181,6 +182,7 @@ feature 2 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -190,3 +192,4 @@ feature 2 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/branch_submit_navigation_comment_deleted.txt b/testdata/script/branch_submit_navigation_comment_deleted.txt index ef6196a4..1e55da55 100644 --- a/testdata/script/branch_submit_navigation_comment_deleted.txt +++ b/testdata/script/branch_submit_navigation_comment_deleted.txt @@ -53,6 +53,7 @@ feature content Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/no-comments.txt -- [] -- golden/recreated-comments.txt -- @@ -64,4 +65,5 @@ feature content Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/end -- diff --git a/testdata/script/branch_submit_navigation_comment_downstack_open.txt b/testdata/script/branch_submit_navigation_comment_downstack_open.txt index 6c46a9d2..0d5a1e34 100644 --- a/testdata/script/branch_submit_navigation_comment_downstack_open.txt +++ b/testdata/script/branch_submit_navigation_comment_downstack_open.txt @@ -59,6 +59,7 @@ feature 2 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/comments-after.txt -- - change: 2 body: | @@ -68,3 +69,4 @@ feature 2 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/branch_submit_navigation_comment_opt_out_multiple.txt b/testdata/script/branch_submit_navigation_comment_opt_out_multiple.txt index 02c23a45..9eaa426c 100644 --- a/testdata/script/branch_submit_navigation_comment_opt_out_multiple.txt +++ b/testdata/script/branch_submit_navigation_comment_opt_out_multiple.txt @@ -64,6 +64,7 @@ foo Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -73,3 +74,4 @@ foo Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/branch_submit_navigation_comment_sync_branch.txt b/testdata/script/branch_submit_navigation_comment_sync_branch.txt index 58698db8..f7aad447 100644 --- a/testdata/script/branch_submit_navigation_comment_sync_branch.txt +++ b/testdata/script/branch_submit_navigation_comment_sync_branch.txt @@ -56,6 +56,7 @@ feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -65,6 +66,7 @@ feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -75,3 +77,4 @@ feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/branch_submit_navigation_comment_sync_downstack.txt b/testdata/script/branch_submit_navigation_comment_sync_downstack.txt index 28e3cbf4..a7d40f85 100644 --- a/testdata/script/branch_submit_navigation_comment_sync_downstack.txt +++ b/testdata/script/branch_submit_navigation_comment_sync_downstack.txt @@ -75,6 +75,7 @@ feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -85,6 +86,7 @@ feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -95,3 +97,4 @@ feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/branch_submit_rename.txt b/testdata/script/branch_submit_rename.txt index 5665032f..4af78bfa 100644 --- a/testdata/script/branch_submit_rename.txt +++ b/testdata/script/branch_submit_rename.txt @@ -88,6 +88,7 @@ New contents of feature1 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/update.json -- [ { @@ -124,3 +125,4 @@ New contents of feature1 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/ci_merge_guard_already_merged.txt b/testdata/script/ci_merge_guard_already_merged.txt new file mode 100644 index 00000000..ba08b8ec --- /dev/null +++ b/testdata/script/ci_merge_guard_already_merged.txt @@ -0,0 +1,34 @@ +# 'ci merge-guard' handles an already-merged PR gracefully. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' + +# set up a fake GitHub remote +shamhub init +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a branch on trunk and submit +git add feature1.txt +gs bc feature1 -m 'Add feature 1' +gs bs --fill +stderr 'Created #1' + +# merge externally +shamhub merge alice/example 1 + +# merged PR was based on main (trunk) — still safe +gs ci merge-guard 1 +stderr 'safe to merge' + +-- repo/feature1.txt -- +This is feature 1 diff --git a/testdata/script/ci_merge_guard_blocked.txt b/testdata/script/ci_merge_guard_blocked.txt new file mode 100644 index 00000000..061efae7 --- /dev/null +++ b/testdata/script/ci_merge_guard_blocked.txt @@ -0,0 +1,43 @@ +# 'ci merge-guard' blocks a PR whose base is not trunk. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' + +# set up a fake GitHub remote +shamhub init +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a stack: main -> feature1 -> feature2 +git add feature1.txt +gs bc feature1 -m 'Add feature 1' +git add feature2.txt +gs bc feature2 -m 'Add feature 2' + +# submit the full stack +gs downstack submit --fill +stderr 'Created #1' +stderr 'Created #2' + +# PR #2 is based on feature1, not trunk — should be blocked +! gs ci merge-guard 2 +stderr 'base is "feature1"' +stderr 'expected trunk "main"' + +# PR #1 is based on main — should be safe +gs ci merge-guard 1 +stderr 'safe to merge' + +-- repo/feature1.txt -- +This is feature 1 +-- repo/feature2.txt -- +This is feature 2 diff --git a/testdata/script/ci_merge_guard_closed.txt b/testdata/script/ci_merge_guard_closed.txt new file mode 100644 index 00000000..6dbfe16e --- /dev/null +++ b/testdata/script/ci_merge_guard_closed.txt @@ -0,0 +1,34 @@ +# 'ci merge-guard' handles a closed (rejected) PR. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' + +# set up a fake GitHub remote +shamhub init +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a branch on trunk and submit +git add feature1.txt +gs bc feature1 -m 'Add feature 1' +gs bs --fill +stderr 'Created #1' + +# close (reject) the PR +shamhub reject alice/example 1 + +# closed PR was based on main (trunk) — still safe +gs ci merge-guard 1 +stderr 'safe to merge' + +-- repo/feature1.txt -- +This is feature 1 diff --git a/testdata/script/ci_merge_guard_deep_stack.txt b/testdata/script/ci_merge_guard_deep_stack.txt new file mode 100644 index 00000000..9ab64efe --- /dev/null +++ b/testdata/script/ci_merge_guard_deep_stack.txt @@ -0,0 +1,53 @@ +# 'ci merge-guard' blocks at every non-trunk level of a deep stack. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' + +# set up a fake GitHub remote +shamhub init +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a deep stack: main -> feat1 -> feat2 -> feat3 +git add feature1.txt +gs bc feature1 -m 'Add feature 1' +git add feature2.txt +gs bc feature2 -m 'Add feature 2' +git add feature3.txt +gs bc feature3 -m 'Add feature 3' + +# submit the full stack +gs downstack submit --fill +stderr 'Created #1' +stderr 'Created #2' +stderr 'Created #3' + +# PR #1 based on main — safe +gs ci merge-guard 1 +stderr 'safe to merge' + +# PR #2 based on feature1, not trunk — blocked +! gs ci merge-guard 2 +stderr 'base is "feature1"' +stderr 'expected trunk "main"' + +# PR #3 based on feature2, not trunk — blocked +! gs ci merge-guard 3 +stderr 'base is "feature2"' +stderr 'expected trunk "main"' + +-- repo/feature1.txt -- +This is feature 1 +-- repo/feature2.txt -- +This is feature 2 +-- repo/feature3.txt -- +This is feature 3 diff --git a/testdata/script/ci_merge_guard_not_found.txt b/testdata/script/ci_merge_guard_not_found.txt new file mode 100644 index 00000000..27194b36 --- /dev/null +++ b/testdata/script/ci_merge_guard_not_found.txt @@ -0,0 +1,24 @@ +# 'ci merge-guard' fails cleanly when the PR does not exist. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' + +# set up a fake GitHub remote +shamhub init +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# no PRs exist — guard should fail with a clear error +! gs ci merge-guard 999 +stderr 'find change #999' + +-- repo/.gitkeep -- diff --git a/testdata/script/ci_merge_guard_safe.txt b/testdata/script/ci_merge_guard_safe.txt new file mode 100644 index 00000000..d1ad9295 --- /dev/null +++ b/testdata/script/ci_merge_guard_safe.txt @@ -0,0 +1,31 @@ +# 'ci merge-guard' allows a trunk-based PR to merge. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' + +# set up a fake GitHub remote +shamhub init +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a branch on trunk and submit +git add feature1.txt +gs bc feature1 -m 'Add feature 1' +gs bs --fill +stderr 'Created #1' + +# PR #1 is based on main (trunk) — should be safe +gs ci merge-guard 1 +stderr 'safe to merge' + +-- repo/feature1.txt -- +This is feature 1 diff --git a/testdata/script/ci_merge_guard_trunk_override.txt b/testdata/script/ci_merge_guard_trunk_override.txt new file mode 100644 index 00000000..3637052d --- /dev/null +++ b/testdata/script/ci_merge_guard_trunk_override.txt @@ -0,0 +1,43 @@ +# 'ci merge-guard --trunk' overrides trunk detection. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' + +# set up a fake GitHub remote +shamhub init +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a stack: main -> feature1 -> feature2 +git add feature1.txt +gs bc feature1 -m 'Add feature 1' +git add feature2.txt +gs bc feature2 -m 'Add feature 2' + +gs downstack submit --fill +stderr 'Created #1' +stderr 'Created #2' + +# PR #2 is based on feature1. +# With --trunk=feature1, it should pass. +gs ci merge-guard --trunk feature1 2 +stderr 'safe to merge' + +# With --trunk=main (correct trunk), it should block. +! gs ci merge-guard --trunk main 2 +stderr 'base is "feature1"' +stderr 'expected trunk "main"' + +-- repo/feature1.txt -- +This is feature 1 +-- repo/feature2.txt -- +This is feature 2 diff --git a/testdata/script/ci_merge_guard_unmanaged.txt b/testdata/script/ci_merge_guard_unmanaged.txt new file mode 100644 index 00000000..0b7e4a07 --- /dev/null +++ b/testdata/script/ci_merge_guard_unmanaged.txt @@ -0,0 +1,39 @@ +# 'ci merge-guard' allows unmanaged PRs by default, +# blocks them with --all. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' + +# set up a fake GitHub remote +shamhub init +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a branch on trunk and submit +git add feature1.txt +gs bc feature1 -m 'Add feature 1' +gs bs --fill +stderr 'Created #1' + +# delete the navigation comment to simulate an unmanaged PR +shamhub delete-comment 1 + +# by default, unmanaged PRs are allowed through +gs ci merge-guard 1 +stderr 'not managed by git-spice, allowing' + +# with --all, unmanaged PRs are blocked +! gs ci merge-guard --all 1 +stderr 'unmanaged PR blocked by --all' + +-- repo/feature1.txt -- +This is feature 1 diff --git a/testdata/script/downstack_submit.txt b/testdata/script/downstack_submit.txt index 6fa9a506..a11088a8 100644 --- a/testdata/script/downstack_submit.txt +++ b/testdata/script/downstack_submit.txt @@ -128,6 +128,7 @@ This is feature 2 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -138,6 +139,7 @@ This is feature 2 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -148,3 +150,4 @@ This is feature 2 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/repo_sync_unsubmitted_upstack_history.txt b/testdata/script/repo_sync_unsubmitted_upstack_history.txt index f24b54dd..a9b24501 100644 --- a/testdata/script/repo_sync_unsubmitted_upstack_history.txt +++ b/testdata/script/repo_sync_unsubmitted_upstack_history.txt @@ -60,6 +60,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -69,3 +70,4 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/stack_edit_inserted_at_bottom_with_downstack_history.txt b/testdata/script/stack_edit_inserted_at_bottom_with_downstack_history.txt index c79faea3..17f7ea99 100644 --- a/testdata/script/stack_edit_inserted_at_bottom_with_downstack_history.txt +++ b/testdata/script/stack_edit_inserted_at_bottom_with_downstack_history.txt @@ -114,6 +114,7 @@ main ◀ Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -125,6 +126,7 @@ main ◀ Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -136,6 +138,7 @@ main ◀ Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 4 body: | This change is part of the following stack: @@ -147,6 +150,7 @@ main ◀ Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/ls.txt -- ┏━□ feature3 (#3) ┏━┻□ feature2 (#2) @@ -165,6 +169,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -176,6 +181,7 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 4 body: | This change is part of the following stack: @@ -187,3 +193,4 @@ main Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/stack_submit.txt b/testdata/script/stack_submit.txt index d836f990..82ec2412 100644 --- a/testdata/script/stack_submit.txt +++ b/testdata/script/stack_submit.txt @@ -148,6 +148,7 @@ This is feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -158,6 +159,7 @@ This is feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -168,6 +170,7 @@ This is feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/pr-1-merged.json -- [ { @@ -253,6 +256,7 @@ This is feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -263,6 +267,7 @@ This is feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -273,6 +278,7 @@ This is feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/ls.txt -- ┏━□ feature3 (#3) ┏━┻□ feature2 (#2) diff --git a/testdata/script/stack_submit_multiple_merged_history.txt b/testdata/script/stack_submit_multiple_merged_history.txt index ba40d9c6..6be62d59 100644 --- a/testdata/script/stack_submit_multiple_merged_history.txt +++ b/testdata/script/stack_submit_multiple_merged_history.txt @@ -67,6 +67,7 @@ This is feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -77,6 +78,7 @@ This is feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -87,6 +89,7 @@ This is feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/post-merge-comments.txt -- - change: 1 body: | @@ -98,6 +101,7 @@ This is feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -108,6 +112,7 @@ This is feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -118,6 +123,7 @@ This is feature 3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/ls.txt -- ┏━□ feature3 (#3) ┏━┻□ feature2 (#2) diff --git a/testdata/script/stack_submit_update_leave_draft.txt b/testdata/script/stack_submit_update_leave_draft.txt index ca804fbf..13c92791 100644 --- a/testdata/script/stack_submit_update_leave_draft.txt +++ b/testdata/script/stack_submit_update_leave_draft.txt @@ -240,6 +240,7 @@ INF CR #3 is up-to-date: $SHAMHUB_URL/alice/example/change/3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -249,6 +250,7 @@ INF CR #3 is up-to-date: $SHAMHUB_URL/alice/example/change/3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -259,6 +261,7 @@ INF CR #3 is up-to-date: $SHAMHUB_URL/alice/example/change/3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + -- golden/comments/pr-1-merged.txt -- - change: 1 body: | @@ -268,6 +271,7 @@ INF CR #3 is up-to-date: $SHAMHUB_URL/alice/example/change/3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -278,6 +282,7 @@ INF CR #3 is up-to-date: $SHAMHUB_URL/alice/example/change/3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -288,3 +293,4 @@ INF CR #3 is up-to-date: $SHAMHUB_URL/alice/example/change/3 Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/testdata/script/upstack_submit_main.txt b/testdata/script/upstack_submit_main.txt index 3c16bd34..78ec7d50 100644 --- a/testdata/script/upstack_submit_main.txt +++ b/testdata/script/upstack_submit_main.txt @@ -192,6 +192,7 @@ main ◀ Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 2 body: | This change is part of the following stack: @@ -201,6 +202,7 @@ main ◀ Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 3 body: | This change is part of the following stack: @@ -211,6 +213,7 @@ main ◀ Change managed by [git-spice](https://abhinav.github.io/git-spice/). + - change: 4 body: | This change is part of the following stack: @@ -221,3 +224,4 @@ main ◀ Change managed by [git-spice](https://abhinav.github.io/git-spice/). + diff --git a/upstack_restack.go b/upstack_restack.go index f6e6e052..4def1ada 100644 --- a/upstack_restack.go +++ b/upstack_restack.go @@ -34,10 +34,10 @@ func (*upstackRestackCmd) Help() string { // RestackHandler implements high level restack operations. type RestackHandler interface { RestackUpstack(ctx context.Context, branch string, opts *restack.UpstackOptions) error - RestackDownstack(ctx context.Context, branch string) error + RestackDownstack(ctx context.Context, branch string, opts *restack.Options) error Restack(context.Context, *restack.Request) (int, error) - RestackStack(ctx context.Context, branch string) error - RestackBranch(ctx context.Context, branch string) error + RestackStack(ctx context.Context, branch string, opts *restack.Options) error + RestackBranch(ctx context.Context, branch string, opts *restack.Options) error } func (cmd *upstackRestackCmd) AfterApply(ctx context.Context, wt *git.Worktree) error {