feat(review): retry transient API failures once; surface quota clearly - #7233
Merged
Conversation
The PR-review workflow failed permanently on any single API hiccup — a 502/503, a dropped connection, a rate limit — with only a fallback comment, so an idle PR sat without a review until someone re-ran `@qwen-code /review` by hand. Now the qwen invocation runs in a budget-guarded retry loop: - A transient outcome (non-quota API error, dropped/aborted run, empty output, error result) retries ONCE after a 60s backoff. - All attempts SHARE the review timeout budget, and a retry is capped at 5 minutes — a cleared transient succeeds fast, and a still-failing retry can't burn another hour (the observed quota run took 61 min, so an unbounded second attempt could blow the job timeout). - A quota-exhausted 429 is NOT retried in-run: its reset is typically hours out. It fails with kind=quota and the fallback comment now states the reset time and how to re-run once it resets — a clear recovery instead of a dead end. - A real timeout or a hard/config failure never retries (unchanged). Detection is exactly as before; only the disposition (retry / quota kind) is new. Behavioral test spawns the extracted loop under bash with a scripted stub qwen: success→1 try, transient→2 then success, persistent-transient→2 then fail, quota→1 try + quota kind + reset time, error-result→retry, hard-exit→no retry. 7/7.
Collaborator
|
Re-run on the new head Template looks good ✓ Problem: Observed, unchanged. The review on #7220 died on a quota 429 and nothing retried — the PR sat without a review until a manual re-trigger. A real operational gap, not a theoretical one. Direction: Aligned. Making the CI review workflow resilient to transient API failures is straightforward infrastructure reliability, squarely in scope. The transient-vs-quota split is the right call — burning a runner waiting on a quota reset that's hours away helps nobody. Size: Not applicable (no core paths — workflow YAML + test files only). Approach: Still minimal and focused. Since the last pass the fix commit hardened exactly the spots flagged in review: the quota-detail Moving on to code review. 🔍 中文说明基于新 head 模板完整 ✓ 问题：已观测到，结论不变。#7220 的 review 因配额 429 失败后无人重试，PR 一直无人 review 直到手动重触发。是真实的运维缺口，而非理论性问题。 方向：对齐。让 CI review 工作流对瞬时 API 失败具备弹性是纯粹的基础设施可靠性改进，完全在范围内。瞬时与配额的区分是正确的——占着 runner 等数小时后的配额重置毫无意义。 规模：不适用（未触及核心路径——仅工作流 YAML + 测试文件）。 方案：仍然最小且聚焦。自上一轮以来，修复提交恰好加固了 review 中指出的位置：配额详情 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
Code ReviewIndependent proposal: extract the existing detection logic into a function, wrap it in a budget-guarded loop (max 2 attempts, shared timeout budget), classify outcomes (transient → retry, quota → fail fast with reset info, timeout/hard → no retry), and test by driving the real bash with stubs. Comparison: The PR matches this almost exactly, and the fix commit
No critical blockers, no convention violations. The Real-Scenario TestingThis is a CI workflow change (GitHub Actions YAML), not user-facing CLI behavior — it can't be driven via Every disposition matches the PR's claims: clean success and hard/timeout/quota failures are not retried; transients (API error, aborted error-result) retry once; quota fails fast with The PR's own suite agrees — it extracts and runs the same real bash: (The retry test grew from 7 to 10 scenarios with the hardening commit.) YAML parses cleanly. 中文说明代码审查独立方案： 将现有检测逻辑提取为函数，用受预算约束的循环包裹（最多 2 次尝试、共享超时预算），分类结果（瞬时 → 重试，配额 → 快速失败并给出重置信息，超时/硬失败 → 不重试），并用 stub 驱动真实 bash 测试。 对比： PR 几乎完全匹配，且修复提交 真实场景测试这是 CI 工作流变更，无法用 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — Clean across every stage; the review feedback was addressed point-by-point and the behavior holds up when the real bash is run. Stepping back: this is one real problem (the #7220 review died on a quota 429 and nothing retried) met by one focused solution (retry transients once, surface quota clearly), with a test that exercises the actual production bash rather than a mock. My independent proposal matched the PR almost line-for-line, so there's no simpler path being missed. What changed since the last pass is the important part. @doudouOUC's review caught a genuine defect — the unguarded quota-detail The budget-sharing design remains the right call (two attempts can never exceed the single-review budget; the 5-minute cap stops a still-failing retry from burning another hour), and the quota-vs-transient split is pragmatic. A human maintainer has already re-reviewed this exact commit and approved. If I had to maintain this in six months, I'd thank the author — well-commented, self-documenting tests, logic you can reason about at a glance. Approving, pinned to the reviewed commit. 中文说明置信度：5/5 — 每个阶段都干净；review 意见已逐条处理，运行真实 bash 验证行为成立。 退一步看：一个真实问题（#7220 的 review 因配额 429 死亡且无人重试）对应一个聚焦方案（瞬时重试一次、配额清晰呈现），测试验证的是实际生产 bash 而非 mock。我的独立方案与 PR 几乎逐行一致，因此不存在被忽略的更简路径。 自上一轮以来最关键的变化：@doudouOUC 的 review 抓到了一个真实缺陷——未加保护的配额详情 预算共享设计依旧正确（两次尝试永不超过单次 review 预算；5 分钟上限防止仍失败的重试再耗一小时），配额与瞬时的区分务实。一位人类维护者已对正是这个提交复审并批准。六个月后维护这段代码，我会感谢作者——注释良好、测试自文档化、逻辑一目了然。 批准，锚定到所审查的提交。 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
requested changes
Jul 19, 2026
doudouOUC
requested changes
Jul 19, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Review Summary
整体设计合理：将单次 qwen 调用重构为 budget-guarded retry loop，区分 transient / quota / timeout / fatal 四类结果，只对 transient 重试一次。共享 budget、quota 不重试、5min cap 等决策都合理。测试直接提取 YAML 中的 bash 执行，保证了真实性。
但有一个 Critical 问题导致 CI 红，需修复后再合并。
Critical: 破坏了现有测试 qwen-resolve-workflow.test.js
CI 失败原因：scripts/tests/qwen-resolve-workflow.test.js 第 244-246 行断言 workflow 中包含：
expect(runStep).toContain(
'fail "Qwen review timed out after ${QWEN_TIMEOUT} minutes." 1 "timeout"',
);本 PR 将该行重构进了 run_review_once() 函数，变成了 REASON="Qwen review timed out after ${QWEN_TIMEOUT} minutes."，而 fail 调用移到了 retry loop 末尾的 fail "$REASON" 1 "$KIND"。旧字符串不再存在，测试必然失败。
修复：更新 qwen-resolve-workflow.test.js 中的断言，改为匹配新的代码结构（如 toContain('REASON="Qwen review timed out after ${QWEN_TIMEOUT} minutes."') 和 toContain("OUTCOME='timeout'")）。
Suggestion: Timeout REASON 在 retry 场景下措辞不精确
run_review_once 中 timeout 的 REASON 始终引用 ${QWEN_TIMEOUT} minutes。如果第一次尝试 transient 失败，retry 被 cap 在 5 分钟后超时，用户看到的消息仍是 "timed out after 180 minutes"（假设 QWEN_TIMEOUT=180），实际 retry 只跑了 5 分钟。建议改为引用实际的 attempt_timeout。
Suggestion: 新测试文件缺少 license header
对比 qwen-autofix-workflow.test.js 等文件，新文件缺少 Apache-2.0 license header。
Suggestion: Quota 检测 grep -qiE 'quota' 可能误匹配
如果未来 API 错误消息中包含 "quota" 但不是配额耗尽（如 "quota configuration error"），会被错误分类为 quota 而不重试。可以考虑更精确的匹配如 grep -qiE 'quota.*(exhaust|exceed|limit|reset)'。
Suggestion: retryLoopSource() 提取逻辑较脆弱
run.lastIndexOf('\ndone') 如果 workflow 中 done 关键字出现在其他位置会出错。建议用更精确的锚点。
doudouOUC
reviewed
Jul 19, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
补充：将上述 review 的各条发现锚定到具体代码行，方便逐条处理。
wenshao
commented
Jul 19, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
— qwen3.8-max-preview via Qwen Code /review
…den the retry tests Review follow-through on #7233's two Criticals and every suggestion: - The quota-detail grep ran unguarded in an assignment under `set -euo pipefail`: a 429 whose message lacks "reset at" exited the step before fail() wrote failure_kind, so the quota-aware fallback never fired. Guarded with `|| true`, and a quota_noreset scenario reproduces the exact message shape that died. - qwen-resolve-workflow.test.js still asserted the pre-refactor literal `fail "… ${QWEN_TIMEOUT} minutes." 1 "timeout"`; it now pins the OUTCOME/REASON pattern the loop actually uses. - The timeout REASON reported the total budget even when a 5-minute retry cap fired; it now names the attempt's own timeout beside the budget, so the fallback's --timeout advice matches what actually expired. - Quota matching tightened to quota-plus-context (exhaust/exceed/limit/ reset) so a transient "quota configuration" style error keeps its retry; the new test file uses the root `yaml` dependency instead of a hoisted js-yaml, carries the license header, anchors the loop extraction on the retry-budget comment instead of lastIndexOf, and gains the two uncovered scenarios: a real timeout is not retried, and an attempt with under 30s of budget never starts.
doudouOUC
approved these changes
Jul 19, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Re-review of 543cd45e — LGTM ✅
复审基于当前 HEAD 543cd45e（不是评论锚定的旧 commit），并独立用真实 bash 跑了一遍 retry loop 验证行为，而非只读 diff。上一轮的全部 Critical 与 Suggestion 均已正确处理。
逐条核对
| 来源 | 问题 | 修复 | 状态 |
|---|---|---|---|
| Critical | 超时断言重构后 qwen-resolve-workflow.test.js 失效致 CI 红 |
断言改为匹配 OUTCOME='timeout' + 新 REASON |
✅ CI Test (ubuntu) 已转绿 |
| Critical | quota 消息无 reset at 时，grep 在 set -euo pipefail 下会中止脚本，fail()/quota fallback 永不触发 |
detail grep 补 || true |
✅ 已独立验证 |
| Suggestion | 超时文案不精确 | 改为 ${attempt_timeout} seconds (of the ${QWEN_TIMEOUT}-minute budget) |
✅ |
| Suggestion | grep 'quota' 误匹配 |
收紧为 quota.*(exhaust|exceed|limit|reset) |
✅ |
| Suggestion | 新测试文件缺 license header | 已补 Apache-2.0 header | ✅ |
| Suggestion | retryLoopSource() 的 lastIndexOf('\ndone') 脆弱 |
改为从 # Retry budget: 锚点起定位 done |
✅ |
| Suggestion | js-yaml 非根依赖（靠 hoisting） |
换成根依赖 yaml 的 parse |
✅ |
| Suggestion | timeout / budget-guard 路径无测试 | 新增 timeout_kill 与 QWEN_TIMEOUT=0 回归用例 |
✅ |
独立验证（真实 bash 跑当前 loop）
success attempts=1 → success 干净成功不重试
transient_then_success attempts=2 → success transient 重试一次即成功
quota (含 reset) attempts=1 → FAIL kind=[quota] (reset…) 不重试，reset 时间已透出
quota_noreset attempts=1 → FAIL kind=[quota] 关键：未被 set -e 杀掉
timeout_kill attempts=0 → FAIL kind=[timeout] 超时不重试
budget=0 attempts=0 → FAIL "ran out of budget" 预算护栏生效
六个场景行为均符合预期，两个 Critical 确认已根除。
唯一遗留（Trivial，可忽略）
超时消息在首次尝试时会显示 timed out after 10800 seconds (of the 180-minute budget)——10800 秒即 180 分钟，略显冗余（仅重试尝试会显示 300s）。纯文案，不影响正确性，无需再改。
这轮迭代质量很高：不仅修复了两个 Critical，还主动补齐了回归测试与依赖稳健性。无新增阻塞问题，LGTM。
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jul 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Collaborator
|
Released in v0.20.1. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Makes the PR-review workflow auto-recover from transient model-API failures instead of dying on the first hiccup, and turns a quota exhaustion from a dead end into a clear, actionable recovery.
Today a single API blip — a 502/503, a dropped connection, a rate limit, an aborted run — fails the review permanently; the only output is a fallback comment. An idle PR then sits without a review until someone re-runs
@qwen-code /reviewby hand. (This is what happened on #7220: the model returned[API Error: 429 … quota exhausted …], the review failed, and nothing re-tried.)How it works
The qwen invocation now runs in a budget-guarded retry loop:
kind=quota, and the fallback comment now states the reset time and exactly how to re-run once it resets.Failure detection is identical to before (same
result-event inspection, same messages); only the disposition — retry vs. quota-aware fail — is new.Reviewer Test Plan
How to verify
npx vitest run scripts/tests/qwen-pr-review-workflow.test.js— 7/7. The test extracts the real retry loop from the workflow and runs it underbashwith a scripted stubqwen(+ stubtimeout/sleep), asserting outcome and attempt count:kind=quota+ reset timePlus a pin that the fallback comment is quota-aware.
YAML parses; every
run:block passesbash -n.Evidence (Before & After)
Tested on
Risk & Scope
retryableclassification only adds a retry when budget remains; with no budget it falls through to the same failure as today.@qwen-code /reviewafter it resets (or a future scheduled re-trigger) — this PR makes that recovery legible, it does not automate the multi-hour wait. Live end-to-end needs a real transient in CI; the loop is fully replayed in the test.quotafailure kind is additive.Linked Issues
Motivated by the stuck review on #7220 (model quota 429). Same "transient failure should self-heal" theme as #7229 (autofix crash no longer strands a PR) and #7230/#7231 (flaky tests).
中文说明
本 PR 做了什么
让 PR review 工作流对模型 API 的瞬时失败自动恢复,而不是一撞就死;并把配额耗尽从死胡同变成清晰、可执行的恢复路径。
现在任何一次 API 抖动(502/503、连接断开、限流、run 中断)都会让 review 永久失败,只留一条 fallback 评论;静止的 PR 就一直没有 review,直到有人手动
@qwen-code /review。#7220 正是如此:模型返回[API Error: 429 … quota exhausted …],review 失败,没有任何重试。实现方式
qwen 调用现在跑在受预算约束的重试循环里:
kind=quota失败,fallback 评论现在会写出重置时间与重跑方式。失败检测与之前完全一致;只有处置(重试 / quota 感知)是新的。
评审验证
npx vitest run scripts/tests/qwen-pr-review-workflow.test.js—— 7/7。测试从工作流里抽出真实重试循环,在bash下用脚本化的假qwen(+假timeout/sleep)驱动,断言结果与尝试次数(见上表);另有 fallback quota 感知的钉串。run:块通过bash -n。风险与范围
@qwen-code /review(或未来的定时重触发)—— 本 PR 让这条恢复路径清晰可见,但不自动化数小时的等待。quota失败类别只增不改。关联 Issue
由 #7220(模型配额 429 卡住 review)驱动。与 #7229(autofix 崩溃不再卡死 PR)、#7230/#7231(flaky 测试)同一"瞬时失败应自愈"主题。