Fix ACP daemon loop review follow-ups

[yiliang114]

What this PR does

This PR lands the follow-up fixes from the review of the already-merged ACP daemon loop guard PR. It makes loop detection a terminal ACP turn path that preserves the final tool-response context, adds a stable invalid-tool bucket so changing error text cannot evade the 3-strike guard, stops unstarted concurrent Agent work once the shared guard fires, and applies the same invalid-call guard to early tool rejection paths.

Why it's needed

PR #6076 was merged before these review fixes landed, so main still lacks several safety hardenings around daemon loop detection. Without this follow-up, Stop-hook continuations and concurrent Agent batches can still waste extra turns or work after a loop has already been detected.

Reviewer Test Plan

How to verify

Reviewers should confirm that repeated invalid ACP tool calls end the current turn without entering a Stop-hook continuation, that changing validation messages for the same tool still trips the 3-strike guard, that excessive tool-call batches stop before tool lookup, and that unstarted concurrent Agent calls are recorded as skipped once loop detection fires.

Evidence (Before & After)

N/A. This is non-UI daemon control-flow behavior covered by focused unit tests.

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested locally
🐧 Linux	⚠️ not tested locally

Environment (optional)

Local macOS checkout, Node.js/npm workspace.

Risk & Scope

• Main risk or tradeoff: loop detection now terminates ACP turns more aggressively in daemon paths after repeated invalid calls, which is intentional for the reported failure mode.
• Not validated / out of scope: full interactive tmux E2E and cross-platform CI matrix were not run locally.
• Breaking changes / migration notes: none.

Linked Issues

Resolves #6084
Follow-up to #6076 and #6075.

中文说明

这个 PR 做了什么

这个 PR 补上已合并的 ACP daemon loop guard PR 的 review follow-up 修复。它把 loop detection 作为 ACP turn 的终止路径并保留最终工具响应上下文，使用稳定的 invalid-tool 分桶避免变化的错误文本绕过 3 次保护，在共享 guard 触发后停止未启动的并发 Agent 工作，并把同样的 invalid-call guard 应用到早期工具拒绝路径。

为什么需要

PR #6076 在这些 review 修复合入前已经被合并，所以 main 仍缺少几个 daemon loop detection 的安全加固点。没有这个 follow-up，Stop hook continuation 和并发 Agent batch 仍可能在 loop 已经检测到后浪费额外 turn 或额外工作。

Reviewer Test Plan

如何验证

Reviewer 应确认重复的 invalid ACP tool call 会结束当前 turn 且不会进入 Stop hook continuation；同一工具即使 validation message 变化也会触发 3 次 guard；过量 tool-call batch 会在 tool lookup 前停止；并发 Agent 调用在 loop detection 触发后，未启动的调用会被记录为 skipped。

Evidence（Before & After）

N/A。这是非 UI 的 daemon 控制流行为，由聚焦单元测试覆盖。

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested locally
🐧 Linux	⚠️ not tested locally

Environment（optional）

本地 macOS checkout，Node.js/npm workspace。

Risk & Scope

• 主要风险或取舍：loop detection 现在会在 daemon 路径里对重复 invalid call 更主动地终止 ACP turn；这是针对已报告失败模式的有意行为。
• 未验证 / 不在范围内：本地没有跑完整 interactive tmux E2E 和跨平台 CI matrix。
• Breaking changes / migration notes：无。

Linked Issues

Resolves #6084
Follow-up to #6076 and #6075.

[qwen-code-ci-bot]

Thanks for the follow-up PR!

Template looks good ✓

On direction: this is a direct review follow-up to the already-merged #6076, hardening daemon loop detection safety. Squarely aligned with the prior work — no direction concerns.

On approach: focused and well-scoped. Two files, four specific hardenings, each clearly traceable to review feedback on #6076. The stable invalid-tool bucketing (key by tool name only) is the right call — it prevents the model from evading the guard by varying error text. The generalized #preserveStoppedToolRun is a clean refactor that avoids duplicating the preserve-and-terminate logic across permission cancellation and loop detection. The sequential pre-batch phase in runToolConcurrently adds some complexity, but it's necessary to catch loops before launching concurrent work. Moving on to code review. 🔍

