Skip to content

fix(ci): stream autofix agent progress #8895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wenshao merged 5 commits into from
Aug 11, 2026
Merged

fix(ci): stream autofix agent progress #8895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
816292a
fix(ci): stream autofix agent progress
qqqys Aug 10, 2026
9b304a2
fix(ci): preserve autofix API error retries
qqqys Aug 10, 2026
2fffcac
fix(ci): trust structured autofix results
qqqys Aug 10, 2026
bff179d
fix(ci): bound autofix stream processing
qqqys Aug 10, 2026
5045d8a
fix(ci): ignore oversized autofix stream lines
qqqys Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 134 additions & 29 deletions .qwen/skills/autofix/scripts/run-agent.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ const QWEN_TIMEOUT_MS = Number(process.env.QWEN_TIMEOUT_MS) || 50 * 60 * 1000;
// Idle watchdog: a wedged sandbox produces NOTHING — four observed hangs
// (#8663 x2, #8761 r3, #8763 r4) each printed their last byte at docker
// container entry and then sat silent for the whole absolute budget,
// burning 2 hours per round for zero work. Legitimate runs are never that
// quiet: the longest silence the fleet tolerates elsewhere is the review
// pipeline's 10-minute stream-idle window for thinking phases on ~1M-token
// contexts, so twice that is the default. Distinct from QWEN_TIMEOUT_MS so
// burning 2 hours per round for zero work. Streamed agent events keep active
// runs observable; the longest silence the fleet tolerates elsewhere is the
// review pipeline's 10-minute stream-idle window for thinking phases on
// ~1M-token contexts, so twice that is the default. Distinct from
// QWEN_TIMEOUT_MS so
// the failure comment says which limit fired; a leg whose absolute budget is
// shorter than this window (the review workflow's 18-minute repair pass)
// always reaches the absolute timer first.
Expand All @@ -37,6 +38,9 @@ const QWEN_IDLE_TIMEOUT_MS =
Number.isFinite(parsedIdleTimeoutMs) && parsedIdleTimeoutMs > 0
? parsedIdleTimeoutMs
: 20 * 60 * 1000;
const MAX_STREAM_JSON_LINE_LENGTH = 1024 * 1024;
const OVERSIZED_STREAM_JSON_LINE_NOTICE =
'[run-agent] dropped oversized stream-json line; full bytes in agent.log\n';
const specs = {
'assess-candidates': {
inputs: ['candidates.json'],
Expand Down Expand Up @@ -176,6 +180,19 @@ function isLoopGuardOutput(output) {
);
}

function streamResultOutput(event) {
if (!event || event.type !== 'result') return '';
return [event.error?.message, event.result]
.filter((value) => typeof value === 'string')
.join('\n');
}

function isLoopGuardResult(event) {
return (
event?.is_error === true && isLoopGuardOutput(streamResultOutput(event))

@qwen-code-ci-bot qwen-code-ci-bot Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)

);
}
Comment on lines +190 to +194

@qwen-code-ci-bot qwen-code-ci-bot Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)


