Skip to content

fix(review): run-local parallel PR FETCH_HEAD race #814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mergify merged 1 commit into from
Oct 9, 2026
Merged

fix(review): run-local parallel PR FETCH_HEAD race #814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions scripts/review/run-local.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,14 +252,19 @@ git -C "$TOP" rev-parse --verify --quiet "$base^{commit}" > /dev/null \
|| die 2 "Basis $base nicht gefunden (git fetch origin main?)."

if [ -n "$pr" ]; then
git -C "$TOP" fetch --quiet origin "pull/$pr/head" 2> /dev/null \
|| die 2 "PR $pr nicht gefunden: 'git fetch origin pull/$pr/head' schlug fehl (Nummer falsch, origin nicht erreichbar oder kein Zugriff)."
head_ref="$(git -C "$TOP" rev-parse FETCH_HEAD)" || die 2 "PR $pr: FETCH_HEAD nicht lesbar."
[ -n "$head_ref" ] || die 2 "PR $pr: FETCH_HEAD ist leer."
# Private ref per PR: concurrent runs in one checkout must not share FETCH_HEAD
# (a second fetch overwrites it before rev-parse). Force-update keeps the ref
# current when the same PR is reviewed again.
pr_ref="refs/pa-review/pr-$pr"
git -C "$TOP" fetch --quiet origin "+pull/$pr/head:$pr_ref" 2> /dev/null \
|| die 2 "PR $pr nicht gefunden: 'git fetch origin +pull/$pr/head:$pr_ref' schlug fehl (Nummer falsch, origin nicht erreichbar oder kein Zugriff)."
head_ref="$(git -C "$TOP" rev-parse --verify "$pr_ref^{commit}")" \
|| die 2 "PR $pr: Ref $pr_ref nicht lesbar."
[ -n "$head_ref" ] || die 2 "PR $pr: Ref $pr_ref ist leer."
[ -n "$label" ] || label="pr$pr"
subject="PR #$pr"
else
head_ref="HEAD"
head_ref="$(git -C "$TOP" rev-parse --verify HEAD)" || die 2 "HEAD nicht lesbar."
branch="$(git -C "$TOP" rev-parse --abbrev-ref HEAD 2> /dev/null)"
if [ -z "$branch" ] || [ "$branch" = HEAD ]; then
branch="head-$(git -C "$TOP" rev-parse --short HEAD)"
Expand Down Expand Up @@ -299,6 +304,8 @@ prompt_file="$out_dir/review_prompt_$label.md"
cat << EOF
# Review request $label: $subject against $base

Head: $head_ref

You are an independent reviewer (not the author${author:+; the author is a $author model}).
Review the change below for correctness bugs, gaps, and safety regressions.
Be concrete: cite file and line, say what breaks and when. Rate each finding
Expand Down
54 changes: 54 additions & 0 deletions scripts/test-review-local.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,60 @@ else
bad "kilo-Protokollname kollidiert: rc=$rc"; echo "$out"; ls "$tmp/k8" 2>&1
fi

# 14. Parallel PR fetches must not share FETCH_HEAD. A git wrapper overwrites
# FETCH_HEAD after a bare `pull/N/head` fetch (the old pattern); a private
# destination ref must keep each prompt on its own PR head.
(
cd "$REPO" || exit 1
git checkout -q -b pr8-branch main
echo "MARKER_PR8_ONLY" > pr8.txt
git add pr8.txt && git commit -q -m pr8
git push -q origin HEAD:refs/pull/8/head
git checkout -q claude/demo-branch
)
sha7="$(git -C "$REPO" ls-remote origin refs/pull/7/head | awk '{print $1}')"
sha8="$(git -C "$REPO" ls-remote origin refs/pull/8/head | awk '{print $1}')"
REAL_GIT="$(command -v git)"
mkdir -p "$tmp/bin-race"
cat > "$tmp/bin-race/git" <<EOF
#!/usr/bin/env bash
REAL_GIT=$(printf '%q' "$REAL_GIT")
joined="\$*"
if [[ "\$joined" == *fetch* && "\$joined" =~ pull/([0-9]+)/head ]] \\
&& [[ ! "\$joined" =~ pull/[0-9]+/head: ]]; then
"\$REAL_GIT" "\$@" || exit \$?
n="\${BASH_REMATCH[1]}"
other=8
[ "\$n" = 8 ] && other=7
if [ "\$1" = "-C" ]; then
"\$REAL_GIT" -C "\$2" fetch --quiet origin "pull/\$other/head"
else
"\$REAL_GIT" fetch --quiet origin "pull/\$other/head"
fi
exit \$?
fi
exec "\$REAL_GIT" "\$@"
EOF
chmod +x "$tmp/bin-race/git"
run env PATH="$tmp/bin-race:$PATH" bash "$RUN" 7 --dry-run --models fake-a:cloud --out-dir "$tmp/race7"
rc7=$rc; out7=$out
run env PATH="$tmp/bin-race:$PATH" bash "$RUN" 8 --dry-run --models fake-a:cloud --out-dir "$tmp/race8"
rc8=$rc; out8=$out
if [ "$rc7" -eq 0 ] && [ "$rc8" -eq 0 ] \
&& grep -q "MARKER_LINE two" "$tmp/race7/review_prompt_pr7.md" 2>/dev/null \
&& ! grep -q "MARKER_PR8_ONLY" "$tmp/race7/review_prompt_pr7.md" 2>/dev/null \
&& grep -q "MARKER_PR8_ONLY" "$tmp/race8/review_prompt_pr8.md" 2>/dev/null \
&& ! grep -q "MARKER_LINE" "$tmp/race8/review_prompt_pr8.md" 2>/dev/null \
&& grep -q "^Head: $sha7\$" "$tmp/race7/review_prompt_pr7.md" 2>/dev/null \
&& grep -q "^Head: $sha8\$" "$tmp/race8/review_prompt_pr8.md" 2>/dev/null; then
ok "parallel PR fetches keep distinct diffs (no shared FETCH_HEAD)"
else
bad "parallel PR FETCH_HEAD race: rc7=$rc7 rc8=$rc8"; echo "$out7"; echo "$out8"
ls "$tmp/race7" "$tmp/race8" 2>&1
head -n 8 "$tmp/race7/review_prompt_pr7.md" 2>&1
head -n 8 "$tmp/race8/review_prompt_pr8.md" 2>&1
fi

echo
if [ "$fails" -eq 0 ]; then
echo "test-review-local: alles gruen."
Expand Down
Loading