fix(pr-agent): discriminate the concurrency group by event and by pr/issue - #31
fix(pr-agent): discriminate the concurrency group by event and by pr/issue#31yakimoto 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
![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_1dc42111-9c32-40ae-aa1d-f81cb96ad3a8) |
|
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 |
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the pr-agent workflow’s concurrency group to include both the triggering event and whether the target is a PR or issue, preventing unrelated runs that share event or number values from cancelling each other while preserving cancel-in-progress behavior. Sequence diagram for discriminated pr-agent concurrencysequenceDiagram
participant Event as GitHub Event
participant Workflow as pr-agent Workflow
participant Group as Concurrency Group
participant Job as pr-agent Job
Event->>Workflow: Trigger workflow
Workflow->>Group: Evaluate group using github.event_name, target kind, and number
Group-->>Workflow: Select isolated concurrency key
Workflow->>Job: Start or cancel-in-progress within selected group
Job-->>Workflow: Apply job if conditions
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a focused, one-file CI fix that narrows concurrency cancellation to the correct event and PR/issue lane. It changes workflow scheduling only, leaving the job logic, permissions, deployments, and production behavior untouched. 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 group to include event name and resource type discriminators, preventing unrelated PR and issue 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 #30 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 #30 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 #30 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 ci: adopt the inline pr-agent lane — a public repo cannot call a private reusable workflow #30 colliding withissue_commenton Issue ci: adopt the inline pr-agent lane — a public repo cannot call a private reusable workflow #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; reduces accidental cancellation of in-flight PR reviews with no runtime or security impact.
Overview
Fixes incorrect workflow cancellation in the
pr-agentGitHub Action by tightening theconcurrency.groupkey so unrelated runs no longer evict each other undercancel-in-progress: true.The group now includes
github.event_nameand aprvsissuediscriminator before the number/ref, instead of keying only on PR/issue number (which is shared across PRs and issues). Inline comments document why both parts are required and that this matches a fleet-wide fix applied elsewhere.No changes to triggers, permissions, reusable workflow pin, or job logic.
Reviewed by Cursor Bugbot for commit 247c3b3. Bugbot is set up for automated code reviews on this repo. Configure here.