fix(cli): fail non-interactive runs on loop detection

[cyphercodes]

What this PR does

This makes non-interactive CLI runs treat loop detection as a failure instead of a successful completion. When the model emits a loop-detected event, pending tool calls are skipped, text output still shows the loop message, and JSON output is marked as an error.

Why it's needed

In CI/non-interactive mode, loop detection means the run could not complete the requested task. Exiting with status 0 and publishing a successful JSON result can make automation report false success.

Reviewer Test Plan

How to verify

Run the focused CLI regression test and confirm loop detection exits with code 1 and does not execute queued tool calls.

Evidence (Before & After)

Before: a loop-detected non-interactive run could exit 0 and JSON output could report isError: false.

After: the new regression covers loop detection returning exit code 1, skipping pending tool calls, and setting isError: true / is_error: true in JSON output.

Commands run locally:

npm ci --ignore-scripts
npm run generate
cd packages/cli && npx vitest run src/nonInteractiveCli.test.ts
cd packages/cli && npx eslint src/nonInteractiveCli.ts src/nonInteractiveCli.test.ts
npm run lint --workspace=packages/cli
npx prettier --check packages/cli/src/nonInteractiveCli.ts packages/cli/src/nonInteractiveCli.test.ts
git diff --check HEAD~1..HEAD

npm run typecheck --workspace=packages/cli and npm run build currently fail in this checkout on existing Ink import/type errors in packages/cli/src/ui/components/shared/BaseTextInput.tsx (ink/dom, ink/components/CursorContext, and cursorCtx unknown); this PR does not touch that surface.

Tested on

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

Environment (optional)

Linux cron container, Node/npm from the repository environment.

Risk & Scope

• Main risk or tradeoff: non-interactive loop detection now fails fast before queued tool calls, matching the error semantics expected by automation.
• Not validated / out of scope: unrelated CLI typecheck/build failures in BaseTextInput.tsx were not changed here.
• Breaking changes / migration notes: none expected beyond correcting the exit status for a failed loop-detected run.

Linked Issues

Fixes #5554

中文说明

此 PR 做了什么

此 PR 让非交互式 CLI 在检测到循环时按失败处理，而不是成功完成。模型发出循环检测事件后，会跳过待执行的工具调用，文本输出仍显示循环提示，并且 JSON 输出会标记为错误。

为什么需要

在 CI/非交互模式下，循环检测表示本次任务没有完成。如果仍以状态码 0 退出并发布成功的 JSON 结果，自动化流程会误报成功。

Reviewer Test Plan

如何验证

运行聚焦的 CLI 回归测试，确认循环检测会返回退出码 1，并且不会执行排队中的工具调用。

证据（修改前后）

修改前：检测到循环的非交互式运行可能以 0 退出，并且 JSON 输出可能报告 isError: false。

修改后：新增回归测试覆盖循环检测返回退出码 1、跳过待执行工具调用，并在 JSON 输出中设置 isError: true / is_error: true。

本地运行命令：见英文部分命令列表。

npm run typecheck --workspace=packages/cli 和 npm run build 当前在此检出中因为 packages/cli/src/ui/components/shared/BaseTextInput.tsx 里既有的 Ink 导入/类型错误失败（ink/dom、ink/components/CursorContext、以及 cursorCtx 为 unknown）；此 PR 不修改该范围。

测试平台

Linux 已测试；macOS 和 Windows 未本地测试。

环境（可选）

Linux cron 容器，使用仓库环境中的 Node/npm。

风险与范围

• 主要风险或取舍：非交互式循环检测现在会在执行排队工具调用前失败退出，符合自动化场景对错误语义的预期。
• 未验证 / 不在范围内：未修改与本 PR 无关的 BaseTextInput.tsx 类型检查/构建失败。
• 破坏性变更 / 迁移说明：除修正循环检测失败时的退出状态外，预计没有破坏性变更。

关联 Issue

Fixes #5554

[wenshao]

@qwen-code /triage

[wenshao]

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

Thanks for the PR, @cyphercodes!

Template looks good ✓ — all required sections present, bilingual body, linked issue #5554.

On direction: This is a clear-cut fix for a real automation bug. Issue #5554 provides excellent evidence — a CI triage run hit loop detection, printed the warning, then exited 0 and reported success. GitHub Actions showed green while the task silently failed. This is exactly the kind of silent failure that erodes trust in CI pipelines. Solidly within scope.

Claude Code CHANGELOG has no direct precedent for loop-detection exit semantics, but the area is clearly relevant — they've shipped multiple fixes around loop detection, stop hooks, and /loop behavior.

