fix(ci): repost the review ack under the command and react to it - #10381
Conversation
The `/review` acknowledgement was edited in place when a previous ack existed, so on a long thread the "review is queued" notice stayed at the position of the FIRST request (comment 2 of 15 on QwenLM#10259) and a requester reading from the bottom concluded nothing had started. Delete the stale ack(s) and post a fresh one so it lands right under the command; react 👀 to the triggering comment; and say in the ack that a command-triggered run executes against the base branch and therefore never shows under the PR's checks — the link is the only way to watch it. Nothing keys on the ack's comment id (qwen-autofix's bot-comment filters and the review's bypass audit match the marker), so recreating is safe. Claude-Session: https://claude.ai/code/session_01GNcnzoA34LZr3rMhsHEcbs
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献！
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewThe approach is the one I'd have picked: replace the PATCH-if-exists branch with delete-all-stale + unconditional post, add a best-effort 👀 on the triggering comment, and pass
One non-blocking note: the old code tolerated a comments-list GET failure ( TestingUnattended CI run — no local execution of PR code; evidence below is the PR's own CI on the reviewed commit, fetched through the API. The check that matters here is Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Not verified: live delete-then-post on a real thread. This is workflow YAML that only executes inside GitHub Actions when a real 中文说明代码审查：方案与我独立想到的做法一致——把"存在则 PATCH"换成"删除所有过期 ack + 无条件发布"，对触发评论加尽力而为的 👀， 测试：无人值守 CI 运行，不执行 PR 代码；以上证据来自 API 拉取的该提交自身 CI。关键 check 是 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — small, focused fix for an observed problem, correct in every spot I checked; one non-blocking nit on a changed failure mode. Going back to my independent proposal — delete the stale ack, post a fresh one under the command, react best-effort — the PR lands exactly there, with details better than I'd have thought to add: it deletes every stale ack rather than just the last (cleaning up any duplicates a PATCH-era failure left behind), and the ack copy now explains why there is no yellow dot to watch. Every edit in the diff serves the stated goal, and the tests pin the regression-prone shape — no PATCH branch creeping back in, no Not 5/5 for one reason: the live delete-then-post only executes when a real Approval deferred until CI lands green on 中文说明回顾：对照我的独立方案——删除过期 ack、在命令下方重新发布、尽力加反应——PR 完全落在这个方案上，且细节更好：删除全部过期 ack 而非仅最新一条（顺带清掉 PATCH 时代失败遗留的重复），ack 文案还解释了为什么没有可关注的"黄点"。diff 中每处改动都服务于既定目标，测试钉住了易回归的形态——PATCH 分支不会悄悄回来、发布前没有 不给 5/5 的原因只有一个：真实的"先删后发"只有在真实 批准将推迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # (qwen-autofix's bot-comment filters, the review's bypass audit) | ||
| # matches the marker. | ||
| # -F would otherwise make gh api default to POST. | ||
| gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" \ |
There was a problem hiding this comment.
[Critical] R1-1: The stale-ack listing pipeline starting at this line is unguarded under set -euo pipefail. The diff removed the old "$( … )" || EXISTING_ACK_ID="" tolerance and re-established it nowhere — the || echo guards cover only the reaction POST above and each DELETE inside the loop. When the paginated GET issues/{n}/comments hits a transient 5xx, timeout, or secondary rate limit, pipefail makes the pipeline non-zero and set -e kills the step before gh pr comment — the requester gets the 👀 (already posted) but no ack, no step-summary explanation, and a red ack job. Pre-change, the same failure still posted a fresh ack; the step's own comment says a failed reaction "must not cost the ack comment below", and the unguarded listing sitting between them does exactly that.
Witness (the step extracted verbatim and driven with a stubbed gh, set -euo pipefail as written):
GET-fail arm: rc=1, call log ends at the listing GET — no `gh pr comment`, empty step summary, 👀 POST already ran
pre-PR arm: rc=0, `gh pr comment` posted under the same GET failure
with guard: rc=0, ack posted — the tolerance flips the outcome
Please make the sweep failure-tolerant before posting. Note the repo's own protocol norm: upsert-bot-comment.sh's header (restated around line 2231 of this workflow) says a FAILED lookup must never fall through to POST — that is how a transient 5xx mints a permanent duplicate — so prefer bounded retry (the shared script's 3-attempt / sleep 10 loop; see the R1-5 thread), or a guarded capture:
STALE_ACK_IDS="$(gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" \
--method GET --paginate -F per_page=100 \
| jq -r '.[] | select(.body | contains("<!-- qwen-review-ack -->")) | select(.user.login == "github-actions[bot]") | .id' || true)"
for STALE_ACK_ID in $STALE_ACK_IDS; do
[ -n "$STALE_ACK_ID" ] || continue
gh api --method DELETE "repos/${GITHUB_REPOSITORY}/issues/comments/${STALE_ACK_ID}" \
|| echo "Could not delete stale acknowledgement ${STALE_ACK_ID}." >> "$GITHUB_STEP_SUMMARY"
doneWhichever shape is chosen: please add an assertion in queued ack placement pinning the tolerance on the listing pipeline (mirroring the existing /content=eyes[^\n]*\n\s*\|\| echo/ pin for the reaction), and confirm it goes red with the tolerance removed.
中文说明
本行开始的旧 ack 列表管道在 set -euo pipefail 下没有保护。本 diff 删除了原先的 "$( … )" || EXISTING_ACK_ID="" 容错，且没有在任何地方重新建立——|| echo 只保护了上方的反应（reaction）POST 和循环内的每个 DELETE。当分页的 GET issues/{n}/comments 遇到瞬时 5xx、超时或次级限流时，pipefail 使管道返回非零，set -e 会在 gh pr comment 之前终止脚本——请求者会看到 👀（已发出）但没有 ack、没有 step summary 说明，ack job 变红。改动前，同样的失败仍会发布新的 ack；本步骤自己的注释说失败的 reaction "must not cost the ack comment below"，而位于两者之间、没有保护的列表请求恰恰会造成这个后果。
证据（用 stub 的 gh 驱动按原样提取的步骤，set -euo pipefail 与原脚本一致）：GET 失败分支：rc=1，调用日志止于列表 GET——没有 gh pr comment，step summary 为空，而 👀 POST 已执行；改动前分支：同样的 GET 失败下 rc=0，gh pr comment 正常发布；加上保护后：rc=0，ack 正常发布——容错改变了结果。
请在发布前让清扫具备容错。注意本仓库自身的协议规范：upsert-bot-comment.sh 的头部（在本 workflow 约 2231 行被重申）规定失败的查询绝不能 fall through 到 POST——那样会让瞬时 5xx 铸出永久重复——因此优先使用有界重试（共享脚本的 3 次尝试 / sleep 10 循环；见 R1-5 线程），或使用带保护的捕获（示例代码见英文部分）。
无论选择哪种形态：请在 queued ack placement 中新增断言钉住列表管道的容错（仿照已有的 /content=eyes[^\n]*\n\s*\|\| echo/ reaction 断言），并确认移除容错后该断言变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| pull_request_review_comment) | ||
| REACTION_PATH="repos/${GITHUB_REPOSITORY}/pulls/comments/${COMMENT_ID}/reactions" ;; | ||
| *) | ||
| REACTION_PATH="" ;; |
There was a problem hiding this comment.
[Suggestion] R1-7: This *) default branch initializing REACTION_PATH="" is a load-bearing guard under set -u, not dead code — and no test pins it. The new reactions test pins the two comment-shape endpoint strings, the fallback, and the COMMENT_ID env, but nothing asserts the default branch exists, so a refactor dropping it (the two explicit branches look exhaustive for "the two comment shapes") passes the suite. A pull_request_review trigger — a shape the job's if: explicitly accepts — then falls through the case with REACTION_PATH unset; the very next line's [ -n "$REACTION_PATH" ] raises REACTION_PATH: unbound variable under set -u and the step aborts before the ack is posted. The entire review-body trigger path loses its acknowledgement after an innocent-looking simplification, suite green.
Witness (probe): deleting the branch leaves the suite 179/179 green; driving the extracted mutant with GITHUB_EVENT_NAME=pull_request_review exits 1 with REACTION_PATH: unbound variable after exactly one gh call, while the base arm exits 0 and posts the ack.
Please add to the reacts 👀 test:
expect(ackRun).toMatch(/\*\)\s*\n\s*REACTION_PATH=""/);Deleting the *) branch must turn it red — please confirm the mutation.
中文说明
这个把 REACTION_PATH="" 初始化的 *) 默认分支在 set -u 下是承重保护，不是死代码——而且没有测试钉住它。新的 reactions 测试钉住了两个评论形态的端点字符串、兜底和 COMMENT_ID 环境变量，但没有任何断言要求默认分支存在，因此删除它的重构（两个显式分支对"两种评论形态"看起来很完备）能通过套件。pull_request_review 触发——job 的 if: 明确接受的一种形态——会穿过 case 而 REACTION_PATH 未设置；下一行的 [ -n "$REACTION_PATH" ] 在 set -u 下抛出 REACTION_PATH: unbound variable，步骤在发布 ack 之前中止。一次看似无害的简化之后，整条 review 正文触发路径失去确认，测试仍然全绿。
证据（探针）：删除该分支后套件仍 179/179 全绿；用 GITHUB_EVENT_NAME=pull_request_review 驱动提取的变异体，退出码 1、报 REACTION_PATH: unbound variable、只有一次 gh 调用，而基础分支退出码 0 且正常发布 ack。
请在 reacts 👀 测试中加入（代码见英文部分）。删除 *) 分支必须让它变红——请确认该变异。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # -F would otherwise make gh api default to POST. | ||
| gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" \ | ||
| --method GET \ | ||
| --paginate \ |
There was a problem hiding this comment.
[Suggestion] R1-5: This new stale-ack lookup re-inlines the repo's marker+author bot-comment protocol instead of extending its canonical home, .github/scripts/upsert-bot-comment.sh — the script this workflow's own comments (around line 1621 and around line 2231) cite as the shared protocol, and whose header states it exists because "the previous per-step copies had already drifted." The new inline copy carries none of its norms: bounded retry (3 attempts, sleep 10), dynamically resolved author login, (.body // "") null guard.
The cost is concrete today, not hypothetical: a single transient 5xx on this listing aborts the whole ack step under set -euo pipefail — the get-fail arm of the R1-1 probe, observed at rc=1 — where the shared script's retry loop fits comfortably inside this job's 5-minute timeout and survives it. And the next fix to the protocol will have to be re-applied here by hand — the exact failure mode upsert-bot-comment.sh was created to end. The 10% that differs here is deliberate (delete-all-matches + unconditional bottom repost instead of PATCH-the-last), so the fix is extending the shared mechanism, not calling it as-is.
Suggested fix: add a repost mode to upsert-bot-comment.sh (parallel to --update-only, e.g. --repost: reuse its retrying lookup, DELETE every matched bot-authored marker comment, then POST the body file) and invoke it from this step. .github/scripts/upsert-bot-comment.test.mjs (stubbed gh+sleep, wired into ci.yml's HELPER_TESTS) should then pin the retry-on-failed-listing behaviour of the new mode — please confirm that test goes red when the retry is removed.
中文说明
这个新的旧 ack 查找把本仓库"标记 + 作者"的 bot 评论协议重新内联了一份，而不是扩展它的规范归属 .github/scripts/upsert-bot-comment.sh——本 workflow 自己的注释（约 1621 行和约 2231 行）都引用该脚本作为共享协议，其头部声明它存在的原因正是"之前各步骤的副本已经开始漂移"。新的内联副本没有携带它的任何规范：有界重试（3 次尝试、sleep 10）、动态解析的作者登录名、(.body // "") 空值保护。
代价今天就具体可见，而非假设：该列表请求遇到一次瞬时 5xx 就会在 set -euo pipefail 下中止整个 ack 步骤——即 R1-1 探针的 GET 失败分支，实测 rc=1——而共享脚本的重试循环完全放得进这个 job 的 5 分钟超时，并能扛过这种失败。且协议的下一次修复将不得不手工重新应用到此处——正是 upsert-bot-comment.sh 被创建出来要终结的失败模式。这里 10% 的差异是刻意的（删除所有匹配 + 无条件在底部重发，而不是 PATCH 最后一条），因此修复方向是扩展共享机制，而不是原样调用。
建议修复：给 upsert-bot-comment.sh 增加一个重发模式（与 --update-only 并列，例如 --repost：复用其带重试的查找，DELETE 所有匹配的 bot 作者标记评论，然后 POST 正文文件），并从本步骤调用。之后 .github/scripts/upsert-bot-comment.test.mjs（stub 的 gh+sleep，已接入 ci.yml 的 HELPER_TESTS）应钉住新模式的"列表失败时重试"行为——请确认移除重试后该测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| --method GET \ | ||
| --paginate \ | ||
| -F per_page=100 \ | ||
| | jq -r '.[] | select(.body | contains("<!-- qwen-review-ack -->")) | select(.user.login == "github-actions[bot]") | .id' \ |
There was a problem hiding this comment.
[Suggestion] R1-4: The user.login == "github-actions[bot]" select is rewritten on a + line of this diff, but no test pins it — the string appears nowhere in the test file — so dropping the filter ships green while turning the delete loop loose on human comments. A human comment quoting the literal marker (as happens in review threads discussing this very workflow) would then be treated as a stale ack, and the next /review run deletes a human-authored comment.
Witness: mutation run — removing the select leaves the suite 179/179 green; a runtime drive of the extracted mutant step against stubbed comments (bot ack id=111, human comment id=222 quoting the marker) DELETEs both, whereas the PR's real filter deletes only 111.
Please add to the deletes stale acks… test:
expect(ackRun).toContain('select(.user.login == "github-actions[bot]")');Removing the login select from the jq filter must turn it red — please confirm the mutation.
中文说明
user.login == "github-actions[bot]" 这个 select 在本 diff 的 + 行上被重写，但没有任何测试钉住它——该字符串在测试文件中完全没有出现——因此删除这个过滤器后测试依然全绿，而删除循环就会对人类评论放开。一条引用了字面标记的人类评论（在讨论这个 workflow 的评审线程中很常见）会被当作旧 ack，下一次 /review 运行会删除一条人类作者的评论。
证据：变异测试——移除该 select 后套件仍 179/179 全绿；用提取的变异步骤驱动 stub 评论（bot ack id=111，引用标记的人类评论 id=222），两个都被 DELETE，而 PR 的真实过滤器只删除 111。
请在 deletes stale acks… 测试中加入（代码见英文部分）。从 jq 过滤器中移除登录名 select 必须让它变红——请确认该变异。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| --method GET \ | ||
| --paginate \ | ||
| -F per_page=100 \ | ||
| | jq -r '.[] | select(.body | contains("<!-- qwen-review-ack -->")) | select(.user.login == "github-actions[bot]") | .id' \ |
There was a problem hiding this comment.
[Suggestion] R1-6: The delete loop's reader-side marker copy (contains("<!-- qwen-review-ack -->") on this line) and the ack body's printf writer-side copy (~line 181) are two independent literals with nothing pinning them byte-identical; the suite pins only the writer side. A one-sided rename — e.g. bumping printf to qwen-review-ack-v2 while sibling copies in qwen-autofix.yml and cleanup.ts keep the old spelling — ships green: the jq select matches zero comments, the loop body never executes, not even the || echo summary line fires, so zero deletions leave no observable signal, and every subsequent /review posts an additional ack while the first stays pinned near the top. That is the "one ack, at the bottom" invariant this PR exists to establish, broken with the suite green. The workflow already defends this exact shape elsewhere: FALLBACK_MARKER is defined once at workflow env: because "the sites must stay byte-identical or the dedup silently posts duplicates."
Witness (probe): renaming the jq-side marker leaves the suite 179/179 green; the extracted mutant step performs zero DELETE calls with exit 0 and summary "Queued acknowledgement posted". Pinning the reader marker to the writer-extracted marker makes the mutant fail 1/180 while passing on clean code.
Pin the reader against the writer in queued ack placement — extract the marker from the printf line (the existing ackLine finder already locates it) and assert the jq line contains the same string — or define the marker once, as FALLBACK_MARKER does. Renaming the marker inside this jq select(.body | contains(...)) while the printf side keeps the original must turn the new assertion red.
中文说明
删除循环的"读端"标记副本（本行的 contains("<!-- qwen-review-ack -->")）与 ack 正文 printf 的"写端"副本（约 181 行）是两个独立的字面量，没有任何东西钉住它们逐字节一致；测试套件只钉了写端。单侧改名——例如把 printf 升级为 qwen-review-ack-v2，而 qwen-autofix.yml 和 cleanup.ts 中的兄弟副本保留旧拼写——后测试依然全绿：jq 的 select 匹配不到任何评论，循环体一次都不执行，连 || echo 的 summary 行都不会触发，零删除不留下任何可观察信号，之后每次 /review 都会多发一条 ack，而第一条仍钉在靠上的位置。这正是本 PR 要建立的"一条 ack、在最底部"不变量，在测试全绿的情况下被破坏。本 workflow 在别处已经防过同样的形态：FALLBACK_MARKER 在 workflow env: 中只定义一次，因为"各站点必须逐字节一致，否则去重会静默发布重复"。
证据（探针）：把 jq 侧标记改名后套件仍 179/179 全绿；提取的变异步骤执行零次 DELETE、退出码 0、summary 为 "Queued acknowledgement posted"。把读端标记钉到从写端提取的标记后，变异体 1/180 失败，干净代码通过。
请在 queued ack placement 中把读端钉到写端——从 printf 行提取标记（已有的 ackLine 定位器已能找到它），断言 jq 行包含相同字符串——或者像 FALLBACK_MARKER 那样把标记只定义一次。在 printf 侧保持原样时，改名本行 jq select(.body | contains(...)) 内的标记必须让新断言变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| | jq -r '.[] | select(.body | contains("<!-- qwen-review-ack -->")) | select(.user.login == "github-actions[bot]") | .id' \ | ||
| | while read -r STALE_ACK_ID; do | ||
| [ -n "$STALE_ACK_ID" ] || continue | ||
| gh api --method DELETE "repos/${GITHUB_REPOSITORY}/issues/comments/${STALE_ACK_ID}" \ |
There was a problem hiding this comment.
[Suggestion] R1-3: This DELETE call's best-effort || echo continuation has no test, unlike the reaction's identical fallback, which the new suite explicitly pins (/content=eyes[^\n]*\n\s*\|\| echo/). Dropping the continuation ships green — a mutation run confirmed the suite stays 179/179 without it. Then a single transient DELETE failure (an API 5xx, secondary rate limit, or a 404 for a comment removed between the GET and the DELETE) kills the loop under set -e, pipefail fails the pipeline, and the step aborts before gh pr comment: no ack at all — precisely the bug this PR's delete-then-post rewrite exists to fix. The runtime probe confirmed the mechanism: an unguarded failing command in the loop body under set -euo pipefail exits before the post; the guarded form posts.
Please mirror the reaction assertion in queued ack placement:
expect(ackRun).toMatch(/--method DELETE[^\n]*\n\s*\|\| echo/);Removing the || echo "Could not delete stale acknowledgement …" continuation must turn it red — please confirm the mutation.
中文说明
这个 DELETE 调用的尽力而为 || echo 续行没有测试，而 reaction 的相同兜底被新测试套件显式钉住了（/content=eyes[^\n]*\n\s*\|\| echo/）。删除该续行后测试依然全绿——变异测试已确认：没有它套件仍是 179/179。此后任何一次瞬时 DELETE 失败（API 5xx、次级限流，或 GET 与 DELETE 之间评论被移除导致的 404）都会在 set -e 下杀死循环，pipefail 使管道失败，步骤在 gh pr comment 之前中止：完全没有 ack——这正是本 PR 的先删后发改写要修复的问题。运行时探针确认了该机制：在 set -euo pipefail 下，循环体中没有保护的失败命令会在发布前退出；有保护的形式正常发布。
请在 queued ack placement 中仿照 reaction 断言（代码见英文部分）。移除 || echo "Could not delete stale acknowledgement …" 续行必须让它变红——请确认该变异。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| gh api --method DELETE "repos/${GITHUB_REPOSITORY}/issues/comments/${STALE_ACK_ID}" \ | ||
| || echo "Could not delete stale acknowledgement ${STALE_ACK_ID}." >> "$GITHUB_STEP_SUMMARY" | ||
| done | ||
| gh pr comment "$PR_NUMBER" \ |
There was a problem hiding this comment.
[Suggestion] R1-8: Delete-then-post is non-atomic. This unguarded gh pr comment runs only after the stale ack(s) have already been deleted, so any single failure at post time — a transient 5xx, connection reset, secondary rate limit (this step now bursts reaction POST + GET + N DELETEs + this POST), or the job's 5-minute timeout expiring mid-sequence — leaves the PR with zero acknowledgements. The replaced PATCH flow left the previous ack in place under the same failure, and per the copy this PR adds, this ack link is "the only way to watch" a command-triggered run. The blast radius is bounded (the 👀 still shows, the review itself is unaffected, the result still arrives as a PR review, and the next /review re-posts the ack), but the regression is real for the exact repeat-/review thread shape this change targets.
Witness (extracted step, stubbed gh): post failing after the delete → exit 1, 0 acks remaining, 1 DELETE call; inverted ordering (capture ids → post → delete) with the same post failure → exit 1, 1 ack remaining. Note: a naive inversion that lists AFTER the post would sweep the fresh ack itself (same marker, same bot user) — the stale ids must be captured before posting.
Fix: capture the stale ids, post the fresh ack, then best-effort delete — any failure then leaves at most a transient duplicate (cleaned up on the next run), never zero — or give the post the bounded retry from the R1-5 direction. The new postIndex toBeGreaterThan ordering assertion pins the current ordering and must be flipped with this fix. Please extend the R1-1-style extracted-step probe (stubbed gh failing pr comment only after the DELETE succeeds) and confirm that, once fixed, the fresh ack exists before any deletion.
中文说明
先删后发不是原子的。这个没有保护的 gh pr comment 只在旧 ack 已被删除之后才运行，因此发布时任何一次失败——瞬时 5xx、连接重置、次级限流（该步骤现在一次性发出 reaction POST + GET + N 个 DELETE + 这个 POST），或 job 的 5 分钟超时在序列中途到期——都会让 PR 上没有任何确认评论。被替换的 PATCH 流程在同一种失败下会保留之前的 ack，而按本 PR 新增的文案，这条 ack 链接是观察命令触发运行的"唯一入口"。影响范围有限（👀 仍在，review 本身不受影响，结果仍会以 PR review 的形式送达，下一次 /review 会重新发布 ack），但对本改动针对的"重复 /review"线程场景，这个回归是真实存在的。
证据（提取的步骤 + stub 的 gh）：DELETE 成功后发布失败 → 退出码 1，剩余 0 条 ack，1 次 DELETE 调用；反转顺序（先捕获 id → 发布 → 删除）在同样的发布失败下 → 退出码 1，剩余 1 条 ack。注意：把列表放到发布之后的朴素反转会把新 ack 自己也扫掉（同样的标记、同样的 bot 用户）——必须在发布前捕获旧 id。
修复：先捕获旧 id，发布新 ack，再尽力删除——任何失败至多留下瞬时重复（下次运行清理），绝不会为零；或者按 R1-5 的方向给发布加有界重试。新增的 postIndex toBeGreaterThan 顺序断言钉住了当前顺序，修复时需要一并反转。请扩展 R1-1 风格的提取步骤探针（stub 的 gh 仅在 DELETE 成功后让 pr comment 失败），并确认修复后新 ack 在任何删除之前就已存在。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| it('reacts 👀 to the triggering comment, best effort', () => { | ||
| expect(ackRun).toContain('-f content=eyes'); | ||
| expect(ackRun).toContain( | ||
| 'repos/${GITHUB_REPOSITORY}/issues/comments/${COMMENT_ID}/reactions', |
There was a problem hiding this comment.
[Suggestion] R1-2: These toContain assertions check that both reaction-endpoint strings appear somewhere in the run script, but never tie each one to its case branch — so swapping the two REACTION_PATH= assignments in the workflow passes every assertion. A future edit that swaps them sends the issue_comment POST to /pulls/comments/{id}/reactions with an issue-comment id; GitHub returns 404, the || echo fallback swallows it, and the ack posts with no 👀 ever appearing — the exact feature this PR adds, silently dead while the suite is green.
Witness: mutation run — swapping the two assignments leaves the suite 179/179 green; adding per-branch anchored regexes makes the mutant fail while passing on clean code.
Please pin the wiring per branch, e.g.:
expect(ackRun).toMatch(/issue_comment\)\s*\n\s*REACTION_PATH="[^"]*\/issues\/comments\//);
expect(ackRun).toMatch(/pull_request_review_comment\)\s*\n\s*REACTION_PATH="[^"]*\/pulls\/comments\//);Swapping the two assignments back must turn these red — please confirm the mutation.
中文说明
这些 toContain 断言只检查两个反应端点字符串出现在运行脚本中的某处，但没有把每一个和它的 case 分支绑定——因此在 workflow 里交换两个 REACTION_PATH= 赋值，所有断言仍然通过。未来的编辑若交换它们，issue_comment 的 POST 会带着 issue 评论 id 打到 /pulls/comments/{id}/reactions；GitHub 返回 404，|| echo 兜底把它吞掉，ack 照常发布但 👀 永远不出现——本 PR 新增的功能就这样在测试全绿的情况下静默失效。
证据：变异测试——交换两个赋值后测试套件仍 179/179 全绿；加上按分支锚定的正则后，该变异体失败，干净代码通过。
请按分支钉住接线，例如（代码见英文部分）。把两个赋值交换回去必须让这些断言变红——请确认该变异。
— qwen3.8-max via Qwen Code /review (v0.22.2)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
|
Released in v0.22.3. |
3 participants
What this PR does
When someone comments
@qwen-code /review, theack-review-requestjob now deletes any earlier acknowledgement and posts a fresh one, so the "review request accepted — [workflow run]" notice always lands directly under the command that asked for it. It also reacts 👀 to the triggering comment (issue comments and review comments; review bodies have no reactions endpoint), and the ack text now says that a command-triggered review runs against the base branch and is therefore not listed under the PR's checks — the link is the only way to watch it.Why it's needed
The ack existed, but it was edited in place whenever a previous ack was on the thread. On #10259 the second
/reviewupdated the ack left by the first one — comment 2 of 15, above the previous day's triage output — so a requester reading from the bottom saw nothing happen, found no pending check on the PR (a command-triggered run'sreview-prcheck attaches to main's commit, not the PR head), and concluded the review had not started while it was in fact running. One ack per PR is still the right count; it just has to be at the right position and say why there is no yellow dot.Recreating the comment is safe: nothing keys on the ack's comment id. Every consumer — the eight bot-comment filters in
qwen-autofix.ymland the review's bypass audit inpackages/cli/src/commands/review/cleanup.ts— matches the<!-- qwen-review-ack -->marker.Reviewer Test Plan
How to verify
ack-review-requeststep: it reactseyesonissues/comments/{id}orpulls/comments/{id}depending on the event (best effort,|| echoso a failed reaction cannot abort the ack underset -e), deletes every existinggithub-actions[bot]comment carrying the marker, then posts the new ack unconditionally — there is noPATCHand noelsebranch.npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-pr-review-workflow.test.js— the newqueued ack placementblock pins the delete-then-post shape, the two reaction endpoints withcontent=eyes, theCOMMENT_IDenv wiring, and the "not listed under the checks of this PR" copy. The existingpins the workflow-run URL into the ack printftest still passes (the printf line keeps[workflow run](%s)and"$RUN_URL").@qwen-code /reviewon a PR that already has an ack from an earlier request; expect the old ack to disappear, a new one to appear as the last comment linking to the new run, and 👀 on the command.Evidence (Before & After)
Before (#10259, 2026-08-28):
/reviewposted at 08:09:37Z → ack comment5435672165(created 2026-08-27, position 2 of 15) silently PATCHed at 08:09:54Z; no reaction on the command; PR checks show nothing for the run. After: N/A until it runs in CI — the shape is pinned by the tests above.Tested on
Environment (optional)
bash -nand shellcheck on the extractedrun:script; scripts test suite (the twounwritable directorytests fail locally as root regardless of this change).Risk & Scope
pull_request_reviewtrigger gets no reaction (GitHub has no reactions API for review bodies); the ack is still never removed when the review completes.Linked Issues
Follow-up observed on #10259 (no tracking issue).
中文说明
本 PR 做了什么
当有人评论
@qwen-code /review时，ack-review-requestjob 现在会先删除线程里已有的确认评论，再新发一条，这样"review request accepted — [workflow run]"这条通知总是紧贴在发出命令的评论下方。同时对触发命令的评论加 👀 反应（issue 评论和 review 评论；review 正文没有 reactions 接口），并在 ack 文案中说明：命令触发的 review 运行在 base 分支上，因此不会出现在 PR 的 checks 列表里——链接是唯一的观察入口。为什么需要
ack 本来就存在，但只要线程里已有一条 ack，它就会被原地编辑。在 #10259 上，第二次
/review更新的是第一次留下的那条 ack——15 条评论中的第 2 条，位于前一天的 triage 输出上方——所以从底部往上看的请求者什么都没看到，PR 上也没有 pending 的 check（命令触发的 run 的review-prcheck 挂在 main 的 commit 上，不在 PR head 上），于是断定 review 没有启动，而实际上它正在运行。每个 PR 只保留一条 ack 仍然是对的，只是它必须在正确的位置，并说明为什么没有黄点。重建评论是安全的：没有任何东西依赖 ack 的评论 id。所有消费方——
qwen-autofix.yml中的 8 处 bot 评论过滤和packages/cli/src/commands/review/cleanup.ts中的绕过审计——都按<!-- qwen-review-ack -->标记匹配。Reviewer Test Plan
如何验证
ack-review-request步骤：根据事件类型对issues/comments/{id}或pulls/comments/{id}加eyes反应（尽力而为，|| echo保证在set -e下反应失败不会中止 ack），删除所有带标记的github-actions[bot]评论，然后无条件发布新 ack——没有PATCH，也没有else分支。npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-pr-review-workflow.test.js——新增的queued ack placement测试块钉住了"先删后发"的形态、两个带content=eyes的反应端点、COMMENT_ID环境变量接线，以及"not listed under the checks of this PR"文案。原有的pins the workflow-run URL into the ack printf测试仍然通过（printf 行保留了[workflow run](%s)和"$RUN_URL"）。@qwen-code /review；预期旧 ack 消失、新 ack 作为最后一条评论出现并链接到新 run、命令评论上出现 👀。证据（Before & After）
Before（#10259，2026-08-28）：08:09:37Z 发出
/review→ ack 评论5435672165（创建于 2026-08-27，位置 15 之 2）在 08:09:54Z 被静默 PATCH；命令上无反应；PR checks 里没有该 run。After：需在 CI 中运行后才有——形态已由上述测试钉住。测试平台
环境（可选）
对抽取出的
run:脚本做了bash -n和 shellcheck；scripts 测试套件（其中两个unwritable directory测试在本地以 root 运行时无论如何都会失败，与本改动无关）。风险与范围
pull_request_review触发方式不加反应（GitHub 没有 review 正文的 reactions API）；review 完成后 ack 仍不会被移除。关联 Issue
在 #10259 上观察到的后续问题（无跟踪 issue）。