feat(autofix): pick up managed fork PRs in real time instead of waiting for the throttled schedule - #7350
Merged
Merged
Conversation
…ng for the throttled schedule The `pull_request_review` trigger already routes feedback straight to the PR it arrived on, but it admitted ONLY in-repo bot PRs — every fork under takeover had to wait for the scheduled scan. That schedule is far slower than it looks: the cron says `*/10`, but GitHub throttles scheduled events on this repo hard enough that the observed interval is 40-70 minutes (and in the same window `pull_request_review` fired 17 times while `schedule` fired once). So the takeover PRs a maintainer is actively iterating on were the ones waiting longest for their feedback to be picked up. Real-time pickup now applies the SAME admission the scheduled scan uses for a fork: allow-edits on, and either the bot's own fork or an explicit autofix/takeover label. Nothing about *what* may run changes — this event runs in base-repo context, and review-address independently re-verifies allow-edits, a live write+ author and a matching live head repo before it touches the branch. Only *when* the same gated work happens changes. Unchanged: non-main targets, untrusted senders, human in-repo PRs and forks that are neither the bot's own nor takeover-labelled are all still ignored, and only `pull_request_review:submitted` triggers (not per-comment events). Tests: a behavioural replay drives the extracted route block with a stubbed gh across eight cases — in-repo bot admitted, in-repo human rejected, bot fork and takeover-labelled fork admitted and routed to that PR, and no-allow-edits, unlabelled fork, non-main base and untrusted sender all rejected. Mutation- verified: restoring the blanket fork rejection turns it red.
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed operational issue with measurements — the Direction: aligned. The autofix loop already manages fork PRs via the scheduled scan; this just moves the pickup from the throttled cron to the event path that's already firing reliably. Changes when, not whether — the trusted-sender gate and review-address re-verification are untouched. No CHANGELOG reference needed for CI infrastructure. Size: not applicable — no core module paths touched ( Approach: the scope feels right. The route step gains a fork branch that applies the same admission the scan already uses (allow-edits + bot fork or takeover label), with a single Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的运维问题，有实测数据—— 方向：对齐。autofix 循环已通过定时扫描管理 fork PR；本 PR 只是将拾起从被限流的 cron 移到已可靠触发的事件路径。改变的是"何时"而非"是否"——可信 sender 门和 review-address 重新校验均未改动。CI 基础设施无需 CHANGELOG 引用。 规模：不适用——未触及核心模块路径（仅 方案：范围合理。route 步骤新增 fork 分支，采用与扫描相同的准入规则（allow-edits + bot fork 或 takeover 标签），通过一次 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
Code ReviewIndependent proposal: given the throttled schedule and reliable event path, I'd replace the blanket fork rejection in the Comparison: the PR matches this almost exactly. The restructuring is clean — base ref check moves to the top (cheapest guard), in-repo and fork paths set One observation (non-blocking): the route doesn't check Verified review-address re-verification (lines ~2070–2120): the prepare step re-fetches live PR metadata and checks state, skip label, author/takeover, base, branch, allow-edits, fork author write+ permission, and head repo match. The route-side check only decides when the same gated work happens. No critical blockers. No AGENTS.md violations. TestingThis is a CI workflow routing change — the testable artifact is the unit test suite that drives the real extracted route block under bash with a stubbed Static checks: YAML parse ✓ · Mutation verification: restored the blanket fork rejection ( 中文说明代码审查独立方案： 鉴于定时扫描被限流而事件路径可靠，我会将 对比： PR 与独立方案几乎完全一致。重构干净——base ref 检查移到最前（最廉价的守卫），in-repo 和 fork 路径设置 一个观察（非阻塞）：route 未检查 fork PR 的 已验证 review-address 重新校验（约第 2070–2120 行）：prepare 步骤重新获取实时 PR 元数据，检查状态、skip 标签、作者/takeover、base、分支、allow-edits、fork 作者 write+ 权限、head 仓库匹配。route 侧仅决定"何时"执行相同的受控工作。 无关键阻塞项。无 AGENTS.md 违规。 测试这是 CI 工作流路由变更——可测试产物是在 bash 下用 stub 静态检查： YAML 解析 ✓ · route 步骤 变异验证： 恢复一刀切 fork 拒绝后，新行为测试和两个更新的断言测试变红，确认测试确实覆盖了新的 fork 准入路径。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean across every stage; the problem is measured, the fix is minimal, and the security model is preserved. This is a tight, well-motivated change. The schedule throttling is real (40–70 min observed spacing on a */10 cron), and the fix does the obvious right thing: admit fork PRs on the event path using the exact same criteria the scan already uses. The What I verified:
No reservations. Ships the feature cleanly. 中文说明置信度：5/5 —— 每个阶段都干净；问题有实测数据支撑，修复最小化，安全模型完整保留。 这是一个紧凑、动机充分的变更。定时扫描限流是真实的（*/10 cron 实测间隔 40–70 分钟），修复做了显而易见的正确事情：在事件路径上用与扫描完全相同的标准准入 fork PR。 已验证：fork 准入镜像扫描自身标准；review-address 独立实时重新校验所有条件；67/67 测试通过，新行为测试在 bash 下驱动真实 route 块覆盖 8 种场景，变异验证确认测试确实有效；YAML 解析和 无保留意见。干净地交付了功能。 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
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. |
wenshao
added
the
autofix/takeover
Collaborator
|
🤝 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 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
requested changes
Jul 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
[Critical] .github/workflows/qwen-autofix.yml:1466 — review-scan's forced-PR predicate unconditionally rejects all fork PRs via (.isCrossRepository == false). When the route step admits a managed fork PR and sets ROUTE_PR, review-scan evaluates the predicate — isCrossRepository is true for forks, so OK is not "true", and review-scan emits targets=[] and has_targets=false. Review-address (gated on has_targets == 'true' at line 1920) never starts. The feature this diff implements is silently discarded. The existing test at line 1640 explicitly validates this rejection and was not updated.
— qwen3.7-max via Qwen Code /review
Comment on lines
+1903
to
+1905
| `if [[ "$*" == *"--json labels,maintainerCanModify"* ]]; then printf '%s' ${JSON.stringify(meta)}; exit 0; fi`, | ||
| `if [[ "$*" == *permission* ]]; then printf '%s' ${JSON.stringify(perm)}; exit 0; fi`, | ||
| 'exit 1', |
Collaborator
There was a problem hiding this comment.
[Suggestion] The mock gh always exits 0 for --json labels,maintainerCanModify, so the workflow's else branch ("could not read fork PR metadata") is never exercised.
Concrete cost: A future change to the else branch that accidentally sets pr_is_managed=true on error would not be caught by the test suite.
Consider adding a test case where the mock gh exits non-zero for the metadata query and asserting DO_REVIEW=false.
— qwen3.7-max via Qwen Code /review
…ate (#7350) The route step now admits managed fork PRs for real-time review pickup, but review-scan's forced-PR predicate still required `.isCrossRepository == false`, so every fork was rejected there: targets=[] / has_targets=false and review-address never started — the feature was silently discarded for the very PRs it added. Admit forced fork PRs under the scheduled scan's OWN fork rules (allow-edits on, plus a live write+ author check mirroring the scan's per-candidate gate); in-repo PRs keep the fail-closed `.isCrossRepository == false` test. review-address still re-verifies allow-edits, a live write+ author and a matching head repo before pushing. Also exercise the route step's metadata-read-failure branch (fails closed) in the workflow tests.
Collaborator
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: Review feedback addressedCommit: No conflict ( 1. [Critical] review-scan's forced-PR predicate discards every fork PR — fixed (required)Confirmed against the code: the route step admits a managed fork PR and sets Change in
Test updates in
2. [Suggestion] metadata-read-failure branch never exercised — addressed (optional)The route-step fork test's mock Verification
中文说明已处理的评审反馈提交： 无冲突（ 1. [Critical] review-scan 的强制 PR 谓词会丢弃所有 fork PR —— 已修复（必需）已对照代码确认：route 步骤会接纳受管 fork PR 并设置
2. [Suggestion] 元数据读取失败分支从未被覆盖 —— 已处理（可选）route 步骤 fork 测试中的 mock 验证
Base-conflict check: no conflict with main. Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
approved these changes
Jul 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
Collaborator
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: No action takenThe automated reviewer ( No code changes are needed for this review round. 中文说明未采取任何操作自动审查机器人（ 本轮审查无需进行任何代码更改。 Base-conflict check: no conflict with main. 🧠 Handled by Qwen Code · model/模型 |
Collaborator
Author
Review —
|
Collaborator
|
🤖 Could not address the latest feedback automatically (round 2/100). A human should take over this PR. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/29793305120 🧠 Handled by Qwen Code · model/模型 |
yiliang114
approved these changes
Jul 21, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. Route mirrors the scan's fork admission (allow-edits + bot-or-takeover) and fails closed on every rejection path — metadata read failure, missing allow-edits, unlabelled human fork, untrusted sender. The forced-PR path's if/then/else also fixes the old // true | not jq trap cleanly. Concurrency group collapses burst reviews per-PR so this won't stack runs.
Tests: 71/72 pass locally; the one timeout ("eligibility recheck", L562) is pre-existing and unrelated.
This was referenced Jul 21, 2026
This was referenced Jul 21, 2026
Collaborator
|
Released in v0.20.1. |
Collaborator
|
PR #7350 已合并（2026-07-21），bot 评论只是发布通知：该改动已随 v0.20.1 发布。无需代码修改或回复。 ✅ completed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Lets the autofix loop pick up feedback on managed fork PRs the moment a review is submitted, instead of leaving them to the scheduled scan.
The
pull_request_reviewtrigger already exists and already routes feedback straight to the PR it arrived on — but it admitted only in-repo bot PRs. Every fork under takeover (autofix/takeover, or the bot's own fork) fell through to the scheduled scan. Real-time pickup now applies the same admission the scan itself uses for a fork: allow-edits on, and either the bot's own fork or an explicitautofix/takeoverlabel.Why it's needed
The scheduled scan is much slower than its cron suggests.
qwen-autofix.ymldeclares*/10 * * * *, but GitHub throttles scheduled events on this repo hard enough that the observed spacing between scheduled runs is 40–70 minutes (measured across the last 30 scheduled runs; it was 20–25 min the previous evening and has been degrading). In the same window,pull_request_reviewfired 17 times whileschedulefired once — the event path is reliable, the cron is not.The practical result was backwards: takeover PRs — the ones a maintainer is actively iterating on and explicitly opted into — were the ones waiting longest for their feedback to be picked up, because they were the only managed PRs with no real-time path.
Why it's safe
This changes when work happens, never whether it may:
pull_request_reviewruns in base-repo context; a fork PR adds no capability the scheduled scan does not already have when it admits the same PR.review-addressindependently re-verifies allow-edits, a live write+ author and a matching live head repo before it touches the branch. The route-side check is a fast filter, not the security boundary.maintargets, untrusted senders, human in-repo PRs, and forks that are neither the bot's own nor takeover-labelled. Onlypull_request_review:submittedtriggers (not per-comment events).Reviewer Test Plan
How to verify
npx vitest run scripts/tests/qwen-autofix-workflow.test.js— 68/68. The new behavioural test drives the real extracted route block under bash with a stubbedgh, across eight cases:autofix/takeover, allow-editsROUTE_PRmainbase / untrusted senderMutation-verified: restoring the blanket fork rejection turns the test red.
Static (run locally with the exact CI toolchain):
js-yamlparses the workflow; the route step passesbash -n; actionlint 1.7.12 clean; prettier clean.Post-merge smoke: submit a review on a takeover fork PR (e.g. fix(review): make agent launches and cleanup resilient #7259) and confirm a
pull_request_reviewautofix run starts within seconds rather than at the next scheduled scan.Evidence (Before & After)
Tested on
Risk & Scope
Linked Issues
Follow-up to #7213 / #7243 (fork takeover) and #7330 (verify gate). Motivated by measured scheduled-scan latency while triaging #7208 / #7259 / #7262.
中文说明
本 PR 做了什么
让 autofix 在 review 提交的瞬间拾起被托管的 fork PR 的反馈,而不是把它们丢给定时扫描。
pull_request_review触发器本就存在、也本就能把反馈直接路由到对应 PR —— 但它只准入 in-repo 的 bot PR。所有处于托管的 fork(autofix/takeover标签,或 bot 自己的 fork)都只能等定时扫描。现在实时拾起采用与扫描自身完全相同的 fork 准入规则:勾选 allow-edits,且要么是 bot 自己的 fork、要么带显式autofix/takeover标签。为什么需要
定时扫描远比它的 cron 慢。
qwen-autofix.yml写的是*/10 * * * *,但 GitHub 对本仓库的定时事件限流严重,实测最近 30 次定时运行的间隔是 40–70 分钟(前一晚还是 20–25 分钟,在持续劣化)。同一时间窗内pull_request_review触发了 17 次,而schedule只有 1 次 —— 事件路径可靠,cron 不可靠。结果是反的:托管 fork PR —— 维护者正在积极迭代、且显式选择加入的那些 —— 反而因为没有实时路径,等得最久。
为什么安全
本 PR 改变的是何时执行,而非是否允许执行:
pull_request_review在基座仓库上下文运行;当扫描准入同一个 PR 时,fork 并不带来任何扫描不具备的能力。review-address在动分支前会独立重新校验 allow-edits、实时 write+ 作者、以及实时 head 仓库匹配。route 侧只是快速过滤,不是安全边界。main目标、不可信 sender、人类 in-repo PR、以及既非 bot 自有也无 takeover 标签的 fork;且只有pull_request_review:submitted触发(非逐条评论事件)。评审验证
npx vitest run scripts/tests/qwen-autofix-workflow.test.js—— 68/68。新增行为测试用 stub 的gh在 bash 下驱动真实提取的 route 块,覆盖 8 种场景:in-repo bot 准入、in-repo 人类拒绝、bot fork 与 takeover 标签 fork 准入(新)、准入后路由到该 PR 号、无 allow-edits / 无标签人类 fork / 非 main / 不可信 sender 全部拒绝。变异验证:改回一刀切拒绝 fork 即变红。bash -n;actionlint 1.7.12 clean;prettier clean。pull_request_review运行在数秒内启动,而非等到下一次定时扫描。风险与范围
关联 Issue
#7213 / #7243(fork 托管)与 #7330(验证门)的后续。由排查 #7208 / #7259 / #7262 时实测到的定时扫描延迟驱动。