Fix PR/MR base branches going stale after pushing stacks by mtsgrd · Pull Request #13697 · gitbutlerapp/gitbutler

mtsgrd · 2026-05-07T15:53:36Z

Problem

When a stack's structure changes and gets pushed, existing PRs keep pointing at their old base branch, showing incorrect diffs on the forge.

Before: main ← A (PR#1, base: main) ← B (PR#2, base: A)
After:  main ← X ← A ← B    ← PR#1 still targets main (wrong)

Fix

After pushing, walk every stack bottom-to-top, compute each review's expected target, and update any that drifted. The GitHub/GitLab update APIs already had base/target_branch fields — they were just always set to None.

Where it runs:

but push — best-effort post-push correction (forge errors are logged, not fatal)
but review — fixes drifted targets when publishing/updating reviews

Key changes:

compute_review_target_updates() — pure function that maps stack branches to expected PR targets
update_review_description_tables() — now handles both footer updates and target branch corrections in a single API call per review, with per-review error resilience
ForgeReviewDescriptionUpdate.target_branch — new optional field; when set, the review's base is updated alongside its footer

Tests

6 unit tests on compute_review_target_updates covering empty stacks, no-review branches, single PRs, full chains, skipped branches, and gaps in the middle.

🤖 Generated with Claude Code

Copilot

Pull request overview

This PR fixes stacked PR/MR base (target) branches going stale after stack structure changes by computing the expected target branch for each review in a stack and then updating the forge (GitHub/GitLab) accordingly. It wires this into both the post-push flow and the review publishing flow, and exposes the new capability through the API/SDK layer.

Changes:

Added compute_review_target_updates() and forge-side update_review_target_branches() to compute and apply base/target corrections for stacked reviews.
Invoked target-branch correction after but push (best-effort) and during but review publishing for already-existing reviews whose target has drifted.
Exposed updateReviewTargets via but-api (NAPI) and the generated but-sdk typings/exports.

Reviewed changes

Copilot reviewed 6 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
packages/but-sdk/src/generated/index.js	Exports the new `updateReviewTargets` binding from the native SDK entrypoint.
packages/but-sdk/src/generated/index.d.ts	Adds `ForgeReviewTargetUpdate` type and `updateReviewTargets()` API typing.
crates/but/src/lib.rs	Awaits the now-async legacy push handler.
crates/but/src/command/legacy/push.rs	Adds post-push hook to update review target branches across stacks.
crates/but/src/command/legacy/forge/review.rs	Detects drift for existing reviews during publish and queues target updates.
crates/but-forge/src/review.rs	Introduces target-update model, pure computation, and forge update implementation (GitHub/GitLab).
crates/but-forge/src/lib.rs	Re-exports new review target update APIs.
crates/but-api/src/legacy/forge.rs	Adds NAPI `update_review_targets()` wrapper calling into `but-forge`.

+    // After pushing, update PR/MR target branches to match the stack structure.
+    // This is best-effort: we don't want a forge API failure to fail the push.
+    if let Err(err) = update_review_targets_for_stacks(ctx).await {
+        tracing::warn!(?err, "Failed to update review target branches after push");
    }


+                    state: None,
+                };
+
+                but_github::pr::update(preferred_account, params, storage).await?;
+            }


+                    target_branch: Some(&update.target_branch),
+                    state_event: None,
+                };
+
+                but_gitlab::mr::update(preferred_account, params, storage).await?;


