set merge action explicitly

Author: skarim

.github/copilot-instructions.md

@@ -17,7 +17,7 @@ No Makefile, no code generation, no external linter config. Standard Go toolchai
 
 - `cmd/`: One Cobra command per file. Each exports `<Name>Cmd(cfg *config.Config)` with logic in `run<Name>()`.
 - `internal/git/`: `Ops` interface (52 methods) wrapping git CLI. `MockOps` for tests. Package-level functions delegate to swappable `ops` variable.
-- `internal/github/`: `ClientOps` interface (18 methods) for GitHub API. `MockClient` for tests. Stack operations use the public Stacks REST API (`/repos/{owner}/{repo}/stacks`); merges use the async merge API (`/repos/{owner}/{repo}/pulls/{n}/merge-async`), which routes to a direct merge or the base branch's merge queue automatically.
+- `internal/github/`: `ClientOps` interface (18 methods) for GitHub API. `MockClient` for tests. Stack operations use the public Stacks REST API (`/repos/{owner}/{repo}/stacks`); merges use the async merge API (`/repos/{owner}/{repo}/pulls/{n}/merge-async`) with an explicit `merge_action` (`direct_merge` or `merge_queue`) selected from the base branch's merge-queue detection.
 - `internal/config/`: `Config` struct passed to all commands. Holds I/O, colors, and test hooks (`SelectFn`, `ConfirmFn`, `InputFn`, `GitHubClientOverride`).
 - `internal/stack/`: Stack file (`.git/gh-stack`, JSON) management with file locking.
 - `internal/tui/`: bubbletea views (`stackview`, `modifyview`).

AGENTS.md

@@ -109,7 +109,7 @@ if errors.As(err, &exitErr) { ... }
 ### Key interfaces
 
 - **`git.Ops`** (`internal/git/gitops.go`): 52 methods wrapping git CLI calls. The production implementation uses `cli/go-gh`'s `client.Command()` via `run()` and `runSilent()` helpers. Package-level functions (e.g., `git.CurrentBranch()`) delegate to a swappable package-level `ops` variable.