中文说明

感谢 follow-up PR！

模板完整 ✓

方向：这是对已合并 #6076 的直接 review 后续，加固 daemon loop detection 安全性。方向完全对齐，无顾虑。

方案：聚焦且范围合理。两个文件，四个具体加固点，每个都能追溯到 #6076 的 review 反馈。稳定的 invalid-tool 分桶（仅按工具名分桶）是正确的做法——防止模型通过变化错误文本绕过保护。通用化的 #preserveStoppedToolRun 是一个干净的重构，避免了在权限取消和 loop detection 之间重复 preserve-and-terminate 逻辑。runToolConcurrently 中的顺序预执行阶段增加了一些复杂度，但这是在启动并发工作之前捕获 loop 所必需的。进入代码审查 🔍

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Code review: Reviewed the diff against an independent proposal. The four hardening items (stable bucketing, turn termination on loop, early-rejection guard, concurrent batch abort) all match what I'd expect from the prior review feedback. No correctness bugs or security concerns found. The near-threshold drain in runToolConcurrently is a reasonable race-condition hedge — when a concurrent batch has N-1 invalid errors, waiting for the remaining promises to settle before checking for loop detection prevents a missed detection window. The runTool early-exit on toolLoopState?.loopDetected correctly handles the case where recordSkippedToolCall isn't provided (pre-existing callers). One minor note: maxConcurrency is capped to DAEMON_INVALID_TOOL_PARAMS_THRESHOLD (3) when toolLoopState is set, which means daemon batches always run at most 3 concurrent — this is a deliberate safety measure and the tests validate it.

Unit tests: All 213 tests in Session.test.ts pass, including 7 new tests covering stable bucketing with varying error messages, disabled-tool exemption, early tool-lookup error guard, exceeding the tool-call cap, concurrent Agent batch abort, sibling Agent interruption after loop, and pre-loop permission cancellation.

✓ src/acp-integration/session/Session.test.ts (213 tests) 19489ms

 Test Files  1 passed (1)
      Tests  213 passed (213)
   Duration  28.24s

Tmux testing: N/A — this is non-UI daemon control-flow behavior covered by unit tests. No user-visible TUI changes to capture.

中文说明

代码审查： 将 diff 与独立方案进行了对比。四个加固项（稳定分桶、loop 时终止 turn、早期拒绝 guard、并发 batch 中止）都与先前 review 反馈的预期一致。未发现正确性 bug 或安全问题。runToolConcurrently 中的近阈值 drain 是合理的竞态条件防护措施——当并发 batch 有 N-1 个无效错误时，在检查 loop detection 之前等待剩余 promise 完成，可以防止漏检窗口。runTool 中对 toolLoopState?.loopDetected 的早期退出正确处理了未提供 recordSkippedToolCall 的情况（已有的调用者）。一个小注：当 toolLoopState 设置时，maxConcurrency 被限制为 DAEMON_INVALID_TOOL_PARAMS_THRESHOLD（3），这意味着 daemon batch 最多 3 个并发——这是有意的安全措施，测试已验证。

单元测试： Session.test.ts 中全部 213 个测试通过，包括 7 个新测试，覆盖了变化错误消息的稳定分桶、禁用工具豁免、早期 tool-lookup 错误 guard、超出工具调用上限、并发 Agent batch 中止、loop 后的兄弟 Agent 中断、以及 pre-loop 权限取消。

Tmux 测试： N/A——这是非 UI 的 daemon 控制流行为，由单元测试覆盖。无可捕获的用户可见 TUI 变更。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