@@ -711,6 +712,13 @@ async fn publish_reviews_for_branch_and_dependents(
                if let Some(review) = reviews.first() {
                    // Ignore other existing reviews for ordering
                    all_reviews_in_order.push(review.clone());
+                    // If the existing review's target has drifted, queue an update.
+                    if review.target_branch != current_target_branch {


slarse · 2026-05-08T06:07:48Z

@mtsgrd This PRs exact title and description were also applied over on #13695 (I'm restoring it so check the history if you want to verify).

I also saw that the buildkite PR conspicuously got rebranded as a speedup PR: #13699

Were those manual blunders or might there be some bug in this PR? It's about updating PRs after all.

krlvi · 2026-05-08T07:46:35Z

this appears to be adding new API surface - we should probably take a deeper look

mtsgrd · 2026-05-08T08:15:33Z

@slarse thanks for cleaning up my mistakes. it's unfortunately ai hallucinations, 99% of the time when I say update my pr description it gets it right, but apparently not every time. I need to make sure I'm more specific in my requests going forward.

Copilot

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

mtsgrd · 2026-05-08T13:27:45Z

Amazing, thanks @estib-vega. I think I've addressed all of your comments if you wouldn't mind another look?

Here's what I understand about the new flow:

                  ┌─────────────┐
                  │  but push   │
                  └──────┬──────┘
                         │
                         ▼
              push branches to remote
                         │
                         ▼
        update_review_targets_for_stacks()
                         │
                         ▼
           compute_review_target_updates()
           (walks stacks, computes expected
            target for each review)
                         │
                         ▼
          ForgeReviewTargetUpdate ──Into──▶ ForgeReviewUpdate
                                            • body: None
                                            • unit_symbol: ""
                                            • target_branch: Some(...)
                                                     │
                                                     │
                  ┌─────────────┐                    │
                  │ but review  │                    │
                  └──────┬──────┘                    │
                         │                           │
                         ▼                           │
            publish / fetch reviews                  │
            (has full ForgeReview)                   │
                         │                           │
                         ▼                           │
          ForgeReview ──Into──▶ ForgeReviewUpdate    │
                                • body: Some(...)    │
                                • unit_symbol: "#"   │
                                • target_branch:     │
                                  Some if drifted,   │
                                  None if matches    │
                                         │           │
                                         ▼           ▼
                                ┌────────────────────────┐
                                │ update_review_footers  │  (but-api)
                                └───────────┬────────────┘
                                            │
                                            ▼
                                  ┌───────────────────┐
                                  │   sync_reviews    │  (but-forge)
                                  └───────────┬───────┘
                                              │
                              has_footer_content?
                              (any unit_symbol non-empty?)
                                    /        \
                                  yes         no
                                  /             \
                           recompute         skip body
                           PR footer
                                  \             /
                                   \           /
                                    ▼         ▼
                              ┌──────────────────────┐
                              │  GitHub/GitLab API   │
                              │  per review:         │
                              │  • body (if present) │
                              │  • base/target_branch│
                              │    (if present)      │
                              └──────────────────────┘

Copilot

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

When a stack's structure changes and gets pushed, existing PRs can end up pointing at the wrong base branch. For example, inserting branch X between main and A means PR#1 should now target X, not main. Before: main ← A (PR#1, base: main) ← B (PR#2, base: A) After: main ← X ← A ← B ← PR#1 still targets main (wrong) Fix: after pushing, walk every stack bottom-to-top, compute each review's expected target, and update any that drifted via the GitHub/GitLab API (base/target_branch fields were already supported but always set to None). - `but push`: best-effort post-push correction (forge errors logged, not fatal) - `but review`: fixes drifted targets when publishing/updating reviews - `compute_review_target_updates()`: pure function with 6 unit tests - `sync_reviews()`: handles both footers and target branches in a single pass, with per-review error resilience

mtsgrd requested a review from estib-vega May 7, 2026 15:53

github-actions Bot added rust Pull requests that update Rust code CLI The command-line program `but` labels May 7, 2026

mtsgrd force-pushed the fix-pr-targets-after-push branch 2 times, most recently from 8e50ba2 to 5204502 Compare May 7, 2026 17:38

mtsgrd changed the title ~~Fix PR/MR targets drifting when stack structure changes~~ Fix PR/MR base branches going stale after pushing stacks May 7, 2026

mtsgrd marked this pull request as ready for review May 7, 2026 22:18

Copilot AI review requested due to automatic review settings May 7, 2026 22:18

Copilot started reviewing on behalf of mtsgrd May 7, 2026 22:18 View session

Copilot AI reviewed May 7, 2026

View reviewed changes

estib-vega reviewed May 8, 2026

View reviewed changes

Comment thread crates/but-api/src/legacy/forge.rs Outdated

mtsgrd force-pushed the fix-pr-targets-after-push branch 2 times, most recently from 70d1130 to c4bb2bd Compare May 8, 2026 10:07

estib-vega reviewed May 8, 2026

View reviewed changes

Comment thread crates/but-forge/src/review.rs

estib-vega reviewed May 8, 2026

View reviewed changes

Comment thread crates/but-forge/src/review.rs Outdated

Comment thread crates/but/src/command/legacy/forge/review.rs Outdated

Copilot AI review requested due to automatic review settings May 8, 2026 13:16

mtsgrd force-pushed the fix-pr-targets-after-push branch from c4bb2bd to b98b678 Compare May 8, 2026 13:16

Copilot started reviewing on behalf of mtsgrd May 8, 2026 13:16 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

Comment thread crates/but-forge/src/review.rs

Comment thread crates/but/src/command/legacy/push.rs

mtsgrd force-pushed the fix-pr-targets-after-push branch from b98b678 to ba48509 Compare May 8, 2026 13:29

Copilot AI review requested due to automatic review settings May 8, 2026 13:42

mtsgrd force-pushed the fix-pr-targets-after-push branch from ba48509 to 59d5ef7 Compare May 8, 2026 13:42

Copilot started reviewing on behalf of mtsgrd May 8, 2026 13:43 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

Comment thread crates/but-forge/src/review.rs

Comment thread crates/but-forge/src/review.rs

Comment thread crates/but/src/command/legacy/forge/review.rs

Comment thread crates/but/src/command/legacy/push.rs

mtsgrd force-pushed the fix-pr-targets-after-push branch from 59d5ef7 to 646a9f0 Compare May 9, 2026 15:30

mtsgrd merged commit 035825c into master May 9, 2026
39 checks passed

mtsgrd deleted the fix-pr-targets-after-push branch May 9, 2026 15:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix PR/MR base branches going stale after pushing stacks#13697

Fix PR/MR base branches going stale after pushing stacks#13697
mtsgrd merged 1 commit into
masterfrom
fix-pr-targets-after-push

mtsgrd commented May 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

slarse commented May 8, 2026

Uh oh!

krlvi commented May 8, 2026

Uh oh!

mtsgrd commented May 8, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

mtsgrd commented May 8, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

mtsgrd commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

slarse commented May 8, 2026

Uh oh!

krlvi commented May 8, 2026

Uh oh!

mtsgrd commented May 8, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

mtsgrd commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mtsgrd commented May 7, 2026 •

edited

Loading

mtsgrd commented May 8, 2026 •

edited

Loading