fix(review): stop the fallback comment from firing on superseded runs

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

@@ -1903,12 +1903,28 @@ jobs:
   # re-runs, which keep the same run id. A review-pr that dies to its own
   # job-level timeout is auto-CANCELLED by GitHub — result 'cancelled' and
   # failure() false — which opens neither a failure-only gate nor the in-job
-  # step, so the gate admits 'cancelled' too; a run-level cancel cancels this
-  # queued job with it, so a live gate evaluation seeing 'cancelled' is
-  # overwhelmingly the timeout case, and the residual manual single-job
-  # cancel just gets a benign retry-guidance comment. The PR number comes
-  # from the event payload, not the dead job's outputs, which do not survive
-  # a crash.
+  # step, so the gate admits 'cancelled' — but only when the upstream chain
+  # finished. `always()` keeps this job running through a RUN-level cancel
+  # (it does not die with the run), so a concurrency supersede used to post
+  # a false "did not complete" while the surviving run was still reviewing:
+  # on PR #9131 a same-head pull_request_target pair started 1s apart, the
+  # newer run cancelled the older inside authorize, and the older run's gate
+  # saw review-pr 'cancelled' (run 32558544379) — same-head, so the in-step
+  # head-moved guard could not catch it. The two cancels are separable in
+  # `needs`: a job-level timeout cancels review-pr ALONE — authorize and
+  # delay-automatic-review completed long before — while a run-level cancel
+  # sweeps the whole chain, so 'cancelled' opens the gate only when neither
+  # upstream job was itself cancelled. A run-level cancel landing AFTER the
+  # chain finished (mid-review) still opens the gate: the push-supersede
+  # flavor is then suppressed by the in-step head-moved guard, the close
+  # flavor (a `closed`-action run joining the PR-scoped group hours in, the
+  # head unchanged) by the in-step PR-state check, and a same-head twin
+  # cannot land that late — its cancel fires at run creation, seconds in.
+  # A manual run-cancel during the delay window goes
+  # silent under this rule (the person who cancelled does not need retry
+  # guidance); a manual cancel of review-pr alone mid-review still posts,
+  # benign as before. The PR number comes from the event payload, not the
+  # dead job's outputs, which do not survive a crash.
   fallback-comment:
     needs:
       [
@@ -1921,7 +1937,9 @@ jobs:
     if: |-
       always() &&
       (needs.review-pr.result == 'failure' ||
-       needs.review-pr.result == 'cancelled' ||
+       (needs.review-pr.result == 'cancelled' &&
+        needs.authorize.result != 'cancelled' &&
+        needs.delay-automatic-review.result != 'cancelled') ||
        needs.authorize.result == 'failure' ||
        needs.review-config.result == 'failure' ||
        needs.delay-automatic-review.result == 'failure' ||

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

@@ -3069,9 +3069,28 @@ describe('fallback comment resilience (PR #8894 incident class)', () => {
     expect(job.if).toContain("needs.review-pr.result == 'failure'");
     // A review-pr that dies to its own job-level timeout is auto-cancelled
     // (result 'cancelled', failure() false), which would open neither this
-    // gate nor the in-job step; a run-level cancel cancels this queued job
-    // too, so a live evaluation seeing 'cancelled' is the timeout case.
-    expect(job.if).toContain("needs.review-pr.result == 'cancelled'");
+    // gate nor the in-job step — but `always()` keeps this job alive through
+    // a RUN-level cancel, so a bare 'cancelled' clause posts a false "did
+    // not complete" from a concurrency-superseded run while its same-head
+    // twin is still reviewing (PR #9131, run 32558544379 — same head, so
+    // the in-step head-moved guard cannot catch it). The two cancels differ
+    // in `needs`: a timeout cancels review-pr ALONE, a run-level cancel
+    // sweeps the upstream chain too. Pin the full compound clause — the
+    // grouping included — so reverting either upstream conjunct fails here.
+    expect(job.if).toContain(
+      "(needs.review-pr.result == 'cancelled' &&\n" +
+        "  needs.authorize.result != 'cancelled' &&\n" +
+        "  needs.delay-automatic-review.result != 'cancelled') ||",
+    );
+    // ...and that clause must be the ONLY place the gate tests review-pr
+    // for 'cancelled': a merge-conflict resolution keeping both sides
+    // re-adds a bare `== 'cancelled'` disjunct beside the intact compound
+    // clause — in any rendering (bare, parenthesized, respaced) — and the
+    // gate opens on every cancelled review-pr again while the pin above
+    // stays green. Counting occurrences catches every rendering.
+    expect(
+      job.if.match(/needs\.review-pr\.result == 'cancelled'/g),
+    ).toHaveLength(1);
     expect(job.if).toContain("needs.authorize.result == 'failure'");
     expect(job.if).toContain("needs.review-config.result == 'failure'");
     expect(job.if).toContain(