fix(autofix): surface thread-resolution guard refusals in the round report and wait out head-propagation lag - #10117
Conversation
github-actions
Bot
added
the
review/self-reported
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@wenshao The PR body doesn't follow the PR template — all five required sections are missing (What this PR does, Why it's needed, Reviewer Test Plan with How to verify / Evidence / Tested on, Risk & Scope, Linked Issues). The current body uses a free-form structure (Problem / Diagnosis / Changes / Tests). Please rewrite the body against the template and re-trigger triage (@qwen-code /triage).
One thing the rewritten body must cover, or it will fail the template's purpose again: the diff carries much more than the described #10106 fix. Beyond the autofix script, design doc, and workflow test, it contains 13 commits of PreToolUse ask-bounce work for #9434 / #9441 (~640 production lines plus ~3,300 test lines across coreToolScheduler, shell.ts, tools.ts, workflow-run-registry, ToolConfirmationMessage, and the permission surfaces) that the body never mentions. "What this PR does" needs to describe the whole diff — or that work should go in its own PR.
中文说明
@wenshao PR 正文没有遵循 PR 模板——五个必需小节全部缺失(What this PR does、Why it's needed、Reviewer Test Plan(含 How to verify / Evidence / Tested on)、Risk & Scope、Linked Issues)。当前正文是自由结构(Problem / Diagnosis / Changes / Tests)。请按模板重写正文,然后重新触发 triage(@qwen-code /triage)。
重写后的正文必须覆盖一件事,否则模板的目的仍会落空:diff 的内容远不止所描述的 #10106 修复。除 autofix 脚本、设计文档和 workflow 测试外,还包含 13 个针对 #9434 / #9441 的 PreToolUse ask-bounce 提交(约 640 行生产代码 + 约 3,300 行测试,横跨 coreToolScheduler、shell.ts、tools.ts、workflow-run-registry、ToolConfirmationMessage 及各 permission 界面),正文对此只字未提。"What this PR does" 需要描述完整 diff——或者把那部分工作拆到单独的 PR。
— Qwen Code · qwen3.8-max
…eport and wait out head-propagation lag The autofix thread-resolution pass could go completely dark on a PR: every skip condition logged only a run-log ::warning::, so on PR #9729 0 of 90 review threads were resolved across ~16 rounds with nothing on the PR saying which guard refused (#10106). Observability: each refusing guard now records a stable name (salvage merge, missing verified_head, verified_head mismatch, live-head drift, thread state unproven, mutation post-check ambiguous), and both round report arms carry one host-authored line naming the guard and counting the selected / resolved / left-behind threads. Healthy rounds report the positive count, so a dead mechanism is visible on the PR it fails on. The line is fixed host strings plus counters only. Root cause on #9729: the PR read model is eventually consistent — a headRefOid read seconds after the round's own push still returns the previous head, so every pushed round declared live-head drift on one stale read and silently skipped. The initial live-head equality check now retries up to five times (bounded, digit-clamped test knob for the delay) before declaring drift; the per-mutation guards stay single-shot, because once the head was observed equal a later mismatch means it actually moved. Fixes #10106
9e94917 to
9e79846
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）为单个提交。 |
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
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Unresolved, please confirm:
- [Critical] Existing CHANGES_REQUESTED review (5028697252, @qwen-code-ci-bot triage): requests the PR body be rewritten against the PR template and triage re-triggered — a process blocker that cannot be ruled on from code; the body still uses a free-fo…
Not reviewed: build-and-test — CI check 'Test (macos-latest, Node 22.x)' was skipped (gated to merge_group/schedule/workflow_dispatch) and its suite (scripts/tests, which covers this diff) did not run locally on macOS; platform-portability coverage rests on static audit only.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查：build-and-test — CI check 'Test (macos-latest, Node 22.x)' was skipped (gated to merge_group/schedule/workflow_dispatch) and its suite (scripts/tests, which covers this diff) did not run locally on macOS; platform-portability coverage rests on static audit only。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| # host strings plus counts — nothing agent-controlled. | ||
| RESOLUTION_NOTE='' | ||
| if [[ "${RESOLUTION_SELECTED_N}" -gt 0 ]]; then | ||
| RESOLUTION_LEFT_N=$(( RESOLUTION_SELECTED_N - CONFIRMED_RESOLVED_N )) |
There was a problem hiding this comment.
[Suggestion] R1-1: The guarded note's "left for a later round" count can overstate residual work. A thread the per-thread pre-guard observed as already resolved by another actor hits the continue branch without incrementing CONFIRMED_RESOLVED_N, so it still lands in RESOLUTION_LEFT_N (= SELECTED_N − CONFIRMED_RESOLVED_N). Duplicate ids in resolved-comments.txt inflate it the same way — counted twice in RESOLUTION_SELECTED_N but confirmable at most once.
Concrete case: a round selects 3 threads; a human resolves one concurrently; the head moves during thread 2's pre-guard → break with live-head drift. The PR report then says "resolved 0 of 3 selected thread(s), 3 left for a later round" when only 2 threads are actually open — the observability line this PR exists to make trustworthy overstates exactly the number a maintainer triages on. The error is in the safe direction (over-reports leftovers; no incorrect resolution), hence Suggestion.
Witness (probe against this PR's own extracted note block + gh stub, unmodified code):
A1_NOTE: ⚠️ Review-thread resolution stopped early — guard: live-head drift; resolved 0 of 3 selected thread(s), 3 left for a later round.
— actually-open threads: 2 (the third was resolved by the other actor).
Fix options: soften the guarded wording to an upper bound ("up to N left for a later round"), or track the another-actor continue (and duplicate ids) in a third counter and subtract it; optionally sort -u the normalized ids when computing RESOLUTION_SELECTED_N.
中文说明
带守卫的注记中 "left for a later round"(留待后续轮次)的计数可能夸大剩余工作量:被逐条前置守卫观察到"已被其他参与者 resolve"的线程会走 continue 分支,不增加 CONFIRMED_RESOLVED_N,因此仍会计入 RESOLUTION_LEFT_N(= SELECTED_N − CONFIRMED_RESOLVED_N)。resolved-comments.txt 中的重复 id 同理——在 RESOLUTION_SELECTED_N 中被计两次,但至多只能确认一次。
具体场景:一轮选中 3 条线程;人工并发 resolve 了其中 1 条;第 2 条的前置守卫时 head 移动 → 以 live-head drift 中止。PR 报告会写 "resolved 0 of 3 selected thread(s), 3 left for a later round",而实际只有 2 条线程仍未关闭——本 PR 要建立信任的可观测性行,恰好在维护者赖以分诊的数字上夸大。误差方向是安全的(只会多报遗留,不会漏报;不会导致错误 resolve),因此定为 Suggestion。
见证(用本 PR 自己的注记合成块 + gh 桩、未改动代码做的探针):
A1_NOTE: ⚠️ Review-thread resolution stopped early — guard: live-head drift; resolved 0 of 3 selected thread(s), 3 left for a later round.
——实际未关闭的线程只有 2 条(第 3 条已被其他参与者 resolve)。
修复选项:将带守卫分支的措辞软化为上限("up to N left for a later round");或用第三个计数器记录"其他参与者已 resolve"(及重复 id)并从中扣除;也可在计算 RESOLUTION_SELECTED_N 时对归一化后的 id sort -u 去重。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if [[ "${CAN_RESOLVE_THREADS}" != 'true' ]]; then | ||
| RESOLUTION_GUARD='live-head drift' |
There was a problem hiding this comment.
[Suggestion] R1-2: The live-head drift guard name conflates two different causes — "the head actually moved" and "every gh pr view read failed". The retry loop discards stderr (2> /dev/null) and coerces every read failure to empty (|| LIVE_PR_HEAD=''), so an expired PAT, a sustained secondary rate limit, or an API outage lasting past the ~20 s window exhausts all 5 attempts and the PR-visible note names live-head drift — sending whoever triages it looking for phantom contributor pushes, with no PR-visible hint that the real cause is API/auth health. That is the blind spot #10106 exists to remove, and the pre-change warning ("could not be proven equal") was careful not to assert drift.
The script itself calls out this exact anti-pattern ~30 lines later for the threads fetch: it captures gh's stderr into a mktemp file and folds the tail into the warning, because "a transient rate limit (back off) reads identically to an expired PAT (rotate) or a network failure without it" — the new loop did not inherit that.
Suggested fix: capture the last attempt's stderr the same way and fold its tail into the ::warning::; or split the guard name — live-head unreadable when no attempt returned any head vs live-head drift when a head was read but never equal — updating the design doc's guard list and the test's guard table to match.
中文说明
live-head drift 守卫名把两种不同原因混在了一起——"head 确实移动了"与"每次 gh pr view 读取都失败了"。重试循环丢弃了 stderr(2> /dev/null),并把每次读取失败都归一成空串(|| LIVE_PR_HEAD=''),因此 PAT 过期、持续的次级限流、或超过 ~20 秒窗口的 API 故障都会耗尽全部 5 次尝试,而 PR 可见的注记写着 live-head drift——让分诊者去寻找并不存在的贡献者 push,PR 上却没有任何线索表明真正原因是 API/凭证健康。这正是 #10106 要消除的盲区;改动前的警告("could not be proven equal")措辞谨慎,并不声称 drift。
脚本自己在约 30 行后的线程拉取处恰好点破了这一反模式:那里把 gh 的 stderr 捕获到 mktemp 文件并把尾部折进警告,因为"没有它,瞬时限流(退避即可)与 PAT 过期(需轮换)或网络故障读起来完全一样"——新的循环没有继承这一点。
建议修复:用同样的方式捕获最后一次尝试的 stderr,并把其尾部折进 ::warning::;或者拆分守卫名——所有尝试都没读到 head 时用 live-head unreadable,读到了但始终不相等时才用 live-head drift——并同步更新设计文档的守卫名列表和测试中的守卫表。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if [[ -s "${WORKDIR}/resolved-comments.txt" ]]; then | ||
| # Same id grammar as the resolve loop below: optional rc: prefix, | ||
| # optional trailing CR, digits only. | ||
| RESOLUTION_SELECTED_N="$(sed 's/\r$//; s/^rc://' "${WORKDIR}/resolved-comments.txt" | grep -cE '^[0-9]+$' || true)" |
There was a problem hiding this comment.
[Suggestion] R1-3: This line adds a fifth independent implementation of the resolved-comments.txt id grammar (optional rc: prefix, optional trailing CR, digits only), and the existing copies already disagree: the resolve loop in this same function (bash parameter expansion), the reply block in this same file (tr -d '\r' + grep -qxE), run-autofix-review-verification.sh (jq, twice), and upsert-deferred-issue.sh (jq — which additionally strips surrounding whitespace, a grammar this counter and the resolve loop do NOT accept).
No live disagreement today — a 14-input battery (CR, bare rc:, rc:rc:7, leading/trailing spaces, embedded CR, duplicates) shows the new counter and the resolve loop classify identically — but the same probe showed upsert-deferred-issue.sh's jq accepting inputs both reject:
counter / resolve loop: 111 333 999 5 5
upsert-deferred-issue.sh jq: 111 333 999 8 9 5 5 5 6
Any future change to the handle grammar must now be replicated in five+ spellings; a miss makes this PR's new observability line report wrong selected/left counts — the silent-wrong-number failure the PR exists to eliminate. Consider folding the counter and the loop's per-line parsing through one shared normalisation, or at minimum making the "Same id grammar as the resolve loop" cross-reference comment bidirectional.
中文说明
这一行是 resolved-comments.txt id 语法(可选 rc: 前缀、可选行尾 CR、纯数字)的第五个独立实现,而且现有副本之间已经不一致:同一函数里的 resolve 循环(bash 参数展开)、同一文件的 reply 块(tr -d '\r' + grep -qxE)、run-autofix-review-verification.sh(jq,两处)、以及 upsert-deferred-issue.sh(jq——还额外去除首尾空白,而本计数器与 resolve 循环并不接受这种输入)。
今天没有实际分歧——用 14 个刁钻输入(CR、裸 rc:、rc:rc:7、首尾空格、内嵌 CR、重复)做的比对显示新计数器与 resolve 循环分类完全一致——但同一探针也显示 upsert-deferred-issue.sh 的 jq 会接受二者都拒绝的输入:
counter / resolve loop: 111 333 999 5 5
upsert-deferred-issue.sh jq: 111 333 999 8 9 5 5 5 6
今后任何对句柄语法的修改都必须同时复制到五处以上;漏掉一处就会让本 PR 新增的可观测性行报告错误的选中/遗留计数——正是本 PR 要消灭的"静默错误数字"失效模式。建议让计数器与循环的逐行解析共用同一个归一化(例如一个小函数二者共用,或在一次归一化遍历中同时计数并输出 id);至少把 "Same id grammar as the resolve loop" 这条交叉引用注释改成双向。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| expect(block).toContain( | ||
| '[[ "${LIVE_HEAD_RETRY_DELAY:-}" =~ ^[0-9]$ ]] || LIVE_HEAD_RETRY_DELAY=5', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R1-4: Nothing pins that the retry loop actually waits between attempts. Deleting the sleep line (.github/scripts/autofix-push-and-report.sh:161) keeps every test green — lagThenConverge converges on attempt 2 regardless of spacing and neverConverges only counts reads (headReadCount === '5'), never timing. In production the five reads would then fire within milliseconds of the round's own push — inside the propagation-lag window this PR exists to wait out — so every read returns the stale head, the drift guard trips again, and resolution silently skips every pushed round: the exact #10106 regression, re-opened. The suite already pins a retry backoff by string elsewhere (expect(step).toContain('sleep $((attempt * 15))')).
Witness (mutation in a scratch tree; comparator proven live):
mutant (sleep line → ':'): target test still passes green
comparator (loop → '1 2 3 4'): same test fails — expected '4' to be '5'
| expect(block).toContain( | |
| '[[ "${LIVE_HEAD_RETRY_DELAY:-}" =~ ^[0-9]$ ]] || LIVE_HEAD_RETRY_DELAY=5', | |
| ); | |
| expect(block).toContain( | |
| '[[ "${LIVE_HEAD_RETRY_DELAY:-}" =~ ^[0-9]$ ]] || LIVE_HEAD_RETRY_DELAY=5', | |
| ); | |
| expect(block).toContain( | |
| '[[ "${live_head_attempt}" == 5 ]] || sleep "${LIVE_HEAD_RETRY_DELAY}"', | |
| ); |
中文说明
没有任何断言锚定"重试循环确实会在两次尝试之间等待"。删除 sleep 行(.github/scripts/autofix-push-and-report.sh:161)后所有测试仍然绿——lagThenConverge 无论间隔多久都会在第 2 次收敛,neverConverges 只计读取次数(headReadCount === '5'),从不计时。在生产中,5 次读取将在本轮自身 push 后的毫秒内全部发出——恰好落在本 PR 要等待的传播延迟窗口之内——于是每次读到的都是旧 head,drift 守卫再次触发,每个 pushed 轮又静默跳过 resolve:#10106 的回归原样复现。本套件在其他地方已经用字符串锚定过重试退避(expect(step).toContain('sleep $((attempt * 15))'))。
见证(在临时树中做变异;比较器已验证有效):
mutant (sleep line → ':'): 目标测试仍然绿
comparator (loop → '1 2 3 4'): 同一测试失败 — expected '4' to be '5'
建议按上方 suggestion 块补一条对 sleep 行的字符串锚定。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if [[ -s "${WORKDIR}/resolved-comments.txt" ]]; then | ||
| # Same id grammar as the resolve loop below: optional rc: prefix, | ||
| # optional trailing CR, digits only. | ||
| RESOLUTION_SELECTED_N="$(sed 's/\r$//; s/^rc://' "${WORKDIR}/resolved-comments.txt" | grep -cE '^[0-9]+$' || true)" |
There was a problem hiding this comment.
[Suggestion] R1-5: The zero-match path of this computation is never exercised — every non-empty test fixture contains at least one valid numeric id — so the || true that keeps it errexit-safe is unpinned. Dropping it is a real regression: grep -c exits 1 on zero matches, and under the step's bash -eo pipefail the assignment aborts the entire push-and-report step after a successful push — dropping the round report and markers. The distinguishing input is reachable: resolved-comments.txt is agent-authored (the resolve loop's per-line [[ "${rc_id}" =~ ^[0-9]+$ ]] || continue defense exists precisely because malformed lines occur), and a file like rc:abc is non-empty (passes -s) yet yields zero matches.
Witness (flip probe driving this exact line under set -euo pipefail, input rc:abc + newline):
mutant (no || true): exit 1 — the assignment aborts the step
original: SURVIVED RESOLUTION_SELECTED_N=0, exit 0
Add one runNote case mirroring the existing empty-file case: write 'rc:abc\n' to resolved-comments.txt, expect status 0 and note ''.
中文说明
这段计算的零匹配路径从未被测试覆盖——所有非空测试夹具都至少包含一个合法数字 id——因此保证其在 errexit 下安全的 || true 没有被锚定。去掉它是一个真实回归:grep -c 在零匹配时以 1 退出,在该 step 的 bash -eo pipefail 下,赋值会中止整个 push-and-report step——而且发生在 push 成功之后,轮次报告与标记全部丢失。区分性输入是可达的:resolved-comments.txt 由 agent 书写(resolve 循环逐条的 [[ "${rc_id}" =~ ^[0-9]+$ ]] || continue 防御,正是因为畸形行会出现),而像 rc:abc 这样的文件非空(能通过 -s)却零匹配。
见证(对这一行原样提取做翻转探针,set -euo pipefail,输入 rc:abc + 换行):
mutant (无 || true): exit 1 — 赋值中止该 step
原始代码: SURVIVED RESOLUTION_SELECTED_N=0, exit 0
建议仿照现有空文件用例补一个 runNote 用例:向 resolved-comments.txt 写入 'rc:abc\n',期望 status 0 且注记为 ''。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/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 冲突，直到移除标签或达到轮次上限。移除 |
- Deduplicate resolved ids and subtract threads another actor resolved before the per-thread guard, so "left for a later round" reports the actual residual instead of overstating it - Name all-failed live-head reads live-head unreadable instead of live-head drift, so an expired PAT or API outage reads differently from a contributor push race on the PR-visible note - Share one id-grammar normalization between the selected-count and the resolve loop instead of two independent spellings - Pin the retry sleep and the zero-valid-id errexit guards in tests; every new guard verified by a mutation probe
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #10117 (review round 2)Commit: Feedback dispositionsrv:5028697252 — CHANGES_REQUESTED (triage): PR body vs template, and "the diff carries undisclosed ask-bounce work"Split into its two claims:
rv:5029791474 — COMMENTED: partial review, gaps disclosedIts actionable content is the five inline findings below, all addressed. The Critical it names is the rv:5028697252 body item above (escalated, not a code claim). Its not-reviewed disclosure is noted: the rc:3862226607 (R1-1) — RESOLVED: "left for a later round" overstated residual workReproduced first: the another-actor rc:3862226628 (R1-2) — RESOLVED:
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Unresolved, please confirm:
- [Critical] Existing CHANGES_REQUESTED review (5028697252, @qwen-code-ci-bot triage): requests the PR body be rewritten against the PR template and triage re-triggered — a process blocker that cannot be ruled on from code; the body still uses a free-fo…
Not reviewed: build-and-test — CI check 'Test (macos-latest, Node 22.x)' was skipped (gated to merge_group/schedule/workflow_dispatch) and its suite (scripts/tests, which covers this diff) did not run locally on macOS; platform-portability coverage rests on static audit only.
Not reviewed: the executable-script lint — .github/scripts/autofix-push-and-report.sh: shellcheck is not installed.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
scripts/tests/qwen-autofix-workflow.test.js:17974 — [review] runNote duplicates runResolve's fixture-reset trio + env contract verbatim.github/scripts/autofix-push-and-report.sh:239 — [review] mid-list stop records live-head drift for unreadable readsscripts/tests/qwen-autofix-workflow.test.js:17963 — [review] arm-membership pin counts script-wide echoes, not arm membershipscripts/tests/qwen-autofix-workflow.test.js:18014 — [review] note assertions never assert the Chinese half
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查：build-and-test — CI check 'Test (macos-latest, Node 22.x)' was skipped (gated to merge_group/schedule/workflow_dispatch) and its suite (scripts/tests, which covers this diff) did not run locally on macOS; platform-portability coverage rests on static audit only。
未审查：the executable-script lint — .github/scripts/autofix-push-and-report.sh: shellcheck is not installed。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| elif [[ "${THREAD_IS_RESOLVED}" == 'true' ]]; then | ||
| RESOLVED_BY_OTHERS_N=$(( RESOLVED_BY_OTHERS_N + 1 )) | ||
| echo "::warning::comment ${rc_id} was resolved by another actor before this round could resolve it" |
There was a problem hiding this comment.
[Suggestion] R2-1: Two selected comment ids that belong to ONE review thread make the loop attribute this round's own resolution to "another actor" and count one thread as two in the PR-visible note. This is reachable: the feedback renderer lists a reply under a Critical root as a separate finding with its own rc: handle, so a round that fixes both writes two ids for one thread. The loop resolves the thread on the first id; the second id still maps to the same thread (the lookup runs against the round-start snapshot where isResolved is still false), the single-shot live guard then reads isResolved=true — set by this round's own first iteration — increments RESOLVED_BY_OTHERS_N and logs "comment N was resolved by another actor". The note then posts "Resolved all 2 selected review thread(s)" for one thread. No wrong mutation occurs — the thread is correctly resolved once — but the accounting line this PR exists to make trustworthy misstates both the thread count and who resolved it. Witness (probe on the unmodified code, one thread carrying databaseIds 111+112, file rc:111\nrc:112): mutations: ["resolve:T_open_1"]; another-actor warning: true; NOTE: 🧵 Resolved all 2 selected review thread(s). — one thread, one mutation, counted as two and attributed to a stranger; a thread-level dedupe flips it to Resolved all 1 selected review thread(s) with no another-actor warning.
One fix shape: record thread ids confirmed resolved this round and, in this arm, treat a thread already in that set as same-thread dedupe (count it under the round's own confirms, no "another actor" warning) — or dedupe RESOLVED_IDS by thread before the loop. Either way, add a fixture with one thread carrying two comment ids asserting no resolved by another actor and Resolved all 1 selected review thread(s).
中文说明
属于同一条 review 线程的两个选中评论 id，会让循环把本轮自己的 resolve 归因给「他人」，并在 PR 可见的注记里把一条线程数成两条。这是可达的：反馈渲染器会把 Critical 根评论下的回复作为独立发现列出、各带自己的 rc: 句柄，因此同时修复二者的轮次会为一条线程写入两个 id。循环用第一个 id resolve 了线程；第二个 id 仍映射到同一线程（查找基于轮次开始时的快照，其中 isResolved 还是 false），随后单次 live 守卫读到 isResolved=true——正是本轮第一次迭代设置的——于是递增 RESOLVED_BY_OTHERS_N 并记录 "comment N was resolved by another actor"。注记随即对一条线程发布 "Resolved all 2 selected review thread(s)"。没有错误的 mutation——线程被正确 resolve 了一次——但本 PR 要让它可信的计数行同时错报了线程数和 resolve 者。见证（在未改动代码上 probe：一条线程带 databaseId 111+112，文件为 rc:111\nrc:112）：mutations: ["resolve:T_open_1"]; another-actor warning: true; NOTE: 🧵 Resolved all 2 selected review thread(s).——一条线程、一次 mutation，却被数成两条并归给陌生人；按线程去重后翻转为 Resolved all 1 selected review thread(s) 且无 another-actor 警告。
一种修法：记录本轮已确认 resolve 的线程 id，在此分支中把已在该集合内的线程按同线程去重处理（计入本轮自己的确认数，不发 "another actor" 警告）——或在循环前按线程对 RESOLVED_IDS 去重。无论哪种，补一个「一条线程带两个评论 id」的 fixture，断言无 resolved by another actor 且注记为 Resolved all 1 selected review thread(s)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| # host strings plus counts — nothing agent-controlled. | ||
| RESOLUTION_NOTE='' | ||
| if [[ "${RESOLUTION_SELECTED_N}" -gt 0 ]]; then | ||
| RESOLUTION_LEFT_N=$(( RESOLUTION_SELECTED_N - CONFIRMED_RESOLVED_N - RESOLVED_BY_OTHERS_N )) |
There was a problem hiding this comment.
[Suggestion] R2-2: RESOLUTION_LEFT_N only subtracts threads discovered resolved DURING the loop (RESOLVED_BY_OTHERS_N). An id whose thread was already resolved BEFORE the thread fetch is filtered out of the thread_id lookup by select(.isResolved | not), never reaches a guard read, increments no counter, and stays in the "left behind" / "not resolved by this round" count. The diff's own partialNote fixture pins the overcount: 'rc:111\r\n333\n999\n' with 333's thread pre-resolved expects "Resolved 1 of 3 selected review thread(s); 2 not resolved by this round" — folding the already-closed 333 together with the genuinely-unmapped 999. SKILL.md tells the agent to re-list previously fixed findings whose fix still holds, and the feedback renderer never filters by thread-resolved state, so every later round reports the same phantom residual and the count can never converge to zero — the "numbers that don't add up round after round" shape #10106 exists to kill. Witness (probe on the unmodified code): partialNote shape → NOTE: 🧵 Resolved 1 of 3 selected review thread(s); 2 not resolved by this round where only one of the two counted ids is genuinely open; a later round selecting only the pre-resolved id → NOTE: 🧵 Resolved 0 of 1 selected review thread(s); 1 not resolved by this round forever. A pre-pass counting already-resolved ids flips it to 1 not resolved / 1 left.
Suggested fix: when the thread_id lookup misses, probe whether the id sits in an already-resolved thread (a second lookup without the select(.isResolved | not) filter) and count it alongside RESOLVED_BY_OTHERS_N (or a separate already-resolved counter subtracted here), so only genuinely-open, unconfirmed threads are reported left behind; update the partialNote fixture to expect "1 not resolved".
中文说明
RESOLUTION_LEFT_N 只减去循环期间发现已被关闭的线程（RESOLVED_BY_OTHERS_N）。一个在线程拉取之前就已被关闭的线程，其 id 会被 select(.isResolved | not) 从 thread_id 查找中过滤掉，永远到不了守卫读取，不递增任何计数器，却一直留在「遗留 / 本轮未关闭」的计数里。diff 自己的 partialNote fixture 就锚定了这个高估：'rc:111\r\n333\n999\n' 且 333 的线程已预先关闭时，期望 "Resolved 1 of 3 selected review thread(s); 2 not resolved by this round"——把已关闭的 333 与真正无映射的 999 混在一起。SKILL.md 要求 agent 重新列出修复仍然成立的既往发现，而反馈渲染器从不按线程关闭状态过滤，因此后续每一轮都报告同样的幻影遗留，计数永远无法收敛到 0——正是 #10106 要消灭的「数字一轮轮对不上」的形态。见证（在未改动代码上 probe）：partialNote 形态 → NOTE: 🧵 Resolved 1 of 3 selected review thread(s); 2 not resolved by this round，而被计数的两个 id 里只有一个真正未关闭；后续某轮只选中该预关闭 id → NOTE: 🧵 Resolved 0 of 1 selected review thread(s); 1 not resolved by this round，永远如此。加一个预扫描统计已关闭线程的 id 后，翻转为 1 not resolved / 1 left。
建议修复：当 thread_id 查找未命中时，再探测该 id 是否位于已关闭的线程中（第二次查找去掉 select(.isResolved | not) 过滤），并将其计入 RESOLVED_BY_OTHERS_N（或单独的已关闭计数器，在此处一并减去），使「遗留」只统计真正未关闭、未被本轮确认的线程；同步把 partialNote fixture 的期望改为 "1 not resolved"。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| [[ "${LIVE_HEAD_RETRY_DELAY:-}" =~ ^[0-9]$ ]] || LIVE_HEAD_RETRY_DELAY=5 | ||
| LIVE_HEAD_EVER_READ='false' | ||
| for live_head_attempt in 1 2 3 4 5; do |
There was a problem hiding this comment.
[Suggestion] R2-3: The new 5-read propagation window runs unconditionally, including in shapes where its premise cannot hold. The documented premise is waiting out propagation of THIS round's OWN push — but the no-op arm pushed nothing (a contributor head landing after verification can never propagate back to VERIFIED_HEAD, so the mismatch cannot converge), and a selection that normalizes to zero valid ids (e.g. rc:abc, a shape this diff's own malformed-file test acknowledges agents write) has nothing to resolve. RESOLVED_IDS/RESOLUTION_SELECTED_N are computed before this ladder, so both shapes are gateable. Cost of not gating: four extra gh pr view calls plus ~20 s of sleeps inside the PAT-bearing step for an outcome identical to a single read — the PR's own neverConverges test runs the no-op shape and asserts headReadCount === '5'. Witness (probe on the unmodified code, production default delay): no-op-arm drift → headReads=5 sleeps=4 guard=live-head drift; zero-ids drift → headReads=5 sleeps=4 note=<empty>; a candidate fix (pushed-flag + empty-ids gate) drops them to headReads=1 sleeps=0 with the note byte-identical and headReads=0 respectively, while the pushed-arm lag control stays headReads=4 sleeps=1 resolved=resolve:T_open_1 — the #10106 wait-out survives the gating.
Suggested fix: retry only when this round actually pushed (single read otherwise — the no-op arm can pass a flag), and skip the ladder entirely when RESOLVED_IDS is empty, since nothing selected needs no head proof.
中文说明
新的 5 次读取传播等待窗口无条件运行，包括其前提不可能成立的形态。文档给出的前提是等待本轮自身 push 的传播——但 no-op 臂什么都没 push（验证之后落地的贡献者 head 永远不可能传播回 VERIFIED_HEAD，不匹配不可能收敛）；而规范化后没有任何有效 id 的选中集合（例如 rc:abc——diff 自己的畸形文件测试也承认 agent 会写出这种内容）则没有任何可 resolve 的对象。RESOLVED_IDS/RESOLUTION_SELECTED_N 在这个阶梯之前就已算好，因此两种形态都可以加门控。不加门控的代价：在持 PAT 的 step 里多 4 次 gh pr view 调用加约 20 秒 sleep，结果却与单次读取完全相同——PR 自己的 neverConverges 测试就运行在 no-op 形态下，并断言 headReadCount === '5'。见证（在未改动代码上 probe，生产默认间隔）：no-op 臂 drift → headReads=5 sleeps=4 guard=live-head drift；零 id drift → headReads=5 sleeps=4 note=<empty>；候选修复（pushed 标志 + 空 id 门控）分别降为 headReads=1 sleeps=0（注记逐字节不变）与 headReads=0，而 pushed 臂滞后对照组保持 headReads=4 sleeps=1 resolved=resolve:T_open_1——#10106 的等待在门控后依然保留。
建议修复：仅在本轮确实 push 过时重试（否则单次读取——no-op 臂可以传入一个标志），并在 RESOLVED_IDS 为空时整体跳过该阶梯——没有选中任何线程就不需要 head 证明。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Addressed all three inline findings (R2-1, R2-2, R2-3) in one batch; all are counting/cost defects in the very resolution-observability line this PR exists to make trustworthy, and each was fixed at the root with a witnessed regression fixture. Feedback points and dispositions[rc:3863854039] R2-1 — two selected ids in ONE thread counted as two and attributed to "another actor" → Fixed. [rc:3863854051] R2-2 — pre-resolved threads stayed in the residual count forever → Fixed. [rc:3863854067] R2-3 — the 5-read propagation window ran unconditionally → Fixed. [rv:5031758615] "Unresolved, please confirm" — existing triage CHANGES_REQUESTED (5028697252) asking for the PR body to be rewritten against the template and triage re-triggered → Escalated to a maintainer (process, not code). Deferred under the convergence posture (4 items) → No action this round. The review records them as "not requested in this round"; they stay as recorded. "Not reviewed" gaps → Informational. shellcheck is not installed on this runner ( Changes
Re-listed from round 1 (fix still holds at HEAD, per the SKILL re-list rule): R1-1 (3862226607), R1-2 (3862226628), R1-3 (3862226637), R1-4 (3862226641), R1-5 (3862226660) — their threads close this round if still open, or count as already resolved if the previous round closed them. Conflict notes: none ( Verification
中文说明本批一次性处理了全部三条行内发现（R2-1、R2-2、R2-3）——它们都是本 PR 赖以立足的「线程关闭可观测性」计数行本身的计数/开销缺陷，均已从根因修复并配有带见证的回归 fixture。 反馈点与处置[rc:3863854039] R2-1 —— 同一条线程中的两个选中 id 被数成两条、并被归因给「他人」→ 已修复。 [rc:3863854051] R2-2 —— 拉取前就已关闭的线程永远留在「遗留」计数里 → 已修复。 [rc:3863854067] R2-3 —— 5 次读取的传播等待窗口无条件运行 → 已修复。 [rv:5031758615]「未决，请确认」—— 既有 triage CHANGES_REQUESTED（5028697252）要求按模板重写 PR 正文并重新触发 triage → 移交维护者决策（流程问题，非代码问题）。 收敛姿态下延后的 4 条 → 本轮不处理。 评审已将其记录为「本轮不要求修改」，维持记录状态。 「未审查」缺口 → 仅告知。 本 runner 未安装 shellcheck（ 改动
按 SKILL 的重列规则重列第 1 轮发现（修复在 HEAD 仍然成立）：R1-1（3862226607）、R1-2（3862226628）、R1-3（3862226637）、R1-4（3862226641）、R1-5（3862226660）——其线程若仍开着则本轮关闭，若上一轮已关闭则计为已关闭。 冲突说明：无（ 验证
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.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- runNote duplicates runResolve's fixture-reset trio + env contract verbatim — already recorded in the round-2 review body (review 5031758615) deferral list
- mid-list stop records live-head drift for unreadable reads — already recorded in the round-2 review body (review 5031758615) deferral list
Unresolved, please confirm:
- [Critical] existing CHANGES_REQUESTED review (5028697252, @qwen-code-ci-bot triage): requests the PR body be rewritten against the PR template and triage re-triggered — a process blocker that cannot be ruled on from code; the body still uses a free-fo…
Not reviewed: build-and-test — CI check 'Test (macos-latest, Node 22.x)' was skipped (gated to merge_group/schedule/workflow_dispatch) and its suite (scripts/tests, which covers this diff) did not run on macOS in this review; platform-portability coverage rests on the probe's declared BSD-sed model plus static audit, and the lane is exactly where R3-2 goes red.
Not reviewed: the executable-script lint — .github/scripts/autofix-push-and-report.sh: shellcheck is not installed.
Not explored to full depth (tool budget reached): "agent 6b": empirical run of BSD sed's \r handling (no macOS/BSD userland available on this Linux runner) — verification stage can confirm by running qwen-autofix-workfl….
Convergence: round 3 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: .github/scripts/autofix-push-and-report.sh (findings in round 2; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查：build-and-test — CI check 'Test (macos-latest, Node 22.x)' was skipped (gated to merge_group/schedule/workflow_dispatch) and its suite (scripts/tests, which covers this diff) did not run on macOS in this review; platform-portability coverage rests on the probe's declared BSD-sed model plus static audit, and the lane is exactly where R3-2 goes red。
未审查：the executable-script lint — .github/scripts/autofix-push-and-report.sh: shellcheck is not installed。
未探索到全部深度（达到工具调用预算）："agent 6b"：empirical run of BSD sed's \r handling (no macOS/BSD userland available on this Linux runner) — verification stage can confirm by running qwen-autofix-workfl…。
收敛情况：第 3 轮发布了 3 条行内评论，其中 3 条是首次提出；上一轮发布了 3 条（其中 3 条首次提出）。发现反复回到同一批文件：.github/scripts/autofix-push-and-report.sh（第 2 轮已出过发现，本轮又有 2 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # One spelling of the id grammar (optional rc: prefix, optional | ||
| # trailing CR, digits only, deduplicated), shared by the counter | ||
| # and the resolve loop below. | ||
| RESOLVED_IDS="$(sed 's/\r$//; s/^rc://' "${WORKDIR}/resolved-comments.txt" | grep -E '^[0-9]+$' | sort -u || true)" |
There was a problem hiding this comment.
[Critical] R3-1: The new shared id normalization strips CR with a GNU-sed-only \r regex escape. BSD sed (the macOS test lane) does not interpret \r as carriage return, so CRLF-terminated ids keep the CR, fail the grep -E '^[0-9]+$' filter, and the extracted-block tests this PR adds go red on ci.yml's test_macos lane — a lane this PR does not run.
The lane: test_macos (macos-latest) is gated to merge_group/schedule/workflow_dispatch — skipped for this PR, but it runs in the merge queue and nightly (test:ci → test:scripts; scripts/tests/vitest.config.ts collects this suite on darwin — the exclude is win32-only). With the fixture 'rc:111\r\n333\n999\n' (CRLF is the production spelling — that is why CR-stripping exists), BSD sed leaves the CR on 111, the digit grep drops it, T_open_1 is never resolved, and expect(matching.resolved).toEqual(['resolve:T_open_1']) plus the guard-note loop's 'resolved 0 of 3 selected thread(s), 3 left for a later round' fail. Pre-change the loop stripped CR portably in bash (rc_id="${rc_id%$'\r'}" — visible as - lines in this diff), so this diff introduces the lane red; the first red lands in the merge queue. Production is unaffected (qwen-autofix.yml runs the script on Linux runners only). ci.yml itself records #9220 — this exact defect class — having shipped to main once.
Witness (probe; declared BSD-sed model on this Linux host — the macOS runner itself was not exercised):
same extracted production block, CRLF fixture:
GNU sed: resolved.log: [resolve:T_open_1] — "confirmed 1 selected review thread(s)" (Linux lane green; PR's own vitest test: 1 passed)
BSD-model: resolved.log: [] — "confirmed 0 selected review thread(s)" (macOS lane red)
Repo precedent exists for exactly this reason: the withGnuSed shim in scripts/tests/qwen-repo-hygiene-workflow.test.js ("this suite also runs in the macOS merge-queue job, where BSD sed needs …") and the darwin rewrite in scripts/tests/qwen-triage-workflow.test.js.
Suggested fix — fix the test side per repo precedent (shim sed on darwin, as withGnuSed does), or restore a portable spelling in the script:
RESOLVED_IDS="$(tr -d '\r' < "${WORKDIR}/resolved-comments.txt" | sed 's/^rc://' | grep -E '^[0-9]+$' | sort -u || true)"(note tr -d also strips interior CRs, a slight semantics change for malformed input; the test-side shim is the smaller change).
Fix witness: scripts/tests/qwen-autofix-workflow.test.js — the rc:111\r\n333\n999\n fixture must yield 'confirmed 1 selected review thread(s) resolved' and 'resolved 0 of 3 selected thread(s), 3 left for a later round' on the macOS lane; after applying the fix, remove it again and confirm those assertions go red.
中文说明
[Critical] R3-1:新的共享 id 归一化用 GNU sed 专属的 \r 转义去 CR。macOS 测试泳道的 BSD sed 不把 \r 解释为回车,带 CR 的 id 会保留 CR、被 grep -E '^[0-9]+$' 过滤,本 PR 新增的抽块测试将在 ci.yml 的 test_macos 泳道变红——而该泳道在本 PR 上并不运行。
泳道情况:test_macos(macos-latest)仅在 merge_group/schedule/workflow_dispatch 触发——本 PR 跳过,但合并队列与每晚例行都会跑(test:ci → test:scripts;scripts/tests/vitest.config.ts 在 darwin 上收集该套件,排除仅对 win32 生效)。用夹具 'rc:111\r\n333\n999\n'(CRLF 正是生产写法,所以才有去 CR)时,BSD sed 保留 111 的 CR,数字 grep 将其丢弃,T_open_1 不会被 resolve,expect(matching.resolved).toEqual(['resolve:T_open_1']) 与守卫注记循环里的 'resolved 0 of 3 selected thread(s), 3 left for a later round' 失败。改动前循环用 bash 可移植地去 CR(rc_id="${rc_id%$'\r'}",diff 中的 - 行可见),因此是本 diff 引入的泳道变红;首次变红将出现在合并队列。生产不受影响(qwen-autofix.yml 只在 Linux runner 上跑该脚本)。ci.yml 自己记录了 #9220——同一类缺陷——曾经进过 main。
证据(探针;在本 Linux 主机上用声明的 BSD-sed 模型——未真正跑 macOS runner):同一抽出生产块、同一 CRLF 夹具,GNU sed 得到 resolved.log: [resolve:T_open_1]、'confirmed 1 selected review thread(s)'(Linux 泳道绿;PR 自带 vitest 测试通过 1 例);BSD 模型得到 resolved.log: []、'confirmed 0 selected review thread(s)'(macOS 泳道红)。
仓库已有同因先例:scripts/tests/qwen-repo-hygiene-workflow.test.js 的 withGnuSed 垫层('该套件也在 macOS 合并队列任务中运行,BSD sed 需要……')与 scripts/tests/qwen-triage-workflow.test.js 的 darwin 改写。
建议修复——按仓库先例在测试侧处理(像 withGnuSed 那样在 darwin 上垫一个 GNU sed),或把脚本改回可移植写法(见上方代码块;注意 tr -d 也会去掉行内 CR,对畸形输入语义略有变化,测试侧垫层是更小的改动)。
修复见证:scripts/tests/qwen-autofix-workflow.test.js——rc:111\r\n333\n999\n 夹具必须在 macOS 泳道得到 'confirmed 1 selected review thread(s) resolved' 与 'resolved 0 of 3 selected thread(s), 3 left for a later round';应用修复后再移除它,确认这些断言变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # host strings plus counts — nothing agent-controlled. | ||
| RESOLUTION_NOTE='' | ||
| if [[ "${RESOLUTION_SELECTED_N}" -gt 0 ]]; then | ||
| RESOLUTION_LEFT_N=$(( RESOLUTION_SELECTED_N - CONFIRMED_RESOLVED_N - RESOLVED_BY_OTHERS_N - ALREADY_RESOLVED_N )) |
There was a problem hiding this comment.
[Suggestion] R3-2: On guard-refused rounds and mid-list breaks, the round-report note counts unique ids as "selected thread(s)" — the same-thread dedupe (~lines 265-269) and the already-resolved probe (~lines 251-262) execute only inside the CAN_RESOLVE_THREADS == 'true' resolve loop, while RESOLUTION_SELECTED_N (line 135) is the deduplicated id count. The design-doc clause this same PR adds (docs/design/autofix-resolve-fixed-review-threads.md: "The counts are thread-accurate: two selected ids that sit in one thread count once, and an id whose thread was already resolved before the fetch is subtracted from the residual") is therefore false for exactly the refusing rounds this observability feature was built for.
Failure shape: a round whose resolved-comments.txt selects a Critical root and its reply (two ids in ONE thread — the R2-1 shape this PR's own tests pin) and then hits any up-front guard (salvage merge, live-head drift, live-head unreadable) posts "resolved 0 of 2 selected thread(s), 2 left for a later round" when ONE thread is selected/left. Under a persistently refusing guard (expired PAT → live-head unreadable, the #10106 scenario) the inflated residual re-posts every round and never converges — the exact non-convergence the R2-2 probe was added to kill, surviving on the skip path. THREADS_JSON is fetched even when the guard refuses (the fetch gate is [[ -s resolved-comments.txt || -s comment-replies.json ]]), so thread-accurate classification is available on the skip path.
Witness (probe — extracted production note block, stub gh, scratch tree):
rc:111+rc:112 (ONE thread), salvage-merge skip → "resolved 0 of 2 selected thread(s), 2 left" (true state: 1 thread)
333 (thread resolved pre-fetch), skip → "resolved 0 of 1 selected thread(s), 1 left" (converges to nothing)
2-thread selection, mid-list drift break → "stopped early … 0 of 3 selected thread(s), 3 left" (true state: 2 threads)
a candidate fix applying the loop's classification over the already-fetched THREADS_JSON flipped every case to thread-accurate
Suggested fix: when composing the note for a round that never entered (or broke out of) the loop, map the selected ids through the already-fetched THREADS_JSON with the same jq probes the loop uses, count distinct threads, and subtract pre-fetch-resolved ones; or, minimally, scope the design-doc clause to rounds that attempted resolution and word the guarded note in ids.
Fix witness: extend the guard-note loop in scripts/tests/qwen-autofix-workflow.test.js — write rc:111\nrc:112\n under any skip guard and assert 'resolved 0 of 1 selected thread(s), 1 left' (today: 0 of 2 … 2 left); removing the fix turns it red.
中文说明
[Suggestion] R3-2:守卫拒绝或中途中止的轮次里,轮次报告注记把去重后的 id 数当作 "selected thread(s)" 来报——同线程去重(约 265-269 行)与已关闭探测(约 251-262 行)只在 CAN_RESOLVE_THREADS == 'true' 的 resolve 循环内执行,而 RESOLUTION_SELECTED_N(135 行)是去重后的 id 计数。本 PR 新加的设计文档句(docs/design/autofix-resolve-fixed-review-threads.md:'计数按线程精确:同一线程里的两个选中 id 只计一次,拉取前已关闭线程的 id 从残留中扣除')恰恰在本可观测性功能针对的拒绝轮次上不成立。
失败形态:某轮 resolved-comments.txt 选中一个 Critical 根评论及其回复(同一线程里的两个 id,即本 PR 测试已锚定的 R2-1 形态),随后任一前置守卫拒绝(salvage merge、live-head drift、live-head unreadable):实际只选中/遗留一条线程,PR 评论却写 'resolved 0 of 2 selected thread(s), 2 left for a later round'。若守卫持续拒绝(PAT 过期 → live-head unreadable,即 #10106 场景),虚高的残留数每轮重发、永不收敛——正是 R2-2 探测要消灭的非收敛,在跳过路径上复活。守卫拒绝时 THREADS_JSON 仍会拉取(拉取门为 [[ -s resolved-comments.txt || -s comment-replies.json ]]),跳过路径上做线程精确分类所需的数据是现成的。
证据(探针——抽出的生产注记块、stub gh、临时树):见上方英文部分的四行探针输出;把循环同款分类应用到已拉取的 THREADS_JSON 的候选修复,能把所有情形翻转为线程精确。
建议修复:对未进入(或提前退出)循环的轮次,在合成注记时用循环同款 jq 探测把选中 id 映射到已拉取的 THREADS_JSON,按不同线程计数并减去拉取前已关闭的线程;最低限度也应把设计文档该句限定为'尝试过 resolve 的轮次',并把守卫注记的单位改为 id。
修复见证:扩展 scripts/tests/qwen-autofix-workflow.test.js 的守卫注记循环——写入 rc:111\nrc:112\n 并施加任一跳过守卫,断言 'resolved 0 of 1 selected thread(s), 1 left'(当前为 0 of 2 … 2 left);移除修复后该断言应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| - **Fail closed for resolution:** an unresolved thread is recoverable; an incorrectly resolved thread can hide a real defect. | ||
| - **Skip resolution after race merge:** rerunning the full deterministic gate inside the PAT-bearing publish step would duplicate expensive logic and run branch-controlled scripts with credentials in scope. A later review round can safely resolve the thread. | ||
| - **Query live PR state immediately before mutation:** workflow concurrency cannot prevent direct contributor pushes. | ||
| - **Bounded wait for the initial live-head equality:** the PR read model is eventually consistent, and a `headRefOid` read seconds after this round's own push routinely still returns the previous head — on PR #9729 every pushed round declared drift on that single stale read and silently skipped (issue #10106). The initial equality check retries briefly before declaring drift, and names the refusal `live-head unreadable` rather than `live-head drift` when no attempt returned any head, so an expired PAT or an API outage reads differently from a contributor push. The per-mutation guards stay single-shot: once the head has been observed equal, a later mismatch means it actually moved. The window runs only when the round pushed and selected at least one thread: its premise is THIS round's own push propagating, and a round that pushed nothing — or selected nothing to resolve — has no head proof to run. |
There was a problem hiding this comment.
[Suggestion] R3-3: This added "Bounded wait" bullet ends "a round that pushed nothing — or selected nothing to resolve — has no head proof to run", which contradicts the implementation, the script's own new comment, and this PR's tests: a no-push round WITH a selection performs ONE live-head read (.github/scripts/autofix-push-and-report.sh:166-167 — LIVE_HEAD_ATTEMPTS=5 then [[ "${ROUND_PUSHED:-}" == 'true' ]] || LIVE_HEAD_ATTEMPTS=1; script comment "so one read decides") and can refuse with live-head drift / live-head unreadable. The clause is true only for the selected-nothing case (zero reads).
Cost: the design doc is this mechanism's normative record — a maintainer diagnosing a live-head drift note on a no-op round (which the round report now visibly produces) consults it and concludes a no-op round cannot produce one; or a future change implemented from this clause deletes the single-read check, removing the live-head guard from the no-op arm entirely (letting a no-op round resolve threads after a contributor pushed).
Witness: verified verbatim at the reviewed commit; the enclosing test executes the real extracted block — npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js -t 'resolves only the review threads whose findings it implemented' → 1 passed, with noopNeverConverges asserting headReadCount '1' and noopDriftNote asserting the live-head drift skip note under ROUND_PUSHED: 'false' — the behavior this clause denies.
Suggested fix — reword to match the pinned behavior, e.g.:
…its premise is THIS round's own push propagating, so a round that pushed nothing decides on a single read instead of the window — a mismatched head there only moves further away — and a round that selected nothing to resolve has no head proof to run at all.
中文说明
[Suggestion] R3-3:新增的 "Bounded wait" 条目结尾写"没推送——或没选中任何线程——的轮次没有 head proof 可跑",与实现、脚本自己的新注释及本 PR 测试矛盾:有选中的未推送轮仍会读一次 live-head(.github/scripts/autofix-push-and-report.sh:166-167——LIVE_HEAD_ATTEMPTS=5 之后 [[ "${ROUND_PUSHED:-}" == 'true' ]] || LIVE_HEAD_ATTEMPTS=1;脚本注释"一次读即决"),并可能以 live-head drift / live-head unreadable 拒绝。该句只对"没选中任何线程"(零次读)成立。
代价:设计文档是该机制的规范记录——维护者排查 no-op 轮报告里的 live-head drift 注记(轮次报告现在会可见地产生它)时查文档,会得出"no-op 轮不可能出现该拒绝"的错误结论;或后人按此句实现改动时删掉单次读检查,使 no-op 臂完全失去 live-head 守卫(贡献者 push 之后 no-op 轮仍可能去 resolve 线程)。
证据:在被审提交上逐字核对;外围测试执行真实抽出的脚本块——npx vitest run … -t 'resolves only the review threads whose findings it implemented' → 1 通过,其中 noopNeverConverges 断言 headReadCount 为 '1',noopDriftNote 断言 ROUND_PUSHED: 'false' 下出现 live-head drift 跳过注记——正是该句否认的行为。
建议修复——改写为与实现一致(见上方英文部分的示例措辞):未推送的轮用单次读代替窗口;只有没选中任何线程的轮才完全没有 head proof。
— qwen3.8-max via Qwen Code /review (v0.22.2)
…reak paths (#10117) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix address-review round — PR #10117Commit: Feedback points and dispositions[Critical] R3-1 — GNU-sed-only [Suggestion] R3-2 — skip and mid-list-break notes count ids, not threads (rc:3865479254) — RESOLVED IN THE CODE. Reproduced test-first: new witness assertions run against the pre-fix code reported [Suggestion] R3-3 — design-doc "Bounded wait" bullet contradicts the implementation (rc:3865479261) — RESOLVED IN THE CODE. Reworded the clause to the pinned behavior: the head proof runs whenever the round selected at least one thread; a pushed round waits out the window, a round that pushed nothing decides on a single read, and only a round that selected nothing has no head proof to run. Witnessed by the existing [rv:5033666744] "Unresolved, please confirm": triage review 5028697252 requests the PR body be rewritten against the PR template and triage re-triggered — ESCALATED for a maintainer's decision (not resolved, deliberately). This is a process blocker on the PR object itself, not a code finding, and this mode cannot act on it: the agent has no GitHub write capability (the workflow owns all network writes), and the address-review flow consumes only round outputs — Round-2 deferred suggestions (runNote/runResolve fixture-reset duplication; mid-list stop naming The review's "Not reviewed" disclosures (macOS lane not exercised here, shellcheck not installed, BSD-sed empirical run) and its convergence/mechanism-health observations required no code action; the R3-1 fix removes the lane red the macOS disclosure points at. Mutation probes (each restored, suite green after)
Verification
中文说明Autofix address-review 轮次 — PR #10117提交： 反馈点与处置[Critical] R3-1 — 共享 id 归一化使用 GNU sed 专属的 [Suggestion] R3-2 — 跳过与中途中止的注记按 id 计数而非线程（rc:3865479254）— 已在代码中解决。 以测试先行方式复现:新见证断言在修复前代码上对"同一线程两个 id + 跳过守卫"的选择报出 [Suggestion] R3-3 — 设计文档 "Bounded wait" 条目与实现矛盾（rc:3865479261）— 已在代码中解决。 将该句改写为与锚定行为一致:只要轮次选中了至少一条线程就会执行 head 证明;推送过的轮次等待完整窗口,未推送的轮次以单次读决断,只有未选中任何线程的轮次才完全没有 head 证明。见证为既有的 [rv:5033666744] "未决,请确认":三角审查 5028697252 要求按 PR 模板重写 PR 正文并重新触发三角审查 — 升级给维护者决定（有意不关闭）。 这是 PR 对象本身上的流程阻塞,不是代码发现,本模式无法处理:agent 没有 GitHub 写权限（所有网络写入由工作流负责）,且 address-review 流程只消费轮次产物—— 第 2 轮延后的建议（runNote/runResolve 夹具重置重复;中途中止把不可读读记为 审查中"未审查"披露（本机未跑 macOS 泳道、shellcheck 未安装、BSD sed 实测未做）与收敛/机制健康观察不需要代码动作;R3-1 的修复消除了 macOS 披露所指的泳道变红。 变异探针（均已还原,还原后套件绿）
验证
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.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- mid-list stop records live-head drift for unreadable reads — already recorded in the round-2 review body (review 5031758615) deferral list
- runNote duplicates runResolve's fixture-reset trio + env contract verbatim — already recorded in the round-2 review body (review 5031758615) deferral list
Unresolved, please confirm:
- [Critical] existing CHANGES_REQUESTED review (5028697252, @qwen-code-ci-bot triage): requests the PR body be rewritten against the PR template and triage re-triggered — a process blocker on the PR object that no code change in this diff can resolve; n…
Not reviewed: build-and-test — CI check 'Test (macos-latest, Node 22.x)' was skipped (gated to merge_group/schedule/workflow_dispatch) and the suite covering this diff did not run on macOS in this review; it ran green on Linux (219/219), and the portable tr CR-strip plus its static pin close the BSD-sed defect class the lane exists to catch.
Not reviewed: the executable-script lint — .github/scripts/autofix-push-and-report.sh: shellcheck is not installed.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
.github/scripts/autofix-push-and-report.sh:374 — [probe] reply mirror block keeps its own id-grammar spelling; the consolidation stops one consumer shortscripts/tests/qwen-autofix-workflow.test.js:18026 — [probe] ROUND_PUSHED arm wiring pinned only by script-wide occurrence counts; a swap of the two assignments survives every test
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查：build-and-test — CI check 'Test (macos-latest, Node 22.x)' was skipped (gated to merge_group/schedule/workflow_dispatch) and the suite covering this diff did not run on macOS in this review; it ran green on Linux (219/219), and the portable tr CR-strip plus its static pin close the BSD-sed defect class the lane exists to catch。
未审查：the executable-script lint — .github/scripts/autofix-push-and-report.sh: shellcheck is not installed。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix address-review round — PR #10117 (review round 4)No code changes this round. The round-4 review posted zero new findings (ledger: 1. [Critical] PR body vs. template + triage re-trigger (triage review 5028697252, re-raised by the round-4 review) — verified, escalated, needs maintainer actionThe template claim is verified. I fetched the live PR body through the public GitHub API: it is free-form ( The "whole diff" half of that review is stale. Triage also said the diff carried 13 commits of PreToolUse ask-bounce work (#9434/#9441) that the body never mentions. That no longer matches the branch: the live head ( Why this round cannot fix it. Editing the PR body is a GitHub write. This round holds no credentials and is forbidden from performing GitHub writes, and the address-review phase of the autofix workflow has no PR-body update path either (no Explicit question for the maintainer: please replace the PR body with the ready-to-paste template-conformant draft below (it describes the whole current diff, including the exact-counts, drift-vs-unreadable, gated head-proof, and portable-CR-strip work the current body does not mention), then re-trigger Ready-to-paste draft PR body (template-conformant, bilingual per repo convention)## What this PR does
The autofix loop resolves review threads for findings a round fixed in code, but that pass can fail closed for several distinct reasons, and until now each refusal was visible only as a run-log warning — nothing on the PR said which guard refused, or that resolution was skipped at all. This PR makes each refusal visible on the PR itself and fixes the one guard found to be over-broad in production. Each refusing guard records a stable name, and both round-report arms carry one host-authored line naming the guard and counting selected, resolved, and left-behind threads; healthy rounds report the positive count on the same line, so the mechanism dying is visible by the numbers going wrong. The counts are thread-accurate on every path — including up-front skips and mid-list stops — so two selected ids in one thread count once, and a thread already resolved before the fetch is subtracted from the residual instead of being reported left behind. The initial live-head equality check now retries briefly (up to five reads, default 5 s apart, delay clamped to a single digit) before declaring drift, waiting out propagation of the round's own push; when no read returns any head at all the refusal is named unreadable rather than drift, so an expired credential or API outage reads differently from a contributor push. The head proof runs only when the round selected at least one thread, and the per-mutation guards stay single-shot. Shared comment-id normalization strips carriage returns portably, so CRLF-authored resolution lists match on BSD and GNU userlands alike. The design doc records the decisions and the workflow contract tests pin the note composition, guard names, counts, retry behavior, and the clamp.
## Why it's needed
Fixes #10106. On PR #9729, 0 of 90 review threads were resolved across ~16 rounds while every round report read like resolution was working: every pushed round tripped the initial live-head equality check, because the PR read model is eventually consistent and a head read seconds after the round's own push routinely still returns the previous head — one immediate read declared drift every time, so every pushed round silently skipped. The guard is correctly strict in intent but was over-broad in implementation, and with refusals invisible on the PR the mechanism could die silently.
## Reviewer Test Plan
### How to verify
Run the workflow contract suite, which extracts the workflow's shell blocks and executes them against stubbed GitHub responses: `npx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow.test.js`. It pins every up-front guard name and count line, the mid-list stop names, the healthy-round positive line, no-guard partial counts, the fetch-incomplete annotation, no-selection silence, both report arms, lag-then-converge (stale first read, resolution proceeds), never-converges (skips only after exhausting all five reads), the delay clamp, and the portable carriage-return strip. CI's Linux Test lane runs the same suite.
### Evidence (Before & After)
N/A — the change is CI workflow machinery (round-report line composition and resolve-pass guards), not user-visible CLI output.
### Tested on
| OS | Status |
| :--------: | :----: |
| 🍏 macOS | ⚠️ |
| 🪟 Windows | ⚠️ |
| 🐧 Linux | ✅ |
The macOS/Windows lanes are gated to merge_group/schedule/workflow_dispatch and did not run for this PR; the Linux lane ran the suite green.
### Environment (optional)
N/A — workflow script plus Node contract tests; no CLI runtime involved.
## Risk & Scope
- Main risk or tradeoff: the resolve pass runs in the PAT-bearing publish step; the bounded wait adds at most ~20 s per pushed round, and fail-closed semantics are unchanged — no thread is ever resolved unless the live head was observed equal to the deterministically verified commit.
- Not validated / out of scope: the macOS/Windows lanes for this diff (gated); the remaining guards were checked against the production failure shape and intentionally left as-is.
- Breaking changes / migration notes: none.
## Linked Issues
Fixes #10106. Observed symptom: PR #9729 (0 of 90 threads resolved across ~16 rounds). Design decisions: `docs/design/autofix-resolve-fixed-review-threads.md`.
<details>
<summary>中文说明</summary>
## 本 PR 做了什么
autofix 循环会为某轮已在代码中修复的发现关闭对应的 review 线程,但该环节可能因多种不同原因失败关闭,而此前每种拒绝都只以 run 日志里的警告形式出现——PR 上没有任何信息说明是哪个守卫拒绝、甚至没有说明 resolve 被跳过。本 PR 让每次拒绝在 PR 上可见,并修复了唯一一个在生产中被证实过宽的守卫。每个拒绝守卫记录稳定名称,两个轮次报告臂各带一行 host 生成的说明,点名守卫并计数选中/已关闭/遗留的线程;健康轮次在同一行报告正数,机制失效时数字出错即暴露。计数在所有路径上保持逐线程准确——包括前置跳过与中途中止——同一线程的两个选中 id 只计一次,抓取前已关闭的线程从遗留数中扣除而不是被报为遗留。初始 live-head 等值检查现在会短暂重试(最多读 5 次,默认间隔 5 秒,间隔钳制为单个数字)再判定漂移,以等待本轮自身 push 的传播;当所有读取都没能返回任何 head 时,拒绝被记为"不可读"而非"漂移",凭据过期或 API 故障与贡献者推送由此可区分。head 证明只在本轮至少选中一个线程时才运行,逐 mutation 的守卫保持单次。共享的评论 id 归一化以可移植方式去除回车符,因此 CRLF 编写的 resolve 清单在 BSD 与 GNU 用户环境上都能匹配。设计文档记录了这些决策,工作流契约测试锚定了注记合成、守卫名、计数、重试行为与钳制。
## 为什么需要
修复 #10106。在 PR #9729 上,约 16 轮中 90 条 review 线程 0 条被关闭,而每轮报告读起来都像 resolve 在正常工作:每个 pushed 轮都踩中初始 live-head 等值检查——因为 PR 读模型是最终一致的,在本轮自己的 push 之后数秒读取 head,常规性地仍返回旧 head;单次立即读每次都判成漂移,于是每个 pushed 轮都静默跳过。该守卫意图上正确地严格,实现上过宽;又因拒绝在 PR 上不可见,该机制可以悄无声息地死掉。
## 评审者测试计划
### 如何验证
运行工作流契约测试套件——它提取工作流的 shell 块,并以打桩的 GitHub 响应执行:`npx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow.test.js`。它锚定了所有前置守卫名与计数行、中途中止守卫名、健康轮正数行、无守卫的部分计数、拉取不完整标注、无选中时静默、两个报告臂、滞后后收敛(首读为旧值仍继续 resolve)、始终不收敛(耗尽全部 5 次读后才跳过)、间隔钳制,以及可移植的回车符去除。CI 的 Linux Test 通道运行同一套件。
### 证据(前后对比)
N/A——改动是 CI 工作流机制(轮次报告行合成与 resolve 环节守卫),不涉及用户可见的 CLI 输出。
### 测试环境
| OS | 状态 |
| :--------: | :----: |
| 🍏 macOS | ⚠️ |
| 🪟 Windows | ⚠️ |
| 🐧 Linux | ✅ |
macOS/Windows 通道仅限 merge_group/schedule/workflow_dispatch 触发,本 PR 未运行;Linux 通道全套测试为绿。
### 环境(可选)
N/A——工作流脚本加 Node 契约测试,不涉及 CLI 运行时。
## 风险与范围
- 主要风险或权衡:resolve 环节运行在持 PAT 的发布 step 中;有界等待为每个 pushed 轮最多多花约 20 秒,且 fail-closed 语义不变——只有观察到 live head 等于确定性验证过的 commit 才会关闭任何线程。
- 未验证/超出范围:本 diff 的 macOS/Windows 通道(受触发条件限制);其余守卫已按生产故障形态逐一核对,刻意保持原样。
- 破坏性变更/迁移说明:无。
## 关联 Issue
修复 #10106。观察到的症状:PR #9729(约 16 轮中 90 条线程 0 条被关闭)。设计决策见 `docs/design/autofix-resolve-fixed-review-threads.md`。
</details>2. Suggestions and probes the reviewer deferred — no action requested this round
All four remain recorded in this thread's review bodies for a future round. I did not implement them this round because the reviewer explicitly deferred them, and adding code against an explicit deferral would grow the diff without unblocking anything — the only unresolved item is the process action in section 1, which code cannot fix. 3. Not-reviewed disclosures — acknowledged
4. Mechanism health — noted, not acted on, exactly as the review itself states.Commands run this round were all read-only (git log/diff/ls-remote, public GitHub API reads of the PR and issue, inspection of the workflow and report script) — no code changed, so no build/test run applies. 中文说明Autofix address-review 轮次 — PR #10117(评审第 4 轮)本轮不做任何代码改动。第 4 轮评审未发布任何新发现(账本: 1. [Critical] PR 正文不符合模板 + 需重新触发 triage(triage 评审 5028697252,第 4 轮评审再次提出)— 已核实,已上报,需维护者操作模板问题已核实。 我通过公开的 GitHub API 读取了当前 PR 正文:其为自由结构( 该评审中"描述完整 diff"的那一半已过时。 triage 还说 diff 包含 13 个针对 #9434/#9441 的 PreToolUse ask-bounce 提交、正文只字未提。这已不符合当前分支:live head( 本轮为何无法修复。 编辑 PR 正文属于 GitHub 写操作。本轮不持有任何凭据、被禁止执行 GitHub 写操作,而且 autofix 工作流的 address-review 阶段也没有任何更新 PR 正文的路径(整个循环中不存在 给维护者的明确问题: 请用下方现成的、符合模板的草稿替换 PR 正文(草稿描述了当前完整 diff,包括现正文未提及的精确计数、drift 与 unreadable 拆分、head 证明门控、可移植回车符去除等工作),然后重新触发 (草稿正文为中英双语,见上方英文部分的折叠块"Ready-to-paste draft PR body",此处不再重复全文。) 2. 评审者已延后的建议与探针发现 — 本轮不要求处理
这四项都仍记录在本线程的评审正文中,留待后续轮次。本轮不实现它们,因为评审者明确延后了它们;在明确延后之下仍添加代码只会扩大 diff 而无助于解锁任何事项——唯一未决事项是第 1 节的流程操作,代码无法修复它。 3. "未审查"披露 — 已知悉
4. 机制健康 — 已知悉,不据此行动,与评审自身所述一致。本轮运行的命令全部为只读(git log/diff/ls-remote、通过公开 GitHub API 读取 PR 与 issue、检查工作流与报告脚本)——未改动任何代码,因此不涉及构建/测试运行。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: Skipped because GitHub had not computed the PR merge ref after several retries — try again shortly. 中文 — 判定：
|
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 179 passed · 0 failed · 179 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：179 通过 · 0 失败 · 179 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10117 deep verification (round 2) —
|
| # | previous-round item | status at the re-measured head |
|---|---|---|
| 1 | Central-claim A/B, cell-2 flip 0/1 → 1/1 resolved |
stands — reproduced with identical counts (table below) |
| 2 | Fail-closed cells (never converges / window cap / no-op single read / all reads fail / moved before mutation) | stands — all re-run, same oracles on both arms |
| 3 | Mutation matrix 8/8 killed | stands, extended — 12/12 killed (M9–M12 added, covering the read-tolerance ` |
| 4 | Gates 219/219 on both arms | stands — identical counts re-measured |
| 5 | shellcheck "not installed and not installable (apt-get has no route)" | superseded — the container runs as uid 1000 (apt needs root) but has working network egress; the official static binary v0.10.0 runs fine and the head script is clean at default severity (liveness: planted echo $1 → SC2086 reported) |
| 6 | Wall-clock retry cost "not timed" | superseded — measured: +20.1 s worst case, clamp defeats all plant shapes (witness 04-wallclock-and-clamp.png) |
| 7 | Findings: none | confirmed — this round also found none |
Scope
A script/workflow PR: .github/scripts/autofix-push-and-report.sh (+166/−24), its test harness scripts/tests/qwen-autofix-workflow.test.js (+301/−3), and a 5-line design-doc addition. No production package code.
- Central claim: the initial live-head equality check retries (up to 5 reads) after a pushed round, waiting out propagation of the round's own push, instead of declaring
live-head drifton one stale read — while fail-closed semantics are unchanged. - Secondary claim 1: every refusing guard records a stable name and both round-report arms carry one host-authored note line with thread-accurate counts; no agent-authored content enters it.
- Secondary claim 2: thread-accurate counting (two ids of one thread count once; a pre-fetch-resolved thread is subtracted),
live-head unreadablevsdriftsplit, portable CR strip viatr, and an empty selection spending zero head-proof reads.
Per-commit attribution was out of reach (see Not covered); the aggregate HEAD^1..HEAD diff is what was verified.
Central claim — A/B load-bearing proof
Harness: harness/ab-harness.mjs extracts the guard+resolve block verbatim from each arm's script (same anchors the repo's own test uses: CAN_RESOLVE_THREADS='false' → the 🧵 confirmed line, +2 lines) and drives it through real bash child processes under set -euo pipefail, with a stubbed gh on PATH that enforces the real CLI invocation shapes (--json headRefOid --jq '.headRefOid // ""', --paginate on the threads fetch, the @tsv filter on the per-thread guard — shape violations exit 2) and counts head reads and invocations separately. Fixture mirrors the repo test: 111 implemented (112 a reply in the same thread), 333 already resolved before the fetch, 999 matches nothing. Base arm ran against git worktree add tmp/base-tree HEAD^1; realpath confound n/a — the harness reads each arm's script file directly.
Read counts below include the post-mutation guard read (initial check + per-thread pre-guard + post-guard per resolved thread).
| # | cell (oracle) | base a82a11a0 |
head b5904c3f |
|---|---|---|---|
| 1 | immediate match: resolve issued, no skip | resolves 1/1, 3 reads | resolves 1/1, 3 reads |
| 2 | lag-then-converge (read 1 stale = own push not propagated, read 2 equal) | skip, 0/1 resolved, 1 read (the #9729 failure) | resolves 1/1, 4 reads |
| 3 | never converges (true drift): skip + reads | skip, 1 read | skip, 5 reads |
| 4 | convergence on read 6 of a 5-read window | skip, 1 read | skip, reads capped at 5 |
| 5 | no-op round (ROUND_PUSHED=false), stale head |
skip, 1 read | skip, 1 read |
| 6 | every gh read fails (expired PAT shape): skip + attempts |
skip, 1 attempt (0 reads) | skip, 5 attempts (0 reads) |
| 7 | head equal at initial check, moved before first mutation | break, 0 resolved, 2 reads | break, 0 resolved, 2 reads (per-mutation guard still single-shot) |
| 8 | zero valid ids (rc:abc): exit 0 + reads |
exit 0, resolves nothing, 1 read spent | exit 0, resolves nothing, 0 reads |
The cell-2 flip — 0/1 resolved → 1/1 resolved on identical input — is the load-bearing proof; cells 3–7 show the fix narrows nothing else; cell 8 shows the empty-selection gate additionally stops spending PAT reads on nothing. Witnesses: 01-ab-base-arm.png (34/34 assertions), 02-ab-head-arm.png (45/45); raw logs logs/ab-base.txt, logs/ab-head.txt. Four harness-expectation errors found during the run were mine, not the PR's, and are recorded for auditability: (1) the post-mutation guard read makes cell 1 cost 3 reads, not 2 (diagnosed by tracing every gh call); (2) cell 6's failed reads count as invocations, not reads, because the stub exits before counting (the harness now counts both); (3) the cell-2 head read count is 4, not 3, because the pre/post mutation guards fall back to LIVE_HEAD once the 2-entry sequence is exhausted; (4) the healthy-note cell first used the 3-id fixture, whose correct note is the partial one — the healthy arm needs a single-id fixture, as in the repo's own healthyNote.
Secondary claim 1 — guard names and the note line (head arm)
Behavioral: the extended block (through the note composition, NOTE<...> printer appended) yields the healthy line Resolved all 1 selected review thread(s) after lag-then-converge with no guard named; the never-converges skip note names live-head drift with the thread-accurate residual resolved 0 of 3 selected thread(s), 2 left for a later round (333's pre-fetch-resolved thread subtracted, 999's miss counted); the all-reads-fail note names live-head unreadable, distinct from a contributor push race. Static (logs/static-checks.txt, 21 assertions): the note is assigned in exactly 3 arms interpolating exactly the 8 host-set variables — and each of those is host-controlled (guard names all 7 from fixed literals incl. the mid-line || RESOLUTION_GUARD='live-head unreadable'; phase/detail strings fixed; counters only 0 / arithmetic / grep -c); echo "${RESOLUTION_NOTE}" appears exactly twice, site 1 (line 622) inside the pushed arm and site 2 (line 667) inside the no-op arm, each behind the non-empty guard; ROUND_PUSHED set exactly once per arm. Base script contains zero RESOLUTION_NOTE occurrences — the observability gap the PR exists to close (asserted, not just observed absent).
Secondary claim 2 — wall-clock bound and clamp injection (logs/wallclock.txt, 26 assertions)
Never-converges with the default knob and real sleep: 20,101 ms for the full 5-attempt window — the production worst-case price of a genuinely dead head (4 sleeps × 5 s + ~0.1 s overhead). Knob 1: 4,104 ms, proving the knob wires through. Through a recording sleep stub, GITHUB_ENV plant shapes 99999999999, abc, 05, 10, -1 each produced exactly 4 sleeps of 5 s (the fallback), while single digit 9 passes through unchanged — the clamp holds against every shape tried, and the window is exactly 4 sleeps (between 5 attempts), never more.
One sibling candidate was probed and disproved: ids with leading zeros pass the ^[0-9]+$ filter, but jq --argjson id 007 parses (to 7) on this container's jq rather than aborting, so a malformed agent-authored id cannot stall the step through that seam.
Mutation matrix — the new tests are not vacuous
harness/mutation-matrix.mjs applies each mutant as a uniqueness-checked exact-string edit (split/join, no regex artifacts), validates with bash -n, runs the single extended test (~7 s/run), restores the pristine script, and finally asserts byte-identical restoration (sha256 337fb19b…). Witness: 03-mutation-matrix.png; log logs/mutation-matrix.txt.
| mutant | what it removes | result | killed by |
|---|---|---|---|
| M0 control | nothing | green (suite live) | — |
| M1 | retry window (ATTEMPTS=1) |
killed | behavioral: lagThenConverge resolves [] not ['resolve:T_open_1'] |
| M2 | retry-delay clamp line | killed | text pin (byte-pins the whole line incl. regex and default) |
| M3 | window size 5→9 | killed | behavioral: head-read count 9 ≠ pinned 5 |
| M4 | empty-selection gate -gt 0→-ge 0 |
killed | behavioral: zero-valid-ids spends 1 read, pinned at 0 |
| M5 | same-thread dedupe | killed | behavioral: second id of one thread surfaces as resolved by another actor |
| M6 | already-resolved probe | killed | behavioral: partial note reports 2 not resolved where 1 not resolved is pinned; skip path reports 3 left where 2 left is pinned |
| M7 | phase string stopped early→halted early |
killed | behavioral — round's positive control |
| M8 | CR strip tr→sed 's/\r$//' |
killed | text pin; and GNU-equivalent: the full 45-check head harness passes unchanged on the sed variant — under GNU sed both spellings strip the fixture's trailing CRs identically, isolating the tr choice to BSD sed (equivalence is fixture-scoped: tr also strips embedded CRs, sed only trailing — no test input carries embedded CRs) |
| M9 | read tolerance || LIVE_PR_HEAD='' |
killed | behavioral: all-reads-fail cell aborts under set -e (status ≠ 0) |
| M10 | id dedupe/ordering (| sort -u) |
killed | behavioral: file-order iteration resolves a different thread first — resolved 1 of 3 ≠ pinned resolved 0 of 3 |
| M11 | zero-match grep -c errexit rescue (|| true) |
killed | behavioral: zero-valid-ids assignment aborts the block |
| M12 | drift/unreadable split (LIVE_HEAD_EVER_READ) |
killed | behavioral: all-reads-fail note reads live-head drift where unreadable is pinned |
12/12 mutants killed, 0 survivors; the positive control (M7) and the M0 green prove the runner can fail and the suite was live. One classification note (carried from the previous round, unchanged): M2 shows the clamp's fallback behavior is pinned inside the repo suite only by the byte-level text pin — every in-suite plant uses the valid 0. This round's wall-clock probe verified the fallback behavior externally (table above), so the gap is completeness reporting only, not an open risk.
Targeted gates
| gate | result |
|---|---|
vitest run scripts/tests/qwen-autofix-workflow.test.js (full file) at HEAD |
219/219 pass (101.5 s) |
same file at base worktree HEAD^1 |
219/219 pass (74.3 s) — identical test count (the PR extends one it rather than adding tests), no pre-existing failures to attribute |
bash -n on head and base scripts |
clean |
.github/scripts/check-workflow-size.sh |
exit 0 — every workflow under the 470000-byte gate |
prettier --check on the changed test file |
clean; liveness probe: planted formatting break caught (exit 1) |
eslint on the changed test file |
clean; liveness probe: planted unused variable reported (no-unused-vars, exit 1) |
shellcheck 0.10.0 (official static binary) on head script |
clean at default severity (exit 0); liveness probe: planted echo $1 reported as SC2086 |
The PR body's claimed pre-existing root-environment test failure again did not reproduce here (all 219 pass on both arms running as uid 1000).
Findings
None. No blocker and no suggestion rose to the level of a finding: the central mechanism is load-bearing (cell-2 flip, re-measured), fail-closed is preserved (every refusing cell — 3–7 — still resolves nothing at head, exactly as base; the window changes only read counts and the lag-then-converge outcome), the empty-selection gate additionally saves reads (cell 8), the note line is host-only content (21 static checks), the retry cost is bounded and measured (+20.1 s worst case, clamp injection-proofed), and every PR-introduced guard — now twelve mutants covering all of them — is pinned by a test that demonstrably fails without it. The one environment correction (shellcheck installable after all) concerns the previous report, not the PR.
Not covered
- BSD/macOS sed behavior of the CR strip: this container ships GNU sed; the M8 GNU-equivalence cell isolates the difference to BSD, where the macOS test lane is the authority. The equivalence is fixture-scoped (embedded CRs would differ; no test input carries them).
- Per-commit attribution: the checkout is depth 2 and shallow —
git rev-list HEAD^1..HEAD^2lists 1 commit while the metadata records 4; the intermediate commits are unreachable. The aggregate diff was verified; per-commit claims were not. - End-to-end reproduction against the real GitHub read model: the A/B reproduces the wire shape of review autofix: thread-resolution pass can go dark with no signal — 0/90 threads resolved on PR #9729 despite resolved-comments.txt #10106 (a stale
headRefOidread seconds after the round's own push) via the stub's sequence, not the server-side eventual consistency that produces it — a handling reproduction, not a trigger reproduction. - Repo-wide gates (other suites,
npm run lint) — CI covers them and the diff touches no package code. - The
shellcheckresult is from v0.10.0 at default severity, which is already the lowest threshold (style— everything reported), matching the PR's claim; no exclusions were applied.
Methodology
One container (node:22-bookworm, node v22, running as uid 1000 node, no GitHub token), CI merge-ref checkout (HEAD = merge commit 055507d8e6, HEAD^1 = base tip a82a11a0a4, HEAD^2 = PR head b5904c3f91; the snapshot's baseRefOid 9b3ccee5… predates the merge-ref and is not the diff base). The A/B and note harnesses drove each arm's extracted block through real bash child processes under set -euo pipefail with a stubbed gh enforcing the real CLI's argument contracts (shape violations exit 2); scratch git repos supplied LOCAL_PUSHED_HEAD; head reads and gh invocations were counted separately because failed reads exit before counting. The wall-clock probe used real sleep for the bound and a recording sleep stub for the clamp injections. Mutants were uniqueness-checked exact-string edits (split/join), bash -n-validated before each filtered vitest run, with the pristine script restored and sha256-verified afterwards. shellcheck v0.10.0 was downloaded as the official static binary after establishing that the previous round's "not installable" was a uid artifact (apt needs root; network egress works). All raw logs live in logs/; harnesses in harness/ are rerunnable. Evidence images were produced with scripts/verify-capture.mjs.
Flakiness gate log
rounds=5 files=1 skipped=0
file scripts/tests/qwen-autofix-workflow.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/qwen-autofix-workflow.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
scripts/tests/qwen-autofix-workflow.test.js: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 2 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 3 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 4 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 5 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action needed (PR #10117)This round found no actionable feedback, so no code changes were made and nothing was committed. The PR head remains at Triage of the single feedback item
Cross-check against the raw workflow dataTo make sure nothing was missed by curation, every comment newer than the last evaluation (2026-08-27T01:05:49Z) was re-read from the raw data:
ResultNo changes, no commit, and no verification commands were required because nothing was changed. Any findings the in-progress sandboxed verification produces will be handled in the round that receives its report. 中文说明Autofix 审查轮次 — 无需任何操作（PR #10117）本轮没有可处理的反馈，因此未做任何代码改动，也没有提交任何内容。PR 头部仍为 对唯一一条反馈的分类
与工作流原始数据的交叉核对为确保策展过程没有遗漏任何内容，已重新阅读上次评估（2026-08-27T01:05:49Z）之后的每一条评论的原始数据：
结果无改动、无提交，也无需运行任何验证命令（没有任何改动）。正在进行中的沙箱验证如果产生 finding，将在收到其报告的那一轮中处理。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 345 passed · 0 failed · 345 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：345 通过 · 0 失败 · 345 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10117 deep verification (round 3) —
|
| # | previous-round item (round 2) | status at the re-measured head |
|---|---|---|
| 1 | Central-claim A/B, cell-2 flip 0/1 → 1/1 resolved |
stands — reproduced with identical oracles on both arms (table below) |
| 2 | Fail-closed cells (never converges / window cap / no-op single read / all reads fail / moved before mutation / zero-valid-ids reads) | stands — all 8 cells re-run on both arms, same expectations |
| 3 | Note line: behavioral guard names + counts, static host-only composition | stands — 56 behavioral + 38 static assertions, all re-run |
| 4 | Mutation matrix 12/12 killed | stands — 12/12 killed again, M0 green, pristine sha-verified; behavioral failure messages captured this round for M1/M6/M9/M12 |
| 5 | Gates 219/219 both arms, bash -n, size, prettier, eslint, shellcheck | stands — identical counts and results re-measured, liveness probes re-planted |
| 6 | Wall-clock bound +20.1 s, clamp defeats all plant shapes | stands — re-measured 20,112 ms worst case; 6 plant shapes (+ empty string) this round |
| 7 | Findings: none | confirmed — this round also found none |
| 8 | (new in round 3) trial merge against the snapshot base | added — see "Round-3 addition" below |
Scope
A script/workflow PR: .github/scripts/autofix-push-and-report.sh (+166/−24), its test harness scripts/tests/qwen-autofix-workflow.test.js (+301/−3), and a 5-line design-doc addition. No production package code.
- Central claim: the initial live-head equality check retries (up to 5 reads, 5 s apart by default) after a pushed round, waiting out propagation of the round's own push, instead of declaring
live-head drifton one stale read — while fail-closed semantics are unchanged. - Secondary claim 1: every refusing guard records a stable name and both round-report arms carry one host-authored note line with thread-accurate counts; no agent-authored content enters it.
- Secondary claim 2: thread-accurate counting (two ids of one thread count once; pre-fetch-resolved and another-actor-resolved threads are subtracted),
live-head unreadablevsdriftsplit, portable CR strip viatr, empty selection spending zero head-proof reads, and a digit-clamped retry-delay knob.
Per-commit attribution was out of reach (see Not covered); the aggregate HEAD^1..HEAD diff is what was verified.
Central claim — A/B load-bearing proof
Harness: harness/ab-harness.mjs extracts the guard+resolve block verbatim from each arm's script (same anchors the repo's own test uses: CAN_RESOLVE_THREADS='false' → the 🧵 confirmed line, +2 lines) and drives it through real bash child processes under set -euo pipefail, with a stubbed gh on PATH that enforces the real CLI invocation shapes (--json headRefOid --jq '.headRefOid // ""', --paginate on the threads fetch, the @tsv filter on the per-thread guard — shape violations exit 2/3) and counts head reads and gh invocations separately. Fixture mirrors the repo test: 111 implemented (112 a reply in the same thread), 333 already resolved before the fetch, 999 matches nothing. Base arm ran against git worktree add tmp/base-tree HEAD^1; realpath confound n/a — each harness reads its arm's script file by path, and the per-arm load was asserted (base tree loads the 36,699-byte script with 0 RESOLUTION_NOTE, repo root the 44,594-byte one with 8).
Read counts include the post-mutation guard read (initial check + per-thread pre-guard + post-guard per resolved thread).
| # | cell (oracle) | base a82a11a0 |
head b5904c3f |
|---|---|---|---|
| 1 | immediate match: resolve issued, no skip | resolves 1/1, 3 reads | resolves 1/1, 3 reads |
| 2 | lag-then-converge (read 1 stale = own push not propagated, read 2 equal) | skip, 0/1 resolved, 1 read (the #9729 failure) | resolves 1/1, 4 reads |
| 3 | never converges (true drift): skip + reads | skip, 1 read | skip, 5 reads |
| 4 | convergence only on read 6 of a 5-read window | skip, 1 read | skip, reads capped at 5 |
| 5 | no-op round (ROUND_PUSHED=false), stale head |
skip, 1 read | skip, 1 read |
| 6 | every gh read fails (expired PAT shape): skip + attempts |
skip, 1 invocation (0 reads) | skip, 5 invocations (0 reads) |
| 7 | head equal at initial check, moved before first mutation | break, 0 resolved, 2 reads | break, 0 resolved, 2 reads (per-mutation guard still single-shot) |
| 8 | zero valid ids (rc:abc): exit 0 + reads |
exit 0, resolves nothing, 1 read spent | exit 0, resolves nothing, 0 reads |
The cell-2 flip — 0/1 resolved → 1/1 resolved on identical input — is the load-bearing proof; cells 3–7 show the fix narrows nothing else (every refusing cell still resolves nothing at head, exactly as base); cell 8 shows the empty-selection gate additionally stops spending PAT reads on nothing. All 41 assertions per arm pass. Witnesses: 01-ab-base-arm.png, 02-ab-head-arm.png; raw logs logs/ab-base.txt, logs/ab-head.txt.
Secondary claim 1 — guard names and the note line
Behavioral (logs/notes-head.txt, 56 assertions, all head arm): the extended block through the note composition yields — healthy round: Resolved all 1 selected review thread(s) with no guard named; never-converges skip: live-head drift with the thread-accurate residual resolved 0 of 3 selected thread(s), 2 left for a later round (333's pre-fetch-resolved thread subtracted, 999's miss counted); all-reads-fail: live-head unreadable, distinct from drift. The round-3 set additionally re-drove every mid-list stop guard: live-head drift break with partial count (resolved 1 of 2 … 1 left), thread state unproven, and mutation post-check ambiguous, plus same-thread dedupe on the break path (resolved 0 of 1), another-actor continue then drift (resolved 0 of 3 … 2 left), duplicate-id and already-resolved-only convergence (Resolved all 1), fetch-incomplete annotation, empty selection and zero-valid-id silence, and the no-op arm's identical note with one read.
Static (logs/static-checks.txt, 38 assertions, both scripts): the note is composed in exactly 3 arms interpolating exactly the 8 host-set variables — each host-controlled: all 8 named guard assignments are fixed literals from the 7-name set (plus the empty initialization), phase/detail strings are fixed literals, counters come only from 0 / arithmetic / grep -c, no command substitution inside the note; echo "${RESOLUTION_NOTE}" appears exactly twice behind non-empty guards, embedding 1 inside the pushed arm and embedding 2 inside the no-op arm (order asserted against the ROUND_PUSHED set-sites); ROUND_PUSHED set exactly once per arm. Base script contains zero RESOLUTION_NOTE / ROUND_PUSHED / RESOLUTION_GUARD occurrences — the observability gap the PR exists to close (asserted, not just observed absent). All 10 mechanism text-pins hold on head and are absent from base.
Secondary claim 2 — wall-clock bound and clamp injection
logs/wallclock.txt, 10 assertions. Never-converges pushed round with the default knob and real sleep: 20,112 ms for the full 5-attempt window — the production worst-case price of a genuinely dead head (4 sleeps × 5 s + ~0.1 s overhead). Knob 1: 4,114 ms, proving the knob wires through. Through a recording sleep stub, GITHUB_ENV plant shapes 99999999999, abc, 05, 10, -1, and empty string each produced exactly 4 sleeps of 5 s, while single digit 9 passes through unchanged — the clamp holds against every shape tried, and the window is exactly 4 sleeps (between 5 attempts), never more. Witness: 04-wallclock-and-clamp.png.
Mutation matrix — the new tests are not vacuous
harness/mutation-matrix.mjs applies each mutant as a uniqueness-checked exact-string edit (split/join), validates with bash -n, runs the single extended it (-t filter, ~5 s/run), restores the pristine script, and finally asserts byte-identical restoration (sha256 337fb19b…, same hash as round 2). Witness: 03-mutation-matrix.png; log logs/mutation-matrix.txt.
| mutant | what it removes | result | killed by (behavioral quote where captured) |
|---|---|---|---|
| M0 control | nothing | green (suite live) | — |
| M1 | retry window (LIVE_HEAD_ATTEMPTS=1) |
killed | expected [] to deeply equal [ 'resolve:T_open_1' ] (lagThenConverge) |
| M2 | retry-delay clamp line | killed | text pin of the whole line incl. regex and default |
| M3 | window size 5→9 | killed | head-read count 9 ≠ pinned 5 |
| M4 | empty-selection gate -gt 0→-ge 0 |
killed | zero-valid-ids spends 1 read, pinned at 0 |
| M5 | same-thread dedupe | killed | second id of one thread surfaces as resolved by another actor |
| M6 | already-resolved probe | killed | note residual 'resolved 0 of 3 selected thread(s), 2…' not contained (re-reports the pre-fetch-resolved thread) |
| M7 | phase string stopped early→halted early (positive control) |
killed | behavioral — proves the runner can fail on a content change |
| M8 | CR strip tr→sed 's/\r$//' |
killed | text pin; and GNU-equivalent: the full 41+56-check head harnesses pass unchanged on the sed variant (logs/m8-ab.txt, logs/m8-notes.txt) — under GNU sed both spellings strip the fixture's trailing CRs identically; the tr choice is pinned to BSD sed, where the macOS lane is the authority (equivalence is fixture-scoped: tr also strips embedded CRs, sed only trailing — no test input carries embedded CRs) |
| M9 | read tolerance || LIVE_PR_HEAD='' |
killed | expected 1 to be +0 — all-reads-fail aborts under set -e instead of skipping cleanly |
| M10 | id dedupe/ordering (| sort -u) |
killed | file-order iteration resolves a different thread first — pinned counts change |
| M11 | zero-match grep -c errexit rescue (|| true) |
killed | zero-valid-ids assignment aborts the block |
| M12 | drift/unreadable split (LIVE_HEAD_EVER_READ) |
killed | note reads live-head drift where guard: \live-head unreadable`` is pinned |
12/12 mutants killed, 0 survivors; the positive control (M7) and the M0 green prove the runner can fail and the suite was live. Kill failures quoted above were captured in a second instrumented pass (logs/kill-M1.txt … kill-M12.txt); every quoted failure names expected-versus-actual values on the intended assertion, after bash -n passed — none is a compile break. Classification note (carried from round 2, unchanged): the clamp's fallback behavior is pinned inside the repo suite only by the byte-level text pin (every in-suite plant uses the valid 0); the wall-clock probe verifies the fallback externally, so the gap is completeness reporting only.
Round-3 addition — trial merge against the snapshot base
The metadata snapshot records baseRefOid 9b3ccee547916c22f35695eccdecb76e59df642b ("feat(acp): enable managed auto-memory lifecycle (#9992)", committed 2026-08-26 09:30) as the current base branch tip, while the merge-ref base HEAD^1 is a82a11a0a4 (2026-08-26 16:24). Direct measurement: a82a11a0a4 is a shallow root, and git merge-base --is-ancestor returns NO in both directions between the two commits; rev-list a82a11a0a4..9b3ccee5 = 8932 — the merge-ref base commit is not in the snapshot base's history. Whatever moved main (rewind or rebuild lag), the question a reviewer needs answered is whether the PR still lands safely, so it was measured directly instead of inferred:
- the 3 files this PR touches are byte-identical between
9b3ccee5anda82a11a0a4(emptygit diffbetween bases for exactly those paths); - the PR's effective diff (
git diff HEAD^1..HEAD) applies to a9b3ccee5worktree cleanly (git apply --3way, 3× "Applied patch … cleanly",logs/trial-apply.txt); - the merged files are byte-identical to the verified head versions (3× sha256 match);
bash -nclean and the full affected suite passes on the merged tree: 219/219 (logs/gate-vitest-trial.txt).
Witness: 05-trial-merge-vs-current-main.png. This is not a finding against the PR — base movement is outside the author's control — but maintainers may want to know the merge ref will be rebuilt when GitHub next refreshes it, and that the rebuild is conflict-free by measurement.
Targeted gates
| gate | result |
|---|---|
vitest run scripts/tests/qwen-autofix-workflow.test.js (full file) at HEAD |
219/219 pass (75.6 s) |
same file at base worktree HEAD^1 |
219/219 pass (74.6 s) — identical test count (the PR extends one it rather than adding tests), no pre-existing failures to attribute |
same file on the trial-merged 9b3ccee5 tree |
219/219 pass (80.2 s) |
bash -n on head, base, and trial-merged scripts |
clean ×3 |
.github/scripts/check-workflow-size.sh |
exit 0 |
prettier --check on the changed test file |
clean; liveness probe: planted formatting break caught (exit 1) |
eslint on the changed test file |
clean; liveness probe: planted unused variable reported (@typescript-eslint/no-unused-vars, exit 1) |
shellcheck 0.10.0 (official static binary) on head script |
clean at default severity (exit 0); liveness probe: planted echo $1 reported as SC2086 |
| flakiness gate: 5 identical full-file rounds at head | 5× exit 0, 219/219 each round, no divergence (logs/flakiness.txt) |
The PR body's claimed pre-existing root-environment test failure (locks the runner file-command backing files against env plants) again did not reproduce here — all 219 pass on every arm running as uid 1000.
Findings
None. No blocker and no suggestion rose to the level of a finding: the central mechanism is load-bearing (cell-2 flip re-measured with identical oracles), fail-closed is preserved (every refusing cell — 3–7 — still resolves nothing at head, exactly as base; the window changes only read counts and the lag-then-converge outcome), the empty-selection gate additionally saves reads (cell 8), the note line is host-only content (38 static checks incl. all three mid-list stop guards driven behaviorally), the retry cost is bounded and measured (20.1 s worst case, clamp injection-proofed against 7 plant shapes), every PR-introduced guard is pinned by a test that demonstrably fails without it (12/12 mutants, behavioral failure messages quoted), and the PR merges cleanly onto the snapshot's current main with the suite green there.
Not covered
- BSD/macOS sed behavior of the CR strip: this container ships GNU sed; the M8 GNU-equivalence cell isolates the difference to BSD, where the macOS test lane is the authority. The equivalence is fixture-scoped (embedded CRs would differ; no test input carries them). The sibling CR-strip consumer (the reply gate) already used
tron base and is untouched — both consumers are consistent. - Per-commit attribution: the checkout is depth 2 and shallow —
git rev-list HEAD^1..HEAD^2lists 1 commit while the metadata records 4; the intermediate commits are unreachable (rev-parse --is-shallow-repository= true). The aggregate diff was verified; per-commit claims were not. - End-to-end reproduction against the real GitHub read model: the A/B reproduces the wire shape of review autofix: thread-resolution pass can go dark with no signal — 0/90 threads resolved on PR #9729 despite resolved-comments.txt #10106 (a stale
headRefOidread seconds after the round's own push) via the stub's sequence, not the server-side eventual consistency that produces it — a handling reproduction, not a trigger reproduction. - Repo-wide gates (other suites,
npm run lint) — CI covers them and the diff touches no package code. Thenode_modules/@qwen-code/*realpath check was performed (qwen-code-core→ head tree'spackages/core) but is moot: none of this PR's gates imports a workspace package. - shellcheck at v0.10.0 default severity only (already the lowest threshold —
stylereports everything); no exclusions applied.
Methodology
One container (node:22-bookworm, node v22.23.2, uid 1000, no GitHub token), CI merge-ref checkout (HEAD = merge commit 055507d8e6, HEAD^1 = base tip a82a11a0a4, HEAD^2 = PR head b5904c3f91; the snapshot's baseRefOid 9b3ccee5… is the current base branch tip and is not in the merge-ref base's history — see the trial-merge section). The A/B, note, and wall-clock harnesses drove each arm's extracted block through real bash child processes under set -euo pipefail with a stubbed gh enforcing the real CLI's argument contracts (shape violations exit 2/3); scratch git repos supplied LOCAL_PUSHED_HEAD; head reads and gh invocations were counted separately because failed reads exit before counting. Wall-clock probes used real sleep for the bound and a recording sleep stub (separate PATH dir, so W1/W2 genuinely sleep) for the clamp injections. Mutants were uniqueness-checked exact-string edits, bash -n-validated before each filtered vitest run, with the pristine script restored and sha256-verified afterwards (337fb19b…); four kills (M1/M6/M9/M12) were re-run under output capture to quote their expected-versus-actual failure messages. The trial merge applied git diff HEAD^1..HEAD with --3way to a 9b3ccee5 worktree and re-ran the suite there. shellcheck v0.10.0 is the official static binary (network egress works in this container; uid 1000 is why apt is unavailable). Evidence images were produced with scripts/verify-capture.mjs. All raw logs live in logs/; harnesses in harness/ are rerunnable.
Flakiness gate log
rounds=5 files=1 skipped=0
file scripts/tests/qwen-autofix-workflow.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/qwen-autofix-workflow.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
scripts/tests/qwen-autofix-workflow.test.js: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 2 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 3 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 4 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 5 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
已按当前 head b5904c3f918d7367b7db17179830c32cad8987f5 完整复核：代码侧未发现新的卡点，focused workflow tests 通过，且与最新 upstream/main 可干净合并。当前卡点仍是 PR 正文没有按仓库模板填写，What this PR does、Why it is needed、Reviewer Test Plan、Risk & Scope、Linked Issues 等必需章节仍缺失，现有 CHANGES_REQUESTED 因此仍然成立。请按模板重写正文后重新触发 triage；本轮不 approve。
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： AutoFix round — no code changesFeedback triage
Ready-to-paste PR body — template-compliant, derived from the actual diff and the round-3 verification report; paste it into the PR description unchanged (it already contains the required Chinese translation block): pr-body.mdWhat this PR doesAutoFix rounds that fix review findings resolve the corresponding review threads in the PAT-bearing publish step, gated by deterministic guards: the pushed head must be the exact commit the gate verified, and the live PR head must still equal it. Before this PR, every guard refusal was visible only as a run-log warning; the round report on the PR said nothing, so a dead resolution mechanism read exactly like a working one. This change makes every refusing guard record a stable name, and both round-report arms — pushed and no-push — now carry one host-authored line naming the guard and counting the selected, resolved, and left-behind review threads in thread units; healthy rounds report the positive count on the same line, so the line whose numbers go wrong when the mechanism dies is the line that proves it working. The line is composed exclusively of fixed host strings and counters — no agent-authored content enters it. The PR also fixes the root cause that made the gap costly: GitHub's PR read model is eventually consistent, so a head read seconds after the round's own push routinely still returns the previous head, and the pass used to declare head drift on that single stale read and skip silently. The initial head-equality check now waits out propagation in a bounded retry window before declaring drift, while the per-mutation guards stay single-shot, because once the head was observed equal a later mismatch means it actually moved. Additionally, the counts are now thread-accurate instead of id-accurate (two selected ids in one thread count once; a thread already resolved before the fetch or resolved by another actor is subtracted from the residual), a head that could not be read at all is reported as Why it's neededOn PR #9729, 0 of 90 review threads were resolved across ~16 rounds with nothing on the PR saying which guard refused (issue #10106): every pushed round read the just-pushed head before propagation completed, declared drift, and skipped — a failure that stayed invisible for days because refusals reached only the run log. Fail closed must not mean fail silent: the maintainer-facing signal has to live on the PR it fails on, and the stale-read race had to stop discarding every pushed round's resolutions. Reviewer Test PlanHow to verify
Evidence (Before & After)Before: a pushed round whose head read raced propagation skipped silently — the report carried no resolution line, and only the run log recorded Tested on
Environment (optional)Repo-level vitest against the workflow test harness; no Qwen Code runtime is involved. The sandboxed deep verification (A/B against base, mutation matrix, wall-clock bound) ran in a token-free Risk & Scope
Linked IssuesFixes #10106 中文说明本 PR 做了什么AutoFix 轮次在修复评审 finding 后，会在持 PAT 的发布步骤中关闭对应的评审线程，并由确定性守卫把关：推送的 head 必须是门禁验证过的确切提交，且 PR 的实时 head 仍与其一致。在本 PR 之前，守卫的每次拒绝都只出现在运行日志的 warning 中；PR 上的轮次报告对此只字不提，导致关闭机制失效时与正常工作的表现完全相同。本次改动让每个拒绝的守卫记录一个稳定名称，并在轮次报告的两个分支（推送与未推送）各加入一行由宿主生成的说明：点名守卫，并以线程为单位统计选中、已关闭、遗留的评审线程；健康的轮次在同一行报告正数计数——机制失效时数字出错的那一行，正是机制正常时证明其工作的那一行。该行仅由固定的宿主字符串与计数组成，不含任何模型生成的内容。本 PR 还修复了让这一缺口代价高昂的根因：GitHub 的 PR 读取模型是最终一致的，本轮自己 push 后几秒钟内读到的 head 常常仍是旧值，而原逻辑仅凭这一次陈旧读取就判定 head 漂移并静默跳过。现在，初始的 head 相等检查会在一个有界的等待窗口内重试后才判定漂移；每次变更前的守卫仍是单次读取——因为一旦观察到 head 相等，之后的不一致就意味着它确实移动了。此外，计数改为按线程而非按 id（同一线程中的两个选中 id 只计一次；抓取前已被关闭、或被其他参与者关闭的线程会从遗留数中扣除）；完全读不到 head 报告为 为什么需要在 PR #9729 上，约 16 轮中 90 条评审线程 0 条被关闭，PR 上却看不到任何守卫拒绝的信息（issue #10106）：每一轮推送后都在传播完成前读取了刚推送的 head，判定漂移并跳过——由于拒绝只进入运行日志，这一故障数天都不可见。失败关闭不等于失败静默：面向维护者的信号必须出现在故障发生的 PR 上；陈旧读取竞态也必须停止让每一轮推送的线程关闭凭空消失。 评审者测试计划如何验证
证据（前后对比）之前：推送轮次的 head 读取与传播竞态时会静默跳过——报告没有任何关闭行，只有运行日志记录 测试环境
环境（可选）仓库级 vitest 运行工作流测试夹具；不涉及 Qwen Code 运行时。沙箱深度验证（与基线 A/B、变异矩阵、墙钟上界）在无凭证的 风险与范围
关联 IssueFixes #10106 Verification
中文说明AutoFix 轮次 — 无代码改动反馈分类
可直接粘贴的 PR 正文——符合模板，依据实际 diff 与第 3 轮验证报告撰写；请原样粘贴为 PR 描述（其中已包含必需的中文翻译块；该正文的中文版完整包含在其自身的"中文说明"折叠块中，未省略）。 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 433 passed · 0 failed · 433 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：433 通过 · 0 失败 · 433 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10117 deep verification (round 4) —
|
| # | previous-round item (round 3) | status at the re-measured head |
|---|---|---|
| 1 | Central-claim A/B, cell-2 flip 0/1 → 1/1 resolved |
stands — reproduced with identical oracles on both arms against the NEW base (table below, 01-ab-cells-base-vs-head.png) |
| 2 | Fail-closed cells (never converges / window cap / no-op single read / all reads fail / moved before mutation / zero-valid-ids reads) | stands — all 8 cells re-run on both arms, same expectations, 88/88 assertions |
| 3 | Note line: behavioral guard names + counts, static host-only composition | stands — 71 behavioral + 33 static assertions re-run |
| 4 | Mutation matrix 12/12 killed | stands — 12/12 killed again against the MERGED test file (the base-side test additions did not mask any mutant), M0 green, pristine sha-verified |
| 5 | Gates both arms, bash -n, size, prettier, eslint, shellcheck | stands — 220/220 both arms (was 219/219: the new base added one test; the PR still extends one it, adds none), all lints clean with re-planted liveness probes |
| 6 | Wall-clock bound ≈20 s, clamp defeats all plant shapes | stands — re-measured 20,193 ms worst case; 6 plant shapes + digit-9 passthrough |
| 7 | Findings: none | confirmed — this round also found none |
| 8 | Trial merge against the snapshot base (round-3 addition) | superseded — this round's merge-ref base d526612e92 is NEWER than the snapshot's baseRefOid 9b3ccee5 (which is an ancestor of round 3's base in the local graph), so the snapshot ref is stale and the verified tree (HEAD) already IS the merge into the newest base; see Corrections |
Scope
A script/workflow PR: .github/scripts/autofix-push-and-report.sh (+190 net on the new base), its test harness scripts/tests/qwen-autofix-workflow.test.js (+304 net), and a 5-line design-doc addition. No production package code; package.json/lockfile untouched, so the base-side control reusing the root node_modules is clean (and the gate closure imports no workspace package — verified by grep).
- Central claim: the initial live-head equality check retries (up to 5 reads, 5 s apart by default) after a pushed round, waiting out propagation of the round's own push, instead of declaring
live-head drifton one stale read — while fail-closed semantics are unchanged. - Secondary claim 1: every refusing guard records a stable name and both round-report arms carry one host-authored note line with thread-accurate counts; no agent-authored content enters it.
- Secondary claim 2: thread-accurate counting (two ids of one thread count once; pre-fetch-resolved and another-actor-resolved threads are subtracted),
live-head unreadablevsdriftsplit, portable CR strip viatr, empty selection spending zero head-proof reads, and a digit-clamped retry-delay knob.
Per-commit attribution was out of reach (see Not covered); the aggregate HEAD^1..HEAD diff is what was verified.
Central claim — A/B load-bearing proof
Harness: harness/ab-harness.mjs extracts the guard+resolve block verbatim from each arm's script (the repo's own anchors: CAN_RESOLVE_THREADS='false' → the 🧵 confirmed line, slice(i, j+2)) and drives it through real bash child processes under set -euo pipefail, with a stubbed gh that enforces the real CLI invocation shapes (--json headRefOid --jq '.headRefOid // ""', --paginate on the threads fetch, the @tsv filter on the per-thread guard — shape violations exit 2/3) and counts successful head reads and total gh invocations separately (failing reads exit before counting). Fixture mirrors the repo test: 111 implemented (112 a reply in the same thread), 222 declined, 444 a second open thread, 333 already resolved before the fetch, 999 matches nothing. Each arm ran in a scratch git repo with an identical fixed commit as VERIFIED_HEAD; the base arm's script came from git worktree add tmp/base-tree HEAD^1, the head arm from the merged tree (sha-asserted byte-identical to HEAD^2's blob). Read counts include the post-mutation guard read (initial check + per-thread pre-guard + post-guard per resolved thread).
| # | cell (oracle) | base d526612e |
head b5904c3f |
|---|---|---|---|
| 1 | immediate match: resolve issued, no skip | resolves 1/1, 3 reads, 5 calls | resolves 1/1, 3 reads, 5 calls |
| 2 | lag-then-converge (read 1 stale = own push not propagated, read 2 equal) | skip, 0/1 resolved, 1 read, 2 calls (the #9729 failure) | resolves 1/1, 4 reads, 6 calls |
| 3 | never converges (true drift): skip + reads | skip, 1 read, 2 calls | skip, 5 reads, 6 calls |
| 4 | convergence only on read 6 of a 5-read window | skip, 1 read, 2 calls | skip, reads capped at 5, 6 calls |
| 5 | no-op round (ROUND_PUSHED=false), stale head |
skip, 1 read, 2 calls | skip, 1 read, 2 calls |
| 6 | every gh read fails (expired PAT shape) |
skip, 0 reads, 2 calls (1 failed head attempt) | skip, 0 reads, 6 calls (5 failed head attempts) |
| 7 | head equal at initial check, moved before first mutation | break, 0 resolved, 2 reads, 3 calls | break, 0 resolved, 2 reads, 3 calls (per-mutation guard still single-shot) |
| 8 | zero valid ids (rc:abc): exit 0 + reads |
exit 0, resolves nothing, 1 read spent | exit 0, resolves nothing, 0 reads |
The cell-2 flip — 0/1 resolved → 1/1 resolved on identical input — is the load-bearing proof, re-measured this round against the new base; cells 3–7 show the fix narrows nothing else (every refusing cell still resolves nothing at head, exactly as base); cell 8 shows the empty-selection gate additionally stops spending PAT reads on nothing. Arm-load checks: base script sha256 3d8bfedb… with 0 RESOLUTION_NOTE/ROUND_PUSHED occurrences; head script sha256 337fb19b… with 8/3. All 88 assertions pass. Witness: 01-ab-cells-base-vs-head.png; raw log logs/ab.txt.
Secondary claim 1 — guard names and the note line
Behavioral (logs/notes.txt, 71 assertions, head arm): the extended block through the note composition yields — healthy round: Resolved all 1 selected review thread(s) with no guard named; never-converges skip: live-head drift with the thread-accurate residual resolved 0 of 3 selected thread(s), 2 left for a later round (333's pre-fetch-resolved thread subtracted, 999's miss counted); all-reads-fail: live-head unreadable, distinct from drift. Additionally re-driven: all five up-front guards, the no-op arm's identical note on one read, every mid-list stop guard (live-head drift break with partial count resolved 1 of 2 … 1 left, thread state unproven, mutation post-check ambiguous), same-thread dedupe on both skip and break paths (resolved 0 of 1), another-actor continue then drift (resolved 0 of 3 … 2 left), duplicate-id and already-resolved-only convergence (Resolved all 1), fetch-incomplete annotation, and the empty-selection and zero-valid-id silence.
Static (logs/static.txt, 33 assertions, both scripts): the note is composed in exactly 3 arms interpolating exactly the 8 host-set variables — all 9 RESOLUTION_GUARD assignments are fixed single-quoted literals from the 7-name set (plus the empty init), no assignment interpolates anything, no command substitution and no unescaped backtick in any note arm, counters derive only from 0 / arithmetic / the grep -c rescue; echo "${RESOLUTION_NOTE}" appears exactly twice, each two lines below an if [[ -n "${RESOLUTION_NOTE}" ]] guard, embedding 1 inside the pushed arm and embedding 2 inside the no-op arm (order asserted against the ROUND_PUSHED set-sites at lines 583/647); the selection gate -gt 0 appears 3× (head-proof, classification, note composition). Base script contains zero occurrences of RESOLUTION_NOTE, RESOLUTION_GUARD, ROUND_PUSHED, LIVE_HEAD_ATTEMPTS, LIVE_HEAD_RETRY_DELAY and no retry loop — the observability gap the PR exists to close (asserted, not just observed absent). Witnesses: 02-notes-host-authored.png, 03-static-composition.png.
Secondary claim 2 — wall-clock bound and clamp injection
logs/wallclock.txt, 19 assertions. Never-converges pushed round with the knob genuinely unset and real sleep: 20,193 ms for the full 5-attempt window — the production worst-case price of a genuinely dead head (4 sleeps × 5 s + ~0.2 s overhead). Knob 1: 4,191 ms, proving the knob wires through. Through a recording sleep stub, GITHUB_ENV plant shapes 99999999999, abc, 05, 10, -1, and empty string each produced exactly 4 sleeps of 5 s, while single digit 9 passes through unchanged (4 sleeps of 9). (Harness note: the first run of this probe tripped over the harness's own fast-cell default LIVE_HEAD_RETRY_DELAY=0; W1 was then re-run with the variable genuinely absent — the 179 ms first reading was harness error, not PR behavior.) Witness: 04-wallclock-and-clamp.png.
Mutation matrix — the new tests are not vacuous (on the merged test file)
harness/mutation-matrix.mjs applies each mutant as a uniqueness-checked exact-string edit (split/join, each target verified to occur exactly once), validates with bash -n, runs the single extended it (-t filter) against the merged test file, restores the pristine script, and finally asserts byte-identical restoration (sha256 337fb19b…). This round matters for the matrix: the new base added tests to the same file, so the merged file — the thing that actually lands — is what must pin the mutants. Witness: 05-mutation-matrix.png; logs logs/mutation-matrix.txt, logs/mutant-M*.txt.
| mutant | what it removes | result | killed by (behavioral quote where captured) |
|---|---|---|---|
| M0 control | nothing | green (suite live) | — |
| M1 | retry window (LIVE_HEAD_ATTEMPTS=5→1) |
killed | expected [] to deeply equal [ 'resolve:T_open_1' ] (lagThenConverge) |
| M2 | retry-delay clamp line | killed | text pin of the whole line incl. regex and default |
| M3 | window size 5→9 | killed | head-read count 9 ≠ pinned 5 |
| M4 | empty-selection gate -gt 0→-ge 0 |
killed | expected '1' to be '0' — zero-valid-ids spends 1 read, pinned at 0 |
| M5 | same-thread dedupe | killed | second id of one thread surfaces as resolved by another actor |
| M6 | already-resolved probe | killed | note residual no longer 'resolved 0 of 3 selected thread(s), 2…' (re-reports the pre-fetch-resolved thread) |
| M7 | phase string stopped early→halted early (positive control) |
killed | behavioral — proves the runner can fail on a content change |
| M8 | CR strip tr→sed 's/\r$//' |
killed | text pin; and GNU-equivalent: all 87 A/B + 71 note assertions pass unchanged on the sed variant (logs/m8-ab.txt — the single non-pass is the harness's sha load check naming the variant, as designed — and logs/m8-notes.txt); under GNU sed both spellings strip the fixture's trailing CRs identically. The tr choice is pinned to BSD sed, where the macOS lane is the authority (equivalence is fixture-scoped: tr also strips embedded CRs, sed only trailing — no test input carries embedded CRs) |
| M9 | read tolerance || LIVE_PR_HEAD='' |
killed | expected 1 to be +0 — all-reads-fail aborts under set -e instead of skipping cleanly |
| M10 | id dedupe/ordering (| sort -u) |
killed | file-order iteration resolves a different thread first — pinned counts change |
| M11 | zero-match grep -c errexit rescue (|| true) |
killed | zero-valid-ids assignment aborts the block |
| M12 | drift/unreadable split (LIVE_HEAD_EVER_READ) |
killed | note reads live-head drift where guard: \live-head unreadable`` is pinned |
12/12 mutants killed, 0 survivors; M0 green and the positive control (M7) prove the runner can fail and the suite was live. Every quoted failure names expected-versus-actual values on the intended assertion, after bash -n passed — none is a compile break, and each failed run red exactly one test (the filtered it). Classification note (carried from round 2, still accurate): the clamp's fallback behavior is pinned inside the repo suite only by the byte-level text pin (every in-suite plant uses the valid 0); the wall-clock probe verifies the fallback externally, so the gap is completeness reporting only.
Corrections
- Round-3 report's base-line figures: round 3 quoted the base script as "36,699 bytes" and the head script as "44,594 bytes" while citing the same sha256 hashes this round re-measured (
3d8bfedb…/337fb19b…). Byte sizes are determined by content, so the byte counts were wrong as stated; the hashes were right. This round measures 37,587 B (base) and 45,699 B (head) with those hashes. No verdict in round 3 depended on the byte counts. - Round-3's "main may have been rewound" framing: round 3 found no ancestry relation between the snapshot's
baseRefOid9b3ccee5and its merge-ref basea82a11a0a4and floated "rewind or rebuild lag". On this round's object graph,9b3ccee5is an ancestor ofa82a11a0a4(checked both directions), i.e. the snapshot ref was simply stale, and this round's merge-ref based526612e92(2026-08-27) is newer still. The merge ref has been rebuilt forward, not rewound.
Findings
None. No blocker and no suggestion rose to the level of a finding: the central mechanism is load-bearing against the new base (cell-2 flip re-measured with identical oracles), fail-closed is preserved (every refusing cell — 3–7 — still resolves nothing at head, exactly as base; the window changes only read counts and the lag-then-converge outcome), the empty-selection gate saves reads (cell 8), the note line is host-only content (33 static checks; all guard names fixed literals), the retry cost is bounded and measured (20.2 s worst case, clamp injection-proofed against 7 plant shapes), every PR-introduced guard is pinned by a test that demonstrably fails without it (12/12 mutants killed on the merged file, behavioral failure messages quoted), and the merged tree — including the base-side test additions from another feature — passes 220/220 with no divergence across 5 flakiness rounds.
Not covered
- BSD/macOS sed behavior of the CR strip: this container ships GNU sed; the M8 GNU-equivalence cell (158 assertions) isolates the difference to BSD, where the macOS test lane is the authority. Equivalence is fixture-scoped (embedded CRs would differ; no test input carries them). The sibling CR-strip consumer (the reply gate) already used
tron base and is untouched — both consumers are consistent. - Per-commit attribution: the checkout is shallow (
git rev-parse --is-shallow-repository= true) —git rev-list HEAD^1..HEAD^2lists 1 commit while the metadata records 4; the intermediate commits are at a graft boundary. The aggregate diff was verified; per-commit claims were not. - End-to-end reproduction against the real GitHub read model: the A/B reproduces the wire shape of review autofix: thread-resolution pass can go dark with no signal — 0/90 threads resolved on PR #9729 despite resolved-comments.txt #10106 (a stale
headRefOidread seconds after the round's own push) via the stub's sequence, not the server-side eventual consistency that produces it — a handling reproduction, not a trigger reproduction. - Repo-wide gates (other suites,
npm run lint) — CI covers them and the diff touches no package code. Thenode_modules/@qwen-code/*realpath concern is moot: no gate input imports a workspace package (grep-verified). - shellcheck at v0.10.0 default severity only (already the lowest threshold —
stylereports everything); no exclusions applied. Base script also clean, measured. - The author-claimed root-environment failure (
locks the runner file-command backing files against env plants) again did not reproduce: all 220 tests pass on every arm, running as uid 1000 (node), not root.
Methodology
One container (node:22-bookworm, node v22.23.2, uid 1000, no GitHub token), CI merge-ref checkout (HEAD = merge commit c7325541ad, HEAD^1 = base tip d526612e92, HEAD^2 = PR head b5904c3f91; the snapshot's baseRefOid 9b3ccee5… is stale — older than and ancestral to earlier bases, see Corrections). The base arm ran in git worktree add tmp/base-tree HEAD^1 resolving vitest by walking up to the root node_modules (clean control: lockfile untouched, no workspace imports). The A/B, note, and wall-clock harnesses drove each arm's extracted block through real bash child processes under set -euo pipefail in scratch git repos (fixed commit as VERIFIED_HEAD), with a stubbed gh enforcing the real CLI's argument contracts (shape violations exit 2/3); head reads and total gh invocations were counted separately because failing reads exit before counting. Wall-clock probes used real sleep for the bound and a recording sleep stub for the clamp injections. Mutants were uniqueness-checked exact-string edits, bash -n-validated before each filtered vitest run, with the pristine script restored and sha256-verified afterwards (337fb19b…); kill messages were quoted from the captured vitest output of each mutant run. shellcheck v0.10.0 is the official static binary (network egress works in this container; uid 1000 is why apt is unavailable); each lint gate carried a planted-violation liveness probe, and the test file was sha-verified after restoration (29645ef9…). Evidence images were produced with scripts/verify-capture.mjs. All raw logs live in logs/; harnesses in harness/ are rerunnable.
Flakiness gate log
rounds=5 files=1 skipped=0
file scripts/tests/qwen-autofix-workflow.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/qwen-autofix-workflow.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
scripts/tests/qwen-autofix-workflow.test.js: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 2 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 3 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 4 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 5 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action needed (PR #10117)No code changes this round. The only new feedback in this window is a passing Feedback triage
No new inline comments, no new reviews with findings, no failed checks, and no Carried-over item (non-code, maintainer-owned)The last remaining blocker named in the earlier maintainer review Verification
中文说明Autofix 评审轮 — 无需改动（PR #10117）本轮无代码改动。本窗口内唯一的最新反馈是一份通过的验证报告和一次维护者批准——二者均未要求任何变更。 反馈分类
本窗口内无新的行内评论、无携带 finding 的新评审、无失败检查、无持续变红的检查。增量仍在计数窗口预算之内（源码净 71/400 行、测试净 165/400 行），因此无需增长审计。 遗留事项（非代码，由维护者处理）此前维护者评审（[rv:5037455841]）指出的最后一个阻塞点是 PR 正文未按仓库模板填写。上一轮已向维护者移交了一份可直接粘贴、符合模板的 PR 正文（第 3 轮总结中的 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 470 passed · 0 failed · 470 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：470 通过 · 0 失败 · 470 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10117 deep verification (round 5) —
|
| # | previous-round item (round 4) | status at the re-measured head |
|---|---|---|
| 1 | Central-claim A/B, cell-2 flip 0/1 → 1/1 resolved |
stands — reproduced with identical oracles on both arms against the NEW base (table below, 01-ab-cells-base-vs-head.png) |
| 2 | Fail-closed cells (never converges / window cap / no-op single read / all reads fail / moved before mutation / zero-valid-ids reads) | stands — all 8 cells re-run on both arms, 104/104 assertions |
| 3 | Note line: behavioral guard names + counts, static host-only composition | stands — 20 behavioral scenarios + static checks re-run, 74/74 |
| 4 | Mutation matrix 12/12 killed | stands — 12/12 killed again against the NEW merged test file (base-side additions masked no mutant); M10's kill point is the normalization-line text pin, which fires before its ordering scenario (see matrix note) |
| 5 | Gates both arms, bash -n, prettier, eslint, shellcheck, workflow-size | stands — 223/223 both arms (was 220/220: the new base added 3 tests; the PR still extends one it, adds none), all lints clean with re-planted liveness probes |
| 6 | Wall-clock bound ≈20 s, clamp defeats all plant shapes | stands — re-measured 20,111 ms worst case; 6 plant shapes + digit-9 passthrough |
| 7 | Findings: none | confirmed — this round also found none |
| 8 | Base movement / stale snapshot base | extended — the base moved a second time; the snapshot baseRefOid 9b3ccee5 remains older than the merge-ref base; on this round's grafted object graph ancestry is not computable in either direction (see Corrections) |
Scope
A script/workflow PR, byte-identical to rounds 3–4: .github/scripts/autofix-push-and-report.sh (+166/−24 = 190 changed lines, net +142), its test harness scripts/tests/qwen-autofix-workflow.test.js (+301/−3 = 304 changed lines, net +298), and a 5-line design-doc addition. No production package code; package.json/lockfile untouched by the PR (both moved on the BASE side: +6/+377), so the base-side control reusing the root node_modules is clean — and the gate closure imports no workspace package (grep-verified, base-facts.txt check 6).
- Central claim: the initial live-head equality check retries (up to 5 reads, 5 s apart by default) after a pushed round, waiting out propagation of the round's own push, instead of declaring
live-head drifton one stale read — while fail-closed semantics are unchanged. - Secondary claim 1: every refusing guard records a stable name and both round-report arms carry one host-authored note line with thread-accurate counts; no agent-authored content enters it.
- Secondary claim 2: thread-accurate counting,
live-head unreadablevsdriftsplit, portable CR strip viatr, empty selection spending zero head-proof reads, and a digit-clamped retry-delay knob.
Per-commit attribution was out of reach (see Not covered); the aggregate HEAD^1..HEAD diff is what was verified.
Central claim — A/B load-bearing proof
Harness: harness/ab-harness.mjs extracts the guard+resolve block verbatim from each arm's script (the repo's own anchors: CAN_RESOLVE_THREADS='false' → the 🧵 confirmed line, slice(i, j+2)) and drives it through real bash child processes under set -euo pipefail, with a stubbed gh that enforces the real CLI invocation shapes (--json headRefOid --jq '.headRefOid // ""', --paginate on the threads fetch, the @tsv filter on the per-thread guard — shape violations exit 2/3) and counts successful head reads and total gh invocations separately (failing reads exit before counting). Fixture mirrors the repo test: 111 implemented (112 a reply in the same thread), 222 declined, 444 a second open thread, 333 already resolved before the fetch, 999 matches nothing. Each cell ran in a scratch git repo whose fixed commit served as VERIFIED_HEAD; the base arm's script came from git worktree add tmp/base-tree HEAD^1, the head arm from the merged tree (sha-asserted byte-identical to HEAD^2's blob).
| # | cell (oracle) | base a268b7d1 |
head b5904c3f |
|---|---|---|---|
| 1 | immediate match: resolve issued, no skip | resolves 1/1, 3 reads, 5 calls | resolves 1/1, 3 reads, 5 calls |
| 2 | lag-then-converge (read 1 stale = own push not propagated, read 2 equal) | skip, 0/1 resolved, 1 read, 2 calls (the #9729 failure) | resolves 1/1, 4 reads, 6 calls |
| 3 | never converges (true drift): skip + reads | skip, 1 read, 2 calls | skip, 5 reads, 6 calls |
| 4 | convergence only on read 6 of a 5-read window | skip, 1 read, 2 calls | skip, reads capped at 5, 6 calls |
| 5 | no-op round (ROUND_PUSHED=false), stale head |
skip, 1 read, 2 calls | skip, 1 read, 2 calls |
| 6 | every gh read fails (expired PAT shape) |
skip, 0 reads, 2 calls (1 failed head attempt) | skip, 0 reads, 6 calls (5 failed head attempts) |
| 7 | head equal at initial check, moved before first mutation | break, 0 resolved, 2 reads, 3 calls | break, 0 resolved, 2 reads, 3 calls (per-mutation guard still single-shot) |
| 8 | zero valid ids (rc:abc): exit 0 + reads |
exit 0, resolves nothing, 1 read spent | exit 0, resolves nothing, 0 reads, 1 call |
The cell-2 flip — 0/1 resolved → 1/1 resolved on identical input — is the load-bearing proof, re-measured this round against the new base (the base script is byte-identical to round 4's base, so the flip is attributable to the PR alone); cells 3–7 show the fix narrows nothing else (every refusing cell still resolves nothing at head, exactly as base; the window changes only read counts and the lag-then-converge outcome); cell 8 shows the empty-selection gate additionally stops spending PAT reads on nothing. Arm-load checks: base script sha256 3d8bfedb… with 0 RESOLUTION_NOTE/ROUND_PUSHED occurrences; head script sha256 337fb19b… with 8/3. All 104 assertions pass. Witness: 01-ab-cells-base-vs-head.png; raw log logs/ab.txt.
Secondary claim 1 — guard names and the note line
harness/notes-harness.mjs, 74 assertions, head arm (logs/notes.txt, witness 03-notes-host-authored.png):
Behavioral (20 scenarios): healthy round yields Resolved all 1 selected review thread(s) with no guard named; never-converges pushed skip names live-head drift with the thread-accurate residual resolved 0 of 3 selected thread(s), 2 left for a later round (333's pre-fetch-resolved thread subtracted, 999's miss counted); all-reads-fail names live-head unreadable, distinct from drift. Additionally driven: the remaining up-front guards (salvage merge, missing verified_head, verified_head mismatch), the no-op arm's byte-identical note on one read, same-thread dedupe on both skip and break paths (resolved 0 of 1), already-resolved-only skip converging to zero left, mid-list stop with partial count (stopped early, resolved 1 of 2 … 1 left), thread state unproven, mutation post-check ambiguous, no-guard partial line with details in the run log, fetch-incomplete annotation, another-actor continue then drift (resolved 0 of 3 … 2 left), duplicate-id and already-resolved-only convergence (Resolved all 1), and the empty-selection and zero-valid-id silence (the latter with 0 head reads asserted).
Static: the note is composed in exactly 3 arms interpolating exactly the 8 host-set variables (RESOLUTION_PHASE, RESOLUTION_PHASE_ZH, RESOLUTION_GUARD, CONFIRMED_RESOLVED_N, RESOLUTION_SELECTED_N, RESOLUTION_LEFT_N, RESOLUTION_DETAIL, RESOLUTION_DETAIL_ZH); all 9 RESOLUTION_GUARD assignments are fixed single-quoted literals from the 7-name set (plus the empty init); no command substitution and no unescaped backtick in any note arm; echo "${RESOLUTION_NOTE}" appears exactly twice, each two lines below an if [[ -n "${RESOLUTION_NOTE}" ]] guard, embedding 1 inside the pushed arm and embedding 2 inside the no-op arm (order asserted against the ROUND_PUSHED set-sites); the selection gate -gt 0 appears 3× (head-proof, classification, note composition). Base script contains zero occurrences of RESOLUTION_NOTE, RESOLUTION_GUARD, ROUND_PUSHED, LIVE_HEAD_ATTEMPTS, LIVE_HEAD_RETRY_DELAY (asserted, not just observed absent).
Secondary claim 2 — wall-clock bound and clamp injection
harness/wallclock-harness.mjs, 28 assertions (logs/wallclock.txt, witness 04-wallclock-and-clamp.png). Never-converges pushed round with the knob genuinely unset and real sleep: 20,111 ms for the full 5-attempt window — the production worst-case price of a genuinely dead head (4 sleeps × 5 s + ~0.1 s overhead; round 4 measured 20,193 ms). Knob 1: 4,113 ms, proving the knob wires through. Through a recording sleep stub, GITHUB_ENV plant shapes 99999999999, abc, 05, 10, -1, and empty string each produced exactly 4 sleeps of 5 s, while single digit 9 passes through unchanged (4 sleeps of 9).
Mutation matrix — the new tests are not vacuous (on the merged test file)
harness/mutation-matrix.mjs applies each mutant as a uniqueness-checked exact-string edit (each target verified to occur exactly once), validates with bash -n, runs the single extended it (resolves only the review threads whose findings it implemented, vitest -t filter — matched exactly 1 test) against the merged test file, restores the pristine script, and finally asserts byte-identical restoration (sha256 337fb19b…). This round matters for the matrix: the new base added 3 tests to the same file, so the merged file — the thing that actually lands — is what must pin the mutants. Witness: 02-mutation-matrix.png; logs logs/mutation-matrix.txt, logs/mutant-M*.txt.
One harness defect was found and fixed during this round: two mutants (M8, M10) contain $' in their replacement text, which plain String.replace interprets as "portion after match" — the first matrix pass corrupted those mutants into syntax-error kills. Switching to function replacement restored valid mutants; only the corrected runs are reported.
| mutant | what it removes | result | killed by (behavioral quote) |
|---|---|---|---|
| M0 control | nothing | green (suite live, 1 test matched) | — |
| M1 | retry window (LIVE_HEAD_ATTEMPTS=5→1) |
killed | expected [] to deeply equal [ 'resolve:T_open_1' ] (lagThenConverge) |
| M2 | retry-delay clamp line | killed | text pin of the whole line incl. regex and default |
| M3 | window size 5→9 | killed | expected '9' to be '5' — head-read count pinned at 5 |
| M4 | empty-selection gate -gt 0→-ge 0 |
killed | expected '1' to be '0' — zero-valid-ids spends 1 read, pinned at 0 |
| M5 | same-thread dedupe | killed | expected '::warning::comment 112 was resolved b…' not to contain 'resolved by another actor' |
| M6 | already-resolved probe | killed | note residual no longer 'resolved 0 of 3 selected thread(s), 2…' (re-reports the pre-fetch-resolved thread) |
| M7 | phase string stopped early→halted early (positive control) |
killed | behavioral — proves the runner can fail on a content change |
| M8 | CR strip tr→sed 's/\r$//' |
killed | text pin of the RESOLVED_IDS line's tr spelling; and GNU-equivalent: all 104 A/B + 74 note assertions pass unchanged on the sed variant (logs/m8-ab.txt, logs/m8-notes.txt) — under GNU sed both spellings strip the fixture's trailing CRs identically. The tr choice is pinned to BSD sed, where the macOS lane is the authority (equivalence is fixture-scoped: tr also strips embedded CRs, sed only trailing — no test input carries embedded CRs) |
| M9 | read tolerance || LIVE_PR_HEAD='' |
killed | expected 1 to be +0 — all-reads-fail aborts under set -e instead of skipping cleanly |
| M10 | id dedupe/ordering (| sort -u) |
killed | text pin of the normalization line (which contains | sort -u) — fires before the ordering scenarios, so the dedupe/order behavior is pinned textually rather than by the file-order fixture; either way removal cannot ship green |
| M11 | zero-match grep -c errexit rescue (|| true) |
killed | expected 1 to be +0 — zero-valid-ids assignment aborts the block |
| M12 | drift/unreadable split (LIVE_HEAD_EVER_READ) |
killed | note reads live-head drift where guard: \live-head unreadable`` is pinned |
12/12 mutants killed, 0 survivors; M0 green and the positive control (M7) prove the runner can fail and the suite was live. Every quoted failure names expected-versus-actual values on the intended assertion, after bash -n passed — none is a syntax break. Classification note (carried from earlier rounds, still accurate): the clamp's fallback behavior is pinned inside the repo suite only by the byte-level text pin (every in-suite plant uses the valid 0); the wall-clock probe verifies the fallback externally, so the gap is completeness reporting only.
Corrections
- Round-4's diff-churn figures: round 4's scope section quoted the script as "+190 net" and the test file as "+304 net". The measured numstat is script +166/−24 and test file +301/−3 — i.e. 190 and 304 are the changed-line totals; the nets are +142 and +298. No verdict depended on these figures; the labels were imprecise.
- Ancestry claims are graph-dependent: round 4 reported
9b3ccee5(the snapshotbaseRefOid) as an ancestor of its merge-ref base, checked both directions. On this round's shallow/grafted object graph,git merge-basecomputes no common ancestor between the round-4 based526612e92and the current basea268b7d175, and--is-ancestorfails in both directions for the snapshot ref — the graft boundary truncates the history needed to decide. The commit dates (9b3ccee52026-08-26 <d526612e922026-08-27 <a268b7d1752026-08-28) still order the movement forward; the merge ref has been rebuilt forward, not rewound. This is a limitation of what the local graph can prove this round, not evidence contradicting round 4's observation on its own graph.
Findings
None. No blocker and no suggestion rose to the level of a finding: the central mechanism is load-bearing against the new base (cell-2 flip re-measured with identical oracles on a byte-identical base script), fail-closed is preserved (every refusing cell — 3–7 — still resolves nothing at head, exactly as base; the window changes only read counts and the lag-then-converge outcome), the empty-selection gate saves reads (cell 8: base spends 1 read, head spends 0), the note line is host-only content (static checks: exactly the 8 host-set variables interpolated, all guard names fixed literals, both arms guarded), the retry cost is bounded and measured (20,111 ms worst case, clamp injection-proofed against 6 plant shapes with digit-9 passthrough), every PR-introduced guard is pinned by a test that demonstrably fails without it (12/12 mutants killed on the merged file with behavioral failure messages quoted, M0 green, positive control live), and the merged tree — including the base-side additions of 3 tests — passes 223/223 with no divergence across 5 flakiness rounds on the head arm and once on the base arm.
Not covered
- BSD/macOS sed behavior of the CR strip: this container ships GNU sed; the M8 GNU-equivalence arm (178 assertions) isolates the difference to BSD, where the macOS test lane is the authority. Equivalence is fixture-scoped (embedded CRs would differ; no test input carries them).
- Per-commit attribution: the checkout is shallow (
git rev-parse --is-shallow-repository= true) —git rev-list HEAD^1..HEAD^2lists 1 commit while the metadata records 4; the intermediate commits sit at the graft boundary. The aggregate diff was verified; per-commit claims were not. - End-to-end reproduction against the real GitHub read model: the A/B reproduces the wire shape of review autofix: thread-resolution pass can go dark with no signal — 0/90 threads resolved on PR #9729 despite resolved-comments.txt #10106 (a stale
headRefOidread seconds after the round's own push) via the stub's sequence, not the server-side eventual consistency that produces it — a handling reproduction, not a trigger reproduction. - Repo-wide gates (other suites,
npm run lint) — CI covers them and the diff touches no package code. Thenode_modules/@qwen-code/*realpath concern is moot: the gate closure imports no workspace package (grep-verified); the base arm's vitest resolves into the rootnode_modules(asserted viaimport.meta.resolvefrom the base tree). - shellcheck at v0.10.0 default severity only (already the lowest threshold —
stylereports everything); no exclusions applied. Base script also clean, measured. - The author-claimed root-environment failure (
locks the runner file-command backing files against env plants) again did not reproduce: it passes here in a filtered run and in all 223-test rounds, running as uid 1000 (node), not root — consistent with the author's own note that it fails only under a root environment. - The base movement's own content (770 files between the two bases, including
qwen-autofix.yml+494 andqwen-triage.yml+45): verified only to the extent it feeds this PR's closure (the merged tree's gates and the test-file mutation matrix exercise the merged result). The movement is main's own shipped state, not this PR's change.
Methodology
One container (node:22-bookworm, node v22.23.2, uid 1000, no GitHub token), CI merge-ref checkout (HEAD = merge commit dbe601c5b8, HEAD^1 = base tip a268b7d175, HEAD^2 = PR head b5904c3f91; the snapshot's baseRefOid 9b3ccee5… is stale — older than both merge-ref bases in the local graph, see Corrections). The base arm ran in git worktree add tmp/base-tree HEAD^1; the base tree has no node_modules of its own and vitest resolves into the root tree's (asserted from inside the base tree with import.meta.resolve) — a clean control because the PR leaves the lockfile untouched and the gate closure imports no workspace package. The A/B, note, and wall-clock harnesses drove each arm's extracted block through real bash child processes under set -euo pipefail in scratch git repos (fixed commit as VERIFIED_HEAD), with a stubbed gh enforcing the real CLI's argument contracts (shape violations exit 2/3); head reads and total gh invocations were counted separately because failing reads exit before counting. Wall-clock probes used real sleep for the bound and a recording sleep stub for the clamp injections. Mutants were uniqueness-checked exact-string edits applied via function replacement (plain String.replace corrupts $' in replacement text — found and fixed this round), bash -n-validated before each filtered vitest run, with the pristine script restored and sha256-verified afterwards (337fb19b…); kill messages were quoted from the captured vitest output of each mutant run (NO_COLOR=1). shellcheck v0.10.0 is the official static binary (network egress works in this container); every lint gate carried a planted-violation liveness probe. Evidence images were produced with scripts/verify-capture.mjs from live re-runs of each harness. All raw logs live in logs/; harnesses in harness/ are rerunnable.
Flakiness gate log
rounds=5 files=1 skipped=0
file scripts/tests/qwen-autofix-workflow.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/qwen-autofix-workflow.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
scripts/tests/qwen-autofix-workflow.test.js: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 2 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 3 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 4 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
round 5 · scripts/tests/qwen-autofix-workflow.test.js: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round report — no action takenRound result: NO ACTION — no code changes, no commits. Feedback triaged this round
Decisions
No verification commands were run because no code was changed and no check failure was reported. The branch head remains at 中文说明Autofix 轮次报告 — 未采取任何操作本轮结果：无操作 —— 没有代码变更，没有提交。 本轮处理的反馈
决策
由于没有变更代码，也没有报告任何检查失败，本轮没有运行任何验证命令。分支 HEAD 仍然停留在 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Independent pass at b5904c3. Verified the script change end to end: each refusing guard records a stable name (salvage merge / missing verified_head / verified_head mismatch / live-head drift / live-head unreadable / thread state unproven / mutation post-check ambiguous) and feeds one host-authored report line built from fixed strings plus counters only, so nothing agent-controlled reaches the note; the propagation wait is bounded (5 reads x 5s on pushed rounds, single read when nothing was pushed — a mismatch there only moves further away), the delay knob rejects anything but a single digit before falling back to the default, and the per-mutation guards stay single-shot once equality was observed. The new classification pass fixes both count shapes: selected ids are re-counted as unique THREADS on every path (two ids of one thread count once, via the SEEN_THREAD_IDS dedupe that also keeps the second id out of the resolve loop), and threads resolved before the fetch are re-found so the residual count converges instead of being re-reported forever. The three open bot suggestions all check out as addressed in this head: R1-4 — the || sleep line is pinned verbatim in the test suite so deleting it goes red, with DELAY=0 behavioral replays; R2-1 — a same-thread second id can no longer be attributed to another actor; R3-2 — thread-accurate counts on the skip and break paths. Local verification on the full PR tree: scripts/tests/qwen-autofix-workflow.test.js 215 passed, 4 skipped. No blockers from my side.
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action needed this roundPR #10117 at head b5904c3 — the only new feedback since the last evaluation is an independent review pass ([rv:5056474763], COMMENTED) that verifies the current head end to end and reports no blockers:
There are no inline comments, no issue-level comments, no failed checks, and no still-red checks to triage, so there is nothing to implement, decline, defer, or escalate. The working tree is clean and remains at the reviewed commit; no commit is made this round. 中文说明本轮无需任何操作PR #10117 当前 head 为 b5904c3 —— 自上次评估以来唯一的新反馈是一次独立审查（[rv:5056474763]，COMMENTED），它对当前 head 做了端到端核验并确认没有阻塞项：
没有内联评论、没有 issue 级评论、没有失败的检查，也没有持续失败的检查需要处理，因此没有需要实现、拒绝、延期或上报的事项。工作树干净且停留在被审查的提交上；本轮不产生任何提交。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM at b5904c3. Verified end to end in my earlier pass: guard names feed one host-authored report line (fixed strings + counters only, nothing agent-controlled); the propagation wait is bounded and single-shot where it should be; the classification pass counts unique threads on every path and re-finds pre-resolved threads so the residual count converges. All three open bot suggestions are addressed in this head (the sleep line is pinned verbatim in the tests). Local run of the full test file: 215 passed, 4 skipped. Approving.
5 participants
Fixes #10106.
Problem
The autofix thread-resolution pass could go completely dark on a PR. Every guard in the fail-closed chain announced its refusal only as a run-log
::warning::, so nothing on the PR said which guard refused or that resolution was skipped at all. On PR #9729, 0 of 90 review threads were resolved across ~16 rounds while every round report read like resolution was working.Diagnosis (from #9729's actual run logs)
Every pushed round tripped the same guard — the initial live-head equality check:
confirmed on runs 32662986102 (the round the issue quotes; its replies landed —
🧵 replied on 14 thread(s)— while resolution skipped), 32687247218, 32828530031, and 32856907644. The root cause is not drift: the PR read model is eventually consistent, and aheadRefOidread seconds after the round's own push routinely still returns the previous head. One immediate read declared drift every time, so every pushed round silently skipped — the guard is correctly strict in intent but over-broad in implementation.Changes
Observability (issue's "resolution skipped: <guard>" ask):
salvage merge,missing verified_head,verified_head mismatch,live-head driftfor the up-front skips;live-head drift,thread state unproven,mutation post-check ambiguousfor a mid-list stop. The previously mergedmissing verified_head | verified_head mismatchbranch is split so the report can tell them apart.⚠️ Review-thread resolution skipped — guard:live-head drift; resolved 0 of 12 selected thread(s), 12 left for a later round.Healthy rounds report the positive count (🧵 Resolved all N…), so the mechanism's death is visible by the same line whose numbers go wrong. Per-thread misses without a stopping guard and an incomplete thread fetch are counted and called out too.Root-cause fix (per-guard verdict: over-broad):
live-head drift, waiting out propagation of the round's own push. Fail-closed semantics are unchanged: no thread is ever resolved unless the live head was observed equal to the deterministically verified commit.GITHUB_ENVplant cannot stall the PAT-bearing step toward the job timeout.The other guards were checked against #9729's shape and left as-is (correctly strict):
salvage mergepushes an unverified merge,missing verified_head/verified_head mismatchmean the pushed object is not the verified one, and gate-rejection rounds never reach the resolve pass by design — their failure reports already say the round did nothing.Tests
scripts/tests/qwen-autofix-workflow.test.js: the resolve-pass harness now also runs the note-composition block — every up-front guard name + counts, mid-list stop names (live-head drift,thread state unproven,mutation post-check ambiguous), healthy-round positive line, no-guard partial counts, fetch-incomplete annotation, no-selection silence, and the two-arm report embedding; plus lag-then-converge (stale first read, resolution proceeds), never-converges (skips only after exhausting all 5 reads), and the digit clamp pin.bash -n,shellcheck(clean at default severity), prettier, eslint, andcheck-workflow-size.shall pass. The one failing test in the file locally (locks the runner file-command backing files against env plants) fails identically on the unmodified base under a root environment and is unrelated.docs/design/autofix-resolve-fixed-review-threads.mdrecords both decisions (round-report observability; bounded wait for the initial live-head equality).中文说明
修复 #10106。
问题
autofix 的线程 resolve 环节可能在一个 PR 上完全哑火:fail-closed 守卫链的每个拒绝都只以 run 日志里的
::warning::形式出现,PR 上没有任何信息说明是哪个守卫拒绝、甚至没有说明 resolve 被跳过。PR #9729 上约 16 轮、90 条 review 线程 0 条被 resolve,而每轮报告读起来都像 resolve 在正常工作。诊断(基于 #9729 的真实 run 日志)
每个 pushed 轮踩中的都是同一个守卫——初始 live-head 等值检查:
在 run 32662986102(issue 引用的那一轮;该轮 replies 正常落地
🧵 replied on 14 thread(s),唯独 resolve 被跳过)、32687247218、32828530031、32856907644 上均已确认。根因不是 drift:PR 读模型是最终一致的,在本轮自己的 push 之后数秒读headRefOid,常规性地仍返回旧 head。单次立即读把传播延迟判成了 drift,于是每个 pushed 轮都静默跳过——该守卫意图上"正确地严格",实现上"过宽"。改动
可观测性(issue 提出的
resolution skipped: <守卫>):salvage merge、missing verified_head、verified_head mismatch、live-head drift;中途中止为live-head drift、thread state unproven、mutation post-check ambiguous。原先合并在一起的missing verified_head | verified_head mismatch分支已拆开,报告可以区分二者。🧵 Resolved all N…),机制失效时同一行的数字会直接暴露。无中止守卫的逐条未关闭、线程拉取不完整也会计数并点明。根因修复(逐守卫裁定:过宽):
live-head drift。fail-closed 语义不变:只有观察到 live head 等于确定性验证过的 commit 才会 resolve 任何线程。GITHUB_ENV植入把这个持 PAT 的 step 拖到 job 超时。其余守卫按 #9729 的形态逐一核对后保持原样(正确地严格):
salvage merge推送的是未验证的 merge,missing verified_head/verified_head mismatch表示推送对象不是验证过的那个,gate-rejection 轮按设计根本不进入 resolve 环节——其失败报告已说明该轮未做任何事。测试
scripts/tests/qwen-autofix-workflow.test.js:resolve 环节 harness 现在同时跑注记合成块——所有前置守卫名+计数、中途中止守卫名(live-head drift、thread state unproven、mutation post-check ambiguous)、健康轮正数行、无守卫的部分计数、拉取不完整标注、无选中时静默、两臂报告嵌入;另有滞后后收敛(首读为旧值仍继续 resolve)、始终不收敛(耗尽全部 5 次读后才跳过)、单数字钳制锚定。bash -n、shellcheck(默认严格度下干净)、prettier、eslint、check-workflow-size.sh全部通过。本地该文件唯一失败的用例(locks the runner file-command backing files against env plants)在未改动基线上、root 环境下同样失败,与本次改动无关。docs/design/autofix-resolve-fixed-review-threads.md已记录两项决策(轮次报告可观测性;初始 live-head 等值的有界等待)。