Fix 7 nit issues from code review

11: Add named constants for phase strings (PhaseApplying, PhaseConflict,
PhasePendingSubmit) in state.go; replace remaining raw literals in
state.go CheckStateGuard.

14: Fix bottomLines comment mismatch — listed 3 items but value is 2.

15: Extract magic number 88 to MinWidthForArt constant in header.go.

16: Remove unused stackview import anchor in model.go — the import
is used via types.go where BranchNode is embedded.

17: Simplify CheckStackLinearity parent resolution — ActiveBaseBranch
already handles skipping merged branches.

18: Fix rename undo matching any rename — add NewName check so only
the specific rename being undone is matched.

20: Add TestUndoRename and TestUndoRename_DoesNotAffectOtherRenames
to validate rename undo behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: skarim

internal/modify/preconditions.go

@@ -38,10 +38,6 @@ func CheckStackLinearity(cfg *config.Config, s *stack.Stack) error {
 		if i == 0 {
 			parentBranch = s.Trunk.Branch
 		} else {
-			parentBranch = s.Branches[i-1].Branch
-		}
-
-		if i > 0 && s.Branches[i-1].IsMerged() {
 			parentBranch = s.ActiveBaseBranch(b.Branch)
 		}
 

internal/modify/state.go

@@ -11,6 +11,12 @@ import (
 
 const stateFileName = "gh-stack-modify-state"
 
+const (
+	PhaseApplying      = "applying"
+	PhaseConflict      = "conflict"
+	PhasePendingSubmit = "pending_submit"
+)
+
 // StateFile holds the state of an in-progress or pending-submit modify operation.
 // It is stored at .git/gh-stack-modify-state.
 type StateFile struct {
@@ -117,10 +123,10 @@ func CheckStateGuard(gitDir string) error {
 	if state == nil {
 		return nil
 	}
-	if state.Phase == "applying" {
+	if state.Phase == PhaseApplying {
 		return fmt.Errorf("a modify session was interrupted — run `gh stack modify --abort` to restore your stack")
 	}
-	if state.Phase == "conflict" {
+	if state.Phase == PhaseConflict {
 		return fmt.Errorf("a modify has unresolved conflicts — run `gh stack modify --continue` or `gh stack modify --abort`")
 	}
 	return nil

internal/stack/stack.go

@@ -317,10 +317,12 @@ func Save(gitDir string, sf *StackFile) error {
 }
 
 // SaveWithLock writes the stack file while the caller already holds the lock.
-// This avoids double-locking when the caller needs to hold the lock across
-// multiple operations (e.g. modify apply).  The staleness check is skipped
-// because the caller owns the lock and has been mutating the StackFile.
-func SaveWithLock(gitDir string, sf *StackFile, _ *FileLock) error {
+// The caller is responsible for acquiring and releasing the lock.
+// Panics if lock is nil to catch programming errors.
+func SaveWithLock(gitDir string, sf *StackFile, lock *FileLock) error {
+	if lock == nil {
+		panic("SaveWithLock called with nil lock")
+	}
 	return writeStackFile(gitDir, sf)
 }
 

internal/tui/modifyview/model.go

@@ -11,7 +11,6 @@ import (
 	"github.com/github/gh-stack/internal/git"
 	"github.com/github/gh-stack/internal/stack"
 	"github.com/github/gh-stack/internal/tui/shared"
-	"github.com/github/gh-stack/internal/tui/stackview"
 )
 
 // modifyKeyMap defines key bindings for the modify view.
@@ -767,7 +766,7 @@ func (m *Model) undoLast() {
 
 	case ActionRename:
 		for i := range m.nodes {
-			if m.nodes[i].Ref.Branch == action.BranchName || (m.nodes[i].PendingAction != nil && m.nodes[i].PendingAction.Type == ActionRename) {
+			if m.nodes[i].Ref.Branch == action.BranchName || (m.nodes[i].PendingAction != nil && m.nodes[i].PendingAction.Type == ActionRename && m.nodes[i].PendingAction.NewName == action.NewName) {
 				m.nodes[i].PendingAction = nil
 				break
 			}
@@ -999,7 +998,7 @@ func (m Model) View() string {
 	// Count fixed bottom lines (always visible, not scrollable).
 	// The bottom section always has 2 lines: one for contextual info
 	// (rename prompt or error, blank when neither) and one for the status bar.
-	bottomLines := 2 // post-scroll newline + context line + status bar
+	bottomLines := 2 // error/status line + status bar (post-scroll newline is inline)
 
 	// Scrolling — reserve space for header and fixed bottom
 	reservedLines := bottomLines
@@ -1090,6 +1089,3 @@ func (m Model) buildHeaderConfig() shared.HeaderConfig {
 
 // Ensure Model satisfies the tea.Model interface.
 var _ tea.Model = Model{}
-
-// Ensure stackview import is used (BranchNode is embedded in ModifyBranchNode).
-var _ = stackview.BranchNode{}

internal/tui/modifyview/model_test.go

@@ -697,3 +697,62 @@ func TestPluralize(t *testing.T) {
 	assert.Equal(t, "drops", pluralize(2, "drop", "drops"))
 	assert.Equal(t, "drops", pluralize(0, "drop", "drops"))
 }
+
+func TestUndoRename(t *testing.T) {
+	nodes := []ModifyBranchNode{
+		makeNode("a", false, 0),
+		makeNode("b", true, 1),
+		makeNode("c", false, 2),
+	}
+	m := New(nodes, testTrunk, "1.0.0")
+
+	// Simulate a rename on node at index 1 (bypass git validation)
+	m.nodes[1].PendingAction = &PendingAction{Type: ActionRename, NewName: "b-renamed"}
+	m.actionStack = append(m.actionStack, StagedAction{
+		Type:         ActionRename,
+		BranchName:   "b",
+		OriginalName: "b",
+		NewName:      "b-renamed",
+	})
+
+	require.NotNil(t, m.nodes[1].PendingAction)
+	assert.Equal(t, ActionRename, m.nodes[1].PendingAction.Type)
+	assert.Equal(t, "b-renamed", m.nodes[1].PendingAction.NewName)
+
+	// Undo
+	m = sendKey(t, m, runeKey('z'))
+	assert.Nil(t, m.nodes[1].PendingAction, "PendingAction should be cleared after undo")
+}
+
+func TestUndoRename_DoesNotAffectOtherRenames(t *testing.T) {
+	nodes := []ModifyBranchNode{
+		makeNode("a", false, 0),
+		makeNode("b", true, 1),
+		makeNode("c", false, 2),
+	}
+	m := New(nodes, testTrunk, "1.0.0")
+
+	// Simulate rename on node 0 (first rename)
+	m.nodes[0].PendingAction = &PendingAction{Type: ActionRename, NewName: "a-renamed"}
+	m.actionStack = append(m.actionStack, StagedAction{
+		Type:         ActionRename,
+		BranchName:   "a",
+		OriginalName: "a",
+		NewName:      "a-renamed",
+	})
+
+	// Simulate rename on node 2 (second rename)
+	m.nodes[2].PendingAction = &PendingAction{Type: ActionRename, NewName: "c-renamed"}
+	m.actionStack = append(m.actionStack, StagedAction{
+		Type:         ActionRename,
+		BranchName:   "c",
+		OriginalName: "c",
+		NewName:      "c-renamed",
+	})
+
+	// Undo the second rename
+	m = sendKey(t, m, runeKey('z'))
+	assert.Nil(t, m.nodes[2].PendingAction, "second rename should be undone")
+	require.NotNil(t, m.nodes[0].PendingAction, "first rename should still be intact")
+	assert.Equal(t, "a-renamed", m.nodes[0].PendingAction.NewName, "first rename new name should be unchanged")
+}

internal/tui/shared/header.go

@@ -18,6 +18,9 @@ const MinWidthForShortcuts = 65
 // MinWidthForHeader is the minimum width to show the header at all.
 const MinWidthForHeader = 50
 
+// MinWidthForArt is the minimum width to show ASCII art in the header.
+const MinWidthForArt = 88
+
 // ShortcutEntry represents a keyboard shortcut for the header.
 type ShortcutEntry struct {
 	Key      string
@@ -173,7 +176,7 @@ func RenderHeader(b *strings.Builder, cfg HeaderConfig, width, height int) {
 	showInfo := true
 
 	// Hide art when viewport is too narrow for art + info + shortcuts
-	if showArt && width < 88 {
+	if showArt && width < MinWidthForArt {
 		showArt = false
 	}