This is a clean follow-up that lands exactly the review fixes that didn't make it into #6076 before merge. Every change traces back to a specific gap: the bucketing fix prevents evasion by varying error text, the turn termination stops Stop-hook continuations from wasting work after a loop, the early-rejection guard closes a path where missing tools could bypass the 3-strike rule, and the concurrent batch abort ensures sibling Agent calls don't keep running after the shared guard fires. The runToolConcurrently changes add complexity (sequential pre-batch phase, near-threshold drain, fill functions), but it's justified by the correctness requirements and well-covered by tests. All 213 tests pass. Approving. ✅

中文说明

这是一个干净的 follow-up，准确地补齐了 #6076 合入前未完成的 review 修复。每处改动都可追溯到具体缺陷：分桶修复防止通过变化错误文本绕过保护；turn 终止阻止 Stop hook continuation 在 loop 后浪费工作；早期拒绝 guard 堵住了缺失工具可以绕过 3 次保护的漏洞；并发 batch 中止确保共享 guard 触发后兄弟 Agent 调用不会继续运行。runToolConcurrently 的改动增加了复杂度（顺序预执行阶段、近阈值 drain、fill 函数），但被正确性需求证明是合理的，测试覆盖充分。全部 213 个测试通过。批准 ✅

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[qwen-code-ci-bot]

Qwen Code review did not complete successfully. Qwen review aborted with an API error before posting comments. See workflow logs.

[DragonnZhang]

Reviewed the full diff (+354/-23 across 2 files) with three parallel passes (correctness, security, undirected audit).

Correctness: The loop-detection propagation is correctly wired through all four daemon turn paths (main prompt, follow-up, cron, background notification). The #preserveStoppedToolRun generalization cleanly handles both permission-cancel and loop-detect terminal states. The runBounded sequential-first-N pre-execution (when calls.length > DAEMON_INVALID_TOOL_PARAMS_THRESHOLD) is a sound way to detect invalid-param loops before launching the full concurrent batch. The concurrency cap (min(configured, 3)) in daemon paths is a deliberate safety trade-off.

Security: All changes are defensive guardrails. No new external inputs or attack surface. The LOOP_DETECTED_CONTEXT_MESSAGE is a static string.

Key changes verified:

• Invalid-tool bucketing keyed by toolName alone (not toolName + error.message) correctly prevents the model from evading the 3-strike guard by varying error text.
• runTool early-exits immediately when toolLoopState.loopDetected is already set, preventing wasted work on unstarted concurrent Agent calls.
• recordDaemonToolCalls at the top of runToolCalls catches batches that exceed the 100-call cap before any tool execution.
• earlyErrorResponse now feeds into recordDaemonInvalidToolParams for missing-tool and disabled-tool errors, closing the gap where early tool-lookup failures bypassed the guard.
• The #buildNextMessageAfterToolRun returns null on loop detection, and #preserveStoppedToolRun adds the context message to history, so the model receives a clear signal on the next turn.

Test coverage is thorough: invalid params with fresh ids, missing-tool lookup errors, the 100-call cap, and unstarted concurrent Agent calls after loop detection. All assertions verify both the stop behavior and the correct history/recording side-effects.

LGTM.

[DragonnZhang]

Incremental review (d6a74ce..730f9f0) — 2 new commits.

Commits reviewed:

• 85af8c7f3 fix(acp): skip remaining calls after loop detection
• 730f9f0bc fix(acp): use loop skip reason after detection

Verdict: APPROVE

Both commits cleanly fix the ACP turn termination behavior after loop detection:

1. The first commit adds appendSkippedAfter() calls in the two loop-detection early-return paths (batch and sequential) in runToolCalls, ensuring remaining function calls after loop detection receive proper error responses instead of being silently dropped.

2. The second commit parameterizes appendSkippedAfter with a message argument and correctly passes LOOP_DETECTED_SKIP_MESSAGE in the loop-detection paths, distinguishing them from the permission-cancel paths that use the default message.

Correctness: Verified all four appendSkippedAfter call sites use the correct message. The runBounded internal skip paths (fillLoopSkippedFrom / fillPermissionSkippedFrom) also use the correct messages. No double-skip or missed-skip scenarios.

