fix(pr-agent): discriminate the concurrency group by event and by pr/issue

Closed
YA
2mo ago
+11-1

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 —

group: pr-agent-${{ github.event.pull_request.number || github.event.issue.number || github.ref }}
cancel-in-progress: true

— 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

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 }}

Both discriminators are load-bearing:

  • event_name stops an issue_comment cancelling the push-triggered pull_request review of the same PR. This is the common case, since that review runs on every synchronize.
  • the pr/issue kind stops issue_comment on PR #30 colliding with issue_comment on Issue #30 — same event, same number, which event_name alone 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#3617 applied 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 own pr_agent then concluded success with 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.yml read 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-progress is true. Repos without cancel-in-progress queue rather than evict and were left alone.

What is deliberately not in this PR

  • No pin bump. Some callers pin the reusable workflow at a stale SHA and are missing separate body fixes; that is tracked apart from this and depends on wave-foundation#1258.
  • No change to triggers, permissions, or the job body. The diff is the concurrency block and the comment above it.

Receipts

  • The patch is applied by exact string match on the old group line. Any repo whose line did not match exactly once, directly under 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.
  • The patched file is parsed as YAML and the resulting concurrency.group asserted to contain both discriminators before anything is written.
  • No other file is touched.

Refs wave-pen#420, wave-pen#386

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with 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.group now keys on github.event_name, a pr vs issue discriminator (so Issue #N and PR #N do not share a group), and the existing number/ref suffix. cancel-in-progress is 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 fc31dd153ee3540602491e2bafbab0a9976fc36c. Bugbot is set up for automated code reviews on this repo. Configure here.