fix(ci): make autofix finding replies idempotent - #9463
Conversation
A crash-and-rerun of an address round, a same-run repair that regenerates the dispositions, or a later round re-declining the same finding all reproduce the same comment-replies.json entry — and the reply step posted it again, landing identical bot replies on one thread (observed 2026-08-16: one identical reply posted three times, QwenLM#9296). The thread fetch now also reads each comment's author and body, and the reply step skips posting when the thread already carries a comment by the autofix bot whose body equals the neutralised body about to be posted. A changed body — new information from a later round — still posts; a threads view without author/body, or a stale/empty one, degrades to the old post-always behavior. The replies API itself is already the no-review-event path, so this PR only adds the missing idempotence (the P1 replies item of QwenLM#9296). Refs QwenLM#9296
|
Thanks for the PR! Template looks good ✓ Problem: this is an observed bug, not theoretical hardening — the description points to a concrete incident (2026-08-16, one identical finding reply posted three times on a merged PR within seconds, tracked in #9296), with a clear mechanism: a crash-and-rerun, a same-run repair regenerating dispositions, or a later round re-declining an unchanged finding each re-enter the same reply into Direction: aligned. The reply path already uses Size: not applicable — no core-package paths are touched. This is a CI-workflow change ( Approach: minimal and focused. The diff (1) extends the review-threads GraphQL fetch with Risk: no elevated risk signals — neither changed file matches the high-risk revert-correlated paths. One process note for later: the branch currently conflicts with Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:这是已观测到的 bug,而非理论性加固——描述指向一起具体事件(2026-08-16,同一条 finding 回复在几秒内被重发三次,见 #9296),机制也清楚:crash 重跑、same-run repair 重新生成 disposition、或后续轮次对未变化的 finding 重新 decline,都会把同一条回复重新写入 方向:对齐。回复路径本来就用 规模:不适用——未触及核心包路径。这是 CI 工作流改动( 方案:最小且聚焦。diff(1)在 review-threads 的 GraphQL 抓取里补上 风险:无升级风险信号——两个改动文件都不命中与 revert 相关的高风险路径。 一个后续要注意的流程问题:当前分支与 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI read the title + "Why it's needed" first and formed my own plan: to stop a re-run re-posting an identical bot reply, the cleanest fix is to read the thread's existing comments before posting and skip when the bot has already left the exact same body — which means the threads fetch has to carry author + body. That's precisely what this PR does, so the approach matches my independent baseline; I didn't find a simpler path it missed. Reading the thread (source of truth) is more robust than persisting a "what I posted" state file across rounds, which would be the fragile alternative. The implementation checks out:
No critical blockers, no security issues, no AGENTS.md violations. The test change adds exactly the right scenarios (identical bot reply → skipped, changed body → posted, human echo → posted, legacy author/body-less nodes → tolerated). Non-blocking observation: the dedup only sees the fetched view (first 100 threads / first 100 comments each), so an identical reply beyond that page would re-post. That's the documented, acceptable degradation, not a defect. Test evidence — the PR's own CI
Every check above is a On the sandboxed lanes: the central claim here is CI-workflow shell behavior, not a product build or a TUI surface, so 中文说明代码审查我先只读标题和"为什么需要",形成自己的方案:要阻止重跑时重发相同的 bot 回复,最干净的修法是在发布前读取 thread 已有评论,当 bot 已留下逐字相同的正文时跳过——这要求线程抓取带上 author + body。这正是本 PR 的做法,与我的独立基线一致,没有找到更简的遗漏路径。读 thread(事实来源)比跨轮次持久化"我发过什么"状态文件更稳健,后者是脆弱的替代方案。 实现经核对无误:
无关键阻塞、无安全问题、无 AGENTS.md 违规。测试改动补齐了恰当的场景(相同 bot 回复→跳过;正文变化→发布;人类复读→发布;缺 author/body 的旧节点→容忍)。 非阻塞观察:去重只看抓取到的视图(前 100 个 thread / 每个前 100 条评论),超出该页的相同回复仍会重发。这是已记录、可接受的退化,不是缺陷。 测试证据 —— PR 自己的 CI上表全部是 关于沙箱通道:本 PR 的核心主张是 CI 工作流 shell 行为,不是产品构建或 TUI 界面,所以 — Qwen Code · qwen3.8-max Reviewed at |
|
⏸️ Deferring to @wenshao — the review is clean, but two things I can't resolve from the diff are blocking approval, and both need an author/maintainer hand:
Nothing here is a code-quality objection — the change is minimal, correct, and test-pinned. Rebasing onto 中文说明⏸️ 转交 @wenshao —— 审查是干净的,但有两点我无法从 diff 解决、且需要作者/维护者出手,阻止了批准:
这里没有任何代码质量上的异议——改动最小、正确、且有测试 pin。rebase 到 |
|
Confidence: 3/5 — the code review itself is clean and I'd merge the change on merit, but a merge conflict with Stepping back: the problem is real and observed (the 2026-08-16 triple-post in #9296), not a hypothetical. The approach matches what I'd have proposed independently — read the thread's existing comments and skip an identical bot reply — and I didn't find a simpler path it missed. The diff is the minimal set: two workflow hunks plus the matching pinned-test scenarios, no drive-by edits, and the degradation (post-always on a stale/empty view) is deliberate and documented. In six months this reads as a small, well-commented, test-pinned gate — I'd thank the author, not curse them. I'm not approving out of momentum; I'm holding for two concrete, checkable reasons below. Why not approve yet:
Neither is a code-quality concern, and neither is something I can resolve from the diff — hence defer rather than approve or request-changes. No 中文说明信心:3/5 —— 代码审查本身是干净的,就改动本身而言我愿意合入,但当前与 退一步看:问题是真实且已观测到的(#9296 里 2026-08-16 的三连发),不是假设。方案与我独立提出的一致——读取 thread 已有评论、跳过相同的 bot 回复——我也没找到它遗漏的更简路径。diff 是最小集合:工作流两处加对应的 pin 测试场景,无顺手改动;退化行为(视图过期/为空时照发)是有意且已记录。六个月后回看,这是一个小而注释充分、有测试 pin 的门——我会感谢作者而非抱怨。我不是因为没理由拒绝才批准;而是因为下面两个具体、可核查的原因而暂缓。 为何暂不批准:
两者都不是代码质量问题,也不是我能从 diff 里解决的——所以是暂缓(defer),而非批准或要求修改。未放置 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| repository(owner:$owner,name:$name){ | ||
| pullRequest(number:$pr){ | ||
| reviewThreads(first:100){nodes{id isResolved comments(first:100){nodes{databaseId}}} pageInfo{hasNextPage}} | ||
| reviewThreads(first:100){nodes{id isResolved comments(first:100){nodes{databaseId author{login} body}}} pageInfo{hasNextPage}} |
There was a problem hiding this comment.
[Suggestion] The new author{login} body fields that supply the idempotence gate are pinned by no test. Measured with a revert probe: reverting only this query hunk (gate and new tests kept) leaves all 180 tests green — the reply test injects THREADS_JSON directly via env, and the mock gh shim matches the query only on its reviewThreads(first:100) prefix, so the field list is never exercised. If a later edit to this 7.6k-line workflow drops author{login} or body here — a simplification, or a copy from one of the file's other GraphQL queries — it passes every test, and in production every thread node then has null author/body, the gate never matches, and the duplicate-reply regression this PR fixes (#9296) returns with no test red. Fix: pin the supplier — extend the mock gh shim to require the threads query contains both author{login} and body before answering, e.g. extend the [[ "$query" == *"reviewThreads(first:100)"* ]] branch to also require *"author{login}"* and *"body"*, exiting 2 otherwise (the shim already uses that convention for other query checks).
中文说明
为幂等门提供数据的新字段 author{login} body 没有任何测试保护。回退探针实测:只回退这一行查询(保留门和新测试),全部 180 个测试仍然绿——回复测试通过环境变量直接注入 THREADS_JSON,mock gh 脚本只按 reviewThreads(first:100) 前缀匹配查询,字段列表从未被真正验证。如果之后有人在这个 7.6k 行的工作流里删掉这里的 author{login} 或 body(简化,或从文件里其他 GraphQL 查询复制而来),所有测试照样通过;而生产环境里每个 thread 节点的 author/body 都会是 null,门永远匹配不上,本 PR 要修的重复回复回归(#9296)会在没有任何测试变红的情况下复活。修复:把供应端钉住——让 mock gh 脚本在应答前校验线程查询同时包含 author{login} 和 body,例如在 [[ "$query" == *"reviewThreads(first:100)"* ]] 分支里追加要求 *"author{login}"* 与 *"body"*,否则 exit 2(该脚本对其他查询检查已用同样约定)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| repository(owner:$owner,name:$name){ | ||
| pullRequest(number:$pr){ | ||
| reviewThreads(first:100){nodes{id isResolved comments(first:100){nodes{databaseId}}} pageInfo{hasNextPage}} | ||
| reviewThreads(first:100){nodes{id isResolved comments(first:100){nodes{databaseId author{login} body}}} pageInfo{hasNextPage}} |
There was a problem hiding this comment.
[Suggestion] The gate can only ever see the first 100 comments of a thread: comments(first:100) fetches no pageInfo, so per-thread comment truncation is both invisible and silent. The fetch above warns on the 100-thread cap (hasNextPage), but nothing warns when a thread's comment list is cut — the payload cannot even detect it today. On long-lived autofix PRs the same thread grows with every round (the #9296 shape — a repeatedly re-posted declination — is exactly such a thread); once a stored bot reply sits past comment 100, a crash-and-rerun or later round regenerating the unchanged declination finds no match and re-posts the duplicate — the exact symptom this change fixes — with no log signal. Suggested fix: fetch comments(first:100){nodes{...} pageInfo{hasNextPage}} and emit the same ::warning:: when any thread's comments are truncated, so the degraded post-always behavior is observable — matching the precedent the thread-cap warning already sets here:
reviewThreads(first:100){nodes{id isResolved comments(first:100){nodes{databaseId author{login} body} pageInfo{hasNextPage}}} pageInfo{hasNextPage}}
中文说明
门最多只能看到一个 thread 的前 100 条评论:comments(first:100) 没有取 pageInfo,因此单 thread 评论截断既不可见、也无日志。上方的抓取只对 100 个线程的上限(hasNextPage)告警,线程内评论列表被截断时没有任何告警——目前的 payload 甚至无法检测到这一点。在长期运行的 autofix PR 上,同一个 thread 会随每轮增长(#9296 的形态——同一条 decline 被反复重发——正是这种 thread);一旦已存的 bot 回复落在第 100 条之后,crash 重跑或后续轮次重新生成相同措辞时找不到匹配,就会再次重发重复回复——正是本改动要修的症状——且没有任何日志信号。建议修复:抓取 comments(first:100){nodes{...} pageInfo{hasNextPage}},并在任一线程评论被截断时发出同样的 ::warning::,让退化为“照发”的行为可观测——与这里线程上限告警已有的先例保持一致:
reviewThreads(first:100){nodes{id isResolved comments(first:100){nodes{databaseId author{login} body} pageInfo{hasNextPage}}} pageInfo{hasNextPage}}
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const deduped = readFileSync(repliedLog, 'utf8').trim().split('\n'); | ||
| expect(deduped).toHaveLength(1); | ||
| expect(deduped[0]).toContain('pulls/7731/comments/100/replies'); | ||
| expect(deduped[0]).toContain('body=Changed reason'); |
There was a problem hiding this comment.
[Suggestion] None of the new fixture bodies contains <!--, so a one-line mutant — the gate comparing the raw decoded body instead of the neutralised REPLY_BODY — survives the suite (measured: mutant applied, 1 passed | 179 skipped). In production, reply bodies containing <!-- are neutralised to <!\-\- before posting (the block's own comment anticipates model output smuggling control markers); a later round regenerating the same raw body would then compare raw against the stored neutralised form, miss the match, and repost the duplicate. The code as written compares the neutralised body correctly — only the pin is missing. Fix: add one scenario where the pending reply body contains <!-- and the existing bot comment's body holds the neutralised <!\-\- form, and assert the skip.
中文说明
新增 fixture 的正文都不含 <!--,因此一个单行变异体——门改为比较原始解码正文而非 neutralised 后的 REPLY_BODY——能在整个测试套件下存活(实测:应用变异体后 1 passed | 179 skipped)。生产环境中,含 <!-- 的回复正文在发布前会被中和为 <!\-\-(该块自己的注释已预料到模型输出可能夹带控制标记);后续轮次重新生成相同原始正文时,会拿原始形式去比对已存的中和形式,匹配失败,重复回复再次发出。当前代码的比较是正确的——缺的只是把这个契约钉住的测试。修复:新增一个场景——待发回复正文含 <!--,thread 上已有的 bot 评论正文为中和后的 <!\-\- 形式,断言跳过发布。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| }, | ||
| ]); | ||
| const humanEcho = readFileSync(repliedLog, 'utf8').trim().split('\n'); | ||
| expect(humanEcho).toHaveLength(1); |
There was a problem hiding this comment.
[Suggestion] Every fixture uses exactly one thread, so a mutant scanning comments across ALL threads (instead of scoping to the thread containing the target id) also survives the suite (measured: cross-thread-scan mutant, 1 passed | 179 skipped). In production that mutant is harmful: the bot posts decline reason X on thread A in one round (short templated reasons can repeat across findings), and a later round's pending reply with the same body X aimed at thread B gets suppressed because thread A carries it — thread B's reviewer sees exactly the silence this block exists to prevent. The code as written scopes correctly — only the pin is missing. Fix: add a two-thread fixture where thread A carries a bot comment with body X and a pending reply with the same body X targets thread B; assert the reply on B still posts.
中文说明
所有 fixture 都只有一个线程,因此一个跨所有线程扫描评论(而不是限定在包含目标 id 的线程内)的变异体同样能在套件下存活(实测:跨线程扫描变异体,1 passed | 179 skipped)。在生产中该变异体有害:bot 在某轮于线程 A 发布了 decline 理由 X(简短的模板化理由可能在不同 finding 间重复),后续轮次一条正文同为 X 的待发回复指向线程 B 时,会因为线程 A 已有该内容而被跳过——线程 B 的 reviewer 看到的正是本块要防止的“沉默”。当前代码的作用域是正确的——缺的只是钉住它的测试。修复:新增双线程 fixture——线程 A 上已有 bot 评论正文 X,正文同为 X 的待发回复指向线程 B,断言 B 上的回复照常发布。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| databaseId: 300, | ||
| author: { login: 'qwen-code-dev-bot' }, |
There was a problem hiding this comment.
[Suggestion] In both new fixtures the compared comment (bot or human) is the LAST node of its thread, so a mutant narrowing the gate's scan to the newest comment — (.[0].comments.nodes // [])[-1:] — passes the whole suite (measured: 180/180 green with the mutant). The common real thread shape is root comment, bot declination X, then a human reply answering it; on that shape a crash-and-rerun regenerating the identical disposition X meets a newest-comment-only gate that sees only the human's comment, finds no match, and re-posts X — the #9296 duplicate returns on the most common thread shape while every test stays green (probe-verified: original gate MATCH/skip, mutant NO MATCH/repost). Fix: add one fixture where a human (or authorless) comment FOLLOWS the bot's matching comment in the same thread, and assert the reply is still skipped.
中文说明
两个新 fixture 里被比对的评论(bot 或人类)都是各自线程的最后一个节点,因此把门的扫描收窄到最新一条评论的变异体——(.[0].comments.nodes // [])[-1:]——能通过整个套件(实测:变异体下 180/180 全绿)。真实线程的常见形态是:根评论、bot 的 decline X、随后人类回复作答;在这种形态下,crash 重跑重新生成相同 disposition X 时,只看最新评论的门只会看到人类那条,匹配失败,X 被再次发布——#9296 的重复回复在最常见线程形态上复活,而所有测试保持绿(探针验证:原门 MATCH/跳过,变异体 NO MATCH/重发)。修复:新增一个 fixture,让一条人类(或无作者)评论跟在该线程中 bot 的匹配评论之后,断言回复仍被跳过。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| author: { login: 'qwen-code-dev-bot' }, | ||
| body: 'Deferred — follow-up.\n\n中文:已延后。', |
There was a problem hiding this comment.
[Suggestion] No reply-gate fixture sets isResolved on a thread, so a mutant adding select(.isResolved | not) to the gate's selector — a plausible harmonisation with the resolve block's selector ~15 lines above in the workflow, which carries exactly that filter — survives the entire suite (measured: 180/180 green with the mutant; probe on a resolved thread flips MATCH→NO MATCH). The harm path is real: after a re-arm marker resets the watermark the next round re-reads all feedback, and the REST pulls/{pr}/comments enumeration carries no resolved state — so the agent can legitimately re-decline an old finding whose thread was resolved after the bot's first reply, and an isResolved-filtered gate would repost the duplicate onto the resolved thread. The current code is correct (the gate ignores isResolved); only the pin is missing. Fix: add isResolved: true to the dedupe scenario's thread object (or add a scenario) and keep the skip assertions.
中文说明
没有任何回复门的 fixture 给线程设置 isResolved,因此在门的选择器上加 select(.isResolved | not) 的变异体——与工作流上方约 15 行处 resolve 块选择器(恰好带这个过滤条件)的“合理对齐”——能在整个套件下存活(实测:变异体下 180/180 全绿;在已解决线程上的探针显示 MATCH→NO MATCH 翻转)。危害路径真实存在:re-arm 标记重置水位线后,下一轮会重新读取全部反馈,而 REST pulls/{pr}/comments 枚举不携带线程解决状态——因此 agent 完全可能对一条“bot 首次回复后被 reviewer 解决”的旧 finding 重新 decline,而带 isResolved 过滤的门会把重复回复重发到已解决的线程上。当前代码是正确的(门忽略 isResolved);缺的只是钉住这一点的测试。修复:在去重场景的线程对象上加 isResolved: true(或新增场景),并保留跳过断言。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
@qwen-code /resolve |
|
Qwen Code attempted to resolve merge conflicts but the run did not complete successfully. Check the workflow run for full logs. |
…mpotence # Conflicts: # .github/workflows/qwen-autofix.yml
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-autofix.yml:6469 — [probe] in-pass double-post: the gate consults only the pre-loop THREADS_JSON snapshot, so two same-thread identical entries in one comment-replies.json both post (unchanged since round 1 — deferred…scripts/tests/qwen-autofix-workflow.test.js:16575 — [probe] no substring-relation fixture: a containment relaxation of the gate's exact-equality survives the whole suite (unchanged since round 1 — deferred by the code-age rule)
中文说明
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| pullRequest(number:$pr){ | ||
| reviewThreads(first:100, after:$endCursor){ | ||
| nodes{id isResolved comments(first:100){nodes{databaseId} pageInfo{hasNextPage}}} | ||
| nodes{id isResolved comments(first:100){nodes{databaseId author{login} body}} pageInfo{hasNextPage}} |
There was a problem hiding this comment.
[Critical] R2-3: This query line moves pageInfo{hasNextPage} OUT of comments(first:100) to the review-thread node level — but pageInfo is not a field of PullRequestReviewThread, so GitHub rejects the entire query. Verified live at this commit: the PR's verbatim query returns Field 'pageInfo' doesn't exist on type 'PullRequestReviewThread' (gh exit 1, error envelope only); the same query with pageInfo{hasNextPage} restored inside comments(...) returns exit 0 with real thread data. Consequence on every run: THREADS_FETCH_OK=false → THREADS_JSON='[]' → (1) the resolve block resolves nothing — a regression of pre-existing behavior; (2) root_id falls back to rc_id, so replies aimed at reply-ids are rejected by GitHub; (3) the idempotence gate — the point of this PR — always sees an empty view and degrades to post-always, so the #9296 duplicate replies recur every round. All tests stay green because the stub gh matches the query by substring and THREADS_JSON is injected by env. The field was relocated by the merge with main (#9390's pagination rewrite).
Witness (live probe at the reviewed commit):
gh api graphql (PR's verbatim query) -> exit 1
"Field 'pageInfo' doesn't exist on type 'PullRequestReviewThread'"
control (pageInfo restored inside comments(...)) -> exit 0, real thread data
Fix: restore the inner position — comments(first:100){nodes{databaseId author{login} body} pageInfo{hasNextPage}} — and drop the stray thread-level pageInfo{hasNextPage}. Note the >100-comment truncation detector at ~line 6362 consumes .comments.pageInfo.hasNextPage: a repair that only deletes the stray field leaves that warning permanently dead while the suite stays green (the existing pin injects pageInfo via THREADS_RAW_STUB and cannot discriminate the two fix shapes).
中文说明
[Critical] R2-3:该查询行把 pageInfo{hasNextPage} 从 comments(first:100) 内移到了 review-thread 节点层级——但 PullRequestReviewThread 类型没有 pageInfo 字段,GitHub 会拒绝整个查询。已在本 commit 实测验证:PR 原样查询返回 Field 'pageInfo' doesn't exist on type 'PullRequestReviewThread'(gh exit 1,只有错误信封);将 pageInfo{hasNextPage} 恢复到 comments(...) 内的对照查询返回 exit 0 和真实线程数据。每次运行的后果:THREADS_FETCH_OK=false → THREADS_JSON='[]' →(1)resolve 块什么都不会 resolve——既有行为的回归;(2)root_id 回退为 rc_id,指向回复 id 的回复会被 GitHub 拒绝;(3)幂等门(本 PR 的目的)永远看到空视图、退化为照发,#9296 的重复回复每轮都会复发。所有测试仍然绿色,因为 stub gh 只按子串匹配查询、且 THREADS_JSON 由环境变量注入。该字段是在与 main 的合并(#9390 的分页重写)中被移位的。
修复:恢复内层位置——comments(first:100){nodes{databaseId author{login} body} pageInfo{hasNextPage}}——并删除错位的线程级 pageInfo{hasNextPage}。注意约 6362 行的 >100 评论截断检测器消费 .comments.pageInfo.hasNextPage:只删除错位字段而不恢复内层字段,会让该警告在套件全绿的情况下永久失效(现有 pin 通过 THREADS_RAW_STUB 注入 pageInfo,无法区分两种修法)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| pullRequest(number:$pr){ | ||
| reviewThreads(first:100, after:$endCursor){ | ||
| nodes{id isResolved comments(first:100){nodes{databaseId} pageInfo{hasNextPage}}} | ||
| nodes{id isResolved comments(first:100){nodes{databaseId author{login} body}} pageInfo{hasNextPage}} |
There was a problem hiding this comment.
[Suggestion] R1-1: Still standing at this commit (revert probe re-measured): the author{login} body fields supplying the idempotence gate are pinned by no test. Reverting only this query hunk (gate and new tests kept) leaves all tests green — the reply-gate tests inject THREADS_JSON downstream of the fetch, and the resolve-block test pins only reviewThreads(first:100, after:$endCursor) and the outer pageInfo{hasNextPage endCursor} order, not the comment field list. A later edit trimming the query back to nodes{databaseId} (e.g. for payload size) would leave every test green while the gate silently degrades to post-always in production and the #9296 duplication recurs. Witness: revert probe at this commit → 2 passed | 178 skipped; grep of scripts/tests for comments(first:100) / author{login}: zero pins. Fix: pin the query string next to the existing pins (~line 16117 of the test file), e.g. expect(block).toContain('comments(first:100){nodes{databaseId author{login} body} pageInfo{hasNextPage}}') — it must nail pageInfo's inner position too (a substring pin of the shipped line would pass while GitHub rejects the query — see the Critical above).
中文说明
[Suggestion] R1-1:在本 commit 仍然存在(revert 探针重新实测):为幂等门提供数据的 author{login} body 字段没有任何测试 pin。只还原这个查询 hunk(保留门和新测试)时所有测试仍绿——回复门测试在 fetch 下游注入 THREADS_JSON,而 resolve 块测试只 pin 了 reviewThreads(first:100, after:$endCursor) 和外层 pageInfo{hasNextPage endCursor} 的顺序,没有 pin 评论字段列表。后续若把查询裁剪回 nodes{databaseId}(例如为了 payload 体积),所有测试仍绿,而生产中门会静默退化为照发,#9296 的重复回复复发。证据:本 commit 上的 revert 探针 → 2 passed | 178 skipped;在 scripts/tests 中 grep comments(first:100) / author{login}:零 pin。修复:在现有 pin 旁(测试文件约 16117 行)pin 查询字符串,例如 expect(block).toContain('comments(first:100){nodes{databaseId author{login} body} pageInfo{hasNextPage}}')——必须同时钉住 pageInfo 的内层位置(对当前行做子串 pin 会在 GitHub 拒绝查询的情况下仍然通过——见上面的 Critical)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const deduped = readFileSync(repliedLog, 'utf8').trim().split('\n'); | ||
| expect(deduped).toHaveLength(1); | ||
| expect(deduped[0]).toContain('pulls/7731/comments/100/replies'); | ||
| expect(deduped[0]).toContain('body=Changed reason'); |
There was a problem hiding this comment.
[Suggestion] R1-3: Still standing at this commit (mutant probe re-measured): none of the new fixture bodies contains <!--, so a one-line mutant — the gate comparing the raw decoded body instead of the neutralised REPLY_BODY — survives the suite (1 passed | 179 skipped with the mutant applied). A future edit comparing the pre-neutralisation body would silently stop matching stored (neutralised) bodies in production, and duplicates would recur with the suite green. Probe flip verified: a fixture whose reply body contains <!-- fails the mutant (duplicate posted) and passes shipped code (skip logged). Fix: add such a fixture, with the stored bot body carrying the sed-neutralised form.
中文说明
[Suggestion] R1-3:在本 commit 仍然存在(突变探针重新实测):新 fixture 的正文都不含 <!--,因此一个单行突变——门比较 base64 解码后的原始正文而非 neutralised 的 REPLY_BODY——能通过整个套件(应用突变后 1 passed | 179 skipped)。未来若把比较改成 neutralise 之前的正文,生产上会与已存储(已 neutralise)的正文永远不匹配,重复回复复发而套件全绿。翻转已验证:回复正文含 <!-- 的 fixture 能让突变失败(重复发布)、对现有代码通过(记录跳过)。修复:新增这样一个 fixture,存储的 bot 正文使用 sed neutralise 之后的形态。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| }, | ||
| ]); | ||
| const humanEcho = readFileSync(repliedLog, 'utf8').trim().split('\n'); | ||
| expect(humanEcho).toHaveLength(1); |
There was a problem hiding this comment.
[Suggestion] R1-4: Still standing at this commit (probe re-measured): every reply-gate fixture uses exactly one thread, so a mutant scanning comments across ALL threads (instead of scoping to the thread containing the target id) survives the suite. A/B at this commit: against a two-thread view where thread 1 holds the bot's duplicate body and the reply targets thread 2, the shipped thread-scoped gate exits 1 (post — correct) while the cross-thread mutant exits 0 (skip — wrong). In production that shape silently suppresses finding B's reply because finding A's thread already carries the same declination — exactly the silence this block exists to prevent. Fix: add a two-thread fixture asserting the reply still posts to thread 2's root while a same-body reply to thread 1 is skipped.
中文说明
[Suggestion] R1-4:在本 commit 仍然存在(探针重新实测):所有回复门 fixture 都只有一个线程,因此跨所有线程扫描评论(而不是限定在包含目标 id 的线程内)的突变能通过套件。本 commit 上的 A/B:在「线程 1 存有 bot 的相同正文、回复目标是线程 2」的双线程视图下,现有的线程限定门 exit=1(发布——正确),跨线程突变 exit=0(跳过——错误)。生产中这种形态会因为 finding A 的线程已有相同措辞而静默吞掉 finding B 的回复——正是本块要防止的静默。修复:新增双线程 fixture,断言回复仍会发往线程 2 的根评论,同时对线程 1 的同正文回复被跳过。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| { databaseId: 222 }, | ||
| { | ||
| databaseId: 300, | ||
| author: { login: 'qwen-code-dev-bot' }, |
There was a problem hiding this comment.
[Suggestion] R1-5: Still standing at this commit (probe re-measured): in both new fixtures the compared comment (bot or human) is the LAST node of its thread, so a mutant narrowing the gate's scan to the newest comment survives the whole suite. A/B at this commit: with the matching bot comment followed by a later unrelated human comment in the same thread, the shipped gate exits 0 (skip — correct) while the newest-only mutant exits 1 (post — duplicate). Fix: add a fixture where the matching bot comment is followed by another comment in the same thread.
中文说明
[Suggestion] R1-5:在本 commit 仍然存在(探针重新实测):两个新 fixture 中被比较的评论(bot 或人类)都是其线程的最后一个节点,因此把门的扫描收窄到最新评论的突变能通过整个套件。本 commit 上的 A/B:匹配的 bot 评论之后同一线程还有一条无关的人类评论时,现有门 exit=0(跳过——正确),只看最新评论的突变 exit=1(发布——重复)。修复:新增一个 fixture,让匹配的 bot 评论之后同线程还有另一条评论。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| author: { login: 'qwen-code-dev-bot' }, | ||
| body: 'Deferred — follow-up.\n\n中文:已延后。', |
There was a problem hiding this comment.
[Suggestion] R1-6: Still standing at this commit (probe re-measured): no reply-gate fixture sets isResolved on a thread, so a mutant adding select(.isResolved | not) to the gate's selector — a plausible harmonisation with the resolve block's selector ~15 lines above in the workflow — survives the suite (null | not is true when the field is absent). With that mutant in place, a reply to a comment in a resolved thread is silently skipped (no post, no distinct log line) and the suite stays green — the intended semantics for resolved threads are unpinned. Fix: add a fixture with isResolved: true on the thread asserting the reply still posts (or is deliberately skipped with a distinct log line, per the intended semantics).
中文说明
[Suggestion] R1-6:在本 commit 仍然存在(探针重新实测):没有任何回复门 fixture 给线程设置 isResolved,因此在门的选择器上加 select(.isResolved | not) 的突变能通过套件——这是一种与工作流上方约 15 行 resolve 块选择器的「合理对齐」,且字段缺失时 null | not 为 true。该突变生效后,对已 resolve 线程中评论的回复会被静默跳过(不发布、也没有独立日志),套件仍全绿——resolved 线程下的预期语义没有被 pin。修复:新增一个线程带 isResolved: true 的 fixture,断言回复仍会发布(或按预期语义被有意跳过并输出独立日志)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
@qwen-code /takeover |
1 similar comment
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
…y-gate contracts (QwenLM#9463)
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Round summary — PR #9463 (address-review)The round-2 review was CHANGES_REQUESTED with one Critical (R2-3) plus six re-raised Suggestions (R1-1…R1-6). All 12 inline findings are addressed and resolved in code in one commit: Critical R2-3 — threads query rejected by GitHub (rc:3813955281)Fixed. The merge with main (#9390's pagination rewrite) relocated Fix: restored
R1-2 — per-thread comment truncation invisible (rc:3810982938)Resolved by the same query fix. The >100-comment truncation detector (consumes R1-1 — gate-supplying fields pinned by no test (rc:3810982933, rc:3813955286)Fixed. Added a substring pin next to the existing query pins in the resolve test: R1-3 — neutralised-body comparison unpinned (rc:3810982942, rc:3813955292)Fixed. Added a scenario where the pending reply body contains R1-4 — thread-scoping unpinned (rc:3810982947, rc:3813955301)Fixed. Added a two-thread scenario: thread 1 carries the bot comment with body X; a pending reply with the same body X targets thread 2. Asserts the reply on thread 2 still posts (remapped to that thread's root 400) while the same-body reply to thread 1 is skipped. A cross-thread-scan mutant would suppress thread 2's reply and fail. R1-5 — full-comment scan unpinned (rc:3810982949, rc:3813955308)Fixed. The dedupe scenario's thread now has a human comment FOLLOWING the matching bot comment in the same thread (the common live shape: root, bot declination, human answer). The skip assertions still hold; a newest-comment-only mutant would re-post the duplicate and fail. R1-6 — isResolved semantics unpinned (rc:3810982953, rc:3813955314)Fixed. The dedupe scenario's thread now carries Not requested this round — reviewer's own deferralsThe round-2 review body records two probe findings as "Deferred under the convergence posture … recorded, not requested in this round" (in-pass double-post through the pre-loop THREADS_JSON snapshot; no substring-containment fixture). Left as-is per that explicit deferral; they remain tracked in the reviewer's ledger and were not implemented or declined here. Issue-level comment ic:5338263958 (deferring to @wenshao)The merge-conflict half is stale: HEAD ( Conflict notes
Verification
中文说明本轮总结 — PR #9463(address-review)第 2 轮审查为 CHANGES_REQUESTED,含 1 个 Critical(R2-3)与 6 个重新提出的 Suggestion(R1-1…R1-6)。全部 12 条行内 finding 均已处理并在代码中解决,合入一个 commit: Critical R2-3 — 线程查询被 GitHub 拒绝(rc:3813955281)已修复。 与 main 的合并(#9390 的分页重写)把 修复:将
R1-2 — 单线程评论截断不可见(rc:3810982938)由同一个查询修复解决。 >100 条评论的截断检测器(消费 R1-1 — 为门提供数据的字段无测试保护(rc:3810982933、rc:3813955286)已修复。 在 resolve 测试中现有查询 pin 旁新增子串 pin: R1-3 — neutralised 正文比较未被钉住(rc:3810982942、rc:3813955292)已修复。 新增场景:待发回复正文含 R1-4 — 线程作用域未被钉住(rc:3810982947、rc:3813955301)已修复。 新增双线程场景:线程 1 上已有正文为 X 的 bot 评论;正文同为 X 的待发回复指向线程 2。断言线程 2 上的回复照常发布(重映射到该线程根评论 400),同时指向线程 1 的同正文回复被跳过。跨线程扫描的变异体会吞掉线程 2 的回复并使断言失败。 R1-5 — 全评论扫描未被钉住(rc:3810982949、rc:3813955308)已修复。 去重场景的线程现在在匹配的 bot 评论之后还有一条人类评论(常见真实形态:根评论、bot 的 decline、人类作答)。跳过断言仍然成立;只看最新评论的变异体会重发重复回复并使断言失败。 R1-6 — isResolved 语义未被钉住(rc:3810982953、rc:3813955314)已修复。 去重场景的线程现在带 本轮不要求处理 — 审查者自身的延后项第 2 轮审查正文记录了 2 条探针 finding,标注「收敛姿态下延后……已记录,本轮不要求修改」(经由循环前 THREADS_JSON 快照的同轮内双发;缺少子串包含关系的 fixture)。按该明确延后保持原样;它们仍由审查者的 ledger 跟踪,本轮既未实现也未拒绝。 Issue 级评论 ic:5338263958(转交 @wenshao)合并冲突一半已过时:HEAD( 冲突说明
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-autofix.yml:5478 — [review] gate comment attributes the 2026-08-16 #9296 incident to a flow this gate structurally cannot catch (human-authored, cross-thread, standalone review submissions)
中文说明
无阻断问题。LGTM!✅
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. CI green. The reply-idempotence gate is correctly scoped to exact duplicates (same bot login + byte-identical body), so a later round with a changed reason still posts; it degrades to post-always on a stale/empty threads view rather than silently dropping replies. Tests pin the GraphQL field shape (author/body + pageInfo's inner position) and the fall-back to a pre-existing authorless/bodyless thread view.
|
Released in v0.21.15. |
What this PR does
Makes the autofix finding-reply step idempotent. The thread fetch now also reads each comment's author and body, and the reply step skips posting when the thread already carries a comment by the autofix bot whose body EQUALS the neutralised body about to be posted. A changed body — new information from a later round — still posts; a threads view without author/body, or a stale/empty one, degrades to the old post-always behavior (best-effort, consistent with the rest of the block).
Why it's needed
The reply path is invoked from every completed address round, and the same finding gets an entry in
comment-replies.jsonagain whenever the round crashes and re-runs, a same-run repair regenerates the dispositions, or a later round's agent re-declines an unchanged finding. Each of these re-posts the same reply on the same thread. Measured on 2026-08-16 (full details in #9296): one identical reply landed three times on a merged PR within seconds of each other.Note on scope: the item in #9296 also mentions using the replies API instead of standalone review submissions — the workflow's reply path already uses
POST /repos/{o}/{r}/pulls/{n}/comments/{id}/replies, which emits nopull_request_reviewevent, so this PR only adds the missing idempotence gate.Reviewer Test Plan
How to verify
This is a CI-workflow change with a pinned test; behavior is also observable in round logs:
npm run test:scripts -- qwen-autofix-workflow.test.jsruns the pinned testanswers the threads it leaves open, which extracts the reply block from the workflow and drives it against a stubbedgh. New scenarios: an identical bot reply already on the thread is skipped (no POST, logged as skipped), a changed body still posts, and the same body last posted by a HUMAN is not treated as a duplicate.identical bot reply already on the threadand does not double-post.Local verification performed: the pinned test passes including the new scenarios; the full pinned suite shows 178/180 green with the 2 failures reproduced on a clean origin/main baseline (bite-check and verification-gate tests, unrelated to this change); the workflow parses as YAML and passes yamllint 1.35.1 (the CI-pinned version) with zero findings; the extracted reply block passes
bash -nand shellcheck at error severity; the dedup jq expression was tested against 7 edge-case fixtures (exact bot match → skip; different body → post; human-authored same body → post; legacy nodes without author/body → post; null author → post; orphan comment id → post; empty threads view → post).Evidence (Before & After)
Before: 2026-08-16 — one identical finding reply posted three times on a merged PR within seconds (#9296). After: the second and later identical posts are skipped at the gate; the first post and any CHANGED follow-up still land.
Tested on
Environment (optional)
N/A — workflow change only; the workflow runs on ubuntu-latest.
Risk & Scope
Linked Issues
Part of #9296 (second P1 item — finding replies). No closing keyword — the issue still tracks the P2 items.
中文说明
这个 PR 做了什么
让 autofix 的 finding 回复步骤幂等。线程抓取现在同时读取每条评论的作者和正文;回复步骤在 thread 中已存在 autofix bot 发的、与即将发布的 neutralised 正文逐字相等的评论时跳过发布。内容变了(新一轮的新理由)照常发布;线程视图缺少 author/body 或过期/为空时,退化为旧的照发行为(best-effort,与该块其余部分一致)。
为什么需要
回复路径在每个完成的 address 轮次都会执行;当轮次 crash 重跑、same-run repair 重新生成 disposition、或后续轮次的 agent 对未变化的 finding 重新 decline 时,同一个 finding 会再次进入
comment-replies.json,每一种情况都会在同一 thread 上重发相同回复。2026-08-16 实测(完整细节见 #9296):同一条回复在几秒内被发了三遍。范围说明:#9296 的条目还提到改用 replies API 而非独立 review 提交——工作流的回复路径本来就用
POST /repos/{o}/{r}/pulls/{n}/comments/{id}/replies(不产生pull_request_review事件),所以本 PR 只补缺失的幂等门。Reviewer 测试计划
如何验证
这是带 pin 测试的 CI 工作流变更,行为也体现在轮次日志中:
npm run test:scripts -- qwen-autofix-workflow.test.js运行 pin 测试answers the threads it leaves open——它从工作流中提取回复块并用 stub 的gh驱动。新场景:thread 上已有相同 bot 回复时跳过(无 POST,日志记录 skipped);内容变化时照常发布;人类发的相同内容不算重复。identical bot reply already on the thread,不会重复发布。已完成的本地验证:pin 测试通过(含新场景);完整 pin 套件 178/180 通过,2 个失败在干净 origin/main 基线上复现(bite-check 与 verification-gate 测试,与本改动无关);工作流 YAML 解析通过,yamllint 1.35.1(CI 固定版本)零问题;提取的回复块通过
bash -n与 error 级 shellcheck;去重 jq 表达式用 7 种边界 fixture 测试(bot 完全匹配→跳过;不同内容→发布;人类同内容→发布;旧形态无 author/body→发布;author 为 null→发布;孤儿评论 id→发布;空线程视图→发布)。证据(改动前后)
改动前:2026-08-16——同一条 finding 回复在几秒内被发布三遍(#9296)。改动后:第二次及之后的相同发布在门处被跳过;首次发布与任何内容变更的后续回复照常落地。
测试环境
环境(可选)
N/A——仅工作流变更;工作流运行在 ubuntu-latest。
风险与范围
关联 Issue
属于 #9296(第二个 P1 项——finding 回复)。不使用关闭关键字——该 issue 还跟踪 P2 项。