addressing review comments

Author: skarim

cmd/merge.go

@@ -63,8 +63,8 @@ prompting, using your last-used merge method unless one is specified.
 
 Only basic pull request state is checked before merging (open and not a draft);
 GitHub evaluates branch protection and repository rules when the merge runs, so
-any such failure is reported back to you. Bypassing merge requirements with admin
-privileges is not supported for stacks.
+any such failure is reported back to you. Bypassing merge requirements is not
+supported for stacks.
 
 If the base branch uses a merge queue, the stack is added to the queue and merges
 once the queue processes it; otherwise it is merged directly.`,
@@ -151,6 +151,15 @@ func runMerge(cfg *config.Config, opts *mergeOptions, args []string) error {
 	// Non-interactive (or --yes): merge the whole stack (or up to the given PR)
 	// without prompting.
 	if !target.hasPR {
+		// A draft or closed pull request partway up the stack blocks everything
+		// above it. Rather than silently merging only the portion below it,
+		// refuse and let the user target an explicit pull request.
+		if blocker != nil {
+			top := candidates[len(candidates)-1].Number
+			cfg.Errorf("cannot merge the whole stack: pull request #%d is %s", blocker.Number, blockerState(blocker))
+			cfg.Printf("Merge up to #%d with `%s`", top, cfg.ColorCyan(fmt.Sprintf("gh stack merge %d", top)))
+			return ErrInvalidArgs
+		}
 		targetPR = candidates[len(candidates)-1].Number
 	}
 	if method == "" {

cmd/merge_test.go

@@ -272,6 +272,32 @@ func TestRunMerge_AlreadyMergedTarget(t *testing.T) {
 	assert.Contains(t, output, "#1 is already merged")
 }
 
+func TestRunMerge_WholeStackBlockedByDraft(t *testing.T) {
+	submitCalled := false
+	cfg, outR, errR := config.NewTestConfig()
+	cfg.GitHubClientOverride = &github.MockClient{
+		GetStackFn: func(n int) (*github.RemoteStack, error) {
+			return remoteStack(5, "main", openStackPR(1, "b1"), draftStackPR(2, "b2"), openStackPR(3, "b3")), nil
+		},
+		RepoMergeConfigFn: func() (*github.RepoMergeConfig, error) {
+			return &github.RepoMergeConfig{MergeAllowed: true, DefaultMethod: "merge"}, nil
+		},
+		MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
+			submitCalled = true
+			return nil, nil
+		},
+	}
+
+	err := runMerge(cfg, fastOptions(), []string{"5"})
+	output := collectOutput(cfg, outR, errR)
+
+	assert.ErrorIs(t, err, ErrInvalidArgs)
+	assert.Contains(t, output, "cannot merge the whole stack")
+	assert.Contains(t, output, "#2 is a draft")
+	assert.Contains(t, output, "gh stack merge 1")
+	assert.False(t, submitCalled, "must not silently merge only the portion below the blocker")
+}
+
 func TestRunMerge_NothingToMerge_AllMerged(t *testing.T) {
 	setupLocalStack(t, 100, "b1", "b1", "b2")
 	cfg, outR, errR := config.NewTestConfig()

docs/src/content/docs/guides/workflows.md

@@ -145,8 +145,8 @@ In an interactive terminal, a short wizard lets you choose how far up the stack
 
 If the base branch uses a merge queue, `gh stack merge` adds the stack to the queue instead of merging directly — it merges once the queue processes it.
 
-:::note[Admin bypass not supported]
-Stack merges currently do not support admin bypass merging.
+:::note[Bypassing merge requirements not supported]
+Stack merges do not support bypassing merge requirements.
 :::
 
 ## Syncing After Merges

internal/tui/mergeview/model.go

@@ -181,10 +181,14 @@ const maxVisibleItems = 10
 
 // visibleItems is the number of pull requests shown in the select window at
 // once — capped at maxVisibleItems and shrunk to fit a short terminal (each
-// item renders on two lines). When the terminal size is unknown, all are shown.
+// item renders on two lines). When the terminal size is unknown, it still caps
+// at maxVisibleItems so the first frame can't overflow a large stack.
 func (m Model) visibleItems() int {
 	n := len(m.opts.PRs)
 	if m.height <= 0 {
+		if n > maxVisibleItems {
+			return maxVisibleItems
+		}
 		return n
 	}
 	// Reserve lines for the header, scroll indicators, summary, and footer.

internal/tui/mergeview/model_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	tea "github.com/charmbracelet/bubbletea"
+	"github.com/charmbracelet/lipgloss"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -91,8 +92,9 @@ func TestSelect_Viewport(t *testing.T) {
 	}
 	m := New(opts)
 
-	// No size yet: all items are shown.
-	assert.Equal(t, 30, m.visibleItems())
+	// No size yet: capped at maxVisibleItems so a large stack can't overflow
+	// the first frame.
+	assert.Equal(t, 10, m.visibleItems())
 
 	// A tall terminal caps the window at maxVisibleItems (10).
 	nm, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 60})
@@ -137,6 +139,21 @@ func TestSelect_ArrowDirection(t *testing.T) {
 	assert.Equal(t, 2, m.cursor)
 }
 
+func TestTruncate_WideRunes(t *testing.T) {
+	// ASCII truncates to the requested width with a trailing ellipsis (plus an
+	// ANSI reset, which has zero display width).
+	ascii := truncate("abcdef", 3)
+	assert.True(t, strings.HasPrefix(ascii, "ab…"))
+	assert.LessOrEqual(t, lipgloss.Width(ascii), 3)
+
+	// Double-width runes must not push the result past the requested display
+	// width (each CJK rune is two cells).
+	assert.LessOrEqual(t, lipgloss.Width(truncate("你好世界", 5)), 5)
+
+	// A string that already fits is returned unchanged.
+	assert.Equal(t, "hi", truncate("hi", 5))
+}
+
 func TestSelect_AdvanceRequiresSelection(t *testing.T) {
 	m := New(baseOptions())
 

internal/tui/mergeview/view.go

@@ -385,13 +385,14 @@ func truncate(s string, width int) string {
 			}
 			continue
 		}
-		if w >= width-1 {
+		rw := lipgloss.Width(string(r))
+		if w+rw > width-1 {
 			b.WriteString("…")
 			b.WriteString("\x1b[0m")
 			break
 		}
 		b.WriteRune(r)
-		w++
+		w += rw
 	}
 	return b.String()
 }