Fix/tool call circuit breaker 5234

[aspnmy]

What this PR does

Why it's needed

Reviewer Test Plan

How to verify

Evidence (Before & After)

Tested on

OS	Status
🍏 macOS
🪟 Windows
🐧 Linux

Environment (optional)

Risk & Scope

• Main risk or tradeoff:
• Not validated / out of scope:
• Breaking changes / migration notes:

Linked Issues

中文说明

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Thanks for the PR, @aspnmy! This addresses a real pain point — tool call infinite loops (#5234) are a known problem class and the always-on circuit breaker approach is the right direction.

Template

All headings are present, but every section is still the unfilled placeholder comment. The reviewer test plan, motivation, risk assessment, and linked issues are all blank. Please fill in the template — it's how reviewers understand what you changed and why. The title references #5234 but there's no Closes #5234 in the body.

Direction

Aligned. The issue #5234 triage identified skipLoopDetection defaulting to true as the root cause — loop detection OFF by default with only MAX_TURNS=100 as backstop. This PR takes a better approach than flipping the default: it adds an always-on safety layer before the skipLoopDetection gate, so the hard cap can't be bypassed by configuration. That's a clean design choice. The two new heuristic detectors (global duplicate, alternating pattern) also cover loop variants that the existing consecutive-identical check misses.

Scope — this needs splitting

The PR bundles three unrelated changes in 15 files, but only 5 files relate to the circuit breaker:

Core fix (keep in this PR):

• loopDetectionService.ts — 3 new detectors + always-on API
• loopDetectionService.test.ts — comprehensive tests
• client.ts — integration point
• client.test.ts — test update
• telemetry/types.ts — new LoopType enums

Unrelated — please split into separate PRs:

• React fix e2e #185 fix (7 files): AgentChatContent.tsx, useShellHistory.ts, useLogger.ts, useCommandMigration.ts, useGitBranchName.ts, useWorktreeSession.ts, plus the README.md news entry. This is a separate bug fix with its own issue.
• Chinese README (1 file, 564 lines): README.zh-CN.md — a full translation deserves its own PR and review.
• Misc changes (2 files): agent.ts (subagent polling warning) and enterPlanMode.ts (exit_plan_mode reveal) — each is an independent feature.

Bug: wrong LoopType in checkTurnToolCallCap

checkTurnToolCallCap logs LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS, but it's not detecting consecutive identical calls — it's enforcing a total turn cap. This makes telemetry and debugging misleading. It should use a new LoopType.TURN_TOOL_CALL_CAP (or at minimum something distinct). Please add the enum value and use it.

---

Flagging the scope split and the LoopType bug before diving into code review. Once those are addressed, moving on to Stage 2. 🔍

中文说明

感谢 @aspnmy 的 PR！这解决了真实的痛点——工具调用死循环（#5234）是已知问题类别，始终开启的断路器方案方向正确。

模板

所有标题都在，但每个部分仍是未填写的占位注释。请填写模板——这是审查者理解你改了什么、为什么改的依据。标题引用了 #5234 但正文中没有 Closes #5234。

方向

对齐。#5234 的 triage 确认了 skipLoopDetection 默认为 true 是根因——循环检测默认关闭，只有 MAX_TURNS=100 保底。本 PR 没有改默认值，而是在 skipLoopDetection 门控之前加了始终生效的安全层，使硬上限无法被配置绕过。设计选择很干净。两个新的启发式检测器（全局重复、交替模式）也覆盖了现有连续相同检测遗漏的循环变体。

范围——需要拆分

PR 把三个无关改动打包在 15 个文件中，但只有 5 个文件与断路器相关：

核心修复（保留在此 PR）：

• loopDetectionService.ts — 3 个新检测器 + 始终开启 API
• loopDetectionService.test.ts — 全面测试
• client.ts — 集成点
• client.test.ts — 测试更新
• telemetry/types.ts — 新 LoopType 枚举

无关改动——请拆分为独立 PR：

• React fix e2e #185 修复（7 个文件）：AgentChatContent.tsx、useShellHistory.ts、useLogger.ts、useCommandMigration.ts、useGitBranchName.ts、useWorktreeSession.ts，加上 README.md 新闻条目。这是独立 bug 修复。
• 中文 README（1 个文件，564 行）：README.zh-CN.md——完整翻译值得独立 PR 和审查。
• 杂项改动（2 个文件）：agent.ts（子代理轮询警告）和 enterPlanMode.ts（exit_plan_mode 显示）——各自是独立功能。

Bug：checkTurnToolCallCap 使用了错误的 LoopType

checkTurnToolCallCap 记录 LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS，但它检测的不是连续相同调用——而是总调用量上限。这导致遥测和调试信息误导。应使用新的 LoopType.TURN_TOOL_CALL_CAP。

---

在进入代码审查之前，先标记范围拆分和 LoopType bug。解决后进入 Stage 2。🔍

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Code Review

Independent proposal (before reading the diff): For tool call infinite loops with skipLoopDetection defaulting to true, I would add an always-on safety layer that runs before the gate — a hard per-turn tool call cap that can't be disabled. I'd also add non-consecutive duplicate detection and alternating pattern detection as new heuristic strategies. Integration in client.ts before the skipLoopDetection check. New LoopType enum values for telemetry. This matches what the PR does.

Assessment: the core approach is sound and matches my independent proposal. The integration point is correct (before the gate), the three new detectors cover the main loop variants, and the test coverage is thorough.

Issues found

1. Bug (blocker): Wrong LoopType in checkTurnToolCallCap

private checkTurnToolCallCap(_toolCallKey: string): boolean {
    this.turnToolCallTotal++;
    if (this.turnToolCallTotal > TURN_TOOL_CALL_CAP) {
      this.lastLoopType = LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS;  // ← wrong

This fires when the total tool calls exceed 100, not when consecutive calls are identical. Telemetry will report "consecutive identical tool calls" when the real cause is "turn cap exceeded." This misleads debugging and analytics. Fix: add LoopType.TURN_TOOL_CALL_CAP = 'turn_tool_call_cap' to the enum and use it here.

2. Design note (non-blocking): New heuristic detectors are gated by skipLoopDetection

The global duplicate and alternating pattern detectors live inside addAndCheckHeuristicLoops, which is behind the skipLoopDetection gate. Since skipLoopDetection defaults to true, users with default settings only get the turn cap — the two new heuristic detectors don't activate. This might be intentional (respect the opt-in choice), but it means most users still won't benefit from the smarter detection. Worth discussing whether these should also be always-on.

3. Minor: getLastLoopType()! non-null assertion

value: { loopType: this.loopDetector.getLastLoopType()! },

Works because checkTurnToolCallCap always sets lastLoopType before returning true, but the ! hides this dependency. A comment or a more specific return type would make it less fragile.

Unrelated files (not reviewed in detail)

The 7 React hook files, Chinese README, agent.ts, and enterPlanMode.ts are out of scope for this review — they should be in separate PRs as flagged in Stage 1.

Test Results

Applied the 5 core fix files to a worktree. Ran all loop-related tests:

 ✓ src/services/loopDetectionService.test.ts (56 tests) 280ms
 ✓ src/core/client.test.ts (202 tests | 196 skipped) 118ms

 Test Files  2 passed (2)
      Tests  62 passed | 196 skipped (258)
   Duration  7.40s

New circuit breaker tests (11 tests) all pass — covering turn cap, global duplicate, alternating pattern, disabledForSession interaction, and pattern reset after breaks.

Real-scenario tmux testing isn't feasible for this change: triggering a model loop requires non-deterministic model behavior. Unit tests are the primary verification for internal loop detection logic.

中文说明

代码审查

独立方案（读 diff 前）：对于 skipLoopDetection 默认为 true 导致的工具调用死循环，我会加一个始终生效的安全层——不可禁用的单轮工具调用硬上限。同时加非连续重复检测和交替模式检测作为新启发式策略。在 client.ts 中 skipLoopDetection 检查之前集成。新增 LoopType 枚举值用于遥测。PR 的实现与此一致。

评估：核心方案合理，与我的独立方案一致。集成点正确（在门控之前），三个新检测器覆盖主要循环变体，测试覆盖全面。

发现的问题

1. Bug（阻塞）：checkTurnToolCallCap 中 LoopType 错误

当总工具调用超过 100 时触发，但记录的是 CONSECUTIVE_IDENTICAL_TOOL_CALLS（连续相同调用）。遥测会报告错误原因。修复：添加 LoopType.TURN_TOOL_CALL_CAP = 'turn_tool_call_cap' 并在此使用。

2. 设计说明（非阻塞）：新启发式检测器受 skipLoopDetection 门控

全局重复和交替模式检测器在 addAndCheckHeuristicLoops 内，受 skipLoopDetection 门控。由于默认为 true，默认配置的用户只能获得轮次上限保护——两个新启发式检测器不会激活。值得讨论是否也应该始终开启。

3. 次要：getLastLoopType()! 非空断言

能工作是因为 checkTurnToolCallCap 在返回 true 前总是设置 lastLoopType，但 ! 隐藏了这个依赖关系。

测试结果

将 5 个核心修复文件应用到 worktree。所有循环相关测试通过：62 个测试通过，0 失败。

新的断路器测试（11 个）全部通过——覆盖轮次上限、全局重复、交替模式、disabledForSession 交互和中断后模式重置。

真实场景 tmux 测试对此改动不可行：触发模型循环需要非确定性模型行为。单元测试是内部循环检测逻辑的主要验证方式。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Reflection

The core circuit breaker is well-built. The always-on safety layer before skipLoopDetection is the right design choice — it respects the existing opt-in flag while providing a hard safety net that can't be bypassed. The three new detectors (turn cap, global duplicate, alternating pattern) cover the main loop variants that the existing consecutive-identical check misses. The test coverage is thorough — 11 new tests, 62 total loop-related tests passing, no regressions.

Going back to my independent proposal: the PR matches it closely. I would have done the same thing. The integration point is correct, the constants are reasonable (cap=100, global dup threshold=6, alternating cycles=3), and the reset logic properly clears all new state.

But the PR as a whole has problems that block merge:

1. Scope: 10 of 15 files are unrelated to the circuit breaker. The React fix e2e #185 fix (7 files), Chinese README (564 lines), agent polling warning, and exit_plan_mode reveal are each independent features bundled into one PR. This makes review harder, increases revert risk, and muddies git blame. Each deserves its own PR.

2. LoopType bug: checkTurnToolCallCap logs CONSECUTIVE_IDENTICAL_TOOL_CALLS when it should use a new TURN_TOOL_CALL_CAP type. This is a straightforward fix — add the enum value and use it.

3. Empty template: The reviewer test plan, motivation, risk assessment, and linked issues are all blank. This makes it harder for maintainers to understand intent and assess risk.

What to do:

• Open a new PR with just the 5 core circuit breaker files + filled template + LoopType fix
• Open separate PRs for: React fix e2e #185 fix, Chinese README, agent polling warning, exit_plan_mode reveal

The core logic is ready — it just needs to be in a focused PR.

中文说明

反思

核心断路器实现得很好。在 skipLoopDetection 之前加始终生效的安全层是正确的设计选择——既尊重了现有的 opt-in 标志，又提供了不可绕过的硬保底。三个新检测器（轮次上限、全局重复、交替模式）覆盖了现有连续相同检测遗漏的主要循环变体。测试覆盖全面——11 个新测试，62 个循环相关测试全部通过，无回归。

与我的独立方案对比：PR 实现高度一致。集成点正确，常量合理（上限=100，全局重复阈值=6，交替周期=3），重置逻辑正确清理所有新状态。

但 PR 整体有阻塞合并的问题：

1. 范围：15 个文件中 10 个与断路器无关。React fix e2e #185 修复（7 个文件）、中文 README（564 行）、子代理轮询警告、exit_plan_mode 显示各自是独立功能，不应打包在一个 PR 中。
2. LoopType bug：checkTurnToolCallCap 记录了错误的循环类型。修复简单——添加新枚举值并使用。
3. 空模板：审查测试计划、动机、风险评估和关联 Issue 全部为空。

建议：

• 开一个新 PR，只包含 5 个核心断路器文件 + 填写模板 + 修复 LoopType
• 分别为 React fix e2e #185 修复、中文 README、子代理轮询警告、exit_plan_mode 显示开独立 PR

核心逻辑已就绪——只需要放在一个聚焦的 PR 中。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

The core circuit breaker logic is solid — well-designed, well-tested, and addresses a real user problem. But the PR needs rework before merge:

1. Split unrelated changes — 10/15 files don't belong here (React #185 fix, Chinese README, agent polling warning, enterPlanMode reveal). Please open separate PRs for each.
2. Fix LoopType bug — checkTurnToolCallCap logs CONSECUTIVE_IDENTICAL_TOOL_CALLS but should use a new TURN_TOOL_CALL_CAP type.
3. Fill in the template — the reviewer test plan, motivation, and linked issues are blank.

Once scoped to just the 5 core files with the LoopType fix, this is ready to ship. 🙏

[aspnmy]

你们官方自己管吧，我没时间哦，我修复了自己用，免得消耗无效的token哦

[wenshao]

Thanks for the fix, @aspnmy — and no worries at all. You found and solved a real problem (#5234), and the core circuit-breaker logic here is genuinely solid: the always-on safety layer before the skipLoopDetection gate is the right call, and the test coverage is thorough.

Completely understand you don't want to spend more time on the rework — there's no obligation at all. We'll take it from here: the core logic is ready, so we'll pull it into a focused PR, apply the small LoopType fix, and keep you credited as the original author. Your fix won't go to waste — thanks for finding it and sending it upstream. 🙏

中文

感谢你的修复，@aspnmy，完全没问题。你发现并解决了一个真实的问题（#5234），这里核心的断路器逻辑确实很扎实：在 skipLoopDetection 门控之前加一层始终生效的安全保护是正确的思路，测试覆盖也很完整。

完全理解你不想再花时间做返工——没有任何义务。后面我们来接手：核心逻辑已经就绪，我们会把它提取成一个聚焦的 PR，顺手修掉那个小的 LoopType 问题，并保留你作为原作者的署名。你的修复不会白费——谢谢你发现这个问题并提交到上游。🙏

[wenshao]

Done — carried the core circuit breaker forward in #5279, with your commit cherry-picked so you stay credited as the author. It also includes the LoopType telemetry fix and the missing CLI labels for the new loop types. The unrelated changes from this PR (React #185, Chinese README, etc.) are left out and welcome as their own PRs whenever you like. Thanks again, @aspnmy! 🙏

中文

已接手——核心断路器在 #5279 里继续推进，cherry-pick 了你的那个 commit，保留你作为作者的署名。同时带上了 LoopType 遥测修复和新循环类型缺失的 CLI 标签。本 PR 里的无关改动（React #185、中文 README 等）没有包含进去，欢迎你有空时各自单独提 PR。再次感谢，@aspnmy！🙏

[aspnmy]

完成——在#5279中推进了核心断路器，你的提交内容被挑选出来，确保你是作者。它还包括遥测修复和新循环类型缺失的CLI标签。这个PR中无关的更改（React #185、中文README等）可以省略，随时作为独立PR欢迎。再次感谢！ 🙏LoopType

中文

感谢理解，我只是为了自己不消耗token，至于增加的多余文件是向自己分支提交的自动代码 便于我在自己分支上查看，因为不习惯看英文，抛弃它就行

[wenshao]

明白了——多余的文件（中文 README、news 条目）按你说的都没带进来。不过你那个 React #185 的修复本身是有价值的，已经单独提成 #5286，commit 保留你为作者。

补充一个发现供参考：这个 minified #185 在 React 19 里其实解码成 "Maximum update depth exceeded"（更新死循环），并不是"卸载后 setState"。真正的触发点是 AgentChatContent 在 effect cleanup 里对 provider 调 setAgentShellFocused(false)——子组件和 provider 同一次卸载时可能喂给嵌套更新死循环；去掉它是关键修复。另外 5 个 hook 的 cancelled 防护是附带的防御性加固（React 19 对卸载后 setState 是静默容忍的）。崩溃我这边没能稳定复现，所以 #5286 暂时用 Refs 而不是 Closes，等维护者在类似环境确认后再关 #5199。

再次感谢你把这些问题挖出来并提交上游 🙏

English

Understood — the extra files (Chinese README, news entry) are left out as you said. Your React #185 fix itself is valuable, so I've put it up as its own PR #5286 with your authorship preserved on the commit.

One finding for context: in React 19 this minified #185 decodes to "Maximum update depth exceeded" (an update loop), not a setState-after-unmount. The real trigger is AgentChatContent calling setAgentShellFocused(false) in effect cleanup — when the child and provider unmount together it can feed the nested-update loop; dropping it is the key fix. The five hook cancelled guards are defensive hardening (React 19 tolerates setState-after-unmount silently). I couldn't reproduce the crash reliably here, so #5286 uses Refs rather than Closes until a maintainer confirms on a comparable setup.

Thanks again for finding these and sending them upstream. 🙏

[aspnmy]

补充一个发现供参考：这个 minified #185 在 React 19 里其实解码成 "Maximum update depth exceeded"（更新死循环），并不是"卸载后 setState"。真正的触发点是 AgentChatContent 在 effect cleanup 里对 provider 调 setAgentShellFocused(false)——子组件和 provider 同一次卸载时可能喂给嵌套更新死循环；去掉它是关键修复。另外 5 个 hook 的 cancelled 防护是附带的防御性加固（React 19 对卸载后 setState 是静默容忍的）。崩溃我这边没能稳定复现，所以 #5286 暂时用 Refs 而不是 Closes，等维护者在类似环境确认后再关

不清楚qwen-code自行修复自行维护的本地的技能，所以我还没时间去关注它是为什么关闭iss