Skip to content

fix(ci): route the autofix convergence-brake handoff through failure.md - #9371

Merged
wenshao merged 1 commit into
QwenLM:mainfrom
wenshao:fix/autofix-brake-handoff-contract
Aug 18, 2026
Merged

fix(ci): route the autofix convergence-brake handoff through failure.md#9371
wenshao merged 1 commit into
QwenLM:mainfrom
wenshao:fix/autofix-brake-handoff-contract

Conversation

@wenshao

@wenshao wenshao commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

When the autofix loop's growth brake decides a PR's diff is not converging, the round feedback tells the agent to stop patching and hand the decision to a maintainer. This change makes that instruction name the exact output file the agent must write its handoff into, and forbids it from writing the wrapper file that the runner-side harness owns. The workflow contract test now pins the instruction so a future edit cannot silently flip it back.

Why it's needed

On PR #9184 round 14 (run 32076785809) the convergence brake fired and the agent did exactly what the feedback asked: no code changes, verified the remaining Critical finding by direct code reading, and wrote a handoff naming the decision, the options (split the PR, redesign, or accept with the tail deferred), and a recommendation. But it wrote that handoff into handoff.md — a file the runner-side harness generates, not one the round's verdict gate accepts (only address-summary.md or no-action.md as success verdicts, plus failure.md as the sole stop verdict). The round was therefore reported as "finished without required output file(s)": a correct defer-to-human burned a round as a failure, and the PR comment carried the generic gate error instead of the agent's handoff analysis.

Writing failure.md is already the established stop path — the runner wraps it into the workflow's handoff comment and exits zero, and the verification gate treats it as a deliberate abort. This change simply routes the brake's handoff through that existing path end to end.

Reviewer Test Plan

How to verify

  • Read the updated non-convergence bullet in the autofix skill: it now tells the agent to write the handoff into failure.md, explains that this is the one stop file the round's output contract accepts, and forbids the agent from writing handoff.md itself (with the reason).
  • Run the workflow contract test filtered on the non-convergence case (test name ends with "escalates to a maintainer-decision handoff when the diff keeps growing past budget"): it pins both directives. Reverting either directive back to the old wording fails the test.
  • Expected behavior on the next brake-triggered round: the runner wraps the agent's failure.md, exits zero, the verification gate reports a deliberate abort, and the handoff comment's "What I found before stopping" section carries the agent's decision analysis instead of the missing-output gate error.

Evidence (Before & After)

N/A — agent-behavior policy text and its contract test; no user-visible/TUI change. Before: see the round-14 comment on PR #9184 ("Could not produce a passing fix ... finished without required output file(s)"). After: the same situation routes through the existing failure.md handoff path.

Tested on

OS Status
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

Environment (optional)

Contract test only: npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js. The 2 unrelated failures in that file (bite check, keeps the green path intact) also fail on the clean base tree in this local environment.

Risk & Scope

  • Main risk or tradeoff: the change is agent-facing policy wording; an ambiguous phrasing could still let a model pick the wrong file — mitigated by naming the file explicitly, stating the reason, and pinning the directive in the contract test.
  • Not validated / out of scope: a live end-to-end brake round (needs a genuinely over-budget PR window to occur); the failure.md path itself is pre-existing and already exercised. Posting inline comment-replies.json replies on the handoff path remains unposted (unchanged by this PR).
  • Breaking changes / migration notes: none.

Linked Issues

Observed on PR #9184 round 14 (run 32076785809); no issue filed.

中文说明

本 PR 做了什么

当 autofix 循环的增长刹车判定某个 PR 的 diff 不收敛时,该轮的 feedback 会指示 agent 停止打补丁、把决策交还给维护者。本次改动让这条指示明确写出 agent 必须把交接内容写入的具体输出文件,并禁止 agent 去写那个由 runner 侧脚本自己生成的包装文件。workflow 契约测试现在把这条指令钉住,防止未来的修改把它悄悄改回去。

为什么需要

