github: Rebuild native stacks after merged prefixes

internal/forge/github/integration_test.go

@@ -300,8 +300,11 @@ func TestIntegration_stackReplacementWithMergedMember(t *testing.T) {
 	bottomBranch := fixturetest.New(_fixtures, "bottomBranch", func() string {
 		return "stack-merged-bottom-" + randomString(8)
 	}).Get(t)
-	insertedBranch := fixturetest.New(_fixtures, "insertedBranch", func() string {
-		return "stack-merged-inserted-" + randomString(8)
+	activeBranch := fixturetest.New(_fixtures, "activeBranch", func() string {
+		return "stack-merged-active-" + randomString(8)
+	}).Get(t)
+	removedBranch := fixturetest.New(_fixtures, "removedBranch", func() string {
+		return "stack-merged-removed-" + randomString(8)
 	}).Get(t)
 	topBranch := fixturetest.New(_fixtures, "topBranch", func() string {
 		return "stack-merged-top-" + randomString(8)
@@ -310,25 +313,28 @@ func TestIntegration_stackReplacementWithMergedMember(t *testing.T) {
 	if forgetest.Update() {
 		builder := forgetest.NewRepositoryBuilder(t, remoteURL)
 		t.Cleanup(func() {
-			for _, branch := range []string{topBranch, insertedBranch, bottomBranch} {
+			for _, branch := range []string{topBranch, removedBranch, activeBranch, bottomBranch} {
 				builder.DeleteRemoteBranch(branch)
 			}
 		})
 
-		// Create one linear ancestry.
-		// The initial native stack omits inserted.
-		// After bottom merges, adding inserted would require replacing a stack
-		// whose historical bottom member cannot be removed.
+		// The initial native stack follows this branch ancestry:
 		//
 		//     main
 		//       |
 		//     bottom (merged before replacement)
 		//       |
-		//    inserted
+		//     active
+		//       |
+		//     removed (omitted from the desired stack)
 		//       |
 		//      top
+		//
+		// After bottom merges, reconcile active -> top.
+		// The removed PR must stay open and keep its base;
+		// only its native membership is removed.
 		builder.CheckoutBranch("main")
-		for _, branch := range []string{bottomBranch, insertedBranch, topBranch} {
+		for _, branch := range []string{bottomBranch, activeBranch, removedBranch, topBranch} {
 			builder.CreateBranch(branch)
 			builder.CheckoutBranch(branch)
 			builder.WriteFile(branch+".txt", "commit for "+branch)
@@ -373,26 +379,36 @@ func TestIntegration_stackReplacementWithMergedMember(t *testing.T) {
 	})
 	require.NoError(t, err, "create bottom change")
 	openChanges = append(openChanges, bottom.ID)
-	inserted, err := repo.SubmitChange(t.Context(), forge.SubmitChangeRequest{
-		Subject: "Native stack merged-member middle " + insertedBranch,
+	active, err := repo.SubmitChange(t.Context(), forge.SubmitChangeRequest{
+		Subject: "Native stack merged-member active " + activeBranch,
 		Body:    "Native stack merged-member integration test",
 		Base:    bottomBranch,
-		Head:    insertedBranch,
+		Head:    activeBranch,
 	})
-	require.NoError(t, err, "create inserted change")
-	openChanges = append(openChanges, inserted.ID)
+	require.NoError(t, err, "create active change")
+	openChanges = append(openChanges, active.ID)
+	removed, err := repo.SubmitChange(t.Context(), forge.SubmitChangeRequest{
+		Subject: "Native stack merged-member removed " + removedBranch,
+		Body:    "Native stack merged-member integration test",
+		Base:    activeBranch,
+		Head:    removedBranch,
+	})
+	require.NoError(t, err, "create removed change")
+	openChanges = append(openChanges, removed.ID)
 	top, err := repo.SubmitChange(t.Context(), forge.SubmitChangeRequest{
 		Subject: "Native stack merged-member top " + topBranch,
 		Body:    "Native stack merged-member integration test",
-		Base:    bottomBranch,
+		Base:    removedBranch,
 		Head:    topBranch,
 	})
 	require.NoError(t, err, "create top change")
 	openChanges = append(openChanges, top.ID)
 
 	plan, err := repo.PlanStackUpdate(t.Context(), []forge.StackChange{
 		{Change: bottom.ID, BaseBranch: "main"},
-		{Change: top.ID, BaseChange: bottom.ID, BaseBranch: bottomBranch},
+		{Change: active.ID, BaseChange: bottom.ID, BaseBranch: bottomBranch},
+		{Change: removed.ID, BaseChange: active.ID, BaseBranch: activeBranch},
+		{Change: top.ID, BaseChange: removed.ID, BaseBranch: removedBranch},
 	})
 	require.NoError(t, err, "plan initial native stack")
 	require.NoError(t, plan.Execute(t.Context()), "create initial native stack")
@@ -420,35 +436,44 @@ func TestIntegration_stackReplacementWithMergedMember(t *testing.T) {
 	openChanges = openChanges[1:]
 
 	desired := []forge.StackChange{
-		{Change: bottom.ID, BaseBranch: "main"},
-		{Change: inserted.ID, BaseChange: bottom.ID, BaseBranch: bottomBranch},
-		{Change: top.ID, BaseChange: inserted.ID, BaseBranch: insertedBranch},
+		{Change: active.ID, BaseBranch: "main"},
+		{Change: top.ID, BaseChange: active.ID, BaseBranch: activeBranch},
 	}
 	plan, err = repo.PlanStackUpdate(t.Context(), desired)
-	require.NoError(t, err, "plan replacement around merged member")
-	require.NoError(t, plan.Execute(t.Context()), "leave merged-member stack unchanged")
+	require.NoError(t, err, "plan removal after merged prefix")
+	require.NoError(t, plan.Execute(t.Context()), "replace the active stack")
 
 	pullRequests, err := gatewayClient.PullRequestsForStackUpdate(
 		t.Context(),
 		cfg.Owner,
 		cfg.Repo,
 		[]int{
 			bottom.ID.(*github.PR).Number,
-			inserted.ID.(*github.PR).Number,
+			active.ID.(*github.PR).Number,
+			removed.ID.(*github.PR).Number,
 			top.ID.(*github.PR).Number,
 		},
 	)
-	require.NoError(t, err, "read native stack after omitted replacement")
-	require.Len(t, pullRequests, 3)
-	require.NotNil(t, pullRequests[1])
-	assert.Nil(t, pullRequests[1].Stack)
+	require.NoError(t, err, "read native stack after removal")
+	require.Len(t, pullRequests, 4)
+	require.NotNil(t, pullRequests[0])
+	assert.Equal(t, githubgateway.PullRequestStateMerged, pullRequests[0].State)
 	require.NotNil(t, pullRequests[2])
-	require.NotNil(t, pullRequests[2].Stack)
-	assert.NotEqual(t, insertedBranch, pullRequests[2].BaseRefName)
-	assert.Equal(t, []githubgateway.PullRequestStackMember{
-		{Number: bottom.ID.(*github.PR).Number, State: githubgateway.PullRequestStateMerged},
-		{Number: top.ID.(*github.PR).Number, State: githubgateway.PullRequestStateOpen},
-	}, pullRequests[2].Stack.Members)
+	assert.Equal(t, githubgateway.PullRequestStateOpen, pullRequests[2].State)
+	assert.Nil(t, pullRequests[2].Stack, "removed PR remains open but unstacked")
+	assert.Equal(t, activeBranch, pullRequests[2].BaseRefName)
+
+	for _, pr := range []*githubgateway.StackUpdatePullRequest{pullRequests[1], pullRequests[3]} {
+		require.NotNil(t, pr)
+		require.NotNil(t, pr.Stack)
+		assert.Equal(t, []githubgateway.PullRequestStackMember{
+			{Number: active.ID.(*github.PR).Number, State: githubgateway.PullRequestStateOpen},
+			{Number: top.ID.(*github.PR).Number, State: githubgateway.PullRequestStateOpen},
+		}, pr.Stack.Members)
+	}
+	assert.Equal(t, pullRequests[1].Stack.Number, pullRequests[3].Stack.Number)
+	assert.Equal(t, "main", pullRequests[1].BaseRefName)
+	assert.Equal(t, activeBranch, pullRequests[3].BaseRefName)
 }
 
 func TestIntegration_DivergentStackMerge(t *testing.T) {

internal/forge/github/stack_program.go

@@ -22,11 +22,17 @@ type githubStackTry struct {
 
 func (githubStackTry) isGitHubStackInstruction() {}
 
-// githubStackUnstack requires complete dissolution of an existing native stack.
-// Retained members count as failure even when the API call succeeds.
+// githubStackUnstack releases an existing stack's unmerged members.
+// Historical merged members may remain in the old resource;
+// retaining any other member blocks the dependent replacement.
 type githubStackUnstack struct {
 	// StackNumber identifies the existing stack within the repository.
 	StackNumber int
+
+	// MergedPullRequests lists members already merged when the plan was built.
+	// Their retained membership cannot prevent retargeting or restacking
+	// the released open PRs.
+	MergedPullRequests []int
 }
 
 func (githubStackUnstack) isGitHubStackInstruction() {}

internal/forge/github/stack_program_compile.go

@@ -2,7 +2,6 @@ package github
 
 import (
 	"fmt"
-	"slices"
 
 	"go.abhg.dev/gs/internal/gateway/github"
 )
@@ -46,7 +45,7 @@ func (c *githubStackCompiler) try(body func()) {
 
 // selection emits an append or new membership for the selected paths.
 // Incompatible existing membership is dissolved first;
-// if GitHub would retain members, it emits only a warning.
+// queued or auto-merge members prevent replacement and produce a warning.
 func (c *githubStackCompiler) selection(selection githubStackSelection) {
 	// GitHub can grow a native stack at its top, but cannot insert, remove,
 	// or reorder individual members in place.
@@ -64,20 +63,24 @@ func (c *githubStackCompiler) selection(selection githubStackSelection) {
 	}
 
 	if remoteStack := selection.remoteStack; remoteStack != nil {
-		// GitHub retains merged and locked members when unstacking.
-		// A replacement must dissolve the whole object before any base
-		// or new membership write can use the desired topology.
-		if slices.ContainsFunc(remoteStack.Members, func(member github.PullRequestStackMember) bool {
-			return member.Locked || member.State == github.PullRequestStateMerged
-		}) {
-			c.warnings = append(c.warnings, fmt.Sprintf(
-				"#%d: Leaving pull request and its upstack in existing GitHub native stack #%d: merged, queued, or auto-merge pull requests prevent restructuring",
-				selection.paths[0][0].number,
-				remoteStack.Number,
-			))
-			return
+		// Unstack can leave historical merged members in the old resource.
+		// Replacement needs the open PRs released, not that history erased.
+		// Record the merged IDs so execution can distinguish harmless history
+		// from members that GitHub could not release.
+		unstack := githubStackUnstack{StackNumber: remoteStack.Number}
+		for _, member := range remoteStack.Members {
+			if member.State == github.PullRequestStateMerged {
+				unstack.MergedPullRequests = append(unstack.MergedPullRequests, member.Number)
+			} else if member.Locked {
+				c.warnings = append(c.warnings, fmt.Sprintf(
+					"#%d: Leaving pull request and its upstack in existing GitHub native stack #%d: queued or auto-merge pull requests prevent restructuring",
+					selection.paths[0][0].number,
+					remoteStack.Number,
+				))
+				return
+			}
 		}
-		c.instructions = append(c.instructions, githubStackUnstack{StackNumber: remoteStack.Number})
+		c.instructions = append(c.instructions, unstack)
 	}
 
 	// Unstack shares the selection's scope, so its failure skips all paths.

internal/forge/github/stack_program_execute.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"slices"
 
 	"go.abhg.dev/gs/internal/gateway/github"
 )
@@ -81,17 +82,22 @@ func (e *githubStackExecutor) unstack(ctx context.Context, instruction githubSta
 	if err != nil {
 		return fmt.Errorf("unstack GitHub native stack #%d: %w", instruction.StackNumber, err)
 	}
-	// GitHub can successfully unstack only part of a resource:
-	// it releases eligible members but retains merged or locked members.
-	// The compiled replacement assumes complete dissolution, not an arbitrary
-	// released subset, so API success alone does not satisfy its prerequisite.
-	// Report retained membership as failure before dependent writes can run;
-	// any members already released remain released.
-	if len(result.RemainingPullRequests) > 0 {
+	// Only the released PRs participate in the replacement.
+	// Historical members known to be merged cannot become open again,
+	// so their remaining in the old resource does not invalidate the plan.
+	// Any other remainder may still block a base or membership write;
+	// stop before dependent instructions run, even though some PRs were released.
+	var retained []int
+	for _, number := range result.RemainingPullRequests {
+		if !slices.Contains(instruction.MergedPullRequests, number) {
+			retained = append(retained, number)
+		}
+	}
+	if len(retained) > 0 {
 		return fmt.Errorf(
 			"unstack GitHub native stack #%d: GitHub retained pull requests %v",
 			instruction.StackNumber,
-			result.RemainingPullRequests,
+			retained,
 		)
 	}
 	return nil

internal/forge/github/stack_program_execute_test.go

@@ -90,10 +90,10 @@ func TestExecuteGitHubStackProgram_retainedMembersBlockReplacement(t *testing.T)
 		&github.UnstackPullRequestStackInput{
 			Owner: "acme", Repo: "repo", StackNumber: 42,
 		},
-	).Return(&github.UnstackPullRequestStackResult{RemainingPullRequests: []int{1}}, nil)
+	).Return(&github.UnstackPullRequestStackResult{RemainingPullRequests: []int{1, 2}}, nil)
 	program := githubStackProgram{
 		githubStackTry{Body: []githubStackInstruction{
-			githubStackUnstack{StackNumber: 42},
+			githubStackUnstack{StackNumber: 42, MergedPullRequests: []int{1}},
 			githubStackTry{Body: []githubStackInstruction{
 				githubStackChangeBase{PullRequestNumber: 2, PullRequestID: "PR_2", BaseBranch: "main"},
 				githubStackCreate{PullRequests: []int{2, 3}},
@@ -108,7 +108,7 @@ func TestExecuteGitHubStackProgram_retainedMembersBlockReplacement(t *testing.T)
 	}
 
 	err := executeGitHubStackProgram(t.Context(), repository, program)
-	require.EqualError(t, err, "unstack GitHub native stack #42: GitHub retained pull requests [1]")
+	require.EqualError(t, err, "unstack GitHub native stack #42: GitHub retained pull requests [2]")
 }
 
 func TestExecuteGitHubStackProgram_nestedFailuresDoNotEscapeTry(t *testing.T) {

internal/forge/github/stack_reconciliation_property_test.go

@@ -88,6 +88,12 @@ func TestReconcileGitHubStacks_validTransitions(t *testing.T) {
 			}
 		}
 		for number, pr := range model.pullRequests {
+			// Replacing an active suffix must not rewrite its merged history,
+			// even when those historical PRs also appear in the desired forest.
+			if pr.state == github.PullRequestStateMerged {
+				assert.Equal(t, beforeBases[number], pr.base, "changed merged PR #%d base", number)
+				continue
+			}
 			if pr.base == beforeBases[number] {
 				continue
 			}
@@ -140,6 +146,36 @@ func TestStackResourceModel_restructure(t *testing.T) {
 	}))
 }
 
+func TestStackResourceModel_restructureAfterMergedPrefix(t *testing.T) {
+	model := &stackResourceModel{
+		pullRequests: map[int]*stackResourceModelPR{
+			1: {head: "a", base: "main", state: github.PullRequestStateMerged, headInRepository: true},
+			2: {head: "b", base: "main", state: github.PullRequestStateOpen, headInRepository: true},
+			3: {head: "c", base: "b", state: github.PullRequestStateOpen, headInRepository: true},
+			4: {head: "d", base: "c", state: github.PullRequestStateOpen, headInRepository: true},
+		},
+		stacks:    map[int][]int{42: {1, 2, 3, 4}},
+		nextStack: 42,
+	}
+	result, err := model.UnstackPullRequestStack(t.Context(), &github.UnstackPullRequestStackInput{
+		Owner: "acme", Repo: "repo", StackNumber: 42,
+	})
+	require.NoError(t, err)
+	assert.Equal(t, []int{1}, result.RemainingPullRequests)
+	assert.Equal(t, []int{1}, model.stacks[42])
+
+	// Historical membership on #1 does not prevent recreating the released
+	// active suffix without #3. Removing #3 leaves its PR and base unchanged.
+	require.NoError(t, model.UpdatePullRequest(t.Context(), &github.UpdatePullRequestInput{
+		PullRequestID: "4", BaseRefName: new("b"),
+	}))
+	require.NoError(t, model.CreatePullRequestStack(t.Context(), &github.CreatePullRequestStackInput{
+		Owner: "acme", Repo: "repo", PullRequests: []int{2, 4},
+	}))
+	assert.Equal(t, map[int][]int{42: {1}, 43: {2, 4}}, model.stacks)
+	assert.Equal(t, "b", model.pullRequests[3].base)
+}
+
 // stackResourceModel models GitHub's native stack operations.
 // It excludes Git commits, transport, and path selection.
 // It enforces GitHub's observed write restrictions