Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions .github/workflows/qwen-code-pr-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +1814 to +1817

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This skip message claims "this run already posted a review", but the guard's window (bot account + head + submitted_at >= createdAt) also matches a SIBLING run's review — reviews carry no run id, so the guard cannot distinguish this run's post from another run's, and the workflow's own concurrency comment acknowledges overlapping same-head runs ("an occasional duplicate review"). The suppression itself is correct — posting the fallback body would contradict the review sitting above it; only the attribution is wrong: an oncall investigating a silent dead run reads in this step's summary that THIS run posted when it never did. Suggest rewording to what the guard actually proved, here and in the fallback-comment job's twin.

Suggested change
elif [ -n "$posted_reviews" ]; then
echo "Skipping fallback comment: this run already posted a review on ${current_head}." >> "$GITHUB_STEP_SUMMARY"
exit 0
fi
elif [ -n "$posted_reviews" ]; then
echo "Skipping fallback comment: a bot review already exists on ${current_head} submitted at or after this run was created." >> "$GITHUB_STEP_SUMMARY"
exit 0
fi
中文说明

【Suggestion】 这条跳过消息声称「this run already posted a review」,但守卫的窗口(bot 账号 + head + submitted_at >= createdAt)同样会匹配兄弟运行发布的 review——review 上不携带 run id,守卫无法区分本次运行的发布与另一次运行的发布,而本 workflow 自己的并发注释也承认同 head 运行会重叠(「an occasional duplicate review」)。静默本身是正确的——此时再发兜底正文会与上方的 review 自相矛盾;只是归属错了:值班排查一次静默的死运行,会在这一步的 summary 里读到「本次运行已发布过 review」,而它从未发布。建议把措辞改为守卫实际证明的事实,此处与 fallback-comment 作业中的孪生副本同步修改。

— qwen3.8-max via Qwen Code /review (v0.21.14)

MAX_TIMEOUT_MINUTES="${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}"
if [ "$FAILURE_KIND" = "timeout" ]; then
if [ "$TIMEOUT_MINUTES" -lt "$MAX_TIMEOUT_MINUTES" ]; then
Expand Down Expand Up @@ -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
Comment on lines +2063 to +2066

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Same attribution wording as the in-job twin: "this run already posted a review" — but the guard's window also matches a sibling run's review (reviews carry no run id), so this step summary can tell the oncall that THIS run posted when the review actually came from an overlapping run. The suppression behavior stays correct; reword the message to what the guard proved.

Suggested change
elif [ -n "$posted_reviews" ]; then
echo "Skipping fallback comment: this run already posted a review on ${pr_head}." >> "$GITHUB_STEP_SUMMARY"
exit 0
fi
elif [ -n "$posted_reviews" ]; then
echo "Skipping fallback comment: a bot review already exists on ${pr_head} submitted at or after this run was created." >> "$GITHUB_STEP_SUMMARY"
exit 0
fi
中文说明

【Suggestion】 与 in-job 那份副本相同的归属措辞:「this run already posted a review」——但守卫的窗口同样会匹配兄弟运行的 review(review 不携带 run id),因此这条 step summary 可能告诉值班「本次运行已发布过 review」,而那条 review 实际来自一次重叠的运行。静默行为本身正确;只需把消息改为守卫实际证明的事实。

— qwen3.8-max via Qwen Code /review (v0.21.14)

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" \
Expand Down
Loading
Loading