在 PR #9184 第 14 轮(run 32076785809)中,收敛刹车触发,agent 完全按 feedback 要求执行:不改代码、通过直接读码核实了剩余的 Critical finding、并写了一份交接,指明决策点、列出选项(拆分 PR、重新设计、或接受现状并把尾巴延后)以及推荐意见。但它把交接写进了 handoff.md —— 这个文件是 runner 侧脚本生成的,并不在该轮 verdict gate 接受的范围内(gate 只接受 address-summary.mdno-action.md 作为成功结论,外加 failure.md 作为唯一的停止结论)。于是该轮被上报为 "finished without required output file(s)":一次正确的 defer-to-human 以失败姿态烧掉了一轮,PR 评论里也只有干巴巴的 gate 报错,而不是 agent 的交接分析。

failure.md 本来就是既有的停止路径 —— runner 会把它包装进 workflow 的交接评论并以零码退出,验证 gate 也将其视为有意中止。本次改动只是让刹车的交接走这条既有路径,端到端打通。

审阅者测试计划

如何验证

  • 阅读 autofix skill 中更新后的 non-convergence 条目:它现在要求 agent 把交接写入 failure.md,说明这是该轮输出契约接受的唯一停止文件,并禁止 agent 自己写 handoff.md(附原因)。
  • 运行按 non-convergence 用例过滤的 workflow 契约测试(测试名以 "escalates to a maintainer-decision handoff when the diff keeps growing past budget" 结尾):它钉住了上述两条指令。把任何一条改回旧措辞都会让测试失败。
  • 下一次刹车触发时的预期行为:runner 包装 agent 的 failure.md、零码退出,验证 gate 上报有意中止,交接评论的 "What I found before stopping" 部分呈现 agent 的决策分析,而不是 missing-output gate 报错。

证据(前后对比)

N/A —— agent 行为策略文本及其契约测试,无用户可见/TUI 变化。Before:见 PR #9184 第 14 轮评论("Could not produce a passing fix ... finished without required output file(s)")。After:同样情形将走既有的 failure.md 交接路径。

测试环境

OS 状态
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

运行环境(可选)

仅契约测试:npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js。该文件中 2 个不相关的失败(bite checkkeeps the green path intact)在本机干净基线树上同样失败,与本 PR 无关。

风险与范围

  • 主要风险或权衡:改动是面向 agent 的策略措辞;含糊的表述仍可能让模型选错文件 —— 通过明确点名文件、说明原因、并在契约测试中钉住指令来缓解。
  • 未验证 / 超出范围:真实的端到端刹车轮次(需要真正出现超预算的 PR 窗口);failure.md 路径本身是既有的且已被使用过。交接路径下不发布内联 comment-replies.json 回复的行为维持不变(本 PR 未改动)。
  • 破坏性变更 / 迁移说明:无。

关联 Issue

观察于 PR #9184 第 14 轮(run 32076785809);未建 issue。

The growth brake tells the address-review agent to stop with a handoff
but never names the output file, so round 14 on QwenLM#9184 wrote handoff.md
— a file owned by run-agent.mjs that the verdict gate does not accept.
The round was reported as missing its required outputs even though the
defer-to-human was correct (run 32076785809). Name failure.md (the stop
file run-agent.mjs wraps into the handoff comment) as the handoff
target, forbid agent-written handoff.md, and pin the directive in the
workflow contract test.
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on 601cca1 and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— 601cca1 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed, not theoretical — and I verified it independently. The round-14 comment on PR #9184 (run 32076785809) shows exactly the failure described: the growth brake fired, the agent deferred to a human, yet the round still landed as Autofix agent finished without required output file(s): address-summary.md, no-action.md, with the handoff analysis buried under the generic gate error.

Direction: aligned. This is the autofix loop's own output contract. Routing the brake's handoff through the pre-existing failure.md stop path (the runner wraps it into the handoff comment and exits zero) is the right call, rather than teaching the verdict gate to also accept handoff.md, which is a runner-owned file.

Size: 2 files, +17/−6 (11 skill-text lines + 6 contract-test lines). No core package paths touched — Stage 0 not applicable.

Approach: minimal and complete — names the file, states the reason, forbids the runner-owned one, and pins both directives in the existing workflow contract test. Nothing to cut, no drive-by changes.

Risk: no elevated risk signals (no high-risk paths touched; skill text and scripts tests only).

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到,不是理论问题——且我已独立核实。PR #9184 第 14 轮(run 32076785809)的评论正是所描述的失败:增长刹车触发、agent 交还给人类,但该轮仍以 Autofix agent finished without required output file(s): address-summary.md, no-action.md 收场,交接分析被笼统的 gate 报错淹没。

方向:对齐。这是 autofix 循环自身的输出契约。让刹车的交接走既有的 failure.md 停止路径(runner 将其包装进交接评论并以零码退出)是正确选择,而不是让 verdict gate 额外接受 runner 自有的 handoff.md

规模:2 个文件,+17/−6(11 行 skill 文本 + 6 行契约测试)。未触及 core 包路径——Stage 0 不适用。

方案:最小且完整——点名文件、说明原因、禁写 runner 自有文件,并在既有 workflow 契约测试中钉住两条指令。没有可砍的部分,也没有顺手改动。

风险:无升级风险信号(未触及高风险路径;仅 skill 文本与 scripts 测试)。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at 601cca1b21ecd1ca0aedc55f8714900ec523c87f · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Code review

Before reading the diff, my independent proposal for this problem was exactly what this PR does: name failure.md explicitly in the non-convergence bullet (it is the only stop file the address-review output contract accepts), forbid the agent from writing handoff.md (runner-owned), and pin both directives in the existing workflow contract test. The PR matches that proposal — I found no simpler path.

Every mechanism claim checks out against the base tree:

  • The address-review mode accepts address-summary.md or no-action.md (anyOutput + exclusiveOutput); anything else fails with finished without required output file(s) and exits 1 — precisely what round 14 of PR fix(review): gate the recovered incremental anchor on the model that certified it #9184 hit.
  • An agent-written failure.md on a clean exit is wrapped into handoff.md, logged, and exits 0 — the deliberate-abort path this PR routes the brake through.
  • handoff.md is written only by the runner's writeHandoff, so the "do not write it yourself" directive matches reality.
  • <workdir>/failure.md matches the notation already used ~20 times across the skill, and both new test assertions match the new skill text verbatim, inside the test the PR names (escalates to a maintainer-decision handoff when the diff keeps growing past budget (non-convergence)).

No correctness, security, or convention findings. The diff carries nothing beyond the stated goal.

CI evidence

Unattended CI run — the PR's code is neither built nor executed here; the evidence below quotes the PR's own CI checks fetched via the API, once, at review time. The job that actually runs the pinned contract test (Test (ubuntu-latest, Node 22.x)npm run test:citest:scripts) was still in flight at fetch time, so the pinning itself is not yet verified by CI. No red checks at this snapshot. Attribution note: the PR body mentions two locally failing tests in the same file on the author's machine — that is the author's claim, not CI evidence; the table below is what counts, and the finalize pass refreshes it once the run lands. The in-progress review-pr and triage check-runs are bot orchestration, not PR CI, so they are not in the table.

Final CI results for 601cca1 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Secret scan (TruffleHog) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Sandboxed lanes: there is no claim here /verify or /tmux could settle, so I'm saying that plainly rather than naming one as a formality. No runtime code changes, so an A/B build comparison has nothing to measure; there is no TUI surface. The one part no review can verify in advance is whether the agent actually follows the new instruction at the next real brake round — the substantiation for that is the pinning test above plus the runner-side stop path I verified statically, and the next brake-triggered round is the natural confirmation point.

中文说明

代码审查

读 diff 之前,我对这个问题的独立方案与 PR 做法完全一致:在非收敛条目中点名 failure.md(address-review 输出契约接受的唯一停止文件)、禁止 agent 写 runner 自有的 handoff.md、并在既有 workflow 契约测试中钉住两条指令。PR 与该方案一致——没有找到更简路径。

所有机制性论断都已对照基线树核实:address-review 模式只接受 address-summary.mdno-action.mdanyOutput + exclusiveOutput),否则报 finished without required output file(s) 并非零退出——正是 PR #9184 第 14 轮遇到的;agent 在正常退出时写的 failure.md 会被包装进 handoff.md、打日志并以零码退出——即本 PR 改让刹车走的有意中止路径;handoff.md 只由 runner 的 writeHandoff 生成,"禁止自己写"与事实相符;<workdir>/failure.md 与 skill 中约 20 处既有记法一致,两条新测试断言与新 skill 文本逐字匹配,且位于 PR 所指名的测试(escalates to a maintainer-decision handoff when the diff keeps growing past budget (non-convergence))内。

