fix(ci): drop pull_request_review events on closed PRs at the route gate - #9299
Conversation
Reviews on merged/closed PRs have nothing to address, yet each one started an autofix run that spun up a runner only to exit no-op. Observed 2026-08-16: 24+ finding-reply reviews on merged QwenLM#9222 and 26 runs on merged QwenLM#9189 within minutes (issue QwenLM#9296). Add a PR open-state clause to the route prefilter; the scheduled scan remains the backstop, and address-time revalidation already drops targets whose PR closed after dispatch.
|
✅ 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 reviewI started with an independent proposal before reading the diff: the waste happens because review events pass the route gate and provision a runner, so the only fix point that avoids provisioning is the expression-level job gate — append No blockers:
No critical findings, no AGENTS.md violations. TestingCI evidence from the PR's own checks, fetched via the API at review time (triage never executes PR code). The full-profile ubuntu suite (fork PRs run the full profile) is still in flight — this PR changes no product code, so the suite mainly guards against incidental breakage. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Sandboxed verification can narrow the remaining gap: 中文说明代码审查:读 diff 前先独立给出方案——浪费的根源是 review 事件通过 route 门后分配了 runner,唯一能在分配前拦截的位置是表达式级 job 门,即在合取式末尾追加 无阻塞问题:新子句的 OR 形态使所有非 review 路径不受影响(schedule / issues / pull_request label / issue_comment / dispatch 第一操作数恒真),open PR 上的 review 仍会路由——本 PR 上的一条人工 review 就在此 head SHA 上触发了 Qwen Autofix route run,保留路径是活的。 测试:证据来自 PR 自身 CI(审查时通过 API 抓取;triage 不执行 PR 代码)。full-profile ubuntu 套件(fork PR 走 full profile)仍在运行——本 PR 不改产品代码,套件主要兜底意外破坏。macOS/Windows 测试与 CLI 集成测试在 ci.yml 中按设计仅在 merge queue 运行,其跳过与本变更无关。下方表格区域由 finalize 流程在 CI 落定后就地更新。 沙盒验证可收窄剩余缺口: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — a minimal, fail-closed gate fix for a measured waste problem; correct by construction. Stepping back: the problem is real and measured — 24 runs spawned by finding-reply bursts on merged #9222, 26 runs in 69 seconds on merged #9189, ~500 runs at a 59% cancellation rate during the 2026-08-16 storm. Those numbers come from #9296's run data, not inference. The implementation matches my independent proposal exactly: the expression-level gate is the only fix point that avoids provisioning runners at all, and the diff carries nothing beyond that one clause and its documentation. Every change is necessary, there are no drive-by edits, and the rest of #9296 is correctly deferred to follow-ups. The dropped event class has nothing to address by definition — a closed PR cannot receive commits — and the scheduled scan remains the backstop for open PRs, so nothing is lost. In six months this reads as one more well-documented clause in a file that keeps records of its incidents. CI is still running on this commit (the ubuntu suite is in flight), so approval is deferred until CI lands green on 中文说明置信度:5/5 —— 针对已量化浪费问题的最小化、fail-closed 门修复,构造上即正确。 退一步看:问题真实且已量化——已合并 #9222 上的 finding 回复风暴产生 24 个 run,已合并 #9189 在 69 秒内产生 26 个 run,2026-08-16 风暴期间约 500 个 run、取消率 59%。这些数字来自 #9296 的 run 数据,不是推测。实现与我的独立方案完全一致:表达式级门是唯一能在分配 runner 之前拦截的修复点,diff 除该子句及其文档外没有任何多余内容。所有改动都是必要的,无顺手修改,#9296 的其余项正确地留给后续 PR。被丢弃的事件类按定义无可处理内容——closed PR 无法接收 commit——schedule scan 仍为 open PR 兜底,没有任何损失。六个月后回看,这只是这个保留事故记录的文件中又一条注释完备的子句。 CI 在此 commit 上仍在运行(ubuntu 套件进行中),因此批准推迟到 CI 在该 commit 上全绿后执行——finalize 流程会把批准落在同一被审查 commit 上;若有检查变红,则会在状态评论中标记。 — Qwen Code · qwen3.8-max Reviewed at |
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-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| # already drops targets whose PR closed after dispatch. | ||
| if: |- | ||
| ${{ github.repository == 'QwenLM/qwen-code' && (github.event_name != 'issue_comment' || (github.event.issue.pull_request && (startsWith(github.event.comment.body, '@qwen-code /takeover') || startsWith(github.event.comment.body, '@qwen-code /retry')))) && (github.event_name != 'pull_request' || github.event.label.name == 'autofix/takeover') }} | ||
| ${{ github.repository == 'QwenLM/qwen-code' && (github.event_name != 'issue_comment' || (github.event.issue.pull_request && (startsWith(github.event.comment.body, '@qwen-code /takeover') || startsWith(github.event.comment.body, '@qwen-code /retry')))) && (github.event_name != 'pull_request' || github.event.label.name == 'autofix/takeover') && (github.event_name != 'pull_request_review' || github.event.pull_request.state == 'open') }} |
There was a problem hiding this comment.
[Suggestion] The new route-gate clause (github.event_name != 'pull_request_review' || github.event.pull_request.state == 'open') has no test pin, although scripts/tests/qwen-autofix-workflow.test.js pins both sibling clauses of this same expression by substring (the /takeover prefilter at ~L5064, the /retry prefilter at ~L14331, and the pull_request label-gate clause at ~L2392). — Failure scenario: the route gate is one ~444-character single-line expression that this file has repeatedly reshaped. A future edit that drops or inverts the clause (e.g. == 'open' → != 'open', or the clause's || → &&) would pass CI silently: the first mutation re-creates the no-op run storms on merged PRs this PR fixes (issue #9296 — 24+ runs within minutes on #9222, 26 on #9189); the second drops every legitimate review on an OPEN PR at the gate and degrades the fleet to the 10-minute scheduled scan — precisely the failure the fork-signal test's verbatim-gate pin was written to prevent (qwen-autofix-fork-bridge-workflow.test.js L210–215).
Witness (mutation test run in the PR worktree): BASE npm run test:scripts → 54 files, 1203 passed | 11 skipped, 0 failures; MUTATION-1 (new clause deleted) → 1203 passed | 11 skipped, 0 failures — survived; MUTATION-2 (== 'open' flipped to != 'open') → 1203 passed | 11 skipped, 0 failures — survived. Both mutations survive the full suite, and a grep across all of scripts/tests/ finds no pin of the new clause for this workflow (the single hit, qwen-autofix-fork-bridge-workflow.test.js:221, belongs to the fork-signal workflow).
Suggested fix — add a pin next to the existing prefilter pins in scripts/tests/qwen-autofix-workflow.test.js:
expect(routeJob).toContain("(github.event_name != 'pull_request_review' || github.event.pull_request.state == 'open')");(or, following the fork-signal precedent, pin the whole route-gate expression verbatim with toBe so connective-order mutations are also caught).
中文说明
[Suggestion] 新增的 route 门子句 (github.event_name != 'pull_request_review' || github.event.pull_request.state == 'open') 没有测试 pin,而 scripts/tests/qwen-autofix-workflow.test.js 对同一表达式中的两个兄弟子句都有子串 pin(/takeover 预过滤约 L5064、/retry 预过滤约 L14331、pull_request label 门子句约 L2392)。— 失败场景:route 门是一个约 444 字符的单行表达式,本文件已多次重塑它。未来某次编辑若删除或反转该子句(如 == 'open' → != 'open',或子句的 || → &&),CI 会静默通过:前者会重现本 PR 要修复的已合并 PR 空转 run 风暴(issue #9296——#9222 数分钟内 24+ 个 run、#9189 26 个);后者会在门处丢弃 open PR 上的所有合法 review,使整个队列退化为 10 分钟一次的定时扫描——这正是 fork-signal 测试逐字 pin 门表达式所要防止的失效模式(qwen-autofix-fork-bridge-workflow.test.js L210–215)。
证据(在 PR worktree 中运行的变异测试):BASE npm run test:scripts → 54 个文件、1203 通过 | 11 跳过、0 失败;变异 1(删除新子句)→ 1203 通过 | 11 跳过、0 失败——存活;变异 2(== 'open' 翻转为 != 'open')→ 1203 通过 | 11 跳过、0 失败——存活。两个变异均在完整套件下存活,且对 scripts/tests/ 全量 grep 未找到针对本工作流新子句的任何 pin(唯一命中 qwen-autofix-fork-bridge-workflow.test.js:221 属于 fork-signal 工作流)。
建议修复——在 scripts/tests/qwen-autofix-workflow.test.js 现有预过滤 pin 旁新增一行:
expect(routeJob).toContain("(github.event_name != 'pull_request_review' || github.event.pull_request.state == 'open')");(或参照 fork-signal 先例,用 toBe 逐字 pin 整个 route 门表达式,以便连词顺序类变异也能被捕获。)
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
@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. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
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.21.14. |
What this PR does
Adds one clause to the autofix router job's expression-level prefilter gate: review-submission events for closed or merged pull requests are dropped before any runner starts. Only review events on open PRs route. The workflow's contract comments are updated to document the gate.
Why it's needed
Reviews on merged/closed PRs have nothing to address, yet each one currently starts an autofix run that spins up a runner only to exit — for fork PRs via the no-secrets exit branch, consuming a job slot for nothing. When findings are replied to in a burst after merge (each reply posted as its own standalone review submission), this produces a storm of no-op runs that then cancel each other through the per-PR concurrency group.
Measured on 2026-08-16 (full details in #9296): a merged PR received 24+ finding-reply reviews within 2.5 minutes (~5s apart), spawning 24 runs; another merged PR spawned 26 runs in 69 seconds. The workflow saw ~500 runs in about 3 hours with a 59% cancellation rate, and this storm pattern is a major contributor.
Nothing is lost by dropping these events: the scheduled scan engages any open PR with review context on its next tick, and targets whose PR closes after dispatch are already dropped by address-time revalidation.
Reviewer Test Plan
How to verify
This is a CI-workflow gate change; behavior is observable in run statistics rather than CLI output:
event_name != 'pull_request_review' || ...).Local verification performed: the full workflow parses as YAML; a truth-table simulation of the gate expression across nine event combinations produced the expected routing decisions, including fail-closed when the PR state is absent from the payload.
Evidence (Before & After)
Before: 2026-08-16 11:47–11:49Z — review events on merged PR #9222 triggered 24 Qwen Autofix runs (23 cancelled, 1 success that exited through the no-op fork branch); merged PR #9189 triggered 26 runs within 69 seconds. After: such events are expected to produce zero runs (dropped at the job gate, no runner provisioned).
Tested on
Environment (optional)
N/A — workflow gate change; no runtime code touched. The workflow itself runs on ubuntu-latest.
Risk & Scope
Linked Issues
Part of #9296 (the P0 item). No closing keyword — the issue tracks further work beyond this PR.
中文说明
这个 PR 做了什么
在 autofix 路由 job 的表达式级预过滤门上加一个子句:针对已关闭/已合并 PR 的 review 提交事件在任何 runner 启动之前就被丢弃,只有 open PR 上的 review 事件会被路由。同时更新了工作流的契约注释以记录该门。
为什么需要
已合并/已关闭 PR 上的 review 没有任何可处理的内容,但目前每条都会启动一个 autofix run——拉起 runner 后空转退出(fork PR 走 no-secrets 退出分支),白白消耗一个 job 槽位。当合并后出现逐条回复 findings 的爆发(每条回复作为独立 review 提交)时,会产生大量空转 run,并通过 per-PR 并发组互相取消。
2026-08-16 实测(完整细节见 #9296):一个已合并 PR 在 2.5 分钟内收到 24+ 条 finding 回复型 review(间隔约 5 秒),产生 24 个 run;另一个已合并 PR 在 69 秒内产生 26 个 run。该工作流约 3 小时内有 ~500 个 run,取消率 59%,此风暴模式是主要贡献者。
丢弃这些事件不会丢失任何东西:schedule scan 会在下一个 tick 接手任何有 review 上下文的 open PR;dispatch 后才关闭的 PR 目标已由 address-time revalidation 兜底丢弃。
Reviewer 测试计划
如何验证
这是 CI 工作流的门变更,行为体现在 run 统计而非 CLI 输出:
event_name != 'pull_request_review' || ...)。已完成的本地验证:完整工作流 YAML 解析通过;对门表达式做了 9 组事件组合的真值表模拟,路由决策全部符合预期,包括 payload 缺少 PR state 时 fail-closed。
证据(改动前后)
改动前:2026-08-16 11:47–11:49Z——已合并 PR #9222 的 review 事件触发 24 个 Qwen Autofix run(23 个 cancelled,1 个 success 但走空转 fork 分支退出);已合并 PR #9189 在 69 秒内触发 26 个 run。改动后:此类事件预期产生零 run(在 job 门处丢弃,不分配 runner)。
测试环境
环境(可选)
N/A——工作流门变更,未触碰任何运行时代码。工作流本身运行在 ubuntu-latest。
风险与范围
关联 Issue
属于 #9296(P0 项)。不使用关闭关键字——该 issue 还跟踪本 PR 之外的后续工作。