fix(restack): pass --update-refs to git rebase by default

[boneskull]

All rebase invocations now pass --update-refs explicitly so that
untracked bookmark branches pointing into the stack are kept in sync
automatically. The flag is always passed explicitly (either
--update-refs or --no-update-refs) to override any ambient
rebase.updateRefs git config setting.

• Adds --no-update-refs flag to restack, submit, and sync as
  a per-invocation escape hatch for users who want to preserve bookmark
  branches.
• --worktrees mode automatically suppresses --update-refs because
  git silently skips refs checked out in other worktrees, which would
  corrupt the stack with no warning (see docs/update-refs.md).
• The resolved setting is persisted to cascade state so gh stack continue resumes remaining branches with the same behavior after a
  conflict.
• Requires Git 2.38+ (ships on all supported platforms).

Closes #112.

[claude]

Claude finished @boneskull's task in 3m 25s —— View job

---

Code Review

• Gather context and read repo instructions
• Read changed files
• Review implementation logic
• Check tests
• Provide final review

---

Overall this is a clean, well-reasoned implementation. The core approach — always passing the flag explicitly to override ambient rebase.updateRefs git config — is the right call. State persistence for conflict resume is handled correctly. A few things worth noting below.

---

internal/git/git.go

The updateRefsArg helper is a nice touch. Clean, self-documenting, no string duplication. Flag ordering in all four methods is correct (--update-refs before --onto and positional args). No issues here.

---

cmd/restack.go — updateRefs resolution

// cmd/restack.go:180
updateRefs := !opts.NoUpdateRefs && len(opts.Worktrees) == 0

Correct. Suppression via worktrees is based on the actual map contents, not the flag, so if --worktrees is set but no branches happen to be in worktrees, --update-refs still fires — which is fine.

---

cmd/sync.go — minor inconsistency for retarget rebase

// cmd/sync.go:338
retargetUpdateRefs := !syncNoUpdateRefsFlag && !syncWorktreesFlag

This uses the raw --worktrees flag rather than checking len(worktrees) == 0 like doRestackWithState does. If --worktrees is active but ListWorktrees() returns an empty map (no active worktrees), the retarget --onto rebase will suppress --update-refs while the main restack loop won't. No correctness issue in practice, but a slight inconsistency with the pattern used everywhere else. Fix this →

---

cmd/continue.go

// cmd/continue.go:107
NoUpdateRefs: !st.UpdateRefs,

This double negation is correct: state stores the resolved positive (UpdateRefs = true means "--update-refs was active"), while the option field is the negative form (NoUpdateRefs = true means "suppress it"). The logic round-trips cleanly through all scenarios (default, --no-update-refs, --worktrees). A one-line comment would help future readers follow the inversion.

---

internal/state/state.go — backward compatibility

// state.go:49
// False (zero value) means --no-update-refs was in effect, which is safe for old state files.
UpdateRefs bool `json:"update_refs,omitempty"`

Old state files will unmarshal this as false, so continue will pass --no-update-refs for any in-progress operation from before this PR. The comment calls this "safe" (no corruption), but since the new default is --update-refs, an upgrade mid-conflict will cause the continue to use --no-update-refs unexpectedly. This is likely a non-issue in practice (the window is tiny), but the trade-off is worth confirming intentionally. A omitempty alternative would be to invert the field name (NoUpdateRefs bool) so the zero value maps to the default "use --update-refs" behavior — but that would make continue.go even harder to read. The current design is acceptable given the comment.

---

Tests — setup errors silently discarded

In git_test.go:857 and git_test.go:884:

exec.Command("git", "-C", dir, "branch", "bookmark", "B").Run()
exec.Command("git", "-C", dir, "checkout", "C").Run()

The setupLinearStack helper already provides a run() closure that calls t.Fatalf on failure, but these in-body setup calls use raw exec.Command(...).Run() and silently discard errors. If a setup step fails (e.g., branch doesn't exist), the test will fail downstream with a confusing message rather than a clear setup failure. Consider using the pattern from setupLinearStack or inlining the error check. Fix this →

---

Tests — overall

The TestRestackMovesUntrackedBookmark / TestRestackNoUpdateRefsFlagPreservesBookmark / TestRestackWorktreesSuppressesUpdateRefs trio gives solid coverage of all three scenarios. The indirect verification approach for the worktrees test (bookmark doesn't move → --no-update-refs was in effect) is sound because --update-refs would update the bookmark even though it silently skips the worktree ref itself.

---

Summary

Two minor issues worth fixing before merge: the sync.go worktrees consistency and the silent test setup errors. The state backward-compatibility trade-off is intentional and acceptable. Everything else looks good.

[Copilot]

Pull request overview

This PR makes gh stack pass git rebase --update-refs explicitly by default (and --no-update-refs when disabled) so bookmark-style branches pointing into a rebased stack are updated predictably, regardless of a user’s rebase.updateRefs git config. It also adds a per-invocation --no-update-refs escape hatch, suppresses update-refs in worktree scenarios, and persists the resolved behavior across gh stack continue after conflicts.

Changes:

• Plumb an updateRefs boolean through all rebase entry points and persist it in cascade state for continue.
• Add --no-update-refs to restack, submit, and sync, and auto-suppress update-refs in worktree-aware flows.
• Add unit/e2e coverage for bookmark movement behavior, and document the Git 2.38+ requirement and new flags.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
README.md	Documents Git 2.38+ requirement and --no-update-refs; adds a worktrees/update-refs note.
internal/state/state.go	Persists resolved update-refs behavior in cascade state.
internal/git/git.go	Adds explicit --update-refs/--no-update-refs handling to rebase wrappers.
internal/git/git_test.go	Adds unit tests covering bookmark movement with update-refs on/off.
cmd/restack.go	Resolves update-refs behavior for an operation, saves it to state, and threads it into rebases.
cmd/continue.go	Restores the stored update-refs behavior when resuming after conflicts.
cmd/submit.go	Adds --no-update-refs flag and plumbs into restack phase.
cmd/sync.go	Adds --no-update-refs flag and applies update-refs suppression consistently across sync flows.
e2e/restack_test.go	Adds e2e tests for bookmark movement with default vs --no-update-refs.
e2e/worktree_test.go	Adds e2e coverage for update-refs suppression in worktree scenarios.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.