无正确性、安全或规范问题。diff 没有超出既定目标的内容。

CI 证据

无人值守 CI 运行——此处不构建、不执行 PR 代码;下方证据为审查时通过 API 一次性抓取的 PR 自身 CI 检查。真正运行钉住契约测试的 job(Test (ubuntu-latest, Node 22.x)npm run test:citest:scripts)抓取时仍在运行,因此钉住本身尚未被 CI 验证。此快照无红色检查。归属说明:PR 正文提到作者本机同文件有两个无关失败——那是作者的说法,不是 CI 证据;以上表为准,finalize 会在 CI 落定后刷新表格。进行中的 review-prtriage 检查是机器人编排任务,不属于 PR CI,故未列入表中。

沙盒验证通道:此处没有 /verify/tmux 能裁决的论断,故直接说明而非走形式点名。没有运行时代码改动,A/B 构建对比无可测之物;也没有 TUI 面。唯一任何审查都无法预先验证的,是下一次真实刹车轮次中 agent 是否真的遵循新指令——其依据是上述钉住测试与我静态核实的 runner 侧停止路径,下一次刹车触发的轮次是自然确认点。

Qwen Code · qwen3.8-max

Reviewed at 601cca1b21ecd1ca0aedc55f8714900ec523c87f · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — everything statically verifiable checks out (observed problem, verified mechanism, minimal diff, pinning test); the one reservation is inherently unprovable in advance: whether the agent actually follows the new instruction at the next real brake round.

Stepping back: this is exactly the kind of fix the gate should wave through. The failure was observed in production (PR #9184 round 14, verified against the actual comment), the fix routes through a stop path that already exists and already works — no new machinery, no gate changes — and the contract test pins the wording so it cannot silently regress. Twenty-three lines across two files with zero drive-by content; my independent proposal matched the PR's approach point for point, and I have nothing simpler to suggest. The only thing no review can give you in advance is the model-compliance proof, and explicit file naming plus the pinning test is the right mitigation for it.

Verdict: approve. The unit suite that runs the pinning test is still in flight, so approval is deferred until CI lands green on 601cca1b21ecd1ca0aedc55f8714900ec523c87f.

中文说明

置信度:4/5 —— 一切可静态核实之处均通过(问题已观测、机制已核实、diff 最小、契约测试钉住);唯一保留意见本质上无法预先证明:下一次真实刹车轮次中 agent 是否真的遵循新指令。

退一步看:这正是 gate 应当放行的修复。失败在生产中观测到(PR #9184 第 14 轮,已对照真实评论核实),修复走的是既已存在、既已可用的停止路径——没有新机制、没有 gate 改动——且契约测试钉住措辞,防止悄悄回退。两个文件共 23 行、零顺手改动;我的独立方案与 PR 逐点一致,没有更简路径可提。唯一任何审查都无法预先给出的是模型遵循性证明,而点名文件加点钉住测试正是对它的正确缓解。

结论:批准。运行钉住测试的单测套件仍在进行,因此批准推迟到 CI 在该提交上落绿。

Qwen Code · qwen3.8-max

Reviewed at 601cca1b21ecd1ca0aedc55f8714900ec523c87f · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship — CI landed green after the review. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅

中文说明

未发现问题。LGTM!✅

— qwen3.8-max via Qwen Code /review (v0.21.13)

@wenshao
wenshao added this pull request to the merge queue Aug 18, 2026
Merged via the queue into QwenLM:main with commit 72ae65d Aug 18, 2026
81 checks passed
qwen-code-dev-bot added a commit to wenshao/qwen-code that referenced this pull request Aug 18, 2026
The SKILL.md conflict pits this PR's handoff output contract against
QwenLM#9371's failure.md stopgap. Resolve it in this PR's favor: the merged
run-agent.mjs accepts an agent-written handoff.md as a first-class
verdict (exit 0, no spec output) and the merged verify gate classifies
it as outcome=handoff (or dirty_handoff over a dirty tree), which is
exactly the acceptance QwenLM#9371's failure.md routing worked around.
Re-pin QwenLM#9371's contract-test assertions to the handoff.md directive so
they lock the new contract instead of the superseded stopgap.
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.14.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants