fix(ci): cap E2E lanes and auto-retry transient main failures once - #10229
Closed
yiliang114 wants to merge 1 commit into
Closed
fix(ci): cap E2E lanes and auto-retry transient main failures once#10229yiliang114 wants to merge 1 commit into
yiliang114 wants to merge 1 commit into
Conversation
The post-merge E2E lanes hit a live model endpoint, and when connectivity to it flakes the lanes do not fail fast: every test burns its full 300s timeout (plus retries), so the 2026-08-26 run spent over 4 hours before reporting, and recent main runs (a82a11a, 84c21b8) went red on 'connect ETIMEDOUT' to the endpoint. The identical tree passed on a re-run (run 32966197395 attempt 2), confirming flake rather than regression — the same pattern behind the recurring 'Main CI failed: E2E Tests' issues. - Cap every lane at 60 minutes (healthy lanes finish in ~17) so a hung endpoint surfaces within the hour instead of holding a runner. - Add an 'E2E Flaky Rerun' watcher that re-runs the failed jobs of a first-attempt push failure exactly once, using CI_BOT_PAT from a job that checks out nothing and runs no repository code. A failure that survives the re-run stays red and still raises the Main CI Failure issue. - Pin both behaviours in workflow tests; record the new workflow in the size baseline.
Collaborator
yiliang114
marked this pull request as draft
August 27, 2026 03:28
Collaborator
|
Thanks for the PR! Template: substantively complete — what / why / test plan / risk are all there — but the body uses shorthand headings ( Problem: observed and verified, not theoretical. Confirmed via the API: run Direction: aligned. This extends to post-merge E2E what the repo already does for PR lanes with Size: +127/−0 across 5 files, no core paths — 58 lines of workflow YAML and 69 lines of test pins. Under every threshold. Approach: the cap + one-shot watcher is the right minimal shape for this. Two questions worth settling before merge:
Risk: no elevated signals — no high-risk paths matched, and the watcher's only inputs are numeric GitHub-set IDs. Moving on to code review. 🔍 中文说明感谢贡献！ 模板：内容实质齐全——What/Why/测试计划/风险都有——但正文使用了缩写标题（ 问题：已观测并核实，不是理论问题。通过 API 确认：run 方向：对齐。这是把仓库在 PR 侧已经用 规模：5 个文件 +127/−0，不触及核心路径——58 行工作流 YAML、69 行测试钉。远低于所有阈值。 方案：超时上限 + 一次性重跑 watcher 是正确的最小形态。合并前值得确认两个问题：
风险：无升级信号——未命中高风险路径，watcher 的输入只有 GitHub 生成的数字 ID。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewIndependent baseline first: for "a lane that loses connectivity to the model endpoint burns 4 hours and leaves What I verified in the diff:
Nits (non-blocking): the cap pin asserts Still open for the author (from Stage 1): the Test evidenceUnattended CI run — the evidence below is the PR's own CI on At post time the completed checks are green and Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The one thing CI cannot demonstrate here is the post-merge trigger itself — the watcher only fires on 中文说明代码审查先给独立基线：针对"失去到模型端点连接的 lane 会烧掉 4 小时并把 main 留红、且没有重试"这个问题，我的方案是：给每个 lane 加 在 diff 中核实到：
小问题（非阻塞）：上限钉只断言 留给作者确认的问题（承接 Stage 1）： 测试证据无人值守 CI 运行——以下证据是 PR 自身在 发布评论时，已完成的检查均为绿色； CI 在这里唯一无法证明的是合并后的触发本身——watcher 只在 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — clean, minimal, well-hardened fix for a thoroughly evidenced pain; the open questions are about follow-up scope, not about this diff. Stepping back: the problem is real and was verified against the API before any code was read — attempt 1 of run It is strictly an improvement over today: even in the pessimistic reading of my Stage 1 question — timeout-killed lanes conclude CI on this commit is still running (the ubuntu unit lane of 中文说明回顾整体：问题是真实的，在读任何代码之前就已通过 API 核实——run 这严格优于现状：即使按 Stage 1 问题的悲观解读——超时被杀的 lane 结论为 该提交上的 CI 仍在运行（ — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
tmux E2E test report (head
|
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
Aug 27, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/e2e-flaky-rerun.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
— qwen3.8-max via Qwen Code /review (v0.22.2)
Comment on lines
+6
to
+7
| # green again; a failure that survives both attempts stays red and still | ||
| # raises the Main CI Failure issue. |
Collaborator
There was a problem hiding this comment.
[Critical] The Main CI Failure issue bot races this watcher, so a first-attempt flake still files and autofix-routes a "Main CI failed: E2E Tests" issue even though the re-run heals it. main-ci-failure-issue.yml subscribes to the same workflow_run completion, and its analyze guard checks only conclusion == 'failure' && head_branch == 'main' && event == 'push' — run_attempt appears nowhere in that file (verified at this commit). On a push to main where one shard flakes with connect ETIMEDOUT — the exact incident this header cites (run 32966197395) — the single attempt-1 completion event fires both watchers at once: this one starts attempt 2, while the issue bot downloads the attempt-1 logs, files the issue, applies type/bug + status/ready-for-agent + autofix/approved, and assigns the autofix bot. When attempt 2 turns green ~20-40 minutes later, nothing closes or de-routes the issue (the bot only acts on conclusion == 'failure', and no workflow in .github/ closes issues), so an autofix agent is dispatched at a healthy tree — on every absorbed flake. This comment says only "a failure that survives both attempts" raises the issue; that is false of the wiring as it stands, and before this change a filed issue at least coincided with a red main.
Witness (live state observed at this commit):
run 32966197395 → conclusion: success, run_attempt: 2, event: push, name: "E2E Tests"
issue 10186 → state: OPEN, labels: [type/bug, status/ready-for-agent], assignee: qwen-code-dev-bot
Coordinate the two responders: gate the issue bot's analyze path on the retried attempt for E2E push runs — e.g. append && (github.event.workflow_run.name != 'E2E Tests' || github.event.workflow_run.run_attempt > 1) in main-ci-failure-issue.yml, leaving SDK Python and scheduled "Qwen Code CI" untouched since neither has a rerun watcher — and decide the fallback explicitly for the case where this watcher itself never fires (expired PAT, API outage), so an attempt-1 failure is not silently unfiled. If first-attempt filing is intended to stay, correct this comment and the "reaches the issue bot untouched" note instead. main-ci-failure-issue.yml is outside this diff, so a linked follow-up is fine. If the gate is added, extend scripts/tests/main-ci-failure-issue-workflow.test.js to pin the new attempt-exclusion clause — removing the clause must turn that test red.
— qwen3.8-max via Qwen Code /review (v0.22.2)
Comment on lines
+31
to
+32
| // push is the only event that earns a retry. Pin the whole clause so a | ||
| // connective mutation fails here. |
Collaborator
There was a problem hiding this comment.
[Suggestion] This comment promises that a connective mutation fails here, but the five toContain substring checks below cannot catch one: flipping any && in the guard to || keeps every substring present, so the suite stays green. Verified by mutation at this commit — flipping the last connective to head_branch == 'main' || run_attempt == 1 left the suite 3/3 green, while the guard then parses as (A && B && C && D) || E (GitHub's && binds tighter than ||) and fires on any run with run_attempt == 1 — green runs, PR-branch runs, scheduled runs — letting the watcher re-trigger itself on every failed completion and defeating the "exactly once" invariant this test is named for. Pin the whole expression exactly instead (the |- block scalar parses to this string verbatim):
expect(job.if).toBe(
"${{ github.repository == 'QwenLM/qwen-code' && github.event.workflow_run.conclusion == 'failure' && github.event.workflow_run.event == 'push' && github.event.workflow_run.head_branch == 'main' && github.event.workflow_run.run_attempt == 1 }}",
);Acceptance check: with the exact match in place, flip any && to || in e2e-flaky-rerun.yml's guard and confirm "retries only first-attempt failed push runs, exactly once" goes red.
— qwen3.8-max via Qwen Code /review (v0.22.2)
| 'web-shell-browser-regression', | ||
| ]; | ||
| for (const lane of lanes) { | ||
| expect(yml.jobs[lane]['timeout-minutes'], lane).toBeGreaterThan(0); |
Collaborator
There was a problem hiding this comment.
[Suggestion] toBeGreaterThan(0) pins only that a cap exists, not the 60-minute value this PR sets — the comment above says "every lane's timeout is pinned", but mutating timeout-minutes: 60 to any positive value (say 240) passes this test. Verified by mutation at this commit: bumping e2e-test-linux to 240 left the suite green, while the documented property — a hung endpoint surfaces within the hour, the 2026-08-26 failure mode this PR exists to remove — silently degrades. Only deleting the key entirely is caught today.
Suggested change
| expect(yml.jobs[lane]['timeout-minutes'], lane).toBeGreaterThan(0); | |
| expect(yml.jobs[lane]['timeout-minutes'], lane).toBe(60); |
Acceptance check: with the pin tightened, change any lane's timeout-minutes in e2e.yml and confirm "caps every lane so a hung model endpoint cannot stall the run" goes red.
— qwen3.8-max via Qwen Code /review (v0.22.2)
Comment on lines
+41
to
+46
| const lanes = [ | ||
| 'e2e-test-linux', | ||
| 'e2e-test-macos', | ||
| 'isolated-nightly', | ||
| 'web-shell-browser-regression', | ||
| ]; |
Collaborator
There was a problem hiding this comment.
[Suggestion] The pin hand-enumerates today's four job ids, so it silently stops covering any lane added later. Verified by mutation at this commit: appending a fifth job to e2e.yml without timeout-minutes left the suite green. That is not speculative — e2e.yml's own header talks about promoting lanes to merge_group once a stable subset is carved out — and an uncapped future lane would reintroduce the hours-long stall this PR exists to prevent while this test stays green. Quantify over the workflow's jobs instead:
for (const [lane, job] of Object.entries(yml.jobs)) {
expect(job['timeout-minutes'], lane).toBe(60);
}(allow named exceptions explicitly if a future job legitimately needs no cap). Acceptance check: with the quantified loop, delete timeout-minutes from any lane — current or future — and confirm the test goes red.
— qwen3.8-max via Qwen Code /review (v0.22.2)
Comment on lines
+30
to
+32
| # Deliberately hosted, NOT the ECS pool — same reasoning as | ||
| # main-ci-failure-issue.yml#file_issue: a recovery job must not queue | ||
| # behind the infrastructure it is recovering from. |
Collaborator
There was a problem hiding this comment.
[Suggestion] The deliberate runs-on: 'ubuntu-latest' choice is asserted only in this comment — the new test pins the trigger, the guard, and the PAT usage, but not runs-on, so a sweep moving the job to the repo's ECS pool passes every test. Verified by mutation at this commit: swapping in the repo's own ECS fromJSON expression kept the suite 3/3 green, and that expression appears in 44 places across other workflows, so such a sweep is plausible. If the ECS pool is the reason the E2E run failed, the recovery job queues behind the dead pool it is recovering from, the re-run never fires, and main stays red with nothing reporting the watcher's stall. Pin the choice in scripts/tests/e2e-flaky-rerun-workflow.test.js:
expect(job['runs-on']).toBe('ubuntu-latest');Acceptance check: with that assertion in place, change runs-on in this workflow and confirm the suite goes red.
— qwen3.8-max via Qwen Code /review (v0.22.2)
| ); | ||
| expect(job.if).toContain("github.event.workflow_run.event == 'push'"); | ||
| expect(job.if).toContain("github.event.workflow_run.head_branch == 'main'"); | ||
| expect(job.if).toContain('github.event.workflow_run.run_attempt == 1'); |
Collaborator
There was a problem hiding this comment.
[Suggestion] This pin is a prefix substring, so a value mutation to run_attempt == 1<digit> keeps the test green while silently killing the retry. Verified by mutation at this commit: changing the guard to run_attempt == 10 left the suite 3/3 green — and since run_attempt starts at 1 and only increments on re-runs, == 10 never matches, so the auto-rerun this PR adds dies silently (e.g. after a stray edit or autofix typo) with the suite green and no other signal. The four sibling checks above are terminated by a closing quote; this one is not. The exact-match fix proposed in the other finding on this test subsumes this hole, or pin the clause on its own:
Suggested change
| expect(job.if).toContain('github.event.workflow_run.run_attempt == 1'); | |
| expect(job.if).toMatch(/github\.event\.workflow_run\.run_attempt == 1(?![0-9])/); |
Acceptance check: with the tightened pin, mutate the workflow guard to run_attempt == 10 and confirm "retries only first-attempt failed push runs, exactly once" goes red.
— qwen3.8-max via Qwen Code /review (v0.22.2)
Collaborator
Author
|
Superseded by #10085, which moves the failing Linux E2E lanes to the ecs-qwen pool (root cause: hosted runners cannot reliably reach the OPENAI_BASE_URL endpoint — see #10242) and carries the same 60-minute cap. Branch kept in case the hosted-only lane caps or the one-shot flaky-rerun watcher are wanted later. |
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
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Post-merge E2E on
mainhas been red repeatedly (e.g. #10224, #10186, and the recurring "Main CI failed: E2E Tests" issues).Bisecting the recent failures shows they are not code regressions:
Run 32966197395 (2026-08-26, commit 5a88324) failed a Linux shard on attempt 1 and passed the identical tree on attempt 2 (~10 min per shard).
Runs 32990071899 (a82a11a) and 33024847833 (84c21b8) failed random
sandbox:noneshards with hundreds of 300 s test timeouts, all downstream of:i.e. intermittent TCP connectivity loss from GitHub-hosted runners to the model endpoint behind
OPENAI_BASE_URL. The docker-sandbox and macOS lanes of the same runs stayed green because they finished before the degradation window.Two compounding problems: a lane that hits this burns every test's full 300 s timeout (plus retries) — the 2026-08-26 run took over 4 hours to fail — and there is nothing that retries a flake, so one connectivity blip leaves
mainred and files an autofix issue.What
timeout-minutes: 60). Healthy lanes finish in ~17 min, so the cap only bites when something is already wrong; a hung endpoint now surfaces within the hour instead of holding a runner half a day.E2E Flaky Rerunwatcher workflow (e2e-flaky-rerun.yml): onworkflow_runcompletion ofE2E Tests, if a first-attempt push run onmainfailed, re-run its failed jobs exactly once viagh run rerun --failedwithCI_BOT_PAT(same convention asqwen-ci-flaky-rerun.yml).run_attempt == 1guard makes it idempotent: the attempt-2 completion never re-triggers it.tscbreak) fail the re-run too and remain visible.main-ci-failure-issue.yml#file_issue), stays hosted rather than on the ECS pool so it never queues behind the infrastructure it recovers from.scripts/tests/e2e-workflow.test.jsgains a timeout pin for all four lanes; newscripts/tests/e2e-flaky-rerun-workflow.test.jspins the trigger, the one-shot guard, and the PAT/no-checkout contract. New workflow recorded in.size-baseline;e2e.ymlgrowth (+529 B) stays within the 4 KB allowance.Verification
node scripts/lint.js --setup --actionlint— clean;yamllint— clean on both files.npx vitest run scripts/tests/e2e-workflow.test.js scripts/tests/e2e-flaky-rerun-workflow.test.js— 6/6 pass.npx vitest run scripts/tests/workflow-size.test.js scripts/tests/main-ci-failure-issue-workflow.test.js— all pass.npx prettier --check— clean.Not addressed here
The root cause (hosted-runner → model-endpoint connectivity) is infrastructure-side; if it persists, moving the Linux E2E lanes onto the
ecs-qwenpool the way #10055 moved the autofix scan lane is the follow-up to consider. This PR bounds the blast radius and makes the suite self-heal single flakes either way.