feat(autofix): auto-update a PR red only from a stale, since-fixed base - #7554
Conversation
A PR can be red purely because it merged a main that was broken then and is fixed now — observed twice today: a web-shell TS break and an agent-registry test, each stranding healthy PRs on a failure with nothing to do with them. The recovery was manual: merge current main and let CI re-run. The scan now does that automatically via GitHub's update-branch (a merge, never a rebase, so no force-push and no dismissed history). The single safety gate is that the SAME failing check is passing on current main. That one condition proves both halves at once: the red is base-inherited (green on main = not the PR's own bug) AND main is healthy on that check right now (so the merge cannot import a fresh breakage). It acts only when the PR is also BEHIND main (compare status behind/diverged) — otherwise the update is a no-op and the red is not stale-base after all. Self-limiting: after the update the PR contains main's head, so it is no longer behind and the next scan will not re-update. A failed update (merge conflict) is logged and the PR is left for a human. Runs before the feedback logic because a stuck-on-stale-base PR often has no new feedback at all — it just sits red — which is exactly what stranded #7490.
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/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 冲突，直到移除标签或达到轮次上限。移除 |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. Two concrete incidents today — a web-shell TS break and an agent-registry test failure on Direction: aligned. This is autofix-reliability infrastructure — it stops the scan loop from leaving healthy PRs terminally red after a transient Size: not applicable — no core paths. 149 additions across 2 files (workflow YAML + test). Approach: the scope feels right. One safety gate ("the same failing check passes on current main") proves both halves at once — the red is base-inherited AND main is healthy on that check right now. The merge-not-rebase choice, the self-limiting property (after update the PR is no longer behind), and the fail-safe (empty green set → no match → no update) are all deliberate and well-reasoned. No unrelated changes, no scope creep. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的实际问题，非理论性加固。今天发生了两次具体事件——main 上的 web-shell TS 错误和 agent-registry 测试失败——导致包括 #7490（第 11 轮）在内的健康 PR 在它们从未触碰的检查上终态搁置。两次的手动修复都是同一步骤：合入当前 main 让 CI 重跑。本 PR 将该步骤自动化。 方向：对齐。这是 autofix 可靠性基础设施——阻止扫描循环在 main 瞬时故障自愈后仍将健康 PR 留在终态红色。与 #7438、#7482、#7490 属同一主线。未触及公共契约、认证或沙箱。 规模：不适用——未触及核心路径。149 行新增，2 个文件（workflow YAML + 测试）。 方案：范围合理。一个安全门（"同一失败检查在当前 main 上通过"）同时证明两半——红是 base 继承的，且 main 此刻在该检查上健康。merge 而非 rebase 的选择、自限性质（更新后 PR 不再落后）、失败安全（空绿色集合 → 不匹配 → 不更新）都是刻意设计且合理。无无关改动，无范围蔓延。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Review & Local Verification Report代码审查设计评价：精准的安全门控自动化。 本 PR 解决了"健康 PR 因合并了当时已损坏的 main 而变红"的问题——自动合并当前 main 并重跑 CI。 安全门控（核心设计）：
这一个条件同时证明了：
实现细节：
测试： 从 workflow YAML 中提取 bash 块，用 mock 结论LGTM。 单一安全门控（check 在 main 上绿）简洁有力，避免了过度工程化的多条件判断。注释解释了 why（#7490 的真实案例）。 |
Code ReviewIndependent proposal: given the problem (PRs go red because they merged a broken Comparison with the diff: the PR's approach matches this proposal exactly. No simpler path missed. Findings — no critical blockers:
Real-Scenario TestingRan the extracted decision block from the PR's workflow YAML under bash with a stubbed Unit tests: 中文说明代码审查独立方案： 给定问题（PR 因合入当时已坏、现已修复的 main 而变红），我的方案是：(1) 每次扫描取一次 main 的 head 与其上当前通过的检查名集合；(2) 对每个候选 PR，算出在 PR 上失败但在 main 上同名通过的检查；(3) 若该集合非空且 PR 落后于 main，调 与 diff 对比： PR 的方案与此完全一致。未发现更简路径。 审查结论——无关键阻塞项：
真实场景测试从 PR 的 workflow YAML 中提取决策块，在 bash 下用桩 单元测试： — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; the problem is real, the safety gate is provably correct, and the implementation is exactly as simple as it can be. This is the kind of PR that makes the autofix loop meaningfully more reliable. The problem was observed twice in a single day — healthy PRs stranded on a The implementation is minimal — 149 lines across 2 files, no abstractions, no config, no new dependencies. The self-limiting property (after the update the PR is no longer behind, so the next scan skips it) means no marker or state tracking is needed. The fail-safe (empty green set → no match → no update) means an API hiccup can't cause spurious merges. The test extracts the actual decision block from the YAML and runs it under bash, which is the right way to test embedded shell logic — it catches drift between the test and the real code. Going back to my independent proposal: the PR matches it exactly. I didn't find a simpler path, and I don't think one exists. Every line in the diff earns its place. 中文说明置信度：5/5 —— 每个阶段都干净；问题真实存在，安全门可证明正确，实现已是最简。 这个 PR 让 autofix 循环的可靠性有了实质性提升。问题在一天内被观测到两次——健康 PR 搁置在已自愈的 main 故障上，每次都需要相同的手动修复。该方案用一个合理的安全门将这个修复自动化：仅当特定失败检查被确认在当前 main 上为绿时才合入 main。这一个条件同时证明两半（红是 base 继承的，且 main 此刻在该检查上健康），对于这类自动化来说，这几乎是最严密的安全论证。 实现极简——149 行，2 个文件，无抽象、无配置、无新依赖。自限性质（更新后 PR 不再落后，下次扫描跳过）意味着不需要 marker 或状态跟踪。失败安全（空绿色集合 → 不匹配 → 不更新）意味着 API 故障不会导致错误合并。测试从 YAML 中提取实际决策块并在 bash 下运行，这是测试嵌入式 shell 逻辑的正确方式——能捕获测试与真实代码之间的漂移。 回到我的独立方案：PR 与之完全一致。未发现更简路径，也不认为存在。diff 中每一行都有其存在价值。 — Qwen Code · qwen3.8-max-preview Reviewed at |
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. ✅
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. |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: No changes made — the only new feedback is an LGTM review from @gwinthis with no actionable items. The review confirms the design (single safety gate: a failing check must be green on current main) and implementation are sound, and explicitly concludes LGTM. There are no inline comments, no change requests, no failed checks, and no still-red checks to address. 中文说明未做任何更改——唯一的新反馈是 @gwinthis 的 LGTM 审查，没有需要处理的事项。 该审查确认了设计（单一安全门控：失败的 check 必须在当前 main 上为绿色）和实现均合理，并明确给出 LGTM 结论。没有行内评论、没有变更请求、没有失败的 check、也没有持续红色的 check 需要处理。 Base-conflict check: no conflict with main. 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: Review feedback addressed[rc:3635373023]
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: Autofix review feedback addressedBoth reviewer suggestions on PR #7554 were implemented. No conflict ( Feedback points1. [rc:3635572375] Missing The stale-base path computed 2. [rc:3635572378] Exclusion filter never exercised by the test helpers — IMPLEMENTED The behavioral test helpers never set
Changes
Conflict notesNone — no merge required. Verification
No settings source changed, so no schema regeneration was needed. The change is workflow YAML plus its colocated unit test, exercised through the Vitest harness rather than the bundled CLI, so no integration run was required. 中文说明已处理 Autofix 审查反馈PR #7554 上审查者的两条建议均已实现。无冲突（ 反馈要点1. [rc:3635572375] stale-base 分支会针对刚读取到的 PR head 计算 2. [rc:3635572378] 排除过滤器从未被测试辅助函数覆盖 — 已实现 行为测试的辅助函数从未设置
改动
冲突说明无 — 无需合并。 验证
未改动任何 settings 源文件，因此无需重新生成 schema。本次改动为 workflow YAML 及其同目录单元测试，通过 Vitest 测试框架而非打包后的 CLI 来验证，因此无需运行集成测试。 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
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: Address review summaryFeedback points
Conflict notesNo conflicts ( Verification
中文说明审查反馈处理总结反馈要点
冲突说明无冲突（ 验证
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/模型 |
…7562) * feat(autofix): auto-rerun a check that died on infrastructure, once A failed check can be red because the machine died, not the code — a self-hosted runner losing the server, the disk filling. #7490's E2E failed with "runner lost communication with the server" and went green on a rerun. The scan now reruns such a check's failed jobs automatically. Detection is a conservative annotation whitelist (INFRA_FAILURE_SIGNATURES) — only unambiguous machine failures, never a test-level timeout, which could be a real regression. The one-shot guard is run_attempt, not a marker: a run already retried to attempt 2 and still infra-failing is persistent, so it is left for a human; after a rerun the attempt increments, so the next scan will not rerun it. Every step is fail-safe (any API error → no rerun), it runs only when the PR actually has a failed check, and the gate carries the same review-address carve-out as the other check selectors so the loop never reruns its own runs. This is the transient-infra sibling of #7554 (stale-base): that merges current main when a check is base-inherited; this reruns when a check died on the runner. Neither touches a check that is a genuine failure. Note: rerun-failed-jobs needs the PAT to hold `actions: write`. * fix(autofix): use POSIX ERE groups in infra-failure regex, cover all signatures in tests (#7562) * fix(autofix): also treat a git fetch/clone transport death as infra #6506's checkout died mid-transfer — "fetch-pack: invalid index-pack output" and "RPC failed; curl 92 ... CANCEL" — which then hung the job into the 20m limit. That is infra, not the PR (it only touches a doc), and a re-run made it green. But the infra-signature whitelist did not cover it, so the auto-rerun did not fire and it waited on a human. Add `invalid index-pack output` and `RPC failed` — the two canonical git-transport-death phrases — to INFRA_FAILURE_SIGNATURES. A co-present job-timeout line does not block the match (one matching line classifies the run), and a BARE timeout with no transport signature is still left alone, since it can be a real regression. Both new signatures are pinned in the test's per-signature loop, plus a case on #6506's real composite annotation and a bare-timeout-is-not-rerun guard. * fix(autofix): paginate annotations and filter Autofix runs in infra-rerun loop (#7562) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: Review feedback addressed[Critical]
|
|
@qwen-code /resolve |
…7562) * feat(autofix): auto-rerun a check that died on infrastructure, once A failed check can be red because the machine died, not the code — a self-hosted runner losing the server, the disk filling. #7490's E2E failed with "runner lost communication with the server" and went green on a rerun. The scan now reruns such a check's failed jobs automatically. Detection is a conservative annotation whitelist (INFRA_FAILURE_SIGNATURES) — only unambiguous machine failures, never a test-level timeout, which could be a real regression. The one-shot guard is run_attempt, not a marker: a run already retried to attempt 2 and still infra-failing is persistent, so it is left for a human; after a rerun the attempt increments, so the next scan will not rerun it. Every step is fail-safe (any API error → no rerun), it runs only when the PR actually has a failed check, and the gate carries the same review-address carve-out as the other check selectors so the loop never reruns its own runs. This is the transient-infra sibling of #7554 (stale-base): that merges current main when a check is base-inherited; this reruns when a check died on the runner. Neither touches a check that is a genuine failure. Note: rerun-failed-jobs needs the PAT to hold `actions: write`. * fix(autofix): use POSIX ERE groups in infra-failure regex, cover all signatures in tests (#7562) * fix(autofix): also treat a git fetch/clone transport death as infra #6506's checkout died mid-transfer — "fetch-pack: invalid index-pack output" and "RPC failed; curl 92 ... CANCEL" — which then hung the job into the 20m limit. That is infra, not the PR (it only touches a doc), and a re-run made it green. But the infra-signature whitelist did not cover it, so the auto-rerun did not fire and it waited on a human. Add `invalid index-pack output` and `RPC failed` — the two canonical git-transport-death phrases — to INFRA_FAILURE_SIGNATURES. A co-present job-timeout line does not block the match (one matching line classifies the run), and a BARE timeout with no transport signature is still left alone, since it can be a real regression. Both new signatures are pinned in the test's per-signature loop, plus a case on #6506's real composite annotation and a bare-timeout-is-not-rerun guard. * fix(autofix): paginate annotations and filter Autofix runs in infra-rerun loop (#7562) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…n fix (QwenLM#7595) A fix that fails to build is not always the fix's fault. QwenLM#7471 stalled when the agent's verification gate failed with "Cannot find module 'update-notifier'" — a dependency main removed in QwenLM#7515, still imported on a branch 32 commits behind. The loop could not tell a stale-base build failure from a genuine one, so it advanced past the feedback and asked a human to take over. In the gate-rejection branch, before the handoff, compare the checked-out head with main; if it is behind or diverged, update-branch (a CAS on REPORT_HEAD) merges main in and the round retries (sentinel ts keeps the feedback live). It self-limits: after the update the PR is current, so a next-round rejection is no longer "behind" and falls through to the human handoff — a genuine fix failure costs at most one base-update. The round is exempt from the consecutive-failure breaker (not the PR's fault), and every API call is fail-safe. This is the agent-gate sibling of QwenLM#7554, which only sees PR status checks, never the gate's own build. Co-authored-by: wenshao <wenshao@example.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 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
ReviewThe idea is right and the block is written in the file's idiom — merge not rebase, 🔴 The green-on-main gate can never match the checks this targets
Measured against the live repo:
A second, independent starvation: Worth separating the two motivating incidents, because they need different fixes:
So the one in-scope case is the one the gate is blind to. Meanwhile the already-merged sibling #7595 gates on behind-main alone with no green-on-main condition — the asymmetry is worth a deliberate decision rather than an accident of which signal happened to be available. Directions, roughly in order of how much I'd trust them:
Whichever route: 🟡 Non-blocking
✅ What holds up
My call: the mechanism is sound, but I'd hold off merging until the green signal points at data that exists — otherwise this lands as a no-op that reads like a fix, and the next stranded PR still needs a human. 中文说明评审思路是对的,代码也符合本文件的既有风格 —— merge 而非 rebase、 🔴 green-on-main 这个门永远匹配不到目标检查
对线上仓库的实测:
还有第二重独立的"饥饿": 两个促成本 PR 的事故值得分开看,它们需要不同的修法:
也就是说,唯一落在本 PR 范围内的场景,恰恰是这个门盲掉的那个。与此同时,已合并的姊妹 PR #7595 只以 behind-main 设门,完全没有 green-on-main 条件 —— 这种不对称应当是一次有意的决策,而不是"哪个信号刚好拿得到"的偶然结果。 几个方向,大致按我的信任程度排序:
无论走哪条: 🟡 非阻塞
✅ 站得住的部分
我的结论:机制本身是成立的,但在绿信号指向真实存在的数据之前,我倾向先不合 —— 否则它落地后看起来像修复,实则是空操作,下一个被搁置的 PR 依然要人工处理。 🤖 Generated with Claude Code — Claude Opus 4.8 (1M context) |
Code review — auto-update a PR red only from a stale, since-fixed baseReviewed at head What works well
🔴 Primary concern — "green on current main" does not prove "base-inherited" (High)The PR's load-bearing claim is:
That implication does not hold. "Check X fails on the PR but passes on current main" is satisfied by two different situations:
Both present identically as red-on-PR / green-on-main. The gate cannot tell them apart, and case 2 is the normal state of any PR mid-iteration — which is exactly the population the scan targets (bot-authored PRs + takeover PRs, This is visible in the test itself. The first assertion — run({ prChecks: [FAIL('Test')], mainGreen: ['Test'] }) // behind → { updated: true }— is labeled "base-inherited red," but Why it bites, concretely — because the block runs before the feedback logic and
So the "only fires when confirmed base-inherited, which is rare" framing is inverted: the rare case (base-inherited) is the intended target, but the common case (self-red + behind) satisfies the same gate. The valuable half of the gate is real and worth keeping: excluding checks that are red on main too correctly avoids importing a live breakage. The gap is only the green-on-main branch conflating cases 1 and 2. What would actually distinguish them — a signal that the failure is not attributable to the PR's own diff. Options, roughly in increasing cost:
Green-on-main alone can't carry the "base-inherited" conclusion; one of these is needed to keep the automation from acting on ordinary broken PRs. 🟡 Secondary — ordering preempts the cheaper infra-rerun (Low)For a behind PR whose infra-flaked check happens to be green on main, the stale-base block fires first and does a full base-merge instead of the targeted single-job rerun the infra block ( Minor notes
VerdictMechanically sound and well-tested. I'd hold on merge until the green-on-main gate is narrowed with a "not caused by the PR's own diff" signal — as written it will act on ordinary self-broken behind PRs, and because it runs before and 中文说明机制实现（ 主要问题(High)——"在当前 main 上为绿"并不能证明"红是 base 继承的"。"检查 X 在 PR 上红、在当前 main 上绿"同时满足两种情况:(1) base 继承、且已在 main 上修复(目标场景,update-branch 有效);(2) PR 自己的新 bug——改动破坏了 X,而 main 从未有此改动,所以 X 在 main 上是绿的(update-branch 无效,bug 会随合并一起带过来)。两者表现完全相同,门无法区分,而情况 (2) 正是迭代中 PR 的常态,也正是扫描的目标群体(bot PR + takeover PR)。 测试第一条 为何有害:该块在反馈逻辑之前、成功即 门有价值的那一半应保留:排除"在 main 上也红"的检查,能正确避免引入实时故障。缺口只在 green-on-main 这一支混淆了两种子情况。需要一个"失败非源于 PR 自身改动"的信号(例如:PR head 是无内容增量的 base-merge 提交;或该检查在 PR 的 merge-base 上也红、仅在当前 main 上变绿;或至少只在本轮没有新反馈需处理时才更新)。 次要(Low):对一个落后 PR、其基础设施抖动的检查恰好在 main 上为绿时,stale-base 会先触发整轮 base-merge,而非 infra 块( 小注: 结论:机制正确、测试充分,但建议在把 green-on-main 门收窄到带"非 PR 自身改动导致"的信号之前先不合入。 |
…imension, bound repetition (#7554)
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 6/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressedAll feedback points from @wenshao's two reviews have been addressed. 🔴 Critical #1 — the gate can never match the checks it was built forFixed. 🔴 Critical #2 — "green on main" does not prove the red is base-inheritedFixed. Added the base dimension: the failing check must now satisfy TWO predicates — (1) green on current main, AND (2) NOT green at the merge-base commit the PR carries. The merge-base is extracted from the compare response ( 🟠 High #1 — "self-limiting, no marker needed" doesn't holdFixed. Two guards now bound repetition:
🟠 High #2 — runs before the in-flight-checks gateFixed. The block is now positioned after both the 🟡 Medium #1 — the
|
Code review — auto-update a PR red only from a stale, since-fixed baseReviewed at head The craftsmanship here is high, and the prior-round concerns are genuinely resolved. My one substantive concern is new, and it lands on the last commit ( What works well
🔴 Predicate 2 contradicts the definition of a stale-base breakage — the update can essentially never fire for the cases it targets
But a stale-base breakage is, by construction, a break that pre-merge CI did not catch — otherwise the base PR would never have merged. This repo has no
These two are mutually exclusive for any genuine stale-base red. Concretely for #7490 — the named motivating case, "went red on a test it never touched": its merge-base is the broken- The result is fail-safe (it degrades to the manual status quo, never a wrong action), but the automation is effectively inert for its own motivating scenarios. The 13-case test can't catch this: it feeds Recommendation — before relying on this, validate empirically: for #7490 (or any real stranded PR), fetch 🟡 Minor
VerdictMechanically clean, safe by construction, well-tested at the block level, and the prior livelock is gone. The blocker-for-usefulness (not for safety) is the 🔴: please confirm predicate 2 can actually be satisfied by a real stranded PR before merging — I believe it can't, which would make the feature a no-op for #7490-class incidents. Everything else is polish. 中文说明在 head 代码工艺很高,上一轮的问题也确实修好了。我唯一的实质担忧是新的,且落在最后一个提交( 做得好的地方: 🔴 predicate 2 与"陈旧 base 变红"的定义自相矛盾——它几乎永远不会为目标场景触发。 建议:合并前做实证——对 #7490(或任一真实被搁置的 PR),取 🟡 次要: predicate 1 有同样的代理替换但方向安全(证的是"main-HEAD 对应 PR 曾绿",非"main-HEAD 现绿";合并队列对必需检查成立);失败的更新每次扫描都会重试(成功才发 marker);遗留的 commit-status context 因 **结论:**机械上干净、构造上安全、块级测试充分、旧 livelock 已除。唯一"影响有用性(非安全)"的是 🔴:合并前请确认 predicate 2 能被真实搁置 PR 满足——我认为不能,若如此则该功能对 #7490 类事件是 no-op。其余皆为打磨。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 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
|
🤖 Addressed the latest review feedback (round 7/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 7/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed🔴 Predicate 2 (BASE_NOT_GREEN) is self-contradictory — @wenshao (review at
|
Code review —
|
| tree | injected selector | result |
|---|---|---|
base c5dddb5e |
select((.conclusion // .state // "") | IN("FAILURE")), no guard at all |
❌ expected [ Array(4) ] to have a length of 5 but got 4 |
PR 1cbb0eaa |
identical injection | ✅ 97/97 pass |
Per AGENTS.md — "a test weakened in this diff so new behavior passes is Critical" — and the regression is real: a future selector that reads the loop's own runs as feedback about the PR now sails through the test written to stop it.
The relaxation is legitimately needed (the new selector genuinely uses a different guard); only the form is wrong. Either count exclusion-only selectors:
const exclusionOnly = (
reviewScanJob.match(/!= "Qwen Autofix"\)\)?\s*\)/g) ?? []
).length;
expect(carveOutCount + exclusionOnly).toBe(scanCheckSelectors.length);or, more robustly, assert per selector that its own text carries one guard or the other:
const guardless = reviewScanJob
.split(/(?=IN\("(?:FAILURE|QUEUED)")/)
.slice(1)
.filter((seg) => {
const sel = seg.slice(0, 400);
return (
!/startswith\("review-address"\)/.test(sel) &&
!/!= "Qwen Autofix"/.test(sel)
);
});
expect(guardless).toHaveLength(0);Suggestion
4. The compare call is unconditional and pulls the full payload — ~60 KB × 60 candidates × every 10 minutes
.github/workflows/qwen-autofix.yml:2121-2127
if [[ -n "${MAIN_HEAD}" && -n "${PR_HEAD_OID}" ]]; then
CMP="$(gh api "repos/${REPO}/compare/${MAIN_HEAD}...${PR_HEAD_OID}" 2> /dev/null || echo '{}')"
CMP_STATUS="$(jq -r '.status // ""' <<< "${CMP}")"
if [[ … ]]; then
STALE_BASE_REDS="$(jq -c -n …)" # pure jq over data already in memory — freeSTALE_BASE_REDS costs nothing (it reads CHECKS_JSON and MAIN_GREEN_CHECKS, both already local) and is the far more selective predicate, yet the expensive network call runs first, for every candidate, including PRs with zero red checks. My replay confirms the compare fires even when the block is a guaranteed no-op. And with no --jq, the whole diff document lands in a shell variable — measured 60 785 bytes for this PR's own compare. With MAX_CANDIDATE_INSPECTIONS: '60' and a */10 cron, that is up to ~60 extra API calls and ~3.6 MB per scan, ~8 600 calls/day.
An earlier revision on this branch (edae6b74) had both right — if [[ "${STALE_BASE_REDS}" != '[]' && -n "${MAIN_HEAD}" && … ]] with --jq '.status'. Restoring that shape is a pure win and does not change behavior.
5. The rescue sits after the round-cap continue, so the most-stranded PRs are excluded
.github/workflows/qwen-autofix.yml:2067-2111 (round cap, continue) precedes the block at :2113. With MAX_ROUNDS: '10', a PR at or past its cap never reaches the stale-base check — but the PR body's own motivation is "stranded six healthy PRs (one at round 11)". The round cap exists to bound agent cycles; update-branch costs zero agent cycles and is exactly the mechanical unstick a capped-and-stranded PR needs. Moving the block above the round-cap gate (still below pending-checks) would cover the motivating case. If the current order is deliberate, the comment at :2117 should say why, since it currently reads as incidental.
Nice to have
:2140—date -u -d '120 minutes ago'forks a subshell per candidate PR inside the loop. It is loop-invariant; hoist it next toPENDING_CUTOFF(:1726), which is already hoisted with exactly this reasoning ("invariant across candidate PRs, computed once").:2132— external commit statuses are silently excluded: aStatusContextinstatusCheckRollupexposes.context, not.name, so.name // .workflowName // ""yields""andselect(. != "")drops it (confirmed by replay). Conservative and probably intended, but the comment at:2125only calls out theCANCELLEDomission — one more line would save the next reader the trace.- The PR body says 94/94; the branch is now at 97/97 after the later commits.
Security
Nothing exploitable found. RED_NAMES derives from check names (attacker-influenceable via a fork's workflow file) but only ever expands inside double quotes and reaches the comment body as a printf %s argument, so there is no injection path — worst case is cosmetic Markdown in a bot comment. expected_head_sha correctly prevents merging into a head the loop never inspected. Fork PRs without allow-edits land in the base-update-failed path, as documented.
中文摘要
在 1cbb0eaa 上审阅（对比 merge-base c5dddb5e）。我抽取真实决策块在 bash -eo pipefail 下用桩 gh 回放、跑了测试（97/97 通过）、对守卫不变量做了 A/B 变异验证，并调用真实 GitHub API 验证 MAIN_GREEN_CHECKS 在本仓库实际解析出什么。
整体设计方向正确：merge 而非 rebase、CAS 锁 head、各处失败安全、ahead 自限、marker 限制重复、PR 侧完全排除 Qwen Autofix、新 marker 已加入 BOT_COMMENT_FILTER。回放确认这些行为都与注释一致。
Critical 1（:2158）：marker 是这个「会改仓库」的动作唯一的重复护栏，却没有走本 job 强制的 SCAN_BOT_ACTOR 身份校验，并且用 > /dev/null 2>&1 || true 吞掉了所有错误。同 job 的另外两处评论写入（:1984 引擎 ack、:2106 上限提示）都做了身份校验并在失败时 ::warning::，其注释明确写了原因。一旦 PAT 轮换（或评论 POST 只是限流失败），去重永远为 false：每约 45 分钟就会对同一个 PR 再做一次 update-branch（CI 重跑约 40 分钟 → main 前进 → compare 再次 diverged），在整个托管队列上反复烧 CI 并作废 approval，而且无任何日志。另外 :2155 的 update-branch 本身也是写操作，却在任何身份校验之前执行。
Critical 2（:1744-1754）：MAIN_GREEN_CHECKS 量的不是 main 的健康度，而是「最后一个已合并 PR 的合并前 CI」。实测链路：main head 4f7429a0b → PR #7576 → head 28504e2cd 的 pull_request check-runs。这些跑的是 PR ⊕ 当时的 main，且 PR 只有绿了才能合，所以这个集合几乎必然包含必需检查。而 main 恰恰是被语义冲突打破的（两个 PR 各自绿、合在一起坏 —— 正是 PR 描述里的 web-shell TS 事故）：此时最后合并的那个 PR 是绿的，门会打开，循环于是把当前已坏的 main 合进所有健康 PR。「合入不会引入新故障」这一条恰好在最需要它的时候不成立。merge queue 本可堵住，但 ci.yml 的 merge_group 最近一次触发是 2026-07-02。另外绿集合还是「抽签」：#7576 上 macOS/Windows 的 Test 是 SKIPPED，因此卡在 macOS Test 的 PR 永远救不回来。建议弱化注释与 PR 描述里的安全性断言，并加一个佐证信号（例如要求最近 N 个已合并 PR 都绿，或核对该检查是否同时红在其它 open PR 上）。
Critical 3（测试 :305-312）：!= "Qwen Autofix" 是 carve-out 表达式的子串，所以每个 carve-out 选择器会被两个计数器各记一次。实测 selectors=5、carveOut=4、fullExclusion=5、和=9 —— 有 4 个单位的松弛。变异验证：在同一中性锚点注入一个完全没有守卫的失败检查选择器，base 树报错 expected length 5 but got 4，PR 树 97/97 全绿。按 AGENTS.md「本 diff 中为让新行为通过而削弱的测试属于 Critical」。修法见上方英文代码块（按选择器逐个断言，或只统计「仅有全排除」的选择器并用 toBe）。
Suggestion 4（:2121-2127）：compare 调用无条件先行且未加 --jq，把整份 diff 文档读进 shell 变量（本 PR 实测 60 785 字节）。而 STALE_BASE_REDS 是纯 jq、零成本且选择性强得多，应先算并短路。按 MAX_CANDIDATE_INSPECTIONS: '60' 与 */10 cron 计，每次扫描多约 60 次 API、约 3.6 MB。本分支早期版本（edae6b74）两点都是对的，建议恢复。
Suggestion 5（:2067-2111 → :2113）：该逻辑位于 round-cap 的 continue 之后，MAX_ROUNDS: '10'，因此达到上限的 PR 永远进不来 —— 但 PR 描述的动机正是「六个健康 PR（其中一个在第 11 轮）」。round cap 限的是 agent 轮次，而 update-branch 不消耗 agent 轮次。建议上移到 round-cap 之前（仍在 pending-checks 之后）；若顺序是刻意的，请在 :2117 注释里写明理由。
Nice to have：:2140 的 date 是循环不变量，应像 PENDING_CUTOFF（:1726）那样提到循环外；:2132 会静默排除外部 commit status（StatusContext 用 .context 而非 .name），:2125 注释只提了 CANCELLED，值得补一句；PR 描述写的 94/94，当前分支已是 97/97。
安全：未发现可利用问题。RED_NAMES 只在双引号内展开、且以 printf %s 参数进入评论正文，无注入路径；expected_head_sha 正确阻止了对未审查 head 的合并。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
…rrect the green-checks safety claim, per-selector guard test, gate the compare call (#7554)
|
🤖 Addressed the latest review feedback (round 8/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 8/100 轮）。改动内容与我反驳保留之处如下： Autofix review response — PR #7554Addressed all three Critical findings, both inline Suggestions (and the Critical
Suggestions
Nice-to-have
Verification
中文说明Autofix 评审回应 — PR #7554已处理全部三个 Critical、两条 inline Suggestion（以及与之重叠的 Suggestion 4）和成本较低的 nice-to-have。其中一条 Suggestion（round-cap 顺序）附带理由推迟，留待维护者决定。无冲突（ Critical
Suggestions
Nice-to-have
验证
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/模型 |
Review — round 3 (head
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
| # --jq '.status': the compare document is ~60KB; only the | ||
| # behind/diverged/ahead status is needed. | ||
| CMP_STATUS="$(gh api "repos/${REPO}/compare/${MAIN_HEAD}...${PR_HEAD_OID}" --jq '.status // ""' 2> /dev/null || echo '')" | ||
| if [[ "${CMP_STATUS}" == 'behind' || "${CMP_STATUS}" == 'diverged' ]]; then |
There was a problem hiding this comment.
[Suggestion] diverged status also triggers update-branch, which could introduce more semantic conflicts. The CAS + merge (revertable) controls blast radius, and the comment acknowledges this risk — acceptable trade-off.
— qwen3.8-max-preview via Qwen Code /review
| # the update would merge a currently-broken main into a healthy PR. The | ||
| # signal also inherits the last PR's matrix shape (a SKIPPED platform | ||
| # job is absent, so a PR stranded on it is never unstuck — fail-safe, | ||
| # but non-deterministic). The blast radius stays recoverable, not zero: |
There was a problem hiding this comment.
[Suggestion] MAIN_GREEN_CHECKS is based on last-merged PR's pre-merge check-runs, not main's current tree (ci.yml has no push trigger). In semantic conflict scenarios this signal may false-positive green. The comment documents this limitation well — merge queue is the proper fix.
— qwen3.8-max-preview via Qwen Code /review
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. Well-designed stale-base auto-update with proper safety gates (failing check must pass on main + behind/diverged + CAS + 2h marker guard). Thorough test coverage. Two inline suggestions on known trade-offs (diverged semantic conflicts, MAIN_GREEN_CHECKS limitation) — both already well-documented in comments.
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 9/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 9/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed (round 4, head
|
…r sites, note .[0] order (#7554)
Reverts QwenLM#7602. The Fleet Shepherd (qwen-fleet-shepherd.yml) now keeps the bot fleet within 25 commits of main by proactively update-branching, and QwenLM#7595 already retries a stale-base gate rejection at ANY behind distance instead of parking — so a PR no longer parks because its base went stale. That leaves QwenLM#7602 firing only on a PR parked by a GENUINE failure that later drifted behind main, where re-arming it just re-runs a real failure on a fresh base — speculative, near-zero value, and it carried its own autofix-handoff marker plus scan/report logic and tests. The retroactive cases it was built for (PRs parked before QwenLM#7595) were already recovered by hand. Keeps QwenLM#7595 (reactive stale-base gate recovery below the shepherd's threshold) and QwenLM#7554 (check-driven stale-base sync) — both cover the sub-25-behind window and triggers the shepherd does not. Co-authored-by: wenshao <wenshao@example.com>
5 participants
What this PR does
Teaches the autofix scan to automatically unstick a PR that is red only because it merged a
mainthat was broken at the time and has since been fixed — by merging currentmainin (GitHub's "Update branch"), gated on one safety condition.Why it's needed
This happened twice today alone:
main, failed the trusted-base build across a whole scan batch, and stranded six healthy PRs (one at round 11) terminally.agent.test.tsfork/registry test was broken onmainfor ~1 hour; fix(autofix): retry a skipped-Prepare instead of stranding the PR terminal #7490 merged that broken snapshot and went red on a test it never touched.In both cases
mainself-healed within the hour, and the fix was the same manual step: merge currentmaininto the PR and let CI re-run. That is exactly what GitHub's "Update branch" does. This automates it.How
Once per scan, the scan fetches
main's head and the set of check names currently passing on it. For each candidate PR, it computes the checks that are failing on the PR but passing onmainby the same name. If that set is non-empty and the PR is behindmain(compare statusbehind/diverged), it callsPUT /pulls/{n}/update-branchand skips the PR this scan (the merge re-runs CI).The single safety gate is "the same failing check passes on current
main." That one condition proves both halves at once:mainmeans it is not the PR's own bug.mainis healthy on that check right now — so mergingmainin cannot import a fresh breakage.Without that gate, auto-updating would churn genuinely-broken PRs and could pull a new
mainregression into a healthy one. With it, the action is safe by construction.Deliberate properties:
update-branchmerges; no force-push, no dismissed history, consistent with the review-in-progress conventions.main's head, so it is no longer behind and the next scan will not re-update. No marker needed.main's check set can't be fetched it is empty, so nothing matches and nothing is updated. A merge conflict on update is logged (base-update-failed) and the PR is left for a human.review-addresscarve-out as the other check selectors, so it never acts on the loop's own run status.Reviewer Test Plan
How to verify
npx vitest run scripts/tests/qwen-autofix-workflow.test.js— 94/94, three runs. The new test replays the real extracted decision block under bash with a stubbedgh(compare + update-branch), over six cases:TestFAILTestgreenTestFAILTestredTestFAILTestgreenLintFAILLintgreenTestOKTestFAILTestgreen, update failsMutation-verified: removing the green-on-main gate (would churn a genuinely-broken PR) turns it red; removing the behind check (would no-op-update a current PR) turns it red.
Static:
js-yamlparses; all 37run:blocks passbash -n; actionlint 1.7.12 clean; prettier clean.Post-merge smoke: after a transient
mainbreakage that a batch of PRs merged, the next scan should log🔀 #N: … merged current main via update-branchfor the affected PRs and they should recover oncemainis green — instead of a manual/retryper PR.Evidence (Before & After)
Tested on
Risk & Scope
dismiss_stale_reviews, dismisses existing approvals. That is the accepted cost of unsticking; it only fires when a check is confirmed base-inherited, which is rare, so it is not a churn source.maincould break between the check and the merge. The window is seconds, and the result is a merge (recoverable), not a force-push.update-branchneeds allow-edits; without it the API returns an error and the PR is logged asbase-update-failedrather than updated. Non-fatal.mainbreakage (a separate base-branch problem); it only stops the loop from leaving healthy PRs stranded on it. It also does not retry a check that is red on both the PR andmain— that is the PR's own failure and stays for the agent/human.Linked Issues
Motivated by two
mainbreakages today (web-shell TS, agent-registry test) that stranded healthy PRs including #7490. Part of the autofix-reliability line: #7438, #7482, #7490.中文说明
本 PR 做了什么
让 autofix 扫描自动解救一个仅仅因为合并了当时坏、现已修复的 main 而变红的 PR —— 通过合入当前 main(GitHub 的 "Update branch"),并以一个安全条件设门。
为什么需要
仅今天就发生两次:main 上的 web-shell TS 错误让受信任 base 构建在整个扫描批次里失败、终态搁置六个健康 PR;
agent.test.ts的 fork/registry 测试在 main 上坏了约 1 小时,#7490 合了那个坏快照、在一个它从未触碰的测试上变红。两次 main 都在一小时内自愈,修复都是同一手动步骤:合入当前 main、让 CI 重跑 —— 正是 "Update branch" 做的事。本 PR 把它自动化。怎么做
每次扫描取一次 main 的 head 与其上当前通过的检查名集合。对每个候选 PR,算出在 PR 上失败、但在 main 上同名通过的检查。若该集合非空且 PR 落后于 main(compare 状态 behind/diverged),则调
PUT /pulls/{n}/update-branch并本轮跳过该 PR(合并会重跑 CI)。唯一的安全门是"同一失败检查在当前 main 上通过",这一条同时证明两半:(1) 红是 base 继承的(main 上绿 = 不是 PR 自己的 bug);(2) main 此刻在这个检查上健康(合入不会引入新故障)。没有这个门,自动更新会搅扰真正坏掉的 PR,还可能把 main 的新回归拉进健康 PR;有了它,该动作由构造保证安全。
刻意的性质:merge 而非 rebase(不 force-push、不作废历史);自限(更新后 PR 已含 main head、不再落后,下次扫描不重复,无需 marker);失败安全(取不到 main 检查集合则为空,不匹配即不更新;合并冲突记为
base-update-failed交人);在反馈逻辑之前运行(卡在旧 base 的 PR 常常根本没有新反馈,#7490 正是如此);stale-base 选择器带与其它检查选择器相同的review-addresscarve-out,绝不作用于循环自己的运行状态。评审验证
npx vitest run scripts/tests/qwen-autofix-workflow.test.js—— 94/94,连跑三次。新测试用桩gh(compare + update-branch)在 bash 下回放真实提取的决策块,覆盖六种情形(见上方英文表格)。run:块过bash -n;actionlint 1.7.12 clean;prettier clean。🔀 #N … merged current main via update-branch,并在 main 变绿后自行恢复,而无需逐个/retry。风险与范围
dismiss_stale_reviews下作废已有批准。这是解卡的既定代价;它只在检查被确诊为 base 继承时触发,较罕见,不会成为搅扰源。base-update-failed而非被更新。非致命。关联 Issue
由今天两次 main 故障(web-shell TS、agent-registry 测试)促成,它们搁置了包括 #7490 在内的健康 PR。属 autofix 可靠性主线:#7438、#7482、#7490。