feat(autofix): seed the takeover round counter with /takeover from N - #9321
Conversation
Taking over a PR that has already been through several review rounds restarted the Critical-only brake from zero: the round counter is window-scoped, and engaging takeover opens a fresh window, so a PR that spent nine human rounds getting to "almost mergeable" got five more suggestion-capable rounds the moment it was managed — the diff grew on nice-to-haves exactly where it should have been converging. `@qwen-code /takeover from N` now seeds the window's counter at N, so CRITICAL_ONLY_AFTER_ROUND is reached in the remainder rather than a full fresh five. This is the one parameterized command form: the literal prefix must still match TAKEOVER_COMMAND byte-for-byte, the tail is a bounded 1-2 digit integer, and the captured value reaches nothing but an integer comparison. Everything else — a prefixed body, a `stop from N` hybrid, a substitution payload — still fails closed. The seed rides as its own `<!-- autofix-round-start N -->` marker on a separate line of the engage ack, never as a field inside `<!-- takeover-ack engaged -->`. That literal is matched with jq contains(), closing `-->` included, at seven read sites — four here and three in the fleet shepherd's paused/resume detector — so an inline field would silently break all of them: the window key would fall back to an older ack and the shepherd would age out a PR that was just re-armed. Same shape as the existing autofix-redcheck marker. Both round readers fall back to the seed instead of a hardcoded 0, read it by created_at equality against the window key (so a superseded window's seed cannot leak forward), and clamp it strictly below the effective cap so a seed can never park a PR at its round cap on the very round it is taken over. The seed is window-scoped like every other census: `/retry` or a bare re-takeover returns the counter to 0. Both engage acks and the Critical-only audit record now name the seed when there is one — otherwise the ack reports "round 4/100" on its first managed round, and the audit record claims five completed rounds on a PR the loop has run twice. The growth brake is deliberately not seeded: its baseline anchors at the window's first measured round, and a pre-takeover baseline is not recoverable, so growth stays measured from engagement.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical — engaging takeover opens a fresh counting window by design (the re-arm ack itself documents the round-counter reset), so taking over a PR late in review restarts the Critical-only brake from zero and hands it a full fresh five suggestion-capable rounds at exactly the point it should be converging. That is the loop's own documented semantics biting, not a hypothetical. Direction: aligned — this is the autofix loop's brake gaining one maintainer-supplied input it cannot infer, entirely inside CI automation. No runtime, sandbox, auth, or public-contract surface involved. Size: not a core-module change ( Approach: the scope feels right — one parameterized command form, one sibling marker, two Risk: no elevated risk signals — none of the changed files match the revert-correlated paths. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:是已观测的行为而非理论假设——接管会按设计开启新的计数窗口(re-arm ack 本身就写明了轮次计数重置),因此在评审后期接管一个 PR 会把 Critical-only 刹车从零重启,恰恰在本应收敛的时点又给出完整的全新五轮 suggestion 窗口。这是循环自身已文档化语义带来的真实问题,不是假想场景。 方向:对齐——这是 autofix 循环的刹车增加了一个维护者才能提供、循环自身无法推断的输入,完全位于 CI 自动化内部,不涉及运行时、沙箱、认证或公共契约。 规模:非核心模块改动( 方案:范围合理——一个带参数的命令形式、一个 sibling 标记、两处 风险:无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent take on this problem was the same shape — parse a bounded integer after a byte-strict prefix, persist it as a sibling marker scoped to the window anchor, clamp it below the effective cap, and swap both Verified against the base workflow, not just the PR's claims:
One finding, not blocking — leading-zero seeds slip past the fail-closed gate. Not verified: no end-to-end run against a live takeover cycle — the author says so explicitly, and the mechanism here is workflow-level behavior, which neither sandboxed lane exercises, so there is no lane to name for it; the verbatim-replay tests in the suite are the substitute, and they run in CI below. Test evidenceThe PR's own CI, fetched via API at the reviewed commit — PR code is never executed during triage. The check that matters here is still running at review time: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 中文说明代码审查:我对这个问题的独立方案与该 PR 形态一致——严格按字节匹配前缀后解析有界整数、以窗口锚点作用域的 sibling 标记持久化种子、clamp 到有效上限之下、把两处 已对照 base workflow 逐项核实(而非仅采信 PR 自述): 一项发现,不阻塞——前导零种子会穿过 fail-closed 门。 未验证:没有针对真实托管周期的端到端跑通——作者已明示;该机制属于 workflow 层行为,两个沙箱通道均无法演练,故无可指名的通道;套件中的逐字回放测试即其替代,正在下方 CI 中运行。 测试证据:通过 API 读取该 PR 自身在受审 commit 上的 CI(triage 全程不执行 PR 代码)。关键检查仍在运行: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal, exceptionally well-tested; the only reservation is the leading-zero seed edge named in the review, which is worth a one-line hardening but doesn't gate a maintainer-only surface. Stepping back: the approach is exactly what I'd have proposed independently — one bounded parameter, one window-scoped sibling marker, two fallback swaps — and it goes beyond that in the places that matter (boundary re-validation, audit wording, The one thing I'd want fixed soon, if not in this PR: Approval deferred until CI lands green on the reviewed commit — the main unit suite (which carries this PR's new workflow tests) is still in flight. The finalize job posts the commit-pinned approval once every check settles green, and withholds it if anything lands red or the head moves. 中文说明总体评价:4/5 —— 干净、最小化、测试纪律极其出色;唯一保留意见是审查中指名的前导零种子边角,值得用一行加固,但不足以拦住一个维护者专用的表面。 退一步看:方案与我独立设想完全一致——一个有界参数、一个窗口作用域的 sibling 标记、两处回退替换——且在关键处更进一步(job 边界复验、审计措辞、 唯一希望尽快修掉(哪怕不在本 PR 内)的问题: 批准推迟到 CI 在受审 commit 上全绿——承载本 PR 新 workflow 测试的主单测套件仍在运行。finalize 任务会在全部检查绿了之后发布绑定该 commit 的批准;任何一项变红或 head 移动则不会批准。 — Qwen Code · qwen3.8-max 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 /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 冲突,直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
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.8-max via Qwen Code /review (v0.21.13)
| # '0' under string comparison, so a bare `!= '0'` renders the | ||
| # seeded wording on every ordinary PR. | ||
| if [[ "${LIVE_ROUND_START:-0}" != '0' ]]; then | ||
| ROUNDS_CLAUSE_EN="the round counter reached ${CRITICAL_ONLY_AFTER_ROUND} (this window was seeded at round ${LIVE_ROUND_START} by \`${TAKEOVER_COMMAND} from ${LIVE_ROUND_START}\`, plus $(( ROUND - LIVE_ROUND_START )) change-producing round(s) since)" |
There was a problem hiding this comment.
[Suggestion] The seeded audit clause interpolates the post-clamp LIVE_ROUND_START into both the "seeded at round N" number and the quoted command — whenever the read-site clamp fires, the escalation record cites a command nobody typed, contradicting the engage ack still visible on the PR. This is the exact "unfalsifiable for the maintainer reading it" confusion this clause was added to prevent. — Failure scenario: a maintainer engages with @qwen-code /takeover from 12; the takeover label is later removed while the window anchor keeps its marker (the path the clamp's own comment names, dropping EFF_MAX_ROUNDS back to the strict 10) → both readers clamp 12→9 → the brake fires on the first managed round and the audit renders "seeded at round 9 by @qwen-code /takeover from 9" while the ack above it says 12.
Witness (verbatim LIVE read+clamp and cause-block replay against an ack fixture seeded from 12):
=== MAX_ROUNDS=10 (takeover label removed; strict cap) ===
LIVE_ROUND_START=9
CAUSE_EN=the round counter reached 5 (this window was seeded at round 9 by `@qwen-code /takeover from 9`, plus 0 change-producing round(s) since)
=== MAX_ROUNDS=100 (label present) ===
LIVE_ROUND_START=12
CAUSE_EN=... seeded at round 12 by `@qwen-code /takeover from 12` ...
Suggested fix: keep the pre-clamp value before clamping (e.g. LIVE_ROUND_START_RAW) and cite it in the clause — "seeded at round 12 (clamped to 9 under the effective cap 10), plus N change-producing round(s) since" — keeping the arithmetic on the clamped value.
中文说明
带种子的审计子句把 clamp 之后的 LIVE_ROUND_START 同时插入"seeded at round N"的数字与被引用的命令——一旦读取点的 clamp 触发,升级审计记录就会引用一条没人发过的命令,与 PR 上仍然可见的 engage ack 相矛盾。这正是本子句声称要防止的"维护者无法证伪"的混乱。失败场景:维护者以 @qwen-code /takeover from 12 接管;随后 takeover 标签被移除而窗口锚点的标记仍在(正是 clamp 自身注释所描述的路径,EFF_MAX_ROUNDS 回落为严格的 10)→ 两处读取都把 12 clamp 到 9 → 刹车在首个托管轮次触发,审计记录渲染 "seeded at round 9 by @qwen-code /takeover from 9",而上方的 ack 写的是 12。建议修复:在 clamp 之前保留原始值(如 LIVE_ROUND_START_RAW),措辞中引用它——"seeded at round 12 (clamped to 9 under the effective cap 10),此后又完成 N 个产生改动的轮次"——算术仍使用 clamp 后的值。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| # read against the live window key, same clamp. MAX_ROUNDS is the | ||
| # matrix-shadowed EFFECTIVE cap here (see the address job's env), so | ||
| # the clamp is against the same ceiling the scan used. | ||
| LIVE_ROUND_START="$(jq -r --arg ab "${AUTOFIX_BOT}" --arg key "${LIVE_REARM_KEY}" ' |
There was a problem hiding this comment.
[Suggestion] The seed read + shape gate + clamp now exists in two near-verbatim copies (scan's ROUND_START at ~3317 and this prepare-side LIVE_ROUND_START), but only the scan copy's floor path is exercised by a behavioral test: the seed test's harness leaves EFF_MAX_ROUNDS unset so both clamp branches are inert in every fixture, the address-side mirror test sets no MAX_ROUNDS and its fixtures carry no autofix-round-start marker, and the cause test injects LIVE_ROUND_START as a canonical value instead of executing this read. — Failure scenario: a future edit that breaks only one copy — deleting either clamp block, flipping -ge to -le (which in production would clamp every seeded window to cap−1), or drifting the LIVE-side jq predicate — passes the whole suite, and scan selection and the live Critical-only brake compute different effective rounds for the same seeded window, engaging the brake inconsistently with no error signal. Verified: both mutants leave 176/176 tests green.
Witness (mutation runs, each reverted): deleting the LIVE-side clamp → Tests 176 passed (176); flipping the scan-side -ge → -le → Tests 176 passed (176).
Suggested fix: set EFF_MAX_ROUNDS in the seed-test harness env and add a seed-≥-cap fixture (e.g. ack(K1, 15) → '9' clamped, ack(K1, 9) → '9' unclamped); add a marker-bearing fixture to the address-side mirror test so both copies are compared on a seeded window, including the clamp case.
中文说明
种子读取 + 形状门控 + clamp 现在以近乎逐字的两份拷贝存在(扫描侧 ~3317 的 ROUND_START 与此处 prepare 侧的 LIVE_ROUND_START),但只有扫描拷贝的下界路径被行为测试执行:种子测试的 harness 未设置 EFF_MAX_ROUNDS,因此两处 clamp 分支在所有 fixture 中都是惰性的;address 侧镜像测试未设置 MAX_ROUNDS 且其 fixture 不含 autofix-round-start 标记;cause 测试直接注入规范值 LIVE_ROUND_START 而不执行这段读取。失败场景:未来只破坏其中一份拷贝的改动——删除任一 clamp 块、把 -ge 翻成 -le(生产上会把每个带种子窗口 clamp 到 cap−1)、或 LIVE 侧 jq 谓词漂移——都能在全套测试绿灯下合入,扫描选择与实时 Critical-only 刹车将对同一个带种子窗口算出不同的有效轮次。已验证:两种变异均保持 176/176 全绿。建议修复:在种子测试 harness 的 env 中设置 EFF_MAX_ROUNDS 并新增种子 ≥ 上限的 fixture(如 ack(K1, 15) → '9' 被 clamp、ack(K1, 9) → '9' 不被 clamp);在 address 侧镜像测试中加入带标记的 fixture,使两份拷贝在带种子窗口上被对比,包括 clamp 分支。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if [[ -n "${CMD_FROM}" && "${CMD_FROM}" =~ ^[0-9]{1,2}$ && "${CMD_FROM}" != '0' ]]; then | ||
| FROM_MARKER="$(printf '\n<!-- autofix-round-start %s -->' "${CMD_FROM}")" |
There was a problem hiding this comment.
[Suggestion] The marker write side (ack emission) is untested — the parser test proves TAKEOVER_FROM is extracted and re-validated, and the seed-read test proves a hand-crafted marker on the window anchor is honored, but nothing verifies the takeover job actually emits the marker into the ack bodies. CMD_FROM/FROM_MARKER/FROM_NOTE appear nowhere in the test file; the runToggle harness already extracts this toggle block verbatim and records full comment bodies, yet no case sets CMD_FROM, so the integration seam between the tested parser and the tested reader is unpinned. — Failure scenario: dropping %s/${FROM_MARKER} from either ack printf, breaking the leading \n, or removing the != '0' guard leaves every test green while the seed parses, crosses the job boundary, and is silently dropped — the feature degrades to unseeded behavior end-to-end. Verified: the mutant leaves 176/176 tests green.
Witness (mutation run, reverted): removing %s/${FROM_MARKER} from both ack printfs → Tests 176 passed (176).
Suggested fix: add a cmdFrom knob to runToggle's env and assert: with cmdFrom: '3', both the engage and re-arm recorded bodies contain <!-- takeover-ack engaged -->\n<!-- autofix-round-start 3 -->; with ''/'0'/'abc', no autofix-round-start appears.
中文说明
标记的写入侧(ack 渲染)没有测试——解析器测试证明 TAKEOVER_FROM 被提取并复验,种子读取测试证明手工构造的窗口锚点标记会被采纳,但没有任何测试验证 takeover job 真的把标记写进了 ack 正文。CMD_FROM/FROM_MARKER/FROM_NOTE 在测试文件中零出现;runToggle harness 已经逐字提取该 toggle 块并记录完整评论正文,却没有一个用例设置 CMD_FROM,因此"已测解析器"与"已测读取器"之间的集成缝隙无人钉住。失败场景:从任一 ack printf 中删掉 %s/${FROM_MARKER}、破坏前导 \n、或移除 != '0' 门,都能在全套测试绿灯下让种子被解析、越过 job 边界、然后被静默丢弃——整个特性退化为无种子行为。已验证:该变异保持 176/176 全绿。建议修复:给 runToggle 的 env 增加 cmdFrom 旋钮并断言:cmdFrom: '3' 时 engage 与 re-arm 记录的正文都包含 <!-- takeover-ack engaged -->\n<!-- autofix-round-start 3 -->;''/'0'/'abc' 时不出现 autofix-round-start。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // The whole point: a PR taken over at round 3 starts there, so with | ||
| // CRITICAL_ONLY_AFTER_ROUND=5 the brake is two managed rounds away | ||
| // instead of five. | ||
| expect(roundOf([ack(K1, 3)])).toBe('3'); |
There was a problem hiding this comment.
[Suggestion] The documented "last marker wins" property of the seed reader is unpinned — both read sites use scan(...) | last, and the workflow comment documents that a hand-written marker further down a bot comment loses to the workflow's own final-line marker, but no fixture places two autofix-round-start markers in one comment, so the one-line mutation | last → | first survives the whole suite. — Failure scenario: a write-access user edits the bot's engage ack to prepend <!-- autofix-round-start 99 --> (GitHub edits preserve user.login and created_at, so the comment still passes both filter halves); last yields the workflow's final value, first yields 99, and the tests cannot distinguish. Verified: the mutation leaves 176/176 green; adding one two-marker fixture turns the seed test RED under the mutation (expected '99' to be '3') and GREEN once the workflow is restored.
| expect(roundOf([ack(K1, 3)])).toBe('3'); | |
| expect(roundOf([ack(K1, 3)])).toBe('3'); | |
| // Last marker wins: a hand-written marker prepended by an edit loses to | |
| // the workflow's own final-line marker (edits keep user.login/created_at). | |
| expect( | |
| roundOf([ | |
| { | |
| user: { login: BOT }, | |
| created_at: K1, | |
| body: 'edited <!-- autofix-round-start 99 -->\n<!-- takeover-ack engaged -->\n<!-- autofix-round-start 3 -->', | |
| }, | |
| ]), | |
| ).toBe('3'); |
中文说明
种子读取器"最后一个标记生效"的既有语义无人钉住——两处读取点都用 scan(...) | last,workflow 注释也写明 bot 评论中更靠上手写的标记会让位于 workflow 自己的末行标记,但没有任何 fixture 在一句评论里放两个 autofix-round-start 标记,因此单行变异 | last → | first 能在全套测试下存活。失败场景:写权限用户编辑 bot 的 engage ack,在前面塞入 <!-- autofix-round-start 99 -->(GitHub 编辑不改变 user.login 与 created_at,该评论仍通过两半过滤);last 得到 workflow 的末行值,first 得到 99,测试无法区分。已验证:变异下 176/176 全绿;加入一个双标记 fixture 后,变异下种子测试变红(expected '99' to be '3'),workflow 还原后变绿。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| # no label churn needed. The watermark is untouched: feedback | ||
| # already addressed is never replayed. | ||
| gh pr comment "${PR}" --repo "${REPO}" --body "$(printf '🔄 Takeover re-armed: the round counter starts a fresh window (previous rounds no longer count toward the cap); management continues.\n\n<details>\n<summary>中文说明</summary>\n\n🔄 已重新武装:轮次计数开启新窗口(此前轮次不再计入上限),托管继续。\n\n</details>\n\n<!-- takeover-ack engaged -->')" | ||
| gh pr comment "${PR}" --repo "${REPO}" --body "$(printf '🔄 Takeover re-armed: the round counter starts a fresh window (previous rounds no longer count toward the cap); management continues.%s\n\n<details>\n<summary>中文说明</summary>\n\n🔄 已重新武装:轮次计数开启新窗口(此前轮次不再计入上限),托管继续。%s\n\n</details>\n\n<!-- takeover-ack engaged -->%s' "${FROM_NOTE}" "${FROM_NOTE_ZH}" "${FROM_MARKER}")" |
There was a problem hiding this comment.
[Suggestion] The re-arm ack contradicts itself when seeded: the hardcoded "(previous rounds no longer count toward the cap)" / 「此前轮次不再计入上限」 clause is false whenever this diff appends FROM_NOTE/FROM_MARKER, because the seed makes prior rounds consume both the cap budget and the Critical-only brake budget. FROM_NOTE's parenthetical "(the rounds this PR spent in review before takeover)" also misdescribes the re-arm case, where the seeded rounds were typically already-managed rounds. — Failure scenario: a maintainer re-arms an already-managed PR with @qwen-code /takeover from 60 (the documented primary use of from N) — the same ack simultaneously says "previous rounds no longer count toward the cap" and "counter starts at 60 … engages after 0 more change-producing round(s)", while only 40 of 100 cap rounds actually remain.
Witness (verbatim replay of the takeover-command step, CMD=add, CMD_FROM=60, label already present):
🔄 Takeover re-armed: the round counter starts a fresh window (previous rounds
no longer count toward the cap); management continues. This window's round
counter starts at 60 (the rounds this PR spent in review before takeover), so
the Critical-only brake engages after 0 more change-producing round(s) ...
(EN and ZH both carry the contradiction; reverting this line to the diff's - form removes it, so the inconsistency is attributable to this diff's combination.)
Suggested fix: when CMD_FROM is set, reword the fresh-window clause on the re-arm path (e.g. "the round counter restarts at N; earlier rounds count toward the cap only via this seed"), and adjust FROM_NOTE's parenthetical for the re-arm case ("rounds already spent on this PR").
中文说明
带种子的重新武装 ack 自相矛盾:只要本 diff 追加 FROM_NOTE/FROM_MARKER,写死的 "(previous rounds no longer count toward the cap)" / 「此前轮次不再计入上限」就是假的——种子会让此前轮次同时占用上限预算与 Critical-only 刹车预算。FROM_NOTE 的括号说明"(即本 PR 托管前已进行的评审轮数)"在重新武装场景也不准确:被种子化的轮次通常已是托管轮次。失败场景:维护者对已托管 PR 重新下达 @qwen-code /takeover from 60(from N 的主要用途)——同一条 ack 既说"此前轮次不再计入上限",又说"计数从 60 起算……再经过 0 轮即进入 Critical-only",而 100 轮上限实际只剩 40 轮。建议修复:CMD_FROM 非空时改写 re-arm 路径的新窗口措辞(如"轮次计数从 N 重启;此前轮次仅通过该种子计入上限"),并为 re-arm 场景调整 FROM_NOTE 的括号说明("本 PR 已消耗的轮次")。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| echo "🔢 #${PR}: round seed ${ROUND_START} clamped to $(( EFF_MAX_ROUNDS - 1 )) (effective cap ${EFF_MAX_ROUNDS})" | ||
| ROUND_START=$(( EFF_MAX_ROUNDS - 1 )) | ||
| fi | ||
| ROUND="$(jq -r --arg key "${REARM_KEY}" --argjson start "${ROUND_START}" 'map(select(.win == $key)) | map(.round) | max // $start' <<< "${MARKERS}")" |
There was a problem hiding this comment.
[Suggestion] The takeover milestone digest's crossing trigger (unchanged, ~6520-6533) keys on the absolute round counter (NEXT_ROUND >= 10 && NEXT_ROUND - MS_LAST >= 10), but this diff's seed floor (max // $start) inflates that counter with pre-takeover rounds. Pre-diff, every window opened at ROUND=0, so the test was equivalent to "10+ window rounds"; the seed breaks that equivalence while the digest's census stays window-scoped. — Failure scenario: @qwen-code /takeover from 60 → within the window's first two managed rounds the PR receives "📊 Takeover milestone — round 61/100 … This many rounds deserves a human look" while its own census shows 1-2 bot rounds — the same "true of the counter, visibly false of the PR" shape this diff's audit-clause wording exists to prevent. Seeds 1-8 fire the first digest early by N rounds.
Witness (verbatim milestone-block replay with stubbed gh): seed 60 + one prior noop marker → DIGEST POSTED — "📊 Takeover milestone — round 61/100, in the current window. Census: 1 pushed fix(es), 1 no-change review(s), 0 timeout(s)" after 2 managed rounds; seed 8 → posts round 10/100 … Census: 2 pushed fix(es); unseeded control posts at round 10. Flipping both tests to window-relative rounds silences the seeded cases and keeps the control posting.
Suggested fix: key the crossing trigger to rounds accumulated in the window — e.g. additionally require the window's marker count ≥ 10 (the census already enumerates WIN_HEADS), or subtract the window's opening round (seed / first marker) before comparing.
中文说明
takeover 里程碑摘要的触发条件(未改动,~6520-6533)以绝对轮次计数为键(NEXT_ROUND >= 10 && NEXT_ROUND - MS_LAST >= 10),而本 diff 的种子下界(max // $start)会把托管前的轮次灌进这个计数器。改动前每个窗口都从 ROUND=0 开启,该条件等价于"窗口内累计 10+ 轮";种子打破了这个等价关系,而摘要的普查统计仍是窗口作用域。失败场景:@qwen-code /takeover from 60 → 窗口的前两个托管轮次内,PR 就收到"📊 Takeover milestone — round 61/100 … 这么多轮值得人工看一眼",而其自身普查只显示 1-2 个 bot 轮次——与本 diff 审计措辞要消除的"对计数器为真、对 PR 显然为假"同形。种子 1-8 会让首次摘要提前 N 轮触发。建议修复:把触发条件改为窗口内累计轮次——例如额外要求窗口标记数 ≥ 10(普查已在枚举 WIN_HEADS),或在比较前减去窗口开启轮次(种子/首个标记)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| FORK_NOTE_ZH='本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。' | ||
| fi | ||
| gh pr comment "${PR}" --repo "${REPO}" --body "$(printf '🤝 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.%s Remove the `%s` label (or comment `%s stop`) to release.\n\n<details>\n<summary>中文说明</summary>\n\n🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。%s移除 `%s` 标签(或评论 `%s stop`)即可释放。\n\n</details>\n\n<!-- takeover-ack engaged -->' "${FORK_NOTE}" "${TAKEOVER_LABEL}" "${TAKEOVER_COMMAND}" "${FORK_NOTE_ZH}" "${TAKEOVER_LABEL}" "${TAKEOVER_COMMAND}")" \ | ||
| gh pr comment "${PR}" --repo "${REPO}" --body "$(printf '🤝 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.%s%s Remove the `%s` label (or comment `%s stop`) to release.\n\n<details>\n<summary>中文说明</summary>\n\n🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。%s%s移除 `%s` 标签(或评论 `%s stop`)即可释放。\n\n</details>\n\n<!-- takeover-ack engaged -->%s' "${FORK_NOTE}" "${FROM_NOTE}" "${TAKEOVER_LABEL}" "${TAKEOVER_COMMAND}" "${FORK_NOTE_ZH}" "${FROM_NOTE_ZH}" "${TAKEOVER_LABEL}" "${TAKEOVER_COMMAND}" "${FROM_MARKER}")" \ |
There was a problem hiding this comment.
[Suggestion] Heal-path seed loss: if this seeded engage ack's gh pr comment post fails (a transient 5xx/rate-limit — the exact case the || echo "::warning::... heals it" fallback on the next line covers), the scan's first-pickup heal ack carries no seed-marker slot and cannot see CMD_FROM (wired only into the takeover-command job's env). The healed ack becomes the window anchor (REARM_KEY = its created_at) and both seed reads only inspect the window-key comment — so the from N seed is permanently and silently dropped. — Failure scenario: @qwen-code /takeover from 12 → the label is applied first (~2053), then the ack post fails transiently → the next scan heals with the pre-feature unseeded ack → the counter starts at 0 and the brake engages after a full fresh 5 rounds instead of the remainder requested. Nothing logs the loss (the only seed log line prints when a seed IS found); recovery (re-issue the command) exists but is un-signalled.
Verified link-by-link at the reviewed commit: label POST precedes the comment POST and the failure is swallowed by the warn fallback; the heal printf (~3224) is the pre-feature body with no slot; both readers filter created_at == $key; and the repo's own behavioral test confirms the shape — roundOf([ack(K1)]) → '0': a window anchored by an unseeded ack reads seed 0.
Suggested fix: retry the engage ack post once inside the ack job before falling back to the warning (e.g. gh pr comment ... || { sleep 5; gh pr comment ...; } || echo "::warning::...") so the transient case keeps the seed — the heal path cannot recover a number that only ever lived in the failed comment body.
中文说明
修复路径会丢种子:如果这条带种子的 engage ack 的 gh pr comment 发送失败(瞬时 5xx/限流——正是下一行 || echo "::warning::... heals it" 兜底所覆盖的场景),扫描首次接管时补发的 heal ack 没有种子标记槽位,也看不到 CMD_FROM(它只接线进 takeover-command job 的 env)。补发的 ack 会成为窗口锚点(REARM_KEY = 其 created_at),而两处种子读取只看窗口 key 评论——于是 from N 种子被永久且静默地丢弃。失败场景:@qwen-code /takeover from 12 → 标签先被应用(~2053),随后 ack 发送瞬时失败 → 下一次扫描用改动前的无种子 ack 补发 → 计数从 0 开始,刹车在完整的全新 5 轮后才触发,而非请求的剩余轮次。没有任何日志记录这次丢失(唯一的种子日志只在发现种子时打印);恢复手段(重新下发命令)存在但无任何提示。建议修复:在 ack job 内先重试一次再落到 warning(如 gh pr comment ... || { sleep 5; gh pr comment ...; } || echo "::warning::..."),让瞬时失败保住种子——heal 路径无法恢复一个只存在于失败评论正文里的数字。
— qwen3.8-max via Qwen Code /review (v0.21.13)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: R2 address-review summary — PR #9321 (takeover round seed)Round 1 review posted 7 Suggestion findings (all inline, automated Findings and dispositions
The review body's "not linted (actionlint embedded-shell limitation)" note is Scope disciplineAll changes stay inside the PR's own footprint (the autofix workflow and its Verification
中文说明R2 评审处理总结 — PR #9321(takeover 轮次种子)第 1 轮评审共提出 7 条 Suggestion 发现(全部为行内评论,来自自动评审器),另有一条无阻断问题的评审正文。七条发现已在单个提交( 发现与处置
评审正文中“未检查(actionlint 内嵌 shell 映射的工具限制)”一条是工具限制,不是发现——无需处理。 范围纪律所有改动均留在 PR 自身足迹内(autofix workflow 及其行为测试文件)。修复追求根因、能减则减:重复的剩余轮数算术合并为一个 验证
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
There was a problem hiding this comment.
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.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-autofix.yml:4771 — [review] The LIVE seed reader's author/created_at trust predicates have no negative fixture (scan-side twin is pinned).github/workflows/qwen-autofix.yml:2093 — [review] The double-failure warning never names the seed loss it causes.github/workflows/qwen-autofix.yml:3335 — [review] The comment states last 's marker resolution backwards (an appended marker wins, not loses)
中文说明
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| FROM_MARKER="$(printf '\n<!-- autofix-round-start %s -->' "${CMD_FROM}")" | ||
| REARM_FRESH_CLAUSE=' (earlier rounds count toward the cap only via this seed)' | ||
| REARM_FRESH_CLAUSE_ZH='(此前轮次仅通过该种子计入上限)' | ||
| FROM_REMAIN="$(( CRITICAL_ONLY_AFTER_ROUND > CMD_FROM ? CRITICAL_ONLY_AFTER_ROUND - CMD_FROM : 0 ))" |
There was a problem hiding this comment.
[Critical] Zero-padded seeds pass every gate but break the toggle job's bash arithmetic — from 08 / from 09 are invalid octal.
The seed grammar admitted by the route parser ([0-9]{1,2}), sanitize_number (^[0-9]+$), and this job's gate (=~ ^[0-9]{1,2}$ + != '0') accept leading-zero spellings, but the value then reaches bare-context bash arithmetic at this line, where a leading zero means octal. No site canonicalizes to decimal.
— Failure scenario: an authorized maintainer comments @qwen-code /takeover from 08 — squarely inside the command's documented "bounded 1-2 digit integer" contract. Every gate passes; the arithmetic raises bash: 08: value too great for base, and on the runner's bash the failing expansion inside the multi-line if skips the rest of the then-body and continues after fi: the step exits green, the label POSTs, and the engage ack POSTs with the marker <!-- autofix-round-start 08 --> but no seed note (FROM_NOTE* never built) — a silently seeded window that reads as unseeded. Downstream, the clamp [[ ... -ge ... ]] errors and is silently skipped, and the Critical-only audit clause re-renders the unseeded "5 change-producing rounds are complete" wording on a seeded PR — the exact misstatement this feature exists to prevent. from 00 sails past != '0' and posts seeded wording while behaving as 0; from 01–07 survive (octal == decimal), which is why this is easy to miss. The parser fail-closed set and the cmdFrom test set never exercise a leading-zero value.
Witness (verbatim toggle-step replay under bash --noprofile --norc -eo pipefail):
CMD_FROM='08' -> exit_code=0; stderr: bash: line 114: 08: value too great for base
marker posted: <!-- autofix-round-start 08 -->; seed wording: NONE
audit replay LIVE_ROUND_START=08 -> CAUSE_EN=[5 change-producing rounds are complete]
with CMD_FROM="$((10#${CMD_FROM}))" inserted -> marker 8, "starts at 8", stderr []
Suggested fix — canonicalize once at capture in the route parser (~line 550), so every downstream consumer sees one canonical decimal:
TAKEOVER_FROM="$((10#${BASH_REMATCH[2]}))"(08→8; 00→0, which then correctly hits the existing != '0' no-seed gate.) Alternatively reject zero-padded forms at the parser ((0|[1-9][0-9]?), keeping from 0 as the explicit no-seed spelling). Optionally also apply 10# at the arithmetic read sites as defense against hand-edited markers — the repo already uses this exact defense elsewhere (qwen-fleet-shepherd.yml AUTO_RELEASE_DAYS=$((10#${AUTO_RELEASE_DAYS})); this same workflow documents the 10# octal hazard at ~5652). Add from 08/from 00 to the parser's test cases.
中文说明
[Critical] 零填充种子能通过所有门控,但会破坏 toggle job 的 bash 算术 —— from 08 / from 09 是非法八进制。
route 解析器允许的语法([0-9]{1,2})、sanitize_number(^[0-9]+$)以及本 job 的门(=~ ^[0-9]{1,2}$ + != '0')都接受零填充写法,但该值随后到达本行的裸上下文 bash 算术——前导零意味着八进制。没有任何一处把它规范化为十进制。
— 失败场景:有权限的维护者评论 @qwen-code /takeover from 08 —— 这完全在该命令自己文档化的"有界 1-2 位整数"契约之内。所有门控通过;算术抛出 bash: 08: value too great for base,而 runner 的 bash 在多行 if 中遇到失败的展开时会跳过 then 体剩余部分、从 fi 之后继续:步骤以绿色退出,标签被 POST,engage ack 带着标记 <!-- autofix-round-start 08 --> 但没有任何种子说明(FROM_NOTE* 从未构建)—— 一个被静默种子化、却读起来像未种子化的窗口。下游的 clamp [[ ... -ge ... ]] 报错并被静默跳过,Critical-only 审计子句会在带种子的 PR 上重新渲染无种子措辞 "5 change-producing rounds are complete" —— 正是本特性要消除的错述。from 00 穿过 != '0',发布种子化措辞却按 0 行事;from 01–07 存活(八进制 == 十进制),因此极易漏测。解析器 fail-closed 测试集与 cmdFrom 测试集从未覆盖任何零填充值。
证据(逐字提取 toggle 步骤、在 bash --noprofile --norc -eo pipefail 下回放):
CMD_FROM='08' -> exit_code=0; stderr: bash: line 114: 08: value too great for base
marker posted: <!-- autofix-round-start 08 -->; seed wording: NONE
audit replay LIVE_ROUND_START=08 -> CAUSE_EN=[5 change-producing rounds are complete]
with CMD_FROM="$((10#${CMD_FROM}))" inserted -> marker 8, "starts at 8", stderr []
建议修复 —— 在 route 解析器捕获时(~550 行)一次性规范化,让所有下游消费者看到唯一的规范十进制:
TAKEOVER_FROM="$((10#${BASH_REMATCH[2]}))"(08→8;00→0,从而正确命中既有的 != '0' 无种子门。)也可以在解析器处直接拒绝零填充形式((0|[1-9][0-9]?),保留 from 0 作为显式无种子写法)。可选地在算术读取点也加 10#,防御手工编辑的标记 —— 仓库其他位置已有完全相同的防御(qwen-fleet-shepherd.yml AUTO_RELEASE_DAYS=$((10#${AUTO_RELEASE_DAYS}));本 workflow ~5652 处也记载了 10# 八进制陷阱)。在解析器测试中补 from 08/from 00 用例。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| ack_pr: '${{ steps.decide.outputs.ack_pr }}' | ||
| ack_base: '${{ steps.decide.outputs.ack_base }}' | ||
| takeover_cmd: '${{ steps.decide.outputs.takeover_cmd }}' | ||
| takeover_from: '${{ steps.decide.outputs.takeover_from }}' |
There was a problem hiding this comment.
[Suggestion] The seed crosses three YAML job-boundary wires that no test pins — this line is one of the four.
Every behavioral harness injects the value directly (CMD_FROM: cmdFrom, ROUND_START: roundStart), because the suite's verbatim-replay harnesses start inside a single job — so deleting or typo-ing any one wire keeps the entire suite green while the shipped feature silently degrades.
— Concrete cost: /takeover from N behaves exactly like bare /takeover (or the digest baseline silently reverts to 0), with no error anywhere; the integration seam between the tested parser and the tested readers is unpinned end-to-end. The suite pins equivalent wires elsewhere (takeover_ack: outputs mapping, FORCED_PR: env).
The four wires: workflow lines 373, 1910, 5190, 5985 (route outputs.takeover_from → toggle CMD_FROM; prepare round_start output → address ROUND_START).
Suggested fix: add toContain pins for the four wiring lines, matching the suite's existing wire-pin convention.
中文说明
[Suggestion] 种子要穿过三条跨 job 的 YAML 接线,而没有任何测试钉住它们 —— 本行是四处之一。
所有行为测试 harness 都直接注入变量值(CMD_FROM: cmdFrom、ROUND_START: roundStart),因为套件逐字回放 harness 从单个 job 内部起步 —— 于是删除或写错任何一条接线,整套测试依旧绿灯,而上线的特性静默退化。
— 具体代价:/takeover from N 表现得与裸 /takeover 完全相同(或 digest 基线静默回落到 0),且没有任何报错;已测解析器与已测读取器之间的集成缝隙端到端无人钉住。套件在其他地方钉住了等价接线(takeover_ack: outputs 映射、FORCED_PR: env)。
四条接线:workflow 第 373、1910、5190、5985 行(route outputs.takeover_from → toggle CMD_FROM;prepare round_start 输出 → address ROUND_START)。
建议修复:为这四条接线行添加 toContain 钉住,与套件既有的接线钉住惯例保持一致。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| env: | ||
| REPO: '${{ github.repository }}' | ||
| CMD: '${{ needs.route.outputs.takeover_cmd }}' | ||
| CMD_FROM: '${{ needs.route.outputs.takeover_from }}' |
There was a problem hiding this comment.
[Suggestion] The seed crosses three YAML job-boundary wires that no test pins — this line is one of the four.
Every behavioral harness injects the value directly (CMD_FROM: cmdFrom, ROUND_START: roundStart), because the suite's verbatim-replay harnesses start inside a single job — so deleting or typo-ing any one wire keeps the entire suite green while the shipped feature silently degrades.
— Concrete cost: /takeover from N behaves exactly like bare /takeover (or the digest baseline silently reverts to 0), with no error anywhere; the integration seam between the tested parser and the tested readers is unpinned end-to-end. The suite pins equivalent wires elsewhere (takeover_ack: outputs mapping, FORCED_PR: env).
The four wires: workflow lines 373, 1910, 5190, 5985 (route outputs.takeover_from → toggle CMD_FROM; prepare round_start output → address ROUND_START).
Suggested fix: add toContain pins for the four wiring lines, matching the suite's existing wire-pin convention.
中文说明
[Suggestion] 种子要穿过三条跨 job 的 YAML 接线,而没有任何测试钉住它们 —— 本行是四处之一。
所有行为测试 harness 都直接注入变量值(CMD_FROM: cmdFrom、ROUND_START: roundStart),因为套件逐字回放 harness 从单个 job 内部起步 —— 于是删除或写错任何一条接线,整套测试依旧绿灯,而上线的特性静默退化。
— 具体代价:/takeover from N 表现得与裸 /takeover 完全相同(或 digest 基线静默回落到 0),且没有任何报错;已测解析器与已测读取器之间的集成缝隙端到端无人钉住。套件在其他地方钉住了等价接线(takeover_ack: outputs 映射、FORCED_PR: env)。
四条接线:workflow 第 373、1910、5190、5985 行(route outputs.takeover_from → toggle CMD_FROM;prepare round_start 输出 → address ROUND_START)。
建议修复:为这四条接线行添加 toContain 钉住,与套件既有的接线钉住惯例保持一致。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| fi | ||
| echo "stale=${STALE}" >> "${GITHUB_OUTPUT}" | ||
| echo "effective_round=${ROUND}" >> "${GITHUB_OUTPUT}" | ||
| echo "round_start=${LIVE_ROUND_START}" >> "${GITHUB_OUTPUT}" |
There was a problem hiding this comment.
[Suggestion] The seed crosses three YAML job-boundary wires that no test pins — this line is one of the four.
Every behavioral harness injects the value directly (CMD_FROM: cmdFrom, ROUND_START: roundStart), because the suite's verbatim-replay harnesses start inside a single job — so deleting or typo-ing any one wire keeps the entire suite green while the shipped feature silently degrades.
— Concrete cost: /takeover from N behaves exactly like bare /takeover (or the digest baseline silently reverts to 0), with no error anywhere; the integration seam between the tested parser and the tested readers is unpinned end-to-end. The suite pins equivalent wires elsewhere (takeover_ack: outputs mapping, FORCED_PR: env).
The four wires: workflow lines 373, 1910, 5190, 5985 (route outputs.takeover_from → toggle CMD_FROM; prepare round_start output → address ROUND_START).
Suggested fix: add toContain pins for the four wiring lines, matching the suite's existing wire-pin convention.
中文说明
[Suggestion] 种子要穿过三条跨 job 的 YAML 接线,而没有任何测试钉住它们 —— 本行是四处之一。
所有行为测试 harness 都直接注入变量值(CMD_FROM: cmdFrom、ROUND_START: roundStart),因为套件逐字回放 harness 从单个 job 内部起步 —— 于是删除或写错任何一条接线,整套测试依旧绿灯,而上线的特性静默退化。
— 具体代价:/takeover from N 表现得与裸 /takeover 完全相同(或 digest 基线静默回落到 0),且没有任何报错;已测解析器与已测读取器之间的集成缝隙端到端无人钉住。套件在其他地方钉住了等价接线(takeover_ack: outputs 映射、FORCED_PR: env)。
四条接线:workflow 第 373、1910、5190、5985 行(route outputs.takeover_from → toggle CMD_FROM;prepare round_start 输出 → address ROUND_START)。
建议修复:为这四条接线行添加 toContain 钉住,与套件既有的接线钉住惯例保持一致。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| # The seed the window opened at (prepare's clamped read; 0 when | ||
| # unseeded): the milestone crossing trigger counts rounds | ||
| # accumulated in the window, not seed-inflated absolute ones. | ||
| ROUND_START: '${{ steps.prepare.outputs.round_start }}' |
There was a problem hiding this comment.
[Suggestion] The seed crosses three YAML job-boundary wires that no test pins — this line is one of the four.
Every behavioral harness injects the value directly (CMD_FROM: cmdFrom, ROUND_START: roundStart), because the suite's verbatim-replay harnesses start inside a single job — so deleting or typo-ing any one wire keeps the entire suite green while the shipped feature silently degrades.
— Concrete cost: /takeover from N behaves exactly like bare /takeover (or the digest baseline silently reverts to 0), with no error anywhere; the integration seam between the tested parser and the tested readers is unpinned end-to-end. The suite pins equivalent wires elsewhere (takeover_ack: outputs mapping, FORCED_PR: env).
The four wires: workflow lines 373, 1910, 5190, 5985 (route outputs.takeover_from → toggle CMD_FROM; prepare round_start output → address ROUND_START).
Suggested fix: add toContain pins for the four wiring lines, matching the suite's existing wire-pin convention.
中文说明
[Suggestion] 种子要穿过三条跨 job 的 YAML 接线,而没有任何测试钉住它们 —— 本行是四处之一。
所有行为测试 harness 都直接注入变量值(CMD_FROM: cmdFrom、ROUND_START: roundStart),因为套件逐字回放 harness 从单个 job 内部起步 —— 于是删除或写错任何一条接线,整套测试依旧绿灯,而上线的特性静默退化。
— 具体代价:/takeover from N 表现得与裸 /takeover 完全相同(或 digest 基线静默回落到 0),且没有任何报错;已测解析器与已测读取器之间的集成缝隙端到端无人钉住。套件在其他地方钉住了等价接线(takeover_ack: outputs 映射、FORCED_PR: env)。
四条接线:workflow 第 373、1910、5190、5985 行(route outputs.takeover_from → toggle CMD_FROM;prepare round_start 输出 → address ROUND_START)。
建议修复:为这四条接线行添加 toContain 钉住,与套件既有的接线钉住惯例保持一致。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| # no label churn needed. The watermark is untouched: feedback | ||
| # already addressed is never replayed. | ||
| gh pr comment "${PR}" --repo "${REPO}" --body "$(printf '🔄 Takeover re-armed: the round counter starts a fresh window (previous rounds no longer count toward the cap); management continues.\n\n<details>\n<summary>中文说明</summary>\n\n🔄 已重新武装:轮次计数开启新窗口(此前轮次不再计入上限),托管继续。\n\n</details>\n\n<!-- takeover-ack engaged -->')" | ||
| gh pr comment "${PR}" --repo "${REPO}" --body "$(printf '🔄 Takeover re-armed: the round counter starts a fresh window%s; management continues.%s\n\n<details>\n<summary>中文说明</summary>\n\n🔄 已重新武装:轮次计数开启新窗口%s,托管继续。%s\n\n</details>\n\n<!-- takeover-ack engaged -->%s' "${REARM_FRESH_CLAUSE}" "${FROM_NOTE_REARM}" "${REARM_FRESH_CLAUSE_ZH}" "${FROM_NOTE_REARM_ZH}" "${FROM_MARKER}")" |
There was a problem hiding this comment.
[Suggestion] The seeded re-arm ack post has no retry/fallback, unlike the engage post this same diff hardened.
The re-arm ack body now carries the seed marker (${FROM_MARKER}), but the retry+warning treatment went only to the ENGAGE path — this post remains a bare single gh pr comment under -eo pipefail, even though the engage retry's own comment ("the seed marker's only copy lives in this body, and the heal ack has no slot to recover it — a transient 5xx must not silently un-seed the window") applies equally here. The scan's heal path does not cover a failed RE-ARM: it only posts a fresh ack for a labeled PR with no ack at all, and the pre-existing engage ack suppresses the dedup.
— Failure scenario: a maintainer re-arms an already-managed PR with @qwen-code /takeover from 60; one transient 5xx on gh pr comment aborts the step — the toggle job goes red, and BOTH the window reset and the seed are lost; nothing re-posts the re-arm. The identical transient on first engagement is absorbed invisibly by the retry above.
Witness (verbatim toggle replay, PATH-stubbed gh failing pr comment once with HTTP 502, CMD=add, CMD_FROM=60):
seeded re-arm: exit_code=1 comment_attempts=1 done_printed=0
seeded engage (PR code): exit_code=0 comment_attempts=2
probe copy with retry shape: exit_code=0 comment_attempts=2
Suggested fix: give the re-arm post the same shape as the engage post — build the body once into a variable, gh pr comment ... || { sleep 5; gh pr comment ...; } || echo "::warning::re-arm ack failed...".
中文说明
[Suggestion] 带种子的 re-arm ack 发送没有重试/兜底,而同一 diff 却给 engage 发送加了这层保护。
re-arm ack 正文现在携带种子标记(${FROM_MARKER}),但重试+警告的处理只给了 ENGAGE 路径 —— 这条发送仍是 -eo pipefail 下裸的单次 gh pr comment,尽管 engage 重试自己的注释("种子标记的唯一副本就在这个正文里,而 heal ack 没有槽位可以恢复它 —— 一次瞬时 5xx 不应静默地解除窗口的种子化")在这里同样成立。扫描的 heal 路径不覆盖失败的 RE-ARM:它只为"有标签但完全没有 ack"的 PR 补发新 ack,而既有的 engage ack 会抑制去重。
— 失败场景:维护者对已托管 PR 重新下达 @qwen-code /takeover from 60;gh pr comment 遇到一次瞬时 5xx 即中止步骤 —— toggle job 变红,窗口重置与种子双双丢失;没有任何东西补发 re-arm。同样的瞬时故障若发生在首次接管,会被上面的重试无声吸收。
证据(逐字提取 toggle 回放,PATH 打桩的 gh 以 HTTP 502 使 pr comment 失败一次,CMD=add、CMD_FROM=60):
seeded re-arm: exit_code=1 comment_attempts=1 done_printed=0
seeded engage (PR code): exit_code=0 comment_attempts=2
probe copy with retry shape: exit_code=0 comment_attempts=2
建议修复:给 re-arm 发送与 engage 发送相同的形态 —— 正文一次性构建进变量,gh pr comment ... || { sleep 5; gh pr comment ...; } || echo "::warning::re-arm ack failed..."。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #9321 (round 3)All six inline findings are resolved in code (commit 5db5062). No conflict resolution was needed ( Feedback points and dispositions[rc:3796710688] [Critical] Zero-padded seeds pass every gate but break the toggle job's bash arithmetic — RESOLVED. [rc:3796710694] / [rc:3796710700] / [rc:3796710705] / [rc:3796710708] [Suggestion] The seed crosses job-boundary wires that no test pins — RESOLVED (one batch). [rc:3796710714] [Suggestion] The seeded re-arm ack post has no retry/fallback — RESOLVED, with one shape deviation. [rv:4952040530] [CHANGES_REQUESTED] review body — NO ACTION THIS ROUND. Changed files
VerificationCommands actually run, in order:
中文说明Autofix 评审轮次 — PR #9321(第 3 轮)全部六条 inline findings 已在代码中解决(commit 5db5062)。无需处理 base 冲突( 反馈点与处置[rc:3796710688] [Critical] 零填充种子能通过所有门控,但会破坏 toggle job 的 bash 算术 —— 已解决。 [rc:3796710694] / [rc:3796710700] / [rc:3796710705] / [rc:3796710708] [Suggestion] 种子穿过的跨 job 接线没有任何测试钉住 —— 已解决(一批完成)。 [rc:3796710714] [Suggestion] 带种子的 re-arm ack 发送没有重试/兜底 —— 已解决,但有一处形态偏差。 [rv:4952040530] [CHANGES_REQUESTED] 评审正文 —— 本轮不处理。 变更文件
验证实际执行的命令(按顺序):
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
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.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
scripts/tests/qwen-autofix-workflow.test.js:4421 — [probe] clamp tests miss the seed == cap boundary — a -ge→-gt mutant at both clamp copies (yml:3367, yml:4798) ships greenscripts/tests/qwen-autofix-workflow.test.js:8064 — [probe] nothing pins LIVE_ROUND_START_RAW capture before the clamp — audit record can cite a clamped seed as the command typed.github/workflows/qwen-autofix.yml:2110 — [review] seeded engage-ack double-failure warning never names the lost seed (also recorded in round 2 at then-line 2093).github/workflows/qwen-autofix.yml:4793 — [probe] marker-read sites accept zero-padded seeds — silent audit-record loss in green runs (R2-1 sibling; the 'jq rejects 08' rebuttal is disproved)
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| expect(transientRearmFailure.log).toContain('re-armed'); | ||
| expect(transientRearmFailure.log).not.toContain('::error::'); |
There was a problem hiding this comment.
[Suggestion] The re-arm ack's new LOUD double-failure arm (|| { echo "::error::re-arm ack comment failed on #${PR} after one retry"; exit 1; }, qwen-autofix.yml:2060) is never executed by any test: the harness gh stub fails at most once per run (! -f comment-failed guard at ~3802), so only the single-failure retry path is exercised — Failure scenario: a future edit dropping the exit 1 arm (or demoting it to the engage path's ::warning::) ships green: after two failed re-arm POSTs the job logs "re-armed", removes the stale-escalation label, and continues with a window that was never reset and whose seed never landed. Nothing heals a missing re-arm, so this abort is the only guard and its deletion would be silent.
Witness (mutation probe on this commit):
demoted-arm mutant → Tests 176 passed (176) (ships green)
+ fail-always double-failure → AssertionError: expected null to be truthy
One way to pin it — let the stub fail N times (e.g. a TOGGLE_COMMENT_FAIL_TIMES=2 knob), then assert:
// replay aborts: two COMMENT-ATTEMPT writes, no takeover-ack engaged re-arm body
expect(log).toContain('::error::');
expect(log).not.toContain('re-armed');中文说明
[Suggestion] re-arm ack 新增的“双失败响亮中止”分支(|| { echo "::error::re-arm ack comment failed on #${PR} after one retry"; exit 1; },qwen-autofix.yml:2060)没有任何测试真正执行到:harness 的 gh 桩每次运行最多失败一次(~3802 行的 ! -f comment-failed 门),因此只有单次失败重试路径被覆盖。— 失败场景:未来若有编辑删掉 exit 1 分支(或把它降级为 engage 路径的 ::warning::),整个测试套件仍为绿色:两次 re-arm POST 都失败后,job 会打印 "re-armed"、移除 stale-escalation 标签,并带着一个从未重置、种子也从未落地的窗口继续执行。缺失的 re-arm 没有任何 heal 路径,这个中止是唯一的护栏,删掉它将是静默的。
证据(在本提交上做的变异探针):
降级分支变异体 → Tests 176 passed (176)(绿色通过)
+ 恒失败的双失败用例 → AssertionError: expected null to be truthy
一种钉住方式:让桩支持失败 N 次(如 TOGGLE_COMMENT_FAIL_TIMES=2 开关),然后断言回放中止(两次 COMMENT-ATTEMPT、无 re-arm 正文)、日志含 ::error:: 且不含 re-armed。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| expect(workflow).toContain( | ||
| "takeover_from: '${{ steps.decide.outputs.takeover_from }}'", | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The wire pins added for R2-2 cover both ENDS of the takeover_from chain (route output declaration at qwen-autofix.yml:373, toggle env consumer at :1916) but not the middle link — the decide step's GITHUB_OUTPUT write at qwen-autofix.yml:775 — which no behavioral harness executes; the sibling round_start write this same PR adds IS pinned verbatim (~8078), so this is an asymmetric omission — Failure scenario: deleting or typo'ing that one write line ships with both existing pins still matching and every replay green (the cmdBranch extraction stops at the issue_comment branch's fi at 631, which precedes the write at 775; seedOf re-performs sanitize_number in its own printf instead of executing the workflow's echo) — steps.decide.outputs.takeover_from reads empty, CMD_FROM stays empty, and /takeover from N silently degrades to an unseeded takeover: five fresh rounds instead of the promised remainder. That is exactly the scenario these pins' own comment (~5560) says they exist to prevent.
Witness (mutation probe on this commit):
delete write line → Tests 176 passed (176)
typo takeover_form → Tests 176 passed (176)
+ pin below → typo arm fails: Tests 1 failed | 175 passed
| expect(workflow).toContain( | |
| "takeover_from: '${{ steps.decide.outputs.takeover_from }}'", | |
| ); | |
| expect(workflow).toContain( | |
| "takeover_from: '${{ steps.decide.outputs.takeover_from }}'", | |
| ); | |
| expect(workflow).toContain( | |
| 'echo "takeover_from=$(sanitize_number "${TAKEOVER_FROM}")" >> "${GITHUB_OUTPUT}"', | |
| ); |
中文说明
[Suggestion] 为 R2-2 新增的接线钉住覆盖了 takeover_from 链条的两端(route 输出声明 qwen-autofix.yml:373、toggle 环境消费端 :1916),但没有覆盖中间环节 —— decide 步骤在 qwen-autofix.yml:775 对 GITHUB_OUTPUT 的写入 —— 没有任何行为 harness 执行到它;同一 PR 新增的姊妹 round_start 写入却被逐字钉住(~8078),因此这是不对称的遗漏。— 失败场景:删除或拼错那一行写入后,既有两个钉住断言仍全部匹配、所有回放测试仍为绿色(cmdBranch 提取在 issue_comment 分支的 fi(631 行)处截止,早于 775 行的写入;seedOf 是在自己的 printf 里重新执行 sanitize_number,而非执行 workflow 的 echo)—— steps.decide.outputs.takeover_from 读到空值、CMD_FROM 为空,/takeover from N 静默退化为无种子接管:重新计满五轮,而不是承诺的剩余轮次。这正是这些钉住的注释(~5560)声称要防止的场景。
证据(在本提交上做的变异探针):
删除写入行 → Tests 176 passed (176)
拼错为 takeover_form → Tests 176 passed (176)
+ 下方钉住断言 → 拼错分支失败:Tests 1 failed | 175 passed
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Round 3 summary — PR #9321Commit: Feedback points and dispositions[rc:3798890289] — [Suggestion] the re-arm ack's LOUD double-failure abort arm is never executed by any test — RESOLVED (implemented). [rc:3798890303] — [Suggestion] the [rv:4954664844] — review body: "Reviewed — no blockers" plus four items deferred under the convergence posture — NOTED, no action. No Verification
中文说明第 3 轮总结 — PR #9321提交: 反馈点及处置[rc:3798890289] — [Suggestion] re-arm ack 的"双失败响亮中止"分支没有任何测试执行到 — 已解决(已实现)。 [rc:3798890303] — [Suggestion] [rv:4954664844] — 评审正文:"已审查——无阻断问题",另含收敛姿态下延后的 4 条 — 已知悉,不处理。 无 验证
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
There was a problem hiding this comment.
No blocking issues. 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.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-autofix.yml:2108 — [probe] engage-ack double-failure fallback arm untested — continue-vs-abort semantics unpinned.github/workflows/qwen-autofix.yml:2037 — [probe] Chinese seed notes in takeover acks have zero assertions — argument-order slip ships green
中文说明
无阻断问题。LGTM!✅
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed the full diff at 1669b859; CI is fully green on this head (39 checks, no failures/pending).
The seed plumbing is disciplined end-to-end:
- Input is bounded at the parser (
from Nregex, 1-2 digits,10#decimal canonicalization), re-validated bysanitize_numberat the job-boundary output, and re-checked + clamped strictly below the effective cap at both read sites (scan and prepare) — a seed can move the Critical-only brake forward but can never park a PR at its cap on takeover. - The seed rides as its own
<!-- autofix-round-start N -->marker rather than a field inside the engage ack, so the seven jqcontains()read sites stay intact; reading it bycreated_atequality against the window key keeps it window-scoped and prevents leakage from superseded windows. - Retry-once-then-LAUD on both engage and re-arm acks is the right call since the marker's only copy lives in those bodies.
- Tests pin the seeded/unseeded wording split, the clamp, the transient-failure retries, and the scan's deliberate ignorance of the Critical-only threshold.
LGTM.
|
Released in v0.21.14. |
What this PR does
Adds one parameterized takeover command,
@qwen-code /takeover from N, which engages the loop on a PR and seeds that counting window's round counter at N instead of 0.CRITICAL_ONLY_AFTER_ROUNDis then reached in the remainder — a PR taken over atfrom 3gets two more suggestion-capable rounds and enters Critical-only on the third, rather than five fresh ones. The bare@qwen-code /takeover,@qwen-code /takeover stop, and every unparameterized path behave exactly as before.The seed rides as its own
<!-- autofix-round-start N -->marker on a separate line of the engage ack, never as a field inside<!-- takeover-ack engaged -->. That literal is matched with jqcontains()— closing-->included — at seven read sites: four in this workflow (the ack dedup, the scan's first-pickup dedup, and the twoREARM_KEYwindow readers) and three inqwen-fleet-shepherd.yml(the paused/resume detector). An inline field would silently break all seven: the window key would fall back to an older engage ack, so the round counter would read a dead window, and the shepherd would stop seeing the engage as a resume signal and age out a PR that was just re-armed. This is the same sibling-marker shape, for the same reason, as the existingautofix-redcheckmarker.Both round readers (scan and prepare) now fall back to the seed rather than a hardcoded
0. The seed is read bycreated_atequality against the window key, so it is window-scoped for free — a superseded window's seed cannot leak into the live one, and@qwen-code /retryor a bare re-takeover opens a window with no marker and returns the counter to 0. It is clamped strictly below the effective cap so a seed can never park a PR at its round cap on the very round it is taken over, which would stop the loop instead of starting it.Both engage acks and the Critical-only audit record name the seed when there is one. Without that, the ack reports "round 4/100" on its first managed round and reads like a bug, and the audit record claims "5 change-producing rounds are complete" on a PR the loop has run twice — true of the counter, visibly false of the PR.
SKILL.mdtells the agent the same thing, so it does not read a second-round Critical-only engagement as a misfire.The growth brake is deliberately not seeded: its baseline anchors at the window's first measured round, and a pre-takeover baseline is not recoverable, so diff growth stays measured from engagement. This is stated in the env comment so it is not later filed as a bug.
Why it's needed
The round counter is window-scoped, and engaging takeover opens a fresh window. That is correct for re-arming, but it means taking over a PR that has already been through several rounds of ordinary review restarts the Critical-only brake from zero. A PR that spent nine human rounds getting to "almost mergeable" then got five more suggestion-capable rounds the moment it was managed — the loop grew the diff on nice-to-haves at exactly the point it should have been converging on Criticals and letting the rest ride to follow-up.
Re-issuing takeover on an already-managed PR made this worse rather than better: it is the documented round-counter reset, so a PR that had just reached the brake had the suggestion valve reopened at full travel.
from Nlets the maintainer state what the loop cannot infer — how much review this PR has already absorbed — and the existing brake does the rest. No second threshold, no parallel mode flag.Reviewer Test Plan
How to verify
Run the workflow test file:
176 passed (176). Two tests are new and both replay real workflow fragments extracted verbatim (drift fails the test), so they are behavioral rather than string assertions:behaviorally parses the takeover round seed and keeps every other body closed— replays the route step'sissue_commentbranch with a PATH-stubbedgh. Confirmsfrom 3/from 12/from 99engage with the seed, surrounding whitespace is trimmed,from 0engages as the explicit no-seed spelling, and twelve fail-closed bodies produce nothing:stop from 3(neither releases nor engages),from 100,from 3x,from -1,from, a double space,takeoverfrom 3, a prefixed body, a suffixed body,; rm -rf /,$(id), and a backtick payload.behaviorally seeds the round counter from the window anchor and only from it— replays the scan's realMARKERS/REARM_KEY/ROUNDtrio. Confirms an empty seeded window reports round 3; a real eval marker outranks the seed as soon as one exists (the seed is a floor, not an offset added to every round); a superseded window's seed cannot leak forward; a bare re-arm drops it to 0; unseeded acks behave exactly as before; and the seed is rejected both from a non-bot comment posted at the window key and from a bot comment that is not the anchor.Mutation-tested — four single-edit mutations, each applied to the workflow and reverted:
expected '7' to be '0')expected '99' to be '0')max // $startreverted tomax // 0at both readersexpected '0' to be '3')expected 'add|3' to be '|')The author-filter mutation survived on the first attempt: the impostor comment in that test was dated away from the window key, so the case exercised the key check twice and the author check not at all. The test now dates it at the window key — GitHub stamps
created_atto the second, so a comment landing in the same second as the engage ack is a real collision and the author filter is the only thing that rejects it — and the mutation is caught.Also verified:
node scripts/lint.js --actionlintexits 0,npm run check-i18npasses,npx eslint scripts/tests/qwen-autofix-workflow.test.jsexits 0, andqwen-fleet-shepherdworkflow tests stay at 17/17 (that file is untouched, but it holds three of the seventakeover-ack engagedread sites).Evidence (Before & After)
Not user-visible in the TUI, but the bot's PR comment is the user-facing surface, so the ack text was rendered by executing the actual
FROM_MARKER/FROM_NOTEblock extracted from the workflow.Before (unchanged for every unparameterized path —
CMD_FROMempty or0):After (
@qwen-code /takeover from 3):The
<!-- takeover-ack engaged -->line is byte-identical in both, on its own line, with the seed marker below it — which is the property the other seven read sites depend on.from 7(past the threshold) renders "after 0 more change-producing round(s)", i.e. Critical-only engages on the first managed round, which is accurate.Tested on
Environment (optional)
Workflow tests only (
vitest run --config ./scripts/tests/vitest.config.ts), plusactionlintandprettier/eslintlocally on macOS. No runtime or sandbox involved.Risk & Scope
sanitize_numberon the way out of the job, and it reaches nothing but an integer comparison and aprintf '%s'of a re-validated number. The twelve fail-closed cases in the new test are the guard against drift here.run-agent idle watchdogtests in this file are flaky on this machine independent of the change (interleaved A/B: 3/12 failures on this branch vs 5/12 on a clean tree), so they are not a signal either way here. Routing suggestions to follow-up issues — the natural companion to this brake — is not in this PR; the deferred Critical-only feedback still goes only to the round report, not to the per-PR tracking issue.autofix-round-startmarker behaves exactly as it does today, so nothing in flight is affected by the deploy.Linked Issues
None.
中文说明
这个 PR 做了什么
新增一个带参数的托管命令
@qwen-code /takeover from N:接管 PR 的同时,把本计数窗口的轮次计数从 N 起算,而不是从 0。这样CRITICAL_ONLY_AFTER_ROUND是在剩余轮次内达到的 —— 以from 3接管的 PR 再经过 2 个可含 suggestion 的轮次,第 3 轮即进入 Critical-only,而不是重新计满 5 轮。裸的@qwen-code /takeover、@qwen-code /takeover stop以及所有无参数路径行为完全不变。种子以独立的
<!-- autofix-round-start N -->标记单独占一行写在 engage ack 中,绝不作为字段塞进<!-- takeover-ack engaged -->。后者是用 jqcontains()按全字面量(含闭合-->)匹配的,读取点共有七处:本 workflow 四处(ack 去重、扫描首次接管去重、两处REARM_KEY窗口读取),以及qwen-fleet-shepherd.yml三处(暂停/恢复探测)。内联字段会静默打断全部七处:窗口 key 会回落到更旧的 engage ack,导致轮次计数读到一个已失效的窗口;而且 shepherd 将不再把这次 engage 识别为恢复信号,会把一个刚刚重新武装的 PR 判为无人应答并 age out。这与现有autofix-redcheck标记是同一种 sibling marker 形态,理由也相同。两处轮次读取(scan 与 prepare)现在回落到种子,而非硬编码的
0。种子按created_at与窗口 key 相等来读取,因此天然具有窗口作用域 —— 被取代的旧窗口的种子无法泄漏到当前窗口;@qwen-code /retry或裸的重新接管会开启一个没有该标记的新窗口,计数回到 0。种子被 clamp 在有效上限之下,因此绝不可能出现"刚被接管的那一轮就把 PR 顶到轮次上限"——那会是停止循环而不是启动循环。两处 engage ack 与 Critical-only 的审计记录在存在种子时都会点明。否则 ack 会在首个托管轮次显示 "round 4/100",看起来像 bug;而审计记录会对一个循环只跑过两轮的 PR 声称"已完成 5 个产生改动的轮次"——对计数器为真,对 PR 显然为假。
SKILL.md同步告知 agent,使其不会把第二轮就触发的 Critical-only 误判为刹车失灵。增长刹车刻意不种子化:它的基线锚定在窗口首个完成测量的轮次,而托管前的基线不可恢复,因此 diff 增长始终从接管时刻起算。这一点写进了 env 注释,以免日后被当作 bug 提报。
为什么需要
轮次计数是窗口作用域的,而接管会开启一个新窗口。这对"重新武装"是正确语义,但也意味着:接管一个已经经历过多轮常规评审的 PR,会把 Critical-only 刹车从零重启。一个花了九轮人工评审才走到"接近可合入"的 PR,在被托管的瞬间又获得五个可含 suggestion 的轮次 —— 循环恰恰在本该收敛于 Critical、把其余留给后续跟进的时点,把 diff growth 花在了锦上添花的改动上。
对已托管的 PR 重新下达接管命令只会让情况更糟:那是有文档记载的轮次计数重置,于是一个刚刚触及刹车的 PR,suggestion 阀门被重新开到最大。
from N让维护者说出循环无法推断的信息 —— 这个 PR 已经吸收了多少轮评审 —— 剩下的交给现有刹车。不引入第二个阈值,也不引入并行的模式开关。评审者测试计划
如何验证
运行 workflow 测试文件:
176 passed (176)。其中两个测试为新增,且都逐字提取真实 workflow 片段回放(片段漂移会导致测试失败),因此是行为测试而非字符串断言:behaviorally parses the takeover round seed and keeps every other body closed—— 用 PATH 打桩的gh回放 route 步骤的issue_comment分支。验证from 3/from 12/from 99带种子接管、首尾空白被裁剪、from 0作为显式无种子写法照常接管,以及十二种必须 fail-closed 的输入均无任何效果:stop from 3(既不释放也不接管)、from 100、from 3x、from -1、from、双空格、takeoverfrom 3、带前缀正文、带后缀正文、; rm -rf /、$(id)、反引号 payload。behaviorally seeds the round counter from the window anchor and only from it—— 回放扫描步骤真实的MARKERS/REARM_KEY/ROUND三元组。验证:空的带种子窗口报告第 3 轮;一旦出现真实 eval 标记即覆盖种子(种子是空窗口的下界,而不是叠加到每一轮上的偏移量);被取代窗口的种子无法向前泄漏;裸的重新武装使其归零;无种子的 ack 行为与本特性存在前完全一致;以及种子会被拒绝——无论它来自恰好落在窗口 key 上的非 bot 评论,还是来自并非窗口锚点的 bot 评论。已做变异测试 —— 四个单点变异,逐个施加到 workflow 后还原:
expected '7' to be '0')expected '99' to be '0')max // $start还原为max // 0expected '0' to be '3')expected 'add|3' to be '|')作者过滤那个变异第一次施加时存活了:该测试中的伪造评论时间戳偏离了窗口 key,因此这个用例把 key 检查测了两遍、把作者检查一遍也没测到。现已把它改为恰好落在窗口 key 上 —— GitHub 的
created_at精确到秒,因此一条与 engage ack 落在同一秒的评论是真实可能的碰撞,而作者过滤是唯一能拒绝它的东西 —— 之后该变异被捕获。另外验证:
node scripts/lint.js --actionlint退出码 0,npm run check-i18n通过,npx eslint scripts/tests/qwen-autofix-workflow.test.js退出码 0,qwen-fleet-shepherd的 workflow 测试保持 17/17(该文件未被改动,但它持有七个takeover-ack engaged读取点中的三个)。证据(改动前后)
本改动在 TUI 中不可见,但 bot 的 PR 评论就是面向用户的界面,因此 ack 文案是通过执行从 workflow 中提取的真实
FROM_MARKER/FROM_NOTE代码块渲染出来的。改动前(所有无参数路径均保持不变 ——
CMD_FROM为空或为0):改动后(
@qwen-code /takeover from 3):两者中
<!-- takeover-ack engaged -->一行字节完全相同、独占一行,种子标记位于其下方 —— 这正是另外七个读取点所依赖的性质。from 7(超过阈值)会渲染为 "after 0 more change-producing round(s)",即 Critical-only 在首个托管轮次即生效,描述是准确的。测试平台
环境(可选)
仅 workflow 测试(
vitest run --config ./scripts/tests/vitest.config.ts),外加在 macOS 本机运行的actionlint与prettier/eslint。不涉及运行时或沙箱。风险与范围
sanitize_number复验,且它只会流向一次整数比较和一个对已复验数字的printf '%s'。新测试中的十二个 fail-closed 用例即是防止此处退化的护栏。run-agent idle watchdog测试在本机与本改动无关地抖动(交替 A/B:本分支 3/12 失败,干净树 5/12 失败),因此它在这里不构成任何方向的信号。把 suggestion 路由到后续跟进 issue —— 这个刹车天然的搭档 —— 不在本 PR 内;Critical-only 延后的反馈目前仍只进入轮次报告,不进入 per-PR 追踪 issue。autofix-round-start标记的窗口行为与今天完全一致,因此部署不会影响任何在飞的流程。关联 Issue
无。