fix(autofix): post the takeover engage ack from the command itself - #8043
Conversation
|
Thanks for the PR! Template: the substance is all here — what, why, and a thorough test plan — though the headings are named Problem: observed, not theoretical. Three distinct failure modes, each tied to a real PR: the Direction: aligned. This is the autofix/takeover loop's own ack plumbing — internal CI tooling, squarely in scope, and it touches no auth/sandbox/model/telemetry/public-contract surface. Making the engage ack independent of a round-trip that has demonstrably failed is the right instinct. Size: not a core-module path ( Approach: the scope feels right. Posting the ack directly from Risk: no elevated signals — neither changed file matches the high-revert-path patterns. Moving on to code review. 🔍 中文说明感谢贡献！ 模板： 实质内容齐全——做了什么、为什么、以及详尽的测试方案——只是小标题用了 问题： 已观测，非理论。三种失败模式各自关联真实 PR： 方向： 对齐。这是 autofix/takeover 循环自身的回执管线——内部 CI 工具，完全在范围内，且不触及 auth/sandbox/model/telemetry/公共契约。让回执不再依赖一个已被证明会失败的事件回环，方向正确。 规模： 非核心模块路径（ 方案： 范围合理。在所有准入门槛通过后由 风险： 无升级信号——两个改动文件均未命中高回滚路径模式。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewBefore reading the diff, my independent take from the description was: post the engage ack from What I verified against the surrounding workflow:
The tests are behavioral in the right way: they exec the actual bash blocks against a mock Test evidence (the PR's own CI)The load-bearing job —
The sandboxed lanes ( 中文说明代码审查读 diff 之前，我仅凭描述给出的独立方案是：在标签应用后由 对照周边 workflow 验证的要点：
测试以恰当的方式做到行为化：对 mock 测试证据（PR 自身的 CI）承重作业—— 沙盒通道（ — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — a clean, well-evidenced fix that does exactly what it says; the only nit is cosmetic (the PR-body headings don't match the template's names, though the substance is all there). Stepping back: this is the right fix for a real, observed problem, and it's built the way I'd build it. The engage ack no longer depends on a The tests are the kind I trust for a workflow change: they exec the actual bash blocks against a mock The one thing I won't do is attest to a green suite that doesn't exist yet: the ubuntu test job that runs this PR's behavioral suite is still in progress at the reviewed commit. So approval is deferred until CI lands green on 中文说明置信度：4/5 —— 一个干净、有据可依、言行一致的修复；唯一的瑕疵是表面性的（PR 正文小标题与模板命名不一致，但实质内容齐全）。 退一步看：这是对真实已观测问题的正确修复，做法也是我会采用的做法。回执不再依赖一个已被证明会不触发的 测试是我对 workflow 改动会信任的那一类：对 mock 唯一我不会做的，是为一个尚不存在的绿套件背书：运行本 PR 行为测试套件的 ubuntu 作业在受审 commit 上仍在进行。因此批准延迟至 CI 在该 commit 上落绿——finalize 作业会在所有检查绿后发出绑定该 commit 的批准，若有检查变红或 head 移动则保留不发。 — 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. |
The engage ack rode a pull_request:labeled round-trip: takeover-command applies the label, the labeled event routes, and the takeover-ack job posts the confirmation. That event has now been observed to simply not fire twice in one day (#7999 — the author read the silence as failure and removed the label; #8002 — an engaged fork PR with no ack for hours), and fork label events can never ack at all since they carry no secrets: a fork /takeover stayed silent until the next scan picked the PR up (2h41m on #7993). takeover-command now posts the engage ack directly after applying the label — every admission gate has already passed at that point, so 'engaged' is truthful for in-repo and fork PRs alike; the fork variant adds the expectation that the first round comes from the next scheduled scan. The route side suppresses the label-path ack when the label sender is the bot (only the ack: the immediate scan still routes), and the review-scan's existing first-pickup ack dedups against the command's comment and heals it if the post failed. Two more silent paths become audible while here: a /takeover on a stacked (non-main-base) PR now refuses out loud instead of dropping with only a log line, and a /takeover stop on a non-main PR now proceeds to remove the label instead of leaving it stuck.
2e5bdc9 to
f96fe83
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未发现问题。LGTM！✅
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
Review — reviewed at
|
| Mutation | Result |
|---|---|
Swap "${FORK_NOTE}" ⇄ "${FORK_NOTE_ZH}" in the engage printf args |
102/102 pass — ships the Chinese sentence inside the English paragraph and vice versa |
|| echo "::warning::…" → || true |
102/102 pass — nothing pins the fallback |
The first survives because expect(forkManaged.writes).toContain('next scheduled scan') matches either half of the body. Splitting on <summary>中文说明</summary> and asserting each note in its own half kills it — worth doing, since the ack-body census already checks that both halves exist and this is the natural completion of it. The second matters because the fallback is load-bearing under bash -e: without it the step aborts red after the label was already applied, which is a worse signal than the silence being fixed.
Net: no correctness blocker in what's here. #1/#2 are the ones I'd want addressed before merge — not because the code is wrong, but because shipping a loud add next to a mute stop re-creates the exact ambiguity ("did it work, or did the event get lost?") from the PR description, now on the release side. #3 is a small, mechanical hardening of the healing claim the PR makes. #4/#5 are polish.
中文说明
审查 —— 基于 f96fe836add56272c3a102aca1d495ca1ec9b4dc
核心改动是对的，我没能把它推翻：回执不再依赖一个已被证明会丢失的事件回环，双向去重也成立。但我发现修复只覆盖了 toggle 的一半——释放侧仍挂在脆弱的回环上，而本 PR 新启用的「非 main 分支上的 stop」是静默的；此外还有一条被本 PR 悄悄解除武装的自愈路径。
以下结论均来自实际回放 bash 代码块，而非仅靠阅读。
已独立验证正确
- 102/102 通过；两个文件
prettier --check干净；YAML 可解析（7 个 job）；对抽取出的route与takeover-commandrun 块跑shellcheck仅报一个既有的SC2129。 - 两条新正文的
printf参数个数正确（engaged 6 个%s/ 6 参数，base-refused 4 / 4）。我从真实 bash 回放中导出了渲染后的正文——结构完好、逐条双语，且 fork 说明在中英文两侧的空格衔接都正确。 takeover_ack只用于门控「仅发评论」的takeover-ackjob（L1561 是唯一消费点），因此抑制它不会丢失任何副作用。- 非 main
stop的新else落空分支是活代码而非死代码：route 的issue_comment路径从不检查baseRefName，所以堆叠 PR 上的/takeover stop确实会到达该 job。 - 去重顺序成立：标签在
T、回执在T+δ，扫描侧LAST_LABELED_TS > LAST_ENGAGE_ACK_TS为假，不会重复回执。
1. 非 main PR 上的 /takeover stop 成了最新的静默路径 —— 中
本 PR 的前提是「静默丢弃就是 bug」，但它启用的堆叠 stop 摘掉标签后什么都不说：命令 job 不发释放回执；随之而来的 pull_request:unlabeled 事件被 route 对非 main PR 明确丢弃；扫描侧只发 engage 回执、从不发 release。回放验证：writes.log 只有 --remove-label，没有任何 COMMENT。于是被新的 base-refused 回执告知「改 base 后重跑命令」的用户，在执行 stop 时看到的现象与本 PR 正在修复的「事件丢失」完全一样。add 侧变响了，stop 侧仍然是哑的。
2. fork 的接管有回执，释放依然永远没有 —— 中
本 PR 之前两个方向对 fork 都不回执，至少是对称的。现在 fork 上 /takeover 会发 🤝 Takeover engaged，而同一个 PR 上的 /takeover stop 什么都不发（fork 的 unlabeled 事件无 secrets，route 直接丢弃；其他地方也没有释放回执）。同一份回放确认（cmd=remove, fork=true → 只有 --remove-label，零评论）。
针对 1+2 的建议（同一处修改）： 在命令的 else 分支里也直接发释放回执，与接管侧完全对称——理由相同：所有门槛已通过、标签写入已成功、而事件回环正是我们不再信任的东西。这需要给 route 的 unlabeled 分支加上与 labeled 分支刚加的同款 sender 抑制，否则 in-repo main PR 会重复回执。如果你更希望本 PR 只聚焦 engage，那也合理——但请在正文里说明，因为「另外两个静默路径改为出声」目前读起来像是堆叠 stop 也变响了，而它并没有。
3. 扫描侧的 3 分钟宽限期，恰好推迟了本 PR 所依赖的那次自愈 —— 中
失败故事是：命令发评论失败 → 只告警 → 「扫描的 first-pickup 回执会补发」。但由同一个 labeled 事件触发的即时扫描不会补发，因为本 PR 没有动的宽限期判断是「in-repo 且标签落地 <3m ⇒ 交给 ack job」——而对 bot 打的标签，route 刚刚把那个 ack job 抑制了。于是补发从「即时扫描」滑到「下一次定时扫描」（*/10 cron 再叠加 GitHub 的调度延迟），并非「下一趟」。强制 PR 路径确实会走到这段代码（CANDIDATES="${FORCED_PR}" → 每候选的 HAS_TAKEOVER），所以这是活路径。
pr-events.json 就在旁边已经取好了，把 actor 与时间戳一起取出、在最后打标签者是 AUTOFIX_BOT 时跳过宽限期，即可让宽限期名副其实。同一处修改还能顺带堵住本 PR 在另一侧打开的竞态：宽限期把 fork 排除在外的理由是「fork 没有 ack job」，而本 PR 让这句话不再成立——若某次定时扫描的 ic.json 抓取恰好落在 fork 打标签与发回执之间，就会重复回执并移动轮次窗口锚点。窗口只有约 1 秒，不构成阻断，但属于同一行修复。
4.「通常一小时内」与 workflow 自己的 cron 矛盾 —— 低
新的 fork 说明承诺「通常一小时内」，而调度是 */10 * * * *，route 自己的日志则写「下一次定时扫描（≤10m）」。考虑到 GitHub 对繁忙仓库的定时任务限流，保守承诺可以理解，但同一个 workflow 的两处对外表述不该差 6 倍。
5. 新测试放过了两个变异 —— 低（测试）
对改动逻辑跑了 7 个变异。被杀掉（好）：去掉 bot-sender 抑制、清空 FORK_NOTE、让堆叠 add 落空去打标签、把堆叠 stop 改回静默丢弃、整段删掉 engage 回执。存活两个：把 "${FORK_NOTE}" 与 "${FORK_NOTE_ZH}" 在 printf 参数里对调（102/102 全过——中文句子会渲染进英文段落，反之亦然）；把 || echo "::warning::…" 改成 || true（102/102 全过——没有任何断言钉住这个兜底）。前者存活是因为 toContain('next scheduled scan') 在正文任一半都能命中，按 <summary>中文说明</summary> 切开、各断言各半即可杀掉；后者重要在于该兜底在 bash -e 下是承重的——没有它，步骤会在标签已打上之后变红退出，那比正在修复的静默更糟。
结论： 现有代码没有正确性阻断项。合并前我最希望处理的是 #1/#2——不是因为代码写错了，而是因为「响亮的 add + 哑的 stop」会在释放侧重新制造 PR 描述里那个「到底成功了还是事件丢了？」的歧义。#3 是对本 PR 自身自愈承诺的一处小而机械的加固，#4/#5 属于打磨。
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
…race on the label actor Review follow-up: the engage-side fix left the release side on the fragile round-trip — a loud add next to a mute stop re-creates the exact 'did it work or did the event get lost?' ambiguity this PR set out to remove, now on release. And a fork or non-main release could never ack at all (fork unlabeled events carry no secrets; the route ignores non-main releases). - takeover-command now posts the release ack directly after removing the label, mirroring the engage side, with the same three variants as the ack job (plain release / bot-authored / bot-authored+skip) chosen from the same PR_INFO the gates used. The route suppresses the unlabeled-path ack when the label sender is the bot. - The scan's first-pickup grace is keyed on the label event's actor: a bot-applied label defers only 45s (the command's own write is seconds behind the label — fork or in-repo alike), so the immediate scan still heals a failed command ack instead of slipping to the next scheduled tick, and an ic.json snapshot taken between the label write and the command ack cannot double-post. A human in-repo label keeps the 3-minute ack-job grace; a human fork still posts right away. - The fork note now says 'usually within minutes', matching the */10 cron instead of contradicting it by 6x. - Tests kill the two surviving mutations from review: the EN/ZH fork-note swap (per-language-half assertions) and the warning fallback downgraded to '|| true' (both fallback strings pinned).
|
All five addressed in d3d4708 — thanks for the replay-level depth, especially catching that the fix only covered half the toggle. #1 + #2 (release side): took your suggested shape — #3 (grace): took the actor-keying and went one step further so both of your concerns land: #4: fork note now says "usually within minutes", matching the #5 (mutations): both killed — the fork note is asserted per language half (split on PR body updated to reflect the release-side symmetry (the old "two more silent paths" wording indeed oversold the stacked — Claude Code (claude-fable-5) |
|
🤝 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
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Round summaryInline findings
Failed checksAll seven listed checks were Issue-level comments@wenshao's five-point review was already addressed in commit Verification
中文说明本轮总结行内发现
失败检查列出的七项检查均为 Issue 级评论@wenshao 的五点审查已在 commit 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action takenThe only feedback this round is an automated review that was downgraded from Approve to Comment solely because CI checks were red — the review itself raised no code concerns ("Reviewed"). All three failing checks ( Local verification confirms the code is sound:
No inline comments, no issue-level comments, and no code-level findings to address. 中文说明未采取任何操作本轮唯一的反馈是一条自动评审，该评审从 Approve 降级为 Comment，原因仅仅是 CI 检查变红——评审本身未提出任何代码问题（"Reviewed"）。 三个失败的检查（ 本地验证确认代码没有问题：
没有行内评论、没有 issue 级别的评论、也没有需要处理的代码层面发现。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Local verification — real builds and behavioral replayVerdict: merge-ready. The change does what it claims, the tests are load-bearing (12/12 mutations killed), and it additionally closes a double-post that has already happened in the wild. One non-blocking wording inaccuracy is documented below. Verified at head Method. The harnesses are independent of the PR's own test file: each extracts the job's 1 · The motivating failures are real — read live from the API, not from the description
#7999's second label at 2 · What the maintainer now actually seesAll 16 command scenarios replayed on both trees. Every comment in this screenshot is the byte-for-byte
Ten scenarios go from silent to acked; six are unchanged. Independent census of the workflow: 17 Both ack posts keep a non-fatal fallback — with 3 · Dedup, the mutation matrix, and the timing finding
4 · It also closes a double-post that already happenedCensus of 40 takeover PRs found #7918: two byte-identical The vulnerable window shrinks from ~26s to ~1s. (The residual ~1s exists because the grace lives only in the empty-ack branch, not the re-engage branch — pre-existing in both trees, out of scope here.) Finding (non-blocking) — "the immediate scan still heals a failed command ack" does not hold at the measured latencyA label-routed scan sets
So when the command's own ack post fails, the immediate scan defers and the heal lands on the next scheduled scan (≤10 min) — not on the immediate one. That is still a large improvement over the status quo and the guard is correct as written; please do not shrink the grace (it is what makes the fork and re-engage races safe). Only the wording needs a pass, in three places: the PR body, the workflow comment ( Minor: the route comment at ~L405 — "engagement and release happen ONLY via the label events below, so the command's whole blast radius is one label toggle" — is now stale; the command posts acks directly in both directions. Gates
中文说明本地验证 —— 真实构建与行为回放结论：可以合并。 改动确实做到了它声称的事，测试是承重的（12/12 变异全被杀死），并且额外关闭了一个线上已经真实发生过的重复回执问题。下面记录一处不阻塞合并的措辞失准。 在隔离 worktree 中基于 head 方法。 各套 harness 与 PR 自带的测试文件相互独立：用真正的 YAML 解析器从 1 · 动机中的故障是真实的（直接查 API，而非采信描述）
#7999 第二次打标签（ 2 · 维护者现在实际看到的内容16 个命令场景在两棵树上全部回放。上方第一张截图中的每条评论都是真实 bash 块 十个场景从静默变为有回执，六个保持不变。对工作流的独立普查：17 条 两处回执发布都保留了非致命兜底 —— 注入 3 · 去重、变异矩阵与时序发现
4 · 它还关闭了一个已经真实发生过的重复回执对 40 个 takeover PR 的普查发现 #7918： 易损窗口从约 26 秒收缩到约 1 秒。（残留的约 1 秒是因为宽限期只存在于"无回执"分支、不在"重新接管"分支 —— 两棵树都如此，属于既有问题，不在本 PR 范围内。） 发现（不阻塞）——"即时扫描仍能补发失败的命令回执"在实测时延下并不成立由标签事件路由的扫描会设置 因此当命令自身的回执发布失败时，即时扫描会推迟，补发落在下一次定时扫描（≤10 分钟），而不是即时那一次。这相对现状仍是巨大改进，防护逻辑本身也没有写错；请不要缩短这个宽限期（它正是 fork 与重新接管竞争得以安全的原因）。需要调整的只是措辞，共三处：PR 正文、工作流注释（ 小问题： route 中约 L405 的注释 —— "engagement and release happen ONLY via the label events below, so the command's whole blast radius is one label toggle" —— 现已过时；命令现在会双向直接发布回执。 门禁
|
…nd blast radius (#8043)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Round summaryAddressed both non-blocking findings from @wenshao's verification review. All changes are comment-only — no logic, no behavior change. Finding 1: "the immediate scan still heals a failed command ack" — wording inaccuracyDecision: Implemented. The reviewer measured that label-routed scans arrive +14s–+33s after the label — inside the 45s grace — so the immediate scan defers, and the heal lands on the next scheduled scan (≤10 min). Fixed the inaccurate claim in two code locations:
The PR body also mentions this claim; updating it requires GitHub API access the skill does not have — the workflow can update it if desired. Finding 2: Route comment "whole blast radius is one label toggle" — staleDecision: Implemented. The command now posts user-visible acks directly in both directions, so the old "blast radius is one label toggle" claim is outdated. Replaced with "the command also posts acks directly in both directions (#7999, #8002)" at ~L407. Not addressedNone — both findings are resolved. Verification
中文说明本轮总结处理了 @wenshao 验证评审中的两项不阻塞合并的发现。所有改动仅涉及注释——无逻辑变更、无行为变更。 发现 1："即时扫描仍能补发失败的命令回执"——措辞失准决定：已实施。 评审者实测发现，由标签事件路由的扫描在标签之后 +14 秒至 +33 秒到达——在 45 秒宽限期之内——因此即时扫描会推迟，补发落在下一次定时扫描（≤10 分钟）。已在两处代码中修正不准确的表述：
PR 正文中也提及了该说法；更新 PR 正文需要 GitHub API 权限，而本 skill 不具备该权限——如需要，工作流可自行更新。 发现 2：Route 注释"whole blast radius is one label toggle"——已过时决定：已实施。 命令现在会双向直接发布用户可见的回执，因此旧的"blast radius is one label toggle"说法已过时。在约 L407 处替换为"the command also posts acks directly in both directions (#7999, #8002)"。 未处理无——两项发现均已解决。 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未发现问题。LGTM！✅
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.7-max via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed[Suggestion] (rc:3677498198) — the three release-ack Decision: Implemented. This is a valuable, in-scope guard and the reviewer's preferred (lowest-cost) option. Change: Added an assertion block to the existing I chose the test over a cross-reference comment because it actively fails on drift instead of relying on a future editor to read and honor a comment; the workflow already carries a "mirrors the ack job verbatim" note, so an additional comment would be narration, not a guard. Conflict notes: none ( Verification
中文说明已处理的评审反馈[建议] (rc:3677498198) —— 三个 release-ack 的 决定： 已实现。这是一个有价值、在范围内的防护，也是评审者首选的（成本最低的）方案。 改动： 在 我选择测试而非交叉引用注释，因为它会在漂移时主动失败，而不是依赖未来的编辑去阅读并遵守注释；workflow 中已经有一条 "mirrors the ack job verbatim" 的说明，因此再加注释只是叙述，而非防护。 冲突说明： 无（ 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未发现问题。LGTM！✅
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at 813b3d4. Traced the ack ownership split end to end: the command job now posts engage/release acks directly (running in base-repo context with CI_DEV_BOT_PAT, so fork PRs can be acked — the old label-event leg structurally never could), while route suppresses TAKEOVER_ACK only when SENDER_LOGIN == AUTOFIX_BOT, so human label toggles still flow through the retained takeover-ack job; grepping every takeover reference at this HEAD found no dangling needs/outputs. Toggle idempotency is right: repeat /takeover with the label present re-arms without touching the label, repeat /takeover stop with it absent writes nothing. The scan-side first-pickup grace is actor-keyed (45s for bot-applied labels awaiting the command's own ack, 3m for in-repo human labels, none for fork human labels) and the jq extraction is behaviorally replayed in tests, as are all four toggle paths and the command/ack-job release-body parity check (each of the 3 variants exactly twice). Ran the changed test file locally at this HEAD: 102/102 passed. CI green. No findings.
|
Released in v0.21.2. |
Problem
The takeover ack rode a
pull_requestlabel-event round-trip in both directions:takeover-commandtoggles the label, the label event routes, and thetakeover-ackjob posts the confirmation. Failure modes, all observed:pull_requestrun was ever created — the author read the silence as failure, removed the label at 06:58, and had to redo the whole thing) and feat(serve): page large text files by byte cursor #8002 (label applied 08:00:55, same event loss, no ack for 2+ hours)./takeoveron a stacked (non-main-base) PR dropped silently, and a non-main release never even reaches the ack job — indistinguishable from a lost event on the user's side.Change
takeover-commandnow posts the ack directly, in both directions:*/10cron).PR_INFOthe gates used. This is the only possible ack for fork and non-main releases.Dedup, all directions:
takeover_ackwhen the label sender is the bot — labeled and unlabeled paths; only the ack, the immediate scan still routes.ic.jsonsnapshot taken between the label write and the command's ack cannot double-post. A human in-repo label keeps the 3-minute ack-job grace; a human-labeled fork posts right away, as before.Manual label toggles keep today's behavior: the label-event ack job acks them, the scan heals races.
Two more silent paths become audible:
/takeoveron a stacked PR posts a bilingual base-refused explanation (re-run the command after retargeting — unlike the label-path variant, no label was applied)./takeover stopon a non-main PR now proceeds to remove the label — with a release ack — instead of leaving it permanently stuck.Tests
|| truedowngrade cannot survive).qwen-autofix-workflow.test.js: 102/102 pass; workflow YAML parses.中文说明
问题
接管回执在两个方向都依赖
pull_request标签事件回环。实测失败模式：标签事件可能根本不触发（2026-07-29 一天两起：#7999、#8002）；fork 标签事件永远无法回执（engage 只能等扫描补发，#7993 静默 2h41m，release 则完全没有回执路径）；堆叠 PR 的/takeover静默丢弃，非 main 的 release 也到不了 ack job。改动
takeover-command现在双向直发回执：打标签后发 engaged（所有准入门槛已通过；fork 变体注明首轮来自下一次定时扫描，通常几分钟内——与*/10cron 一致）；摘标签后发 released（与 ack job 相同的三个变体：普通释放 / bot 作者 / bot 作者+skip，基于门槛所用的同一份PR_INFO）——这是 fork 与非 main 释放唯一可能的回执。去重（全方向）：route 在打/摘标签者为 bot 时仅抑制回执（即时扫描照常路由）；扫描 first-pickup 宽限期按标签事件的 actor 分档——bot 打的标签只等 45 秒（命令的写入紧随标签几秒内，fork 与 in-repo 一致），因此即时扫描仍能补发失败的命令回执、不会滑到下一次定时扫描，且落在标签写入与回执之间的
ic.json快照不会双发；人工 in-repo 标签保持 3 分钟 ack-job 宽限期；人工 fork 照旧立即补发。手工打标签行为不变。另外两个静默路径改为出声：堆叠 PR 的
/takeover发双语拒绝说明；非 main 的/takeover stop现在会真正移除标签并发布释放回执。测试
toggle 回放：双向回执全变体、按语言半区断言（EN/ZH printf 参数对调不可存活）、两个告警兜底 pin（降级为
|| true不可存活）；route 回放：bot 打/摘标签均只抑制回执、人工摘标签照常回执；扫描 pin：按 actor 分档宽限期（45s/3m）；回执正文普查 12 → 17（逐条双语）；102/102 通过，YAML 解析正常。