feat(ui): add ui.showToolCallArgs to render tool-call arguments inline - #10565
feat(ui): add ui.showToolCallArgs to render tool-call arguments inline#10565TianYuan1024 wants to merge 20 commits into
Conversation
The main transcript folds read/search/list batches into a single "Read 3 files" summary line, and every tool header shows only the invocation's human summary — which for most built-in tools drops the parameters entirely (Edit shows just the filename). That leaves no way to see what a tool was actually called with while debugging an MCP server or a tool schema. Add an opt-in `ui.showToolCallArgs` (default false, so the compact baseline is unchanged). When on, the type-based partition is disabled so every call renders on its own row, and the raw arguments print as a JSON row under the tool header. Args are capped at 1000 characters inline; Ctrl+O lifts the cap and expands result output as before. Raw arguments are carried on the tool display object from both the live scheduler and the resume path, so a resumed session shows the same rows as a live one. The daemon path carries no args, so the row is skipped there, and the setting is TUI-only — the web shell is unaffected. MCP invocations already return their serialized params as the description, so the args row is suppressed when it would print the same payload twice.
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
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.
Test Plan (not a blocker): src/a.ts — no such file or directory.
中文说明
已审查。 建议见行内评论。
Test Plan（非阻断）：src/a.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Sanitize the arguments row through the shared sanitizeTerminalText pipeline. JSON.stringify escapes C0 controls and escapeAnsiCtrlCodes neutralizes ESC-prefixed sequences, but neither strips Unicode bidi overrides, so a malicious argument could visually reorder the very payload the row exists to expose (Trojan Source, CVE-2021-42572). The sanitizer runs last so the dedup comparison and the hidden-character count still read the raw JSON. Carry the raw arguments through the two remaining builders of the tool display object — the agent view and the accepted-speculation renderer — so the setting no longer half-applies, showing arguments in the main transcript but silently never in those views. The agent view needed the value threaded through its intermediate call map as well as the message metadata. Only tear down the compact partition when the group actually has arguments to show. Daemon-attached sessions carry none across the boundary, so gating on the setting alone expanded every fold while rendering zero argument rows — a noisier transcript that reads as "these tools were called without arguments". Memory-only groups now expand too, since the "Wrote 1 memory" badge hid exactly the parameters the setting recovers; groups of running parallel subagents deliberately keep their compact roster, because expanding them alongside the live agent panel reintroduces the viewport overflow and scroll-snap loop of QwenLM#5798. The setting description and the docs row now state both exceptions instead of promising every tool call. Reuse the exported key-hint constant in the truncation suffix so the message cannot drift from the actual binding, and pin the cap test to the exact output: it previously passed with the cap mutated to 100 or the hidden-character arithmetic broken.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Test Plan (not a blocker): src/a.ts — no such file or directory.
中文说明
已审查——无阻断问题。 建议见行内评论。
Test Plan（非阻断）：src/a.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
… cap The truncated arguments row tells the user to press Ctrl+O for the rest, but the agent view never passed full-detail down to its transcript items, so the key did nothing there. Thinking blocks in that view already honored the toggle, since the item renderer reads the context itself; only the tool side was missing. Read the same state the main transcript reads and forward it to both render sites, which makes the advertised escape hatch real rather than qualifying it away in prose. Extract the accepted-speculation display builder out of the submit handler so it can be unit-tested like the other three builders of the tool display object. It was the only one of the four with no test pinning that it carries the raw arguments, so a later refactor dropping them would have shipped green. The settings dialog renders the setting's description and was the only surface not mentioning that inline rows are truncated at 1000 characters; say so there, and stop calling the rows "full".
The new agent-view test built a ThoughtExpandedValue with `toggleExpanded` instead of `toggle`, which broke `tsc --build` and failed the CLI package build before any test or lint step could run.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. 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 explored to full depth (tool budget reached): chunk 3: executed verification of the three test files in my territory ( agentHistoryAdapter.test.ts , AgentChatContent.fullDetail.test.tsx , ToolGroupMessage.test.tsx….
Test Plan (not a blocker): src/a.ts — no such file or directory.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/messages/ToolMessage.tsx:932 — [probe] fullDetail→uncapped wiring untesteddocs/users/configuration/settings.md:140 — [review] settings.md conflates the two compact-view casespackages/cli/src/ui/components/messages/ToolMessage.tsx:1149 — [probe] Args cap splits surrogate pairs (UTF-16 slice)packages/cli/src/ui/components/agent-view/agentHistoryAdapter.test.ts:354 — [probe] Adapter tool_result merge branch unpinned for argspackages/cli/src/ui/components/messages/ToolGroupMessage.tsx:237 — [probe] Group-expand .some() quantifier unpinned
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 3：executed verification of the three test files in my territory ( agentHistoryAdapter.test.ts , AgentChatContent.fullDetail.test.tsx , ToolGroupMessage.test.tsx…。
Test Plan（非阻断）：src/a.ts — no such file or directory。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 5 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
The agent-view test fixture held only an assistant message, so every item was committed and the live area never rendered — the test captured one of the two render sites and a mutation dropping full-detail from the executing/confirming block survived it. Add an unmatched tool call, which the adapter maps to a still-executing tool group and the committed/live split keeps in the live area, and assert both sites receive the flag. Without this a running tool group would keep advertising ctrl+o for a key that does nothing on it, which is the half-bug the agent-view wiring set out to remove.
tmux e2e report — head
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- surrogate-pair split in the inline-args cap (ToolMessage.tsx:1149) — already reported (round-3 deferred list, review 5063380208)
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/a.ts — no such file or directory.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx:147 — [probe] parallel-agent panel exemption unpinned by tests
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：src/a.ts — no such file or directory。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Truncating the arguments row counted UTF-16 code units, so the cut could land between the two halves of a surrogate pair — an emoji or a supplementary-plane character in an argument — and leave a lone high surrogate that the terminal draws as a replacement glyph. Drop the orphan and count it among the hidden characters, so the reported total stays honest. Also pin the parallel-agent carve-out. Running agents deliberately keep their dense roster under this setting, because rendering them inline while the live agent panel also lists them overflows the viewport and triggers the clear-screen scroll-snap loop of QwenLM#5798. No test held that exemption in place, so a later edit made in the name of consistency could have reintroduced the bug silently.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
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/a.ts — no such file or directory.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/messages/ToolGroupMessage.tsx:230 — [review] D5-1: daemon 'no args across boundary' premise is false — rawInput is on the wire (transcript-replay.ts:359), the TUI adapter drops it, so ui.showToolCallArgs silen…
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：src/a.ts — no such file or directory。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
… frame The test rendered the real InlineParallelAgentsDisplay against the bare mock config, whose missing background-task registry made the render throw; ink swallowed it and the frame came out empty. Its only assertion was a negation, which an empty frame satisfies on its own, so the test held nothing in place. Give it the stubbed registry config the rest of this file already uses for that component, and assert the panel actually rendered before asserting the per-agent rows did not.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R6-1 settings.md conflates the two compact-view cases under a false shared reason (settings.md:140) — already reported (round-3 deferred list, review 5063380208)
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): chunk 4: executing ToolGroupMessage.test.tsx / ToolMessage.test.tsx to confirm green — blocked by the vitest globalSetup build guard, and the prerequisite npm run b….
Test Plan (not a blocker): src/a.ts — no such file or directory.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/messages/ToolMessage.tsx:1160 — [probe] D6-1: SessionPreview advertises a dead (ctrl+o) hint on capped args rows — the preview never forwards fullDetail nor handles Ctrl+Opackages/cli/src/ui/components/messages/ToolMessage.tsx:1160 — [probe] D6-2: inline-args truncation suffix is hardcoded English, bypassing the i18n t() every sibling transcript hint usesdocs/users/configuration/settings.md:140 — [probe] D6-3: 'running' qualifier understates the status-agnostic parallel-agent compact-view carve-out (schema + vscode schema too)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 4：executing ToolGroupMessage.test.tsx / ToolMessage.test.tsx to confirm green — blocked by the vitest globalSetup build guard, and the prerequisite npm run b…。
Test Plan（非阻断）：src/a.ts — no such file or directory。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@wenshao @chiga0 @yiliang114 Could you take a look when you have a moment? |
|
@qwen-code /triage |
|
Sandboxed verification: Skipped because the pre-execution risk screen refused this sponsored run: the diff adds long opaque single-line content. A maintainer who has reviewed the diff can run the verification manually in a disposable environment. 中文 — 判定：
|
|
Thanks for the PR! Template looks good ✓ — every section is filled in, including the bilingual summary and an honest "not driven in a live terminal" note. Problem: this answers an open feature request (#9767), not a hypothesis — users debugging MCP integrations currently have no way to see tool-call arguments inline, since the compact baseline summarizes them away and Direction: aligned. The retired Size: touches core-ish paths ( Approach: scope feels right for the stated goal. The two visible behaviors — unfolding information-gathering batches and printing the raw args row — are both necessary to satisfy the issue (collapsed rows would hide individual calls). The carve-outs are reasoned rather than scope creep: MCP payloads already surface as the tool description, so deduping them avoids double-printing; the daemon boundary carries no args, so the row is skipped there; the key is registered TUI-only so the web shell neither shows nor persists it. The follow-up about JSON-looking descriptions being dropped by the summary line is correctly left out of this PR. One thing I'll look at closely in code review is the always-on part — raw args now ride along on the tool display object even when the setting is off. Risk: no elevated risk signals — no revert-correlated paths touched. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 各部分齐全，包含双语说明，并如实标注了"未在实际终端中驱动"。 问题：本 PR 响应的是一个开放的 feature request（#9767），而非假设——调试 MCP 集成的用户目前无法在主视图中内联看到工具调用参数，因为精简基线会把参数摘要掉，而 方向：对齐。已移除的 规模：触及核心路径（ 方案：范围与目标匹配。两个可见行为——展开信息收集类批次、打印原始参数行——对满足 issue 都是必要的（折叠行会隐藏单个调用）。各处取舍是有理据的，不属于范围蔓延：MCP 的载荷本身就是工具描述，去重可避免重复打印；守护进程边界不携带参数，该行直接跳过；配置项登记为仅限终端界面，web shell 既不显示也不持久化。关于"长得像 JSON 的描述会被摘要行丢弃"的后续问题被正确地留在本 PR 之外。代码审查阶段我会重点看始终生效的那部分——即使设置关闭，原始参数现在也会随工具展示对象一起携带。 风险：无升级风险信号——未触及与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI wrote my independent proposal before reading the diff (schema key → carry the scheduler's existing Security surface is handled: the args row is model- and MCP-controlled text and goes through Two non-blocking notes:
Everything else follows house conventions: kebab-case hook file mirroring the Files changed (31 files, grouped into 23 rows)
Testing — the PR's own CI, read via the APICI on the reviewed head is fully settled — every
Sandboxed verification would settle the remaining gap: 中文说明代码审查读 diff 之前我先写了独立方案（设置键 → 把调度器已持有的 args 引用带到展示对象 → 在每个工具名下渲染有上限的单行 JSON → 停止批次折叠 → MCP 去重）。PR 与之一一对应，且有两处更好：展开门槛是 安全面已处理：参数行是模型与 MCP 可控文本，经过 两条非阻塞意见：
其余均符合仓库惯例：kebab-case 的 hook 文件镜像 测试 —— 通过 API 读取本 PR 自己的 CI被审 commit 上的 CI 已全部落定——所有 沙箱验证剩余缺口可用 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — a clean, well-tested delivery of a requested feature; the two nits (stale description line, always-on walk note) don't block. Stepping back: this is what a good fork contribution looks like. It answers an open feature request (#9767) with exactly the scope the issue asked for — an opt-in switch, default off, compact baseline untouched. My independent proposal and the diff converged on the same shape, and where the PR diverged it was smarter than my baseline: gating the unfold on renderable args rather than the raw setting keeps daemon-attached sessions compact, and wiring The author has clearly iterated — the commit history is a sequence of review-round responses (agent-view fullDetail, cap disclosure, surrogate safety, vacuous-test hardening), and the test suite is better than the diff had to be: oracle pinning against the documented 1000-char cap, a Trojan-Source bidi case, empty-frame traps closed with positive-then-negative assertions. CI on the reviewed head is fully green and fully settled. The one live-terminal signal is a third-party tmux report from two commits back; the delta since is exactly the surrogate-cap fix and two test commits, all unit-pinned, and the sponsored Not a 5 because of the one-liner in the PR body claiming the memory badge is untouched when the diff deliberately (and sensibly) bypasses it under the setting, and because the always-on escape-walk over carried args deserves to be remembered when someone profiles history rendering later. Neither blocks. Approving. 🚀 中文说明置信度：4/5 —— 对一个已立项需求的干净、测试充分的交付；两条小意见（描述过时的一行、始终生效的遍历说明）不构成阻塞。 退一步看：这是一次质量很高的 fork 贡献。它以 issue 所要求的范围回应了一个开放的 feature request（#9767）——一个可选开关，默认关闭，精简基线完全不变。我的独立方案与 diff 收敛到同一形状，而 PR 偏出的两处比我的基线更聪明：以"有可渲染参数"而非裸设置作为展开门槛，使守护进程会话保持精简；把 作者显然在多轮迭代——提交历史是一连串评审回应（子代理视图 fullDetail、上限披露、代理对安全、空测试加固），测试套件比 diff 的最低要求更好：对着文档承诺的 1000 字符上限钉住 oracle、Trojan Source bidi 用例、用先正向后反向的断言封住空帧陷阱。被审 head 上的 CI 全绿且全部落定。唯一的真实终端信号来自两份 commit 之前的第三方 tmux 报告；其后的差量恰好是代理对修复与两个测试 commit，均有单测钉住；若维护者需要，可用上文命名的代跑 没给 5 分的原因：PR 正文中有一行声称内存徽章未动，而 diff 实际在设置开启时有意（且合理地）绕过了它；另外，对携带参数的始终生效的转义遍历值得在日后有人给历史渲染做性能剖析时被记起。两者都不构成阻塞。 批准合并。🚀 — Qwen Code · qwen3.8-max Reviewed at |
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. ✅
Local verification report — head
|
| # | Claim | Method | Result |
|---|---|---|---|
| 1 | Default off ⇒ rendering unchanged | Same scenarios on the PR build and the merge-base build | Captures byte-identical |
| 2 | On ⇒ read/search/list batch stops folding, one row per call | Real TUI, 3 read_file calls |
✅ |
| 3 | On ⇒ raw args printed under each header, including params the summary drops | Real TUI, read + grep + edit | ✅ old_string/new_string visible |
| 4 | 1000-char inline cap with a +N chars marker |
1400-char and 200 000-char content args |
✅ +456, +199057; counts exact |
| 5 | Ctrl+O lifts the cap | Real key injection | ✅ full payload, no marker |
| 6 | Cap does not split a surrogate pair (2d461a3f) |
Argument aligned so the pair straddles index 999/1000, plus a hunk-revert counterfactual build | ✅ guard present → clean cut; guard reverted → U+FFFD |
| 7 | MCP payload printed once, not twice | Real stdio MCP server | ✅ |
| 8 | Resumed session shows the same rows | --resume into the session |
✅ identical |
| 9 | Daemon boundary carries no args ⇒ row skipped, view unchanged | Real reduceDaemonEventToTuiUpdates → real ToolGroupMessage |
✅ off and on identical |
| 10 | Key is TUI-only, web shell neither exposes nor persists it | Real qwen serve, GET/POST /workspace/settings |
✅ 400 disallowed_key; absent from the exposed list |
| 11 | Memory badge bypassed so the params are visible | 2 managed-memory writes | ✅ |
| 12 | Discoverable, no restart needed | /settings search + live toggle |
✅ on-screen history re-renders |
| 13 | Both render modes | ui.useTerminalBuffer true and false |
✅ identical |
| 14 | Args row is sanitised | Bidi override + ESC in an argument value | ✅ U+202E stripped, ESC JSON-escaped |
| 15 | Unit / lint / typecheck | 13 affected suites, eslint, tsc --build packages/cli |
526 passed, both clean |
1. The switch
Same build, same prompt, same three read_file calls — only the setting differs. The OFF pane is byte-identical to the same run on the merge-base build, so the default rendering really is untouched.
2. What the compact header hides
This is the case that motivates the setting: with it off, an edit shows only the filename and the strings it replaced never appear anywhere in the transcript.
3. The cap, and Ctrl+O lifting it
1456 − 1000 = 456, and the marker says +456. Pushed further: a 200 000-character content argument capped at … +199057 chars (ctrl+o), the TUI stayed responsive, and process RSS moved 174 064 KB → 174 112 KB across the turn.
4. The surrogate-pair guard, with a counterfactual
Attribution is by hunk-revert, not by comparing against an older head: I removed only the 0xd800..0xdbff guard from formatInlineToolArgs, re-bundled, and re-ran the same scenario. The bug reproduces (U+FFFD before the ellipsis, and the hidden count short by one) and disappears when the guard is restored. After Ctrl+O the emoji itself survives intact.
5. MCP de-duplication and the memory badge
6. The two boundaries
7. Discoverability, live toggle, resume
Notes (none blocking)
a. The hasRenderableToolCallArgs comment cites a case that isn't reachable — but the gate is still load-bearing for a different one.
The comment says gating on the raw setting alone "would expand every Read 3 files fold on an attached session". A daemon-attached session has no such fold to expand: TOOL_NAME_TO_CATEGORY is keyed on display names, and the daemon adapter sets name from the ACP kind, so isCollapsibleTool('read_file') === false while isCollapsibleTool('ReadFile') === true. Daemon groups already render one row per call, before and after this PR.
What the gate does buy is the empty-args case on the live path: a batch of calls invoked with {} stays collapsed instead of expanding into three rows that would each render no arguments.
isCollapsibleTool("ReadFile") = true <- live scheduler name
isCollapsibleTool("read_file") = false <- daemon adapter name (ACP kind)
live group, args = {}, setting ON -> ✓ Read file1.txt, file2.txt, file3.txt (gate holds the fold)
Worth a one-line comment correction; the code is right either way.
b. The MCP dedup compares serialisations, so it is key-order sensitive.
formatInlineToolArgs suppresses the row when description re-serialises to the same JSON. It correctly handles a pretty-printed description, but a description whose key order differs from args still gets a row:
MCP: description IS the args json -> undefined (suppressed)
MCP: description pretty-printed -> undefined (suppressed)
MCP: same json, different key ORDER in description -> {"owner":"Q","repo":"r"} (row rendered)
Not reachable through McpToolInvocation, whose getDescription() serialises the very same object, so this is informational rather than a defect.
c. The always-on cost is small but not zero.
The description says carrying args "costs no extra allocation" — that holds, it is the same reference. The traversal it adds to escapeAnsiCtrlCodes(item) is what has a price. Measured on the real function:
content arg size |
base shape (no args) |
PR shape (args carried) |
|---|---|---|
| 1 KB | 0.0008 ms | 0.0015 ms |
| 100 KB | 0.0008 ms | 0.0318 ms |
| 1 MB | 0.0003 ms | 0.3294 ms |
| 5 MB | 0.0003 ms | 1.6968 ms |
Per escape pass, memoised on history-item identity. At realistic argument sizes this is noise; I mention it only so the number is on the record rather than the qualitative claim.
Method / how to reproduce
- Builds: PR head
49bb0e8221and merge-base3a0c4c6108, each a fullnpm run build+npm run bundle, driven asnode dist/cli.js. - Provider: a local OpenAI-compatible SSE server that routes on the last user message and emits a fixed
tool_callsbatch per scenario (reads,mixed,bigargs,hugeargs,surrogate,mcp,memops), then plain text once tool results come back. - Surrogate alignment: filler length solved so
JSON.stringify(args).charCodeAt(999) === 0xD83DandcharCodeAt(1000) === 0xDE00; the oracle is the raw bytes oftmux capture-paneat the cut, not the rendered glyph. - MCP: a 30-line stdio JSON-RPC server exposing one tool, wired through
mcpServersin workspace settings. - Daemon boundary: the real
reduceDaemonEventToTuiUpdatesbuilds the group from asession_updateevent; that group is then rendered through the realToolGroupMessageunder both settings. qwen serve: real daemon on127.0.0.1:18566,--no-web --workspace <dir>;ui.compactModeused as the positive control for the same route.- Environment caveat: this box's
node_moduleswas missing@opentui/*,@tanstack/react-tableand some web-shell deps. Those are local install drift, not PR faults;packages/web-shellcould not be built here and is untouched by this PR anyway.
中文版报告
本地验证报告 —— head 49bb0e8221，Linux 环境
我从源码分别构建了本 PR 的 head 与其 merge-base（3a0c4c6108）（npm run build && npm run bundle），并在真实 pty（tmux，120×45）中驱动真实的 dist/cli.js，配合一个脚本化的 OpenAI 兼容 provider、一个真实的 stdio MCP 服务，以及一个真实的 qwen serve 守护进程。
这份报告是对此前针对 5c6ef8ce95 的 tmux 报告的补充：那个 head 落后四个提交，因此代理对错误对（surrogate pair）的保护（2d461a3f）与并行子代理测试修复（ea9bc0b5）都不在其覆盖范围内。
结论：未发现阻塞合并的问题。 PR 描述中的每一条行为主张都在 Linux 上复现成功，包括描述里标注为"仅推演、未端到端跑通"的那些。文末有三条不阻塞的说明。
PR 自带的表格把 🐧 Linux 标为
验证项
| # | 主张 | 方法 | 结果 |
|---|---|---|---|
| 1 | 默认关闭 ⇒ 渲染完全不变 | 同一批场景分别跑 PR 构建与 merge-base 构建 | 输出逐字节一致 |
| 2 | 开启 ⇒ 读取/搜索/列举批次不再折叠，每个调用独占一行 | 真实 TUI，3 次 read_file |
✅ |
| 3 | 开启 ⇒ 表头下方打印原始参数，包含摘要丢弃的参数 | 真实 TUI，read + grep + edit | ✅ 能看到 old_string/new_string |
| 4 | 1000 字符内联上限 + +N chars 标记 |
1400 与 200 000 字符的 content 参数 |
✅ +456、+199057，计数精确 |
| 5 | Ctrl+O 解除上限 | 真实按键注入 | ✅ 完整载荷，标记消失 |
| 6 | 上限不会切断代理对（2d461a3f） |
构造参数使代理对恰好横跨 999/1000，并做了 hunk 回退的反事实构建 | ✅ 有保护→干净截断；回退→出现 U+FFFD |
| 7 | MCP 载荷只打印一次 | 真实 stdio MCP 服务 | ✅ |
| 8 | 恢复会话呈现相同行 | --resume 回到该会话 |
✅ 完全一致 |
| 9 | 守护进程边界不带参数 ⇒ 跳过该行且视图不变 | 真实 reduceDaemonEventToTuiUpdates → 真实 ToolGroupMessage |
✅ 开关两态完全一致 |
| 10 | 该配置项仅限终端界面，Web Shell 不暴露也不写入 | 真实 qwen serve，GET/POST /workspace/settings |
✅ 400 disallowed_key；不在暴露列表中 |
| 11 | 绕过内存徽章以便看到参数 | 2 次托管内存写入 | ✅ |
| 12 | 可发现、无需重启 | /settings 搜索 + 实时切换 |
✅ 屏幕上已有的历史即刻重渲染 |
| 13 | 两种渲染模式 | ui.useTerminalBuffer true / false |
✅ 一致 |
| 14 | 参数行经过消毒 | 参数值中放入 bidi 覆盖符与 ESC | ✅ U+202E 被剥离，ESC 被 JSON 转义 |
| 15 | 单测 / lint / 类型检查 | 13 个受影响套件、eslint、tsc --build packages/cli |
526 通过，两者均干净 |
截图说明
- 开关本身：同一构建、同一提示、同样三次
read_file，只有设置不同。关闭态与 merge-base 构建逐字节一致，说明默认渲染确实未受影响。 - 精简表头隐藏了什么：这正是该设置存在的理由——关闭时编辑操作只显示文件名，它实际替换的字符串在整个对话里无处可见。
- 上限与 Ctrl+O：1456 − 1000 = 456，标记正好写
+456。进一步压测：200 000 字符的content参数被截断为… +199057 chars (ctrl+o)，TUI 保持响应，进程 RSS 在该轮前后为 174 064 KB → 174 112 KB。 - 代理对保护 + 反事实：归因方式是 hunk 回退，而非与旧 head 比较：我只移除了
formatInlineToolArgs里0xd800..0xdbff那段保护，重新打包并跑同一场景。缺陷复现（省略号前出现U+FFFD，且隐藏字符数少一），恢复保护后消失。按 Ctrl+O 后该 emoji 完整无损。 - MCP 去重与内存徽章。
- 两条不该越过的边界。
- 可发现性、实时切换与会话恢复。
说明（均不阻塞）
a. hasRenderableToolCallArgs 的注释举了一个实际到不了的场景——但这个把关本身在另一个场景下确实起作用。
注释说，若只用原始开关把关，"会把附着会话上的每一个 Read 3 files 折叠展开"。但守护进程附着的会话根本不存在这样的折叠：TOOL_NAME_TO_CATEGORY 是按展示名建索引的，而守护进程适配器用 ACP 的 kind 填 name，因此 isCollapsibleTool('read_file') === false，而 isCollapsibleTool('ReadFile') === true。守护进程分组在本 PR 前后都是每个调用一行。
这个把关真正买到的是实时路径上的空参数场景：一批以 {} 调用的工具会保持折叠，而不是展开成三行、每行都渲染不出任何参数。
isCollapsibleTool("ReadFile") = true <- 实时调度器使用的名字
isCollapsibleTool("read_file") = false <- 守护进程适配器使用的名字（ACP kind）
实时分组，args = {}，设置开启 -> ✓ Read file1.txt, file2.txt, file3.txt （折叠被保住）
值得改一行注释；代码本身两种说法下都是对的。
b. MCP 去重比较的是序列化结果，因此对键顺序敏感。
formatInlineToolArgs 在 description 重新序列化后与 args 相同时抑制该行。它能正确处理美化过的描述，但如果描述的键顺序与 args 不同，仍会渲染出参数行：
MCP：描述就是参数 json -> undefined （被抑制）
MCP：描述是美化打印的 -> undefined （被抑制）
MCP：同样的 json，但键顺序不同 -> {"owner":"Q","repo":"r"} （渲染出该行）
由于 McpToolInvocation 的 getDescription() 序列化的就是同一个对象，这条路径实际走不到，因此仅作说明而非缺陷。
c. 始终生效的那部分开销很小，但不是零。
描述里说携带 args"不产生额外分配"——这一点成立，它确实是同一个引用。有代价的是它给 escapeAnsiCtrlCodes(item) 增加的遍历。对真实函数实测：
content 参数大小 |
基线形状（无 args） |
PR 形状（携带 args） |
|---|---|---|
| 1 KB | 0.0008 ms | 0.0015 ms |
| 100 KB | 0.0008 ms | 0.0318 ms |
| 1 MB | 0.0003 ms | 0.3294 ms |
| 5 MB | 0.0003 ms | 1.6968 ms |
按每次转义遍历计，并且按历史条目标识做了 memo。在现实参数规模下这属于噪声；我列出来只是想把具体数字而不是定性描述留在记录里。
方法 / 复现
- 构建：PR head
49bb0e8221与 merge-base3a0c4c6108，各自完整npm run build+npm run bundle，以node dist/cli.js驱动。 - Provider：本地 OpenAI 兼容 SSE 服务，按最后一条用户消息路由，为每个场景（
reads、mixed、bigargs、hugeargs、surrogate、mcp、memops）发出固定的tool_calls批次，待工具结果回来后再回纯文本。 - 代理对对齐：解出填充长度，使
JSON.stringify(args).charCodeAt(999) === 0xD83D且charCodeAt(1000) === 0xDE00；判定依据是tmux capture-pane在截断处的原始字节，而不是渲染出的字形。 - MCP：一个 30 行的 stdio JSON-RPC 服务，暴露单个工具，通过工作区设置的
mcpServers接入。 - 守护进程边界：由真实的
reduceDaemonEventToTuiUpdates从session_update事件构建分组，再把该分组交给真实的ToolGroupMessage在开关两态下渲染。 qwen serve：真实守护进程监听127.0.0.1:18566，--no-web --workspace <dir>；用ui.compactMode作为同一路由的正向对照。- 环境说明：这台机器的
node_modules缺少@opentui/*、@tanstack/react-table及若干 web-shell 依赖。这是本地安装漂移而非 PR 的问题；packages/web-shell在此无法构建，而本 PR 也未改动它。
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Reviewed head 49bb0e82. One finding, inline; not blocking.
Scope. Source: all 13 non-test files, the docs row and the 9 locale files. The 4 large test patches (~635 lines) were read by title and then mutation-tested rather than read line-by-line. Not reviewed: behaviour in a real terminal (none available here), macOS/Windows.
CI on this head. Test (ubuntu-latest) is green — and that job also carries the generated-schema gate (ci.yml:540-551), so settings.schema.json staleness is covered by CI rather than by me. Test (macos-latest), Test (windows-latest), Integration Tests (CLI, No Sandbox) and Post Coverage Comment are SKIPPED; I ran the affected suites locally instead of inferring from them. No process.platform branch appears in this diff, so the platform skips cost little.
Ran locally (detached worktree at head, node 24.20):
- 8 affected suites: 333 tests pass.
- Efficacy matrix — 12 behaviours reverted one at a time, 12/12 caught: args-row JSX,
useShowToolCallArgs->false,hasRenderableToolCallArgsremoved fromforceExpandAlland fromallMemOpsComplete, the surrogate-pair trim, the MCP dedup, theargscarry-through in each of the four builders (live / resume / agent-adapter / speculation), theTUI_ONLY_SETTINGSentry, and the agent-view live-areafullDetail. That last one is the previously flagged "pinned by no test" — it bites now. - Height probe: the numbers quoted inline.
Cross-checked against the 7 earlier rounds. Of the 12 existing suggestions, 11 are fixed at this head. R6-1 is still open and I did not repeat it. Searching every prior comment and review body for availableTerminalHeight, height budget, countOneLineToolCalls, scrollback and live frame returns zero hits, so the inline finding is new to this thread — including to the round that cites #5798 itself, which used it only as a reason not to widen the parallel-agent gate.
Deferred to one-liners, no new threads (this PR is deep into review rounds):
docs/users/configuration/settings.md:140— "because no arguments are available to render" is false for the parallel-agent case: those calls do carry args, and the exemption atToolGroupMessage.tsx:305-310exists for the #5798 reason. Only the daemon case is a genuine absence. Same root asR6-1.ToolMessage.tsx:1155-1162— the cap and the+N charscounter are counted in UTF-16 code units while the rest of the file truncates viatoCodePoints, so the advertised hidden count is off by one per astral character. Cosmetic.
Checked and clean, so nobody re-litigates: TUI_ONLY_SETTINGS has a live consumer at workspace-settings.ts:141 and WEB_SHELL_SETTINGS is an add-list that does not contain the key; all 9 locale files updated; live-vs-resume MCP suppression holds because the resume path calls the same tool.build(args).getDescription() and the tool_result merge mutates in place so args survives; baseDisplayProperties is spread into every mapToDisplay arm; Ctrl+O reaches the agent view through a global handler with no view guard; IndividualToolCallDisplay is never serialized outside packages/cli/src/ui, so printing raw args adds no new durable write or wire exposure; requiresRestart: false is sound because the hook reads the context at render time.
I am leaving this at a comment rather than an approval for two reasons: there is one thing I would want looked at before this ships, and one dimension (a real terminal) I genuinely could not test. Neither reflects doubt about the rest — the four-builder symmetry, the sanitization and the test pinning are careful work.
Reviewed with AI assistance.
The `ui.showToolCallArgs` row renders outside every height budget `ToolGroupMessage` keeps: `availableTerminalHeightPerToolMessage` reaches only the result renderers, and `countOneLineToolCalls` still counts a result-less tool as exactly one line. Its only bound was 1000 characters, so at contentWidth 100 a single pending batch of six calls drew 72 rows into a 20-row frame — the condition under which ink's `shouldClearTerminalForFrame` wipes scrollback on every repaint (QwenLM#5798). Cap the row at two wrapped rows (still never more than 1000 characters, whichever is tighter), measured in columns so a full-width CJK argument wraps at half the character count, and reserve the `+N chars` marker inside that budget so the marker itself is not what spills over. Ctrl+O still lifts both caps. Also reserve the rows in `staticHeight`, the way `collapsibleSummaryHeight` already is, so the per-tool result budget does not hand out height the args rows have spent. The truncation now walks code points rather than UTF-16 code units, which removes the lone-surrogate special case and makes `+N chars` agree with the rest of the file's `toCodePoints` accounting — a code-unit count over-reported by one per astral character. Docs and the setting description now state the line cap. The `hasRenderableToolCallArgs` comment cited a daemon-side `Read 3 files` fold that cannot occur (`isCollapsibleTool` keys on display names, the daemon adapter fills `name` from the ACP kind); it now names the case the gate actually buys, an empty-args batch on the live path. New `ToolGroupMessage.heightBudget.test.tsx` renders the real ToolGroupMessage -> ToolMessage chain (the sibling suite mocks ToolMessage away, so it cannot see how tall the row draws) and pins the frame to the terminal height; dropping the width argument turns it red at 72 rows.
79bd583
|
@wenshao The The failureThat is the only red test in the job: It tracks the runner, not the PR
Sampling every
Four of the five hosted-runner jobs fail on this exact assertion, across three different PRs by three different authors. None of the six ECS jobs do. Locally, on an idle M-series Mac, both budget tests in that file finish in 247 ms. Why it started now#10648 ( - const startedAt = performance.now();
- expect(performance.now() - startedAt).toBeLessThan(1000);
+ const startedCpuUsage = process.cpuUsage();
+ const cpuUsage = process.cpuUsage(startedCpuUsage);
+ expect((cpuUsage.user + cpuUsage.system) / 1000).toBeLessThan(
+ maxClassificationCpuMs,
+ );The body under measurement is The previously green head on this branch, Not fixing it in this PRThe budget belongs to #10648 and to CI, not to a TUI PR, and whether the right answer is a larger budget, a runner-aware budget, or a return to wall clock is not my call. Happy to open a separate PR if that helps. 中文说明@wenshao 本 PR 上 失败内容这是整个 job 里唯一一条红的测试： 决定因素是 runner，不是 PR
抽样了最近几小时内所有能取到的
5 个托管 runner 的 job 里有 4 个挂在同一条断言上，分属 3 个不同作者的 3 个 PR；6 个 ECS job 全部通过。本地（空闲的 M 系列 Mac）该文件的两条预算测试共耗时 247ms。 为什么现在才开始失败#10648（ - const startedAt = performance.now();
- expect(performance.now() - startedAt).toBeLessThan(1000);
+ const startedCpuUsage = process.cpuUsage();
+ const cpuUsage = process.cpuUsage(startedCpuUsage);
+ expect((cpuUsage.user + cpuUsage.system) / 1000).toBeLessThan(
+ maxClassificationCpuMs,
+ );被度量的是 本分支此前绿的那个 head 不在本 PR 中修复这条预算属于 #10648 和 CI，不属于一个 TUI PR；到底该放宽阈值、按 runner 区分、还是改回墙钟计时，也不该由我决定。如果需要，我可以另开一个 PR。 |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Partially reviewed — one finding, inline; gaps disclosed.
Scope: all 32 changed files reviewed in the diff. Full test files read by diff. Cross-file context: ToolGroupMessage.tsx full (for height budget), useReactToolScheduler.ts live path, resumeHistoryUtils.ts resume path, agentHistoryAdapter.ts agent path, AppContainer.tsx speculative path, settingsSchema.ts, types.ts.
CI: web-shell E2E Smoke ✅. Integration Tests (CLI, No Sandbox) — SKIPPED (not a build or unit-test pass). Test (macos-latest, Node 22.x) — SKIPPED. Unit-test suite not run locally; no toolchain in this environment.
Not reviewed: platform runtime behavior (can only reason from code).
Cross-check against prior reviews:
Prior reviews (chiga0 @ head 49bb0e82): filed the height budget issue at ToolMessage.tsx:981. The current head fixes it for non-collapsible tools with inlineArgsHeight, but the gap for collapsible tools (below) was not raised. Confirmed as a new residual issue.
Bot reviews R1–R5: suggestions about agent path, bidi sanitization, description accuracy, speculative path test — all appear addressed in the current diff. R5 parallel-agent test vacuity concern was not re-raised here since that test's positive assertion (expect(frame).toContain('Parallel agents')) guards against the vacuous-pass risk.
Reviewed with AI assistance.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- docs exclusion sentence misdescribing both compact-view cases (docs/users/configuration/settings.md:141) — already reported (round-3 deferred list, review 5063380208; re-dropped as R6-1 in review 5068708701)
- agentHistoryAdapter.test.ts:354 tool_result merge branch unpinned for args — already reported (round-3 deferred list, review 5063380208)
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/a.ts — no such file or directory.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/ui/components/agent-view/AgentChatContent.tsx:265 — [probe] Critical [fails-closed] [new-surface] D7-1: Ctrl+O fullDetail unlocks uncapped rendering in the agent-view live area — no height backstop, #5798-class scrollback w…packages/cli/src/ui/components/messages/ToolGroupMessage.heightBudget.test.tsx:65 — [probe] D7-2: height-budget test never exercises the inlineArgsHeight reservation — dropping the reservation leaves both assertions green (mutation probe)docs/users/configuration/settings.md:141 — [probe] D7-3: settings.md asserts a batch-level height invariant the code does not provide (probe: 7-call batch draws 21 rows into a 20-row viewport)packages/cli/src/ui/components/messages/ToolGroupMessage.tsx:485 — [probe] D7-4: phantom 2-row height reservation for tools whose args row never renders (MCP/error-path dedup) — probe: 12 result rows lost for 6 MCP calls at H=73packages/cli/src/ui/components/messages/ToolMessage.tsx:1187 — [probe] D7-5: wide-char (emoji/CJK) args wrap to 3 rows against the 2-row reservation — real ink render, 103 of 207 widths violatepackages/cli/src/ui/components/messages/ToolMessage.test.tsx:2080 — [probe] D7-6: surrogate-pair cap test fixture sits at the cut boundary, not astride it — naive-slice mutant survives the shipped fixture (mutation probe)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：src/a.ts — no such file or directory。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 6 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R8-1 docs batch-level height guarantee (settings.md:141) — already reported (round-7 deferred list as D7-3, review 5078284431)
- R8-2 inlineArgsHeight reservation unpinned by tests (ToolGroupMessage.tsx:485) — already reported (round-7 deferred list as D7-2, review 5078284431)
- R8-3 phantom 2-row reservation for deduped MCP args rows (ToolGroupMessage.tsx:485) — already reported (round-7 deferred list as D7-4, review 5078284431)
- R8-6 'running' qualifier on the parallel-subagent carve-out (settings.md:141) — already reported (round-6 deferred list as D6-3, review 5068708701)
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 full unit suite timed out at the harness deadline under machine load (no full-suite verdict; affected suites ran green in targeted runs), and the test-efficacy probe was inconclusive.
Test Plan (not a blocker): src/a.ts — no such file or directory.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/ui/components/messages/ToolMessage.tsx:1187 — [probe] Critical [fails-closed] [new-surface] word-aware wrapping defeats the 2-line args cap; a pending batch can overflow the terminal (#5798)packages/vscode-ide-companion/schemas/settings.schema.json:473 — [probe] 'Does not change result-output truncation' is false under height pressure (probe: 29 -> 21 result rows)docs/users/configuration/settings.md:141 — [probe] daemon clause misdescribes the compact view; Ctrl+O can never show args theredocs/users/configuration/settings.md:141 — [probe] 'Useful when debugging MCP integrations' is a no-op for MCP callspackages/cli/src/ui/components/messages/ToolGroupMessage.tsx:407 — [probe] budget divisor counts results that shouldCollapseResult never draws (probe: 2/15 vs 15/15 rows)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 4 条建议级发现已在 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 full unit suite timed out at the harness deadline under machine load (no full-suite verdict; affected suites ran green in targeted runs), and the test-efficacy probe was inconclusive。
Test Plan（非阻断）：src/a.ts — no such file or directory。
收敛姿态下延后（第 8 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 5 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
7 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R9-1 height-budget test fixture cannot exercise the inlineArgsHeight reservation — already reported (round-7 deferred list as D7-2, review 5078284431; re-noted as R8-2 in review 5081609082)
- R9-2 phantom 2-row reservation for deduped MCP args rows — already reported (round-7 deferred list as D7-4, review 5078284431)
- R9-3 'Does not change result-output truncation' description claim — already reported (round-8 deferred list, review 5081609082)
- R9-4 surrogate-pair cap fixture mutation-blindness — already reported (round-7 deferred list as D7-6, review 5078284431)
- R9-5 docs batch-level height guarantee — already reported (round-7 deferred list as D7-3, review 5078284431; re-noted as R8-1 in review 5081609082)
- R9-6 agentHistoryAdapter tool_result merge branch unpinned for args — already reported (round-3 deferred list, review 5063380208; re-noted in review 5078284431)
- R9-7 'running' qualifier understates the status-agnostic parallel-agent compact-view carve-out — already reported (round-6 deferred list as D6-3, review 5068708701; re-noted as R8-6 in review 5081609082)
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/a.ts — no such file or directory.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round; 4 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/ui/components/messages/ToolMessage.tsx:1187 — [probe] Critical [fails-closed] [new-surface] CJK/full-width args wrap to 3 rows against the 2-line reservation at odd widths (re-confirms D7-5)packages/cli/src/ui/components/messages/ToolMessage.tsx:1220 — [probe] Critical [fails-closed] [new-surface] narrow-width marker floor overdraws the 2-row cap; innerWidth<=0 draws 1013 rows (new this round)packages/cli/src/ui/components/agent-view/AgentChatContent.tsx:265 — [probe] Critical [fails-closed] [new-surface] fullDetail lifts every height cap on the agent route, which has no backstop (re-confirms D7-1)packages/cli/src/ui/components/messages/ToolMessage.tsx:1003 — [probe] Critical [fails-closed] [new-surface] word-aware wrap defeats the 2-line args cap for spaced payloads (re-confirms round-8 deferred Critical)packages/cli/src/ui/components/messages/ToolMessage.test.tsx:2140 — [probe] uncapped full-detail sanitize guard has no test witness (new this round)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 7 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：src/a.ts — no such file or directory。
收敛姿态下延后（第 9 轮，非阻断）——已记录，本轮不要求修改；其中 4 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 5 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Deep verification (maintainer-local round) — verdict:
|
| cell | tree | setting | rendered tool region |
|---|---|---|---|
| A | head | off | folded summary (✓ Searched 'foo' in ~/ws/src, read …) |
| B | head | on | one row per call, each with its args JSON; edit exposes old_string/new_string |
| C | base | off | identical to A |
| D | base | on | identical to A/C — the setting is inert without this PR |
A ≡ C (default unchanged) · C ≡ D (load-bearing control) · B ≠ A (the change is real).
Secondary claims — all held
| claim | measurement | result |
|---|---|---|
| MCP payload printed once, not twice | real stdio MCP server; head+on frame ≡ head+off frame |
PASS |
| args row bounded to 2 wrapped rows | 4000-char write_file arg → exactly 2 rows + … +3955 chars (ctrl+o); on/off row delta = exactly 2 at 30, 60 and 100 cols |
PASS |
Ctrl+O lifts the cap |
truncated row becomes the complete JSON incl. closing brace; head-off+Ctrl+O ≡ base-on+Ctrl+O (result expansion unchanged) |
PASS |
| bidi overrides stripped on the new row | probe + live cell render {"content":"aaaXXXbbbccc"}; the result preview row still carries raw bidi on both base arms → pre-existing, not introduced here |
PASS |
Corrections to the PR description (description only, not code)
- "Arguments are capped at 1000 characters inline" is stale. At a known row width the binding cap is
min(1000, floor(rowWidth) * 2)columns — 188 columns at a 100-col terminal. ThesettingsSchema.tsdescription ("capped at 2 wrapped lines (and never more than 1000 characters)") is the accurate one; the body describes an earlier commit. - The illustrative "After" block invents two parameter names. It shows
absolute_path(ReadFile) andinclude(Grep); the real schemas areread_file: {file_path, offset, limit, pages}andgrep_search: {pattern, glob, path, limit}(read-file.ts:547,grep.ts:687). Since the feature exists to show the actual arguments, a reviewer diffing the body against a live session will see names that never occur.
Findings (both Suggestion, neither blocking)
F1 — the row-cap is violated for rowWidth ≤ 11. formatInlineToolArgs reserves the marker's columns and floors the payload budget at 1 (Math.max(1, budget - markerWidth)), so at small budgets it emits a ~23-column string into a rowWidth * 2 budget. Probe over the real exported function, rowWidth 1..120: violations=11 (w=1 rendered=23 allowed=2 … w=11 rendered=23 allowed=22) — precisely the overflow TOOL_ARGS_INLINE_MAX_LINES and the ToolGroupMessage height reservation exist to prevent. Reachability: innerWidth ≈ cols - 4, so the window is terminal ≲ 15 columns; the cap was measured holding at 30/60/100 cols, and the live consequence below ~20 cols is unmeasured (the harness could not drive the TUI that narrow meaningfully).
F2 — payloads that fit the row budget are truncated anyway, advertising "+1 chars". markerWidth is reserved before the code knows truncation is needed, so effective capacity is budget - markerWidth (167 cols at 100-col width) instead of budget (188). Measured band: 183–192-column payloads cut with hidden = 16..25. In the PR's own flagship scenario the live Edit row renders …"new_string":"export const a = 2;"… +1 chars (ctrl+o) — the hidden character is the closing }, so the row shows invalid-looking JSON plus a 19-column marker saying "1 char hidden" (visible in image 1).
Measured candidate fix for F1+F2 (applied in a scratch copy, then reverted)
Measure the whole payload against budget first and return it whole when it fits; reserve markerWidth only once truncation is known to be needed; return undefined when budget <= markerWidth + 1 (a row too narrow to hold its own marker). With the patch: F1 violations 11 → 0; F2 degenerate band 10 → 4 (the 4 residual are genuinely over budget). But one of the PR's own tests goes red — never cuts a surrogate pair in half at the cap boundary expects json.slice(0, 979) + "… +3 chars" for a 1000-code-unit payload whose column width is 999, i.e. it pins the reserve-first semantics. So the fix is not drop-in: that expectation is a decision the author owns. The other 539 assertions stayed green.
Gates and mutation matrix
- Affected suites: head 529/529 (10 files) · base 490/490 (8 files) ·
tsc --noEmit -p packages/cliclean. - Mutation "disable MCP de-dup" → 2 of the PR's tests red (
skips the row when the description already IS the args JSON (MCP),prints an MCP payload once, not twice) — guard pinned. - Mutation "
TOOL_ARGS_INLINE_MAX_LINES2 → 200" → 4 of the PR's tests red, including bothToolGroupMessage.heightBudgettests andbounds the row to two wrapped rows when the row width is known— cap pinned. - Post-restore re-run of the gate: 529/529 (tree left pristine).
Not covered
Daemon path (args row skipped) — reasoned from daemon-tui-adapter.ts:433 (name = ACP kind, so those groups never fold) but not executed; resume path (unit tests green, no live resume); web-shell rejection of the key (unit tests green, live route not hit); live consequence of F1 at ≤ ~15 cols; the hasRenderableToolCallArgs fold-teardown gate was not mutated (behavior proven by the live A/B instead); perf cost of the always-on escapeAnsiCtrlCodes(item) walk now traversing args (HistoryItemDisplay.tsx:267) — verified present, not measured; a read-only observation that inlineArgsHeight reserves 2 phantom rows for MCP tools whose row is suppressed by de-duplication; Windows/Linux; real-model sessions.
Methodology
Detached worktrees at the resolved baseRefOid/headRefOid, each with its own npm ci + build (both exit 0); root and packages/cli manifests byte-identical between arms, so the A/B is code-only. Frames captured with tmux capture-pane -p (assertions) and -e (screenshots above). Unit probes import the real exported formatInlineToolArgs with no mocks, run under the tree's own vitest. Raw logs, harness (ab-tui.mjs) and per-cell frames are in the local round's artifact dir; the probe source was deleted after the run.
5 participants
What this PR does
Adds an opt-in setting,
ui.showToolCallArgs(defaultfalse), that renders every tool call on its own row with its full raw arguments printed inline underneath the tool header, instead of the type-based compact summary. When it is off nothing changes — the compact baseline stays exactly as it is today.Turning it on does two things. It stops information-gathering batches (read / search / list) from collapsing into a single summary row, so each call is visible individually. And it prints the arguments the tool was actually invoked with as a one-line JSON row under each header. Arguments are capped at 1000 characters inline so a large file-content parameter cannot bury the conversation;
Ctrl+Olifts that cap, and continues to own result-output expansion exactly as before. The setting deliberately does not touch thinking-block folding or result truncation — it only ever adds the arguments row.The raw arguments are carried on the tool display object from both the live scheduler and the session-resume path, so a resumed session shows the same rows as a live one. They are held as the same object reference the scheduler already keeps, so there is no extra copy. The daemon boundary does not carry arguments, so the row is simply skipped there, and the key is registered as TUI-only — the web shell neither exposes nor persists it.
One detail worth calling out: MCP invocations already return their serialized parameters as the tool description, so for those the arguments row would print the same payload twice. It is suppressed in that case, and a description that merely resembles JSON still gets its arguments row.
Why it's needed
The main transcript folds information-gathering batches into a single summary line, and each tool header shows only the invocation's human-readable summary. For most built-in tools that summary drops the parameters entirely — an edit shows just the filename, a read just the path. There is currently no way to see what a tool was actually called with while it stays in the main view, which makes debugging an MCP server or a tool schema considerably harder than it needs to be.
This is intentionally a narrow switch for tool-call verbosity rather than a revival of the old global compact/verbose mode, which was deliberately retired. The default remains the single stable compact baseline; this only gives people who are debugging tool integrations a way to opt into seeing the calls in full.
Reviewer Test Plan
How to verify
Put
{"ui": {"showToolCallArgs": true}}in your settings and start an interactive session. Ask something that fans out into several file reads and searches. With the setting off you should see those calls folded into one summary row, as today. With it on, each call should occupy its own row with its arguments printed beneath it — including the parameters that are normally summarized away, such as the old and new strings on an edit.If you have an MCP server configured, invoke one of its tools and confirm its payload appears exactly once rather than twice. Then press
Ctrl+Oand confirm result output expands and the argument row is no longer truncated. Finally remove the setting and restart to confirm the view returns to the current compact baseline with no residue.The unit suites covering the affected areas pass, including new cases for the MCP de-duplication, the character cap and its lifting under full detail, unserializable arguments, the daemon path where no arguments exist, and the web shell rejecting the key.
Evidence (Before & After)
Rendering the real tool-group and tool-message components with a mixed batch (two information-gathering calls, an edit, and an MCP call):
Before (
ui.showToolCallArgsabsent orfalse— unchanged behavior)After (
ui.showToolCallArgs: true)Note the last row: the MCP payload is printed once, not twice, and the edit now exposes the parameters that the compact header omits.
Tested on
Environment (optional)
Unit tests and component-level rendering of the real components on macOS. The interactive session steps above were not driven end to end in a live terminal.
Risk & Scope
Separately, and left out of this PR on purpose: the compact summary line discards any description that parses as a JSON object, on the assumption that it is an error fallback. That misfires on MCP tools, whose descriptions are legitimately JSON. In practice MCP calls are never routed into that summary, so it is not reachable today — but it is worth a follow-up.
Linked Issues
Closes #9767
中文说明
这个 PR 做了什么
新增一个可选设置
ui.showToolCallArgs（默认false）：开启后，每个工具调用各占一行，并在工具名下方内联打印它被调用时的完整原始参数，取代当前按类型折叠的精简摘要。关闭时行为完全不变——精简基线与今天完全一致。开启后做两件事。一是让信息收集类的批次（读取 / 搜索 / 列举）不再折叠成单行摘要，每个调用都单独可见。二是在每个工具名下方以单行 JSON 打印该工具实际收到的参数。参数在内联时截断到 1000 字符，避免超大的文件内容参数淹没整个对话；按
Ctrl+O可解除该上限，并且它仍然像以前一样负责结果输出的展开。这个设置刻意不改动思考块折叠与结果截断——它只会新增参数行。原始参数由实时调度路径和会话恢复路径共同带到工具展示对象上，因此恢复的会话与实时会话呈现完全相同的行。参数持有的是调度器本来就持有的同一个对象引用，没有额外拷贝。守护进程边界不传递参数，那条路径会直接跳过该行；该配置项同时被登记为仅限终端界面使用，Web Shell 既不暴露也不写入它。
有一点值得单独说明：MCP 调用的工具描述本身就是序列化后的参数，因此对它们而言参数行会把同一份内容打印两次。这种情况下参数行会被抑制；而仅仅"长得像" JSON 的描述仍然会正常渲染参数行。
为什么需要
主对话视图会把信息收集类批次折叠成一行摘要，而每个工具名后面只显示该次调用的人类可读摘要。对大多数内置工具而言，这个摘要完全丢弃了参数——编辑只显示文件名，读取只显示路径。目前没有任何办法在主视图里看到工具实际是用什么参数调用的，这让调试 MCP 服务或工具 schema 变得比应有的困难得多。
这里刻意做成一个窄口径的工具调用详细度开关，而不是复活已被有意移除的全局精简/详细二态模式。默认仍然是那个单一、稳定的精简基线；本改动只是给正在调试工具集成的人一个选择，让他们能完整看到调用内容。
复核测试方案
如何验证
在设置中写入
{"ui": {"showToolCallArgs": true}}并启动交互式会话。提一个会触发多次文件读取与搜索的问题。设置关闭时，这些调用应当像今天一样折叠成一行摘要；开启后，每个调用应各占一行，参数打印在其下方——包括平时被摘要掉的参数，例如编辑操作的原字符串与新字符串。如果你配置了 MCP 服务，调用它的某个工具，确认其参数只出现一次而非两次。然后按
Ctrl+O，确认结果输出展开、参数行不再被截断。最后删除该设置并重启，确认视图回到当前的精简基线且没有任何残留。受影响区域的单元测试全部通过，其中新增用例覆盖了 MCP 去重、字符上限及其在全详情模式下的解除、无法序列化的参数、守护进程路径下参数缺失的情形，以及 Web Shell 拒绝该配置键。
证据（前后对比）
用真实的工具组与工具消息组件渲染一个混合批次（两个信息收集类调用、一个编辑、一个 MCP 调用）：
改动前（
ui.showToolCallArgs未设置或为false——行为不变）改动后（
ui.showToolCallArgs: true）请注意最后一行：MCP 的参数只打印了一次而不是两次；编辑操作则暴露出了精简表头省略掉的参数。
测试平台
运行环境（可选）
在 macOS 上运行单元测试，并对真实组件做了组件级渲染。上述交互式会话步骤未在实际终端中端到端跑通。
风险与范围
另外，有一点刻意未纳入本 PR：精简摘要行会丢弃任何能解析成 JSON 对象的描述，因为它假定那是错误回退产生的内容。这对 MCP 工具是误伤——它们的描述本来就是合法的 JSON。实践中 MCP 调用不会被路由进那条摘要，所以目前无法触发，但值得后续单独处理。
关联 Issue
Closes #9767