function killQwen(child, signal) {
try {
process.kill(-child.pid, signal);
Expand All @@ -190,8 +207,10 @@ function runQwen(options, prompt) {
flags: 'w',
});
log.on('error', () => {});
let outputTail = '';
let loopDetected = false;
let diagnosticTail = '';
let stdoutCarry = '';
let discardingOversizedStdoutLine = false;
let terminalResult;
let settled = false;
let timedOut = false;
let idleTimedOut = false;
Expand All @@ -207,23 +226,93 @@ function runQwen(options, prompt) {
let sandboxRemoval = null;

return new Promise((resolve) => {
const child = spawn(options.qwenBin, ['--yolo', '--prompt', prompt], {
stdio: ['inherit', 'pipe', 'pipe'],
detached: true,
});
const child = spawn(
options.qwenBin,
[
'--yolo',
'--output-format',
'stream-json',
'--include-partial-messages',
Comment on lines +233 to +235

@qwen-code-ci-bot qwen-code-ci-bot Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)

@qqqys qqqys Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修复 + 验证证据：聚焦分类用例 10/10 通过；完整 workflow 154/154 断言通过。

Comment on lines +233 to +235

@qwen-code-ci-bot qwen-code-ci-bot Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)

@qqqys qqqys Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修复 + 验证证据：stream_event 仅保留在 agent.log，不再复制到 step stdout；相关用例 10/10、完整 workflow 158/158 断言通过。

'--prompt',
prompt,
],
{
stdio: ['inherit', 'pipe', 'pipe'],
detached: true,
},
);

const appendDiagnostic = (text) => {
diagnosticTail = (diagnosticTail + text).slice(-20_000);
};

const consumeStreamJsonLine = (line, terminated) => {
if (!line.trim()) return;
try {
const event = JSON.parse(line);
lastOutputAt = Date.now();
if (event?.type === 'result') terminalResult = event;
if (event?.type !== 'stream_event') {
process.stdout.write(`${line}${terminated ? '\n' : ''}`);
}
} catch {
appendDiagnostic(`${line}${terminated ? '\n' : ''}`);
process.stdout.write(`${line}${terminated ? '\n' : ''}`);
}
};

const consumeStreamJson = (text, final = false) => {
const parts = text.split('\n');
for (const [index, part] of parts.entries()) {
const terminated = index < parts.length - 1;
if (!discardingOversizedStdoutLine) {
const remaining = MAX_STREAM_JSON_LINE_LENGTH - stdoutCarry.length;
if (part.length <= remaining) {
stdoutCarry += part;
} else {
stdoutCarry = '';
discardingOversizedStdoutLine = true;
}
}
if (terminated) {
if (discardingOversizedStdoutLine) {
process.stdout.write(OVERSIZED_STREAM_JSON_LINE_NOTICE);
} else {
consumeStreamJsonLine(stdoutCarry, true);
}
stdoutCarry = '';
discardingOversizedStdoutLine = false;
}
Comment on lines +277 to +285

@qwen-code-ci-bot qwen-code-ci-bot Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)

@qqqys qqqys Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修复 + 验证证据：5045d8a370；丢弃超大行时现输出单条可观测提示，相关 4/4 通过。

}
if (final) {
if (discardingOversizedStdoutLine) {
process.stdout.write(OVERSIZED_STREAM_JSON_LINE_NOTICE);
} else {
consumeStreamJsonLine(stdoutCarry, false);
}
stdoutCarry = '';
discardingOversizedStdoutLine = false;
}
};

const finish = (result) => {
if (settled) return;
settled = true;
clearTimeout(timer);
clearTimeout(killTimer);
clearInterval(idleTimer);
const apiErrorInfo = recoverableApiError(outputTail);
consumeStreamJson('', true);
const terminalOutput = streamResultOutput(terminalResult);
Comment on lines +304 to +305

@qwen-code-ci-bot qwen-code-ci-bot Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)

@qqqys qqqys Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修复 + 验证证据：跨 chunk 与无换行终止 result 均有回归覆盖；相关用例 10/10、完整 workflow 158/158 断言通过。

const apiErrorInfo = recoverableApiError(
result.status === 0
? terminalOutput
: `${diagnosticTail}\n${terminalOutput}`,
);
Comment on lines +306 to +310

@qwen-code-ci-bot qwen-code-ci-bot Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)

const payload = {
...result,
timedOut,
idleTimedOut,
loopDetected: loopDetected || isLoopGuardOutput(outputTail),
loopDetected: isLoopGuardResult(terminalResult),

@qwen-code-ci-bot qwen-code-ci-bot Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)

// A RECOVERABLE model error means qwen never evaluated the feedback —
// the workflow retries it rather than advancing the watermark.
apiError: apiErrorInfo.error,
Expand All @@ -237,8 +326,7 @@ function runQwen(options, prompt) {
}
};

const record = (chunk, stream) => {
lastOutputAt = Date.now();
const record = (chunk, stream, source) => {
const text = chunk.toString('utf8');
if (!sandboxName) {
lineCarry += text;
Expand All @@ -256,14 +344,18 @@ function runQwen(options, prompt) {
}
}
}
outputTail = (outputTail + text).slice(-20_000);
if (!loopDetected && isLoopGuardOutput(outputTail)) loopDetected = true;
if (source === 'stdout') {
consumeStreamJson(text);
} else {
lastOutputAt = Date.now();
appendDiagnostic(text);
stream.write(chunk);
}
log.write(chunk);
stream.write(chunk);
};

