Repository navigation
fix(ci): watchdog silent sandbox hangs and reap the containers they leak - #8816
Merged
Merged
Conversation
added 5 commits
August 9, 2026 06:34
A deterministic rejection in the autofix verification gate is only chargeable to the round if the same check passes without the round's commit. The gate charged every red to the fix unconditionally, and run 31276008548 measured what that costs when the premise is false: PR 8614's branch predated #8693's tsconfig guard while node_modules came from the post-#8693 trusted base, so `npm run build` was equally red at origin/<branch> — 63 minutes of accepted agent work discarded, an 18-minute repair burned on a failure the repair agent is forbidden to touch (it may only amend the round's own fix), thirteen rounds in a row, and the same again on the #8616 leg. On rejection the gate now re-runs the failing check at origin/<branch> (the branch as pushed, before the round) in the same environment: - baseline green: today's path exactly — outcome=failed, retryable=true, the repair pass gets its chance. - baseline red too: outcome=failed with preexisting=true and NO retryable. The repair step keys on retryable and is skipped — it cannot reach a failure outside the round's diff by construction — and gate-rejection.md says outright that the branch needs a base update (merge main), which flows into the failure comment as-is. Fail-closed toward today's semantics: any A/B infrastructure problem (missing ref, checkout failure) charges the fix as before, and a restore failure after the baseline run rejects outright since the tree can no longer be trusted. The round's work is still not pushed — this changes the verdict's honesty and cost, not the push policy. Tested by executing the real script in a real two-remote git repo with an npm stub whose failures are keyed by commit SHA: round-caused red (baseline green), pre-existing red (both red), and the untouched green path. Mutation-tested, 3 of 3 caught: skipping the A/B, claiming pre-existing without measuring, and dropping the tree restore.
All seven findings verified before fixing; the three Criticals were each a way the A/B compared something other than the check that failed. R1-1 — the contracts check feeds on stdin, which its first run drains; the baseline leg re-ran against EOF and checked an empty file list. R1-3 — the schema check's verdict rides on packages/core/dist, which the core-rebuild guard built from ROUND sources and which, being gitignored, survives the detach. Both checks are now A/B-exempt (run_check_no_ab): their baseline verdicts prove nothing, and their rejections stay where the repair agent can actually act on them. R1-2 — a workspace the round ADDS does not exist at the baseline, and npm exits 1 there with "No workspaces found" (measured; --if-present forgives a missing script, not a missing workspace) — a round-caused failure misread as pre-existing, skipping the one repair that can fix the round's own package. The per-package loop now A/Bs only when the workspace exists at origin/<branch>. R1-4 — a chatty PASSING baseline used to flood the tail -c 3000 evidence window and push the actual failure text out of gate-rejection.md, the sole carrier into the repair feedback, the PR comment, and the next round's LAST_REJECTION. The baseline transcript now goes to a side log and only a FAILING tail is merged back, where it is the evidence. R1-5 — the pre-existing paragraph pushed gate-rejection.md past the report's head -c 3500 cap, truncating the closing fence for branch names past 44 characters. Cap raised to 3900, invariant comment updated with the new arithmetic. R1-6 — preexisting=true had no read site. It now flows verify → Finalize verification → the failure report, whose headline swaps the generic gate clause for "PRE-EXISTING failure … needs a base update (merge main)". R1-7 — the no-round-commit guard was unpinned (deleting it kept all tests green). Now exercised through the core-rebuild path, the one A/B-eligible check that runs before the commit gate. Four new behavioral scenarios (chatty baseline, no-commit round, A/B-exempt checks, round-added workspace) plus workflow pins for the forwarding, the clause, and the cap. Mutation-tested, 4 of 4 caught: schema back to A/B (3 tests), guard dropped, side log reverted, no-commit guard dropped.
Ten findings across two rounds, each verified before fixing. The three deepest share one lesson: the A/B is only sound for a check whose inputs travel entirely with the git ref, and whose failure it can IDENTIFY, not merely observe. R2-1 — rc=1 at both legs does not make them the same failure: the branch can fail for reason A while the round fails for reason B, and a baseline infrastructure hiccup is a nonzero exit too. Pre-existing now requires a MATCHING failure identity — tsc diagnostics normalized to file + error code (positions shift with the round's edits), compared via comm(1) on a per-check transcript. No diagnostics on either side means identity cannot be established and the round stays charged. R2-2 / R2-7 — gitignored dist survives the detach carrying the ROUND's build, so any dist-consuming check A/Bs reverted sources against round-built artifacts: package tests (channel-base resolved through dist exports) and typecheck (sdk-typescript resolves core's d.ts — probe-verified three-arm flip). Both are now A/B-exempt, as is lint, leaving `npm run build` — the incident class, and the one check that rebuilds its own inputs from the checked-out sources — as the sole A/B candidate. The workspace-existence guard dissolves with it. R2-3 — the fixture inherited the caller's global git config; a failing global pre-commit hook broke all seven cases. The harness now isolates GIT_CONFIG_GLOBAL/SYSTEM for every git child, and the suite is proven green under a deliberately hostile hooksPath. R2-4 — Finalize verification now selects preexisting from the same attempt whose outcome it selects (repair verification included). R2-5 / R2-8 — the "merge main" advice is now conditional at both layers: the script paragraph states the measured fact and hedges the remedy; the report headline uses the compare the step already ran — behind/diverged gets the base-update clause, an up-to-date branch is told its own pre-round code needs attention. R2-6 — the rejection document now sizes its evidence tail against its preamble (floor 500 bytes, total under the 3900-byte render cap), so the closing fence can no longer be truncated off by a long branch name. R2-9 — dissolved by R2-2: package tests no longer A/B, the guard and its uncovered positive branch are gone. R2-10 — the baseline-evidence merge is now pinned: the pre-existing scenario asserts the baseline leg's own failure line (keyed by its SHA) reaches gate-rejection.md. Eight behavioral scenarios; mutation-tested 5 of 5: identity dropped, typecheck re-enrolled, package tests re-enrolled, evidence merge dropped, fixed tail restored.
…c prose Nine findings, all refinements — the design held, the edges did not. Identity now keeps the diagnostic MESSAGE (file + code collide: two unrelated TS2339s in one file compared equal, skipping a repair that could have shipped — probe-reproduced by the review), and the fixture emits a SHIFTED position on the baseline leg so the position strip is load-bearing instead of decorative (deleting the sed survived every test before; it fails one now). vite/esbuild failures still yield an empty signature by design — documented as the fail-closed limit rather than half-widened. The fail_signature assignments take `|| true`: grep exits 1 on the normal no-match case and survives errexit today only because the caller sits in an if-condition — a future unconditional call site would crash the gate verdict-less. The restore-failure branch is now stageable and staged: the baseline leg recreates (untracked) a file the branch tracks, the checkout back refuses, and the test pins retryable-not-preexisting with the 'could not restore' label. Relaxing the branch to `|| true` fails it. Prose synced to the mechanisms that replaced it: the render-cap invariant restates against the dynamic tail budget (the old 3000-based arithmetic would misguide the next retune), the no-round-commit guard comment names the core rebuild (schema/contracts left the A/B last round), the describe wording counts both A/B-eligible builds, and the pre-existing clauses no longer claim "the repair pass was skipped" — with REPAIR_PREEXISTING forwarded, repair may have RUN; they now state the invariant that is true either way: repair may only amend the round's own fix, so it cannot reach this failure. Mutation-tested, 3 of 3 caught: position strip dropped, message dropped from the identity, restore rejection relaxed.
Four autofix rounds have died the same way (#8663 twice, #8761 r3, #8763 r4): the agent's last output is the sandbox wrapper's "ContainerName (regular): …" line at docker container entry, then nothing — not one event — until the 2-hour absolute budget kills the round. Four different runners, two image versions: systemic, not a bad machine. Where exactly the container wedges is still unknown (that needs docker state on the runner); what is certain from the logs is the shape — a wedged sandbox produces NOTHING, and a legitimate run is never silent for long (the fleet's longest tolerated quiet is the review pipeline's 10-minute stream-idle window for thinking phases). Two mitigations, each aimed at a measured half of the damage: - run-agent.mjs gains an idle watchdog (QWEN_IDLE_TIMEOUT_MS, default 20 minutes = 2x that longest legitimate silence): zero output for the window kills the agent with a distinct "idle-timeout … the sandbox likely hung at startup" detail, so the failure comment names the right knob and a hung round costs 20 minutes instead of 120. Polled, not reset-per-chunk — a busy stream should not spend its time re-arming timers. - Both sandboxed jobs reap stale qwen-code-* containers at job start: a budget kill reaps the HOST-side docker client, not the container, so every killed sandbox keeps running on the persistent runner — observed directly when a later leg's container-name counter found qwen-code-0.21.8-0 already occupied and picked -1. One job per runner at a time makes any container alive at job start stale by definition. Tested by executing the real run-agent.mjs end to end with stub agents: the hang shape (one line, then silence) dies at the idle window naming the idle limit, and a slow-but-talking agent that outputs every 400ms across a 1500ms window survives to a clean exit — the test that distinguishes a watchdog from a disguised absolute timer. Mutation- tested, 3 of 3 caught: watchdog disabled, last-output tracking dropped (the disguised-timer regression), cleanup dropped from a job.
Contributor
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. |
Collaborator
Author
|
@qwen-code /takeover |
Collaborator
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-dev-bot
added
the
autofix/takeover
Collaborator
Collaborator
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action takenThe only feedback item this round is the failed check Evidence
There were no review, inline, or issue-level findings from trusted maintainers or the automated reviewer this round (the round was triggered by the 中文说明Autofix 评审轮次：无需处理本轮唯一的反馈项是失败检查 证据
本轮没有来自受信维护者或自动评审器的 review、行内或 issue 级发现（本轮由 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
wenshao
commented
Aug 9, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
.github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted. [Critical] .github/scripts/run-autofix-review-verification.sh:69 SC2016 — Expressions don't expand in single quotes, use double quotes for that. [lint] [Critical] .github/scripts/run-autofix-review-verification.sh:69 SC2016 — Expressions don't expand in single quotes, use double quotes for that. [lint]
中文说明
.github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。 [Critical] .github/scripts/run-autofix-review-verification.sh:69 SC2016 — Expressions don't expand in single quotes, use double quotes for that. [lint] [Critical] .github/scripts/run-autofix-review-verification.sh:69 SC2016 — Expressions don't expand in single quotes, use double quotes for that. [lint]
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
Comment on lines
+142
to
+144
| if [[ -z "${sig_head}" || -z "${sig_base}" ]] || | ||
| ! comm -12 <(printf '%s\n' "${sig_head}") <(printf '%s\n' "${sig_base}") \ | ||
| | grep -q .; then |
Collaborator
Author
There was a problem hiding this comment.
[Critical] The pre-existing verdict fires on ANY one shared signature line: comm -12 intersection non-empty → preexisting=true, so a round-caused failure that shares a single normalized diagnostic (file + error code + message, position stripped) with the baseline is mislabeled pre-existing — the repair is skipped and the report names the wrong remedy. — Failure scenario: the round fixes a pre-existing error and introduces a different defect with an identical file+code+message at another position (or adds a second instance while the old one remains). sig_head ∩ sig_base ≠ ∅ → preexisting=true, retryable unset → the repair step is skipped and the round's commit is discarded, even though the head's actual failure is round-caused and repairable; the report tells the human "base update needed / pre-round code needs attention" — the wrong remedy. Probe (real script): head = baseline's 5 diagnostics + 5 round-caused TS8888 → preexisting=true, no retryable; a subset-rule patch flips it to retryable=true.
Suggested change
| if [[ -z "${sig_head}" || -z "${sig_base}" ]] || | |
| ! comm -12 <(printf '%s\n' "${sig_head}") <(printf '%s\n' "${sig_base}") \ | |
| | grep -q .; then | |
| # Pre-existing only when the round's failing set is a SUBSET of the | |
| # baseline's (the round added nothing new): | |
| if [[ -z "${sig_head}" || -z "${sig_base}" ]] || | |
| ! comm -23 <(printf '%s\n' "${sig_head}") <(printf '%s\n' "${sig_base}") \ | |
| | grep -q .; then |
中文说明
[Critical] “pre-existing” 判定只看任意一条共享签名：comm -12 交集非空即 preexisting=true。因此只要本轮引入的失败与基线共享一条归一化诊断（文件 + 错误码 + 消息，位置已被剥离），整轮失败就会被误判为 pre-existing——repair 被跳过，报告给出错误处置。— 故障场景：本轮修复了一个既有错误，又在另一位置引入一个 文件+错误码+消息 完全相同的不同缺陷（或旧错误未除又新增同款）。sig_head ∩ sig_base ≠ ∅ → preexisting=true、retryable 未置位 → 本可修复该缺陷的 repair 阶段被跳过，本轮提交被丢弃；报告还误导人类去 "merge main / 检查 round 前代码"。已用真实脚本探针复现：head = 基线 5 条诊断 + 5 条本轮新增 TS8888 → preexisting=true 且无 retryable；改为子集规则后翻转为 retryable=true。修复方向：仅当本轮失败集是基线失败集的子集（本轮未新增任何诊断）时才判 pre-existing。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
Comment on lines
+838
to
+839
| command -v docker > /dev/null || exit 0 | ||
| STALE="$(docker ps -aq --filter 'name=qwen-code-')" |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] The same 13-line "Remove stale sandbox containers" block is copied verbatim into both sandboxed jobs (issue-autofix and review-address), and the new test expect(workflow.split(step).length - 1).toBe(2) locks the duplication in — though the repo already has the reuse mechanism for exactly this (composite actions under .github/actions/). — Concrete cost: a future change (an extra name filter, a --filter status=exited refinement, a new image prefix) applied to one job only silently diverges the two sandboxed jobs — the precise class of leftover-container wedge this step exists to prevent, since one job's leak wedges the other's name counter.
Suggested change
| command -v docker > /dev/null || exit 0 | |
| STALE="$(docker ps -aq --filter 'name=qwen-code-')" | |
| - name: 'Remove stale sandbox containers' | |
| uses: './.github/actions/reap-stale-sandbox-containers' |
中文说明
[Suggestion] 这 13 行 "Remove stale sandbox containers" 步骤在两个沙箱 job（issue-autofix 与 review-address）中被逐字复制，新测试 expect(workflow.split(step).length - 1).toBe(2) 还把这份重复钉死——而本仓库已有现成复用机制（.github/actions/ 下的 composite action）。— 具体代价：未来任何改动（新增名称过滤、--filter status=exited 精化、新镜像前缀）只改一个 job 时，两个沙箱 job 会悄然分叉——这正是本步骤要防的“残留容器卡死命名计数器”那一类问题。建议抽成 composite action 后 uses: 引用。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
Collaborator
There was a problem hiding this comment.
Deferred (budget round). Valid: the block is duplicated verbatim and the test pins the duplication. With the reap filter changed this round, extracting the composite action is more valuable, not less — queued so both jobs move together and the test's toBe(2) becomes a single-action assertion.
中文说明
暂缓（预算轮）。成立：该块被逐字复制，且测试把复制钉死。本轮收割过滤器刚改动，抽取 composite action 的价值只增不减——已列入计划，使两个 job 同步迁移，测试的 toBe(2) 改为对单一 action 的断言。
| if [ -n "${STALE}" ]; then | ||
| echo "removing stale sandbox containers:" | ||
| docker ps -a --filter 'name=qwen-code-' --format ' {{.Names}} ({{.Status}})' | ||
| printf '%s\n' "${STALE}" | xargs -r docker rm -f > /dev/null |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] The reaper lives only at job start, but the leak is caused by the agent-kill path — and this PR's own idle watchdog is now the most frequent killer. The sandbox container is started with --rm, which fires only when the container's main process exits; killing the host-side docker client (exactly what killQwen does to the CLI's process group) orphans the container. — Concrete cost: on a wedged leg the idle watchdog kills the CLI at 20 min, and the container — holding the workspace bind-mount, the daemon, and the API-key env — keeps running on the persistent runner until the NEXT job's start (tens of minutes to hours later), during which the leak this PR describes is still happening. The name IS recoverable at kill time: the CLI prints ContainerName (regular): qwen-code-… to stderr, captured in agent.log/outputTail.
Suggested change
| printf '%s\n' "${STALE}" | xargs -r docker rm -f > /dev/null | |
| # At kill time in run-agent.mjs (idle/absolute/loop-guard paths): | |
| # extract the container name from the captured output and | |
| # execFileSync('docker', ['rm', '-f', name]) — best-effort, wrapped so it | |
| # can never mask the verdict. Keep the job-start reaper as belt-and- | |
| # suspenders for hard kills (job timeout) no process can catch. |
中文说明
[Suggestion] 收割步骤只在 job 开始时运行，但泄漏的制造者是 agent 杀进程路径——而本 PR 新增的空闲看门狗恰恰成了最频繁的杀手。沙箱容器以 --rm 启动，只有容器主进程退出时才会触发清理；杀掉宿主侧 docker 客户端（killQwen 对 CLI 进程组所做的正是这个）会让容器成为孤儿。— 具体代价：wedged leg 在 20 分钟被看门狗杀掉后，容器（持有工作区 bind-mount、daemon 与 API-key 环境变量）会一直运行到下一次 job 开始（数十分钟到数小时），本 PR 所描述的泄漏在此期间依旧存在。名字在杀进程时是可获取的：CLI 向 stderr 打印 ContainerName (regular): qwen-code-…，已进入 agent.log/outputTail。建议在 run-agent.mjs 的各杀进程路径按名 docker rm -f（best-effort，不得掩盖判定），job 开头收割保留作兜底。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
Collaborator
There was a problem hiding this comment.
Deferred (budget round), now promoted: with this round's reap restricted to exited/dead containers (it must not touch running ones on the shared daemon), the kill paths in run-agent.mjs become the designated place to reclaim RUNNING orphans — best-effort docker rm -f on the container name parsed from the captured ContainerName (regular): … output, wrapped so it can never mask the verdict. Queued first with the run-agent batch.
中文说明
暂缓（预算轮），优先级提升：本轮把收割限制为仅 exited/dead 容器后（共享守护进程上不得触碰运行中容器），run-agent.mjs 的杀进程路径成为回收运行中孤儿容器的指定位置——从捕获的 ContainerName (regular): … 输出解析容器名做 best-effort docker rm -f，并包裹以确保永不掩盖判定。已与 run-agent 批次一起列为首要事项。
Collaborator
There was a problem hiding this comment.
[Critical] Revalidated on head 51e9034004bdf7c28652d4381ce36a2d66772b28: the latest safety fix makes this a blocking functional gap. The workflow and its new test both state that a budget/idle kill reaps only the host-side Docker client and that the sandbox container keeps running. The startup reaper now selects only status=exited and status=dead, so that exact running orphan does not match on the next job either; it remains indefinitely, still occupying a qwen-code-* name. In other words, the filter avoids killing a concurrent job safely, but it no longer implements the leak-reaping mitigation advertised by this PR. Please make ownership unambiguous and remove the container owned by this agent in the timeout kill path (or use an equivalent labeled-owner mechanism), while keeping unrelated running containers safe, and add a behavioral test for a running orphan. Re-expanding the startup filter to all running containers would reintroduce the shared-daemon race from the earlier Critical.
Comment on lines
+142
to
+143
| if [[ -z "${sig_head}" || -z "${sig_base}" ]] || | ||
| ! comm -12 <(printf '%s\n' "${sig_head}") <(printf '%s\n' "${sig_base}") \ |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] The empty-signature fail-closed branch is never exercised: the stub npm prints a tsc-style diagnostic on every staged failure, so sig_head/sig_base are non-empty in every A/B test — the documented "KNOWN LIMIT" (vite/esbuild failures → empty signature → deliberately charged to the round) has no test in either direction. — Concrete cost: a mutation that relaxes the signature regex, or makes empty signatures compare as "matching", would mislabel a round-caused vite failure as pre-existing — skipping the only repair that could fix it and posting a wrong "base update needed" comment; a stricter regex would recreate the 18-minute repair waste this PR exists to eliminate.
中文说明
[Suggestion] 空签名 fail-closed 分支从未被测试覆盖：桩 npm 在每次 staged 失败时都打印 tsc 风格诊断，因此所有 A/B 测试里 sig_head/sig_base 都非空——注释中记载的 "KNOWN LIMIT"（vite/esbuild 失败 → 空签名 → 故意归责本轮）两个方向都没有测试。— 具体代价：若未来放宽签名正则、或让空签名按“匹配”比较，本轮导致的 vite 失败会被误判 pre-existing——跳过唯一能修复它的 repair 并发布错误的 "base update needed" 评论；若收紧正则，则会把每次 pre-existing 失败都归责本轮，重新制造本 PR 要消灭的 18 分钟 repair 浪费。建议补一个只输出非 tsc 标记（无 error TS 行）的 runGate 变体，断言 retryable=true 且无 preexisting=true。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
Collaborator
There was a problem hiding this comment.
Deferred (budget round). Valid: the empty-signature fail-closed branch (vite/esbuild shapes) has no test in either direction. Queued: a runGate variant whose stub emits no error TS line, asserting retryable=true without preexisting=true.
中文说明
暂缓（预算轮）。成立：空签名 fail-closed 分支（vite/esbuild 形态）两个方向都没有测试。已列入计划：新增桩不输出任何 error TS 行的 runGate 变体，断言 retryable=true 且无 preexisting=true。
Comment on lines
+4604
to
+4606
| if [[ "${PREEXISTING}" == 'true' ]]; then | ||
| echo "preexisting=true" >> "${GITHUB_OUTPUT}" | ||
| fi |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] The new workflow tests pin the env wiring and report clauses for the preexisting flag — but not the line that actually PRODUCES final_verify.outputs.preexisting (this guarded echo). Deleting this echo (or its guard) passes all 16 new tests. — Concrete cost: if it is dropped, every pre-existing rejection renders the generic "the verification gate rejected the attempt" clause instead of the honest "PRE-EXISTING failure … the branch needs a base update (merge main)" — the operator's only cue to merge main disappears, silently.
Suggested change
| if [[ "${PREEXISTING}" == 'true' ]]; then | |
| echo "preexisting=true" >> "${GITHUB_OUTPUT}" | |
| fi | |
| if [[ "${PREEXISTING}" == 'true' ]]; then | |
| echo "preexisting=true" >> "${GITHUB_OUTPUT}" | |
| fi |
中文说明
[Suggestion] 新加的 workflow 测试钉住了 preexisting 标志的 env 接线与报告分句，却唯独没钉住真正产出 final_verify.outputs.preexisting 的这行受保护的 echo。删除这行（或它的守卫）后全部 16 个新测试依然通过。— 具体代价：一旦被删，所有 pre-existing 拒绝都会退化为通用措辞 "the verification gate rejected the attempt"，而不会出现诚实的 "PRE-EXISTING failure … the branch needs a base update (merge main)"——运维者“去 merge main”的唯一提示会悄然消失。建议把现有 flag 选择正则测试延伸覆盖到这行 guard 与 echo。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
Collaborator
There was a problem hiding this comment.
Deferred (budget round). Valid: the guarded echo that produces final_verify.outputs.preexisting is unpinned — deleting it passes all current tests and silently drops the "merge main" cue. Queued: extend the flag-selection regex test to cover the guard and the echo.
中文说明
暂缓（预算轮）。成立：产出 final_verify.outputs.preexisting 的受保护 echo 未被钉住——删除它后现有测试全部通过，"去 merge main" 的提示会悄然消失。已列入计划：将 flag 选择正则测试延伸覆盖该守卫与 echo。
| : result.timedOut | ||
| ? `timeout (${QWEN_TIMEOUT_MS}ms)` | ||
| : result.idleTimedOut | ||
| ? `idle-timeout (no output for ${QWEN_IDLE_TIMEOUT_MS}ms — the sandbox likely hung at startup)` |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] The new idle-timeout marker flows into the failure report's timeout branch, which for the hang case advises raising the time budget — the wrong knob, contradicting the design goal stated in the code comment and test. CAUSE="ran out of time before finishing (…)" with LAST_FIX="…split the PR or raise the agent time budget…", and the TIMEOUT_WINDOW_CAP census counts the hang as "time-budget exhaustion" whose headline tells the operator to raise the budget. — Concrete cost: when the idle watchdog fires, the failure comment tells the operator to raise the time budget — for a wedged sandbox, raising the budget changes nothing (the hang burns whatever budget is set); the census can stop a PR early based on a diagnosis that misattributes hang cost to budget size.
中文说明
[Suggestion] 新的 idle-timeout 标记会流入失败报告的超时分支，而该分支针对挂起场景给出的建议是调大时间预算——旋钮拧错，与代码注释和测试声明的设计目标相悖。CAUSE="ran out of time before finishing (…)"、LAST_FIX="…split the PR or raise the agent time budget…"，TIMEOUT_WINDOW_CAP 普查还把挂起计为“time-budget exhaustion”，标题建议提高预算。— 具体代价：看门狗触发后，失败评论让运维去调大时间预算——对 wedged 沙箱而言调大预算毫无作用（挂起会烧掉任何预算）；普查还可能基于把挂起成本错算为预算不足的诊断提前叫停 PR。建议报告步骤按标记内容分支：AGENT_TIMEOUT 以 idle-timeout 开头时给出挂起专用处置并豁免预算建议。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
Collaborator
There was a problem hiding this comment.
Deferred (budget round). Valid: the idle-timeout marker flows into the timeout branch whose advice (raise the budget) cannot cure a wedge. Queued together with the census finding above as one idle-class disposition: branch the report step on the marker prefix, with wedge-specific handling and no budget-raise advice.
中文说明
暂缓（预算轮）。成立：idle-timeout 标记流入超时分支，而该分支的建议（调大预算）无法修复挂死。已与上面的计数发现合并为一个空闲类别处置列入计划：报告步骤按标记前缀分支，给出挂死专用处置且不再建议调大预算。
Comment on lines
+84
to
+85
| baseline_also_fails() { | ||
| # A deterministic rejection is only chargeable to this round if the same |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] The PR is titled and described as "Two mitigations" for the sandbox hang, but roughly half the diff (~350 of +732 lines: baseline_also_fails, fail_signature, run_check_no_ab, the FIRST_PREEXISTING/REPAIR_PREEXISTING plumbing, and the report clause) is a rework of the review-verification gate that is absent from the description. — Concrete cost: the A/B rework changes observable autofix-loop behaviour — a pre-existing failure now emits preexisting=true and suppresses retryable=true, which changes whether the repair step runs and swaps the failure report's clause. A reviewer or merger reading the description cannot evaluate or even discover this gate behaviour change; the gate's retry semantics ship undisclosed under a sandbox-hang title.
中文说明
[Suggestion] PR 标题与描述只讲“两项缓解”沙箱挂起，但 diff 约一半（+732 行中的约 350 行：baseline_also_fails、fail_signature、run_check_no_ab、FIRST_PREEXISTING/REPAIR_PREEXISTING 接线与报告分句）是对评审验证门禁的重构，而描述对此只字未提。— 具体代价：A/B 重构改变了 autofix 循环可观察行为——pre-existing 失败现在输出 preexisting=true 并抑制 retryable=true，改变 repair 是否运行并切换失败报告措辞。只读描述的人无从评估甚至无从发现这次门禁行为变更；门禁重试语义在“沙箱挂起”标题下未披露地合入。建议在描述中增加 "What else this PR does" 一节，或拆成独立 PR。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
Collaborator
There was a problem hiding this comment.
Deferred — not actionable from this round: the gate rework is real and should be disclosed in the PR description, but editing the PR body is a GitHub write this headless round cannot perform (the workflow owns PR creation/updates). Flagged here so a maintainer (or the workflow's PR-body path) adds a "What else this PR does" section covering the baseline A/B gate, the preexisting plumbing, and the changed retry/report semantics before merge.
中文说明
暂缓——本轮无法执行：门禁重构确实存在且应在 PR 描述中披露，但修改 PR 正文属于 GitHub 写操作，本无头轮次无权执行（PR 的创建/更新由 workflow 负责）。在此标记，请维护者（或 workflow 的 PR 正文路径）在合入前补充 "What else this PR does" 一节，说明基线 A/B 门禁、preexisting 接线以及变更后的重试/报告语义。
| fi | ||
| # Only a FAILING baseline transcript with a matching signature is | ||
| # evidence — merge its tail into the window, where it backs the label. | ||
| tail -c 1500 "${ab_log}" >> "${GATE_LOG}" 2> /dev/null || true |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] For a pre-existing verdict, the fixed-size evidence window (tail_budget = 3300 − preamble, ~2812 bytes) must hold BOTH the round's failing output AND this appended 1500-byte baseline tail — so a verbose failing-check log pushes the matched diagnostic entirely out of the rendered document, and the "with a matching failure signature" claim is backed by no visible diagnostic. — Failure scenario: any A/B-eligible check whose failure output is longer than ~1.3 KB with the tsc diagnostic not in its final ~1.2 KB (a monorepo tsc -b build where the matched error is in an early project), with a matching-signature baseline leg. Probe (real script, stub npm printing the diagnostic then ~11 KB noise on both legs): gate-rejection.md contained 0 occurrences of the matched diagnostic while the preamble asserted the match; rendering the comm -12 intersection instead flipped occurrences 0→1. The human and the next round's LAST_REJECTION must trust an unverifiable label; if the one matched line was itself a partial-overlap coincidence, the skipped repair is invisible in the evidence.
Suggested change
| tail -c 1500 "${ab_log}" >> "${GATE_LOG}" 2> /dev/null || true | |
| # Render the matched signature lines as evidence (the common | |
| # `file: error TS####: msg` lines) instead of the raw 1500-byte tail: | |
| comm -12 <(printf '%s\n' "${sig_head}") <(printf '%s\n' "${sig_base}") \ | |
| >> "${GATE_LOG}" 2> /dev/null || true |
中文说明
[Suggestion] 对 pre-existing 判定而言，固定大小的证据窗口（tail_budget = 3300 − preamble，约 2812 字节）必须同时容纳本轮失败输出和这里追加的 1500 字节基线尾部——因此冗长的失败日志会把被匹配的诊断整个挤出渲染文档，“with a matching failure signature”的说法背后没有任何可见诊断。— 故障场景：任一 A/B 候选检查失败输出超过约 1.3 KB 且 tsc 诊断不在其最后约 1.2 KB 内（monorepo tsc -b 中匹配错误出现在较早 project），且基线 leg 签名匹配。探针（真实脚本，桩 npm 打印诊断后跟约 11 KB 噪音）：gate-rejection.md 中匹配诊断出现 0 次，而 preamble 断言匹配存在；改为渲染 comm -12 交集后 0→1 翻转。人类读者与下一轮 LAST_REJECTION 只能信任无法核验的标签；若那条匹配行本身只是部分重叠巧合，被跳过的 repair 在证据中完全不可见。建议把匹配的签名行本身渲染进窗口，或补充冗长日志夹具。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
Collaborator
There was a problem hiding this comment.
Deferred (budget round). Valid: on verbose failing logs the matched diagnostic can be pushed out of the fixed-size evidence window while the preamble still asserts the match. Queued: render the comm -12 intersection as the evidence for a pre-existing verdict (or add the verbose-log fixture), so the claim is always visible in the document.
中文说明
暂缓（预算轮）。成立：冗长失败日志下，被匹配的诊断可能被挤出固定大小的证据窗口，而 preamble 仍断言匹配存在。已列入计划：pre-existing 判定改为渲染 comm -12 交集作为证据（或补充冗长日志夹具），使该论断在文档中始终可见。
Comment on lines
+5342
to
+5343
| if [[ "${PREEXISTING}" == 'true' ]]; then | ||
| if [[ "${CMP_R:-}" == 'behind' || "${CMP_R:-}" == 'diverged' ]]; then |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] A pre-existing gate rejection is counted by the consecutive-failure circuit breaker (CONSEC_FAIL) even though it is the same "not the round's fault" class the breaker's own comment exempts — at the cap the breaker replaces the pre-existing headline with "PR is too large or conflicts with a fast-moving main; rebase, split, reduce" and stamps MARK_ROUND=MAX_ROUNDS, skipping the PR forever. — Failure scenario: a PR whose pre-round state is broken (the exact #8614 class this PR targets): each addressed item rejects pre-existing (watermark advances, item dropped, no repair), so several items in a row each burn a full agent round and increment CONSEC_FAIL (nothing in the preexisting path resets it). At the cap the breaker overwrites the pre-existing remedy ("merge main" / "pre-round code needs attention") with "rebase, split, or reduce" — which cannot cure a broken pre-round commit — and the terminal marker makes future scans skip the PR, contradicting the pre-existing headline's promise that "the loop stays engaged". Probe (verbatim breaker block, 5 pre-existing rejections, cap=5): MARK_ROUND=10, CONSEC_FAIL=5, headline replaced.
Suggested change
| if [[ "${PREEXISTING}" == 'true' ]]; then | |
| if [[ "${CMP_R:-}" == 'behind' || "${CMP_R:-}" == 'diverged' ]]; then | |
| # Exempt pre-existing rejections from the breaker — or have the | |
| # pre-existing branch declare the terminal state deliberately with its | |
| # own headline: | |
| [[ "${PREEXISTING:-}" != 'true' ]] && … |
中文说明
[Suggestion] pre-existing 门禁拒绝会被计入连续失败熔断器（CONSEC_FAIL），尽管它与熔断器注释中豁免的"非本轮过错"类别同质——达到上限后熔断器会用 "PR is too large or conflicts with a fast-moving main; rebase, split, reduce" 覆盖 pre-existing 头条并打上 MARK_ROUND=MAX_ROUNDS，从此永久跳过该 PR。— 故障场景：PR 的 round 前状态已坏（正是本 PR 针对的 #8614 类）：每处理一个 item 都 pre-existing 拒绝（watermark 前进、item 丢弃、无 repair），连续多轮各烧满一轮 agent 时间并累加 CONSEC_FAIL（pre-existing 路径没有任何重置）。到上限后熔断器把 pre-existing 处置（"merge main" / "pre-round code needs attention"）覆盖成 "rebase, split, or reduce"——对一个已坏的 pre-round 提交毫无作用——终态标记还让后续扫描跳过 PR，与 pre-existing 头条"loop stays engaged"的承诺相悖。探针（逐字熔断块，5 次 pre-existing，cap=5）：MARK_ROUND=10、CONSEC_FAIL=5、头条被替换。建议在熔断条件中豁免 PREEXISTING，或让 pre-existing 分支以自有头条显式宣告终态。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
Collaborator
There was a problem hiding this comment.
Deferred (budget round). Valid: pre-existing rejections increment CONSEC_FAIL and at the cap get their headline replaced by rebase/split advice that cannot cure a broken pre-round state. Behavior change with a design edge — an exemption must not let a broken pre-round branch burn unbounded rounds — so it is queued with an explicit terminal state for the pre-existing class (own headline + deliberate MARK_ROUND handling) rather than a bare exemption.
中文说明
暂缓（预算轮）。成立：pre-existing 拒绝会计入 CONSEC_FAIL，达上限后其头条被 rebase/拆分建议覆盖，而后者无法修复已坏的轮前状态。这是带设计边界的行为变更——豁免不能让已坏的轮前分支无限烧轮——因此列入计划时为 pre-existing 类别设计显式终态（自有头条 + 明确的 MARK_ROUND 处理），而非简单豁免。
Collaborator
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 1/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31327011711 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 2/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31333022318 🧠 Handled by Qwen Code · model/模型 |
…ive kill
Budget-warning round — the five Criticals from both reviewers, no
suggestions (each deferred with a recorded reply).
fail_signature: `[^\n]*` in an ERE bracket expression does not mean
"rest of line" — in POSIX bracket expressions `\` is literal, so it
matched "neither backslash nor the letter n" and truncated every tsc
message at its first n. Nearly every real message has an early n
("Cannot find name", "is not assignable"), so distinct same-file
failures collapsed into identical signatures and a round-caused failure
could be labeled pre-existing, skipping the repair. grep is
line-oriented: `.*` is exactly the rest of the line. New fixture: two
messages differing only after their first n.
Pre-existing verdict: the intersection test mislabeled in both
directions. A round that ADDS a diagnostic sharing one normalized line
with the baseline was called pre-existing (repair skipped for a
round-caused, repairable failure); and `comm -12 | grep -q` under
`set -eo pipefail` SIGPIPEs comm (exit 141) once the shared output
outruns the pipe buffer, charging true pre-existing failures to the
round — the exact 18-minute repair waste the gate exists to kill.
Pre-existing now means the round's failing set is a SUBSET of the
baseline's, and the difference is captured before testing. New fixture:
a round adding a second diagnostic to a failing baseline.
Restore failure after the baseline leg: was retryable=true with HEAD
still detached at the baseline commit — the repair agent works in that
very checkout and does no git recovery, so its commit would land on the
baseline and be orphaned. Now rejected non-retryable (reject_fix grows
a third arg); the next round starts clean from the trusted checkout.
The restoreClash test pins the new semantics.
Stale-container reap: the premise "a runner runs one job at a time, so
any live qwen-code-* container is stale" holds per runner registration,
but the filter queries the docker daemon, which is per host — and this
pool runs several registrations on one OS. With per-issue/PR
serialization only, a concurrent job's sandbox is a substring match
away from `docker rm -f`. The reap now takes only provably-dead
containers (--filter status=exited/dead, both jobs) and the comment
says why a running one is left alone.
Preamble printf: the `\`` escapes sat inside a single-quoted format
where backslash is literal, so every pre-existing rejection rendered
raw backticks instead of code spans (shellcheck SC2016). Backticks
need no escaping there. Also syncs the side-log comment to the dynamic
tail_budget it actually renders.
Verified: scripts suite 140/140 (was 138; the two new fixtures and the
rewritten restoreClash test all fail against the pre-fix script),
npm run build / typecheck / lint pass, bash -n clean.
Collaborator
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： AutoFix review round 5 — summaryBudget warning in effect (the two previous rounds exhausted the agent time budget), so this round took the blocking Critical subset only, verified it, and committed it; every Suggestion is deferred with a recorded reply on its own thread ( Commit: Resolved (Criticals + one adjacent comment fix)
Deferred (Suggestions — replies posted on each thread)16 findings deferred under the budget warning: the run-agent batch (idle guard No VerificationCommands actually run on this runner, in order:
中文说明AutoFix 评审第 5 轮 — 总结预算警告生效中（前两轮均耗尽 agent 时间预算），因此本轮只处理阻断性 Critical 子集，验证后提交；每条 Suggestion 都在其各自线程中以记录在案的理由暂缓（见 提交： 已解决（Critical + 一处相邻注释修复）
暂缓（Suggestions — 已在各自线程回复）共 16 条发现依预算警告暂缓：run-agent 批次（空闲守卫补查 无 验证本 runner 上实际执行的命令（按顺序）：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
requested changes
Aug 10, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
Round-6 verification at head 51e9034. First the good: all five prior Criticals are confirmed fixed at head (bracket-class regex with regression test, non-retryable restore-failure verdict, exited/dead-only reap filter, comm -23 with full capture, subset semantics with the added-diagnostic test), the watchdog semantics check out (lastOutputAt refreshed by both stdout and stderr, process-group SIGTERM→SIGKILL with ESRCH-safe fallback, all three timers cleared in finish(), no misreport race on normal completion, 20-min default is 2× the fleet's longest tolerated quiet and a false fire only costs a retryable round), and the A/B gate rework is fail-closed throughout with genuinely behavioral tests (+511 lines executing the real artifacts). The deferred suggestions are dispositioned with rationale and I verified the deferral claims.
One new P2 that blocks, not raised in prior rounds: the reap step can fail an entire round at setup. Under the default bash -eo pipefail, printf '%s\n' "${STALE}" | xargs -r docker rm -f exits nonzero when two jobs on different registrations of the same host list the same stale container and the second rm hits 'No such container' (or any transient daemon error like 'removal already in progress') — killing the step before any real work, which is the exact failure class this PR exists to eliminate; the window is widest at first deploy when dirty hosts still hold residue. Likewise STALE="$(docker ps -aq …)" aborts the step when the daemon is unreachable — the command -v docker || exit 0 guard only covers docker being absent. This is pure hygiene, one line per copy (qwen-autofix.yml:846 and :3513): printf '%s\n' "${STALE}" | xargs -r docker rm -f >/dev/null 2>&1 || true, and || true (or an explicit daemon check) on the listing. Minor adjacent note: the name filter is a substring match and also catches qwen-code-integration-test-* containers — harmless since those are exited+--rm, but worth knowing.
Also noted for the record (already dispositioned, not blocking): the reap deliberately excludes running containers so the client-killed orphans this PR's own watchdog creates still need the queued kill-time docker rm -f; and the QWEN_IDLE_TIMEOUT_MS parse admits negative/0 with zero live exposure today. One line per copy away from approve.
doudouOUC
previously requested changes
Aug 10, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the complete 4-file diff at exact head 51e9034004bdf7c28652d4381ce36a2d66772b28, including existing review threads, workflow ownership and failure paths, all new preexisting and idle-timeout read/write sites, the sandbox launcher, and current checks.
The latest commit fixes the earlier signature parsing, pipefail, restore-state, subset-comparison, and cross-registration live-container deletion Criticals. One Critical remains: the stale-container mitigation now filters to exited/dead containers even though the failure mode leaves the sandbox running, so the target orphan is never reaped. Details and the required ownership-safe direction are in #8816 (comment).
Verification on this head: the focused review-gate/watchdog/reaper run passed 18 tests; node --check and bash -n passed. CI Test, Desktop Shell, and web-shell smoke checks are green. Requesting changes only for the blocking orphan-cleanup gap; previously recorded Suggestions remain non-blocking under the repository review-round policy.
Collaborator
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix review round 6 — summaryHead after this round: Blocking findings — all fixed
Suggestions — disposition
Adjunct note from @yiliang114 acknowledged, no change: the Verification
中文说明Autofix 评审第 6 轮 — 摘要本轮后的 head： 阻断性发现 — 全部修复
建议 — 处置
@yiliang114 的附带说明确认知悉，不作修改： 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
… unpinned arms - Wrap every docker call in the stale-container reap with timeout 30: an alive-but-wedged daemon blocks docker ps indefinitely, and the existing || guards only catch nonzero exits, not hangs (R3-1). - Make the kill-path container removal async in run-agent.mjs: the spawnSync blocked the event loop between SIGTERM and the 10s SIGKILL backstop for up to its 30s timeout — in exactly the wedged-daemon scenario the watchdog exists for. The main flow awaits the removal so the leak warning stays deterministic (R3-6). - Split the pre-existing gate clause for an empty CMP_R: a transient compare-API failure is "never measured", not "measured not-behind", and must not assert the branch's own code is at fault (R3-7). - Swap the timeout breaker's closing remedy to the sandbox investigation when every counted timeout was idle, mirroring the round-level split (R3-11). - Tests: pin the budget kill path separately from the idle kill path (R3-3), parameterize the idle-window parse guard over -1/0/NaN (R3-5), add a stderr-only liveness case (R3-12), pin the strict-subset A/B arm via a baseline-superset fixture knob (R3-15), and pin the breaker's current-round idle increment (R3-18).
Collaborator
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Addressed all nine round-3 suggestions; each fix is pinned by a new or updated test, and every pin was mutation-checked (the exact defect each finding names was injected and confirmed to flip its test red before reverting). No base-conflict merge was needed ( Implemented
Verification
中文说明已处理第三轮全部九条建议；每项修复都由新增或更新的测试钉住，且每个钉住都经过了变异检查（先注入各发现所指出的确切缺陷、确认对应测试变红后再还原）。无需基线冲突合并（ 已实施
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action takenThis round had no actionable feedback, so no code changes were made and no commit was created.
The PR head remains at the last verified commit ( 中文说明Autofix 评审轮次：未执行任何操作本轮没有可处理的反馈，因此未做任何代码改动，也未创建新的提交。
PR 的 head 仍停留在上一个经过验证的提交（ Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
approved these changes
Aug 10, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
Re-review at head 74cd5e2 — my round-6 P2 is fixed and proven under live repro at both copies: the reap listing now survives a dead daemon (|| STALE=''), every rm is per-container with timeout 30 and a trailing || true, and I reproduced all three failure classes under bash -eo pipefail with a stub docker (concurrent-rm race, daemon unreachable, wedged daemon bounded at the timeout) — the step exits 0 in each. The hang-bounding goes beyond my suggested fix and closes the gap || true alone would have left. The kill-path orphan reap (the last open Critical) is in: ContainerName parsed from this run's own launcher output with a validated name pattern, removed on both budget and idle kills via the shared escalateKill, now async so the event loop is not blocked between SIGTERM and the SIGKILL backstop; the idle guard also checks timedOut and QWEN_IDLE_TIMEOUT_MS rejects -1/0/NaN, all pinned by tests. CI green and mergeable.
Non-blocking leftovers: the lineCarry name parser's 256-char cap can silently miss the ContainerName line after a very long single startup line (degrade-not-crash, extremely unlikely); escalateKill can run twice (idempotent signals/rm, harmless); and the two #8765 unique improvements (pre-detach empty-head-signature short-circuit, dist steering note) are not ported — per the coordination plan, land this first and port them as a follow-up, then close #8765 as subsumed. Ship it.
qwen-code-dev-bot
added a commit
that referenced
this pull request
Aug 10, 2026
Main already carries this PR's A/B gate work through the stacked #8816 squash, which fixed the same round-5 gate defects with newer, stronger implementations (subset failure-identity comparator, non-retryable restore failure via reject_fix, unescaped printf backticks, three-arm CMP_R clause). Keep those, and re-apply this PR's additions that main lacks: the pre-detach head-signature decision, the dist-rebuilt warning on both retryable A/B exits, the side-log cleanup entries, the stale-base note on the embedded rejection, and the matching fixture knobs and scenarios.
qwen-code-dev-bot
pushed a commit
to qqqys/qwen-code
that referenced
this pull request
Aug 10, 2026
…wenLM#8878) QwenLM#8816's branch accidentally carried QwenLM#8765's early commits, and the takeover loop evolved the gate further there (subset identity via comm -23, the retryable third arg, subset fixtures) — so QwenLM#8765 closes as subsumed, and this PR ports what main still lacks: the two improvements its reviewers named for porting, plus the open round-6/7 findings that survive on main's gate. - Pre-detach short-circuit: an empty head signature (vite/esbuild/ crash — the KNOWN LIMIT class) fails closed regardless of the baseline, so decide it BEFORE paying the detach + full baseline re-run + restore. - Build-dirt guard: the A/B'd build REWRITES a tracked file (the vscode companion settings schema), and the undiscarded rewrite makes either checkout refuse — degrading a real verdict into the crash path. `git restore -- .` before both checkouts; tracked-only, and the tree was asserted clean before the deterministic checks. - Restore-failure semantics: a plain outcome=failed is an EVALUATED rejection — the watermark advances and a transient git failure strands the item as a permanent human handoff. The gate now leaves outcome unset (the gate-crashed path retries next scan) and still writes the detail document so the crash comment explains itself. - The dist-rebuilt steering note seeds the repair feedback on both retryable A/B exits — the repair agent's only warning that dist/ holds baseline-built artifacts. - The stale-base retry handoff prefixes its embedded rejection with a the-base-has-moved note, so the retry agent is not steered toward no-action by framing written before the auto-update. - The two A/B side logs joined the repair step's cleanup list. - Tests: identity-less short-circuit, tracked-dirt survival, verdict-less restore crash, long-preamble render cap, PREEXISTING clause selection through the executable report harness, and the stale-framing note pin. Mutation-tested, 5 of 5 caught: short-circuit dropped, restore guards dropped, restore-failure reverted to the evaluated rejection, dist note dropped, stale-framing note dropped. Co-authored-by: verify <verify@local>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Two mitigations for the silent 2-hour sandbox hangs that have been eating autofix rounds:
run-agent.mjs(QWEN_IDLE_TIMEOUT_MS, default 20 min): zero output for the window kills the agent with a distinctidle-timeout … the sandbox likely hung at startupdetail — a hung round costs 20 minutes instead of 120, and the failure comment names the right knob.Why it's needed
Four autofix rounds died identically (#8663 ×2, #8761 r3, #8763 r4): the last output is the sandbox wrapper's
ContainerName (regular): …line at docker container entry, then nothing — not one event — for the entire 2-hour budget:Four different runners (hk ×2, sg ×2), two image versions (0.21.7 / 0.21.8) — systemic, not a bad machine. Where the container wedges internally is still unknown (that needs docker state on a runner — flagged for whoever can shell in); what the logs prove is the shape: a wedged sandbox produces nothing, while a legitimate run is never silent for long — the fleet's longest tolerated quiet anywhere is the review pipeline's 10-minute stream-idle window for thinking phases on ~1M-token contexts. The watchdog default is twice that.
The container leak is observed directly, not inferred: the hung #8763 leg's container-name counter found
qwen-code-0.21.8-0already occupied and picked-1— a leftover from an earlier kill (sandbox.tsnames by countingdocker ps -a). The workflow had no cleanup anywhere. One job per runner at a time makes anyqwen-code-*container alive at job start stale by definition.Reviewer Test Plan
How to verify
Expected: 138/138 (3 new). Full scripts suite: 50 files, 1076 passed.
yamllint+node --checkclean.The new tests execute the real
run-agent.mjsend to end with stub agents:failure.mdnamesidle-timeout (no output for …)and not the absolute-budget wordingEvidence (Before & After)
Before is the four linked hangs (2 h × 4 ≈ 8 runner-hours for zero work). After: the same shape dies in 20 minutes with an honest failure detail, and each job start clears the leaked containers the previous kills left behind.
Mutation-tested — 3 of 3 caught:
Tested on
Risk & Scope
QWEN_IDLE_TIMEOUT_MSis the env knob. The reaper touches only containers namedqwen-code-*on a runner that, by GitHub's one-job-per-runner model, cannot have a live sibling.docker ps -a+docker logson a hung container, the diagnosis can go deeper.Linked Issues
Diagnosed across #8663, #8761 and #8763 (the comment that prompted this).
中文说明
What this PR does
针对一直在吞噬 autofix 轮次的"沙箱静默挂起 2 小时"问题的两项缓解:
run-agent.mjs增加无输出看门狗(QWEN_IDLE_TIMEOUT_MS,默认 20 分钟):窗口内零输出即杀,失败详情为独立的idle-timeout …(沙箱疑似启动时挂起)——挂起一轮的代价从 120 分钟降为 20 分钟,失败评论指向正确的旋钮。Why it's needed
四个 autofix 轮次以完全相同的方式死亡(#8663 ×2、#8761 r3、#8763 r4):最后一行输出是沙箱包装器在进入 docker 容器时打印的
ContainerName (regular): …,之后整整 2 小时预算内一个事件都没有:四台不同 runner(hk ×2、sg ×2)、两个镜像版本(0.21.7 / 0.21.8)——系统性问题,不是坏机器。容器内部究竟卡在哪仍未知(需要有人上 runner 看 docker 状态,已注明);日志能证明的是形态:挂死的沙箱什么都不产出,而合法运行不会长时间沉默——整个体系容忍的最长安静是 review 流水线为 ~1M token 上下文思考阶段设的 10 分钟流空闲窗口。看门狗默认取其两倍。
容器泄漏是直接观察到的而非推断:挂起的 #8763 leg 的容器命名计数器发现
qwen-code-0.21.8-0已被占用而选了-1——正是此前某次超时杀留下的残骸(sandbox.ts按docker ps -a计数命名)。workflow 此前没有任何清理。GitHub 一台 runner 同时只跑一个 job,因此 job 开始时还活着的任何qwen-code-*容器都必然是残留。Reviewer Test Plan
How to verify
预期 138/138(新增 3 条)。scripts 全量:50 个文件、1076 passed。
yamllint+node --check干净。新测试端到端执行真实的
run-agent.mjs配 stub agent:failure.md写明idle-timeout (no output for …)而不是绝对预算的措辞Evidence (Before & After)
Before 即链接的四次挂起(2 小时 × 4 ≈ 8 runner-小时零产出)。After:同样的形态 20 分钟内死亡并给出诚实的失败详情,且每次 job 启动都会清掉此前超时杀遗留的容器。
变异测试——3 个全部被捕获:
Tested on
Risk & Scope
QWEN_IDLE_TIMEOUT_MS就是旋钮。收割只触碰名为qwen-code-*的容器,而 GitHub 一 runner 一 job 的模型保证它不可能有活着的同胞。docker ps -a+docker logs,诊断可以更进一步。Linked Issues
跨 #8663、#8761、#8763 诊断(引发本 PR 的评论)。