fix(pr-agent): discriminate the concurrency group by event and by pr/issue - #99
fix(pr-agent): discriminate the concurrency group by event and by pr/issue#99yakimoto 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
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
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_51da1027-939f-4eef-9b75-892b53a9ac2f) |
|
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 |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the pr-agent workflow’s concurrency key so runs are isolated by event and by whether the numbered target is a pull request or issue, preventing unrelated runs from cancelling one another while leaving triggers, permissions, jobs, and workflow pins unchanged. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
codeant-ai
Bot
added
the
size:S
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a focused, one-file GitHub Actions concurrency fix that prevents unrelated PR and issue runs from cancelling each other. It leaves triggers, permissions, reusable workflow logic, and production application behavior unchanged. Not approved because:
Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more. |
PR Summary by QodoFix pr-agent workflow concurrency key collisions across events and PR/issue
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
|
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 group to discriminate by event name and resource kind, preventing unrelated runs from cancelling each other. 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 |
|
Closing as redundant — my error, and worth naming rather than deleting quietly. This repo already has #98 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 #98 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 #98 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. |
1 participant
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 chore: add eslint 9 flat config + lint CI gate #30 colliding withissue_commenton Issue chore: add eslint 9 flat config + lint CI gate #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
Cursor Bugbot is generating a summary for commit 150b8b0. Configure here.
Summary by Sourcery
Separate PR-agent concurrency groups by triggering event and resource type to avoid unrelated runs cancelling one another.
Bug Fixes:
Enhancements: