feat(autofix): consume the review convergence signals — a circuit breaker for the takeover loop - #10122
Open
wenshao wants to merge 1 commit into
Open
feat(autofix): consume the review convergence signals — a circuit breaker for the takeover loop#10122wenshao wants to merge 1 commit into
wenshao wants to merge 1 commit into
Conversation
…it breaker The review side has published a machine-readable convergence diagnosis since #9623 (recommendation codes matched from measurement), but nothing on the autofix/takeover side consumed it: on a non-converging PR the review names the pathology round after round while the loop's own brakes (round counter, growth budgets, failure caps) bound only the loop's own telemetry — the brakes slow each side, neither can stop the pair (#10107, measured on #9729: first diagnosis in round 3, ~13 more rounds after it). Review CLI: the posted ledger marker now carries the round's matched recommendation codes as `rec` — written off the same derivation the result and the rendered paragraph use, shape-bounded on the streak rung, write-only (parseLedger deliberately does not read it back). Autofix workflow: a convergence-signal circuit breaker. The scan counts the trailing streak of review-bot rounds whose codes intersect CONVERGENCE_SIGNAL_CODES, bounded by max(window key, newest trusted-human activity); at CONVERGENCE_BREAK_ROUNDS (default 3, repo-var tunable) the PR parks — one bilingual notice, no dispatch, no label — and resumes when a maintainer responds (fresh N-round runway), on /retry, or on re-engagement. Prepare mirrors the reading live and discards via STALE, like the growth-audit conflict park. 'land-and-defer' is deliberately not a signal code: it is an exit, not a divergence. Tests: serializer/compose coverage for the marker field, behavioral replays of both workflow gates, a lockstep pin between them, and a vocabulary pin of the workflow's code list inside RECOMMENDATION_CODES.
github-actions
Bot
added
the
review/self-reported
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed and measured, not theoretical. #10107 documents the runaway on #9729 — the review named the failure mode in round 3 and repeated it through round 15; both sides' brakes engaged and the loop ran ~13 more rounds regardless (~50 runner-hours, PR grew +1.7k → +5.6k lines). The missing piece is named precisely: the review side has published machine-readable convergence codes since #9623, and nothing on the autofix side consumes them. Direction: aligned. This is the repo's own automation plumbing, implementing exactly what the issue proposed — consume the codes that already exist rather than invent a new diagnosis, and keep the threshold on the consumer side so the review module's "measures, holds no policy" contract stays intact. No upstream CHANGELOG reference applies to internal CI tooling; the area is clearly relevant. Size: not a core path — Approach: the scope feels right. The two halves meet at the posted ledger marker — the one cross-account surface the workflow already parses — and Risk: no elevated risk signals (no high-risk paths matched). The residual risk — a wrongly-tripped park — is bounded by design: one comment, self-lifting on any maintainer response, Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测且有测量，不是理论问题。#10107 记录了 #9729 上的失控——评审第 3 轮就点名失败模式并重复到第 15 轮；双方刹车都触发了，循环仍又跑了约 13 轮（约 50 runner 小时，PR 从 +1.7k 行涨到 +5.6k 行）。缺失环节点得很准：评审侧自 #9623 起就发布机器可读的收敛码，autofix 侧没有任何东西消费它们。 方向：对齐。这是仓库自身的自动化管线，实现的正是 issue 提议的方向——消费已有的码而不是发明新诊断，且把阈值放在消费侧，保住评审模块"只测量、不持有策略"的契约。内部 CI 工具在上游 CHANGELOG 中无对应条目；该领域显然相关。 规模：非核心路径—— 方案：范围合理。两半在已发布的 ledger 标记处交汇——这是 workflow 已在解析的唯一跨账号表面——且 风险：无升级风险信号（未命中高风险路径）。残余风险——误触发驻停——按设计既有界：一条评论、任何维护者回应即自解除、 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewIndependent baseline before reading the diff: given "the takeover loop cannot see the review's convergence diagnosis", I would (1) publish the existing recommendation codes on the posted ledger marker — the one cross-account surface the workflow already parses, (2) count a trailing streak of signal-bearing rounds past the newest human/window boundary in the scan, parking at a threshold with a single self-lifting notice, and (3) close the dispatch race where a target is emitted moments before the tripping review lands. The PR matches this almost exactly; it goes beyond the baseline only where the race is real (the prepare mirror), and it pins each property with an executed test. I did not find a materially simpler path. No critical blockers. Verified against the code at the reviewed commit:
One standing non-blocking observation: the boundary+streak jq appears verbatim twice (scan gate and prepare leg). The lockstep pin makes any future divergence loud rather than silent — the right trade for a workflow file — but that pair is the part to edit carefully later. sequenceDiagram
participant P1 as Review bot
participant P2 as Ledger marker on the PR
participant P3 as Autofix scan
participant P4 as Prepare leg
participant P5 as Maintainer
P1->>P2: posts review, marker carries rec codes
P3->>P2: counts trailing signal rounds past the boundary
P3-->>P2: at threshold, parks and posts one notice
P4->>P2: re-derives live, discards via STALE
P5->>P3: any response moves the boundary, loop resumes
Test evidence — this PR's own CI (per policy this run never builds or executes PR code)CI classified this PR The behavioral claims rest on tests that execute the real bash/jq blocks against fixture state — not mocks — and CI's unit job runs both suites ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Not verified: live loop behavior on a real non-converging PR — that takes days of rounds to observe, and the executed block-level tests are the substitute. The park is self-lifting and the threshold is tunable via 中文说明代码审查读 diff 之前的独立基线：给定"takeover 循环看不见评审的收敛诊断"，我会(1)把已有的建议码发布到已发布的 ledger 标记上——workflow 已在解析的唯一跨账号表面；(2)在 scan 中统计最新人类/窗口边界之后的尾部连续信号轮数，达到阈值即驻停并发一条自解除通知；(3)堵住触发评审落地前一刻 target 已发出的派发竞态。PR 与这个基线几乎完全一致；唯一超出基线的是 prepare 镜像——针对的正是真实竞态——且每条性质都有可执行测试钉住。没有找到更简的路径。 无 Critical 阻塞项。已按被审 commit 的代码核对：
一个长期的非阻塞观察：边界+streak 的 jq 在 scan 门与 prepare 腿逐字出现两次。lockstep 钉让未来的分歧显形而非静默——对 workflow 文件是正确的取舍——但这一对是日后改动时要小心的部分。 （时序图见英文正文） 测试证据本次运行按规则不构建、不执行 PR 代码；证据取自本 PR 自己的 CI。CI 将本 PR 判为 未验证：真实不收敛 PR 上的活循环行为——需数天轮次才能观测，可执行的块级测试是其替代。驻停自解除、阈值可经 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — clean static review with executed behavioral tests and a bounded failure mode; approval deferred only until CI lands green on the reviewed commit. Stepping back: the problem is real and costed (~50 runner-hours on one PR, measured), the fix consumes signals that already exist instead of adding a parallel diagnosis, and the scope is exactly what #10107 asked for — the issue's third bullet is deliberately split out rather than bundled. The implementation is careful in the right places: one origin for the codes, a write-only carrier, fail-open on unknown markers, fail-closed on unreadable labels, a once-per-boundary notice, and a lockstep pin where the logic legitimately appears twice. My independent proposal was materially the same and no simpler — the one added part (the prepare mirror) closes a race that is real on the review's own trigger. If I maintain this in six months, the lockstep jq pair is the part I'd watch, and its verbatim pin makes drift loud rather than silent. The default threshold of 3 is a policy guess, but the park self-lifts on any maintainer response and the knob is a repo var, so being wrong is cheap. Approving — CI on the reviewed commit is still in progress, so the approval lands automatically once every check completes green on it, and is withheld if anything lands red or the head moves. 中文说明Confidence: 4/5 —— 静态审查干净、行为测试可执行、失败模式有界；仅延后到被审 commit 的 CI 转绿后批准。 退一步看：问题真实且有成本度量（单个 PR 约 50 runner 小时），修复消费的是已有信号而非新增平行诊断，范围恰是 #10107 所求——issue 的第三条被刻意拆出而非夹带。实现在该谨慎处谨慎：码单一来源、只写载体、未知标记向开放侧失败、不可读标签失败关闭、每边界一次的通知，以及在逻辑合理出现两次处的 lockstep 钉。我的独立方案实质相同且不更简——唯一新增（prepare 镜像）关闭的是评审自身触发路径上的真实竞态。若六个月后由我维护，需要盯的是那对 lockstep jq，而逐字钉让分歧显形而非静默。默认阈值 3 是策略猜测，但驻停自解除、旋钮是仓库变量，猜错的代价很低。 予以批准——被审 commit 的 CI 仍在进行，批准将在其上所有检查转绿后自动落地；若有检查转红或 head 移动，则不予批准。 — Qwen Code · qwen3.8-max Reviewed at |
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. |
qwen-code-ci-bot
approved these changes
Aug 26, 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 — CI landed green after the review. ✅
Collaborator
Author
|
@qwen-code /takeover |
qwen-code-dev-bot
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 冲突，直到移除标签或达到轮次上限。移除 |
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
3 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
Gives the takeover loop the off-ramp #10107 asks for, in two halves that meet at the posted review body.
Review CLI — publish the codes where an outside consumer can reach them. The convergence diagnosis has had a machine-readable half since #9623 (
recommendations: [{code, basis}]), but it rode only the composed result and the durable artifact — both in-process surfaces. The posted ledger marker now carries the round's matched codes asrec, written off the same derivation the result and the rendered paragraph use, so the codes a consumer wires and the sentences a human reads cannot describe different rounds. The field rides the streak rung (small, zero-omitted, above the byte-shed cascade — the PR whose loop is not converging is exactly the one whose marker sits at the cap), is shape-bounded on write, and is write-only:parseLedgerdeliberately does not read it back, because nothing CLI-side consumes a prior round's codes and recovering them would hand the next round a value another account's writable surface controls.Autofix workflow — the circuit breaker. The scan now counts the trailing streak of review-bot rounds whose
reccodes intersectCONVERGENCE_SIGNAL_CODES(root-cause-triage,batch-fixes,stem-surface), deduped by ledger round so a dismissed review and its re-run count once, reset by any marker round without a signal code (a healthy round, or one from a CLI predating the field — fail open). The streak is bounded by max(window key, newest trusted-human activity):/retryand re-engagement reset the breaker exactly like every other census, and a maintainer response IS the resume signal — every human touch grants a fresh runway. AtCONVERGENCE_BREAK_ROUNDS(default 3, tunable viaQWEN_AUTOFIX_CONVERGENCE_BREAK_ROUNDS) the PR parks in the growth-audit-conflict mold: one bilingual notice naming the streak and the codes (once per boundary, marker-deduped, PAT-verified, consent-rechecked), then silence — no dispatch, no runner, no round.Prepare branch and feedbackmirrors the same reading live and discards viaSTALE, covering targets emitted moments before the tripping review lands and forced dispatches; a test pins the two gates' jq programs verbatim-identical. Downshifting automatic re-review falls out for free: reviews are push-triggered, so a loop that stops pushing stops re-reviewing, while on-demand review keeps working.Deliberate choices, argued in
qwen-autofix.md#af-148..150: noNEEDS_HUMAN_LABEL(that label marks stops only a re-arm can lift; a self-lifting park wearing it would leave the label lying the moment a comment resumed the loop), no failed-check wake leg (this park's whole claim is that more automatic rounds are the problem, and outside CI going red must not re-open the loop with zero human input), andland-and-deferis not a signal code (it means "this loop can end by merging" — pausing on it would park exactly the PR a human should merge). The threshold lives on the consumer side on purpose: the review module's contract is that it measures and holds no policy.Why it's needed
Measured on #9729: the convergence observation named the failure mode in round 3 and repeated it through round 15; both sides' brakes engaged (critical floor from ~round 5; growth brake, Critical-only) and the loop ran ~13 more rounds regardless — ~50 runner-hours, the PR growing +1.7k → +5.6k lines, round 15 still posting fresh Criticals in loop-written code. Every existing brake bounds the loop's own telemetry (rounds, bytes, failures); the one diagnosis that matters on a non-converging pair comes from the review, and until now its only listener was a fix loop that cannot act on "split this PR / batch the fixes / stop" — while the human who could act is exactly the one takeover removed from the loop. #9623's own risk note tracked this as "the caller-side enforcement that consumes these codes is a separate PR"; this is that PR.
Reviewer Test Plan
How to verify
packages/cli:npx vitest run src/commands/review/— 5087 passed, 15 skipped.npx tsc --noEmitandnpx eslintover the touched files are clean. New coverage: the serializer carries/bounds/omitsrecand survives both byte-squeeze rungs; compose end-to-end asserts the marker's codes equalresult.recommendationsand that a no-diagnosis round writes none;parseLedgerstays blind to the field; the--comment-escape holds for it.npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js— 221/222 (the one failure,locks the runner file-command backing files against env plants, fails identically on unmodifiedmainin this environment — chmod-based fixtures do not bind under root — as do twoqwen-pr-review-workflowtests of the same class). New coverage executes the real scan block and the real prepare block against fixture state: the three-signal trip, healthy/pre-field-marker reset, exit-code non-counting, dismissed-round dedup, markerless-review transparency, human-response resume (and non-resume for the review bot, untrusted logins, and@qwen-code /commands), window-key reset, the malformed-knob fallback, and the mirror's discard/park/resume; plus the vocabulary pin (workflow codes ⊆RECOMMENDATION_CODES,land-and-deferpresent in the vocabulary and absent from the signal list) and the verbatim jq lockstep pin.recwrite dropped (5 tests), the marker decoupled fromresult.recommendations(compose pin), the streak made non-consecutive (else 0→else .— behavioral + lockstep), the human-activity boundary dropped (resume case), the prepare mirror'sSTALEassignment dropped (mirror test), andland-and-deferadded to the signal list (vocabulary pin).bash -npasses on both edited run blocks; the workflow YAML-parses;check-workflow-size.shis green with the.size-baselinebump this PR carries (+15,088 bytes of logic, prose inqwen-autofix.md);npm run check-i18npasses.Evidence (Before & After)
N/A — no user-visible TUI change. The ledger marker gains one field on rounds that produced a diagnosis; the new PR comment posts only when the breaker trips, and its body is asserted in the workflow tests (it deliberately does not begin with the fleet shepherd's terminal
🤖 AutoFix stoppedprefix).Tested on
Environment (optional)
Unit and workflow-replay tests only; no daemon or sandbox involved.
Risk & Scope
/retryclears it — while the failure it prevents ran for days. Rollout is inherently gradual: markers written by today's CLI carry norec, and such rounds reset the streak, so nothing parks retroactively and the breaker arms only as newly-deployed review rounds actually diagnose.land-and-defer,land-with-residual-risk) is a different feature; review autofix: decline auto-fixing a subsystem with a multi-round successor-defect history #9907 (per-subsystem decline) stays open as the narrower sibling; feat(review): warn when a subsystem's Criticals keep regrowing across rounds #10010'ssuccessor-chainjoins the breaker by a one-line addition toCONVERGENCE_SIGNAL_CODESonce its code exists — the vocabulary pin will hold the addition honest. On the issue's third bullet (start takeover-managed PRs at the critical posting floor from round 1): recommend yes, as a small separateqwen-code-pr-review.ymlchange (--severity-floor criticalwhen the takeover label is present) — kept out of this PR so the breaker's behavior lands and is observable on its own.recis optional and ignored by every existing parser (parseLedgerbuilds from known fields); old markers read back unchanged; the breaker treats their rounds as non-evidence.Linked Issues
Closes #10107. Follows #9623 / #9526 / #9938; related #9907, #10010.
中文说明
这个 PR 做了什么
为 takeover 循环补上 #10107 要求的"下匝道",分两半实现，交汇点是已发布的评审正文。
Review CLI——把建议码发布到外部消费者够得着的地方。 收敛诊断自 #9623 起就有机器可读的一半（
recommendations: [{code, basis}]），但只随合成结果与持久化 artifact 传出——都是进程内表面。现在已发布的 ledger 标记携带本轮匹配的码（rec字段），与结果字段、渲染段落出自同一次派生，因此消费者接线的码与人类读到的句子不可能描述不同轮次。该字段与 streak 同档位（小、零省略、位于字节裁剪级联之上——loop 不收敛的 PR 恰是标记顶到字节上限的 PR），写侧限形，且只写不读：parseLedger刻意不回读——CLI 侧没有任何东西消费上一轮的码，回读只会把另一账号可写表面控制的值交给下一轮。Autofix workflow——断路器本体。 scan 统计 review-bot 各轮
rec与CONVERGENCE_SIGNAL_CODES（root-cause-triage、batch-fixes、stem-surface）相交的尾部连续轮数：按 ledger 轮次去重（被 dismiss 的评审与其重跑只算一轮），任何不带信号码的标记轮清零（健康轮，或字段问世前的 CLI——向开放侧失败）。计数边界取 max(窗口键, 最新可信人类活动)：/retry与重新接管像其他 census 一样重置断路器，而维护者的回应本身就是恢复信号——每次人类介入都授予新的跑道。达到CONVERGENCE_BREAK_ROUNDS（默认 3，可用QWEN_AUTOFIX_CONVERGENCE_BREAK_ROUNDS调整）即按 growth-audit conflict 停摆模式驻停：一条双语通知点名轮数与码（每边界一次、标记去重、PAT 校验、许可复查），此后静默——不派发、不占 runner、不消耗轮次。Prepare branch and feedback实时镜像同一读数、经STALE丢弃，覆盖在触发评审落地前一刻已发出的 target 与强制派发；有测试钉住两侧 jq 程序逐字一致。自动重评的降档是免费得到的：评审由 push 触发，循环停止推送即停止重评，而按需评审照常可用。有意为之的取舍（详见
qwen-autofix.md#af-148..150）：不打NEEDS_HUMAN_LABEL（该标签标记只有 re-arm 才能解除的停止；自解除的驻停戴上它，维护者一条评论恢复循环的瞬间标签就在说谎）；不设失败检查唤醒腿（本驻停的全部主张就是"再跑自动轮次才是问题"，外部 CI 变红不得在零人类输入下重开循环）；land-and-defer不算信号码（它意味着"这个循环可以靠合入结束"——据它驻停恰好停住了人类应当合入的 PR）。阈值刻意放在消费侧：review 模块的契约是只测量、不持有策略。为什么需要
在 #9729 上的实测：收敛观察第 3 轮就点名失败模式并重复到第 15 轮；双方刹车都触发了（review 约第 5 轮起 critical floor；autofix 的 growth brake、Critical-only），循环照样又跑了约 13 轮——约 50 runner 小时，PR 从 +1.7k 行涨到 +5.6k 行，第 15 轮仍在循环新写的代码上发出新 Critical。现有每个刹车都只约束循环自身的遥测（轮数、字节、失败）；不收敛的这一对上真正要紧的诊断来自评审，而它至今唯一的听众是一个无法执行"拆 PR / 攒一批修 / 停下"的修复循环——能执行的人恰恰被 takeover 移出了环。#9623 的风险说明将此追踪为"消费这些码的调用侧执行是另一个 PR"；本 PR 就是那个 PR。
复核测试计划
如何验证
packages/cli：npx vitest run src/commands/review/—— 5087 通过、15 跳过；npx tsc --noEmit与受影响文件的npx eslint干净。新覆盖：序列化器携带/限形/省略rec且在两级字节挤压下存活；compose 端到端断言标记中的码等于result.recommendations、无诊断轮不写码；parseLedger对该字段保持失明；--注释转义对它同样成立。npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js—— 221/222（唯一失败locks the runner file-command backing files against env plants在未修改的main上同样失败——chmod 型夹具在 root 下不生效；qwen-pr-review-workflow的两个同类测试亦然）。新覆盖以真实 scan 块与真实 prepare 块对夹具状态执行：三信号触发、健康轮/旧字段标记清零、退出码不计数、dismiss 轮去重、无标记评审透明、人类回应恢复（review bot、不可信登录、@qwen-code /命令均不恢复）、窗口键重置、坏旋钮回退默认值、镜像的丢弃/驻停/恢复；另有词表钉（workflow 码 ⊆RECOMMENDATION_CODES，land-and-defer在词表内且不在信号表内）与 jq 逐字 lockstep 钉。rec写入（5 个测试红）、标记与result.recommendations解耦（compose 钉红）、streak 改为非连续计数（行为 + lockstep 红）、删除人类活动边界（恢复用例红）、镜像不置STALE（镜像测试红）、把land-and-defer加进信号表（词表钉红）。bash -n；workflow 可被 YAML 解析；check-workflow-size.sh在本 PR 携带的.size-baseline上调后为绿（+15,088 字节均为逻辑，散文在qwen-autofix.md）；npm run check-i18n通过。证据（前后对比）
N/A —— 无用户可见 TUI 变化。产生诊断的轮次其 ledger 标记多一个字段；新 PR 评论仅在断路器触发时发布，其正文在 workflow 测试中有断言（刻意不以 fleet shepherd 的终止前缀
🤖 AutoFix stopped开头）。测试环境
仅单元与 workflow 回放测试；未涉及 daemon 或沙箱。
风险与范围
/retry立即清除——而它防止的失败曾跑了好几天。上线天然是渐进的：今日 CLI 写出的标记没有rec，这类轮次会清零 streak，因此不会追溯驻停，断路器只随新部署的评审轮真正诊断时才逐步武装。land-and-defer、land-with-residual-risk）采取行动是另一个功能；review autofix: decline auto-fixing a subsystem with a multi-round successor-defect history #9907（按子系统拒绝）作为更窄的兄弟项保持开放；feat(review): warn when a subsystem's Criticals keep regrowing across rounds #10010 的successor-chain落地后向CONVERGENCE_SIGNAL_CODES加一行即可接入——词表钉会保证加得诚实。关于 issue 的第三条（takeover 管理的 PR 从第 1 轮进 critical 发布下限）：建议采纳，作为单独的小qwen-code-pr-review.yml改动（接管标签在场时传--severity-floor critical）——刻意不放进本 PR，让断路器的行为先独立落地、独立观察。rec为可选字段，现有解析器全部忽略（parseLedger只取已知字段）；旧标记回读不变；断路器把旧轮次当作非证据。关联 Issue
Closes #10107。承接 #9623 / #9526 / #9938；相关 #9907、#10010。