fix(ci): keep no-op review requests out of the PR review concurrency group - #9210
fix(ci): keep no-op review requests out of the PR review concurrency group#9210wenshao wants to merge 5 commits into
Conversation
…group Co-authored-by: Qwen-Coder <[email protected]>
|
|
|
Thanks for the PR! Template looks good ✓ Problem: observed, and I verified the incident rather than taking it on faith. On PR #9091's head Direction: aligned — this is the repo's own review automation losing work on an everyday flow (push the last fix, request teammates' review). #8945 tracks the burst family. Size: 13 production lines (12+1 in the workflow YAML, of which 10 are a comment documenting the race) + 45 test lines. No core package paths touched. Approach: the scope feels right. Routing non-bot Risk: no elevated risk signals — no high-risk paths matched. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到,并且我核实了事故本身。PR #9091 的 head 方向:对齐——这是仓库自身评审自动化在日常操作(推最后一个修复、请队友 review)下丢工作。#8945 跟踪该 burst 家族。 规模:13 行生产改动(workflow YAML 12+1,其中 10 行是记录竞态的注释)+ 45 行测试。未触及核心包路径。 方案:范围合理。把非 bot 的 风险:无升级风险信号——未命中任何高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent proposal first: I would have done the same thing — gate the shared PR group on the same bot-requester condition that Truth-table check across all five routing classes:
Two structural details are load-bearing, and both are pinned. The gate must sit inside the PR-group conjunct chain: The disclosed tradeoff is real but bounded: a bot-directed request can still supersede a pending synchronize run, but the review then happens via the explicit request path, so nothing is lost; human/team request runs are no longer cancelled by Testing evidenceUnattended CI run — PR code is never executed in this environment; the evidence below is the PR's own CI read via the API at the reviewed commit. This change has no user-visible product behavior (workflow concurrency routing), so no tmux real-scenario pass applies either. The one check that matters for this PR is still in flight: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Sandboxed verification would settle one thing a green suite cannot: 中文说明代码审查:我的独立方案与此 PR 完全一致——用 测试证据:无人值守 CI 运行——本环境从不执行 PR 代码;以上证据为经 API 读取的 PR 自身 CI(针对被审 commit)。本改动无用户可见的产品行为(workflow 并发路由),因此也不适用 tmux 真实场景测试。关键检查仍在运行: 沙盒验证: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — observed incident verified against real run history, minimal fix that matches my independent proposal exactly, and both load-bearing details pinned by tests. Stepping back: this is the gate working the way it should. The problem isn't hypothetical — I confirmed on PR #9091's head that four runs entered the shared group in the same second and the sole survivor was an all-skipped no-op, so the push was never reviewed. The fix is two functional lines: non-bot review requests get a per-run group, exactly like comment/review events already do, while lifecycle events and bot-directed requests keep coalescing in the PR group. I walked the truth table for all five routing classes and found no behavior change where none was intended. The two pins are the kind of rigor I'd want here: the expression shape IS the fix (operator precedence makes placement load-bearing), and the bot literal is tied to the constant the workflow publishes because the group expression can't read job outputs. The disclosed tradeoffs — bot requests can still supersede a pending synchronize run, If I'm maintaining this in six months, the 10-line incident comment at the routing site tells me exactly why the expression has this shape, and the pins stop anyone from silently breaking it. That's the whole job done. The only open item is mechanical, not judgmental: the ubuntu test leg (the one that runs the new pin suite) is still in flight on the reviewed commit. Approval is deferred until CI lands green on 中文说明置信度:5/5 —— 事故已在真实 run 历史中核实,修复极小且与我的独立方案完全一致,两个承重点均有测试 pin 保护。 退一步看:这是门控应有的样子。问题不是假设——我在 PR #9091 的 head 上确认:同一秒内四个 run 进入共享组,唯一幸存者是全部 skip 的空跑,该 push 从未被评审。修复只有两行功能性改动:非 bot 的 review 请求改走每 run 独立组(与评论/review 事件已有的待遇一致),生命周期事件与指向 bot 的请求继续在 PR 组合并。五类路由的真值表逐一核对,凡无意改动之处均无行为变化。两个 pin 正是此处应有的严谨:表达式形状本身就是修复(运算符优先级使位置成为承重点),bot 字面量绑定到 workflow 发布的常量(组表达式读不到 job 输出)。PR 自述的两个取舍真实但有界:bot 请求仍可能顶掉 pending 的 synchronize run,但评审会走显式请求路径执行; 六个月后维护这段代码时,路由处的 10 行事故注释会说明表达式为何是这个形状,pin 则阻止任何人无声破坏它。职责完成。 唯一未决项是机械性的而非判断性的:运行新 pin 套件的 ubuntu 测试腿在被审 commit 上仍在跑。批准推迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
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. ✅
This comment has been minimized.
This comment has been minimized.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
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.21.12)
| // it carries the literal; pin it against the constant it mirrors so a | ||
| // rename of the bot cannot desync the two. |
There was a problem hiding this comment.
[Suggestion] The bot-login sync pin covers two of the three literal copies. The literal qwen-code-ci-bot sits in three places in the workflow: the new concurrency group expression (line ~67), precheck-pr.if (line ~79), and review-config's Set review constants step (line ~184). This test pins the first two together, but nothing pins precheck-pr.if, so the comment's "cannot desync" claim holds for two of the three sites. — Failure scenario: a bot rename that updates the two pinned sites leaves CI green while precheck-pr.if keeps the old login; on fork PRs a bot-directed review_requested then fails precheck's if, skipping authorize (the fork branch requires decision == 'allow_triage') and review-pr — bot-requested reviews on fork PRs silently stop while same-repo PRs keep working, so casual testing never surfaces it.
Witness (probe):
renamed the two pinned sites to qwen-renamed-bot, left precheck-pr.if stale:
all four workflow suites green (115/115, 172/172, 29/29, 5/5) <- desync invisible to CI
adding the assertion below to the same tree:
AssertionError: expected ... to contain 'github.event.requested_reviewer.login == 'qwen-renamed-bot''
Suggested fix — add to this test (and change "the two" in the comment to "all three sites"):
expect(parse(workflow).jobs['precheck-pr'].if).toContain(
`github.event.requested_reviewer.login == '${botLogin}'`,
);中文说明
这个 bot-login 同步 pin 测试只覆盖了三处字面量副本中的两处。字面量 qwen-code-ci-bot 在 workflow 中有三处:新的并发组表达式(约第 67 行)、precheck-pr.if(约第 79 行)、review-config 的 Set review constants 步骤(约第 184 行)。本测试只把前两处钉在一起,而 precheck-pr.if 没有任何 pin,因此注释里"rename of the bot cannot desync the two" 的主张对三处中的两处成立。— 失败场景:bot 改名时只更新了两个被 pin 的位置,CI 保持绿色,但 precheck-pr.if 残留旧登录名;对 fork PR,指向 bot 的 review_requested 将不满足 precheck 的 if,进而跳过 authorize(fork 分支要求 decision == 'allow_triage')与 review-pr —— fork PR 上请求 bot 评审会静默失效,而同仓库 PR 仍正常工作,随手测试无法暴露。
证据(probe):把两个被 pin 的位置改名为 qwen-renamed-bot(同步更新逐字期望)并保持 precheck-pr.if 不变 —— 四个 workflow 套件全绿(115/115、172/172、29/29、5/5),脱同步对 CI 不可见;在同一棵树上加入下方断言后翻红(AssertionError: expected ... to contain ...)。树已还原。
建议修复:在同一测试中加入下方断言(并把注释中的 "the two" 改为 "all three sites")。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Address-review summary — PR #9210 (round 1)Feedback points and dispositions
No other feedback: the review was COMMENTED with no blockers; no failed or still-red checks; no deferred non-Critical section, no maintainer-decision section. No conflicts ( Verification
中文说明处理评审摘要 — PR #9210(第 1 轮)反馈点及处理
无其他反馈:评审为 COMMENTED、无阻断问题;无失败或持续红色的 check;无 Deferred non-Critical 部分、无 maintainer-decision 部分。无冲突( 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
Resolve the EOF conflict in qwen-pr-review-workflow.test.js by keeping both appended suites: this PR's concurrency-routing tests and main's #9204 review_requested burst-coalescing tests. The workflow changes are orthogonal: the concurrency group routes non-bot review_requested runs to per-run groups (this PR) while review-config.if and authorize.if skip those siblings before they spend compute (#9204). Co-authored-by: Qwen-Coder <[email protected]>
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #9210Feedback triageThe workflow-supplied feedback contained no actionable findings this round: the Reviews, Inline comments, Issue-level comments, Failed checks, and Still-red checks sections were all empty. Diff growth this window was source 0 / test 3 lines, well inside budget. The round's only task was the base-conflict resolution requested by the workflow ( Conflict resolution
Merging
Merge result audited byte-for-byte: the merged tree's delta vs Committed as merge commit Dispositions
Verification
中文说明Autofix 评审轮次 — PR #9210反馈分类本轮工作流提供的反馈中没有可处理的发现:Reviews、Inline comments、Issue-level comments、Failed checks、Still-red checks 各节均为空。本统计窗口的 diff 增长为源码 0 行 / 测试 3 行,远低于预算。 本轮唯一任务是工作流要求的基线冲突解决( 冲突解决本 PR 开启期间
合并
合并结果经过逐字节审计:合并后的树相对 以合并提交 处理结论
验证
Base-conflict check · 基分支冲突检查: conflicted with main — resolved in this push. · 与 main 有冲突——已在本次推送中解决。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
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.21.12)
| const botLogin = parse(workflow) | ||
| .jobs['review-config'].steps.find( | ||
| (s) => s.name === 'Set review constants', | ||
| ) | ||
| .run.match(/bot_login=([a-zA-Z0-9-]+)/)[1]; |
There was a problem hiding this comment.
[Suggestion] R2-1: Duplicated, drifted bot-login extraction recipes. This new block extracts the review-config bot login by step name with an unguarded [1]; the pre-existing #8945 describe block ~20 lines below extracts the same value positionally (steps[0]) with a match guard and a differently-spelled regex character class. Two recipes for one value in the same file. — Failure scenario: a later edit to the single-step review-config job breaks the two sites in different ways — renaming the Set review constants step makes this extraction throw a TypeError on .run of undefined while the positional one keeps matching; prepending a step makes the positional site silently yield '' while this by-name one survives. Whoever changes the workflow must find and sync two independent recipes.
Witness (probe, both mutation arms against the real workflow YAML):
baseline : new="qwen-code-ci-bot" | old="qwen-code-ci-bot"
step renamed : new=THROWS TypeError: Cannot read properties of undefined (reading 'run') | old="qwen-code-ci-bot"
step prepended: new="qwen-code-ci-bot" | old=""
Suggested fix — hoist one extraction to module scope and use it in both describe blocks (by-name lookup, guarded); the fix spans two locations, so a regular block rather than a one-click suggestion:
// module scope, shared by both describe blocks
const botLogin =
parse(workflow)
.jobs['review-config']
.steps.find((s) => s.name === 'Set review constants')
?.run.match(/bot_login=([A-Za-z0-9-]+)/)?.[1] ?? '';中文说明
[Suggestion] R2-1:重复且已漂移的 bot-login 提取写法。本新增代码块按 step 名提取 review-config 的 bot login,[1] 无保护;下方约 20 行处既有的 #8945 describe 块用位置索引(steps[0])提取同一个值,带 match 保护,正则字符类写法也不同。同一文件中一个值对应两套提取写法。— 失败场景:之后若编辑单 step 的 review-config job,两处会以不同方式损坏——把 Set review constants 改名会使本处提取在 undefined 的 .run 上抛 TypeError,而位置索引写法继续匹配;在该 job 前插入一个 step 会使位置索引写法静默得到 '',而按名查找的写法幸存。修改 workflow 的人必须找到并同步两套互相独立的写法。
证据(probe,对真实 workflow YAML 的两种变异,见上方英文部分输出):改名 step 只破坏新写法(TypeError),前置插入 step 只破坏旧写法(静默得到空串)。
建议修复:把其中一处提取提升到模块作用域,两个 describe 块共用(按名查找并加 ?./?? '' 保护);修复跨两处,故用普通代码块而非一键 suggestion,见上方英文部分。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Address review summary — PR #9210Feedback points[Suggestion] R2-1 (
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 4": none. (The full vitest suite run was skipped as disproportionate — node_modules would require a fresh monorepo install — but its only perf-relevant output, th….
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.
中文说明
未探索到全部深度(达到工具调用预算):"agent 4":none. (The full vitest suite run was skipped as disproportionate — node_modules would require a fresh monorepo install — but its only perf-relevant output, th…。
未检查(工具限制,非阻断):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.21.12)
| (github.event.action != 'review_requested' || | ||
| github.event.requested_reviewer.login == 'qwen-code-ci-bot') && |
There was a problem hiding this comment.
[Critical] R3-1: The new gate admits bot-directed review_requested runs into the shared PR group on the requested reviewer's identity, but whether such a run reviews anything is decided later by authorize on the requester's write permission. A requester without write permission produces a guaranteed all-skipped run that can still supersede a PENDING lifecycle run — re-opening the exact #9091 lost-review race this PR closes. The Risk & Scope claim that a superseding bot request is safe because "the review then happens via the explicit request" holds only for write-permission requesters.
Failure scenario: a fork PR author pushes a fix → the synchronize run cancels the in-flight review and sits PENDING behind the still-terminating run (the window this diff's own comment documents from #9091) → the author requests qwen-code-ci-bot → that run matches the new gate, joins the shared group, supersedes the pending synchronize run, then fails authorize (principal="$SENDER", needs write, fail-closed) and skips review-pr — the pushed head is never reviewed.
Witness (live GitHub data, this repo):
PR #9091, head 622c164d, four runs created 2026-08-15T06:03:21Z:
3 cancelled with zero jobs; sole survivor all-skipped (review-pr=skipped,
delay-automatic-review=skipped, ack-review-request=skipped, resolve-pr=skipped)
Collaborator API: re2zero => read (requested reviewers, fork PR #9199);
zjgzx1988 => read (fork PR #9142)
PR #9181 timeline: review_requested actor=doudouOUC requested=qwen-code-ci-bot
The bot is not in CODEOWNERS, so this needs a manual bot request — but "push the last fix, then request review" is the everyday flow this PR's own description cites, and requesting the bot is the published way to ask for a review.
Suggested fix (author's choice):
- Route ALL
review_requestedruns (bot included) to the per-run group — gate the PR group ongithub.event.action != 'review_requested'alone. An authorized bot request still runs its own immediate review (it skipsdelay-automatic-review); it just can no longer supersede a pending lifecycle run. Cost: an occasional duplicate review of the same head — compute, not lost reviews. The verbatim test pin would need the matching update. - Or keep the coalescing and have
authorize's deny path for bot-directed requests re-arm the lost lifecycle review (trigger a replacement review on deny).
If the coalescing is kept as-is, the concurrency comment should state this residual race explicitly instead of the unconditional "must coalesce".
中文说明
[Critical] R3-1:新增门控依据被请求 reviewer 的身份放行指向 bot 的 review_requested run 进入共享 PR 组,但该 run 是否真正执行评审,是之后由 authorize 依据请求者的写权限决定的。没有写权限的请求者会产生一个所有 job 必然跳过的空跑,而这个空跑仍然能顶掉处于 PENDING 状态的生命周期 run——重新打开了本 PR 要关闭的 #9091 丢评审竞态。Risk & Scope 中"顶掉的 bot 请求是安全的,因为评审会改由显式请求路径执行"的说法只对拥有写权限的请求者成立。
失败场景:fork PR 作者 push 修复 → synchronize run 取消在跑评审并 PENDING 等待其退出组(正是本 diff 注释依据 #9091 记录的窗口)→ 作者请求 qwen-code-ci-bot → 该 run 匹配新门控、进入共享组、顶掉 pending 的 synchronize run,随后 authorize(principal="$SENDER",要求 write,fail-closed)拒绝、review-pr 跳过——这次 push 完全没有被评审。
证据(本仓库实时 GitHub 数据,见上方英文部分代码块):#9091 事故 run 历史(3 个零 job 被取消、唯一幸存者全跳过);协作者权限 API 显示 read 权限的 fork 作者确实会请求 reviewer(#9199、#9142);#9181 时间线有 fork 作者请求 bot 的记录。
bot 不在 CODEOWNERS 中,因此需要手动请求 bot——但"推完最后一个修复、随即请求评审"正是本 PR 描述引用的日常流程,而请求 bot 就是公开的"求评审"方式。
建议修复(由作者选择):(1) 把所有 review_requested run(含 bot)都路由到每 run 独立组——PR 组门控只保留 github.event.action != 'review_requested'。已授权的 bot 请求仍会立即执行自己的评审(跳过 delay-automatic-review),只是不再能顶掉 pending 的生命周期 run;代价是偶尔重复评审同一 head——耗算力,不丢评审。逐字 pin 测试需同步更新。(2) 或保留合并逻辑,让 authorize 对 bot 请求的拒绝路径重新触发被丢的生命周期评审(拒绝时补触发一次评审)。若维持现状,请在并发注释中明确写出这一残留竞态,而不是现在无条件的"must coalesce"。
— qwen3.8-max via Qwen Code /review (v0.21.12)
A bot-directed review_requested run joined the shared PR group on the requested reviewer's identity, but whether it reviews anything is decided later by authorize on the requester's write permission. A requester without write produces a guaranteed all-skipped run that can still supersede a lifecycle run sitting PENDING behind a still-terminating review — the same lost-review race as #9091, through the bot-request door. Gate the shared group on the action alone so no review_requested run can supersede a pending lifecycle run; an authorized bot request still reviews immediately, at the cost of an occasional duplicate review of the same head.
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 4/100 轮)。改动内容与我反驳保留之处如下: Addressed feedback — PR #9210, round 3[Critical] R3-1 (rc:3789395309) —
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
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.
中文说明
未发现问题。LGTM!✅
未检查(工具限制,非阻断):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.21.12)
What this PR does
Changes the review workflow's concurrency routing so a
review_requestedevent for a human or team reviewer no longer joins the shared PR-scoped group. Such runs get a per-run group instead, the same treatment the comment and review command events already have. Lifecycle events (opened, synchronize, reopened, ready_for_review, closed) and bot-directed review requests keep sharing the PR group, so a new push still restarts the delayed automatic review, closing a PR still stops an in-flight review, and an explicit request of the bot still coalesces with the automatic run for the same head. The routing carries a comment recording the incident that motivated it, and the change is pinned by a verbatim expression test plus a sync pin tying the gate's bot literal to the constant the workflow publishes.Why it's needed
A pending run in a concurrency group is replaced by any newer run of the same group —
cancel-in-progressdoes not protect it. PR #9091 hit exactly that: a push and three human review requests landed within the same minute. The synchronize run cancelled the in-flight review of the previous head, then queued behind it while it terminated; the threereview_requestedruns — guaranteed no-ops, since the review job only runs when the bot itself is the requested reviewer — replaced the synchronize run while it was still pending. The sole survivor skipped the review job by design, the in-flight review round was cancelled, and the push was never reviewed at all. This is the #8945 burst family, but the cost there was bounded startup waste; here a real review silently evaporated, and the triggering flow — push the last fix, then request teammates' review — is an everyday one.Reviewer Test Plan
How to verify
Run the workflow pin suites:
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-pr-review-workflow.test.js scripts/tests/qwen-resolve-workflow.test.js— the new describe block pins the group expression verbatim (the shape IS the fix: dropping the parentheses around the disjunction or moving the gate behind the fallback silently re-ships the race), and the resolve suite's existing concurrency pins (closed-cancellation, verbatimcancel-in-progress) stay green. The live concurrency semantics cannot be exercised pre-merge becausepull_request_targetevaluates the workflow from the base ref; post-merge, the check is that on a PR receiving a human "Request review" next to a push, the synchronize run survives the delay gate and reaches the review job, while the request runs complete as instant all-skipped runs in their own groups, and a bot-directed request burst still coalesces to one survivor exactly as today.Evidence (Before & After)
N/A — CI workflow change; the incident evidence is the run history on PR #9091 (synchronize run cancelled with zero jobs, no-op request run as sole survivor, in-flight review killed).
Tested on
Environment (optional)
Unit tests via vitest against the parsed workflow; the pre-existing local failures in the autofix suites reproduce identically on the untouched base (macOS bash 3.2 lacks
mapfile) and are unrelated.Risk & Scope
closedevent, but they complete within seconds as all-skipped runs, so there is nothing to stop.Linked Issues
Related: #8945 (review_requested burst family; this closes its data-loss variant, not the startup-waste one).
中文说明
本 PR 做了什么
修改评审 workflow 的并发路由:指向人类 reviewer 或团队的
review_requested事件不再进入 PR 共享并发组,改用每 run 独立的组——与评论/review 命令事件已有的待遇一致。生命周期事件(opened、synchronize、reopened、ready_for_review、closed)与指向 bot 的 review 请求继续共享 PR 组,因此新 push 仍会重启延迟自动评审、关闭 PR 仍会中止在跑评审、显式请求 bot 仍会与同一 head 的自动评审合并。路由处附有记录事故动机的注释,改动本身由逐字表达式 pin 测试与一个把门控中的 bot 字面量同 workflow 发布常量绑定的同步 pin 保护。为什么需要
并发组里 pending 状态的 run 会被同组任何更新的 run 顶掉——
cancel-in-progress保护不了它。PR #9091 恰好踩中:一次 push 与三个人类 review 请求落在同一分钟内。synchronize run 取消了上一 head 的在跑评审,随后在其退出组之前排队等待;三个review_requestedrun——必然的空跑,因为评审 job 只在 bot 自己被请求时才运行——在 synchronize run 仍处 pending 时将其逐一顶掉。唯一幸存者按设计跳过评审 job,在跑的评审轮被取消,这次 push 完全没有被评审。这属于 #8945 burst 家族,但那里的代价有界(仅启动开销),这里却是一次真实评审无声蒸发,而触发流程——推最后一个修复、随即请队友 review——是日常操作。审阅者验证方案
如何验证
运行 workflow pin 套件:
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-pr-review-workflow.test.js scripts/tests/qwen-resolve-workflow.test.js——新增 describe 块逐字 pin 了组表达式(形状本身就是修复:丢掉析取式的括号或把门控挪到 fallback 之后都会无声地重现竞态),resolve 套件既有的并发 pin(closed 取消、逐字cancel-in-progress)保持绿色。合并前无法实测真实并发语义,因为pull_request_target取 base 分支的 workflow 文件;合并后的检验标准是:某 PR 在 push 旁收到人类 "Request review" 时,synchronize run 能穿过延迟门进入评审 job,而请求 run 各自在自己的组里瞬间全 skip 完成;指向 bot 的请求 burst 仍与现状完全一致地合并为一个幸存者。证据(前后对比)
N/A——CI workflow 变更;事故证据在 PR #9091 的 run 历史里(synchronize run 零 job 被取消、空跑请求 run 成为唯一幸存者、在跑评审被杀)。
已测试平台
环境(可选)
vitest 对解析后的 workflow 做单元测试;autofix 套件的本地预存失败在未改动的基线上同样复现(macOS bash 3.2 缺
mapfile),与本 PR 无关。风险与范围
closed事件取消,但它们秒级全 skip 完成,本就无需中止。关联 Issue
相关:#8945(review_requested burst 家族;本 PR 关闭其丢评审变体,不解决启动浪费变体)。