On approach: The scope is tight — two files, one concern. The strategy is sound: extract message formatting into formatLoopDetectedMessage, add a loopDetected flag to the event processing loop, then check it at every exit point (main stream, drain batch, cron scheduler, final holdback). Every edit in the diff serves the stated goal; no drive-by refactors or scope creep.

The one thing worth noting: emitLoopDetectedResult() duplicates ~15 lines from emitStructuredSuccess() (abort, flush, finalize, compute usage, emit result). The duplication is acceptable here — error vs. success paths with different isError values — but if this pattern grows, a shared helper with an isError parameter would be the natural next step. Not a blocker.

Moving on to code review. 🔍

中文说明

感谢 PR，@cyphercodes！

模板完整 ✓ — 所有必填部分齐全，双语正文，关联 issue #5554。

方向： 这是一个明确的自动化 bug 修复。Issue #5554 提供了充分的证据 — CI triage 运行触发了循环检测，打印了警告，但以 0 退出并报告成功。GitHub Actions 显示绿色，而任务实际失败了。这正是侵蚀 CI 管线信任的那种静默失败。完全在范围内。

Claude Code CHANGELOG 没有循环检测退出语义的直接先例，但该领域明显相关 — 他们在循环检测、停止钩子和 /loop 行为方面发布过多个修复。

方案： 范围紧凑 — 两个文件，一个关注点。策略合理：将消息格式化提取到 formatLoopDetectedMessage，在事件处理循环中添加 loopDetected 标志，然后在每个退出点（主流、drain batch、cron 调度器、最终 holdback）检查它。diff 中的每处编辑都服务于声明的目标；没有顺手重构或范围蔓延。

值得注意的一点：emitLoopDetectedResult() 从 emitStructuredSuccess() 复制了约 15 行代码（abort、flush、finalize、计算 usage、emit result）。这里的重复是可接受的 — 错误与成功路径有不同的 isError 值 — 但如果这个模式增长，带 isError 参数的共享 helper 会是自然的下一步。不构成阻塞。

进入代码审查 🔍

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Code Review

Independent proposal (before reading the diff): Given the bug — loop detection exits 0 in non-interactive mode — I would: (1) add a loopDetected boolean flag in the event processing scope, (2) set it when GeminiEventType.LoopDetected fires, (3) check it after the stream loop to return 1 and emit an error result, (4) propagate the check to drain/cron paths, (5) add tests. This is essentially what the PR does.

Comparison with the diff: The PR's approach matches my proposal and is well-executed. A few specifics:

• formatLoopDetectedMessage() extraction is clean — the old emitLoopDetectedMessage() mixed formatting with I/O; splitting them lets both the TEXT-mode stderr write and the JSON error result reuse the same message string.
• The loopDetected flag is checked at all the right exit points: after the main stream loop (L1156), after drain batch (L1387), in the drain loop iteration guard (L1360), in checkCronDone (L1484), and in the final holdback loop (L1551). No gaps.
• The if (!loopDetected) guard before calling emitLoopDetectedMessage prevents overwriting the first, more specific loop type message if multiple LoopDetected events arrive — correct.
• emitLoopDetectedResult() skips the holdback-for-background-tasks step that emitStructuredSuccess() does. This is correct: when loop detection fires, there's no reason to wait for background task notifications — the run is being aborted.
• In the drain batch inner loop (L1387), return without emitting the result is correct — control flows back to drainLocalQueue(), then to checkCronDone, which stops the scheduler, then the final holdback loop picks up the loopDetected flag and calls emitLoopDetectedResult().

No critical blockers found. No security holes, no regressions, no AGENTS.md violations.

Reuse check: The new logic doesn't duplicate anything already available elsewhere in the codebase. The emitLoopDetectedResult() duplication with emitStructuredSuccess() is noted in Stage 1 — acceptable for now.

Lint & formatting: ESLint and Prettier both pass on the changed files. ✓

Test Results

Unit Tests (tmux capture)

Before (main branch — no loop detection tests)

 RUN  v3.2.4 /home/runner/work/qwen-code/qwen-code/packages/cli
      Coverage enabled with v8

 ✓ src/nonInteractiveCli.test.ts > runNonInteractive > should process input and write text output 13ms
 ✓ src/nonInteractiveCli.test.ts > runNonInteractive > on EPIPE, destroys stdout and returns normally instead of process.exit 2ms
 ✓ src/nonInteractiveCli.test.ts > runNonInteractive > should handle a single tool call and respond 5ms
 ✓ src/nonInteractiveCli.test.ts > runNonInteractive > should ignore duplicate provider tool-call ids across rounds 3ms
 ...
 ✓ src/nonInteractiveCli.test.ts > runNonInteractive > --resume with active worktree (Phase C) > cleans up the sidecar when the worktree dir is gone (stale --resume) 6ms

 Test Files  1 passed (1)
      Tests  51 passed | 1 skipped (52)
   Duration  12.34s

Note: no loop detection tests exist on main. The two tests added by this PR are absent.

After (this PR — 2 new loop detection tests added, both pass)

 RUN  v3.2.4 /home/runner/work/qwen-code/qwen-code/packages/cli
      Coverage enabled with v8

 ✓ src/nonInteractiveCli.test.ts > runNonInteractive > should process input and write text output 13ms
 ✓ src/nonInteractiveCli.test.ts > runNonInteractive > on EPIPE, destroys stdout and returns normally instead of process.exit 2ms
 ✓ src/nonInteractiveCli.test.ts > runNonInteractive > returns non-zero and skips pending tool calls after loop detection 3ms
 ✓ src/nonInteractiveCli.test.ts > runNonInteractive > marks JSON output as an error when loop detection halts the run 3ms
 ✓ src/nonInteractiveCli.test.ts > runNonInteractive > should handle a single tool call and respond 4ms
 ...
 ✓ src/nonInteractiveCli.test.ts > runNonInteractive > --resume with active worktree (Phase C) > cleans up the sidecar when the worktree dir is gone (stale --resume) 6ms

 Test Files  1 passed (1)
      Tests  53 passed | 1 skipped (54)
   Duration  12.36s

Both new tests pass:

• returns non-zero and skips pending tool calls after loop detection — verifies exit code 1, tool calls not executed, stderr message emitted ✓
• marks JSON output as an error when loop detection halts the run — verifies exit code 1, JSON output has is_error: true with loop detection message ✓

All 51 pre-existing tests continue to pass — no regressions.

中文说明

代码审查

独立方案（阅读 diff 前）：鉴于这个 bug — 循环检测在非交互模式下以 0 退出 — 我会：(1) 在事件处理作用域添加 loopDetected 布尔标志，(2) 在 GeminiEventType.LoopDetected 触发时设置，(3) 在流循环结束后检查它以返回 1 并发出错误结果，(4) 将检查传播到 drain/cron 路径，(5) 添加测试。这基本就是 PR 的做法。

与 diff 的比较： PR 的方案与我的独立方案一致，执行良好。具体细节：

• formatLoopDetectedMessage() 提取干净 — 旧版 emitLoopDetectedMessage() 把格式化和 I/O 混在一起；拆分后 TEXT 模式的 stderr 写入和 JSON 错误结果都可以复用同一个消息字符串。
• loopDetected 标志在所有正确的退出点都有检查：主流循环后、drain batch 后、drain 循环迭代守卫、checkCronDone、最终 holdback 循环。没有遗漏。
• 调用 emitLoopDetectedMessage 前的 if (!loopDetected) 守卫防止覆盖第一次更具体的循环类型消息 — 正确。
• emitLoopDetectedResult() 跳过了 emitStructuredSuccess() 的等待后台任务步骤。这是正确的：循环检测触发时，没有理由等待后台任务通知 — 运行正在被中止。
• drain batch 内部循环中的 return（不发出结果）是正确的 — 控制流回到 drainLocalQueue()，然后到 checkCronDone，停止调度器，最终 holdback 循环拾取 loopDetected 标志并调用 emitLoopDetectedResult()。

未发现关键阻塞问题。 无安全漏洞、无回归、无 AGENTS.md 违规。

测试结果

单元测试

修改前（main 分支）： 52 个测试，51 通过，1 跳过。无循环检测测试。

修改后（本 PR）： 54 个测试，53 通过，1 跳过。两个新测试均通过：

• returns non-zero and skips pending tool calls after loop detection — 验证退出码 1，工具调用未执行，stderr 消息已发出 ✓
• marks JSON output as an error when loop detection halts the run — 验证退出码 1，JSON 输出含 is_error: true ✓

所有 51 个既有测试继续通过 — 无回归。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Reflection

Stepping back: this PR fixes exactly what issue #5554 describes — a silent CI failure mode where loop detection prints a warning but the process exits successfully. The kind of bug that nobody notices until they're debugging why a workflow "passed" but produced no output.

The implementation is straightforward and correct. My independent proposal matched the PR's approach, and after reading the code in detail, I don't see a materially simpler path. The loopDetected flag pattern — set on event, check at every exit point — is the minimal solution for this problem in a complex event-driven loop with multiple exit paths (main stream, drain batch, cron scheduler, holdback loop).

The code is clean, well-tested, and does one thing. The two regression tests directly validate the fix: exit code 1 on loop detection, JSON is_error: true, and tool calls skipped. All 51 existing tests continue to pass.

The minor duplication between emitLoopDetectedResult() and emitStructuredSuccess() is noted but acceptable — these are error vs. success paths with different semantics, and extracting a shared helper would add complexity for ~15 lines of overlap.

Verdict: Ships the fix cleanly, tests confirm it works, no regressions. Approving. ✅

中文说明

反思

退一步看：这个 PR 修复的正是 issue #5554 描述的问题 — 一种静默的 CI 失败模式，循环检测打印了警告但进程以成功退出。这种 bug 在有人调试为什么 workflow "通过" 了却没有产出之前，没人会注意到。

实现直接且正确。我的独立方案与 PR 的方案一致，仔细阅读代码后，我没有看到更简单的路径。loopDetected 标志模式 — 在事件上设置，在每个退出点检查 — 是这个复杂事件驱动循环中多个退出路径问题的最小解决方案。

代码干净、测试充分、只做一件事。两个回归测试直接验证了修复：循环检测时退出码 1，JSON is_error: true，工具调用被跳过。所有 51 个既有测试继续通过。

emitLoopDetectedResult() 与 emitStructuredSuccess() 之间的轻微重复已记录但可接受 — 这是错误与成功路径，语义不同，提取共享 helper 会为约 15 行重叠增加复杂度。

结论： 干净地修复了问题，测试确认有效，无回归。批准。 ✅

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[wenshao]

✅ Local verification — real CLI E2E (tmux), LGTM

I verified this PR locally by building the real CLI bundle and driving it end-to-end against a mock OpenAI server that trips the turn_tool_call_cap loop detector — the exact loop type from #5554. Tested all three output formats with a clean before/after differential. Behaves exactly as intended; recommending merge.

Environment

• Linux x64, Node v22, built from this PR head (e38d742e6) on top of v0.18.5.
• Real bundle (dist/cli.js, ~6.15 MB esbuild output), not a mocked unit harness.
• Loop trigger: a mock /v1/chat/completions returning 105 tool calls in one streamed response, each with a distinct name and distinct args, so only the always-on per-turn cap (turnToolCallTotal > 100) can fire — no heuristic detector can shortcut it (and the cap is independent of model.skipLoopDetection). This drives the identical core path as the multi-turn accumulation in the original incident.

1) Unit tests

npx vitest run src/nonInteractiveCli.test.ts   →  53 passed | 1 skipped

Both new tests pass by name:

• ✓ returns non-zero and skips pending tool calls after loop detection
• ✓ marks JSON output as an error when loop detection halts the run

Non-vacuous check: reverting only nonInteractiveCli.ts to base (keeping the new test file) makes both new tests fail with expected +0 to be 1 — confirming they genuinely guard the fix.

2) End-to-end before/after (real CLI, same loop scenario)

Output format	BASE (pre-PR)	PR 5564
text	exit 0, loop msg on stderr (printed twice)	exit 1, loop msg on stderr (once)
json	exit 0, is_error:false, subtype:"success"	exit 1, is_error:true, subtype:"error_during_execution"
stream-json	exit 0, is_error:false, subtype:"success"	exit 1, is_error:true, subtype:"error_during_execution"
num_turns	2 (kept going after the loop)	1 (halted immediately)
model requests / run	2 (extra continuation round-trip)	1 (pending work skipped)

PR json/stream-json error.message:

Loop detection halted the run (turn_tool_call_cap: the model exceeded the maximum
number of tool calls allowed in a single turn). This is an always-on per-turn
tool-call cap and cannot be disabled via `model.skipLoopDetection`.

The BASE column is the bug exactly as described in #5554: a loop-halted run reports is_error:false / subtype:"success" and exits 0 → CI sees false success. The PR turns this into a clean failure across every output mode, and the lower turn/request counts confirm pending tool calls are skipped rather than executed + continued.

3) Happy-path control (no regression)

Same PR build against a normal completion (no loop):

text  → exit 0, stdout: "All done. PONG-5564."
json  → exit 0, is_error:false, subtype:"success", result:"All done. PONG-5564."

The new failure path is scoped strictly to loop detection; successful runs still exit 0.

4) Static checks

• eslint src/nonInteractiveCli.ts src/nonInteractiveCli.test.ts → clean
• prettier --check (both files) → clean
• npm run typecheck --workspace=packages/cli (tsc --noEmit) → pass (exit 0)

Note on the PR description's typecheck/build caveat

The description says npm run typecheck / npm run build fail on pre-existing Ink errors in BaseTextInput.tsx (ink/dom, ink/components/CursorContext). That's an environment artifact, not a code issue: those subpath exports are added by patches/ink+7.0.3.patch, applied by the postinstall: patch-package hook. A npm ci --ignore-scripts (as in the PR's command list) skips it. After applying patches (normal install), both typecheck and the esbuild bundle build succeed. Nothing for this PR to change — just flagging so reviewers don't block on it.

Verdict

LGTM — fixes #5554 correctly and minimally, regression tests are real, no happy-path regression, static checks green. 👍

中文版本（点击展开）

✅ 本地真实验证 —— 真实 CLI 端到端（tmux），建议合并

我在本地构建了真实的 CLI bundle，并用一个 mock OpenAI server 驱动它端到端运行，触发 turn_tool_call_cap 循环检测器（正是 #5554 中的循环类型）。覆盖了三种输出格式，并做了干净的修改前后对比。行为完全符合预期，建议合并。

环境

• Linux x64，Node v22，基于本 PR head（e38d742e6，构建于 v0.18.5 之上）。
• 真实 bundle（dist/cli.js，约 6.15 MB esbuild 产物），不是 mock 单测桩。
• 循环触发方式：mock 的 /v1/chat/completions 在一次流式响应里返回 105 个工具调用，每个工具名与参数都不同，因此只有「始终开启的每轮工具调用上限」（turnToolCallTotal > 100）会触发，任何启发式检测器都无法抢先（且该上限不受 model.skipLoopDetection 影响）。这与原始事故中跨轮累积所走的 core 代码路径完全一致。

1）单元测试

npx vitest run src/nonInteractiveCli.test.ts   →  53 通过 | 1 跳过

两个新增测试按名称通过：

• ✓ returns non-zero and skips pending tool calls after loop detection
• ✓ marks JSON output as an error when loop detection halts the run

非空验证： 仅把 nonInteractiveCli.ts 回退到 base（保留新测试文件），两个新测试都会失败（expected +0 to be 1），证明它们确实在守护该修复。

2）端到端修改前后对比（真实 CLI，相同循环场景）

输出格式	BASE（修复前）	PR 5564
text	退出码 0，stderr 打印循环提示（打印两次）	退出码 1，stderr 打印循环提示（一次）
json	退出码 0，is_error:false，subtype:"success"	退出码 1，is_error:true，subtype:"error_during_execution"
stream-json	退出码 0，is_error:false，subtype:"success"	退出码 1，is_error:true，subtype:"error_during_execution"
num_turns	2（循环后仍继续）	1（立即中止）
每次运行的模型请求数	2（多了一次继续请求）	1（跳过了待执行的工作）

PR 在 json/stream-json 下的 error.message：

Loop detection halted the run (turn_tool_call_cap: the model exceeded the maximum
number of tool calls allowed in a single turn). This is an always-on per-turn
tool-call cap and cannot be disabled via `model.skipLoopDetection`.

BASE 那一列正是 #5554 描述的 bug：被循环中止的运行却报告 is_error:false / subtype:"success" 并以 0 退出 → CI 误判为成功。本 PR 在所有输出模式下都把它变成了干净的失败；更低的轮数/请求数也证明待执行的工具调用被跳过，而不是执行后继续。

3）正常路径对照（无回归）

同一个 PR build，针对正常完成（无循环）：

text  → 退出码 0，stdout: "All done. PONG-5564."
json  → 退出码 0，is_error:false，subtype:"success"，result:"All done. PONG-5564."

新增的失败路径严格限定在循环检测；成功运行仍然以 0 退出。

4）静态检查

• eslint（两个文件）→ 通过
• prettier --check（两个文件）→ 通过
• npm run typecheck --workspace=packages/cli（tsc --noEmit）→ 通过（退出码 0）

关于 PR 描述里 typecheck/build 失败的说明

描述提到 npm run typecheck / npm run build 会因 BaseTextInput.tsx 中既有的 Ink 错误（ink/dom、ink/components/CursorContext）而失败。这是环境问题，而非代码问题：这些子路径导出由 patches/ink+7.0.3.patch 添加，通过 postinstall: patch-package 钩子应用。npm ci --ignore-scripts（如 PR 命令列表所示）会跳过它。正常安装（应用 patch）后，typecheck 和 esbuild 打包都能成功。本 PR 无需改动，仅提示评审者不必为此卡住。

结论

LGTM —— 正确且最小化地修复了 #5554，回归测试真实有效，无正常路径回归，静态检查全绿。👍