feat(autofix): defer verified out-of-footprint findings to a surviving follow-up queue - #9189
Conversation
…g follow-up queue
Anti-drift closure for the review loop: a finding that is REAL but whose
fix lies outside the PR's footprint previously had only wrong outcomes —
implement it (scope drift), decline it (the finding is lost when the PR
merges and nobody re-reads its threads), or push it onto a maintainer.
- SKILL gains the fourth disposition, Defer to follow-up: verified +
out-of-footprint → record {id, path, reason} in
deferred-findings.json, reply on the thread that it is deferred, leave
the thread open. Decline stays for what is not worth doing anywhere;
defer is for what is worth doing elsewhere.
- The report step upserts these into one per-PR 'Deferred review
findings' issue (marker-keyed, append-only by rc id, agent text
token-neutralized and length-capped), for BOTH pushed and no-op
outcomes. Best-effort: an upsert failure never fails a round.
Deliberately no ready-for-agent label — feeding the bot's own
deferrals back into its issue queue is a human authorization.
- deferred-findings.json rides the artifact dump and the repair
cleanup; the neutralization ledger grows to ten sites.
- Tests: shape validation (non-empty array of numeric-id items), line
building (dedupe by rc id against the existing issue body, newline
flattening, truncation), and wiring pins for both call sites.
|
Re-run — this gate pass covers the round-15 head; prior passes covered the first commit and the round-12 and round-14 heads. Template looks good ✓ Problem: unchanged and still observed, not theoretical. The footprint gates from #8996/#9156 bound where a round may change code, but a finding the agent verifies as real whose fix lives outside that boundary had no correct disposition — implementing it drifts the PR, declining loses it at merge, escalating bounces verified work back to a maintainer. The #8996 backlog living on in open threads of a merged PR remains the concrete case study. Direction: aligned. Same series as the merged anti-bloat (#8981) and footprint (#8996/#9156) work, entirely inside the autofix CI/skill infrastructure — no product surface, no auth/sandbox/telemetry/public-contract impact. CHANGELOG has no direct reference to this mechanism, but the area is clearly relevant. Size: no core module paths touched. 4 files, +1899/−6 overall; the test battery carries +1250/−1, leaving ~650 production lines (including the SKILL doc) — under the large-PR advisory. Round 15's production delta is ~30 lines across the upsert script and the workflow. Approach: scope remains disciplined. Every round-15 hunk maps to a named finding from the round-14 review: the sibling identity that collided once two findings differed only past the 500-char reason cap, the repair-side carry merge whose argument order inverted the script's newer-first precedence, stray whitespace around a resolved id that let a fixed finding re-enter the queue, the last two wrapper-authored warnings missing the Risk: no elevated risk signals — none of the changed files match the revert-correlated path patterns. Moving on to code review. 🔍 中文说明Re-run——本次门检查覆盖第 15 轮 head；此前分别覆盖第一个提交、第 12 轮与第 14 轮 head。 模板完整 ✓ 问题：不变，仍是已观测而非理论性问题。#8996/#9156 的足迹门规定了轮次可以在哪里改代码，但 agent 已验证为真、修复位于边界之外的 finding 没有正确的处置——实现它会偏离主线，decline 会在合并时丢失，escalate 则把已验证的工作压回维护者。#8996 的 backlog 活在已合并 PR 的开放线程里，仍是现成的案例。 方向：对齐。与已合并的防膨胀（#8981）和足迹（#8996/#9156）系列一脉相承，完全在 autofix CI/skill 基础设施内部——不涉及产品面、auth/sandbox/telemetry/公共契约。CHANGELOG 无直接引用，但该领域明显相关。 规模：未触及核心模块路径。4 个文件，+1899/−6；测试套件占 +1250/−1，生产代码约 650 行（含 SKILL 文档）——低于大 PR 提示线。第 15 轮生产增量约 30 行，分布在 upsert 脚本与工作流。 方案：范围依然克制。第 15 轮的每一块改动都对应第 14 轮评审点名的 finding：两个 finding 仅在 reason 500 字符上限之后不同时会相撞的 sibling 身份、repair 侧 carry 合并参数顺序与脚本"新者优先"相反、已解决 id 两侧的杂散空白会让已修复的 finding 重新入队、最后两条 wrapper 自撰告警缺 风险：无升级风险信号——改动文件均不匹配与 revert 相关的路径模式。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-run — this pass reviews the round-15 delta in full and re-verifies the load-bearing wiring at the current head rather than taking the diff's word for it. My independent expectation for this delta, written before reading it: "un-truncate the sibling identity" must derive the rv/ic dedupe key from the UNCAPPED path+reason (the normkey itself carries no cap) while the corpus check keeps comparing rendered forms — the issue stores only rendered lines, so cross-round the cap can cost a visible duplicate but never a silent loss; "align the carry precedence" must swap the repair-side What I verified at this head:
No blockers. Standing non-blocking notes, unchanged: a backtick can appear in reason text and visually break a bullet's code span (no forgery channel survives it), and the trusted-base content transport means a PR editing the upsert script exercises its change one merge late — inherent to the trust model, unit suite still covers the logic pre-merge. TestingUnattended CI run — this review does not execute PR code; the evidence below is the PR's own CI on the reviewed commit, fetched via the API (single fetch, no polling). The finalize job updates the table if CI moves. CI on
The green ubuntu leg is the load-bearing one: it executes the changed Sandboxed-verification scope note: the 中文说明Re-run——本次完整审查第 15 轮增量，并在当前 head 上重新核实关键接线，而不是仅凭 diff 的说法。 读 diff 前我写下的独立预期："解除 sibling 身份的截断"应从未截断的 path+reason 推导 rv/ic 去重键（normkey 本身无上限），而语料比对继续比渲染形态——issue 只存渲染后的行，跨轮时上限至多造成可见重复，绝不静默丢失；"对齐 carry 优先级"应把 repair 侧 jq -s 'add' 的参数换成本轮在前——unique_by 保留组内首个，脚本自己的 union 已把该顺序定为新鲜度规则。diff 与两点预期吻合，其余改动全部是第 14 轮点名的 finding，无新范围。 在当前 head 上核实的内容：
无阻断项。沿用至今的非阻塞备注不变：reason 文本中的反引号可能在渲染时打断 bullet 的代码样式（无伪造通道存活）；可信 base 内容传输意味着修改 upsert 脚本的 PR 要晚一次合并才能行使改动——信任模型固有属性，单测在合并前仍覆盖逻辑。 测试说明：无人值守 CI 运行——本审查不执行 PR 代码；以上证据为被审提交上 PR 自身 CI 经 API 单次抓取的结果，finalize 任务会在 CI 变动时更新表格。2258cfbf 的 CI 全绿且已定型，无未完成的 pull_request 运行。绿色 ubuntu 腿是载荷腿：执行改动的 qwen-autofix-workflow.test.js 套件（对录制式 gh 桩执行真实脚本）。跳过的平台腿与本 PR 历次提交一致（未触及产品代码）。该 SHA 上更早的一次 attempt-2 运行数分钟内被取消——是被取代的重复运行，不是失败。 沙箱验证范围说明：/verify 腿在第 12 轮 head 上通过（1489/1489 断言）、在第 15 轮推送前又通过一次，针对当前 head 的新一轮正由本次 triage 运行执行——完成后报告会发在本线程。在此之前增量没有任何内容依赖作者自述：绿色套件通过执行真实脚本钉死每个新行为，而不是断言 diff 的文字。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — round 15 lands exactly the fixes its review round named, each structurally done and test-pinned, CI green and settled; the two standing cosmetic notes remain the only reservations. Stepping back: this is still the loop working as designed. The round-14 review named fourteen findings; the round-15 delta answers the ones that needed code — and I checked each answer at the head instead of trusting the commit message. The identity fix is the interesting one: two siblings that differed only past the 500-char reason cap used to collide and one vanished, which is precisely the silent loss this feature exists to prevent; the fix keys intra-batch dedupe on the uncapped text while the corpus check keeps comparing rendered forms, accepting a possible visible duplicate across rounds and never a loss. My independent proposal for both headline items matched the diff, and the carry-precedence swap now makes the repair side and the script side enforce the same newer-first rule instead of contradicting each other. What also counts in this PR's favor at this point: a maintainer has already approved this exact head, the green suite executes the real upsert script end-to-end rather than asserting prose about it, and fifteen rounds of this loop have steadily removed attack surface (agent-writable paths, loader side channels, forgeable markers) rather than adding it. The two reservations are unchanged and cosmetic: backticks in reason text can visually break a rendered bullet's code span (no forgery channel survives), and the trusted-base content transport means a PR editing the upsert script exercises its change one merge late. Neither is worth holding the PR for. CI is green and settled on the reviewed head — no pending 中文说明回顾全局：这仍是该循环按设计运转的样子。第 14 轮评审点名十四条 finding；第 15 轮增量回答了其中需要改代码的部分——而我在 head 上逐条核实了这些回答，不是只信提交说明。身份修复是最值得看的一处：两个仅在 reason 500 字符上限之后不同的 sibling 过去会相撞、其中一个消失——这正是本功能要防止的静默丢失；修复让批内去重以未截断文本为键，语料比对继续比渲染形态，接受跨轮可能出现可见重复，但绝不丢失。我对两个标题项的独立方案与 diff 吻合；carry 优先级的交换使 repair 侧与脚本侧从此执行同一条"新者优先"规则，而不是互相矛盾。 此刻同样有利于本 PR 的是：一位维护者已在这个精确 head 上批准，绿色套件端到端执行真实 upsert 脚本而非断言关于它的文字，且十五轮循环在不断移除攻击面（agent 可写路径、loader 旁路、可伪造 marker）而非增加。两条保留意见不变且均为外观层面：reason 文本中的反引号可能在渲染时打断 bullet 的代码样式（无伪造通道存活）；可信 base 内容传输意味着修改 upsert 脚本的 PR 要晚一次合并才能行使改动。两者都不值得扣住 PR。 被审 head 上 CI 全绿且已定型，无未完成的 pull_request 运行——批准不受阻，已钉死在所审的精确提交上。 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
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.
[Critical] R1-10 通用 finding disposition 实际只支持 inline review comment。Failure scenario: review body 或 issue-level comment 中的已验证足迹外 finding 没有 inline comment id 和 thread，却被规则要求写 numeric inline id 并回复线程；agent 无法满足协议，finding 会丢失或被错误标记。Witness: not run — SKILL disposition 与 feedback 三种来源 schema 直接矛盾。Suggested fix: 引入 source-qualified 稳定键（rc/rv/ic），仅 rc 回复线程，或为非 inline 来源定义独立持久化路径。
中文说明
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
[Critical] R1-10 通用 finding disposition 实际只支持 inline review comment。Failure scenario: review body 或 issue-level comment 中的已验证足迹外 finding 没有 inline comment id 和 thread，却被规则要求写 numeric inline id 并回复线程；agent 无法满足协议，finding 会丢失或被错误标记。Witness: not run — SKILL disposition 与 feedback 三种来源 schema 直接矛盾。Suggested fix: 引入 source-qualified 稳定键（rc/rv/ic），仅 rc 回复线程，或为非 inline 来源定义独立持久化路径。
— gpt-5.6-sol via Qwen Code /review (v0.21.11)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Unresolved, please confirm:
- [Critical] .github/workflows/qwen-autofix.yml:5531 — existing claim that failure/terminal-handoff rounds lose verified deferrals: this review confirmed no thread reply is promised without the upsert (no broken-promise path), but did not exhaustively trace every terminal path's persistence, so it cannot rule on the claim.
- [Critical] .github/workflows/qwen-autofix.yml:5534 — existing claim that this PAT-bearing step trusts a predictable same-user file that PR-controlled build/test can replace: the pattern pre-exists across the workflow's other publish sites, but this diff adds a new issue-creation consumer; this run could not rule on the trust-boundary claim.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": did not exercise live gh --paginate behavior against the real API (only the tests' local jq replays), and did not inspect the ecs-qwen runner images' jq/gh ve….
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.
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5534): the shape gate does not validate path — a non-string path passes the gate, aborts the line-builder's jq, and || new_lines='' silently wipes the whole batch. Independently reproduced by this review's probe (gate exit 0, builder exit 5, silent return).
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5541): the tracking-issue lookup's failure modes (awk early-exit SIGPIPE under pipefail; transient errors; multi-line bodies detaching the marker from the issue number) are conflated with 'issue not found', minting permanent duplicate issues or wedging on a garbage URL. Independently reproduced by this review's bash/jq probes.
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5545): a failed GET of a found issue's body becomes an empty body, and the PATCH then replaces the entire issue body — destroying the marker and all previously recorded deferrals. Independently reproduced: probe PATCH body contained only the new round's lines.
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5552): unanchored substring dedupe contains("rc:<id> ") lets reason prose mentioning rc:N permanently suppress a genuine later deferral of id N. Independently reproduced: extracted program emitted empty output for the genuine item.
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5550): nothing cross-checks resolved-comments.txt against deferred-findings.json — upsert_deferred_issue never reads the resolved list, so an id can be resolved as implemented and deferred to the follow-up queue in the same round, a contradictory state. Mechanism confirmed by reading the function at the reviewed commit.
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5553): path is rendered without newline/backtick flattening — a shape-valid record can forge additional - rc: queue lines and poison later dedupe. Independently reproduced: forged line emitted and the genuine later item suppressed.
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5560): appending by whole-body replacement is a stale read-modify-write with no retry (diverging from the 3×/sleep-10 convention 13 lines below in the same step), and the failure path still prints the success line. Independently reproduced: patch-fail probe printed warning + success echo, exit 0.
中文说明
未决，请确认：共 2 条（原文未翻译，列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：did not exercise live gh --paginate behavior against the real API (only the tests' local jq replays), and did not inspect the ecs-qwen runner images' jq/gh ve…。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5534): the shape gate does not validate path — a non-string path passes the gate, aborts the line-builder's jq, and || new_lines='' silently wipes the whole batch. Independently reproduced by this review's probe (gate exit 0, builder exit 5, silent return).
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5541): the tracking-issue lookup's failure modes (awk early-exit SIGPIPE under pipefail; transient errors; multi-line bodies detaching the marker from the issue number) are conflated with 'issue not found', minting permanent duplicate issues or wedging on a garbage URL. Independently reproduced by this review's bash/jq probes.
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5545): a failed GET of a found issue's body becomes an empty body, and the PATCH then replaces the entire issue body — destroying the marker and all previously recorded deferrals. Independently reproduced: probe PATCH body contained only the new round's lines.
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5552): unanchored substring dedupe contains("rc:<id> ") lets reason prose mentioning rc:N permanently suppress a genuine later deferral of id N. Independently reproduced: extracted program emitted empty output for the genuine item.
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5550): nothing cross-checks resolved-comments.txt against deferred-findings.json — upsert_deferred_issue never reads the resolved list, so an id can be resolved as implemented and deferred to the follow-up queue in the same round, a contradictory state. Mechanism confirmed by reading the function at the reviewed commit.
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5553): path is rendered without newline/backtick flattening — a shape-valid record can forge additional - rc: queue lines and poison later dedupe. Independently reproduced: forged line emitted and the genuine later item suppressed.
[Critical] Affirmed on re-check (existing comment at .github/workflows/qwen-autofix.yml:5560): appending by whole-body replacement is a stale read-modify-write with no retry (diverging from the 3×/sleep-10 convention 13 lines below in the same step), and the failure path still prints the success line. Independently reproduced: patch-fail probe printed warning + success echo, exit 0.
— qwen3.8-max via Qwen Code /review (v0.21.11)
- Extracted to a trusted staged script callable from ALL outcome paths —
the failure/handoff path persists verified findings too (a failed
round's commit dying says nothing about the findings' validity).
- Append-only durability: the tracking issue's body is written once;
every later round POSTS a comment — no read-modify-write can race a
maintainer's edits, and a failed body/comments read SKIPS the round
(never mistaken for empty history). Success is logged only when the
write call succeeded; failures say NOT persisted.
- Structured lookup: jq filtering over the real bodies (no line-joined
awk under pipefail), pull requests excluded, lookup failure skips
rather than creating duplicates.
- Dedupe is line-anchored ('- rc:<id> ' at line start, body+comments
corpus) with intra-batch unique_by; ids the round resolved in code are
excluded (a finding cannot be implemented and outstanding at once).
- Shape gate covers path (string when present); path bytes are
charset-sanitized so a crafted path cannot forge queue bullets.
- Publication-trust posture recorded: the deferred lines are the same
agent-authored trust class as every other published output — marker
neutralization, mention-free sanitized charset, length caps, and a
20-item batch cap bound the surface.
- Tests: the real script runs against a recording gh stub — create,
append+dedupe (body and comments), PR-carrying-marker exclusion,
anchored dedupe vs free-text mentions, read-fail skip,
resolved-exclusion, shape-gate loudness, write-fail honesty, and
forged-path sanitization; the neutralization ledger returns to nine
workflow sites with the script-side tenth pinned in place.
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.
Unresolved, please confirm:
- [Critical] Round-1 blocker thread 3786290009 (PAT-bearing step trusts a same-user-writable file): the publication-content facet is now bounded and neutralized (shape gate, charset, caps, marker escape), but its mention-firing remnant (raw @ passing through reason) cannot be adjudicated without a live-render probe (no scratch repo configured) — carried in Needs Human Review as R2-5; the execution-trust facet of the same claim is re-filed this round as R2-2 (staged-script swap) and R2-3 (gh transport hygiene).
- [Critical] R1-10 (round-1 self-review body): the defer protocol keys on inline review-comment ids — code confirms the addressing model is inline-id-only for every disposition (resolved-comments.txt, comment-replies.json, thread resolution all key on them), pre-existing and unchanged by this diff; whether that limits this disposition addition is a scope call the code cannot answer.
Not reviewed: the executable-script lint — .github/scripts/upsert-deferred-issue.sh: shellcheck is not installed.
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.
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 2 条（原文未翻译，列表见上方英文部分）。
未审查：the executable-script lint — .github/scripts/upsert-deferred-issue.sh: shellcheck is not installed。
未检查（工具限制，非阻断）：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.12)
- Pass the known-id corpus to jq via --rawfile: a large corpus in one --arg argv element hits Linux MAX_ARG_STRLEN and the swallowed exec failure would silently drop the round's deferrals. - Digest-gate the staged upsert script: record upsert_sha256 at stage time (expression context) and verify before each of the three invocations; RUNNER_TEMP is agent-writable in between. A mismatch skips persistence, never the round. - Add the gh hygiene preamble (GH_HOST pin, GH_TOKEN unset, fresh GH_CONFIG_DIR) to the review-address failure/handoff report step — the one PAT-bearing gh step that lacked it. - Query the tracking-issue lookup with state=all so a maintainer-closed issue is appended to instead of forking a duplicate. - Enforce integer positive finding ids in the shape gate (a float id's dot is a regex wildcard in the anchored dedupe and never index()-matches resolved ids). - Clip the 20-item batch loudly and qualify success messages with kept/total counts instead of claiming full persistence. - Tests: digest + hygiene wiring pins; stub knobs for list/comments fetch failures; append-write failure, multiline-reason flattening, bad-id, loud-cap, and state=all cases.
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.
Unresolved, please confirm:
- [Critical] .github/workflows/qwen-autofix.yml (round-1 blocker thread 3786290009, mention-firing remnant): raw
@passes through reason sanitization (only CR/LF flattening + 500-cap) into bot-published issue comments; the mechanism is code-confirmed, but whether GitHub renders it as a firing mention could not be render-probed this run — no QWEN_REVIEW_SCRATCH_REPO configured. - [Critical] .qwen/skills/autofix/SKILL.md (R1-10, round-1 self-review body): the disposition protocol keys on inline review-comment ids; code confirms the addressing model is inline-id-only, pre-existing and unchanged by this diff — whether that limits this disposition addition is a scope call the code cannot answer.
Not reviewed: reverse audit — reached the round cap of 5 without two consecutive dry rounds (rounds 3 and 4 each reported new findings; round 5 was dry).
Not reviewed: the executable-script lint — .github/scripts/upsert-deferred-issue.sh: shellcheck is not installed.
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.
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 2 条（原文未翻译，列表见上方英文部分）。
未审查：reverse audit — reached the round cap of 5 without two consecutive dry rounds (rounds 3 and 4 each reported new findings; round 5 was dry)。
未审查：the executable-script lint — .github/scripts/upsert-deferred-issue.sh: shellcheck is not installed。
未检查（工具限制，非阻断）：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.12)
… holes (review round 3) - Sweep BASH_ENV/ENV and imported BASH_FUNC_*%% functions plus proxy (HTTPS_PROXY/HTTP_PROXY/ALL_PROXY + lowercase) and SSL_CERT_FILE/DIR at all four gh-hygiene sites: both families are GITHUB_ENV-plantable and bypass the TRUSTED_PATH pin (child-bash startup) or reroute/ decrypt PAT-bearing HTTPS. Also de-shadow gate-critical names and hash -r, since a planted BASH_ENV runs before the step body. - Pin PATH to the staged trusted value (guarded for pre-stage crashes) and drop the loader trio in the failure/handoff report step — its digest gate previously ran under ambient PATH/LD_PRELOAD. - Failure-path upsert: skip with a plain notice when stage never ran (empty digest is not a tamper alarm), and verify the PAT's bot identity before writing (POST_HANDOFF's check is skipped on the fixed/noop-outcome path); correct the guard comment that claimed parity with the handoff guards. - Fold the twice-pasted digest-gate + upsert block in 'Push and report' into a step-local run_deferred_upsert(), matching the resolve_and_reply_threads convention. - Dedupe corpus reads bot-authored comments only, so a third party commenting on the public tracking issue cannot suppress a finding. - Tests: hygiene sweep pins ordered before the first gh call; placement assertions (function defined once, called after both resolve arms; failure invocation inside the DRY_RUN/STALE/token guard slice); digest/identity/notice pins; behavioral cases for foreign-author suppression, intra-batch duplicate ids, markerless-issue create path, creator/marker anchors, and marker neutralization through the append path.
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.
Unresolved, please confirm:
- [Critical] Thread 3786290009 (mention-firing remnant, rounds 2-3): raw
@still passes reason sanitization (script reason handling is onlygsub("[\r\n]+"; " ")+ 500-cap) into bot-published issue comments — mechanism code-confirmed at HEAD; whether GitHub renders it as a firing mention cannot be render-probed this run (no QWEN_REVIEW_SCRATCH_REPO configured). - [Critical] R1-10 (round-1 self-review body blocker): the disposition protocol keys on inline review-comment ids — code confirms the addressing model is inline-id-only for every disposition, pre-existing and unchanged by this diff; whether that limits the defer disposition is a scope call the code cannot answer.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
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.
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 2 条（原文未翻译，列表见上方英文部分）。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：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.12)
…he unsound in-shell sweep (review round 4) Round 4 (R4-1, five Criticals of one class) showed the in-shell BASH_FUNC/proxy denylist sweep the prior round added is unsound: it bootstraps trust from the very shell namespace it sanitizes, and a planted BASH_FUNC_env%%/unset%%/command%%, an expand_aliases alias, a readonly -f shadow or a DEBUG trap each defeat it — ending in the staged upsert script executing with CI_DEV_BOT_PAT in env. - Replace it with sound isolation: both upsert sites (run_deferred_upsert in 'Push and report', and the failure/handoff path) run the digest gate, the PAT identity check and the staged script in a fresh '/usr/bin/env -i … bash --norc -c' child. /usr/bin/env is invoked by absolute path — bash never does function/alias lookup on a slash-bearing word, so a planted BASH_FUNC_env%% cannot intercept it — and env -i drops every BASH_FUNC_*, BASH_ENV, SHELLOPTS, alias and trap before any gated work. GH_CONFIG_DIR is minted inside the clean child (its mktemp cannot be shadowed there), closing the mktemp-shadow hole in the failure step's preamble. - Remove the sweep and the in-step gh/PATH preamble the prior round added to all four PAT gh steps; the three pre-existing steps revert to their prior posture. Hardening the pre-existing PAT gh calls (handoff/report comment, push, publish) against BASH_FUNC/transport plants is noted as separate, out of this feature's scope. - Script: accept a contract-valid empty array as a clean no-op instead of a false 'malformed' alarm; 'set +C' so a planted read-only SHELLOPTS=noclobber cannot silently empty the dedupe corpus (belt to the env -i child that already drops SHELLOPTS). - Tests: replace the sweep pins with clean-child pins (absolute-path env -i at both sites, GH_CONFIG_DIR/PATH inside the child, failure launch inside the guard slice); add no-findings exit-0, empty-array no-op and set +C cases. Behavioral probe: a fully tainted parent (BASH_FUNC/alias/BASH_ENV plants) cannot reach into the env -i child.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Unresolved, please confirm:
- [Critical] .github/workflows/qwen-autofix.yml (blocker thread 3786290009, mention-firing remnant, rounds 2-4): raw
@still passes reason sanitization in upsert-deferred-issue.sh at HEAD (onlygsub("[\r\n]+"; " ")+ 500-cap) into bot-published issue comments — mechanism code-confirmed at the reviewed commit; whether GitHub renders it as a firing mention cannot be render-probed this run (no QWEN_REVIEW_SCRATCH_REPO configured). - [Critical] .qwen/skills/autofix/SKILL.md (R1-10, round-1 self-review body blocker): the defer disposition protocol keys on inline review-comment ids — code confirms the addressing model is inline-id-only for every disposition, pre-existing and unchanged by this diff; whether that limits the Defer-to-follow-up disposition is a scope call the code cannot answer, and no maintainer ruling exists on the PR.
Not explored to full depth (tool budget reached): "agent 4": none — all checks above ran to completion within budget..
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.
中文说明
未决，请确认：共 2 条（原文未翻译，列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 4"：none — all checks above ran to completion within budget.。
未检查（工具限制，非阻断）：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.12)
…ate, mktemp/cap guards (review round 5) - R5-1 (Critical): LD_PRELOAD/LD_AUDIT/LD_LIBRARY_PATH is the one channel env -i cannot block — ld.so maps a planted library into /usr/bin/env itself at execve, before -i wipes anything. Neutralize it with a command-prefix assignment (LD_PRELOAD= LD_AUDIT= LD_LIBRARY_PATH=) before /usr/bin/env at both upsert launch sites: a pure shell parameter assignment no BASH_FUNC can shadow, applied to env's own environment. Probe: a parent LD_PRELOAD=/evil.so no longer reaches the env binary or the child. - R5-3: shape gate rejects integer-valued floats jq renders in scientific notation past 2^53 (1e21 -> "1E+21") — the '+' is a regex-active byte in the anchored dedupe. Add a <2^53 bound and a tostring plain-digits belt. - R5-4: guard mktemp failure (a known /tmp-exhaustion CI state) so it warns and skips instead of a silent exit 0 that violates the header contract. - R5-2: the cap warning no longer promises 're-defer in a later round' (impossible — the eval-watermark filters evaluated feedback out permanently); it names the dropped bullets for a maintainer. - Tests: LD prefix, GH_HOST-in-child and gate/exec ordering pins; tie the two near-verbatim clean-child bodies together (R5-6); source pins for both --paginate sites (R5-5); if-!-echo gate-condition pin (R5-9); failure step added to the sweep-removal regression loop (R5-11); behavioral cases for sci-notation id, mktemp failure, prefix-colliding dedupe boundary (R5-10) and the reworded cap.
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.
Unresolved, please confirm:
- [Critical] R1-10 (.qwen/skills/autofix/SKILL.md, round-1 self-review body blocker): the defer disposition protocol keys on inline review-comment ids — code confirms the addressing model is inline-id-only for every disposition, pre-existing and unchanged by this diff; whether that limits the Defer-to-follow-up disposition is a scope call the code cannot answer, and no maintainer ruling exists on the PR.
Not explored to full depth (tool budget reached): "agent 1b": none — I completed all the checks I planned..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
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.
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 1b"：none — I completed all the checks I planned.。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：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.12)
…ess (review round 6) - R6-9 (Critical): the reason is agent-influenced prose published under the bot identity, so a raw @ fired real mentions from the tracking issue. Defuse before rendering: @ gets a trailing ZWSP, and the entity spellings GitHub decodes BEFORE its mention filter (@ @ @ @) get their & escaped. Both measured inert against the real renderer; \@ and leaving & alone are not. Paths were already charset-reduced. Byte-exact probe: 2/2 @ defused, all four entity spellings escaped, no raw spelling survives. - R6-8: LD_* cannot be enumerated — LD_TRACE_LOADED_OBJECTS is presence-tested, so even the empty prefix assignment leaves trace mode on and /usr/bin/env prints its libs and exits 0 without ever running the child (probed). Verify the RESULT instead: the child prints a liveness sentinel first and its absence is reported. The inspection uses bash builtins only — an external grep would itself print-and-exit-0 under trace mode, neutering the check (measured: the first grep-based attempt failed exactly this way). - R6-4: guard the child's own GH_CONFIG_DIR mktemp; an empty value falls back to the shared ~/.config/gh. - R6-6: a line-builder (jq/sed) failure warns instead of exiting silently as 'nothing new' — the last path skipping the header contract. - R6-3: write-failure warnings say the findings are LOST (watermark- gated, never retried) and name the bullets, matching the round-5 cap wording fix. - Tests: allow-list entry pins (R6-7), sentinel/builtin-inspection and child-mktemp pins, identity-check ordering (R6-5), plus behavioral cases for mention defusing, reason-type and id-0 gate clauses (R6-2) and line-builder failure.
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.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
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.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5。
未检查（工具限制，非阻断）：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.12)
…hem across repair (review round 7) - R7-1 (Critical): only inline comments carried an id in feedback.md, so a verified out-of-footprint finding raised in a review body or an issue-level PR comment could not be deferred and was lost at merge. Feedback now renders [rv:<id>] and [ic:<id>] alongside the existing [rc:<id>], the record takes an optional "source", and bullets anchor under a per-source prefix so id spaces cannot collide. The resolved-id exclusion stays inline-only (that is what resolved-comments.txt holds). SKILL documents all three sources and that only inline findings have a thread to reply on. - R7-2 (Critical): every abort path said "skipping ... this round", implying a retry that cannot happen (the eval watermark filters this round's feedback out of every later round and the next reset wipes the file). All six now report the findings as LOST and dump the raw deferrals for manual recovery, with :: neutralized — the dump is agent-influenced and a raw :: at line start is a workflow command. - R7-3 (Critical): 'Repair deterministic rejection' deleted deferred-findings.json before any upsert site ran, so run 1's deferrals died in a repaired round. It now carries them into a sidecar the upsert unions in (merging if an earlier repair left one), and the sidecar rides the artifact dump. A/B probe: base arm loses run 1's deferral, fix arm persists both. - R7-4: a present-but-non-string path (false) passed the gate because // treats false as absent; the gate now tests .path|type directly. - R7-5: LD_PROFILE/LD_PROFILE_OUTPUT/LD_DEBUG/LD_DEBUG_OUTPUT are non-blocking loader file-write channels the liveness sentinel cannot catch, so they join the command-prefix neutralization (probed inert when empty). - Tests: per-source anchoring and dedupe, unknown-source rejection, path:false, carry-only and carry-merge cases, LOST dump with :: neutralization, plus wiring pins for the carry, the feedback ids and the extended LD prefix.
…uses Clears the two backlog items the review has re-raised every round since round 2 (R2-6, R2-10); both live in the file this PR adds. - R2-6: the tracking-issue lookup ran a full --paginate over every issue the bot has ever opened, on every round that defers anything, keeping only the first match. It now walks newest-first pages and stops at the first marker match: one request in the common case, a short page ends the scan (corpus exhausted -> create), and a 10-page cap bounds the worst case. Reaching the cap without a match SKIPS rather than opening a second tracking issue for the same PR. - R2-10: every gh call discarded stderr, so a rate limit, an expired PAT, a transport error and a 404 rendered identically in the feature's only signal. All five calls now capture stderr to one sink and the warnings name the cause, :: neutralized like every other echoed API/agent content. Measured on the shipped script with a recording gh stub: first-page hit 1 request, empty corpus 1 request + create, page-2 hit 2 requests, cap 10 requests + skip, and the 403/401 bodies reaching the warning text.
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.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5 (rounds 4 and 5 each reported new findings).
Not reviewed: the executable-script lint — .github/scripts/upsert-deferred-issue.sh: shellcheck is not installed.
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.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5 (rounds 4 and 5 each reported new findings)。
未审查：the executable-script lint — .github/scripts/upsert-deferred-issue.sh: shellcheck is not installed。
未检查（工具限制，非阻断）：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.12)
Round 8 raised 15 findings, none Critical: two behavioural, the rest test pins the reviewer proved vacuous by mutation. - R8-3: the carry-merge failure branch discarded THIS run's deferrals with no raw dump — the one loss path in the feature without recovery output. It now prints the set (:: neutralized) before deleting it. - R8-1/2/5/6/7/8/9/10/11/12/13/14/15: pins that survived their own mutations. Notably: the negative sweep pins now assert the PROPERTY (no BASH_FUNC / unset -f / hash -r in non-comment lines) instead of round 3's exact spelling; allow-list entries must sit inside the env -i argument list, not merely somewhere in the step; the identity check is pinned whole so a fail-OPEN mutation cannot pass; the repair cleanup's deletion pin is spelling-independent and allows exactly the one delete that follows a merge; the staging pins are scoped to the stage step with cp ordered before the digest record; and the script's <!-- escape site gets the count+canonical treatment its workflow siblings already had. Each new pin was mutation-verified: 8/8 injected regressions turn the suite red (differently-spelled sweep, relocated allow-list entry, deleted GH_CONFIG_DIR export, fail-open identity check, re-added cleanup deletion, deleted re-print loops, ascending lookup order, no-op sed spelling).
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
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.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：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.12)
…e (review round 9)
- R9-1 (Critical): the digest gate was check-then-use — sha256sum read
the staged path and bash re-opened it, two opens of a path this PR
itself calls agent-writable. The child now reads the script ONCE and
runs those exact bytes (bash -c "$UPSERT_SRC"), so the bytes hashed
are the bytes executed. A/B against an inotify-driven same-user
rename(2) watcher: old shape 20/20 payload executions with the gate
never firing, new shape 0/20 (legit 20/20, control 20/20).
- R9-2 (Critical): unique_by([source, id]) collapsed DISTINCT findings
sharing one review-body or issue-comment id — the two sources this PR
adds — and reported success while losing them. Dedupe identity and the
corpus check are now per rendered line for those sources (inline
comments keep id identity and the cross-round anchor). Probe: two
findings under one review id now both persist ("3 of 3 new"),
byte-identical records still collapse, inline behaviour unchanged.
- R8-4 (re-raised): fixed structurally instead of by the suggested
prefix entry, which is a no-op — LD_SHOW_AUXV is presence-tested, so
an empty assignment still dumps 22 auxv lines (measured; env -u does
not help either). Loader side channels write to the LAUNCH process's
stdout, so that stdout is discarded and the child logs to a private
file; path and read-back are fork-free ($$ expansion, $(<file)) so a
polluted parent cannot leak noise into the value. Measured: with
LD_SHOW_AUXV planted the log holds 0 auxv lines and the child still
runs; with LD_TRACE planted the sentinel is absent and the warning
fires.
- R9-3/4/5: the manual-recovery dumps now say when they truncate, and
name the full byte count.
- R9-11: the artifact dump neutralizes :: in the agent-written files it
prints, like every other echo of them.
- Tests: pins for the single-read exec, the private log, the fork-free
parent handling, the identity check's ENFORCEMENT (R9-9), an
allow-list that must hold ONLY the sanctioned entries (R9-10), the
sentinel comparison inside the re-print loop (R9-13), and R9-2's
multi-finding cases. Also fixes an argList slice that anchored on a
comment mention and silently widened to the whole step.
… pin backlog Clears the eleven items carried from round 9. - R9-18: a carried sidecar that PARSES but fails the shape gate used to abort this round's valid deferrals too — asymmetric with the unparseable-carry branch, which persists this round only. The gate is now a function applied to the merged set, with a retry on this round's own file; the carry is dumped and named LOST. Measured on the real script: valid own + gate-invalid carry -> own persisted, carry dumped; valid own + unparseable carry -> own persisted; invalid own -> loud total abort, nothing written. - R9-20 / R9-7: the union argument order IS the freshness guarantee (jq unique_by keeps first-of-group in original order), pinned at both sites; measured: a duplicate id keeps this round's text, not the carried one. - R9-3/4/5 follow-up: the three truncation dumps became one dump_file helper instead of a fourth copy. - Pins: --paginate anchored to the comments call (R9-8), the stale two-sites comment corrected (R9-6), the no-in-shell-sweep property widened past function-unset spellings to alias/trap/proxy forms (R9-12), the repair cleanup's deletion pin extended to rm -rf and to any second multi-line list (R9-16), runUpsert's spawnSync bounded like its sibling harness (R9-17), the explicit review_comment spelling covered (R9-19), and the 20-item cap's survivor set pinned from a MEASURED run whose sort order and input order disagree (R9-14) — the four records written first are the ones dropped. Mutation-verified 6/6: relocating --paginate, an alias-form sweep, an rm -rf deletion, either union order swapped, and dropping the poisoned-carry fallback all turn the suite red.
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Not explored to full depth (tool budget reached): chunk 1: None. I was able to read all files in full and verify all relevant code paths..
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.
中文说明
已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
未探索到全部深度（达到工具调用预算）：chunk 1：None. I was able to read all files in full and verify all relevant code paths.。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— deepseek-v4-flash via Qwen Code /review (v0.21.11)
commented
Aug 16, 2026
|
@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: 130 passed · 0 failed · 130 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：130 通过 · 0 失败 · 130 总计 Verification reportPR #9189 Deep Verification — feat(autofix): defer verified out-of-footprint findings to a surviving follow-up queue (follow-up round)Verdict: 中文摘要
Previous-finding status (follow-up round)The previous report verified round-12 head
Central claim and A/B proof (re-measured at the new head)Central claim: a finding the agent verified as real but whose fix lies outside the PR footprint is recorded in Secondary claims: (1) rounds 13–14's new guards (single-document gate, BSD-wc strip, array-only carry union, stage-absence tolerance, A/B cells (harness
|
| cell | oracle | result |
|---|---|---|
| head append | recorded gh calls + exit code | exit 0; one POST issues/42/comments; body has - rc:7 and - rv:21; dups - rc:8/- rc:9 suppressed; third-party mention did not suppress; zero creates; no PATCH; honest (2 of 2 new) — 11/11 |
| head create (H2) | no marker issue, short page ⇒ corpus exhausted | exactly one POST issues with marker + bullet + per-PR title — 6/6 |
| base (expected red) | script absence | script file absent at HEAD^1; zero deferred-findings/run_deferred_upsert references in base workflow; no Defer disposition in base SKILL — 3/3 (control fails as predicted ⇒ assertion passes) |
Witness: evidence/01-ab-load-bearing-head-vs-base.png.
Merge integrity: every harness and gate ran in a worktree at the merge commit, so the numbers already describe what lands. The base-side evolution carried into the merge (184 files since the PR's merge-base) touches two of the PR's four files — qwen-autofix.yml (409 changed lines base-side) and the workflow test file (985 changed lines base-side) — and GitHub auto-merged both; the union is validated behaviorally: the merged test file passes 174/174 (base-side pins and PR-side pins together) and all extraction harnesses parsed/executed the merged workflow. The script and SKILL.md had no base-side evolution.
Delta cells: rounds 13–14 guards vs the round-12 control (harness 02-delta-probes.mjs, 32/32)
Control = the exact script the previous round certified (git show 8cd6b0522c:.github/scripts/upsert-deferred-issue.sh), driven through the same stub.
| cell | round-12 control (the bug) | new head |
|---|---|---|
MD1 [valid]\n[] |
silent exit 0, zero output — findings evaporate | loud not a single JSON document + raw dump, zero writes |
MD2 [bad-id]\n[valid] |
bad-id bullet smuggled into the POST body (- rc:x ) |
rejected loudly before any write |
| MD3 multi-doc carry | n/a | carry costs only the carry: dumped, named, this round still publishes (1 of 1) |
WC1 BSD-padded wc stub |
( 1 of 1 new) interpolated into the user-visible line |
(1 of 1 new) clean |
| WC2 cap path under padded wc | n/a | persisting 20 of 25 clean; the 5 dropped bullets named |
RA1R4-B directory/FIFO at resolved-comments.txt |
n/a | treated as unusable, finding published, no hang |
| R13-2 non-array carry | n/a | rejected at the union (both inputs must BE arrays), this round persists |
| IDEM repeat upsert | n/a | exit 0, zero write calls, no LOST claim (re-measured from previous round) |
| R13-7 stage step | n/a | absent script ⇒ exit 0 under set -e + empty heredoc value; present script ⇒ round-trip byte-identical (minus heredoc trailing-NL semantics, irrelevant to bash -c); random delimiter differs across runs |
Witness: evidence/02-delta-multidoc-wc-round12-vs-head.png.
Round-14 __upsert_trusted__ carve-out — the new attack surface (harness 03-wrapper-probes.mjs, 26/26)
The wrappers were extracted verbatim from the parsed YAML and executed for real.
- T1 forgery (5/5): the agent fully controls
deferred-findings.jsonbytes; a malformed own file whose raw lines are__upsert_trusted__::error::forged-by-agentand::error::raw-workflow-commandreaches stdout only as;;error;;…— zero lines starting with a live::workflow command, zero write calls, wrapper exit 0. The carve-out is forgery-proof because every agent-content output path in the script neutralizes::before emission (dump_filepipes throughsed 's/::/;;/g'; constructed bullets always start- rc:/- rv:/- ic:), so a marker-prefixed line can never still carry live::s. - T2 annotation survival: forcing the child's
mktemp -dto fail, head emits a real::warning::could not create a gh config dir…annotation; the round-12 wrapper control demoted the same warning to;;warning;;…— the round-14 fix proven load-bearing at the wrapper level. - T4 identity check (failure-path twin): wrong actor ⇒ live annotation naming the actor, zero writes; bot actor ⇒ persisted.
- W1 hostile env (6/6): parent poisoned with a hijack
gh/bashon PATH,BASH_FUNC_bash%%/BASH_FUNC_env%%,SHELLOPTS=noclobber,LD_AUDIT/LD_DEBUG, and a real gcc-built evil .so onLD_PRELOADwhose constructor records every process that loads it: findings persisted (1 of 1), the .so loaded only in the parent pid — no child exec, no hijack log, no function-hijack output, sentinel consumed, exit 0. - W2/W4/U1: script-emitted
::error::neutralized; emptyUPSERT_SRCskips plainly; fresh text wins the carry union.
Witness: evidence/03-wrapper-marker-forgery-probes.png.
Mutation matrix — the new pins are load-bearing (6/6 killed, control green)
Each mutant disables exactly one guard in a scratch worktree; the suite (narrowed to the workflow test) must go red on the intended assertion. Adjudication scripted per mutant log (mutations/*.log).
| mutant | guard removed | killed by |
|---|---|---|
| M0 (positive control) | set +C line |
toContain('set +C') pin |
| M1 | | tr -d ' ' on the wc -l count |
behavioral: expected (1 of 1 new), got ( 1 of 1 new) under the padding-wc stub |
| M2 | __upsert_trusted__ branch (→ __upsert_never__) |
loop-structure regex pin |
| M3 | the own-file single_doc gate |
behavioral: multi-doc case no longer warns not a single JSON document |
| M4 | stage step's 2> /dev/null || true |
tolerance pin |
| M5 | array-only union (→ plain jq -s 'add') |
union regex pin |
Control: unmutated suite 174/174 (an initial exit-1 with all tests green was a vitest onTaskUpdate RPC timeout under runner load — re-run exit 0, tests 174/174 both times; adjudged environmental). Witness: evidence/04-mutation-matrix-six-kills.png.
jq semantics across versions (harness 05-jq-semantics.mjs, 20/20)
The carry-union filter changed in round 13, so the closure was re-measured on the container's jq 1.6 and a downloaded jq 1.7.1: unique_by first-of-group holds both ways in both versions; the round-13 union gate merges two arrays and rejects array+object and non-array pairs loudly (exit 4, no output); single_doc counts 1/2/junk correctly; the 2^53 and scientific-notation id rejections hold. The freshness guarantee is not jq-version-dependent.
Targeted gates (harness 06-gates.mjs, 25/25)
- PR suite at head (merged tree): 174/174; at base: 173/173 — the PR adds exactly one test, attribution clean. Witness:
evidence/05-suite-head-174-green.png. (The PR body's "173" predates the added test.) npm run test:scriptsat head: 1067/1067 tests, 53/54 files; the 54th file (install-script.test.js) fails collection withoutzip— A/A-proven environmental (identical failure at base; the PR does not touch the file).- shellcheck (pinned 0.11.0): PR script 0 errors, 9 style warnings (env-contract + style, see Corrections); liveness-proven (planted SC2034/SC2086 caught, exit 1).
- actionlint (pinned 1.7.12): 0 findings across all workflows; liveness-proven (planted duplicate key caught, exit 1).
- ESLint scoped to the changed test file: clean; liveness-proven at the parse-error level (the resolved config sets
no-unused-varsoff for this path). bash -n: the new script and all 58 workflow run blocks parse.
Corrections
- The previous report's "shellcheck/actionlint 对 PR 文件零告警" does not reproduce for shellcheck at the new head: the wrapper runs
--enable=all --severity=style, and the PR script carries 9 style-level warnings (4× SC2154 — the documented caller-provided env contractWORKDIR/PR/REPO/AUTOFIX_BOT; 5× SC2312 — "consider invoking separately" on command substitutions). 0 errors. Related pre-existing tooling note: the wrapper's shellcheck pipeline ends in| sed, which masks shellcheck's exit status in POSIX sh — measured: a planted violation exits 0 through the wrapper pipeline but 1 via the binary directly. This gate is therefore cited by findings count, liveness-proven (planted SC2034/SC2086 caught, exit 1). Neither item is a PR defect; both are corrections to how the gate is read. - The previous report's "depth-2 浅克隆仅三个提交可达" is refined:
rev-listtraversal is graft-limited (returns the known false-small 1), but the PR's full 19-commit chain is object-present (the cat-file parent walk reaches a 3960-commit chain). The three commits added since the previous verified head were each diffed and their claims individually driven; earlier commits rely on the previous round's measurements plus this round's full re-measurement of the final state.
Findings
1. Nit (new) — one wrapper-authored warning still demoted: the round-14 invariant covers two of its three messages. Round 14's stated goal is "wrapper warnings stay annotations": wrapper-authored lines get a __upsert_trusted__ marker and are emitted verbatim. The mktemp-dir-failure and identity-failure warnings were marked; the third wrapper-authored message was not — inside both clean children, bash -c "${UPSERT_SRC}" || echo "::warning::deferred-findings upsert failed; continuing" fires when the script exits non-zero, and because it carries no marker the re-emit loop neutralizes it.
Measured (probe T3): stdout shows ;;warning;;deferred-findings upsert failed; continuing — the feature's own failure signal, demoted from annotation to plain text (still visible in the log; no correctness or security impact — the string is fixed wrapper text with no interpolation).
Repro: run run_deferred_upsert (verbatim extraction) with UPSERT_SRC='exit 3' and inspect stdout.
Suggested fix (measured)
Prefix the marker at both sites: echo "__upsert_trusted__::warning::deferred-findings upsert failed; continuing" (in run_deferred_upsert and the failure-step twin). Measured in a scratch copy: the line then reaches stdout as a live ::warning::deferred-findings upsert failed; continuing annotation, wrapper exit unchanged (0), and no forgery surface is added (the line interpolates nothing). The suite pins the loop structure, not this echo's string (grep: zero matches), so no pin changes.
2. Info (carried, stands) — repair-step carry-first precedence. Re-measured unchanged: jq -s 'add' "${WORKDIR}/deferred-findings.carry.json" … "${WORKDIR}/deferred-findings.json" puts the older generation first, the opposite of the script's this-round-first freshness semantics — duplicates between two carried generations would keep the stale text inside the sidecar. Unreachable in today's topology per the code's own comment, and the script-side union lets the fresh run win over the sidecar anyway. No action required for this PR.
No injection attempts were observed in the PR text.
Not covered
- yamllint gate:
pip3: Permission deniedin this container (same as the previous round). Compensated by three mechanisms: every harness parses the full 7000+-line workflow with js-yaml, actionlint 1.7.12 validates structure (clean, liveness-proven), andbash -naccepts all 58 run blocks plus the new script. - Real-API replay calibration: no credentials in the sandbox; the gh-stub harnesses remain uncalibrated replays of the wire shape — they prove the script's handling of the documented API semantics (creator filter, body+comments corpus, comment-append, pagination cap), not that GitHub responds exactly as stubbed.
previous-report.mdis an output of the verify lane, not of the workflow under test, so it does not calibrate the wire. - Per-commit verification of the 16 pre-round-12 commits: carried from the previous round's measurements; the final state they produce was fully re-measured this round. The 3 newer commits were verified individually (delta diffs + targeted cells).
- Trial merge into a newer main: the snapshot's
baseRefOid(c396fe3d…, object-present) has drifted from the merge-ref base (cd54a50d25); no token to query live main. Substituted by running everything on the merge commit itself (the current-main union at run time). - Full Actions runtime execution (
if:evaluation,failure()/cancelled()semantics, GITHUB_OUTPUT heredoc parsing by the real runner): verified by verbatim extraction + emulation and the suite's structural pins, not an Actions run. The heredoc round-trip emulates runner parsing (lines joined with\n). - RA1R4-B's
-rclause: root bypasses permission bits (-ris always true for root), so an unreadable-file corpus cannot be driven in this container; the directory and FIFO shapes were driven (both treated as unusable, no hang). - Repo-wide ESLint / Prettier: ESLint scoped to the changed test file (clean; liveness proven at the parse-error level — note the resolved config sets
no-unused-varsoff for this path). The no-arg lint wrapper also runsprettier --write ., which was deliberately not run. install-script.test.js(the 54th file oftest:scripts): collection throws withoutzip, absent here. A/A-proven environmental: byte-identical failure at base, file untouched by the PR. The PR body's "1164 passed / 13 skipped" presumeszippresent; measured here 1067 passed with that file's tests uncollected.- Working-tree drift in the verification checkout (environmental, carried from the previous round): the main checkout's
.qwen/skills/autofix/SKILL.mdworking blob still equals the HEAD^1 blob (the PR's hunk reverted). Left untouched as user-owned; every run used clean worktrees. The PR's suite fails against that drift by design, which is why gates ran in worktrees.
Methodology
Environment: node:22-bookworm container (Node v22.23.2, jq 1.6 + downloaded jq 1.7.1, gcc present; zip and yamllint absent), npm ci + npm run build pre-run by the workflow. Verified head c05e79963a checked out as merge commit 00b3528596; scratch worktrees under tmp/: pr9189-head (HEAD), pr9189-base (HEAD^1 — control; PR leaves package.json/lockfile untouched, so reusing root node_modules is a clean control), pr9189-mut (mutation scratch). Harnesses 01–06 (.mjs, in this directory) drive the real artifacts: the actual .github/scripts/upsert-deferred-issue.sh and verbatim-extracted workflow blocks (js-yaml parse of the merged qwen-autofix.yml) executed under bash against recording gh stubs with file-based config (survives env -i); the hostile-env probes planted PATH/BASH_FUNC/SHELLOPTS/LD_* channels and a gcc-built constructor-instrumented .so. Mutation matrix: 6 single-guard mutations, suite narrowed to the workflow test, kill = suite red with an AssertionError in the target test (scripted adjudication of per-mutant logs), files restored and verified after each. Gates: vitest via --config ./scripts/tests/vitest.config.ts; linters via node scripts/lint.js --setup pinned binaries (shellcheck 0.11.0, actionlint 1.7.12), liveness-proven with planted violations; shellcheck cited by findings count because the wrapper's trailing sed masks its exit status. Three harness-side parsing bugs (heredoc regex vs empty content, ANSI codes in tee'd logs, one wrong expected string) were caught by the harnesses themselves during the round and fixed; all reported numbers come from the final runs. Raw logs (logs/*.log, mutations/*.log) and result JSONs (results-0*.json) live alongside this report; evidence captures via scripts/verify-capture.mjs.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: reverse audit — reached the round cap of 5 without two consecutive dry rounds (rounds 1-5 each reported new findings).
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally.
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.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：reverse audit — reached the round cap of 5 without two consecutive dry rounds (rounds 1-5 each reported new findings)。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally。
未检查（工具限制，非阻断）：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.12)
…dence (review round 15) Zero Criticals this round; five behavioural findings among the pins. - The rv/ic intra-batch identity was derived from the RENDERED line, i.e. after the 500-char reason cap, so two siblings differing only past the cap collided and one vanished silently — the same silent-loss class as the CJK and long-path entrances. It now comes from the uncapped path+reason; the corpus check still compares rendered forms (that is all the issue stores), so cross-round the cap can cost a duplicate, never a loss. - The repair carry union put the OLDER set first, inverting the newer-wins precedence the script documents for its own union. - The resolved-id parser dropped any line with stray surrounding whitespace, so a padded `rc:<id>` no longer suppressed its finding. - The clean child's catch-all warning lacked the trusted marker added last round, so the feature's most common failure message was still demoted out of annotation form. - The truncation notice pointed at the artifact dump even when the dumped file is a merge temp outside WORKDIR, which is never uploaded. Pins: the stage step's `id: 'stage'` (the link whose break empties every UPSERT_SRC), the empty-content skip branch, the capture's `|| true`, and the trusted marker on every warning inside the child. Mutation-verified 6/6.
left a comment
There was a problem hiding this comment.
LGTM — approving per maintainer request. Note at 2258cfb: review-pr check still in progress; the two cancelled 'route' jobs are duplicate review-event burst runs (scheduling noise, same pattern as prior bursts), not code failures.
commented
Aug 16, 2026
|
@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: 96 passed · 0 failed · 96 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：96 通过 · 0 失败 · 96 总计 Verification reportPR #9189 Deep Verification — feat(autofix): defer verified out-of-footprint findings to a surviving follow-up queue (follow-up round 2)Verdict: 中文摘要
Previous-finding status (follow-up round)The previous report verified round-14 head
Central claim and A/B proof (re-measured at the new head)Central claim: a finding the agent verified as real but whose fix lies outside the PR footprint is recorded in Secondary claims: (1) round 15's new guards (uncapped A/B cells (harness
|
| cell | oracle | result |
|---|---|---|
| head append | recorded gh calls + exit code | exit 0; one POST issues/42/comments; body has - rc:7 and - rv:21; dups - rc:8/- rc:9 suppressed; third-party mention did not suppress; zero creates; no PATCH; honest (2 of 2 new) — 11/11 |
| head create (H2) | no marker issue, short page ⇒ corpus exhausted | exactly one POST issues with marker + bullets + per-PR title — 7/7 |
| base (expected red) | mechanism absence | script absent at base; zero deferred-findings/run_deferred_upsert refs in base workflow; no Defer disposition in base SKILL — 3/3 (control fails as predicted ⇒ assertion passes) |
Witness: evidence/01-ab-central-head-vs-base.png.
Merge integrity (harness 00-merge-integrity.sh, 8/8, witness evidence/00-merge-integrity-eight-checks.png): round 15 sits directly on the previously verified head; script and SKILL.md land verbatim in the merge; the two union files (workflow + test) byte-match a clean 3-way auto-merge reconstruction (no conflict resolution); the base evolution since the previous round's base tip (cd54a50d25 → 3186d4ea67, the #9212 commits) touches zero of the PR's four files. The inherited worktree drift (main checkout's SKILL.md equals the HEAD^1 blob) was left untouched as user-owned; every run used clean worktrees (assertion I7 documents it).
Delta cells: round-15 behaviors vs the round-14 control (harness 02-delta-r14.mjs, 19/19)
Control = the exact script the previous round certified (git show c05e79963a:.github/scripts/upsert-deferred-issue.sh), driven through the same stub. Witness: evidence/02-delta-round14-vs-round15-cells.png.
| cell | round-14 control | head |
|---|---|---|
| SIB1 two rv siblings differing only past the 500-char reason cap | one silently vanishes (1 of 1) | both persist (2 of 2) |
| SIB2 siblings differing within the cap | 2 of 2 | 2 of 2 (no over-split) |
| SIB3 identical siblings | 1 of 1 | 1 of 1 (dedupe intact) |
WS1 resolved id rc:7 (stray whitespace) |
re-published the resolved finding (write) | suppressed, zero writes |
WS2 resolved id rc:7\r\n (CRLF regression control) |
suppressed | suppressed |
WS3 rc: 7 (internal space) |
publishes | publishes (semantics unchanged) |
| CAP1 cross-round corpus: past-cap sibling of an already-tracked line | n/a | suppressed (see Findings #3) |
| CAP2 genuinely different reason vs same corpus | n/a | publishes (append) |
DUMP1 >4000-byte merge temp dumped in lost() |
claims "the full file is in this run's artifact dump" | honest: "temporary merge product and is NOT in the artifact dump"; WORKDIR carry still claims in-dump |
Previous findings re-driven (harness 03-prev-findings.mjs, 21/21)
F1a–F1d (marker fix, both wrapper sites, both arms) and F2a/F2b (repair precedence, both arms) as in the status table above; plus regressions of the round-14 guarantees: T1 forgery (agent-controlled bytes incl. embedded __upsert_trusted__::error::… reach stdout with zero live :: lines, finding still persisted), T2 mktemp-dir failure (real annotation survives live, zero writes), T4 identity check on the failure twin (wrong actor ⇒ live annotation naming it, zero writes), IDEM repeat upsert (exit 0, zero writes, no LOST). Witness: evidence/03-previous-findings-fixed.png.
Mutation matrix — 7/8 killed on the intended assertion (harnesses 04-mutations.mjs / 04b)
Each mutant disables exactly one guard in the head worktree; the narrowed suite must go red on the intended assertion. Adjudication scripted per mutant log (mutations/*.log). Witness: evidence/04-mutation-matrix-seven-kills-one-gap.png.
| mutant | guard removed | killed by |
|---|---|---|
| M0 (positive control) | set +C line |
toContain('set +C') pin |
| M1 | fullkey identity → capped .key (round-14 form) |
behavioral: expected (2 of 2 new), got 1 of 1 |
| M2 | resolved-id whitespace trim (round-14 parsing) | behavioral: padded resolved id re-published (-f title= appeared) |
| M3 | repair union order (carry-first) | updated regex pin |
| M4 | __upsert_trusted__ on the fallback warning (both sites) |
R14-1 marker loop |
| M5 | dump_file message split | SURVIVED — suite green both runs (the exit-1 in one log is the environmental vitest onTaskUpdate RPC signature with 0 red tests, same as documented by the previous round). Adjudicated a coverage gap, see Findings #2 |
| M6 | wrapper empty-content skip | new regex pin |
| M7 | id: 'stage' on the stage step |
new pin |
Post-matrix unmutated control: green. The three round-15-new pins that lock pre-existing guards (empty-content skip, id: 'stage', trailing \|\| true) each kill their mutant — the pins are live, not vacuous.
Targeted gates (harness 05-gates.sh + suite runs, 13/13)
- PR suite at head (merged tree): 174/174; at base: 173/173 — the PR adds exactly one test; round 15 added assertions inside existing tests. (The PR body's "173" predates the added test, as in the previous round.)
npm run test:scriptsat head: 1067 passed, 53/54 files; at base: 1066 passed with the same single-file failure — A/A-proven environmental:install-script.test.jsthrows at collection withoutzip, absent in this container, file untouched by the PR. The head run's lone "error" is the environmental vitest RPC signature with 0 failing tests.- shellcheck (pinned 0.11.0, repo flags
--enable=all --severity=style): PR script 0 error-severity lines, 9 style-level lines (4× SC2154 documented caller-env contract, 5× SC2312) — the same set as the previous round, cited by findings count because the wrapper's trailingsedmasks its exit status; liveness-proven (planted SC2034/SC2086 caught, exit 1). - actionlint (pinned 1.7.12): 0 findings on the merged workflow; liveness-proven (planted duplicate key caught).
bash -n: the PR script and all 58 workflow run blocks parse.- ESLint scoped to the changed test file: clean; liveness-proven at the parse-error level.
Witness: evidence/05-lint-gates-live-proven.png.
Findings
1. Suggestion (new, measured) — the new fullkey identity reintroduces silent sibling loss at a thinner corner: path/reason boundary collision. Round 15's fix derives intra-batch identity from the uncapped raw = path + " " + reason. Concatenation with a space separator means a space-shifted boundary yields an identical key: {path: "docs ui", reason: "x finding"} and {path: "docs", reason: "ui x finding"} both normkey to docs ui x finding, so unique_by merges them and one finding is silently dropped while the round reports success. Measured (SIB4): head publishes 1 of 2; the round-14 control published 2 of 2 for the same input (its rendered lines differ). This is the exact "silent loss, the one outcome this feature exists to prevent" failure mode the commit targets, surviving in a corner that requires two findings under one review/issue-comment id whose path/reason boundary aligns after a space — thin, non-blocking, but real. (review_comment ids are unaffected: id-only identity.) Witness: evidence/06-fullkey-boundary-corner.png.
Suggested fix (measured)
Key on the separate normalized fields instead of the concatenation (bindings added where .path/.reason are still in scope):
- | map(.id as $id
+ | map(.id as $id
+ | ((.path // "?") | normkey) as $pkey
+ | (.reason | normkey) as $rkey
...
- | . + {key: (.line | normkey), fullkey: (.raw | normkey)})
+ | . + {key: (.line | normkey), fullkey: (.raw | normkey), pkey: $pkey, rkey: $rkey})
...
- | unique_by(if .src == "review_comment" then [.src, .id] else [.src, .id, .fullkey] end)
+ | unique_by(if .src == "review_comment" then [.src, .id] else [.src, .id, .pkey, .rkey] end)Measured in a scratch copy against the same five cells: SIB1 past-cap siblings still 2 of 2 (the round-15 fix preserved), SIB2 distinct 2 of 2, SIB3 identical 1 of 1 (dedupe intact), SIB4 boundary collision now 2 of 2, SIB5 punctuation-variant paths still merge (documented tolerance). The suite is green with and without the patch — it pins nothing along this axis, so the fix should ship with its fixture: a runUpsert case with the SIB4 input asserting (2 of 2 new) (the suite's existing beyondCap case is the template).
2. Info (completeness, not a merge condition) — the dump_file message split (M5) is unpinned. The mutant that reverts the round-15 "temporary merge product" wording survived with the suite fully green twice: nothing asserts which message a truncated dump carries. Classified a coverage gap, not dead code — the branch is reachable and harness 02's DUMP1 cell proves the head behavior correct (and the round-14 message wrong). A one-line pin on the LOST-path output would close it.
3. Info (carried-into-new-text wording) — the new fullkey comment's "cross-round the cap can cost a duplicate, never a loss" is not literally true in a constructed corner. Measured (CAP1): when the corpus already stores a rendered line, a finding whose reason differs only past the 500-char cap renders identically and is suppressed — the tail bytes are never persisted. Bounded in practice: the scenario requires the same source id to defer again in a later round, which the eval watermark filters (ids are one-round citizens, repair re-runs included — the union handles those intra-round). The corpus can only ever store rendered capped forms, so this is inherent to the storage, not a defect introduced by round 15; the comment's "never a loss" could say "never a visible loss" without changing behavior.
No injection attempts were observed in the PR text.
Not covered
- yamllint gate:
pip3: Permission deniedin this container (same as both previous rounds). Compensated by three mechanisms: every harness parses the full 7000+-line workflow with js-yaml, actionlint 1.7.12 validates structure (clean, liveness-proven), andbash -naccepts all 58 run blocks plus the new script. - Real-API replay calibration: no credentials in the sandbox; the gh-stub harnesses remain uncalibrated replays of the wire shape — they prove the script's handling of the documented API semantics (creator filter, body+comments corpus, comment-append, short-page exhaustion), not that GitHub responds exactly as stubbed.
previous-report.mdis an output of the verify lane, not of the workflow under test, so it does not calibrate the wire. - Per-commit verification of the 19 pre-round-15 commits: depth-2 graft-limited checkout (rev-list returns the known false-small 1; the chain is object-present). Round 15's single commit was verified individually (delta cells + both previous findings re-driven); earlier commits rely on the previous rounds' measurements plus this round's full re-measurement of the final state (every behavior above was re-driven at the new head, not diffed from old reports).
- Full Actions runtime execution (
if:evaluation,failure()/cancelled()semantics, heredoc parsing by the real runner): verified by verbatim extraction + emulation and the suite's structural pins, not an Actions run. - RA1R4-B's
-rclause: root bypasses permission bits, so an unreadable-file corpus cannot be driven here (carried). Closure for carrying: the-f/-rguard text is byte-identical between round 14 and 15 (the delta's script hunks touch only the dump message, the resolved-id parsing, and the identity-key region), and the directory/FIFO shapes were driven in the previous round. - Hostile parent-environment battery (PATH hijack, BASH_FUNC, SHELLOPTS, LD_*, gcc-built evil .so): carried from the previous round with closure — the push wrapper and the failure block each differ from round 14 by exactly one line, the marker line (assertions W1/W2), so the env -i isolation mechanism is byte-identical; this round re-drove the agent-content channels that DID change (T1 forgery, T2 mktemp failure, T4 identity).
- Trial merge into a newer main: no token to query live main. Substituted by merge-integrity assertions I4–I6 (clean auto-merge reconstruction + zero base-side overlap with the PR files since the previous base tip).
- Repo-wide ESLint / Prettier: ESLint scoped to the changed test file only (clean, liveness-proven). The no-arg lint wrapper also runs
prettier --write ., which was deliberately not run.
Methodology
Environment: node:22-bookworm container (Node v22.23.2, jq 1.6, gcc present; zip and yamllint absent), npm ci + npm run build pre-run by the workflow. Verified head 2258cfbf8d checked out as merge commit c16d7ba0a8; scratch worktrees under tmp/: pr9189-head (merge commit), pr9189-base (HEAD^1 — control; PR touches no package files, and the base worktree's deps resolve via realpath into the root node_modules (assertion I8), so it is a clean control). The main checkout's inherited SKILL.md drift (equals the HEAD^1 blob) was left untouched; nothing ran from that tree. Harnesses 00–06 (in this directory; results-07.json records the suite/ESLint gate runs) drive the real artifacts: the actual upsert-deferred-issue.sh and verbatim-extracted workflow blocks (js-yaml parse of the merged qwen-autofix.yml) executed under bash against a self-locating recording gh stub (fixtures + log beside the binary, so it survives the wrapper's env -i child). Round-14 controls were extracted from git show c05e79963a. Mutation matrix: 8 single-guard mutations in the head worktree, suite narrowed to the workflow test, kill = suite red with the intended AssertionError (scripted pattern adjudication of per-mutant logs), files restored and verified clean after each. Gates: vitest via --config ./scripts/tests/vitest.config.ts; linters via node scripts/lint.js --setup pinned binaries, liveness-proven with planted violations. Three harness-side bugs (dropped $a in the stub logger, a field-extraction prefix bug, an end-marker off-by-newline) were caught by the harnesses' own impossible outputs during the round and fixed; all reported numbers come from the final runs. Raw logs (logs/*.log, mutations/*.log), result JSONs (results-0*.json) and controls live alongside this report; evidence captures via scripts/verify-capture.mjs.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
commented
Aug 16, 2026
|
Merged as Where it landed. 17 review rounds, 216 findings, 42 of them Critical, across 21 commits. Every Critical was addressed; the loop ended at zero unresolved Criticals with a bot approval and approvals from @yiliang114 and @DragonnZhang. What the rounds were actually about. The feature itself is small — a fourth SKILL disposition plus the script that persists it. Most of the review volume came from where it runs: a PAT-bearing step on a shared runner, where every added line is examined at maximum depth. Rounds 9–12 illustrate the trap: each hardening answer (staged copy → digest gate → clean child → LD prefix → liveness sentinel → log file) put new code on the privileged path, and the next round's Criticals were holes in that code rather than in the feature. Six consecutive Criticals were defects I had introduced one round earlier. What broke the loop. Round 13 stopped patching and removed the two agent-writable paths the machinery existed to defend: the script's content now travels in expression context (no staged copy, so nothing to verify — the digest gate, its check-then-use window and the FIFO/unbounded-read class all disappeared), and the child's output travels on fd 3 with fd 1/2 discarded (no log file to plant, race or bound). Net −113 lines, and nine findings from earlier rounds dissolved because their subject no longer existed. New Criticals per round went 6 → 2 → 4 → 1 → 1 → 0 after that. The findings that justified the effort. A TOCTOU on the digest gate (measured: 20/20 payload executions against an inotify-driven same-user watcher, 0/20 after the fix); three separate silent-loss entrances in the deferral identity (CJK text erased by an ASCII-only key, long paths pushing the reason out of a truncated key, and siblings differing only past the 500-char cap); two What is not done. 58 review threads remain open by deliberate batch discipline — assertion strength and comment accuracy, no correctness risk — plus one cross-workflow effect that cannot be fixed here: That last part is the feature's own doctrine applied to itself: verified, out of footprint, so it goes to a follow-up queue rather than being implemented here or quietly dropped. 中文说明已合入 结果。 17 轮评审、216 条发现（其中 42 条 Critical）、21 次提交。所有 Critical 均已处理；最终以零未决 Critical 收口，获得 bot 批准以及 @yiliang114、@DragonnZhang 的批准。 这些轮次到底在审什么。 特性本身很小——SKILL 的第四种处置，加上持久化它的脚本。评审量主要来自它运行的位置：共享 runner 上带 PAT 的步骤，每加一行都会被按最高标准审查。第 9–12 轮是典型陷阱：每个加固方案（暂存副本 → 摘要门 → 干净子进程 → LD 前缀 → 存活哨兵 → 日志文件）都在特权路径上放了新代码，于是下一轮的 Critical 落在那些代码上而不是特性上。连续六个 Critical 是我上一轮自己引入的缺陷。 循环是怎么断的。 第 13 轮停止打补丁，改为删掉这套机械本来要守护的两条 agent 可写路径：脚本内容改走表达式上下文（没有暂存副本，也就没有要校验的东西——摘要门、check-then-use 窗口、FIFO/无界读整类一并消失），子进程输出改走 fd 3、fd 1/2 丢弃（没有日志路径可植入、可竞态、需限长）。净删 113 行，此前九条发现因对象不复存在而直接消解。此后每轮新 Critical：6 → 2 → 4 → 1 → 1 → 0。 值回票价的那些发现。 摘要门的 TOCTOU（实测：面对 inotify 驱动的同用户 watcher，旧写法 20/20 执行 payload，修复后 0/20）；deferral 身份键上三个各自独立的静默丢失入口（纯 ASCII 键抹掉中文、长路径把 reason 挤出截断窗口、兄弟条目仅在 500 字符之后不同）；两条 没做完的部分。 58 个评审线程出于批量纪律仍然开着——断言强度与注释准确性，无正确性风险——外加一条无法在此修复的跨工作流影响： 最后这一步正是把本特性自己的教义用在了它自己身上：已验证、超出足迹，所以进 follow-up 队列，而不是在这里实现或悄悄丢掉。 |
left a comment
There was a problem hiding this comment.
中文说明
— qwen3.8-max via Qwen Code /review (v0.21.11)
| ascii_downcase | gsub("[[:punct:]]+"; " ") | gsub("\\s+"; " ") | ||
| | sub("^ "; "") | sub(" $"; ""); | ||
| ($resolved | split("\n") | ||
| | map(sub("^\\s+"; "") | sub("\\s+$"; "") | sub("^rc:"; "") |
There was a problem hiding this comment.
[Critical] The whitespace tolerance this round adds to the upsert's parse of resolved-comments.txt is not shared by the file's three other consumers, breaking the cross-consumer parity the feature documents. Probe over the verbatim-extracted parsers at this commit: upsert [padded] => [7] (tolerant), BITE_ENFORCE [padded] => [], TESTSIDE [padded] => [], resolve loop [padded] => SKIPPED, reply block [padded] => NO MATCH, pre-diff upsert [padded] => []. A padded resolved-claim therefore suppresses publication in the upsert while the thread is never resolved on GitHub and BITE_ENFORCE never sees the claim — the validation gate that earns suppression of a Critical claim never fires. The parity invariant is documented at run-autofix-review-verification.sh:866-868 ("Ids tolerate the rc: prefix and CR the other consumers strip"). — Failure scenario: resolved-comments.txt is agent-written, and stray whitespace is the exact shape R14-14 was added to absorb. Run 1 defers comment 7 → repair → run 2 claims it fixed 7 with rc:7 while run 1's carry still contains id 7: the upsert counts 7 as resolved and silently drops the deferred rc:7 finding from the batch — watermark-gated, so no later round re-derives it: permanent silent loss, while the thread stays open and the enforcement gate never saw the claim.
Witness (probe over verbatim-extracted parsers at this commit):
upsert [padded] => [7] BITE [padded] => [] TESTSIDE [padded] => []
resolve [padded] => SKIPPED reply [padded] => NO MATCH prediff upsert [padded] => []
BITE with the suggested strip [padded] => [7] (flip)
Suggested fix: normalize once for all consumers — apply the same sub("^\\s+"; "") | sub("\\s+$"; "") before the ^rc: strip in both BITE_ENFORCE and TESTSIDE jq programs and in the workflow resolve loop's rc_id cleanup; or justify in a comment why the upsert alone should be tolerant.
中文说明
问题： 本轮为 upsert 解析 resolved-comments.txt 新增的空白容忍没有同步到该文件的另外三个消费方，打破了功能自身文档化的跨消费方一致性。对逐字提取的四个解析器在本提交上探针：upsert [带空白] => [7]（容忍）、BITE_ENFORCE [带空白] => []、TESTSIDE [带空白] => []、resolve 循环 [带空白] => 跳过、回复块 [带空白] => 不匹配、改动前 upsert [带空白] => []。因此一条带空白的 resolved 声明会在 upsert 中抑制发布，但线程在 GitHub 上从未被 resolve，BITE_ENFORCE 也看不到该声明——让 Critical 声明得以被抑制的校验门从未触发。一致性不变式在 run-autofix-review-verification.sh:866-868 有文档（"Ids tolerate the rc: prefix and CR the other consumers strip"）。
失败场景： resolved-comments.txt 由 agent 写入，行首尾空白正是 R14-14 要吸收的形状。run 1 延后了评论 7 → repair → run 2 声称用 rc:7 修复了 7，而 run 1 的 carry 中仍含 id 7：upsert 把 7 视为已解决并静默丢弃该延后项——watermark 封锁后没有任何后续轮次会重新推导它：永久静默丢失，而线程仍然 open、执行门从未看到该声明。
证据： 见上方探针输出（逐字提取的解析器，在本提交上运行，含建议修复的翻转对照）。
建议修复： 对所有消费方统一归一化——在 BITE_ENFORCE、TESTSIDE 两个 jq 程序及工作流 resolve 循环的 rc_id 清理中，都在去掉 ^rc: 前缀之前先做同样的空白去除；或用注释说明为什么只让 upsert 容忍。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| exec >&3 | ||
| printf "%s\n" "__upsert_child_live__" | ||
| if ! GH_CONFIG_DIR="$(mktemp -d "${RUNNER_TEMP}/autofix-gh-config.XXXXXX")"; then | ||
| echo "__upsert_trusted__::warning::could not create a gh config dir; deferred findings NOT persisted this round" |
There was a problem hiding this comment.
[Critical] R7-2 remnant (re-check of blocker thread 3789077234 — still standing at this commit; also carries the unresolved half of round-2 ledger entry R2-8): b1106cc gave the script's six abort paths the lost() standard — LOST wording, watermark-gated note, raw dump with :: neutralized — but the five workflow clean-child gate-trip messages still do not meet it: "could not create a gh config dir … NOT persisted this round" (5784, twin 6776), the identity-check message (6782), and "upsert child never started … NOT persisted this round" (5796, twin 6790). c05e799 gave them markers so they render as annotations; the wording and the missing dump were not addressed. The drop is permanent — the eval watermark filters this round's feedback out of every later round (the script's own documented invariant, lines 66-69), and 'Reset autofix workspace' rm -rf's WORKDIR — yet the message reads as a retryable skip, and resolve_and_reply_threads has ALREADY posted the thread reply promising the finding "is deferred to the follow-up queue" before the upsert runs (5966/6023 before 5970/6027). The fallback upsert in 'Report dry-run / failure' never runs on a healthy round whose gate trips inside the step (child exit 0 + || true). — Failure scenario: RUNNER_TEMP exhaustion fails the child's mktemp -d (a known CI state per the script's own comment), or the PAT blips → exit 0 with "NOT persisted this round"; report.md posts and advances the eval watermark; the next reset wipes deferred-findings.json — the PR thread publicly claims the finding is queued, the tracking issue never gets it, the only log line reads like a retryable skip, and the items are recoverable only by artifact download, not from the log.
Witness: not run — reproducing requires a live GitHub Actions runner mid-round (RUNNER_TEMP exhaustion or a PAT blip) with real PR thread posting; each leg quoted verbatim from the tree at this commit: reply-before-persist ordering (5966/5970, 6023/6027), the five messages (5784/5796, 6776/6782/6790), watermark advance via report.md, workspace wipe (941-943), fallback gate (6179-6180).
Suggested fix: adopt the script's own lost() standard in the five wrapper gate-trip messages — drop the "this round" wording, state that the findings are LOST (watermark-gated — no later round re-derives them), and cat the raw deferred-findings.json (with :: neutralized) so the items are recoverable from the log.
中文说明
问题： R7-2 残留（阻断线程 3789077234 的复查——在本提交上仍然存在；同时承接第二轮台账 R2-8 未解决的一半）：b1106cc12c 已让脚本的六条中止路径达到 lost() 标准——LOST 措辞、watermark 封锁说明、:: 中和后的原文转储——但工作流的五条 clean-child 门触发信息仍未达标："could not create a gh config dir … NOT persisted this round"（5784，孪生 6776）、身份检查信息（6782）、"upsert child never started … NOT persisted this round"（5796，孪生 6790）。c05e7996 给它们加了 marker 使其能渲染为 annotation；措辞与缺失的转储没有处理。丢弃是永久的——eval watermark 会把本轮反馈从所有后续轮次中过滤（脚本自身文档化的不变式，66-69 行），且 'Reset autofix workspace' 会 rm -rf 掉 WORKDIR——但信息读起来像一次可重试的跳过，而且在 upsert 运行之前 resolve_and_reply_threads 已经发出承诺该 finding "已延后到 follow-up 队列" 的线程回复（5966/6023 先于 5970/6027）。'Report dry-run / failure' 中的兜底 upsert 在步骤内门触发的健康轮次中永远不会运行（child exit 0 + || true）。
失败场景： RUNNER_TEMP 耗尽使子进程的 mktemp -d 失败（脚本自身注释称之为已知 CI 状态），或 PAT 瞬断 → 以 "NOT persisted this round" 退出 0；report.md 发布并推进 eval watermark；下次重置清空 deferred-findings.json——PR 线程公开宣称 finding 已入队，追踪 issue 却始终没有它，唯一的日志读起来像可重试的跳过，条目只能靠下载工件找回，日志里没有。
证据： 未运行——复现需要真实的 GitHub Actions 运行中途（RUNNER_TEMP 耗尽或 PAT 瞬断）与真实线程发布；每一环均逐字引自本提交的代码树（见上）。
建议修复： 让五条包装层门触发信息采用脚本自身的 lost() 标准——去掉 "this round" 措辞，写明这些 finding 已 LOST（watermark 封锁——没有后续轮次会重新推导），并 cat 原始 deferred-findings.json（:: 中和）使条目可从日志恢复。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| def normkey: | ||
| ascii_downcase | gsub("[[:punct:]]+"; " ") | gsub("\\s+"; " ") |
There was a problem hiding this comment.
[Critical] R10-17 remnant (re-check of blocker thread 3790282130 — still standing at this commit): the rv/ic identity changed this round to fullkey = normkey of the UNCAPPED raw text (2258cfb), but normkey still folds only case/punctuation/whitespace — word-level rewording of the same finding passes both unique_by and the corpus check and publishes a permanent duplicate. The feature's own repair flow triggers it whenever run 2's regenerated reason differs in wording from the carry: run 1 rejected → repair rebuilds feedback.md and moves run 1's file to the carry sidecar → SKILL obliges run 2 to re-defer the same verified finding with regenerated reason → both wordings publish. The carry-merge precedence change (this round FIRST) only decides which wording wins an equal-key collision, not whether distinct wordings collide. Neither originally suggested reconciliation exists at this commit (no per-(src,id) carry/own drop; carried lines are not fed into run 2). The identity comment accepts "erring toward a visible duplicate over a silent loss" — but it motivates normkey specifically as the answer to reworded re-emission, which it does not deliver at word level; and each duplicate consumes a 20-cap slot, so clipped genuine siblings are watermark-gated LOST. — Failure scenario: run 1 defers an rv finding and is rejected → repair carries wording A → run 2 re-defers the same finding with regenerated wording B → "tracked in new issue #77 (2 of 2 new)": one finding published twice, permanently (append-only corpus; both wordings become stored variants), consuming a 20-cap slot — and clipped siblings are watermark-gated LOST.
Witness (probe at this commit, real script + recording gh stub):
carry wording A + own wording B, one rv id
→ "tracked in new issue #77 (2 of 2 new)", both wordings in one published body
cross-round arm: corpus carrying wording A + own wording B
→ "appended to issue #42 (1 of 1 new)" appending wording B — both wordings accumulate permanently
Suggested fix: reconcile carry vs own per (src,id) group for non-rc sources — when this round re-emits a carried group, drop the carried group's lines wholesale; alternatively feed the carried lines into run 2's prompt for verbatim re-emission. Update the runUpsert carry pins accordingly.
中文说明
问题： R10-17 残留（阻断线程 3790282130 的复查——在本提交上仍然存在）：本轮 rv/ic 身份改为 fullkey = 未截断原文的 normkey（2258cfbf），但 normkey 仍只折叠大小写/标点/空白——对同一 finding 的词级改写仍会同时通过 unique_by 与语料检查，发布永久重复项。功能自身的 repair 流程在 run 2 重新生成的 reason 措辞与 carry 不同时必然触发：run 1 被拒绝 → repair 重建 feedback.md 并把 run 1 的文件移入 carry 附属文件 → SKILL 要求 run 2 用重新生成的 reason 再次延后同一已验证 finding → 两种措辞都被发布。carry 合并优先级改动（本轮优先）只决定相同键冲突时哪种措辞胜出，不决定不同措辞是否冲突。原建议的两种调和手段在本提交均不存在（没有按 (src,id) 的 carry/own 取舍；carry 行也没有喂给 run 2）。身份注释接受"宁可可见重复也不静默丢失"——但它把 normkey 作为"改写重发"的专门答案来论证，而词级改写它并不能消化；且每个重复项消耗 20 条上限的名额，被裁掉的真实兄弟项将因 watermark 封锁而永久丢失。
失败场景： run 1 延后一个 rv finding 后被拒绝 → repair 带入措辞 A → run 2 用重新生成的措辞 B 再次延后同一 finding → "tracked in new issue #77 (2 of 2 new)"：一个 finding 被永久发布两次（语料只增不改，两种措辞都成为已存储变体），占用 20 条上限名额——被裁剪的兄弟项将 watermark 封锁丢失。
证据： 本提交上的探针（真实脚本 + 记录型 gh stub）：carry 措辞 A + 本轮措辞 B、同一 rv id → "2 of 2 new" 且两种措辞同在一个发布正文；跨轮臂：语料已含措辞 A + 本轮措辞 B → "appended (1 of 1 new)" 追加措辞 B——两种措辞永久累积。
建议修复： 对非 rc 来源按 (src,id) 组调和 carry 与 own——本轮重新发出了某个 carried 组时，整体丢弃该 carried 组的行；或者把 carried 行喂进 run 2 的提示词以便逐字重发。同步更新 runUpsert 的 carry 钉住用例。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // The enumeration primitives: a sweep has to WALK the environment to | ||
| // decide what to clear, whatever vocabulary it then uses. | ||
| expect(code).not.toMatch(/\bcompgen -[ev]/); | ||
| expect(code).not.toMatch(/\bdeclare -x\b/); |
There was a problem hiding this comment.
[Suggestion] R2-2 (round-2 ledger, still standing): the drift alarm's /\bdeclare -x\b/ and /\bexport -n\b/ regexes (this line and 8279) retain the trailing \b that the adjacent comment (8262-8264) says was deliberately removed from unset -f / trap - for exactly this reason — the flag-joined standard spellings declare -xf and export -np escape the alarm. The round-2 reply claimed "Fixed in c05e799 — trailing \b dropped on both", but neither that commit nor 2258cfb touches these lines; both regexes stand unchanged at this commit. — Failure scenario: a future child that enumerates the environment via declare -xf or export -np (standard bash spellings) passes the drift alarm silently — the alarm exists to flag sweep re-introduction for human review, and the standard-spelling evasion defeats it while the adjacent comment claims the hole is closed.
Witness (probe reading the live regexes from this file, one-line fix applied and reverted):
BASE: declare -x: EVaded "declare -xf" export -n: EVaded "export -np"
(unset -f: CAUGHT "unset -fv name" trap -: CAUGHT "trap - ERR EXIT")
FIXED: declare -x: CAUGHT "declare -xf" export -n: CAUGHT "export -np"
Suggested fix: drop the trailing \b on both regexes: /\bdeclare -x/ and /\bexport -n/, matching the unset -f / trap - forms above them.
中文说明
问题： R2-2（第二轮台账，仍然存在）：漂移告警的 /\bdeclare -x\b/ 与 /\bexport -n\b/（本行与 8279 行）仍带着尾部 \b——而相邻注释（8262-8264）明确说正是为此把 unset -f / trap - 的尾部 \b 去掉了：带旗标的标准拼写 declare -xf 与 export -np 会逃过告警。第二轮回复声称 "Fixed in c05e799 — trailing \b dropped on both"，但 c05e799 与 2258cfb 都没有改动这两行；在本提交上两个正则原样存在。
失败场景： 未来的 child 用 declare -xf 或 export -np（标准 bash 拼写）枚举环境时会静默通过漂移告警——告警的存在就是为了把清扫重新引入标记出来交人工审查，标准拼写的逃逸让它失效，而相邻注释却声称这个洞已关闭。
证据： 从本文件读取现行正则的探针（应用单行修复后翻转，随后还原）：BASE 下 declare -xf/export -np 逃逸（unset -fv/trap - 兄弟被捕获）；修复后两者均被捕获。
建议修复： 去掉两个正则的尾部 \b：/\bdeclare -x/ 与 /\bexport -n/，与上方的 unset -f / trap - 形式一致。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| else "rc" end) as $pfx | ||
| | select(($src != "review_comment") or (($done | index($id)) | not)) | ||
| | {src: $src, id: $id, | ||
| raw: ((.path // "?") + " " + .reason), |
There was a problem hiding this comment.
[Suggestion] The new intra-batch identity flattens two structured fields into one string before normalizing — raw: ((.path // "?") + " " + .reason) then fullkey: (.raw | normkey) — so the path/reason boundary is erased and unique_by can silently drop a DISTINCT sibling, the exact outcome the surrounding comment says fullkey exists to prevent ("keeping genuinely distinct findings apart. Erring toward a visible duplicate over a silent loss"). normkey folds case and punctuation, so any pair where one path's tail plus one reason's head re-splits into another path/reason pair collides. — Failure scenario (probe at this commit, real script): same rv id, {path: "docs/Getting started.md", reason: "files should be split"} vs {path: "docs/Getting started", reason: ".md files should be split"} → "1 of 1 new" — one distinct sibling silently dropped, no warning; likewise path src/a.ts + reason x vs path src/a + reason ts x. A dropped sibling is watermark-gated — no later round re-derives it.
Suggested fix: key on the tuple of normalized parts so no boundary exists to shift — carry path and reason through the reshape as separate fields, then unique_by(if .src == "review_comment" then [.src, .id] else [.src, .id, ((.path // "?") | normkey), (.reason | normkey)] end) (jq orders arrays structurally). Probe-verified: both collision pairs flip to "2 of 2 new" while an exact duplicate still folds to "1 of 1 new".
中文说明
问题： 新的批内身份在归一化之前把两个结构化字段拍平成一个字符串——raw: ((.path // "?") + " " + .reason) 再 fullkey: (.raw | normkey)——path/reason 的边界因此被抹掉，unique_by 可能静默丢弃一个不同的兄弟项，恰是周围注释声称 fullkey 要防止的结局（"keeping genuinely distinct findings apart. Erring toward a visible duplicate over a silent loss"）。normkey 折叠大小写与标点，因此任何"一个 path 的尾部 + 一个 reason 的头部"可以重新切分成另一组 path/reason 的组合都会碰撞。
失败场景（本提交探针，真实脚本）： 同一 rv id，{path: "docs/Getting started.md", reason: "files should be split"} 与 {path: "docs/Getting started", reason: ".md files should be split"} → "1 of 1 new"——一个不同的兄弟项被静默丢弃、无告警；同理 path src/a.ts + reason x 对比 path src/a + reason ts x。被丢弃的兄弟项受 watermark 封锁——没有后续轮次会重新推导。
建议修复： 用归一化后的部件元组作键，使边界无从移动——把 path 与 reason 作为独立字段带过重整形，然后 unique_by(if .src == "review_comment" then [.src, .id] else [.src, .id, ((.path // "?") | normkey), (.reason | normkey)] end)（jq 对数组按结构排序）。探针验证：两个碰撞组合翻转为 "2 of 2 new"，完全重复项仍折叠为 "1 of 1 new"。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // R14-14: a resolved id survives stray surrounding whitespace. | ||
| const paddedResolved = runUpsert({ | ||
| findings: '[{"id":7,"reason":"r"}]', | ||
| resolved: ' rc:7 \n', |
There was a problem hiding this comment.
[Suggestion] R14-14 pins the new resolved-id whitespace tolerance with a SINGLE-line corpus (' rc:7 \n'), where whole-file stripping and per-line stripping are indistinguishable — a mutant that moves the two sub()s before split("\n") (stripping only the first line's leading and the last line's trailing whitespace) survives the entire suite (verified at this commit: 173/173 green under the mutant), so the "per-line" part of the behaviour this hunk adds is unpinned. Checked against the other resolved: corpora (CRLF case, plain cases, the 40000-line zero-padded case) — all behave identically under the mutant. — Failure scenario: a real multi-line corpus with a padded interior line ('rc:6\n rc:7 \n') yields only [6] under the mutant: id 7 is not recognised as resolved, so a finding the agent already resolved in code is published to the tracking issue as outstanding — the exact contradiction the resolved filter exists to prevent. Flip probe: head script → no write; mutant → "tracked in new issue #77 (1 of 1 new)" publishing id 7.
Suggested fix: extend R14-14 to a multi-line padded corpus, e.g. findings [{id:6},{id:7},{id:8}], resolved 'rc:6\n rc:7 \nrc:8 \n', asserting no write calls — and assert the lookup ran so a whole-branch crash cannot pass vacuously.
中文说明
问题： R14-14 用单行语料（' rc:7 \n'）钉住新增的 resolved-id 空白容忍——整文件去除与逐行去除在此不可区分。把两个 sub() 移到 split("\n") 之前的突变体（只去首行行首与末行行尾空白）能通过整个套件（本提交上已验证：突变下 173/173 绿灯），因此本 hunk 新增行为的"逐行"部分没有被钉住。已对照其余 resolved: 语料（CRLF 用例、普通用例、40000 行零空白用例）核查——突变下行为全部相同。
失败场景： 真实的多行语料中带空白的内行（'rc:6\n rc:7 \n'）在突变体下只得到 [6]：id 7 不被识别为已解决，于是 agent 已在代码中解决的 finding 被作为未决项发布到追踪 issue——恰是 resolved 过滤器要防止的矛盾。翻转探针：本提交脚本 → 无写入；突变体 → "tracked in new issue #77 (1 of 1 new)" 发布了 id 7。
建议修复： 把 R14-14 扩为多行带空白语料，例如 findings [{id:6},{id:7},{id:8}]、resolved 'rc:6\n rc:7 \nrc:8 \n'，断言无写入调用——并断言 lookup 确实运行过，防止整个分支崩溃却空过。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| step.indexOf("' > /dev/null 2>&1 ; } 3>&1 )"), | ||
| ); | ||
| const wrapperWarnings = | ||
| child.match(/echo "(?:__upsert_trusted__)?::warning::[^"]*"/g) ?? []; |
There was a problem hiding this comment.
[Suggestion] The R14-1 capture regex matches only warnings with the EXACT marker or with NO marker — a warning whose marker is renamed/typo'd (e.g. __upsert_trustedX__) matches neither alternative and silently drops out of the assertion array; length > 0 plus the per-captured-line toContain('__upsert_trusted__') then pass on the survivors. A single-site marker mutation is invisible. — Failure scenario (regex/runtime probe at this commit): renaming only the identity-check marker to __upsert_trustedX__ → the capture drops 3→2 and every assertion still passes. A future edit that renames the marker on one child echo only (five sites across the two twins — e.g. a half-completed migration to a per-run random marker) ships green; at runtime that line misses the replay loop's elif and renders as plain ;;warning;; text — for the identity-check warning, the most security-relevant of the three, exactly the demotion the marker was added to prevent.
Suggested fix: also pin the counts (expect(wrapperWarnings.length).toBe(2) / toBe(3) per child), or capture /echo "[^"]*::warning::[^"]*"/ and assert every captured line starts with __upsert_trusted__ (probe-verified: this flags the renamed line).
中文说明
问题： R14-1 的捕获正则只匹配带精确 marker 或完全没有 marker 的告警——marker 被改名/拼错（如 __upsert_trustedX__）的告警两个分支都不匹配，会静默掉出断言数组；随后 length > 0 与逐行的 toContain('__upsert_trusted__') 在剩余项上照样通过。单点 marker 突变不可见。
失败场景（本提交上的正则/运行时探针）： 只把身份检查的 marker 改名为 __upsert_trustedX__ → 捕获数从 3 掉到 2 且所有断言仍通过。未来只在一个 child echo 上改名 marker 的编辑（两个孪生共五处——例如向每轮随机 marker 迁移到一半）会绿灯发布；运行时该行错过重放循环的 elif，渲染为普通 ;;warning;; 文本——对三者中安全相关性最强的身份检查告警而言，恰是加 marker 要防止的降级。
建议修复： 同时钉住数量（每个 child expect(wrapperWarnings.length).toBe(2) / toBe(3)），或改用 /echo "[^"]*::warning::[^"]*"/ 捕获并断言每条捕获行都以 __upsert_trusted__ 开头（探针验证：这能标出被改名的行）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| expect(repairStep).toContain( | ||
| '::warning::could not merge carried deferrals across the repair', | ||
| ); | ||
| // The merge-failure path QUARANTINES this round's set instead of deleting |
There was a problem hiding this comment.
[Suggestion] The quarantine pin below uses toContain on the mv string — proving it exists SOMEWHERE in repairStep, but nothing pins it into the merge-failure branch or AFTER the raw dump. The branch-specific behaviour this comment claims ("QUARANTINES this round's set … which is what makes the warning's artifact pointer true") is unpinned. — Failure scenario: two enumerated mutants survive the suite at this commit: (a) reorder the mv BEFORE the head -c 4000 … | sed dump inside the failure branch — the dump then reads the already-renamed file, emits nothing, and the warning's "Raw content follows" claim is false in the log — suite green; (b) relocate the mv out of the else-branch — the failure path leaves deferred-findings.json in place, which the quarantine's own workflow comment says the upsert would "union back in as if it had merged" — suite green (the deletion-count pin, carry.next ordering pin, and artifact-list pin are all unaffected by an mv). Comparator control: deleting the mv outright fails the pin (harness alive).
Suggested fix: extend the R8-3 regex to run through the quarantine — /could not merge carried deferrals[\s\S]*?head -c 4000 "\$\{WORKDIR\}\/deferred-findings\.json" \| sed 's\/::\/;;\/g'[\s\S]*?mv "\$\{WORKDIR\}\/deferred-findings\.json" \\\n\s*"\$\{WORKDIR\}\/deferred-findings\.unmerged\.json"/ — verified: kills both mutants, passes on the unmodified workflow.
中文说明
问题： 下方的隔离钉住对该 mv 字符串用 toContain——只证明它存在于 repairStep 的某处，没有把它钉进合并失败分支、也没有钉在原始转储之后。本注释声称的分支专属行为（"QUARANTINES this round's set … which is what makes the warning's artifact pointer true"）没有被钉住。
失败场景： 两个已枚举的突变在本提交的套件上存活：(a) 把 mv 重排到失败分支内 head -c 4000 … | sed 转储之前——转储读到的是已改名的文件，什么都不输出，告警里 "Raw content follows" 的承诺在日志中为假——套件绿灯；(b) 把 mv 移出 else 分支——失败路径上 deferred-findings.json 原样留着，隔离自己的 workflow 注释说 upsert 会把它"当作已合并重新 union 回去"——套件绿灯（删除计数钉、carry.next 顺序钉、工件清单钉都不受 mv 影响）。对照控制：整体删除 mv 会使该钉失败（harness 存活）。
建议修复： 把 R8-3 正则延伸到隔离步骤——/could not merge carried deferrals[\s\S]*?head -c 4000 "\$\{WORKDIR\}\/deferred-findings\.json" \| sed 's\/::\/;;\/g'[\s\S]*?mv "\$\{WORKDIR\}\/deferred-findings\.json" \\\n\s*"\$\{WORKDIR\}\/deferred-findings\.unmerged\.json"/——已验证：两个突变都被杀死，未改动的工作流通过。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| # cap collide and one vanish. The corpus check below still compares | ||
| # rendered forms — that is all the issue stores — so cross-round the cap | ||
| # can cost a duplicate, never a loss. | ||
| | . + {key: (.line | normkey), fullkey: (.raw | normkey)}) |
There was a problem hiding this comment.
[Suggestion] The normkey folding applied to the new fullkey — the case/punctuation/whitespace absorption the surrounding comment re-asserts as the design — is pinned by no test. Every rv/ic identity test uses reasons that differ at word/character level or are byte-identical; none merge siblings differing ONLY in case/punctuation/whitespace. Surviving one-line mutant on this added line: fullkey: .raw (drop normkey) — verified against every identity test and the full suite (173/173 green under the mutant at this commit). — Failure scenario: the repair flow unions carry + this round into one unique_by batch; run 2's regenerated reason differs from the carried wording only in case/punctuation ("Use Optional Chaining here." vs "use optional chaining here") — the exact churn normkey exists to absorb. Under the mutant both publish as a permanent duplicate ("2 of 2 new" for one finding), consuming a 20-cap slot; clipped siblings are watermark-gated LOST.
Suggested fix: extend the R14-13 block with a case/punct-variant pair under one review id, e.g. [{id:21,source:'review',reason:'Fix The Null Check!'},{id:21,source:'review',reason:'fix the null check'}], asserting "(1 of 1 new)" and a single - rv:21 bullet — verified to fail the mutant and pass on the real code.
中文说明
问题： 新 fullkey 所套的 normkey 折叠——周围注释重新确认为设计的"大小写/标点/空白吸收"——没有任何测试钉住。所有 rv/ic 身份测试的 reason 要么词/字符级不同、要么逐字节相同；没有一个用例合并仅大小写/标点/空白不同的兄弟项。本新增行上存活的一行突变：fullkey: .raw（丢弃 normkey）——已对照全部身份测试与完整套件验证（本提交上突变体 173/173 绿灯）。
失败场景： repair 流程把 carry + 本轮 union 进同一个 unique_by 批；run 2 重新生成的 reason 与 carry 措辞仅大小写/标点不同（"Use Optional Chaining here." 对比 "use optional chaining here"）——恰是 normkey 要吸收的搅动。突变体下两者作为永久重复项都被发布（一个 finding "2 of 2 new"），占用 20 条上限名额；被裁剪的兄弟项 watermark 封锁丢失。
建议修复： 在 R14-13 块中增加同一 review id 下的大小写/标点变体对，例如 [{id:21,source:'review',reason:'Fix The Null Check!'},{id:21,source:'review',reason:'fix the null check'}]，断言 "(1 of 1 new)" 且只有一条 - rv:21 条目——已验证该用例使突变体失败、真实代码通过。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| expect(stageStep).toMatch( | ||
| /_upsert_delim="EOF_\$\(head -c 16 \/dev\/urandom/, | ||
| ); | ||
| // …and the CLOSING delimiter: an unterminated heredoc would swallow the |
There was a problem hiding this comment.
[Suggestion] The heredoc closing-delimiter assertion below pins open/cat/close adjacency of the upsert_src heredoc, but nothing pins that the trio is written to $GITHUB_OUTPUT — the workflow wraps them in { … } >> "${GITHUB_OUTPUT}" (qwen-autofix.yml:3993-4001), and that group redirect is the only thing making steps.stage.outputs.upsert_src exist. This commit ADDED an id: 'stage' pin, which pins step identity but not the redirect — deleting the group wrapper leaves the id line intact and every pin passing. — Failure scenario (mutation probe at the previous head; the pin regex is unchanged at this commit): an edit that drops the { / } >> "${GITHUB_OUTPUT}" wrapper (leaving the three lines adjacent) passes every pin; the heredoc goes to step stdout instead of $GITHUB_OUTPUT; upsert_src is empty in every round; both consumers fall into their pinned -z guard and log the benign-looking "deferred-findings upsert skipped: stage step never ran" — deferred findings are silently never persisted by any round, suite green.
Suggested fix: extend the adjacency regex to pin the group closer: …\s*cat [^\n]*\n\s*echo "\$\{_upsert_delim\}"\n\s*\} >> "\$\{GITHUB_OUTPUT\}" — verified to fail the mutant and pass on the real workflow.
中文说明
问题： 下方的 heredoc 结束分隔符断言钉住了 upsert_src heredoc 的 开/cat/闭 相邻关系，但没有钉住这三行被写入 $GITHUB_OUTPUT——工作流把它们包在 { … } >> "${GITHUB_OUTPUT}"（qwen-autofix.yml:3993-4001）里，而这个组重定向是让 steps.stage.outputs.upsert_src 存在的唯一环节。本提交新增了 id: 'stage' 钉住，但它钉的是步骤身份、不是重定向——删除组包装后 id 行完好、所有钉住照样通过。
失败场景（上一 head 上的突变探针；该钉住正则在本提交未变）： 删除 { / } >> "${GITHUB_OUTPUT}" 包装（三行保持相邻）的编辑能通过所有钉住；heredoc 流向步骤 stdout 而非 $GITHUB_OUTPUT；每一轮 upsert_src 都为空；两个消费方落入各自被钉住的 -z 守卫并记录看似无害的 "deferred-findings upsert skipped: stage step never ran"——延后 findings 在每一轮都被静默地永不持久化，套件绿灯。
建议修复： 把相邻关系正则延伸到钉住组收尾：…\s*cat [^\n]*\n\s*echo "\$\{_upsert_delim\}"\n\s*\} >> "\$\{GITHUB_OUTPUT\}"——已验证使突变体失败、真实工作流通过。
— qwen3.8-max via Qwen Code /review (v0.21.11)
commented
Aug 17, 2026
|
Released in v0.21.13. |
6 participants
What this PR does
Adds the missing anti-drift disposition to the autofix review loop. SKILL's address-review gains a fourth outcome, Defer to follow-up: a finding the agent VERIFIED as real but whose fix lies outside the PR's footprint (or its mainline purpose) is recorded in a machine-readable
deferred-findings.json({id, path, reason}), answered on its thread, and left open — neither implemented in-PR (scope drift) nor declined (finding lost). The report step upserts these into one per-PR "Deferred review findings from PR #N" issue — marker-keyed, append-only by rc id, agent text token-neutralized and length-capped — for both pushed and no-op outcomes, so verified off-mainline findings survive the merge as a schedulable queue. Deliberately noready-for-agentlabel: routing the bot's own deferrals back into its issue phase is a human authorization, not a default.Why it's needed
The footprint gates (#8996/#9156) tell a round WHERE it may change code; nothing said what happens to a real finding that lands outside that boundary. All three existing dispositions were wrong for it: implementing drifts the PR off its mainline, declining loses the finding the moment the PR merges (nobody re-reads merged PRs' threads — our own #8996 backlog living in open threads is the case study), and escalating pushes triage onto a maintainer for something the agent already verified. This closes the loop: 防膨胀 (growth brake) bounds how much a round adds, 防跑偏 now has an exit ramp — verified off-mainline work becomes a surviving follow-up queue instead of in-PR drift.
Reviewer Test Plan
How to verify
npm run test:scripts(54 files, 1164 passed / 13 skipped) ornpx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow(173). The new suite validates the shape gate (only a non-empty array of numeric-id items passes), replays the line-building jq (dedupe against the existing issue body by rc id, newline flattening, truncation, marker neutralization), and pins the wiring: the upsert runs after both shared resolve/reply call sites, the JSON rides the artifact dump and repair cleanup, and the neutralization ledger counts ten sites.Evidence (Before & After)
N/A — CI workflow/skill logic. Before: a verified out-of-footprint finding either drifted the PR or evaporated at merge. After: it lands in a per-PR tracking issue with its verification reason, and the thread reply says so.
Tested on
Risk & Scope
Linked Issues
Follow-up to #8981 (防膨胀) and #8996/#9156 (footprint boundaries) — this adds the disposition those boundaries imply.
中文说明
本 PR 做了什么
为 autofix 评审循环补上缺失的防跑偏处置。SKILL 的 address-review 新增第四种结局 Defer to follow-up:agent 已验证为真、但修复位于 PR 足迹(或主线目的)之外的 finding,记录进机读的
deferred-findings.json({id, path, reason}),在原线程回复并保持 open——既不在本 PR 实现(跑偏),也不 decline(丢失)。报告步骤将其 upsert 到每 PR 一个的 "Deferred review findings from PR #N" issue——按 marker 定位、按 rc id 追加去重、agent 文本经 token 中和与长度封顶——pushed 与 no-op 两种结局都执行,让已验证的离主线 finding 以可排期队列的形式在合并后存活。刻意不打ready-for-agent标签:把 bot 自己的延后项回流进其 issue 阶段属于人类授权,不是默认行为。为什么需要
足迹门(#8996/#9156)规定了轮次可以在哪里改代码;但对落在边界之外的真实 finding,原有三种处置都不对:实现会使 PR 偏离主线,decline 会让 finding 在合并瞬间失联(没人回头读已合 PR 的线程——我们自己 #8996 的 backlog 活在开放线程里就是现成案例),escalate 则把 agent 已验证过的东西又压回维护者。本 PR 闭环:防膨胀(增长刹车)限制轮次加多少,防跑偏现在有了出口——已验证的离主线工作变成存活的 follow-up 队列,而非 PR 内漂移。
评审验证方案
如何验证
npm run test:scripts(54 文件,1164 通过/13 跳过)或单文件(173)。新套件验证形状门(仅接受数字 id 的非空数组)、回放行构建 jq(对既有 issue 正文按 rc id 去重、换行拍平、截断、marker 中和),并钉住接线:upsert 在两个共享 resolve/reply 调用点之后执行、JSON 进工件转储与 repair 清理、中和台账计十处。证据(Before & After)
N/A —— CI workflow/skill 逻辑。Before:已验证的足迹外 finding 要么带偏 PR、要么随合并蒸发。After:它带着验证理由落入每 PR 追踪 issue,线程回复注明去向。
测试平台
风险与范围
关联 Issue
#8981(防膨胀)与 #8996/#9156(足迹边界)的 follow-up——补上边界所隐含的处置出口。