sdf
← All posts
show-and-tell

Anatomy of a Cascade Rebase Bug: When git rebase --onto Gets the Wrong Boundary

How a subtle mismatch between two SHAs in a fast-path optimization caused sdf sync to embed stale parent commits inside downstream branches -- and the detective work to find it.

Pavel Pascari ·

There's a class of bug that only appears when you squint at commit graphs long enough. The tool runs, prints green checkmarks, pushes branches — and three syncs later you'd notice that git log main..your-branch shows five commits where there should be four. One of them is a ghost: a commit from a parent branch that has no business being there.

For weeks, nothing seemed wrong. The tests passed. The optimization was doing what I'd asked for. Then one day I was reviewing a PR diff and saw a commit I'd never written sitting in a branch where it didn't belong. Then I checked another branch. Same thing. The bug hadn't spread through the code — it had spread through history.

This post is about finding and fixing that exact bug in SDF's sdf sync cascade rebase. It's a story about git rebase --onto, the difference between two SHAs that look like they should be the same, and why a fast-path optimization needs to be very careful about which question it asks.

The bug was introduced in PR #189 as part of an “update-tip” optimization, remained for one release, and was fixed in PR #214 — 25 days in the wild. It emerged during an agentic coding loop, made it through review, and was ultimately caught once the symptoms became too visible to overlook.

The setup: how cascade rebase works

SDF manages stacked PRs — chains of dependent branches where each one builds on the previous. A three-branch stack looks like this:

main (M1) → branchA [a1] → branchB [b1] → branchC [c1]

Each node in the stack stores a BaseTip — the SHA of the parent branch's tip at the time of the last sync. When sdf sync runs, it walks the chain and checks whether each branch's BaseTip still matches its parent's current tip. If not, it rebases.

The rebase command is the three-argument form:

git rebase --onto <new-parent> <old-base> <branch>

This tells Git: "take the commits between <old-base> and <branch>, and replay them onto <new-parent>." The <old-base> is the boundary — it defines which commits belong to the branch versus which belong to the parent. Get it wrong, and you either lose commits or include ones that shouldn't be there.

If you've ever wondered why git rebase --onto has three arguments instead of two, this bug is a good illustration.

How git rebase --onto works: take commits after oldbase on branch, replay them onto newbase

The animated diagram above shows the three roles in a git rebase --onto call. The three-argument form:

git rebase --onto <newbase> <upstream> <branch>

is really answering: "which commits are mine?" The range <upstream>..<branch> defines the commits to replay. Everything in that range gets cherry-picked onto <newbase>. Everything outside it stays behind.

When <upstream> is wrong — pointing to a commit that's not actually in the branch's ancestry, or pointing too far back — Git falls back to merge-base heuristics. Sometimes it guesses right. Sometimes it includes commits from a sibling rebase (same original commit, different base), producing the "phantom commit" symptom.

In our case, the corrupted BaseTip was a sibling SHA: a different rebase of the same original commit onto a different base. SHAs d93407b and 7954b4c are both "branchA rebased onto main" — just onto different versions of main. Neither is an ancestor of the other. Using the wrong one as <upstream> breaks the boundary.

The bug: one check, two SHAs

SDF has a feature where sdf sync (without the --full flag) intentionally skips rebasing when main advances from unrelated work. The idea is: if someone merged a totally unrelated PR into main, you don't want to rebase your entire stack just because the base moved. You want to keep working and only rebase when you choose to (via sdf sync --full).

To implement this, the sync loop tracks two different values for the base branch:

  • preFFBaseTip — the local main tip before fast-forwarding to the remote
  • currentParentTip — the actual current tip of main after fast-forward

The compareTip variable controls whether a branch "looks stale":

compareTip := currentParentTip
if !opts.fromHead && parent == s.Base && opts.preFFBaseTip != "" {
    compareTip = opts.preFFBaseTip  // use the old tip for comparison
}

When compareTip != node.BaseTip, something changed. The next question is: does the branch need a real rebase, or can we just update the bookkeeping?

There's a fast path for the second case — the "update-tip" path. If the parent's tip is already an ancestor of the branch (meaning the branch already includes those changes), no rebase is needed. Just update BaseTip and move on.

Here's where the bug lived:

if compareTip != node.BaseTip {
    if gitpkg.IsAncestor(compareTip, node.Branch) {  // ← THE BUG
        node.BaseTip = currentParentTip               // ← stores THIS
        continue  // skip rebase
    }
    // ... actual rebase ...
}

Spot it? The IsAncestor check uses compareTip (which might be preFFBaseTip — the old base), but then stores currentParentTip (the new base) as BaseTip.