-- **`github.ClientOps`** (`internal/github/client_interface.go`): 17 methods for GitHub API (PRs, stacks, merges). Stack operations use the public Stacks REST API (`/repos/{owner}/{repo}/stacks`): `ListStacks`, `FindStackForPR`, `GetStack`, `CreateStack`, `AddToStack` (delta append), `Unstack`. Async stack merges use `RepoMergeConfig` (GraphQL: allowed merge methods + viewer's default), `MergeStackAsync`, and `GetAsyncMergeResult` (`/repos/{owner}/{repo}/pulls/{n}/merge-async`). Injected via `cfg.GitHubClientOverride` in tests.
+- **`github.ClientOps`** (`internal/github/client_interface.go`): 18 methods for GitHub API (PRs, stacks, merges). Stack operations use the public Stacks REST API (`/repos/{owner}/{repo}/stacks`): `ListStacks`, `FindStackForPR`, `GetStack`, `CreateStack`, `AddToStack` (delta append), `Unstack`. Async stack merges use `RepoMergeConfig` (GraphQL: allowed merge methods + viewer's default), `BaseBranchUsesMergeQueue` (GraphQL: detects a base-branch merge queue to select the explicit `merge_action`), `MergeStackAsync`, and `GetAsyncMergeResult` (`/repos/{owner}/{repo}/pulls/{n}/merge-async`). Injected via `cfg.GitHubClientOverride` in tests.
 - **`config.Config`** (`internal/config/config.go`): Central configuration passed to all commands. Holds I/O streams, color functions, and test hook fields (`SelectFn`, `ConfirmFn`, `InputFn`, `RepoOverride`).
 
 ### Stack file

cmd/merge.go

@@ -313,7 +313,7 @@ func runMergeInteractive(cfg *config.Config, client github.ClientOps, stackNumbe
 		}
 	}
 
-	submit, poll := mergeFuncs(client)
+	submit, poll := mergeFuncs(client, mergeActionFor(usesMergeQueue))
 
 	model := mergeview.New(mergeview.Options{
 		PRs:               candidates,
@@ -373,7 +373,7 @@ func runMergeHeadless(cfg *config.Config, client github.ClientOps, base string,
 		cfg.Printf("Merging %s into %s via %s...", list, base, method)
 	}
 
-	res, err := client.MergeStackAsync(targetPR, method)
+	res, err := client.MergeStackAsync(targetPR, method, mergeActionFor(usesMergeQueue))
 	if err != nil {
 		if errors.Is(err, github.ErrAsyncMergeUnavailable) {
 			warnAsyncMergeUnavailable(cfg)
@@ -439,10 +439,10 @@ func runMergeHeadless(cfg *config.Config, client github.ClientOps, base string,
 }
 
 // baseBranchUsesMergeQueue reports whether the stack's base branch merges through
-// a merge queue. Detection only tailors the wizard (skipping the method step and
-// switching to enqueue wording), so a lookup failure falls back to the
-// direct-merge flow: the async merge always sends merge_action "default" and the
-// server still routes to the queue when the branch requires one.
+// a merge queue. Detection tailors the wizard (skipping the method step and
+// switching to enqueue wording) and selects the explicit merge_action. On a
+// lookup failure it falls back to the direct-merge flow (method step shown,
+// "direct_merge" sent), matching what the user is shown.
 func baseBranchUsesMergeQueue(client github.ClientOps, base string) bool {
 	uses, err := client.BaseBranchUsesMergeQueue(base)
 	if err != nil {
@@ -451,11 +451,23 @@ func baseBranchUsesMergeQueue(client github.ClientOps, base string) bool {
 	return uses
 }
 
+// mergeActionFor maps merge-queue detection to the explicit async-merge action: a
+// detected queue forces "merge_queue" (the server rejects it when the branch has
+// no queue, guarding against a wrong guess), and everything else forces a
+// "direct_merge". Being explicit ensures the merge matches the wizard the user
+// saw rather than letting the server choose the routing.
+func mergeActionFor(usesMergeQueue bool) string {
+	if usesMergeQueue {
+		return github.MergeActionMergeQueue
+	}
+	return github.MergeActionDirectMerge
+}
+
 // mergeFuncs returns submit/poll closures that adapt the GitHub client to the
-// mergeview injection points.
-func mergeFuncs(client github.ClientOps) (mergeview.SubmitFunc, mergeview.PollFunc) {
+// mergeview injection points. The merge action is fixed for the session.
+func mergeFuncs(client github.ClientOps, mergeAction string) (mergeview.SubmitFunc, mergeview.PollFunc) {
 	submit := func(targetPR int, method string) (mergeview.MergeStatus, error) {
-		res, err := client.MergeStackAsync(targetPR, method)
+		res, err := client.MergeStackAsync(targetPR, method, mergeAction)
 		if err != nil {
 			return mergeview.MergeStatus{}, err
 		}

cmd/merge_test.go

@@ -91,7 +91,7 @@ func TestRunMerge_NoArg_MergesWholeStack(t *testing.T) {
 		RepoMergeConfigFn: func() (*github.RepoMergeConfig, error) {
 			return &github.RepoMergeConfig{MergeAllowed: true, SquashAllowed: true, RebaseAllowed: true, DefaultMethod: "squash"}, nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
 			gotPR, gotMethod = pr, method
 			return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
 		},
@@ -111,14 +111,15 @@ func TestRunMerge_NoArg_MergesWholeStack(t *testing.T) {
 
 func TestRunMerge_StackNumberArg(t *testing.T) {
 	var gotPR int
+	gotAction := "unset"
 	cfg, outR, errR := config.NewTestConfig()
 	cfg.GitHubClientOverride = &github.MockClient{
 		GetStackFn: func(n int) (*github.RemoteStack, error) {
 			assert.Equal(t, 7, n)
 			return remoteStack(7, "main", openStackPR(10, "a"), openStackPR(11, "b")), nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
-			gotPR = pr
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
+			gotPR, gotAction = pr, mergeAction
 			return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
 		},
 	}
@@ -128,11 +129,12 @@ func TestRunMerge_StackNumberArg(t *testing.T) {
 
 	require.NoError(t, err)
 	assert.Equal(t, 11, gotPR)
+	assert.Equal(t, github.MergeActionDirectMerge, gotAction, "a non-queue base sends an explicit direct_merge action")
 	assert.Contains(t, output, "Merged #10, #11 into main")
 }
 
 func TestRunMerge_MergeQueue_Headless(t *testing.T) {
-	gotMethod := "unset"
+	gotMethod, gotAction := "unset", "unset"
 	cfg, outR, errR := config.NewTestConfig()
 	cfg.GitHubClientOverride = &github.MockClient{
 		GetStackFn: func(n int) (*github.RemoteStack, error) {
@@ -142,8 +144,8 @@ func TestRunMerge_MergeQueue_Headless(t *testing.T) {
 			assert.Equal(t, "main", base)
 			return true, nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
-			gotMethod = method
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
+			gotMethod, gotAction = method, mergeAction
 			return &github.AsyncMergeResult{
 				Status:  github.AsyncMergeStatusEnqueued,
 				Details: github.AsyncMergeDetails{Message: "Pull request was added to the merge queue."},
@@ -156,19 +158,20 @@ func TestRunMerge_MergeQueue_Headless(t *testing.T) {
 
 	require.NoError(t, err)
 	assert.Equal(t, "", gotMethod, "a merge queue picks the method; none is sent")
+	assert.Equal(t, github.MergeActionMergeQueue, gotAction, "an explicit merge_queue action is sent")
 	assert.Contains(t, output, "merge queue")
 }
 
 func TestRunMerge_MergeQueue_IgnoresMethodFlag(t *testing.T) {
-	gotMethod := "unset"
+	gotMethod, gotAction := "unset", "unset"
 	cfg, outR, errR := config.NewTestConfig()
 	cfg.GitHubClientOverride = &github.MockClient{
 		GetStackFn: func(n int) (*github.RemoteStack, error) {
 			return remoteStack(7, "main", openStackPR(10, "a"), openStackPR(11, "b")), nil
 		},
 		BaseBranchUsesMergeQueueFn: func(base string) (bool, error) { return true, nil },
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
-			gotMethod = method
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
+			gotMethod, gotAction = method, mergeAction
 			return &github.AsyncMergeResult{
 				Status:  github.AsyncMergeStatusEnqueued,
 				Details: github.AsyncMergeDetails{Message: "queued"},
@@ -183,11 +186,12 @@ func TestRunMerge_MergeQueue_IgnoresMethodFlag(t *testing.T) {
 
 	require.NoError(t, err)
 	assert.Equal(t, "", gotMethod, "the requested method is ignored under a merge queue")
+	assert.Equal(t, github.MergeActionMergeQueue, gotAction)
 	assert.Contains(t, output, "ignoring the merge method")
 }
 
 func TestRunMerge_MergeQueueDetectionError_FallsBackToDirect(t *testing.T) {
-	gotMethod := "unset"
+	gotMethod, gotAction := "unset", "unset"
 	cfg, outR, errR := config.NewTestConfig()
 	cfg.GitHubClientOverride = &github.MockClient{
 		GetStackFn: func(n int) (*github.RemoteStack, error) {
@@ -199,8 +203,8 @@ func TestRunMerge_MergeQueueDetectionError_FallsBackToDirect(t *testing.T) {
 		RepoMergeConfigFn: func() (*github.RepoMergeConfig, error) {
 			return &github.RepoMergeConfig{MergeAllowed: true, DefaultMethod: "merge"}, nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
-			gotMethod = method
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
+			gotMethod, gotAction = method, mergeAction
 			return &github.AsyncMergeResult{
 				Status:  github.AsyncMergeStatusMerged,
 				Details: github.AsyncMergeDetails{SHA: "abc1234"},
@@ -213,6 +217,7 @@ func TestRunMerge_MergeQueueDetectionError_FallsBackToDirect(t *testing.T) {
 
 	require.NoError(t, err)
 	assert.Equal(t, "merge", gotMethod, "detection failure falls back to a direct merge with a method")
+	assert.Equal(t, github.MergeActionDirectMerge, gotAction, "the direct-merge UI sends an explicit direct_merge action")
 	assert.Contains(t, output, "Merged")
 }
 
@@ -227,7 +232,7 @@ func TestRunMerge_PRNumberArg(t *testing.T) {
 			assert.Equal(t, 2, n)
 			return remoteStack(5, "main", openStackPR(1, "b1"), openStackPR(2, "b2"), openStackPR(3, "b3")), nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
 			gotPR = pr
 			return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
 		},
@@ -249,7 +254,7 @@ func TestRunMerge_SquashFlag(t *testing.T) {
 		GetStackFn: func(n int) (*github.RemoteStack, error) {
 			return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
 			gotMethod = method
 			return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
 		},
@@ -367,7 +372,7 @@ func TestRunMerge_WholeStackBlockedByDraft(t *testing.T) {
 		RepoMergeConfigFn: func() (*github.RepoMergeConfig, error) {
 			return &github.RepoMergeConfig{MergeAllowed: true, DefaultMethod: "merge"}, nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
 			submitCalled = true
 			return nil, nil
 		},
@@ -405,7 +410,7 @@ func TestRunMerge_SubmitNotMergeable(t *testing.T) {
 		GetStackFn: func(n int) (*github.RemoteStack, error) {
 			return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
 			return nil, errors.New("the stack can no longer be merged as requested; refresh and try again")
 		},
 	}
@@ -424,7 +429,7 @@ func TestRunMerge_PollFailedConflict(t *testing.T) {
 		GetStackFn: func(n int) (*github.RemoteStack, error) {
 			return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
 			return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
 		},
 		GetAsyncMergeResultFn: func(pr int, uuid string) (*github.AsyncMergeResult, error) {
@@ -446,7 +451,7 @@ func TestRunMerge_AlreadyMergedOnSubmit(t *testing.T) {
 		GetStackFn: func(n int) (*github.RemoteStack, error) {
 			return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
 			return &github.AsyncMergeResult{Status: github.AsyncMergeStatusMerged, Details: github.AsyncMergeDetails{SHA: "abc"}}, nil
 		},
 	}
@@ -464,7 +469,7 @@ func TestRunMerge_Enqueued(t *testing.T) {
 		GetStackFn: func(n int) (*github.RemoteStack, error) {
 			return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
 			return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
 		},
 		GetAsyncMergeResultFn: func(pr int, uuid string) (*github.AsyncMergeResult, error) {
@@ -485,7 +490,7 @@ func TestRunMerge_EnqueuedOnSubmit(t *testing.T) {
 		GetStackFn: func(n int) (*github.RemoteStack, error) {
 			return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
 			return &github.AsyncMergeResult{Status: github.AsyncMergeStatusEnqueued, Details: github.AsyncMergeDetails{Message: "Pull request was added to the merge queue."}}, nil
 		},
 	}
@@ -503,7 +508,7 @@ func TestRunMerge_AsyncMergeUnavailable(t *testing.T) {
 		GetStackFn: func(n int) (*github.RemoteStack, error) {
 			return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
 			return nil, github.ErrAsyncMergeUnavailable
 		},
 	}
@@ -554,7 +559,7 @@ func TestRunMerge_DefaultMethodFallsBackToAllowed(t *testing.T) {
 			// Viewer default is a method the repo no longer allows.
 			return &github.RepoMergeConfig{SquashAllowed: true, DefaultMethod: "merge"}, nil
 		},
-		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
+		MergeStackAsyncFn: func(pr int, method, mergeAction string) (*github.AsyncMergeResult, error) {
 			gotMethod = method
 			return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
 		},

internal/github/client_interface.go

@@ -18,7 +18,7 @@ type ClientOps interface {
 	AddToStack(stackNumber int, prNumbers []int) (*RemoteStack, error)
 	Unstack(stackNumber int) (*RemoteStack, bool, error)
 	RepoMergeConfig() (*RepoMergeConfig, error)
-	MergeStackAsync(prNumber int, method string) (*AsyncMergeResult, error)
+	MergeStackAsync(prNumber int, method, mergeAction string) (*AsyncMergeResult, error)
 	GetAsyncMergeResult(prNumber int, uuid string) (*AsyncMergeResult, error)
 	PRTitles(numbers []int) (map[int]string, error)
 	BaseBranchUsesMergeQueue(baseRef string) (bool, error)

internal/github/merge_async.go

@@ -19,6 +19,18 @@ const (
 	MergeMethodRebase = "rebase"
 )
 
+// Merge action values for the async merge API's merge_action field. "default"
+// lets the server choose between a direct merge and the base branch's merge
+// queue; "direct_merge" and "merge_queue" force the respective path. Sending an
+// explicit action makes the caller's merge-queue detection authoritative: the
+// server rejects "merge_queue" on a branch with no queue rather than silently
+// merging directly.
+const (
+	MergeActionDefault     = "default"
+	MergeActionDirectMerge = "direct_merge"
+	MergeActionMergeQueue  = "merge_queue"
+)
+
 // ErrAsyncMergeUnavailable indicates the async merge API is not available for
 // the repository (or the token lacks access). Surfaced on a 404 from the submit
 // endpoint.
@@ -196,21 +208,26 @@ func (c *Client) BaseBranchUsesMergeQueue(baseRef string) (bool, error) {
 // a stacked PR this merges all members of the stack up to and including
 // prNumber. A blank method lets the server apply its default.
 //
-// The merge_action is always "default", which lets the server route the stack
-// to a direct merge or the base branch's merge queue automatically, based on
-// the repository's rules and configuration.
+// mergeAction selects the routing: MergeActionDirectMerge or MergeActionMergeQueue
+// force the respective path, and MergeActionDefault (also the fallback for an
+// empty value) lets the server choose. Sending an explicit action makes the
+// caller's merge-queue detection authoritative — the server rejects
+// "merge_queue" on a branch with no queue instead of silently merging directly.
 //
 // On success the returned result is populated for the 200 (already merged) and
 // 202 (enqueued for background processing) responses. A 404 returns
 // ErrAsyncMergeUnavailable, a 409 (a request already exists) returns a clear
 // "already exists" error, and any other non-2xx status is returned as-is.
-func (c *Client) MergeStackAsync(prNumber int, method string) (*AsyncMergeResult, error) {
+func (c *Client) MergeStackAsync(prNumber int, method, mergeAction string) (*AsyncMergeResult, error) {
+	if mergeAction == "" {
+		mergeAction = MergeActionDefault
+	}
 	type reqBody struct {
 		MergeMethod string `json:"merge_method,omitempty"`
 		MergeAction string `json:"merge_action"`
 	}
 
-	body, err := json.Marshal(reqBody{MergeMethod: method, MergeAction: "default"})
+	body, err := json.Marshal(reqBody{MergeMethod: method, MergeAction: mergeAction})
 	if err != nil {
 		return nil, fmt.Errorf("marshaling request: %w", err)
 	}