Skip to content

Commit 339f07c

Browse files
committed
fix: naming alignment + runtime HEAD_SHA refresh in copilot-review-fix.yml
Same two issues caught on catalog #225 round 2 (commit 76cb3b9). Mirror to OSS #379 so they ship together. 1. Comment naming: replace `already_triggered` → `already` to match the actual variable name used in this workflow's script (`already_triggered` only appears in copilot-clean-label.yml's cron path). 2. Stale HEAD_SHA risk introduced by `cancel-in-progress: false`: refresh HEAD via `gh pr view --json headRefOid` at runtime and exit early when the payload SHA no longer matches the current PR HEAD.
1 parent 0ac3cb2 commit 339f07c

1 file changed

Lines changed: 30 additions & 2 deletions

File tree

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

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ on:
1313
# tick" so cron only resumes on the tick after, ≈60 min later).
1414
#
1515
# Multiple events for the same PR are deduped by the marker-based check
16-
# below (`already_triggered` / `local_in_progress` jq passes), so the
16+
# below (`already` / `local_in_progress` jq passes), so the
1717
# concurrency group below uses `cancel-in-progress: false` and lets
1818
# queued events run sequentially.
1919
pull_request_review:
@@ -35,7 +35,7 @@ jobs:
3535
# before its first step removes the `copilot-review` label,
3636
# leaving the label stuck on the PR. Re-adding an already-present
3737
# label fires no event, so the manual retry path breaks. Marker-
38-
# based dedup later in the job (`already_triggered` /
38+
# based dedup later in the job (`already` /
3939
# `local_in_progress`) already prevents duplicate `@claude` posts,
4040
# so queueing is the safe choice.
4141
cancel-in-progress: false
@@ -119,6 +119,34 @@ jobs:
119119
# has no TTL (HEAD-scoped, cleared by next push). Same semantics
120120
# as the cron path in copilot-clean-label.yml.
121121
LOCAL_MARKER_MAX_AGE_MIN=30
122+
123+
# Refresh HEAD_SHA at runtime, not from the event payload.
124+
# `cancel-in-progress: false` (above) means an older queued
125+
# event can run after a newer push has advanced the PR HEAD;
126+
# the payload-derived `github.event.pull_request.head.sha`
127+
# would then post a marker / `@claude` trigger keyed to a
128+
# stale SHA, while the cron / event-driven dedup looks up
129+
# the current SHA and would still post a duplicate trigger
130+
# for the newer HEAD. Read the current PR HEAD via
131+
# `gh pr view` and exit early if the payload SHA no longer
132+
# matches it (the newer event will run separately and
133+
# handle the current HEAD). Fail-closed on `gh pr view`
134+
# error: skipping is safer than posting a possibly-stale
135+
# trigger.
136+
payload_sha="${HEAD_SHA:-}"
137+
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
140+
fi
141+
if [ -z "$current_sha" ] || [ "${#current_sha}" -lt 40 ]; then
142+
echo "::warning::missing/short current PR HEAD '$current_sha' — skip @claude trigger"
143+
exit 0
144+
fi
145+
if [ -n "$payload_sha" ] && [ "$payload_sha" != "$current_sha" ]; then
146+
echo "stale event: payload HEAD=$payload_sha, current HEAD=$current_sha — skip @claude trigger (newer event will handle current HEAD)"
147+
exit 0
148+
fi
149+
HEAD_SHA="$current_sha"
122150
if [ -z "${HEAD_SHA:-}" ] || [ "${#HEAD_SHA}" -lt 40 ]; then
123151
echo "::warning::missing/short HEAD_SHA '$HEAD_SHA' — skip @claude trigger"
124152
exit 0

0 commit comments

Comments
 (0)