fix(autofix): keep a still-red check visible until its head is judged - #7438
Conversation
A red check is a persistent STATE, but the scan only counted checks that failed AFTER the watermark. The moment the watermark passed the failure the PR went quiet while still red. Measured on the live fleet: #6451 watermark 10:55 3 reds completed 09:30, 09:30, 09:51 #7357 watermark 09:18 1 red completed 07:59 #7390 watermark 11:27:37 red completed 11:27:37 — a strict `>` hid it the instant it appeared All three sat red for hours while every scan logged "nothing new", and #6451 wrote two consecutive no-ops whose reasoning never mentions the three failures, because they were not in its feedback at all. A currently-red check now counts as feedback until the head it ran against has been evaluated. The address job records that head in its own `autofix-redcheck` marker — carried inside the eval comment, so no ts/acted/round parser changes and the agent still never sees it as feedback — and the scan skips a PR whose recorded head still matches. That bounds this to ONE look per head rather than every scan, which is what keeps a permanently-red PR from being re-selected forever. The head comes from the REMOTE, not local HEAD: after a rejected push the two differ, and recording a sha that never landed would suppress the reds on the head that actually exists. Empty on failure — matches no marker, so the reds stay visible. Two existing count assertions are replaced by the property they stood for: every check selector in the scan carries the address carve-out.
|
@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 冲突,直到移除标签或达到轮次上限。移除 |
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. |
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with solid evidence. Three real PRs (#6451, #7357, #7390) sat red for hours while every scan logged "nothing new" — the watermark-based counter made them invisible the moment it passed the failure. The #6451 case is particularly clear: two consecutive no-ops whose reasoning only mentions reviews, because the three failures were never in its feedback file. Direction: squarely within the autofix-reliability line. A red check is a persistent state, not an event — treating it as one is the right fix. No CHANGELOG reference needed for CI infrastructure. Size: not applicable — Approach: the scope feels right. Embedding the marker inside the existing eval comment (rather than a new comment type or field on Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到的 bug,证据充分。三个真实 PR(#6451、#7357、#7390)红了数小时,而每次扫描都记录"nothing new"——基于水位线的计数器在水位越过失败的那一刻就让它们不可见了。#6451 的案例尤其清晰:连续两次 no-op,理由只谈评审,因为那三个失败根本不在反馈文件里。 方向:完全在 autofix 可靠性主线内。红检查是持续状态而非事件——按状态处理是正确的修法。CI 基础设施无需 CHANGELOG 引用。 规模:不适用—— 方案:范围合理。将标记嵌入现有 eval 评论(而非新评论类型或 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code ReviewIndependent proposal: given "red checks go invisible once the watermark passes them", I'd track whether the current head has been evaluated for reds, count currently-red checks when it hasn't, gate selection on that count, and record the head so we don't re-select. The PR does exactly this. Findings: no blockers. The implementation is tight:
The test extracts the real decision block from the YAML and runs it with bash across five head/red combinations, then asserts the idle gate, the three marker sites, and the remote-head invariant. Thorough. TestingThis is a CI workflow change ( actionlint not available in this environment; the PR author reports 1.7.12 clean. 中文说明代码审查独立方案: 给定"红检查在水位越过后不可见"的问题,我会追踪当前 head 是否已被评估红检查,未评估时统计当前红检查数,据此门控选择,并记录 head 以避免重选。PR 的做法与此完全一致。 发现: 无阻塞项。实现紧凑:
测试从 YAML 中提取真实判定块并用 bash 运行,覆盖五种 head/红组合,然后断言 idle 门控、三个标记位和远端 head 不变量。全面。 测试这是 CI 工作流变更(
— Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. The problem is real and well-measured: three PRs sat red for hours while the scan reported them idle, because the watermark-based counter treated a persistent state as a one-time event. The fix is the minimal correct thing — count currently-red checks until the head they ran against has been evaluated, then go quiet. No new comment types, no parser changes, no new state to manage. The marker rides inside the existing eval comment, the head comes from the remote (fail-open), and the idle gate is the one line that makes the whole thing work. My independent proposal matched the PR's approach exactly. The test is the best kind: it extracts the real decision block from the YAML and runs it with bash, covers the five meaningful combinations, and explicitly asserts the idle gate — the mutation that the author's own first version missed. The property-based selector assertion is a nice tightening of the existing test. 89/89 tests pass, YAML parses, all 37 中文说明置信度:5/5 —— 每个阶段都干净;毫不犹豫即可合并。 问题是真实且有实测的:三个 PR 红了数小时而扫描报告它们空闲,因为基于水位线的计数器把持续状态当成了一次性事件。修复是最小正确做法——统计当前红检查直到其所在 head 被评估过,然后安静。无新评论类型、无解析器改动、无新状态管理。标记寄生在现有 eval 评论里,head 取自远端(失败方向安全),idle 门控是让整个修复生效的那一行。 我的独立方案与 PR 做法完全一致。测试是最好的那种:从 YAML 提取真实判定块用 bash 运行,覆盖五种有意义的组合,并显式断言 idle 门控——作者自己第一版测试遗漏的变异。基于属性的选择器断言是对现有测试的良好收紧。 89/89 测试通过,YAML 可解析,37 个 — 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 reviewReviewed the diff plus the surrounding scan/report logic. I checked out the head and executed the extracted What it doesCounts a currently-red check as feedback ( Strengths
Findings1. Dead
Suggest deleting the block at 1240–1246 (issue-autofix has no red-check loop to record for) and changing that assertion to 2. A freshly-red check is double-reported in the log (nit). 3. Selection ≠ feedback content (efficacy caveat — acknowledged in the PR). 4. The VerdictCorrect, faithfully mirrors the existing selector convention, and genuinely well-tested. Only #1 is worth acting on before merge (dead code + a wrong test count); #2–#4 are optional/informational. |
Review found the assignment had landed in issue-autofix's "Report dry-run
/ failure" step, which emits no redcheck marker and has no ${PR} in scope
— dead code plus a malformed, swallowed API call. Verifying it surfaced a
second half the review did not state: review-address's OWN handoff step
emits the marker at line 3362 with REPORT_HEAD never assigned in that
step, since shell variables do not cross step boundaries. Neither step
sets `set -u`, so it expanded empty and the marker recorded no head —
fail-open, but the handoff path never recorded one.
Deletes the dead assignment, adds the missing one, and rewords the scan
log so the two overlapping counts no longer read as a sum.
The test now asserts the PAIRING per step block — emits iff defines —
rather than counting each kind. Counting was what let this through: both
counts were "right". The first fix for it keyed the sets by step NAME,
which merged the two identically-named "Report dry-run / failure" steps
and still passed with the bug reintroduced; keying by step block catches
it.
|
Finding 1 confirmed and fixed in What you foundThe assignment had landed in What checking it turned upMapping every assignment and every emission to its (job, step) pair: So Both are fixed: dead assignment deleted, missing one added. Finding 2 (nit) — takenThe two counts genuinely overlap for a freshly-red check on an unjudged head. Reworded so the log no longer reads as a sum: The test lesson, which is the part worth recordingMy assertion counted the two kinds separately ( The first replacement asserted the pairing but keyed the sets by step name. const emits = body.includes('<!-- autofix-redcheck head=${REPORT_HEAD} -->');
const defines = body.includes('REPORT_HEAD="$(gh api');
expect(emits).toBe(defines);Mutation-verified by mechanically moving the assignment back into 90/90; 中文说明第 1 条已确认并在 你发现的赋值落在了 核实后追加发现的把每一处赋值与每一处发出按 (job, step) 配对后(见上方英文代码块): 两处均已修:删掉死赋值、补上缺失的那个。 第 2 条(nit)已采纳刚变红且 head 未被评估的检查确实会被两个计数同时统计。日志措辞已改为不再读作相加: 真正值得记录的是测试教训我的断言把两类分别计数(3 个 marker、2 处赋值)。代码是错的,而两个计数都是对的 —— 这正是它能上线的原因。 我的第一版改法虽然改成了断言配对,但用步骤名作键;而 变异验证方式:用脚本把赋值机械地挪回 90/90; |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: Review feedback addressedFinding 1 — Dead
|
1d7c65d
doudouOUC
left a comment
There was a problem hiding this comment.
Three blocking state-transition issues remain on the current head; inline details below.
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: You are review agent 4 — Agent 4: Performance & efficie..., You are review agent 5 — Agent 5: Test coverage. — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.
[Critical] Blocker 1 still stands (verified, high confidence) — .github/workflows/qwen-autofix.yml:2629: The prepare-step "Failed checks" renderer still filters by completedAt > watermark. When N_RED_NOW > 0 selects a PR with still-red checks, the agent receives an empty "Failed checks" section because the red checks completed before the watermark. The scan correctly selects the PR, but the agent has no check name/status to reproduce. Failure: PR #6451 scenario — 3 red checks completed 09:30-09:51, watermark 10:55 → agent sees empty feedback → posts no-op → PR stays red.
[Critical] Blocker 2 still stands (verified, low confidence — requires external push during agent work) — .github/workflows/qwen-autofix.yml:2948: REPORT_HEAD is fetched via gh api at the start of "Push and report" step, after checkout and agent work (up to 80 min gap). A push between checkout and this API call captures a different head than the one actually evaluated. Concurrency group serializes autofix runs but not external manual pushes.
[Critical] Blocker 3 still stands (verified, high confidence) — .github/workflows/qwen-autofix.yml:3365: The handoff step unconditionally emits <!-- autofix-redcheck head=${REPORT_HEAD} --> even on sentinel/retry handoffs (MARK_TS='9999-12-31T23:59:59Z'). This makes RED_HEAD == LIVE_HEAD on the next scan, N_RED_NOW = 0, and the PR goes idle — defeating the sentinel retry mechanism. The test validates emitter/definer pairing but does NOT test suppression on sentinel handoffs.
— qwen3.7-max via Qwen Code /review
…ck tracking (#7438) - Forward persistent red checks into agent feedback: the scan selects via N_RED_NOW but the prepare renderer only showed checks that failed AFTER the watermark, leaving the agent with an empty Failed checks section. Add a Still-red checks section with the complement filter. - Omit the redcheck marker on sentinel/retry handoffs: a sentinel ts means the agent evaluated nothing, so recording a judged head would suppress the retry the handoff promises. - Record the checked-out head, not the report-time remote head: capture the SHA in prepare before agent mutations and forward it as a step output, so a mid-run branch move cannot stamp an unevaluated head as judged.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] .github/workflows/qwen-autofix.yml:2628 — Still-red checks not forwarded into agent feedback: the Failed checks renderer still filters by completedAt > $wm, so a PR selected solely via N_RED_NOW > 0 (old red checks) produces an empty ## Failed checks section. The agent receives no check names/statuses to reproduce, writes a no-op report that records RED_HEAD=LIVE_HEAD, and silences the PR on the next scan — the exact scenario this PR targets recurs after one wasted round.
— qwen3.7-max via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: Feedback point 1 — Forward persistent reds into agent feedback [rc:3624064607]Decision: Implemented. Root cause: The scan selects a PR via Change: Added a "Still-red checks (persisting from before the last evaluation)" section to the feedback renderer, immediately after "Failed checks". It uses the complement filter ( Test: New test Feedback point 2 — Do not mark retry handoffs as having judged the red head [rc:3624064614]Decision: Implemented. Root cause: The handoff step unconditionally wrote Change: Wrapped the redcheck marker emission in Test: Assertion that Feedback point 3 — Record the head that was actually evaluated [rc:3624064623]Decision: Implemented. Root cause: Change: Captured Test: Assertions that prepare captures and outputs the SHA, that no report step calls Verification
中文说明反馈点 1 — 将持续红色检查转发到 agent 反馈中 [rc:3624064607]决定: 已实现。 根因: 扫描通过 变更: 在反馈渲染器中,紧接 "Failed checks" 之后添加了 "Still-red checks (persisting from before the last evaluation)" 部分。它使用互补过滤器( 测试: 新测试 反馈点 2 — 不要将重试交接标记为已评判红色 head [rc:3624064614]决定: 已实现。 根因: 交接步骤无条件写入 变更: 将 redcheck 标记的发出包裹在 测试: 断言 反馈点 3 — 记录实际被评判的 head [rc:3624064623]决定: 已实现。 根因: 变更: 在 prepare 步骤中,checkout 之后立即(在任何 agent 变更之前)捕获 测试: 断言 prepare 捕获并输出 SHA,没有报告步骤调用 验证
Base-conflict check: no conflict with main. Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. 🧠 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.
— qwen3.7-max via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: Review feedback addressedCritical findings (already fixed in prior commits on this branch)All three Critical findings raised by both @doudouOUC and @qwen-code-ci-bot were already resolved by commits
Suggestion: empty-LIVE_HEAD fail-closed test (rc:3624803904)Implemented. Added two behavioral assertions to the
These lock in the ConflictNo conflict ( Verification
中文说明已处理的审查反馈Critical 发现(已在本分支的先前提交中修复)@doudouOUC 和 @qwen-code-ci-bot 提出的三个 Critical 发现均已在本轮审查之前由提交
建议:空 LIVE_HEAD 失败关闭测试(rc:3624803904)已实现。 在
这些断言锁定了 冲突无冲突( 验证
Base-conflict check: no conflict with main. Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed the exact current head. I found one non-blocking concurrency gap; details and a focused regression expectation are inline.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
…ale gate (#7438) Two near-simultaneous scans can both enqueue the same PR with the same watermark. When the first serialized job ends in a no-op, it records a redcheck marker for the head it judged but leaves both the eval timestamp and the round UNCHANGED — so the live-watermark/round revalidation never fires, and the second job re-runs the agent and posts a duplicate report for the same head. Parse the latest live redcheck marker during prepare (mirroring the scan's RED_HEAD parse) and add its head match against CHECKED_OUT_HEAD as a third stale-duplicate signature, reusing the existing "nothing newer" revalidation so newer feedback or a live conflict still keeps the target actionable.
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: Autofix review feedback addressedAddressed the one inline finding from @doudouOUC's review of the current head. No conflict was reported ( Feedback point
Decision: implemented. This is a real concurrency gap. The existing stale-duplicate gate triggers only when a sibling advanced the live watermark ( Changes
Verification
中文说明已处理的 Autofix 评审反馈处理了 @doudouOUC 针对当前 head 的评审中的一条行内意见。未报告冲突( 反馈点
决定:已实现。 这是一个真实的并发缺口。现有的过期重复门只在 sibling 推进了 live watermark( 改动
验证
Base-conflict check: no conflict with main. Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
ytahdn
left a comment
There was a problem hiding this comment.
LGTM. 增量 review 无 Critical 缺陷。
核心确认:
- N_RED_NOW 逻辑:
LIVE_HEAD空 → fail-closed(不选择);RED_HEAD == LIVE_HEAD→ 已评估(不选择);否则统计当前红色 check。jq 过滤正确排除 Qwen Autofix workflow - Redcheck marker:
REPORT_HEAD从CHECKED_OUT_HEAD(prepare 中 mutation 前捕获);空 head → marker 不匹配正则 → fail-open(reds 保持可见);sentinel ts 不记录(agent 未评估,下次重试) - Stale signature (c):
LIVE_RED_HEAD == CHECKED_OUT_HEAD检测 no-op 同 head 重复,与 (a)(b) OR 组合;空CHECKED_OUT_HEAD不误标 stale - Still-red section:
<= $wm与 Failed checks 的> $wm互补,相同 conclusion 过滤 + carve-out - Recording/scan 不对称性:recording fail-open(空 head → reds 可见),scan fail-closed(空 head → 不选择),有意为之且正确
- 测试覆盖全面:行为测试 + 结构断言 + per-step 配对 + stale signature (c) 4 种场景
— qwen3.7-plus via Qwen Code /review
What this PR does
Makes a still-failing check visible to the scan, instead of only at the instant it turned red — while keeping it to one look per head so a permanently-red PR is not re-selected forever.
Why it's needed
The scan counts a failed check as feedback only when it
completedAt > watermark. But a red check is a persistent state, not an event: once the watermark passes the failure, the PR is red and invisible at the same time.Measured on the live fleet:
10:55:1609:30:00/09:30:12/09:51:0509:18:0607:59:4611:27:3711:27:37— equal, and the test is a strict>09:22:56All three sat red for hours while every scan logged
✅ nothing new. #6451 wrote two consecutive no-ops whose reasoning mentions reviews only — correctly, because the three failures were never in its feedback file. Its log holds a one-command fix:How
A currently-red check counts as feedback until the head it ran against has been evaluated:
headRefOidand compares it with the head recorded on the PR.<!-- autofix-redcheck head=… -->marker, emitted at all three report sites (pushed / no-action / handoff).The marker is separate from
autofix-evalon purpose: it rides inside the same comment, so nots/acted/roundparser changes, and the comment still matches the existing bot-comment filter — the agent never sees it as feedback.The head comes from the remote, not local HEAD. After a rejected push (three happened today: #7262, #7355, #7395) the two differ, and recording a sha that never landed would suppress the reds on the head that actually exists. On failure it is empty, which matches no marker — so the reds stay visible. Fail-open.
Why this does not churn: the recorded head only changes when someone pushes. A permanently-red PR with no new commits is examined exactly once and then goes quiet again, which is the behaviour today minus the initial blind spot.
Reviewer Test Plan
How to verify
npx vitest run scripts/tests/qwen-autofix-workflow.test.js— 90/90 (a run in three hits the pre-existing load flakes; see Risk). The new test replays the real extracted decision block:abc123abc123abc123old999abc123abc123abc123Mutation-verified, four reverts each turning it red:
git rev-parse HEADN_RED_NOWfrom the idle gateThe last one was not caught by my first version of the test; every other assertion passed without the gate. It is the reason the test now extracts and asserts the gate itself.
Static, run locally with the exact CI toolchain:
js-yamlparses; all 37run:blocks passbash -n; actionlint 1.7.12 clean; prettier clean.Post-merge smoke: the next scan should select refactor(cli): rewrite Fleet View to match Claude Code agent view UI #6451 with
… + 3 still-red on <sha>, and the scan after that should leave it alone.Evidence (Before & After)
Tested on
Risk & Scope
CANCELLEDis deliberately excluded from the still-red set although the watermark-based counter includes it: a cancelled check is usually a superseded run (41% ofreview-prruns are cancelled by concurrency), and treating that as persistent feedback would wake the loop on ordinary churn.eligibility recheck,takeover-command toggleandclassifies permanent API failuresare subprocess load flakes — an earlier controlled A/B showed unmodifiedmainfailing the same ones at a comparable rate, and all pass in isolation.main's files and re-running), so log-forwarding is a separate question, not a prerequisite.autofix-redcheckmarker simply get their first look.Linked Issues
Found by tracking why three managed PRs stayed red for hours while the scan reported them idle. Part of the autofix-reliability line: #7330, #7350, #7351, #7354, #7392, #7396, #7412, #7416.
中文说明
本 PR 做了什么
让仍然红着的检查对扫描可见,而不是只在它刚变红的那一瞬间可见;同时限定"每个 head 只看一次",避免长期红的 PR 被反复选中。
为什么需要
扫描只在
completedAt > 水位线时把失败检查计为反馈。但红检查是持续状态而非事件:水位线一旦越过该失败,PR 就同时处于"红着"和"不可见"。实测(见上方英文表格):#6451 水位线 10:55,3 个红完成于 09:30/09:30/09:51;#7357 水位线 09:18,红完成于 07:59;#7390 水位线与红检查完全相等(11:27:37),而判定用严格
>,因此它诞生即不可见。对照组 #6506 无红,也无此问题。三者都红了数小时,而每次扫描都记录
✅ nothing new。#6451 连写两次 no-op,理由只谈评审 —— 这是正确的,因为那三个失败根本不在它的反馈文件里。而它的日志里躺着一条命令就能修的问题(见上方英文代码块)。怎么做
当前为红的检查会一直计为反馈,直到它所在的 head 被评估过:扫描读取实时
headRefOid与 PR 上记录的 head 比较;address 作业在三个报告出口(已推送 / 无需改动 / 交接)都写下<!-- autofix-redcheck head=… -->。该标记刻意独立于
autofix-eval,但寄生在同一条评论里:因此ts/acted/round的解析器一律不动,评论也仍然命中既有的 bot 评论过滤器 —— agent 永远不会把它当成反馈。head 取自远端而非本地 HEAD。 推送被拒时两者不同(今天就发生了三次:#7262、#7355、#7395),记录一个从未落地的 sha 会把真实 head 上的红压掉。取值失败时为空,匹配不上任何标记,红保持可见 —— 失败方向是安全的。
为什么不会空转:记录的 head 只在有人推送时改变。长期红且无新提交的 PR 只会被检查一次,随后重新安静。
评审验证
npx vitest run scripts/tests/qwen-autofix-workflow.test.js—— 90/90。新测试回放真实提取的判定块,覆盖五种组合(见上方英文表格)。git rev-parse HEAD(推送被拒时压掉真实 head 的红)、少写一处标记(该路径静默不记录)、以及从 idle 判定里去掉N_RED_NOW(算出来却不用 —— 正是整个 bug)。最后这一条没有被我第一版测试抓到:其余断言在缺少该判定时全部照过。这正是测试现在直接提取并断言该判定本身的原因。run:块过bash -n;actionlint 1.7.12 clean;prettier clean。… + 3 still-red on <sha>选中 refactor(cli): rewrite Fleet View to match Claude Code agent view UI #6451,再下一次则应重新安静。风险与范围
CANCELLED刻意不计入"仍然红"的集合,尽管基于水位线的旧计数包含它:被取消的检查通常是被取代的运行(review-pr有 41% 因 concurrency 被取消),把它当作持续反馈会让循环被日常抖动唤醒。eligibility recheck、takeover-command toggle、classifies permanent API failures是子进程负载 flake;此前受控 A/B 显示未修改的main以相当频率红同样用例,单独跑均通过。main的文件重跑,诊断出 17 个测试失败),因此"喂日志"是另一个独立问题,而非本改动的前提。autofix-redcheck标记的 PR 只是获得它的第一次检查。关联 Issue
在追查"三个托管 PR 红了数小时而扫描一直报 idle"时发现。属 autofix 可靠性主线:#7330、#7350、#7351、#7354、#7392、#7396、#7412、#7416。