diff --git a/.github/workflows/qwen-code-pr-review.yml b/.github/workflows/qwen-code-pr-review.yml index fd6f66c8d47..6b683dd3794 100644 --- a/.github/workflows/qwen-code-pr-review.yml +++ b/.github/workflows/qwen-code-pr-review.yml @@ -1756,6 +1756,65 @@ jobs: echo "Fallback comment dedup lookup failed; deferring to the fallback-comment job." >> "$GITHUB_STEP_SUMMARY" exit 0 fi + # Same guard as the fallback-comment job's, for the same reason: a + # review posted moments before this step runs makes every body + # below — each one ending in a retry instruction — contradict the + # review already on the PR. Scoped to the bot's own account and to a + # submission at or after this run was CREATED, so a stale review from + # an earlier run cannot silence a genuinely dead one; an unavailable + # creation time declines to fire and posts. + # What a match proves, exactly: a bot review of this PR was + # submitted while this run was alive. It is deliberately NOT keyed on + # the reviewed head. Two revisions of this guard were, and the head + # is not a stable attribute of a run: a push moves the PR's head + # between the post and this step, and a re-run recomputes the + # reviewed head from a later attempt — in both, THIS run's own review + # stops matching and the contradictory comment ships. The window is + # anchored on `createdAt`, not `startedAt`, against the same class of + # drift: re-running a failed job keeps the run id (the dedup above + # relies on that) while run-level `startedAt` moves to the + # re-executed attempt — measured on runs 32219268680 (created + # 05:23:57Z, startedAt 05:51:26Z) and 32218596441 (05:13:04Z → + # 05:22:05Z). + # + # Under this workflow's per-run concurrency an overlapping run's + # review can also fall inside the window, and then this run's failure + # goes unannounced. Accepted: that silence coincides with a bot + # review of this PR a reader can see, which is exactly the state that + # makes this comment's claim false. What the bot-author and + # creation-time clauses rule out is silence with NO review at all. + # + # The account is not this pipeline's alone: finalize-release.yml, + # qwen-triage-finalize.yml, and the triage skill all post approvals + # under it. Excluding those bodies by name cannot be finished — it + # shipped missing one ("LGTM, looks ready to ship. ✅"), and any + # producer rewording fails in the dangerous direction: a foreign + # LGTM buys silence for a genuinely dead run. So the filter matches + # positively on what only this pipeline's composed reviews carry: + # every composed body ends in the "via Qwen Code /review" + # attribution footer or carries the invisible qwen-review-ledger + # marker — at least one rides every body, a zero-findings APPROVE + # included — and no foreign approval carries either. A marker that + # ever changes shape stops the guard firing and the comment posts: + # the pre-guard status quo, not a masked dead run. + run_created="$(gh run view "${GITHUB_RUN_ID:?}" --repo "$GITHUB_REPOSITORY" --json createdAt --jq '.createdAt' 2>/dev/null)" || run_created="" + posted_reviews="" + # Three outcomes, and the guard must not be silent about the third: + # a lookup that DIED degrades to the false comment this whole change + # removes, and an oncall reading the log could not tell it from "no + # review matched". Every sibling lookup in this step announces its + # failures; this one says so too, then posts. + if [ -z "$run_created" ]; then + echo "::warning::already-posted guard unavailable (no run creation time); posting the fallback comment" + echo "Already-posted guard unavailable (run creation time missing); proceeding to post." >> "$GITHUB_STEP_SUMMARY" + elif ! posted_reviews="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --paginate \ + --jq ".[] | select(.user.login == \"$bot_login\") | select(.submitted_at >= \"$run_created\") | select((.body // \"\") | contains(\"via Qwen Code /review\") or contains(\"qwen-review-ledger\")) | .id" 2>/dev/null)"; then + echo "::warning::already-posted guard unavailable (reviews listing failed); posting the fallback comment" + echo "Already-posted guard unavailable (reviews listing failed); proceeding to post." >> "$GITHUB_STEP_SUMMARY" + elif [ -n "$posted_reviews" ]; then + echo "Skipping fallback comment: a bot review of this PR was submitted after this run was created." >> "$GITHUB_STEP_SUMMARY" + exit 0 + fi MAX_TIMEOUT_MINUTES="${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}" if [ "$FAILURE_KIND" = "timeout" ]; then if [ "$TIMEOUT_MINUTES" -lt "$MAX_TIMEOUT_MINUTES" ]; then @@ -1944,6 +2003,67 @@ jobs: echo "Skipping fallback comment: PR #${PR_NUMBER} is ${pr_state}." >> "$GITHUB_STEP_SUMMARY" exit 0 fi + # A run that DID post its review must not be announced as one that + # could not. The review job can fail AFTER the post — the CLI exiting + # silently, a cleanup step dying — and this body's claim ("failed + # before a review could be posted"), with its retry instruction, then + # contradicts the review sitting right above it. Measured on PR + # #9342: the review posted at 11:56:34Z, the job failed at 12:00:53Z, + # and this comment landed at 12:01:00Z asking for a fresh ~3-hour + # review; the autofix takeover loop reads the same feed a human does. + # + # What a match proves, exactly: a bot review of this PR was + # submitted while this run was alive. It is deliberately NOT keyed on + # the reviewed head. Two revisions of this guard were, and the head + # is not a stable attribute of a run: a push moves the PR's head + # between the post and this step, and a re-run recomputes the + # reviewed head from a later attempt — in both, THIS run's own review + # stops matching and the contradictory comment ships. The window is + # anchored on `createdAt`, not `startedAt`, against the same class of + # drift: re-running a failed job keeps the run id (the dedup above + # relies on that) while run-level `startedAt` moves to the + # re-executed attempt — measured on runs 32219268680 (created + # 05:23:57Z, startedAt 05:51:26Z) and 32218596441 (05:13:04Z → + # 05:22:05Z). + # + # Under this workflow's per-run concurrency an overlapping run's + # review can also fall inside the window, and then this run's failure + # goes unannounced. Accepted: that silence coincides with a bot + # review of this PR a reader can see, which is exactly the state that + # makes this comment's claim false. What the bot-author and + # creation-time clauses rule out is silence with NO review at all. + # + # The account is not this pipeline's alone: finalize-release.yml, + # qwen-triage-finalize.yml, and the triage skill all post approvals + # under it. Excluding those bodies by name cannot be finished — it + # shipped missing one ("LGTM, looks ready to ship. ✅"), and any + # producer rewording fails in the dangerous direction: a foreign + # LGTM buys silence for a genuinely dead run. So the filter matches + # positively on what only this pipeline's composed reviews carry: + # every composed body ends in the "via Qwen Code /review" + # attribution footer or carries the invisible qwen-review-ledger + # marker — at least one rides every body, a zero-findings APPROVE + # included — and no foreign approval carries either. A marker that + # ever changes shape stops the guard firing and the comment posts: + # the pre-guard status quo, not a masked dead run. + run_created="$(gh run view "${GITHUB_RUN_ID:?}" --repo "$GITHUB_REPOSITORY" --json createdAt --jq '.createdAt' 2>/dev/null)" || run_created="" + posted_reviews="" + # Three outcomes, and the guard must not be silent about the third: + # a lookup that DIED degrades to the false comment this whole change + # removes, and an oncall reading the log could not tell it from "no + # review matched". Every sibling lookup in this step announces its + # failures; this one says so too, then posts. + if [ -z "$run_created" ]; then + echo "::warning::already-posted guard unavailable (no run creation time); posting the fallback comment" + echo "Already-posted guard unavailable (run creation time missing); proceeding to post." >> "$GITHUB_STEP_SUMMARY" + elif ! posted_reviews="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --paginate \ + --jq ".[] | select(.user.login == \"$bot_login\") | select(.submitted_at >= \"$run_created\") | select((.body // \"\") | contains(\"via Qwen Code /review\") or contains(\"qwen-review-ledger\")) | .id" 2>/dev/null)"; then + echo "::warning::already-posted guard unavailable (reviews listing failed); posting the fallback comment" + echo "Already-posted guard unavailable (reviews listing failed); proceeding to post." >> "$GITHUB_STEP_SUMMARY" + elif [ -n "$posted_reviews" ]; then + 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})." body="$(printf '%s\n\n%s' "$FALLBACK_MARKER" "$body")" gh pr comment "$PR_NUMBER" \ diff --git a/scripts/tests/qwen-pr-review-workflow.test.js b/scripts/tests/qwen-pr-review-workflow.test.js index 3fd5f6cbd82..55f2a27c767 100644 --- a/scripts/tests/qwen-pr-review-workflow.test.js +++ b/scripts/tests/qwen-pr-review-workflow.test.js @@ -3216,6 +3216,20 @@ describe('fallback comment resilience (PR #8894 incident class)', () => { expect(r.status).toBe(0); }); + // The stub answers the guard's reviews and run-view lookups by running the + // caller's own `--jq` filter — that filter IS the thing under test — so + // these cases need jq on PATH. Windows runners have none, and a stub that + // silently produced nothing there would report the guard as broken rather + // than untested. Probed once, skipped honestly. + const hasJq = (() => { + try { + execFileSync('jq', ['--version'], { stdio: 'ignore' }); + return true; + } catch { + return false; + } + })(); + // Executed shape: run the step's REAL bash with a stub gh that logs every // call. The stub pre-applies the dedup filter's semantics to the fixture // (the filter's author scope is pinned by the text test above). @@ -3227,6 +3241,9 @@ describe('fallback comment resilience (PR #8894 incident class)', () => { runHead = '', prHead = '', useInJobStep = false, + reviews = '[]', + runCreated = '', + runStartedAttempt = '', } = {}, ) { const dir = mkdtempSync(join(tmpdir(), 'fallback-comment-')); @@ -3253,14 +3270,36 @@ describe('fallback comment resilience (PR #8894 incident class)', () => { '#!/bin/bash', 'echo "gh $*" >> "$CALLS"', 'cmd="$1"; sub="${2:-}"', + // Hoisted: both the run-view and the reviews branches run the + // caller's own --jq, so the extraction cannot live inside one of them. + 'filter=""; prev=""', + 'for a in "$@"; do if [ "$prev" = "--jq" ]; then filter="$a"; fi; prev="$a"; done', 'if [ "$cmd" = "api" ] && [ "$sub" = "user" ]; then', ' [ "${SCENARIO:-}" = "lookup_fail" ] && exit 1', ' echo "qwen-code-ci-bot"; exit 0', 'fi', 'if [ "$cmd" = "run" ] && [ "$sub" = "view" ]; then', + ' case "$*" in', + ' *createdAt*|*startedAt*)', + ' [ "${SCENARIO:-}" = "runstart_fail" ] && exit 1', + // Real --jq over an object carrying BOTH fields, exactly as the + // reviews stub does: a `case` on "$*" answers a combined + // `--json createdAt,startedAt --jq .startedAt` from whichever + // substring branch comes first, so the discriminator between the + // two anchors would silently stop discriminating. + ' printf \'{"createdAt":"%s","startedAt":"%s"}\' "${RUN_CREATED:-}" "${RUN_STARTED_ATTEMPT:-}" | jq -r "$filter"; exit 0 ;;', + ' esac', ' [ "${SCENARIO:-}" = "runview_fail" ] && exit 1', ' echo "${RUN_HEAD:-}"; exit 0', 'fi', + // The reviews lookup runs the step's REAL --jq filter over the + // fixture: the guard under test IS that filter (author scope and + // submission time — no head clause, which `attributes by TIME, not + // by head` pins), so a stub that pre-applied it would pin nothing. + 'if [ "$cmd" = "api" ] && [ "${sub#repos/}" != "$sub" ]; then', + ' [ "${SCENARIO:-}" = "reviews_fail" ] && exit 1', + ' printf "%s" "$REVIEWS_JSON" | jq -r "$filter"; exit 0', + 'fi', 'if [ "$cmd" = "pr" ] && [ "$sub" = "view" ]; then', ' case "$*" in', ' *comments*)', @@ -3268,13 +3307,17 @@ describe('fallback comment resilience (PR #8894 incident class)', () => { ' cat "$COMMENTS_FILE"; exit 0 ;;', ' *state,headRefOid*)', ' [ "${SCENARIO:-}" = "state_fail" ] && exit 1', - ' printf "OPEN\\t%s\\n" "${PR_HEAD:-}"; exit 0 ;;', - ' *headRefOid*)', - ' [ "${SCENARIO:-}" = "prview_fail" ] && exit 1', - ' echo "${PR_HEAD:-}"; exit 0 ;;', + ' state=OPEN; [ "${SCENARIO:-}" = "pr_closed" ] && state=MERGED', + ' printf "%s\\t%s\\n" "$state" "${PR_HEAD:-}"; exit 0 ;;', + // Live again: the fallback job reverted to a state-only query when + // the guard stopped keying on the head, so this branch has a caller + // once more (the in-job step keeps the combined shape above). ' *state*)', ' [ "${SCENARIO:-}" = "state_fail" ] && exit 1', ' [ "${SCENARIO:-}" = "pr_closed" ] && echo "MERGED" || echo "OPEN"; exit 0 ;;', + ' *headRefOid*)', + ' [ "${SCENARIO:-}" = "prview_fail" ] && exit 1', + ' echo "${PR_HEAD:-}"; exit 0 ;;', ' esac', 'fi', 'if [ "$cmd" = "pr" ] && [ "$sub" = "comment" ]; then', @@ -3291,7 +3334,18 @@ describe('fallback comment resilience (PR #8894 incident class)', () => { try { stdout = execFileSync( 'bash', - ['-c', useInJobStep ? inJobStep.run : step.run], + [ + '-c', + // The runner substitutes `${{ vars.* }}` before bash ever sees the + // script; feeding the raw expression to bash is a `bad substitution` + // that skips the assignment and leaves the variable unset — the + // timeout body then compares against an empty string. Substituting + // here is what makes "the step's real bash" true. + (useInJobStep ? inJobStep.run : step.run).replace( + /\$\{\{ vars\.QWEN_REVIEW_MAX_TIMEOUT_MINUTES \}\}/g, + '180', + ), + ], { encoding: 'utf8', env: { @@ -3314,6 +3368,9 @@ describe('fallback comment resilience (PR #8894 incident class)', () => { CALLS: calls, COMMENTS_FILE: commentsFile, POSTED: posted, + REVIEWS_JSON: reviews, + RUN_CREATED: runCreated, + RUN_STARTED_ATTEMPT: runStartedAttempt, }, }, ); @@ -3419,7 +3476,12 @@ describe('fallback comment resilience (PR #8894 incident class)', () => { }); expect(r.status).toBe(0); expect(r.posted).not.toBe(''); - expect(r.calls).not.toContain('run view'); + // Pinned on the head lookup itself, not on `gh run view` as a whole: + // the already-posted guard below asks the same command for this run's + // createdAt on every event, and a blanket "no run view" assertion would + // read that as a head comparison it never makes. + expect(r.calls).not.toContain('headSha'); + expect(r.calls).not.toContain('--json headRefOid'); }); it('degrades to POSTING when the head comparison lookups fail', () => { @@ -3450,6 +3512,285 @@ describe('fallback comment resilience (PR #8894 incident class)', () => { expect(r.posted).not.toBe(''); }); + // A run can fail AFTER posting its review — the CLI exiting silently, a + // cleanup step dying — and both fallback bodies then announce a review + // sitting right above them as one that could not be posted, retry + // instruction attached. Measured on PR #9342: review posted 11:56:34Z, + // review-pr failed 12:00:53Z, the comment landed 12:01:00Z asking for a + // fresh ~3-hour review; the autofix takeover loop reads the same feed a + // human does. The guard is a FILTER (author scope and submission + // time), so these run the step's real bash over review fixtures. + // The run was CREATED at 09:08:38Z; a re-run of its failed job later moved + // run-level startedAt to 11:30:00Z. Attempt 1's review sits between them — + // the shape that separates the two anchors. + const RUN_CREATED = '2026-08-18T09:08:38Z'; + const RUN_RESTARTED = '2026-08-18T11:30:00Z'; + const AFTER = '2026-08-18T11:56:34Z'; + const MID_RERUN = '2026-08-18T10:00:00Z'; + const BEFORE = '2026-08-17T10:00:00Z'; + const reviewFixture = (login, commit, submitted, body = null) => + JSON.stringify([ + { + id: 1, + user: { login }, + commit_id: commit, + submitted_at: submitted, + body, + }, + ]); + + // The bot account posts more than this pipeline's reviews: + // finalize-release.yml approves release PRs under the same CI_BOT_PAT, + // qwen-triage-finalize.yml posts a deferred APPROVE under + // QWEN_CODE_BOT_TOKEN || CI_BOT_PAT, and the triage skill posts its own + // commit-pinned APPROVE through the reviews API. In-window approvals like + // these must not buy the silence that only THIS pipeline's own review + // earns. + const FOREIGN_APPROVAL_BODIES = [ + 'Automated second approval for the release version bump.', + 'LGTM, looks ready to ship — CI landed green after the review. ✅', + 'LGTM, looks ready to ship. ✅', + ]; + + // What the guard recognizes a review THIS pipeline composed by: every + // composed body carries the "via Qwen Code /review" attribution footer or + // the invisible qwen-review-ledger marker — at least one, never neither — + // and no foreign approval carries either. Matching on that evidence is how + // the guard stays closed to a producer set no exclusion list can finish. + const REVIEW_FOOTER = '_— qwen3.8-max via Qwen Code /review (v0.21.14)_'; + const REVIEW_LEDGER = ''; + const COMPOSED_REVIEW_BODIES = [ + // Attribution on: the footer and the ledger marker both ride the body. + `No issues found. LGTM! ✅\n\n${REVIEW_FOOTER}\n\n${REVIEW_LEDGER}`, + // Attribution off: no footer, but the ledger marker still rides. + `No issues found. LGTM! ✅\n\n${REVIEW_LEDGER}`, + // Pre-ledger bundles posted the footer alone. + `No issues found. LGTM! ✅\n\n${REVIEW_FOOTER}`, + ]; + + for (const useInJobStep of [false, true]) { + const site = useInJobStep ? 'in-job step' : 'fallback job'; + + it.skipIf(!hasJq)( + `${site} stays silent when THIS run already posted its review`, + () => { + // Every shape compose-review can post must buy the silence: the + // guard attributes by the markers a composed body carries, so each + // marker alone — and both together — has to match. + for (const body of COMPOSED_REVIEW_BODIES) { + const r = runFallbackStep('default', { + useInJobStep, + prHead: 'HEADSHA1', + runCreated: RUN_CREATED, + runStartedAttempt: RUN_RESTARTED, + reviews: reviewFixture('qwen-code-ci-bot', 'HEADSHA1', AFTER, body), + }); + expect(r.status, body).toBe(0); + expect(r.posted, body).toBe(''); + expect(r.summary, body).toContain( + 'a bot review of this PR was submitted', + ); + } + }, + ); + + it.skipIf(!hasJq)( + `${site} still posts when no review can be attributed to this run`, + () => { + // Each clause alone must keep the fallback speaking, or a stale or + // foreign review buys silence on a genuinely dead pipeline: an earlier + // run's review (outside the window), another account's, an unsubmitted + // (PENDING) one, and none at all. The head is deliberately not a clause + // — see the attribute-by-TIME test below. + const cases = { + stale: reviewFixture( + 'qwen-code-ci-bot', + 'HEADSHA1', + BEFORE, + COMPOSED_REVIEW_BODIES[0], + ), + foreign: reviewFixture( + 'someone-else', + 'HEADSHA1', + AFTER, + COMPOSED_REVIEW_BODIES[0], + ), + pending: reviewFixture( + 'qwen-code-ci-bot', + 'HEADSHA1', + null, + COMPOSED_REVIEW_BODIES[0], + ), + none: '[]', + }; + for (const [name, reviews] of Object.entries(cases)) { + const r = runFallbackStep('default', { + useInJobStep, + prHead: 'HEADSHA1', + runCreated: RUN_CREATED, + runStartedAttempt: RUN_RESTARTED, + reviews, + }); + expect(r.posted, name).not.toBe(''); + } + }, + ); + + it.skipIf(!hasJq)( + `${site} still posts when the only in-window reviews are foreign approvals`, + () => { + // The guard's author + window clauses match ANY review the account + // posts, and the account also approves release PRs (finalize-release + // .yml), posts deferred triage approvals (qwen-triage-finalize.yml), + // and approves through the triage skill's reviews-API call. None of + // these bodies carries a composed-review marker, so none may silence + // the fallback while THIS pipeline's review is absent — the LGTM + // would mask a dead run. + for (const body of FOREIGN_APPROVAL_BODIES) { + const r = runFallbackStep('default', { + useInJobStep, + prHead: 'HEADSHA1', + runCreated: RUN_CREATED, + runStartedAttempt: RUN_RESTARTED, + reviews: reviewFixture('qwen-code-ci-bot', 'HEADSHA1', AFTER, body), + }); + expect(r.posted, body).not.toBe(''); + } + }, + ); + + it.skipIf(!hasJq)( + `${site} survives a job re-run: attempt 1's review still silences it`, + () => { + // Re-running a failed job keeps the run id but moves run-level + // startedAt to the re-executed attempt (measured: runs 32219268680 and + // 32218596441 report startedAt ~28 and ~9 minutes after createdAt). + // Anchored there, attempt 1's review reads as older than "this run", + // and a re-run that fails before posting contradicts it — the very + // shape this guard exists to stop. The stub answers createdAt and + // startedAt with DIFFERENT values, so this fails if the guard reads + // the wrong field. + const r = runFallbackStep('default', { + useInJobStep, + prHead: 'HEADSHA1', + runCreated: RUN_CREATED, + runStartedAttempt: RUN_RESTARTED, + reviews: reviewFixture( + 'qwen-code-ci-bot', + 'HEADSHA1', + MID_RERUN, + COMPOSED_REVIEW_BODIES[0], + ), + }); + expect(r.status).toBe(0); + expect(r.posted).toBe(''); + expect(r.summary).toContain('a bot review of this PR was submitted'); + }, + ); + + it.skipIf(!hasJq)( + `${site} says so in the log when the guard cannot run`, + () => { + // A lookup that DIED degrades to the false comment this change + // removes, and silence there leaves an oncall unable to tell it from + // "no review matched". Both unavailable paths announce themselves. + for (const scenario of ['runstart_fail', 'reviews_fail']) { + const r = runFallbackStep(scenario, { + useInJobStep, + prHead: 'HEADSHA1', + runCreated: RUN_CREATED, + runStartedAttempt: RUN_RESTARTED, + reviews: reviewFixture('qwen-code-ci-bot', 'HEADSHA1', AFTER), + }); + expect(r.posted, scenario).not.toBe(''); + expect(r.stdout, scenario).toContain( + '::warning::already-posted guard', + ); + expect(r.summary, scenario).toContain( + 'Already-posted guard unavailable', + ); + } + }, + ); + + it.skipIf(!hasJq)( + `${site} posts when this run's creation time is unavailable`, + () => { + // Without a start time there is no proof the review landed during THIS + // run, and posting wins over silence — the same call the head-moved + // guard makes when its comparison is unavailable. + const r = runFallbackStep('runstart_fail', { + useInJobStep, + prHead: 'HEADSHA1', + runCreated: RUN_CREATED, + runStartedAttempt: RUN_RESTARTED, + reviews: reviewFixture('qwen-code-ci-bot', 'HEADSHA1', AFTER), + }); + expect(r.posted).not.toBe(''); + }, + ); + + it.skipIf(!hasJq)( + `${site} posts when the reviews lookup itself fails`, + () => { + // Same direction as every other lookup this step makes for a SKIP + // decision: a failed listing is never read as "a review exists". + const r = runFallbackStep('reviews_fail', { + useInJobStep, + prHead: 'HEADSHA1', + runCreated: RUN_CREATED, + runStartedAttempt: RUN_RESTARTED, + reviews: reviewFixture('qwen-code-ci-bot', 'HEADSHA1', AFTER), + }); + expect(r.posted).not.toBe(''); + }, + ); + } + + it.skipIf(!hasJq)( + "attributes by TIME, not by head — a moved head cannot hide this run's review", + () => { + // The head is not a stable attribute of a run: a push moves the PR's head + // between the post and this step, and a re-run recomputes the reviewed + // head from a later attempt. Two revisions of this guard keyed on it and + // both re-opened the #9342 contradiction through one of those doors. What + // the guard proves now is narrower and stable — a bot review of this PR + // submitted while this run was alive — so a review on ANY head inside the + // window silences the comment. + for (const useInJobStep of [false, true]) { + const r = runFallbackStep('default', { + useInJobStep, + prHead: 'NEWSHA', + runCreated: RUN_CREATED, + runStartedAttempt: RUN_RESTARTED, + reviews: reviewFixture( + 'qwen-code-ci-bot', + 'OLDSHA', + AFTER, + COMPOSED_REVIEW_BODIES[0], + ), + }); + expect(r.status, String(useInJobStep)).toBe(0); + expect(r.posted, String(useInJobStep)).toBe(''); + expect(r.summary, String(useInJobStep)).toContain( + 'a bot review of this PR was submitted after this run was created', + ); + } + }, + ); + + it('carries no cross-job head wiring to drift', () => { + // An earlier revision published review-pr's reviewed head as a job output + // and read it here. The guard no longer keys on the head at all, so the + // wiring is gone rather than left as an untested chain whose silent + // breakage would restore the fresh-head comparison. + expect(doc.jobs['review-pr'].outputs).toBeUndefined(); + expect(step.env.REVIEWED_HEAD_SHA).toBeUndefined(); + expect(step.run).not.toContain('REVIEWED_HEAD_SHA'); + expect(inJobStep.run).not.toContain('commit_id =='); + expect(step.run).not.toContain('commit_id =='); + }); + it('in-job step dedupes on a fallback comment this run already has', () => { // Re-runs of failed jobs keep the run id: when a prior attempt died // before its in-job step, the fallback-comment job already posted for