-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(autofix): stop counting idle timeouts toward the timeout cap #9673
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
536d209
af470fe
3bd4a50
ee13b2e
36e0a45
4a47aac
d016403
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -124,6 +124,7 @@ task-oriented guides — what a maintainer types and what happens next — see: | |
| - [70. review-address · Report dry-run / failure — -c drops any partial multi-byte sequence a byte-level head -c may have split, so the…](#af-070) | ||
| - [71. review-address · Report dry-run / failure — Bilingual companion. Repo convention is English first, Chinese in a collapsed <details>.…](#af-071) | ||
| - [72. review-address · Report dry-run / failure — Flip the status comment out of "working" so a finished round never leaves a live-looking…](#af-072) | ||
| - [73. review-address · Report dry-run / failure — Idle (silent-sandbox) timeouts are EXCLUDED from the cumulative timeout cap.…](#af-073) | ||
|
|
||
| --- | ||
|
|
||
|
|
@@ -2090,3 +2091,68 @@ publishing" and report a successful round as a failed one. An empty | |
| 'stale' (prepare itself crashed) still finalises — that IS this job's | ||
| round, and it is exactly the case that must not stay "working". | ||
| ``` | ||
|
|
||
| <a id="af-073"></a> | ||
|
|
||
| ### 73. review-address · Report dry-run / failure — Idle (silent-sandbox) timeouts are EXCLUDED from the cumulative timeout cap. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] R1-1: The new Witness: at the reviewed commit the file holds 71 anchors but 70 contents-table entries — the difference is exactly Add the entry for section 73 to the Contents block (after the entry-72 line, ~line 126), matching the house style — the test's regex only requires the 中文说明新增的 证据:在被审提交上,文件有 71 个锚点但只有 70 条目录条目,差集恰为 修复:在 Contents 区块(第 72 条之后,约第 126 行)为第 73 节添加条目,沿用既有风格——测试的正则只要求 — qwen3.8-max via Qwen Code /review (v0.21.15) |
||
|
|
||
| In `review-address` · `Report dry-run / failure`. | ||
|
|
||
| ```text | ||
| TIMEOUT_WINDOW_CAP exists to stop a PR that is too big to finish a | ||
| round inside the agent's time budget; its remedy says so ("split or | ||
| reduce the PR, or raise the agent time budget AND its step backstop"). | ||
| An idle timeout is a different failure entirely: run-agent.mjs's idle | ||
| watchdog kills the round after QWEN_IDLE_TIMEOUT_MS (20m) because the | ||
| sandbox produced no output at all — the four observed hangs (#8663 x2, | ||
| #8761 r3, #8763 r4) each printed their last byte at docker container | ||
| entry and then sat silent. Nothing about the PR caused it, and the | ||
| breaker's own headline already told the reader that "no budget increase | ||
| can cure" it. Counting a failure whose prescribed remedy is | ||
| inapplicable is what parked healthy PRs. | ||
|
|
||
| Measured on 2026-08-21, over the preceding 14 days: 119 timeouts, of | ||
| which 58 (49%) were idle. 51 windows tripped this cap, every one of | ||
| them at exactly N=3. Of the 12 open PRs then carrying | ||
| autofix/needs-human, 9 had been stopped here — #8332 at 24 rounds, | ||
| #8368 at 28, #8276 at 16, all still producing pushed rounds when they | ||
| were parked. With idle rounds counted, the fleet timeout rate was | ||
| 8.5% per round, so a window accumulated three of them in ~35 rounds by | ||
| arithmetic alone, independent of whether the PR was stuck. Excluding | ||
| idle drops the rate to 4.3%, which needs ~69 rounds — beyond the | ||
| deepest window ever observed (22/100). | ||
|
|
||
| The escape hatch that makes the exclusion safe: an idle round pushes | ||
| nothing and matches none of CONSEC_FAIL's streak-reset needles | ||
| ("Addressed the latest review feedback", "no changes needed", "AutoFix | ||
| could not start", "updated a stale base"), so a persistently wedged | ||
| sandbox still terminates the PR at CONSECUTIVE_FAILURE_CAP. What no | ||
| longer terminates it is idle rounds INTERLEAVED with real progress — | ||
| which is the intended change: that PR is not stuck, the runner is. | ||
|
|
||
| Two consequences inside the block. IDLE_N's needle became the full | ||
| emitted headline prefix ('AutoFix ran out of time before finishing | ||
| (idle-timeout') rather than a bare 'idle-timeout' substring: IDLE_N is | ||
| now subtracted from TIMEOUT_N, so it MUST be a subset of it, and a | ||
| loose needle could otherwise match provider error text that | ||
| API_ERROR_DETAIL puts on the same first line and drive the difference | ||
| negative. And the all-idle remedy branch is gone as unreachable: the | ||
| guard now fires only when BUDGET_TIMEOUT_N alone reaches the cap, so a | ||
| tripped window always holds at least TIMEOUT_WINDOW_CAP genuine budget | ||
| timeouts — idle rounds can outnumber budget ones in it, but the budget | ||
| remedy applies because those budget timeouts exist, not because they | ||
| are the majority. | ||
|
|
||
| Idle rounds stay visible through a job-log ::warning:: rather than a PR | ||
| comment — the signal belongs to whoever owns the runners, and infra | ||
| noise should not spend a comment on someone's PR. The census and its | ||
| warning run outside the cap's terminal guard: the all-idle shape stops | ||
| via the consecutive breaker with that breaker's headline, and the | ||
| terminal run's log is exactly where the wedged runner must be named. | ||
|
|
||
| The same exclusion applies to the prepare step's PRIOR_TIMEOUTS census | ||
| (af-049): its budget warning tells the agent to narrow scope — the | ||
| budget remedy again — and an idle round never exhausted any budget, so | ||
| it must not steer the narrowing. Idle rounds are excluded there with | ||
| the same needle the cap census uses. | ||
| ``` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| name: 'Qwen Autofix' | ||
|
|
||
| # One workflow for the whole autonomous-fix lifecycle: | ||
|
|
@@ -245,7 +245,9 @@ | |
| # pushed rounds in between, so the consecutive cap never fired and the PR | ||
| # kept walking into the same wall; #7846 the same, twice. Counted over the | ||
| # current counting window (window-scoped like every other census), so a | ||
| # re-arm clears it along with the round counter. | ||
| # re-arm clears it along with the round counter. Counts BUDGET timeouts | ||
| # only: silent-sandbox (idle) timeouts are infra, not PR size, and are | ||
| # excluded — see qwen-autofix.md#af-073. | ||
| TIMEOUT_WINDOW_CAP: '3' | ||
| # Do not claim more issues when too many existing autofix PRs are still open. | ||
| MAX_OPEN_AUTOFIX_PRS: '5' | ||
|
|
@@ -4991,6 +4993,9 @@ | |
| # Time-budget exhaustions SINCE THE LAST SUCCESSFUL ROUND mean | ||
| # the standard address-everything prompt is not converging at | ||
| # Full rationale → qwen-autofix.md#af-049 | ||
| # Idle (silent-sandbox) timeouts are excluded like in the cap | ||
| # census: the narrowing advice targets budget exhaustion, and an | ||
| # infra-killed round never had any budget to exhaust (af-073). | ||
| PRIOR_TIMEOUTS="$(jq -r --arg ab "${AUTOFIX_BOT}" --arg key "${LIVE_REARM_KEY}" ' | ||
| [ .[] | select((.user.login // "") == $ab) | ||
| | select((.body // "") | contains("<!-- autofix-eval ")) | ||
|
|
@@ -4999,7 +5004,7 @@ | |
| ] | sort_by(.created_at) | ||
| | map((.body | gsub("\r"; "") | split("\n")[0])) | ||
| | (map(test("Addressed the latest review feedback|no changes needed")) | rindex(true) // -1) as $lastok | ||
| | [ .[($lastok + 1):][] | select(contains("AutoFix ran out of time before finishing")) ] | length' "${WORKDIR}/ic.json" 2> /dev/null || true)" | ||
| | [ .[($lastok + 1):][] | select(contains("AutoFix ran out of time before finishing") and (contains("AutoFix ran out of time before finishing (idle-timeout") | not)) ] | length' "${WORKDIR}/ic.json" 2> /dev/null || true)" | ||
| if [[ "${PRIOR_TIMEOUTS}" -ge 1 ]]; then | ||
| echo | ||
| echo '## Budget warning: previous round(s) ran out of time' | ||
|
|
@@ -6674,9 +6679,31 @@ | |
| # full agent budget with nothing to show (observed on #7929: | ||
| # three timeouts with successes in between; #7846 twice). The | ||
| # census reuses PRIOR_HEADS, so it is window-scoped exactly | ||
| # like the consecutive one and a re-arm clears it. Only | ||
| # overrides a would-be RETRY: a round already terminal keeps | ||
| # its own headline (the consecutive breaker included). | ||
| # like the consecutive one and a re-arm clears it. Only the | ||
| # cap gate below overrides a would-be RETRY: a round already | ||
| # terminal keeps its own headline (the consecutive breaker | ||
| # included). The idle census and its warning run OUTSIDE that | ||
| # guard: the all-idle shape terminates via the consecutive | ||
| # breaker above, and that terminal run's job log is exactly | ||
| # where the wedged runner must be named. | ||
| # Idle (silent-sandbox) timeouts are EXCLUDED from this cap: | ||
| # a wedged runner/docker is not this PR being too big, an idle | ||
| # round dies at QWEN_IDLE_TIMEOUT_MS having produced no byte | ||
| # (a fraction of a real round), and no budget increase cures | ||
| # one — so this breaker's remedy does not apply to them. A | ||
| # persistently wedged sandbox stays bounded by | ||
| # CONSECUTIVE_FAILURE_CAP, which an idle round DOES feed. | ||
| # Full rationale → qwen-autofix.md#af-073 | ||
| IDLE_N="$(grep -c 'AutoFix ran out of time before finishing (idle-timeout' <<< "${PRIOR_HEADS}" || true)" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R1-3: Still stands — reported in rounds 1-2, deferred by the autofix loop, re-verified at this commit. The idle-vs-budget reclassification this PR introduces is applied at only one of the census sites that count the same timeout headline: the sibling af-049 Witness: running the prepare step's jq verbatim against a fixture of one pushed round + one pure idle round yields 中文说明[Suggestion] R1-3:仍然成立 —— 已在第 1-2 轮报告,被 autofix 循环顺延,本轮在该提交上重新核实。本 PR 引入的 idle/预算重分类只应用在了统计同一超时标题的普查点之一:prepare 步骤里的姊妹点位 af-049 见证:对一个"一次推送轮 + 一次纯 idle 轮"的夹具逐字运行 prepare 步骤的 jq,得 — qwen3.8-max via Qwen Code /review (v0.21.15) |
||
| if [[ "${AGENT_TIMEOUT:-}" == 'idle-timeout'* ]]; then | ||
| IDLE_N=$(( IDLE_N + 1 )) | ||
| fi | ||
|
Comment on lines
+6698
to
+6700
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R1-4: Still stands — reported in rounds 1-2, deferred by the autofix loop. Idle classification now lives in three independently-pinned places — this current-round prefix test, the census grep needle, and the headline assembly ( 中文说明[Suggestion] R1-4:仍然成立 —— 已在第 1-2 轮报告,被 autofix 循环顺延。idle 分类现在存在于三个独立钉住的位置 —— 本处的当前轮前缀判断、普查 grep 匹配串、以及由 run-agent.mjs 的 detail 模板喂给的标题拼装( — qwen3.8-max via Qwen Code /review (v0.21.15) |
||
| # Excluding idle from the cap must not hide it. The job log is | ||
| # the right surface: it reaches the operator without spending a | ||
| # PR comment on infra noise. | ||
| if [[ "${IDLE_N}" -gt 0 ]]; then | ||
| echo "::warning::#${PR}: ${IDLE_N} silent-sandbox (idle) timeout(s) this counting window — excluded from the ${TIMEOUT_WINDOW_CAP}-timeout cap; check the sandbox image and the runner docker daemon" | ||
|
Comment on lines
+6704
to
+6705
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R3-1: Of this idle-census expect(mixedTrips.log).toContain(
`excluded from the ${timeoutCap}-timeout cap; check the sandbox image and the runner docker daemon`,
);Witness: tail mutant and cap-swap mutant each ship the existing suite green; with the pin added the cap-swap mutant fails — expected 中文说明[Suggestion] R3-1:这条 idle 普查 见证:尾部变异与上限替换变异各自让现有套件全绿;补上断言后,上限替换变异失败 —— 期望 — qwen3.8-max via Qwen Code /review (v0.21.15) |
||
| fi | ||
| if [[ "${MARK_ROUND}" != "${MAX_ROUNDS}" ]]; then | ||
| # Needle matches the emitted headline verbatim — first lines | ||
| # can embed provider error text (API_ERROR_DETAIL puts up to | ||
|
|
@@ -6686,38 +6713,32 @@ | |
| if [[ -n "${AGENT_TIMEOUT:-}" ]]; then | ||
| TIMEOUT_N=$(( TIMEOUT_N + 1 )) | ||
| fi | ||
| # Idle (silent-sandbox) timeouts share the census — they burn | ||
| # the same full budget — but no budget increase cures them, so | ||
| # when the window contains any, the breaker says so. | ||
| IDLE_N="$(grep -c 'idle-timeout' <<< "${PRIOR_HEADS}" || true)" | ||
| if [[ "${AGENT_TIMEOUT:-}" == 'idle-timeout'* ]]; then | ||
| IDLE_N=$(( IDLE_N + 1 )) | ||
| fi | ||
| if [[ "${TIMEOUT_N}" -ge "${TIMEOUT_WINDOW_CAP}" ]]; then | ||
| # IDLE_N's needle is TIMEOUT_N's needle extended with the | ||
| # idle cause's opening token — every line matching it also | ||
| # matches TIMEOUT_N's, so IDLE_N can never exceed TIMEOUT_N | ||
| # and the subtraction below can never go negative. | ||
| BUDGET_TIMEOUT_N=$(( TIMEOUT_N - IDLE_N )) | ||
| if [[ "${BUDGET_TIMEOUT_N}" -ge "${TIMEOUT_WINDOW_CAP}" ]]; then | ||
| MARK_ROUND="${MAX_ROUNDS}" | ||
| # The headline states what the census MEASURED — the | ||
| # window's cumulative count — not "stopped after N | ||
| # timeouts": the round that trips this can itself have | ||
| # failed differently (a gate rejection landing on a window | ||
| # that already carries the cap — the exact rollout state | ||
| # of #7929/#7846). | ||
| # No all-idle branch here: the guard above only fires when | ||
| # BUDGET_TIMEOUT_N alone reaches the cap, so a tripped | ||
| # window always holds at least TIMEOUT_WINDOW_CAP genuine | ||
| # budget timeouts — idle rounds can outnumber budget ones | ||
| # in it, but the budget remedy is always the right one. | ||
| IDLE_CLAUSE='' | ||
| IDLE_CLAUSE_ZH='' | ||
| if [[ "${IDLE_N}" -gt 0 ]]; then | ||
| IDLE_CLAUSE=" ${IDLE_N} of those were silent-sandbox (idle) timeouts that no budget increase can cure — investigate the sandbox image and runner docker daemon for those." | ||
| IDLE_CLAUSE_ZH="其中 ${IDLE_N} 次是静默 sandbox(idle)超时,提高预算也治不了 —— 请针对这些排查 sandbox 镜像与 runner 的 docker daemon。" | ||
| fi | ||
| # Mirror the round-level split: when EVERY counted timeout | ||
| # was idle, the closing remedy must not prescribe the | ||
| # budget increase the clause above just declared useless. | ||
| REMEDY='split or reduce the PR (or raise the agent time budget AND its step backstop together)' | ||
| REMEDY_ZH='拆分或缩减该 PR(或同时提高 agent 时间预算与其步骤兜底)' | ||
| if [[ "${IDLE_N}" -ge "${TIMEOUT_N}" ]]; then | ||
| REMEDY='investigate the sandbox image and runner docker daemon' | ||
| REMEDY_ZH='排查 sandbox 镜像与 runner 的 docker daemon' | ||
| IDLE_CLAUSE=" The window also holds ${IDLE_N} silent-sandbox (idle) timeout(s), which no budget increase can cure and which do NOT count toward this cap — investigate the sandbox image and runner docker daemon separately." | ||
| IDLE_CLAUSE_ZH="本窗口另有 ${IDLE_N} 次静默 sandbox(idle)超时,提高预算也治不了,且不计入本上限 —— 请另行排查 sandbox 镜像与 runner 的 docker daemon。" | ||
|
Comment on lines
+6737
to
+6738
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R3-1: The expect(mixedTrips.headline).toContain('also holds 1 silent-sandbox');
expect(mixedTrips.headlineZh).toContain('本窗口另有 1 次静默');Witness: both clause mutants ship 中文说明[Suggestion] R3-1:插入到本熔断停机子句中的 见证:两个子句变异均让 — qwen3.8-max via Qwen Code /review (v0.21.15) |
||
| fi | ||
| HEADLINE="🤖 AutoFix stopped: this counting window now contains ${TIMEOUT_N} time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is ${TIMEOUT_N} full agent runs that pushed nothing.${IDLE_CLAUSE} A human should ${REMEDY}, then comment \`${RETRY_COMMAND}\` to re-arm. Until then future scans will skip this PR." | ||
| HEADLINE_ZH="🤖 AutoFix 已停止:当前计数窗口内已累计 ${TIMEOUT_N} 次时间预算耗尽(含其间推送过的轮次;本轮本身可能以别的方式失败)。即 ${TIMEOUT_N} 次完整 agent 运行没有推送任何内容。${IDLE_CLAUSE_ZH}应由人工${REMEDY_ZH},然后评论 \`${RETRY_COMMAND}\` 重新武装。在此之前,后续扫描将跳过本 PR。" | ||
| HEADLINE="🤖 AutoFix stopped: this counting window now contains ${BUDGET_TIMEOUT_N} agent time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is ${BUDGET_TIMEOUT_N} full agent runs that pushed nothing.${IDLE_CLAUSE} A human should split or reduce the PR (or raise the agent time budget AND its step backstop together), then comment \`${RETRY_COMMAND}\` to re-arm. Until then future scans will skip this PR." | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R3-1: Of the two expect(mixedTrips.headline).toContain(`That is ${timeoutCap} full agent runs`);
expect(mixedTrips.headline).not.toContain(`That is ${timeoutCap + 1} full agent runs`);
expect(mixedTrips.headlineZh).toContain(`即 ${timeoutCap} 次完整`);Witness: the second-occurrence mutant ships 中文说明[Suggestion] R3-1:这条标题里有两处 见证:第二处插值的变异让 — qwen3.8-max via Qwen Code /review (v0.21.15) |
||
| HEADLINE_ZH="🤖 AutoFix 已停止:当前计数窗口内已累计 ${BUDGET_TIMEOUT_N} 次时间预算耗尽(含其间推送过的轮次;本轮本身可能以别的方式失败)。即 ${BUDGET_TIMEOUT_N} 次完整 agent 运行没有推送任何内容。${IDLE_CLAUSE_ZH}应由人工拆分或缩减该 PR(或同时提高 agent 时间预算与其步骤兜底),然后评论 \`${RETRY_COMMAND}\` 重新武装。在此之前,后续扫描将跳过本 PR。" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Witness: probe extending the sentinel tuple with 中文说明
(证据见上:探针在该变异下观测到中文侧 4 对英文侧 3;基线两侧均为 3。) — qwen3.8-max via Qwen Code /review (v0.21.15)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R1-8: 中文说明
— qwen3.8-max via Qwen Code /review (v0.21.15) |
||
| fi | ||
| fi | ||
| fi | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Critical] The new
af-073anchor is added without a Contents-table entry, and that fails this repo's existing design-record consistency testscripts/tests/workflow-size.test.js("lists every section in the contents table"): the test asserts the TOC link targets equal every<a id="af-N">anchor in this file. This diff adds anchoraf-073but the table still ends at entry 72, so the suite fails — the PR's ownTest (ubuntu-latest, Node 22.x)lane is red with this exact assertion right now.Witness:
Add the entry for section 73 to the Contents block (after the entry-72 line, ~line 126), matching the house style — the test's regex only requires the
- [73. ...](#af-073)shape:中文说明
新增的
af-073锚点没有对应的目录条目,这会使仓库既有的设计档案一致性测试scripts/tests/workflow-size.test.js("lists every section in the contents table")失败:该测试断言目录链接目标与本文件中的每个<a id="af-N">锚点一一对应。本次 diff 添加了锚点af-073,但目录仍止于第 72 条,因此该测试套件失败——本 PR 自己的Test (ubuntu-latest, Node 22.x)通道现在正是因为这条断言而变红。(证据见上方代码块:PR 侧该测试 1 failed | 56 passed;换回合并基线的输入文件后 57/57 全绿;线上 CI 同一断言失败。)
修复:在 Contents 区块(第 72 条之后,约第 126 行)为第 73 节添加条目,沿用既有风格——测试的正则只要求
- [73. ...](#af-073)这一形状。— qwen3.8-max via Qwen Code /review (v0.21.15)