Skip to content

Commit 3d526d5

Browse files
committed
fix(ci): preserve manual label retry through gh pr view failure + correct stale-event comment
Two follow-up issues from round-3 Copilot review: 1. **`gh pr view` failure path repeats round-2's manual-label silent no-op bug**: round-3 split the stale-SHA branch by event type, but the `gh pr view --json headRefOid` failure path right above kept a single `exit 0` that applies to all event types. On the manual label path the earlier step has already removed the `copilot-review` label, so an exit here is a silent no-op (no `@claude` trigger, no feedback). Fix: split the failure path the same way as the stale-SHA branch. Manual label path falls back to the payload SHA and proceeds (operator gets a CI run for what we can verify, the marker dedup downstream still skips cleanly if the payload SHA was stale, and the operator at least sees a real run). Auto event paths keep the fail-closed `exit 0` (Copilot's next review or the cron fallback drives the PR forward). 2. **Stale-event comment overstates push semantics**: round-3's "stale auto event" comment said "a newer push triggers a fresh review/comment event that will reach this workflow". A bare push does NOT emit `pull_request_review` / `pull_request_review_comment` events; only Copilot's review/comment does. Reworded to "a fresh Copilot review/comment for the newer HEAD will fire its own event" and added the explicit cron-fallback escape hatch ("otherwise cron fallback handles within ~30–60 min") so the reader understands the actual recovery path, not an aspirational one.
1 parent 4c77a4d commit 3d526d5

1 file changed

Lines changed: 31 additions & 8 deletions

File tree

.github/workflows/copilot-review-fix.yml

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,30 @@ jobs:
134134
# error: skipping is safer than posting a possibly-stale
135135
# trigger.
136136
payload_sha="${HEAD_SHA:-}"
137+
current_sha=""
137138
if ! current_sha=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json headRefOid --jq '.headRefOid'); then
138-
echo "::warning::PR #$PR_NUMBER: gh pr view --json headRefOid failed — skipping @claude trigger (will be re-driven by cron / next event)"
139-
exit 0
139+
current_sha=""
140+
if [ "${GITHUB_EVENT_NAME:-}" = "pull_request" ]; then
141+
# Manual label path: the earlier step has already
142+
# removed `copilot-review`, so an exit here would be
143+
# a silent no-op (no `@claude` trigger, no PR
144+
# feedback). Fall back to the payload SHA and
145+
# proceed; if the payload SHA was stale, the marker
146+
# dedup further down will skip cleanly. The operator
147+
# gets a CI run for what we can verify (payload
148+
# HEAD), which is better than nothing.
149+
echo "::warning::PR #$PR_NUMBER: gh pr view --json headRefOid failed on manual label path — falling back to payload HEAD ($payload_sha) and proceeding (label was already removed; silent exit would break the manual retry path)"
150+
current_sha="$payload_sha"
151+
else
152+
# Auto event paths: skipping is safe — the same
153+
# Copilot review/comment that fired this run will
154+
# re-fire on the next cron tick / re-request, so
155+
# cron / a fresh Copilot review will eventually
156+
# drive this PR forward without us posting a
157+
# possibly-stale trigger now.
158+
echo "::warning::PR #$PR_NUMBER: gh pr view --json headRefOid failed on auto event path — skipping @claude trigger (will be re-driven by cron / next Copilot review)"
159+
exit 0
160+
fi
140161
fi
141162
if [ -z "$current_sha" ] || [ "${#current_sha}" -lt 40 ]; then
142163
echo "::warning::missing/short current PR HEAD '$current_sha' — skip @claude trigger"
@@ -156,12 +177,14 @@ jobs:
156177
echo "stale label event: payload HEAD=$payload_sha, current HEAD=$current_sha — proceeding with current HEAD (manual label path: no newer event will reach this workflow)"
157178
else
158179
# Auto event paths (`pull_request_review` /
159-
# `pull_request_review_comment`): a newer push triggers
160-
# a fresh review/comment event that will reach this
161-
# workflow with the up-to-date HEAD, so it's safe (and
162-
# correct) to skip this stale-SHA run rather than post
163-
# a duplicate trigger.
164-
echo "stale auto event: payload HEAD=$payload_sha, current HEAD=$current_sha — skip @claude trigger (newer event will handle current HEAD)"
180+
# `pull_request_review_comment`): a fresh Copilot
181+
# review/comment for the newer HEAD will fire its
182+
# own event reaching this workflow with the up-to-
183+
# date HEAD; if Copilot does NOT re-review, the
184+
# cron fallback in `copilot-clean-label.yml` covers
185+
# this PR within ~30–60 min. Either way, skipping
186+
# this stale-SHA run is correct.
187+
echo "stale auto event: payload HEAD=$payload_sha, current HEAD=$current_sha — skip @claude trigger (newer Copilot review/comment for current HEAD will fire its own event, otherwise cron fallback handles within ~30–60 min)"
165188
exit 0
166189
fi
167190
fi

0 commit comments

Comments
 (0)