Tests: Both test cases extended with read_after_loop calls to verify post-loop function calls are properly skipped. Full part-ID assertions confirm no calls are silently dropped.

Security: No concerns — changes are limited to error message routing and skip logic for already-queued tool calls.

[qwen-code-ci-bot]

No new issues found. LGTM. ✅

The three [Critical] comments posted by qwen-code-ci-bot on earlier revisions of this PR have all been addressed by subsequent commits:

• Turn-cap path (#preserveStoppedToolRun with empty parts[]) — fixed in 004ddf5f: the turn-cap early-return at runToolCalls now produces a functionResponse part for every skipped call, so #preserveStoppedToolRun always has a proper anchor to attach the LOOP_DETECTED_CONTEXT_MESSAGE.
• Missing skip responses after loop detection in runToolCalls — fixed in 85af8c7f: both the concurrent batch path (shouldStopForLoop) and the sequential path (r.loopDetected) now call appendSkippedAfter(..., LOOP_DETECTED_SKIP_MESSAGE), and the unstarted-call layer (fillLoopSkippedFrom / early runTool loopDetected check) closes the same gap one level deeper.
• Concurrent loop-detection path after Promise.race untested — valid observation on the prior revision; the Promise.race branch is a defensive layer that's hard to reach deterministically with synchronous mocks. The deeper runTool-top check and fillLoopSkippedFrom cover the same functional behavior and are exercised by the existing tests.

The two [Suggestion] items (bucketing by tool name only, concurrency capped at the threshold) are intentional design decisions per the PR description — the stable-bucket fix is exactly the core bug being closed, and the cap at the threshold is the minimal-cost way to get sequential early termination without restructuring the dispatch loop. The invalidToolParamErrorCount dead-state observation is harmless and trivially cleanable.

The code is tight, the four new test cases cover each fix precisely, and the #preserveStoppedToolRun generalization is the minimum needed to serve both termination paths.

— qwen3.7-max via Qwen Code /review

[DragonnZhang]

Incremental review of commit 004ddf5fb (fix: resolve acp loop review comments) on top of previously approved 730f9f0bc.

Changes reviewed:

• Removed redundant invalidToolParamErrorCount field from DaemonToolLoopState; replaced its usage with invalidToolParamErrors.size > 0 (semantically equivalent).
• Loop-detection early return in runToolCalls now returns proper functionResponse parts for each skipped call instead of an empty parts: [] array. This fixes a real issue where #preserveStoppedToolRun would add history entries missing function responses for the model's own calls.
• New test for concurrent Agent batch loop detection via Promise.race path.
• Added addHistory assertion verifying both function-response error parts and the LOOP_DETECTED_CONTEXT_MESSAGE text part are preserved.

No high-confidence issues found. The changes are correct and improve correctness of the loop-detection path.

[DragonnZhang]

Incremental review of commit d4aff4d16 (fix(cli): narrow daemon invalid tool bucketing):

No high-confidence issues found.

The change correctly narrows recordDaemonInvalidToolParams to only the two pre-lookup error paths (missing function name, tool not found in registry) by gating it behind a new recordInvalidToolParams opt-in flag. Disabled tools, hook-blocked tools, and runtime errors are now excluded from the invalid-params loop detection bucket, which is the right semantic — the model can't fix a disabled tool by changing parameters. The DAEMON_TURN_TOOL_CALL_CAP (100 calls) still provides a general safety net for any runaway tool-call loop including disabled-tool scenarios.

Test coverage is appropriate: the new test validates the negative case (disabled tool does not trigger loop detection via invalid-params path), complementing the existing positive case (tool not found does trigger it).

[qwen-code-ci-bot]

Downgraded from Approve to Comment: CI still running.

Additional suggestions (not inline):

• Session.ts:225 — DAEMON_INVALID_TOOL_PARAMS_THRESHOLD is overloaded for 3 purposes: loop-detection threshold, concurrency cap, and sequential preamble size. Changing it for one purpose silently affects all three. Consider documenting the coupling or extracting a separate constant.
• Session.test.ts:2631 — The turn-cap test uses expect.arrayContaining(...) which only checks that read_0 and the context text exist somewhere in parts. It doesn't verify all 101 calls received functionResponse parts or the exact count. Consider exact assertions.

— qwen3.7-max via Qwen Code /review

[qqqys]

The previous critical loop-detection issues are addressed at this head. The turn-cap and loop-skip paths now produce function responses and record skipped calls, and the latest CI is green. No new critical blocker found.

— GPT-5 Codex via Qwen Code /review

[qwen-code-ci-bot]

No review findings. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[wenshao]

✅ Local verification report — real-binary tmux E2E + mutation testing

Maintainer verification of this PR at head 6b4eb8e5a (merge-base with origin/main: 7b9e3188). I built the real qwen bundle, ran the focused unit suite, mutation-tested every load-bearing change, and — to fill the gap the PR description flags as not run locally (“full interactive tmux E2E”) — drove a real qwen serve daemon against a fake OpenAI endpoint so the loop guard fires end-to-end in the shipped artifact.

Verdict: behavior is correct and the new tests are load-bearing. Recommend merge, with one non-blocking consideration noted at the end.

Environment: macOS (darwin 25.5.0), Node v22.22.2, npm ci + npm run build (tsc) + npm run bundle all exit 0; node dist/cli.js --version → 0.19.3.

---

1. Unit tests — full ACP Session.test.ts

npx vitest run src/acp-integration/session/Session.test.ts
→ Test Files 1 passed | Tests 212 passed (212)

No regressions from the #preserveCancelledPermissionToolRun → #preserveStoppedToolRun rename or the recordSkippedToolCall hoist. The 6 loop-guard tests in the conversation_finished telemetry block all pass.

2. Mutation testing — each change is load-bearing

Every change was reverted on src (vitest aliases to source) and the targeted test re-run. Each anchor was asserted to match exactly once before mutating, so no result is a silent no-op.

#	Change	Mutation applied	Test	Result
A	Stable invalid-tool bucket (key = tool name)	revert to `${toolName}\0${error.message}`	…repeated invalid tool parameters with fresh ids	🔴 caught
B	Loop-detected = terminal turn (skips Stop-hook)	disable the if (toolRun.loopDetected) → end_turn in #executePromptInner	same test (stopHookCalls must be 0)	🔴 caught
C	100-call cap preserves tool responses	parts: []	…exceeding the daemon tool-call cap	🔴 caught
E	Early tool-lookup guard records a strike	drop recordInvalidToolParams: true on “Tool not found”	…stops early tool lookup errors	🔴 caught
D	Stop unstarted concurrent Agent work	D1 seq pre-pass / D2 concurrency cap / D3 post-race fill, each alone	…concurrent Agent…	🟢 survives (defense-in-depth)
D	(same)	D1 + D2 combined → unstarted agents actually execute	…does not start unstarted concurrent Agent calls	🔴 caught

The concurrent-skip behavior (D) is implemented with redundant guards, so no single-line revert breaks it — that is robust, not vacuous. Disabling two guards at once forces the unstarted agents to run and the test catches it, confirming the cluster is genuinely exercised.

3. Real-binary E2E (tmux) — the loop guard fires in a live daemon

Real qwen serve daemon → real qwen --acp child running the patched Session.ts; a fake OpenAI server demands a non-existent tool (foobar_loop_tool) on every turn. The authoritative signal is how many times the daemon actually calls the model (fakeServer.requests.length). I ran an A/B: PR head vs. the same bundle rebuilt with change #4 reverted.

Build	Model turns	Daemon stop reason
PR head (with fix)	3	[WARN] Stopping ACP turn after repeated tool parameter errors from foobar_loop_tool → end_turn
Control (change #4 reverted, rebuilt)	101	[WARN] Stopping ACP turn after 101 tool calls in one turn (100-call cap only)

So in the shipped binary, change #4 cuts wasted model turns from 101 → 3 when a model loops on an invalid/unknown tool, and the turn ends cleanly (end_turn) with the loop-detection context preserved. This is exactly the failure mode in #6084.

4. One non-blocking consideration (for a conscious ack before merge)

runConcurrent now caps maxConcurrency = Math.min(configured, 3) and runs the first 3 calls of any batch sequentially whenever toolLoopState is present — which is every daemon turn, not only looping ones. Net effect: a legitimate batch of N>3 concurrent tool/subagent calls no longer runs fully in parallel — the first 3 are serialized and the remainder are capped at concurrency 3. For fast tools this is negligible; for parallel Agent subagents or other long-running tools it reduces parallelism. It’s a deliberate tradeoff to detect a loop before flooding work, but it isn’t mentioned in the PR’s Risk & Scope. Flagging so it’s an intentional decision, not a surprise.

---

Reproduction: worktree at PR head, npm ci && npm run build && npm run bundle; E2E reuses integration-tests/fake-openai-server.ts + @qwen-code/sdk DaemonClient against qwen serve, asserting requests.length.

🇨🇳 中文版（完整对应）

✅ 本地验证报告 —— 真实二进制 tmux E2E + 变异测试

我作为维护者在 head 6b4eb8e5a（与 origin/main 的 merge-base：7b9e3188）上验证本 PR：构建了真实的 qwen bundle，跑了聚焦单测，对每一处承重改动做了变异测试；并且针对 PR 描述中明确“本地未跑”的部分（“完整 interactive tmux E2E”），用真实 qwen serve daemon 打一个伪 OpenAI 端点，让 loop guard 在真实产物里端到端地触发。

结论：行为正确，新增测试都承重，建议合并，文末有一条不阻塞合并的提示。

环境： macOS（darwin 25.5.0），Node v22.22.2；npm ci + npm run build(tsc) + npm run bundle 均 exit 0；node dist/cli.js --version → 0.19.3。

1. 单元测试 —— 完整 ACP Session.test.ts

npx vitest run src/acp-integration/session/Session.test.ts
→ Test Files 1 passed | Tests 212 passed (212)

#preserveCancelledPermissionToolRun → #preserveStoppedToolRun 重命名、recordSkippedToolCall 上移这些重构没有引入回归。conversation_finished telemetry 块里的 6 个 loop-guard 测试全部通过。

2. 变异测试 —— 每处改动都承重

每处改动在 src 上回退（vitest 别名到源码），重跑对应测试。变异前都断言锚点恰好匹配一次，所以没有“静默没改到”的假结果。

#	改动	施加的变异	测试	结果
A	稳定的 invalid-tool 分桶（key = 工具名）	回退到 `${toolName}\0${error.message}`	…repeated invalid tool parameters with fresh ids	🔴 被抓
B	loop 检测 = 终止 turn（绕过 Stop-hook）	禁掉 #executePromptInner 里的 if (toolRun.loopDetected) → end_turn	同上（stopHookCalls 必须为 0）	🔴 被抓
C	100-call 上限保留工具响应	parts: []	…exceeding the daemon tool-call cap	🔴 被抓
E	早期工具查找也记一次 strike	去掉 “Tool not found” 上的 recordInvalidToolParams: true	…stops early tool lookup errors	🔴 被抓
D	loop 触发后停止未启动的并发 Agent	D1 顺序预跑 / D2 并发上限 / D3 race 后补填，各自单独	…concurrent Agent…	🟢 存活（多重防线）
D	（同上）	D1 + D2 组合 → 未启动的 agent 真的执行了	…does not start unstarted concurrent Agent calls	🔴 被抓

并发跳过逻辑（D）用了多重冗余防线，所以单行回退抓不到——这是健壮，不是测试空过。同时禁掉两道防线就会让未启动的 agent 真正执行，测试随即翻红，证明这块逻辑确实被测到。

3. 真实二进制 E2E（tmux）—— loop guard 在真 daemon 里触发

真实 qwen serve daemon → 真实 qwen --acp 子进程跑打过补丁的 Session.ts；伪 OpenAI 每个 turn 都要求调用一个不存在的工具（foobar_loop_tool）。权威判据是 daemon 实际调用模型的次数（fakeServer.requests.length）。我做了 A/B：PR head vs. 同一 bundle 但把改动 #4 回退后重建。

构建	模型调用次数	daemon 终止原因
PR head（含修复）	3	[WARN] Stopping ACP turn after repeated tool parameter errors from foobar_loop_tool → end_turn
对照（回退改动 #4，重建）	101	[WARN] Stopping ACP turn after 101 tool calls in one turn（仅靠 100-call 上限）

也就是说，在实际产物里，当模型在一个非法/未知工具上打转时，改动 #4 把白白浪费的模型调用从 101 次降到 3 次，并且 turn 干净地以 end_turn 收尾、保留了 loop 检测上下文。这正是 #6084 报告的失败模式。

4. 一条不阻塞合并的提示（合并前最好有意识地确认）

runConcurrent 现在 maxConcurrency = Math.min(configured, 3)，并且只要存在 toolLoopState 就把每个 batch 的前 3 个调用改成顺序执行——而 daemon 的每个 turn 都有 toolLoopState，不只是正在打转的那些。结果：一个 N>3 的正常并发工具/子代理 batch 不再完全并行——前 3 个被串行化，剩下的并发度被压到 3。对快工具影响可忽略；但对并行 Agent 子代理或其它长耗时工具会降低并行度。这是为了在“放出大量并发工作之前”先发现 loop 而做的有意取舍，但 PR 的 Risk & Scope 里没提。提出来，好让它成为一个有意的决定而不是意外。

---

复现：在 PR head 建 worktree，npm ci && npm run build && npm run bundle；E2E 复用 integration-tests/fake-openai-server.ts + @qwen-code/sdk 的 DaemonClient 打 qwen serve，断言 requests.length。

[qwen-code-ci-bot]

[Suggestion] fillLoopSkippedFrom emits a phantom emitStart event for skipped calls

This call uses the default emitStart = true, which emits a toolCallEmitter.emitStart({status: 'pending'}) followed by emitError for tool calls that were never launched. Two other paths that skip calls due to loop detection correctly pass emitStart = false:

• Turn-cap path (line ~3815): recordSkippedToolCall(fc, LOOP_DETECTED_SKIP_MESSAGE, false)
• runTool loop-detected guard (line ~4243): recordSkippedToolCall(fc, LOOP_DETECTED_SKIP_MESSAGE, false)

The inconsistency means UI/telemetry consumers see orphaned "pending" start events for calls that never dispatched, and the trace differs depending on which detection path fires.

Suggested change

	await recordSkippedToolCall(calls[i], LOOP_DETECTED_SKIP_MESSAGE),
	await recordSkippedToolCall(calls[i], LOOP_DETECTED_SKIP_MESSAGE, false),

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

[Suggestion] invalidToolErrorNearThreshold drain-and-recheck branch has no test coverage

This near-threshold heuristic — drain all in-flight work when any tool's error counter reaches threshold - 1, then re-check for loop detection — is a distinct branch that existing tests never exercise. The concurrent loop-detection tests either trigger loop detection immediately (all 3 calls throw synchronously, so the first results.some(loopDetected) check at line 4065 catches it) or go through the pre-loop sequential path instead.

To cover this path, a test needs at least one concurrent call whose error brings the count to threshold while another promise in the set resolves first with a non-loop result, so Promise.race returns before the threshold-crossing call completes. For example: pre-seed invalidToolParamErrors to count=2 for a tool, include one call that throws the threshold-crossing error asynchronously (after a microtask), and one that resolves quickly with a normal result.

— qwen3.7-max via Qwen Code /review

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