child.stdout.on('data', (chunk) => record(chunk, process.stdout));
child.stderr.on('data', (chunk) => record(chunk, process.stderr));
child.stdout.on('data', (chunk) => record(chunk, process.stdout, 'stdout'));
child.stderr.on('data', (chunk) => record(chunk, process.stderr, 'stderr'));
child.on('error', (error) => finish({ error, status: null, signal: null }));
child.on('close', (status, signal) =>
finish({ error: null, status, signal }),
Expand Down Expand Up @@ -394,7 +486,24 @@ const result = await runQwen(options, prompt);
// timeout) so the leak warning and the removal itself settle before this
// process exits and the next step inspects the host.
if (result.sandboxRemoval) await result.sandboxRemoval;
if (result.error || result.signal || result.status !== 0) {
const missingOutputs = missing(options.workdir, spec.outputs);
const presentOutputs = spec.outputs.filter(
(name) => !missingOutputs.includes(name),
);
const hasOutputVerdict = spec.anyOutput

@doudouOUC doudouOUC Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Suggested change
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)

@qqqys qqqys Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修复 + 验证证据：聚焦分类用例 10/10 通过；完整 workflow 154/154 断言通过。

? presentOutputs.length > 0
: missingOutputs.length === 0;
Comment on lines +493 to +495

@qwen-code-ci-bot qwen-code-ci-bot Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)

@qqqys qqqys Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修复 + 验证证据：聚焦分类用例 10/10 通过；完整 workflow 154/154 断言通过。

const apiErrorWithoutVerdict =
result.status === 0 &&
result.apiError &&
Comment on lines +496 to +498

@qwen-code-ci-bot qwen-code-ci-bot Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)

@qqqys qqqys Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修复 + 验证证据：聚焦分类用例 10/10 通过；完整 workflow 154/154 断言通过。

Comment on lines +496 to +498

@qwen-code-ci-bot qwen-code-ci-bot Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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:

Suggested change
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)

!hasOutputVerdict &&
!existsSync(file(options.workdir, 'failure.md'));
if (
result.error ||
result.signal ||
result.status !== 0 ||
apiErrorWithoutVerdict
) {
const detail = result.error
? result.error.message
: result.idleTimedOut
Expand All @@ -403,7 +512,9 @@ if (result.error || result.signal || result.status !== 0) {
? `timeout (${QWEN_TIMEOUT_MS}ms)`
: result.signal
? `signal ${result.signal}`
: `status ${String(result.status)}`;
: apiErrorWithoutVerdict
? 'recoverable API error without an agent verdict'
: `status ${String(result.status)}`;
if (!existsSync(file(options.workdir, 'failure.md'))) {
if (result.loopDetected) {
writeFailure(
Expand Down Expand Up @@ -462,7 +573,7 @@ if (result.error || result.signal || result.status !== 0) {
`Qwen failed during ${options.mode}: ${detail}; preserving agent-written failure.md.`,
);
}
process.exit(result.status ?? 1);
process.exit(result.status === 0 ? 1 : (result.status ?? 1));
}

if (existsSync(file(options.workdir, 'failure.md'))) {
Expand All @@ -475,18 +586,12 @@ if (existsSync(file(options.workdir, 'failure.md'))) {
process.exit(0);
}

const missingOutputs = missing(options.workdir, spec.outputs);
const presentOutputs = spec.outputs.filter(
(name) => !missingOutputs.includes(name),
);
if (spec.exclusiveOutput && presentOutputs.length > 1) {
const message = `Autofix agent wrote mutually exclusive output files: ${presentOutputs.join(', ')}.`;
writeFailure(options.workdir, message);
fail(message);
}
const ok = spec.anyOutput
? missingOutputs.length < spec.outputs.length
: missingOutputs.length === 0;
const ok = hasOutputVerdict;
if (!ok) {
const message = `Autofix agent finished without required output file(s): ${missingOutputs.join(', ')}.`;
writeFailure(options.workdir, message);
Expand Down
Loading
Loading