Skip to content

SRE-637: Fix false positive stale approval dismissal on base-branch-only updates#47

Open
TimDiekmann wants to merge 1 commit intomainfrom
t/sre-637-stale-approval-dismissal-false-positive-on-base-branch-only
Open

SRE-637: Fix false positive stale approval dismissal on base-branch-only updates#47
TimDiekmann wants to merge 1 commit intomainfrom
t/sre-637-stale-approval-dismissal-false-positive-on-base-branch-only

Conversation

@TimDiekmann
Copy link
Copy Markdown
Member

@TimDiekmann TimDiekmann commented Apr 8, 2026

Summary

Fixes false positive stale approval dismissals when only the base branch advances (e.g., squash-merge of a stacked PR).

Problem

When a stacked PR's base gets squash-merged, the base branch advances but the PR branch is untouched. The shifted merge-base causes git range-diff to report false differences:

main:       A ─── B ─────────── S (squash merge of featureA)
                   \
featureA:           C (gets squash-merged)
                     \
featureB:             D (approved, untouched)

Before squash-merge (previous run):

  • PREV_BASE_SHA = C, PREV_HEAD_SHA = D
  • merge-base(C, D) = C → range: C..D (just featureB's commit)

After squash-merge (current run):

  • BASE_SHA = S, HEAD_SHA = D (unchanged!)
  • merge-base(S, D) = B (shifted!) → range: B..D (featureA + featureB)
range-diff C..D vs B..D:
  -:  ------- > 1:  92ee94d featureA commit    ← false positive!
  1:  404d18d = 2:  404d18d featureB commit

Fix

Extracts the range-diff logic into a testable function run_check_range_diff_stale in range-diff-stale.sh. The function checks if HEAD_SHA == PREV_HEAD_SHA before running range-diff — same HEAD SHA = same git content by definition, so the review can't be stale.

The fix lives in the same code that action.yml calls, not as an untestable YAML short-circuit.

Tests

  • Parser test (test_base_branch_advance_is_not_stale): verifies the range-diff parser reports stale=true for the shifted merge-base, documenting the false positive
  • E2E test (test_base_branch_advance_e2e_is_not_stale): reproduces the full stacked PR squash-merge scenario, calls run_check_range_diff_stale directly, asserts stale=false
    • Fails without the fix (range-diff false positive → stale=true)
    • Passes with the fix (HEAD unchanged → stale=false)

Related

@TimDiekmann TimDiekmann requested a review from a team April 8, 2026 16:35
@TimDiekmann TimDiekmann self-assigned this Apr 8, 2026
@augmentcode
Copy link
Copy Markdown

augmentcode bot commented Apr 8, 2026

🤖 Augment PR Summary

Summary: Prevents false stale-approval dismissals when a PR’s base branch advances but the PR head commit remains unchanged.

Changes: Adds an early exit in the range-diff step when HEAD_SHA == PREV_HEAD_SHA, skipping range-diff entirely for base-only updates.

Tests: Adds a repo-based reproduction of the shifted merge-base range-diff false positive, plus a structural assertion that the short-circuit exists in action.yml.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@TimDiekmann TimDiekmann force-pushed the t/sre-637-stale-approval-dismissal-false-positive-on-base-branch-only branch from 248ea29 to 0f5d34d Compare April 8, 2026 16:41
…nly updates

When only the base branch advances (e.g., squash-merge of a stacked PR),
HEAD_SHA remains unchanged. The shifted merge-base caused git range-diff
to report differences even though the PR's own commits were untouched.

Short-circuit the range-diff step when HEAD_SHA == PREV_HEAD_SHA: if the
PR branch head hasn't changed, the reviewed code hasn't changed.

Adds two tests:
- Integration test reproducing the stacked PR squash-merge scenario
- Structural test verifying the short-circuit exists in action.yml
@TimDiekmann TimDiekmann force-pushed the t/sre-637-stale-approval-dismissal-false-positive-on-base-branch-only branch from 0f5d34d to 7775608 Compare April 8, 2026 16:59
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