fix(core): halt turns on repeated identical tool errors

[yiliang114]

What this PR does

Adds an always-on repeated-tool-error guard to the core loop detector. recordToolResult / recordToolResultByCallId now fingerprint the error payload (functionResponse.response.error) of every failed tool result and halt the turn through the existing LoopDetected path after 3 consecutive error results carry the same signature. Successful results neither advance nor reset the streak — interleaved reads between failing calls must not mask a dead end — and a different error signature restarts the count. Oversized error messages reuse the existing persistence-stub normalization so identical underlying errors fingerprint identically. A new telemetry loop type repeated_tool_error and its non-interactive CLI label (marked always-on, like the consecutive-identical-call guard) complete the wiring.

Why it's needed

Fixes #10887. Production sessions burned 5-14M tokens in dead-end loops: the model kept re-running failing operations with varied arguments (every (tool, args) pair unique) while the same error returned on every call — e.g. git exit 128 / permission denied, with 83% of 153 calls erroring in one case. No existing detector inspects tool results (except the task_list fingerprinting from #9450), so the identical error class never accumulated: argument-based repetition never triggers on varied args, interleaved successful reads keep resetting the stagnation detectors, and 153 calls stays far below the 1000-call backstop. The turn only ended on external truncation.

The existing repeated-tool-failure-guard.ts is not a duplicate: it is ACP-session-only, defaults to shadow (no enforcement), and keys on classified (tool name, error type) rather than repeated error payloads across all runtimes.

Out of scope: the issue's second suggestion (per-session token hard budget) is a larger product decision and intentionally not part of this fix.

Reviewer Test Plan

How to verify

Red test on main before the fix, green with it — the reproduction feeds the detector the reported production shape (distinct failing calls, same error payload, interleaved successes):

cd packages/core
npx vitest run src/services/loopDetectionService.test.ts -t "Repeated tool-error detection"

• Before this PR: the two halt tests fail — recordToolResult returns false for 10 consecutive identical errors across distinct calls (no loop signal at all).
• With this PR: the third consecutive identical error signature returns true with LoopType.REPEATED_TOOL_ERROR; the unpaired-callId path (recordToolResultByCallId, the client.ts wiring) fires the same way.

Regression layer (same file, all passing): 5 new tests in describe('Repeated tool-error detection (issue #10887)') cover the interleaved-success shape, below-threshold + different-signature restart, reset(), and in-session disable; the full file (150 tests) confirms the consecutive-identical-call guard, stagnation, chanting/content detection, caps, and the #9450 stateful task_list guards are unchanged. Plus npm run typecheck in packages/core and packages/cli, and eslint on the changed files.

Evidence (Before & After)

N/A (detector-level behavior, no user-visible UI change; the halt surfaces through the existing loop-detected path).

Before (red test on unfixed code):

FAIL ... halts when the same error keeps returning across distinct calls with interleaved successes
AssertionError: expected false to be true
FAIL ... fires from recordToolResultByCallId for calls this service never streamed
AssertionError: expected false to be true
Tests  2 failed | 3 passed (5)

After:

✓ src/services/loopDetectionService.test.ts (150 tests) 933ms
Tests  150 passed (150)

Tested on

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

Environment (optional)

Unit tests only (vitest in packages/core), Linux, Node v24.

Risk & Scope

• Main risk or tradeoff: the guard halts after 3 byte-identical error results in a row (issue-suggested threshold). A genuinely transient error that repeats its exact payload three times (e.g. three identical timeouts before the network recovers) now halts the turn and reports instead of continuing — that is the issue's requested behavior; successes and changed errors never accumulate.
• Not validated / out of scope: fuzzy/similarity matching of error text (byte-identical signatures only — matches the issue's "error message hash" request); the per-session token budget (issue suggestion 2); the ACP-only repeated-tool-failure-guard.ts remains independent and unchanged.
• Breaking changes / migration notes: none. New telemetry loop_type value repeated_tool_error; detection is always-on like the other result-agnostic safeties (model.skipLoopDetection does not disable it, matching its never-productive trigger; an explicit in-session disable is honored).

Linked Issues

Fixes #10887

中文说明

本 PR 做了什么

在核心循环检测器中新增「重复工具错误」常驻守卫。recordToolResult / recordToolResultByCallId 现在会对每个失败工具结果的错误载荷（functionResponse.response.error）做指纹，当连续 3 次错误结果携带相同签名时，走现有 LoopDetected 路径终止当前轮次。成功结果既不推进也不重置连击——失败调用之间穿插的读操作不能掩盖死循环——出现不同的错误签名则重新开始计数。超大错误消息复用现有的持久化 stub 归一化，保证底层相同的错误指纹一致。配套新增遥测类型 repeated_tool_error 及非交互 CLI 的标签（与「连续相同调用」守卫一样标记为 always-on）。

为什么需要

修复 #10887。生产会话在死循环里烧掉 500 万–1400 万 token：模型不断变体参数重跑失败操作（每个 (tool, args) 组合都唯一），而每次调用都返回同一个错误——例如 git exit 128 / permission denied，其中一个案例 153 次调用里 83% 报错。现有检测器都不检查工具结果（除 #9450 的 task_list 指纹机制），所以相同错误类别从不累积：参数各异使基于参数的重复检测永不触发，穿插的成功读操作不断重置停滞检测，153 次调用又远低于 1000 次兜底，只能靠外部截断才停。

已有的 repeated-tool-failure-guard.ts 不是重复实现：它只作用于 ACP session、默认 shadow（不强制）、且按分类后的 (工具名, 错误类型) 计数，覆盖不到所有运行时的重复错误载荷场景。

范围外：issue 建议 2（每会话 token 硬预算）是更大的产品决策，不在本次修复内。

评审测试计划

如何验证

修复前红测试、修复后绿——复现按生产会话形态喂给检测器（各异的失败调用、相同错误载荷、穿插成功结果）：

cd packages/core
npx vitest run src/services/loopDetectionService.test.ts -t "Repeated tool-error detection"

• 本 PR 前：两个终止用例失败——连续 10 次相同错误（不同调用）recordToolResult 始终返回 false（完全无循环信号）。
• 本 PR 后：第 3 次连续相同错误签名即返回 true，LoopType 为 REPEATED_TOOL_ERROR；未配对 callId 路径（recordToolResultByCallId，即 client.ts 的接线）同样触发。

回归层（同一测试文件，全部通过）：新增 5 个用例覆盖穿插成功形态、低于阈值+不同签名重启、reset()、会话内禁用；全文件 150 个用例确认「同名同参 5 次」守卫、停滞、复读/内容检测、调用上限、#9450 的 task_list 结果感知守卫均不受影响。另有 packages/core 与 packages/cli 的 npm run typecheck、改动文件的 eslint。

前后对比

N/A（检测器层行为，无用户可见 UI 变化；终止通过现有 loop-detected 路径呈现）。

修复前（未修复代码上的红测试）：

FAIL ... halts when the same error keeps returning across distinct calls with interleaved successes
AssertionError: expected false to be true
FAIL ... fires from recordToolResultByCallId for calls this service never streamed
AssertionError: expected false to be true
Tests  2 failed | 3 passed (5)

修复后：

✓ src/services/loopDetectionService.test.ts (150 tests) 933ms
Tests  150 passed (150)

已测试平台

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

环境（可选）

仅单元测试（packages/core 下 vitest），Linux，Node v24。

风险与范围

• 主要风险/权衡：守卫在连续 3 次字节级相同的错误结果后终止（issue 建议的阈值）。真正的瞬时错误若连续 3 次返回完全相同的载荷（如网络恢复前连续 3 次相同超时），现在会终止并报告而不是继续——这正是 issue 要求的行为；成功结果和变化的错误从不累积。
• 未验证/范围外：错误文本的模糊/相似度匹配（只做字节级相同签名——对应 issue 的"错误信息 hash"诉求）；每会话 token 预算（建议 2）；ACP 专属的 repeated-tool-failure-guard.ts 保持独立不动。
• 破坏性变更/迁移说明：无。新增遥测 loop_type 值 repeated_tool_error；检测为 always-on（model.skipLoopDetection 不能关闭，与其"从不产出"的触发性质一致；显式的会话内禁用仍生效）。

关联 Issue

Fixes #10887

[qwen-code-ci-bot]

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

[qwen-code-ci-bot]

Thanks for sticking with this one — three rounds of Critical findings, all of them addressed with mutation-verified tests rather than argued away. Re-running the gate against 8a8240c4.

Template looks good ✓ — every required heading is present, including the Reviewer Test Plan and the Chinese translation.

Problem: observed, not theoretical. Issue #10887 carries four named production sessions (21c6b139, 726277af, 7d6711bd, ed66010d) burning 5–14.6M tokens over 49–138 rounds, one with a measured 83% tool-error rate (127/153), and none of them terminated by anything except external truncation. The gap is real and structural: every existing detector keys on call identity or arguments, so a dead end that varies its (tool, args) on every retry while the same error comes back never accumulates a signal. This clears the "does the problem exist" bar comfortably.

Direction: aligned. Runaway-turn cost is squarely core mission — this is the same class of always-on circuit breaker the turn tool-call cap and the consecutive-identical-call guard already occupy, and it reuses the existing halt path instead of inventing a second one. I checked the claude-code CHANGELOG for a comparable mechanism and found no direct reference to halting on a repeated tool error; the nearest signals are the /usage Loops breakdown for runaway tasks and a fix for auto mode repeatedly re-denying sandboxed network access in long sessions, so the area is clearly relevant even though nobody has shipped this exact guard. The scope decision to leave the issue's second suggestion (per-session token hard budget) out is the right call — that is a product decision, not a bugfix.

Size: core paths are touched, so the breakdown matters — 491 production lines across 9 files, 807 test lines across 4 files, no generated or schema files. The test-to-production ratio is better than 1.6:1, which is what you want to see for a guard whose whole risk surface is false positives. At 491 production lines this sits just under the 500-line maintainer-awareness threshold and well under the 1000-line advisory, and the title is a fix, so neither the large-core-refactor block nor a size escalation applies.

Approach: the shape is right, and specifically the round-based counting is the right call — that was the substance of @qqqys's Critical, and collapsing a parallel batch into one piece of evidence is the correct model of what a "retry" is. Producer-owned digests (shell.ts embedding a sha256 of the stable failure core) instead of unbounded consumer-side heuristic stripping is also the correct direction; that was the round-2 Critical and the fix closes the class rather than the instance. Two notes, neither blocking:

• The PR description no longer matches the implementation. It still says recordToolResult / recordToolResultByCallId fingerprint "every failed tool result" and halt "after 3 consecutive error results", and its How-to-verify cites a 150-test file. The shipped code does the opposite by design — recordToolResult's own new docstring says the guard is not fed there, the guard is fed once per round through recordToolErrorBatch, and the file is now at 161 tests. Per-result versus per-round is exactly the distinction the first Critical turned on, so a maintainer who reads the description first will build the wrong mental model. Worth a description refresh before merge.
• fitText in the batch-budget finalizer now stamps a digest line onto every truncated tool result, not just error payloads. That is defensible (the finalizer cannot tell an error from a success, and the stateful-read guard consumes the same digest), but it does mean a model-visible format change well beyond the error path — flagged so it is a deliberate choice rather than a side effect.

Risk: elevated — Stage 1e matched packages/core/src/tools/shell.ts, which is on this repo's revert-correlated path list. That does not block anything, but it sets the review depth: no Stage 2 enrichment skipped, CI evidence required before any approval, and a named sandboxed lane for the behavioural claim (in Stage 2). The shell.ts change here is additive — a digest line appended to the failure block only, inside the !result.aborted branch, with successes byte-identical — so the blast radius is narrower than the path match suggests, but it is still the file where a green suite that does not pin the change is most expensive.

Moving on to code review. 🔍

中文说明

感谢坚持推进这个 PR——三轮 Critical 发现，全部用「变异验证过的测试」正面修掉，而不是找理由绕开。本次针对 8a8240c4 重新过闸门。

模板完整 ✓——所有必需小节齐全，包含 Reviewer Test Plan 与中文翻译。

问题：已观测，非理论性。Issue #10887 给出了四个具名生产会话（21c6b139、726277af、7d6711bd、ed66010d），在 49–138 轮里烧掉 5–14.6M token，其中一个实测工具错误率 83%（127/153），且除了外部截断之外没有任何机制终止它们。缺口是真实且结构性的：现有检测器都以「调用身份或参数」为键，因此每次重试都变换 (tool, args)、但错误始终相同的死循环永远无法累积信号。这一条完全越过「问题是否存在」的门槛。

方向：对齐。失控轮次的成本属于核心使命——它与「单轮工具调用上限」「连续相同调用」守卫同属常驻熔断器这一类，并且复用现有终止路径，而不是另造一套。我查了 claude-code 的 CHANGELOG，没有找到「因重复工具错误而终止」的直接对应条目；最接近的信号是 /usage 的 Loops 明细（用于发现失控任务）以及一条「长会话中 auto mode 反复重新拒绝沙箱网络访问」的修复，所以即便没人做过完全相同的守卫，这个方向明显是相关的。把 issue 的第二条建议（每会话 token 硬预算）排除在外是正确取舍——那是产品决策，不是 bugfix。

规模：触及核心路径，因此需要拆分——生产代码 491 行、9 个文件；测试 807 行、4 个文件；无生成/schema 文件。测试与生产代码比优于 1.6:1，对一个「全部风险都在误报上」的守卫来说正是希望看到的比例。491 行生产代码刚好低于 500 行的维护者关注阈值，也远低于 1000 行大 PR 建议线，且标题是 fix，因此既不触发大规模核心重构硬阻断，也不触发规模升级。

方案：整体形态正确，尤其是「按轮次计数」这一点——那正是 @qqqys 那条 Critical 的实质，把一个并行批次收敛为一份证据，才是对「重试」的正确建模。「由生产方持有摘要」（shell.ts 内嵌稳定失败核心的 sha256）而非「消费方无边界启发式剥离」，方向也正确；那是第二轮的 Critical，这次修法关闭的是整个类别而不是单个实例。两点备注，均不阻塞：

• PR 描述已与实现不一致。 描述仍写着 recordToolResult / recordToolResultByCallId 对「每个失败工具结果」做指纹、并在「连续 3 次错误结果」后终止，How-to-verify 还引用了一个 150 测试的文件。而实际代码刻意相反——recordToolResult 新增的 docstring 明确说明该守卫不在那里喂入，守卫通过 recordToolErrorBatch 每轮喂入一次，该测试文件现在是 161 个用例。「按结果」与「按轮次」正是第一条 Critical 的关键分野，先读描述的维护者会建立错误的心智模型。建议合并前更新描述。
• 批预算 finalizer 里的 fitText 现在会给每一个被截断的工具结果打上摘要行，而不只是错误载荷。这可以接受（finalizer 无法区分错误与成功，且有状态读守卫消费同一摘要），但确实意味着一个远超错误路径的、模型可见的格式变化——提出来是为了让它成为有意选择而非副作用。

风险：升级——Stage 1e 命中 packages/core/src/tools/shell.ts，该路径在本仓库的「revert 相关路径」名单上。这不阻断任何事情，但决定了 review 深度：不跳过任何 Stage 2 增强项、批准前必须有 CI 证据、并为行为性结论点名沙箱验证通道（见 Stage 2）。这里对 shell.ts 的改动是增量的——仅在 !result.aborted 分支内向失败块追加一行摘要，成功路径逐字节不变——所以实际影响面比路径命中所暗示的要窄，但它仍然是「绿色测试套件却没有钉住改动」代价最高的那个文件。

进入代码审查 🔍

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 8a8240c4cfb1fc9c689219c398b1c2c3a41ee7b4 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review

Reviewed 8a8240c4 statically (no PR code executed — see the testing section for what that means).

My independent proposal before reading the diff, from the title and the Why section alone: fingerprint the error payload of failed tool results inside LoopDetectionService, count model rounds rather than individual calls (a parallel batch of same-tool failures is one event the model has not seen, not N retries), exclude synthetic non-failures, get the stable identity from the producer rather than heuristically stripping rendered text on the consumer side, reuse the existing LoopDetected halt path, and wire both production feed sites. That is very close to what shipped, and the two places the PR goes beyond my baseline — the fitText digest and the telemetry evidence fields — are both responses to findings from earlier rounds rather than scope creep. So the approach matches or exceeds my proposal. One Critical below, on the second of those two additions.

Critical: error_excerpt ships raw tool-error text to a default-on third-party endpoint, unredacted — and for the shell shape it leads with the command line

The observability fix from R1-8 adds two fields to LoopDetectedEvent. The signature is fine. The excerpt is a data-egress problem:

• checkRepeatedToolError passes errorExcerpt: raw, where raw is the un-normalized functionResponse.response.error string — extractToolErrors returns {raw, normalized} and only normalized is hashed.
• The LoopDetectedEvent constructor does details.errorExcerpt.slice(0, 200). Truncation only — no redaction, no control-character stripping.
• For the shell failure shape that issue 10887 is actually about, response.error is llmContent (shell.ts, the third executionError branch: message: typeof llmContent === 'string' ? llmContent : returnDisplayMessage). That block's leading lines are, in order, Command: <the command>, Directory: <cwd>, Output: …. So the first 200 characters are dominated by the command line the model ran and the working directory — not by the failure evidence the field was added to carry.
• logLoopDetected (telemetry/loggers.ts:677-690) sends it two ways: QwenLogger.getInstance(config)?.logLoopDetectedEvent(event) → properties.error_excerpt → batched to gb4w8c3ygj-default-sea.rum.aliyuncs.com; and, when the OTel SDK is initialized, attributes: { ...getCommonAttributes(config), ...event } spreads error_excerpt into OTel log attributes too, so it reaches any configured exporter.
• QwenLogger.getInstance gates only on config.getUsageStatisticsEnabled(), which Config defaults to true (packages/core/src/config/config.ts:2623: params.usageStatisticsEnabled ?? true). This is on by default, not opt-in.

Why that is concretely dangerous rather than theoretical: agent shell commands routinely carry credentials inline — git clone https://x-access-token:ghs_…@github.com/…, curl -H "Authorization: Bearer …", npm publish with a registry token, psql "postgres://user:[REDACTED]@host/db". Those land in the Command: line, i.e. inside the first ~40 characters of the excerpt. And this guard fires precisely when a command keeps failing — a bad-credential command retried three rounds running is one of the most likely triggers this PR itself creates. redactUrlCredentials would catch the URL-userinfo subset (its own tests cover https://user:token@example.com/org/repo.git and https://ghp_token@github.com/owner/repo) but not the header/env/DSN forms.

This also reverses a decision documented in the same file. lastChantExcerpt is deliberately kept off the event — "the excerpt rides the debug log instead, so a headless reasoning-channel halt … leaves an artifact" — and the diff amends that comment to carve out an exception rather than following it. The exception is what puts raw text on a wire instead of in a local log.

Suggested fix, smallest first: drop error_excerpt and keep error_signature alone. The sha256 already satisfies the stated need ("pages arrive with the identity of the failing payload"), and oncall can match it against the debug log or a local reproduction without the CLI ever uploading command text. If the excerpt is genuinely wanted, route it through the existing exported sanitizer truncateSpanError (telemetry/session-tracing.ts:472-474, which is exactly truncateSpanText(redactUrlCredentials(stripAnsiAndControl(s)))), and consider taking it from normalized rather than raw — the normalized form has already had the Command:/Directory:/PGID lines stripped for shell blocks, which removes most of the secret surface and keeps the diagnostic value.

Same hunk, secondary: error_excerpt is placed in properties, while every other verbose or free-form payload in qwen-logger.ts uses snapshots (execution_summary :589, tool_output_truncated :605, token counts :643, truncated_sequence :758). The code comment cites KittySequenceOverflowEvent.truncated_sequence as the convention being followed, but that field rides snapshots, not properties.

Suggestion: extractAnchoredFullDigest duplicates extractAnchoredStubDigest verbatim

The new extractAnchoredFullDigest in packages/core/src/tools/tool-response-finalizer.ts and the pre-existing extractAnchoredStubDigest in packages/core/src/services/loopDetectionService.ts:215-233 have identical bodies — same indexOf(FULL_OUTPUT_DIGEST_LABEL, searchFrom) scan, same line-anchoring test (index === 0 || text[index - 1] === '\n'), same /^[0-9a-f]{64}$/ plus terminator check, same searchFrom advance. Only the parameter name differs (text vs value).

These are the producer and consumer ends of one contract, so they have to keep parsing identically or fingerprints silently diverge — which is the exact failure mode the digest was introduced to prevent. AGENTS.md lists unnecessary duplication as a reviewable violation, and the shared home is obvious: tools/truncation.ts already owns FULL_OUTPUT_DIGEST_LABEL (:34) and the buildStub that mints the line (:520+), and loopDetectionService.ts already imports from it.

Non-blocking coverage notes (both fail safe — the guard under-fires, never over-fires)

• Shell spawn failures bypass the digest entirely. executionError's second branch (result.error truthy) sets the model-facing message to result.error.message, not llmContent, so neither a digest line nor a Process Group PGID: marker reaches normalizeToolErrorText. That message can embed the wrapper command — which is why llmContent runs .replace(commandToExecute, this.params.command) — so it fingerprints per-call and the streak never accumulates. The reported incident shape (command runs, exits non-zero → third branch → llmContent with digest) is covered; ENOENT/EACCES-class spawn failures are not.
• A round carrying two distinct error signatures resets the streak. Siblings collapse correctly via seen, but each distinct signature then runs the update in first-occurrence order, so a session failing [A, B] every round leaves the streak pinned at 1 forever. Worth knowing as the coverage bound for sessions that fail in more than one way at once — this is a design consequence of "a different error signature restarts it", not a bug.

Sequence diagram

The round-based batching across two runtimes is the crux of the fix, so the flow is worth seeing end to end:

sequenceDiagram
    participant P1 as Shell tool (shell.ts)
    participant P2 as Finalizer (tool-response-finalizer.ts)
    participant P3 as Scheduler (coreToolScheduler.ts)
    participant P4 as Chat runtime (client.ts)
    participant P5 as Agent runtime (agent-core.ts)
    participant P6 as Guard (loopDetectionService.ts)
    participant P7 as Telemetry (qwen-logger.ts)
    P1->>P3: failure block plus anchored sha256 of stable core
    P2->>P3: oversized result, envelope plus anchored sha256
    P3->>P4: functionResponse.response.error
    P3->>P5: functionResponse.response.error
    P4->>P6: recordToolErrorBatch (every part of this round)
    P5->>P6: recordToolErrorBatch (every part of this round)
    P6->>P6: normalize, skip synthetic, collapse siblings
    P6->>P6: streak reaches 3 identical signatures across rounds
    P6->>P7: LoopDetectedEvent repeated_tool_error plus excerpt
    P6-->>P4: true, halt turn
    P6-->>P5: true, halt turn

Loading

Files changed (13 of 13 shown)

File	What changed
packages/core/src/services/loopDetectionService.ts	The guard itself: threshold constant, streak state, recordToolErrorBatch, synthetic-payload exclusion, MCP/shell/stub normalization, reset() clears the streak
packages/core/src/services/loopDetectionService.test.ts	Bulk of the test delta — round-based cases, sibling collapse, interleaved successes, MCP shape, persisted-stub shape, disable path
packages/core/src/core/client.ts	Chat-runtime wiring: collects the round's parts, one batch-level call, halt block hoisted out of the per-part loop
packages/core/src/core/client.test.ts	New runFailingToolTurns helper driving real sendMessageStream ToolResult rounds to pin the wiring
packages/core/src/agents/runtime/agent-core.ts	Agent-runtime wiring: one batch call per executed round, sets LOOP_DETECTED terminate mode
packages/core/src/tools/shell.ts	Failure blocks gain an anchored sha256 of the stable failure core; successes unchanged
packages/core/src/tools/shell.test.ts	Pins the digest line on failures and its absence on successes
packages/core/src/tools/tool-response-finalizer.ts	fitText stamps a digest line, reusing an inner producer digest when one exists; adds the duplicated extractor noted above
packages/core/src/core/coreToolScheduler.ts	Exports CANCELLED_TOOL_ERROR_PREFIX; two inline cancellation sites now consume it (payload text byte-identical)
packages/core/src/telemetry/types.ts	New REPEATED_TOOL_ERROR loop type plus the two optional event fields — where the Critical above lives
packages/core/src/telemetry/qwen-logger/qwen-logger.ts	Forwards the two new fields into RUM properties
packages/cli/src/nonInteractiveCli.ts	User-facing label for the new loop type, added to the always-on hint list
packages/cli/src/nonInteractiveCli.test.ts	Pins the always-on hint and the absence of the skipLoopDetection escape text

Testing evidence

This is an unattended CI run, so per the gate's rules I did not build or execute any PR-derived code — everything below is the PR's own CI, read through the API, plus static reading of the diff. 114 check-runs exist on this commit and none has failed.

Final CI results for 8a8240c (auto-updated by the triage finalize job after CI completed):

Check	Conclusion
Test (ubuntu-latest, Node 22.x)	❌ failure
Classify PR	✅ success
Dependency CVE audit	✅ success
Desktop Shell (ubuntu-22.04)	✅ success
Desktop Shell (windows-2022)	✅ success
Integration Tests (no-AK, No Sandbox)	✅ success
Lint & Static (ubuntu-latest, Node 22.x)	✅ success
OpenTUI no-flicker gate	✅ success
Secret scan (TruffleHog)	✅ success
TUI parity snapshots (ink vs opentui)	✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x)	✅ success

_{One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。}

The Linux unit suite — the one leg that would actually run the 807 new test lines — was still in progress when I fetched, so I have no pass/fail result for it. I fetched once and did not poll: this repo's unit suite runs roughly half an hour, longer than any sensible in-agent wait, and the finalize workflow rewrites the table above in place once CI settles. macOS and Windows unit legs are skipped, and Integration Tests (CLI, No Sandbox) is skipped as well — the same integration gap both prior review rounds disclosed, so there is still no integration-level evidence for the halt path in a real session. A /verify run was triggered earlier (Actions run 33873730633) and is also still in flight; no sandboxed report has landed yet.

Explicitly not verified, and why:

• Not verified: that the guard halts a real end-to-end session in the 10887 shape. The CLI integration leg is skipped and I never ran the code.
• Not verified: the author's reported unit counts (161/161 loopDetectionService, 385/385 coreToolScheduler) and the mutation-check claims. Those are the author's numbers, quoted from the PR thread as attribution — not evidence I reproduced, and the one CI leg that could confirm them is still running.
• Not verified: cross-platform behaviour. Both non-Linux unit legs are skipped, and the PR's own Tested-on table marks macOS and Windows ⚠️.
• Not verified: that the new telemetry fields behave correctly at the RUM endpoint. Nothing in CI asserts on uploaded payload contents, which is precisely why the Critical above is invisible to a green suite.

Sandboxed verification would settle part of this: @qwen-code /verify — that a real session with varied arguments and the same git error every round actually halts at round 3, and that a single parallel batch of three or more same-tool denials does not halt, is not observable from this diff or from CI, because the CLI integration leg is skipped and every test calls the service or the client directly. The author has write access, so @qwen-code /tmux is also available if a TUI-level halt message matters. To be clear about the limits: neither lane would catch the Critical — an unredacted field in an uploaded telemetry payload is not something an A/B load-bearing harness asserts on, so that one needs a code change, not more verification.

中文说明

代码审查

针对 8a8240c4 做静态审查（未执行任何 PR 代码——这对证据意味着什么见下方测试部分）。

读 diff 之前，我只根据标题和「为什么需要」写下的独立方案是：在 LoopDetectionService 内对失败工具结果的错误载荷做指纹，按模型轮次而非单次调用计数（一个并行批次里的同工具失败是「模型尚未看到的一个事件」，不是 N 次重试），排除合成的非失败载荷，让生产方提供稳定身份而不是消费方启发式剥离渲染文本，复用现有 LoopDetected 终止路径，并在两个生产喂入点接线。这与实际实现非常接近；PR 超出我基线的两处——fitText 摘要与遥测证据字段——都是对前几轮发现的回应，而非范围蔓延。所以方案与我的独立提案相当甚至更好。下面一条 Critical，正出在这两处追加中的第二处。

Critical：error_excerpt 会把原始工具错误文本发往「默认开启」的第三方端点，且未做脱敏——而对 shell 形态来说，它开头就是命令行

R1-8 的可观测性修复给 LoopDetectedEvent 加了两个字段。签名那个没问题，摘要那个是数据外泄问题：

• checkRepeatedToolError 传的是 errorExcerpt: raw，raw 是未归一化的 functionResponse.response.error 原串——extractToolErrors 返回 {raw, normalized}，只有 normalized 参与哈希。
• LoopDetectedEvent 构造函数只做 details.errorExcerpt.slice(0, 200)：仅截断，无脱敏、无控制字符清理。
• 对 issue 10887 真正涉及的 shell 失败形态，response.error 就是 llmContent（shell.ts 里 executionError 的第三个分支：message: typeof llmContent === 'string' ? llmContent : returnDisplayMessage）。该块的开头几行依次是 Command: <命令>、Directory: <工作目录>、Output: …。所以前 200 字符的主体是模型执行的命令行与工作目录——而不是这个字段本该承载的失败证据。
• logLoopDetected（telemetry/loggers.ts:677-690）会把它送往两处：QwenLogger.getInstance(config)?.logLoopDetectedEvent(event) → properties.error_excerpt → 批量发往 gb4w8c3ygj-default-sea.rum.aliyuncs.com；以及当 OTel SDK 已初始化时，attributes: { ...getCommonAttributes(config), ...event } 会把 error_excerpt 展开进 OTel 日志属性，从而到达任何已配置的导出端。
• QwenLogger.getInstance 只以 config.getUsageStatisticsEnabled() 为门槛，而 Config 对它的默认值是 true（packages/core/src/config/config.ts:2623：params.usageStatisticsEnabled ?? true）。也就是说这是默认开启，而非选择性加入。

为什么这是具体风险而不是理论风险：agent 执行的 shell 命令经常内联携带凭据——git clone https://x-access-token:ghs_…@github.com/…、curl -H "Authorization: Bearer …"、带 registry token 的 npm publish、psql "postgres://user:[REDACTED]@host/db"。这些都落在 Command: 行里，也就是摘要的前 ~40 个字符内。而这个守卫恰好在命令反复失败时触发——一条凭据错误的命令连跑三轮，正是本 PR 自己制造出来的最可能触发场景之一。redactUrlCredentials 能覆盖 URL userinfo 这一子集（它自己的测试就覆盖 https://user:token@example.com/org/repo.git 与 https://ghp_token@github.com/owner/repo），但覆盖不了 header、环境变量、DSN 这些形态。

这同时反转了同一文件里已有明文记录的决定。lastChantExcerpt 是刻意不放进事件的——「摘要改走 debug 日志，这样 headless 推理通道终止时……仍能留下可判别真伪的痕迹」——而本 diff 是修改该注释来开一个例外，而不是遵循它。正是这个例外把原始文本送上了网络，而不是留在本地日志。

修复建议，按改动量从小到大：直接去掉 error_excerpt，只保留 error_signature。sha256 已经满足其声明的诉求（「page 到达时即带有失败载荷的身份」），oncall 可以用它对照 debug 日志或本地复现，而 CLI 完全不必上传命令文本。如果确实需要摘要，请走已有的导出脱敏函数 truncateSpanError（telemetry/session-tracing.ts:472-474，即 truncateSpanText(redactUrlCredentials(stripAnsiAndControl(s)))），并考虑取自 normalized 而非 raw——归一化形态对 shell 块已经剥掉了 Command:/Directory:/PGID 行，这既去掉了大部分密钥面，又保留了诊断价值。

同一处 hunk 的次要问题：error_excerpt 被放进 properties，而 qwen-logger.ts 里其他所有冗长或自由格式的载荷都走 snapshots（execution_summary :589、tool_output_truncated :605、token 计数 :643、truncated_sequence :758）。代码注释声称遵循 KittySequenceOverflowEvent.truncated_sequence 的惯例，但那个字段走的是 snapshots，不是 properties。

建议：extractAnchoredFullDigest 与 extractAnchoredStubDigest 逐字重复

packages/core/src/tools/tool-response-finalizer.ts 新增的 extractAnchoredFullDigest，与既有的 packages/core/src/services/loopDetectionService.ts:215-233 的 extractAnchoredStubDigest，函数体完全相同——同样的 indexOf(FULL_OUTPUT_DIGEST_LABEL, searchFrom) 扫描、同样的行首锚定判断（index === 0 || text[index - 1] === '\n'）、同样的 /^[0-9a-f]{64}$/ 加终止符校验、同样的 searchFrom 推进。只有形参名不同（text 对 value）。

这两份是同一个契约的生产端与消费端，必须保持解析一致，否则指纹会静默分叉——而这正是引入摘要要防止的失效模式。AGENTS.md 把「不必要的重复」列为可评审的违规项，且共享归属很明确：tools/truncation.ts 已经拥有 FULL_OUTPUT_DIGEST_LABEL（:34）与铸造该行的 buildStub（:520+），而 loopDetectionService.ts 本来就从它导入。

非阻塞的覆盖面备注（两者都是安全方向——守卫只会漏报，不会误报）

• Shell spawn 失败完全绕过摘要。executionError 的第二个分支（result.error 为真）把模型可见消息设为 result.error.message 而非 llmContent，因此既没有摘要行、也没有 Process Group PGID: 标记能到达 normalizeToolErrorText。该消息可能内嵌 wrapper 命令——这正是 llmContent 要执行 .replace(commandToExecute, this.params.command) 的原因——于是它按调用唯一地生成指纹，连击永远无法累积。issue 报告的形态（命令能运行、以非零码退出 → 第三分支 → 带摘要的 llmContent）是覆盖到的；ENOENT/EACCES 这类 spawn 失败没有覆盖。
• 单轮内携带两个不同错误签名会重置连击。兄弟调用通过 seen 正确收敛，但随后每个不同签名都会按首次出现顺序执行更新，因此每轮都以 [A, B] 失败的会话，连击会永远停在 1。作为「同一时刻以多种方式失败」的会话的覆盖边界值得知道——这是「不同错误签名重新开始计数」的设计后果，不是 bug。

测试证据

本次为无人值守 CI 运行，因此按闸门规则我没有构建或执行任何 PR 派生代码——以下全部是通过 API 读取的 PR 自身 CI，加上对 diff 的静态阅读。该 commit 上共有 114 个 check-run，无一失败。

CI 表格见上方英文部分（不重复）。Linux 单测任务——唯一会真正跑那 807 行新增测试的环节——在我取值时仍在进行中，所以我没有它的通过/失败结果。我只取一次、不轮询：本仓库单测套件约需半小时，超过任何合理的 agent 内等待预算，且 CI 落定后 finalize 工作流会就地重写上方表格。macOS 与 Windows 单测环节被跳过，Integration Tests (CLI, No Sandbox) 同样被跳过——与前两轮审查披露的集成缺口一致，因此关于「真实会话中的终止路径」仍然没有集成级证据。此前触发的 /verify 运行（Actions run 33873730633）也仍在进行，尚无沙箱报告落地。

明确未验证项及原因：

• 未验证： 守卫能在 10887 形态的真实端到端会话中终止。CLI 集成环节被跳过，我也从未运行代码。
• 未验证： 作者报告的单测数量（loopDetectionService 161/161、coreToolScheduler 385/385）与变异验证结论。那些是作者的数字，此处仅作为归属引用自 PR 讨论串——不是我复现的证据，而唯一能确认它们的 CI 环节仍在运行。
• 未验证： 跨平台行为。两个非 Linux 单测环节均被跳过，PR 自己的 Tested-on 表格也把 macOS 与 Windows 标为 ⚠️。
• 未验证： 新增遥测字段在 RUM 端点的实际行为。CI 中没有任何断言检查上传载荷的内容，这也正是上面那条 Critical 对绿色套件完全不可见的原因。

沙箱验证可以澄清其中一部分：@qwen-code /verify —— 「参数各异、每轮返回同一个 git 错误的真实会话确实在第 3 轮终止」以及「单个并行批次内三个及以上同工具拒绝不会终止」，这两点从本 diff 和 CI 都观察不到，因为 CLI 集成环节被跳过，而所有测试都是直接调用 service 或 client。作者具备写权限，因此若在意 TUI 层的终止提示文案，@qwen-code /tmux 同样可用。需要说清局限：这两条通道都抓不到那条 Critical——上传遥测载荷里的未脱敏字段不是 A/B 载荷证明会去断言的东西，所以它需要改代码，而不是更多验证。

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 8a8240c4cfb1fc9c689219c398b1c2c3a41ee7b4 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 2/5 — the detection logic is genuinely good and every prior Critical is verifiably fixed, but the new telemetry excerpt uploads raw command lines to a default-on third-party endpoint, and I won't approve that as-is.

Stepping back: this is the fourth pass on this PR and the trajectory has been unusual in the best way. Three rounds produced five Critical findings — sibling-batch miscounting, volatile text in the fingerprint, synthetic recovery payloads, per-call artifact paths, and then the whole class of consumer-side heuristic stripping — and not one was argued away. Each was fixed at the level of the class rather than the instance, and the round-2 fix in particular (moving identity ownership to the producer instead of adding a fifth stripping heuristic) is the right instinct: it converts an unbounded surface into a bounded contract. If I had to maintain this in six months I would thank the author for that decision.

I checked every prior Critical against the code as it stands at 8a8240c4, not against the thread's resolution flags:

• @qqqys's sibling-batch Critical — fixed. recordToolErrorBatch receives the whole round and seen collapses repeat signatures within it, so N parallel calls are one piece of evidence. Both runtimes call it exactly once per round (client.ts after the per-part loop, agent-core.ts over toolCallResult.results.flatMap(...)). I also confirmed it cannot be double-fed: requestByCallId is populated only for stateful read tools and recordToolResultByCallId returns false early otherwise, so the per-part path never reaches this guard.
• R1-1 / round-2 R1-1 (volatile fingerprint text, unbounded stripping) — fixed. shell.ts embeds a producer-computed sha256 over the stable failure core; normalizeToolErrorText prefers that anchored digest and keeps the line filter only as a fallback.
• R1-2 (synthetic payloads) — fixed twice over. isSyntheticToolError excludes the orphan-repair constant and the cancellation prefix, and the unknown-callId branch no longer feeds the guard at all.
• R1-3 (per-callId artifact paths) — fixed. fitText stamps a digest and reuses an inner producer digest when one exists; Tool output truncated. joined the stub prefixes.
• R1-4 (MCP), R1-5 (stale docstrings), R2-1 (duplicated cancellation literal) — fixed. The MCP normalization keys on the server payload after the last separator, and the cancellation prefix is now a producer-owned export consumed at both inline sites.

So the guard itself I would merge. What stops me is the field added to close R1-8, and it stops me because of when it fires rather than how often: the excerpt is the raw response.error, the shell shape of that string begins with Command: and Directory:, and the guard's whole purpose is to trigger on a command that fails three rounds running. A wrong credential in a git clone URL, a bearer token in a curl -H, a registry token in an npm publish — those are not exotic inputs for an agent shell, they are ordinary ones, and they sit in the first forty characters of what gets uploaded. usageStatisticsEnabled defaults to true, so this reaches the Aliyun RUM endpoint (and OTel attributes via the ...event spread) without anyone opting in. Once that data is collected it cannot be un-collected, which is why I treat it as blocking even though the fix is small.

It also quietly reverses a decision this same file documents for the chanting detector — raw excerpt stays off the event and rides the debug log — and the diff edits that comment to make an exception instead of following it. I think the exception is the wrong call, and the cheapest resolution is to keep error_signature (which fully satisfies "pages arrive with the identity of the failing payload") and drop the excerpt, or at minimum run it through the exported truncateSpanError and take it from normalized rather than raw.

Two smaller things I'd fold into the same pass: extractAnchoredFullDigest is a verbatim copy of extractAnchoredStubDigest and belongs in truncation.ts beside the constant and the producer it parses; and the PR description still describes the pre-round-based design, which will mislead the next reader on exactly the axis the first Critical turned on.

On the evidence question I want to be plain rather than comfortable. The Linux unit suite was still running when I wrote this, so nothing in CI yet confirms the 807 new test lines pass; the CLI integration leg is skipped, as it was for the two prior rounds; and the author's test counts and mutation claims are the author's, quoted with attribution in Stage 2, not something I reproduced — I did not execute any PR code. What I can stand behind is the static reading: the production-side fixes above are real and I traced each one through the diff. I'd also note that no amount of extra verification settles the Critical — an unredacted field in an uploaded payload is not something an A/B harness or a tmux capture asserts on. @qwen-code /verify would still be worth running for the behavioural claim (a real session halting at round 3, and a three-denial parallel batch not halting), but it is not the thing blocking here.

Requesting changes on the one Critical. Not approving — and since the verdict is not approve, there is no deferred-approval marker on this comment either, so CI landing green will not turn this into an approval behind my back.

中文说明

置信度：2/5 —— 检测逻辑本身确实做得好，此前每一条 Critical 都已可验证地修复；但新增的遥测摘要会把原始命令行上传到「默认开启」的第三方端点，这一点我不能按现状批准。

退一步看：这是本 PR 的第四轮审查，其走向在最好的意义上是不寻常的。三轮里共产出五条 Critical——兄弟批次误计数、指纹中的易变文本、合成的会话恢复载荷、按调用唯一的产物路径，以及最后是「消费方启发式剥离」这整个类别——没有一条被找理由绕开。每条都在类别层面而非实例层面修掉；尤其第二轮的修法（把身份归属交给生产方，而不是再加第五个剥离启发式）是正确的直觉：它把一个无边界的面收敛成一个有边界的契约。如果半年后由我来维护这段代码，我会因为这个决定感谢作者。

我把此前每条 Critical 都对照 8a8240c4 的实际代码做了核验，而不是只看讨论串的 resolved 标记：

• @qqqys 的兄弟批次 Critical —— 已修复。 recordToolErrorBatch 接收整轮结果，seen 在轮内收敛重复签名，因此 N 个并行调用只算一份证据。两个运行时都恰好每轮调用一次（client.ts 在逐 part 循环之后，agent-core.ts 对 toolCallResult.results.flatMap(...)）。我还确认它不会被重复喂入：requestByCallId 只为有状态读工具填充，而 recordToolResultByCallId 在其他情况下提前返回 false，所以逐 part 路径根本到不了这个守卫。
• R1-1 / 第二轮 R1-1（指纹中的易变文本、无边界剥离）—— 已修复。 shell.ts 内嵌由生产方对稳定失败核心计算的 sha256；normalizeToolErrorText 优先取该锚定摘要，行过滤器只作为兜底保留。
• R1-2（合成载荷）—— 双重修复。 isSyntheticToolError 排除 orphan-repair 常量与取消前缀，且 unknown-callId 分支已完全不再喂入该守卫。
• R1-3（按调用唯一的产物路径）—— 已修复。 fitText 会打上摘要，并在已存在内层生产方摘要时复用它；Tool output truncated. 已加入 stub 前缀集合。
• R1-4（MCP）、R1-5（过期 docstring）、R2-1（重复的取消字面量）—— 已修复。 MCP 归一化以最后一个分隔符之后的服务端载荷为键；取消前缀现在是由生产方导出、两处内联站点共同消费的常量。

所以守卫本身我是愿意合的。拦住我的是为关闭 R1-8 而新增的那个字段，而它之所以拦住我，关键在于它在什么时候触发，而不是触发频率：摘要是原始的 response.error，shell 形态下这个串以 Command: 和 Directory: 开头，而这个守卫存在的全部意义就是在某条命令连续三轮失败时触发。git clone URL 里的错误凭据、curl -H 里的 bearer token、npm publish 里的 registry token——对 agent shell 来说这些不是罕见输入，而是常规输入，而它们正好落在被上传内容的前四十个字符里。usageStatisticsEnabled 默认为 true，因此这些数据会在无人选择性加入的情况下到达 Aliyun RUM 端点（并通过 ...event 展开进入 OTel 属性）。这类数据一旦被收集就无法收回，这也是我把它视为阻塞项的原因，即便修复量很小。

它还悄悄反转了同一文件为复读检测器写明的决定——原始摘要不进事件、改走 debug 日志——而本 diff 是修改那段注释来开例外，而不是遵循它。我认为这个例外是错的取舍，而代价最小的收敛方式是保留 error_signature（它已完全满足「page 到达时即带有失败载荷的身份」这一诉求）并去掉摘要，或至少让它走已导出的 truncateSpanError，并取自 normalized 而非 raw。

另有两处较小的问题，建议在同一轮里一并处理：extractAnchoredFullDigest 是 extractAnchoredStubDigest 的逐字拷贝，应当与它所解析的常量及生产方一起放进 truncation.ts；以及 PR 描述仍在描述「按轮次」改造之前的设计，而这恰好会在第一条 Critical 的关键分界上误导下一位读者。

关于证据，我想说得直白而不是让自己舒服。我写下这段时 Linux 单测任务仍在运行，所以 CI 目前还没有任何东西能确认那 807 行新增测试是通过的；CLI 集成环节被跳过，与前两轮情况一致；作者的测试数量与变异验证结论属于作者本人，已在 Stage 2 中带归属引用，不是我复现的结果——我没有执行任何 PR 代码。我能负责的是静态阅读：上面那些生产侧修复是真实的，我逐条顺着 diff 追过了。我还要指出，再多的额外验证也解决不了那条 Critical——上传载荷里的未脱敏字段，不是 A/B 证明或 tmux 抓屏会去断言的东西。@qwen-code /verify 对行为性结论仍然值得跑一次（真实会话在第 3 轮终止，以及三个拒绝组成的并行批次不终止），但它不是这里的阻塞点。

以这一条 Critical 请求修改。不予批准——并且由于结论不是批准，本条评论也不带任何延迟批准标记，因此 CI 转绿不会在我背后把它变成一次批准。

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 8a8240c4cfb1fc9c689219c398b1c2c3a41ee7b4 · re-run with @qwen-code /triage}

[qqqys]

Critical: the error streak counts sibling calls from one parallel batch as sequential retries, so a single denied/cancelled batch ends the turn as a "loop" before the model has seen any error

Reviewed at head a577e6e2bf54085a149217fb3422ad31f0d32061.

Location — packages/core/src/services/loopDetectionService.ts: checkRepeatedToolError (line 642), reached from recordToolResult at line 468 and from recordToolResultByCallId.

Trigger condition. Both production call sites feed the guard one part at a time in a loop over a whole batch:

• packages/core/src/core/client.ts:3877 — for (const part of requestToSend) { … recordToolResultByCallId(functionResponseId, [part]) }
• packages/core/src/agents/runtime/agent-core.ts:1235 — for (const toolResult of toolCallResult.results) { loopDetector.recordToolResult(…) }

So a single assistant turn that emits N parallel tool calls advances the streak N times. Several production error strings carry no per-call content, so siblings collide byte-for-byte:

Source	Message	Per-call unique?
permission hard deny — coreToolScheduler.ts:2865	Tool "${reqInfo.name}" is denied.	no — tool name only
background-agent auto-deny — coreToolScheduler.ts:3505 (fires for every permission-requiring call when getShouldAvoidPermissionPrompts())	Tool "${reqInfo.name}" requires permission, but background agents cannot prompt for confirmation. The tool call was denied.	no
user cancel — createCancelledResponse, coreToolScheduler.ts:964	[Operation Cancelled] Reason: Tool call cancelled by user.	no
execution timeout — createToolTimeoutResult, coreToolScheduler.ts:319	Tool execution timed out after ${display}. …	no — identical for every call sharing a per-tool timeout

createErrorResponse puts exactly that string into functionResponse.response.error, which is what extractToolErrorText fingerprints.

Measured against this head (real LoopDetectionService, the real strings above, recorded one part at a time exactly as client.ts does — three distinct commands in one turn):

DENY batch fired-per-call:   [false,false,true]   lastLoopType: repeated_tool_error
CANCEL batch fired-per-call: [false,false,true]   lastLoopType: repeated_tool_error
control (3 different tools): [false,false,false]

The commands were git push origin main, rm -rf build, npm publish — all different, all denied by the same rule. The third one halts.

Impact. On the third sibling the guard sets the sticky loopDetected, and client.ts:3889-3893 then runs reportError('Loop detected'), endCurrentInteraction('error', 'loop detected', 'loop_detected') and fireLoopDetectedStopFailure(loopType), returning the turn. So a permission rule denying one batch, a background agent auto-denying its first batch, or a user cancelling ≥3 pending calls terminates the turn with the user-facing text "the model kept receiving the same tool error without making progress" and records a loop-detection telemetry event — even though the model emitted all of those calls from a single state and never saw any of the errors. It had no opportunity to adapt, which is precisely the premise the guard's own comment rests on ("a corrected approach that still fails identically is exactly the dead end to surface").

For timeouts this also breaks an explicit contract written at the producer: createToolTimeoutResult is documented as "Reported as a normal tool error so the model can adapt (narrow scope, retry, etc.) instead of the session hanging." Under this guard three sibling timeouts end the session instead.

Two further notes on why this is not covered by existing precedent:

1. This file has already been bitten by exactly this class of false positive, and the fix went the other way — loopDetectionService.ts:128-131: "Thresholds were raised from 5/10 because a prompt like 'summarize this project' legitimately opens with list_directory + several parallel read_file calls in a single turn, which previously tripped the detector." The new guard is threshold 3, tool-agnostic, always-on, and counted per part.
2. The comment's analogy to the consecutive-identical-call guard does not hold: that guard requires identical arguments (a genuine repeat of the same action), whereas this one matches identical error text across different calls — which is what any same-tool parallel batch produces when the message is fixed.

Fix direction. Count model rounds, not parts: collapse siblings so at most one error signature is recorded per batch/round, or only advance the streak when the signature repeats across a round boundary. "Successes neither advance nor reset" can stay — the defect is specifically that N simultaneous calls are treated as N retries. A regression test should feed a single batch of ≥3 same-tool denials (distinct args, identical message) and assert the turn does not halt.

---

中文说明

Critical：错误连击把「同一个并行批次里的兄弟调用」当成了「顺序重试」，因此一个被拒绝/被取消的批次会在模型还没看到任何错误之前就以「检测到循环」终止本轮。

位置：loopDetectionService.ts 的 checkRepeatedToolError（642 行），由 468 行的 recordToolResult 与 recordToolResultByCallId 调用。

触发条件：两个生产调用点都是逐个 part 循环喂入整个批次——client.ts:3877 对 requestToSend 里每个 functionResponse 各调一次 recordToolResultByCallId，agent-core.ts:1235 对 toolCallResult.results 各调一次 recordToolResult。因此模型一次发出 N 个并行调用，连击就推进 N 次。而多条生产错误串不含任何单次调用信息，兄弟调用之间逐字节相同：权限硬拒绝 Tool "X" is denied.（coreToolScheduler.ts:2865，只有工具名）、后台 agent 自动拒绝（3505 行，getShouldAvoidPermissionPrompts() 下对每个需要授权的调用都触发）、用户取消 [Operation Cancelled] Reason: Tool call cancelled by user.（964 行）、执行超时（319 行，同一超时值下完全相同）。createErrorResponse 正是把该串放进 functionResponse.response.error，即 extractToolErrorText 取指纹的地方。

在本 head 上实测（真实服务、真实错误串、按 client.ts 的方式逐个记录，一轮内三条不同命令）：拒绝批次 [false,false,true]、取消批次 [false,false,true]、对照组（三个不同工具）[false,false,false]，lastLoopType 均为 repeated_tool_error。三条命令是 git push origin main / rm -rf build / npm publish，互不相同，被同一条规则拒绝，第三条即终止。

影响：第三个兄弟调用把粘性的 loopDetected 置位，client.ts:3889-3893 随即执行 reportError('Loop detected')、endCurrentInteraction('error','loop detected','loop_detected') 与 fireLoopDetectedStopFailure(loopType) 并返回本轮。于是一条权限规则拒绝一个批次、后台 agent 的第一个批次被自动拒绝、或用户取消 ≥3 个待执行调用，都会以「模型一直收到相同的工具错误且毫无进展」这句话结束本轮，并记录一次 loop-detection 遥测——而模型是从同一个状态一次性发出这些调用的，从未看到其中任何一个错误，根本没有机会调整；这与守卫自身注释所依赖的前提（「纠正后仍以完全相同方式失败才是要暴露的死胡同」）相矛盾。对超时而言还破坏了生产方写明的契约：createToolTimeoutResult 的注释是「作为普通工具错误上报，以便模型调整（缩小范围、重试等），而不是让会话挂住」。

另外两点说明为何不能以既有先例为由忽略：本文件曾被同一类误报咬过，而修法是往相反方向走的（128-131 行：阈值从 5/10 上调，因为「像『总结这个项目』这样的提示会正当地以 list_directory + 单次轮内多个并行 read_file 开头，此前会触发检测器」）；而注释里与「连续相同调用守卫」的类比也不成立——那个守卫要求参数相同（同一动作的真实重复），本守卫匹配的却是不同调用之间的错误文本相同，这正是同一工具并行批次在消息固定时的必然产物。

修复方向：按模型轮次而非 part 计数——合并兄弟调用使每个批次/轮次最多记录一个错误签名，或仅在签名跨轮边界重复时推进连击。「成功结果既不推进也不重置」可以保留，缺陷具体在于把 N 个同时发出的调用当成了 N 次重试。回归测试应喂入单个批次内 ≥3 个同工具拒绝（参数不同、消息相同），断言本轮不终止。

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not reviewed: reverse audit — stopped before round 4 by the review time budget.

Test Plan (not a blocker): src/services/loopDetectionService.test.ts — no such file or directory; Tests 150 passed — this review observed 28113, 23156, 1919, 298, 1772, 504, 5822, 94 passed; 3 passed — this review observed 28113, 23156, 1919, 298, 1772, 504, 5822, 94 passed.

[Critical] Re-affirming the existing blocker from review 5101833884 (@qqqys), still standing at the reviewed head: the error streak counts sibling calls from ONE parallel batch as sequential retries. Both production feed sites record results one part at a time in a loop over the whole batch (client.ts:3877 per requestToSend part; agent-core.ts:1235 per toolCallResult.results entry), and fixed-string errors — permission deny 'Tool "X" is denied.' (coreToolScheduler.ts:2865), background-agent auto-deny (:3505), user cancel '[Operation Cancelled]' (:964), execution timeout (:319) — collide byte-for-byte across siblings, so a single denied/cancelled/timed-out batch of >=3 ends the turn as a 'loop' before the model has seen any of the errors. Code at every cited site is byte-identical to the reviewed head a577e6e (only a merge of main landed after it); the original review carries the probe ([false,false,true] for deny and cancel batches of three distinct commands). Fix direction from that review: count model rounds, not parts — collapse siblings so at most one error signature is recorded per batch, or advance the streak only across round boundaries.

中文说明

仅完成部分审查，审查缺口已披露。

未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

未审查：反向审计——评审时间预算不足，未能开始第 4 轮。

Test Plan（非阻断）：src/services/loopDetectionService.test.ts — no such file or directory; Tests 150 passed — this review observed 28113, 23156, 1919, 298, 1772, 504, 5822, 94 passed; 3 passed — this review observed 28113, 23156, 1919, 298, 1772, 504, 5822, 94 passed。

[Critical] Re-affirming the existing blocker from review 5101833884 (@qqqys), still standing at the reviewed head: the error streak counts sibling calls from ONE parallel batch as sequential retries. Both production feed sites record results one part at a time in a loop over the whole batch (client.ts:3877 per requestToSend part; agent-core.ts:1235 per toolCallResult.results entry), and fixed-string errors — permission deny 'Tool "X" is denied.' (coreToolScheduler.ts:2865), background-agent auto-deny (:3505), user cancel '[Operation Cancelled]' (:964), execution timeout (:319) — collide byte-for-byte across siblings, so a single denied/cancelled/timed-out batch of >=3 ends the turn as a 'loop' before the model has seen any of the errors. Code at every cited site is byte-identical to the reviewed head a577e6e (only a merge of main landed after it); the original review carries the probe ([false,false,true] for deny and cancel batches of three distinct commands). Fix direction from that review: count model rounds, not parts — collapse siblings so at most one error signature is recorded per batch, or advance the streak only across round boundaries.

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Thanks for the thorough analysis, @qqqys — confirmed and fixed in e962446.

Verified at head (e81c8773a, post-main-merge): both production call sites fed the guard one part at a time over a whole batch (client.ts ~L3866 per-part recordToolResultByCallId loop over requestToSend; agent-core.ts ~L1233 per-result recordToolResult loop over toolCallResult.results), and checkRepeatedToolError advanced the streak per call — so a batch of N siblings with a fixed error message (permission deny, background-agent auto-deny, user cancel, shared timeout) advanced the streak N times and tripped at the third sibling before the model had seen any error.

Fix — counting model rounds, per your suggested direction:

• New round-level API LoopDetectionService.recordToolErrorBatch(responseParts): client.ts now collects the message's functionResponse parts and records them once per ToolResult message; agent-core.ts flattens the round's results and records once per executed batch.
• checkRepeatedToolError collapses siblings: the streak advances at most once per distinct error signature per round (first-occurrence order). N simultaneous calls from one model state are one piece of evidence; the repeat is the same signature returning on the NEXT round.
• The per-call error-guard invocation was removed from recordToolResult and from the unknown-callId branch of recordToolResultByCallId (reverted to if (!request) return false;); unknown-callId errors still reach the guard via the round-level call.
• "Successes neither advance nor reset" and "a different signature restarts the streak" are unchanged.

Regression test counts sibling calls of one parallel batch as ONE round, not as retries: one batch of 4 identical Tool "run_shell_command" is denied. results (distinct callIds) does not trip the guard and leaves lastLoopType null; the same error returning on the next two rounds trips at round 3. Mutation-verified: removing the in-batch collapse turns the test red. The three-sibling-timeout shape (which broke the createToolTimeoutResult contract) is covered by the same collapse, and synthetic orphan-repair/cancellation payloads are additionally excluded from the fingerprint entirely (separate R1-2 fix in the same commit).

Verification: loopDetectionService suite 155/155; 663 tests across the six directly-hit suites (loopDetectionService, tool-response-finalizer + integration, truncation, client, agent-core); tsc --noEmit clean on packages/core; eslint/prettier clean. Genuinely repeated sequential errors still halt at threshold 3 — the guard's loop-halting behavior for the issue-#10887 dead-end shape is preserved.

[qqqys]

The blocking finding from my earlier review is addressed at eb846eb5de — verified empirically, clearing my request-changes.

Original finding (head a577e6e2bf): the error-repetition guard was keyed on the result payload but fed one part at a time, so the siblings of ONE parallel batch counted as sequential retries — three distinct denied/cancelled calls in a single turn reached the threshold before the model had seen any of the errors.

Fix verified at this head:

1. Both runtimes now feed the guard once per round through recordToolErrorBatch — client.ts once per ToolResult message, agent-core.ts once per executed batch. Grep confirms these are the only two production call sites; the per-part loop now only feeds the request-paired guard.
2. checkRepeatedToolError collapses sibling calls of a round via a per-round signature set — the streak advances at most once per distinct signature per round, in first-occurrence order.
3. Synthetic non-failures are excluded: ORPHAN_TOOL_USE_REPAIR_REASON (verified exported at llm-chat.ts:1526) and the [Operation Cancelled] Reason: prefix.

Empirical probe against this head (driving the real LoopDetectionService with fixture strings different from the PR's own suite; the PR's 10 new tests also pass locally, and Test/Lint/Integration/Desktop are green in CI):

• one batch of 3 same-text denies → does not fire (previously fired) ✓
• the same deny text returning on rounds 2 and 3 → fires with repeated_tool_error ✓
• user-cancel payloads × 5 rounds → never fires ✓
• orphan-repair payloads × 5 rounds → never fires ✓
• rotating distinct errors × 9 rounds → never fires ✓

