Skip to content

fix: stop the score action from clobbering the base ref - #87

Merged
2 commits merged into
mainfrom
fix/score-pr-ref-clobbering
Sep 27, 2026
Merged

2 commits merged into
mainfrom
fix/score-pr-ref-clobbering

Conversation

@vitali87

Copy link
Copy Markdown
Owner

Problem

scripts/score_pr.py turned the PR's base and head into local branches named after them:

git branch -f <base> origin/<base>
git branch -f <head> pr-split/head-<n>

A fork PR opened from the fork's own main (very common) has HEAD_BRANCH == BASE_BRANCH == main, so the second command re-pointed main at the PR head. git diff main...main is empty, pr-split split produced a 0-group plan, and the PR was reported as within limits. Verified in a scratch clone with a real refs/pull/7/head: the old script left main == pr-split/head-7 != origin/main.

Fix

Don't materialise branches at all. Pass origin/<base> and the fetched head ref (pr-split/head-<n>, or origin/<head> when there is no PR number) straight to pr-split split; branch_exists uses rev-parse --verify, and extract_diff/merge_base accept any ref (verified end to end: 6 groups on a 240-LOC PR).

Tests

  • head named like base: no git branch calls, split invoked with pr-split/head-7 --base origin/main
  • branch event without PR number: origin/feature --base origin/main

Both fail on base. Local review: 5/5. 446 tests pass, ruff clean.

Note: the action still reports 0 groups until #53 (plan-file key) merges; the two fixes are independent.

score_pr.py materialised the PR's base and head as local branches named
after them. A fork PR opened from the fork's own 'main' has
HEAD_BRANCH == BASE_BRANCH, so the second 'git branch -f' pointed the
base at the PR head, the diff came out empty, and the PR was reported as
within limits. Pass origin/<base> and the fetched head ref straight to
pr-split instead; rev-parse --verify accepts any ref.
@coderabbitai

coderabbitai Bot commented Aug 30, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 45 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: ada2fc37-8625-41c1-b009-2c8bd4bbe978

📥 Commits

Reviewing files that changed from the base of the PR and between 671a081 and fc5b24d.

📒 Files selected for processing (2)
  • scripts/score_pr.py
  • tests/test_score_pr_refs.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Main's split now requires --base to be a local branch, so keep creating the
local base branch from origin/<base> and only stop materialising the head:
pass the fetched head ref (pr-split/head-<n> or origin/<head>) directly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
@vitali87 vitali87 closed this pull request by merging all changes into main in 8ca9e3b Sep 27, 2026
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