When those two values differ, the code is asking: "is the old base an ancestor of this branch?" — but then records: "the branch is based on the new base." But the new base might not be an ancestor at all. The branch was never rebased onto it.

The corruption cascade

Here's the concrete scenario that breaks things:

Setup: main@M1 → branchA[a1]. BaseTip = M1.

  1. main advances to M2. Someone manually rebases branchA onto M2 (outside SDF).
  2. main advances further to M3.
  3. sdf sync (without --full) runs:
    • preFFBaseTip = M2 (local main before fast-forward)
    • currentParentTip = M3 (main after fast-forward)
    • compareTip = M2 (using pre-FF value since parent == main)
    • M2 != M1 (BaseTip) → something changed
    • IsAncestor(M2, branchA) → true (branchA was rebased onto M2)
    • node.BaseTip = M3 ← but M3 is NOT an ancestor of branchA!

BaseTip is now lying. It claims the branch is based on M3, but the branch has never seen M3.

On the next sync, when a downstream branch needs rebasing:

git rebase --onto branchA <corrupted-BaseTip> branchB

The <corrupted-BaseTip> doesn't properly identify the boundary between branchA's commits and branchB's commits. The result: commits from branchA's old rebase get included in branchB's history. Ghost commits.

This is the mechanism that makes cascade rebase bugs systemic. The corruption doesn't stay local — it encodes itself into the metadata that drives the next rebase. Each sync propagates the lie one branch further down the stack. The invariant that was broken: BaseTip must always be an ancestor of the branch it describes. Once that invariant fails, every downstream rebase computes the wrong commit boundary.

The fix: ask the right question

The fix is one line per call site:

