feat(autofix): re-arm a stranded PR with @qwen-code /retry instead of deleting a marker - #7354
Conversation
… deleting a marker Recovering a stranded managed PR meant running `gh api -X DELETE` against the bot's own autofix-eval marker comment. That needed raw API access and the comment id, erased the audit trail, and was undiscoverable unless you had read the workflow — it came up twice while triaging #7246, #7329 and #7336. `@qwen-code /retry` now posts a single `<!-- autofix-rearm -->` marker, which does both halves of what the deletion did: - The scan's watermark ignores eval markers written BEFORE the newest re-arm, so the feedback those markers buried is read again. The watermark stays global otherwise — this is an explicit, maintainer-issued exception, which is exactly what the deletion was, only recorded instead of destructive. - The marker also opens a fresh counting window (it joins the engage ack in REARM_KEY), so the round counter resets and a terminal round stops skipping the PR. That also means the existing "a re-arm supersedes queued old-window jobs" guard covers /retry for free. The address job's live recheck mirrors both, so a run selected before a re-arm still discards itself instead of stamping an old-sequence marker. Authorization is the takeover command's, unchanged and reused rather than reinvented: exact body match, live permission lookup, in-repo-only author privilege. The route prefilter now admits the second command. The job verifies CI_DEV_BOT_PAT authenticates as the bot before commenting, because both scanners only count markers authored by it. The marker is registered as a control comment so the agent never sees the re-arm as feedback to address. Tests: the real extracted scan block is replayed over synthetic comment fixtures — stranded (watermark held, round 2), after /retry (watermark released, window reset, round 0), a marker written after the re-arm counting again, and a re-arm from a non-bot author correctly ignored. Both halves mutation-verified.
|
Thanks for the PR! Template looks good ✓ Problem: observed and well-documented. The manual marker deletion came up twice during #7246 / #7329 / #7336 triage — each time a maintainer had to find the comment ID and run a destructive Direction: aligned. This is autofix reliability infrastructure — the Size: not applicable — changes are in Approach: the scope feels right. Two files, one workflow + one test. Reuses the takeover command's authorization rather than inventing a second policy, and the blast radius is one comment. Every edit in the diff serves the stated goal — no drive-by changes. The watermark-release + round-reset design mirrors what the deletion did, just non-destructively. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测且有据可查。在 #7246 / #7329 / #7336 的排查中两次出现手工删除标记评论的操作——每次都需要维护者找到评论 ID 并执行破坏性的 方向:对齐。这是 autofix 可靠性基础设施—— 规模:不适用——改动在 方案:范围合理。两个文件,一个 workflow + 一个测试。复用 takeover 命令的授权而非另造一套策略,爆炸半径仅一条评论。diff 中每处改动都服务于既定目标,无夹带。水位线释放 + 轮次重置的设计镜像了删除操作的效果,只是非破坏性的。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code ReviewIndependent proposal: I would have done essentially what this PR does — add a second command to the existing route prefilter, reuse the takeover authorization (same permission check, same author rules), post a bot-authored marker that the scan recognizes as a watermark-release + window-reset event. The key design decision is whether to introduce a new authorization path or reuse the existing one; reusing is clearly correct since both commands summon bot activity on a managed PR. Comparison with the diff: the PR matches this approach exactly. A few things done well:
No critical blockers or convention violations found. The diff is focused — every hunk serves the TestingThis is a CI workflow change (not user-facing CLI), so real-scenario testing is the test suite + static validation: The two new tests extract the real scan block from the workflow YAML and replay it over synthetic comment fixtures — mutation-verified per the PR description (dropping the re-arm from either the watermark filter or the window key turns the test red). 中文说明代码审查独立方案: 我基本上会做和这个 PR 一样的事——在现有 route 预过滤中加入第二个命令,复用 takeover 授权(相同的权限检查、相同的作者规则),发一条 bot 署名的标记,扫描将其识别为水位线释放 + 窗口重置事件。关键设计决策是引入新的授权路径还是复用现有的;复用显然是正确的,因为两个命令都是在托管 PR 上召唤 bot 活动。 与 diff 对比: PR 完全匹配此方案。几个做得好的地方:
未发现关键阻塞或规范违反。diff 聚焦——每个 hunk 都服务于 测试这是 CI workflow 变更(非用户可见 CLI),因此真实场景测试即测试套件 + 静态验证:
两个新测试从 workflow YAML 中提取真实扫描块,在合成评论夹具上回放——按 PR 描述经变异验证。 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 — clean across every stage; the problem is real, the fix is minimal, and the tests prove both halves work. This is exactly the kind of PR that's easy to review and easy to maintain. The motivation is concrete (two manual recoveries during recent triage), the solution reuses existing authorization instead of adding a parallel policy, and the blast radius is one comment. The scan/address mirroring is consistent, the control-comment registration is complete across all three filter sites, and the PAT identity check is a nice defensive touch. My independent proposal matched the PR's approach — I didn't find a simpler path it missed. The two new tests replay the real extracted scan block (not a reimplementation), and the mutation-verification claim is credible: the watermark filter and window key are independently necessary, and the tests assert both. Six months from now, the 中文说明置信度:5/5 —— 每个阶段都干净;问题真实、修复最小化、测试证明两半都有效。 这正是那种容易审查、容易维护的 PR。动机具体(近期排查中两次手工恢复),方案复用现有授权而非添加并行策略,爆炸半径仅一条评论。扫描/address 镜像一致,控制评论注册在所有三处过滤点都完整,PAT 身份校验是很好的防御性补充。 我的独立方案与 PR 的方法一致——没有找到更简路径。两个新测试回放真实提取的扫描块(非重新实现),变异验证可信:水位线过滤和窗口键各自独立必要,测试对两者都有断言。 六个月后, — 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. |
|
🤝 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 冲突,直到移除标签或达到轮次上限。移除 |
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
| ).toBe(0); | ||
| }); | ||
|
|
||
| it('re-arms a stranded PR from a marker instead of a deleted comment', () => { |
There was a problem hiding this comment.
[Suggestion] The address-side stale check (LIVE_EVAL_WM, LIVE_REARM_KEY, LIVE_MAX_ROUND) mirrors the scan-side logic but is not behaviorally exercised under bash — only the scan-side equivalent is. The existing toContain assertions at lines 264 and 1585 verify variable presence, not correctness.
Failure scenario: if a future edit to the address-side jq filters diverges from the scan-side (e.g., drops the $rearm filter from LIVE_EVAL_WM, or uses a different variable name), no test catches it. A queued address job could stamp an old-sequence eval marker into a fresh window after /retry, defeating the re-arm.
Concrete cost: the scan-side and address-side jq blocks are copy-pasted (~40 lines each, 4 jq expressions), and drift between them has already been the class of bug this feature prevents (stranded PRs from watermark/round desync). Adding the same behavioral harness for the LIVE_* variables closes the symmetry gap.
Suggested fix: extract the address-side block from prepareBranchAndFeedbackStep via the same regex pattern the scan-side test uses, and assert LIVE_EVAL_WM/LIVE_REARM_KEY/LIVE_MAX_ROUND match EVAL_WM/REARM_KEY/ROUND for the same input fixtures.
— 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 addressedSuggestion: address-side stale check lacks behavioral test coverageSource: Finding: The address-side stale check ( Decision: Implemented. The scan-side and address-side jq blocks are copy-pasted (~40 lines each, 4 jq expressions), and drift between them is exactly the class of bug this feature prevents. Change: Added
Verification:
中文说明已处理的审查反馈建议:地址侧过期检查缺少行为测试覆盖来源: 发现: 地址侧过期检查( 决定: 已实现。扫描侧和地址侧的 jq 代码块是复制粘贴的(各约 40 行,4 个 jq 表达式),两者之间的漂移正是此功能所防止的 bug 类型。 变更: 添加了
验证:
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. Suggestions are inline. Not reviewed: Agent 0: Issue fidelity & root-cause ownership, Agent 4: Performance & efficiency, Agent 7: Build & test verification — its prompt was built, but no agent on record was launched with it.
— qwen3.7-max via Qwen Code /review
| ); | ||
| expect(workflow).toContain("RETRY_COMMAND: '@qwen-code /retry'"); | ||
| expect(workflow).toContain('<!-- autofix-rearm -->'); | ||
| expect(workflow).toContain('<!-- (autofix-eval|autofix-rearm|qwen-triage|'); |
There was a problem hiding this comment.
[Suggestion] The address-side bot comment filter regexes (autofix-rearm added at 3 sites: workflow lines ~2267, ~2338, ~2415) have no per-site structural assertion. This toContain check is a single substring match that passes as long as the pattern exists anywhere in the file — if a future edit drops autofix-rearm from one of the three copy-pasted address-side filters, the test still passes because the other instances match.
Failure scenario: copy-paste drift removes autofix-rearm from one address-side filter → the address agent sees the <!-- autofix-rearm --> marker as actionable review feedback and burns a cycle addressing a control comment.
Consider adding per-site assertions or extracting the filter regex into a shared constant:
| expect(workflow).toContain('<!-- (autofix-eval|autofix-rearm|qwen-triage|'); | |
| expect(workflow).toContain('<!-- (autofix-eval|autofix-rearm|qwen-triage|'); | |
| // Verify all four filter sites (scan + 3 address) include autofix-rearm | |
| const filterMatches = [...workflow.matchAll(/autofix-eval\|autofix-rearm\|qwen-triage/g)]; | |
| expect(filterMatches.length).toBeGreaterThanOrEqual(4); |
— qwen3.7-max via Qwen Code /review
| elif [[ -z "${CMD}" && -z "${RETRY_REQ}" ]]; then | ||
| echo "🧭 command ignored: body is not an exact command" | ||
| elif [[ "${ISSUE_STATE}" != 'open' ]]; then | ||
| echo "🧭 takeover command ignored: PR is not open" |
There was a problem hiding this comment.
[Suggestion] Two of the five diagnostic messages were generalized from "takeover command ignored" to "command ignored" (lines 342, 344), but this one and two others (lines 348, 397) still say "takeover command ignored" even though they now apply to /retry as well.
Failure scenario: an authorized sender posts @qwen-code /retry on a closed PR → the log prints "🧭 takeover command ignored: PR is not open", referencing the wrong command.
| echo "🧭 takeover command ignored: PR is not open" | |
| echo "🧭 command ignored: PR is not open" |
— qwen3.7-max via Qwen Code /review
…t all filter sites (#7354)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: Review feedback addressed1. Per-site filter assertion for
|
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
| expect(job).toContain("gh api user --jq '.login'"); | ||
| expect(job).toContain('expected ${AUTOFIX_BOT}'); | ||
| expect(job).toContain('gh pr comment "${PR}"'); |
There was a problem hiding this comment.
[Suggestion] The retry-command job's bash script (PAT identity check + marker posting) is only structurally verified — no test executes the script with fixture data to confirm the error paths or the comment body format.
Failure scenario: if the printf format string in gh pr comment --body were malformed (e.g., a missing <!-- autofix-rearm --> marker, or a shell quoting error in the '"'"' escape), the job would post a comment that scanners ignore while printing "🔄 re-armed PR #N" — the PR stays stranded with no indication of failure. The existing toContain('<!-- autofix-rearm -->') check at line 3293 matches the marker in the workflow text at multiple sites (jq filters, BOT_COMMENT_FILTER), so a typo in the printf body would not be caught.
Consider extracting the retry-command job's bash into a testable block (like the scan/address tests do) and running it with a mocked gh on PATH, verifying: (a) it calls gh api user, (b) it calls gh pr comment with a body containing <!-- autofix-rearm -->, and (c) it exits non-zero on actor mismatch.
— qwen3.7-max via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: Autofix review round summaryAddressed the automated reviewer's suggestion on the retry-command job. No Feedback point
Decision: implementedThis is an early review round and the suggestion is valuable, in scope, and ChangesAdded one behavioral test,
The test asserts the extracted block is non-empty, so it fails loudly rather Conflict notesNone — Verification
Commit: 中文说明Autofix 评审轮次总结处理了自动评审器针对 retry-command 任务提出的建议。不存在冲突( 反馈点
决定:已实现当前处于评审早期轮次,该建议有价值、在范围内,并且与本文件中扫描/处理任务已有的测试方式一致(提取真实的 bash 块,在 改动新增了一个行为测试
该测试断言提取出的代码块非空,因此当工作流步骤被重命名或重构时,它会显式失败,而不会空跑通过。 冲突说明无 —— 验证
提交: 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
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.20.1. |
What this PR does
Adds
@qwen-code /retry— a one-comment way to re-arm a stranded managed PR, replacing the current recovery of deleting the bot's own marker comment by hand.Why it's needed
When a PR gets stranded (a falsely-advanced feedback watermark, or a terminal round), the only way back was:
That requires raw API access and the comment id, erases the audit trail, and is undiscoverable unless you have read the workflow. It came up twice while triaging #7246, #7329 and #7336 — each time a maintainer had to hand-run a destructive API call to un-stick a PR whose fix the agent had already written.
How
@qwen-code /retryposts a single<!-- autofix-rearm -->marker. That marker does both halves of what the deletion did:REARM_KEY, so it opens a fresh counting window: the round restarts at 0 and a terminal round stops skipping the PR. The existing "a re-arm supersedes queued old-window jobs" guard therefore covers/retryfor free.The address job's live recheck mirrors both, so a run selected before a re-arm still discards itself instead of stamping an old-sequence marker into the fresh window.
Why it's safe
CI_DEV_BOT_PATauthenticates as the bot before commenting — both scanners only count markers authored by it, so a mis-scoped PAT would otherwise post a comment that silently does nothing.Reviewer Test Plan
How to verify
npx vitest run scripts/tests/qwen-autofix-workflow.test.js— 69/69. The real extracted scan block is replayed over synthetic comment fixtures:EVAL_WM…08:30:00Z(held)none<!-- autofix-rearm -->""(released)created_at…10:30:00Z(counts again)…08:30:00Z(ignored)Mutation-verified: dropping the re-arm from the watermark filter, or from the window key, each turns the test red.
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: comment
@qwen-code /retryon a stranded PR and confirm the bot posts the re-arm marker and the next scan re-selects the PR at round 0.Evidence (Before & After)
Tested on
Risk & Scope
/takeover, and the round cap still bounds what follows.Linked Issues
Motivated by the manual recoveries on #7246 / #7329 / #7336. Part of the same autofix-reliability line as #7330, #7350, #7351.
中文说明
本 PR 做了什么
新增
@qwen-code /retry—— 用一条评论重新武装被搁置的托管 PR,取代现在手工删除 bot 自己的标记评论这种恢复方式。为什么需要
当 PR 被搁置(水位线被误推进,或轮次到终态),唯一的复位办法是:
它需要裸 API 权限和评论 id、抹掉审计痕迹,且不读 workflow 根本无从得知。在排查 #7246、#7329、#7336 时出现了两次 —— 每次都要维护者手工执行破坏性 API 调用,去解救一个 agent 其实早已写好修复的 PR。
怎么做
@qwen-code /retry发一条<!-- autofix-rearm -->标记。该标记完成删除操作的两半:REARM_KEY,因此开启一个新的计数窗口:轮次从 0 重新开始,终态轮次不再让扫描跳过该 PR。既有的"re-arm 使旧窗口已排队作业失效"保护也因此免费覆盖/retry。address 作业的实时复核对两者同样镜像,所以在 re-arm 之前被选中的运行会自行丢弃,而不会把旧序列的标记打进新窗口。
为什么安全
CI_DEV_BOT_PAT的身份确为 bot —— 两个扫描器都只统计由它撰写的标记,PAT 作用域不对会导致评论静默失效。评审验证
npx vitest run scripts/tests/qwen-autofix-workflow.test.js—— 69/69。真实提取的扫描块在合成评论夹具上回放:两条 eval 标记(搁置)→ 水位线保持、round=2;加入 re-arm 标记 → 水位线释放、窗口键变更、round=0;re-arm 之后写的标记重新计入;非 bot 作者的 re-arm 被忽略。两半均经变异验证(分别去掉水位线过滤与窗口键中的 re-arm,测试各自变红)。run:块过bash -n;actionlint 1.7.12 clean;prettier clean。@qwen-code /retry,确认 bot 发出 re-arm 标记且下一次扫描以 round 0 重新选中该 PR。风险与范围
/takeover相同的发起人,后续仍有轮次上限约束。关联 Issue
由 #7246 / #7329 / #7336 上的手工恢复驱动。与 #7330、#7350、#7351 同属 autofix 可靠性主线。