feat(ci): auto-open a deflake fix issue for confirmed flaky tests - #7231
Conversation
The CI Failure Patrol reruns flaky failures but never fixes them, so
the same tests flake forever on a rerun treadmill. This closes the
loop: when the patrol classifies a rerun as a nondeterministic TEST
(not infra), it now also opens ONE deflake issue that the existing
autofix issue pipeline develops into a reviewable stabilization PR.
- ci-flaky-patrol SKILL: a rerun decision whose cause is a specific
named flaky test carries an optional flakyTest {file, name}; infra
reruns (ENOSPC, network, runner death) never do.
- ci-flaky-rerun.mjs: validates flakyTest (malformed → the whole
decision is rejected, so a bad classification can't open a bogus
issue); after a rerun, ensureDeflakeIssue upserts a deflake issue
deduped by a stable (file, name) marker — one open issue per flaky
test across all PRs — labeled status/ready-for-agent + autofix/
approved so the scheduled autofix scan picks it up.
- .qwen/skills/deflake/SKILL.md: constrains the fix to four
assertion-preserving patterns (raise timeout/poll budget, stabilize
timing/waiting, make randomness/time deterministic, isolate
interference) and forbids skipping/deleting/loosening the check;
write failure.md if none applies or the failure looks like a real
bug. The produced PR is reviewable, never auto-merged.
Tests: deflakeKey stability/collision-freedom, the bilingual issue
body, one-issue-per-test dedup, no issue for infra reruns, and
malformed-flakyTest rejection. 34/34 across both patrol suites.
|
Thanks for the PR! Template looks good ✓ Problem: observed operational gap — flaky tests rerun forever on a treadmill without ever being fixed. The PR links related work (#7230, #7229) that reduces flakes; this closes the loop on the ones that remain. Real problem, clearly evidenced by the CI patrol's own existence. Direction: aligned. CI self-healing is a natural extension of the existing patrol + autofix infrastructure. No auth/sandbox/model/public-contract concerns. CHANGELOG has no direct reference but the area is clearly relevant — the patrol and autofix systems are established project infrastructure. Size: not applicable (no core module paths touched — Approach: the scope feels right. Reuses both existing systems (patrol classification → autofix issue pipeline) with minimal new surface. The four allowed fix patterns in the deflake SKILL are well-constrained, and the "write failure.md and stop" escape hatch is the right call. Dedup by content hash is clean. Best-effort issue creation (try/catch, retry on next occurrence) is the correct failure mode. No unrelated changes or scope creep. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到的运维缺口——flaky 测试永远在重跑跑步机上,从不被修复。PR 关联了 #7230、#7229(减少 flaky),本 PR 闭合剩余 flaky 的环路。问题真实,CI patrol 的存在本身就是证据。 方向:对齐。CI 自愈是现有 patrol + autofix 基建的自然延伸。无 auth/sandbox/model/公共契约顾虑。 规模:不适用(未触及核心模块路径)。 方案:范围合理。复用两个现有系统,新增面极小。deflake SKILL 的四类修法约束得当,"写 failure.md 停下"的逃生口正确。内容哈希去重干净。尽力创建 issue(try/catch,下次重试)是正确的失败模式。无无关改动。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code ReviewIndependent proposal: I'd extend the patrol decision schema with an optional Comparison with the diff: the PR matches this almost exactly. The implementation is clean and well-structured:
No critical blockers. No AGENTS.md violations. The code follows the existing file's style and conventions. Real-Scenario TestingThis is CI infrastructure (not a user-facing CLI feature), so the real-scenario test exercises the exported functions directly and runs the full test suite: All 38 tests pass. The 中文说明代码审查独立方案: 扩展 patrol 决策 schema 加可选 与 diff 对比: PR 几乎完全匹配。实现干净、结构良好。无关键阻塞项,无 AGENTS.md 违规。 真实场景测试CI 基建(非用户可见 CLI 功能),直接调用导出函数 + 跑完整测试套件。38/38 通过,语法检查通过,backtick 注入防护验证通过。 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 — clean across every stage; tight, well-tested CI automation that closes a real operational gap. This is exactly the kind of PR I like to see: a real problem (flaky tests rerun forever), a minimal solution that reuses existing infrastructure (patrol + autofix pipeline), and strong safety properties (malformed input rejects the deflake path without dropping the rerun, dedup prevents issue spam, the resulting PR is human-reviewed). The deflake SKILL's constraint to four assertion-preserving patterns with a The implementation matches my independent proposal almost line-for-line. The null-separator in the hash key, the backtick stripping for issue body safety, the best-effort try/catch — these are the details that show the author thought about failure modes. 38 tests cover every seam including the adversarial ones (backtick injection, over-length fields, creation failure). Ships it. ✅ 中文说明置信度:5/5 —— 每个阶段都干净;紧凑、充分测试的 CI 自动化,闭合真实运维缺口。 问题真实(flaky 永远重跑),方案最小化复用现有基建(patrol + autofix 流水线),安全属性强(畸形输入不丢重跑、去重防刷屏、产出 PR 经人评审)。deflake SKILL 约束在四类保留断言的修法 + failure.md 逃生口,设计正确。实现与独立方案几乎逐行匹配。38 个测试覆盖每个接缝包括对抗性场景。 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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. |
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
…arkup injection Addresses the two Criticals + suggestions on #7231: - **Critical: a malformed/over-length flakyTest no longer kills the rerun.** flakyTest validation is removed from validDecision (which gated the PRIMARY action on secondary metadata — a >200-char nested test name or a null silently dropped a valid rerun). Well-formedness is now checked in ensureDeflakeIssue, which simply skips the deflake issue when the metadata is bad; the rerun always stands. - **Critical: markup/mention injection via the test path/name.** file and name are code-span-stripped of backticks (which cannot be escaped inside a span and would break out into live Markdown, turning into a mention in a bot-created issue) and both now sit in code spans. safeReason alone did not close this (it does not touch backticks). - Best-effort deflake: ensureDeflakeIssue is wrapped in try/catch so a transient createIssue failure — after the marker is already posted — no longer surfaces as a misleading "skipping PR" and permanently suppresses the deflake; it retries on the next flaky occurrence. - Run link uses the patrol's own repo (client.repo) instead of the dead target.repo, so deflake issues on a fork don't 404. - Body reworded: it no longer claims the rerun already passed (it runs right after the rerun is triggered) — it says a real deterministic failure is NOT flakiness and must not be stabilized. - SKILL: bound file/name to 200 chars, and note a malformed one is ignored (never drops the rerun). Tests: malformed flakyTest keeps rerun (no createIssue); long title truncates ≤240; backtick path/name cannot inject; run link honors the repo; a throwing createIssue leaves the rerun intact. 38/38.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
| const title = | ||
| `deflake: ${decision.flakyTest.file} \u203a ${decision.flakyTest.name}`.slice( | ||
| 0, | ||
| 240, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The title is truncated with UTF-16 code-unit .slice(0, 240), which can split an astral-plane character (e.g. an emoji in a verbatim test name — the patrol SKILL says to take the name "verbatim from the log"). When such a character straddles code-unit index 240 of the composed string, the slice keeps the high surrogate and drops the low one; Node then encodes the lone surrogate as U+FFFD, so the created issue gets a garbled (replacement-character) title. — Concrete cost: a flaky test whose file is near the 200-char cap and whose name has an emoji straddling index 240 yields a deflake: … title ending in a replacement character instead of the real test name. Rare trigger, cosmetic harm — the dedup marker lives in the body (built from the un-sliced values), so dedup is unaffected. The same UTF-16 root cause also makes wellFormedFlakyTest's .length <= 200 reject a name that is ≤200 code points but >200 code units.
| const title = | |
| `deflake: ${decision.flakyTest.file} \u203a ${decision.flakyTest.name}`.slice( | |
| 0, | |
| 240, | |
| ); | |
| const title = Array.from( | |
| `deflake: ${decision.flakyTest.file} \u203a ${decision.flakyTest.name}`, | |
| ) | |
| .slice(0, 240) | |
| .join(''); |
— qwen3.8-max-preview via Qwen Code /review
| '--search', | ||
| `${marker} in:body`, |
There was a problem hiding this comment.
[Suggestion] hasOpenIssueWithMarker interpolates the entire HTML-comment marker (<!-- qwen-deflake key=<hex> -->) into the in:body search, diverging from this file's proven dedup pattern — prsWithMarkers() searches the bare distinctive token (in:comments qwen-ci-flaky-rerun), not a full punctuation-laden HTML comment. — Concrete cost: GitHub's search tokenizer strips/splits punctuation tokens (<!--, -->, key=), so the implicit AND-of-terms can resolve to zero hits even when a matching issue exists; dedup then silently always returns false and every rerun of a still-flaky test opens a fresh status/ready-for-agent / autofix/approved issue (each dispatching the autofix pipeline → duplicate competing PRs). The current unit tests mock this method, so they cannot catch a query that never matches. Search bare distinctive tokens instead — mirroring prsWithMarkers — while keeping the per-test hex key for specificity, e.g. qwen-deflake <hexKey> in:body (both qwen-deflake and the hex key are substrings of the body's <!-- qwen-deflake key=<hexKey> --> marker).
— qwen3.8-max-preview via Qwen Code /review
| '--label', | ||
| labels.join(','), |
There was a problem hiding this comment.
[Suggestion] The two new GhClient wrappers — hasOpenIssueWithMarker (the gh issue list --search … --limit 1 call + JSON.parse(output).length > 0 reduction) and createIssue (this --title/--body/--label labels.join(',') arg construction) — have no unit test. Every deflake test replaces them with the client() mock, so the real arg-building and output-parsing is never exercised. — Concrete cost: a wrong search qualifier (dedup silently always-false → a fresh deflake issue on every flaky rerun) or a labels.join(',') regression (deflake issues the autofix loop never picks up) would ship with all tests green. This file already unit-tests other GhClient methods by stubbing api.gh (comments, currentPr) — mirror that, e.g.:
const api = new GhClient('QwenLM/qwen-code');
let args;
api.gh = async (a) => { args = a; return '[]'; };
await api.hasOpenIssueWithMarker('<!-- qwen-deflake key=x -->');
expect(args[args.indexOf('--search') + 1]).toBe('<!-- qwen-deflake key=x --> in:body');
// plus a createIssue case asserting args has '--label' then 'status/ready-for-agent,autofix/approved'— qwen3.8-max-preview via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.20.1. |
|
Agent run timed out after 1800000ms ❌ failed |
What this PR does
Closes the flaky-test loop. Today the CI Failure Patrol (
qwen-ci-flaky-rerun.yml) reruns flaky failures but never fixes them, so the same tests flake forever on a rerun treadmill. This makes a confirmed flaky test automatically produce a reviewable stabilization PR via the existing autofix issue pipeline.Flow (reuses both systems, minimal new surface):
flakyTest { file, name }— infra reruns (ENOSPC, network, runner death) never do.ensureDeflakeIssue(inci-flaky-rerun.mjs, called after a rerun) opens one deflake issue per flaky test, deduped by a stable(file, name)marker — one open issue across all PRs, never a spam of duplicates — labeledstatus/ready-for-agent+autofix/approvedso the scheduled autofix scan develops it..qwen/skills/deflake/SKILL.mdthat constrains the change to four assertion-preserving patterns and produces a reviewable PR (never auto-merged).Design decisions (as requested)
failure.mdand stops for a human.flakyTestrejects the whole patrol decision (a bad classification can never open a bogus issue); the deflake PR is reviewed by a human before merge; dedup guarantees one issue per test.Reviewer Test Plan
How to verify
npx vitest run scripts/tests/ci-flaky-rerun.test.js scripts/tests/ci-flaky-rerun-workflow.test.js— 34/34. New behavioral coverage:deflakeKeyis stable and collision-free across(file, name); the marker format is pinned.createIssue(correct title/labels/marker); an already-open issue → deduped (rerun, no second issue).flakyTest) → never opens an issue.flakyTest(empty/missing/oversized/wrong-action) → the decision is rejected, no action at all.node --check .github/scripts/ci-flaky-rerun.mjs; prettier clean on all four files.Evidence (Before & After)
Tested on
Risk & Scope
flakyTeston concrete per-test evidence, malformed fields reject the decision, and the resulting PR is human-reviewed (never auto-merged). Dedup caps it at one issue per test.flakyTestis optional and additive; existing patrol behavior (rerun/comment/no_action) is unchanged when it is absent.Linked Issues
Builds on the flake work in #7230 (root-cause timeout fixes) and #7229 (crash no longer strands a PR): those reduce the flakes; this fixes the ones that remain instead of rerunning them.
中文说明
本 PR 做了什么
闭合 flaky 测试的环路。现在 CI Failure Patrol 只重跑 flaky,不修复,于是同样的测试永远在重跑跑步机上。本 PR 让已确认的 flaky 测试自动产出一个可评审的稳化 PR,复用现有 autofix issue 流水线。
流程(复用两个系统,新增面很小):
flakyTest { file, name };基建重跑(ENOSPC、网络、runner 崩)不带。ensureDeflakeIssue(在ci-flaky-rerun.mjs,重跑后调用)按稳定的(file, name)marker 去重,为每个 flaky 测试开一个 deflake issue(跨所有 PR 只一个,不刷屏),打上status/ready-for-agent+autofix/approved,由定时 autofix 扫描开发。.qwen/skills/deflake/SKILL.md修复,约束在四类保留断言的修法,产出可评审的 PR(绝不自动合并)。设计取舍(按你的选择)
failure.md交人工。flakyTest畸形则整条 patrol 决策作废(错误分类开不出假 issue);deflake PR 合并前经人评审;去重保证每个测试一个 issue。评审验证
npx vitest run scripts/tests/ci-flaky-rerun.test.js scripts/tests/ci-flaky-rerun-workflow.test.js—— 34/34。新增行为覆盖:deflakeKey稳定且不碰撞、marker 格式钉死;双语 issue 正文含 marker/SKILL 指针/文件/run 链接/折叠中文;确认 flaky → 重跑且恰好一次createIssue(标题/标签/marker 正确),已开则去重(重跑但无第二个 issue);基建重跑(无flakyTest)不开 issue;畸形flakyTest→ 决策作废、零动作。node --check;四个文件 prettier 干净。风险与范围
flakyTest、畸形字段作废决策、产出的 PR 经人评审(绝不自动合并);去重上限为每测试一个 issue。flakyTest可选且只增;缺省时既有 patrol 行为不变。关联 Issue
承接 #7230(根因超时修复)与 #7229(崩溃不再卡死 PR):那两个减少 flaky,本 PR 修掉剩下的,而非重跑它们。