feat(cli): report retained tool-result stats in /doctor memory - #8875
Conversation
Add a tool-result retention section to /doctor memory (and --json) covering the phase-1 diagnostics from #4184: retained tool-result count/total/largest, oversized flagging against the 30k widest legal per-tool budget, plus duplication signals for UI history and compression input. Sizes and counts only (never content); history is scanned by reference so the diagnostic adds no memory pressure.
E2E test reportRuntime: Scenario 1 — fresh session. Scenario 2 — large shell output (mitigation active). Agent ran Scenario 3 — multiple tool calls, raised global threshold. With Scenario 4 — "toolResultRetention": {
"toolResultCount": 0,
"totalChars": 0,
"largestResultChars": 0,
"oversizedResultCount": 0,
"oversizedThresholdChars": 30000,
"largeOutputsInUIHistory": 0,
"presentInCompressionInput": false
}Oversized "yes" branch. Unreachable in normal operation (per-tool/global layers bound every retained result at or below the 30k diagnostic threshold); covered deterministically by unit tests in both packages ( |
|
Re-run against Thanks for the PR! Template looks good ✓ — all sections present, bilingual, with a concrete reviewer test plan and before/after evidence. Problem: observed and documented. This implements the phase-1 diagnostics requested by #4184 (open, Direction: aligned. Size: core paths are touched. 321 production-logic lines (doctorCommand 148, tool-result-retention 145, truncation 18, core index 5, coreToolScheduler 5), 672 test lines, 291 doc lines (design note + E2E report — both are #4184 acceptance criteria, not scope creep). Below the 500-line maintainer-awareness bar; Approach: the scope feels right. One pure core analyzer that walks the live history by reference, wired additively into the readable and Risk: no high-risk path matches; no elevated risk signals. Moving on to code review. 🔍 中文说明针对 感谢贡献！ 模板完整 ✓ —— 各节齐全、中英双语，含具体评审验证方案与 before/after 证据。 问题：已观测、有记录。本 PR 实现 #4184（open， 方向：对齐。 规模：触及 core 路径。生产逻辑 321 行（doctorCommand 148、tool-result-retention 145、truncation 18、core index 5、coreToolScheduler 5），测试 672 行，文档 291 行（设计说明 + E2E 报告——两者都是 #4184 的验收标准，不属于范围蔓延）。低于 500 行维护者关注线； 方案：范围合理。一个按引用遍历会话历史的纯 core 分析函数，增量接入可读与 风险：未命中高风险路径；无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent proposal for this problem: a pure core function that walks the live session history by reference, measures each The PR matches this shape and goes beyond it in the places that matter:
No critical findings at this head. Eight review rounds have mined this diff thoroughly — round 8 left only Suggestion-level items, all doc/label-wording or test-load-bearing nits:
None of these block merge; they fold naturally into a follow-up or the next touch of these files. Test evidence — the PR's own CI, read via the API (PR code never executed in this run)CI on the reviewed head is fully settled — zero pending
Not verified here: live TUI rendering of the retention section — this is an unattended run, so no local tmux drive was attempted. The live-behavior claim is not resting on the author's word alone, though: @wenshao independently built the PR and exercised all four reviewer scenarios against a real model session (verdict posted up-thread), and the oversized branches are pinned by 19 core + 12 CLI retention unit tests. Sandboxed verification would settle the remainder A/B against the base build: 中文说明代码审查：我的独立方案是——纯 core 函数按引用遍历会话历史，用压缩管线自己的尺寸模型度量每个 测试证据：来自 PR 自身 CI（API 读取；本运行未执行任何 PR 代码）。被评审 head 的 CI 已完全收敛——无待决 未在此验证：retention 段落的真机 TUI 渲染——本次为无人值守运行，不做本地 tmux 驱动。但真机行为主张并非仅凭作者一面之词：@wenshao 已独立本地构建并在真实模型会话中走完全部四个评审场景（结论见上文线程），超限分支另有 core 19 个 + CLI 12 个单测钉住。如需对 base 构建做 A/B 沙箱验证收尾： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean across every stage; the remainder is doc/label wording the round-capped review left behind, and the live-session wiring has a maintainer's independent real-session verification behind it. Stepping back: this is the diagnostics half of an open roadmap issue (#4184), with the mitigation half (#4880) merged in June. The implementation matches my independent proposal and exceeds it where it counts — shared size model with the compression pipeline, per-tool budget calibration, sentinel skips, and an oversized counter that behaves as a regression alarm rather than an everyday signal. Every scary-sounding path degrades safely: no history → section omitted, history read throws → section omitted with a debug trace, truncation disabled ( After eight review rounds the diff has been mined past the point where I'd trust a fresh pair of eyes to find more: round 8's leftovers are Suggestions about doc accuracy and label wording, none of which change behavior. The author answered every item with a fix or a reasoned decline, and the declines are documented in-thread. The one standing process artifact: the earlier CHANGES_REQUESTED review (2026-08-11) objected to the integration suite being skipped on PRs — that is @wenshao's independent real-session verification (up-thread) covers the live-behavior lane, and the sandboxed Approving, pinned to the reviewed commit. ✅ 中文说明置信度：4/5 —— 各阶段均干净；剩余问题是被轮次上限截断的评审留下的文档/措辞类建议，且真机会话接线已有维护者的独立真机验证背书。 退一步看：这是开放 roadmap issue（#4184）的诊断部分，缓解部分（#4880）已于六月合并。实现与我的独立方案一致，并在关键处做得更好——与压缩管线共享尺寸模型、按工具预算校准、哨兵跳过，超限计数器表现为回归警报而非日常噪音。所有听起来危险的路径都安全降级：无历史→整段省略，历史读取抛错→省略并留调试痕迹，截断关闭（Infinity）→不误报， 八轮评审之后，这个 diff 已被挖掘到我不认为新鲜目光还能找出更多的程度：第 8 轮遗留的全部是关于文档准确性与标签措辞的 Suggestion，均不改变行为。作者对每条都给出修复或有理由的拒绝，拒绝项均在线程中留档。唯一遗留的流程产物：此前的 CHANGES_REQUESTED 评审（2026-08-11）质疑 PR 上集成测试被跳过——那是 @wenshao 的独立真机验证（见上文线程）覆盖了真机行为验证通道，沙箱 批准，锚定到被评审的 commit。✅ — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 1/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31414262324 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 2/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31417991439 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — stopped at the 5-round cap with rounds 4 and 5 still surfacing confirmed findings (no dry convergence).
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all planned checks completed within budget..
中文说明
未审查：reverse audit — stopped at the 5-round cap with rounds 4 and 5 still surfacing confirmed findings (no dry convergence)。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — all planned checks completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
🤖 AutoFix stopped: this counting window now contains 3 time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is 3 full agent runs that pushed nothing. 3 of those were silent-sandbox (idle) timeouts that no budget increase can cure — investigate the sandbox image and runner docker daemon for those. A human should investigate the sandbox image and runner docker daemon, then comment What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31421767926 🧠 Handled by Qwen Code · model/模型 |
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停：本 PR 达到轮次上限（100/100）。评论 |
…har sizes Address review round 1: measure retained tool results with the compression pipeline's estimatePartChars (raw chars for string outputs, image estimate for nested media) instead of JSON.stringify, fixing false oversized flags on newline-dense compliant outputs; compare each result against its own tool's declared budget resolved from the tool registry instead of a fixed 30k threshold, so compliant high-budget results (e.g. MCP) are never flagged; scope the UI-history duplication scan to tool_group result displays so model text is excluded and rendered tool outputs are actually detected; omit toolResultRetention from --json when unavailable; log retention collection failures via debug logger; fix the design note's IO-error persistence bullet; re-verify the E2E report with the new measurement basis.
Review round 1 — triage and fixes (ae30398)Thanks for the thorough probe-verified findings. All substantive ones are fixed in ae30398; details below. Fixed
Not adopted
E2E report re-run with the new measurement basis (all scenarios + 中文说明针对第一轮 review 的逐条处理（提交 ae30398）：
未采纳：barrel 导出的独立 import 测试——CLI 测试本就通过包根 barrel 导入该函数，删除导出会直接挂构建与测试，额外存在性测试无增量信号。 E2E 报告已按新测量口径全部重跑并更新。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget..
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
- Canonicalize tool names before registry lookup (legacy aliases resolve) - Skip sentinel-prefixed results and apply combined-pass 2x tolerance - Fall back to configured global threshold for tools declaring no budget - Compare UI history displays against per-tool budgets - Document persistence gate in design note; re-verify e2e scenario 3
|
Round-3 triage (fixed in 39b5bb4):
Verification: build + typecheck clean, prettier clean, core 14/14, CLI 46/46, tmux E2E scenarios 2/3/4 re-run on 39b5bb4. 中文说明第三轮 10 条意见：9 条修复、1 条说明后保留。
验证：build/typecheck 干净、prettier 干净、core 14/14、CLI 46/46，tmux 场景 2/3/4 已在 39b5bb4 上重跑。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
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 at the 5-round cap without dry convergence; round 5's sole candidate duplicated the PR's existing open comment 3750020138 (sentinel-injection bypass of the alarm, still standing at this commit).
Not explored to full depth (tool budget reached): Context: PR #8875 adds a "Tool result retention" diagnost...: none — all checks I intended completed within budget. I did not run a full npm run build && npm run bundle for an exact byte delta, as no bundle-size claim is…; You are review agent reverse-audit — Reverse audit agen...: none — all planned checks completed within budget..
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：reverse audit — stopped at the 5-round cap without dry convergence; round 5's sole candidate duplicated the PR's existing open comment 3750020138 (sentinel-injection bypass of the alarm, still standing at this commit)。
未探索到全部深度（达到工具调用预算）：Context: PR #8875 adds a "Tool result retention" diagnost...：none — all checks I intended completed within budget. I did not run a full npm run build && npm run bundle for an exact byte delta, as no bundle-size claim is…；You are review agent reverse-audit — Reverse audit agen...：none — all planned checks completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
… compression pipeline Review (round 4) fixes: - R4-3: Extract COMBINED_PASS_TOLERANCE_FACTOR into truncation.ts and share it between the scheduler's combined pass and the retention diagnostics so both use the same tolerance factor. - R4-4: Add imageTokenEstimate option to analyzeToolResultRetention (defaults to DEFAULT_IMAGE_TOKEN_ESTIMATE); doctorCommand now resolves it via resolveSlimmingConfig (env > settings > default), the same source the compression pipeline uses. Export resolveSlimmingConfig from the core barrel. - R4-5/R4-6: Add tests for configurable imageTokenEstimate and Infinity threshold guard. - R4-7: Guard oversizedThresholdChars against Infinity — when truncation is disabled (threshold <= 0 → Infinity), report 0 instead of Infinity so JSON.stringify does not drop the key to null. - R3-3: Fix UI history budget resolution — IndividualToolCallDisplay.name stores the tool's displayName (e.g. 'Shell'), not the registry key (e.g. 'shell'), so getTool(displayName) returned undefined and budgets fell back to the global threshold. Build a displayName → maxOutputChars map from getAllTools() at scan time. - R4-12: Declare string-only scope for UI history scanning in code comment and design doc §5 — structured display objects (file diffs, ANSI captures, agent result summaries) are out of scope for phase 1. - 3750020138: Update design doc §5 to reflect resolveSlimmingConfig alignment and display-name map.
第四轮 review triage（commit
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): Context: PR #8875 (QwenLM/qwen-code), review round 5 at H...: none — all checks above completed within budget.; Context: PR #8875 (QwenLM/qwen-code), review round 5 at H...: none — finished within budget.; Context: PR #8875 (QwenLM/qwen-code), review round 5 at H...: none — finished within budget.; Context: PR #8875 (QwenLM/qwen-code), review round 5 at H...: none — all checks above completed within budget.; chunk 2: did not execute doctorCommand.test.ts (no installed dependencies in the worktree)..
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：Context: PR #8875 (QwenLM/qwen-code), review round 5 at H...：none — all checks above completed within budget.；Context: PR #8875 (QwenLM/qwen-code), review round 5 at H...：none — finished within budget.；Context: PR #8875 (QwenLM/qwen-code), review round 5 at H...：none — finished within budget.；Context: PR #8875 (QwenLM/qwen-code), review round 5 at H...：none — all checks above completed within budget.；chunk 2：did not execute doctorCommand.test.ts (no installed dependencies in the worktree).。
— qwen3.8-max via Qwen Code /review (v0.21.9)
…nel gaps R5-1: threshold uses config.getTruncateToolOutputThreshold() first so disabled (Infinity) doesn't fall to 0 R5-3: UI scan uses budget * COMBINED_PASS_TOLERANCE_FACTOR to match API-history 2x tolerance R5-7: oversized check uses rawChars (output.length) instead of estimatePartChars (which adds 64 wrapper floor) R5-8: read output ?? error and check <persisted-output> sentinel R5-12: UI lookup consults ToolDisplayNamesMigration for legacy names Tests: 3 core + 1 CLI updated for rawChars/2x; 3 core + 2 CLI added Doc: test counts (core 14→19, CLI 9→11), scenario 4 JSON, 2x wording
R5-9: clarify shell/MCP truncate in-tool before gate; split recovery paths R5-15: note sentinel skip in combined-pass re-check; fix mermaid routing R5-10: describe two-stage persist failure (fallback saves full payload)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
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 at the 5-round cap without dry convergence; round 5 surfaced 4 confirmed findings.
Not explored to full depth (tool budget reached): Context: PR #8875 (QwenLM/qwen-code) adds tool-result-ret...: none — all five invariants fully walked within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — completed all checks I planned.; You are review agent reverse-audit — Reverse audit agen...: none — all checks completed within budget., and 1 more.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：reverse audit — stopped at the 5-round cap without dry convergence; round 5 surfaced 4 confirmed findings。
未探索到全部深度（达到工具调用预算）：Context: PR #8875 (QwenLM/qwen-code) adds tool-result-ret...：none — all five invariants fully walked within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — completed all checks I planned.；You are review agent reverse-audit — Reverse audit agen...：none — all checks completed within budget.，另有 1 条。
— qwen3.8-max via Qwen Code /review (v0.21.9)
R6 review: truncateAndSaveToFile's token-aware fallback returns the original content (sentinel-less) when the wrapped form would not be smaller, so the diagnostic's oversized check must tolerate that band. - Export TRUNCATION_FALLBACK_ENVELOPE_SLACK=500 from truncation.ts - Apply slack to the rawChars > 2x budget comparison - Adjust 4 test fixtures broken by the new threshold boundary - Fix 7 factual errors in design/E2E docs (mermaid, project-hash, web-search 102k, re-entrancy guard, shell/MCP bullet) - Add JSDoc caveat for MCP-disconnect limitation (R6-16)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
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 after 3 of 5 rounds without dry convergence; rounds 2-3 only re-surfaced items already on the PR's discussed threads.
中文说明
已审查。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：reverse audit — stopped after 3 of 5 rounds without dry convergence; rounds 2-3 only re-surfaced items already on the PR's discussed threads。
— qwen3.8-max via Qwen Code /review (v0.21.10)
Bot R6-10 re-report: design doc §5 claimed three markers are skipped (prefix, in-body marker, <persisted-output> stub), but the diagnostic only checks two (prefix and <persisted-output> stub). Aligned the doc to match the code. The re-entrancy guard bullet still describes the scheduler's three-marker isAlreadyTruncated check, which is accurate.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: reverse audit — stopped at the 5-round cap without dry convergence; every round (1-5) surfaced findings, and rounds 3-5 included confirmed ones.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): Context: PR #8875 (QwenLM/qwen-code), review round 8 — re...: none — all checks I started were completed within budget.; Context: PR #8875 (QwenLM/qwen-code), review round 8 — re...: did not read compactString 's exact cap in packages/core/src/utils/toolResultDisplayCompaction.ts ; the scenario's reachability estimate relies on the ~32k fi…; Context: PR #8875 (QwenLM/qwen-code), review round 8 — re...: I did not independently re-probe the R8-3/R8-5 slack-asymmetry claims from the CLI side; those live in other chunks and are already on the list.; Context: PR #8875 (QwenLM/qwen-code), review round 8 — re...: I did not exhaustively enumerate every maxOutputChars override beyond the shipped tools I sampled (grep, ripGrep, agent, shell, web-search, read-file, MCP, re….
中文说明
已审查。 建议见行内评论。
未审查：reverse audit — stopped at the 5-round cap without dry convergence; every round (1-5) surfaced findings, and rounds 3-5 included confirmed ones。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：Context: PR #8875 (QwenLM/qwen-code), review round 8 — re...：none — all checks I started were completed within budget.；Context: PR #8875 (QwenLM/qwen-code), review round 8 — re...：did not read compactString 's exact cap in packages/core/src/utils/toolResultDisplayCompaction.ts ; the scenario's reachability estimate relies on the ~32k fi…；Context: PR #8875 (QwenLM/qwen-code), review round 8 — re...：I did not independently re-probe the R8-3/R8-5 slack-asymmetry claims from the CLI side; those live in other chunks and are already on the list.；Context: PR #8875 (QwenLM/qwen-code), review round 8 — re...：I did not exhaustively enumerate every maxOutputChars override beyond the shipped tools I sampled (grep, ripGrep, agent, shell, web-search, read-file, MCP, re…。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| it('does not flag a result measured at exactly 2x the budget (strict >)', () => { | ||
| const exact = 'z'.repeat(OVERSIZED_TOOL_RESULT_THRESHOLD_CHARS * 2); | ||
| const stats = analyzeToolResultRetention([toolResultContent(exact)]); | ||
| expect(stats.oversizedResultCount).toBe(0); |
There was a problem hiding this comment.
[Suggestion] R8-1: This boundary test's fixture sits 500 chars below the implementation's real decision boundary — neither the > strictness nor the slack band is pinned. The oversized condition is rawChars > budget * COMBINED_PASS_TOLERANCE_FACTOR + TRUNCATION_FALLBACK_ENVELOPE_SLACK (60,500 at the default threshold), but this test feeds exactly 60,000 chars — 500 below the boundary. Probe-verified at this commit: removing the slack term from the condition, or flipping > to >=, keeps all 19 core + 12 CLI retention tests green. — Failure scenario: a refactor dropping the slack term makes compliant retained results count as oversized — /doctor memory reports a false Oversized results count plus the /compress hint — and the suite named to catch it stays green while appearing to cover the boundary.
| it('does not flag a result measured at exactly 2x the budget (strict >)', () => { | |
| const exact = 'z'.repeat(OVERSIZED_TOOL_RESULT_THRESHOLD_CHARS * 2); | |
| const stats = analyzeToolResultRetention([toolResultContent(exact)]); | |
| expect(stats.oversizedResultCount).toBe(0); | |
| it('does not flag a result measured at exactly 2x the budget (strict >)', () => { | |
| const exact = 'z'.repeat( | |
| OVERSIZED_TOOL_RESULT_THRESHOLD_CHARS * 2 + | |
| TRUNCATION_FALLBACK_ENVELOPE_SLACK, | |
| ); | |
| const stats = analyzeToolResultRetention([toolResultContent(exact)]); | |
| expect(stats.oversizedResultCount).toBe(0); |
中文说明
[Suggestion / 建议] R8-1：该边界测试的 fixture 比实现真正的判定边界低 500 字符——> 的严格性与 slack 区间均未被钉住。超限条件为 rawChars > budget * COMBINED_PASS_TOLERANCE_FACTOR + TRUNCATION_FALLBACK_ENVELOPE_SLACK（默认阈值下为 60,500），而该测试恰好注入 60,000 字符——比边界低 500。已在该提交探针验证：从条件中删除 slack 项、或把 > 翻转为 >=，全部 19 个 core + 12 个 CLI retention 测试仍为绿。故障场景：删除 slack 项的重构会使合规滞留结果被计入超限——/doctor memory 误报超限计数并给出 /compress 提示——而声称覆盖该边界的测试套件却依然为绿。上方 suggestion 把 fixture 移到真实边界（60,500）：严格 > 不计、>= 或去掉 slack 都会计，从而获得判别力。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| back to the configured global truncation threshold), skip results already | ||
| carrying a truncation marker (prefix, in-body marker, or `<persisted-output>` | ||
| stub), and apply the scheduler's combined-pass 2x tolerance plus a small |
There was a problem hiding this comment.
[Suggestion] R8-2: This sentence claims oversized counting skips results carrying an "in-body marker", but analyzeToolResultRetention only checks startsWith(TOOL_OUTPUT_TRUNCATED_PREFIX) and startsWith('<persisted-output>') — no in-body ... [CONTENT TRUNCATED] ... check exists and no test covers one; design note §5 in this same PR lists only the prefix and stub forms. Ledger R6-10's "doc aligned with code (Option B)" fix aligned §5 but missed this sentence. — Failure scenario: behaviorally inert today (the only marker-only path is size-bounded below the flag bound), but the documented guarantee is enforced by neither code nor test — a future marker-only path above the flag bound would false-positive the regression alarm uncaught, and a debugger relying on this doc assumes a skip that does not exist. Suggested fix: align this sentence with §5 and the code ("prefix or <persisted-output> stub"), or extend alreadyTruncated with the in-body-marker check and add a unit test.
中文说明
[Suggestion / 建议] R8-2：该句声称超限计数会跳过"携带正文内标记"的结果，但 analyzeToolResultRetention 只检查 startsWith(TOOL_OUTPUT_TRUNCATED_PREFIX) 与 startsWith('<persisted-output>')——没有正文内 ... [CONTENT TRUNCATED] ... 检查，也没有测试覆盖；同一 PR 的设计说明 §5 也只列出前缀与 stub 两种形式。清单 R6-10 的"文档与代码对齐（方案 B）"修复对齐了 §5，却漏掉了这一句。故障场景：当前行为上无影响（唯一仅带正文标记的路径尺寸受限于标记阈值之下），但该文档承诺既无代码也无测试保障——未来若出现高于标记阈值的"仅正文标记"路径，回归警报将不被发现地误报；依赖本文档的排查者会假设存在一个实际不存在的跳过逻辑。建议：将该句与 §5 及代码对齐（"前缀或 <persisted-output> stub"），或为 alreadyTruncated 增加正文标记检查并补充单测。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| * The retained original can sit up to ~prefix-length above the budget before | ||
| * the fallback kicks in, so the diagnostics comparison must tolerate that band. | ||
| */ | ||
| export const TRUNCATION_FALLBACK_ENVELOPE_SLACK = 500; |
There was a problem hiding this comment.
[Suggestion] R8-3: TRUNCATION_FALLBACK_ENVELOPE_SLACK = 500 undersizes the token-aware fallback band on deep paths. truncateAndSaveToFile's wrapper is ~360 fixed chars plus the spill-file path and tool name; probe-measured at this commit, the false-positive window opens (4-78 chars wide) once tempdir+tool-name length exceeds ~128 — CI-style runtime dirs and long MCP tool names such as mcp__playwright__browser_take_screenshot. — Failure scenario: a combined-pass result landing in (2xbudget + 500, 2xbudget + wrapper-overhead] triggers the token-aware fallback and is retained sentinel-less; analyzeToolResultRetention then flags it — /doctor memory reports a truncation bypass that never happened, precisely in the deep-path environments where retention pressure is most often diagnosed. Note: a bare constant raise breaks the small-budget "supports a custom threshold" test (511-char fixture at threshold 5), so the fix needs envelope-relative sizing. Suggested fix: export the wrapper's fixed overhead from truncation.ts and derive the slack from it so the two cannot drift.
中文说明
[Suggestion / 建议] R8-3：TRUNCATION_FALLBACK_ENVELOPE_SLACK = 500 在深路径下小于 token 感知兜底带的真实宽度。truncateAndSaveToFile 的包裹文本约 360 个固定字符，另加落盘文件路径与工具名；已在该提交探针实测：当 tempdir+工具名长度超过约 128 时（CI 风格的运行时目录、mcp__playwright__browser_take_screenshot 之类的长 MCP 工具名），误报窗口打开（宽 4-78 字符）。故障场景：合并通道中长度落在 (2倍预算 + 500, 2倍预算 + 包裹开销] 的结果会触发 token 感知兜底、以无哨兵形式滞留，analyzeToolResultRetention 随后将其标记——/doctor memory 报告一个从未发生的"截断绕过"，而恰恰是深路径环境最常诊断滞留压力。注意：直接调大常量会破坏小预算的 "supports a custom threshold" 测试（阈值 5 时的 511 字符 fixture），修复需按包裹开销相对定值。建议：从 truncation.ts 导出包裹文本的固定开销并由其推导 slack，使两者不再失同步。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| appended only after the raw body is bounded, then the assembled string is | ||
| re-checked against a doubled budget — unless the body already carries the | ||
| truncation sentinel (re-entrancy skip), in which case only the batch budget | ||
| bounds it. |
There was a problem hiding this comment.
[Suggestion] R8-4: The §2 "Metadata integrity" / "Re-entrancy guard" bullets claim the doubled-budget re-check skips any body carrying "the truncation sentinel" — defined just above as all three forms (prefix, in-body marker, <persisted-output> stub) — but the combined second pass skips only startsWith(TOOL_OUTPUT_TRUNCATED_PREFIX) (coreToolScheduler.ts:5152-5156, unchanged by this diff). <persisted-output> stubs and marker-only bodies are still re-checked. — Failure scenario: a >28k result persisted to a stub, then a PostToolUse hook appending ~48k+ chars of metadata (postHookResult.additionalContext is unbounded user output), pushes the assembled string past 2x the global budget — the combined pass re-truncates the stub body, nesting a truncation envelope around the persisted stub and spilling a second file, contradicting the "truncation headers never nest" invariant this section states. No data loss (the pointer survives in the spilled content) and the trigger needs unusually large hook metadata — but this is the acceptance-criterion artifact misdescribing a guard it exists to document. Suggested fix: reword the bullets to match the code (only TOOL_OUTPUT_TRUNCATED_PREFIX-leading bodies are skipped), or, if the invariant is intended, extend the combined-pass skip to isAlreadyTruncated(content).
中文说明
[Suggestion / 建议] R8-4：§2 的 "Metadata integrity" / "Re-entrancy guard" 条目声称"加倍预算复检会跳过任何携带截断哨兵的正文"——而上文将哨兵定义为全部三种形式（前缀、正文内标记、<persisted-output> stub）——但合并第二通道只跳过 startsWith(TOOL_OUTPUT_TRUNCATED_PREFIX)（coreToolScheduler.ts:5152-5156，本 diff 未改动）。<persisted-output> stub 与仅带正文标记的正文仍会被复检。故障场景：超过 28k 的结果被落盘为 stub 后，PostToolUse 钩子追加约 48k+ 字符的元数据（postHookResult.additionalContext 是不受限的用户输出），把拼装字符串推过 2 倍全局预算——合并通道会再次截断该 stub 正文，在已落盘 stub 外再套一层截断信封并落盘第二个文件，与本节所述"截断头部永不嵌套"的不变量矛盾。无数据丢失（指针仍存在于落盘内容中），触发需要异常大的钩子元数据——但这是验收标准文档在错误描述它本应记录的守卫。建议：按代码改写该条目（只跳过以 TOOL_OUTPUT_TRUNCATED_PREFIX 开头的正文）；若该不变量确为预期，则把合并通道的跳过扩展为 isAlreadyTruncated(content)。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| if ( | ||
| Number.isFinite(budget) && | ||
| tool.resultDisplay.length > budget * COMBINED_PASS_TOLERANCE_FACTOR | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R8-5: This UI-history counter diverges from the API-history calibration its comment claims ("match[es] the API-history calibration") in two ways: it compares against budget * COMBINED_PASS_TOLERANCE_FACTOR with no TRUNCATION_FALLBACK_ENVELOPE_SLACK (+500), and it has no analogue of the API counter's alreadyTruncated sentinel skip — it compares PRE-truncation display text (truncation only touches model content; displays are merely capped at MAX_RETAINED_TOOL_RESULT_DISPLAY_CHARS = 32,000). Probe-verified at this commit: truncateToolOutputThreshold: 8000 + a budget-less tool + a 16,300-char result prints Oversized results (above tool budget): 0 directly above Oversized also rendered in UI history: 1 item(s); adding the slack flipped the slack-band arm, but the sentinel-bearing variant still shows 0/1 — so the slack alone does not close it. — Failure scenario: (a) a budget-less result in (2B, 2B+500] reaching UI history is counted by this loop but not the API count — a self-contradicting diagnostic; (b) a correctly sentinel-truncated result (API side skips it) is counted here because its pre-truncation display exceeds 2B — flagging a result the pipeline bounded correctly. The only realistic window is budget-less tools with threshold < 16k (every budgeted built-in has 2xbudget > the 32k display cap); the existing 65k-display fixture is unreachable in a live session and no test exercises the reachable window. Suggested fix: derive the UI signal from the API-side analysis so both counters measure the same artifact, or add the slack + a sentinel-aware skip, relabel the line, and correct the calibration comment; add a low-threshold test.
中文说明
[Suggestion / 建议] R8-5：该 UI 历史计数器与其注释声称的"与 API 历史口径一致"存在两处偏离：比较时使用 budget * COMBINED_PASS_TOLERANCE_FACTOR 而没有 TRUNCATION_FALLBACK_ENVELOPE_SLACK（+500），也没有 API 计数器 alreadyTruncated 哨兵跳过的对应物——它比较的是截断前的显示文本（截断只作用于模型内容；显示仅受 MAX_RETAINED_TOOL_RESULT_DISPLAY_CHARS = 32,000 上限约束）。已在该提交探针验证：truncateToolOutputThreshold: 8000 + 无预算工具 + 16,300 字符结果会先打印 Oversized results (above tool budget): 0，紧跟着打印 Oversized also rendered in UI history: 1 item(s)；补上 slack 后 slack 区间一支翻转，但带哨兵的一支仍显示 0/1——说明仅补 slack 不够。故障场景：(a) 无预算工具落在 (2B, 2B+500] 的结果进入 UI 历史时被该循环计数、API 计数却不计——自相矛盾的诊断；(b) 已被哨兵正确截断的结果（API 侧跳过）在此因其截断前显示超过 2B 而被计数——把流水线已正确收口的结果标为超限。唯一现实窗口是阈值 < 16k 的无预算工具（所有带预算的内置工具 2倍预算 都大于 32k 显示上限）；现有 65k 显示 fixture 在真实会话中不可达，也没有测试覆盖可达窗口。建议：让 UI 信号直接复用 API 侧分析结果，使两个计数测量同一对象；或补上 slack + 哨兵感知跳过、改写报告行标签并更正"口径一致"注释；补充低阈值测试。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const alreadyTruncated = | ||
| typeof output === 'string' && | ||
| (output.startsWith(TOOL_OUTPUT_TRUNCATED_PREFIX) || | ||
| output.startsWith('<persisted-output>')); |
There was a problem hiding this comment.
[Suggestion] R8-7: The sentinel skip list misses buildStub's degraded (non-file) variant — it starts with "Output too large (N KB). (reason)" and carries neither sentinel prefix. persistAndTruncateToolResult returns it when disk persistence is skipped (a single result over 50MB, the 500MB session disk budget exhausted, an invalid callId, or disk persistence unavailable). The ~2090-char bounded stub lands in response.output/response.error via the error-gate, timeout, and batch-finalizer routes with no subsequent truncation pass; for a budget-less tool the budget here is the configured global threshold (the settings schema has no minimum; only ≤ 0 disables). Probe-verified: the stub is flagged oversized for threshold ≤ 794 (bound 2T+500 < 2090); the implied startsWith('Output too large') fix flipped the probe to 0. — Failure scenario: truncateToolOutputThreshold ≤ 794 plus any persistence-skip branch → /doctor memory reports a false oversized result and the /compress hint for a session the pipeline bounded correctly. Suggested fix: export the degraded-stub header from truncation.ts (e.g. OUTPUT_STUB_PREFIX = 'Output too large') and add it to this disjunction — ideally shared with isAlreadyTruncated so all sentinel checks agree.
中文说明
[Suggestion / 建议] R8-7：哨兵跳过列表漏掉了 buildStub 的降级（未落盘）变体——它以 "Output too large (N KB). (reason)" 开头，不携带任何一种哨兵前缀。persistAndTruncateToolResult 在跳过磁盘落盘时返回该形态（单个结果超过 50MB、500MB 会话磁盘预算耗尽、callId 无效或磁盘落盘不可用）。这个约 2090 字符、已受界的 stub 会经错误门限、超时与批量收尾三条路径进入 response.output/response.error，且后续不再有任何截断通道；对无预算工具，此处的预算即配置的全局阈值（设置 schema 无下限，仅 ≤ 0 表示禁用）。已探针验证：阈值 ≤ 794 时该 stub 被标记超限（界 2T+500 < 2090）；按隐含修复补上 startsWith('Output too large') 后探针翻转为 0。故障场景：truncateToolOutputThreshold ≤ 794 + 任一跳过落盘分支 → /doctor memory 对一个流水线已正确收口的会话误报超限并给出 /compress 提示。建议：从 truncation.ts 导出降级 stub 头部常量（如 OUTPUT_STUB_PREFIX = 'Output too large'）并加入该判断——最好与 isAlreadyTruncated 共用，使所有哨兵检查口径一致。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| ` Tool results in history: ${stats.toolResultCount}`, | ||
| ` Total retained: ${stats.totalChars} chars`, | ||
| ` Largest result: ${stats.largestResultChars} chars`, | ||
| ` Oversized results (above tool budget): ${stats.oversizedResultCount}`, |
There was a problem hiding this comment.
[Suggestion] R8-8: This label says "above tool budget", but the counter only measures results above 2x budget + 500 slack (budget * COMBINED_PASS_TOLERANCE_FACTOR + TRUNCATION_FALLBACK_ENVELOPE_SLACK) — understating the counted band by up to a factor of two. The core analyzer JSDoc is honest ("measured above 2x budget"); this user-facing label drops the tolerance. Probe-verified: a 45,000-char result — 50% above its 30k tool budget, inside the combined-pass tolerance this PR's own constants document as legitimate — is not counted. — Failure scenario: a user investigating context bloat sees Oversized results (above tool budget): 0 next to a large Total retained: N chars and concludes nothing exceeds its tool's declared budget, when results up to 2x budget are legitimately retained — the diagnostic this PR exists to provide misdirects the investigation.
| ` Oversized results (above tool budget): ${stats.oversizedResultCount}`, | |
| ` Oversized results (above 2x tool budget): ${stats.oversizedResultCount}`, |
中文说明
[Suggestion / 建议] R8-8：该行标签写的是"超过工具预算"，但计数器实际只测量超过 2 倍预算 + 500 slack（budget * COMBINED_PASS_TOLERANCE_FACTOR + TRUNCATION_FALLBACK_ENVELOPE_SLACK）的结果——计数区间被低估了最多一倍。core 分析器的 JSDoc 是如实的（"按超过 2 倍预算测量"），这条面向用户的标签却丢掉了容差。已探针验证：一个 45,000 字符的结果——超出其 30k 工具预算 50%，落在本 PR 自己的常量文档明确承认为合规的合并通道容差内——不会被计数。故障场景：排查上下文膨胀的用户看到 Oversized results (above tool budget): 0 与很大的 Total retained: N chars 并列，会得出"没有任何结果超过工具预算"的结论，而实际上 2 倍预算内的结果都是合规滞留的——本 PR 赖以存在的诊断把排查引向了错误方向。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const oversized = 'z'.repeat( | ||
| OVERSIZED_TOOL_RESULT_THRESHOLD_CHARS * 2 + | ||
| TRUNCATION_FALLBACK_ENVELOPE_SLACK + | ||
| 1, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R8-10: No test pins that the oversized comparison measures RAW string length (rawChars) rather than the estimatePartChars output (chars, +64 wrapper floor). The implementation deliberately compares rawChars (the adjacent comment says so), but chars is computed two lines above for totals/largest, making rawChars > → chars > a plausible simplifying refactor. Mutation-verified at this commit: substituting chars > keeps all 19 core + 49 CLI doctorCommand tests green. — Failure scenario: that refactor false-flags every correctly-tolerated result whose raw length lands in (2xbudget+436, 2xbudget+500] — e.g. a 60,480-char result (bound 60,500) estimates at 60,544 → /doctor memory reports Oversized results: 1 plus the /compress hint for a correctly bounded session — and the regression ships test-green. Suggested fix: add a discriminating fixture in the (bound−64, bound] band, e.g. 'z'.repeat(OVERSIZED_TOOL_RESULT_THRESHOLD_CHARS * 2 + TRUNCATION_FALLBACK_ENVELOPE_SLACK - 32) (raw 60,468 ≤ 60,500; estimate 60,532 > 60,500) asserting oversizedResultCount is 0 — verified to fail under the mutation and pass under the implementation.
中文说明
[Suggestion / 建议] R8-10：没有任何测试钉住"超限比较测量的是原始字符串长度（rawChars）而非 estimatePartChars 输出（chars，含 64 字符外壳下限）"。实现刻意比较 rawChars（相邻注释亦如此说明），但 chars 就在上方两行为总量/最大值而计算，rawChars > → chars > 是一个看似合理的简化重构。已在该提交做变异验证：替换为 chars > 后，全部 19 个 core + 49 个 CLI doctorCommand 测试仍为绿。故障场景：该重构会把原始长度落在 (2倍预算+436, 2倍预算+500] 的合规结果全部误标——例如 60,480 字符的结果（界 60,500）估值为 60,544 → /doctor memory 对已正确收口的会话报告 Oversized results: 1 并给出 /compress 提示——回归在测试全绿中上线。建议：在 (界−64, 界] 区间补一个有判别力的 fixture，如 'z'.repeat(OVERSIZED_TOOL_RESULT_THRESHOLD_CHARS * 2 + TRUNCATION_FALLBACK_ENVELOPE_SLACK - 32)（原始 60,468 ≤ 60,500；估值 60,532 > 60,500），断言 oversizedResultCount 为 0——已验证该 fixture 在变异下失败、在原实现下通过。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
|
||
| | Non-goal | Enforcement | | ||
| | ------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | Do not upload tool results | Offload target is a local file under the session temp dir only; no network path exists in the truncation code | |
There was a problem hiding this comment.
[Suggestion] R8-11: The offload target is not a session-scoped temp dir — every offload path writes to the per-PROJECT temp dir ~/.qwen/tmp/<project-hash> (getToolResultsDir() = getProjectTempDir()/tool-results; truncateAndSaveToFile spills to getProjectTempDir()), shared across all sessions of a project and swept on a 24h cadence, not at session end. The same "session temp" wording survives at line 26 (§2 mermaid: "Full payload persisted to session temp file") and lines 59-60 (§2 bullet: "persisted to a session / temp file"), contradicting §2's own correct <project-hash> path and the stub's "may be cleaned up after 24 hours" note. All three locations were named by round-6 thread R6-13, whose fix replaced the <session-hash> placeholder but not this wording (verified present at this commit). — Failure scenario: §4 is the issue's acceptance-criteria privacy artifact; an auditor of where offloaded tool output lands — output the code itself warns can contain secrets — concludes the files are session-scoped and die with the session, when they persist cross-session-readable until the 24h sweep. Substantive guarantees (local-only, 0o600, no network path) hold — this is a wording fix, not a behavior hole. Suggested fix: replace "session temp dir/file" with "project temp dir (~/.qwen/tmp/<project-hash>)" in all three locations, noting files persist across sessions until the 24h cleanup sweep.
中文说明
[Suggestion / 建议] R8-11：落盘目标并不是"会话级临时目录"——所有落盘路径写入的是按项目划分的临时目录 ~/.qwen/tmp/<project-hash>（getToolResultsDir() = getProjectTempDir()/tool-results；truncateAndSaveToFile 落盘到 getProjectTempDir()），同一项目的所有会话共享，按 24 小时周期清理，而非会话结束时清理。同样的"session temp"措辞还残留于第 26 行（§2 mermaid："Full payload persisted to session temp file"）与第 59-60 行（§2 条目："persisted to a session / temp file"），与 §2 自己给出的正确 <project-hash> 路径及 stub 中"可能在 24 小时后被清理"的提示相矛盾。三处位置在第六轮 R6-13 线程中均被点名，该轮修复只替换了 <session-hash> 占位符，未改这些措辞（已在本提交核实仍存在）。故障场景：§4 是该 issue 验收标准中的隐私文档；审计落盘输出位置的人——代码自己都警告这些输出可能含敏感信息——会以为文件随会话生灭，而实际上它们跨会话可读、直到 24 小时清理。实质保障（仅本地、0o600、无网络路径）依然成立——这是措辞修复而非行为漏洞。建议：三处均改为"project temp dir（~/.qwen/tmp/<project-hash>）"，并注明文件跨会话保留、直至 24 小时清理。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| Unreachable in normal operation (per-tool/global layers bound every result at | ||
| or below its declared budget). Covered deterministically by unit tests: |
There was a problem hiding this comment.
[Suggestion] R8-12: The parenthetical "(per-tool/global layers bound every result at or below its declared budget)" is false — the scheduler's combined pass explicitly permits up to 2x budget (+ envelope slack) after metadata is appended: this PR's own COMBINED_PASS_TOLERANCE_FACTOR JSDoc says "compliant retained content can legitimately measure up to twice its tool's budget", and the companion design note §3 lists the combined pass as "2x of the applicable budget". The oversized yes-branch is unreachable because the ALARM bound mirrors that 2x+slack tolerance, not because results never exceed their budget. — Failure scenario: an engineer extending this plan (e.g. the phase-2 UI scan it defers to) or writing regression assertions from it takes "bound at or below its declared budget" as the pipeline invariant, treats a legitimately retained ~45-60k shell result (inside the 2x tolerance) as a truncation-layer violation, and files a false regression or asserts retention <= budget in a test that fails on compliant behavior. Suggested fix: reword to "Unreachable in normal operation (layers bound every result at or below 2x budget + envelope slack — the combined-pass tolerance the alarm is calibrated to)".
中文说明
[Suggestion / 建议] R8-12：括号内的"（每工具/全局层把每个结果收口在其声明预算之内或之下）"不成立——调度器的合并通道明确允许在元数据追加后达到 2 倍预算（+信封余量）：本 PR 自己的 COMBINED_PASS_TOLERANCE_FACTOR JSDoc 写着"合规滞留内容可以合法地达到其工具预算的两倍"，配套设计说明 §3 也把合并通道列为"适用预算的 2 倍"。超限"是"分支不可达，是因为警报阈值镜像了该 2 倍+余量容差，而不是因为结果从不超过预算。故障场景：扩展本方案（如其延后的 phase-2 UI 扫描）或据其编写回归断言的工程师会把"收口在声明预算之内"当作流水线不变量，把合规滞留的约 45-60k shell 结果（位于 2 倍容差内）当作截断层违规，提交虚假回归报告，或在测试中断言 retention <= budget 导致合规行为使测试失败。建议改写为："正常操作不可达（各层把每个结果收口在 2 倍预算 + 信封余量之内——即警报所校准的合并通道容差）"。
— qwen3.8-max via Qwen Code /review (v0.21.10)
Maintainer verification — real local build & live sessionI built this PR locally and exercised it against a real model session (not mocks) to check the claims in the description before merge. Verdict: the feature works as specified — recommend merge. All four reviewer scenarios reproduce, the reported numbers are exactly reconstructible from the live history, and the oversized branch does fire end-to-end when a truncation layer is actually bypassed. Two description-only corrections are needed (no code change), plus one signal that is worth a follow-up. Environment
Local gates: 1. Before / after (base vs head, same command, same fixture)Base ends at
Scenario 1 reproduces exactly as described: all zeros, 2. Reported numbers are exactly reconstructible ✅This was the part I most wanted to falsify, so I compared the report against the raw session JSONL rather than trusting the display. Scenario 2 — model runs Ground truth from the session record: the single retained Cross-checked again on an MCP session (two results:
Scenario 4 —
Also verified: 3. No false positives on compliant high-budget tools ✅I wrote a stdio MCP server returning a 120,000-char payload — large, but well inside MCP's declared 500k budget. The offload layer stubbed it to a 4. The oversized branch really fires ✅ (fault injection)The description says the "yes" branch is unreachable in normal operation. I confirmed that claim is stronger than stated — and then forced the branch open to prove the alarm has teeth. Bounding is at least three layers deep, plus tool-internal caps. Patching the built bundle to neuter
Only after lifting the batch budget as well did a genuinely unbounded result reach history — and the diagnostic caught it immediately, with all three signal lines:
(The fault injection patched the built bundle only; the PR's source is untouched by it.) Findings
|
| Tool | Budget | Signal threshold | Reachable under 32k cap? |
|---|---|---|---|
grep / ripgrep |
20,000 | 40,000 | no |
run_shell_command |
30,000 | 60,000 | no |
agent |
32,000 | 64,000 | no |
web_search / MCP |
102k / 500k | ≥204,000 | no |
| tools declaring none | 25,000 (global default) | 50,000 | no |
The smallest reachable threshold is 40,000 > 32,000, so the counter cannot leave 0 unless a user sets tools.truncateToolOutputThreshold below 16,000.
Measured directly in the oversized session above: the same 1.1 M-char result was retained in UI history — at exactly 32,000 chars — while the report printed Oversized also rendered in UI history: 0 item(s). So the answer given to the linked issue's "is it also duplicated in the UI history?" criterion is a permanent "no" in a case where the honest answer is "yes, 32,000 chars are duplicated".
Not a crash and not a false positive, so I don't consider it merge-blocking, but as written the signal cannot do the job the issue asked of it. Cheapest fixes: compare against MAX_RETAINED_TOOL_RESULT_DISPLAY_CHARS instead of budget × 2, or just report the retained display chars (UI history retains N chars of tool displays) — a number that is always meaningful.
ℹ️ C. Test counts in the Chinese section of the description are stale
The 中文说明 block says "core 侧 9 个用例、CLI 侧 doctorCommand 42 个用例". Actual, verified locally: 19 and 49. The English section already has the right numbers.
ℹ️ D. Notes
.qwen/e2e-tests/already exists onmain, so shipping the e2e note there is consistent with repo convention.- The section only ever emits sizes and counts — I saw no content leakage in any of the runs, including the 1.1 M-char one. Safe to paste publicly, as claimed.
Recommendation
Merge after fixing the PR description (A and C) — no code change required for that. Item B is a real limitation of one of the three required signals; happy to see it handled either in this PR or as a tracked follow-up on #4184, at the maintainers' preference.
中文版本（点击展开）
维护者验证 —— 本地真实构建 + 真实模型会话
我在本地构建了这个 PR，并用真实模型会话（非 mock）跑通验证，以便在合并前核对描述中的各项声明。
结论：功能符合描述，建议合并。 四个评审场景全部复现，报告中的数字可以从活动历史中精确反推，超限分支在截断层真被绕过时确实会触发。需要修正的是两处纯描述问题（不涉及代码），外加一个建议后续处理的信号。
环境
| Head | 0c0e4fc5c3（feat/doctor-tool-result-retention） |
| Base | 8c90697ace（PR base，独立构建作为"改动前"对照臂） |
| 运行方式 | 各臂打包出的 dist/cli.js，在 tmux（200x50）中驱动 |
| 模型 | 真实 gpt-5.6-sol，OpenAI 兼容端点，API Key 认证 |
| 隔离 | 独立 QWEN_HOME + 一次性 fixture 工程，YOLO 审批，无沙箱 |
| 平台 | macOS 26.6，Node v24.18.1 |
本地门禁：tool-result-retention.test.ts 19/19 通过，doctorCommand.test.ts 49/49 通过，npm run lint 干净，npm run typecheck 干净。PR 的 CI 全绿（0 失败）。
1. 改动前后对照
Base 的报告止于 recommendation；Head 追加了新段落。场景 1 完全复现：全 0、Oversized also in UI history ...: 0 item(s)、Oversized also in compression input: no。
2. 报告数字可精确反推 ✅
这是我最想证伪的部分，所以没有只看界面，而是拿会话 JSONL 原始记录对账。
场景 2（模型执行 yes demo | head -c 150000，150,000 字节，被现有截断门落盘）报告 Total retained: 4576 chars。原始记录中唯一滞留的 functionResponse.output 是 4,512 字符，且以 Tool output was too large and has been truncated 开头。4512 + 64（estimatePartChars 给 functionResponse 的包装下限）= 4,576，精确吻合，不是近似。
在 MCP 会话上再次对账（两条结果：tool_search 252 字符 + mcp__bigout__emit_big 2,410 字符）：报告 totalChars: 2790 = 252 + 2410 + 2×64，largestResultChars: 2474 = 2410 + 64，同样精确。
场景 4（--json）七个字段齐全，数值与可读报告一致。另外验证：/doctor memory --sample 段落只出现一次（第二处插入点没有造成重复），headless qwen -p "/doctor memory --json" 也正确输出 toolResultRetention。
3. 高预算工具的合规结果没有误报 ✅
我写了一个 stdio MCP server 返回 120,000 字符载荷——很大，但远在 MCP 声明的 500k 预算内。offload 层把它变成 <persisted-output> 预览，诊断识别了该标记，Oversized results 保持 0。按工具解析预算的逻辑与代码注释一致。
4. 超限分支确实会触发 ✅（故障注入）
描述称"yes"分支在正常运行下不可达。我确认这个结论比描述的还要更强——然后强行打开该分支，证明这个告警不是摆设。
限界至少有三层，另加工具自身的上限。把打包产物中的 truncateToolOutput 和 persistAndTruncateToolResult 双双废掉后，仍然没有产生超限结果：
| 注入的绕过 | 工具 | 滞留字符 | 实际限界者 |
|---|---|---|---|
| 两层都关 | run_shell_command（seq 1 20000） |
60,384 | shell 自身的 maxBufferedOutputBytes（约 64 KB） |
| 两层都关 | grep（3,000 条匹配） |
14,419 | grep 内部结果上限 |
| 两层都关 | MCP，110 万字符 | 200,064 | toolOutputBatchBudget（200,000） |
只有把批量预算也抬高之后，真正无界的结果才进入历史——诊断立刻捕获，三条信号线齐发：Oversized results: 1、compression input: yes (shared by reference, no extra copy)、以及 /compress 提示行。1,100,064 > 500,000 × 2 + 500 = 1,000,500，边界算术与文档完全一致。
（故障注入只改了打包产物，PR 源码未被触碰。）
发现
PR 正文让评审者设置 tools.truncateToolOutputThreshold: 100000、执行 seq 1 20000，并"预期 Oversized results >= 1、compression input: yes、出现 /compress 提示行"。
我严格照做，跑了两遍，结果稳定：Oversized results: 0、compression input no、无提示行。
原因是结构性的：ShellTool 自己声明了 maxOutputChars = 30_000，调度器对 shell 输出根本不会套用全局阈值——调高 truncateToolOutputThreshold 对这条路径毫无影响。唯一可观测的效果是 oversizedThresholdChars 正确变成了 100000。
随 PR 交付的 e2e 记录（.qwen/e2e-tests/... 场景 3）已经正确记为 Oversized results: 0，所以代码和交付文档都是对的，错的只有 PR 描述。按 PR 正文操作的评审者会得到一个假的"验证失败"。建议把第 3 步的预期改成实际行为（0，用来证明缓解机制生效），"yes"分支指向单测。
Oversized also rendered in UI history 在默认配置下结构性恒为 0
这一条我认为值得后续处理。交互式 UI 历史对每条字符串 resultDisplay 有 MAX_RETAINED_TOOL_RESULT_DISPLAY_CHARS = 32_000 的硬上限（toolResultDisplayCompaction.ts），而新信号只统计超过 预算 × 2 的显示：
| 工具 | 预算 | 信号阈值 | 32k 上限下可达？ |
|---|---|---|---|
grep / ripgrep |
20,000 | 40,000 | 否 |
run_shell_command |
30,000 | 60,000 | 否 |
agent |
32,000 | 64,000 | 否 |
web_search / MCP |
102k / 500k | ≥204,000 | 否 |
| 未声明预算的工具 | 25,000（全局默认） | 50,000 | 否 |
最小可达阈值是 40,000 > 32,000，因此除非用户把 tools.truncateToolOutputThreshold 设到 16,000 以下，这个计数器不可能离开 0。
在上面的超限会话中我直接量过：那条 110 万字符的结果确实滞留在 UI 历史里——正好 32,000 字符——而报告打印的是 Oversized also rendered in UI history: 0 item(s)。也就是说，对关联 issue 中"是否也在 UI 历史中重复？"这条验收标准，给出的是一个恒定的"否"，而诚实的答案是"是，重复了 32,000 字符"。
不崩溃、也没有误报，所以我不认为它阻塞合并；但按当前写法，这个信号无法完成 issue 交给它的任务。最省事的修法：改成与 MAX_RETAINED_TOOL_RESULT_DISPLAY_CHARS 比较，或者干脆直接报告 UI 历史中滞留的显示字符数（这个数字永远有意义）。
ℹ️ C. 描述中文段落的用例数字过期
中文说明 里写的是"core 侧 9 个用例、CLI 侧 doctorCommand 42 个用例"，本地实测为 19 与 49。英文段落的数字是对的。
ℹ️ D. 其他
.qwen/e2e-tests/在main上已存在，把 e2e 记录放这里符合仓库惯例。- 该段落只输出大小与计数，所有运行（包括 110 万字符那次）都没有看到内容泄漏，可以放心公开粘贴，与描述一致。
建议
修正 PR 描述（A 与 C）后合并 —— 这两项不需要改代码。B 是三个必需信号之一的真实局限，在本 PR 内处理或作为 #4184 的后续跟踪都可以，由维护者决定。
|
@qwen-code /triage |
|
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: 575 passed · 0 failed · 575 total 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：575 通过 · 0 失败 · 575 总计 Verification reportPR #8875 Deep Verification —
|
| Cell | Environment | Observable oracle | Result |
|---|---|---|---|
| head report | dist built at merge HEAD, scripted scenario | retention section present; 3 / 165342 / 100114 / oversized 1; UI duplication 1 item(s); compression yes (shared by reference…); /compress hint |
23/23 PASS |
head --json |
same | toolResultRetention keys exact, incl. oversizedThresholdChars: 25000, presentInCompressionInput: true |
included above |
| base report | base worktree 33c330d0, rebuilt core+cli, same scenario |
base memory report produced (positive control, 696 chars) without any retention line | 9/9 PASS |
base --json |
same | diagnostics present, toolResultRetention key absent |
included above |
| base export check | base core dist | analyzeToolResultRetention is undefined |
included above |
Witnesses: 01-ab-head-retention-report.png, 02-ab-base-report-absent.png.
Control purity (the monorepo symlink trap): the base worktree got a mixed node_modules
(external deps → head entries, since the PR changes no lockfile; @qwen-code/* → base packages).
A probe module placed beside the base dist resolved @qwen-code/qwen-code-core to
tmp/base-tree/packages/core/dist/index.js (realpath asserted) — the base harness executed base code,
and that core has neither analyzeToolResultRetention nor COMBINED_PASS_TOLERANCE_FACTOR.
Secondary claim — scheduler refactor is behavior-preserving. coreToolScheduler.ts replaces the
combined-pass literal * 2 with * COMBINED_PASS_TOLERANCE_FACTOR. Base dist carries
content.length > baseThreshold * 2; head dist carries baseThreshold * COMBINED_PASS_TOLERANCE_FACTOR
with the constant asserted === 2 by the harness; the 356-test scheduler suite is green (see §4).
Secondary claim — no added memory pressure / compression sharing. getHistoryShallow() maps
copyContentContainer (shallow spread of content + parts array — geminiChat.ts:1228); the scan reads
output.length raw (no serialization), so payloads are never cloned. Compression input is assembled
from chat.getHistoryShallow(true) (chatCompressionService.ts:414/459) — same payload objects, so
presentInCompressionInput: yes (shared by reference, no extra copy) is accurate, and the derivation
is documented in a code comment with its revisit condition.
2. Corrections
None to prior review rounds. One correction to the PR description itself is recorded as Finding F4
(the "30k threshold" phrasing conflates the direct-API fallback constant with the production
fallback, which is the configured global threshold — 25k by default).
3. Findings
F1 — Medium: Reviewer Test Plan step 3 is structurally unreachable (and self-contradictory)
The plan says: set tools.truncateToolOutputThreshold to 100000, run seq 1 20000, expect
Oversized results (above tool budget): >= 1 plus the compression-input yes line and the
/compress hint. Reproduced against the real code (03-calibration-and-scenario3.png,
logs/calibration-head.log):
S0 real `seq 1 20000` output 108,894 chars
S1 real shell in-tool gate (threshold: 30_000, retained 4,430 chars,
previewChars: 4000, keep both — shell.ts:2906) sentinel-prefixed + full spill file
S2 diagnostic on that history entry Oversized = 0 (plan promises >= 1)
S3 positive control: raw 108,894 chars sentinel-less in history → Oversized = 1
S4 same raw retention, tool without declared budget, threshold 100k → Oversized = 0
Why it cannot work:
- The shell tool truncates in-tool at a hardcoded 30k (
shell.ts:2919) before any scheduler
layer;tools.truncateToolOutputThresholddoes not influence it. Every large shell output enters
history sentinel-prefixed, and the diagnostic correctly skips sentinel-carrying results. - Even if the layers were bypassed for shell, the shell budget resolves from the registry
(ShellTool.maxOutputChars === 30_000) independently of the setting — so the plan's own math
(2×30k + 500) applies, but only to outputs the gate never produced. - For tools without a declared budget the raised threshold moves the boundary up (S4: a raw
108.9k retention is masked at boundary 200.5k) — the setting change weakens the alarm for exactly
the tools the fallback covers.
The plan's own note concedes "the counter fires only when a truncation layer is bypassed" — raising a
setting is not a bypass. The PR's E2E doc (scenario 3 there, expecting Oversized: 0) is correct;
the PR body step 3 (English: seq 1 20000, Chinese: seq 1 9000) is the artifact that needs fixing.
Suggested fix (text-only): rewrite step 3 to say the oversized branch is exercised deterministically
by the named unit tests because shipped mitigations make it unreachable in normal operation — which is
what S3 proves possible only via an actual bypass. The code itself needs no change (S2/S3 show the
shipped behavior is right).
F2 — Suggestion: the display-name UI-budget test passes for the wrong reason
Mutation M8 removed the entire displayNameBudgets lookup chain from doctorCommand.ts
(04-mutation-matrix.png, logs/mutant-M8-remove-displayname-map.log): 49/49 tests still pass.
The fixture of should resolve UI budgets by display name, not registry key is a 28,000-char Shell
display — below both the fallback boundary (2×25k = 50k) and the mapped boundary (2×30k = 60k), so it
is counted by neither arm. The test comment's arithmetic ("28k > 25k") ignores the * 2 tolerance of
the comparison it claims to test. The production code is correct: with a discriminating 55k fixture,
head stays green (map resolves Shell → 30k, 55k < 60k — logs/mutant-R1-head-55k.log) while M8 goes
red (falls back to 25k; 55k > 50k — logs/mutant-R2-m8-55k.log). The suite is green on both sides of
the fix axis, i.e. it pins nothing there; the 55k fixture is the one that would go red and should ship
with (or instead of) the 28k one.
F3 — Nit: E2E doc still describes a three-marker skip for the diagnostic
.qwen/e2e-tests/2026-08-10-doctor-memory-tool-result-retention.md (read from git show HEAD: — see
Methodology) says the oversized count skips results carrying "(prefix, in-body marker, or
<persisted-output> stub)". The diagnostic checks two prefix markers
(startsWith(TOOL_OUTPUT_TRUNCATED_PREFIX) || startsWith('<persisted-output>')); the in-body marker
belongs to the scheduler's separate isAlreadyTruncated check. This is the same R6-10 drift the final
commit (0c0e4fc5) corrected in design doc §5 — this instance was missed.
F4 — Nit: description numbers drift from the code
- "flags any result above a 30k-character threshold": in production the fallback budget is the
configured global truncation threshold (default 25,000,DEFAULT_TRUNCATE_TOOL_OUTPUT_THRESHOLD);
OVERSIZED_TOOL_RESULT_THRESHOLD_CHARS = 30_000is only the direct-API default, and
doctorCommandalways passes the configured value. The A/B run shows
oversizedThresholdChars: 25000under defaults. - English step 3 says
seq 1 20000is "~130k chars"; measured 108,894. The Chinese section names a
different command (seq 1 9000).
F5 — Completeness report (survivors classified; not merge conditions)
Mutation matrix on the new guards (positive control M0 landed one known mutant first — 2 sentinel
tests red — before any survivor was believed):
| Mutant | Guard removed/changed | Suite result | Classification |
|---|---|---|---|
| M0 | sentinel skip (!alreadyTruncated) |
killed (2 red) | pinned (positive control) |
| M1 | Number.isFinite(budget) |
survived 19/19 | dead code — rawChars > Infinity*2+500 is always false; arithmetic already protects infinite budgets. Harmless defensive clause |
| M2 | + 500 envelope slack |
survived 19/19 | coverage gap — no suite fixture in the (2x, 2x+500] band; harness cells A2 (60,500 → not flagged) / A3 (60,501 → flagged) pin the behavior as correct |
| M3 | error-key fallback (output ?? error) |
killed (1 red) | pinned |
| M4 | sentinel startsWith → includes |
survived 19/19 | coverage gap — no mid-string-sentinel fixture; harness cell B3 (sentinel mid-string still flagged) pins the prefix-only semantics as correct |
| M5 | oversizedThresholdChars Infinity→0 guard |
killed (1 red) | pinned |
| M6 | oversized never increments | killed (4 red, expected-vs-actual) | central assertion non-vacuous |
| M7 | canonicalToolName (CLI) |
killed (1 red) | pinned |
| M8 | displayName→budget map (CLI) | survived 49/49 | unpinned test — see F2 |
The boundary/sentinel/budget behavior itself was verified correct by the 29-cell harness
(logs/calibration-head.log), so the survivors are reporting about test pinning, not defects.
4. Targeted gates (all on head)
| Gate | Result |
|---|---|
packages/core retention + truncation + slimming suites |
96/96 pass (19 + 40 + 37) — matches the PR's claimed 19 core tests |
packages/cli doctorCommand.test.ts |
49/49 pass — matches the PR's claimed 49 |
packages/core coreToolScheduler.test.ts (scheduler-refactor gate) |
356/356 pass |
| eslint on all 7 changed files | clean (exit 0); liveness proven — a planted unused-var probe was reported (1 error, exit 1), then removed |
| typecheck/build | npm run build completed at HEAD before this round (CI contract); dist outputs verified present and executable |
Witness: 05-gates-and-equivalence.png. Raw logs in logs/gate-*.log, logs/lint-changed-files.log.
5. Not covered
- Real-session interactive E2E (plan steps 1–2 with a live model): no model credentials in this
sandbox. The step-2 shape was reproduced at the wire level: realseqbytes through the real
gate yield one retained result of a few thousand chars andOversized: 0(S1/S2), matching the
author's reported observation (4,543 chars for their command variant). Interactive-path rendering
is covered by the 49-test suite (which includes the interactiveaddItemcase). - Per-commit attribution: the checkout is depth-2;
git rev-list HEAD^1..HEAD^2yields 1 commit
while the metadata snapshot lists 9 — the shallow boundary makes intermediate commits unreachable.
Verified the aggregateHEAD^1..HEADdiff only. - Repo-wide gates: only the affected suites above were run (per the targeted-gates rule); no
repo-wide test, full lint, or independent typecheck pass. - Windows/macOS rendering: the report is platform-independent string output; nothing
platform-specific exists in the changed code. - Design-doc §2 persistence-gate internals (
maybePersistLargeToolResult) beyond constant
spot-checks (gate headroom 3k, web-search 102k = 100k+2k, grep 20k, agent 32k, batch 200k — all
match), and the mermaid diagram's layout. - Working-tree anomaly:
.qwen/e2e-tests/2026-08-10-doctor-memory-tool-result-retention.mdis
deleted in the checkout's working tree while tracked at HEAD; the containing directory is
root-owned and read-only here, so it could not be restored. Verified A/A-neutral: it is a markdown
doc consumed by no build or test. Contents were read viagit show HEAD:.
6. Methodology
Environment: CI verify container (node:22-bookworm), working tree at refs/pull/8875/merge
(depth 2); npm ci + npm run build pre-run at HEAD. Base arm: git worktree add tmp/base-tree HEAD^1, external deps symlinked from the (lockfile-unchanged) head node_modules, @​qwen-code/*
relinked to base packages, tsc --build per package (core clean; cli needed the repo's
generate-git-commit-info.js artifact, generated at the base commit). Runtime resolution purity
proven by a probe module beside each dist (realpath quoted in §1). Harnesses drive the compiled
dist/ directly — no mocks of code under test; oracles are hand-computed numbers and real-producer
bytes (seq 1 20000). Every assertion is scripted (harness/*.mjs, rerunnable); per-cell
stdout/stderr in logs/; mutant cycles applied via harness/apply-mutant.mjs +
harness/run-mutant.sh with git restore after each cycle and a clean-diff check. The two
harness-oracle fixes during the round (sentinel prefix length 48 not 49; barrel does not export
TOOL_OUTPUT_TRUNCATED_PREFIX, imported from the truncation dist module instead) were harness bugs,
not code behaviors — final logs reflect the corrected runs. Assertion accounting: A/B 23+9,
calibration 29, vitest gates 96+49+356, mutation predictions 11 (incl. R1/R2 reverse runs), lint 2
= 575; fail counts only unexpected outcomes (expected base-arm absence and predicted mutant
outcomes are encoded as passes).
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.11. |
5 participants
What this PR does
/doctor memorycan now tell you how much conversation memory is being held by tool results. It adds a "Tool result retention" section to the memory report that counts the tool results currently retained in the live session history, sums their total character footprint, reports the single largest result, and flags any result above a 30k-character threshold — with a hint to run/compresswhen oversized results are found. It also reports the two duplication signals called for by the issue: whether oversized outputs are also retained in the UI history (as rendered text items), and whether they are part of the compression input (they are, but compression reads the live history by reference, so no extra copy is held). The same stats are included in--jsonoutput so they can be attached to bug reports or parsed by tooling. Only sizes and counts are reported, never content, so the output stays safe to paste publicly. The scan reads the live history by reference (no cloning), so the diagnostic itself adds no memory pressure — which matters because users run it precisely when the process is already strained. The PR also ships the design note documenting the offload/preview state transitions and privacy model of the layered truncation mitigation, satisfying the issue's documentation acceptance criterion.Why it's needed
In long sessions, OOM risk usually comes from oversized tool outputs being retained in history and taxing every later turn, not from classic leaks. The mitigation side of this problem is already solved — oversized tool output is truncated and spilled to disk before it enters history — but the diagnostics side was missing: when a user still hits memory pressure,
/doctor memorycould only report process-level numbers (RSS, heap, handles) and had no way to answer "is retained tool output the cause?". This PR completes the remaining acceptance criteria of the linked issue: phase-1 retention diagnostics (all three signals: aggregate size/count, oversized flagging, and presence checks for UI history and compression inputs) plus the design note for the already-shipped mitigation. The oversized counter also doubles as a regression alarm: if any future change lets an unbounded result slip into history, the report names it by size and count immediately.Reviewer Test Plan
How to verify
Run the CLI (
npm run dev) and try three scenarios, confirming the report reflects live history state each time:/doctor memory. Expect the new "Tool result retention" section at the end of the report with all zeros,Oversized also in UI history ...: 0 item(s)andOversized also in compression input: no.yes demo | head -c 150000) and approve it. The output is spilled to a file by the existing truncation gate. Run/doctor memoryagain — expectTool results in history: 1with only a few thousand chars retained (the preview stub), andOversized results: 0with no hint line.tools.truncateToolOutputThresholdto100000in project settings, run a command likeseq 1 20000(~130k chars), then/doctor memory. ExpectOversized results (above tool budget): >= 1,Oversized also in compression input: yes (shared by reference, no extra copy), plus the/compresshint line. Remove the setting afterwards. (Note: the oversized check compares against 2x the tool's own budget plus a small envelope slack — for shell that is 2x30k + 500 = ~60.5k — so a retained result must exceed that to be flagged. Under normal operation the truncation gate prevents this; the counter fires only when a truncation layer is bypassed, which is the regression scenario the signal is designed to catch.)--json: run/doctor memory --jsonand confirm atoolResultRetentionobject withtoolResultCount,totalChars,largestResultChars,oversizedResultCount,oversizedThresholdChars,largeOutputsInUIHistory,presentInCompressionInputappears in the payload.Unit tests:
cd packages/core && npx vitest run src/utils/tool-result-retention.test.ts(19 tests) andcd packages/cli && npx vitest run src/ui/commands/doctorCommand.test.ts(49 tests).Evidence (Before & After)
Before (main): the report ends at
recommendation, with no visibility into retained tool output.After, empty session:
After, real session with multiple shell tool calls (truncation threshold raised to simulate a mitigation bypass; mitigation keeps every retained result at/below the shell tool's 30k budget):
Both captured via tmux against
npm run devwith real model interaction (full E2E report posted as a separate comment). The oversized "yes" paths (counter > 0, compression-input flag, hint line) are covered by unit tests in both packages, since the shipped mitigations make them unreachable in normal operation.Tested on
Environment (optional)
npm run devin an interactive tmux session with API key auth; no sandbox.Risk & Scope
Linked Issues
Closes #4184 — covers all three acceptance criteria: phase-1 retention diagnostics (this PR), the design note documenting the offload/preview state transitions and privacy model (this PR), and tests showing large outputs are bounded in hot memory/context while recoverable locally (landed earlier with the layered tool-output truncation work).
中文说明
这个 PR 做了什么
/doctor memory现在可以告诉你会话内存中有多少是被工具输出占据的。它在内存报告末尾新增 "Tool result retention" 段落：统计当前会话历史中滞留的工具结果数量、总字符数、最大单条结果大小，并标记超过 30k 字符阈值的结果——发现超限时额外提示使用/compress回收空间。同时报告 issue 要求的两个重复信号：超限输出是否也滞留在 UI 历史中（作为渲染文本条目），以及是否进入压缩输入（会进入，但压缩按引用读取活动历史，不产生额外拷贝）。同样的数据也包含在--json输出中，方便贴进 bug 报告或被工具解析。报告只包含大小和数量，绝不含内容，因此可以放心公开粘贴。扫描直接读历史引用、不做克隆，诊断本身不产生额外内存压力——这点很重要，因为用户正是在进程吃紧时才运行它。PR 还附带了记录分层截断缓解机制的 offload/preview 状态流转与隐私模型的设计说明，满足 issue 的文档验收标准。为什么需要
长会话的 OOM 风险通常来自超大工具输出滞留在历史里、给之后每一轮持续加负担，而不是传统意义的泄漏。这个问题的缓解侧已经解决——超大工具输出在进入历史前就会被截断并落盘——但诊断侧是缺失的：用户遇到内存压力时，
/doctor memory只能报进程级数字（RSS、堆、句柄），回答不了"是不是滞留的工具输出导致的"。本 PR 完成 issue 剩余的验收标准：phase 1 滞留诊断（全部三个信号：聚合大小/数量、超限标记、以及 UI 历史与压缩输入的存在性检查）加上已交付缓解机制的设计说明。超限计数器同时也是回归警报：未来任何改动如果让无界结果漏进历史，报告会立刻按大小和数量点名。评审验证方案
运行 CLI（
npm run dev）验证三个场景，确认报告实时反映历史状态：/doctor memory，预期末尾出现全 0 的 retention 段落，含Oversized also in UI history ...: 0 item(s)与Oversized also in compression input: no。yes demo | head -c 150000）并批准，输出会被现有截断门落盘；再次/doctor memory，预期Tool results in history: 1、仅几千字符滞留（预览 stub）、Oversized results: 0且无 Hint 行。tools.truncateToolOutputThreshold改为100000，执行seq 1 9000后/doctor memory，预期Oversized results (above tool budget): >= 1、Oversized also in compression input: yes (shared by reference, no extra copy)并出现/compress提示行；测完删掉该设置。（注意：shell 工具自身声明了 30k 硬预算，实际触发该路径需要 envelope 附加内容把滞留结果推过 30k——正是该信号要捕获的回归场景。）--json：执行/doctor memory --json，确认 payload 中出现toolResultRetention对象（含toolResultCount、totalChars、largestResultChars、oversizedResultCount、oversizedThresholdChars、largeOutputsInUIHistory、presentInCompressionInput）。单测：core 侧 9 个用例、CLI 侧 doctorCommand 42 个用例全部通过。
实测证据
改动前（main）：报告在
recommendation处结束，对滞留工具输出没有任何可见性。改动后空会话显示全 0 段落（含两条重复检测行）；真实多次工具调用会话显示滞留统计与重复信号均为安全状态（缓解机制把每条滞留结果压在 shell 工具 30k 预算内），均通过 tmux 真机交互截取（完整 E2E 报告另发评论）。超限的 "yes" 分支（计数 > 0、压缩输入标记、Hint 行）由两个包的单测覆盖——已交付的缓解机制使其在正常操作中不可达。风险与范围
关联 Issue
Closes #4184——三条验收标准全部覆盖：phase 1 滞留诊断（本 PR）、记录 offload/preview 状态流转与隐私模型的设计说明（本 PR）、以及证明大输出在热内存中被限制且本地可恢复的测试（此前随分层工具输出截断工作交付）。