Skip to content

Commit 0ac3cb2

Browse files
committed
fix(ci): cron timing doc + concurrency label-stuck race in copilot-review-fix.yml
Two issues caught by Copilot review on the parallel vuls-reach back-port (vuls-saas/vuls-reach#22), addressed there in 821e3da. The same bugs exist verbatim in the OSS copy of `copilot-review-fix.yml` (workflow files were copy-portable across catalog/oss/vuls-reach in the recent unification). Mirror the fix here so OSS doesn't ship the bugs. 1. **Stale "~5 min" cron-fallback claim** (lines 7-11): comment said "covers suppressed cases within ~5 min" but `copilot-clean-label.yml` actually uses `*/30` (cron fires every 30 minutes, worst case ~60 min — marker posted just after a tick stays fresh on the next tick). Updated to match the documented 30–60 min recovery window used elsewhere. 2. **`cancel-in-progress: true` race with label removal** (line 29): the first step on the labeled-PR path is "Remove copilot-review label if present" (so the label acts as a single-shot trigger). With `cancel-in-progress: true`, a new Copilot review/comment event for the same PR cancels the label-triggered run before that first step completes, leaving the label stuck on the PR. Re-adding an already- present label fires no event, so the manual retry path silently breaks. Switched to `cancel-in-progress: false` so queued events run sequentially; the marker-based dedup later in the job (`already_triggered` / `local_in_progress` jq passes) already prevents duplicate `@claude` posts, so queueing is the safe choice. The same bugs exist in catalog (#223 already merged) — follow-up PR will mirror the fix there.
1 parent 6faa677 commit 0ac3cb2

1 file changed

Lines changed: 16 additions & 4 deletions

File tree

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@ on:
88
# (created) events are unreliable for the Copilot bot due to the agentic
99
# architecture's GITHUB_TOKEN-driven event suppression. They are still listened
1010
# for as a best-effort fast path; the cron-driven fallback in
11-
# `copilot-clean-label.yml` covers the suppressed cases within ~5 min.
11+
# `copilot-clean-label.yml` covers the suppressed cases within ~30–60 min
12+
# (cron fires every 30 minutes, worst case is "marker posted just after a
13+
# tick" so cron only resumes on the tick after, ≈60 min later).
1214
#
13-
# Multiple comments may trigger multiple events; concurrency cancel-in-progress
14-
# ensures only the last one runs.
15+
# Multiple events for the same PR are deduped by the marker-based check
16+
# below (`already_triggered` / `local_in_progress` jq passes), so the
17+
# concurrency group below uses `cancel-in-progress: false` and lets
18+
# queued events run sequentially.
1519
pull_request_review:
1620
types: [submitted]
1721
pull_request_review_comment:
@@ -26,7 +30,15 @@ jobs:
2630
trigger-claude:
2731
concurrency:
2832
group: copilot-review-${{ github.event.pull_request.number }}
29-
cancel-in-progress: true
33+
# `cancel-in-progress: false` is intentional. With `true`, a new
34+
# Copilot review/comment event can cancel a label-triggered run
35+
# before its first step removes the `copilot-review` label,
36+
# leaving the label stuck on the PR. Re-adding an already-present
37+
# label fires no event, so the manual retry path breaks. Marker-
38+
# based dedup later in the job (`already_triggered` /
39+
# `local_in_progress`) already prevents duplicate `@claude` posts,
40+
# so queueing is the safe choice.
41+
cancel-in-progress: false
3042
# Run when: Copilot submits a review OR posts a review comment OR
3143
# 'copilot-review' label is added by the maintainer.
3244
# Only for same-repo PRs authored by kotakanbe (fork PRs cannot access

0 commit comments

Comments
 (0)