From dca7e44cbfff9803150bcdf74676d790844547e8 Mon Sep 17 00:00:00 2001 From: Edmund Kohlwey Date: Tue, 23 Jun 2026 10:41:07 -0400 Subject: [PATCH 1/2] restack: add Options parameter to RestackBranch/Stack/Downstack Introduces restack.Options{AutoResolve} and threads an opts *Options parameter through the high-level restack entry points (RestackBranch, RestackStack, RestackDownstack) and the merge/sync RestackHandler interfaces, with all current callers passing nil. This is foundational plumbing for the restack auto-resolve feature. Landing it on a branch below the consuming stacks lets every branch inherit the 3-arg signature, rather than the signature change living on a branch parallel to its callers. --- branch_restack.go | 2 +- downstack_restack.go | 2 +- internal/handler/merge/handler.go | 5 ++-- internal/handler/merge/handler_test.go | 6 ++--- internal/handler/merge/mocks_test.go | 13 ++++++----- internal/handler/restack/branch.go | 5 +++- internal/handler/restack/branch_test.go | 10 ++++---- internal/handler/restack/downstack.go | 5 +++- internal/handler/restack/downstack_test.go | 4 ++-- internal/handler/restack/handler.go | 27 ++++++++++++++++++++++ internal/handler/restack/stack.go | 5 +++- internal/handler/sync/handler.go | 4 ++-- internal/handler/sync/handler_test.go | 2 +- internal/handler/sync/mocks_test.go | 12 +++++----- stack_restack.go | 2 +- upstack_restack.go | 6 ++--- 16 files changed, 74 insertions(+), 36 deletions(-) diff --git a/branch_restack.go b/branch_restack.go index 32b8dc590..f84b803b3 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/downstack_restack.go b/downstack_restack.go index e3f95a420..87a273f65 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 8172b2334..0f921e882 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 317d2a05b..f1d1f165f 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 9d4523a57..c59410e48 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 8d880183f..889d9a272 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 95664e688..c7435831c 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 e4df6f7ab..60c0ab42d 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 fa633e067..a99ac3399 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 7ff1f6d45..93d0dadc1 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 f37ad358b..592e99eab 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/sync/handler.go b/internal/handler/sync/handler.go index 4aa7da792..af116412d 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 b6450f281..415897d24 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 51c7adcf5..de0af2909 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/stack_restack.go b/stack_restack.go index 88ef562be..7d7b4d689 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/upstack_restack.go b/upstack_restack.go index f6e6e0520..4def1ada7 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 { From 221cfdcb77fb637cd4c1ce0e13594061c1687f3a Mon Sep 17 00:00:00 2001 From: Edmund Kohlwey Date: Mon, 1 Jun 2026 07:40:39 -0400 Subject: [PATCH 2/2] sync: detect self-merged stacks pushed directly to trunk When a user merges a stack by pushing the stack tip directly to trunk on the remote (e.g., 'git push origin --force-with-lease HEAD:main'), the forge often does not report the affected change requests as merged. On the supported-forge path, repo sync therefore failed to clean up the stack, leaving each tracked branch with a stale BaseHash that later restacks would replay against trunk, causing conflicts. Run the same local-ancestor merge check the unsupported-forge path already uses as a fallback after the forge API queries: any tracked branch whose head is reachable from the new trunk hash is treated as merged, regardless of forge state. Branches with submitted change requests still attach their ChangeID for downstack history propagation; unsubmitted branches log "merged into trunk locally". --- .../unreleased/Fixed-20260601-073704.yaml | 3 + internal/handler/sync/handler.go | 109 +++++++++++++++--- .../repo_sync_self_merged_submitted_stack.txt | 53 +++++++++ ...epo_sync_self_merged_unsubmitted_stack.txt | 47 ++++++++ 4 files changed, 195 insertions(+), 17 deletions(-) create mode 100644 .changes/unreleased/Fixed-20260601-073704.yaml create mode 100644 testdata/script/repo_sync_self_merged_submitted_stack.txt create mode 100644 testdata/script/repo_sync_self_merged_unsubmitted_stack.txt diff --git a/.changes/unreleased/Fixed-20260601-073704.yaml b/.changes/unreleased/Fixed-20260601-073704.yaml new file mode 100644 index 000000000..2ccd4e03d --- /dev/null +++ b/.changes/unreleased/Fixed-20260601-073704.yaml @@ -0,0 +1,3 @@ +kind: Fixed +body: 'repo sync: detect locally-merged branches on supported forges when commits land on trunk via direct push, cleaning up the stack without restack conflicts.' +time: 2026-06-01T07:37:04.600167-04:00 diff --git a/internal/handler/sync/handler.go b/internal/handler/sync/handler.go index af116412d..6680d7cb3 100644 --- a/internal/handler/sync/handler.go +++ b/internal/handler/sync/handler.go @@ -400,7 +400,7 @@ func (h *Handler) SyncTrunk(ctx context.Context, opts *TrunkOptions) (retErr err } } else { // Supported forge. Check for merged CRs and upstream branches. - branchesToDelete, err = h.findForgeFinishedBranches(ctx, branchGraph, candidates, opts.ClosedChanges) + branchesToDelete, err = h.findForgeFinishedBranches(ctx, branchGraph, candidates, trunkEndHash, opts.ClosedChanges) if err != nil { return fmt.Errorf("find finished CRs: %w", err) } @@ -475,6 +475,24 @@ func (h *Handler) SyncTrunk(ctx context.Context, opts *TrunkOptions) (retErr err return nil } +// localAncestorMergedSet returns the names of branches whose head commits +// are reachable from trunkHash. These branches have been merged locally, +// whether by a forge merge that landed on the local trunk +// or by a direct push to trunk that bypassed the forge. +func (h *Handler) localAncestorMergedSet( + ctx context.Context, + knownBranches []spice.LoadBranchItem, + trunkHash git.Hash, +) map[string]struct{} { + merged := make(map[string]struct{}) + for _, b := range knownBranches { + if h.Repository.IsAncestor(ctx, b.Head, trunkHash) { + merged[b.Name] = struct{}{} + } + } + return merged +} + // findLocalMergedBranches finds branches that have been merged // by inspecting what's reachable from the trunk. // @@ -485,19 +503,22 @@ func (h *Handler) findLocalMergedBranches( knownBranches []spice.LoadBranchItem, trunkHash git.Hash, ) ([]branchDeletion, error) { - // Find branches that have been merged by checking - // if they are reachable from the trunk. - var branchesToDelete []branchDeletion + merged := h.localAncestorMergedSet(ctx, knownBranches, trunkHash) + if len(merged) == 0 { + return nil, nil + } + + branchesToDelete := make([]branchDeletion, 0, len(merged)) for _, b := range knownBranches { - if h.Repository.IsAncestor(ctx, b.Head, trunkHash) { - h.Log.Infof("%v was merged", b.Name) - branchesToDelete = append(branchesToDelete, branchDeletion{ - BranchName: b.Name, - UpstreamName: b.UpstreamBranch, - }) + if _, ok := merged[b.Name]; !ok { + continue } + h.Log.Infof("%v was merged", b.Name) + branchesToDelete = append(branchesToDelete, branchDeletion{ + BranchName: b.Name, + UpstreamName: b.UpstreamBranch, + }) } - return branchesToDelete, nil } @@ -505,6 +526,7 @@ func (h *Handler) findForgeFinishedBranches( ctx context.Context, branchGraph *spice.BranchGraph, knownBranches []spice.LoadBranchItem, + trunkHash git.Hash, closedChangeHandling ClosedChanges, ) ([]branchDeletion, error) { type submittedBranch struct { @@ -740,6 +762,49 @@ func (h *Handler) findForgeFinishedBranches( } } + // Fallback: a branch whose head is already reachable from trunk + // has been merged locally, regardless of forge state. + // This catches direct pushes to trunk that bypassed the CR flow, + // and unsubmitted branches whose commits landed on trunk + // via some other path. + if locallyMerged := h.localAncestorMergedSet(ctx, knownBranches, trunkHash); len(locallyMerged) > 0 { + submittedByName := make(map[string]*submittedBranch, len(submittedBranches)) + for _, b := range submittedBranches { + submittedByName[b.Name] = b + } + + for _, b := range knownBranches { + if _, ok := locallyMerged[b.Name]; !ok { + continue + } + if _, ok := finishedBranches[b.Name]; ok { + continue + } + + upstreamBranch := b.UpstreamBranch + if upstreamBranch == "" { + upstreamBranch = b.Name + } + + var changeID forge.ChangeID + if sb, ok := submittedByName[b.Name]; ok { + changeID = sb.Change + h.Log.Infof("%v: %v was merged", b.Name, changeID) + } else { + h.Log.Infof("%v: merged into trunk locally", b.Name) + } + + finishedBranches[b.Name] = finishedBranch{ + Name: b.Name, + Base: b.Base, + UpstreamBranch: upstreamBranch, + ChangeID: changeID, + Merged: true, + } + mergedDownstacks[b.Name] = b.MergedDownstack + } + } + if len(finishedBranches) == 0 { return nil, nil } @@ -776,11 +841,19 @@ func (h *Handler) findForgeFinishedBranches( must.Bef(ok, "topologically sorted branch %q must be finished", name) must.Bef(branch.Merged, "topologically sorted branch %q must be merged", name) - changeIDJSON, err := h.RemoteRepository.Forge().MarshalChangeID(branch.ChangeID) - if err != nil { - h.Log.Warn("Unable to serialize ChangeID for merged branch. Not propagating to merge history.", - "branch", name, "changeID", branch.ChangeID, "error", err) - continue + // A branch detected as locally merged may not have a ChangeID + // because it was never submitted to the forge. + // Skip its own contribution to the merge history, + // but still propagate any downstack history it carries. + var changeIDJSON json.RawMessage + if branch.ChangeID != nil { + marshaled, err := h.RemoteRepository.Forge().MarshalChangeID(branch.ChangeID) + if err != nil { + h.Log.Warn("Unable to serialize ChangeID for merged branch. Not propagating to merge history.", + "branch", name, "changeID", branch.ChangeID, "error", err) + continue + } + changeIDJSON = marshaled } for above := range branchGraph.Aboves(name) { @@ -788,7 +861,9 @@ func (h *Handler) findForgeFinishedBranches( // is the branch's own merged downstack and the branch itself. var newHistory []json.RawMessage newHistory = append(newHistory, mergedDownstacks[name]...) - newHistory = append(newHistory, changeIDJSON) + if changeIDJSON != nil { + newHistory = append(newHistory, changeIDJSON) + } // Combine with anything else already in the merged downstack. // (Normally this will be empty.) newHistory = append(newHistory, mergedDownstacks[above]...) diff --git a/testdata/script/repo_sync_self_merged_submitted_stack.txt b/testdata/script/repo_sync_self_merged_submitted_stack.txt new file mode 100644 index 000000000..dcf02ad1b --- /dev/null +++ b/testdata/script/repo_sync_self_merged_submitted_stack.txt @@ -0,0 +1,53 @@ +# 'repo sync' must detect a stack that was self-merged by pushing +# the stack tip directly to trunk on the remote, +# even when the change requests for the stack are still open on the forge +# (the forge has not auto-detected the direct push as a merge). + +as 'Test ' +at '2026-06-01T12:00:00Z' + +# 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 + +# Build a local stack main -> feat1 -> feat2 with submitted PRs. +git add feat1.txt +gs bc -m 'Add feat1' feat1 + +git add feat2.txt +gs bc -m 'Add feat2' feat2 + +gs stack submit --fill +stderr 'Created #1' +stderr 'Created #2' + +# Self-merge by pushing the stack tip straight to trunk on the remote, +# bypassing the change request flow. The CRs remain Open on the forge. +git push origin feat2:main + +# Move off the stack tip so 'repo sync' can advance trunk freely. +gs trunk + +# Sync. Both branches should be deleted: their commits are reachable +# from trunk locally even though the forge still reports the CRs as Open. +gs repo sync +stderr 'feat1: #1 was merged' +stderr 'feat2: #2 was merged' + +! git rev-parse --verify feat1 +! git rev-parse --verify feat2 + +-- repo/feat1.txt -- +Contents of feat1 +-- repo/feat2.txt -- +Contents of feat2 diff --git a/testdata/script/repo_sync_self_merged_unsubmitted_stack.txt b/testdata/script/repo_sync_self_merged_unsubmitted_stack.txt new file mode 100644 index 000000000..44833ab95 --- /dev/null +++ b/testdata/script/repo_sync_self_merged_unsubmitted_stack.txt @@ -0,0 +1,47 @@ +# 'repo sync' must detect a stack that was self-merged by pushing +# the stack tip directly to trunk on the remote, +# even when no change requests were submitted. + +as 'Test ' +at '2026-06-01T12:00:00Z' + +# 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 + +# Build a local stack main -> feat1 -> feat2 without submitting PRs. +git add feat1.txt +gs bc -m 'Add feat1' feat1 + +git add feat2.txt +gs bc -m 'Add feat2' feat2 + +# Self-merge by pushing the stack tip straight to trunk on the remote. +git push origin feat2:main + +# Move off the stack tip so 'repo sync' can advance trunk freely. +gs trunk + +# Sync. Both branches should be detected as merged into trunk locally, +# even though no PRs were ever submitted for them. +gs repo sync +stderr 'feat1: merged into trunk locally' +stderr 'feat2: merged into trunk locally' + +! git rev-parse --verify feat1 +! git rev-parse --verify feat2 + +-- repo/feat1.txt -- +Contents of feat1 +-- repo/feat2.txt -- +Contents of feat2