Skip to content

Commit 7a2b98e

Browse files
committed
fix(ci): the agy local-diff fallback cannot merge-base in a shallow clone
Synced from RustyNES (PR #375), stacked on the file-count fix in this same branch. That fix alone is not enough: it gets the fallback started, and then the run dies one step later. [agy-review] diff exceeds GitHub's 300-file API limit; falling back to a local git diff [agy-review] fetched PR refs using git's ambient credentials [agy-review] could not compute the merge base for PR #373 The workflow checks out with `fetch-depth: 1`, deliberately -- the comment says the fallback "fetches exactly the two refs it needs on demand, so a full history clone would be paid on every run for a path most runs never take". But two refs fetched into a shallow repository arrive as DISCONNECTED shallow histories, with no common ancestor for `git merge-base` to find, so it fails even though both fetches succeeded. Asks the compare API for the merge base and fetches that single commit instead of unshallowing. Diffing two commits needs both trees, not the history between them, so a shallow fetch of the merge base suffices, and a repo with a large history does not pay a full clone on a path that exists only because the PR is already unusually big. `--deepen=250` covers a server that refuses a bare-SHA fetch. Verified upstream against a genuinely shallow clone -- the earlier attempt was checked in a full clone, where `git merge-base` succeeds and the bug cannot be seen. Reproduced the runner's exact failure at `--depth 1`, then confirmed the fix in that same clone: merge base resolved via the API (identical to a full clone's `git merge-base`), SHA fetch accepted, resulting diff 12,021 lines / 586,242 bytes.
1 parent 3e03914 commit 7a2b98e

1 file changed

Lines changed: 27 additions & 3 deletions

File tree

‎scripts/agy-review.sh‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ trap cleanup EXIT
175175
# but is NOT harmless now that isCrossRepository gates whether an untrusted diff
176176
# reaches agy: a lookup failure must never be indistinguishable from "same-repo".
177177
diff_file="$(mktemp)"; meta_file="$(mktemp)"; diff_err="$(mktemp)"
178-
gh pr view "$PR" --repo "$REPO" --json title,isCrossRepository,baseRefName > "$meta_file" \
178+
gh pr view "$PR" --repo "$REPO" --json title,isCrossRepository,baseRefName,headRefOid > "$meta_file" \
179179
|| { log "gh pr view failed; refusing to review without knowing the PR's head repo"; exit 1; }
180180

181181
# THE FORK GATE (see the trust model at the agy invocation below). The workflow `if:`
@@ -260,8 +260,32 @@ if ! gh pr diff "$PR" --repo "$REPO" > "$diff_file" 2>"$diff_err"; then
260260
log "could not fetch PR #${PR} refs for the local diff fallback"
261261
exit 1
262262
fi
263-
merge_base="$(git merge-base "$base_local" "$pr_ref")" || {
264-
log "could not compute the merge base for PR #${PR}"; exit 1; }
263+
# The workflow clones with `fetch-depth: 1`, so the two refs above arrive as
264+
# DISCONNECTED shallow histories -- there is no common ancestor for
265+
# `git merge-base` to find, and it fails even though both refs fetched fine.
266+
# (A full local clone hides this completely, which is how it got missed.)
267+
#
268+
# Ask the API for the merge base and fetch that one commit, rather than
269+
# unshallowing: a repo with a large history would pay a full clone on a path
270+
# that only exists because the PR is already unusually big. Diffing two
271+
# commits needs both trees, not the history between them, so a shallow fetch
272+
# of the merge base is enough.
273+
merge_base="$(git merge-base "$base_local" "$pr_ref" 2>/dev/null || true)"
274+
if [ -z "$merge_base" ]; then
275+
head_sha="$(jq -r '.headRefOid // empty' "$meta_file")"
276+
api_base="$(gh api "repos/${REPO}/compare/${base_ref}...${head_sha}" \
277+
--jq '.merge_base_commit.sha' 2>/dev/null || true)"
278+
if [ -n "$api_base" ] && [ "$api_base" != "null" ]; then
279+
if git fetch --no-tags --quiet origin "$api_base" 2>/dev/null \
280+
|| git fetch --no-tags --quiet --deepen=250 origin "${fetch_refspecs[@]}" 2>/dev/null; then
281+
merge_base="$(git merge-base "$base_local" "$pr_ref" 2>/dev/null || echo "$api_base")"
282+
log "shallow clone: merge base ${merge_base} resolved via the compare API"
283+
fi
284+
fi
285+
fi
286+
if [ -z "$merge_base" ]; then
287+
log "could not compute the merge base for PR #${PR}"; exit 1
288+
fi
265289
git diff "$merge_base" "$pr_ref" > "$diff_file" || {
266290
log "local git diff failed for PR #${PR}"; exit 1; }
267291
log "local diff: $(wc -l < "$diff_file") lines, $(wc -c < "$diff_file") bytes"

0 commit comments

Comments
 (0)