ci(autofix): show a live-progress status comment while a round runs - #7738
Conversation
Takeover engages, and then the PR thread goes quiet: review-address runs the agent for up to 80 minutes plus a verification gate, but nothing reaches the thread until "Push and report" at the very end. Observed on #7731 — 43 minutes of silence with no way to tell a working round from a stuck one. The agent's output already streams live to the Actions log; only the link was missing. Announce the round up front with that link, and flip the same comment to a terminal state when the round ends. Upserted by marker so one comment per PR is edited each round (edits notify nobody) instead of stacking against a 100-round cap. Both steps are gated on the stale-duplicate flag: the per-PR concurrency group runs a discarded duplicate AFTER the real round finalised, so an ungated finalize would overwrite that round's "finished" with its own "ended without publishing". Best-effort throughout — a failed status post warns and never costs a round.
|
@qwen-code /takeover |
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
🤝 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 冲突,直到移除标签或达到轮次上限。移除 |
|
Thanks for the PR! Template looks good ✓ — all major sections present with thorough detail. Problem: observed and well-evidenced. The PR cites concrete timestamps from #7731 — takeover engaged at 01:55, Direction: aligned. This is the takeover-loop counterpart of #7654 (which added the same visibility for Size: not applicable — no core module paths ( Approach: the scope feels right. Two additive steps (post + finalize), both best-effort, both gated on the stale-duplicate flag, upsert by marker to avoid comment stacking. The PR explicitly documents what was deliberately not done (per-phase agent-driven status line) and why. Every edit in the diff serves the stated goal — no unrelated changes, no drive-by refactors. I don't see a materially simpler path. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ —— 所有主要章节齐全,内容详实。 问题:已观测且有充分证据。PR 引用了 #7731 的具体时间戳——接管于 01:55 启动, 方向:对齐。这是 #7654(为 规模:不适用——未触及核心模块路径。两个改动文件均为 CI 基础设施。 方案:范围合理。两个新增步骤(发布 + 定稿),均尽力而为,均 gate 了 stale 标志,按 marker upsert 避免评论堆积。PR 明确记录了"特意没做"的内容及原因。diff 中每项改动都服务于既定目标——无无关改动、无顺手重构。没有看到更简路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: I would add two steps to Comparison with the diff: the PR's approach matches this almost exactly. No simpler path missed. No critical blockers found. Specific observations:
No AGENTS.md violations. The change follows existing patterns in the workflow file (same CI Test EvidenceThe main test suite ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Real-Scenario TestingN/A — this is a CI workflow change ( 中文说明代码审查独立方案: 在 与 diff 对比: PR 方案与上述几乎完全一致,未发现更简路径。 未发现关键阻塞项。具体观察:
无 AGENTS.md 违规。改动遵循 workflow 文件中的既有模式。 CI 测试证据主测试套件( 真实场景测试不适用——这是 CI workflow 变更,非用户可见 TUI 变更。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is a tight, well-motivated change. The problem is concrete (43 minutes of thread silence on #7731, with a 120-minute ceiling), the solution is the minimum that works (two additive steps, no agent cooperation needed), and the implementation follows every existing pattern in the workflow file. The stale-gate race condition was caught in self-audit and pinned by a regression test — that's exactly the kind of defensive thinking this workflow needs. The "deliberately not done" section is as useful as the code itself: it shows the author considered the per-phase status line and rejected it for the right reason (the Actions log is already finer-grained, and a mid-round agent is the worst time to rely on agent-maintained state). The test coverage is thorough — the new test case pins every load-bearing property (marker, URL, gates, PATCH-only, best-effort, outcome branching), and the stale-gate count assertion was updated to catch regressions. The mutation-verification the author reports (each mutation killing exactly the intended test) is the right way to validate that the tests actually pin behavior rather than coincidentally matching strings. If I had to maintain this in six months, I'd thank the author. The comments in the YAML explain why each property is load-bearing, not just what the code does. The best-effort design means a status-comment failure never blocks a round. The PATCH-only finalize means a round that never announced can't gain a phantom comment. Approval deferred until CI lands green on 中文说明置信度:5/5 —— 每个阶段均干净;毫不犹豫可以合并。 这是一个紧凑、动机充分的改动。问题是具体的(#7731 上线程静默 43 分钟,上限 120 分钟),方案是最小可行解(两个新增步骤,无需 agent 配合),实现遵循 workflow 文件中的所有既有模式。stale 门控竞态在自审中发现并由回归测试钉住——这正是此 workflow 所需的防御性思维。"特意没做"部分与代码本身同样有价值:作者考虑了逐阶段状态行并因正确理由否决(Actions 日志已更细粒度,轮次中段是最不适合依赖 agent 维护状态的时刻)。 测试覆盖充分——新测试钉住每个承重性质(marker、URL、门控、仅 PATCH、尽力而为、结果分支),stale 门控计数断言已更新以捕获回归。作者报告的变异验证(每个变异恰好杀死目标测试)是验证测试真正钉住行为的正确方式。 如果六个月后由我维护,我会感谢作者。YAML 中的注释解释了每个性质为什么是承重的,而非仅描述代码做了什么。尽力而为设计意味着状态评论失败绝不阻塞轮次。仅 PATCH 的 finalize 意味着从未发过状态的轮次不会凭空获得幻影评论。 审批推迟至 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
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.
LGTM, looks ready to ship — CI landed green after the review. ✅
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
| STATUS_ID="$(gh api "repos/${REPO}/issues/${PR}/comments" --paginate | | ||
| jq -rs --arg m "${MARKER}" --arg ab "${AUTOFIX_BOT}" \ | ||
| '[ .[][] | select((.user.login // "") == $ab) | ||
| | select((.body // "") | contains($m)) ] | last | .id // empty')" || | ||
| STATUS_ID='' | ||
| if [[ -n "${STATUS_ID}" ]]; then | ||
| gh api --method PATCH "repos/${REPO}/issues/comments/${STATUS_ID}" \ | ||
| -f body="${BODY}" > /dev/null || | ||
| echo "::warning::Failed to update the autofix status comment on PR #${PR}; continuing." | ||
| else | ||
| gh api "repos/${REPO}/issues/${PR}/comments" -f body="${BODY}" > /dev/null || | ||
| echo "::warning::Failed to post the autofix status comment on PR #${PR}; continuing." | ||
| fi |
There was a problem hiding this comment.
[Suggestion] Both "Post" and "Finalize" steps independently fetch all PR comments (gh api --paginate) and scan for the same marker-based comment ID. On a heavily-managed PR with hundreds of comments across up to 100 rounds, this paginated scan runs twice per round for no additional benefit — the Post step already has the answer when its shell exits.
The workflow already passes step outputs via $GITHUB_OUTPUT extensively (37 existing uses). Consider giving this step an id: and writing STATUS_ID to $GITHUB_OUTPUT in both branches (existing-comment PATCH and new-comment POST). The Finalize step can then check ${{ steps.post_status.outputs.comment_id }} first, falling back to the full scan only when that output is empty (which covers the always() case where Post was skipped).
Concrete cost: two paginated API round-trips per round on every managed PR.
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Implemented — and taken one step further than suggested.
You are right that the two scans are redundant: the announcement either finds the id or creates it, so it now writes comment_id to $GITHUB_OUTPUT (the POST branch captures a freshly created id via --jq '.id'), and the finalize consumes it.
Where I deviated: the fallback scan is removed outright, not kept. An empty id means this round never announced — its step was skipped, or the post itself failed. In that case no comment claims this round is working, so there is nothing to flip: a previous round's comment is already in a terminal state, and the next round's announcement re-PATCHes it regardless. Keeping a fallback would add a second code path that only ever runs when there is nothing for it to do. So the finalize got shorter, not just cheaper — one scan per round instead of two, and less code.
Pinned by the existing test: comment_id handoff present in both branches, and --paginate asserted absent from the finalize. Mutation-verified — cutting the handoff, or reintroducing a scan in the finalize, each turns that test red.
中文说明
已实现,并且比建议更进一步。
两次扫描确实冗余:公告步骤要么找到 id、要么刚创建了它,因此现在把 comment_id 写入 $GITHUB_OUTPUT(POST 分支用 --jq '.id' 捕获新建 id),finalize 直接消费。
偏离之处:fallback 扫描被彻底删除,而非保留。 id 为空意味着本轮从未公告过(步骤被跳过,或发布本身失败)。此时没有任何评论声称本轮在运行,也就无可翻转:上一轮的评论已是终态,而下一轮的公告无论如何都会重新 PATCH 它。保留 fallback 只会多出一条"仅在无事可做时才执行"的代码路径。所以 finalize 变得更短,而不只是更省 —— 每轮一次扫描而非两次,代码也更少。
已由现有测试钉住:两个分支的 comment_id 交接,以及断言 finalize 中不含 --paginate。变异验证:切断交接、或在 finalize 中重新引入扫描,都会让该测试变红。
Addresses review feedback: the announcement and the finalize each ran their own paginated comment scan, twice per round on a PR that can accumulate hundreds of comments over 100 rounds. The announcement already knows the id — it either found one or just created one — so it now writes it to $GITHUB_OUTPUT (capturing the id of a freshly posted comment via --jq) and the finalize consumes that. The finalize's scan is removed outright rather than kept as a fallback: an empty id means this round never announced, so no comment claims the round is working, there is nothing to flip, and a previous round's comment is already terminal (the next round's announcement re-PATCHes it either way). One scan per round instead of two, and less code.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
中文说明
— qwen3.7-max via Qwen Code /review
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.7-max via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressed[rc:3651875449] Assert finalize step's error-handling guards — ImplementedThe post step's best-effort guards ( [rc:3651875452] Assert finalize step's
|
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: Blocked: cannot reproduce or attribute the CI failure to this PRFeedback this roundNo review comments, inline comments, or issue comments. The only feedback is two
Why this is not attributable to the PR changeThe PR is additive and touches only two files:
It changes no package source, no dependency manifest/lockfile, and no settings Local verification (all PR-attributable checks pass)Ran from the repository root at HEAD
Run log: https://github.com/QwenLM/qwen-code/actions/runs/30190816314 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.7-max via Qwen Code /review
| BODY="$(printf '%s\n\n🔄 **AutoFix is working on this PR** — round %s/%s. [Watch live progress](%s); this round posts its report here when it finishes.\n\n<details>\n<summary>中文说明</summary>\n\n🔄 **AutoFix 正在处理此 PR** —— 第 %s/%s 轮。[查看实时进度](%s);本轮结束后会在此发布报告。\n\n</details>' \ | ||
| "${MARKER}" "${ROUND_DISPLAY}" "${MAX_ROUNDS}" "${RUN_URL}" \ | ||
| "${ROUND_DISPLAY}" "${MAX_ROUNDS}" "${RUN_URL}")" | ||
| STATUS_ID="$(gh api "repos/${REPO}/issues/${PR}/comments" --paginate | |
There was a problem hiding this comment.
[P2] --paginate fetches all PR comments on every round
The paginated scan pulls every comment on the PR to find the status marker. On a heavily-iterated PR (100 rounds × multiple comments per round), this can mean hundreds of API calls per round just to locate one comment.
No server-side search-by-body exists in the GitHub REST API, so this is the pragmatic choice. Two possible mitigations for the future:
- Cache in repo variable/dispatch state: persist the
comment_idacross rounds so only round 1 needs the scan. - Use
search/issue-commentswithrepo:X in:body autofix-status: the search API supports body filtering, though it has its own rate limits and index lag.
Not blocking — just flagging for awareness on high-round PRs.
| STATUS_ID="$(gh api "repos/${REPO}/issues/${PR}/comments" --paginate | | ||
| jq -rs --arg m "${MARKER}" --arg ab "${AUTOFIX_BOT}" \ | ||
| '[ .[][] | select((.user.login // "") == $ab) | ||
| | select((.body // "") | contains($m)) ] | last | .id // empty')" || |
There was a problem hiding this comment.
[P2] contains($m) is substring match — consider startswith($m)
The marker <!-- autofix-status --> is always written at position 0 in the comment body (the printf template starts with ${MARKER}). Using contains($m) technically matches if the marker appears anywhere in the body, which could false-positive on a comment that quotes or references the marker text.
startswith($m) is more precise and costs nothing extra:
| select((.body // "") | startswith($m))Extremely unlikely to matter in practice given how unique the marker is, but a one-character change for strict correctness.
yiliang114
left a comment
There was a problem hiding this comment.
Review Summary
Well-designed addition that solves a real visibility problem during long autofix rounds. The code is heavily commented (in a good way — the rationale for every gate and design choice is inline), error handling is best-effort throughout, and the test coverage is thorough.
Analysis
| Area | Verdict |
|---|---|
| Comment upsert | ✅ Idempotent. Marker-based scan + PATCH-on-existing / POST-on-missing is correct. Re-runs find and update the existing comment. last | .id // empty safely handles empty results. |
| Error handling | ✅ Both steps use || echo "::warning::..." — failures never block the autofix round. set -uo pipefail catches unset variables without killing the job on comment failures. |
| Race conditions | ✅ The stale gate prevents discarded duplicates from posting or overwriting status. Per-PR concurrency group serializes rounds. The finalize step is also stale-gated — good. |
| Security | ✅ No sensitive data in the comment body — only public GitHub URLs, round numbers, and the run link. CI_DEV_BOT_PAT is used only for API calls, never embedded in output. |
| Test coverage | ✅ 72 lines of new assertions covering: marker, gates, PATCH-only finalize, id handoff, Chinese localization, and the nothing to finalize early-exit path. |
| UX | ✅ Clear bilingual format, round counter, live run link, and terminal states (✅/ |
Findings (all P2, non-blocking)
--paginatecost (line 2972): Fetches all PR comments on every round. Acceptable given GitHub API limitations, but could be optimized by cachingcomment_idacross rounds.contains($m)vsstartswith($m)(line 2975): Substring match is technically less precise, though the unique marker makes false positives virtually impossible.
Conclusion
No P0 or P1 issues. The implementation is solid and the two P2 suggestions are optional improvements. LGTM.
yiliang114
left a comment
There was a problem hiding this comment.
Approve ✅
Solid implementation. Best-effort status comment that never blocks the autofix round. Idempotent marker-based upsert, correct stale-gate protection, id handoff from post to finalize avoids redundant scans. Thorough test coverage.
2 P2 inline comments posted (paginate optimization opportunity, contains vs startswith tightening). All non-blocking.
doudouOUC
left a comment
There was a problem hiding this comment.
One post-merge issue found in the final status transition.
| ROUND_DISPLAY="${EFFECTIVE_ROUND:-${ROUND}}" | ||
| # 'fixed'/'noop' are the two outcomes that published a round report; | ||
| # anything else means the round stopped before publishing one. | ||
| if [[ "${OUTCOME:-}" == 'fixed' || "${OUTCOME:-}" == 'noop' ]]; then |
There was a problem hiding this comment.
Suggestion: OUTCOME=fixed/noop only proves that verification produced a publishable result; it does not prove that the preceding Push and report step actually pushed or posted its comment. For example, a transient git push or gh pr comment failure leaves OUTCOME=fixed, while Report dry-run / failure deliberately suppresses its handoff for fixed/noop. This finalizer then rewrites the live status to “finished / report below” even though no report exists (and possibly no fix was pushed). Please key the success wording on the Push and report step outcome or an explicit published=true output, and add a regression case for the publish-failure path.
doudouOUC
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
|
Released in v0.21.1. |
|
Acknowledged. The comment on PR #7738 is just the release bot ( ✅ completed |
What this PR does
Makes an in-flight AutoFix round visible in the PR thread. When a round starts, it posts a status comment carrying a link to the live Actions run; when the round ends, it flips that same comment to a terminal state.
Why
Takeover engages — and then the thread goes quiet.
review-addressruns the agent for up to 80 minutes (timeout-minutes: 80) plus a verification gate (build/typecheck/lint/tests), but nothing reaches the PR untilPush and reportat the very end. Thetakeover-ackcomment says "engaged" and carries no run link, and everyautofix-*marker (eval,rearm,redcheck) is end-of-round bookkeeping.Measured on #7731: takeover engaged
01:55,review-addressstarted01:55:59, and at02:39it was still running — 43 minutes of silence, with no way to tell a working round from a stuck one. The job cap is 120 minutes, so it can be worse.The agent's output already streams live to the Actions log. Only the link was missing.
How
Two steps in
review-address, no agent/skill dependency:Post autofix status comment— afterPrepare branch and feedback, before the agent →<!-- autofix-status -->🔄 AutoFix is working on this PR — round N/M. [Watch live progress](run).Finalize autofix status comment(always()) — edits the same comment to✅ round N finished/⚠️ ended without publishing a report(fromsteps.verify.outputs.outcome), keeping the run link. The verdict itself stays in the round report this job already posts.Three properties that are load-bearing rather than incidental:
TAKEOVER_MAX_ROUNDS(100) of them; edits also notify nobody, so a long round-trip stays quiet.qwen-pr-head-write-<pr>) serialises duplicate address jobs, so a discarded duplicate runs after the real round already finalised. Ungated, its finalize would overwrite that round's✅ finishedwith its own⚠️ ended without publishing— reporting a successful round as a failed one. (Found in self-audit, not by a test; a regression test now pins it.)Best-effort throughout (
set -uo pipefail+ warn-and-continue): a failed status post never costs a round.Deliberately not done: a per-phase status line driven by the agent. The live Actions log is finer-grained than anything we could synthesise, needs no agent cooperation, and a mid-round agent is exactly when an agent-maintained line is least reliable.
Reviewer Test Plan
npx vitest run scripts/tests/qwen-autofix-workflow.test.js— 103/103, three consecutive clean runs. (This suite has known load-flakes —eligibility recheck,permanent API failures terminal— which appeared in some runs and are unrelated.)Watch live progress; breaking finalize's PATCH-only exit; adding a POST fallback to finalize (proves PATCH-only is genuinely pinned, not a string coincidence); removing finalize's stale guard (kills both the new test and thestale-gate count assertion).quote-type: single, allow-quoted-quotes). prettier clean.16findings on this branch vs16onmain— byte-identical rule counts, so no new lint findings; the reported lines are all pre-existing steps, none of them the two added here.bash -nclean on both run blocks.Risk & Scope
CI_DEV_BOT_PATas the round report, so the status edit is same-identity.npm ci+ build (a few minutes), not at second zero. That is the price of announcing only rounds that will actually run.Linked Issues
Follow-up to #7654, which did the same for
/triage; this is the takeover-loop counterpart. Motivated by the silence observed on #7731.中文说明
本 PR 做了什么
让正在进行的 AutoFix 轮次在 PR 线程里可见:轮次开始时发一条带实时 Actions 运行链接的状态评论,轮次结束时把同一条改为终态。
为什么
接管之后线程就安静了。
review-address跑 agent 最长 80 分钟(timeout-minutes: 80),外加验证门禁(build/typecheck/lint/测试),但直到最后一步Push and report才有内容进入 PR。takeover-ack只说"已接管"且不带运行链接,而所有autofix-*标记(eval/rearm/redcheck)都是轮次结束后的记账。在 #7731 上实测:
01:55接管,review-address01:55:59启动,到02:39仍在运行 —— 静默 43 分钟,无法区分"正在干活"与"卡住了"。job 上限是 120 分钟,还可能更久。agent 的输出本就实时流进 Actions 日志,缺的只是那个链接。
怎么做
review-address中两个步骤,不依赖 agent/skill:Post autofix status comment—— 在Prepare branch and feedback之后、agent 之前 →<!-- autofix-status -->🔄 正在处理此 PR —— 第 N/M 轮。[查看实时进度](run)。Finalize autofix status comment(always())—— 把同一条改为✅ 第 N 轮已完成/⚠️ 结束但未发布报告(依据steps.verify.outputs.outcome),保留运行链接。裁决本身仍在本 job 已有的轮次报告里。三个承重性质(而非可有可无):
qwen-pr-head-write-<pr>)会串行化重复的 address job,因此被丢弃的那个在真实轮次定稿之后才运行。若不 gate,它的 finalize 会把该轮的✅ 已完成覆写成自己的⚠️ 未发布报告—— 把成功的一轮报成失败。(自审发现,非测试发现;现已有回归测试钉住。)全程尽力而为(
set -uo pipefail+ 告警后继续):状态评论失败绝不拖垮轮次。**特意没做:**由 agent 维护的逐阶段状态行。实时 Actions 日志比我们能合成的任何文本都细,且不依赖 agent 配合;而轮次中段正是 agent 最忙、这种状态行最不可靠的时候。
评审验证
npx vitest run scripts/tests/qwen-autofix-workflow.test.js—— 103/103,连续三次干净。(该 suite 有已知负载 flake:eligibility recheck、permanent API failures terminal,与本 PR 无关。)Watch live progress;破坏 finalize 的 PATCH-only 出口;给 finalize 加 POST 兜底(证明 PATCH-only 是真被钉住,而非字符串巧合);删掉 finalize 的 stale 守卫(同时杀死新测试与stale计数断言)。16条 vsmain16条 —— 规则计数完全一致,无新增;报告行全是既有步骤,不含新加的两个。两个 run 块bash -n干净。风险与范围
CI_DEV_BOT_PAT,故状态编辑是同身份。npm ci+ build 之后(几分钟),而非第 0 秒。这是"只公告真正会运行的轮次"的代价。关联 Issue
#7654 的跟进(那个针对
/triage),本 PR 是接管循环的对应件。起因是 #7731 上观察到的静默。