// Before (buggy):
if gitpkg.IsAncestor(compareTip, node.Branch) {

// After (fixed):
if gitpkg.IsAncestor(currentParentTip, node.Branch) {

The compareTip variable should only control whether to enter the check ("has anything changed?"). The ancestor verification must always use the actual current parent tip — because that's the value we're about to store. If currentParentTip isn't an ancestor of the branch, the branch needs a real rebase, period.

This change was needed in three places: the execution loop (runSyncFrom), the plan computation (computeSyncPlan), and the hasRealWork pre-scan.

Defense in depth: the rebased map

While tracing through the code, I noticed another asymmetry. The plan computation (computeSyncPlan) maintains a rebased map — when a branch is rebased, its children are force-checked regardless of SHA comparison:

rebased := make(map[string]bool)
// ...
needsRebase := rebased[parent]  // force cascade if parent was rebased

The execution loop (runSyncFrom) had no such map. It relied entirely on SHA comparison for cascade propagation: after rebasing branchA, branchA's tip changes, so branchB's BaseTip differs from currentParentTip, triggering branchB's rebase.

This works in practice — a rebase always produces a new SHA. But it's fragile by design. The plan says one thing; the execution relies on a different mechanism. I added a matching rebased map to the execution loop:

forceCascade := rebased[parent]
if forceCascade || (node.BaseTip != "" && compareTip != node.BaseTip) {
    if !forceCascade && gitpkg.IsAncestor(currentParentTip, node.Branch) {
        // update-tip fast path — only when NOT force-cascading
        // ...
    }
    // rebase path...
}

And wired it through the --continue path, so that after manual conflict resolution, downstream branches know their parent was just rebased:

return runSyncFrom(root, s, progress.ResumeIndex+1, nil, result, bus,
    map[string]bool{progress.PausedAt: true})

What I tested

The existing test suite only covered computeSyncPlan (the dry-run planner). The execution path (runSyncFrom) had zero test coverage. I added six tests:

TestWhat it proves
UpdateTipAncestorCheckPlan shows "rebase" (not "update-tip") when only preFFBaseTip is ancestor
UpdateTipNoFalseCorruptionBaseTip is always a valid ancestor after sync
CascadePropagationFull cascade through 3-branch stack, correct commit counts
ForceCascadeFromRebasedParentAll branches rebased, verified via SyncResult
CascadesDownstream--continue cascades to downstream branches
WarnOnMergeBaseFallbackWarning emitted when BaseTip is corrupted

The first two are the ones that caught the bug — they failed before the fix and passed after. The rest are characterization tests that document the correct cascade behavior.

How the tests looked the other way

Here's the part that stings. The bug in PR #189 didn't sneak past the test suite — the test suite caught it. Five existing tests failed against the new "update-tip" optimization. Every one of them was asserting that downstream branches should be rebased after a merge. Every one of them was correct.

Instead of questioning the implementation, the tests were rewritten to agree with it. The pattern was the same in each:

TestBefore (correct)After (rewritten)
MergedHeadexpect rebase branchB onto main + pushexpect no rebases, no pushes, just update-tip
CascadeFromMergeexpect 2 rebases (branchB + branchC) + 2 pushesexpect no rebases, no pushes, 1 update-tip
MultipleMergedexpect rebase branchC onto main + pushexpect no rebases, no pushes, just update-tip
MergedMiddleexpect rebase branchC onto branchAexpect no rebases, just update-tip
StackScopedWithMergeexpect rebase branchB onto mainexpect no rebases, just update-tip

The comments were rewritten too. CascadeFromMerge went from "Both branchB and branchC should be rebased" to "branchB can be handled by tip refresh; branchC stays unchanged." The new assertions came with plausible explanations.

The reason the rewritten tests passed is subtle: the test repo setups happened to create scenarios where compareTip and currentParentTip were the same value. The IsAncestor check used the wrong variable but got the right answer. The tests passed for the wrong reason, and the bug hid behind them for 25 days.

Self-reflection: agentic loops and code review

PR #189 was written in an agentic coding loop. An AI agent wrote the implementation and rewrote the failing tests. The PR had well-commented code and plausible test assertions. I reviewed it and merged it.

I didn't ask why five cascade tests all flipped from "rebase expected" to "no rebase expected." The explanations in the rewritten test comments sounded reasonable. The test suite was green. The optimization was doing what I'd asked for — skipping unnecessary rebases. I moved on.

This is the same failure mode that happens on any team. When a colleague sends a PR that changes tests alongside implementation, the reviewer needs to ask: "are these test changes correct, or are they papering over a problem?" When five tests all weaken their assertions in the same direction, that's a signal worth pausing on.

But the system made it hard to catch. The agentic loop produces coherent, well-commented code. It rewrites tests with explanations that sound like the right reasoning. The volume of changes is high, and the reviewer's trust calibration drifts. The agent won't push back. It won't say "I'm not sure about this" or "these test changes seem suspicious." It will confidently rewrite the assertions with a comment explaining why the new behavior is correct. There's no friction, no hesitation — and those are exactly the signals a reviewer unconsciously relies on.

The human is the last line of defense. And in this case, I didn't hold it.

The contributing factors weren't any single decision — they were the combination: a fast-path optimization that split a check from its store, an agent that rewrote tests to match, test setups that happened to hide the divergence, and a reviewer who trusted the green signal. No single factor would have been enough. Together, they created a 25-day window.

The practices that catch bugs in team settings — skeptical PR review, questioning test changes that weaken assertions, requiring a second opinion on subtle invariants — are exactly the practices that catch bugs in agentic settings. There's nothing new here. The tools changed, but the discipline didn't.

Takeaways

Fast paths need to validate what they store, not just what they check. The bug was a mismatch between the condition variable (compareTip) and the stored value (currentParentTip). The fast path asked "is X an ancestor?" but recorded "Y is the base." When X ≠ Y, corruption follows.

Plan and execution should use the same cascade mechanism. Having computeSyncPlan use a rebased map while runSyncFrom relied on SHA comparison meant the plan could say "rebase needed" while execution might disagree. They should converge.

Test the execution path, not just the planner. It's tempting to only test the function that computes the plan — it's pure, deterministic, easy to assert on. But the bugs live in the loop that actually calls git rebase --onto, handles conflicts, updates state, and pushes. That's where the SHAs interact with real Git.

When tests fail against new code, interrogate the code first. Five tests going from "expect rebase" to "expect no rebase" after adding an optimization is a signal. Those tests were written when the behavior was correct. Changing them should require the same burden of proof as changing the implementation.

Agentic workflows need the same review rigor as team workflows. An AI agent writing code is a colleague who never pushes back, never says "I'm not sure about this," and always produces plausible explanations. The practices that catch bugs in team settings — skeptical PR review, questioning test changes, requiring a second pair of eyes — are exactly the practices that catch bugs in agentic settings. The tools changed. The discipline didn't.

Any system that rewrites history and generates code needs stronger invariants than human review alone. This is the deeper lesson. The bug wasn't exotic — it was a mismatch between two variables in a conditional. What made it dangerous was the context: history-rewriting operations where correctness is invisible, combined with a workflow that produces confident explanations for incorrect behavior. When your system rewrites commit graphs, "the tests pass" isn't enough. You need invariants that can't be rewritten along with the tests.

The fix is in PR #214. Two files changed, three call sites fixed, one map added. The kind of bug that takes longer to understand than to fix.