fix(autofix): cumulative timeout breaker, narrowed retry prompt, truthful handoff wording - #8044
Conversation
…arrow the retry prompt The consecutive-failure cap resets on every pushed round, so it never fires on the observed failure shape: timeouts interleaved with successes. #7929 burned three 50-minute agent timeouts (with pushed rounds in between) and #7846 two — each a full agent budget with nothing to show, invisible to the breaker by construction. Three changes: - Cumulative timeout breaker (TIMEOUT_WINDOW_CAP=3): time-budget exhaustions are counted over the whole counting window, successes in between included. At the cap the PR goes terminal with a skip-until-/retry headline, exactly like the consecutive breaker. The census reuses PRIOR_HEADS, so it is window-scoped and a re-arm clears it. - Prompt narrowing from the second attempt: when the current window already contains a timeout marker, the prepare step appends a budget warning to feedback.md — smallest blocking subset first, commit early, defer the rest explicitly — instead of re-running the identical address-everything prompt into the same wall. - Truthful gate-rejection handoff: 'A human should take over this PR' read as a full release, but the loop keeps managing the PR (new feedback, base conflicts) — #7929 posted it and then kept pushing rounds. The headline now says exactly which half is over: this item will not be retried automatically, management continues.
56e8496 to
057aead
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
|
Thanks for the PR! Template: headings differ from the repo template (Problem/Changes/Tests vs What/Why/Reviewer Test Plan/Risk & Scope/Linked Issues), but the substance is all here — clear motivation with evidence, detailed change description, test summary, and Chinese translation. Not blocking. Problem: observed, not theoretical. #7929's actual sequence (timeout → push → timeout → push → timeout) and #7846 (same shape, twice) are concrete evidence that the consecutive-failure cap is blind to interleaved timeouts. The wording contradiction ("a human should take over" followed by continued pushing) is also a real, observed confusion. Direction: aligned — this is the autofix loop fixing its own failure-handling gaps. No product-direction concerns. Size: not applicable — Approach: three focused changes, each directly addressing a stated problem. The cumulative timeout breaker mirrors the existing consecutive breaker's structure (window-scoped, re-arm clears, terminal headline). The prompt narrowing is scoped to "since last successful round" so a recovered PR stops seeing the warning. The wording fix discriminates gate rejections from other failure paths via Risk: no elevated risk signals — no high-risk paths matched. Moving on to code review. 🔍 中文说明感谢贡献! 模板:标题与仓库模板不同(Problem/Changes/Tests vs What/Why/Reviewer Test Plan/Risk & Scope/Linked Issues),但实质内容齐全——有证据的动机、详细的改动说明、测试摘要、中文翻译。不阻塞。 问题:已观测到的,非理论性的。#7929 的实际序列(超时→推送→超时→推送→超时)和 #7846(同型两次)是具体证据,证明连败熔断对穿插超时完全失明。文案矛盾("人工接管"后继续推送)也是真实观测到的困惑。 方向:对齐——这是 autofix 循环修复自身的失败处理缺陷。无产品方向顾虑。 规模:不适用——两个文件均为 CI 基础设施,非核心模块路径。 方案:三个聚焦改动,各自直接对应一个已陈述的问题。累计超时熔断复用连败熔断的结构(窗口作用域、re-arm 清零、终态标题)。提示词收窄以"上次成功轮以来"为作用域,恢复的 PR 不再看到警告。文案修复通过 风险:无升级风险信号——未匹配高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: given the problem (interleaved timeouts invisible to the consecutive cap, misleading handoff wording), I would (1) add a window-scoped cumulative timeout counter that doesn't reset on success, terminating at a cap like the consecutive breaker, (2) narrow the retry prompt after a timeout so the agent doesn't re-run the identical failing strategy, and (3) rewrite the handoff headline to state exactly what ends and what continues. Comparison with the diff: the PR matches this proposal closely. The implementation is clean and well-structured:
Test coverage is thorough: behavioral replays exercise the actual jq/bash against fixture data (interleaved shape, one-short, window scoping, author filtering, both-breakers precedence, API-error and stale-base exemptions), and string pins are mutation-resistant (threshold values, verbatim needles, CAUSE text lockstep). No correctness bugs, security issues, regressions, or convention violations found. Testing
No failures. The three cancelled checks (ubuntu unit test, web-shell E2E, coverage) are expected for a PR that touches only Sandboxed verification: N/A — this is a CI workflow change with no user-visible TUI surface. The behavioral replay tests in the PR already exercise the actual bash and jq code paths against fixture data. Real-scenario testing: N/A — no user-visible behavior; the change is internal to the autofix CI loop. 中文说明代码审查独立方案: 给定问题(穿插超时对连败熔断不可见、移交文案误导),我会 (1) 添加窗口作用域的累计超时计数器(不因成功重置),到上限即终态;(2) 超时后收窄重试提示词;(3) 重写移交标题,准确说明哪一半结束、哪一半继续。 与 diff 对比: PR 与此方案高度吻合。 实现干净、结构良好:
测试覆盖全面:行为回放对 fixture 数据执行实际 jq/bash(穿插型、差一、窗口作用域、作者过滤、双熔断优先级、API 错误和 stale-base 豁免),字符串 pin 抗突变(阈值、逐字匹配串、CAUSE 文案锁死)。 未发现正确性 bug、安全问题、回归或规范违反。 测试无失败。三个取消的检查(ubuntu 单测、web-shell E2E、覆盖率)对于仅触及 沙箱验证:不适用——CI 工作流变更,无用户可见 TUI 界面。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This PR fixes two real, observed gaps in the autofix loop's failure handling, with evidence from #7929 and #7846. The implementation mirrors the existing consecutive breaker's structure (window-scoped, re-arm clears, terminal headline), so it reads as a natural extension rather than a bolt-on. The prompt narrowing is a pragmatic addition — re-running the identical address-everything prompt into the same wall is exactly the waste the PR describes, and the "since last success" scoping means a recovered PR stops seeing the warning. The wording fix is precise: it discriminates gate rejections from other failure paths via The test suite is the strongest part — behavioral replays that exercise the actual jq and bash against fixture data, with mutation-resistant pins on thresholds, needles, and CAUSE text lockstep. The both-breakers-fire precedence test and the API-error/stale-base exemption pins are exactly the kind of regression guards that prevent a future refactor from silently breaking the semantics. Every change in the diff is necessary for the stated goal. No scope creep, no drive-by edits. 中文说明置信度:5/5 — 每个阶段都干净;毫不犹豫地合并。 此 PR 修复了 autofix 循环失败处理中两个真实的、已观测到的缺陷,有 #7929 和 #7846 的证据。实现复用连败熔断的结构(窗口作用域、re-arm 清零、终态标题),读起来是自然扩展而非外挂。提示词收窄是务实的补充——用一模一样的提示词撞同一面墙正是 PR 描述的浪费,"上次成功以来"的作用域意味着恢复的 PR 不再看到警告。文案修复精确:通过 测试套件是最强的部分——行为回放对 fixture 数据执行实际 jq 和 bash,pin 抗突变(阈值、匹配串、CAUSE 文案锁死)。双熔断优先级测试和 API 错误/stale-base 豁免 pin 正是防止未来重构静默破坏语义的回归守卫。 diff 中每个改动都是目标所必需的。无范围蔓延,无顺手编辑。 — Qwen Code · qwen3.8-max-preview Reviewed at |
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
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
ReviewVerified against What I confirmed works:
The problem is real and well-evidenced, and the cumulative-window design is the right shape for it. Findings below are all non-blocking. [Suggestion] The narrowing prompt tells the agent to defer into the wrong artifact
This section is injected precisely when the agent is under budget pressure, i.e. exactly when it will take the cheapest path. Pointing it at "your summary" sanctions the outcome the skill contract exists to prevent. Worth restating the contract instead: Related, same block, line 3032: [Suggestion] The precedence assertion has no teeth
expect(bothCapped.headline).toContain('consecutive');The timeout headline contains if [[ "${MARK_ROUND}" != "${MAX_ROUNDS}" ]]; then → if true; thenstill passes 103/103, while the replay's headline flips to expect(bothCapped.headline).toContain('consecutive rounds that failed to push');
// or
expect(bothCapped.headline).not.toContain('time-budget exhaustions');(The two off-by-one mutants I tried were caught: [Suggestion] The prompt narrowing has no behavioral testEvery other census in this file gets a bash replay; this one is string pins only ( if [[ "${PRIOR_TIMEOUTS}" -ge 1 ]] → if [[ "${PRIOR_TIMEOUTS}" -ge 99 ]]— survives 103/103. The feature could go inert and CI stays green. The four-case probe I ran above is about 30 lines against a temp [Suggestion] The breaker can attribute a gate rejection to timeouts
The round that actually stopped the PR did not time out, and the new (accurate) gate-rejection wording is discarded. In steady state the 3rd timeout trips the breaker itself so this is hard to reach — but it is directly reachable at rollout, since #7929 and #7846 already carry ≥3 in-window timeout markers today, so the first failing round of any cause after merge lands here. Cheapest fix is wording that matches what the census measures: "this counting window contains N time-budget exhaustions" rather than "stopped after N time-budget exhaustions". Given this PR is about handoff comments saying what actually happened, it seems worth keeping consistent. [Suggestion] The narrowing never decays inside a window
[Suggestion]
|
|
@qwen-code /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 冲突,直到移除标签或达到轮次上限。移除 |
All six review findings addressed:
- The narrowing prompt now restates the SKILL deferral contract instead
of pointing at 'your summary': deferred findings stay out of
resolved-comments.txt and get a comment-replies.json entry so every
open thread carries its reason — the summary-only shortcut is exactly
what a budget-pressured agent would otherwise take. 'skip refactors
entirely' became 'decline with a one-line reason' for the same
never-drop-silently rule.
- The narrowing census counts timeouts SINCE THE LAST SUCCESSFUL ROUND,
not cumulatively: a push falsifies 'not converging', and one old
timeout must not degrade every later round of a 100-round window. The
breaker stays cumulative (a push does not make the next timeout
cheaper in budget terms).
- The breaker headline states what the census measured ('this counting
window now contains N time-budget exhaustions') instead of 'stopped
after N' — the tripping round can itself be a gate rejection, which
is the exact rollout state of #7929/#7846.
- Both census needles anchor on the verbatim emitted headline ('AutoFix
ran out of time before finishing') — first lines can embed provider
error text, so the loose phrase could count a model error as a
timeout.
- The precedence assertion got teeth: 'consecutive' alone matched both
branches; it now asserts the consecutive breaker's own phrase and the
absence of the timeout one (the if-true guard mutation now fails).
- The narrowing census got a behavioral replay over fixture ic.json:
push-resets, trailing counts, legacy no-win markers under 'none',
old-window exclusion, author filtering — plus a pin on the -ge 1
trigger so a threshold mutation cannot leave the feature inert.
|
All six addressed in f9f9fd1:
— Claude Code (claude-fable-5) |
Review —
|
| Check | Result |
|---|---|
vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js @ f9f9fd11 |
103/103 pass |
prettier --check on both changed files |
clean |
node scripts/lint.js --yamllint / --shellcheck |
clean / exit 0, no new hits on changed files |
Old wording Could not address the latest feedback — any remaining consumer? |
none in the repo; #8046 already greps Could not (address the latest feedback|produce a passing fix), so the reword doesn't break its census |
| Grep needle ↔ emit line lockstep | CAUSE="ran out of time before finishing (${AGENT_TIMEOUT})" → headline 🤖 AutoFix ran out of time before finishing (…); both the breaker grep and the prepare-step jq anchor on the verbatim prefix, and both are pinned |
AGENT_TIMEOUT + API_ERROR_DETAIL double-count? |
unreachable — run-agent.mjs:336 writes agent-api-error only under if (result.apiError && !result.timedOut), and the repair path rm -fs both first |
| Status comment pollution of the census | none — it carries <!-- autofix-status -->, not <!-- autofix-eval |
Breaker replay (13 scenarios, block extracted from the YAML via js-yaml and sourced under set -eo pipefail with a stub gh):
- feat(web-shell): add contextual task panels #7929 shape
timeout, push, timeout, push+ current timeout → terminal, timeout-breaker headline ✅ - both caps armed (4 consecutive timeouts + current timeout) → consecutive headline wins ✅
- transient (non-auth) API error / stale-base retry /
PREPARE_OUTCOME=cancelled→ exempt, no terminal ✅ - auth API error → not exempt ✅; round already terminal → keeps its own headline ✅
- old-window timeouts → not counted; legacy no-
win=markers under keynone→ counted ✅
Narrowing census replay (8 extra cases beyond the PR's own): gate-rejection / infra-failure / stale-base between timeouts do not reset (→ 2), noop does (→ 1), push-then-nothing → 0, empty window → 0, the breaker's own headline isn't miscounted as a timeout → 0. All correct.
1. The breaker headline claims more than the census measured (nice-to-have)
The census is cumulative over the window with no ratio or recency bound, so a window with plenty of progress can trip it. Replayed:
[timeout, timeout, ×40 push] + current round times out → TERMINAL
"…Re-running the same per-round budget against this PR is not converging…"
40 pushed rounds is the definition of converging. The counting rule itself is defensible and I'd keep it — a push genuinely doesn't make the next timeout cheaper, and the mitigation is real: Report dry-run / failure only runs on failure() || cancelled() || dry_run, so the breaker can never stop a round that pushed. But the sentence asserts a property the census cannot establish, which is the same class of problem change #3 of this PR fixes in the handoff wording. The follow-up commit already made the count-vs-cause part honest ("this round itself may have failed differently"); the non-convergence clause is the one leftover.
Suggestion — say what was measured and drop the inference:
…this counting window now contains N time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is N full agent runs that pushed nothing. A human should split or reduce the PR (or raise the agent time budget), then comment
/retryto re-arm.
(If you'd rather keep the claim, gating the trip on something like "timeouts ≥ cap and timeouts ≥ pushes in this window" would earn it — but that's a bigger change and #7929's shape would still trip it, so the wording fix is the cheaper honest option.)
2. The case the new headline was written for isn't tested
f9f9fd11's message calls a gate rejection landing on an already-capped window "the exact rollout state of #7929/#7846", and the headline carries a parenthetical for it — but every breaker test drives a round where the current round either is the timeout or is one short of the cap. I confirmed the case works:
[timeout, push, timeout, push, timeout] + current round = gate rejection → terminal, "time-budget exhaustions"
The mutation this leaves open: wrapping the whole timeout block in if [[ -n "${AGENT_TIMEOUT:-}" ]] (a plausible "only count when this round timed out" cleanup) keeps all 103 tests green while silently deleting the documented case. One line, passes today:
// A NON-timeout failure landing on an already-capped window still trips it —
// the #7929/#7846 rollout state the headline's parenthetical describes.
expect(run([TIMEOUT_HEAD, PUSH, TIMEOUT_HEAD, PUSH, TIMEOUT_HEAD])).toMatchObject({
terminal: true,
});3. The timeout breaker's exemptions are inherited, not pinned
The consecutive breaker has explicit tests for the transient-API-error / stale-base / pre-agent-infra exemptions. The timeout block gets them purely by sitting inside the same outer if — but it reads self-contained (own MARK_ROUND != MAX_ROUNDS guard, own census, own headline), so a future refactor that hoists it one level would mass-terminate every in-flight PR during a provider outage with no test failing. That's exactly the scenario the outer guard's comment block warns about. Verified they hold today; one line pins it:
// Inherited from the outer guard: a provider outage must not mass-terminate.
expect(
run(Array(5).fill(TIMEOUT_HEAD), {
apiErrorDetail: '429 rate limited',
apiErrorKind: 'transient',
}),
).toMatchObject({ terminal: false });Nit
The prepare-step census scopes by LIVE_REARM_KEY while the report-step one scopes by WINDOW. They're equal on the normal path (the window-superseded guard at the top of prepare forces it), and they can only diverge on the CONFLICT=true path, which bypasses that guard. Consistent with the neighbouring LIVE_MAX_ROUND census, so this is existing convention rather than something introduced here — noting it only so the "window-scoped like every other census" comment isn't read as stronger than it is.
Things I checked and liked
- Reusing
PRIOR_HEADSrather than issuing a second census keeps the two breakers structurally identical and makes the re-arm semantics fall out for free. +1for the current round keyed onAGENT_TIMEOUTis the right signal: line 3724 routes every non-emptyAGENT_TIMEOUTinto the no-verdict branch, so it can't fire on a round that actually pushed.- Splitting the two consumers (breaker cumulative, prompt since-last-success) is the right call and the inline comments explain why each is what it is — that reasoning would have been invisible otherwise.
- The narrowing prompt restating the real deferral contract (
resolved-comments.txt/comment-replies.json) instead of "defer it in your summary" closes the exact shortcut a budget-pressured agent would take, and it composes with the existingdeferred-feedback.mdre-surfacing.
…ted guarantees Second-round review follow-ups: - The breaker headline no longer infers 'is not converging' — a window can carry 40 pushes and still trip the (deliberately cumulative) cap, so the sentence now states only what the census measured: N full agent runs that pushed nothing. - Two guardrail tests pin behavior that held only by construction: a non-timeout failure landing on an already-capped window still trips the breaker (the documented rollout state — an 'only count when this round timed out' cleanup would silently delete it), and the transient-API-error exemption inherited from the outer guard (a refactor hoisting the block out of it must not mass-terminate every in-flight PR during a provider outage).
|
All three taken in 7f32419:
On the nit: agreed it's existing convention ( — Claude Code (claude-fable-5) |
Review — cumulative timeout breaker + narrowed retry promptSolid change, and the tests are unusually non-vacuous for a workflow PR: the census is replayed against fixture What I verified
🟠 The new handoff headline asserts a cause it can't know
The workflow already knows this — the comment at There's also a live path where this mislabels a timeout: in In a PR whose thesis is that the wording must match the behaviour, adding a false specific is worth avoiding. GATE_CLAUSE=''
[[ -s "${WORKDIR}/gate-rejection.md" ]] && GATE_CLAUSE=' — the verification gate rejected the attempt'
HEADLINE="🤖 Could not produce a passing fix for this feedback (round ${MARK_ROUND}/${MAX_ROUNDS})${GATE_CLAUSE}. This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own."Dropping the clause entirely works too — the "which half is over" sentence is the part that fixes #7929's contradiction, and it stands on its own. 🟡 Calibration: the cap never decaysI replayed the breaker over every
🟡 Test coverage gapThe new replay pins the API-error exemption "so a refactor that hoists the timeout block out of the guard cannot mass-terminate every in-flight PR" — but not the 🟢 Nits
Nothing else: the census needles match the emitted headlines verbatim, the new headline contains none of the consecutive breaker's reset substrings (checked against all four), no other consumer in the repo keys on the old wording, and the 🤖 Reviewed with Claude Code · model |
…imeout breaker's stale-base exemption Third-round review follow-ups: - The handoff headline no longer claims 'the verification gate rejected the attempt' for every outcome=failed verdict: that branch is reached for five distinct failures (failure.md abort, dirty tree, unchanged branch, missing summary, and a real gate rejection), only one of which is a gate decision. The clause is now gated on gate-rejection.md, which reject_fix is the sole writer of — so the other four paths keep the neutral framing instead of repeating the very wording-doesn't-match-behaviour bug this PR fixes. A replay test pins both halves: the clause appears when gate-rejection.md exists and is absent when it does not. - The timeout breaker's stale-base exemption is now pinned by a replay (five in-window timeouts plus a stale-base retry current round stays retryable), matching the existing transient-API-error pin: a refactor hoisting the timeout block out of the outer guard would otherwise delete this exemption silently. - The prepare-step narrowing census jq now fails open (2> /dev/null || true) like its report-step sibling, so a malformed ic.json drops the budget warning instead of aborting prepare under errexit and turning the round into a terminal 'could not start evaluation' handoff. - The decay comment now matches the code: the warning resets on a push/no-op round but fires on every failing round until then (gate rejections included), which is correct since nothing has converged.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressedThanks for the replay-verified review. Four findings are addressed in code, one is declined with reasoning, and the older inline suggestion is confirmed already resolved. No conflict ( 🟠 Handoff headline asserted a cause it can't know — fixedThe GATE_CLAUSE=''
[[ -s "${WORKDIR}/gate-rejection.md" ]] && GATE_CLAUSE=' — the verification gate rejected the attempt'
HEADLINE="🤖 Could not produce a passing fix for this feedback (round ${MARK_ROUND}/${MAX_ROUNDS})${GATE_CLAUSE}. …"This also fixes the live mislabel you flagged in 🟡 Timeout breaker's stale-base exemption was not pinned — fixedAdded the missing replay alongside the existing transient-API-error pin: five in-window timeouts plus a stale-base-retry current round stays retryable ( 🟢 Nit 1: bare
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
.github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
.github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: 🤖 AutoFix reviewed the latest feedback and made no changes this round — there was nothing actionable to address. What the feedback contained
Why the cited CI result is not a defect in this PR The three checks listed as failing — This PR touches only Local verification of the change
No code change was warranted; the open item is the cancelled CI runs, which a re-run (not a code edit) resolves. 中文说明🤖 AutoFix 审查了最新反馈,本轮未做任何更改——没有可处理的内容。 反馈包含什么
为什么所引用的 CI 结果不是本 PR 的缺陷 被列为失败的三个检查—— 本 PR 仅改动 对更改的本地验证
没有理由做代码更改;未解决的事项是被取消的 CI 运行,它需要重新运行(而非代码编辑)来解决。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Review — round 2 (
|
| # | Round-1 finding | Mutation | R1 | R2 |
|---|---|---|---|---|
| 1 | Deferral pointed at "your summary" | — (wording) | — | fixed: now restates the SKILL.md contract verbatim (out of resolved-comments.txt, into comment-replies.json); skip … entirely → decline … with a one-line reason |
| 2 | Precedence assertion had no teeth | MARK_ROUND != MAX_ROUNDS → true |
survived 103/103 | CAUGHT |
| 3 | Prompt narrowing had no behavioural test | -ge 1 → -ge 99 |
survived 103/103 | CAUGHT |
| 4 | Breaker could attribute a gate rejection to timeouts | GATE_CLAUSE unconditional |
— | CAUGHT; headline now states what the census measured |
| 5 | Narrowing never decayed in a window | drop the rindex reset |
— | CAUGHT |
| 6 | grep -c 'ran out of time' too loose |
needle → 'ran out of time' |
— | CAUGHT |
Control mutations also caught: TIMEOUT_N + 1 → + 0, and rewording the breaker headline.
Closed-loop check (no hand-written fixtures)
The test's TIMEOUT_HEAD is a hand-written literal, so I closed the loop against the real code instead: ran the workflow's own CAUSE/HEADLINE block (yml 3752–3799) with AGENT_TIMEOUT set, captured what it actually emits, and fed that text into both shipped censuses.
emitted: 🤖 AutoFix ran out of time before finishing (timeout (3000000ms)) (attempt 2/100) — …
breaker census grep -c 'AutoFix ran out of time before finishing' → 1 ✅ arms
narrowing census jq …contains("AutoFix ran out of time before finishing") → 1 ✅ fires
Replaying the shipped PRIOR_TIMEOUTS jq across window shapes confirms #5 is really fixed: T→1, T,PUSH→0, T,NOOP→0, T,PUSH,T→1, T,T→2.
[Suggestion] The emit template ↔ census needle coupling is still unpinned
Both new code comments warn that a mismatch makes the census "silently count zero" — but nothing tests the concatenation. The needle is AutoFix ran out of time before finishing; the text is built as HEADLINE="🤖 AutoFix ${CAUSE} …" with CAUSE="ran out of time before finishing (…)". The test pins the CAUSE= line and the needle separately, so a change to the template breaks the join while both pins stay green.
Mutation-verified — adding one character:
HEADLINE="🤖 AutoFix ${CAUSE} (attempt … → HEADLINE="🤖 AutoFix: ${CAUSE} (attempt …still passes 103/103, while the real emit becomes 🤖 AutoFix: ran out of time… and both censuses drop to 0 — the breaker and the prompt narrowing both go inert with CI fully green. That is the whole feature failing silently, which is exactly what this PR exists to prevent.
One line restores it (present twice at HEAD, absent under the mutation, and not currently in the test file):
expect(reviewAddressReportStep).toContain('HEADLINE="🤖 AutoFix ${CAUSE} (attempt');Deriving TIMEOUT_HEAD from the emit block instead of hardcoding it would be stronger still.
[Suggestion] A stale gate-rejection.md can survive into the repair path
GATE_CLAUSE keys on [[ -s "${WORKDIR}/gate-rejection.md" ]], and the comment says presence means the gate ran. It does — but not necessarily on the attempt that ended the round. The Repair deterministic rejection step's rm -f list clears eight files and omits gate-rejection.md, and the gate's failure.md early-exit sets outcome=failed without rewriting it. Replayed with the real cleanup list and the real early-exit:
attempt 1 gate rejects → writes gate-rejection.md
repair rm -f → address-summary.md no-action.md failure.md handoff.md
gate-output.log agent-api-error agent-api-error-kind agent-timeout
(gate-rejection.md NOT removed)
repair agent aborts → writes failure.md
repair gate → outcome=failed, never rewrites gate-rejection.md
report step → [[ -s gate-rejection.md ]] = TRUE (stale)
→ headline claims "the verification gate rejected the attempt"
…but the round ended in an agent abort. The new failedNoGate test only covers the case where the file never existed. Cheapest fix is one line in the repair step's rm -f list — safe, because gate-rejection.md is cat-ed into feedback.md before the cleanup runs:
"${WORKDIR}/gate-output.log" \
+ "${WORKDIR}/gate-rejection.md" \That also makes the code comment's claim exactly true.
[Nit] The narrowing census resets on 2 of the breaker's 4 "not the PR's fault" headlines
The consecutive breaker resets on Addressed the latest review feedback, no changes needed, AutoFix could not start —, and updated a stale base. The narrowing census uses only the first two, so:
T, INFRA → 1 T, STALE → 1 (narrowing still fires)
For AutoFix could not start — that's arguably right (the agent never ran, so it says nothing about convergence). For updated a stale base the agent did run and finish within budget — the fix was rejected only because the base was stale — which falsifies "not converging at this budget" about as well as a push does. Impact is small and in the safe direction (one extra round of a "prioritise" hint), so purely optional.
Verified as fine
set -esafety on the new jq:2> /dev/null || truecan yield an emptyPRIOR_TIMEOUTS, and[[ "" -ge 1 ]]evaluates to false (rc=1) without erroring — inside anifcondition it cannot kill the step. Confirmed underset -euo pipefail.rindexandsort_by(.created_at)are fine on jq 1.7 (runner) —rindexhas existed since 1.5.WORKDIRis job-level env (review-address, line 2421), and the pre-existinggate-rejection.mdread at line 4012 is in the same step as the newGATE_CLAUSEat 3846 — so the file is genuinely in scope; the discriminator itself is sound.- Both changed steps pass
bash -nafter extraction from the YAML. - The narrowing text now matches SKILL.md 198–206 and 263–276 on both points raised in round 1.
中文说明
结论:可以合并
针对新 head 423c66d 本地复验:把真实的 workflow bash/jq 抽出来回放,并重跑第 1 轮的原始变异,确认后续提交是真正加上了约束,而不只是改了措辞。
六条第 1 轮建议全部落实,其中最关键的两条(M1/M2)已从"变异存活"翻转为"变异被抓"。以下新发现均为非阻塞。
基线
vitest → 103/103 通过(423c66d 干净树);prettier --check 通过;YAML 正常解析(7 个 job,3 < 5 < 100);actionlint 零新增——与 merge-base 相同的 15 条既有 shellcheck info,仅行号偏移。
六条建议的变异复验
| # | 第 1 轮问题 | 变异 | R1 | R2 |
|---|---|---|---|---|
| 1 | 延后指向 "your summary" | —(文案) | — | 已修:改为逐字复述 SKILL.md 契约;skip … entirely → decline … 并给出一行理由 |
| 2 | 优先级断言没有牙齿 | 守卫 → true |
存活 103/103 | 被抓 |
| 3 | 提示词收窄无行为测试 | -ge 1 → -ge 99 |
存活 103/103 | 被抓 |
| 4 | 熔断可能把 gate 拒绝归因为超时 | GATE_CLAUSE 无条件 |
— | 被抓;文案已改为陈述 census 实际测量的内容 |
| 5 | 收窄在窗口内不衰减 | 去掉 rindex 复位 |
— | 被抓 |
| 6 | grep 串过松 | 还原为松串 | — | 被抓 |
对照变异(+1→+0、熔断文案改写)同样被抓。
闭环验证(不使用手写 fixture)
测试里的 TIMEOUT_HEAD 是手写字面量,因此我改用真实代码闭环:运行 workflow 自己的 CAUSE/HEADLINE 块(yml 3752–3799,设置 AGENT_TIMEOUT),捕获实际输出,再喂给两个真实 census —— 均为 1,熔断与收窄都是活的。回放 PRIOR_TIMEOUTS jq 也确认第 5 条已修:T→1、T,PUSH→0、T,NOOP→0、T,T→2。
【建议】文案模板与 census 匹配串的耦合仍未锁定
两处新注释都警告"不匹配就会静默计零",但没有测试锁定拼接结果。匹配串是 AutoFix ran out of time before finishing,而文本由 HEADLINE="🤖 AutoFix ${CAUSE} …" 拼出;测试分别 pin 了 CAUSE= 行与匹配串,却没 pin 模板本身。变异验证:仅加一个冒号 🤖 AutoFix: ${CAUSE} 仍然 103/103 全绿,而真实输出变为 🤖 AutoFix: ran out of time…,两个 census 全部归零 —— 熔断与收窄整体失效而 CI 全绿,正是本 PR 要防的失败模式。一行即可修复:
expect(reviewAddressReportStep).toContain('HEADLINE="🤖 AutoFix ${CAUSE} (attempt');【建议】陈旧的 gate-rejection.md 会残留进 repair 路径
GATE_CLAUSE 以该文件是否存在为判据。但 Repair deterministic rejection 的 rm -f 清理了 8 个文件,唯独没清 gate-rejection.md,而 gate 的 failure.md 提前返回只写 outcome=failed、不会重写它。用真实清理列表与真实提前返回回放确认:第 1 次 gate 拒绝 → repair agent 中止写 failure.md → 报告步骤读到陈旧文件 → 文案声称"验证门拒绝了此次尝试",而该轮实际结束于 agent 中止。新增的 failedNoGate 测试只覆盖了文件从未存在的情况。最省的修法是在 repair 的 rm -f 列表里加一行(安全:该文件在清理之前已被 cat 进 feedback.md)。
【吹毛求疵】收窄 census 只复位于熔断 4 个文案中的 2 个
熔断复位于 4 个"不是 PR 的错"文案,收窄只用了前两个,因此 T, INFRA → 1、T, STALE → 1 仍会触发。could not start 不复位是合理的(agent 根本没跑);但 updated a stale base 时 agent 确实在预算内跑完了,只是因 base 陈旧被拒——这和一次推送一样能证伪"预算不够收敛"。影响很小且偏保守,纯属可选。
确认无问题
- 新 jq 的
set -e安全性:2> /dev/null || true可能产生空值,而[[ "" -ge 1 ]]为假(rc=1)且不报错,位于if条件中不会中断步骤(已在set -euo pipefail下确认)。 rindex/sort_by(.created_at)在 runner 的 jq 1.7 下正常(rindex自 1.5 即有)。WORKDIR是 job 级 env(2421 行),且既有的 4012 行读取与新增的 3846 行GATE_CLAUSE在同一步骤内,文件确实在作用域内,判据本身成立。- 两个改动步骤从 YAML 抽出后
bash -n均通过。 - 收窄文案已与 SKILL.md 198–206、263–276 两处对齐。
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at HEAD 423c66d5. The logic is approve-quality and I found no defects; withholding the approval on one factual point about CI state, detailed at the end.
Verified independently against the HEAD workflow text, not the description:
- The prompt-narrowing census replayed independently. I extracted the real jq from
Prepare branch and feedbackand ran it against fixtureic.jsonshapes outside the test suite:timeout,push,timeout → 1,push,timeout,timeout → 2, legacy no-win=markers under keynone→ 2, old-window markers under a fresh key → 0, a human quoting the timeout headline verbatim → 0, push-only → 0. All six match. Therindex(true) // -1then.[($lastok+1):]construction is right for the$lastok == -1case (slices the whole array), so a window with no successful round yet counts every timeout in it. - The two counters cannot double-count the current round.
ic.jsonis fetched at L2692 in the prepare step — before this round's comment is ever posted — and the report step reads that same file, soPRIOR_HEADSexcludes the in-flight round and the+1for a setAGENT_TIMEOUTis exactly the current round, counted once. LIVE_REARM_KEYis in scope where the new census uses it: both L2759 and L3029 are inside the singlePrepare branch and feedbackstep (L2555–L3060), same shell — no cross-step variable assumption.- Both degenerate-input paths behave. Executed locally:
grep -cagainst an emptyPRIOR_HEADSyields0with|| trueabsorbing the exit-1, and[[ "" -ge 1 ]]underset -euo pipefailevaluates false without aborting — so a failed jq or an empty census silently declines to fire rather than crashing the step or tripping the breaker. - Precedence and exemption inheritance are structural, not incidental. The timeout block sits inside the consecutive breaker's outer guard, so it inherits every exemption verbatim (transient API errors, non-
success|failureprepare outcomes,STALE_BASE_RETRY) — the property that stops a provider outage from mass-terminating every in-flight PR — and its ownMARK_ROUND != MAX_ROUNDSguard means a round already terminal keeps its headline. The test pins both inherited exemptions specifically against a refactor that hoists the block out. - The census needle and the emit line are pinned to each other (
CAUSE="ran out of time before finishing (${AGENT_TIMEOUT})"↔ the grep string), which is the failure mode that would otherwise make the breaker silently count zero forever. - The
GATE_CLAUSEdiscriminator is correct:reject_fixis the only writer ofgate-rejection.md, so-son it distinguishes a real gate rejection from thefailure.md/ dirty-tree / unchanged-branch / missing-summary paths that reach the sameoutcome=failedbranch — and the new test asserts the negative case too. Naming the gate unconditionally would have reproduced the exact wording-vs-behaviour mismatch this PR fixes. - The new headline is safe for the existing counter: it contains none of the reset substrings (
Addressed the latest review feedback,no changes needed,AutoFix could not start —,updated a stale base), so failure counting is unchanged.
The CI point. The only Qwen Code CI run for this HEAD (30457892653) ended cancelled, not failed — ##[error]The operation was canceled mid-npm run build, with the coverage and web-shell jobs recorded as fail at 0s purely because their parent run was torn down. No test assertion failed, and this diff touches only qwen-autofix.yml and its own test file, so it cannot influence a ci.yml build; the cancellation is infrastructure, not this PR. But the consequence stands on its own: the required Test (ubuntu-latest, Node 22.x) check has no passing result on 423c66d5, and it is the job that runs the 103 tests including everything added here. The bot's approval was auto-downgraded for the same reason.
Please re-run the cancelled CI run — I will approve as soon as it lands green, with no further review needed on the code.
中文总结
在 HEAD 423c66d5 上完成审查:逻辑达到 approve 标准、未发现缺陷,仅因一项 CI 事实暂缓批准。
- 提示词收窄的 census 已独立复现:从真实步骤中抽出 jq,在测试套件之外对 fixture 运行,六种情形(超时/推送穿插=1、连续两次=2、legacy 无 win= 归 none=2、旧窗口=0、人类引用原文=0、仅推送=0)全部吻合;
rindex(true) // -1+ 切片在"窗口内尚无成功轮"时正确统计全部超时。 - 两个计数器不会重复计入当前轮:
ic.json在 prepare 步骤 L2692 抓取(早于本轮评论发布),report 步骤复用同一文件,因此PRIOR_HEADS不含在飞轮次,AGENT_TIMEOUT的 +1 恰好只计一次。 LIVE_REARM_KEY作用域正确:L2759 与 L3029 同属Prepare branch and feedback(L2555–3060)单一步骤、同一 shell。- 退化输入路径实测安全:空
PRIOR_HEADS下grep -c返回 0 且|| true吸收退出码;[[ "" -ge 1 ]]在set -euo pipefail下为假且不中断——jq 失败或 census 为空时静默不触发,既不崩溃也不误熔断。 - 优先级与豁免继承是结构性的:超时块位于连败熔断的外层守卫内,逐字继承全部豁免(transient API 错误、非 success/failure 的 prepare、stale-base retry),并有自身
MARK_ROUND != MAX_ROUNDS守卫;测试专门钉住两条继承豁免以防重构上提。 - census 匹配串与 emit 行互相钉死,堵住"熔断永久计零"的静默失效。
GATE_CLAUSE判别正确:gate-rejection.md仅由reject_fix写入,可精确区分真实门禁拒绝与其他outcome=failed路径,测试覆盖反例;无条件加该措辞会重演本 PR 要修的矛盾。- 新文案不含任何连败熔断的清零子串,失败计数不受影响。
CI 事实:该 HEAD 唯一的 CI run 结论是 cancelled(构建中途 The operation was canceled,coverage 与 web-shell 的 0 秒 fail 只是父 run 被拆除的副产物),无任何测试断言失败;且本 diff 只改 qwen-autofix.yml 与其测试,不可能影响 ci.yml 构建——取消属基础设施原因,非本 PR 所致。但结论独立成立:required 的 Test (ubuntu-latest, Node 22.x) 在 423c66d5 上没有通过记录,而它正是运行本 PR 新增测试的 job;bot 的批准也因此被自动降级。
请重跑被取消的 CI;转绿后我会立即批准,代码侧无需再审。
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Round summaryAddressed 2 of 3 new findings from @wenshao's round-2 review; declined 1 nit. Feedback points
@doudouOUC reviewNo code changes needed — the CI cancellation on ConflictNo conflict ( Verification
中文说明本轮总结处理了 @wenshao 第 2 轮审查中 3 条新发现中的 2 条;拒绝了 1 条吹毛求疵。 反馈要点
@doudouOUC 审查无需代码变更 — 冲突无冲突( 验证
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/模型 |
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #8044One inline suggestion was open this round; it is addressed in code. Feedback
Conflict notesNone — Change
Commit: Verification
中文说明Autofix 审查轮次 — PR #8044本轮只有一条行内建议处于开放状态;已在代码中处理。 反馈
冲突说明无 — 改动
提交: 验证
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/模型 |
doudouOUC
left a comment
There was a problem hiding this comment.
.github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
.github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.7-max via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: No action takenThe feedback on this round contains no actionable items:
No code changes were made this round. 中文说明未采取任何操作本轮反馈中不包含任何需要处理的项目:
本轮未做任何代码更改。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未发现问题。LGTM!✅
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
Local verification — round 3 (
|
| # | Round-2 finding | Round-2 | Round 3 |
|---|---|---|---|
| 1 | HEADLINE= template joins AutoFix + ${CAUSE}; tests pinned the two halves separately, so mutating the join left both censuses counting 0 with 103/103 green |
SURVIVED | CAUGHT |
| 2 | Repair deterministic rejection's rm -f omitted gate-rejection.md, so a repair-path abort inherited attempt 1's file |
present | CAUGHT |
Finding 2 also replayed against the real cleanup block, with every prior-attempt artifact pre-seeded:
BASE (main) gate-rejection.md SURVIVES cleanup -> next attempt inherits attempt 1 [STALE]
PR 8044 HEAD gate-rejection.md cleared -> discriminator reflects THIS attempt [OK]
2. The motivating case, base vs PR
#7929's actual sequence — timeout → push → timeout → push, with this round the third timeout — replayed through each tree's own breaker block against a real ic.json. The only variable between arms is the workflow file.
CONSEC_FAIL |
TIMEOUT_N |
Outcome | |
|---|---|---|---|
| base | 1 | (no breaker) | round 4 of ~50 min gets spent |
| PR 8044 | 1 | 3 | terminal, skipped until /retry |
CONSEC_FAIL=1 in both arms is the PR's premise proven rather than asserted: a push resets it, so the consecutive breaker is blind to this shape by construction. A boundary control one timeout short (TIMEOUT_N=2) correctly does not arm — the cap is exact, not a blanket "any timeout stops it".
3. Closed-loop emit → census oracle, with a negative control
The emit block produces the headline; both censuses consume that exact string.
| Arm | TIMEOUT_N |
PRIOR_TIMEOUTS |
|
|---|---|---|---|
| PR head (A/A) | 3 / cap 3 | 2 | breaker arms, narrowing appends |
drift mutant (AutoFix → AutoFix: ) |
0 | 0 | feature fully inert |
The negative control is what makes the pass meaningful: a one-character change to the prefix silently zeroes both censuses. That mutation is now caught by the suite (M1), so this seam is pinned.
4. Mutation matrix — 9/9 caught
Each mutation applied to a clone, proven to have landed, then the real suite run against it. Baseline 103 passed (103); every mutant 2 failed | 101 passed.
TIMEOUT_WINDOW_CAP 3→99 · -ge→-gt · drop precedence guard · narrowing gate → false · drop current-round self-count · loosen census needle · drop the no-op reset branch · plus M1/M2 above.
5. "Failure counting is unchanged" — checked against all nine headlines
Rather than checking only the new headline, I ran every HEADLINE= the workflow can emit through the reset alternation lifted from line 3939. The three intentional reset headlines classify as RESET; both headlines this PR adds or rewords classify as failure. Claim holds.
6. Merge landscape — trial merges built and tested, not just diffed
| Combination | Merge | Suite on the merged tree |
|---|---|---|
origin/main alone (control) |
— | 102 passed (102) |
| PR 8044 alone | — | 103 passed (103) |
PR 8044 × origin/main |
clean | 103 passed (103) |
| PR 8044 × PR #8046 (open) | clean | 104 passed (104) |
At round 2 these two PRs auto-merged cleanly and landed red: #8046 cross-pinned a census needle to the exact gate-rejection wording this PR rewrites. That is now resolved — #8046 matches Could not (address the latest feedback|produce a passing fix). Re-checked here because a clean auto-merge is the dangerous case, not the safe one.
actionlint: 15 findings on base, 15 on the PR, diff empty — no new lint findings. All PR checks green.
Non-blocking notes
- The description is behind the code. It covers the gate-rejection wording change, but neither round-3 addition appears: the
gate-rejection.mdrepair-cleanup fix (263daa6e) and the headline-template pin. Since the body becomes the squashed commit message, worth a line each. - Running the suite under heavy parallel CPU load made
auto-reruns a check that died on infrastructure…fail once; it passes in isolation on base, PR, and both merged trees. Pre-existing load sensitivity, unrelated to this PR — noting it so it isn't misread as a merge defect if it surfaces in CI. bash: /gate-rejection.md: Read-only file systemon stderr during the suite is pre-existing — it reproduces identically at merge-base97aaa38. A replay writes with an unsetWORKDIR; harmless, not this PR's.
Scope of this verification
This exercises the workflow's bash and jq by lifting and executing it locally; it is not a live Actions run. Behaviour that depends on real GitHub API paging, runner env, or genuine 50-minute agent timeouts is covered by replay only.
中文版本
本地验证 — 第 3 轮(ad38bce)
在 ad38bce 的独立干净 worktree 中重新验证,基线为 merge-base 97aaa38。结论:可以合并。 第 2 轮的两个问题都已修复,并且我通过重跑上一轮存活的变异来做行为验证——diff 看起来对,并不能证明修复是真正起作用的。
方法:所有被测 bash 都是从 qwen-autofix.yml 原样抽取后执行的。harness 中不重新手写任何 headline、匹配串或清理列表,因此模板与匹配串各自"看起来都对"却已经脱节的情况无法蒙混过关。
1. 第 2 轮两个问题已修复,且修复是真正起作用的
| # | 第 2 轮问题 | 第 2 轮 | 第 3 轮 |
|---|---|---|---|
| 1 | HEADLINE= 模板把 AutoFix 与 ${CAUSE} 拼接;测试分别 pin 了两半,因此只变异拼接处时两个 census 都数到 0,而 103/103 仍全绿 |
存活 | 被杀死 |
| 2 | Repair deterministic rejection 的 rm -f 漏了 gate-rejection.md,导致 repair 路径的 abort 继承第 1 次尝试的文件 |
存在 | 被杀死 |
问题 2 另外用真实清理块做了回放(预置所有上次尝试的残留文件):
BASE (main) gate-rejection.md 在清理后仍存在 -> 下次尝试继承第 1 次的结果 [STALE]
PR 8044 HEAD gate-rejection.md 已清除 -> 判别依据反映本次尝试 [OK]
2. 动机场景:base 与 PR 的对照
#7929 的真实序列——超时 → 推送 → 超时 → 推送,本轮为第三次超时——分别用各自代码树自己的熔断块对真实 ic.json 回放。两个 arm 之间唯一变量就是 workflow 文件。
CONSEC_FAIL |
TIMEOUT_N |
结果 | |
|---|---|---|---|
| base | 1 | (无熔断) | 会再花掉第 4 个约 50 分钟的轮次 |
| PR 8044 | 1 | 3 | 终态,/retry 前跳过 |
两个 arm 的 CONSEC_FAIL 都等于 1,这把 PR 的前提从"断言"变成了"实测":推送会清零,因此连败熔断按构造就看不见这种形态。差一次的边界对照(TIMEOUT_N=2)正确地不触发——阈值是精确的,而不是"只要有超时就停"。
3. 闭环 emit → census 预言机,带反向对照
由 emit 块产出 headline,两个 census 消费同一字符串。
| Arm | TIMEOUT_N |
PRIOR_TIMEOUTS |
|
|---|---|---|---|
| PR head(A/A) | 3 / 上限 3 | 2 | 熔断触发、提示词收窄生效 |
漂移变异(AutoFix → AutoFix: ) |
0 | 0 | 整个特性完全失效 |
反向对照才让"通过"有意义:前缀改一个字符就会静默地把两个 census 归零。该变异现在已被测试杀死(M1),这个接缝已被 pin 住。
4. 变异矩阵 — 9/9 全部被杀死
每个变异应用到克隆副本、先证明变异确实落地,再跑真实测试套件。基线 103 passed (103);每个变异体均为 2 failed | 101 passed。
TIMEOUT_WINDOW_CAP 3→99、-ge→-gt、去掉优先级守卫、收窄门置 false、去掉当前轮自计数、放松 census 匹配串、去掉 no-op 清零分支,外加上面的 M1/M2。
5. "失败计数不受影响" — 对全部 9 条 headline 检查
我没有只检查新 headline,而是把工作流可能发出的每一条 HEADLINE= 都过一遍第 3939 行抽取出的清零判断。三条本就该清零的 headline 判为 RESET;本 PR 新增/改写的两条都判为 failure。该结论成立。
6. 合并全景 — 试合并是真的构建并跑了测试,而不只是看 diff
| 组合 | 合并 | 合并后代码树的测试 |
|---|---|---|
仅 origin/main(对照) |
— | 102 passed (102) |
| 仅 PR 8044 | — | 103 passed (103) |
PR 8044 × origin/main |
干净 | 103 passed (103) |
| PR 8044 × PR #8046(未合并) | 干净 | 104 passed (104) |
第 2 轮时这两个 PR 干净自动合并却跑红:#8046 把一个 census 匹配串交叉 pin 到了本 PR 要改写的那句 gate-rejection 文案上。该问题现已解决——#8046 已放宽为 Could not (address the latest feedback|produce a passing fix)。这里重新检查,是因为干净的自动合并才是危险情况,而非安全情况。
actionlint:base 15 条、PR 15 条,diff 为空——没有新增 lint 问题。PR 所有检查均为绿。
非阻塞事项
- 描述落后于代码。 描述涵盖了 gate-rejection 文案改动,但第 3 轮新增的两处均未提及:
gate-rejection.md的 repair 清理修复(263daa6e)与 headline 模板 pin。由于 PR body 会成为 squash 后的提交信息,建议各补一行。 - 在 CPU 高并发负载下跑套件时,
auto-reruns a check that died on infrastructure…失败过一次;单独运行时在 base、PR 及两个合并树上均通过。属于既有的负载敏感问题,与本 PR 无关——特此说明,以免在 CI 中出现时被误判为合并缺陷。 - 套件运行时 stderr 出现的
bash: /gate-rejection.md: Read-only file system是既有现象——在 merge-base97aaa38上可完全复现。某个回放在WORKDIR未设置时写文件所致;无害,与本 PR 无关。
本次验证的边界
本次验证通过在本地抽取并执行工作流的 bash 与 jq 来进行,不是一次真实的 Actions 运行。依赖真实 GitHub API 分页、runner 环境或真正 50 分钟 agent 超时的行为,仅通过回放覆盖。
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round — no code changeThis round's feedback is @wenshao's round-3 local verification report, with a Classification of the three non-blocking notes
Outcome: no commit this round. The PR stands as verified merge-ready; the only 中文说明Autofix 审查轮次 — 无代码改动本轮反馈是 @wenshao 的第 3 轮本地验证报告,结论为可以合并。其中没有 三条非阻塞事项的分类
结论:本轮不提交。PR 维持经验证的可合并状态;唯一的后续事项是事项 1 中的 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at ad38bce. The cumulative breaker closes the exact hole described: TIMEOUT_N counts window-scoped timeout headlines from PRIOR_HEADS plus the current round's AGENT_TIMEOUT, so timeout→push→timeout sequences now arm at TIMEOUT_WINDOW_CAP=3 regardless of interleaved pushes; persistence rides the existing autofix-eval comments with strict win= filtering, so a re-arm opens a fresh window and clears the count. Coexistence with CONSECUTIVE_FAILURE_CAP is ordered — the consecutive breaker fires first and the cumulative one is guarded by MARK_ROUND != MAX_ROUNDS, and a test pins that the headline isn't overwritten. The narrowed prompt only triggers on PRIOR_TIMEOUTS >= 1 counted since the last successful/no-change round and appends to feedback.md rather than replacing it, so no empty-prompt path. Boundary tests cover cap-at-N (not N+1), legacy no-win markers, old-window exclusion, and human-quoted headlines; the census jq is replayed against fixtures, not just string-pinned, so removing the increment or the window filter fails CI. Ran the changed test file locally at this HEAD: 103/103 passed. CI green. No findings.
…und (QwenLM#8046) * feat(autofix): post a takeover milestone digest every tenth pushed round The takeover round cap (100) bounds runaway but carries no signal about when a human should step in: QwenLM#7469 ground to round 12 over seven days of takeover with fifteen pushes, and the only place that trajectory was visible was the Actions logs. Every 10th pushed round under takeover, the report step now posts a window-scoped census on the PR itself — pushed fixes, no-change reviews, timeouts, rejected attempts, base updates — plus the three options (keep going / split or reduce / release takeover). The digest is a separate comment with its own autofix-milestone marker and no autofix-eval marker, so every census (round, consecutive-failure, watermark) ignores it and the feedback filters keep it out of the agent's prompt. Posting is best-effort: a digest failure never fails a good push. The rejected-attempt count matches both the current and the reworded gate-rejection headline so the census cannot silently zero; base updates carry no win= field and are windowed by timestamp instead (the window key is the engage ack's created_at). * feat(autofix): milestone digest review follow-ups - Residual bucket: every outcome the four buckets missed (crash, model error, gate error, infra) now lands in 'other round(s)' — a window that burned 80% of its budget on crashes was rendering as four zeros, QUIETER than a healthy window, the inverse of the digest's premise. - Crossing trigger: fire on the first pushed round once 10+ rounds have accumulated since the last digest in this window, instead of an exact %10 hit — failure rounds advance the counter too, so push@9/crash@10/ push@11 skipped the digest forever on exactly the failure-heavy PRs it exists for. - The success log is chained to the post; a failed comment no longer logs 'posted' after its own warning. - WINDOW=none says 'since the PR opened (no counting window yet)' instead of claiming a current window while counting all time. - A census that parses zero window markers at round 10+ skips the digest instead of posting a fabricated all-zero one. - autofix-milestone joins BOT_COMMENT_FILTER (marker inventory). - The timeout needle anchors on the verbatim emitted headline. - The pin-only test became a behavioral replay: the digest block runs under bash with a stubbed gh over fixture ic.json histories — bucket counts, residual loudness, crossing suppression/resumption, old-window isolation, none-window phrasing, non-pushed/non-takeover gating, and the empty-census skip. The OUTCOME == fixed conjunct is pinned. * test(autofix): cross-pin digest census needles to headline emission sites (QwenLM#8046) * test(autofix): widen rejected-headline cross-pin to match QwenLM#8044 reword (QwenLM#8046) * test(autofix): behaviorally cover the digest comment-failure branch (QwenLM#8046) --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
|
Released in v0.21.2. |





Problem
Two related honesty/efficiency gaps in the review-address failure handling, both measured on live PRs:
1. The consecutive-failure cap cannot see interleaved timeouts.
CONSECUTIVE_FAILURE_CAP=5resets on every pushed round — but a push does not make the next timeout any cheaper. #7929's actual sequence was timeout → push → timeout → push → timeout: three 50-minute agent budgets burned with nothing to show, and the breaker never armed by construction. #7846 hit the same shape twice. Each retry re-ran the identical address-everything prompt into the same wall.2. The gate-rejection handoff wording contradicts the loop's behavior. "Could not address the latest feedback automatically … A human should take over this PR" reads as a full release — but the loop keeps managing the PR (new feedback, base conflicts). #7929 posted exactly this and then kept pushing rounds; anyone reading the thread sees a bot that says it quit and didn't.
Changes
Cumulative timeout breaker (
TIMEOUT_WINDOW_CAP: '3'): time-budget exhaustions are counted over the whole counting window — pushed rounds in between included. At the cap the round goes terminal with a skip-until-/retryheadline, exactly like the consecutive breaker (which keeps precedence when both would fire). The census reusesPRIOR_HEADS, so it is window-scoped like every other census and a re-arm (/takeoverre-engage or/retry) clears it.Prompt narrowing from the second attempt: when the current window already contains a timeout marker,
prepareappends a "Budget warning" section tofeedback.md— address the smallest blocking subset first, commit as soon as it's done, explicitly defer the rest — instead of re-running the identical prompt. Window-scoped viaLIVE_REARM_KEY.Truthful handoff wording: the gate-rejection headline now states exactly which half is over — "This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own." (This matches the existing semantics: this path advances the watermark, so the item is genuinely not retried; management continues.)
The new headline contains none of the consecutive-breaker's success-reset substrings, so failure counting is unchanged.
Tests
qwen-autofix-workflow.test.js: 103/103 pass; workflow YAML parses.中文说明
问题
连败熔断看不见穿插的超时。
CONSECUTIVE_FAILURE_CAP=5被任何一次推送清零——但推送不会让下一次超时变便宜。feat(web-shell): add contextual task panels #7929 的实际序列是 超时→推送→超时→推送→超时:三次 50 分钟 agent 预算烧光、零产出,熔断按构造永不触发;feat(skills): add auto-skill curator #7846 同型两次。且每次重试都在用一模一样的提示词撞同一面墙。放弃文案与行为矛盾。"A human should take over this PR" 读起来像完全释放,但循环仍在管理该 PR(新反馈、base 冲突)。feat(web-shell): add contextual task panels #7929 发完这句后继续推了多轮——读者看到的是一个说了退出却没退出的 bot。
改动
累计超时熔断(
TIMEOUT_WINDOW_CAP: '3'):在整个计数窗口内累计超时(含穿插的成功轮)。到上限即终态 + skip-until-/retry提示,与连败熔断完全同构(两者同时触发时连败优先)。复用PRIOR_HEADS,窗口作用域一致,re-arm 自动清零。第二次尝试起收窄提示词:当前窗口已有超时标记时,prepare 在 feedback.md 末尾追加"预算警告"——先做最小阻塞子集、尽早提交、其余显式延后。按
LIVE_REARM_KEY窗口作用域。如实的移交文案:改为准确说明哪一半结束了——"此项需要人工;循环保持接管,继续处理新反馈与 base 冲突,但不会自动重试此项"。(与现有语义一致:该路径推进水位线,此项确实不重试;管理继续。)
新文案不含连败熔断的成功清零子串,失败计数不受影响。
测试
连败回放增加超时熔断用例(#7929 穿插型触发 / 差一不触发 / 旧窗口不计 / 双触发时连败优先 / census 匹配串与 CAUSE 文案锁死);新增提示词收窄 pin 测试;三处文案 pin 更新;103/103 通过,YAML 解析正常。