fix(workflows): add concurrency group to pr-review-trigger to prevent triplicate reviews on fork PRs

[derekmisler]

Problem

When multiple pull_request events fire simultaneously for the same fork PR (e.g. ready_for_review + review_requested × 2), each triggered a separate PR Review - Trigger run. All completed, each spawned a PR Review via workflow_run, and all three ran in parallel — posting reviews in triplicate.

The downstream PR Review workflow's concurrency group uses github.run_id on the workflow_run path (unique per run), so there was no deduplication at that level.

Fix

Add a concurrency group to .github/workflows/pr-review-trigger.yml keyed by PR number:

concurrency:
  group: pr-review-trigger-${{ github.event.pull_request.number || github.run_id }}-${{ github.event.comment.id || 'review-request' }}
  cancel-in-progress: true

Only the last simultaneous trigger survives → one workflow_run → one review. Comment events use the comment ID as suffix so distinct replies are never dropped by another comment on the same PR.

Changes

• Added .github/workflows/pr-review-trigger.yml — the canonical fork-PR trigger workflow with the concurrency group included. This is the template consumers use (documented in .agents/skills/add-pr-reviewer-to-repo/SKILL.md section 4b).

[docker-agent]

Assessment: 🟢 APPROVE

The concurrency group fix is well-designed. The key expression correctly groups simultaneous pull_request events for the same PR under one concurrency key (sharing the review-request suffix) while preserving independent groups for distinct review comments (keyed by comment.id). This cleanly solves the triplicate-review problem described in the PR.

One low-severity observation surfaced by the analysis: see the inline comment on the cancel-in-progress line.

[docker-agent]

[LOW] cancel-in-progress: true may cancel a run mid-artifact-upload, leaving downstream review-pr.yml without the context artifact

The concurrency group correctly cancels older in-flight runs when a newer trigger arrives for the same PR. However, if a subsequent pull_request event for the same PR arrives while the current winning run's "Upload context" step is still in-flight, that run would be cancelled mid-upload.

The downstream review-pr.yml is triggered via workflow_run which fires on completed — including cancelled runs. If GitHub marks a cancellation-mid-upload run as completed, the downstream workflow could fire but find the artifact missing or empty, losing the PR context (PR number, head SHA, event name) it needs to proceed.

The risk is low in practice (the window is narrow and collisions on the exact same concurrency group after the job starts are unusual), and the verifier notes that GitHub Actions typically only fires workflow_run for successful completions in most configurations. However, the downstream workflow should defensively handle a missing or empty artifact (e.g., exit early with a clear error) so that a mis-triggered run fails loudly rather than silently reviewing the wrong PR.

Suggestion: In review-pr.yml's artifact-download step, add an explicit check that the downloaded context files are non-empty and that pr_number.txt contains a valid number before proceeding.

[derekmisler]

Good catch on the cancellation window. review-pr.yml already has two guards that cover this: the resolve-context job's Read context step checks [ ! -f /tmp/context/event_name.txt ] || [ ! -f /tmp/context/pr_number.txt ] and exits 1 on missing files (line ~142), and the review job's Resolve PR number step validates the value is a positive integer before any downstream work starts (line ~224). A mid-upload cancellation would result in a failed resolve-context job, not a silent wrong-PR review. The risk you describe — workflow_run firing on a cancelled run and finding the artifact missing — is also bounded by GitHub only triggering workflow_run when the calling workflow reaches a terminal state, and the existing checks fail loudly rather than silently in that edge case.

[docker-agent]

I was wrong about the missing defensive checks — thank you for the detailed correction! I should have read review-pr.yml more carefully before flagging this. The existing guards (failing on missing files at ~142 and validating the PR number format at ~224) already handle the edge case I described, ensuring a mid-upload cancellation results in a loud failure in resolve-context rather than silent misbehavior downstream.

I'll retract this finding. The concurrency configuration is safe as-is.