Uh oh!
There was an error while loading. Please reload this page.
fix(pr-agent): discriminate the concurrency group by event and by pr/issue - #30
fix(pr-agent): discriminate the concurrency group by event and by pr/issue#30yakimoto wants to merge 1 commit into
Conversation
…issue Concurrency is evaluated at WORKFLOW level, before any job `if:`, so a run the reusable lane would skip has already joined the group and evicted whatever was in it. PRs and Issues share one number sequence, so the old key collapsed every event on number N onto one group under cancel-in-progress. Measured across the fleet: 118 of 137 callers carried the undiscriminated key. On claude-workstation, the worst case, that cost 29 success / 1,625 cancelled / 5,735 skipped across 7,389 all-time runs. Both discriminators are load-bearing: `event_name` separates a push-triggered pull_request review from an issue_comment on the same PR, and the pr/issue kind separates issue_comment on PR #N from issue_comment on Issue #N. Proven live on claude-workstation#3617 before this fan-out: pr_agent concluded success with the agent step actually run, not cancelled and not skipped. Refs wave-pen#420, wave-pen#386
🤖 CodeAnt AI — Review Status
|
There was a problem hiding this comment.
We failed to fetch pull request #30.
You can try again by commenting this pull request with @sourcery-ai review, or contact us for help.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_470fe8dc-d33c-497a-af12-6965612b8bc3) |
Warning Review limit reachedNext included review available in 2 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 91 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Comment |
PR Summary by QodoFix pr-agent concurrency key to separate events and PR vs issue runs
AI Description
Diagram
High-Level Assessment
Files changed (1) |
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a one-file GitHub Actions concurrency-key fix that only changes how pr-agent runs are grouped and cancelled. Triggers, permissions, job logic, application code, and production behavior remain unchanged. Not approved because:
Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more. |
Note Automatic reviews are paused because your team has used its included automatic processing for this billing period (headroom scales with your seat count). You can still comment "Gitar review" to run one anytime, and automatic reviews resume on their own by September 1. Add seats for more headroom. Code Review✅ ApprovedUpdates the pr-agent workflow concurrency key to discriminate by event name and resource type, preventing cross-event cancellation between pull requests and issues. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
yakimoto
commented
Aug 24, 2026
Closing as redundant — my error, and worth naming rather than deleting quietly. This repo already has #29 open from the wave-pen#418 wave, on group: pr-agent-${{ github.event_name }}-${{ (github.event.pull_request.number || github.event.issue.pull_request) && 'pr' || 'issue' }}-${{ github.event.pull_request.number || github.event.issue.number || github.ref }}So this PR was a duplicate that would have conflicted on the same file, and #29 is strictly better besides: it also replaces the stale pinned reusable ( How it happened: the fan-out script checked idempotence by looking for an existing PR from its own branch name, which is the wrong question. The right one is whether any open PR already modifies the target file. Ten repos in the vulnerable set had exactly that, and all ten got a duplicate before the check caught it. Merging #29 is the action here. Nothing is lost by closing this. Refs wave-pen#420, wave-pen#418 |
Code Review by Qodo🐞 Bugs (0)📘 Rule violations (0)📎 Requirement gaps (0)Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can switch off images and animations for a plain-text comment |
Qodo FixerNo findings are available for this PR yet. Findings appear here once Qodo has reviewed the PR. |
One line of YAML. This repo is one of 118 callers of 137 measured carrying the same defect.
The defect
Concurrency is evaluated at workflow level, before any job
if:. A run that the pr-agent lane would go on to skip has therefore already joined the group and evicted whatever was in it. PRs and Issues also share one number sequence. So this group —— collapses every event touching number N onto a single key, and each new one kills the last. A comment on Issue #30 cancels the in-flight review of PR #30, then skips itself.
The fix
Both discriminators are load-bearing:
event_namestops anissue_commentcancelling the push-triggeredpull_requestreview of the same PR. This is the common case, since that review runs on everysynchronize.pr/issuekind stopsissue_commenton PR fix(pr-agent): discriminate the concurrency group by event and by pr/issue #30 colliding withissue_commenton Issue fix(pr-agent): discriminate the concurrency group by event and by pr/issue #30 — same event, same number, whichevent_namealone does not separate. Three repos carry the event-only key from an earlier pass and retain exactly this residual, so it is an observed gap rather than a hypothetical one.Why this is not speculative
Proven live before this fan-out.
claude-workstation#3617applied this identical change to the fleet's worst case — 29 success / 1,625 cancelled / 5,735 skipped across 7,389 all-time runs — and its ownpr_agentthen concludedsuccesswith the agent step actually run, not cancelled and not skipped. The retry step correctly skipped because attempt 1 succeeded.It is also the same expression already running in production on 16 repos from the wave-pen#418 wave (
api-spec,wave-foundation-public,wave-realtime-edge,wave-modules, and others).How this repo was selected
Every repo in the org was enumerated and its
.github/workflows/pr-agent.ymlread off its default branch — not a working tree, not a code-search index, both of which can disagree with what ships. Classification was four-valued so an unreadable repo could never render as a safe one; 0 came back unreadable, so 118 is a count and not a floor.A repo was marked vulnerable only if it satisfies all three: it triggers on more than one numbered event, its group carries no discriminator, and
cancel-in-progressistrue. Repos withoutcancel-in-progressqueue rather than evict and were left alone.What is deliberately not in this PR
wave-foundation#1258.Receipts
concurrency:was reported and skipped rather than pattern-rewritten — a regex that quietly rewrites a line it did not fully understand is how a one-line fix becomes 118 defects.concurrency.groupasserted to contain both discriminators before anything is written.Refs wave-pen#420, wave-pen#386
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Low Risk
CI-only concurrency key change in pr-agent; no application code, secrets, or job triggers modified.
Overview
Fixes pr-agent GitHub Actions runs cancelling each other when unrelated events share the same PR/issue number.
The workflow
concurrency.groupnow keys ongithub.event_name, aprvsissuediscriminator (so Issue #N and PR #N do not share a group), and the existing number/ref suffix.cancel-in-progressis unchanged.A block comment documents why both discriminators matter (workflow-level concurrency runs before job
if:skips) and notes this matches a fleet-wide fix pattern.Reviewed by Cursor Bugbot for commit fc31dd1. Bugbot is set up for automated code reviews on this repo. Configure here.