fix(ci): keep no-op review requests out of the PR review concurrency group

.github/workflows/qwen-code-pr-review.yml

@@ -50,10 +50,23 @@ on:
 
 concurrency:
   # PR lifecycle events share a PR-scoped group so new pushes restart the delay
-  # and closed PRs stop any in-flight lifecycle review.
-  # Comment/review events use per-run groups to avoid cancelling active reviews.
+  # and closed PRs stop any in-flight lifecycle review. Every review_requested
+  # run — the bot-directed one included — gets a per-run group: membership is
+  # decided here, before `authorize` runs, but whether a bot request reviews
+  # anything is `authorize`'s call on the REQUESTER's write permission. A
+  # requester without write produces a guaranteed all-skipped run, and as a
+  # shared-group member that no-op can supersede a lifecycle run sitting
+  # PENDING behind a still-terminating review — a pending run is replaced by
+  # any newer run in the group, cancel-in-progress notwithstanding. That is
+  # the exact race that lost the automatic review on PR #9091, left open for
+  # anyone who can request the bot without write permission. The per-run group
+  # costs only an occasional duplicate review when an authorized bot request
+  # lands while the lifecycle run for the same head still queues: compute,
+  # never a lost review. Comment/review events use per-run groups to avoid
+  # cancelling active reviews.
   group: >-
     ${{ github.event_name == 'pull_request_target' &&
+    github.event.action != 'review_requested' &&
     format('qwen-pr-review-pr-{0}', github.event.pull_request.number) ||
     format('qwen-pr-review-run-{0}', github.run_id) }}
   cancel-in-progress: "${{ github.event_name == 'pull_request_target' && (github.event.action == 'synchronize' || github.event.action == 'closed') }}"

scripts/tests/qwen-pr-review-workflow.test.js

@@ -31,6 +31,13 @@ const workflowFiles = readdirSync(workflowsDir).filter((f) =>
   /\.ya?ml$/.test(f),
 );
 
+// Single shared recipe for the review-config bot login; every suite that pins
+// it reads this constant so the extraction cannot drift between call sites.
+const botLogin =
+  parse(workflow)
+    .jobs['review-config'].steps.find((s) => s.name === 'Set review constants')
+    ?.run.match(/bot_login=([A-Za-z0-9-]+)/)?.[1] ?? '';
+
 function runReviewStep() {
   const doc = parse(workflow);
   const step = doc.jobs['review-pr'].steps.find((s) => s.name === 'Run review');
@@ -2518,6 +2525,50 @@ describe('bot comment markers', () => {
   });
 });
 
+describe('qwen pr review concurrency routing', () => {
+  // A PENDING run in a concurrency group is replaced by any newer run of the
+  // same group — cancel-in-progress does not protect it. On PR #9091 a push
+  // and three human review requests landed in the same minute: the
+  // synchronize run cancelled the in-flight review but queued behind it while
+  // it terminated, and the review_requested runs — no-ops, since review-pr
+  // only runs when the bot itself is the requested reviewer — superseded it
+  // while pending. The sole survivor skipped review-pr, so the push was never
+  // reviewed. Routing only the human-requested siblings to a per-run group
+  // leaves the race open: group membership is fixed here, before `authorize`
+  // runs, but whether a bot-directed request reviews anything is `authorize`'s
+  // call on the REQUESTER's write permission — a requester without write
+  // produces a guaranteed all-skipped run, and as a shared-group member that
+  // no-op can supersede the pending lifecycle run, losing the review the same
+  // way. Every review_requested run therefore gets a per-run group; an
+  // authorized bot request still reviews immediately, it just can no longer
+  // supersede the lifecycle run for the same head.
+  const group = parse(workflow).concurrency.group;
+
+  it('keeps every review_requested run out of the shared PR group', () => {
+    // Verbatim, like the cancel-in-progress pin in the resolve suite: the
+    // shape IS the fix — `&&` binds tighter than `||`, so exactly the
+    // lifecycle actions reach the PR group and every review_requested (bot
+    // included) falls through to the per-run group. Any requested_reviewer
+    // clause here re-admits the unauthorized-requester no-op and silently
+    // re-ships the race.
+    expect(group).toBe(
+      "${{ github.event_name == 'pull_request_target' && " +
+        "github.event.action != 'review_requested' && " +
+        "format('qwen-pr-review-pr-{0}', github.event.pull_request.number) || " +
+        "format('qwen-pr-review-run-{0}', github.run_id) }}",
+    );
+  });
+
+  it('gates the review_requested jobs on the published bot login', () => {
+    // The group expression no longer names the requested reviewer; the
+    // job-level gates still must. Pin the surviving literal against the
+    // review-config constant so a bot rename cannot desync the sites.
+    expect(parse(workflow).jobs['precheck-pr'].if).toContain(
+      `github.event.requested_reviewer.login == '${botLogin}'`,
+    );
+  });
+});
+
 describe('review_requested burst coalescing (#8945)', () => {
   // Opening a same-repo PR that touches CODEOWNERS-covered paths auto-requests
   // every owner individually, so one PR open emits one `review_requested` run
@@ -2528,11 +2579,6 @@ describe('review_requested burst coalescing (#8945)', () => {
   // siblings at the job level so they complete as instant all-skipped runs.
   const doc = parse(workflow);
 
-  const botLoginMatch = doc.jobs['review-config'].steps[0].run.match(
-    /bot_login=([A-Za-z0-9-]+)/,
-  );
-  const botLogin = botLoginMatch ? botLoginMatch[1] : '';
-
   const botRequestedClause = new RegExp(
     [
       String.raw`\(\s*github\.event_name != 'pull_request_target' \|\|`,