fix(review): give cancelled runs an accurate fallback body instead of the failure comment

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

@@ -2009,9 +2009,18 @@ jobs:
   # 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.
+  # guidance); a cancel landing mid-review still posts, but with the
+  # cancellation body, not the failure one — the step branches on
+  # REVIEW_PR_RESULT. For a cancel nothing failed and nothing retries
+  # automatically, so the failure body's claims are false and read as a
+  # pipeline outage to the PR author (issue #10109: run 32875478404 was
+  # run-cancelled two minutes into the review, no successor run existed,
+  # and the full "pipeline failed" comment posted anyway). The cancelled
+  # case cannot simply go silent: the job-level-timeout flavor — the very
+  # reason 'cancelled' is admitted here — reaches this job as the same
+  # result, so one accurate body covers both flavors. The PR number comes
+  # from the event payload, not the dead job's outputs, which do not
+  # survive a crash.
   fallback-comment:
     needs:
       [
@@ -2046,6 +2055,7 @@ jobs:
         env:
           GH_TOKEN: '${{ secrets.CI_BOT_PAT }}'
           PR_NUMBER: '${{ github.event.pull_request.number || github.event.issue.number || github.event.inputs.pr_number }}'
+          REVIEW_PR_RESULT: '${{ needs.review-pr.result }}'
           RUN_URL: '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}'
         run: |-
           set -uo pipefail
@@ -2169,7 +2179,20 @@ jobs:
             echo "Skipping fallback comment: a bot review of this PR was submitted after this run was created." >> "$GITHUB_STEP_SUMMARY"
             exit 0
           fi
-          body="**Qwen Code review did not complete successfully.** The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with \`@qwen-code /review\`. See [workflow logs](${RUN_URL})."
+          # A cancelled review-pr reaches this job through the gate's
+          # compound clause with two indistinguishable-in-`needs` flavors:
+          # its own job-level timeout (auto-CANCELLED by GitHub), and a
+          # run/job cancel landing after the upstream chain finished. For
+          # neither is the failure body true — nothing failed, nothing is
+          # retried automatically (issue #10109) — so the cancelled case
+          # gets one body accurate for both flavors. It keeps the
+          # `[workflow logs](RUN_URL)` markdown link the cross-job dedup
+          # anchors on, and the retry instruction the timeout flavor needs.
+          if [ "$REVIEW_PR_RESULT" = "cancelled" ]; then
+            body="**Qwen Code review was cancelled before a review could be posted.** Nothing failed and nothing is retried automatically: the run was cancelled — by an operator, an upstream event, or the job exceeding its execution time limit. If you still want a review of this PR, request one with \`@qwen-code /review\`. See [workflow logs](${RUN_URL})."
+          else
+            body="**Qwen Code review did not complete successfully.** The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with \`@qwen-code /review\`. See [workflow logs](${RUN_URL})."
+          fi
           body="$(printf '%s\n\n%s' "$FALLBACK_MARKER" "$body")"
           gh pr comment "$PR_NUMBER" \
             --repo "$GITHUB_REPOSITORY" \

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

@@ -3008,10 +3008,13 @@ describe('fallback comment resilience (PR #8894 incident class)', () => {
     expect(
       inJobStep.run.match(/See \[workflow logs\]\(\$\{RUN_URL\}\)\./g),
     ).toHaveLength(4);
-    // Same invariant for the fallback job's body: the cross-job dedup
-    // matches `actions/runs/<id>)`, anchored on the markdown link's closing
-    // paren — a body that rendered the URL differently would escape it.
-    expect(step.run).toContain('See [workflow logs](${RUN_URL}).');
+    // Same invariant for the fallback job's TWO bodies (failure and
+    // cancelled): the cross-job dedup matches `actions/runs/<id>)`, anchored
+    // on the markdown link's closing paren — a body that rendered the URL
+    // differently would escape it.
+    expect(
+      step.run.match(/See \[workflow logs\]\(\$\{RUN_URL\}\)\./g),
+    ).toHaveLength(2);
     expect(inJobStep.run).toContain(
       `body="$(printf '%s\\n\\n%s' "$FALLBACK_MARKER" "$body")"`,
     );
@@ -3025,6 +3028,16 @@ describe('fallback comment resilience (PR #8894 incident class)', () => {
     ).toBeLessThan(inJobStep.run.indexOf('gh pr comment'));
   });
 
+  it('wires the needs result the cancelled-body branch keys on (issue #10109)', () => {
+    // The step's bash runs under `set -u`, so dropping this env wiring
+    // fails the step loudly instead of silently reverting every cancelled
+    // run to the false "pipeline failed" body.
+    expect(step.env.REVIEW_PR_RESULT).toBe('${{ needs.review-pr.result }}');
+    expect(step.run).toContain(
+      'if [ "$REVIEW_PR_RESULT" = "cancelled" ]; then',
+    );
+  });
+
   it('scopes the dedup to the authenticated bot login, resolved dynamically', () => {
     // upsert-bot-comment.sh protocol: only comments by the authenticated
     // login are dedup targets, or a participant posting the marker suppresses
@@ -3281,6 +3294,7 @@ describe('fallback comment resilience (PR #8894 incident class)', () => {
       reviews = '[]',
       runCreated = '',
       runStartedAttempt = '',
+      reviewPrResult = 'failure',
     } = {},
   ) {
     const dir = mkdtempSync(join(tmpdir(), 'fallback-comment-'));
@@ -3408,6 +3422,7 @@ describe('fallback comment resilience (PR #8894 incident class)', () => {
               REVIEWS_JSON: reviews,
               RUN_CREATED: runCreated,
               RUN_STARTED_ATTEMPT: runStartedAttempt,
+              REVIEW_PR_RESULT: reviewPrResult,
             },
           },
         );
@@ -3434,6 +3449,33 @@ describe('fallback comment resilience (PR #8894 incident class)', () => {
     expect(r.posted).toContain('actions/runs/12345');
   });
 
+  it('posts the cancellation body, not the failure one, for a cancelled review-pr', () => {
+    // Issue #10109: a cancelled review-pr reaches the gate two ways — its
+    // own job-level timeout, and a run/job cancel landing after the
+    // upstream chain finished (run 32875478404) — and for neither is
+    // "pipeline failed / retried automatically" true. Silence would regress
+    // the timeout flavor, so the cancelled case gets its own body.
+    const cancelled = runFallbackStep('default', {
+      reviewPrResult: 'cancelled',
+    });
+    expect(cancelled.status).toBe(0);
+    expect(cancelled.posted.startsWith(`${marker}\n\n`)).toBe(true);
+    expect(cancelled.posted).toContain('cancelled');
+    // The claims the issue calls out must not ride a cancellation...
+    expect(cancelled.posted).not.toContain('did not complete successfully');
+    expect(cancelled.posted).not.toContain('The review pipeline failed');
+    expect(cancelled.posted).not.toContain(
+      'A transient error is retried automatically',
+    );
+    // ...while the `)`-anchored run URL the cross-job dedup matches and the
+    // retry instruction (the job-timeout flavor's reader needs it) stay.
+    expect(cancelled.posted).toContain('actions/runs/12345)');
+    expect(cancelled.posted).toContain('@qwen-code /review');
+    // The failure path keeps the original body.
+    const failed = runFallbackStep('default', { reviewPrResult: 'failure' });
+    expect(failed.posted).toContain('did not complete successfully');
+  });
+
   it('dedupes on the marker plus this run URL', () => {
     // Mirrors a prior fallback comment's rendered shape: the run URL is a
     // markdown link, so the run id is always immediately followed by ')'.