fix(ci): stream autofix agent progress - #8895
Conversation
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Re-running the gate after the four fix commits that landed since my last pass. Template looks good ✓ Problem: observed, and now demonstrated end to end — six managed PRs stranded at the cumulative timeout breaker, and the maintainer's real-stack A/B in this thread measured the mechanism directly: the same productive 5-tool round emits one stdout chunk (the last one, at ~37 s) on Direction: aligned — internal autofix runner tooling with no product surface; making active runs observable so the existing watchdog can distinguish tool work from a wedged sandbox is the right-shaped fix. Size: not applicable — no core package paths touched ( Approach: scope still right. The PR grew from 1 to 5 commits, but each additional commit answers a specific review finding — preserving the API-error retry sentinels, trusting structured verdicts, bounding stream processing, and dropping oversized lines — rather than speculative hardening. No drive-by changes. Risk: no elevated risk signals — neither changed file matches the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明在上次审查之后新增的四个修复 commit 上重新过一遍门禁。 模板完整 ✓ 问题：已观测到，且如今已被端到端证实——6 个托管 PR 卡在累计超时熔断；本线程中 maintainer 的真实环境 A/B 直接测量了机制：同一个 5 次工具调用的有效轮次，在 方向：对齐——autofix runner 内部工具，没有产品面；让活跃运行可观测、使现有 watchdog 能区分工具工作与卡死的 sandbox，是形态正确的修复。 规模：不适用——未触及核心包路径（ 方案：范围依然合理。PR 从 1 个 commit 增加到 5 个，但每个新增 commit 都是在回应一条具体的审查意见——保留 API 错误重试哨兵、信任结构化结论、给流处理加上界、丢弃超大行——而不是推测性加固。没有夹带其他改动。 风险：无升级风险信号——两个改动文件均不在与 revert 相关的高风险路径列表中。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review (full re-review of My independent take before reading the diff was the same as last round: pass the streaming flags at spawn, pin them with a regression test, and then audit every downstream reader of the agent's output, because the format change is the risky part. The PR does exactly that — and the fix for my earlier blocking finding is not a patch, it's a restructuring: loop-guard and API-error detection no longer scan a raw 20 KB output tail, they read the parsed terminal What I verified at this head:
The one commit that postdates the maintainer's real-stack verification ( No blocking findings. Two non-blocking follow-ups, both already named in the maintainer's verification: the idle-timeout failure text still says "the sandbox likely hung at startup" although the likeliest cause is now a long tool call, and a single tool call longer than the idle window is still invisible mid-flight (inherent to stream-json granularity — turn boundaries tick now, tool progress events don't exist yet). Neither belongs in this PR. CI test evidence (fetched once via API for the reviewed commit; unattended runs never execute PR code): Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Behavioural substantiation. The central claim no longer rests on the author's word: the maintainer drove the real bundled CLI against the real runner with a mock provider at 中文说明代码审查（对 不看 diff 时我的独立思路与上轮相同：spawn 时传流式参数、用回归测试锁定它们，然后逐一审计 agent 输出的所有下游读取者——因为格式变更才是风险所在。PR 正是这么做的——而且对我此前阻塞项的修复不是打补丁，而是重构：循环守卫与 API 错误检测不再扫描 20 KB 的原始输出尾部，改为读取解析后的 terminal 在当前 head 上核实的内容：
唯一晚于 maintainer 真实环境验证的 commit（ 无阻塞项。两个非阻塞的后续跟进，maintainer 的验证里也都点到了：idle-timeout 的失败文案仍写着 "the sandbox likely hung at startup"，而现在最可能的原因其实是长工具调用；单次工具调用如果超过 idle window，执行中途仍然不可见（受限于 stream-json 的粒度——现在回合边界会计时，但工具进度事件尚不存在）。两者都不属于本 PR 的范围。 CI 测试证据（通过 API 对被审 commit 一次性抓取；无人值守运行不会执行 PR 代码）：抓取时 行为佐证。 核心行为结论不再依赖作者自述：maintainer 已在本线程用真实打包 CLI + 真实 runner + mock provider（在 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the earlier blocker is resolved cleanly and substantiated on the real stack by the maintainer; two non-blocking follow-ups remain (failure-text wording, single long tool call still invisible mid-flight), neither attributable to this diff's choices. Stepping back: last round I asked for one change — the exit-0 API-error case was silently dropping the retry sentinel. The author didn't patch around it; they moved detection from raw-tail substring matching onto the parsed terminal result event, which closes a whole false-positive class I hadn't even raised (markers inside streamed tool results), and each of the four follow-up commits answers a specific, named concern. The maintainer then mutation-tested every piece against the real bundled CLI with a mock provider and called all of them load-bearing — that is exactly the evidence this PR needed, and it's in this thread. The only commit after that verification is a strict tightening of the oversized-line bound, which I reviewed line by line and which carries its own tests. If I had to maintain this in six months, the runner's output handling reads as a coherent protocol parser with bounded buffers and an artifact that keeps everything — thank-them material, not curse-them material. My honest remaining hesitation is only that CI on this exact head hasn't landed yet, and the unit suite is the one signal I haven't seen for So: approval deferred until CI lands green on 中文说明置信度：4/5 —— 此前的阻塞项已被干净地解决，并由 maintainer 在真实环境中佐证；剩两个非阻塞的后续跟进（失败文案措辞、单次长工具调用中途仍不可见），都不是本 diff 自身选择造成的。 整体来看：上一轮我只提了一个修改要求——退出码 0 的 API 错误会悄悄丢掉重试哨兵。作者没有绕着它打补丁，而是把检测从"原始输出尾部子串匹配"移到解析后的 terminal result 事件上，顺带关闭了我甚至没有提出的一整类误报（流式 tool_result 内容里出现的标记），并且四个后续 commit 每一个都对应一条明确的、点过名的顾虑。之后 maintainer 用真实打包 CLI + mock provider 对每一部分做了变异测试，确认全部是必要的——这正是本 PR 需要的证据，就在本线程里。那次验证之后唯一的 commit 是对超大行边界的严格收紧，我逐行审过，且自带测试。 如果六个月后由我来维护，runner 的输出处理读起来像一个协议解析器：缓冲有界、artifact 完整保留——是值得感谢而不是抱怨的代码。我唯一诚实的保留是：这个 head 的 CI 还没跑完，单测套件是我在 所以：批准推迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Needs one change before merge — switching the headless run to stream-json alters the exit-code semantics of mid-round API errors, which drops the agent-api-error retry sentinel and strands transient failures. Full chain and a suggested fix are in my review comment above. 🙏
|
已修复 + 验证证据：9b304a2e73；新增回归先红后绿，相关 API-error/idle 回归 9/9、独立验证 25 个唯一用例通过；完整 workflow 文件 151/151 断言通过（Vitest 汇总后仍触发已知 onTaskUpdate RPC timeout）；Prettier、node --check、git diff --check 通过。 |
doudouOUC
left a comment
doudouOUC
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.
中文说明
已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
| const presentOutputs = spec.outputs.filter( | ||
| (name) => !missingOutputs.includes(name), | ||
| ); | ||
| const hasOutputVerdict = spec.anyOutput |
There was a problem hiding this comment.
[Suggestion] hasOutputVerdict and ok compute the same boolean with different expressions, creating a maintenance hazard.
hasOutputVerdict (line 413: spec.anyOutput ? presentOutputs.length > 0 : missingOutputs.length === 0) and ok (line 516: spec.anyOutput ? missingOutputs.length < spec.outputs.length : missingOutputs.length === 0) are mathematically equivalent — presentOutputs.length = spec.outputs.length - missingOutputs.length, so presentOutputs.length > 0 equals missingOutputs.length < spec.outputs.length. A future change to output-verification semantics must update both in lockstep; the different formulations make it easy to miss one.
| const hasOutputVerdict = spec.anyOutput | |
| const ok = hasOutputVerdict; |
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
中文说明
hasOutputVerdict（第 413 行：spec.anyOutput ? presentOutputs.length > 0 : missingOutputs.length === 0）和 ok（第 516 行：spec.anyOutput ? missingOutputs.length < spec.outputs.length : missingOutputs.length === 0）在数学上是等价的——presentOutputs.length = spec.outputs.length - missingOutputs.length，所以 presentOutputs.length > 0 等价于 missingOutputs.length < spec.outputs.length。未来对输出验证语义的修改需要同时更新两个表达式，不同的表述方式容易遗漏其一。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
已修复 + 验证证据：聚焦分类用例 10/10 通过；完整 workflow 154/154 断言通过。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; This PR switches the autofix headless agent to streamed o...: none — all checks I started completed within budget..
Not reviewed: verification and reverse audit — each ran and opened its brief, but neither was launched with the prompt the CLI built — the launches were written by hand, so the posted findings cannot be counted as verified, and what the agents were actually asked is not what this skill certifies.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；This PR switches the autofix headless agent to streamed o...：none — all checks I started completed within budget.。
未审查：验证与反向审计——两者都运行并打开了各自的 brief，但都不是用 CLI 构建的 prompt 启动的——启动 prompt 是手写的，发布的发现不能算作已验证，agent 实际被要求做的也不是本 skill 所认证的内容。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| const apiErrorWithoutVerdict = | ||
| result.status === 0 && | ||
| result.apiError && |
There was a problem hiding this comment.
[Critical] The new exit-0 gate trusts recoverableApiError()'s raw-byte regex over the child's stdout tail — but this PR's --output-format stream-json switch makes that stdout carry every user/tool_result envelope, including the full content of files and PR text the agent only read. JSON does not escape [ or ], so a literal [API Error: 429 quota exceeded] inside read content matches the marker regex, and an exit-0 run without complete outputs is then classified as a retryable API failure: failure.md embeds the marker, both agent-api-error/agent-api-error-kind sentinels are written, and the run exits 1.
Failure scenario: a PR under autofix contains the marker text in a file the agent reads late in the run (within the 20 KB tail), and the round exits 0 without complete outputs → the workflow takes the cause-aware retry branch instead of the terminal branch: the watermark does not advance, the marker's first line is echoed into the PR failure comment (AutoFix could not reach the model — …), and every retry reproduces the condition — burning rounds until the caps and stranding the feedback. No attacker is strictly required: this repository's own sources (e.g. errorParsing.test.ts, and this PR's own new test) contain the marker string. Probe-verified A/B against the merge base: the identical input on the base tree was a terminal finished without required output file(s) with no sentinels — this classification flip is introduced by this diff.
Suggested fix: corroborate before treating an exit-0 run as an API failure — parse the tail line-wise as JSON and only accept the marker from assistant/system-emitted content (excluding user/tool_result message lines), or require the terminal result envelope before setting result.apiError on the status-0 path.
中文说明
新的 exit-0 分支依赖 recoverableApiError() 对子进程 stdout 尾部做原始字节正则匹配——但本 PR 切换到 --output-format stream-json 后，该 stdout 会携带每一条 user/tool_result 消息，包括 agent 只是读到的文件与 PR 全文内容。JSON 不会转义 [ 和 ]，因此被读取内容里的字面 [API Error: 429 quota exceeded] 也会命中 marker 正则，于是"退出码 0 且缺少完整输出"的运行会被归类为可重试的 API 失败：failure.md 写入该 marker、agent-api-error/agent-api-error-kind 两个哨兵文件被写出、进程以 1 退出。
失败场景：被 autofix 托管的 PR 在某个文件中埋有该 marker 文本，agent 在运行后期（尾部 20 KB 窗口内）读到它，且该轮以退出码 0 结束、没有产出完整输出 → workflow 进入按原因分类的重试分支而不是终止分支：水位线不前进，marker 首行被回显到 PR 失败评论（AutoFix could not reach the model — …），且每次重试都会复现该条件——一直烧到轮次上限、feedback 被搁置。严格来说甚至不需要攻击者：本仓库自身的源码（如 errorParsing.test.ts，以及本 PR 新增的测试）就包含该 marker 字符串。已通过对照 merge base 的 A/B 探针验证：相同输入在 base 树上是不带哨兵的终止性 finished without required output file(s)——这一分类翻转正是本 diff 引入的。
建议修复：在把 exit-0 运行当作 API 失败之前增加佐证——将尾部按行做 JSON 解析，只接受来自 assistant/system 输出内容的 marker（排除 user/tool_result 消息行），或在 status-0 路径上要求终端 result 事件成立后才置位 result.apiError。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：聚焦分类用例 10/10 通过；完整 workflow 154/154 断言通过。
| '--output-format', | ||
| 'stream-json', | ||
| '--include-partial-messages', |
There was a problem hiding this comment.
[Critical] stream-json puts every message — including tool_result payloads carrying the full file/shell/PR content the agent read — on the child's stdout, and isLoopGuardOutput() is a bare includes() scan of that now-untrusted byte stream (folded into outputTail by record(), latched permanently). Read content containing turn_tool_call_cap or Loop detection halted the run latches loopDetected; in the failure branch the loop arm precedes the sentinel-writing arm, so handoff.md is written and the agent-api-error/agent-timeout sentinels are skipped. No attacker needed: both marker strings live in this very repository, which the fleet dogfoods (Loop detection halted the run in packages/cli/src/nonInteractiveCli.ts:205, turn_tool_call_cap in packages/core/src/telemetry/types.ts:487 and loop-detection sources).
Failure scenario: an autofix round reads a file containing either literal, then hits a mundane transient 429 / timeout / non-zero exit → the round is classified as terminal tool-call-loop: the watermark advances past the feedback item (consumed, never retried) and the PR comment posts the false diagnosis "needs a human", where the sentinel path would have retried with the cause-aware budget. Probe-confirmed A/B: identical 429+exit-1 inputs differing only in one marker-bearing tool_result envelope — without it the run is transient-retryable with sentinels; with it, all sentinels absent. Complements the exit-0-gate finding above: that one fabricates retries; this one suppresses a warranted retry.
Suggested fix: derive loop detection from the structured stream — parse stdout line-wise as JSON and set loopDetected only from the terminal result event (isError + loop-guard errorMessage), never from a substring match anywhere in the byte stream. Genuine loop detection is preserved: emitLoopDetectedResult emits both markers inside that terminal envelope in stream-json too.
中文说明
stream-json 会把每一条消息——包括携带 agent 所读取的文件/shell/PR 全文内容的 tool_result 载荷——都写到子进程的 stdout 上，而 isLoopGuardOutput() 是对这条如今不可信的字节流做裸 includes() 扫描（由 record() 汇入 outputTail，一旦置位即永久锁存）。被读取的内容只要含有 turn_tool_call_cap 或 Loop detection halted the run，就会锁存 loopDetected；在失败分支里 loop 分支先于哨兵写入分支执行，于是写出 handoff.md、跳过 agent-api-error/agent-timeout 哨兵。无需攻击者：这两个 marker 字符串就存在于本仓库——而这套 fleet 正以本仓库为 dogfood（Loop detection halted the run 见 packages/cli/src/nonInteractiveCli.ts:205，turn_tool_call_cap 见 packages/core/src/telemetry/types.ts:487 与循环检测源码）。
失败场景：某一轮 autofix 读取了包含任一 marker 的文件，随后遇到普通的瞬时 429 / 超时 / 非零退出 → 该轮被归类为终止性 tool-call-loop：水位线越过该 feedback 条目（被消费、不再重试），PR 评论给出"需要人工接管"的错误诊断；而走哨兵路径本可以按原因分类的预算重试。已通过 A/B 探针确认：完全相同的 429+exit-1 输入，唯一差异是一个携带 marker 的 tool_result 消息——没有它时运行被归为可重试的瞬时错误并写出哨兵；有它时所有哨兵缺失。与上面 exit-0 分支的问题互为镜像：那一个伪造重试，这一个压制本应发生的重试。
建议修复：从结构化流推导循环检测——把 stdout 按行解析为 JSON，仅在终端 result 事件（isError + 循环守卫的 errorMessage）上置位 loopDetected，而不是在字节流的任意位置做子串匹配。真实的循环检测不受影响：stream-json 下 emitLoopDetectedResult 同样会把两个 marker 放进该终端消息。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：聚焦分类用例 10/10 通过；完整 workflow 154/154 断言通过。
| const hasOutputVerdict = spec.anyOutput | ||
| ? presentOutputs.length > 0 | ||
| : missingOutputs.length === 0; |
There was a problem hiding this comment.
[Suggestion] The output-contract predicate is now spelled twice in this file in two formulations: hasOutputVerdict here, and the unchanged ok computation below (~line 515: spec.anyOutput ? missingOutputs.length < spec.outputs.length : missingOutputs.length === 0). They are equivalent today only because presentOutputs is the exact complement of missingOutputs — but they gate different things: the new exit-0 API-error retry branch vs the missing-output success branch.
Concrete cost: any future change to the output contract (a fourth mode, an output that may legitimately be empty, altered anyOutput semantics) must be applied to both formulations; updating one and missing the other silently desyncs the retry gate from the success gate — misclassifying rounds in a mechanism whose whole purpose is correct classification. Verified behavior-preserving: every branch between the two computations exits, and nothing between them writes a spec output file.
Suggested fix: reuse the hoisted value at the later site — replace the ok computation with const ok = hasOutputVerdict;.
中文说明
输出契约谓词现在在本文件中以两种写法出现了两次：此处的 hasOutputVerdict，以及下方未改动的 ok 计算（约第 515 行：spec.anyOutput ? missingOutputs.length < spec.outputs.length : missingOutputs.length === 0）。二者目前等价仅仅因为 presentOutputs 是 missingOutputs 的精确补集——但它们把守不同的分支：新的 exit-0 API 错误重试分支 vs 缺输出的成功分支。
具体代价：未来任何对输出契约的修改（第四种模式、某个允许为空的输出、anyOutput 语义变化）都必须同时改两处写法；只改一处会让重试分支与成功分支悄悄失步——在一个以正确分类为全部职责的机制里造成轮次误分类。已验证复用是行为无关的：两处计算之间的所有分支都会退出进程，且中间没有任何代码写 spec 输出文件。
建议修复：在后面一处复用已提升的值——把 ok 的计算替换为 const ok = hasOutputVerdict;。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：聚焦分类用例 10/10 通过；完整 workflow 154/154 断言通过。
| it('flags a recoverable stream-json API error even when qwen exits zero', () => { | ||
| withRunnerDir((dir) => { | ||
| writeFileSync(join(dir, 'feedback.md'), 'feedback\n'); |
There was a problem hiding this comment.
[Suggestion] The verdict guards on the new exit-0 branch — !hasOutputVerdict and !existsSync(failure.md) in apiErrorWithoutVerdict — have no negative test: no test streams a recoverable API-error marker from a stub that ALSO writes a verdict file and exits 0. Probe-verified: with the guards intact such a run exits 0 with no failure.md and no sentinels; deleting !hasOutputVerdict && makes the identical input exit 1 with failure.md + both sentinels despite the verdict file existing — and that mutation survives the current suite.
Failure scenario: with --include-partial-messages a run can render a transient [API Error: 429 …] mid-stream (the CLI retries 429s with backoff), then recover, write address-summary.md, and exit 0. The guards exist precisely so that round is a success; a regression deleting either guard would turn recovered, verdict-producing rounds into failures — burning a round, stalling the watermark, and eventually tripping the consecutive-failure cap on a PR that was actually addressed.
Suggested fix — add a negative test next to this one:
it('keeps a recovered exit-zero run with a verdict out of the API-error retry path', () => {
withRunnerDir((dir) => {
writeFileSync(join(dir, 'feedback.md'), 'feedback\n');
const stub = writeWorkdirStub(dir, [
"process.stdout.write('[API Error: 429 quota exceeded]\\n');",
"writeFileSync(workdir + '/address-summary.md', 'summary\\n');",
'process.exit(0);',
]);
const result = runAddressReview(dir, stub);
expect(result.status).toBe(0);
expect(existsSync(join(dir, 'failure.md'))).toBe(false);
expect(existsSync(join(dir, 'agent-api-error'))).toBe(false);
});
});中文说明
新 exit-0 分支上的两个裁决守卫——apiErrorWithoutVerdict 中的 !hasOutputVerdict 与 !existsSync(failure.md)——没有负向测试：没有任何用例让 stub 在输出可恢复 API 错误 marker 的同时写入裁决文件并以 0 退出。探针验证：守卫完好时这种运行以 0 退出、不产生 failure.md 与哨兵；删掉 !hasOutputVerdict && 后，相同输入在裁决文件存在的情况下仍以 1 退出并写出 failure.md + 两个哨兵——而该突变在当前测试套件中存活。
失败场景：启用 --include-partial-messages 后，一次运行可能在中途渲染瞬时 [API Error: 429 …]（CLI 会带退避地重试 429），随后恢复、写出 address-summary.md 并以 0 退出。这两个守卫正是为了让该轮被记为成功；任何删掉任一早手的回归都会把已恢复且产出裁决的轮次变成失败——浪费一轮、卡住水位线，并最终在一个实际已被处理的 PR 上触发连续失败上限。
建议修复——在此测试旁补一个负向用例（代码见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：聚焦分类用例 10/10 通过；完整 workflow 154/154 断言通过。
| expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( | ||
| '[API Error: 429 quota exceeded]', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The only new operator-facing string this diff introduces — 'recoverable API error without an agent verdict' — is asserted nowhere; this test's failure.md assertion matches only the ${result.apiError} suffix that the pre-existing non-zero-exit path already produces. Probe-verified: replacing the new detail ternary arm with false degrades failure.md to Qwen failed during address-review: status 0. [API Error: …] and all four of this test's assertions still pass. The suite elsewhere pins operator-facing wording for exactly this reason (cf. the idle-timeout (no output for 1200ms assertions).
| expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( | |
| '[API Error: 429 quota exceeded]', | |
| ); | |
| expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( | |
| '[API Error: 429 quota exceeded]', | |
| ); | |
| expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( | |
| 'recoverable API error without an agent verdict', | |
| ); |
中文说明
本 diff 引入的唯一新运维可见字符串——'recoverable API error without an agent verdict'——没有任何断言覆盖；此测试对 failure.md 的断言只匹配到既有非零退出路径本就产生的 ${result.apiError} 后缀。探针验证：把新增的 detail 三元分支替换为 false 后，failure.md 退化为 Qwen failed during address-review: status 0. [API Error: …]，而本测试的全部四个断言仍然通过。套件在其他地方正是出于同样原因锁定运维可见措辞（参见 idle-timeout (no output for 1200ms 相关断言）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：聚焦分类用例 10/10 通过；完整 workflow 154/154 断言通过。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): Change summary: This PR switches the AutoFix headless age...: recommended build-lint gate not executed — the review worktree has no node_modules ( NO_NODE_MODULES confirmed) and npm install was not attempted within …; Change summary: This PR switches the AutoFix headless age...: full npm run test:scripts run of qwen-autofix-workflow.test.js to confirm the new tests are green in-suite (CI is presumed green; my review concerns what th…; Change summary: This PR switches the AutoFix headless age...: did not trace to completion the exact wording core loop detection surfaces as a subagent task's terminateReason (affects only the likelihood of the loop-marke…; Change summary: This PR switches the AutoFix headless age...: running scripts/tests/qwen-autofix-workflow.test.js under vitest (no node_modules in the worktree; npm ci not attempted) — substituted with the live stub-dr….
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：Change summary: This PR switches the AutoFix headless age...：recommended build-lint gate not executed — the review worktree has no node_modules ( NO_NODE_MODULES confirmed) and npm install was not attempted within …；Change summary: This PR switches the AutoFix headless age...：full npm run test:scripts run of qwen-autofix-workflow.test.js to confirm the new tests are green in-suite (CI is presumed green; my review concerns what th…；Change summary: This PR switches the AutoFix headless age...：did not trace to completion the exact wording core loop detection surfaces as a subagent task's terminateReason (affects only the likelihood of the loop-marke…；Change summary: This PR switches the AutoFix headless age...：running scripts/tests/qwen-autofix-workflow.test.js under vitest (no node_modules in the worktree; npm ci not attempted) — substituted with the live stub-dr…。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| const consumeStreamJson = (text, final = false) => { | ||
| stdoutCarry += text; | ||
| const lines = stdoutCarry.split('\n'); |
There was a problem hiding this comment.
[Suggestion] stdoutCarry accumulates all stdout bytes since the last newline with no size cap (the outputTail it replaced was bounded at 20 KB), and every arriving chunk re-concatenates and re-splits the entire carry even when no line completed. — Failure scenario: a wedged child emitting newline-free stdout refreshes lastOutputAt on every chunk, so the idle watchdog never fires and the carry grows for the entire absolute budget (default 50 min) — potentially GB-scale — until the runner OOMs with no failure.md and no sentinels; that is the pathological-child case the watchdog exists for. Large legitimate single lines (a tool_result embedding big file/shell output) also pay O(L²) string work across ~64 KB pipe chunks. Probe-confirmed: 40 MB newline-free stdout → 591 MB peak RSS vs 74 MB for a newline-terminated control.
Suggested fix — gate the split on an actually-completed line (the carry is newline-free by construction), and/or cap the carry and flush overflow into appendDiagnostic:
| const consumeStreamJson = (text, final = false) => { | |
| stdoutCarry += text; | |
| const lines = stdoutCarry.split('\n'); | |
| const consumeStreamJson = (text, final = false) => { | |
| stdoutCarry += text; | |
| if (!final && !text.includes('\n')) return; | |
| const lines = stdoutCarry.split('\n'); |
中文说明
stdoutCarry 会累积上一个换行符之后的全部 stdout 字节且没有大小上限（被它替换的 outputTail 有 20 KB 上限），而且即使没有凑齐一整行，每个到达的 chunk 也会把整个 carry 重新拼接并重新 split 一遍。— 失败场景：一个卡死的子进程持续输出不带换行的 stdout，每个 chunk 都会刷新 lastOutputAt，于是 idle watchdog 永远不触发，carry 在整个绝对预算（默认 50 分钟）内持续增长——可能达到 GB 级——直到 runner OOM 退出，既没有 failure.md 也没有任何哨兵；这正是 watchdog 要防的病态子进程场景。合法的大单行（嵌入大文件/shell 输出的 tool_result）也会在以 ~64 KB pipe chunk 送达时付出 O(L²) 的字符串处理代价。已用探针确认：40 MB 无换行 stdout → 峰值 RSS 591 MB，而带换行的对照仅 74 MB。
建议修复——只在确实凑齐了一行时才 split（carry 按构造不含换行），和/或给 carry 加上限并把溢出部分 flush 到 appendDiagnostic（见上方 suggestion）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：stdout carry 受 1 MiB 上限约束且无语义字节流不再续命；相关用例 10/10、完整 workflow 158/158 断言通过。
| const apiErrorWithoutVerdict = | ||
| result.status === 0 && | ||
| result.apiError && |
There was a problem hiding this comment.
[Suggestion] The exit-0 API-error gate authorizes the retryable classification by pattern-matching the [API Error: …] marker in text the model itself controls — Failure scenario: on exit 0 streamResultOutput(terminalResult) reduces to the model's last assistant message, so a prompt-injected run (the agent reads attacker-controlled PR files / feedback.md) that writes no verdict can end its final message parroting a literal [API Error: 429 quota exceeded] from read content → apiErrorWithoutVerdict fires → agent-api-error sentinel (kind transient) → capped retries instead of terminating at finished without required output file(s). Impact is bounded (the round/consecutive-failure caps terminate persistent failure, forgery cannot produce success or watermark advance, and stalling into the absolute timeout already achieves the retryable classification without forgery), and the stream-json protocol carries no provenance signal to bind the marker to a real error — hence a documentation/tightening decision rather than a blocker.
Suggested fix — make the trade-off explicit at the gate:
| const apiErrorWithoutVerdict = | |
| result.status === 0 && | |
| result.apiError && | |
| // NOTE: on exit 0 the marker comes from the model's own final message, which | |
| // a prompt-injected run can parrot; bounded by the transient-retry caps, and | |
| // stalling into the absolute timeout achieves the same retryable classification. | |
| const apiErrorWithoutVerdict = | |
| result.status === 0 && | |
| result.apiError && |
中文说明
exit-0 API 错误分支是靠在模型自己控制的文本里做模式匹配来批准"可重试"分类的——失败场景：exit 0 时 streamResultOutput(terminalResult) 实际上就是模型的最后一条 assistant 消息，因此一个被 prompt 注入的运行（agent 会读取攻击者可控的 PR 文件 / feedback.md）在不写裁决文件的情况下，可以在最后一条消息里复述所读内容中的字面 [API Error: 429 quota exceeded] → apiErrorWithoutVerdict 触发 → 写出 agent-api-error 哨兵（transient 类）→ 进入有上限的重试，而不是以 finished without required output file(s) 终止。影响是有界的（轮次/连续失败上限会终止持续性失败，伪造无法制造成功、推进水位线，且拖到绝对超时本就会得到同样的可重试分类），stream-json 协议里也没有可用的来源信号把 marker 绑定到真实错误——因此这是一个"写清楚/收紧"的决策，而非阻塞项。
建议修复——在该分支处把这一取舍写明（见上方 suggestion）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| consumeStreamJson('', true); | ||
| const terminalOutput = streamResultOutput(terminalResult); |
There was a problem hiding this comment.
[Suggestion] No test exercises consumeStreamJson's line-reassembly: every stub writes complete \n-terminated result lines in a single write, so both the cross-chunk carry (stdoutCarry) and the final unterminated-line flush here are unpinned. — Failure scenario: a future simplification dropping this final flush or discarding the carry survives every test in the suite; afterwards a chunk-split result event is never parsed, terminalResult stays undefined, and the exit-0 API-error gate silently stops retrying quota/5xx failures — stranding PRs exactly as before this PR. Both shapes were probe-confirmed to trip the gate correctly today, so this is a missing pin on working code.
Suggested fix — next to the other exit-0 tests in scripts/tests/qwen-autofix-workflow.test.js:
it('classifies a chunk-split result line on exit zero', () => {
withRunnerDir((dir) => {
writeFileSync(join(dir, 'feedback.md'), 'feedback\n');
const stub = writeWorkdirStub(dir, [
"const line = qwenResultLine({ result: '[API Error: 429 quota exceeded]' });",
'process.stdout.write(line.slice(0, 20));',
'setTimeout(() => {',
' process.stdout.write(line.slice(20));',
' process.exit(0);',
'}, 300);',
]);
const result = runAddressReview(dir, stub);
expect(result.status).toBe(1);
expect(existsSync(join(dir, 'agent-api-error'))).toBe(true);
});
});中文说明
没有任何测试覆盖 consumeStreamJson 的跨块重组：所有 stub 都在一次 write 中写出完整的、以换行结尾的 result 行，因此跨 chunk 的 carry（stdoutCarry）与这里的"末尾未换行行 flush"都没有被测试锁定。— 失败场景：未来某个"简化"删掉这个 final flush 或丢弃 carry，可以通过现有全部测试；之后被 chunk 拆开的 result 事件将永远不会被解析，terminalResult 保持 undefined，exit-0 API 错误分支会悄悄停止对 quota/5xx 失败的重试——PR 会像本 PR 修复之前一样被搁置。两种形态今天都能正确触发该分支（已用探针确认），所以这只是对工作代码缺一个锁定测试。
建议修复——在 scripts/tests/qwen-autofix-workflow.test.js 的其他 exit-0 用例旁补上面的用例。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：跨 chunk 与无换行终止 result 均有回归覆盖；相关用例 10/10、完整 workflow 158/158 断言通过。
|
|
||
| function isLoopGuardResult(event) { | ||
| return ( | ||
| event?.is_error === true && isLoopGuardOutput(streamResultOutput(event)) |
There was a problem hiding this comment.
[Suggestion] The is_error === true guard in isLoopGuardResult — the deliberate narrowing from round 1 — has no pinning test: deleting the condition survives the whole suite (all 154 tests), because every loop-guard test uses isError: true. — Failure scenario: an agent whose final SUCCESS text quotes a loop marker (e.g. a summary echoing grepped log output containing Loop detection halted the run) combined with a non-zero exit would then be classified loop-guard — writing the terminal handoff.md/human-takeover instead of the ordinary retryable failure path. Probe-confirmed: the mutant survives 154/154, and the test below passes on current code and flips against the mutant.
Suggested fix:
it('does not treat a successful result quoting a loop marker as loop-detected', () => {
withRunnerDir((dir) => {
writeFileSync(join(dir, 'feedback.md'), 'feedback\n');
const stub = writeWorkdirStub(dir, [
"process.stdout.write(qwenResultLine({ result: 'saw \"Loop detection halted the run\" in logs' }));",
'process.exit(1);',
]);
const result = runAddressReview(dir, stub);
expect(result.status).toBe(1);
expect(existsSync(join(dir, 'handoff.md'))).toBe(false);
expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain(
'Qwen failed during address-review',
);
});
});中文说明
isLoopGuardResult 中的 is_error === true 守卫——第 1 轮修复时刻意做的收窄——没有锁定测试：删掉这个条件后整套测试（154 个）依然全部通过，因为所有 loop-guard 测试都使用 isError: true。— 失败场景：一个最终 SUCCESS 文本里引用了 loop marker 的 agent（例如总结里复述 grep 到的日志输出，其中含有 Loop detection halted the run），再叠加非零退出，就会被归类为 loop-guard——写出终止性的 handoff.md/人工接管，而不是走普通的可重试失败路径。已用探针确认：该突变体在 154/154 下存活，而上面的用例在当前代码上通过、对突变体翻转。
建议修复——补上面的用例。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| const apiErrorInfo = recoverableApiError( | ||
| result.status === 0 | ||
| ? terminalOutput | ||
| : `${diagnosticTail}\n${terminalOutput}`, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The exit-0 branch's deliberate exclusion of diagnosticTail (stderr + unparseable stdout) is untested: a mutant that includes diagnosticTail on status 0 survives every test (no test writes an [API Error marker to stderr with exit 0). — Failure scenario: in production, tool/debug output printing [API Error: 429 …] to stderr while the run exits 0 without a verdict would flip a terminal missing-output failure into the transient-retry path — re-running the round against noise instead of writing finished without required output file(s). The status≠0 inclusion IS pinned; the status-0 exclusion is not. Probe-confirmed: the mutant survives 154/154 and the test below flips it.
Suggested fix:
it('keeps an exit-zero run with a stderr-only marker out of the retry path', () => {
withRunnerDir((dir) => {
writeFileSync(join(dir, 'feedback.md'), 'feedback\n');
const stub = writeWorkdirStub(dir, [
"process.stderr.write('[API Error: 429 quota exceeded]\\n');",
'process.exit(0);',
]);
const result = runAddressReview(dir, stub);
expect(result.status).toBe(1);
expect(existsSync(join(dir, 'agent-api-error'))).toBe(false);
expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain(
'finished without required output file(s)',
);
});
});中文说明
exit-0 分支刻意排除 diagnosticTail（stderr + 无法解析的 stdout）这一点没有测试覆盖：把 diagnosticTail 也并入 status 0 分类的突变体能通过全部测试（没有任何用例在 exit 0 时向 stderr 写 [API Error marker）。— 失败场景：生产环境中工具/调试输出在 stderr 打印 [API Error: 429 …]、而运行以 0 退出且没有裁决文件时，会把终止性的"缺输出"失败翻转成瞬时重试路径——对着噪声重跑一轮，而不是写 finished without required output file(s)。status≠0 的包含关系是有测试锁定的；status-0 的排除没有。已用探针确认：突变体在 154/154 下存活，上面的用例能使其翻转。
建议修复——补上面的用例。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| '--output-format', | ||
| 'stream-json', | ||
| '--include-partial-messages', |
There was a problem hiding this comment.
[Suggestion] --include-partial-messages turns the child's stdout into a per-token delta stream, and the unchanged stream.write(chunk) forwarder in record() mirrors all of it into the runner's stdout — i.e., the GitHub Actions step log — duplicating agent.log 1:1. — Failure scenario: measured with a stub streaming one 10 KB write_file as input_json_deltas: 74,271 bytes of step stdout next to a byte-identical 74,271-byte agent.log (7.43 step-log bytes per byte of file content — full file contents stream through input_json_delta). A develop-issue run writing a multi-file fix multiplies step-log volume many-fold versus TEXT mode, increasing Actions log storage/egress and slowing log rendering for the 50-minute budget runs, while agent.log already retains the complete record and is uploaded with the WORKDIR artifact. Filtering what is forwarded leaves watchdog refresh untouched (lastOutputAt is set before routing).
Suggested fix — forward only non-stream_event lines to process.stdout (keep writing the full stream to agent.log), e.g. route stdout through the already-parsed lines in consumeStreamJson and echo only the ones that are not stream_events:
// in record(), replace the unconditional stream.write(chunk) for stdout:
if (source === 'stderr') stream.write(chunk);
// stdout echo happens per parsed line inside consumeStreamJson,
// skipping lines whose parsed event type is 'stream_event'中文说明
--include-partial-messages 会把子进程的 stdout 变成逐 token 的 delta 流，而 record() 里未改动的 stream.write(chunk) 转发会把这些全部镜像到 runner 的 stdout——也就是 GitHub Actions 的 step 日志——与 agent.log 1:1 重复。— 失败场景：用 stub 以 input_json_delta 流式输出一次 10 KB 的 write_file 实测：step stdout 为 74,271 字节，旁边是字节级完全相同的 74,271 字节 agent.log（每字节文件内容对应 7.43 字节的 step 日志——完整文件内容会经由 input_json_delta 流过）。一次写多文件修复的 develop-issue 运行相比 TEXT 模式会把 step 日志体积放大许多倍，增加 Actions 日志存储/流量开销、拖慢这些 50 分钟预算运行的日志渲染，而 agent.log 本就保留完整记录并随 WORKDIR 工件上传。过滤"转发内容"不影响 watchdog 刷新（lastOutputAt 在路由之前就已设置）。
建议修复——只把非 stream_event 行转发到 process.stdout（完整流继续写入 agent.log），例如让 stdout 经由 consumeStreamJson 中已解析的行输出、跳过 stream_event 类型的行（见上方示意）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：stream_event 仅保留在 agent.log，不再复制到 step stdout；相关用例 10/10、完整 workflow 158/158 断言通过。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): Change summary: This PR switches the AutoFix headless age...: none — all planned checks completed within budget.; Change summary: This PR switches the AutoFix headless age...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; Change summary: This PR switches the AutoFix headless age...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget..
中文说明
未探索到全部深度（达到工具调用预算）：Change summary: This PR switches the AutoFix headless age...：none — all planned checks completed within budget.；Change summary: This PR switches the AutoFix headless age...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；Change summary: This PR switches the AutoFix headless age...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| appendDiagnostic(`${stdoutCarry}${part.slice(0, remaining)}`); | ||
| stdoutCarry = ''; | ||
| discardingOversizedStdoutLine = true; |
There was a problem hiding this comment.
[Critical] R3-1: The oversized-line discard path feeds the first 1 MiB of an untrusted stream-json stdout line into diagnosticTail, which recoverableApiError() scans on every non-zero exit — re-opening, for lines over 1 MiB, the raw-byte marker-matching class the round-2 Criticals addressed and the new ignores ... markers from streamed tool results tests pin only below 1 MiB. — Failure scenario: a tool result or file read serialized into a single stdout line over 1 MiB carries a literal [API Error: 429 ...] string (this repo's own test sources contain ~48 such literals, so no attacker is required), and the run then exits non-zero for an unrelated reason (crash/OOM/signal) → probe-confirmed A/B at this commit: agent-api-error + agent-api-error-kind=transient sentinels are written and failure.md blames an API error that never happened, so the workflow retries a permanent failure on the transient budget instead of failing terminally; with a placeholder instead of the raw bytes both sentinels are absent. agent.log already retains the full raw bytes via log.write(chunk) and nothing else reads diagnosticTail, so the untrusted prefix can be dropped without information loss.
| appendDiagnostic(`${stdoutCarry}${part.slice(0, remaining)}`); | |
| stdoutCarry = ''; | |
| discardingOversizedStdoutLine = true; | |
| appendDiagnostic('[oversized stream-json line dropped; see agent.log]\n'); | |
| stdoutCarry = ''; | |
| discardingOversizedStdoutLine = true; |
中文说明
超长行丢弃分支会把一条不可信 stream-json stdout 行的前 1 MiB 喂进 diagnosticTail，而 recoverableApiError() 在每次非零退出时都会扫描它——对于超过 1 MiB 的行，这重新打开了第 2 轮两个 Critical 所处理过的"对不可信流做原始字节 marker 匹配"这一缺陷类别（新增的 ignores ... markers from streamed tool results 测试只钉住了 1 MiB 以下的情况）。失败场景：某个被序列化为单条 stdout 行且超过 1 MiB 的 tool result 或文件读取内容中带有字面 [API Error: 429 ...] 字符串（本仓库自身的测试源码里就有约 48 处该字面量，因此甚至不需要攻击者），随后进程因无关原因（崩溃/OOM/信号）非零退出 → 已在当前 commit 上通过 A/B 探针确认：agent-api-error + agent-api-error-kind=transient 哨兵被写出、failure.md 归因于从未发生的 API 错误，workflow 于是按瞬时预算重试一个永久性失败；若以占位符替代原始字节，则两个哨均缺失。agent.log 已通过 log.write(chunk) 保留全部原始字节，且除 recoverableApiError 外没有别的读取者，因此丢弃这段不可信前缀不损失任何信息。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：5045d8a370；超大 tool_result 误分类回归先红后绿，相关 4/4 通过；node --check、Prettier、ESLint、git diff --check 通过。
| function isLoopGuardResult(event) { | ||
| return ( | ||
| event?.is_error === true && isLoopGuardOutput(streamResultOutput(event)) | ||
| ); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R2-4: The deliberate is_error === true narrowing here has no pinning test (round-2 comment, still standing). Verified at this commit: no test emits a SUCCESS (is_error: false) result event carrying the loop markers in its result text and asserts non-loop classification — ignores loop-guard markers from streamed tool results puts the markers in a tool_result envelope and its terminal event carries an API-error message without loop markers, so a mutant deleting the is_error === true condition survives it too (and the whole suite). — Failure scenario: if a later edit drops the condition (e.g. to also catch loop markers quoted in success text), a run whose final model response merely QUOTES Loop detection halted the run while succeeding is misclassified as loop-detected: handoff.md is written, the round is reported as needing a human, and the watermark advances past feedback the agent had actually evaluated — no test fails to warn. Suggested fix: add a test whose stub emits qwenResultLine({ result: 'turn_tool_call_cap Loop detection halted the run' }) (is_error defaults to false) and writes its verdict, exit 0; assert status 0 and no handoff.md.
中文说明
此处有意添加的 is_error === true 收窄没有钉住它的测试（第 2 轮评论，仍然成立）。已在当前 commit 上核实：没有任何测试会发出一个携带 loop marker 但 is_error: false 的 SUCCESS result 事件并断言其不被判为循环——ignores loop-guard markers from streamed tool results 把 marker 放在 tool_result 消息里，且其终端事件携带的是 API 错误消息（不含 loop marker），因此删除 is_error === true 条件的变异体在该测试乃至全套用例下都能存活。失败场景：若后续修改删掉该条件（例如想同时捕获成功文本中引用的 loop marker），一次实际上成功的运行仅因模型最终回复"引用"了 Loop detection halted the run 就会被误判为循环检测：写出 handoff.md、该轮被报告为需要人工接管、水位线越过 agent 其实已评估的 feedback——且没有任何测试会失败示警。建议修复：新增测试，stub 发出 qwenResultLine({ result: 'turn_tool_call_cap Loop detection halted the run' })（is_error 默认 false）并写出 verdict、以 0 退出；断言 status 为 0 且无 handoff.md。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( | ||
| 'recoverable API error without an agent verdict', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R2-5: The exit-0 branch's deliberate exclusion of diagnosticTail (stderr + unparseable stdout) from API-error scanning is pinned by no test (round-2 comment, still standing). Probe-verified mutant at this commit: replacing the conditional scan with an unconditional recoverableApiError(${diagnosticTail}\n${terminalOutput}) still passes the full suite (1118/1118). — Failure scenario: with the mutant, a stub writing [API Error: 429 quota exceeded] to stderr and exiting 0 with no verdict writes agent-api-error (spurious transient retry); the original correctly fails terminal with finished without required output file(s). If that regression shipped, stderr noise on an exit-0 run would trigger spurious retry rounds and no test would catch it. Suggested fix: add a test — stub writes [API Error: 429 quota exceeded]\n to stderr (no result line, no outputs), exits 0; assert non-zero status, agent-api-error absent, failure.md says finished without required output file(s).
中文说明
exit-0 分支有意把 diagnosticTail（stderr + 无法解析的 stdout）排除在 API 错误扫描之外，但没有任何测试钉住这一点（第 2 轮评论，仍然成立）。已在当前 commit 上以探针验证变异体：把条件式扫描替换为无条件的 recoverableApiError(${diagnosticTail}\n${terminalOutput})，全套用例仍 1118/1118 通过。失败场景：在该变异体下，一个把 [API Error: 429 quota exceeded] 写到 stderr、以 0 退出且无 verdict 的 stub 会写出 agent-api-error（诱发错误的瞬时重试）；而原始代码会正确地以 finished without required output file(s) 终止。若该回归被引入，exit-0 运行中的 stderr 噪音将触发无谓的重试轮次，且没有测试能抓住它。建议修复：新增测试——stub 向 stderr 写入 [API Error: 429 quota exceeded]\n（无 result 行、无输出文件）、以 0 退出；断言状态非零、agent-api-error 不存在、failure.md 为 finished without required output file(s)。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| )});`, | ||
| "process.stdout.write('x'.repeat(21_000));", |
There was a problem hiding this comment.
[Suggestion] R3-2: The enclosing test detects loop guard output before it falls out of the log tail keeps the old mechanism's name and this 21,000-byte scroll-out payload after this diff removed tail-based detection — loop detection now keys solely on the terminal result event (isLoopGuardResult(terminalResult)); there is no tail anything can "fall out of". — Failure scenario: the test passes for a different reason than its name claims; a future maintainer reading the suite would conclude a 20 KB tail-window race still governs loop detection and could preserve or "fix" behaviour around a mechanism that no longer exists — or reintroduce raw-tail matching on that basis. Suggested fix: rename to reflect the new mechanism (e.g. detects the loop guard from the terminal result event even when later output floods the log) and either drop the 'x'.repeat(21_000) line or comment that it now stands in for post-result trailing output.
中文说明
外层测试 detects loop guard output before it falls out of the log tail 在本 diff 移除"基于输出尾部"的检测机制之后，仍保留旧机制的名字和这段 21,000 字节的冲刷载荷——现在循环检测只依赖终端 result 事件（isLoopGuardResult(terminalResult)），已经不存在任何东西可以"掉出"的"尾部"。失败场景：该测试通过的原因与其名字所宣称的不同；未来阅读用例的维护者会误以为 20 KB 尾部窗口竞争仍然支配着循环检测，从而围绕一个已不存在的机制保留或"修复"行为——甚至据此重新引入原始尾部匹配。建议修复：改名以反映新机制（例如 detects the loop guard from the terminal result event even when later output floods the log），并删除 'x'.repeat(21_000) 这行，或加注释说明它现在代表 result 之后的尾部输出。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：5045d8a370；测试已按终端 result 语义重命名，相关 4/4 通过。
| withRunnerDir((dir) => { | ||
| writeFileSync(join(dir, 'feedback.md'), 'feedback\n'); | ||
| const stub = writeWorkdirStub(dir, [ | ||
| "const qwenResultLine = (value) => `${JSON.stringify({ type: 'result', subtype: 'success', is_error: false, ...value })}\\n`;", |
There was a problem hiding this comment.
[Suggestion] R3-3: This stub redefines qwenResultLine inline instead of reusing the shared helper added at line 286 (five sibling tests embed the shared helper's output verbatim), and the enclosing test uses writeWorkdirStub whose prepended writeFileSync import and --workdir prompt-parsing are unused here (the stub writes no workdir files). The tool_result envelope builder is additionally pasted verbatim in two new tests (~10912 and ~10941). — Failure scenario: two definitions of the stream-json result-event shape now exist; the inline copy already differs (hardcodes subtype:'success'/is_error:false, cannot emit error results). When the CLI's result schema changes, only tests using the shared helper track it, and this test can pass or fail on a shape the production parser never sees; the unused workdir plumbing misleads the next reader into thinking the stub needs it. Suggested fix: build the line with the shared qwenResultLine helper in the test process and use writeQwenStub with pre-sliced/trimmed literals; optionally add a qwenToolResultLine(content) helper for the two tool-result fixtures.
中文说明
此 stub 在内部重新定义了 qwenResultLine，而没有复用第 286 行新增的共享 helper（其余 5 个同类测试都是逐字内嵌共享 helper 的输出）；且外层测试使用的 writeWorkdirStub 所预置的 writeFileSync 导入与 --workdir prompt 解析在这里都没有用到（该 stub 不写任何 workdir 文件）。此外，tool_result 消息构造器在两个新测试（约 10912 与 10941 行）中被逐字粘贴。失败场景：现在存在两份 stream-json result 事件形状的定义；内嵌副本已经有差异（写死 subtype:'success'/is_error:false，无法发出错误 result）。当 CLI 的 result 模式变化时，只有使用共享 helper 的测试会跟随，而该测试可能在一个生产解析器根本不会见到的形状上通过或失败；无用的 workdir 管线也会误导后续读者以为 stub 需要它。建议修复：在测试进程里用共享 qwenResultLine helper 构造该行，并改用 writeQwenStub 配合预先 slice/trim 好的字面量；可选地为两个 tool-result 夹具新增 qwenToolResultLine(content) helper。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| expect(r.stdout).not.toContain(payload); | ||
| expect(r.agentLog).toContain(payload); |
There was a problem hiding this comment.
[Suggestion] R3-4: No test pins that stream_event (partial-message) events refresh the idle watchdog — the PR's headline mechanism. Probe-verified mutant at this commit: gating lastOutputAt = Date.now() in consumeStreamJsonLine on event?.type !== 'stream_event' leaves all 8 idle-watchdog tests passing — keeps partial events ... emits a single stream_event and exits before any idle window could fire; requests streamed partial progress ... uses stand-in type:'progress' events, never stream_event. — Failure scenario: if such a mutation shipped later, a long thinking/tool phase whose only traffic for >20 min is partial-message deltas (exactly what --include-partial-messages produces) would be idle-killed mid-work — the precise false-kill this PR exists to prevent; the probe reproduced it (idle-killed + agent-timeout sentinel under the mutant, exit 0 unmodified). Suggested fix: add an idle-watchdog case whose stub emits only {"type":"stream_event",...} lines (e.g. every 400ms × 8, idleMs: 1500), then writes its verdict; assert status 0 and empty failure.md.
中文说明
没有测试钉住 stream_event（partial-message）事件会刷新 idle watchdog——而这正是本 PR 的核心机制。已在当前 commit 上以探针验证变异体：把 consumeStreamJsonLine 中的 lastOutputAt = Date.now() 加上 event?.type !== 'stream_event' 条件后，全部 8 个 idle-watchdog 测试依旧通过——keeps partial events ... 只发出一个 stream_event 并在 idle 窗口触发前退出；requests streamed partial progress ... 使用的是替身 type:'progress' 事件，从不发 stream_event。失败场景：若此类变异日后被引入，一段长达 20 分钟以上只有 partial-message 增量流量（正是 --include-partial-messages 产生的形态）的思考/工具阶段会在工作中被 idle 误杀——正是本 PR 要防止的那种误杀；探针已复现该行为（变异体下被 idle 杀掉并写出 agent-timeout 哨兵，未变异时正常退出 0）。建议修复：新增 idle-watchdog 用例，stub 只发 {"type":"stream_event",...} 行（如每 400ms 一条 × 8，idleMs: 1500），然后写出 verdict；断言 status 为 0 且 failure.md 为空。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：5045d8a370；idle 回归现使用真实 stream_event，相关 4/4 通过。
| expect(r.stdout.length).toBeLessThan(10_000); | ||
| expect(r.agentLog.length).toBe(2_097_152); |
There was a problem hiding this comment.
[Suggestion] R3-5: The oversized-line discard only tests the UNTERMINATED case; the flag reset when an oversized line IS terminated (after which subsequent protocol events must parse again) is untested. Probe-verified mutant at this commit: deleting discardingOversizedStdoutLine = false; from the terminated branch keeps the full suite green. — Failure scenario: probe — a 1,048,577-char TERMINATED line followed by a loop-guard result event, exit 1: the mutant latches discard mode past the newline, silently drops the terminal result event, and classifies the run as generic status 1 with no handoff; the original writes the loop-guard failure + handoff. A single >1MB event line (large tool results do exceed 1MB) would drop every later event, including the one driving loop/API-error classification. Suggested fix: extend the bounds test (or add a sibling): emit the oversized line WITH trailing newline, then a result line; assert step stdout stays bounded AND the result event is still classified (loop handoff or agent-api-error).
中文说明
超长行丢弃只测试了"无换行终止"的情形；当超长行确实被换行终止时（其后必须恢复解析后续协议事件）的标志位重置没有测试。已在当前 commit 上以探针验证变异体：删除 terminated 分支中的 discardingOversizedStdoutLine = false;，全套用例仍然全绿。失败场景：探针——先写一条 1,048,577 字符且带换行终止的行，再写一条 loop-guard result 事件，以 1 退出：变异体会把丢弃模式锁存到换行之后，悄无声息地丢掉终端 result 事件，把该运行归为普通的 status 1 且不写 handoff；原始代码则写出 loop-guard failure + handoff。单条超过 1MB 的事件行（大型 tool result 确实会超过 1MB）将丢弃其后的所有事件，包括驱动循环/API 错误分类的那一条。建议修复：扩展现有边界测试（或新增一个）：发出带换行结尾的超长行，再发一条 result 行；断言 step stdout 仍受限，且 result 事件仍被正确分类（loop handoff 或 agent-api-error）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：5045d8a370；已补超大换行后恢复解析回归，相关 4/4 通过。
| }); | ||
|
|
||
| it('never fires while the agent keeps talking, however slowly', () => { | ||
| it('never fires while the agent emits protocol events, however slowly', () => { |
There was a problem hiding this comment.
[Suggestion] R3-6: The positive half of the step-output contract — non-stream_event protocol events and unparseable lines are echoed to step stdout — has no presence assertion anywhere; keeps partial events ... only asserts the negative half. Probe-verified mutant at this commit: deleting the process.stdout.write(...) echo from both the parse-success and catch branches of consumeStreamJsonLine leaves the full suite (1118 tests) green — every run-agent stdout assertion in the file is an absence/size check or a marker-file check. — Failure scenario: if that regression shipped, autofix step logs would lose all agent events and unparseable output, leaving operators debugging a failed round with only the runner's final line. Cost is observability, not classification. Suggested fix: in an existing/new runner test, assert result.stdout contains an emitted protocol line (e.g. the result line), complementing the partial-suppression assertion.
中文说明
step 输出契约的"正向一半"——非 stream_event 协议事件与无法解析的行会被回显到 step stdout——在任何地方都没有存在性断言；keeps partial events ... 只断言了反向一半。已在当前 commit 上以探针验证变异体：删除 consumeStreamJsonLine 解析成功分支与 catch 分支中的 process.stdout.write(...) 回显，全套 1118 个用例仍全绿——文件中所有针对 run-agent stdout 的断言都是"不存在/大小"检查或 marker 文件检查。失败场景：若该回归被引入，autofix 的 step 日志将丢失所有 agent 事件与无法解析的输出，排查失败轮次的运维人员只能看到 runner 的最后一行。代价是可观测性，而非分类正确性。建议修复：在现有/新增的 runner 测试中断言 result.stdout 包含某条被发出的协议行（如 result 行），与 partial 抑制断言互补。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| if (terminated) { | ||
| if (!discardingOversizedStdoutLine) { | ||
| consumeStreamJsonLine(stdoutCarry, true); | ||
| } | ||
| stdoutCarry = ''; | ||
| discardingOversizedStdoutLine = false; | ||
| } |
There was a problem hiding this comment.
[Suggestion] R3-7: An oversized stdout line is dropped with zero breadcrumb in step output — even non-JSON garbage gets echoed (catch branch), but a discarded line produces nothing. If the dropped line was the terminal result event, terminalResult silently stays unset, degrading loop/API-error classification to a bare status 1. Probe-confirmed at this commit: an oversized arm with a 1,100,133-byte loop-guard result line produced step stdout = 0 bytes, failure.md = status 1, no handoff.md — versus a 233-byte control arm which echoed the event and wrote the loop-guard failure + handoff. — Failure scenario: a maintainer debugging a failed round sees step-log events up to some point and then nothing indicating a >1 MiB event was swallowed, while agent.log contains it raw; the per-line 1 MiB cap is discoverable only by reading consumeStreamJson. Cost: wasted debugging time and an unexplained classification miss whenever the dropped line carried the result event. Suggested fix: emit one placeholder line per dropped line (e.g. in the discard branch: process.stdout.write('[run-agent] dropped oversized stream-json line; full bytes in agent.log\n')) — one line per logical line, so no flooding; note this restores observability only, the classification miss for oversized result events remains.
中文说明
超长 stdout 行被丢弃时在 step 输出中不留任何痕迹——连无法解析的 JSON 垃圾都会被回显（catch 分支），但被丢弃的行什么都不产生。如果被丢弃的正是终端 result 事件，terminalResult 会悄无声息地保持未设置，循环/API 错误分类退化为普通的 status 1。已在当前 commit 上以探针确认：超长组（1,100,133 字节的 loop-guard result 行）step stdout 为 0 字节、failure.md 为 status 1、无 handoff.md——而 233 字节的对照组回显了事件并写出 loop-guard failure + handoff。失败场景：维护者排查失败轮次时，只会看到 step 日志在某个位置之前的事件，之后没有任何迹象表明一条 >1 MiB 的事件被吞掉，而 agent.log 里其实有原始内容；每行 1 MiB 的上限只能通过阅读 consumeStreamJson 才能发现。代价：浪费排查时间，且每当被丢弃的行恰好是 result 事件时，就会出现一次无法解释的分类缺失。建议修复：每丢弃一行就输出一条占位行（例如在丢弃分支中：process.stdout.write('[run-agent] dropped oversized stream-json line; full bytes in agent.log\n')）——每个逻辑行一行，不会刷屏；注意这只恢复可观测性，超长 result 事件导致的分类缺失仍然存在。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
已修复 + 验证证据：5045d8a370；丢弃超大行时现输出单条可观测提示，相关 4/4 通过。
| timedOut, | ||
| idleTimedOut, | ||
| loopDetected: loopDetected || isLoopGuardOutput(outputTail), | ||
| loopDetected: isLoopGuardResult(terminalResult), |
There was a problem hiding this comment.
[Suggestion] R3-8: The runner this PR reworks is one of two copy-forked run-agent.mjs scripts — .qwen/skills/repo-hygiene/scripts/run-agent.mjs shares ~120 lines of scaffolding. After this PR, autofix detects the loop guard from the structured terminal result event while the fork still scans raw bytes of the last-20 KB output tail (loopDetected || isLoopGuardOutput(outputTail)) and has no idle watchdog — the exact defect classes this PR fixes remain live in the fork. — Failure scenario: when marker injection into an untrusted stream or a silent-sandbox wedge bites the repo-hygiene fork, the ~80 lines of stream-json consumption/result-event classification added here must be re-implemented from scratch against a drifted copy. Not newly wrong for repo-hygiene today (it still uses text output, where raw matching is the status quo) — the cost is the divergence. Suggested fix: extract the shared runner core into one script both skills invoke, or leave cross-reference comments in each fork so the next fix lands in both; if deliberately deferred, a follow-up issue tracking the repo-hygiene fork.
中文说明
本 PR 重构的 runner 是两个复制分叉的 run-agent.mjs 脚本之一——.qwen/skills/repo-hygiene/scripts/run-agent.mjs 与其共享约 120 行脚手架。本 PR 之后，autofix 从结构化的终端 result 事件检测循环守卫，而分叉仍在扫描最后 20 KB 输出尾部的原始字节（loopDetected || isLoopGuardOutput(outputTail)），且没有 idle watchdog——本 PR 修掉的缺陷类别在分叉中依然存活。失败场景：当"不可信流中的 marker 注入"或"静默 sandbox 卡死"在 repo-hygiene 分叉上发作时，这里新增的约 80 行 stream-json 消费/result 事件分类逻辑必须在一个已经漂移的副本上从头实现。对 repo-hygiene 而言今天并非新引入的错误（它仍使用 text 输出，原始字节匹配是现状）——代价在于分叉本身。建议修复：把共享的 runner 核心抽成一个两个 skill 共同调用的脚本，或在每个分叉中留下互相引用的注释以便下次修复同时落地；若是有意推迟，开一个跟进 issue 追踪 repo-hygiene 分叉。
— qwen3.8-max via Qwen Code /review (v0.21.9)
Maintainer verification: real stack, not stubsI rebuilt this PR locally and drove the real Both revisions share the same bundle: this PR touches only Verdict: the fix does what it claims, on the real stack, and every piece of it is load-bearing. I'd merge it. Four measured caveats are listed at the end — none of them is a regression, but #1 and #3 are worth a follow-up. 1. The premise is real: text output is silent for the whole roundSame 5-tool-call round, same mock provider, only the output flags differ.
The idle watchdog reads exactly this stream, so on 2. End-to-end A/B through the real runner
The middle row is the control: the round itself is healthy on base too — the 15 s failure is purely the watchdog misreading silence, exactly the diagnosis in the PR body. Step-output volume stays under control: head's 3. Mutation A/B — is each piece load-bearing?
One-line reversions of the head runner, replayed against the same real stack:
So Loop-guard path preserved. I drove the real CLI into its own loop detector (mock repeating an identical tool call). It emits API-error retry path — the second commit is necessary, not gold-plating. Under stream-json the real CLI reports a fatal provider error in band and exits 0, where text mode exited 1:
Without 4. Test suiteRan the full contract file on both revisions on this machine (macOS, Node 24):
Both runs then print Caveats worth a follow-up (none is a regression)
Harness details (so this is reproducible)
中文版本 / Chinese version维护者验证：真实环境，而非 stub我在本地重新构建了这个 PR，用真实的 两个版本共用同一份 bundle：本 PR 只改了 结论：修复在真实环境下确实成立，并且改动的每一部分都是必要的，我认为可以合并。 末尾列了四点实测到的注意事项——都不是回归，但第 1 和第 3 点值得后续跟进。 1. 前提属实：text 输出在整轮里完全静默同一个 5 次工具调用的轮次、同一个 mock provider，只有输出参数不同：
idle watchdog 读的正是这条流，所以在 2. 通过真实 runner 的端到端 A/B
中间那行是对照组：这一轮在 base 上本身是健康的——15 s 的失败纯粹是 watchdog 误读静默，正是 PR 描述的诊断。 step 输出量也是可控的：head 的 3. 变异 A/B——每一处改动是否都不可或缺？对 head runner 做单行回退，在同一真实环境重放：
因此 loop guard 路径未受影响。 我让真实 CLI 触发了它自己的循环检测（mock 重复同一个工具调用）。它输出 API error 重试路径——第二个 commit 是必要的，不是过度设计。 在 stream-json 下，真实 CLI 会把致命的 provider 错误放在流内上报并以 0 退出，而 text 模式是以 1 退出：
没有 4. 测试套件在本机（macOS，Node 24）对两个版本跑了完整的合约测试文件：
两次运行随后都打印 值得后续跟进的注意事项（均非回归）
|
|
@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: 258 passed · 0 failed · 258 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：258 通过 · 0 失败 · 258 总计 Verification reportPR 8895 Deep Verification —
|
| # | Cell (stub behavior) | base | head | Oracle |
|---|---|---|---|---|
| 01 | silent wedge (prints nothing) | exit 1, idle-timeout (no output for 1500ms, sentinel |
same | watchdog preserved |
| 02 | productive run; streams protocol events only when the flags are present, else buffers (faithful to the real CLI's mode difference) | exit 1, idle-killed at ~1.5 s, verdict never written (the bug) | exit 0, verdict written, ~1.8 s | the load-bearing flip |
| 03 | productive, stderr-only chatter | exit 0 | exit 0 | stderr still counts as liveness on head |
| 04 | chatty non-JSON stdout | exit 0 (any byte counted) | exit 1, idle-timeout | intentional narrowing, see Finding 1 |
| 05 | exit 0 + success result line carrying [API Error: 429 …], no verdict |
exit 1, generic missing-output failure, no retry sentinel (retry lost) | exit 1, recoverable API error without an agent verdict, agent-api-error + kind transient |
S1: base's hole, closed by head |
| 06 | faithful text-mode API error (stderr, exit 1) | exit 1, sentinel transient |
exit 1, sentinel transient |
parity: retries preserved across the format switch |
| 07 | loop guard as structured result event (current CLI shape), exit 1 | exit 1, loop failure + handoff (raw match on JSON text) | exit 1, loop failure + handoff (structured match) | parity: loop handoff preserved |
| 08 | loop guard on stderr only, exit 1 (text-mode shape) | exit 1, loop failure + handoff | exit 1, generic status 1, no handoff |
detection surface narrowed, see Finding 1 |
| 09 | one unterminated 2 MiB stdout line, then verdict | exit 0, 2 097 276 B on step stdout | exit 0, 53 B on step stdout, agent.log retains 2 097 223 B | S2: bounding works |
| 10 | stream_event partial with 10 KB payload, then verdict |
exit 0, payload floods step stdout | exit 0, payload absent from stdout, present in agent.log | S2: partial filtering works |
Result: 76/76 assertions, 0 unexpected. Witness: 01-ab-matrix-base-vs-head.png (full matrix as it printed, including the cell-02 flip and totals). Raw log: logs/ab-run1.log.
Attribution across the bundled commits (intermediate variant)
The PR bundles four commits; commit 2's tree (9b304a2e) is locally present, commit 3's object is absent from the depth-2 checkout, so the split achievable is {1+2} vs {3+4}. Running the commit-2 script on the discriminating cells (15/15 assertions, logs/commit2-run.log):
| Behavior | base | commit2 (1+2) | head (1+2+3+4) |
|---|---|---|---|
| Productive flag-aware run survives (cell 02) | killed | survives — the flags alone fix the central bug | survives |
| Exit-0 API error retried (cell 05) | retry lost | retry preserved — commit 2's apiErrorWithoutVerdict already does it |
retry preserved |
| Non-JSON stdout counts as liveness (cell 04) | yes | yes (raw counting) | no — commits 3+4 |
| 2 MiB line bounded / partials filtered (cells 09/10) | n/a (passthrough) | n/a (passthrough) | yes — commits 3+4 |
| Loop guard detection | raw tail | raw tail (matches inside JSON text) | structured result event |
Real-CLI grounding (what stubs cannot prove)
Witness: 03-real-cli-accepts-flags-and-streams.png; raw log: logs/cli-flag-grounding.log.
- Control:
--definitely-not-a-flag→Unknown arguments: …— a wrong flag name would fail loudly, not silently degrade. - Validator live:
--include-partial-messageswithout--output-format stream-json→--include-partial-messages requires --output-format stream-json— the PR's combo is exactly the sanctioned pair (packages/cli/src/config/config.ts:996). - PR combo on the built CLI (
node packages/cli/dist/index.js --yolo --output-format stream-json --include-partial-messages --prompt hi): parsed, initialized, and emitted livestream_eventNDJSON partials (thinking_deltachunks) on stdout, with the one non-JSON line (the yolo safety warning) verified to go to stderr (gemini.tsx→writeStderrLine) — stdout stays pure NDJSON in production. - Loop-guard wire shape (code trace, no live loop induced): in stream-json mode
emitLoopDetectedResult(nonInteractiveCli.ts) emits{type:'result', is_error:true, error:{message:'Loop detection halted the run…'}}and exits 1 — precisely the fields head'sisLoopGuardResultreads. In TEXT mode the message goes to stderr instead, which is why head's narrowed surface (cell 08) is safe against the current CLI. Similarly,GeminiEventType.Errorin JSON modes is folded into assistant text (BaseJsonOutputAdapter.ts:642), producing the exit-0-with-[API Error…]-text shape that cell 05 exercises.
Corrections
None (first verification round).
Findings
1. Low — liveness and loop-detection surfaces narrowed by design; zero exposure against the current CLI, noted for future CLI changes.
Repro: node <artifact>/harness/run-ab.mjs --only 04-nonjson-stdout and --only 08-loop-stderr-text (base vs head cells diverge as described). Two behavior deltas follow from trusting only the protocol:
(a) non-JSON stdout no longer refreshes the watchdog (cell 04: base exit 0 → head idle-killed); stderr bytes still count on both arms (cell 03), and the current CLI emits only NDJSON on stdout in stream-json mode (warning verified to stderr), so no known production run changes outcome;
(b) the loop guard is recognized only inside the terminal result event (cell 08: a text-mode stderr-only loop report degrades from terminal handoff to a generic retryable status 1 failure). Head always passes the streaming flags, and the current CLI is code-verified to package loop detection into the result event in that mode, so exposure today is zero. Both are the intended "trust structured results" semantics; the note exists so a future CLI change that prints banners to stdout or reports loops out-of-band gets caught by these tests, not by a fleet of idle-killed rounds.
2. Low — bound boundary: a >1 MiB terminal result line is discarded whole.
Repro: node <artifact>/harness/run-ab.mjs --only 09-oversized-unterminated-line (head bounds stdout to 53 B; the discarded prefix is visible in diagnosticTail only for non-zero exits).
MAX_STREAM_JSON_LINE_LENGTH protects the runner's line buffer (verified: cell 09 keeps step stdout at 53 B and the artifact at full size), but a single JSON line above the cap never reaches JSON.parse, so if the terminal result event itself exceeded 1 MiB its error.message/result fields would not feed loop/API-error detection; only the first 20 KB survive via the diagnostic tail, and only for non-zero exits. Reaching this requires a pathological >1 MiB single-line result; the trade-off (memory bound over detection completeness on that shape) is the explicit point of commit 4, and the PR's own test pins the ≤cap behavior. Reported so the boundary is on the record, not as a defect.
3. Note — author's local onTaskUpdate RPC timeout did not reproduce. The description reports the full workflow file reaching 150/150 twice but the local Vitest worker then exiting 1 on an unrelated onTaskUpdate RPC timeout. Here the suite ran clean: 158/158 tests, exit 0, 45.4 s (logs/suite-head.log).
Reviewer Test Plan, walked step by step
| Plan step | Result |
|---|---|
| "a headless stub emitting partial stream events remains alive beyond the idle window" | ✓ suite test requests streamed partial progress… + A/B cell 02 head (exit 0, verdict written) |
| "a genuinely silent stub is still terminated and reported as an idle timeout" | ✓ suite test kills a silent agent at the idle window… + A/B cell 01 head (exit 1, idle-timeout (no output for 1500ms…) |
| "Removing either streaming flag should make the new regression fail" | ✓ mutants M0/M1 — each flips exactly that one test red (1 failed | 157 passed), nothing else |
Vacuity / mutation matrix
Positive control: unmutated head script → 158/158 green. Every mutant below is a single-point edit to the head script; the suite ran in full for each (45 s/run) so collateral kills are visible. All six killed; no survivors. Witness: 02-mutation-matrix-each-guard-pinned.png; raw logs logs/mutation-M*.log (recapture in logs/mutation-recapture.out reproduced identical kills).
| Mutant | Edit | Suite at mutant | Red test(s) — attribution |
|---|---|---|---|
| M0 | drop --output-format stream-json |
1 failed | 157 passed | requests streamed partial progress… — the PR's new regression, pinned to commit 1's flag |
| M1 | drop --include-partial-messages |
1 failed | 157 passed | same test — the flag pair is jointly pinned |
| M2 | remove lastOutputAt refresh on parsed lines |
2 failed | 156 passed | the above + never fires while the agent emits protocol events… — liveness is pinned to protocol parsing, commit 3 |
| M3 | apiErrorWithoutVerdict := false |
2 failed | 156 passed | flags a recoverable stream-json API error even when qwen exits zero + classifies split and unterminated stream-json result lines — commit 2's path |
| M4 | MAX_STREAM_JSON_LINE_LENGTH := Infinity |
1 failed | 157 passed | bounds an unterminated stdout line while retaining the artifact — commit 4's bound |
| M5 | isLoopGuardResult := () => false |
3 failed | 155 passed | writes agent output to a log and marks loop guard failures for handoff, detects loop guard output before it falls out of the log tail, does not flag an API error that appears after a real verdict or a loop guard — structured loop detection (commit 3), including its precedence over API-error retry |
Each red test is the one its commit claims to cover; no mutant was killed by an unrelated test.
Not covered
- Live ECS sandbox run and re-arming of PRs that already tripped the cumulative timeout breaker — declared out of scope by the PR itself; the watchdog semantics were verified via the real runner script with compressed windows instead.
- Per-commit attribution for commits 3 and 4 individually — commit 3's object (
2fffcac4) is absent from the depth-2 checkout andbff179ddd9is a shallow boundary, so only the {1+2} vs {3+4} split (via commit 2's tree) was exercised. The aggregateHEAD^1..HEADdiff is what the tables above verify. - Windows — the PR marks it N/A; the script is platform-agnostic Node but the suite is excluded on win32 by
scripts/tests/vitest.config.ts. - A full autofix round against a real model — grounding used one trivial
--prompt hiturn to prove flag parsing and livestream_eventemission; running a real autofix mode would execute yolo tool calls and was deliberately not done. repo-hygiene's siblingrun-agent.mjs— same pattern, not touched by this PR, still on the old text-mode liveness; not verified here.- Consumers of the step's stdout format change — checked structurally: no workflow step pipes or parses run-agent stdout (exit code + workdir sentinels are the contract, pinned by the suite); visual log impact only.
- Container-state observation (not a PR finding): in the main checkout,
.qwen/skills/autofix/scripts/run-agent.mjswas root-owned (mode 0555, directory 0555) with content byte-identical to the base blob (sha25631f601e3…, written 2026-08-10 17:00, before commit 1), so the in-place suite would have silently tested base code; see Methodology for the workaround. Left exactly as found.
Methodology
Environment: the CI verify container (node:22-bookworm lane sample), merge-ref checkout at depth 2 (HEAD = merge 2ee1a57b71, HEAD^1 = base 8c90697ace, HEAD^2 = head bff179ddd9); npm ci + npm run build pre-done. Script variants were extracted verbatim with git show <commit>:.qwen/skills/autofix/scripts/run-agent.mjs into scratch/variants/{base,commit2,head}/ (the file imports only node: builtins, so there are no workspace symlink confounds to assert on), each with a minimal sibling SKILL.md the script's prompt builder reads. The A/B harness (harness/run-ab.mjs, run-commit2.mjs) spawns the real script with stub qwen executables modeling each scenario, under QWEN_IDLE_TIMEOUT_MS=1500, and asserts exit codes, failure.md/sentinel contents, and byte counts (76 + 15 scripted assertions). The contract suite ran via vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js from a fresh owned worktree at HEAD (tmp/pr8895-suite-tree, scratch, removed after the round) because the main checkout's copy of the script is root-owned/read-only and pinned to the base blob (verified byte-identical to HEAD^1 by sha256 before and after; the suite tree's copy re-verified as the HEAD blob 0478f8db after all mutations). Mutations were applied there one at a time with pristine-restore between runs (harness/run-mutations.mjs). Real-CLI grounding used the built packages/cli/dist/index.js. Images were produced with scripts/verify-capture.mjs. Raw logs: logs/ (ab-run1.log, commit2-run.log, suite-head.log, mutation-M*.log, mutation-recapture.out, cli-flag-grounding.log).
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
@qwen-code /triage |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow 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. ✅
|
Sandboxed verification: Skipped because the PR is not open for verification (state=MERGED, draft=false). 中文 — 判定：
|
|
Released in v0.21.10. |
4 participants
What this PR does
AutoFix now asks the headless Qwen process to emit streamed partial progress. The existing idle watchdog can therefore distinguish active tool work from a sandbox that stopped producing output. A regression test requires both stream JSON output and partial messages before treating a long-running headless task as active.
Why it's needed
The default text output buffers non-interactive agent events until the final result. AutoFix was using stdout and stderr as its liveness signal, so productive rounds could modify source and tests for 20 minutes without emitting a chunk and then be killed as a silent sandbox. Six managed PRs recently reached the cumulative timeout breaker after this failure pattern.
Reviewer Test Plan
How to verify
Run the AutoFix workflow contract test and confirm that a headless stub emitting partial stream events remains alive beyond the idle window, while a genuinely silent stub is still terminated and reported as an idle timeout. Removing either streaming flag should make the new regression fail.
Evidence (Before & After)
N/A (non-UI workflow behavior). Before the fix, the regression exited 1 after the 1.5-second idle window. With both streaming flags, the same regression exits 0 after approximately 4 seconds. The focused watchdog group passes 5/5 and sandbox cleanup passes 3/3. The complete workflow file reached 150/150 passing assertions twice; the local Vitest worker then reported an unrelated
onTaskUpdateRPC timeout and returned exit 1.Tested on
Environment (optional)
Node.js 22, local workflow contract tests with a stubbed Qwen executable.
Risk & Scope
Linked Issues
N/A
中文说明
本 PR 做了什么
AutoFix 现在会要求无头 Qwen 进程输出流式 partial progress。现有 idle watchdog 因而能够区分仍在执行工具工作的 agent 与已经停止输出的 sandbox。新增回归测试要求 stream JSON 输出和 partial messages 两个条件同时存在，才把长时间运行的无头任务视为活跃。
为什么需要
默认 text 输出会把非交互 agent 事件缓冲到最终结果才写出。AutoFix 原先把 stdout 和 stderr 作为存活信号，因此即使某轮已经持续修改源码和测试，也可能 20 分钟没有输出 chunk，随后被误杀并报告为 silent sandbox。最近有 6 个托管 PR 因相同模式触发累计超时熔断。
Reviewer 测试计划
如何验证
运行 AutoFix workflow 合约测试，确认持续发出 partial stream event 的无头 stub 在超过 idle window 后仍能完成，同时真正静默的 stub 仍会被终止并报告 idle timeout。删除任意一个流式参数都应让新增回归失败。
修复前后证据
N/A（非 UI workflow 行为）。修复前，回归测试在 1.5 秒 idle window 后以状态 1 退出；同时启用两个流式参数后，同一回归约 4 秒后以状态 0 退出。聚焦 watchdog 分组 5/5 通过，sandbox cleanup 3/3 通过。完整 workflow 文件两次都达到 150/150 断言通过，但本地 Vitest worker 随后报告无关的
onTaskUpdateRPC timeout 并返回状态 1。测试平台
环境（可选）
Node.js 22，本地 workflow 合约测试，使用 stub Qwen 可执行文件。
风险与范围
关联 Issue
N/A