Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions internal/forge/shamhub/merge_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand All @@ -337,9 +353,6 @@ func (sh *ShamHub) prepareMergeRange(
)
}

if i == 0 {
prepared.rootBaseHash = baseHash
}
prepared.changes[i] = preparedMergeRangeChange{
index: changeIndex,
change: change,
Expand All @@ -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()
Expand All @@ -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(
Expand Down
47 changes: 47 additions & 0 deletions internal/forge/shamhub/merge_range_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 23 additions & 25 deletions internal/handler/merge/merge_range_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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{
Expand Down Expand Up @@ -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}
Expand All @@ -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)
Expand Down Expand Up @@ -294,26 +296,20 @@ 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,
gitRepo: mockGit,
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{})
Expand Down Expand Up @@ -461,23 +457,25 @@ 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.
mockForge := forgetest.NewMockRepository(ctrl)
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).
Expand Down
12 changes: 7 additions & 5 deletions internal/handler/merge/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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}
}
Expand Down
56 changes: 56 additions & 0 deletions internal/handler/merge/workspace_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
63 changes: 63 additions & 0 deletions testdata/script/downstack_merge_native_range_advanced_trunk.txt
Original file line number Diff line number Diff line change
@@ -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 <test@example.com>'
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
Loading