fix(autofix): state the primary agent budget and use the step's headroom - #8257
Conversation
|
Thanks for the PR! Template: the body uses its own headings ( Problem: observed, not theoretical. The #8005 round-9 timeout (run 30646547838) is cited with per-step timings — the agent died at exactly 50m03s on its own timer while the step cap sat at 80m, so a third of the step was unreachable and every "ran out of time" round ended for a reason this file never mentioned. I confirmed the mechanism in the base code: Direction: aligned. This is internal autofix CI tooling and squarely in scope; it touches no auth/sandbox/model-selection/public-contract surface (the existing secret-forwarding note is unchanged). No CHANGELOG signal to cite — this is repo-specific CI infra, not a product feature. Size: not applicable — no core paths. 61 additions across Approach: the scope is right and the change is minimal — one env var ( Risk: no elevated risk signals — neither changed file matches the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献! 模板:正文使用了自己的小标题( 问题:是已观测到的,而非理论性的。PR 引用了 #8005 第 9 轮超时(run 30646547838)的逐步耗时——agent 恰好在 50分03秒时被自己的计时器终止,而 step 上限悬在 80 分钟,因此 step 有三分之一永远用不到,每一次"超时"轮次都以一个本文件从未提及的原因结束。我在基线代码里确认了机制: 方向:对齐。这是内部 autofix CI 工具,完全在范围内;不触及 auth/sandbox/模型选择/公共契约面(既有的 secret 转发说明保持不变)。没有可引用的 CHANGELOG 信号——这是仓库专属的 CI 基础设施,不是产品功能。 规模:不适用——未触及核心路径。共 61 行新增,分布在 方案:范围合理,改动最小——一个环境变量( 风险:无升级风险信号——两个改动文件都未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewMy independent take before reading the diff: set Verified against the base code:
No correctness, security, or convention issues. The change is one env var plus comments plus a test; no new secret surface (the value comes from TestingThis is an unattended CI run — I did not build or run the PR's code (per the static-review rule). The signal below is the PR's own CI on the reviewed commit. The claim under review is a static workflow invariant (budget under cap with margin), which the new test substantiates by construction — it parses the actual YAML and asserts the relationship, and the PR's mutation table reports it catches a removed / over-cap / too-close budget. I could not independently re-run that mutation table here; I'm treating it as the author's claim, but the test's design does pin the invariant. The relevant suite is still running, so confirmation is pending — the table below updates in place once CI settles. No user-visible / TUI behavior changes, so real-scenario (tmux) testing is N/A.
The ubuntu suite carries 中文说明代码审查我在读 diff 之前的独立想法:在主步骤上显式设置 已对照基线代码核实:
无正确性、安全性或规范问题。改动就是一个环境变量加注释加一个测试;没有新的 secret 面(值来自 测试这是一次无人值守的 CI 运行——我未构建或运行 PR 的代码(遵循静态审查规则)。下方信号是 PR 自身在被审查 commit 上的 CI。 本次审查的主张是一个静态工作流不变量(预算低于上限且有余量),新测试从构造上就证实了它——它解析真实的 YAML 并断言该关系;PR 的变异表报告说它能捕获"移除/超上限/过近"的预算。我无法在此独立重跑该变异表;我将其视为作者的主张,但测试的设计确实钉住了该不变量。相关测试套件仍在运行,因此确认尚待——下方表格会在 CI 落定后就地更新。 无用户可见 / TUI 行为变化,因此真实场景(tmux)测试为 N/A。 ubuntu 套件承载 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean, minimal, well-evidenced fix from a maintainer; the only open item is the ubuntu suite still running, which the deferred approval waits on. Stepping back: this is exactly the kind of PR the gate should wave through. The problem is measured (a real CI run, per-step timings), the fix is the smallest thing that solves it (one env var, made visible next to the cap it must stay under), and the test pins the invariant that actually broke — the two numbers drifting apart — rather than restating them. My independent proposal was the same idea; the PR's test is better than the lazy version I'd have written. The restraint in not building a width-scaled budget on five data points is the right KISS call. The one mild reservation: the inline comments are on the verbose side. They document a real invariant, so I wouldn't block on it, but a reviewer may decide some of the rationale belongs in the commit message rather than the YAML. Approval is deferred only until CI lands green on the reviewed commit — the ubuntu suite carrying the workflow test is still in progress. Once it's green, this is ready to ship. 中文说明置信度:4/5 —— 来自维护者的干净、最小、证据充分的修复;唯一未决项是仍在运行的 ubuntu 套件,延迟批准正在等待它。 退一步看:这正是 gate 应当放行的那类 PR。问题是实测的(真实的 CI run、逐步耗时),修复是解决它的最小改动(一个环境变量,放在它必须低于的上限旁边使其可见),测试钉住了真正被破坏的不变量——两个数字彼此漂移——而不是把它们重述一遍。我的独立方案是同一个想法;PR 的测试比我本来会写的偷懒版本更好。拒绝在五个数据点上造一个按宽度缩放的预算,是正确的 KISS 决定。 唯一的一点保留:内联注释偏多。它们记录了一个真实的不变量,因此我不会因此阻塞,但 reviewer 可以决定其中一部分理由该放进 commit message 而非 YAML。 批准仅延迟到 CI 在被审查 commit 上转绿——承载工作流测试的 ubuntu 套件仍在运行。一旦转绿,即可合入。 — Qwen Code · qwen3.8-max-preview Reviewed at |
The primary attempt took run-agent.mjs's 50-minute default while its step capped at 80, so a third of that step was unreachable and every "ran out of time" round ended for a reason this file never named. Raising it surfaced the larger problem: the budget was never checked as a whole. Measured on one scan (run 30646547838): setup 5-7m in earlier steps, so it never competes with the agent step; Triage and address 50m03s on #8005 round 9 (its own timer) and 12m45s on #8211; the Verification gate 22m48s on #8211 — the largest consumer in the job, and unbounded; push, report and finalize 3-4s. The old arithmetic ("80 + 20 leaves ~50 for setup, two verification passes, and reporting") predates that measurement: the real worst case was 7 + 80 + 23 + 20 + 23 = 153 against a 150-minute job, and a JOB timeout cancels the always() reporters, which is the silent round the design exists to prevent. Sized for the PRs that actually exhaust the budget (47 and 35 changed files): agent 120m under a 130m step cap, each verification gate bounded at 60m — 2.6x the measured pass, and a graceful degrade because both gates already carry continue-on-error, so a bound turns a job kill into the ordinary verification-failure path that reports. Worst case 7 + 130 + 60 + 20 + 60 + 3 = 280 against a 300-minute job, itself under the 360-minute ceiling ubuntu-latest imposes regardless. Raising the budget does not make a doomed round cheaper — each exhausted budget still pushes nothing, which is why TIMEOUT_WINDOW_CAP stops a PR after three in one window. It buys rounds that were close to finishing. The test now asserts the SUM against the job cap and the count of bounded steps, because asserting the numbers individually is exactly what let 153-against-150 pass. Mutations checked: budget over its step cap, either verification bound removed, job cap back to 150, job cap over the runner ceiling, and continue-on-error dropped are each caught.
1e3cf0a to
07d1e1a
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)为单个提交。 |
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.
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
| # buys rounds that were close to finishing, not rounds that were | ||
| # never going to. QWEN_AUTOFIX_TIMEOUT_MS overrides it without a | ||
| # code change; a malformed value falls back to the runner default. | ||
| QWEN_TIMEOUT_MS: '${{ vars.QWEN_AUTOFIX_TIMEOUT_MS || 7200000 }}' |
There was a problem hiding this comment.
[Suggestion] The vars.QWEN_AUTOFIX_TIMEOUT_MS override has no upper clamp, so the budget↔cap invariant this PR adds a test to protect only holds for the hardcoded fallback 7200000. — Failure scenario: a maintainer sets the repo variable to a round "give it more time" value such as 9000000 (150 min); a non-empty string is truthy in GitHub expressions, so the expression evaluates to 9000000, not the fallback. run-agent.mjs:21 then arms its internal timer at 150 min, but the step's timeout-minutes: 130 kills the process first — timedOut stays false, the agent-timeout marker (run-agent.mjs:329-330) is never written, and the report step misclassifies the round as a crash, which is exactly the misreport the new comment warns about. The headroom is only 10 minutes, so any override ≥ 7,800,000 ms breaks the invariant, and the new test (which only sees the literal fallback) cannot catch it.
Recommended fix — state the ceiling next to the override (and optionally clamp in run-agent.mjs with Math.min(parsed, cap) so the marker is still written):
# The override MUST stay below 7,800,000 ms (the 130-minute step cap above);
# at or above it the step cap wins the race, agent-timeout is never written,
# and the round is reported as a crash.
QWEN_TIMEOUT_MS: '${{ vars.QWEN_AUTOFIX_TIMEOUT_MS || 7200000 }}'中文说明
[Suggestion] vars.QWEN_AUTOFIX_TIMEOUT_MS 覆盖值没有上限钳制,因此本 PR 新增测试所保护的「预算↔上限」不变量只对硬编码回退值 7200000 成立。—— 失败场景:维护者把仓库变量设成一个取整的「多给点时间」的值,例如 9000000(150 分钟);非空字符串在 GitHub 表达式中为真,因此表达式求值为 9000000 而非回退值。run-agent.mjs:21 随即把内部计时器定为 150 分钟,但 step 的 timeout-minutes: 130 会先杀掉进程——timedOut 保持为 false,agent-timeout 标记(run-agent.mjs:329-330)永远不会写出,上报步骤便把该轮误判为崩溃,而这正是新增注释所警告的误报。余量只有 10 分钟,因此任何 ≥ 7,800,000 ms 的覆盖值都会破坏该不变量,而新测试(只能看到字面回退值)无法捕获它。
建议修复——在覆盖值旁注明上限(并可选地在 run-agent.mjs 中用 Math.min(parsed, cap) 钳制,使标记仍会写出):见上方代码块。
— qwen3.8-max-preview via Qwen Code /review
ReviewThe diagnosis holds up. I re-derived the arithmetic against the branch and re-ran the suite:
Five things I'd change or want stated, roughly in order of how much they matter. 1. The new invariant cannot catch the bug class it was written for
I verified it rather than reasoning about it. Inserting - name: 'Extra unbounded work'
if: |-
${{ always() }}
run: |-
echo hibefore
2. The override escapes the invariant, and the comment advertises it without a ceiling
The test only pins the margin for the literal default — 3. Three comments now describe the old numbers as current
( 4.
|
|
@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 冲突,直到移除标签或达到轮次上限。移除 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #8257Addressed the automated reviewer's inline suggestion and all five points from @wenshao's review (the two the reviewer flagged as must-fix before landing, plus the three smaller notes). One out-of-scope item is deliberately deferred. No conflict ( Inline finding
@wenshao's review (issue-level)
Out of scope (deferred)
Mutation evidence for point #1The reviewer demonstrated that inserting an unbounded step left the old suite green (107/107). Against the new assertion:
Verification
中文说明Autofix 审查轮次 — PR #8257本轮处理了自动审查器的行内建议,以及 @wenshao 审查中的全部五点(其中两点是审查者标注的合入前必修项,另外三点为较小的注记)。有一项范围外条目被有意推迟。无冲突( 行内发现
@wenshao 的审查(issue 级)
范围外(推迟)
第 1 点的变异证据审查者曾演示:插入一个无约束步骤会让旧测试套件仍全绿(107/107)。在新断言下:
验证
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/模型 |
ReviewReviewed at Verified
Issues1. The override ceiling is documented but unenforced. The comment states This is the PR's own thesis reappearing one layer up: a budget nobody can see is a budget nobody re-checks. The fix is cheap and turns the comment into an invariant — clamp in the step's CAP_MS=7200000 # must stay <= the step's timeout-minutes above
if [[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]+$ ]] || (( QWEN_TIMEOUT_MS > CAP_MS )); then
echo "::warning::QWEN_TIMEOUT_MS=${QWEN_TIMEOUT_MS} exceeds the step backstop; clamping to ${CAP_MS}"
QWEN_TIMEOUT_MS="${CAP_MS}"
fi
export QWEN_TIMEOUT_MSThe test can then assert the clamp exists and that 2. The breaker headline now recommends the one action the new comment forbids. 3. Worst case there is roughly 4. 5. "the queueing tail above is unchanged" overclaims. True of the cap: 300 vs 150 does not itself lengthen anything. Not true of the budget: the 81-minute tail was measured against rounds ending at 40–57m, and 50m → 120m roughly doubles slot-hold time for precisely the timeout-prone PRs this change targets. Suggest: "the cap does not change the tail; the raised budget does, for the PRs that exhaust it." Style6. Test placement. All ~90 new lines went into 7. Comment volume. ~45 lines of comment for one env var and one RiskThe change is well-argued and the measurements are real. The residual cost is worth stating plainly: worst case is now ~4.7h of wall time before a stuck round reports back to the thread, None of the above blocks merging. (1) and (2) are the two I'd want before this lands, since they concern a knob whose misuse silently reintroduces the misreport the PR is closing. 中文小结已验证: 主要问题:
风格: 新增的约 90 行断言塞进了一个名字只覆盖三分之一内容的 均不阻塞合入,其中 (1)(2) 建议落地前处理。 |
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
| const longSteps = stepBlocks.filter((b) => | ||
| /node [^\n]*run-agent\.mjs|bash [^\n]*run-autofix-review-verification\.sh/.test( |
There was a problem hiding this comment.
[Suggestion] The longSteps detector hard-codes two script names, so the whole-budget sum invariant only guards steps that run run-agent.mjs or run-autofix-review-verification.sh. The comment above is also internally contradictory: 'A new long step shows up here and must carry its own bound' cannot hold at the same time as 'Cheap setup/report steps are allowlisted implicitly by not matching' — a new long step running a new script does not match, so it is silently allowlisted as if cheap. — Failure scenario: a maintainer adds an unbounded long step running a different script (e.g. bash …/run-full-e2e-suite.sh); longSteps stays at 4, worstCaseMin still reads 295 ≤ 300, every assertion stays green, but the real worst case now overruns the 300-minute job cap → GitHub cancels the always() reporters and the round goes silent, the exact failure this PR closes.
Fix: tighten the comment to state the real boundary ('a new step running one of these two scripts shows up here'), or broaden the detector (e.g. sum the bounds of every run: step in the job and treat an unbounded one as infinite) so a new long step on any script is caught.
中文说明
longSteps 检测器把两个脚本名写死了,因此整体预算求和不变量只保护运行 run-agent.mjs 或 run-autofix-review-verification.sh 的步骤。上方注释也自相矛盾:「新的长步骤会出现在这里,必须自带约束」与「廉价的 setup/report 步骤因不匹配而被隐式列入白名单」无法同时成立——一个运行新脚本的长步骤并不匹配,因此会被静默地当作廉价步骤列入白名单。— 失败场景:维护者新增一个运行不同脚本的无约束长步骤(例如 bash …/run-full-e2e-suite.sh);longSteps 仍为 4,worstCaseMin 仍读作 295 ≤ 300,所有断言依旧全绿,但真实最坏情况此时已越过 300 分钟的 job 上限 → GitHub 会取消 always() 上报步骤,该轮就此静默,正是本 PR 要消灭的失败。
修复:把注释收紧到真实边界(「运行这两个脚本之一的新步骤会出现在这里」),或拓宽检测器(例如对 job 内每个 run: 步骤的约束求和、把无约束者视为无穷大),使任何脚本上的新长步骤都能被捕获。
— qwen3.8-max-preview via Qwen Code /review
| const worstCaseMin = | ||
| stepCaps.reduce((a, b) => a + b, 0) + SETUP_AND_REPORT_MIN; | ||
| expect(worstCaseMin).toBeLessThanOrEqual(jobCapMin); |
There was a problem hiding this comment.
[Suggestion] This test file is not collected by any CI job. No workflow in .github/workflows/ runs npm run test:scripts (or otherwise invokes scripts/tests/), so the budget-sum invariant, the verification-gate bounds, the QWEN_TIMEOUT_MS margin and the continue-on-error checks this PR adds never run in any pipeline. — Failure scenario: a future change to qwen-autofix.yml breaks the invariant (drops a gate's timeout-minutes, or sets QWEN_TIMEOUT_MS above the step backstop); none of the 107 assertions here runs in CI, so the regression ships silently and the next autofix round misreports a timeout as a crash (the agent-timeout marker is never written because the step cap fires first). The file passing locally (107/107) is exactly what masks this.
Fix: add a CI step that runs npm run test:scripts (the script already exists in the root package.json) so these assertions gate changes to the autofix workflow. The unreachability predates this PR (hence Suggestion, not Critical), but the assertions this PR adds inherit it.
中文说明
本测试文件没有被任何 CI job 收集。.github/workflows/ 里没有任何 workflow 运行 npm run test:scripts(或以其他方式调用 scripts/tests/),因此本 PR 新增的预算求和不变量、验证门约束、QWEN_TIMEOUT_MS 余量以及 continue-on-error 检查在任何流水线里都不会运行。— 失败场景:未来对 qwen-autofix.yml 的改动破坏了该不变量(移除某个验证门的 timeout-minutes,或把 QWEN_TIMEOUT_MS 设到超过 step 兜底上限);这里 107 条断言没有一条会在 CI 中运行,于是回归会静默合入,下一次 autofix 轮次便把超时误报为崩溃(step 上限先触发,agent-timeout 标记永不写出)。本文件在本地全绿(107/107)恰恰掩盖了这一点。
修复:新增一个运行 npm run test:scripts 的 CI 步骤(根 package.json 里已有该脚本),让这些断言能够把关对 autofix 工作流的改动。该不可达性早于本 PR(因此定为 Suggestion 而非 Critical),但本 PR 新增的断言继承了它。
— qwen3.8-max-preview via Qwen Code /review
There was a problem hiding this comment.
Declined — this one is a false positive. The assertions in this file do gate changes in CI: ci.yml runs npm run test:ci as its main unit-test step (the Run tests and generate reports step, plus the macOS/Windows merge-queue jobs), and the root package.json defines test:ci as ... && npm run test:scripts, which collects scripts/tests/qwen-autofix-workflow.test.js. So a future change that breaks the budget-sum invariant fails CI on every PR today; the failure scenario (regression ships silently because none of the assertions run) cannot occur. A separate step would be a redundant duplicate of that existing gate, so I'm leaving it out to avoid diff growth. Happy to add a dedicated step anyway if a maintainer wants the script tests visible as their own CI line rather than chained inside test:ci.
中文说明
拒绝——这一条是误报。本文件的断言确实在 CI 中把关:ci.yml 把 npm run test:ci 作为其主要单测步骤运行(Run tests and generate reports 步骤,外加 macOS/Windows 的 merge-queue job),而根 package.json 将 test:ci 定义为 ... && npm run test:scripts,后者会收集 scripts/tests/qwen-autofix-workflow.test.js。因此,未来任何破坏预算求和不变量的改动今天都会在每个 PR 的 CI 上失败;其失败场景(因为断言从不运行而静默合入回归)不可能发生。单独新增一个步骤只是对现有这道门的冗余重复,因此为避免 diff 膨胀我不加入。如果维护者希望把脚本测试作为独立的一条 CI 显示、而不是链在 test:ci 内部,我也乐意照做。
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressed1. Override ceiling enforced at runtime (Required)Decision: Implemented. Added a clamp in the The test derives the clamp ceiling from the workflow and asserts it stays under the step backstop ( 2. Breaker headline no longer recommends a forbidden action (Required)Decision: Implemented. Updated both the 3.
|
OverviewTwo things, and the second is the more valuable one:
The framing is right: the bug worth fixing here wasn't the 50-minute default, it was that nothing checked the sum. The new test asserts that sum rather than the individual numbers, which is the correct lesson. I verified the key claims against the head tree rather than taking them from the description:
Issues1.
|
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
| BUDGET_CAP_MS=7200000 | ||
| if [[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]+$ ]] || (( QWEN_TIMEOUT_MS > BUDGET_CAP_MS )); then |
There was a problem hiding this comment.
[Suggestion] The clamp compares the override using bash arithmetic, where a leading-zero integer is octal — so a zero-padded value silently bypasses the guard. — Failure scenario: a maintainer sets repo variable QWEN_AUTOFIX_TIMEOUT_MS=08000000 (zero-padded). The ^[0-9]+$ regex passes, but (( 08000000 > 7200000 )) errors out ("value too great for base" — 8 is not a valid octal digit) and the condition evaluates false, so the clamp does not fire and the value is exported unchanged with no ::warning::. run-agent.mjs:21 then reads it with JS Number() (always decimal) = 8,000,000 ms = 133 min, arming the timer past the 130-minute step backstop — the cap fires first, agent-timeout is never written, and the round is misreported as a crash, the exact case this clamp says it prevents. Reproduced empirically under bash. Fix: force base-10 with the 10# prefix (the regex guard already short-circuits non-digit values).
| BUDGET_CAP_MS=7200000 | |
| if [[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]+$ ]] || (( QWEN_TIMEOUT_MS > BUDGET_CAP_MS )); then | |
| BUDGET_CAP_MS=7200000 | |
| if [[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]+$ ]] || (( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS )); then |
中文说明
[建议] 该钳制使用 bash 算术进行比较,而 bash 中前导零整数会被解释为八进制——因此补零的取值会静默绕过这道防护。— 失败场景:维护者将仓库变量设为 QWEN_AUTOFIX_TIMEOUT_MS=08000000(补零)。^[0-9]+$ 正则通过,但 (( 08000000 > 7200000 )) 会报错("value too great for base"——8 不是合法八进制位),条件求值为假,于是钳制不触发,该值原样导出且没有 ::warning::。随后 run-agent.mjs:21 用 JS Number()(恒为十进制)读到 8,000,000 ms = 133 分钟,把计时器定到超过 130 分钟的 step 兜底上限——step 上限先触发,agent-timeout 永不写出,该轮被误报为崩溃,正是此钳制声称要防止的情形。已在 bash 下实证复现。修复:用 10# 前缀强制十进制(正则守卫已先行短路非数字取值)。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: AutoFix round summaryAddressed the actionable review feedback on the round-level budget PR. Fixed the octal clamp bypass, aligned the pending-check staleness bound with the 300-minute job cap (in code and in the test), held the clamp ceiling to the same margin rule the fallback already enforces, and corrected the stale comments. Two maintainer judgment calls are escalated below rather than decided here, per the review's verdict. Inline review comments
Maintainer comments (@wenshao)
Also noted (not a code change)
Conflict notesNo conflict; VerificationCommands actually run this round (all from the repository root):
中文说明AutoFix 本轮摘要处理了本轮关于「轮级预算」PR 的可操作评审反馈。修复了八进制钳制绕过问题,让 pending-check 过期边界与 300 分钟的 job 上限对齐(代码与测试两处),把钳制上限收敛到 fallback 已在执行的同一条余量规则,并更正了过时的注释。按照评审结论,下方有两项维护者层面的判断项被上交给维护者决定,而非在此自行拍板。 行内评审评论
维护者评论(@wenshao)
另需说明(非代码改动)
冲突说明无冲突; 验证本轮实际运行的命令(均在仓库根目录执行):
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/模型 |
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
| expect(addressStep).toContain('BUDGET_CAP_MS=7200000'); | ||
| expect(addressStep).toMatch( | ||
| /QWEN_TIMEOUT_MS.*exceeds the step backstop; clamping/, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The clamp block's if condition ([[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]+$ ]] || (( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS ))) is only string-matched here, never executed — so mutations to that condition survive green. — Failure scenario: flipping > to < (passes over-cap values through, clamps under-cap ones), removing the ! from the regex test, or dropping 10# (a zero-padded octal value slips past) each leaves toContain('BUDGET_CAP_MS=7200000') and the warning-string match passing, so the suite stays green while the guard is broken. The adjacent replays the handoff decision and terminal-round transitions under bash test already extracts workflow bash and runs it with controlled inputs; the clamp block can be replayed the same way (e.g. QWEN_TIMEOUT_MS=9999999 → 7200000, =100 → unchanged, =abc → 7200000, =07200000 → clamped without error), asserting the exported value.
// extract BUDGET_CAP_MS=… through export QWEN_TIMEOUT_MS from addressStep, then:
for (const [input, want] of [['9999999','7200000'],['100','100'],['abc','7200000'],['07200000','7200000']]) {
const out = execFileSync('bash', ['-c', clampBlock + '\nprintf "%s" "$QWEN_TIMEOUT_MS"'],
{ env: { ...process.env, QWEN_TIMEOUT_MS: input } }).toString();
expect(out).toBe(want);
}中文说明
clamp 代码块的 if 条件([[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]+$ ]] || (( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS )))在这里只被字符串匹配,从未真正执行——因此对该条件的变异会让测试套件仍然全绿。失败路径:把 > 翻成 <(放行超上限值、钳制低于上限的值)、去掉正则测试里的 !、或去掉 10#(带前导零的八进制值会绕过守卫),toContain('BUDGET_CAP_MS=7200000') 与告警字符串匹配都仍然通过,于是守卫已坏而套件依旧绿色。相邻的 replays the handoff decision and terminal-round transitions under bash 测试已经有现成模式:从 workflow 提取 bash 并在受控输入下运行;clamp 代码块可用同样方式回放(例如 QWEN_TIMEOUT_MS=9999999 → 7200000、=100 → 不变、=abc → 7200000、=07200000 → 被钳制且不报错),并对导出的值做断言。
— qwen3.8-max-preview via Qwen Code /review
| BUDGET_CAP_MS=7200000 | ||
| if [[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]+$ ]] || (( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS )); then |
There was a problem hiding this comment.
[Suggestion] The clamp's (( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS )) wraps modulo 2^64, so a value ≥ 2^63 bypasses the guard with no warning and no clamp. — Failure scenario: a repo variable of ≥19 digits (e.g. 18446744073709551617) makes 10#… evaluate to a small or negative number, the comparison is false, and the unclamped value reaches run-agent.mjs, whose Number() accepts it and arms setTimeout(…, ~1.8e19) — Node's TimeoutOverflowWarning then sets the delay to 1 ms, killing the agent instantly and misclassifying the round as a crash: the exact misreport the comment above ("a misconfigured variable degrades to a warning, not a misreport") claims to prevent. Verified on GNU bash 5.2 (ubuntu-latest): 18446744073709551617, 72000000000000000000, and 9223372036854775808 all pass through unclamped — bash wraps here rather than saturating at INTMAX_MAX. The trigger requires a maintainer to set an absurd (~292-year) timeout, so this is low-likelihood; a one-line length guard closes it.
| BUDGET_CAP_MS=7200000 | |
| if [[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]+$ ]] || (( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS )); then | |
| BUDGET_CAP_MS=7200000 | |
| if [[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]+$ ]] || (( ${#QWEN_TIMEOUT_MS} > 18 )) || (( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS )); then |
中文说明
clamp 的 (( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS )) 会按 2^64 取模回绕,因此 ≥ 2^63 的值会绕过守卫——既不告警也不钳制。失败路径:一个 ≥19 位的仓库变量(如 18446744073709551617)会让 10#… 求值成一个很小或为负的数,比较结果为假,未钳制的值传到 run-agent.mjs,其 Number() 接受该值并设定 setTimeout(…, ~1.8e19)——随后 Node 的 TimeoutOverflowWarning 会把延时设为 1 ms,瞬间杀掉 agent 并把该轮误报为崩溃:正是上方注释(「配置错误的变量会降级为告警,而非误报」)声称要避免的误报。已在 GNU bash 5.2(ubuntu-latest)上验证:18446744073709551617、72000000000000000000、9223372036854775808 全部未被钳制——bash 在这里是回绕,而非在 INTMAX_MAX 处饱和。触发需要维护者设置一个荒谬的(约 292 年)超时,因此概率很低;一行长度守卫即可堵上。
— qwen3.8-max-preview via Qwen Code /review
| # change: the run block clamps it to the 7,200,000 ms ceiling (the | ||
| # fallback itself), so raising the budget still requires editing this | ||
| # default and the step backstop, while a misconfigured variable |
There was a problem hiding this comment.
[Suggestion] This comment states the clamp ceiling is "the fallback itself" (clamp == budget), but no test enforces clamp >= budget — the two can drift apart with a green suite. — Failure scenario: a maintainer following the raise instructions below bumps the fallback 7200000 → 8400000 (140m), the step backstop 130 → 150, and the job cap 300 → 320; every assertion stays green (150 − 140 = 10 ≥ 1, 120 ≤ 149), but at runtime (( 10#8400000 > 7200000 )) clamps the budget back to 120m with only a ::warning::, so the raise silently never takes effect. The raise instructions here (and in the LAST_FIX/HEADLINE strings) name the fallback and the step backstop but not BUDGET_CAP_MS, which is exactly the value that must move too. Add expect(clampMs).toBeGreaterThanOrEqual(budgetMs); next to the other clamp assertions in the test, and name BUDGET_CAP_MS in these instructions.
中文说明
本注释声称 clamp 上限就是「回退值本身」(clamp == budget),但没有任何测试强制 clamp >= budget——两者可能在套件全绿的情况下悄然漂移。失败路径:维护者照下方「调高预算」的说明把回退值 7200000 → 8400000(140 分钟)、step 兜底 130 → 150、job 上限 300 → 320;所有断言依旧绿色(150 − 140 = 10 ≥ 1、120 ≤ 149),但运行时 (( 10#8400000 > 7200000 )) 会把预算重新钳制回 120 分钟,只留下一条 ::warning::,于是这次「调高」悄悄从未生效。这里的调高说明(以及 LAST_FIX/HEADLINE 字符串)点名了回退值与 step 兜底,却没有点名 BUDGET_CAP_MS——而那恰恰是必须一起改动的值。建议在测试中其余 clamp 断言旁补上 expect(clampMs).toBeGreaterThanOrEqual(budgetMs);,并在这些说明里点名 BUDGET_CAP_MS。
— qwen3.8-max-preview via Qwen Code /review
Review —
|
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 4/100 轮)。改动内容与我反驳保留之处如下: Autofix review round summary (PR #8257)Addressed all three inline review findings plus the maintainer's matching suggestions in one commit ( Inline findings (automated reviewer)
Maintainer issue-level review (@wenshao,
|
OverviewTwo related fixes to
I verified the load-bearing claims rather than reading them:
The sum-based invariant test is the right instinct — asserting the numbers individually is exactly what let 153-against-150 pass green, and identifying long steps by what they execute rather than by whether they already carry a bound means a new unbounded gate shows up here. Good. Findings1. The clamp is one-sided, and the uncovered side is the more likely typo — mediumThe new guard clamps only the ceiling. Everything below it passes through untouched. Replaying the actual block from the workflow against
That is precisely the misreport the clamp was added to prevent, arrived at from the other direction. (The Also: Suggested fix, symmetric with the existing one: BUDGET_CAP_MS=7200000
# Floor: below this every round is a guaranteed timeout, and a minutes-shaped
# value in a milliseconds variable (120 -> 120ms) would burn the whole
# TIMEOUT_WINDOW_CAP budget reporting a timeout the maintainer just "fixed".
BUDGET_FLOOR_MS=300000
if [[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]{1,8}$ ]] \
|| (( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS )) \
|| (( 10#${QWEN_TIMEOUT_MS} < BUDGET_FLOOR_MS )); then
echo "::warning::QWEN_TIMEOUT_MS=${QWEN_TIMEOUT_MS} is not an integer in [${BUDGET_FLOOR_MS}, ${BUDGET_CAP_MS}] ms; clamping to ${BUDGET_CAP_MS}"
QWEN_TIMEOUT_MS="${BUDGET_CAP_MS}"
fiThe bash-replay harness in the test already makes this nearly free to pin — add 2. The new 60m gate bound introduces a timeout mode with no marker — lowBounding the gates is right, and the degrade path is correct (verified above). But the resulting round is classified by 3.
|
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
| BUDGET_CAP_MS=7200000 | ||
| if [[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]{1,8}$ ]] || (( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS )); then |
There was a problem hiding this comment.
[Suggestion] The clamp guard's regex ^[0-9]{1,8}$ admits 0 and any all-zero / leading-zero string, so a zero value slips past both the ::warning:: and the "fall back to the same ceiling" guarantee documented in the comment above. — Failure scenario: a maintainer sets repo variable QWEN_AUTOFIX_TIMEOUT_MS=0 → Number('0') is 0 (falsy), so run-agent.mjs:21 (Number(process.env.QWEN_TIMEOUT_MS) || 50 * 60 * 1000) falls back to its own 50-minute internal default, NOT the 7,200,000 ms ceiling the comment promises — a silent 50-minute budget with no warning. Worse, 0100 → Number('0100') = 100 → a 100 ms timer that SIGTERMs the agent on launch, every round, accumulating timeout-exhaustion counts toward the "AutoFix stopped" headline. The round is still correctly reported as a timeout (the agent-timeout marker is written), so this is not a misreport — but it contradicts the invariant this PR exists to make airtight. The executed-clamp test covers '100', '7200000', '9999999', 'abc', '09999999', '9223372036854775808' — every case except the 0 that slips through.
| BUDGET_CAP_MS=7200000 | |
| if [[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]{1,8}$ ]] || (( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS )); then | |
| BUDGET_CAP_MS=7200000 | |
| if [[ ! "${QWEN_TIMEOUT_MS}" =~ ^[1-9][0-9]{0,7}$ ]] || (( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS )); then |
(Rejecting zero / leading-zero in the regex keeps the documented contract and the existing 10# octal guard intact; also add expect(runClamp('0')).toBe('7200000') to the executed-clamp cases.)
中文说明
[建议] 钳制守卫的正则 ^[0-9]{1,8}$ 会放行 0 以及任意全零 / 带前导零的字符串,因此零值会同时绕过 ::warning:: 告警和上方注释所承诺的「回落到同一上限」保证。— 失败场景:维护者把仓库变量设为 QWEN_AUTOFIX_TIMEOUT_MS=0 → Number('0') 为 0(falsy),于是 run-agent.mjs:21(Number(process.env.QWEN_TIMEOUT_MS) || 50 * 60 * 1000)回落到它自己的 50 分钟内部默认值,而不是注释所承诺的 7,200,000 ms 上限——得到一个没有任何告警的静默 50 分钟预算。更糟的是 0100 → Number('0100') = 100 → 一个 100 ms 的计时器,会在每一轮启动时立即 SIGTERM 掉 agent,从而不断累积超时耗尽计数,最终触发「AutoFix stopped」标题。该轮次仍会被正确地上报为超时(agent-timeout 标记会写出),因此这不是误报——但它违背了本 PR 存在意义所在的那条不变量。已执行钳制代码块的测试覆盖了 '100'、'7200000'、'9999999'、'abc'、'09999999'、'9223372036854775808'——唯独漏掉了会溜过去的 0。
(在正则中拒绝零 / 前导零既能守住注释承诺的契约,又不影响现有的 10# 八进制守卫;同时给已执行钳制的用例补一条 expect(runClamp('0')).toBe('7200000')。)
— qwen3.8-max-preview via Qwen Code /review
The review's Finding 1: the guard clamped only the ceiling, and the uncovered side is the likelier typo. Every comment in this file, the PR body and the operator message speak in MINUTES; this one variable wants MILLISECONDS. A maintainer told to "raise the agent time budget" who sets QWEN_AUTOFIX_TIMEOUT_MS=120 armed a 120 ms timer — every round SIGTERMs instantly, writes agent-timeout, and reports "ran out of time (timeout (120ms))" until TIMEOUT_WINDOW_CAP trips and AutoFix stops on the PR, advising the human to raise the budget they just raised. No warning anywhere in that loop, which is the exact misreport the clamp exists to prevent, reached from the other direction. A 60000 ms floor rejects every minutes-shaped value, and it also closes the `0`/`000` hole the review noted alongside it — those passed the bare regex while the message asserted the value had to be positive. The message now names the units, because a units confusion is the whole failure mode. Replayed the review's own table against the extracted block, stdout and stderr separated: 7200000, 3600000 and the floor itself pass untouched; 120, 60, 0, 000 and 59999 all clamp with a warning, alongside the over-cap, malformed, octal and int64 cases the previous round closed. The test pins both boundaries from each side (59999 clamps, 60001 does not) and asserts the warning names MILLISECONDS.
|
Finding 1 addressed at The floor. Your table, replayed against the new block (extracted from the workflow, stdout and stderr separated so the warning is not mistaken for the value — my first pass did exactly that and read every clamped case as 50 min):
The test pins the boundary from both sides — On your other items:
Suite 108/108 at 中文说明Finding 1 已在 下限。 你那张表,在新块上重放(从工作流中抽取,stdout 与 stderr 分离,以免把警告误当成取值——我第一遍恰恰犯了这个错,把每个被截断的用例都读成了 50 分钟):见上表。 测试从两侧钉住边界—— 其余各条: #3(测试建立 套件在 Reviewed with Claude Code (Fable 5). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/tests/qwen-autofix-workflow.test.js:8354
- The test hard-codes that there are exactly 4 “long steps” (run-agent + two verification gates). That makes the invariant brittle: if the workflow later adds another step that runs one of these scripts with a proper timeout, the safety properties still hold but this test will fail just because the count changed.
Prefer asserting a minimum (so missing steps still fail) and then summing whatever long steps exist to enforce the job-cap invariant.
// Primary + repair agent steps and their two verification gates.
expect(longSteps).toHaveLength(4);
.github/workflows/qwen-autofix.yml:3510
- The clamp warning references only
QWEN_TIMEOUT_MS, but operators will typically be editing the repo variableQWEN_AUTOFIX_TIMEOUT_MS. Mentioning the source variable in the warning makes it much easier to diagnose misconfiguration from the Actions log, without changing the behavior.
echo "::warning::QWEN_TIMEOUT_MS=${QWEN_TIMEOUT_MS} is not an integer of MILLISECONDS in [${BUDGET_FLOOR_MS}, ${BUDGET_CAP_MS}] (120 means 120ms, not 120 minutes); clamping to ${BUDGET_CAP_MS}"
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
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round — no new code changeThis round's only actionable feedback was one inline suggestion from the automated reviewer; it is already resolved in the code, so no new commit is needed. Inline suggestion
|
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. The budget math is sound: worst case 280m fits under the 300m job cap (and the 360m runner ceiling). The 10m margin between the 120m budget and 130m step backstop correctly protects the agent-timeout marker write path. Bounding the two verification gates at 60m with continue-on-error: true turns a job-level kill into graceful degrade. The clamp block with floor guard (60000ms) prevents the minutes-vs-milliseconds confusion, and 10# + {1,8} width bound handle octal/overflow edge cases. Test asserts the SUM invariant rather than individual constants, and executes the actual clamp under bash — mutation-tested 7 variants. Well-reasoned scope decision to not build size-based heuristics on 5 data points.
|
Released in v0.21.3. |
Problem
The primary address attempt never stated its own budget, so it took
run-agent.mjs's 50-minute default while the step that wraps it caps at 80. A third of the step was unreachable, and everyAutoFix ran out of time before finishing (timeout (3000000ms))round ended for a reason this workflow does not mention anywhere.Measured on the #8005 round-9 timeout (run 30646547838, job
91211427276), per-step:Triage and addressVerification gateThe setup runs in earlier steps, so it never competed with the agent for the 80 minutes. The step cap was doing nothing except sitting 30 minutes above the number that actually ends a round.
Raising the agent's budget surfaced the larger problem: the round's budget was never checked as a whole. The
Verification gateis the job's largest consumer at 22m48s and had no step bound at all — only the job timeout held it. So the real worst case was7 + 80 + 23 + 20 + 23 = 153against a 150-minute job, and a JOB timeout cancels thealways()reporters, which is exactly the silent round this design exists to prevent. Every assertion in the suite passed while that was true.Change
Sized for the PRs that actually exhaust the budget (47 and 35 changed files), so this does not need revisiting per PR:
QWEN_TIMEOUT_MS)QWEN_AUTOFIX_TIMEOUT_MSoverridesTriage and addressstep capVerification gateRepair verification gatereview-addressjobWorst case is now
7 (setup) + 130 + 60 + 20 (repair) + 60 + 3 (report) = 280against 300, itself under the 360-minute ceilingubuntu-latestimposes regardless.The 10-minute margin under the step cap has a job: the cap is the backstop for a runaway that ignores the internal timer, and if it fires first the internal kill path never writes
agent-timeout— the file the report step reads to tell a timeout apart from a crash. That path only writes the file and exits, which is what the measured 3 seconds were.Bounding the two verification gates is a graceful degrade, not a new way to fail: both already carry
continue-on-error: true, so a bound turns a job-level kill into the ordinary verification-failure path —Finalize verificationsees an empty outcome, falls through itscasetoexit 1, and thealways()report step posts.This buys rounds that were close to finishing, not rounds that were never going to. An exhausted budget still pushes nothing, which is the whole reason the breakers exist, and
TIMEOUT_WINDOW_CAPstill stops a PR after three time-budget exhaustions in one window. #8005 currently carries one.What this deliberately does not do
The timeouts are not obviously size-driven, and I did not build a heuristic on data that thin. Across the five PRs autofix touched in that scan:
#8077 timed out on 186 added lines while #8211 finished with 1333, so line count is not it; file breadth fits better (47 and 35 timed out, 14/11/2 did not). That is a shape in five points, not an established cause — a width-scaled budget would be a knob built on a guess, so this change raises the flat budget and leaves the question open.
Tests
The test derives both numbers from the workflow and asserts the margin between them, rather than restating them as two literals — a pair asserted as two constants cannot catch the pair drifting, which is exactly how they came to be 30 minutes apart.
The invariant asserted is the sum against the job cap, plus the count of bounded steps — asserting the numbers individually is precisely what let 153-against-150 pass.
Verification gatebound removedRepair verification gatebound removedcontinue-on-errordropped from a gateThe fallback behaviour was executed rather than read — the real expression from
run-agent.mjs:21against each input:scripts/tests/qwen-autofix-workflow.test.js107/107 (andqwen-triage-workflow.test.js116/116 unaffected); actionlint, yamllint, eslint--max-warnings 0clean.中文说明
问题
主处理尝试从未声明自己的预算,因此沿用了
run-agent.mjs的 50 分钟默认值,而包裹它的 step 上限是 80 分钟。step 有三分之一永远用不到,并且每一次AutoFix ran out of time before finishing (timeout (3000000ms))的轮次,其真正结束原因在本工作流里根本没有出现过。在同一次扫描中实测(run 30646547838),逐步耗时:
Triage and addressVerification gatesetup 跑在更早的 step 里,因此从未与 agent 争夺那 80 分钟。step 上限除了悬在真正结束轮次的那个数字之上 30 分钟以外,什么也没做。
调高 agent 预算时暴露出更大的问题:这一轮的预算从未被作为整体检查过。
Verification gate是本 job 中最大的消耗项(22分48秒),却完全没有 step 约束——只有 job 上限兜着。于是真实最坏情况是7 + 80 + 23 + 20 + 23 = 153,而 job 上限是 150;且 job 级超时会连always()的上报步骤一起取消,正是本设计要避免的「静默轮次」。在这个事实成立期间,测试套件的每一条断言都是通过的。改动
按真正会耗尽预算的那类 PR(47 与 35 个改动文件)来定尺寸,以免逐个 PR 反复调整:
QWEN_TIMEOUT_MS)QWEN_AUTOFIX_TIMEOUT_MS覆盖Triage and addressstep 上限Verification gateRepair verification gatereview-addressjob最坏情况变为
7(setup)+ 130 + 60 + 20(修复)+ 60 + 3(上报)= 280,对 300;而 300 本身也在ubuntu-latest无论如何都会施加的 360 分钟上限之内。step 上限之下的那 10 分钟余量是有职责的:该上限是用来拦住忽略内部计时器的失控 agent 的兜底,一旦它先触发,内部终止路径就不会写出
agent-timeout——那正是上报步骤用来区分「超时」与「崩溃」的文件。该路径只写这个文件然后退出,实测的那 3 秒就是它。给两个验证门加约束是优雅降级,而不是新增一种失败方式:两者本就带有
continue-on-error: true,因此加上约束会把 job 级的击杀转化为普通的验证失败路径——Finalize verification读到空 outcome,落入其case的兜底分支exit 1,随后always()的上报步骤照常发出。这买到的是那些差一点就能跑完的轮次,而不是那些本来就跑不完的轮次。 预算耗尽依旧什么都推不出去,这正是熔断器存在的理由,
TIMEOUT_WINDOW_CAP仍会在一个窗口内三次预算耗尽后停掉该 PR。#8005 目前是 1 次。本次刻意没有做的事
这些超时并不明显由体量驱动,我不会在这么薄的数据上造启发式。该次扫描中 autofix 处理的五个 PR:
#8077 只新增 186 行却超时,而 #8211 有 1333 行反而跑完,因此不是行数;文件宽度拟合得更好(47 与 35 超时,14/11/2 未超)。但这是五个点上的形状,不是已确立的因果——按宽度缩放的预算会是建立在猜测上的旋钮,所以本次只调高统一预算,把这个问题留着。
测试
断言的不变量是各步上限之和对 job 上限,外加受约束步骤的数量——把这些数字逐个断言,正是让「153 对 150」一路通过的原因。
Verification gate约束Repair verification gate约束continue-on-error回退行为是被执行验证的,不是阅读得出的——用
run-agent.mjs:21的真实表达式跑各输入:scripts/tests/qwen-autofix-workflow.test.js107/107(qwen-triage-workflow.test.js116/116 未受影响);actionlint、yamllint、eslint--max-warnings 0均干净。