fix(autofix): retry a verification-gate crash instead of burying the agent's fix - #7351
Merged
Conversation
…agent's fix A gate failure had two very different meanings collapsed into one outcome. When the gate DECLARES a verdict (outcome=failed) it evaluated the agent's attempt and rejected it, so advancing the watermark is right — the same feedback would reproduce the same rejection, and MAX_ROUNDS bounds it. But when the gate dies WITHOUT a verdict it never judged the work at all, and advancing buries a fix the agent had already written: the next scan sees "nothing new" and the PR sits until a human deletes the marker by hand. That is exactly how the nested-package ENOENT stranded #7329 and #7336. Both agents had implemented the review feedback — the handoff even quoted the implemented changes — but the gate crashed on its own bug while resolving packages/channels/*, the commit was discarded, and the PRs read as "Could not address the latest feedback automatically". Two halves: - The review-address gate now declares every rejection it can legitimately reach: build, typecheck, lint and the per-package tests each call a `reject_fix` helper that writes outcome=failed before exiting. (The resolver call is deliberately left undeclared — a resolver error IS a gate bug.) - The handoff treats an EMPTY outcome on a non-success job as the gate's own crash and routes it to the existing sentinel/retry path, so the feedback stays live and the next scan retries. The round still increments, so a persistently crashing gate is bounded exactly as before, and the headline names the real cause ("hit a verification-gate error before reaching a verdict") and, on the final attempt, points at the gate logs. Unchanged: a declared rejection still advances and reads as before, a no-output crash keeps its own wording and retry, and a crash before the feedback was read stays terminal. Tests: the real extracted decision block is replayed under bash across declared rejection (advances to NEWEST), gate crash (sentinel + retry + round+1), no output (sentinel, original wording), the round cap (operator fix), and a successful job (never a crash); plus the reject_fix helper is driven for real to prove a rejection writes outcome=failed. Both mutation-verified — dropping the crash arm, or unwiring one known rejection, turns them red.
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with concrete evidence — #7329 and #7336 were stranded when the gate crashed on its own ENOENT bug while resolving Direction: aligned. This makes the managed-PR loop fail honestly — a gate crash retries instead of burying the agent's work. Same line as #7229, #7247, #7330. Size: not applicable (CI workflow + test file, no core module paths). Approach: the scope feels right. Two halves, both necessary — you can't detect a crash without first making rejections explicit ( Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，有具体证据——#7329 和 #7336 在门因自身 ENOENT bug 崩溃时被搁置。两个 agent 都已实现了评审反馈，但 handoff 像门拒绝了它们一样推进了水位线，PR 一直卡到有人手工删除 marker。门的 bug 本身已由 #7330 修复，但这个失败模式是通用的。 方向：对齐。让被托管 PR 循环诚实失败——门崩溃时重试而非埋掉 agent 的成果。与 #7229、#7247、#7330 同一主线。 规模：不适用（CI 工作流 + 测试文件，未触及核心模块路径）。 方案：范围合理。两半都必不可少——不先让拒绝显式声明（ 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
Code ReviewIndependent proposal: given the problem (gate crash and gate rejection collapsed into one signal), I would (1) make each legitimate rejection path write Comparison: the PR does exactly this, and does it minimally. The No critical blockers. No AGENTS.md violations. The comments in the workflow explain the non-obvious invariant (why empty outcome = crash, not rejection) — appropriate for a CI file where the "why" isn't derivable from the code structure alone. TestingThis is a CI workflow change — no TUI to drive. The meaningful real-scenario test is the extracted decision block replayed under bash, which the test suite does. I also drove it directly in tmux: Decision block driven directly under bash — gate crash vs declared rejection: Gate crash → sentinel timestamp (feedback stays live, next scan retries). Declared rejection → watermark advances (feedback consumed). Exactly the split the PR promises. Also verified: YAML parses cleanly ( 中文说明代码审查独立方案： 鉴于问题（门崩溃和门拒绝被压成同一个信号），我会 (1) 让每个合法拒绝路径在退出前显式写 对比： PR 完全这样做了，而且是最小实现。 无关键阻塞项。无 AGENTS.md 违规。工作流中的注释解释了非显而易见的不变量（为什么空 outcome = 崩溃而非拒绝）——对于 CI 文件来说这是合适的。 测试这是 CI 工作流变更——没有 TUI 可驱动。有意义的真实场景测试是在 bash 下回放提取的决策块，测试套件已经这样做了。我也在 tmux 中直接驱动了它： 69/69 测试通过。决策块在 bash 下直接驱动——门崩溃 vs 已声明拒绝： 门崩溃 → 哨兵时间戳（反馈保持存活，下次扫描重试）。已声明拒绝 → 水位线推进（反馈已消费）。正是 PR 承诺的分流。 另外验证：YAML 解析正常（ — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. The problem is real and well-evidenced: #7329 and #7336 sat stranded because a gate crash looked identical to a rejection. The fix is the minimum needed — make rejections explicit, treat an empty verdict as a crash, retry instead of bury. 42 production lines, 100 lines of tests, no drive-by changes. The decision block replays correctly under bash (gate crash → sentinel/retry, declared rejection → watermark advance), all 69 tests pass, YAML and prettier are clean. The approach matches my independent proposal exactly. I don't see a simpler path — you need both halves (explicit verdicts + crash detection) and neither can be cut. The comments explain the non-obvious invariant well, and the tests pin each rejection's declaration so an unwired future check gets caught. 中文说明置信度：5/5 —— 每个阶段都干净；毫不犹豫地合并。 问题是真实的且有充分证据：#7329 和 #7336 因为门崩溃与拒绝看起来完全一样而被搁置。修复是最小必要的——让拒绝显式声明，把空裁决视为崩溃，重试而非埋掉。42 行生产代码，100 行测试，无顺手改动。决策块在 bash 下回放正确（门崩溃→哨兵/重试，已声明拒绝→水位线推进），69 项测试全部通过，YAML 和 prettier 干净。 方案与我的独立提议完全一致。看不到更简的路径——两半（显式裁决 + 崩溃检测）都不可砍。注释很好地解释了非显而易见的不变量，测试钉住了每条拒绝的声明，未来漏接的检查会被抓到。 — 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
previously 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
Collaborator
Author
|
@qwen-code /resolve |
Merge origin/main into the verification-gate crash retry branch. The conflict was in scripts/tests/qwen-autofix-workflow.test.js: - HEAD added gate-crash retry and reject_fix verdict tests - origin/main added re-arm marker and /retry authorization tests Both sets of tests are independent; kept all of them. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution SummaryPR: #7351 — fix(autofix): retry a verification-gate crash instead of burying the agent's fix Conflicted File
What ConflictedBoth sides added new
How It Was ResolvedBoth sides are independent test blocks with no semantic overlap — they test different features of the autofix system. Kept all 6 tests in sequence: the 2 HEAD tests first, followed by the 4 origin/main tests. The only structural fix was restoring the closing Auto-merged File
|
qwen-code-ci-bot
previously approved these changes
Jul 21, 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
Collaborator
Author
Code Review —
|
Collaborator
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: Addressed review feedback on PR #7351Feedback classification
ChangesNit 1 — shared comment opening (line ~2888): Nit 2 — preservation comment (line ~2907): Conflict resolutionNo conflict ( Verification
中文说明已处理 PR #7351 的审查反馈反馈分类
变更内容Nit 1 — 共享注释开头（约第 2888 行）： Nit 2 — 保留性注释（约第 2907 行）： 冲突解决无冲突（ 验证结果
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 21, 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
yiliang114
approved these changes
Jul 21, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM — the logic is correct and the tests replay the real extracted blocks convincingly.
Non-blocking, for a follow-up: the amount of branching logic living inside run: | blocks (GraphQL queries, jq pipelines, while-read loops, multi-arm conditionals) is getting hard to maintain — tests have to regex-extract YAML substrings and replay them under bash, which couples test correctness to YAML indentation. Worth extracting these into .github/scripts/autofix-*.mjs standalone scripts (same pattern as web-shell-visuals-publish.mjs) so they get proper linting, direct unit-test imports, and reuse across workflows. Not blocking this PR — the current approach works and is well-tested; just flagging the direction for when the next chunk of logic lands here.
wenshao
pushed a commit
that referenced
this pull request
Jul 21, 2026
Two conflicts, both in the review-address report step, both resolved by combining the two sides rather than choosing one: - Artifact listing: kept this branch's agent-api-error markers AND main's resolved-comments.txt. - Retry routing: main (#7351) added a gate-crash route and this branch added a model-API route to the SAME if-chain. The condition now covers all three no-verdict causes, and the cause naming runs most-specific first — a model death also leaves OUTCOME empty on a failed job, so testing the API error first keeps a provider blip from being reported as a gate problem. The cause-split retry budget (auth capped below MAX_ROUNDS) applies across the merged chain. Test env for the marker-name contract now sets JOB_STATUS=success: an agent that reached a verdict leaves the job green, so without it every case in that test would have read as a gate crash.
This was referenced Jul 21, 2026
wenshao
pushed a commit
that referenced
this pull request
Jul 21, 2026
This PR was stacked on ci/autofix-gate-crash-retry (#7351), which has since merged, so it now targets main directly. That turned the stale in-branch copy of #7351 into conflicts against its reviewed final form. Resolved by keeping this PR's own additions and taking main's version of everything that belongs to #7351 or #7364: - Gate helpers: kept this branch's GATE_LOG capture and run_check, which are what this PR adds; main's plain reject_fix was their predecessor. - Artifact listing: union of gate-rejection.md and resolved-comments.txt. - Handoff comments: took main's wording — it covers both no-verdict causes and is accurate that the agent's commit dies with the runner. - Tests: both sides added an it() at the same anchor. Kept both and supplied the seam terminator; the trailing rmSync in the shared context belongs to main's test, which has no cleanup of its own.
This was referenced Jul 21, 2026
RehDen
pushed a commit
to RehDen/qwen-code
that referenced
this pull request
Jul 21, 2026
…at it broke (QwenLM#7368) * fix(autofix): retry a verification-gate crash instead of burying the agent's fix A gate failure had two very different meanings collapsed into one outcome. When the gate DECLARES a verdict (outcome=failed) it evaluated the agent's attempt and rejected it, so advancing the watermark is right — the same feedback would reproduce the same rejection, and MAX_ROUNDS bounds it. But when the gate dies WITHOUT a verdict it never judged the work at all, and advancing buries a fix the agent had already written: the next scan sees "nothing new" and the PR sits until a human deletes the marker by hand. That is exactly how the nested-package ENOENT stranded QwenLM#7329 and QwenLM#7336. Both agents had implemented the review feedback — the handoff even quoted the implemented changes — but the gate crashed on its own bug while resolving packages/channels/*, the commit was discarded, and the PRs read as "Could not address the latest feedback automatically". Two halves: - The review-address gate now declares every rejection it can legitimately reach: build, typecheck, lint and the per-package tests each call a `reject_fix` helper that writes outcome=failed before exiting. (The resolver call is deliberately left undeclared — a resolver error IS a gate bug.) - The handoff treats an EMPTY outcome on a non-success job as the gate's own crash and routes it to the existing sentinel/retry path, so the feedback stays live and the next scan retries. The round still increments, so a persistently crashing gate is bounded exactly as before, and the headline names the real cause ("hit a verification-gate error before reaching a verdict") and, on the final attempt, points at the gate logs. Unchanged: a declared rejection still advances and reads as before, a no-output crash keeps its own wording and retry, and a crash before the feedback was read stays terminal. Tests: the real extracted decision block is replayed under bash across declared rejection (advances to NEWEST), gate crash (sentinel + retry + round+1), no output (sentinel, original wording), the round cap (operator fix), and a successful job (never a crash); plus the reject_fix helper is driven for real to prove a rejection writes outcome=failed. Both mutation-verified — dropping the crash arm, or unwiring one known rejection, turns them red. * feat(autofix): feed the gate's rejection back so the retry can fix what it broke QwenLM#7208 was handed to a human over a two-character fix. The agent implemented two review findings, the gate refused the commit because it did not compile (TS4111: `truncated` comes from an index signature, use `['truncated']`), and the loop stopped there — round 5/100, "A human should take over this PR". Nothing in the loop could have recovered on its own, because the reason was never carried anywhere the loop could read it: - the handoff comment showed only the agent's optimistic summary, so neither a human nor the next round could see WHY it was refused; - the feedback filter (correctly) excludes the bot's own comments, so a retry re-read only the original review points; - so `@qwen-code /retry` would have re-run the same agent against the same input and produced the same non-compiling change. The compiler had already said exactly what was wrong. The loop just threw it away. Three pieces carry it instead: - Each deterministic check now runs through `run_check`, which tees its output to a gate log; `reject_fix` writes the label plus the tail of that output to gate-rejection.md. (A four-backtick fence keeps captured ``` output from breaking out when this is posted as a comment.) - The handoff comment carries that block between `<!-- autofix-gate-rejection-start/end -->` markers, so a human sees the real reason next to the summary instead of a report that reads like success. - `Prepare branch and feedback` lifts it back out of the bot's newest comment and puts it at the top of the next round's feedback: "Your previous attempt was REJECTED by the verification gate — fix this first." So a mechanical rejection now closes inside the loop, which is the point of takeover. A rejection the agent cannot fix still burns rounds and ends at the same handoff, bounded exactly as before. Tests: the round trip is exercised end to end — a failing check's compiler output lands in gate-rejection.md with its label, the handoff delimits it, and the prepare step recovers the text (markers stripped) from the newest bot comment while a round that pushed yields nothing to replay. Both halves mutation-verified. QwenLM#7351's verdict test is retargeted to run_check. * fix(autofix): declare the gate verdict before writing its detail file CI caught this and macOS could not: reject_fix wrote gate-rejection.md first and outcome=failed second, so a failure to write the detail took the verdict with it. An empty outcome on a failed job is the signal for "the gate never reached a verdict" — a crash, which is RETRIED — so a clean rejection whose detail write failed would be re-attempted every round instead of being reported once. The verdict is now written first and the detail write is non-fatal. The ordering is pinned by a STATIC assertion, not only the behavioural one: bash 3.2 suspends set -e through a `||`-invoked function and bash 5 does not, so the wrong order runs clean on macOS and aborts on a Linux runner. That is exactly how it shipped green locally and red in CI, and a guard that depends on the reviewer's bash would let it happen again. * fix(autofix): escape the gate-rejection detail for real The gate-rejection publish site used `sed 's/<!--/<!\-\-/g'` — single backslashes, which sed reads as escaped literal `-`, so the replacement is byte-identical to the match and the whole command is a no-op on both GNU and BSD sed. The other four publish sites use `\\-\\-` correctly. That mattered: the detail is `tail -c 3000` of build/typecheck/lint/test output, published verbatim in a bot-authored comment. The scan parses markers by matching the literal `<!-- autofix-eval ts=`, and it only counts markers in bot-authored comments — so any check output containing that string would have been parsed as a real eval marker. The existing test counted the CORRECT spelling and asserted there were four of them. A fifth site with the wrong spelling did not match the counted string, so the count stayed at four and the test stayed green. It now asserts every `s/<!--/…/g` site is byte-identical to the correct form, which fails on exactly this bug. Reported by qwen-code-ci-bot on PR QwenLM#7368. * chore(autofix): correct stale "ALL FOUR" escape-site comment to five (QwenLM#7368) * chore(autofix): document the head/tail byte-limit invariant (QwenLM#7368) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This was referenced Jul 21, 2026
Collaborator
|
Released in v0.20.1. |
Collaborator
|
PR #7351 已合并（2026-07-21），新评论只是 qwen-code-ci-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
Makes the autofix handoff distinguish a verification-gate rejection from the gate's own crash, and retry the crash instead of burying the agent's work.
A gate failure had two very different meanings collapsed into one signal:
outcome=failed) — it evaluated the agent's attempt and rejected it. Advancing the feedback watermark is right: the same feedback would reproduce the same rejection, andMAX_ROUNDSbounds the loop.Why it's needed
This is exactly how the nested-package
ENOENTstranded #7329 and #7336. Both agents had implemented the review feedback — the handoff comment even quoted the implemented changes — but the gate crashed on its own bug while resolvingpackages/channels/*, the commit was discarded with the runner, and both PRs read as "Could not address the latest feedback automatically. A human should take over this PR." They stayed stuck at round 1 until their eval markers were deleted manually.The gate bug itself is fixed (#7330), but the failure mode is general: any gate-side crash — a future gate bug, a resolver error, an infra blip — silently converts a good fix into a stranded PR.
How
Two halves, both in the review-address path:
build,typecheck,lintand the per-package tests now each call areject_fixhelper that writesoutcome=failedbefore exiting. The resolver call is deliberately left undeclared — a resolver error is a gate bug, not a judgement on the agent's fix.Unchanged: a declared rejection still advances and reads as before; a no-output crash keeps its own wording and its retry; a crash before the feedback was read stays terminal.
Reviewer Test Plan
How to verify
npx vitest run scripts/tests/qwen-autofix-workflow.test.js— 69/69. The real extracted decision block is replayed under bash:MARK_TSoutcome=failed)NEWEST(advances)NEWESTPlus the
reject_fixhelper is driven for real: a failing check writesoutcome=failedand exits non-zero.Mutation-verified: dropping the crash arm from the retry condition, or unwiring one known rejection (
npm run build), each turns the corresponding test red.Static (run locally with the exact CI toolchain):
js-yamlparses; all four affected steps passbash -n; actionlint 1.7.12 clean; prettier clean.Evidence (Before & After)
Tested on
Risk & Scope
outcome=failed, it degrades to a retry instead of a report. That is the safe direction (bounded byMAX_ROUNDS, and the handoff still posts every attempt), and a test now pins each existing rejection's declaration so an unwired one is caught.outcome=, so it has no watermark contract to protect.ifchain; whichever lands second resolves a few lines in that condition.Linked Issues
Motivated by #7329 / #7336, whose strandings prompted #7330. Same "make the managed-PR loop fail honestly" line as #7229, #7247, #7330.
中文说明
本 PR 做了什么
让 autofix 的 handoff 区分验证门的拒绝与门自身的崩溃,并对崩溃执行重试,而不是把 agent 的成果埋掉。
门失败原本被压成了同一个信号,而它有两种截然不同的含义:
outcome=failed)—— 它评判了 agent 的尝试并拒绝。推进反馈水位线是对的:同一批反馈会复现同样的拒绝,且MAX_ROUNDS有界。为什么需要
#7329 和 #7336 正是这样被卡住的。两个 agent 都已实现了评审反馈(handoff 评论里甚至引用了已实现的改动),但门在解析
packages/channels/*时因自身 bug 崩溃,提交随 runner 丢弃,两个 PR 都显示为"Could not address the latest feedback automatically"。它们停在 round 1,直到 eval marker 被手工删除。门的那个 bug 已由 #7330 修复,但这个失败模式是通用的:任何门侧崩溃 —— 未来的门 bug、解析器错误、基础设施抖动 —— 都会静默地把一个好修复变成搁置的 PR。
怎么做
两半,都在 review-address 路径:
build、typecheck、lint和逐包测试现在各自调用reject_fix,在退出前写outcome=failed。解析器调用刻意不声明 —— 解析器出错本身就是门的 bug,不是对 agent 修复的裁决。不变:已声明的拒绝仍推进、措辞如旧;无产出崩溃保留自己的措辞与重试;读取反馈前的崩溃仍为终态。
评审验证
npx vitest run scripts/tests/qwen-autofix-workflow.test.js—— 69/69。真实提取的决策块在 bash 下回放:已声明拒绝→推进到 NEWEST;门崩溃→哨兵 + 重试 + round+1;无产出→哨兵(措辞不变);门崩溃触顶→"检查门日志后 re-arm";job 成功→绝不判为崩溃。另外reject_fix被真实驱动:失败检查会写outcome=failed并非零退出。npm run build),对应测试各自变红。bash -n;actionlint 1.7.12 clean;prettier clean。风险与范围
outcome=failed,它会退化为重试而非报告。这是安全方向(有MAX_ROUNDS上界,且每次尝试仍会发 handoff),并且现在有测试钉住每条既有拒绝的声明,漏接会被抓到。outcome=,没有需要保护的水位线契约。if链上新增了模型 API 错误臂);后合入者解决那几行冲突。关联 Issue
由 #7329 / #7336 的搁置驱动(它们也促成了 #7330)。与 #7229、#7247、#7330 同一"让被托管 PR 循环诚实失败"主线。