Non-blocking observations, no action requested: when one round carries several different error signatures, the last-processed one owns the streak slot, which can under-count a persistent error that shares its rounds with another — but that failure direction is under-halting, i.e. pre-PR behavior. The red Dependency CVE audit lane is the stale-base class (the lane is green on main's HEAD 60161cb6; this diff touches no dependencies), and the cancelled web-shell E2E smoke is pool contention — neither is caused by this diff.

This is a comment, not an approval — leaving the merge decision to the maintainers and ci-bot's re-review of the new head.

中文说明

我之前 review 提出的阻塞性问题在 eb846eb5de 已修复——经实测验证，撤回我的 request-changes。

原问题（a577e6e2bf）： 错误重复守卫按结果负载计键，但逐 part 喂入，导致同一并行批次的兄弟调用被当作连续重试——单个回合内三次不同的被拒/取消调用就达到阈值，而模型尚未看到任何错误。

已在当前 head 验证的修复：

1. 两个运行时都改为每轮通过 recordToolErrorBatch 喂一次（client.ts 每条 ToolResult 消息一次，agent-core.ts 每个执行批次一次）——grep 确认这是仅有的两个生产调用点，逐 part 循环现在只喂请求配对守卫。
2. checkRepeatedToolError 通过每轮签名集合折叠兄弟调用——每个不同签名每轮最多推进一次计数。
3. 合成非失败负载被排除：ORPHAN_TOOL_USE_REPAIR_REASON（已在 llm-chat.ts:1526 验证导出）和 [Operation Cancelled] Reason: 前缀。

针对当前 head 的实证探针（使用与 PR 自身测试不同的 fixture 字符串驱动真实 LoopDetectionService；PR 的 10 个新测试本地同样通过，CI 中 Test/Lint/Integration/Desktop 均绿）：

• 单个 3 个同文本拒绝的批次 → 不触发（此前会触发）✓
• 同一拒绝文本在第 2、3 轮再次出现 → 以 repeated_tool_error 触发 ✓
• 用户取消负载 × 5 轮 → 从不触发 ✓
• orphan 修复负载 × 5 轮 → 从不触发 ✓
• 轮换的不同错误 × 9 轮 → 从不触发 ✓

非阻塞观察（无需处理）：一轮内携带多个不同错误签名时，最后处理的签名占据 streak 槽位，可能导致与另一错误共享轮次的持续错误被少计——但该失败方向是少停，即回到 PR 之前的行为。红色的 Dependency CVE audit 属于基线过旧类（main HEAD 60161cb6 上该 lane 为绿，本 diff 不涉及依赖），web-shell E2E smoke 的取消是资源池争用——均非本 diff 引起。

此为评论而非批准——合并决定留给 maintainer 与 ci-bot 对新 head 的复审。

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

• RA3 MCP functionCall fingerprint gap — already reported as R1-4 (comment 3926193221), acknowledged and deferred by the author

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not reviewed: build-and-test — packages/cli unit suite timed out at full deadline (infrastructure); packages/sdk-typescript, vscode-ide-companion, web-shell, webui suites never ran under the whole-call budget; core's 14 failing test files are untouched by this diff (path rule; test-delta base rerun timed out so it could not rule).

Not reviewed: test-efficacy — probe inconclusive, harnessValidated null (no green probe baseline; neither validated nor refuted).

Test Plan (not a blocker): src/services/loopDetectionService.test.ts — no such file or directory; Tests 150 passed — this review observed 23152, 1919 passed; 3 passed — this review observed 23152, 1919 passed.

中文说明

仅完成部分审查，审查缺口已披露。

本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。

未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

未审查（原文为英文）：build-and-test — packages/cli unit suite timed out at full deadline (infrastructure); packages/sdk-typescript, vscode-ide-companion, web-shell, webui suites never ran under the whole-call budget; core's 14 failing test files are untouched by this diff (path rule; test-delta base rerun timed out so it could not rule).

未审查（原文为英文）：test-efficacy — probe inconclusive, harnessValidated null (no green probe baseline; neither validated nor refuted).

Test Plan（非阻断）：src/services/loopDetectionService.test.ts — no such file or directory; Tests 150 passed — this review observed 23152, 1919 passed; 3 passed — this review observed 23152, 1919 passed。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

@qwen-code /triage

[qwen-code-ci-bot]

Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 50 passed · 0 failed · 50 total

Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。

脚本断言：50 通过 · 0 失败 · 50 总计

抖动门：✅ 4 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR #10916 — Deep Verification Report

Verdict: findings — the central claim is proven load-bearing (clean A/B against the base build; 50/50 scripted assertions passed, fail=0), and every targeted gate is green. Six non-blocking findings are reported below: two unpinned load-bearing surfaces, one measured model-visible regression in the batch-budget finalizer, one blind spot in the guard's own counting rule, one latent runtime divergence, and one stale number in the PR description.

• Verified head OID: 8a8240c4cfb1fc9c689219c398b1c2c3a41ee7b4 (git rev-parse HEAD^2)
• Base control: 80497a74d0e807f4640b60f7fe482bb97202408a (HEAD^1, the merge-ref base tip)
• Assertions: pass=50, fail=0, total=50 (assertions.json)
• Gates: packages/core + packages/cli typecheck clean; affected suites green (997 core tests across 6 files, 136 CLI tests); eslint clean on all 9 changed files.

中文摘要

结论：findings（有发现，非阻塞）。

A/B 结论：中心主张成立。用真实 shell 工具（真实 spawn 子进程）产出真实错误载荷，喂给真实编译产物里的 LoopDetectionService，分别跑 head 与 base 两个构建。issue #10887 报告的生产形态（git exit 128、每轮参数不同、中间穿插成功读取）在 head 上第 3 轮即以 repeated_tool_error 终止；base 上 6 轮完全没有任何循环信号。13 个场景全部符合预期，50 条脚本断言全过（base 侧按预期"红"计为通过）。

归因单元（S4/S4b）把这次修复里捆绑的两个机制拆开了：多行命令的续行变化，只靠消费端的行锚定易变行过滤器无法收敛（S4b 不终止）；是 shell.ts 生产端摘要让它成立（S4 第 3 轮终止）。

主要发现：

1. agent-core.ts 的接线无任何测试覆盖（M13 变异存活，997 个测试全绿），但它是承重的——S11 单元证明 agent 运行时的终止完全依赖这一行。
2. 遥测 error_excerpt 的 200 字符截断无测试覆盖（M14 存活）。
3. fitText 的摘要行是模型可见的，且对成功结果同样生效：每个被截断的槽位恒定挤掉 85 字符真实内容；"零内容"阈值从每槽 ≤29 字符上移到 ≤114 字符（实测每槽 100 时 head 全部 200 槽为 0 字符，base 仍有 71 字符）。默认 200k 预算下不触发。
4. 守卫自身的计数规则有盲区：同一轮内出现 ≥2 个不同错误签名时，每轮自我重置，这样的死循环无论跑多少轮都不会触发（S8，8 轮未终止）。
5. client.ts 会过滤掉没有 functionResponse.id 的 part，agent-core.ts 不会——同一守卫在两个运行时结论不同（S11）。经排查调度器所有错误响应都带 id，故为潜在而非已证实的缺陷。
6. PR 描述里的测试数（150）与实际（161）不符。

未覆盖范围：逐 commit 归因（浅克隆，9 个 commit 只可达 1 个）；真实模型驱动的端到端轮次；shell 超时分支；真实 MCP server；交互式 TUI 呈现；base 构建中那个既有的 @lydell/node-pty 类型错误（与本 PR 无关，两侧同一文件同一依赖）。

1. Scope

Central claim — an always-on guard fingerprints the error payload of failed tool results and halts the turn through the existing LoopDetected path after 3 consecutive rounds carry the same signature, where base produces no loop signal at all for that shape.

Secondary claims tested — (a) successful results neither advance nor reset the streak; (b) sibling calls of one parallel batch collapse into one round of evidence; (c) varied-argument retries of the same underlying failure fingerprint identically (this is the crux of #10887, and it is carried by producer-side normalisation in shell.ts, mcp-tool.ts shapes and the finalizer, not by the detector alone); (d) synthetic payloads (orphan repair, user cancellation) are excluded; (e) the wiring reaches a real halt in client.ts and agent-core.ts.

Explicitly out of scope — see §6.

2. Central claim: A/B against the base build

Both arms drive real producers into the real detector: a real ShellToolInvocation spawns a real child process and returns the real model-facing error.message; that payload is fed as functionResponse.response.error into the real LoopDetectionService compiled from each tree. Nothing on the path under test is stubbed. The only harness-supplied element is the runtime wiring, transcribed from client.ts (per-part recordToolResultByCallId, then one recordToolErrorBatch per ToolResult message) and from agent-core.ts, so each build is driven the way its own runtime drives it.

scenario (real commands, real spawns)	head	base	oracle
S1 git exit 128, varied -m arg, interleaved successful read — the #10887 shape	halts r3, repeated_tool_error	never halts	loop type + round
S2 command-not-found (exit 127), fixed tool, varied args	halts r3	never	loop type + round
S3 EACCES (exec a directory), varied args	halts r3	never	loop type + round
S4 multi-line command, varied continuation line, stable output	halts r3	never	loop type + round
S4b identical to S4 with the producer digest line stripped	never halts	never	attribution
S5 four identical sibling failures in ONE round	halts r3 (not r1)	never	round
S6 [Operation Cancelled] Reason: … × 6 rounds	never	never	no halt
S7 exactly 2 rounds (threshold boundary)	never	never	no halt
S8 two distinct signatures inside every round × 8	never	never	no halt (gap, §5.4)
S9 alternating signatures across rounds × 8	never	never	no halt (by design)
S10 fail, fail, clean round, fail	halts r4	never	round
S11 id-less parts, client.ts wiring	never	never	divergence (§5.5)
S11 id-less parts, agent-core.ts wiring	halts r3	never	divergence (§5.5)

The central claim holds: 4/4 dead-end shapes flip from "no loop signal at all" on base to a repeated_tool_error halt on head at exactly the 3rd round, while the four negative controls (S6, S7, S9, and S5's first round) correctly do not halt.

S4b is the attribution cell the two-cell A/B cannot reach. The fix bundles two mechanisms — a producer-side digest emitted by shell.ts, and a consumer-side volatile-line filter in loopDetectionService.ts. Stripping only the digest line from an otherwise identical payload leaves the guard unable to collapse a varied multi-line command (never halts), so for that shape the producer digest is what carries the fix, not the filter. Both halves are independently pinned by tests (M06 kills 2, M09 kills 1, and the M10 combination kills 3 — so neither is redundant defence).

Control hygiene. git diff --name-only HEAD^1..HEAD contains no package.json or package-lock.json, so the dependency tree is identical on both arms and reusing the root node_modules is a clean control. packages/core declares zero @qwen-code/* workspace dependencies, so the internal-symlink confound (a "base" harness silently loading head code through node_modules/@qwen-code/*) cannot arise here — verified rather than assumed. Base dist was confirmed guard-free (recordToolErrorBatch count 0 in dist/src/services/loopDetectionService.js, stableFailureCoreLines count 0 in dist/src/tools/shell.js) and head dist confirmed to carry both (4 and 3).

3. Fingerprint identity — the mechanism, checked against the real finalizer

The guard's whole value rests on "identical underlying errors fingerprint identically". Driven through the real compiled enforceFunctionResponseBudget (the exported seam that calls fitText), with identical inputs on both arms:

property	head	base
same text fitted at 20 000 and 4 000 chars → one digest	a02d4916… = a02d4916… ✅	no digest emitted
previews still differ (digest does not collapse them)	✅	✅
re-truncating an already-stubbed result reuses the inner producer digest	ffff… reused ✅	passed through
two different outputs → two different digests	a02d4916… ≠ 636aa122… ✅	n/a

The inner-digest reuse matters: without it the envelope's per-call unique artifact path would be hashed, fingerprinting every retry uniquely and silently disabling the guard for exactly the largest errors. M11 (disabling that reuse) is killed by a test, so the property is pinned.

4. Corrections to the PR description

These are corrections to the description, not requests to change code.

• "the full file (150 tests)" — at the verified head, src/services/loopDetectionService.test.ts collects 161 tests (Tests 884 passed across the three affected core files: 161 + 326 shell + 397 client). The 150 in the Before/After block is stale relative to the head that was merged-rebased.
• "Only the fingerprint derivation changes — the model-facing message is untouched." This is stated in the MCP normaliser's doc comment and is true for MCP (verified: normalizeMcpToolError only derives fingerprint text). It should not be read as covering the finalizer change: fitText rewrites the text the model actually reads, for successful results as well as errors — see §5.3, which is the one place this PR does change model-visible output.
• Per-commit claims were not individually exercised. The snapshot lists 9 commits; the depth-2 merge-ref checkout makes exactly 1 reachable (git rev-list HEAD^1..HEAD^2 → 8a8240c4, and git rev-parse --is-shallow-repository → true). Everything here verifies the aggregate HEAD^1..HEAD diff.

5. Findings

Ordered by severity. None blocks the central claim; all are concrete and reproducible.

5.1 The agent-runtime wiring is load-bearing and pinned by nothing (Suggestion)

Removing the recordToolErrorBatch call from agents/runtime/agent-core.ts (M13) leaves all 997 tests green. Yet S11 shows the agent-runtime wiring is exactly what makes an id-less-part round halt — the call is not decorative.

grep -rln recordToolErrorBatch packages/core/src packages/cli/src --include=*.test.ts returns only loopDetectionService.test.ts and client.test.ts; no test reaches the agent path. So the guard's second runtime entry point has no regression protection: a future refactor can drop it silently, and the agent runtime would quietly stop halting on repeated tool errors while the whole suite stayed green.

Reproduce:

cd /__w/qwen-code/qwen-code
python3 tmp/pr10916-verify-20260904-125905/harness/mutation-matrix.py   # row M13

Suggested fix (not applied — a test is the author's call): one agent-runtime test that drives a round of failing tool results through the AgentCore loop and asserts AgentTerminateMode.LOOP_DETECTED. The fixture that would go red is precisely M13.

5.2 Telemetry error_excerpt truncation is unpinned (Suggestion)

M14 (removing .slice(0, 200) from LoopDetectedEvent) also survives 997/997. grep -rn error_excerpt … --include=*.test.ts returns nothing. The comment says the truncation exists to "avoid shipping full tool payloads" — a privacy/size property with no assertion behind it, so it can be lost unnoticed. Low severity; naming it because the mutation matrix would otherwise report a clean sweep.

5.3 The new digest line is charged against the model's preview budget (Suggestion — measured regression, bounded)

fitText now builds header = "${artifactHeader}\n${FULL_OUTPUT_DIGEST_LABEL}${digest}", and that header is deducted from the same per-slot budget as the preview. The digest is 85 characters (\n + Full output sha256: + 64 hex). Measured with the real compiled finalizer on both arms, identical inputs, budget 20 000:

parts in batch	per-slot	HEAD surviving body chars	BASE surviving body chars	Δ
16	1250	1136	1221	−85
40	500	386	471	−85
100	200	86	171	−85
140	142	29	114	−85
200	100	0 (all 200 slots)	71	−71

Two consequences:

1. Universal: every batch-budget-truncated result — including successful ones, which the guard never reads — now shows the model 85 fewer characters of real content and gains a Full output sha256: line the description does not mention as model-facing.
2. Threshold shift: the slot size below which the model receives no content at all moves from ≤29 chars (base header is 22) to ≤114 chars (head header is 107). At per-slot 100 head delivered 0 body characters in all 200 slots while base still delivered 71.

What does NOT hold — bounding it honestly: at the shipped DEFAULT_TOOL_OUTPUT_BATCH_BUDGET = 200_000 a normal result is not truncated at all (a 4 028-char result passes through byte-identical, no digest added), and reaching per-slot ≤114 at that default would need ~1 755 parts in a single batch. So this is not reachable in the default configuration; it bites when an operator lowers toolOutputBatchBudget (config.ts:1174, a real setting) or when a batch is very large. The fingerprint properties themselves are unaffected (§3).

Reproduce:

node tmp/pr10916-verify-20260904-125905/harness/probe-finalizer-bisect.mjs /__w/qwen-code/qwen-code/packages/core/dist
node tmp/pr10916-verify-20260904-125905/harness/probe-finalizer-bisect.mjs /__w/qwen-code/qwen-code/tmp/base-tree/packages/core/dist

A minimal fix would keep the digest out of the preview budget — emit it only when maxChars leaves room, or account for it before allocateTextBudget splits the budget. Not applied or measured, so treat it as a direction rather than a validated patch; the fixture that would pin it is a fitText case at a slot size between 29 and 114 asserting non-empty content.

5.4 A round carrying two distinct error signatures resets itself every round (Suggestion — blind spot in the counting rule)

checkRepeatedToolError keeps a single toolErrorStreakSignature slot and iterates the round's distinct signatures in first-occurrence order, so the last distinct signature of a round overwrites the first. A dead-end loop whose parallel batch fails with ≥2 distinct errors therefore never accumulates:

round 1: [A, B] -> A sets streak(A)=1, B sets streak(B)=1
round 2: [A, B] -> A != B so streak resets to A=1, then B resets to B=1
... 8 rounds: never reaches 3

Measured: S8 ran 8 such rounds against the real detector — no halt. Base also does not halt, so this is not a regression; it is a limit on how much of #10887 the guard actually covers. It matters because the issue describes 153 calls with 83% erroring under varied arguments, and varied arguments do not always yield a byte-identical payload (S2 shows the tool name itself lands in the bash output line, so varying it varies the signature legitimately).

This is consistent with the description's own framing ("the same error returned on every call"), so it may be intended scope rather than a defect — but it is not stated as an accepted limitation, and per the accepted-tradeoff convention an unnamed boundary is worth naming. A per-signature streak map (instead of one slot) would close it. Not applied.

5.5 client.ts and agent-core.ts disagree about id-less parts (Nice to have — latent)

client.ts skips any part without functionResponse.id before collecting it into toolResultParts, so such a part never reaches the batch guard; agent-core.ts flat-maps every responseParts with no id filter. Same guard, two runtimes, different answers: S11 halts at r3 through the agent wiring and never halts through the client wiring on identical input.

Bounded: every error-producing path I traced sets id: request.callId — coreToolScheduler.ts createErrorResponse/createCancelledResponse, turn.ts:250, llm-chat.ts:1757. The one id-less producer found, mcp-tool.ts:1215, builds a result payload (and its error field is an object, not the string the guard reads) rather than a scheduler error response. So no live producer of id-less error parts was demonstrated; this is a latent divergence, not a reproducible defect. Worth a one-line comment at the client.ts filter noting the guard inherits it, since the filter's original purpose was the request-pairing guard.

5.6 No injection attempt observed

PR text was treated as untrusted input throughout. The description contains no instruction directed at the verifier (no "skip the A/B", no "report merge-ready", no "known-flaky suite"), and its falsifiable claims were tested rather than believed — including the two that did not survive contact (§4).

6. Not covered

• Per-commit attribution. 9 commits in the snapshot, 1 reachable at depth 2 (§4). Only the aggregate diff was verified.
• A real model-driven turn. The A/B drives real producers and the real detector through transcribed runtime wiring, but no live LLM session. This reproduces the wire shape of [core] No early termination on repeated tool errors: sessions burn 5-14M tokens in dead-end loops #10887 (real failing shell calls, real error payloads, real detector decision), not the model-side behaviour that produces a dead-end loop.
• client.ts / agent-core.ts end-to-end through their own loops. Their wiring is covered by the PR's own client.test.ts additions (M12 kills 1) and by my transcription in S11 — not by booting a full turn.
• The shell timeout branch (timeoutSummary, EXECUTION_TIMEOUT) and the result.error spawn-failure branch: neither was reached with a real trigger. Both bypass the new producer digest, so their fingerprint stability is unmeasured. The description names repeated identical timeouts as an accepted tradeoff; that acceptance is untested here.
• Real MCP server. normalizeMcpToolError was verified by mutation (M07 killed) against the documented buildMcpToolError shape, not against a live MCP server's payloads.
• Interactive TUI rendering of the new loop label, and the headless stderr path.
• Repo-wide gates. Only the affected files/workspaces were run; no full npm run test, no npm run preflight, no integration suite.
• Base build's single type error. src/services/shellExecutionService.ts(13,27) TS7016 for @lydell/node-pty appeared when compiling the base worktree. This is environmental, not a PR effect: that file is absent from git diff --name-only HEAD^1..HEAD (count 0) and the dependency tree is identical on both arms, so the same error holds on head. tsc --build still emitted all 1 335 JS files, and the base dist was verified guard-free before use. Head tsc --noEmit for both packages exits 0 (§7).
• repeated-tool-failure-guard.ts interaction. The description argues it is not a duplicate (ACP-only, shadow default, classified keys). Not independently verified.

7. Gates and methodology

gate	result
packages/core affected suites (6 files: loopDetectionService, client, shell, tool-response-finalizer ×2, telemetry/loggers)	997 passed, 0 failed (unmutated baseline M00)
packages/cli src/nonInteractiveCli.test.ts	136 passed, 1 skipped
tsc --noEmit core / cli	both exit 0, empty logs (re-run with the tool's own exit status, not a piped tail's)
eslint on all 9 changed files	exit 0, empty log
eslint live-gate proof	a planted genuine violation (const genuinelyUnusedLocal = 1; in a scratch packages/core/src/services/zz-lint-probe.ts) was reported — @typescript-eslint/no-unused-vars, exit 1 — then the file was removed and the tree confirmed clean. An exit-0 eslint that matched nothing would look identical to a pass, so the green above is a measurement, not an assumption.
Mutation matrix	13/16 killed; baseline green; positive control M01 killed 14

Mutation matrix (each mutant a single-point source edit, 997 tests per core run; survivors classified per the convention — coverage gap vs dead code vs redundant defence):

mutant	result	classification
M00 unmutated baseline	0 red	gate is live
M01 recordToolErrorBatch → false	14 red	positive control — proves the harness can fail this suite
M02 threshold 3 → 2	12 red	pinned
M03 sibling-collapse removed	1 red	pinned
M04 synthetic exclusion (both clauses)	1 red	pinned
M05 cancellation clause alone	1 red	pinned (not layered-blind)
M06 consumer volatile-line filter off	2 red	pinned
M07 MCP normaliser off	1 red	pinned
M08 producer-digest preference off	2 red	pinned
M09 shell.ts digest emission off	1 red	pinned
M10 M06 + M09 together	3 red	combination row: 3 > max(2,1), so both layers are independently load-bearing — neither is redundant defence
M11 finalizer inner-digest reuse off	1 red	pinned
M12 client.ts batch wiring removed	1 red	pinned
M13 agent-core.ts wiring removed	0 red	coverage gap (§5.1) — behaviour is correct, nothing asserts it
M14 telemetry .slice(0,200) removed	0 red	coverage gap (§5.2)
M15 CLI always-on hint membership removed	1 red	pinned
M16 stripPersistenceEnvelope newline reverted	1 red	pinned

The combination row M10 is included because M06 and M09 defend the same hazard (per-call volatile text in shell failures) from two directions; had either single revert survived alone, the pair would still have been load-bearing and the survivor would have been misreported as a coverage gap. Both were killed individually and the combination killed more, so the classification is unambiguous.

Methodology. CI verify job: node:22-bookworm container, Node v22.23.2, working tree = refs/pull/10916/merge at depth 2, npm ci + npm run build already completed at HEAD. The base control was built with git worktree add tmp/base-tree HEAD^1 and npm run build -w packages/core inside it; the worktree needed packages/core/node_modules linked from the head tree because that directory is not hoisted to the repo root, which is safe here only because the PR changes no manifest or lockfile (verified) and packages/core has no @qwen-code/* workspace dependency (verified). That scratch worktree was removed once the A/B cells were captured (git worktree remove --force tmp/base-tree, then git worktree prune; tracked tree confirmed clean), so the reproduce commands above that cite tmp/base-tree/packages/core/dist need it rebuilt first:

git worktree add tmp/base-tree 80497a74d0e807f4640b60f7fe482bb97202408a
ln -sfn "$PWD/packages/core/node_modules" tmp/base-tree/packages/core/node_modules
(cd tmp/base-tree/packages/core && npm run build)   # emits despite one pre-existing type error, see §6

Harnesses live in harness/ as .mjs/.py and import the compiled dist/ of each arm by absolute path — ab-real-producer.mjs (A/B), probe-finalizer.mjs and probe-finalizer-bisect.mjs (finalizer), adjudicate.mjs and consolidate.mjs (ledgers), mutation-matrix.py (mutants). Raw per-arm output is in logs/ (head-ab.json, base-ab.json, head-finalizer.json, base-finalizer.json, head-bisect.json, base-bisect.json, head-budget.json, base-budget.json, mutation-matrix.json, mutation-matrix.log, gates*.log, eslint.log, eslint-probe.log, tsc-core.log, tsc-cli.log, adjudication.json, ledger.json). Every number in this report and in assertions.json comes from a scripted comparison that ran; expectations are encoded in the ledger scripts so an arm failing exactly as predicted counts as a pass and fail counts only unexpected outcomes. The working tree was restored and confirmed clean (git status --porcelain --untracked-files=no → empty) after the mutation matrix and again after the lint probe.

Flakiness gate log

rounds=5 files=4 skipped=0
file packages/cli/src/nonInteractiveCli.test.ts: (cd packages/cli) npx --no-install vitest run ./src/nonInteractiveCli.test.ts
file packages/core/src/core/client.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/client.test.ts
file packages/core/src/services/loopDetectionService.test.ts: (cd packages/core) npx --no-install vitest run ./src/services/loopDetectionService.test.ts
file packages/core/src/tools/shell.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/shell.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/cli/src/nonInteractiveCli.test.ts: PPPPP
  packages/core/src/core/client.test.ts: PPPPP
  packages/core/src/services/loopDetectionService.test.ts: PPPPP
  packages/core/src/tools/shell.test.ts: PPPPP

verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
round 1 · packages/core/src/core/client.test.ts: P (exit 0)
round 1 · packages/core/src/services/loopDetectionService.test.ts: P (exit 0)
round 1 · packages/core/src/tools/shell.test.ts: P (exit 0)
round 2 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
round 2 · packages/core/src/core/client.test.ts: P (exit 0)
round 2 · packages/core/src/services/loopDetectionService.test.ts: P (exit 0)
round 2 · packages/core/src/tools/shell.test.ts: P (exit 0)
round 3 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
round 3 · packages/core/src/core/client.test.ts: P (exit 0)
round 3 · packages/core/src/services/loopDetectionService.test.ts: P (exit 0)
round 3 · packages/core/src/tools/shell.test.ts: P (exit 0)
round 4 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
round 4 · packages/core/src/core/client.test.ts: P (exit 0)
round 4 · packages/core/src/services/loopDetectionService.test.ts: P (exit 0)
round 4 · packages/core/src/tools/shell.test.ts: P (exit 0)
round 5 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
round 5 · packages/core/src/core/client.test.ts: P (exit 0)
round 5 · packages/core/src/services/loopDetectionService.test.ts: P (exit 0)
round 5 · packages/core/src/tools/shell.test.ts: P (exit 0)

Evidence images

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

— Qwen Code · sandboxed verification

[qwen-code-ci-bot]

@yiliang114 Requesting changes on one new Critical — everything from the prior three rounds is verifiably fixed at this head, and I'm not re-raising any of it.

The blocker: error_excerpt uploads raw tool-error text to a default-on third-party endpoint, unredacted, and for the shell shape it leads with the command line.

The chain, all at 8a8240c4:

• checkRepeatedToolError passes errorExcerpt: raw — the un-normalized functionResponse.response.error; only normalized gets hashed.
• LoopDetectedEvent does .slice(0, 200) — truncation only, no redaction, no control-character stripping.
• For a shell failure, response.error is llmContent (shell.ts, third executionError branch), whose leading lines are Command: <cmd>, Directory: <cwd>, Output: …. So the uploaded excerpt is dominated by the command line and working directory, not by the failure evidence.
• logLoopDetected (telemetry/loggers.ts:677-690) sends it to QwenLogger → properties.error_excerpt → gb4w8c3ygj-default-sea.rum.aliyuncs.com, and spreads ...event into OTel attributes as well.
• QwenLogger.getInstance gates only on getUsageStatisticsEnabled(), which Config defaults to true (config/config.ts:2623). On by default.

Why this is blocking rather than a nit: the guard fires precisely when a command fails three rounds running, and agent shell commands routinely carry credentials inline — git clone https://x-access-token:ghs_…@github.com/…, curl -H "Authorization: Bearer …", npm publish with a registry token, a postgres://user:[REDACTED]@host/db DSN. Those sit in the first ~40 characters of the excerpt. redactUrlCredentials covers the URL-userinfo subset but not header/env/DSN forms. Collected data cannot be un-collected.

This also reverses a decision documented in the same file: lastChantExcerpt deliberately stays off the event and rides the debug log, and the diff edits that comment to carve out an exception instead of following it.

Smallest fix: drop error_excerpt, keep error_signature. The sha256 already satisfies the stated need ("pages arrive with the identity of the failing payload"). If the excerpt stays: route it through the exported truncateSpanError (telemetry/session-tracing.ts:472-474), take it from normalized rather than raw (already stripped of Command:/Directory:/PGID for shell blocks), and put it in snapshots — where every other free-form payload in qwen-logger.ts lives (execution_summary, truncated_sequence), including the kitty field the code comment cites as its precedent.

Two smaller items worth folding into the same pass, neither blocking on its own:

• extractAnchoredFullDigest (tool-response-finalizer.ts) is a verbatim copy of extractAnchoredStubDigest (loopDetectionService.ts:215-233) — identical bodies, only the parameter name differs. They are the two ends of one parsing contract, so they must not drift. Move one into tools/truncation.ts, beside FULL_OUTPUT_DIGEST_LABEL and the buildStub that mints the line.
• The PR description still documents the pre-round-based design (recordToolResult fingerprinting "every failed tool result", 150 tests). The shipped guard is fed once per round via recordToolErrorBatch and recordToolResult's own docstring now says it is not fed there. Per-result vs per-round is exactly the axis the first Critical turned on, so please refresh the description.

Full detail, the file map, the flow diagram, and the CI evidence (including what I could not verify, and why) are in the Stage 2 comment; my reasoning on the verdict is in Stage 3.

Not approving, and no deferred-approval marker is attached — CI landing green will not convert this into an approval.

中文说明

@yiliang114 以一条新的 Critical 请求修改——前三轮的所有问题在此 head 上都已可验证地修复，我不再重提任何一条。

阻塞项：error_excerpt 会把原始工具错误文本上传到「默认开启」的第三方端点，未脱敏；而对 shell 形态来说，它开头就是命令行。

完整链路（均基于 8a8240c4）：

• checkRepeatedToolError 传的是 errorExcerpt: raw——未归一化的 functionResponse.response.error；只有 normalized 参与哈希。
• LoopDetectedEvent 只做 .slice(0, 200)——仅截断，无脱敏、无控制字符清理。
• shell 失败时 response.error 就是 llmContent（shell.ts 第三个 executionError 分支），其开头几行是 Command: <命令>、Directory: <工作目录>、Output: …。因此上传的摘要主体是命令行与工作目录，而非失败证据。
• logLoopDetected（telemetry/loggers.ts:677-690）把它送往 QwenLogger → properties.error_excerpt → gb4w8c3ygj-default-sea.rum.aliyuncs.com，同时通过 ...event 展开进 OTel 属性。
• QwenLogger.getInstance 只以 getUsageStatisticsEnabled() 为门槛，而 Config 对其默认值为 true（config/config.ts:2623）。默认开启。

为什么这是阻塞项而非小问题：该守卫恰好在命令连续三轮失败时触发，而 agent 的 shell 命令经常内联携带凭据——git clone https://x-access-token:ghs_…@github.com/…、curl -H "Authorization: Bearer …"、带 registry token 的 npm publish、postgres://user:[REDACTED]@host/db 这类 DSN。它们正好落在摘要的前 ~40 个字符内。redactUrlCredentials 能覆盖 URL userinfo 子集，但覆盖不了 header、环境变量、DSN 形态。已收集的数据无法收回。

这也反转了同一文件中已有明文记录的决定：lastChantExcerpt 刻意不进事件、改走 debug 日志，而本 diff 是修改该注释来开例外，而非遵循它。

最小修复： 去掉 error_excerpt，保留 error_signature。sha256 已满足其声明诉求（「page 到达时即带有失败载荷的身份」）。若要保留摘要： 请走已导出的 truncateSpanError（telemetry/session-tracing.ts:472-474），取自 normalized 而非 raw（对 shell 块已剥掉 Command:/Directory:/PGID），并放进 snapshots——qwen-logger.ts 里其他所有自由格式载荷都在那里（execution_summary、truncated_sequence），包括代码注释引为先例的那个 kitty 字段。

另有两处较小的问题，建议在同一轮一并处理，单独看都不构成阻塞：

• extractAnchoredFullDigest（tool-response-finalizer.ts）是 extractAnchoredStubDigest（loopDetectionService.ts:215-233）的逐字拷贝——函数体完全相同，只有形参名不同。它们是同一解析契约的两端，不能各自漂移。请把其中一份移入 tools/truncation.ts，与 FULL_OUTPUT_DIGEST_LABEL 及铸造该行的 buildStub 放在一起。
• PR 描述仍在记录「按轮次」改造之前的设计（recordToolResult 对「每个失败工具结果」做指纹、150 个测试）。实际守卫是通过 recordToolErrorBatch 每轮喂入一次，而 recordToolResult 自己的 docstring 现在明确写着不在那里喂入。「按结果」与「按轮次」正是第一条 Critical 的关键分界，请更新描述。

完整细节、文件地图、流程图，以及 CI 证据（包含我无法验证的部分及原因）见 Stage 2 评论；关于结论的推理见 Stage 3。

不予批准，且未附带任何延迟批准标记——CI 转绿不会把它转换成一次批准。

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 8a8240c4cfb1fc9c689219c398b1c2c3a41ee7b4 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

• R3 F7 duplicated digest-scanner consolidation (extractAnchoredFullDigest / extractAnchoredStubDigest into a shared helper in truncation.ts) — already acknowledged by the author as a follow-up (comment 3931044787)

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Test Plan (not a blocker): src/services/loopDetectionService.test.ts — no such file or directory; Tests 150 passed — this review observed 28114, 23171, 1919, 298, 1772, 504, 5822, 94 passed; 3 passed — this review observed 28114, 23171, 1919, 298, 1772, 504, 5822, 94 passed.

Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:

• packages/core/src/core/client.ts:3893 — [probe] D3-1 the always-on guarantee has no behavioural test with skipLoopDetection=true (deferred by the code-age rule: the feed site is unchanged since the round-2 head)
• packages/core/src/services/loopDetectionService.ts:796 — [probe] D3-2 multi-signature streak semantics are unpinned by any test (deferred by the code-age rule; the under-halt was raised as non-blocking by @qqqys in review 5108105671)

Convergence: round 3 posted 9 inline comment(s), 9 of them reported for the first time; the previous round posted 4 (4 new). Findings keep coming back to the same files: packages/core/src/services/loopDetectionService.ts (findings in rounds 1, 2; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

仅完成部分审查，审查缺口已披露。

本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。

未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Test Plan（非阻断）：src/services/loopDetectionService.test.ts — no such file or directory; Tests 150 passed — this review observed 28114, 23171, 1919, 298, 1772, 504, 5822, 94 passed; 3 passed — this review observed 28114, 23171, 1919, 298, 1772, 504, 5822, 94 passed。

收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。

收敛情况：第 3 轮发布了 9 条行内评论，其中 9 条是首次提出；上一轮发布了 4 条（其中 4 条首次提出）。发现反复回到同一批文件：packages/core/src/services/loopDetectionService.ts（第 1、2 轮已出过发现，本轮又有 2 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）

— qwen3.8-max via Qwen Code /review (v0.23.0)

[qwen-code-ci-bot]

[Suggestion] R3-2: The AgentCore (headless/sub-agent) half of this guard's runtime wiring has no test — every wiring test targets the LlmClient path in client.test.ts. This is the still-open sibling of R2-2 ("neither runtime wiring site of this guard is exercised by any test"), whose client.ts half was fixed in 8a8240c; the agent-core half remains.

Deleting this block — or a refactor changing toolCallResult.results so error parts no longer flow — ships fully green: nothing under packages/core/src/agents/** references recordToolErrorBatch or REPEATED_TOOL_ERROR. The #10887 dead-end burn (repeated failing calls with varied args) would then continue unbounded inside Task-tool sub-agents — the runtime most prone to unsupervised token spend — and CI would never notice.

Witness:

mutant run (recordToolErrorBatch block deleted from agent-core.ts):
agent-headless.test.ts: Tests 68 passed (68) — ships green
grep packages/core/src/agents/** for recordToolErrorBatch|REPEATED_TOOL_ERROR -> exactly one hit: agent-core.ts:1253 itself

Fix direction: add a test in agent-headless.test.ts modelled on the existing 'should stop consecutive identical tool calls with fresh ids' test (~line 2172): a tool whose invocation returns a functionResponse carrying { error: 'fatal: not a git repository' } for 3 rounds with varied args per round; expect AgentTerminateMode.LOOP_DETECTED with loopType repeated_tool_error. The mocked tool result must place the failure at functionResponse.response['error'] (extractToolErrors skips non-string payloads), not in llmContent. The new test is its own witness — it goes red (terminates MAX_TURNS instead of LOOP_DETECTED) if this wiring is removed; please confirm by deleting the block and watching it fail.

中文说明

守卫运行时接线的 AgentCore（无头/子 agent）一半没有任何测试——所有接线测试都针对 client.test.ts 中的 LlmClient 路径。这是 R2-2（「守卫的两个运行时接线站点均无测试」）仍未关闭的一半：client.ts 一半已在 8a8240c 修复，agent-core 一半仍缺。

删除这个块——或某个使错误 part 不再流入的 toolCallResult.results 重构——都能全绿通过：packages/core/src/agents/** 下没有任何代码引用 recordToolErrorBatch 或 REPEATED_TOOL_ERROR。#10887 的死循环烧 token（变体参数的重复失败调用）将在 Task 工具子 agent 内部无界继续——那是最容易无监督消耗 token 的运行时——而 CI 永远不会察觉。

见证（见英文段）：删除 agent-core.ts 中 recordToolErrorBatch 块的变异体下，agent-headless.test.ts 68 个用例全部通过。

修复方向：在 agent-headless.test.ts 中仿照现有 'should stop consecutive identical tool calls with fresh ids'（约 2172 行）新增用例：某工具的调用返回携带 { error: 'fatal: not a git repository' } 的 functionResponse，连续 3 轮、每轮参数各异；断言以 loopType repeated_tool_error 终止于 AgentTerminateMode.LOOP_DETECTED。mock 的工具结果必须把失败放在 functionResponse.response['error']（extractToolErrors 跳过非字符串载荷），而不是 llmContent。新测试自身即见证——移除该接线后应变红（以 MAX_TURNS 而非 LOOP_DETECTED 终止）；请删除该块确认其失败。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Verified at head d136ac9: grep -rn 'recordToolErrorBatch\|REPEATED_TOOL_ERROR' packages/core/src/agents/ returns exactly one hit — agent-core.ts:1253 itself — so the AgentCore/headless wiring is indeed unexercised, while the client.ts half got its wiring test (client.test.ts:8445ff, from 8a8240c). The modelled test exists at agent-headless.test.ts:2172.

Deferred, not declined: this closeout round has a scope fuse (PR additions 1559 > 1500 → no code changes, replies only), so the suggested wiring test cannot land in this pass. Leaving the thread unresolved to keep the signal visible; the proposed shape (3 rounds × varied args, error at functionResponse.response['error'], expect LOOP_DETECTED/repeated_tool_error) is the right one for the follow-up.

[yiliang114]

Confirmed. grep -rn "recordToolErrorBatch\|REPEATED_TOOL_ERROR" packages/core/src/agents/ returns exactly one hit — agent-core.ts:1253, the production line itself. Test files referencing recordToolErrorBatch are core/client.test.ts and services/loopDetectionService.test.ts only; nothing under agents/**. So deleting the block at agent-core.ts:1244-1256 ships green, and the headless/sub-agent runtime — the one most exposed to unsupervised spend — keeps the #10887 dead end unbounded.

Test to add (not added this round): packages/core/src/agents/agent-headless.test.ts, sibling of 'should stop consecutive identical tool calls with fresh ids' (~:2172). Tool whose invocation returns a functionResponse with response: { error: 'fatal: not a git repository' } for 3 rounds, args varied per round; assert AgentTerminateMode.LOOP_DETECTED with loopType repeated_tool_error. The failure must sit at functionResponse.response.error — extractToolErrors skips non-string payloads (loopDetectionService.ts:780-781).

PR is at +1562, over this pass's 1500-line cap, so no code changes this round. Leaving unresolved.

[qwen-code-ci-bot]

[Suggestion] R3-3: The MCP error-message contract is mirrored as private string literals in this consumer — and re-mirrored in the test — instead of being producer-owned: buildMcpToolError (mcp-tool.ts:864-876) composes the message from inline template fragments, with no shared constant and no contract test. This diff deliberately applies the opposite pattern to cancellations — CANCELLED_TOOL_ERROR_PREFIX is exported from coreToolScheduler.ts with a comment naming the "producer-owns-the-shape pattern" — which makes the MCP omission concrete rather than stylistic.

When the producer wording changes (e.g. reported tool error for function call: → failed while executing call:), normalizeMcpToolError returns null, the payload falls through to hashing the full message — which embeds the function-call JSON whose args the dead-end loop varies per retry — so every retry fingerprints uniquely, the streak never accumulates, and the exact MCP dead-end regression this PR fixes returns, with loopDetectionService.test.ts and mcp-tool.test.ts both green.

Witness:

mutant run (producer wording changed to ' failed while executing call: ' in mcp-tool.ts only):
drifted-wording fired per round: [false,false,false,false,false,false] <- identical server payload, guard dead
loopDetectionService.test.ts -t "fires on repeated MCP errors...": 1 passed | 160 skipped <- drift invisible

Fix direction: export the message fragments (or a small message-builder) from mcp-tool.ts and import them here and in the test — or add a contract test driving recordToolErrorBatch with a message built by the real buildMcpToolError. Note mcp-tool.test.ts:351 hardcodes the same wording — a third copy the fix must consolidate or keep in sync. Please add the contract test and confirm it goes red when the call-marker text is renamed in mcp-tool.ts only.

中文说明

MCP 错误消息契约在该消费者处被镜像为私有字符串字面量——测试中又再镜像一次——而非由生产者持有：buildMcpToolError（mcp-tool.ts:864-876）以内联模板片段拼出消息，没有共享常量，也没有契约测试。本 diff 对取消场景刻意采用了相反模式——从 coreToolScheduler.ts 导出 CANCELLED_TOOL_ERROR_PREFIX 并在注释中点名「生产者持有形态模式」——这使 MCP 处的缺失是具体的而非风格问题。

当生产者措辞变化（如 reported tool error for function call: → failed while executing call:）时，normalizeMcpToolError 返回 null，载荷落入对完整消息做哈希——其中内嵌函数调用 JSON，其参数正是死循环每次重试变化的内容——于是每次重试指纹都唯一，连击永不累积，本 PR 所修复的 MCP 死循环回归会原样复现，而 loopDetectionService.test.ts 与 mcp-tool.test.ts 双双保持绿色。

见证（见英文段）：仅在 mcp-tool.ts 变更生产者措辞的变异体下，守卫 6 轮均不触发，MCP 用例 1 passed | 160 skipped，漂移不可见。

修复方向：从 mcp-tool.ts 导出这些消息片段（或一个小的消息构造器），在此处与测试中导入——或新增契约测试，用真实 buildMcpToolError 构造的消息驱动 recordToolErrorBatch。注意 mcp-tool.test.ts:351 硬编码了同样的措辞——第三份副本，修复时须合并或保持同步。请新增契约测试，并确认仅重命名 mcp-tool.ts 中的调用标记文本时该测试变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Verified at head d136ac9: the wording still lives in four places — the producer template fragments (mcp-tool.ts:866, 872), the consumer private literals (loopDetectionService.ts:71-73), mcp-tool.test.ts:351, and the inline reconstruction at loopDetectionService.test.ts:3971. The drift failure mode is real in the code: a marker wording change makes normalizeMcpToolError return null, and the fallback then hashes the full message including the volatile function-call JSON args, so the streak never accumulates.

Deferred, not declined: the fix spans a producer export + consumer + ≥2 test files and needs a contract decision (shared constant/builder vs. a contract test). Combined with this round's scope fuse (PR additions 1559 > 1500 → no code changes), this needs a maintainer decision rather than an automated pass. Leaving unresolved.

[yiliang114]

Confirmed, and the asymmetry is real rather than stylistic: loopDetectionService.ts:71-73 holds three private literals mirroring wording that buildMcpToolError composes inline at mcp-tool.ts:864-874 with no shared constant, while this same diff does apply the producer-owns-the-shape pattern to cancellations — CANCELLED_TOOL_ERROR_PREFIX exported at coreToolScheduler.ts:1047 and imported at loopDetectionService.ts:23.

Cheapest fix that keeps this PR minimal: one contract test rather than exporting the fragments. In loopDetectionService.test.ts, build the error with the real buildMcpToolError (import it) and drive recordToolErrorBatch across 3 rounds with varied args, asserting the guard fires. That test goes red on producer wording drift, which is the failure mode described here — normalizeMcpToolError returning null, falling through to hashing the full message, streak never accumulating. Exporting the fragments (and consolidating the third copy at mcp-tool.test.ts:351) is a reasonable follow-up but isn't needed to close the gap.

Not adding this round (+1562, over this pass's 1500-line cap). Leaving unresolved.

[qwen-code-ci-bot]

[Suggestion] R3-4: The telemetry evidence chain added this round is asserted by zero tests across its whole length: the LoopDetectedEvent details constructor argument, the error_signature/error_excerpt fields and the 200-char truncation (telemetry/types.ts:516-528), the logLoopDetected call carrying them (loopDetectionService.ts:808-811), and this qwen-logger passthrough. The new firing tests in loopDetectionService.test.ts assert only the boolean return and getLastLoopType().

Swapping errorSignature/errorExcerpt at the fire site, dropping the details argument, renaming one of these spread keys, or deleting the .slice(0, 200) truncation all compile cleanly and keep every test green. Telemetry pages would then silently receive a 64-char hex where oncall expects the failure text — or nothing at all — and deleting the slice would silently ship full tool-error payloads (which can embed large or sensitive command output) into telemetry: the exact blind spot these fields were added to fix.

Witness:

mutant run (swap fields at loopDetectionService.ts:809-810 + delete .slice(0,200) at types.ts:527 + rename spread key here):
loopDetectionService.test.ts + qwen-logger.test.ts: Tests 204 passed (204)
grep error_signature|errorSignature|errorExcerpt|error_excerpt over packages/**/*.test.ts -> zero matches

Fix direction: in loopDetectionService.test.ts (loggers already mocked), assert that when the streak trips, logLoopDetected receives a LoopDetectedEvent with error_signature equal to the sha256 of the normalized error text and error_excerpt truncated to 200 chars (feed an error longer than 200 chars once — the constructor truncates at types.ts:527, so account for the slice); in qwen-logger.test.ts, add a case asserting loop_detected properties include error_signature/error_excerpt when present and omit them for other loop types. The new assertions are their own witness — removing the details argument or the 200-char slice turns the first red, renaming a spread key turns the second; please confirm both mutations.

中文说明

本轮新增的遥测证据链在全链路上没有任何测试断言：LoopDetectedEvent 的 details 构造参数、error_signature/error_excerpt 字段及 200 字符截断（telemetry/types.ts:516-528）、携带它们的 logLoopDetected 调用（loopDetectionService.ts:808-811），以及此处 qwen-logger 的透传。loopDetectionService.test.ts 中新的触发用例只断言布尔返回值和 getLastLoopType()。

在触发点交换 errorSignature/errorExcerpt、删除 details 实参、重命名这些展开键之一、或删除 .slice(0, 200) 截断，都能通过编译且所有测试保持绿色。遥测页面将悄悄收到 oncall 期望看到失败文本处的 64 位十六进制串——或什么都没有——而删除截断会把完整工具错误载荷（可能包含大量或敏感命令输出）悄悄送入遥测：正是这些字段要修复的盲区本身。

见证（见英文段）：三合一变异体（交换字段+删除截断+重命名展开键）下，204 个测试全部通过；全部测试文件中对这几个字段名的 grep 零匹配。

修复方向：在 loopDetectionService.test.ts（loggers 已被 mock）中断言连击触发时 logLoopDetected 收到的 LoopDetectedEvent 的 error_signature 等于归一化错误文本的 sha256、error_excerpt 被截断为 200 字符（喂一次超过 200 字符的错误——构造器在 types.ts:527 截断，断言须计入该截断）；在 qwen-logger.test.ts 中新增用例，断言 loop_detected 属性在字段存在时包含 error_signature/error_excerpt、其他循环类型时不包含。新断言自身即见证——删除 details 实参或 200 字符截断使第一个变红、重命名展开键使第二个变红；请对两种变异分别确认。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Partially covered at head d136ac9 — the "zero tests across the whole length" premise is stale for the constructor end: d136ac9 added loggers.test.ts:564-589 pinning the LoopDetectedEvent 200-char error_excerpt truncation and surrogate safety. The residual gap is real: loopDetectionService.test.ts asserts only loop_type on logLoopDetected (249, 331), and qwen-logger.test.ts asserts only terminate_reason (405, 419) — zero grep hits for error_signature/error_excerpt in either suite.

Deferred, not declined: scope fuse (PR additions 1559 > 1500 → no code changes this round). Leaving unresolved; the proposed assertions (fire site: signature == sha256(normalized text) and excerpt truncated; qwen-logger: fields present iff loop_detected) are the right minimal shape for the follow-up.

[yiliang114]

Partly stale at head 6439e09; the remaining half is real.

Stale: grep -rl "error_signature\|errorSignature\|errorExcerpt\|error_excerpt" packages/*/src --include=*.test.ts is not zero — it hits telemetry/loggers.test.ts, where describe('LoopDetectedEvent error_excerpt truncation') (:564-589) pins the 200-char cut and the trailing-surrogate trim (landed with R3-5). So "deleting the .slice(0, 200) keeps every test green" no longer holds.

Still live:

• Fire site: the only logLoopDetected payload assertions in loopDetectionService.test.ts are expect.objectContaining({ loop_type: ... }) at :249 and :331, both for other loop types. Swapping errorSignature/errorExcerpt at loopDetectionService.ts:833-834 ships green.
• Sink passthrough: qwen-logger.test.ts has no loop_detected property assertions (only terminate_reason: 'loop_detected' at :405/:419, a different event). Renaming either spread key at :743-749 ships green.

Tests to add: in loopDetectionService.test.ts, when the streak trips assert logLoopDetected received an event whose error_signature equals the sha256 of the normalized error text and whose error_excerpt equals the first 200 chars of the raw error; in qwen-logger.test.ts, assert the loop_detected RUM event's properties carry both keys.

Leaving unresolved scoped to those two, not the truncation half.

[qwen-code-ci-bot]

[Suggestion] R3-6: No test pins that the digest line survives the shell's real truncateToolOutput keep='both' pass for failures above the 30KB threshold — the new comment in shell.ts relies on "truncateToolOutput's keep='both' tail retention preserves it through truncation", but every truncation test in this file spies truncateToolOutput itself, so the property is asserted only by prose.

A future change flipping keep='both' to 'head', moving the digest line, or slicing tail lines differently would drop the digest from every truncated (>30KB) failure block with no red test; those failures would then fingerprint via the truncation envelope, which embeds a per-call random spill path (shell_<12-hex>.output), silently disabling the error-repetition guard for all large failures. The property holds today — this is a pure pin-the-property gap.

Witness:

probe driving the REAL truncateToolOutput (threshold 30_000, previewChars 4_000, keep='both') on a 37_920-char failure block:
P5 >30KB keep-both: block=37920 truncated=4405 digest-survives=true
every truncation test in shell.test.ts spies truncateToolOutput itself (lines 3855, 3917, 3956, 3992, 4271, 4325, 8617)

Fix direction: add a case here with output exceeding the (mocked-config) threshold that runs the real truncateAndSaveToFile path and asserts the truncated llmContent/error.message still contains the line-anchored Full output sha256: digest. The new test is its own witness — mutation keep: 'both' → keep: 'head' in shell.ts's truncateToolOutput call must turn it red; please confirm.

中文说明

没有测试钉住「摘要行能在超过 30KB 阈值失败的 shell 真实 truncateToolOutput keep='both' 通道中存活」——shell.ts 的新注释依赖「truncateToolOutput 的 keep='both' 尾部保留使其在截断后仍存在」，但本文件所有截断测试都 spy 了 truncateToolOutput 本身，该性质只由文字声明。

未来若把 keep='both' 翻转为 'head'、移动摘要行、或以不同方式切片尾部行，所有截断后（>30KB）的失败块都会丢失摘要且无测试变红；这些失败将改经截断信封做指纹——其中嵌有每次调用随机的溢出路径（shell_<12-hex>.output）——从而为所有大失败悄悄关闭错误重复守卫。该性质目前成立——这是纯粹的「钉住性质」缺口。

见证（见英文段）：驱动真实 truncateToolOutput（阈值 30_000、previewChars 4_000、keep='both'）处理 37_920 字符失败块：截断后摘要存活；文件内所有截断测试均 spy 了 truncateToolOutput。

修复方向：在此新增用例，输出超过（mock 配置的）阈值、走真实 truncateAndSaveToFile 路径，断言截断后的 llmContent/error.message 仍包含行锚定的 Full output sha256: 摘要。新测试自身即见证——把 shell.ts 中 truncateToolOutput 调用的 keep: 'both' 变异为 keep: 'head' 须使其变红；请确认。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Confirmed at head d136ac9: every truncation test in shell.test.ts spies truncateToolOutput (3855, 3917, 3956, 3992, 4271, 4325, 8617), and the digest tests (3745-3796) use FAKE_BLOCK_DIGEST with a mocked hash module — so no test drives the real truncateAndSaveToFile path above the 30KB threshold. Agree this is a pure pin-the-property gap; the property holds at head (your probe: digest-survives=true).

Deferred, not declined: scope fuse (PR additions 1559 > 1500 → no code changes this round). Leaving unresolved; the suggested real-truncation case with the keep:'both' → 'head' mutation check is the right shape for the follow-up.

[yiliang114]

Confirmed: every truncation test in shell.test.ts spies the module (spyOn(truncationModule, 'truncateToolOutput') at :3855, :3917, :3956, :3992, :4271, :4325, :8617), and the file has zero hits for truncateAndSaveToFile or keep: 'both'. The property shell.ts's comment leans on — "truncateToolOutput's keep='both' tail retention preserves it through truncation" — is asserted by prose only.

Agreed this is pin-the-property, not a current defect: the digest does survive today. It's still worth pinning because the failure mode is silent and total — a keep: 'head' flip drops the digest from every >30KB failure block, those then fingerprint via the truncation envelope's per-call spill path, and the error-repetition guard is dead for exactly the largest failures.

Test to add: next to the digest tests (~:3745), drive output above the mocked-config threshold through the real truncateAndSaveToFile path with no spy, and assert the truncated llmContent / error.message still contains the line-anchored Full output sha256: line. Mutant check: keep: 'both' → keep: 'head' must turn it red.

Not adding this round (+1562, over this pass's 1500-line cap). Leaving unresolved.

[qwen-code-ci-bot]

[Suggestion] R3-7: The signal-termination arm of this digest gate has no test witness — removing the isSignalTermination(result.signal) disjunct ships green. The new digest tests below exercise only the exit-code arm (exitCode: 1, signal: null), and the only non-aborted signal: 15 case in this file asserts unrelated properties. Signal-killed failures — OOM killer, SIGTERM on eviction, aborted: false — are the exact case this first disjunct exists for: without the digest they fall back to stripShellBlockVolatiles, which the service's own comment says cannot fully enumerate volatile text (multi-line-command continuation lines leak through), so a retried dead-end loop of multi-line commands killed by signal fingerprints uniquely per call and the streak never accumulates.

Witness:

mutant run (removed isSignalTermination(result.signal) || from this gate):
shell.test.ts: Tests 326 passed (326); the four PR-relevant suites: Tests 1269 passed (1269) — no test reds
candidate fix (new test resolving signal: 15, aborted: false):
  intact code: passes (327/327 in file)
  mutant: fails and is the only failure — 1 failed | 326 passed

Fix direction: add a case that resolves the mocked execution with a non-null signal (e.g. signal: 'SIGTERM', aborted: false, non-zero or null exit code) and asserts result.llmContent contains Full output sha256: ${FAKE_BLOCK_DIGEST} and lastCreateHashInput equals the stable failure core. The gate must stay equivalent to the error-object branch-3 condition (the block builder only runs in the non-aborted else branch), and aborted: true results must remain digest-free. The new test is its own witness — removing the disjunct above must turn it red; please confirm.

中文说明

该摘要门的信号终止分支没有测试见证——移除 isSignalTermination(result.signal) 析取项可全绿通过。下方新的摘要测试只覆盖退出码分支（exitCode: 1、signal: null），而文件中唯一未中止的 signal: 15 用例断言的是无关性质。被信号杀死的失败——OOM killer、被驱逐时的 SIGTERM，aborted: false——正是第一个析取项存在的原因：没有摘要时回落到 stripShellBlockVolatiles，而服务自身注释承认它无法完全枚举 volatile 文本（多行命令的续行会漏过），于是被信号杀死的命令若以多行命令形式重试死循环，每次调用指纹都唯一，连击永不累积。

见证（见英文段）：移除该析取项的变异体下 shell.test.ts 326 全过、四个 PR 相关套件 1269 全过；候选修复（新增 signal: 15、aborted: false 用例）在完整代码下通过、在变异体下是唯一失败。

修复方向：新增用例，以非空 signal（如 signal: 'SIGTERM'、aborted: false、退出码非零或为 null）resolve mock 的执行，断言 result.llmContent 包含 Full output sha256: ${FAKE_BLOCK_DIGEST} 且 lastCreateHashInput 等于稳定失败核心。该门必须保持与错误对象分支 3 的条件等价（块构造器只在未中止的 else 分支运行），且 aborted: true 的结果必须保持无摘要。新测试自身即见证——移除上面的析取项须使其变红；请确认。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Confirmed at head d136ac9: the gate at shell.ts:2859 is isSignalTermination(result.signal) || isShellExitError(...). The digest tests (shell.test.ts:3745ff) exercise only the exit-code arm; the non-aborted signal: 15 cases (3607, 3651) assert error reporting/returnDisplay, the aborted: true case (3673) asserts cancellation, and 4194 asserts only hint omission — so removing the isSignalTermination disjunct would indeed ship green.

Deferred, not declined: scope fuse (PR additions 1559 > 1500 → no code changes this round). Leaving unresolved; the suggested signal: 15/aborted: false digest witness (with the aborted: true digest-free invariant) is the right minimal shape for the follow-up.

[yiliang114]

Confirmed. The only digest tests are shell.test.ts:3745 ('embeds the stable failure-core digest in error blocks for the loop guards' — resolves exitCode: 1, error: null, no signal) and :3780 ('keeps successful shell blocks digest-free'). Nothing drives the isSignalTermination(result.signal) disjunct at shell.ts:2859-2860; the signal: 15 cases in the file (:2714, :3091, :3480, :3607, :3651, :3673, :4215) assert unrelated properties. Removing the disjunct ships green.

This is a real production shape rather than a hypothetical one — OOM-kill and SIGTERM-on-eviction failures with aborted: false reach the digest only through that arm, and without it they fall back to stripShellBlockVolatiles, which this service's own comment (loopDetectionService.ts:696-698) says cannot enumerate multi-line command continuation lines.

Test to add: sibling of :3745 resolving { signal: 'SIGTERM', aborted: false, exitCode: null, error: null }, asserting result.llmContent contains Full output sha256: ${FAKE_BLOCK_DIGEST} and lastCreateHashInput equals the stable failure core. Worth asserting aborted: true stays digest-free in the same test so the gate can't silently widen.

Not adding this round (+1562, over this pass's 1500-line cap). Leaving unresolved.

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

• digest-extractor consolidation (extractAnchoredFullDigest / extractAnchoredStubDigest / the tool-response-finalizer copy) — already reported in the round-3 review body (review 5113311977) and the stage-3 triage comment, acknowledged by the …

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.

Test Plan (not a blocker): src/services/loopDetectionService.test.ts — no such file or directory; Tests 150 passed — this review observed 1919 passed; 3 passed — this review observed 1919 passed.

Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:

• packages/core/src/services/loopDetectionService.ts:437 — [probe] multi-signature rounds overwrite the single-slot streak — a loop returning the same set of >=2 distinct errors every round never accumulates (deferred by the code-age rule: an…
• packages/core/src/services/loopDetectionService.ts:623 — [probe] timeout error payloads embed the per-call timeout duration — escalating or alternating timeouts reset the streak every round (deferred by the code-age rule: anchored on code u…
• packages/core/src/tools/tool-response-finalizer.ts:270 — [probe] fitText sub-header allocation keeps the per-call unique artifact path but drops the digest line — identical errors fingerprint uniquely per call (deferred by the code-age rule…
• packages/core/src/services/loopDetectionService.ts:755 — [probe] MCP errors with image content carry a truncation envelope with a per-call random artifact path in the fingerprinted server payload (deferred by the code-age rule: anchored on …

Convergence: round 4 posted 8 inline comment(s), 3 of them reported for the first time; the previous round posted 9 (9 new). Findings keep coming back to the same files: packages/core/src/services/loopDetectionService.ts (findings in round 3; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

仅完成部分审查，审查缺口已披露。

本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。

未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

未审查：反向审计——在 5 轮的反审轮数上限内未收敛。

Test Plan（非阻断）：src/services/loopDetectionService.test.ts — no such file or directory; Tests 150 passed — this review observed 1919 passed; 3 passed — this review observed 1919 passed。

收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。

收敛情况：第 4 轮发布了 8 条行内评论，其中 3 条是首次提出；上一轮发布了 9 条（其中 9 条首次提出）。发现反复回到同一批文件：packages/core/src/services/loopDetectionService.ts（第 3 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）

— qwen3.8-max via Qwen Code /review (v0.23.0)

[qwen-code-ci-bot]

[Critical] R4-1: The repeated_tool_error telemetry uploads the raw tool-error excerpt to a default-on third-party endpoint, unredacted — and for shell failures the excerpt leads with the command line and working directory. This re-asserts the blocking concern from the stage-3 triage comment, which still stands at this head: checkRepeatedToolError passes errorExcerpt: raw (the un-normalized functionResponse.response.error), LoopDetectedEvent applies only .slice(0, 200) plus the surrogate trim — truncation, no redaction — and logLoopDetected sends it to QwenLogger → properties.error_excerpt and spreads ...event into OTel attributes. The guard fires precisely when a command fails three rounds running, and agent shell commands routinely carry credentials inline — git clone https://x-access-token:ghs_…@github.com/…, curl -H "Authorization: Bearer …", npm publish with a registry token, a postgres://user:…@host/db DSN — and those sit in the first ~40 characters of the uploaded excerpt. usageStatisticsEnabled defaults to true, so this reaches the RUM endpoint without anyone opting in, and collected data cannot be un-collected. The surrogate fix landed this round (R3-5) makes the cut well-formed; it does not redact anything.

Witness:

witness: not run — an unredacted field in an uploaded payload is not executable in a harness; mechanism re-traced at HEAD d136ac99d: loopDetectionService.ts:835 passes errorExcerpt: raw; types.ts:530-535 applies slice(0, 200) + surrogate trim only, no redaction; qwen-logger.ts:744-749 emits error_excerpt into RUM properties (usage statistics default-on).

Smallest fix: drop error_excerpt and keep error_signature — the sha256 already satisfies the stated need (pages arrive with the identity of the failing payload). If the excerpt stays, take it from normalized rather than raw (already stripped of Command:/Directory:/PGID for shell blocks) and route it through the exported truncateSpanError, consistent with the lastChantExcerpt decision documented in this same file — raw excerpt stays off the event and rides the debug log; this diff edited that comment to carve out an exception instead of following it.

Any fix must keep error_signature computable from the repeated payload alone — that field is the whole observability gain this feature added (loopDetectionService.ts:829-836). The acceptance criterion: a unit test asserting that a LoopDetectedEvent built from a shell-shaped error whose Command: line contains a credential-shaped string does not carry that string in error_excerpt (or carries no error_excerpt at all); removing the redaction/removal must make it red.

中文说明

repeated_tool_error 遥测会把原始工具错误摘要未脱敏地上传到「默认开启」的第三方端点——而对 shell 失败来说，摘要开头就是命令行与工作目录。本条重新确认 stage-3 triage 评论中的阻塞项，在该 head 上仍然存在：checkRepeatedToolError 传的是 errorExcerpt: raw（未归一化的 functionResponse.response.error），LoopDetectedEvent 只做 .slice(0, 200) 加代理项修剪——仅截断、无脱敏——logLoopDetected 把它送往 QwenLogger → properties.error_excerpt，并通过 ...event 展开进 OTel 属性。该守卫恰好在命令连续三轮失败时触发，而 agent 的 shell 命令经常内联携带凭据——git clone https://x-access-token:ghs_…@github.com/…、curl -H "Authorization: Bearer …"、带 registry token 的 npm publish、postgres://user:…@host/db 这类 DSN——它们正好落在上传摘要的前 ~40 个字符内。usageStatisticsEnabled 默认为 true，因此数据会在无人选择性加入的情况下到达 RUM 端点，且已收集的数据无法收回。本轮落地的代理项修复（R3-5）只是让截断在编码上合法，并没有脱敏任何内容。

最小修复：去掉 error_excerpt，保留 error_signature——sha256 已满足其声明诉求（page 到达时即带有失败载荷的身份）。若要保留摘要：取自 normalized 而非 raw（对 shell 块已剥掉 Command:/Directory:/PGID），并走已导出的 truncateSpanError，与同一文件中已有明文记录的 lastChantExcerpt 决定保持一致——原始摘要不进事件、改走 debug 日志；本 diff 是修改那段注释来开例外，而不是遵循它。

任何修复都必须保证 error_signature 仍可仅从重复载荷计算得出——该字段是本特性带来的全部可观测性收益（loopDetectionService.ts:829-836）。验收标准：新增单测断言——用 Command: 行含凭据形态字符串的 shell 形态错误构造 LoopDetectedEvent，其 error_excerpt 不含该字符串（或根本不带 error_excerpt）；移除脱敏/移除逻辑后该测试必须变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Verified real at head d136ac9: checkRepeatedToolError passes errorExcerpt: raw — the un-normalized error string (services/loopDetectionService.ts:830-834); LoopDetectedEvent applies only .slice(0, 200) plus a surrogate trim, no redaction (telemetry/types.ts:511-538); logLoopDetected routes to QwenLogger by default and spreads ...event into OTel attributes (telemetry/loggers.ts:677-694). logLoopDetectedEvent then emits error_excerpt into RUM properties for gb4w8c3ygj-default-sea.rum.aliyuncs.com (telemetry/qwen-logger/qwen-logger.ts:78, 736-757), and usageStatisticsEnabled defaults to true (config/config.ts:2623), so the upload is default-on; the only credential redactor in the tree, redactUrlCredentials (telemetry/session-tracing.ts:473), sits on the span path, not this one. A fix is owed but this round is scope-fused (PR additions 1559 > 1500, replies only) and the change crosses a security boundary — leaving unresolved for human decision on the drop-excerpt vs redact approach.

[yiliang114]

Confirmed at head 6439e09 — the mechanism is real, with two corrections to the framing.

Trace:

• loopDetectionService.ts:834 passes errorExcerpt: raw, where raw is the un-normalized functionResponse.response.error (extractToolErrors, :780-786).
• For shell failures that string is the block built at shell.ts:2834-2839, whose first line is Command: ${this.params.command} and second is Directory: ...; shell.ts:3132-3140 makes that same block error.message. So .slice(0, 200) puts the command line at the head of the uploaded excerpt — exactly where inline credentials sit.
• types.ts:532-536 applies only the 200-char cut plus the trailing-surrogate trim. No redaction.
• Sink: qwen-logger.ts:747-748 → properties.error_excerpt → RUM host gb4w8c3ygj-default-sea.rum.aliyuncs.com (qwen-logger.ts:79), gated only by getUsageStatisticsEnabled(), which defaults to true (config.ts:2626, params.usageStatisticsEnabled ?? true; getInstance returns undefined when disabled, qwen-logger.ts:172-180).
• The repo already owns the redactor this bypasses: truncateSpanError(s) = redactUrlCredentials(stripAnsiAndControl(s)), exported at telemetry/session-tracing.ts:472 (redactUrlCredentials in extension/redaction.ts:18). And the in-file precedent at loopDetectionService.ts:450-456 (lastChantExcerpt is "Deliberately NOT part of the LoopDetected event payload") was edited by this diff to add the (REPEATED_TOOL_ERROR is the exception: ...) carve-out.

Corrections:

1. The endpoint is Aliyun RUM — Alibaba Cloud's own service, i.e. first-party infra for this product, not a third party. The concern stands regardless (default-on remote upload of raw unredacted text), but "third-party" overstates it.
2. The same raw block already reaches the same endpoint on main via logToolCallEvent → properties.error_message = event.error (qwen-logger.ts:552, types.ts:225 = call.response.error?.message). So the leak class pre-exists this PR and is wider than error_excerpt. Filed Usage-statistics telemetry uploads raw tool-error text (including shell command lines) to RUM without redaction #11198 for that sink-wide half.

error_excerpt itself is new here (see the types.ts / qwen-logger.ts diffs), so this PR is the cheapest place not to add a second instance. Needs a human call on strategy:

• (a) drop error_excerpt, keep error_signature — the sha256 of the normalized text is the actual observability gain;
• (b) keep the excerpt but pass normalized instead of raw (for shell blocks that reduces to <persisted-stub>sha256:<digest> or the PGID/Command/Directory-stripped text, so no command line);
• (c) route through truncateSpanError.

(a) and (b) are both about one line. (c) alone is insufficient — redactUrlCredentials catches URL-embedded credentials only, not Authorization: Bearer ..., registry tokens passed as flags, or secrets echoed in output.

Not changing code this round: the PR is at +1562, over this patrol pass's 1500-line cap. Leaving unresolved.

[qwen-code-ci-bot]

[Suggestion] This digest reuse trusts any line-anchored Full output sha256: <64hex> line anywhere in the content — including marker lines quoted by peer content — dropping the invariant the old code held, that the stub digest always identifies the full content. This PR mints that marker into every persisted stub, every fitText envelope header, and every shell failure block, so sessions that fold earlier truncated tool output into files (reports, logs, cat of persisted stubs) and later re-read them oversized supply the trigger: two distinct oversized results quoting the same digest line are both stubbed with the quoted digest, and stripPersistenceEnvelope reduces both to the identical <persisted-stub>sha256:<quoted> fingerprint — the #9450 stateful-read guard then counts distinct results as an identical repetition (spurious halt on a turn that was making progress), and the new error-repetition guard accumulates its streak across genuinely different errors (early halt). Note the asymmetry: the consumer side gates its digest trust on shell shape (the Process Group PGID: check in normalizeToolErrorText), but this producer-side reuse has no such gate.

Witness:

probe at HEAD: two distinct >28 KB contents (sha256 f34c9cf7... != 1072082f...) each quoting the same digest line were both stubbed with the quoted digest 8aff5113... — identical fingerprint for different content; four genuinely different oversized errors quoting that line fired the guard [false,false,true,true] (early halt across distinct errors). Fix arm: gating the reuse on 'Process Group PGID:' flipped both probes (stub digests became the per-content full hashes; fired [false,false,false,false]).

Suggested change

	const fullDigest =
	extractAnchoredFullDigest(content) ??
	crypto.createHash('sha256').update(content).digest('hex');
	const fullDigest =
	(content.includes('Process Group PGID:')
	? extractAnchoredFullDigest(content)
	: null) ??
	crypto.createHash('sha256').update(content).digest('hex');

Apply the same gate to the identical reuse in fitText (tool-response-finalizer.ts:271-273). Shell failure blocks unconditionally carry Process Group PGID: ${result.pid ?? '(none)'} (shell.ts:2837, in blockLines), so the gate cannot drop a legitimate shell producer digest. For the fix witness: a new truncation.test.ts case — content carrying a line-anchored Full output sha256: line but no Process Group PGID: marker must keep the full-content digest in the stub; removing the gate must make it red — plus the loopDetectionService.test.ts complement: two distinct oversized errors quoting the same digest must not accumulate one streak.

中文说明

该 digest 复用信任内容中任何位置的行锚定 Full output sha256: <64hex> 行——包括对端内容引用的标记行——丢掉了旧代码持有的不变量：stub digest 必须标识完整内容。本 PR 会把该标记铸进每个持久化 stub、每个 fitText 信包头、每个 shell 失败块，因此把先前截断的工具输出折进文件（报告、日志、cat 持久化 stub）后又超大重新读取的会话就会提供触发条件：两个不同的超大结果引用同一行 digest 时，都会以被引用的 digest 打 stub，stripPersistenceEnvelope 把两者归约为相同的 <persisted-stub>sha256:<quoted> 指纹——#9450 有状态读守卫会把不同结果计为相同重复（在正在推进的轮次上误停），新的错误重复守卫会跨真正不同的错误累积连击（提前误停）。注意不对称性：消费方把 digest 信任限定在 shell 形态（normalizeToolErrorText 的 Process Group PGID: 检查），而这里的生产方复用没有这个门。

修复：把复用限定到 shell 形态载荷（建议块），并对 fitText（tool-response-finalizer.ts:271-273）的相同复用同样处理。shell 失败块无条件携带 Process Group PGID: ${result.pid ?? '(none)'}（shell.ts:2837，位于 blockLines），因此该门不会丢掉合法的 shell 生产方 digest。验收：新增 truncation.test.ts 用例——内容含行锚定的 Full output sha256: 行但不含 Process Group PGID: 标记时，stub 必须保留全文内容 digest；移除该门后测试变红——另在 loopDetectionService.test.ts 补：两个引用同一 digest 的不同超大错误不得累积同一连击。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Verified real at head d136ac9: extractAnchoredFullDigest accepts any line-anchored Full output sha256: occurrence anywhere in content (tools/truncation.ts:438-455), and buildStub reuses it with no shell-shape gate (tools/truncation.ts:573-574) — so oversized content quoting an earlier-minted marker line inherits the quoted digest and two distinct results reduce to one fingerprint; the consumer-side trust IS gated on the Process Group PGID: marker (services/loopDetectionService.ts:723) while this producer and fitText (tools/tool-response-finalizer.ts:271-273) are not. The trigger is reachable because this PR mints the marker into every persisted stub, fitText header, and shell failure block, so sessions that fold those lines into files (reports, logs, cat of persisted stubs) and later re-read them oversized supply it. Deferred under the scope fuse (PR additions 1559 > 1500, no code changes this round); candidate for follow-up.

[yiliang114]

Confirmed, including that this PR is what drops the invariant. On main buildStub hashed the content unconditionally (const fullDigest = crypto.createHash('sha256').update(content).digest('hex')); this diff made it extractAnchoredFullDigest(content) ?? hash(content) (truncation.ts:573-575) and added the same reuse in fitText (tool-response-finalizer.ts:270-272). extractAnchoredFullDigest (truncation.ts:438-456) returns the first line-anchored Full output sha256: <64hex> found anywhere in the content, with no shape gate.

The asymmetry is as described: the consumer gates its digest trust on shell shape (Process Group PGID: at loopDetectionService.ts:723), the producer reuse does not. So oversized content that merely quotes such a line — a report or log that folded in an earlier persisted stub, cat of a stub file, re-read oversized — is stubbed under the quoted digest, and stripPersistenceEnvelope (loopDetectionService.ts:294-301) reduces two genuinely distinct results to one <persisted-stub>sha256:<quoted> fingerprint. That feeds both guards: #9450's stateful-read guard reads distinct results as a repetition (spurious halt on a turn that was progressing) and the error guard accumulates its streak across different errors (early halt).

The trigger is narrow, but the failure is a false halt that is user-visible and hard to attribute. The suggested gate looks correct and cannot cost a legitimate producer digest, since shell failure blocks always carry the PGID line (shell.ts:2838, inside blockLines); it needs to land at both reuse sites.

This is a behavior change rather than a test, and the PR is at +1562 — over this pass's 1500-line cap — so no code changes this round. Leaving unresolved for a human call.

[qwen-code-ci-bot]

[Suggestion] Oversized MCP tool errors past the scheduler's error gate are fingerprinted on the sha256 of the full message — which includes the varied function-call args — so the error-repetition streak never accumulates; this is the exact per-retry-unique fingerprinting that normalizeMcpToolError (loopDetectionService.ts:745) exists to prevent for the un-stubbed shape. A dead-end loop calling an MCP tool with varied args while the server returns the same large error payload every round: once the message exceeds getTruncateToolOutputThreshold() + GATE_HEADROOM (25 000 + 3 000 = 28 000 chars, coreToolScheduler.ts:5779-5790), persistAndTruncateToolResult stubs it; buildMcpToolError embeds no anchored digest, so extractAnchoredFullDigest returns null and this fallback hashes the whole message — every retry's stub digest is unique, each round reduces to a distinct <persisted-stub>sha256:<unique> signature, and REPEATED_TOOL_ERROR never fires while the session keeps burning tokens. The stub no longer starts with MCP tool ', so normalizeMcpToolError never sees it.

Witness:

probe at HEAD, three-way split: (1) oversized MCP errors, identical server payload, varied args, 4 rounds — fired [false,false,false,false], all 4 stub digests distinct; (2) contrast arm below the gate, same varied args — fired [false,false,true,true], scoping the defect to the oversized band; (3) fix arm embedding sha256(server payload) as an anchored FULL_OUTPUT_DIGEST_LABEL line flipped the oversized arm to [false,false,true,true].

Have buildMcpToolError embed a producer-owned digest — a sha256 of the server payload anchored as a FULL_OUTPUT_DIGEST_LABEL line, as shell.ts now does for failure cores — so buildStub/fitText reuse it instead of hashing the full message. The embedded digest must satisfy the extractor's shape: line-anchored, exactly 64 lowercase hex chars ending the line (/^[0-9a-f]{64}$/ + terminator undefined/\n/\r, truncation.ts:449-451). For the fix witness: a sibling of 'fires on repeated MCP errors with varied function-call JSON but identical server payloads' that drives each round's MCP error through the real persistAndTruncateToolResult (mirroring the scheduler-gate band test) and asserts firing at round 3; removing the producer digest must make it red.

中文说明

超过调度器错误门的超大 MCP 工具错误，会以整条消息（含变化的 function-call 参数）的 sha256 为指纹，错误重复连击因此永不累积——这正是 normalizeMcpToolError（loopDetectionService.ts:745）为未 stub 形态专门防止的「按次唯一指纹」。死循环以变体参数调用 MCP 工具、服务端每轮返回同一个大错误载荷时：一旦消息超过 getTruncateToolOutputThreshold() + GATE_HEADROOM（25 000 + 3 000 = 28 000 字符，coreToolScheduler.ts:5779-5790），persistAndTruncateToolResult 会将其 stub 化；buildMcpToolError 不内嵌锚定 digest，extractAnchoredFullDigest 返回 null，该兜底就对整条消息取哈希——每次重试的 stub digest 都不同，每轮归约为不同的 <persisted-stub>sha256:<unique> 签名，会话持续烧 token 而 REPEATED_TOOL_ERROR 永不触发。此时 stub 不再以 MCP tool ' 开头，normalizeMcpToolError 根本看不到它。

修复：让 buildMcpToolError 内嵌生产方持有的 digest——对服务端载荷取 sha256 并以 FULL_OUTPUT_DIGEST_LABEL 行锚定，如同 shell.ts 现在对失败核心所做——使 buildStub/fitText 复用而非哈希整条消息。内嵌 digest 必须满足提取器形状：行锚定、恰好 64 个小写十六进制字符且以行尾结束（/^[0-9a-f]{64}$/ + 终止符 undefined/\n/\r，truncation.ts:449-451）。验收：仿照「变体 function-call JSON、相同服务端载荷的重复 MCP 错误触发」新增一个用例，让每轮 MCP 错误经过真实 persistAndTruncateToolResult（仿照调度器门波段测试），断言第 3 轮触发；移除生产方 digest 后必须变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Verified real at head d136ac9: buildMcpToolError embeds safeJsonStringify(functionCall) — the varied args — and no digest line (tools/mcp-tool.ts:852-887), so an oversized MCP error past the scheduler error gate (core/coreToolScheduler.ts:5779-5790) hits buildStub's full-message fallback hash (tools/truncation.ts:573-574) and fingerprints uniquely per retry; the stub no longer starts with MCP tool ', so normalizeMcpToolError (services/loopDetectionService.ts:745) never sees the oversized shape and the streak never accumulates while the loop keeps burning tokens. Deferred under the scope fuse (PR additions 1559 > 1500, no code changes this round); candidate for follow-up.

[yiliang114]

Confirmed as a false negative in the guard this PR exists to add.

buildMcpToolError (mcp-tool.ts:864-874) composes MCP tool '<name>' ... with response: <payload> with no anchored digest line. Past the scheduler gate, extractAnchoredFullDigest returns null, so buildStub hashes the whole message — which embeds the function-call JSON whose args the dead-end loop varies per retry. The stub then begins with <persisted-output> / Output too large (, not MCP tool ', so normalizeMcpToolError (loopDetectionService.ts:745-746, requires that prefix on the raw string) never runs and normalizeToolErrorText falls through to a per-retry-unique <persisted-stub>sha256:.... The streak never accumulates in that band while the session keeps burning tokens.

Below the gate the same varied-args shape does fire, because the raw MCP tool ' text reaches normalizeMcpToolError and reduces to the server payload — consistent with the three-way split described here.

The fix is producer-side: have buildMcpToolError embed a sha256 of the server payload as a FULL_OUTPUT_DIGEST_LABEL line, matching what shell.ts:2858-2869 now does for failure cores, so both reuse sites pick it up instead of hashing the full message. That's a code change in a third file, and the PR is at +1562 (over this pass's 1500-line cap), so not this round. Leaving unresolved.

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

⚠️ Round 5, and the diff has grown 4.5x since this review first measured it (192 → 872 source diff lines). The findings below are anchored to the current patch, so they can only say where this approach leaks — never that a different approach would retire all of them at once. Before fixing them, a human should decide whether the shape of the change is still right. Advisory only: this does not affect the verdict, and nothing here is a blocker.

1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

• R5-10 digest-extractor consolidation (extractAnchoredFullDigest / extractAnchoredStubDigest / the tool-response-finalizer copy) — already reported (stage-2 triage comment 5525389470; round-3 review body 5113311977; round-4 review body 51162…

2 candidate finding(s) this round's reviewers re-derived matched entries already carried on this PR and were set aside before verification (R3-2, R3-4) — a matched posted finding is ruled in the previous-round status as always, and a matched deferral stays on the standing deferral record.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox), Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI, and no suite ran locally either: build-test's whole-call budget ran out before its test phase (packages/core built green; packages/cli's build was killed by the leftover budget). Only Test (ubuntu-latest, Node 22.x) is green at this head, so the ~1150 new test lines are evidenced by that one job — Linux only, which matters for a change to shell.ts signal and exit-code handling that the PR's own Tested-on table marks macOS and Windows as untested.

Not reviewed: reverse audit — stopped after round 3 of a 5-round cap and did not reach two consecutive dry rounds: rounds 1 and 2 both reported findings (chunk 3 dry in round 1 only) and round 3 reported on 4 of 6 chunks. Stopped by the orchestrator to leave budget for verification, compose and submission rather than by convergence, so gaps a fourth and fifth pass would have found are not excluded.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": whether agent-core.ts:1253 's roundResultParts covers one full executed batch (I verified the batch-per-round premise only for the client.ts + use-llm-stream…; chunk 1: executing the new tests to confirm the witnesses (this worktree has no node_modules / dist ; npm ci + npm run build exceeds the remaining tool budget), so ….

Test Plan (not a blocker): src/services/loopDetectionService.test.ts — no such file or directory.

Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:

• packages/core/src/services/loopDetectionService.ts:821 — [probe] D5-1 multi-signature rounds overwrite the single-slot streak, and whether a round fires depends on part order (re-derivation of a round-4 deferral)
• packages/core/src/tools/tool-response-finalizer.ts:273 — [probe] D5-2 fitText folds the 85-char digest line into the header, so a small per-slot allocation returns a chopped digest and zero preview (re-derivation of a round-4 deferral)
• packages/core/src/core/client.test.ts:8511 — [probe] D5-3 both new client.ts wiring tests feed one functionResponse part per round, so the batch-per-message placement is unpinned
• packages/core/src/services/loopDetectionService.test.ts:3428 — [probe] D5-4 the test named 'restarts the streak on a different error' never observes a restart — every assertion is false/null
• packages/core/src/services/loopDetectionService.test.ts:3638 — [probe] D5-5 the new digest?: boolean fixture option is a dead switch (0 of 6 call sites pass it)
• packages/core/src/services/loopDetectionService.test.ts:3784 — [probe] D5-6 the keep-both fixtures hand-copy the producer's envelope wording, so marker drift between the two modules is invisible
• packages/core/src/services/loopDetectionService.ts:238 — [probe] D5-7 'Tool output truncated.' added to STUB_PRODUCER_PREFIXES as a bare literal while this diff exports CANCELLED_TOOL_ERROR_PREFIX as the producer-owned pattern
• packages/cli/src/nonInteractiveCli.ts:201 — [review] D5-8 the new loop-type label copy is asserted by no test; the new test pins only the always-on hint routing
• packages/core/src/services/loopDetectionService.ts:723 — [probe] D5-9 'Process Group PGID:' is a private literal in both modules, unpinned at the producer end — a producer rename disables the guard with 489 tests green
• packages/core/src/core/client.ts:3970 — [probe] D5-10 headless teammate-merged rounds go out as SendMessageType.Teammate, so their tool errors are neither fed to the guard nor allowed to accumulate
• packages/core/src/tools/truncation.ts:572 — [probe] D5-11 one 'Full output sha256:' label now denotes two quantities: the full saved content, or the four-line failure core for shell blocks
• packages/cli/src/nonInteractiveCli.ts:218 — [probe] D5-12 the interactive surface's always-on guard enumerations (dialog note, skipLoopDetection description) still name only three guards
• packages/core/src/services/loopDetectionService.ts:59 — [probe] D5-13 a second, independently calibrated repeated-tool-failure guard with the opposite escalation policy, an unbounded streak window, and an overlapping loop type
• packages/core/src/agents/runtime/agent-core.ts:1254 — [probe] D5-14 a repeated_tool_error subagent stop is rendered downstream as 'duplicate tool-call loop detected'
• packages/core/src/core/client.ts:3971 — [probe] D5-15 mid-turn human steer input neither changes the signature nor resets the streak, so the guard halts a turn the user is actively steering
• packages/core/src/services/loopDetectionService.test.ts:3702 — [probe] D5-16 the advisory test hand-invents the producer's long-run hint text instead of importing the exported buildLongRunningForegroundHint

Convergence: round 5 posted 10 inline comment(s), 3 of them reported for the first time; the previous round posted 8 (3 new). Findings keep coming back to the same files: packages/core/src/tools/shell.ts (findings in round 3; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

仅完成部分审查，审查缺口已披露。

⚠️ 第 5 轮，且自本审查首次测量以来 diff 已增长 4.5 倍（源码 diff 行数 192 → 872）。下方的发现都锚定在当前这版补丁上，因此它们只能指出这个方案在哪里漏了，而无法说明换一个方案就能一次性消除全部问题。在动手修复之前，应由人来判断这次改动的整体形态是否仍然正确。仅供参考：本段不影响判定结论，其中也没有任何阻断项。

本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。

本轮评审重新推导出的 2 条候选发现与本 PR 已携带的条目匹配，已在验证前搁置（R3-2, R3-4）——被匹配的已发布条目照常在上一轮状态区裁定，被匹配的延后条目仍保留在延后清单记录中。

未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox), Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI, and no suite ran locally either: build-test's whole-call budget ran out before its test phase (packages/core built green; packages/cli's build was killed by the leftover budget). Only Test (ubuntu-latest, Node 22.x) is green at this head, so the ~1150 new test lines are evidenced by that one job — Linux only, which matters for a change to shell.ts signal and exit-code handling that the PR's own Tested-on table marks macOS and Windows as untested.

未审查（原文为英文）：reverse audit — stopped after round 3 of a 5-round cap and did not reach two consecutive dry rounds: rounds 1 and 2 both reported findings (chunk 3 dry in round 1 only) and round 3 reported on 4 of 6 chunks. Stopped by the orchestrator to leave budget for verification, compose and submission rather than by convergence, so gaps a fourth and fifth pass would have found are not excluded.

未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：whether agent-core.ts:1253 's roundResultParts covers one full executed batch (I verified the batch-per-round premise only for the client.ts + use-llm-stream…；chunk 1：executing the new tests to confirm the witnesses (this worktree has no node_modules / dist ; npm ci + npm run build exceeds the remaining tool budget), so …。

Test Plan（非阻断）：src/services/loopDetectionService.test.ts — no such file or directory。

收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 16 条（原文未翻译，列表见上方英文部分）。

收敛情况：第 5 轮发布了 10 条行内评论，其中 3 条是首次提出；上一轮发布了 8 条（其中 3 条首次提出）。发现反复回到同一批文件：packages/core/src/tools/shell.ts（第 3 轮已出过发现，本轮又有 2 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）

— qwen3.8-max via Qwen Code /review (v0.23.0)

[qwen-code-ci-bot]

[Critical] R4-1: The repeated_tool_error telemetry uploads the raw tool-error excerpt to a default-on third-party endpoint, unredacted — and for shell failures the excerpt leads with the command line and the working directory. This is the round-4 blocker re-asserted: it still stands verbatim at this head. checkRepeatedToolError passes errorExcerpt: raw (the un-normalized functionResponse.response.error), LoopDetectedEvent applies only .slice(0, 200) plus the surrogate trim — truncation, no redaction — and logLoopDetected sends it to QwenLogger as properties.error_excerpt while also spreading ...event into the OTel attributes. The guard fires precisely when a command has failed three rounds running, and agent shell commands routinely carry credentials inline (git clone https://x-access-token:ghs_…@github.com/…, curl -H "Authorization: Bearer …", npm publish with a registry token, a postgres://user:…@host/db DSN), and those sit in the first ~40 characters of the uploaded excerpt. usageStatisticsEnabled defaults to true, so this reaches the RUM endpoint without anyone opting in, and collected data cannot be un-collected. The surrogate fix that landed in round 3 makes the cut well-formed; it does not redact anything.

Witness:

witness: not run — an unredacted field in an uploaded payload is not executable in a harness.
Mechanism re-traced at head 6439e0921d:
  loopDetectionService.ts:834   errorExcerpt: raw
  telemetry/types.ts:527-535    slice(0, 200) + lone-surrogate trim only, no redaction
  qwen-logger.ts:744-749        error_excerpt -> RUM properties (usage statistics default-on)
packages/core/src/services/loopDetectionService.ts has 0 changed hunks between round 4's
head d136ac99 and this head, and this thread's own reply reads "Verified real at head d136ac99".

Smallest fix: drop error_excerpt and keep error_signature — the sha256 already satisfies the stated need, that pages arrive with the identity of the failing payload. If the excerpt stays, take it from normalized rather than raw (already stripped of Command:/Directory:/PGID for shell blocks) and route it through the exported truncateSpanError, consistent with the lastChantExcerpt decision documented in this same file. This is the same field a second new finding this round covers on the trace sink, so please decide the excerpt's fate once, for both sinks.

Any fix must keep error_signature computable from the repeated payload alone — that field is the whole observability gain this feature added (loopDetectionService.ts:829-836). The acceptance criterion is a unit test asserting that a LoopDetectedEvent built from a shell-shaped error whose Command: line contains a credential-shaped string does not carry that string in error_excerpt (or carries no error_excerpt at all); please remove the redaction or removal afterwards and confirm that test goes red.

中文说明

repeated_tool_error 遥测会把未脱敏的原始工具错误摘要上传到「默认开启」的第三方端点——而对 shell 失败来说，摘要开头就是命令行与工作目录。这是第 4 轮阻塞项的再次确认：在本 head 上原样存在。checkRepeatedToolError 传的是 errorExcerpt: raw（未归一化的 functionResponse.response.error），LoopDetectedEvent 只做 .slice(0, 200) 加代理项修剪——仅截断、无脱敏——logLoopDetected 把它作为 properties.error_excerpt 送往 QwenLogger，同时通过 ...event 展开进 OTel 属性。该守卫恰好在命令连续三轮失败时触发，而 agent 的 shell 命令经常内联携带凭据（git clone https://x-access-token:ghs_…@github.com/…、curl -H "Authorization: Bearer …"、带 registry token 的 npm publish、postgres://user:…@host/db 这类 DSN），它们正好落在上传摘要的前 ~40 个字符内。usageStatisticsEnabled 默认为 true，因此数据会在无人选择性加入的情况下到达 RUM 端点，且已收集的数据无法收回。第 3 轮落地的代理项修复只是让截断在编码上合法，并没有脱敏任何内容。

最小修复：去掉 error_excerpt，保留 error_signature——sha256 已满足其声明诉求（page 到达时即带有失败载荷的身份）。若要保留摘要：取自 normalized 而非 raw（对 shell 块已剥掉 Command:/Directory:/PGID），并走已导出的 truncateSpanError，与同一文件中已有明文记录的 lastChantExcerpt 决定保持一致。本轮另有一条新发现涉及同一字段在 trace sink 上的泄露，请一并决定该摘要的去留。

任何修复都必须保证 error_signature 仍可仅从重复载荷计算得出——该字段是本特性带来的全部可观测性收益（loopDetectionService.ts:829-836）。验收标准：新增单测断言——用 Command: 行含凭据形态字符串的 shell 形态错误构造 LoopDetectedEvent，其 error_excerpt 不含该字符串（或根本不带 error_excerpt）；随后移除脱敏/移除逻辑并确认该测试变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Verdict: REAL and not disputed — but this is a re-mint of an already-answered thread at the same path:line. Nothing lands this round; thread stays unresolved.

Where this is already answered (not restating it here):

• Same anchor, same claim: thread PRRT_kwDOPB-92c6fZFel (loopDetectionService.ts:834, R4-1) — fix(core): halt turns on repeated identical tool errors #10916 (comment) carries the full re-verification at this exact head 6439e0921d: raw → types.ts 200-char cut + trailing-surrogate trim only, no redaction → qwen-logger.ts properties.error_excerpt → RUM host, gated only by getUsageStatisticsEnabled() which defaults to true.
• The OTel half of the same field: thread PRRT_kwDOPB-92c6fsJkT (telemetry/types.ts:511, R5-2) — fix(core): halt turns on repeated identical tool errors #10916 (comment), answered 2026-09-07: error_excerpt is absent from SENSITIVE_ATTRIBUTE_KEYS (log-to-span-processor.ts:58-65).
• The sink-wide half is already filed: Usage-statistics telemetry uploads raw tool-error text (including shell command lines) to RUM without redaction #11198 (OPEN).

What I re-read at head to confirm the re-mint is not stale (confirmation only, not a new argument): loopDetectionService.ts:830-838 still passes errorExcerpt: raw, and raw is still the un-normalized response['error'] string produced by extractToolErrors (:777-787). The finding itself records 0 changed hunks in this file since d136ac99, which matches what I see.

Two framing corrections already posted on the sibling still apply and I am not re-litigating them: the endpoint is Aliyun RUM — first-party infra for this product, not a third party (the default-on unredacted upload concern stands regardless); and the same raw block already reaches the same endpoint on main via logToolCallEvent → properties.error_message, so the leak class pre-exists this PR and is wider than error_excerpt (#11198). error_excerpt is nonetheless new in this diff, so this PR is the cheapest place not to add a second instance.

Decision owed — unchanged, and now spanning two sinks, so please decide the excerpt's fate once: (a) drop error_excerpt, keep error_signature; (b) keep the excerpt but pass normalized instead of raw; (c) route through truncateSpanError. (c) alone is insufficient. This is also a security-boundary call (category ②) → human-gated.

Why nothing lands this round: the PR is at +1562/−45, over the 1500-addition scope fuse this closeout sweep runs under, and findings are still being minted round after round without converging — so this is a replies-only round. A re-mint of an unresolved sibling does not get resolved while that sibling is open, so PRRT_kwDOPB-92c6fZFel and this thread both stay unresolved.

[qwen-code-ci-bot]

[Critical] R4-2: [certifies-falsely] [regression] The digest that identifies a truncated result is recovered by scanning rendered text, so the first line-anchored Full output sha256: <64hex> match found anywhere in untrusted content becomes that result's authoritative fingerprint. This is carried from round 4 as R4-2 and escalated: the mechanism is now measured end to end through the real producers, and it spans three sites rather than one — buildStub here, fitText (tool-response-finalizer.ts:270-273), and the consumer's PGID-gated branch (loopDetectionService.ts:722-729). shell.ts interpolates Output: ${result.output} raw into the failure block and appends the producer's own digest last, so a digest-shaped line the command itself printed precedes the producer's and wins the first-match scan. Three genuinely different failures whose output echoes one such line therefore collapse to a single signature and the always-on guard halts a turn that was attempting three different things; conversely identical failures whose output echoes a varying digest line fingerprint uniquely and the guard never fires. The same scan at buildStub also breaks the invariant stated two lines above it — that the digest stays sensitive to mutations anywhere in the full content (issue 9450) — and stripPersistenceEnvelope is shared with the issue-9450 stateful-read guard, so peer-authored task_list content quoting one such line collapses two different boards.

Witness:

[probe] real ShellTool + real LoopDetectionService + real persistAndTruncateToolResult
        + real enforceFunctionResponseBudget
ZZA2 first anchored label index: 67  last: 233  producer digest is last line: true
P1 producer digests distinct: 3 [c1cfadddeed8, 814ad12eca1e, 0d6225ace0df]
P1 fired per round (embedded identical digest line): [ false, false, true ]  loopType: repeated_tool_error
P1-control fired per round (no embedded line):        [ false, false, false ] loopType: null
P1-mirror fired per round (identical failure, varying embedded line): [ false, false, false ]
P2 sha256(contentA) === sha256(contentB)? false
P2 envelope digest A/B: ffff…   P2 IDENTICAL envelope digests for distinct contents: true
  reverting exactly the `extractAnchoredFullDigest(content) ??` hunk -> false  (so the merge
  base handled this trigger correctly: baseline regression)
P3 fitText digest A: eeee…  B: eeee…  IDENTICAL for distinct slots: true

The fix that closes the class is to stop round-tripping the digest through model-visible text: carry it structurally, as an explicit producerDigest argument to persistAndTruncateToolResult/buildStub/fitText and a digest field on the shell error object that the scheduler puts on functionResponse.response, so content cannot impersonate it. A positional rule cannot be the fix — shell blocks put the producer digest last, buildStub puts it third before a preview that can itself quote one, and shell.ts:3052/3084 append advisories after it, so neither first-match nor last-match is producer-owned in every shape.

A positional change to the shared helper must not be the fix either: buildStub emits its digest before the untrusted preview (truncation.ts:579 and :589, followed by Preview (up to ${PREVIEW_SIZE_CHARS} chars): ) and stripPersistenceEnvelope reads it with the same first-match scanner, so switching that scanner to last-match would let preview content hijack the stub fingerprint; and FULL_OUTPUT_DIGEST_LABEL = 'Full output sha256: ' (truncation.ts:34) is also consumed by the issue-9450 stateful-read guard, so the label cannot be repurposed per consumer. Please add a case to loopDetectionService.test.ts feeding three rounds of shell failure blocks with different Exit Code:/Command:/PGID whose Output: embeds one identical line-anchored digest line, asserting recordToolErrorBatch returns false on all three with getLastLoopType() null, plus the mirror (identical cores with a varying embedded label still reaching true on round 3), and a truncation.test.ts case asserting that oversized non-stub-shaped content embedding a mid-string digest line still yields sha256(full content) — then remove the structural fix and confirm those go red while the existing reuses the producer-anchored failure-core digest… and keeps the full-content digest… tests stay green.

中文说明

用于标识截断结果的摘要，是通过扫描渲染文本恢复的：只要在不可信内容里的任意位置找到第一个行锚定的 Full output sha256: <64hex>，它就成了该结果的权威指纹。本条承接第 4 轮的 R4-2 并升级严重级：机制本轮已用真实生产者完整实测，且涉及三处而非一处——此处的 buildStub、fitText（tool-response-finalizer.ts:270-273），以及消费端 PGID 门控分支（loopDetectionService.ts:722-729）。shell.ts 把 Output: ${result.output} 原样插入失败块，并把生产者自己的摘要追加在最后，因此命令自身输出中的摘要形态行会排在生产者摘要之前并赢得首个匹配。于是三个真正不同的失败，只要输出中回显了同一行摘要，就会坍缩为同一签名，让「始终开启」的守卫终止一个正在做三件不同事情的轮次；反过来，输出中回显变化摘要行的相同失败每次指纹都不同，守卫永不触发。buildStub 上的同一扫描还破坏了它上方两行声明的不变量（摘要必须对完整内容中任意位置的改动保持敏感，issue 9450），而 stripPersistenceEnvelope 与 issue 9450 的有状态读守卫共享，所以对端撰写的 task_list 内容引用这样一行，就会让两个不同的看板坍缩成一个。

见证见上方英文段的探针输出（三个不同生产者摘要 → 1 个指纹并触发；无嵌入行对照组不触发；镜像组不触发；回退该 hunk 即翻转，说明合并基线本来正确处理了这一触发条件）。

能关闭这一类的修复是：不要把摘要经由模型可见文本往返传递，而是以数据结构承载——给 persistAndTruncateToolResult/buildStub/fitText 增加显式的 producerDigest 参数，并在 shell 的 error 对象上带一个摘要字段由调度器放进 functionResponse.response，使内容无法冒充它。位置规则（取首个/取末个）不能作为修复：shell 块把生产者摘要放在最后，buildStub 把它放在第三行、其后是本身可能引用摘要的预览，而 shell.ts:3052/3084 还会在其后追加提示行——两种取法都不是在所有形态下由生产者掌控。

同样不要改共享扫描器的取法：buildStub 在不可信预览之前输出摘要（truncation.ts:579、:589，其后是 Preview (up to ${PREVIEW_SIZE_CHARS} chars): ），而 stripPersistenceEnvelope 用同一个首匹配扫描器读取它，改成末匹配会让预览内容劫持 stub 指纹；且 FULL_OUTPUT_DIGEST_LABEL = 'Full output sha256: '（truncation.ts:34）也被 issue 9450 的有状态读守卫消费，该标签不能按消费者各自改义。请在 loopDetectionService.test.ts 新增用例：三轮 Exit Code:/Command:/PGID 各异、但 Output: 中嵌入同一行锚定摘要的 shell 失败块，断言三次 recordToolErrorBatch 均返回 false 且 getLastLoopType() 为 null；再加镜像用例（相同失败核心配变化的嵌入行，第三轮仍应为 true）；并在 truncation.test.ts 断言：非 stub 形态的超大内容中段嵌入摘要行时仍应得到 sha256(完整内容)。随后移除结构化修复并确认这些用例变红，而既有的 reuses the producer-anchored failure-core digest… 与 keeps the full-content digest… 保持绿。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Verdict: REAL and not disputed — re-mint of two already-answered threads one line away (truncation.ts:574). Nothing lands this round; thread stays unresolved.

Where this is already answered (not restating it here):

• PRRT_kwDOPB-92c6fZFe_ (truncation.ts:574) — fix(core): halt turns on repeated identical tool errors #10916 (comment), answered at this head: main hashed the content unconditionally, this diff made it extractAnchoredFullDigest(content) ?? hash(content), the producer reuse carries no shell-shape gate while the consumer gates its digest trust on Process Group PGID:, and both task_list can falsely trigger duplicate tool-call loop detection while team state changes #9450's stateful-read guard and this PR's error guard are fed by the collapse.
• PRRT_kwDOPB-92c6fZFfL (truncation.ts:574) — fix(core): halt turns on repeated identical tool errors #10916 (comment) — the MCP/buildStub corner of the same helper.

What I re-read at head 6439e0921d to confirm the two claims this escalation adds (the third site, and that a positional rule cannot be the fix):

• truncation.ts:573-575 is exactly extractAnchoredFullDigest(content) ?? crypto.createHash('sha256').update(content).digest('hex').
• extractAnchoredFullDigest (:438-455) scans from index 0 and returns the first line-anchored Full output sha256: <64hex>. Its own doc (:431-436) rules out a mid-line mention but says nothing about a line-anchored quote inside untrusted content — that is precisely the gap, so the first-match scan is as described.
• The invariant the finding says is broken is stated in the comment immediately above the reuse (:568-571 — "the digest must stay sensitive to mutations anywhere in the full content (issue task_list can falsely trigger duplicate tool-call loop detection while team state changes #9450)"). Confirmed: the reuse drops it whenever content quotes such a line.
• A positional rule is indeed unavailable. buildStub emits its digest at :579 (isFilePath) / :589 (non-file), and the untrusted Preview (up to ${PREVIEW_SIZE_CHARS} chars): follows at :581 / :591 — so flipping the shared scanner to last-match lets preview content hijack the stub fingerprint. Agreed, and worth recording since it rules out the cheap fix.
• Consumer asymmetry re-confirmed: loopDetectionService.ts:725-730 only trusts the extracted digest when the enveloped text includes Process Group PGID:; the producer reuse at truncation.ts:573 has no such gate.

One cite I did not independently re-verify: fitText (tool-response-finalizer.ts:270-273) as the third site. I took that from the existing sibling answer rather than re-reading the file this round — flagging it so the record is accurate about what was checked where.

Fix direction (not applied): carry the digest structurally instead of round-tripping it through model-visible text — an explicit producerDigest argument to persistAndTruncateToolResult/buildStub/fitText plus a digest field on the shell error object that the scheduler puts on functionResponse.response. That lands in ≥3 production files and changes a helper the #9450 stateful-read guard also depends on.

Why nothing lands this round: +1562/−45, over the 1500-addition scope fuse; non-converging findings; replies-only round. This is category ④ (expanding the diff well beyond the PR's promise) and it mutates shared infrastructure another issue's guard relies on → human-gated. Thread stays unresolved while both siblings are unresolved.

[qwen-code-ci-bot]

[Critical] R3-3: [certifies-falsely] [new-surface] MCP tool errors have no producer-owned stable identity, so the consumer re-derives one from a rendered message whose entrance space is unbounded — third-party server payload, server-chosen tool name, model-chosen args. This is carried from round 3 (where it was the mirrored private literals) and escalated on an executed witness, and it absorbs round 4's R4-3 as a third corner of the same class. Corner (a) is a false positive that halts a productive turn: normalizeMcpToolError splits on error.lastIndexOf(' with response: '), which lands inside a server payload that happens to contain that phrase, so three genuinely different failures normalize to the same string and the always-on guard aborts the turn at round 3. Corner (b) is a false negative: the image branch of buildMcpToolError renders getDisplayFromParts(truncatedContent.parts), which for an oversized part is a truncateToolOutput envelope carrying a per-call random artifact path, and because normalizeToolErrorText returns the MCP branch before stripPersistenceEnvelope that path is fingerprinted verbatim, so an identical repeated MCP failure never accumulates. Corner (c) is the same defect past the scheduler's error gate: an MCP message over getTruncateToolOutputThreshold() + GATE_HEADROOM (28 000 by default) is replaced by a buildStub envelope whose digest is a sha256 over the whole message including the safeJsonStringify(functionCall) args that normalizeMcpToolError exists to strip, so a dead-end loop retrying an MCP write tool with a ~30 KB argument varying each round never fires — while the doc on extractToolErrors in this same hunk claims identical underlying errors fingerprint identically no matter how they were produced. The same ordering also splits one failure across two namespaces when growing args straddle the gate, resetting a streak already at 2.

Witness:

[probe] messages built with the producer's own template + the real safeJsonStringify,
        fed to the real guard
P4 message 0: MCP tool 'q' reported tool error for function call: {"name":"q","args":{"query":"A"}}
              with response: [{"text":"query A failed with response: 500"}]   (…B…, …C…)
P4 fired per round:                              [ false, false, true ]  loopType: repeated_tool_error
P4-control fired per round (marker-free payloads): [ false, false, false ] loopType: null
P4b marker inside the ARGS instead:                [ false, false, false ]  (does not collapse —
    the producer's separator follows the args JSON, so the trigger is a marker in the payload)
P9 outputFile: /tmp/zzverify-mcp-3nx1uE/dw_query_385c2d37b154.output
P9 fired per round (identical oversized MCP payload): [ false, false, false ] loopType: null

The class-closing fix is the one this PR already applies elsewhere: let the producer own the boundary. Have buildMcpToolError append a line-anchored FULL_OUTPUT_DIGEST_LABEL line digesting the stable server payload only (the args-free text after the last ' with response: '), the way shell.ts:2860-2869 digests the stable failure core, and have normalizeMcpToolError return the same ${PERSISTED_STUB_DIGEST_PREFIX}<digest> form when the message carries that anchored digest — so the raw and persisted shapes of one MCP failure land in one namespace, and the guard fails closed to "not an MCP payload" when the label is absent instead of searching a mid-string English phrase with lastIndexOf.

Two premises the fix must respect: packages/core/src/tools/mcp-tool.test.ts:351-353 pins the exact rendered message, so a producer-side boundary change must update that assertion in the same commit; and the anchored-digest scanners admit only a label that starts a line followed by exactly 64 chars matching /^[0-9a-f]{64}$/ with the line ending right after, taking the first such match, so the new producer line must be the only one in the message or the volatile full-text hash wins again (that is the R4-2 class, filed separately above). Please add a case to loopDetectionService.test.ts feeding three rounds of same-tool MCP errors whose payloads differ but share a trailing ' with response: ' suffix and asserting recordToolErrorBatch stays false with getLastLoopType() not REPEATED_TOOL_ERROR, plus a case feeding the same MCP failure raw, then as the buildStub envelope with different args, then raw again, asserting the third round returns true — the existing MCP test uses a marker-free 66-char payload and stays green either way, so without these the fix is unwitnessed; remove the producer digest afterwards and confirm they go red.

中文说明

MCP 工具错误没有由生产者掌控的稳定身份，因此消费端只能从一条渲染后的消息里重新推导身份，而这个入口空间是无界的——第三方服务端载荷、服务端选定的工具名、模型选定的参数。本条承接第 3 轮（当时是「以私有字面量镜像契约」），并基于已执行的见证升级严重级，同时把第 4 轮的 R4-3 作为同一类别的第三个入口吸收进来。入口 (a) 是会终止正常轮次的误报：normalizeMcpToolError 以 error.lastIndexOf(' with response: ') 切分，当服务端载荷本身含有该短语时切点会落进载荷内部，于是三个真正不同的失败归一化为同一字符串，「始终开启」的守卫在第 3 轮就终止轮次。入口 (b) 是漏报：buildMcpToolError 的图片分支渲染 getDisplayFromParts(truncatedContent.parts)，对超大 part 得到的是带每次调用随机产物路径的 truncateToolOutput 信封；由于 normalizeToolErrorText 在 stripPersistenceEnvelope 之前就从 MCP 分支返回，该路径被原样纳入指纹，因此相同的重复 MCP 失败永不累积。入口 (c) 是同一缺陷在调度器错误门之后的形态：超过 getTruncateToolOutputThreshold() + GATE_HEADROOM（默认 28 000）的 MCP 消息会被 buildStub 信封替换，而该信封的摘要是对**整条消息（含 safeJsonStringify(functionCall) 参数）**做 sha256——参数正是 normalizeMcpToolError 要剥掉的东西；于是「每轮变换约 30 KB 参数重试某个失败的 MCP 写入工具」这种死循环永不触发，而同一 hunk 中 extractToolErrors 的文档却声称「无论以何种方式产生，底层相同的错误指纹一致」。参数增长跨越该门时，同一失败还会被劈成两个命名空间，把已累积到 2 的连击重置。

见证见上方英文段探针输出（P4 触发、对照组与「标记在参数内」组均不触发；P9 图片分支不触发）。

能关闭这一类的修复正是本 PR 在别处已采用的做法：让生产者掌控边界。请让 buildMcpToolError 追加一行锚定的 FULL_OUTPUT_DIGEST_LABEL，只对稳定的服务端载荷（最后一个 ' with response: ' 之后的、不含参数的文本）计算摘要——就像 shell.ts:2860-2869 对稳定失败核心所做的那样；并让 normalizeMcpToolError 在消息带有该锚定摘要时返回同样的 ${PERSISTED_STUB_DIGEST_PREFIX}<digest> 形态。这样一个 MCP 失败的原始形态与持久化形态会落在同一命名空间，并且在标签缺失时守卫会保守地判定「不是 MCP 载荷」，而不是用 lastIndexOf 去搜索消息中段的英文短语。

修复须尊重两个前提：packages/core/src/tools/mcp-tool.test.ts:351-353 钉住了渲染消息的确切文本，因此生产者侧的边界改动必须在同一次提交里更新该断言；锚定摘要扫描器只接受「行首标签 + 恰好 64 个 /^[0-9a-f]{64}$/ 字符 + 行随即结束」，并且取第一个匹配，所以新的生产者行必须是消息中唯一的一行，否则又会是易变的全文本哈希获胜（那属于上文另立的 R4-2 类）。请在 loopDetectionService.test.ts 新增两个用例：其一，三轮同工具 MCP 错误、载荷各异但都以 ' with response: ' 结尾，断言 recordToolErrorBatch 保持 false 且 getLastLoopType() 不是 REPEATED_TOOL_ERROR；其二，同一个 MCP 失败先以原始形态、再以参数不同的 buildStub 信封、再回到原始形态喂入三轮，断言第三轮返回 true。既有的 MCP 用例使用不含标记的 66 字符载荷，两种实现下都保持绿，因此没有这两个用例，修复就没有见证；完成后移除生产者摘要并确认它们变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Verdict: REAL. Corners (a) and (b) are new claims and I verified both independently at head 6439e0921d; corner (c) and the literal-mirroring claim at this anchor are re-mints. Nothing lands this round; thread stays unresolved.

Already answered (cross-link, not restated):

• The literal-mirroring / producer-wording-drift claim at this anchor: PRRT_kwDOPB-92c6fTKig (loopDetectionService.ts:73) — fix(core): halt turns on repeated identical tool errors #10916 (comment) (three private literals at :71-73 mirroring wording buildMcpToolError composes inline, no shared constant — while this same diff does apply producer-owns-the-shape to cancellations via CANCELLED_TOOL_ERROR_PREFIX). A second re-post of that, PRRT_kwDOPB-92c6fZFes, was resolved as a duplicate on 2026-09-04.
• Corner (c) — oversized MCP past the scheduler gate hitting buildStub's full-message hash including the varied args: PRRT_kwDOPB-92c6fZFfL (truncation.ts:574) — fix(core): halt turns on repeated identical tool errors #10916 (comment), confirmed at this head.

Corner (a) — false positive that halts a productive turn. Verified. normalizeMcpToolError (loopDetectionService.ts:745-762) takes callMarker = error.indexOf(MCP_TOOL_ERROR_CALL_MARKER) but responseMarker = error.lastIndexOf(MCP_TOOL_ERROR_RESPONSE_MARKER), then slices serverPayload from after the last occurrence. The only bail-out is responseMarker <= callMarker. So when the third-party payload itself contains that English phrase, the split lands inside the payload and everything before it — including the text that actually differs between failures — is discarded. The producer confirms the entrance space is unbounded: buildMcpToolError (mcp-tool.ts:871-876) interpolates safeJsonStringify(rawResponseParts) verbatim after that separator, so server-controlled text decides where the fingerprint boundary falls. Three genuinely different failures whose payloads share a trailing … with response: 500 reduce to one signature, and the always-on guard halts the turn at round 3.

Corner (b) — false negative. Verified, and the ordering is real rather than hypothetical. The image branch (mcp-tool.ts:858-869) builds the message from getDisplayFromParts(truncatedContent.parts) where truncatedContent = await this.truncateTextParts(imageContent), so an oversized part arrives as a truncation envelope carrying a per-call artifact path. normalizeToolErrorText (:722-724) returns the MCP branch before stripPersistenceEnvelope ever runs — const mcpNormalized = …; if (mcpNormalized !== null) return mcpNormalized; and only then const enveloped = stripPersistenceEnvelope(error). That per-call path therefore sits inside serverPayload and is fingerprinted verbatim, so an identical repeated MCP failure never accumulates.

Fix direction (not applied): let the producer own the boundary — buildMcpToolError appends a line-anchored FULL_OUTPUT_DIGEST_LABEL line digesting the stable server payload only, and normalizeMcpToolError returns the ${PERSISTED_STUB_DIGEST_PREFIX}<digest> form when that line is present, failing closed to "not an MCP payload" when it is absent instead of searching a mid-string English phrase with lastIndexOf. Both constraints the finding names check out against the tree: the rendered message is pinned by mcp-tool.test.ts, and the new producer line must be the only anchored label in the message or the R4-2 class (PRRT_kwDOPB-92c6fsJkH) reopens.

Why nothing lands this round: +1562/−45, over the 1500-addition scope fuse; replies-only round. This fix reaches a third production file plus a pinned test assertion — category ④ (beyond the PR's promise) and category ① (it is a contract decision about what an MCP error's identity is) → human-gated. Thread stays unresolved, and so does its unresolved sibling PRRT_kwDOPB-92c6fTKig.

[qwen-code-ci-bot]

[Critical] R1-1: The structural fix landed for the shell failure block, but this class is not closed — two shell producer shapes still reach response.error without ever building a failure core or emitting a digest line, so the fingerprint falls back to rendered text with per-call volatile content in it. All four entrances this thread reported in round 2 are verifiably fixed (multi-line commands, pre-stubbed re-truncation, the long-run advisory on the block path, and the payload: fallback), and each is pinned by a test; what remains is the class claim itself, that any producer shape carrying per-call volatile text defeats the fingerprint. Entrance A is the result.error branch: shouldAppendLongRunHint (shell.ts:2946-2951) gates only on !result.aborted && !isSignalTermination(result.signal) && elapsedMs >= longRunThreshold and does not exclude result.error, and shell.ts:3095-3107 deliberately preserves the hint for exactly that combination, so the model-facing message is result.error.message + '\n\n---\n' + longRunHint — no Process Group PGID: block, no digest line — while buildLongRunningForegroundHint embeds Math.round(elapsedMs / 1000). Every round therefore hashes different digits and the streak resets. Entrance B is the no-output timeout: a timeout takes the aborted branch, where llmContent is the config-derived constant Command timed out after <N>ms before it could complete. There was no output before it timed out., identical for every silently-hanging command, and normalizeToolErrorText has nothing to key on — so three unrelated hung commands collapse into one signature and the third halts the turn reporting a repetition that never happened, with headless exiting 1 and no escape but the interactive dialog's in-session disable. The new comment above this gate asserts the opposite coverage: that only failures become model-facing response.error payloads and only they feed the guard.

Witness:

[probe] Entrance A — real LoopDetectionService + real buildLongRunningForegroundHint, 6 rounds/arm
ARM A error-branch varied-advisory-seconds  {"perRound":[false,false,false,false,false,false],"firedAtRound":null}
ARM B error-branch fixed-advisory-seconds   {"perRound":[false,false,true],"firedAtRound":3}
ARM C error-branch no-advisory              {"perRound":[false,false,true],"firedAtRound":3}
ARM D failure-block PGID+digest varied-adv. {"perRound":[false,false,true],"firedAtRound":3}  <- covered by this PR
payload: "spawn /usr/local/bin/thing EIO\n\n---\nNote: this foreground command ran for 61s. …"
candidate fix (strip the advisory in normalizeToolErrorText): ARM A -> [false,false,true], suite 163 passed

[probe] Entrance B — real ShellTool + real convertToFunctionErrorResponse, three DIFFERENT commands
llmContent identical across 3 different commands: true
error.message identical across 3 different commands: true   (type: execution_timeout)
recordToolErrorBatch per round = [false,false,true]   getLastLoopType() = repeated_tool_error
interleaved-success sequence = [false x12, true]   <- 10 successful rounds between them do not clear it
flip-arm (payload carries the command) = [false,false,false]   flip-control (same command x3) = [false,false,true]

Give the two shapes the same producer-owned identity the failure block now has. For entrance A, either append the anchored failure-core digest line to the result.error message too, or strip a trailing ^Note: this foreground command ran for \d+s\. advisory in normalizeToolErrorText/stripShellBlockVolatiles. For entrance B, build the timeout payload from the same block builder, or include this.params.command in the timeout error.message, so three different hung commands hash differently while the same command hanging three times still fires; excluding EXECUTION_TIMEOUT from the streak the way isSyntheticToolError excludes cancellations is cheaper but loses the genuine repeated-timeout dead end.

Two premises: the \n\n---\n divider before the advisory is a published boundary for other consumers — shell.ts:3108-3113 names firePostToolUseFailureHook, telemetry grouping, SIEM alerting and hook-side error parsers as splitting on it — so a producer-side fix must keep the divider and the advisory in error.message and change only the fingerprint derivation; and the identical timeout template is emitted at two producer sites, shell.ts:1810 (sedEditCancelledResult) and shell.ts:2787-2789 (timeoutSummary), so changing only one leaves the sed-edit timeout and the foreground timeout in two signature namespaces. Please add a case to loopDetectionService.test.ts feeding three rounds of spawn EIO\n\n---\n${buildLongRunningForegroundHint((61 + i) * 1000)} and asserting false, false, true, plus a case driving three rounds of the real producer's timeout payload for three different commands asserting false on all three and then true when the first command's timeout repeats — and remove the identity fix afterwards to confirm both go red.

中文说明

结构化修复已在 shell 失败块上落地，但这一类问题尚未关闭——仍有两种 shell 生产者形态会在从未构建失败核心、也从未输出摘要行的情况下到达 response.error，于是指纹退回到含每次调用易变内容的渲染文本。本线程第 2 轮报告的四个入口均已可验证地修复（多行命令、预持久化后的再截断、块路径上的长耗时提示、payload: 回退），且各自都有测试钉住；剩下的是类别命题本身：任何携带每次调用易变文本的生产者形态都能击穿指纹。入口 A 是 result.error 分支：shouldAppendLongRunHint（shell.ts:2946-2951）只判断 !result.aborted && !isSignalTermination(result.signal) && elapsedMs >= longRunThreshold，并未排除 result.error，而 shell.ts:3095-3107 恰恰刻意为该组合保留提示，因此面向模型的消息是 result.error.message + '\n\n---\n' + longRunHint——既无 Process Group PGID: 块也无摘要行——而 buildLongRunningForegroundHint 内嵌 Math.round(elapsedMs / 1000)。于是每轮哈希的数字都不同，连击被重置。入口 B 是无输出的超时：超时走 aborted 分支，llmContent 是由配置决定的常量 Command timed out after <N>ms before it could complete. There was no output before it timed out.，对每一个静默挂起的命令都相同，normalizeToolErrorText 无从取键——于是三个互不相关的挂起命令坍缩为同一签名，第三个就终止轮次并报告一次从未发生的重复；headless 以 1 退出，除了交互式对话框的会话内禁用之外没有出路。此门上方新增的注释断言了相反的覆盖面：只有失败会成为面向模型的 response.error 载荷，也只有它们喂给守卫。

见证见上方英文段探针输出（入口 A：变化秒数臂 6 轮不触发，而固定秒数/无提示/失败块三臂均在第 3 轮触发，候选修复可翻转且 163 个测试全绿；入口 B：三个不同命令的 llmContent 与 error.message 完全相同，逐轮 [false,false,true]，中间插入 10 个成功轮也不清除，携带命令的翻转臂不触发）。

请给这两种形态与失败块相同的、由生产者掌控的身份。入口 A：要么把锚定的失败核心摘要行也追加到 result.error 消息上，要么在 normalizeToolErrorText/stripShellBlockVolatiles 中剥离结尾的 ^Note: this foreground command ran for \d+s\. 提示。入口 B：用同一个块构建器生成超时载荷，或把 this.params.command 放进超时的 error.message，使三个不同的挂起命令哈希不同、而同一命令挂起三次仍能触发；像 isSyntheticToolError 排除取消那样把 EXECUTION_TIMEOUT 排除在连击之外更省事，但会丢掉真正的重复超时死循环。

两个前提：提示前的 \n\n---\n 分隔线是对外发布的边界——shell.ts:3108-3113 点名 firePostToolUseFailureHook、遥测分组、SIEM 告警与 hook 侧错误解析器都按它切分——因此生产者侧修复必须保留分隔线与提示本身在 error.message 中，只改指纹推导；且同一超时模板在两处生产者输出，shell.ts:1810（sedEditCancelledResult）与 shell.ts:2787-2789（timeoutSummary），只改一处会让 sed 编辑超时与前台超时落在两个签名命名空间。请在 loopDetectionService.test.ts 新增用例：喂入三轮 spawn EIO\n\n---\n${buildLongRunningForegroundHint((61 + i) * 1000)}，断言 false、false、true；再新增一例，用真实生产者的超时载荷对三个不同命令喂三轮断言全部 false，随后重复第一个命令的超时断言 true。完成后移除身份修复，确认两例都变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Verdict: REAL — both remaining entrances verified at head 6439e0921d. These are new claims, not re-mints: the four round-2 entrances and the (28k, 30k] buildStub band are genuinely fixed and I am not reopening them. Nothing lands this round; thread stays unresolved.

Already closed at this gate, for the record: PRRT_kwDOPB-92c6fTKhg (shell.ts:2869) — buildStub shadowing the producer digest in the (28k gate, 30k shell threshold] band — was fixed in 3b1ce40 and is resolved. Adjacent anchors PRRT_kwDOPB-92c6fZFe0 (:2858) and PRRT_kwDOPB-92c6fZFe5 (:2861) were resolved as duplicate re-posts. The still-open thread at this gate is PRRT_kwDOPB-92c6fTKjN (:2862, R3-7 — the signal-termination arm has no test witness), which is a different claim from this one and stays open on its own merits.

Entrance A — verified. shouldAppendLongRunHint (shell.ts:2946-2951) is longRunThreshold !== null && !result.aborted && !isSignalTermination(result.signal) && elapsedMs >= longRunThreshold — it does not exclude result.error. And in executionError (shell.ts:3115-3143) the result.error branch is the second arm, ahead of the failure-block arm: message: result.error.message + (longRunHint ? '\n\n---\n' + longRunHint : ''). That payload carries neither the Process Group PGID: block nor the digest line pushed at :2859-2865, while the hint embeds per-run elapsed seconds. Consumer side: normalizeToolErrorText (:722-733) → not MCP → stripPersistenceEnvelope → no PGID → stripShellBlockVolatiles (:684-690), which early-returns the text unchanged when !text.includes('Process Group PGID:'). The advisory's varying seconds are therefore inside the hashed text and the streak resets every round. The in-diff comment at :2853-2856 ("Only failures become model-facing response.error payloads … and only they feed the error-repetition guard") does assert the opposite coverage. Worth recording that the producer's own comment at :3100-3107 already says this combination is rare (spawn/setup failures typically resolve in <1s) and is preserved for slow-spawn edges — so the population is narrow, but the path is real, not hypothetical.

Entrance B — verified, and this is the sharper half. timeoutSummary (shell.ts:2786-2789) is the config-derived constant Command timed out after ${effectiveTimeout}ms before it could complete. with no command identity, and it is the first arm of executionError (:3115-3121) with type: ToolErrorType.EXECUTION_TIMEOUT; the no-output llmContent is that constant plus ' There was no output before it timed out.' (:2795-2799). It is not filtered out on the way in: extractToolErrors (:777-787) skips only isSyntheticToolError, which at :666-670 matches exactly two things — error === ORPHAN_TOOL_USE_REPAIR_REASON or error.startsWith(CANCELLED_TOOL_ERROR_PREFIX). EXECUTION_TIMEOUT is neither, so the timeout payload feeds the guard. Fingerprint: no MCP prefix, no PGID → text hashed as-is → every silently-hanging command under the same configured timeout produces the identical signature, so three unrelated hung commands trip the threshold and halt a turn that was not a loop. The second producer site is real too: sedEditCancelledResult (shell.ts:1806-1824) emits the same template with type: ToolErrorType.EXECUTION_TIMEOUT, so fixing only the foreground site leaves sed-edit timeouts in a second signature namespace.

Fix direction (not applied): give both shapes producer-owned identity. Entrance A — append the anchored failure-core digest to the result.error message too, or strip a trailing ^Note: this foreground command ran for \d+s\. advisory in normalizeToolErrorText/stripShellBlockVolatiles; the consumer-side strip is the smaller diff and does not touch the published \n\n---\n boundary that :3108-3113 names firePostToolUseFailureHook, telemetry grouping, SIEM alerting and hook-side error parsers as splitting on. Entrance B — build the timeout payload from the same block builder, or include this.params.command in the timeout error.message, at both producer sites. Excluding EXECUTION_TIMEOUT from the streak the way cancellations are excluded is cheaper but loses the genuine repeated-timeout dead end, which is squarely #10887's shape.

Issue tracking: #10887 is this PR's origin issue; #9733 ("loop detection false-positives on verification cycles and kills unattended turns unrecoverably") already tracks the false-positive-halt class that Entrance B is a new instance of. I did not file a separate follow-up: both entrances only exist because this PR adds the always-on guard, so they are PR-scoped defects that this open thread tracks, not standalone defects in main.

Why nothing lands this round: +1562/−45, over the 1500-addition scope fuse this closeout sweep runs under; round 17 with findings still being minted; replies-only. Entrance B is additionally category ① — whether an always-on guard may halt a turn on three unrelated timeouts is a behaviour-contract decision, and the cheaper option trades away real #10887 coverage → human-gated. Thread stays unresolved.

[qwen-code-ci-bot]

[Critical] R5-1: [certifies-falsely] [new-surface] The failure identity this diff introduces hashes only Output:/Error:/Exit Code:/Signal:, so it carries zero command identity — and because the equation happens in the producer, the consumer only ever sees the hash and no consumer-side change can un-collapse it. Every silently-failing shell command that shares an exit code therefore produces the same signature, and the always-on three-strike guard halts a turn of legitimate, unrelated work. result.output is empty for any command that fails without writing to stdout/stderr, and isShellExitError returns true for any exitCode >= 2 and for exit 1 unless the executable is one of grep|rg|diff|test|[|[[ (shell.ts:386-393) — git and cmp are not exempt. So git diff --quiet, git merge-base --is-ancestor …, cmp -s a b, pgrep -f …, command -v jq, nc -z host 5432 and curl -sf <dead-url> (exit 22, -s suppresses stderr) all reduce to the identical core Output: (empty) / Error: (none) / Exit Code: 1 / Signal: (none). Successful rounds neither advance nor reset the streak — deliberately, and pinned by this PR's own test — so three such probes spread across one long productive turn reach the threshold and abort it: headless prints that this is an always-on guard which cannot be disabled via model.skipLoopDetection and exits 1 with no answer, discarding the turn's remaining work; interactive shows a loop-detected confirmation for behaviour that was not a loop. The sharpest instance measured is three git diff --quiet -- <different file> probes, each exit 1 meaning "yes, this file changed" — three informative answers, one signature, turn halted. Before this diff no guard consumed these payloads and the block's Command: line distinguished them. The consumer half of the same defect is that the streak signature carries no tool or call identity and has no lifetime bound inside a turn.

Witness:

[probe] real ShellTool producer (mocked execution service, real crypto/fs) -> real LoopDetectionService
ZZG distinct commands: 3        (git diff --quiet …, cmp -s …, git merge-base --is-ancestor …)
ZZG distinct producer error messages: 3
ZZG distinct failure-core digests: 1     <- the collapse
ZZG fired per round: [false,false,true]   ZZG lastLoopType: repeated_tool_error
ZZD distinct commands: 3 (git diff --quiet -- shell.ts / loopDetectionService.ts / README.md)
ZZD distinct failure-core digests: 1      ZZD fired per round: [false,false,true]
ZZC issue-shape control (git exit 128, varied args): digests 1, fired [false,false,true]  <- the PR's purpose
P5a tool name varies, text identical: [false,false,true]    P5c three silent curl -sf exit 22: [false,false,true]
fix flip (executable folded into the hashed core):
  ZZG distinct failure-core digests: 2  fired [false,false,false]  <- FLIPPED
  ZZC fired [false,false,true]                                   <- the PR's purpose preserved
  ZZD fired [false,false,true]                                   <- STILL FIRES (see below)
baseline: at merge base 101b003f93 recordToolErrorBatch / REPEATED_TOOL_ERROR /
          FULL_OUTPUT_DIGEST_LABEL all have 0 occurrences

Fold the failing command's identity into the hashed core, reusing the reduction isShellExitError already performs — getExitStatusSegment(command) then tokeniseSegment(...)[0] then path.basename(path.win32.basename(ex)).replace(/\.(?:exe|cmd|bat)$/i, '') (shell.ts:406-442) — for example hashing an Executable: <name> line, which can stay out of blockLines if it should not be shown to the model. Issue 10887's case (repeated git exit 128 with varied arguments) still fingerprints identically, while git diff --quiet and cmp -s no longer share a signature. Note the measured limit of the executable-only form: the ZZD arm above shows three git diff --quiet -- <different file> probes all reduce to Executable: git plus the identical silent core and still halt, so covering that case needs the subcommand or first argument token as well, or a consumer-side bound — decaying the streak after N error-free rounds, or requiring the signature on N rounds within a window.

Three premises the fix must respect: loopDetectionService.ts:687 reduces a shell block with .filter((line) => !/^(Command|Directory|Process Group PGID): /.test(line)), so the new core line must not be labelled Command:/Directory:/Process Group PGID: or the no-digest fallback path strips it (use Executable: git); the digest is read by three line-anchored scanners admitting only /^[0-9a-f]{64}$/ after FULL_OUTPUT_DIGEST_LABEL (truncation.ts:34), so the core must stay a plain sha256 hex, and whatever fields are hashed must be present in the raw block because the buildStub / keep='both' / fitText envelopes all reuse that same digest rather than recomputing it; and two tests this diff adds pin the current semantics and must stay green or be changed deliberately — expect(service.recordToolErrorBatch(errorResult(denied, 'round-3'))).toBe(true) (same-tool denials do halt on round 3) and keeps the streak alive across a fully successful round (so any decay window must be wider than one error-free round). Please add a shell.test.ts case asserting lastCreateHashInput differs for two failures with identical Output/Error/Exit Code/Signal but different executables (git diff --quiet vs cmp -s a b, both exitCode 1, output ''), extend the existing expect(lastCreateHashInput).toBe([...].join('\n')) at shell.test.ts:3771 with the new line, and add a loopDetectionService.test.ts case asserting three rounds of different silent exit-1 commands return false while three rounds of the same program with varied args still return true on the third — then remove the executable from the core and confirm they go red.

中文说明

本 diff 引入的失败身份只对 Output:/Error:/Exit Code:/Signal: 做哈希，因此完全不含命令身份——而且这一等价发生在生产者侧，消费端只看得到哈希，任何消费端改动都无法把它们拆开。于是所有共享同一退出码的静默失败 shell 命令都会产生相同签名，而「始终开启」的三振守卫会终止一个由合法且互不相关的工作组成的轮次。任何未向 stdout/stderr 写入内容就失败的命令，其 result.output 为空；而 isShellExitError 对任意 exitCode >= 2 返回 true，对 exit 1 仅在可执行文件属于 grep|rg|diff|test|[|[[（shell.ts:386-393）时豁免——git 与 cmp 都不在豁免之列。因此 git diff --quiet、git merge-base --is-ancestor …、cmp -s a b、pgrep -f …、command -v jq、nc -z host 5432、curl -sf <死链>（exit 22，-s 抑制 stderr）全都归约为同一核心 Output: (empty) / Error: (none) / Exit Code: 1 / Signal: (none)。成功轮次既不推进也不重置连击（这是刻意设计，并由本 PR 自己的测试钉住），所以在一个长而高效的轮次里分散出现的三次此类探测就会达到阈值并终止该轮次：headless 会打印「这是始终开启的守卫，无法通过 model.skipLoopDetection 关闭」并以 1 退出、不给任何答案，丢弃该轮次剩余的工作；交互式则会为并非循环的行为弹出循环确认。实测最尖锐的一例是三次 git diff --quiet -- <不同文件> 探测，每次 exit 1 的含义都是「是，这个文件变了」——三个有信息量的答案、一个签名、轮次被终止。本 diff 之前没有守卫消费这些载荷，且块中的 Command: 行本可区分它们。同一缺陷的消费端一半是：连击签名不含工具或调用身份，且在轮次内没有生命周期上界。

见证见上方英文段探针输出（三个不同命令 → 3 条不同错误消息但 1 个失败核心摘要，逐轮 [false,false,true]；issue 形态对照组仍按预期触发；翻转臂把可执行文件折入核心后不再触发、且 PR 目的保留，但同一可执行文件的 ZZD 臂仍然触发；合并基线上三个符号均为 0 次出现）。

请把失败命令的身份折入被哈希的核心，复用 isShellExitError 已经在做的归约——getExitStatusSegment(command) → tokeniseSegment(...)[0] → path.basename(path.win32.basename(ex)).replace(/\.(?:exe|cmd|bat)$/i, '')（shell.ts:406-442）——例如哈希一行 Executable: <name>；若不希望对模型可见，可以只进摘要而不进 blockLines。issue 10887 的情形（git exit 128 反复失败、参数各异）仍会指纹一致，而 git diff --quiet 与 cmp -s 不再共享签名。注意「仅可执行文件」这一形式的实测边界：上面的 ZZD 臂表明三次 git diff --quiet -- <不同文件> 都会归约为 Executable: git 加同一静默核心并仍然终止轮次，因此要覆盖该情形还需要子命令或第一个参数 token，或者在消费端加上界——在 N 个无错误轮次后衰减连击，或要求同一签名在一个窗口内的 N 轮出现。

修复须尊重三个前提：loopDetectionService.ts:687 用 .filter((line) => !/^(Command|Directory|Process Group PGID): /.test(line)) 归约 shell 块，所以新的核心行不能以 Command:/Directory:/Process Group PGID: 命名，否则无摘要回退路径会把它剥掉（请用 Executable: git）；摘要由三个行锚定扫描器读取，只接受 FULL_OUTPUT_DIGEST_LABEL 之后恰好 /^[0-9a-f]{64}$/（truncation.ts:34），所以核心必须仍是纯 sha256 十六进制，且被哈希的字段必须存在于原始块中，因为 buildStub / keep='both' / fitText 信封都是复用同一摘要而非重算；本 diff 新增的两个测试钉住了当前语义，必须保持绿或被刻意修改——expect(service.recordToolErrorBatch(errorResult(denied, 'round-3'))).toBe(true)（同一工具的拒绝确实在第 3 轮终止）与 keeps the streak alive across a fully successful round（因此任何衰减窗口都必须宽于一个无错误轮次）。请在 shell.test.ts 新增用例，断言 Output/Error/Exit Code/Signal 相同但可执行文件不同的两个失败（git diff --quiet 与 cmp -s a b，均 exitCode 1、output ''）其 lastCreateHashInput 不同；把新行补进 shell.test.ts:3771 既有的 expect(lastCreateHashInput).toBe([...].join('\n'))；并在 loopDetectionService.test.ts 新增一例，断言三轮不同的静默 exit-1 命令返回 false，而同一程序参数各异的三轮仍在第三轮返回 true。随后从核心中移除可执行文件，确认它们变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Verdict: REAL — the cross-command collapse is in the producer at head 6439e0921d, and the consumer cannot undo it. Not fixed this round (scope note at the bottom); leaving this thread unresolved.

What I verified:

• Digest input is exactly the four stable core lines — shell.ts:2828-2833: Output:, Error:, Exit Code:, Signal:. Command: and Directory: live only in blockLines (:2834-2839) and are never hashed. The digest is pushed under the gate isSignalTermination(result.signal) || isShellExitError(this.params.command, result.exitCode) at :2859-2862. Because the equation happens in the producer, the consumer only ever compares hex (loopDetectionService.ts:820-824) — the claim that no consumer-side change can un-collapse it is correct.
• The affected population is wide. isShellExitError (shell.ts:428-443) returns true for any exitCode >= 2, and for exit 1 unless the last segment's executable basename is in EXIT_ONE_IS_NOT_ERROR_COMMANDS (:386-393 = grep, rg, diff, test, [, [[). git, cmp, pgrep, nc, curl are not exempt. For a command that fails without writing anything, result.output is empty and finalError is '(none)' (:2822-2824) — so git diff --quiet, git merge-base --is-ancestor …, cmp -s a b, nc -z host 5432, curl -sf <dead-url> (exit 22) all hash to the identical core Output: (empty) / Error: (none) / Exit Code: 1|22 / Signal: (none).
• Accumulation does not require consecutive failing rounds. checkRepeatedToolError (loopDetectionService.ts:812-845) makes no state change when the round carries no error parts, and the only reset is per-prompt (:1958-1959). Successful rounds neither advance nor reset — documented intent at :806-808 — so three unrelated silent probes spread across one long productive turn reach REPEATED_TOOL_ERROR_THRESHOLD = 3 (:59).
• The guard is always-on: recordToolErrorBatch (:623-627) checks only loopDetected and disabledForSession, and is not behind model.skipLoopDetection. The halt lands as client.ts:3990 return turn plus the dialog described in the R6-1 thread at client.ts:3971.

Minimal fix (not applied): mix a coarse command identity into the digest — the resolved executable basename, which isShellExitError already computes at shell.ts:435-441 (getExitStatusSegment → tokeniseSegment → path.basename, with the .exe|.cmd|.bat strip) — as a fifth entry in stableFailureCoreLines. One production file (shell.ts) plus tests. That stops unrelated commands collapsing while keeping #10887's actual shape accumulating: the reported dead end was the same executable (git, exit 128) retried with varied arguments, so argv[0] is stable across the loop while the full command line is not.

Explicitly not the fix: hashing the whole Command: line. That reintroduces precisely the per-retry volatility the digest was added to remove (the comment at shell.ts:2840-2858, and the multi-line continuation lines the consumer's stripShellBlockVolatiles cannot enumerate — loopDetectionService.ts:683-690).

Why it is not applied this round: the PR is at +1562/−45, over the 1500-addition scope fuse this closeout sweep runs under, and it is in round 17 with findings still being minted. This one is also a design trade-off (false-positive halt vs. false-negative non-accumulation) rather than a mechanical bug, so it needs a maintainer decision — lift the fuse, or split the guard. Thread stays unresolved.

[qwen-code-ci-bot]

[Critical] R5-2: [certifies-falsely] [new-surface] This new content-bearing attribute is not added to SENSITIVE_ATTRIBUTE_KEYS in telemetry/log-to-span-processor.ts:58-65, so the one telemetry sink that scrubs error text and tool args by default exports it verbatim while dropping its siblings error, error_message, function_args, prompt and response_text. logLoopDetected builds its OTel attributes as {...getCommonAttributes(config), ...event} (telemetry/loggers.ts:686), so the event's own error_excerpt property becomes a log-record attribute key named exactly error_excerpt. With a traces endpoint configured and no logs endpoint, createHttpExporters (telemetry/sdk-exporters-http.ts:57-70) bridges every log record into an exported span and sdk-impl.ts:439-443 installs LogToSpanProcessor as the only log processor; log-to-span-processor.ts:180-181 then copies any attribute not in the scrub set, with includeSensitiveSpanAttributes defaulting to false. So a user with trace-only OTLP export who has not opted into sensitive span attributes runs curl -H "Authorization: Bearer sk-live-…" … or psql "postgres://user:pass@host/db", it fails identically three rounds running, the guard fires — and the credential-bearing command line plus the working directory are exported as a span attribute on a span whose function_args and error_message were stripped. That is exactly the leak the scrub list and its test were written to prevent; log-to-span-processor.test.ts:166-190 uses those two keys as its secret-bearing fixtures.

Witness:

[probe] real logLoopDetected -> real OTel LoggerProvider({processors:[LogToSpanProcessor]})
        with the shipped default includeSensitiveSpanAttributes: false
PROBE loop span attributes keys: ['error_excerpt','error_signature','event.name','event.timestamp',
                                 'log.bridge','loop_type','prompt_id','session.id']
PROBE loop span error_excerpt = "Command: curl -H \"Authorization: Bearer sk-live-SECRET-TOKEN\"
  https://api.example.com/v1/deploy\nDirectory: /home/dev/private-project\nOutput: curl: (22) …"
PROBE comparator span attributes = {"safe":"visible","log.bridge":true}
  <- same processor, same run: error_message / function_args / prompt all dropped, so the
     comparator is alive and the scrub is working for everything except this key
flip: adding 'error_excerpt' to the set -> PROBE loop span error_excerpt = undefined,
      error_signature retained

Add 'error_excerpt' to SENSITIVE_ATTRIBUTE_KEYS in telemetry/log-to-span-processor.ts:58, leaving error_signature unscrubbed — it is a 64-hex fingerprint, not content. Note the interaction with the R4-1 thread on loopDetectionService.ts: if that is resolved by dropping error_excerpt entirely this becomes moot, and if the excerpt stays the scrub-list entry is needed regardless, so the two are worth landing together.

Two things to weigh, both measured rather than inferred. The scrub must stay an opt-out and not a deletion — log-to-span-processor.ts:180-181 gates on (this.includeSensitiveSpanAttributes || !SENSITIVE_ATTRIBUTE_KEYS.has(key)), so a user who sets telemetry.includeSensitiveSpanAttributes: true must still receive the excerpt. And for calibration: the same bridge with the same defaults already exports request_text, the full untruncated request contents including this very error payload, emitted by logApiRequest (loggers.ts:437-441) for every non-internal request and likewise absent from the scrub list — that is pre-existing code this diff does not touch, so it is not a finding here, but it does mean the marginal exposure over base is small for a trace-only exporter. Please extend log-to-span-processor.test.ts:166 ('drops sensitive attributes before exporting bridged spans') with error_excerpt: 'Command: curl -H "Authorization: Bearer secret"' in the input record and expect(attrs).not.toHaveProperty('error_excerpt') alongside the existing assertions, mirror it in the opt-in test at :197 so the escape hatch stays pinned, then remove the list entry and confirm the new assertion goes red.

中文说明

这个新的、含内容的属性没有被加入 telemetry/log-to-span-processor.ts:58-65 的 SENSITIVE_ATTRIBUTE_KEYS，因此唯一一个默认会脱敏错误文本与工具参数的遥测出口会原样导出它，同时却丢弃它的同类 error、error_message、function_args、prompt 与 response_text。logLoopDetected 以 {...getCommonAttributes(config), ...event} 构造 OTel 属性（telemetry/loggers.ts:686），所以事件自身的 error_excerpt 属性会成为恰好名为 error_excerpt 的日志记录属性键。当只配置了 traces 端点而没有 logs 端点时，createHttpExporters（telemetry/sdk-exporters-http.ts:57-70）会把每条日志记录桥接成导出的 span，sdk-impl.ts:439-443 把 LogToSpanProcessor 装为唯一的日志处理器；随后 log-to-span-processor.ts:180-181 会复制任何不在脱敏集合中的属性，而 includeSensitiveSpanAttributes 默认为 false。于是：一个只做 trace 导出、且没有选择接收敏感 span 属性的用户，执行 curl -H "Authorization: Bearer sk-live-…" … 或 psql "postgres://user:pass@host/db"，它连续三轮同样失败，守卫触发——携带凭据的命令行与工作目录就被作为 span 属性导出，而同一个 span 上的 function_args 与 error_message 却已被剥除。这正是脱敏列表及其测试要防的泄露；log-to-span-processor.test.ts:166-190 正是用这两个键作为含密文的夹具。

见证见上方英文段探针输出（真实 logLoopDetected → 真实 OTel LoggerProvider，出厂默认下桥接 span 的属性键含 error_excerpt，其值就是含 Authorization: Bearer sk-live-SECRET-TOKEN 的命令行与 /home/dev/private-project 目录；同一次运行的对照 span 表明 error_message/function_args/prompt 都已被丢弃，即脱敏机制正常工作、只漏了这个键；把该键加入集合后翻转为 undefined 且 error_signature 保留）。

请把 'error_excerpt' 加入 telemetry/log-to-span-processor.ts:58 的 SENSITIVE_ATTRIBUTE_KEYS，error_signature 保持不脱敏——它是 64 位十六进制指纹，不是内容。注意与 loopDetectionService.ts 上 R4-1 线程的关联：若那边以彻底移除 error_excerpt 收尾，本条即失去意义；若摘要保留，则无论如何都需要这个脱敏项，因此两者值得一起落地。

两点权衡，均为实测而非推断。其一，脱敏必须保持为「可选择退出」而非删除——log-to-span-processor.ts:180-181 的条件是 (this.includeSensitiveSpanAttributes || !SENSITIVE_ATTRIBUTE_KEYS.has(key))，所以设置了 telemetry.includeSensitiveSpanAttributes: true 的用户仍必须收到该摘要。其二，作为量级参照：同一桥接在同样默认下已经导出 request_text，即完整未截断的请求内容，其中就包含这段错误载荷；它由 logApiRequest（loggers.ts:437-441）为每个非内部请求发出，同样不在脱敏列表中——那是本 diff 未触及的既有代码，因此此处不作为发现，但确实意味着对只做 trace 导出的用户而言，相对基线的增量暴露很小。请扩展 log-to-span-processor.test.ts:166（'drops sensitive attributes before exporting bridged spans'），在输入记录中加入 error_excerpt: 'Command: curl -H "Authorization: Bearer secret"'，并在既有断言旁加上 expect(attrs).not.toHaveProperty('error_excerpt')；同时在 :197 的选择加入用例中镜像一份以钉住该逃生口；随后移除列表项并确认新断言变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Verdict: REAL — error_excerpt is absent from the scrub set at head 6439e0921d. Not fixed this round (scope note at the bottom); leaving this thread unresolved.

Chain, as read:

• telemetry/types.ts:496-527 — LoopDetectedEvent gains error_signature?: string and error_excerpt?: string, assigned in the constructor from details.
• telemetry/loggers.ts:677-698 — logLoopDetected builds attributes = { ...getCommonAttributes(config), ...event }, so the emitted log record carries an attribute keyed literally error_excerpt.
• telemetry/log-to-span-processor.ts:58-65 — SENSITIVE_ATTRIBUTE_KEYS = { 'error', 'error.message', 'error_message', 'prompt', 'function_args', 'response_text' }. No error_excerpt.
• log-to-span-processor.ts:174-187 — the copy loop keeps an attribute when this.includeSensitiveSpanAttributes || !SENSITIVE_ATTRIBUTE_KEYS.has(key). With the default false, error_excerpt is copied while its content-bearing siblings error_message, function_args, response_text are dropped.
• telemetry/sdk-exporters-http.ts:52-71 — the bridge is constructed in the else if (tracesUrl) arm, i.e. whenever a traces endpoint is configured and no logs endpoint is, and it is handed options.logToSpan.includeSensitiveSpanAttributes; telemetry/config.test.ts:159-162 pins that option's default to false.

What the payload contains makes this more than a consistency nit: loopDetectionService.ts:834 passes errorExcerpt: raw — the un-normalized functionResponse.response.error, not the fingerprint text — and for shell failures that raw block leads with Command: <the command line> and Directory: <cwd> (shell.ts:2834-2839), which is where inline credentials sit. LoopDetectedEvent truncates but does not redact.

Minimal fix (not applied): add 'error_excerpt' to SENSITIVE_ATTRIBUTE_KEYS (log-to-span-processor.ts:58-65) — one file, one line — putting it behind the same includeSensitiveSpanAttributes opt-in as error_message/function_args. (error_signature is a sha256 hex of the fingerprint text and can stay exported.)

Scope boundary worth stating: that one line closes only the OTel log-to-span bridge. Whether the excerpt should be redacted at the source is a separate decision — the same field is uploaded to the RUM endpoint at telemetry/qwen-logger/qwen-logger.ts:736-753 (properties.error_excerpt), which no span-attribute scrub set reaches. That is the subject of the sibling unresolved thread at loopDetectionService.ts:834 (R4-1), and it needs a maintainer call on redaction policy rather than a key list.

Why it is not applied this round: the PR is at +1562/−45, over the 1500-addition scope fuse this closeout sweep runs under, and it is in round 17 with findings still being minted. Thread stays unresolved.

[qwen-code-ci-bot]

[Suggestion] R3-2: The AgentCore half of this guard's runtime wiring is still exercised by no test — carried from round 3 and re-derived independently three times this round. A grep over packages/core/src/agents/** for recordToolErrorBatch, REPEATED_TOOL_ERROR and repeated_tool_error returns exactly one hit, this line itself; the diff adds tests only for client.ts and for the service in isolation. The call sits behind if (terminateMode !== AgentTerminateMode.LOOP_DETECTED) immediately after a for loop that breaks with terminateMode already set, and its input is a flatMap over toolCallResult.results, so any reorder of that branch, an earlier break/return added to the round, or processFunctionCalls ceasing to return every executed result silently deletes the guard from the subagent runtime with the whole suite green. The cost lands on the runtime nobody is watching: a subagent that hits the reported dead end keeps re-running it to its round or time budget, burning tokens with no user in the loop and no repeated_tool_error attribution in the subagent journal. Equally unpinned is the batch shape — moving this feed inside the per-result loop directly above would make one parallel batch of three identical failures trip the threshold before the model has seen any error, and no runtime-level test would notice. The harness already exists: agent-headless.test.ts pins the sibling result-aware guard from issue 9450 at :2245, :2333 and :2414.

Witness:

witness: not run — grep over packages/core/src/agents for recordToolErrorBatch /
REPEATED_TOOL_ERROR / repeated_tool_error returns 1 hit (agent-core.ts:1253, the source
line); no test file matches. The mutation-survival claim is read from the call graph.

Add two tests to agent-headless.test.ts in the issue-9450 harness: one driving three consecutive rounds whose tool results carry a byte-identical functionResponse.response.error and asserting scope.getTerminateMode() is AgentTerminateMode.LOOP_DETECTED with the finish event's loopType equal to 'repeated_tool_error'; and one driving a single round carrying three sibling calls with the same error, asserting no halt, then two more such rounds, asserting the halt.

Two premises: the guard is deliberately always-on, unlike the heuristic tier in the same file — !this.runtimeContext.getSkipLoopDetection() && gates only loopDetector.addAndCheckHeuristicLoops(event) (agent-core.ts:941-942) and recordToolErrorBatch returns early only on this.loopDetected / this.disabledForSession (loopDetectionService.ts:623-627) — so a test that sets getSkipLoopDetection() true and still expects the halt is asserting intended behaviour, not a bug; and the duplicate-provider-call path breaks out before this guard runs (agent-core.ts:1225), so a fixture reusing the duplicate-call shape from agent-headless.test.ts:2169 never reaches recordToolErrorBatch and would assert a halt for the wrong reason. The acceptance criterion is that deleting this block turns the first test red on both the terminate mode and the loopType assertion, and moving the feed into the per-result loop turns the second red on round 1 instead of round 3, while a varying-error negative control stays green — please apply both mutations and confirm.

中文说明

该守卫运行时接线的 AgentCore 一半仍然没有任何测试覆盖——本条承接第 3 轮，并在本轮被独立地重新发现三次。在 packages/core/src/agents/** 中检索 recordToolErrorBatch、REPEATED_TOOL_ERROR 与 repeated_tool_error，只有一处命中，即本行自身；本 diff 只为 client.ts 与该服务的孤立单元测试新增了用例。该调用位于 if (terminateMode !== AgentTerminateMode.LOOP_DETECTED) 之后、紧随一个会以已置位的 terminateMode 执行 break 的 for 循环，其输入是对 toolCallResult.results 的 flatMap；因此该分支的任何重排、轮次中提前加入的 break/return、或 processFunctionCalls 不再返回每个已执行结果，都会在全套测试保持绿色的情况下悄悄把守卫从子 agent 运行时中删除。代价落在无人观察的运行时上：命中所述死循环的子 agent 会一直重跑到其轮次或时间预算，持续消耗 token，既无用户介入，子 agent 日志中也没有 repeated_tool_error 归因。批次形态同样未被钉住——把这个喂入移进紧邻上方的逐结果循环，会让一次含三个相同失败的并行批次在模型看到任何错误之前就触发阈值，而运行时层面的测试不会察觉。测试骨架已存在：agent-headless.test.ts 在 :2245、:2333、:2414 钉住了 issue 9450 的同类结果感知守卫。

见证见上方英文段（未执行：检索仅 1 处命中即源码行本身，无测试文件匹配；变异存活结论来自调用图阅读）。

请在 agent-headless.test.ts 的 issue-9450 骨架中新增两个用例：其一，驱动连续三轮、工具结果携带字节一致的 functionResponse.response.error，断言 scope.getTerminateMode() 为 AgentTerminateMode.LOOP_DETECTED 且结束事件的 loopType 等于 'repeated_tool_error'；其二，驱动一轮携带三个同类调用（同一错误），断言不终止，再驱动两轮同样的批次，断言终止。

两个前提：与同文件中的启发式层级不同，该守卫是刻意「始终开启」的——!this.runtimeContext.getSkipLoopDetection() && 只约束 loopDetector.addAndCheckHeuristicLoops(event)（agent-core.ts:941-942），而 recordToolErrorBatch 仅在 this.loopDetected / this.disabledForSession 时提前返回（loopDetectionService.ts:623-627）——因此把 getSkipLoopDetection() 设为 true 仍期望终止的测试，断言的是预期行为而非缺陷；另外重复的 provider 调用路径会在该守卫运行前 break（agent-core.ts:1225），所以复用 agent-headless.test.ts:2169 的重复调用形态的夹具永远到不了 recordToolErrorBatch，会因错误的原因断言终止。验收标准是：删除该代码块后第一个用例在终止模式与 loopType 两个断言上都变红；把喂入移进逐结果循环后第二个用例在第 1 轮而非第 3 轮变红；而变化错误的反向对照保持绿。请分别施加这两种变异并确认。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[qwen-code-ci-bot]

[Suggestion] R3-4: The telemetry evidence chain added for this guard is still asserted by zero tests, at both ends — carried from round 3 and re-derived twice this round. At the sink, logLoopDetectedEvent has no test at all: qwen-logger.test.ts never constructs a LoopDetectedEvent, and its only loop_detected references at :405/:419 are a SubagentExecutionEvent's terminate_reason. At the event, the entire new describe('Repeated tool-error detection') block makes zero assertions on the mocked logLoopDetected, although sibling guards in the same file routinely pin it at :249, :331, :2334, :2562, :2600, :2644 and :2719. So deleting both spreads here leaves the suite green while RUM exception events keep carrying only prompt_id/error_type and the oncall evidence these fields were added for never arrives, with nothing detecting the loss; and dropping the third argument of new LoopDetectedEvent(LoopType.REPEATED_TOOL_ERROR, this.promptId, {...}) at loopDetectionService.ts:832-835, or swapping errorExcerpt: raw for the normalized text, likewise leaves all ~1150 new test lines green — loggers.test.ts:560-590 still passes because it constructs the event from literals — while pages arrive with a bare loop type, or with the opaque <persisted-stub>sha256:<hex> instead of readable failure text. Logging on every round rather than only the firing round, or logging twice, is equally invisible.

Witness:

witness: not run — grep over loopDetectionService.test.ts finds no errorSignature /
errorExcerpt / error_signature / error_excerpt match and no logLoopDetected match at or
after the new block's start (:3316); grep over packages/ for logLoopDetectedEvent matches
only loggers.ts:683, telemetry/types.ts, qwen-logger.ts and the new loggers.test.ts block.
qwen-logger.test.ts never constructs a LoopDetectedEvent.

Add the two missing assertions. In the firing test of loopDetectionService.test.ts, assert loggers.logLoopDetected was called exactly once with an event whose loop_type is 'repeated_tool_error', whose error_signature is the 64-hex sha256 of the normalized payload, and whose error_excerpt is the leading 200 chars of the raw error. And add a logLoopDetectedEvent case to qwen-logger.test.ts mirroring journals the loop detector attribution on subagent loop stops (:399): a REPEATED_TOOL_ERROR event with both fields asserting properties: expect.objectContaining({ error_signature: …, error_excerpt: … }), and an event constructed without details asserting expect.not.objectContaining({ error_signature: expect.anything() }).

Two premises: telemetry/types.ts:532 applies const cut = details.errorExcerpt.slice(0, 200); and drops a lone trailing high surrogate at :533-535, so an assertion written against a stub or shell-block payload must expect the 200-char cut rather than the full raw string — the first test's 68-char gitError survives whole, which is why it is the right place for the assertion; and qwen-logger.test.ts:399-401 states the convention this fix follows, that a loop stop must stay attributable in the journal (issue 9450 requirement 7). The acceptance criterion is that removing the details argument at loopDetectionService.ts:832-835, or moving the logLoopDetected call out of the threshold branch, reds the first assertion, and deleting either spread here reds the second — please apply those mutations and confirm.

中文说明

为该守卫新增的遥测证据链在两端仍然零测试断言——本条承接第 3 轮，本轮又被重新发现两次。出口端：logLoopDetectedEvent 完全没有测试，qwen-logger.test.ts 从未构造 LoopDetectedEvent，其中 :405/:419 仅有的 loop_detected 引用是某个 SubagentExecutionEvent 的 terminate_reason。事件端：新增的整个 describe('Repeated tool-error detection') 块对被 mock 的 logLoopDetected 做了零断言，而同文件中同类守卫在 :249、:331、:2334、:2562、:2600、:2644、:2719 都惯例性地钉住了它。因此删除此处两个展开式后测试套件仍绿，而 RUM 异常事件将继续只带 prompt_id/error_type，为这些字段而加的 oncall 证据永不到达，且没有任何东西能发现这一损失；同样地，删掉 loopDetectionService.ts:832-835 中 new LoopDetectedEvent(LoopType.REPEATED_TOOL_ERROR, this.promptId, {...}) 的第三个参数，或把 errorExcerpt: raw 换成归一化文本，也会让约 1150 行新测试全部保持绿——loggers.test.ts:560-590 依然通过，因为它用字面量构造事件——而 page 到达时只有一个裸的循环类型，或是不可读的 <persisted-stub>sha256:<hex> 而非可读的失败文本。每轮都记录（而非仅在触发轮记录）、或记录两次，同样不可见。

见证见上方英文段（未执行：对 loopDetectionService.test.ts 与 packages/ 的检索结果表明新块之后无任何相关断言，且 qwen-logger.test.ts 从未构造该事件）。

请补上这两处缺失断言。在 loopDetectionService.test.ts 的触发用例中，断言 loggers.logLoopDetected 恰好被调用一次，且事件 loop_type 为 'repeated_tool_error'、error_signature 为归一化载荷的 64 位十六进制 sha256、error_excerpt 为原始错误的前 200 字符。并在 qwen-logger.test.ts 中仿照 journals the loop detector attribution on subagent loop stops（:399）新增一个 logLoopDetectedEvent 用例：带两个字段的事件断言 properties: expect.objectContaining({ error_signature: …, error_excerpt: … })，不带 details 构造的事件断言 expect.not.objectContaining({ error_signature: expect.anything() })。

两个前提：telemetry/types.ts:532 会执行 const cut = details.errorExcerpt.slice(0, 200);，并在 :533-535 丢弃结尾孤立的高代理项，因此针对 stub 或 shell 块载荷写的断言必须期望 200 字符截断值而非完整原始串——第一个用例中 68 字符的 gitError 会完整保留，这也是它适合放该断言的原因；qwen-logger.test.ts:399-401 陈述了本修复遵循的约定，即循环终止必须在日志中保持可归因（issue 9450 需求 7）。验收标准是：移除 loopDetectionService.ts:832-835 的 details 参数、或把 logLoopDetected 调用移出阈值分支，会使第一处断言变红；删除此处任一展开式会使第二处变红。请施加这些变异并确认。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[qwen-code-ci-bot]

[Suggestion] R3-7: The signal-termination arm of this digest gate still has no test witness — carried from round 3 and re-derived independently by two reverse-audit passes this round. The two cases shell.test.ts adds resolve { exitCode: 1, error: null } (the exit-code arm) and { exitCode: 0 } (the success arm); nothing resolves a signal termination. So removing isSignalTermination(result.signal) || from this gate leaves every assertion in that file passing, and signal-killed failures — the OOM-kill and SIGTERM-from-container-shutdown shapes the adjacent comment itself enumerates — then ship without a producer digest. The consumer's fallback cannot cover them: stripShellBlockVolatiles drops only whole lines starting with Command: , Directory: or Process Group PGID: , so for a multi-line command the continuation lines survive, each retry of an OOM-killed multi-line build fingerprints uniquely, the streak never reaches 3, and the guard silently stops covering the retry-an-OOM-killed-command loop with no test to catch the regression.

Witness:

witness: not run — read of the two new shell.test.ts cases' resolved execution results
against this gate predicate: neither resolves a non-null `signal`. Re-derived by the
round-1 and round-2 chunk-6 reverse auditors independently.

Add a shell.test.ts case resolving { output: '', exitCode: null, signal: 'SIGKILL', error: null } and asserting both that result.llmContent contains Full output sha256: ${FAKE_BLOCK_DIGEST} and that lastCreateHashInput equals the four-line stable core.

The premise this must not break: the gate has to stay a superset of the error-payload gate at shell.ts:3130-3131 ((!result.aborted && isSignalTermination(result.signal)) || isShellExitError(...)), because that coincidence is what guarantees every shell failure reaching response.error carries a digest — narrowing either one independently reopens a digest-less shape. The acceptance criterion is that deleting isSignalTermination(result.signal) || from this gate removes the digest line from a signal-killed block and turns the new test red; please apply that mutation and confirm.

中文说明

该摘要门的信号终止分支仍然没有测试见证——本条承接第 3 轮，本轮又被两次反向审计独立地重新发现。shell.test.ts 新增的两个用例分别解析 { exitCode: 1, error: null }（退出码分支）与 { exitCode: 0 }（成功分支），没有任何用例解析信号终止。因此从该门中移除 isSignalTermination(result.signal) || 后，该文件的所有断言仍然通过，而被信号杀死的失败——即邻近注释自己列举的 OOM kill 与容器关闭时 SIGTERM 形态——就会在没有生产者摘要的情况下发布。消费端的回退无法覆盖它们：stripShellBlockVolatiles 只删除以 Command: 、Directory: 或 Process Group PGID: 开头的整行，因此对多行命令其续行会保留，OOM 杀死的多次多行构建重试每次指纹都不同，连击永远到不了 3，守卫便悄无声息地不再覆盖「重试被 OOM 杀死的命令」这一循环，且没有测试能捕获该回归。

见证见上方英文段（未执行：将两个新用例解析出的执行结果与该门谓词对照阅读，二者都未解析出非 null 的 signal；第 1 轮与第 2 轮的分块 6 反向审计各自独立重新得出同一结论）。

请在 shell.test.ts 新增一个解析 { output: '', exitCode: null, signal: 'SIGKILL', error: null } 的用例，同时断言 result.llmContent 含 Full output sha256: ${FAKE_BLOCK_DIGEST}，且 lastCreateHashInput 等于那四行稳定核心。

不可破坏的前提：该门必须保持为 shell.ts:3130-3131 处错误载荷门（(!result.aborted && isSignalTermination(result.signal)) || isShellExitError(...)）的超集，因为正是这一重合保证了每个到达 response.error 的 shell 失败都带有摘要——单独收窄任一侧都会重新打开一个无摘要的形态。验收标准是：从该门删除 isSignalTermination(result.signal) || 会使被信号杀死的块失去摘要行，并让新用例变红。请施加该变异并确认。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[qwen-code-ci-bot]

[Suggestion] R3-6: No test pins that this digest line survives the shell's real truncateToolOutput — carried from round 3 and still standing. The new comment eight lines above asserts the property ("truncateToolOutput's keep='both' tail retention preserves it through truncation"), and every test that exercises a truncated shape hand-builds the envelope instead of calling the producer. This round measured what that costs on the sibling marker: rewording truncation.ts:202 alone made the real keep-both envelope stop firing the guard while loopDetectionService.test.ts stayed at 163 passed, because the fixtures carry a hand copy of the producer's wording and so match a stale constant. The same blind spot covers this line's position — it is the block's last line, and keep='both' retains a head slice plus a tail slice, so whether it survives depends on the real head/tail budgets (truncation.ts:97-186) and on previewChars = Math.min(4000, outputThreshold), none of which any test drives. A future change to those budgets that drops the digest out of the retained tail would leave the whole suite green while every truncated shell failure fingerprints on its per-call envelope again — the exact regression this digest was added to prevent.

Witness:

[probe] sibling measurement, real producer driven through the guard (scratch tree)
INTACT  A2 REAL keep-both, no digest => {"fired":true, "perRound":[false,false,true]}
        loopDetectionService.test.ts => 163 passed
MUTANT  truncation.ts:202 reworded, marker constant untouched
        A1_real_has_marker=false   <- mutation landed
        A2 REAL keep-both, no digest => {"fired":false,"perRound":[false x5]}
        loopDetectionService.test.ts => 163 passed   <- STILL GREEN
witness for this line specifically: not run — no test drives the real truncateToolOutput,
so the digest's survival through the retained tail is asserted by comment only.

Add one case that drives the real producer end to end: take a shell failure block above the shell output threshold through the real truncateAndSaveToFile with keep: 'both' — the pattern loopDetectionService.test.ts:3903-3947 already uses with vi.importActual — then feed the resulting text to recordToolErrorBatch for three rounds with varied Command:/PGID and assert the third returns true.

The premise is that the digest line must keep the shape extractAnchoredStubDigest recognizes — FULL_OUTPUT_DIGEST_LABEL = 'Full output sha256: ' (truncation.ts:34) starting a line, followed by exactly 64 hex chars ending the line — and that keep='both' takes whole lines from the end (truncation.ts:162-183), so the fix must not rely on a partial tail slice. The acceptance criterion is that the new test goes red when the digest line is moved off the block's last position or when the keep='both' tail budget is shortened past it, and stays green for the shapes the hand-built fixtures model; please apply both mutations and confirm.

中文说明

没有任何测试钉住这行摘要能在 shell 真实的 truncateToolOutput 之后存活——本条承接第 3 轮，仍然存在。上方八行的新注释断言了该性质（「truncateToolOutput 的 keep='both' 尾部保留会使其在截断后得以保留」），而所有涉及截断形态的测试都是手工构造信封，而非调用生产者。本轮实测了这种做法在同类标记上的代价：仅改写 truncation.ts:202，就让真实的 keep-both 信封不再触发守卫，而 loopDetectionService.test.ts 仍是 163 通过——因为夹具携带的是生产者措辞的手工副本，于是与过期常量相匹配。同一盲点覆盖这行的位置：它是块的最后一行，而 keep='both' 保留的是头部切片加尾部切片，因此它能否存活取决于真实的头/尾预算（truncation.ts:97-186）以及 previewChars = Math.min(4000, outputThreshold)，而这些都没有任何测试驱动。未来对这些预算的改动若把摘要挤出被保留的尾部，整套测试仍会全绿，而每个被截断的 shell 失败又会按每次调用的信封重新指纹——正是这个摘要被引入以防止的回归。

见证见上方英文段（同类实测：仅改写 truncation.ts:202 时，真实 keep-both 信封从 [false,false,true] 变为不触发，而测试文件仍 163 全绿；针对本行的见证为未执行——没有测试驱动真实的 truncateToolOutput，因此摘要在保留尾部中的存活仅由注释断言）。

请新增一个端到端驱动真实生产者的用例：把一个超过 shell 输出阈值的失败块，用真实的 truncateAndSaveToFile（keep: 'both'）处理——即 loopDetectionService.test.ts:3903-3947 已用 vi.importActual 采用的模式——然后把结果文本喂给 recordToolErrorBatch 三轮、Command:/PGID 各异，断言第三轮返回 true。

前提是摘要行必须保持 extractAnchoredStubDigest 能识别的形状——FULL_OUTPUT_DIGEST_LABEL = 'Full output sha256: '（truncation.ts:34）位于行首，其后恰好 64 个十六进制字符并以行尾结束——且 keep='both' 是从末尾按整行取用（truncation.ts:162-183），所以修复不能依赖不完整的尾部切片。验收标准是：把摘要行移出块末位置、或把 keep='both' 尾部预算缩短到越过它时，新用例变红；而对手工夹具所建模的形态保持绿。请施加这两种变异并确认。

— qwen3.8-max via Qwen Code /review (v0.23.0)

[yiliang114]

Verification pass at head 6439e09 — no code changes

This pass independently re-verified the 8 outstanding review findings instead of adding more code. The PR is at +1562, over the 1500-line cap this patrol operates under, and it has been through four review rounds that kept minting new findings — so the loop was stopped deliberately and the findings were checked against the source instead.

Result: all 8 are real at this head. One needs a human decision on privacy policy; two are functional defects in the guard itself; five are unpinned invariants / missing test witnesses. Nothing was resolved, because nothing was refuted outright.

Needs a decision (blocking):

• loopDetectionService.ts:834 — the new error_excerpt telemetry field uploads the raw tool-error text, unredacted, to the default-on usage-statistics RUM endpoint. For shell failures that text begins with Command: <full command line> (shell.ts:2834-2839), so inline credentials land in the first 200 chars. types.ts:532-536 truncates only. Two framing corrections are in the thread: the endpoint is Aliyun RUM (first-party, not third-party), and the same raw block already reaches it on main via error_message — that wider pre-existing half is now Usage-statistics telemetry uploads raw tool-error text (including shell command lines) to RUM without redaction #11198. error_excerpt itself is new in this PR, so the choice here is (a) drop the field and keep error_signature, (b) pass normalized instead of raw, or (c) route through truncateSpanError (insufficient alone — it only redacts URL-embedded credentials).

Functional defects in the guard (not test gaps):

• truncation.ts:574 — the digest reuse added here (extractAnchoredFullDigest(content) ?? hash(content), plus the same in fitText) trusts a line-anchored Full output sha256: line anywhere in the content, including one quoted by peer content. main hashed unconditionally, so this PR is what drops the "stub digest identifies the full content" invariant. Consumer side gates on shell shape (Process Group PGID:), producer side does not.
• truncation.ts:574 — oversized MCP tool errors past the scheduler gate get fingerprinted on the sha256 of the full message, which embeds the args a dead-end loop varies per retry, and the stub loses the MCP tool ' prefix so normalizeMcpToolError never runs. The guard silently does not fire in that band.

Missing test witnesses (each verified by grep, exact test + assertion written in its thread):

• agent-core.ts:1253 — the AgentCore/headless half of the wiring has no test anywhere under packages/core/src/agents/**; deleting the block ships green.
• shell.ts:2862 — the isSignalTermination(result.signal) arm of the digest gate has no witness; the only digest test uses exitCode: 1 with no signal.
• shell.test.ts:3765 — no test drives the real truncateToolOutput keep='both' path; every truncation test spies the module, so digest survival through truncation is prose-only.
• loopDetectionService.ts:73 — the MCP error-message contract is mirrored as private literals here and in tests, while this same diff does export CANCELLED_TOOL_ERROR_PREFIX for cancellations. One contract test through the real buildMcpToolError closes it; exporting the fragments is optional.
• qwen-logger.ts:746 — partly stale: loggers.test.ts:564-589 now pins the 200-char cut and surrogate trim, so "deleting the slice ships green" no longer holds. Still live: no test asserts the fire-site field mapping, and no test asserts this sink passthrough.

No commits, no pushes, and no /review or /triage triggered from this pass — the head already has review-pr running, and another review round is what keeps adding findings here.

[wenshao]

Partially reviewed — gaps disclosed.

⚠️ Round 6, and the diff has grown 4.5x since this review first measured it (192 → 872 source diff lines); the reverse audit also stopped at its round cap without converging. The findings below are anchored to the current patch, so they can only say where this approach leaks — never that a different approach would retire all of them at once. Before fixing them, a human should decide whether the shape of the change is still right. Advisory only: this does not affect the verdict, and nothing here is a blocker.

1 candidate finding(s) this round's reviewers re-derived matched entries already carried on this PR and were set aside before verification (R3-2) — a matched posted finding is ruled in the previous-round status as always, and a matched deferral stays on the standing deferral record.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and did not run locally; this diff changes shell signal and exit-code handling and the PR's own Tested-on table marks Windows untested.

Not reviewed: test-efficacy — the probe kit could not validate its own harness (harnessValidated null: the positive control never ran, because the probe tree's restore deletes the built dist that vitest's prerequisite guard requires), so nothing is licensed in either direction about whether the ~1150 new test lines would go red without the guard.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": I did not execute packages/core vitest for loopDetectionService.test.ts , loggers.test.ts or shell.test.ts — the three new-test analyses above are hand-t….

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.

Test Plan (not a blocker): src/services/loopDetectionService.test.ts — no such file or directory; Tests 150 passed — this review observed 28826, 23581, 1949, 298, 1818, 509, 6324 passed; 3 passed — this review observed 28826, 23581, 1949, 298, 1818, 509, 6324 passed.

Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:

• packages/core/src/core/client.ts:3971 — [probe] D6-1 teammate-merged rounds go out as SendMessageType.Teammate, so their tool errors are neither fed to the guard nor allowed to accumulate
• packages/core/src/services/loopDetectionService.test.ts:3784 — [probe] D6-2 the keep-both fixtures hand-copy the producer's envelope wording, so marker drift between the two modules is invisible to this suite
• packages/core/src/services/loopDetectionService.ts:666 — [probe] D6-3 a third fabricated-notice family (agent-core capability/policy denials) is admitted as tool-failure evidence and can terminate a productive subagent run
• packages/core/src/services/loopDetectionService.ts:687 — [probe] D6-4 volatile text inside the retained Output:/Error: lines defeats the digest, so varied-argument and noisy-output dead ends never accumulate
• packages/core/src/services/loopDetectionService.ts:719 — [probe] D6-5 the MCP reducer returns before stripPersistenceEnvelope, so a per-call artifact path inside the server payload enters the fingerprint
• packages/core/src/services/loopDetectionService.ts:821 — [probe] D6-6 the streak has no decay window, so identical intermittent errors 592 rounds apart halt a productive turn
• packages/core/src/services/loopDetectionService.ts:830 — [probe] D6-7 no local debug artifact says which error repeated when the guard fires, unlike the chanting guard it is named as the exception to
• packages/core/src/services/loopDetectionService.ts:832 — [probe] D6-8 nothing asserts the guard populates its own telemetry evidence (producer half of the R3-4 chain)
• packages/core/src/telemetry/qwen-logger/qwen-logger.ts:747 — [probe] D6-9 error_excerpt bypasses the telemetry.logPrompts content gate its only sibling free-text error field honours
• packages/core/src/tools/tool-response-finalizer.ts:270 — [probe] D6-10 no test distinguishes the fitText digest's value from its mere presence (a value-blind mutation survives 190/190)
• packages/core/src/tools/tool-response-finalizer.ts:273 — [probe] D6-11 the 85-char digest line widens fitText's degenerate band, returning a chopped digest and zero content preview
• packages/core/src/agents/runtime/agent-core.ts:1253 — [review] R3-2 the AgentCore half of this guard's runtime wiring is still exercised by no test anywhere
• packages/core/src/telemetry/qwen-logger/qwen-logger.ts:744 — [probe] R3-4 the telemetry evidence chain added for this guard is still asserted by zero tests at the sink end
• packages/core/src/tools/shell.ts:2861 — [review] R3-7 the signal-termination arm of this digest gate still has no test witness
• packages/core/src/tools/shell.ts:2864 — [review] R3-6 no test pins that this digest line survives the shell's real truncateToolOutput keep='both' pass

Convergence: round 6 posted 2 inline comment(s), 2 of them reported for the first time. Findings keep coming back to the same files: packages/core/src/services/loopDetectionService.ts (findings in rounds 3, 4; 1 more now). (Evidence: the previous round was recovered from a marker this account did not post, so those rounds may not be this account's own.) A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)

Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)

中文说明

仅完成部分审查，审查缺口已披露。

⚠️ 第 6 轮，且自本审查首次测量以来 diff 已增长 4.5 倍（源码 diff 行数 192 → 872）；反向审计也在轮数上限处停止且未收敛。下方的发现都锚定在当前这版补丁上，因此它们只能指出这个方案在哪里漏了，而无法说明换一个方案就能一次性消除全部问题。在动手修复之前，应由人来判断这次改动的整体形态是否仍然正确。仅供参考：本段不影响判定结论，其中也没有任何阻断项。

本轮评审重新推导出的 1 条候选发现与本 PR 已携带的条目匹配，已在验证前搁置（R3-2）——被匹配的已发布条目照常在上一轮状态区裁定，被匹配的延后条目仍保留在延后清单记录中。

未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

未审查（原文为英文）：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and did not run locally; this diff changes shell signal and exit-code handling and the PR's own Tested-on table marks Windows untested.

未审查（原文为英文）：test-efficacy — the probe kit could not validate its own harness (harnessValidated null: the positive control never ran, because the probe tree's restore deletes the built dist that vitest's prerequisite guard requires), so nothing is licensed in either direction about whether the ~1150 new test lines would go red without the guard.

未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：I did not execute packages/core vitest for loopDetectionService.test.ts , loggers.test.ts or shell.test.ts — the three new-test analyses above are hand-t…。

未审查：反向审计——在 5 轮的反审轮数上限内未收敛。

Test Plan（非阻断）：src/services/loopDetectionService.test.ts — no such file or directory; Tests 150 passed — this review observed 28826, 23581, 1949, 298, 1818, 509, 6324 passed; 3 passed — this review observed 28826, 23581, 1949, 298, 1818, 509, 6324 passed。

收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 15 条（原文未翻译，列表见上方英文部分）。

收敛情况：第 6 轮发布了 2 条行内评论，其中 2 条是首次提出。发现反复回到同一批文件：packages/core/src/services/loopDetectionService.ts（第 3、4 轮已出过发现，本轮又有 1 条）。（证据说明：上一轮的数据来自并非本账号发布的标记，上述轮次可能不属于本账号。）一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）

机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）

— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.23.0)

[wenshao]

[Critical] R6-1: [certifies-falsely] [new-surface] This halt returns the turn before the round's tool results are written to chat history, so the model's functionCall turn is left unpaired and the next send's orphan repair tells the model that results which really arrived were lost to a crash.

turn.run(model, requestToSend, signal) at client.ts:4087 is the only place this round's content enters history (llm-chat.ts:2997-2998, "Add user content to history ONCE before any attempts"), and the shared halt block below this feed ends in return turn. So on the round the guard trips, history ends with model[functionCall] and no answering user[functionResponse].

Nothing compensates on the interactive path. The TUI already called markToolsAsSubmitted before submitting (use-llm-stream.ts:5479), and its LoopDetected handler only opens a dialog (:2450-2477) — unlike every other non-submitting branch, which writes the pairing explicitly for exactly this reason (:5377-5381: "unless the responses are written now the model's function calls stay unanswered and the next /goal resume sends a history with an unpaired call"; also :5406, :5485, :5635, :5663). The dialog's disable arm then tells the user "Loop detection has been disabled for this session. Please try your request again.", and that next send runs repairOrphanedToolUseTurns (llm-chat.ts:3011-3014), synthesizing Tool execution result was not recorded — likely interrupted by network failure, abort, or process exit. Treat as failure and retry if needed. (llm-chat.ts:1526-1528). The model is therefore told that a call which did execute — and whose result the user watched land on screen — was lost to a crash and should be retried, at the precise moment the user has disabled every guard (disableForSession is honored by the always-on tier too, loopDetectionService.ts:625-628). The dead-end retry this halt exists to stop becomes unbounded, and the real result is absent from context.

This is newly reachable rather than merely pre-existing: at the merge base the only detector that could halt at this pre-send point was recordToolResultByCallId, which needs a requestByCallId entry populated only for STATEFUL_READ_TOOLS (task_list), so an ordinary run_shell_command error round could not reach it. One caveat in fairness — the invariant violation itself is not brand new: a frozen task_list board tripping GLOBAL_TOOL_CALL_DUPLICATE reached the same return turn at base. This diff widens it from one stateful-read polling path to every tool-error round. The agent-runtime arm is a different harm: agent-core.ts:1253-1255 breaks before currentMessages = toolCallResult.messages at :1260, but that run terminates, so there is no next send and no fabricated repair — there the cost is dropped productive work. Headless also exits 1 on the halt, so the falsified-history harm needs a session that survives it: the interactive TUI, or a --resume of the persisted transcript.

Witness:

probe — the PR's own client.test.ts harness, with mockTurnRunFn delegating to the REAL
LlmChat.sendMessageStream (so the history push and the per-send orphan repair are the
product's own code, not a model), real LoopDetectionService, args varied per round.
Comparator validated: rounds 0-2 show turnRunCalled=true AND the real functionResponse
landing in history, so it can report a difference.

PR arm (unmodified):
  PROBE_ROUND 3 turnRunCalled=false chatSendCount=3 events=["loop_detected"]
  PROBE_HISTORY after_round_3 len=7
    [...,{"role":"model","parts":["functionCall(run_shell_command#fail-2)"]}]  <- ends UNPAIRED
  PROBE_FUNCTION_RESPONSES_IN_HISTORY [..., {"id":"fail-2","name":"run_shell_command",
    "response":{"error":"Tool execution result was not recorded — likely interrupted by
    network failure, abort, or process exit. Treat as failure and retry if needed."}}]

Mutant arm (loopHalt = false in place of recordToolErrorBatch — models the merge base):
  PROBE_HALT round=-1 loopType=undefined
  PROBE_FUNCTION_RESPONSES_IN_HISTORY [fail-0..fail-4 all =
    {"error":"fatal: not a git repository (or any of the parent directories): .git"}]
    <- no fabricated entry for any executed round

base-axis read at merge base 101b003f93: recordToolErrorBatch and REPEATED_TOOL_ERROR
have zero grep hits; requestByCallId is populated only under
`if (event.value.callId && stateful)`.

Fix direction: on the pre-send loopHalt path, pair the unanswered calls with the real results instead of dropping them — this.getChat().addHistory({ role: 'user', parts: toolResultParts }) before return turn, mirroring use-llm-stream.ts:5406 — and in agent-core.ts assign currentMessages = toolCallResult.messages (or append the finalized parts to the run's history) before the break at :1257. If the intent is that the model must not see round 3's error, write a truthful synthetic response for those callIds rather than leaving the pair dangling, so the repair's "not recorded / interrupted by network failure, abort, or process exit" text is never injected for a call that executed.

The pairing write must fire only when turn.run is skipped: llm-chat.ts:2997-2998 pushes the user content once before any attempts, so a write on a path that also sends would duplicate the round's functionResponses — the hazard restoreStrippedRetryEntries gates on the push counter for at client.ts:3113-3143.

Please extend the new halt test in client.test.ts (helper runFailingToolTurns, :8489-8546) with a history assertion — after the LoopDetected event, client.getChat().getHistory() contains a role: 'user' entry whose functionResponse.id === 'fail-2', equivalently repairOrphanedToolUseTurns(history) returns injected: [] — then remove the history write and confirm that test goes red. Both tests as added stay green either way today, which is the gap.

中文说明

R6-1：[certifies-falsely] [new-surface] 这个终止路径在本轮工具结果写入对话历史之前就 return turn 了，导致模型的 functionCall 轮次没有配对；下一次发送时的 orphan 修复会告诉模型，那些确实已经返回的结果「因崩溃/中断而丢失，请重试」。

client.ts:4087 的 turn.run(model, requestToSend, signal) 是本轮内容进入历史的唯一入口（llm-chat.ts:2997-2998，注释为「Add user content to history ONCE before any attempts」），而这个喂入点下方的共享终止块以 return turn 结束。因此守卫触发的那一轮，历史会以 model[functionCall] 结尾，没有配对的 user[functionResponse]。

交互路径上没有任何补偿：TUI 在提交前已经调用了 markToolsAsSubmitted（use-llm-stream.ts:5479），而它的 LoopDetected 处理只弹一个对话框（:2450-2477）——与其他所有「不提交」分支不同，那些分支都显式写入了配对，理由正是这个（:5377-5381：「unless the responses are written now the model's function calls stay unanswered and the next /goal resume sends a history with an unpaired call」；另见 :5406、:5485、:5635、:5663）。对话框的「禁用」分支接着告诉用户「Loop detection has been disabled for this session. Please try your request again.」，而下一次发送会执行 repairOrphanedToolUseTurns（llm-chat.ts:3011-3014），合成出「Tool execution result was not recorded — likely interrupted by network failure, abort, or process exit. Treat as failure and retry if needed.」（llm-chat.ts:1526-1528）。于是模型被告知：一个确实执行过（而且用户已在屏幕上看到结果）的调用因崩溃丢失、应当重试——而此时用户刚好禁用了所有守卫（disableForSession 对 always-on 层同样生效，loopDetectionService.ts:625-628）。这个终止本要阻止的死循环重试变成无界的，真实结果也不在上下文里。

这是新引入的可达路径，不只是既有问题：在 merge base 上，唯一能在该「发送前」位置终止的检测是 recordToolResultByCallId，它需要 requestByCallId 中存在条目，而该映射只为 STATEFUL_READ_TOOLS（task_list）填充，所以普通的 run_shell_command 错误轮根本到不了这里。公平起见也要说明一点：这个不变式被破坏本身并非全新——base 上一个冻结的 task_list 看板触发 GLOBAL_TOOL_CALL_DUPLICATE 也会走到同一个 return turn；本 diff 把它从「一条 stateful-read 轮询路径」扩大到「每一个工具错误轮」。agent 运行时那半边是另一种损害：agent-core.ts:1253-1255 在 :1260 的 currentMessages = toolCallResult.messages 之前 break，但那次 run 随即终止，不存在「下一次发送」，也就没有伪造修复——那里的代价是被丢弃的已完成工作。headless 在终止时直接 exit 1，所以「历史被伪造」这一损害需要一个在终止后仍存活的会话：交互式 TUI，或对已持久化 transcript 执行 --resume。

修复方向：在发送前的 loopHalt 路径上，用真实结果去配对那些未被应答的调用，而不是直接丢弃——在 return turn 之前执行 this.getChat().addHistory({ role: 'user', parts: toolResultParts })（参照 use-llm-stream.ts:5406）；在 agent-core.ts 中，于 :1257 的 break 之前赋值 currentMessages = toolCallResult.messages（或把 finalize 后的 parts 追加到该 run 的历史）。如果设计意图就是「不让模型看到第 3 轮的错误」，也请为这些 callId 写入如实的合成响应，而不是让配对悬空，从而避免把「not recorded / interrupted by network failure, abort, or process exit」这段文字注入到一个确实执行过的调用上。

约束：llm-chat.ts:2997-2998 是「任何 attempt 之前只写入一次用户内容」，因此配对写入必须只在 turn.run 被跳过时发生；在同时也会发送的路径上写入会重复本轮的 functionResponse——这正是 client.ts:3113-3143 中 restoreStrippedRetryEntries 用 push 计数器去防范的问题。

请在 client.test.ts 新增的终止用例（辅助函数 runFailingToolTurns，:8489-8546）里补一条历史断言：LoopDetected 事件之后，client.getChat().getHistory() 中存在一个 role: 'user' 条目，其 functionResponse.id === 'fail-2'（等价地，repairOrphanedToolUseTurns(history) 返回 injected: []）；然后移除该历史写入，确认这个测试变红。今天这两个新增用例无论有没有该写入都是绿的，这正是缺口所在。

— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.23.0)

[yiliang114]

Verdict: REAL — traced end to end at head 6439e0921d. Not fixed this round (scope note at the bottom); leaving this thread unresolved.

Trigger path I read:

• client.ts:3971 — loopHalt = this.loopDetector.recordToolErrorBatch(toolResultParts), inside the SendMessageType.ToolResult branch; toolResultParts is every functionResponse part of that message, collected in the loop above it (the same loop that feeds recordToolResultByCallId at :3957).
• The shared halt block is client.ts:3973-3991 and ends in return turn; at :3990.
• requestToSend only enters history inside turn.run(model, requestToSend, signal) at client.ts:4087 → llm-chat.ts:2997-2998 (// Add user content to history ONCE before any attempts. / this.history.push(userContent);). Returning at :3990 skips it, so history ends on model[functionCall] with no answering user[functionResponse].
• Nothing on the interactive path compensates. use-llm-stream.ts:2476-2481 (handleLoopDetectedEvent) only sets loopDetectionConfirmationRequest; handleLoopDetectionConfirmation (:2450-2473) just addItems text, and its disable arm prints "Loop detection has been disabled for this session. Please try your request again." Contrast the cancelled-batch branch at use-llm-stream.ts:5380-5382, which calls llmClient.addHistory({ role: 'user', parts: responsesToSend }) for exactly this reason — see the comment at :5373-5379 ("unless the responses are written now the model's function calls stay unanswered and the next /goal resume sends a history with an unpaired call"). markToolsAsSubmitted already ran at :5479, so those callIds are never re-submitted.
• The next send then runs the inline repair at llm-chat.ts:3011-3014, injecting ORPHAN_TOOL_USE_REPAIR_REASON (llm-chat.ts:1526-1528): "Tool execution result was not recorded — likely interrupted by network failure, abort, or process exit. Treat as failure and retry if needed." So the model is told results that did arrive were lost to a crash, and is invited to retry the very calls the guard just halted on — with loop detection now disabled for the session if the user picked that arm.
• Same shape in the subagent runtime: agent-core.ts:1250-1253 feeds the round, sets LOOP_DETECTED at :1254, and breaks at :1258 — before currentMessages = toolCallResult.messages at :1260.

One correction to the [new-surface] tag, because it changes who owns the fix: the return turn before the history push is not new. origin/main client.ts:3953-3975 returns from the identical place for the recordToolResultByCallId (#9450) detection. What this PR changes is the reach — #9450 only fires for stateful-read tools (task_list), while recordToolErrorBatch is always-on (loopDetectionService.ts:623-627 checks only loopDetected and disabledForSession; it is not behind model.skipLoopDetection) and fires on any tool error repeating three rounds. A narrow pre-existing edge becomes a routine path, so fixing it here is right.

Minimal fix (not applied): write the pairing before returning — in the halt block at client.ts:3973-3991, push this round's parts into history before return turn, e.g. this.getChat().addHistory(createUserContent(toolResultParts)) (getChat().addHistory is the receiver already used at client.ts:670-671, createUserContent is imported at :16), and mirror it in the agent-core.ts:1253 twin by assigning currentMessages = toolCallResult.messages before the break. Two production files plus tests. Whoever implements it should also decide whether the pre-send microcompactHistoryBeforeSend call at the end of the same branch needs to see that content, since the write now happens without a send.

Why it is not applied this round: the PR is at +1562/−45, over the 1500-addition scope fuse this closeout sweep runs under, and it is in round 17 with findings still being minted. This needs an owner decision — lift the fuse for a targeted fix, or split the guard out. A reply alone does not close a real Critical, so the thread stays unresolved.

[wenshao]

[Critical] R6-2: [certifies-falsely] [new-surface] The streak is a single last-signature slot, so a round carrying two distinct error signatures overwrites its own accumulated evidence and the guard never fires on a mixed-error dead end — contradicting the contract this method documents for itself.

seen collapses duplicate signatures within a round, but each distinct signature then overwrites the same slot, so a round ending on B destroys A's accumulated count. Both production feed sites deliver a whole parallel batch as one round — client.ts:3971 passes every functionResponse part of one ToolResult message, agent-core.ts:1250-1253 flat-maps toolCallResult.results — so a batch failing two ways is one round with two signatures.

Concretely: a dead end whose parallel batch fails the same two ways every round (npm run build → A and npm test → B, retried verbatim) runs unbounded. Round 1 ends (A,1); round 2 gives A→(A,2) then B→(B,1); round 3 gives A→(A,1) then B→(B,1). The count never reaches REPEATED_TOOL_ERROR_THRESHOLD = 3. The likelier variant is a repeating failure A plus one stray different sibling error in some rounds (a 5-call batch where one call hits a unique transient error), which resets A's accumulated streak to 1 — so the token burn this PR exists to stop continues undetected while the code reports no loop. Part order decides whether evidence survives at all.

That contradicts the method's own contract eight lines up (:793-794, "halts the turn when the same error signature returns on REPEATED_TOOL_ERROR_THRESHOLD consecutive rounds") and the comment at :803 ("the streak advances at most once per distinct signature per round"): A does not advance at all. Rounds 4 and 5 of this PR both recorded this shape as a non-blocking deferral; this round measured it end to end and the harm is a withheld detection on the exact workload the guard was added for, so it is filed as a blocker.

Witness:

probe at HEAD (real LoopDetectionService, real recordToolErrorBatch):
  B1 rounds [A,B] x6            -> [false,false,false,false,false,false] loopType= null
  B2 rounds [B,A] x6            -> [false,false,false,false,false,false]
  B4 [A] then [A,B] x5          -> [false,false,false,false,false,false]
  B5 [A],[A,B],[A],[A],[A],[A]  -> [false,false,false,false,true,true]
  control B3 rounds [A] x6      -> [false,false,true,true,true,true] loopType= repeated_tool_error

flip (per-signature Map round counter, pruning only on rounds that carried errors):
  B1 -> [false,false,true,true,true,true]
  B2 -> [false,false,true,true,true,true]
  B4 -> [false,false,true,true,true,true]
  B3 unchanged, and all 163 tests of the PR's own loopDetectionService.test.ts still pass

second independent probe (different verifier, different harness — a fabricated
non-failure payload riding in the firing round):
  PROBE-B3 order=real-first      [false,false,true]  repeated_tool_error
  PROBE-B3 order=duplicate-first [false,false,false] null   <- streak reset from 3 to 1

Fix direction: replace the two scalars with a per-signature round counter advanced at most once per round — build the round's distinct signature set first (the seen Set already does), delete map entries absent from a round that carried errors, increment each present signature once, and fire when any count reaches REPEATED_TOOL_ERROR_THRESHOLD, keeping first-occurrence order for the telemetry excerpt.

Three existing pins must survive, and they pull in different directions: does not halt below the threshold and restarts the streak on a different error (:3428-3439; errA, errA, errB, errA, errA → every call false, getLastLoopType() null) means a round whose only signature differs must still restart at 1; keeps the streak alive across a fully successful round (:3993-4003; err, success, err, err → fourth call true) means an error-free round must not clear counts; and counts sibling calls of one parallel batch as ONE round, not as retries (:3372-3395) means four byte-identical siblings in one batch must still return false. New state must also be cleared in reset() at :1958-1959, or a signature's round count survives across prompts and fires on the resumed turn's first failing round.

Please add a case feeding the same two-distinct-error batch for three consecutive rounds and asserting recordToolErrorBatch returns true on the third with getLastLoopType() === LoopType.REPEATED_TOOL_ERROR — it is red today — then revert to the single slot and confirm it goes red again.

中文说明

R6-2：[certifies-falsely] [new-surface] 连击状态只有「最后一个签名」一个槽位，因此一轮里携带两个不同错误签名时，它会覆盖掉自己已累积的证据，混合型错误的死循环永远不会触发守卫——这与该方法自己写明的契约相矛盾。

seen 只在同一轮内折叠重复签名，但每个不同签名随后都会覆盖同一个槽位，所以一轮以 B 结束就销毁了 A 已累积的计数。两个生产喂入点都把整个并行批次作为「一轮」交付——client.ts:3971 传入一条 ToolResult 消息里的每个 functionResponse part，agent-core.ts:1250-1253 对 toolCallResult.results 做 flatMap——因此「一个批次以两种方式失败」就是「一轮里有两个签名」。

具体地：一个每轮都以同样两种方式失败的并行批次死循环（npm run build → A，npm test → B，原样重试）会无界运行。第 1 轮结束于 (A,1)；第 2 轮 A→(A,2) 然后 B→(B,1)；第 3 轮 A→(A,1) 然后 B→(B,1)。计数永远到不了 REPEATED_TOOL_ERROR_THRESHOLD = 3。更常见的变体是：持续失败的 A 加上某些轮里一个偶发的不同兄弟错误（5 个调用的批次中有一个碰到唯一的瞬时错误），这会把 A 已累积的连击重置为 1——于是本 PR 要阻止的 token 燃烧继续发生且无人察觉，而代码报告「没有循环」。part 的顺序直接决定证据能否留存。

这与八行之上该方法自己的契约矛盾（:793-794：「halts the turn when the same error signature returns on REPEATED_TOOL_ERROR_THRESHOLD consecutive rounds」），也与 :803 的注释矛盾（「the streak advances at most once per distinct signature per round」）：A 根本没有推进。本 PR 的第 4、5 轮都把这个形态记为非阻断的延后项；本轮做了端到端实测，其损害是「在守卫正是为之而加的工作负载上漏检」，因此按阻断项提出。

修复方向：把这两个标量换成「按签名的轮次计数器」，每轮最多推进一次——先构造本轮的不同签名集合（seen 已经做了），在携带错误的轮次里删除本轮未出现的 map 条目，对出现的每个签名各加一，任一计数达到 REPEATED_TOOL_ERROR_THRESHOLD 即触发，并保留首次出现顺序以供遥测摘要使用。

三条既有断言必须同时保住，而它们方向不同：does not halt below the threshold and restarts the streak on a different error（:3428-3439；errA, errA, errB, errA, errA → 每次调用都是 false，getLastLoopType() 为 null）要求「本轮只有一个且不同的签名」时仍须重置为 1；keeps the streak alive across a fully successful round（:3993-4003；err, success, err, err → 第 4 次调用为 true）要求无错误的轮次不得清空计数；counts sibling calls of one parallel batch as ONE round, not as retries（:3372-3395）要求同一批次内 4 个逐字节相同的兄弟错误仍返回 false。新状态还必须在 reset()（:1958-1959）中清空，否则某个签名的轮次计数会跨 prompt 存活，并在恢复后的本轮第一个失败轮就触发。

请补充用例：连续三轮喂入同一个「两个不同错误」的批次，断言第三次 recordToolErrorBatch 返回 true 且 getLastLoopType() === LoopType.REPEATED_TOOL_ERROR——今天它是红的；然后改回单槽位实现，确认它再次变红。

— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.23.0)

[yiliang114]

Verdict: REAL — confirmed at head 6439e0921d. Not fixed this round (scope note at the bottom); leaving this thread unresolved.

What the code does:

• The streak state is a single last-signature slot: loopDetectionService.ts:437-438 — private toolErrorStreakSignature: string | null = null; / private toolErrorStreakCount = 0;.
• checkRepeatedToolError (:812-845): seen (:814, :818-819) collapses duplicate signatures within the round, but every distinct signature falls into the else at :823-824 and overwrites the slot with count = 1. The threshold test at :826 therefore only ever observes the count of the round's last distinct signature, in first-occurrence (= part) order.
• Both production feed sites deliver a whole parallel batch as one round: client.ts:3971 passes every functionResponse part of one ToolResult message, and agent-core.ts:1250-1253 flat-maps toolCallResult.results.
• Nothing restores a destroyed count: the only reset is per-prompt, :1958-1959 inside reset().

So the reported arithmetic holds verbatim. A dead end whose batch fails the same two ways every round (A then B): round 1 → (A,1) then (B,1); round 2 → (B,1) again; the slot never carries a count above 1 and REPEATED_TOOL_ERROR_THRESHOLD = 3 (:59) is unreachable. The likelier variant is worse for detection: a repeating failure A plus one stray sibling error in some rounds resets A to 1 on those rounds. And which signature owns the slot at round end is decided by part order, so the guard's outcome is order-dependent on a payload the provider assembles.

This also contradicts the contract in the method's own doc comment at :801-805 — "the streak advances at most once per distinct signature per round, in first-occurrence order". A single last-signature counter cannot advance per distinct signature; only the last one survives the round. Either the state or the comment is wrong, and the state is what ships.

Minimal fix (not applied): replace the two scalars with a per-signature streak map — Map<string, number>, advanced once per distinct signature per round (keep the seen collapse so N siblings stay one piece of evidence), fire when any entry reaches the threshold, and preserve the documented "successful results neither advance nor reset" semantics (:806-808). One production file: fields :437-438, checkRepeatedToolError :812-845, reset :1958-1959 — plus tests for the mixed-batch and stray-sibling shapes. The one-line alternative (record only the round's first distinct signature) removes the order-dependence but silently discards evidence for every other failure in the batch, so I would not take it.

Why it is not applied this round: the PR is at +1562/−45, over the 1500-addition scope fuse this closeout sweep runs under, and it is in round 17 with findings still being minted. Owner decision needed — lift the fuse or split the PR. Thread stays unresolved.

[doudouOUC]

Review — head 6439e092, base main (draft)

+1 562 / −45 across 16 files. Author is a repository admin, so the two-tier core gate does not block. This PR has been through roughly sixty review rounds, so per AGENTS.md I am reporting Criticals only and deferring everything else. No local test run; all three findings are verified by reading the code at this exact SHA.

Critical 1 — the error-repetition guard cannot fire when a round carries more than one distinct error

The streak is tracked in a single signature slot:

for (const { raw, normalized } of errors) {
  const signature = createHash('sha256').update(normalized).digest('hex');
  if (seen.has(signature)) continue;
  seen.add(signature);
  if (this.toolErrorStreakSignature === signature) {
    this.toolErrorStreakCount++;
  } else {
    this.toolErrorStreakSignature = signature;
    this.toolErrorStreakCount = 1;
  }
  if (this.toolErrorStreakCount >= REPEATED_TOOL_ERROR_THRESHOLD) { ... }
}

REPEATED_TOOL_ERROR_THRESHOLD is 3. Work the loop for a round carrying two distinct errors A and B: A sets the slot to A with count 1; B does not match the slot, so it overwrites to B with count 1. Next round, same two errors: A overwrites to A/1, B overwrites to B/1. The counter is pinned at 1 forever. It cannot reach 3, so the guard can never fire.

This is not an edge case — it is the normal shape of a stuck agent, which typically retries a small set of failing calls together rather than one call in isolation. The seen set correctly collapses identical siblings into one piece of evidence per round, which is what the doc comment describes; but distinct siblings evict each other, which the comment does not anticipate. The feature is inert for exactly the workload it targets.

A per-signature counter map (keyed by signature, incremented once per round, with entries dropped when a signature is absent from a round if a reset is wanted) would restore the intended semantics. This corroborates @wenshao's unresolved thread on this PR from the same angle.

Critical 2 — the loop-detected event carries the full command line and working directory to telemetry, unredacted

extractToolErrors keeps the raw payload, and the halt logs it verbatim:

errors.push({ raw: error, normalized: ... });
...
logLoopDetected(this.config, new LoopDetectedEvent(LoopType.REPEATED_TOOL_ERROR, this.promptId, {
  errorSignature: signature,
  errorExcerpt: raw,
}));

raw is functionResponse.response.error. For the shell tool on the exit-error path, that message is llmContent, which is blockLines.join('\n') — and blockLines opens with:

`Command: ${this.params.command}`,
`Directory: ${this.params.directory || '(root)'}`,

So the user's full command line (which routinely carries hostnames, tokens, file paths, and secrets passed as arguments) and their working directory go into the event.

Both sinks receive it. logLoopDetected sends to QwenLogger unless explicitly disabled — options.recordToQwenLogger !== false, and the call site passes no options, so it is on by default — and then spreads the whole event into OTel log attributes:

const attributes: LogAttributes = { ...getCommonAttributes(config), ...event };

I looked for a redaction step and there is none applicable: the telemetry layer's only protection here is a size cap (DEFAULT_SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH), not a key allowlist or denylist, so there is nothing that would strip these fields.

Also worth correcting in passing: the comment above the call describes "the (truncated) raw excerpt," but raw is passed whole. Either truncate it as the comment claims, or — better — derive the excerpt from normalized, or from stableFailureCoreLines only, which deliberately excludes command and directory.

Critical 3 — halting discards tool results whose side effects already happened

On the halt path in client.ts, return turn fires before the tool-result memory consumption and before the history write further down. The tool calls in that batch have already executed — files written, commands run — but their results never reach the conversation history. The next turn therefore has no record that the work was done, and an agent may repeat destructive operations.

To be fair about attribution: this return turn shape is pre-existing; the diff refactors the duplicate-count guard's early return into a shared loopHalt branch rather than introducing it. What the diff does change is the trigger. The new error-repetition guard can now discard a batch in which some calls succeeded, on the strength of other calls in the same batch having failed repeatedly — the duplicate-count guard it shares the exit with could not do that. So the exposure genuinely widens here even though the mechanism is inherited. Writing the executed results to history before returning would close it for both guards.

Deferred

One further point I looked at and am deliberately not raising as a Critical, so it does not add a round: stableFailureCoreLines omits command identity, so two unrelated commands that fail with identical output, error text, exit code, and signal collapse to one signature — which, once Critical 1 is fixed, becomes a false-positive halt risk. But the block comment states this exclusion is deliberate and gives its reasoning (a dead-end loop varies the command by definition, and multi-line commands defeat the consumer's line filter). That is a considered trade-off, not an oversight, so it belongs in a follow-up discussion rather than in this PR's remaining rounds. Recording it here so it is not silently dropped.

Verdict

C=3. Critical 1 is the one that matters most: as written, the feature this PR exists to add does not activate on the case it was built for, so the other two are about a code path that currently cannot be reached — but they need to be right before it becomes reachable. Limitation: no local test run.