Skip to content

fix for rebase in sync cmd#95

Open
skarim wants to merge 2 commits into
mainfrom
skarim/sync-rebase
Open

fix for rebase in sync cmd#95
skarim wants to merge 2 commits into
mainfrom
skarim/sync-rebase

Conversation

@skarim
Copy link
Copy Markdown
Collaborator

@skarim skarim commented May 17, 2026

fix: sync performs cascade rebase even when trunk is already up-to-date

Previously, gh stack sync gated the cascade rebase on whether trunk
or stack branches were fast-forwarded during the current run. This meant
that if the user had already updated trunk locally (e.g., git pull),
sync would skip the rebase entirely even though stack branches hadn't
been rebased onto the current trunk.

This change:

  • Adds stackNeedsRebase() to detect stale branches regardless of
    whether trunk was updated in this run
  • Extracts shared helpers (fastForwardTrunk, cascadeRebase,
    resolveOriginalRefs) from duplicated code in sync.go and rebase.go
    into utils.go, reducing ~450 lines of duplication
  • Fixes rebase.go to skip queued branches (was only skipping merged),
    consistent with sync's behavior via IsSkipped()
  • Refactors rebase --continue to reuse the shared cascade helper

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


Stack created with GitHub Stacks CLIGive Feedback 💬

Previously, `gh stack sync` gated the cascade rebase on whether trunk
or stack branches were fast-forwarded during the current run. This meant
that if the user had already updated trunk locally (e.g., `git pull`),
sync would skip the rebase entirely even though stack branches hadn't
been rebased onto the current trunk.

This change:
- Adds `stackNeedsRebase()` to detect stale branches regardless of
  whether trunk was updated in this run
- Extracts shared helpers (`fastForwardTrunk`, `cascadeRebase`,
  `resolveOriginalRefs`) from duplicated code in sync.go and rebase.go
  into utils.go, reducing ~450 lines of duplication
- Fixes rebase.go to skip queued branches (was only skipping merged),
  consistent with sync's behavior via `IsSkipped()`
- Refactors rebase --continue to reuse the shared cascade helper

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@skarim skarim force-pushed the skarim/sync-rebase branch from cedf847 to c46e74e Compare May 22, 2026 16:26
@skarim skarim marked this pull request as ready for review May 23, 2026 21:31
Copilot AI review requested due to automatic review settings May 23, 2026 21:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes gh stack sync so it performs a cascade rebase when stack branches are stale relative to trunk, even if trunk itself was already updated locally before running sync. It also reduces duplication by extracting shared trunk fast-forward / original-ref resolution / cascade rebase logic into common helpers and aligns rebase behavior with sync by skipping queued branches too.

Changes:

  • Add stackNeedsRebase() and use it in sync to trigger cascade rebases even when trunk wasn’t fast-forwarded in the current run.
  • Extract shared helpers (fastForwardTrunk, resolveOriginalRefs, cascadeRebase) into cmd/utils.go and refactor sync/rebase to use them.
  • Update tests to cover the “trunk up-to-date but stack stale” scenario and basic stackNeedsRebase cases.
Show a summary per file
File Description
cmd/utils.go Adds shared helpers for original ref resolution, trunk FF, cascade rebase, and stale-stack detection.
cmd/utils_test.go Adds unit tests for stackNeedsRebase().
cmd/sync.go Uses shared helpers and changes rebase gating to include stale-stack detection.
cmd/sync_test.go Adds regression test ensuring stale stack triggers rebase even when trunk is already up-to-date.
cmd/rebase.go Refactors to use shared helpers; updates skip logic to include queued branches; reuses cascade helper in --continue.

Copilot's findings

Tip

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

  • Files reviewed: 5/5 changed files
  • Comments generated: 3

Comment thread cmd/utils.go Outdated
Comment thread cmd/utils.go Outdated
Comment thread cmd/rebase.go
- resolveOriginalRefs now returns (map, error) instead of silently
  swallowing RevParseMap failures; sync warns and skips rebase, rebase
  aborts with a clear error
- cascadeRebase uses a new Err field on the result struct to distinguish
  fatal errors (e.g. checkout failure) from recoverable conflicts;
  callers no longer enter conflict-recovery flow for non-conflict errors
- continueRebase validates that remaining branch indices are contiguous
  in stack order, erroring out if the stack was reordered between
  conflict and --continue

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants