diff --git a/internal/forge/shamhub/merge_range.go b/internal/forge/shamhub/merge_range.go index 06f6984df..4286ace1f 100644 --- a/internal/forge/shamhub/merge_range.go +++ b/internal/forge/shamhub/merge_range.go @@ -149,6 +149,10 @@ type preparedMergeRange struct { // rootBaseHash is the target branch value used by the publishing CAS. rootBaseHash string + // rootMergeBaseHash is the fork point of the bottom change and its target. + // It can precede rootBaseHash when the target advanced independently. + rootMergeBaseHash string + // changes retains the validated server records in bottom-to-top order. changes []preparedMergeRangeChange } @@ -322,7 +326,19 @@ func (sh *ShamHub) prepareMergeRange( expected.Number, baseHash, prepared.changes[i-1].headHash, ) } - if err := sh.gitCmd( + if i == 0 { + // The target may have advanced since the bottom change was submitted. + // Apply the stack's delta from its fork point onto the current target; + // using the current target as the merge base would discard its new work. + mergeBase, err := sh.gitCmd( + ctx, owner, repo, "merge-base", baseHash, headHash, + ).OutputChomp() + if err != nil { + return preparedMergeRange{}, fmt.Errorf("find change %d merge base: %w", expected.Number, err) + } + prepared.rootBaseHash = baseHash + prepared.rootMergeBaseHash = mergeBase + } else if err := sh.gitCmd( ctx, owner, repo, @@ -337,9 +353,6 @@ func (sh *ShamHub) prepareMergeRange( ) } - if i == 0 { - prepared.rootBaseHash = baseHash - } prepared.changes[i] = preparedMergeRangeChange{ index: changeIndex, change: change, @@ -365,7 +378,7 @@ func (sh *ShamHub) buildMergeRangeCommit( repo, "merge-tree", "--write-tree", - "--merge-base="+prepared.rootBaseHash, + "--merge-base="+prepared.rootMergeBaseHash, prepared.rootBaseHash, top.headHash, ).OutputChomp() @@ -391,7 +404,7 @@ func (sh *ShamHub) buildMergeRangeCommit( // Squashing rewrites ancestry: parent follows the new commits, while // base follows the original stack heads. Pinning base applies only // this PR's delta onto parent, including reversions of lower changes. - base := prepared.rootBaseHash + base := prepared.rootMergeBaseHash parent := prepared.rootBaseHash for _, change := range prepared.changes { tree, err := sh.gitCmd( diff --git a/internal/forge/shamhub/merge_range_test.go b/internal/forge/shamhub/merge_range_test.go index 093186ac0..67b398da3 100644 --- a/internal/forge/shamhub/merge_range_test.go +++ b/internal/forge/shamhub/merge_range_test.go @@ -74,6 +74,53 @@ func TestStackRepository_MergeRange_squash(t *testing.T) { ) } +func TestStackRepository_MergeRange_advancedTrunk(t *testing.T) { + for _, tt := range []struct { + name string + method forge.MergeMethod + commits string + }{ + {name: "Merge", method: forge.MergeMethodMerge, commits: "1"}, + {name: "Squash", method: forge.MergeMethodSquash, commits: "2"}, + } { + t.Run(tt.name, func(t *testing.T) { + fixture := newMergeRangeFixture(t) + fixture.request.Method = tt.method + + // Advance the receiving branch without changing either submitted head. + workDir := t.TempDir() + worktree, err := git.Clone(t.Context(), + fixture.sh.RepoURL("alice", "example"), workDir, + git.CloneOptions{Log: silogtest.New(t)}, + ) + require.NoError(t, err) + writeCommitAndPush(t, worktree, workDir, + "trunk.txt", "trunk\n", "Advance trunk", "main") + trunkHash, err := worktree.Head(t.Context()) + require.NoError(t, err) + + operation, err := fixture.plan().Merge(t.Context(), fixture.request) + require.NoError(t, err) + require.NotNil(t, operation) + assertMergeRangeCompleted(t, fixture) + + // Merging the stack must retain work added on both sides of the fork. + for _, name := range []string{"trunk", "bottom", "top"} { + contents, err := fixture.sh.gitCmd(t.Context(), "alice", "example", + "show", "main:"+name+".txt", + ).OutputChomp() + require.NoError(t, err) + assert.Equal(t, name, contents) + } + count, err := fixture.sh.gitCmd(t.Context(), "alice", "example", + "rev-list", "--first-parent", "--count", trunkHash.String()+"..main", + ).OutputChomp() + require.NoError(t, err) + assert.Equal(t, tt.commits, count) + }) + } +} + func TestStackRepository_MergeRange_squashUpstackRevert(t *testing.T) { fixture := newMergeRangeFixture(t) fixture.request.Method = forge.MergeMethodSquash diff --git a/internal/handler/merge/merge_range_test.go b/internal/handler/merge/merge_range_test.go index 5c2a36a56..b8548fc4a 100644 --- a/internal/handler/merge/merge_range_test.go +++ b/internal/handler/merge/merge_range_test.go @@ -83,10 +83,10 @@ func TestMergeScheduler_nativeLinearPathWithUnselectedDivergence(t *testing.T) { mockService := NewMockService(ctrl) mockGit := NewMockGitRepository(ctrl) - for idx, branch := range []string{"feat1", "feat2", "feat3"} { + for idx, branch := range []string{"feat2", "feat3"} { mockService.EXPECT().VerifyRestacked(gomock.Any(), branch).Return(nil) mockGit.EXPECT().PeelToCommit(gomock.Any(), branch). - Return(git.Hash(fmt.Sprintf("head-%d", idx+1)), nil) + Return(git.Hash(fmt.Sprintf("head-%d", idx+2)), nil) } syncHandler := &recordingSyncHandler{} @@ -96,8 +96,10 @@ func TestMergeScheduler_nativeLinearPathWithUnselectedDivergence(t *testing.T) { gitRepo: mockGit, sync: syncHandler, }) + bottom := testPlanEntry("feat1", "main", pr1) + bottom.headHash = "head-1" // captured during preflight err := h.executePlan(t.Context(), testMergePlanWithBases( - testPlanEntry("feat1", "main", pr1), + bottom, testPlanEntry("feat2", "feat1", pr2), testPlanEntry("feat3", "feat2", pr3), ), mergeExecutionOptions{ @@ -199,10 +201,8 @@ func TestMergeScheduler_nativeRangeUnsupportedFallsBack(t *testing.T) { } mockService := NewMockService(ctrl) - mockService.EXPECT().VerifyRestacked(gomock.Any(), "feat1").Return(nil) mockService.EXPECT().VerifyRestacked(gomock.Any(), "feat2").Return(nil).Times(2) mockGit := NewMockGitRepository(ctrl) - mockGit.EXPECT().PeelToCommit(gomock.Any(), "feat1").Return(git.Hash("head-1"), nil) mockGit.EXPECT().PeelToCommit(gomock.Any(), "feat2").Return(git.Hash("head-2"), nil).Times(2) syncHandler := &recordingSyncHandler{operations: operations} @@ -212,8 +212,10 @@ func TestMergeScheduler_nativeRangeUnsupportedFallsBack(t *testing.T) { gitRepo: mockGit, sync: syncHandler, }) + bottom := testPlanEntry("feat1", "main", pr1) + bottom.headHash = "head-1" err := h.executePlan(t.Context(), testMergePlanWithBases( - testPlanEntry("feat1", "main", pr1), + bottom, testPlanEntry("feat2", "feat1", pr2), ), mergeExecutionOptions{MergeTimeout: time.Second}) require.NoError(t, err) @@ -294,16 +296,8 @@ func TestMergeScheduler_nativeFallbackSerializesWorkspaceChanges(t *testing.T) { }) mockService := NewMockService(ctrl) mockGit := NewMockGitRepository(ctrl) - for idx, branch := range []string{"feat1", "feat2"} { - mockService.EXPECT(). - VerifyRestacked(gomock.Any(), branch). - Return(nil). - Times(idx + 1) - mockGit.EXPECT(). - PeelToCommit(gomock.Any(), branch). - Return(git.Hash(fmt.Sprintf("head-%d", idx+1)), nil). - Times(idx + 1) - } + mockService.EXPECT().VerifyRestacked(gomock.Any(), "feat2").Return(nil).Times(2) + mockGit.EXPECT().PeelToCommit(gomock.Any(), "feat2").Return(git.Hash("head-2"), nil).Times(2) h := newTestHandler(t, ctrl, testHandlerOpts{ forgeRepo: rangeRepo, service: mockService, @@ -311,9 +305,11 @@ func TestMergeScheduler_nativeFallbackSerializesWorkspaceChanges(t *testing.T) { sync: syncHandler, }) done := make(chan error, 1) + bottom := testPlanEntry("feat1", "main", pr1) + bottom.headHash = "head-1" go func() { done <- h.executePlan(t.Context(), []*mergeItem{ - testPlanEntry("feat1", "main", pr1), + bottom, testPlanEntry("feat2", "feat1", pr2), testPlanEntry("feat3", "main", pr3), }, mergeExecutionOptions{}) @@ -461,6 +457,7 @@ func TestRangeMergeQueueItem_Run_partialFallbackFailure(t *testing.T) { testPlanEntry("feat2", "feat1", pr2), testPlanEntry("feat3", "feat2", pr3), } + plan[0].headHash = "head-1" // The range covers feat1 and feat2. After native refusal, feat1 merges // individually but feat2 becomes blocked. Only feat2 may be reported as // failed; feat1 remains merged and dependent feat3 must be skipped. @@ -468,16 +465,17 @@ func TestRangeMergeQueueItem_Run_partialFallbackFailure(t *testing.T) { mockService := NewMockService(ctrl) mockGit := NewMockGitRepository(ctrl) for _, member := range []struct { - id fakeChangeID - branch string - head git.Hash - checks int + id fakeChangeID + branch string + head git.Hash + checks int + preparations int }{ - {pr1, "feat1", "head-1", 1}, - {pr2, "feat2", "head-2", 2}, + {pr1, "feat1", "head-1", 1, 0}, + {pr2, "feat2", "head-2", 2, 2}, } { - mockService.EXPECT().VerifyRestacked(gomock.Any(), member.branch).Return(nil).Times(member.checks) - mockGit.EXPECT().PeelToCommit(gomock.Any(), member.branch).Return(member.head, nil).Times(member.checks) + mockService.EXPECT().VerifyRestacked(gomock.Any(), member.branch).Return(nil).Times(member.preparations) + mockGit.EXPECT().PeelToCommit(gomock.Any(), member.branch).Return(member.head, nil).Times(member.preparations) mockForge.EXPECT(). ChangeStatuses(gomock.Any(), []forge.ChangeID{member.id}). Return([]forge.ChangeStatus{{State: forge.ChangeOpen, HeadHash: member.head}}, nil). diff --git a/internal/handler/merge/workspace.go b/internal/handler/merge/workspace.go index 695e84cca..724011a68 100644 --- a/internal/handler/merge/workspace.go +++ b/internal/handler/merge/workspace.go @@ -51,9 +51,9 @@ func (w *mergeWorkspace) PrepareChange(ctx context.Context, item *mergeItem) err return nil } -// PrepareRange aligns every member and captures its resulting head commit. -// Unlike an ordinary item, a range must also verify its bottom member: -// the provider requires the entire range to be restacked before merging it. +// PrepareRange aligns members with their branch bases and collects their heads. +// Like ordinary merging, a range does not rebase its trunk-based bottom merely +// because trunk advanced. Its preflight head remains valid. func (w *mergeWorkspace) PrepareRange(ctx context.Context, items []*mergeItem) ([]forge.ChangeHead, error) { release, err := w.acquire(ctx) if err != nil { @@ -63,8 +63,10 @@ func (w *mergeWorkspace) PrepareRange(ctx context.Context, items []*mergeItem) ( heads := make([]forge.ChangeHead, len(items)) for idx, item := range items { - if err := w.prepare(ctx, item); err != nil { - return nil, fmt.Errorf("prepare %q: %w", item.branch, err) + if item.base != w.Trunk { + if err := w.prepare(ctx, item); err != nil { + return nil, fmt.Errorf("prepare %q: %w", item.branch, err) + } } heads[idx] = forge.ChangeHead{Change: item.changeID, HeadHash: item.headHash} } diff --git a/internal/handler/merge/workspace_test.go b/internal/handler/merge/workspace_test.go new file mode 100644 index 000000000..8cf06b94c --- /dev/null +++ b/internal/handler/merge/workspace_test.go @@ -0,0 +1,56 @@ +package merge + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "go.abhg.dev/gs/internal/forge" + "go.abhg.dev/gs/internal/git" + "go.abhg.dev/gs/internal/handler/restack" + "go.abhg.dev/gs/internal/handler/submit" + "go.abhg.dev/gs/internal/spice" +) + +func TestMergeWorkspace_PrepareRange_bottomAfterBaseMerge(t *testing.T) { + ctrl := gomock.NewController(t) + service := NewMockService(ctrl) + // The range starts above an earlier queue item that has now merged. + // Its bottom still needs preparation onto trunk despite being first in range. + service.EXPECT().VerifyRestacked(gomock.Any(), "bottom"). + Return(&spice.BranchNeedsRestackError{Base: "main"}) + restackHandler := NewMockRestackHandler(ctrl) + restackHandler.EXPECT().RestackBranch(gomock.Any(), &restack.BranchRequest{ + Branch: "bottom", + }).Return(nil) + submitHandler := NewMockSubmitHandler(ctrl) + submitHandler.EXPECT().Submit(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, req *submit.Request) error { + assert.Equal(t, "bottom", req.Branch) + return nil + }) + gitRepo := NewMockGitRepository(ctrl) + gitRepo.EXPECT().PeelToCommit(gomock.Any(), "bottom"). + Return(git.Hash("restacked-bottom"), nil) + + workspace := &mergeWorkspace{ + Repository: gitRepo, + Service: service, + Restack: restackHandler, + Submit: submitHandler, + Sync: new(recordingSyncHandler), + Progress: new(recordingMergeProgress), + Trunk: "main", + gate: make(chan struct{}, 1), + } + item := testPlanEntry("bottom", "merged", "pr-bottom") + item.headHash = "preflight-bottom" + heads, err := workspace.PrepareRange(t.Context(), []*mergeItem{item}) + require.NoError(t, err) + assert.Equal(t, []forge.ChangeHead{{ + Change: fakeChangeID("pr-bottom"), HeadHash: "restacked-bottom", + }}, heads) +} diff --git a/testdata/script/downstack_merge_native_range_advanced_trunk.txt b/testdata/script/downstack_merge_native_range_advanced_trunk.txt new file mode 100644 index 000000000..c9033366c --- /dev/null +++ b/testdata/script/downstack_merge_native_range_advanced_trunk.txt @@ -0,0 +1,63 @@ +# An aligned native stack can merge without rebasing onto a newer trunk. + +[!git:2.40.0] skip # native stack merges require merge-tree --merge-base + +as 'Test ' +at '2026-10-26T00:03:25Z' + +cd repo +git init +git config spice.experiment.merge true +git config spice.forge.shamhub.stacks on +git commit --allow-empty -m 'Initial commit' + +shamhub-setup +shamhub new origin alice/example.git +shamhub register alice +git push origin main +env SHAMHUB_USERNAME=alice +gs auth login + +git add bottom.txt +gs bc bottom -m 'Add bottom' +git add middle.txt +gs bc middle -m 'Add middle' +git add top.txt +gs bc top -m 'Add top' +gs downstack submit --fill + +# Trunk advances independently; the members still align with each other. +gs branch checkout main +git add trunk.txt +git commit -m 'Advance trunk' +git push origin main +git tag before-merge +gs branch checkout top + +gs downstack merge --no-prompt +stderr 'All 3 change.s. merged.' +! stderr 'restacked' +! stderr 'Updated #' + +# One atomic merge preserves both trunk's new work and the stack's work. +git rev-list --first-parent --count before-merge..main +stdout '^1$' +git log -1 --format=%s main +stdout '^Merge changes #1 through #3$' +git show main:trunk.txt +stdout '^trunk$' +git show main:bottom.txt +stdout '^bottom$' +git show main:middle.txt +stdout '^middle$' +git show main:top.txt +stdout '^top$' + +-- repo/bottom.txt -- +bottom +-- repo/middle.txt -- +middle +-- repo/top.txt -- +top +-- repo/trunk.txt -- +trunk