feat(web-shell): reveal full tool detail and auto-collapse finished tools - #5088
Merged
Conversation
…ools Long tool descriptions were hard-capped at 120 characters and finished tools (shell/edit/write) stayed expanded indefinitely, so commands were unreadable and the transcript filled with stale output. - Lift the 120-char description cap so the full command/path reaches the DOM; collapsed rows ellipsise via CSS (adapts to width) and a click reflows the full text into a wrapped block below the header. - Add a leading disclosure chevron; any row with detail output or a long description is now expandable. - Auto-collapse a tool to its one-line summary once it completes successfully. Running tools stay expanded (live output) and failures stay expanded (error visible); agents keep their own manual expand state.
qwen-code-ci-bot
requested changes
Jun 13, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Hey @wenshao — thanks for the PR, the expand/collapse UX looks like a real improvement. 👋
One thing before I dive in: the PR body doesn't follow our pull request template. The current headings (## What, ## Scope, ## Testing) don't match the required structure — we need:
## What this PR doesand## Why it's needed— so reviewers understand motivation without reading code## Reviewer Test Planwith### How to verify,### Evidence (Before & After), and### Tested on— this is the big one; without it, reviewers can't confirm the change works and review gets delayed## Risk & Scopeand## Linked Issues
Could you restructure the body to match the template? The content you already have is good — mostly a reorganization. Once that's done I'll pick the review back up.
中文说明
@wenshao 你好——PR 的展开/收起交互确实是个实用的改进 👋
不过在开始代码审查之前，PR 正文需要按照我们的 PR 模板 重新组织一下。现在的标题（## What、## Scope、## Testing）和模板要求的不一致，缺少：
## What this PR does和## Why it's needed—— 让审查者不读代码也能理解动机## Reviewer Test Plan，包含### How to verify、### Evidence (Before & After)、### Tested on—— 这个最重要，没有它审查者无法确认改动生效，审查会被延迟## Risk & Scope和## Linked Issues
能否按模板重新组织一下？现有内容质量不错，主要是结构调整。改完后我会继续审查。
— Qwen Code · qwen3.7-max
Collaborator
Author
|
@qwen-code-ci-bot — restructured the body to match the template: One note on Evidence: this is a DOM-behavior change and a live cross-browser screenshot isn't producible in my environment, so I pinned the observable before/after with jsdom unit tests (9 new, all green) plus an ASCII before/after and step-by-step verify instructions. Happy to add real browser captures if you'd prefer them. 中文说明@qwen-code-ci-bot —— 已按模板重组正文： 关于 Evidence 补充一句：这是一处 DOM 行为改动，我的环境无法产出真实跨浏览器截图，因此用 jsdom 单测（新增 9 个，全绿）+ ASCII 前后对比 + 分步验证说明来锁定可观测的前后差异。如果你更希望要真实浏览器截图，我可以补上。 |
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
Thanks @wenshao — the expand/collapse UX is a real improvement for anyone who's stared at a wall of stale tool output in a long web-shell session. Template looks good ✓ On direction: this solves two genuine readability papercuts in the web-shell transcript. The 120-char hard cap truncating before the DOM is a real bug — widening the window should help, and now it does. Auto-collapsing finished tools keeps the transcript scannable. Both changes are squarely in the web-shell's wheelhouse. No CHANGELOG reference in Claude Code, but the area is clearly relevant (transcript UX is core to the web-shell experience). On approach: the scope feels right — two focused changes, well-tested, no scope creep. The 中文说明感谢 @wenshao — 展开/折叠 UX 对长时间 web-shell 会话中堆满陈旧输出的问题是一个实质性的改进。 模板完整 ✓ 方向：解决了 web-shell 对话流里两个真实的可读性痛点。120 字符硬截断在 DOM 之前就截掉内容是真正的 bug——拉宽窗口理应有帮助，现在确实可以了。已完成的工具自动收起让对话流保持可扫读。两个改动都在 web-shell 的职责范围内。Claude Code CHANGELOG 无直接参考，但该领域显然相关（对话流 UX 是 web-shell 体验的核心）。 方案：范围合适——两个聚焦的改动，测试充分，无范围蔓延。 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewIndependent proposal (before reading the diff): I'd raise the 120-char hard cap in Comparison with the diff: the PR matches my proposal and exceeds it in a few details I wouldn't have thought of upfront:
No correctness bugs, no security concerns, no regressions spotted. The implementation follows project conventions — no over-abstraction, no unnecessary new files, tests are co-located. Test Results11 new tests, all passing ✓ — covering description relocation, chevron toggle, auto-collapse on finish (mount + transition), running-stays-expanded, failed-stays-expanded, agent regression guard, user-expanded override, long command truncation, and pathological length bounding. The 5 TypeScript: no errors in any of the PR's changed files ( Build: 中文说明代码审查独立方案（读 diff 之前）：我会提高 与 diff 对比：PR 与我的方案一致，且在几个我事先没想到的细节上更好：
无正确性 bug、无安全隐患、无回归。实现遵循项目惯例——无过度抽象、无多余新文件、测试就近放置。 测试结果11 个新增测试，全部通过 ✓——覆盖描述搬迁、chevron 切换、完成即收起（mount + 过渡）、运行保持展开、失败保持展开、Agent 回归守护、用户展开覆盖、长命令截断、病理长度边界。 5 个 TypeScript：PR 改动文件无错误。两个预先存在的无关测试文件错误。 — Qwen Code · qwen3.7-max |
Collaborator
|
Stepping back: this is a clean, well-scoped UX fix. The motivation is real — anyone who's used the web-shell for a long session knows the pain of scrolling past walls of stale tool output and truncated commands. The implementation matches my independent proposal and improves on it with The test coverage is thorough — 11 new tests that pin exact DOM-level observable outcomes, including the tricky edge cases (user-expanded override surviving completion, agent panel exclusion, live in_progress→completed transition). All pass. No type errors in changed files. This ships a focused improvement with no scope creep, no over-engineering, and the right abstractions at the right level. The code is straightforward — if I had to maintain this in six months, I'd thank the author for the clear separation between "should this row be expandable?" ( Approving. ✅ 中文说明退一步看：这是一个干净、范围合理的 UX 修复。动机真实——任何在 web-shell 长会话中扫过一堆陈旧工具输出和截断命令的人都能体会。实现与我的独立方案一致，并通过 测试覆盖充分——11 个新测试精确锁定 DOM 层面可观测结果，包括棘手的边界情况（用户主动展开在完成后保持、Agent 面板排除、运行中→完成的实时过渡）。全部通过。改动文件无类型错误。 这是一个聚焦的改进，无范围蔓延、无过度工程、抽象层级恰当。代码直白——如果六个月后要维护，我会感谢作者在"该行是否可展开"（ 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
previously approved these changes
Jun 13, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
commented
Jun 13, 2026
Contributor
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. |
ytahdn
reviewed
Jun 14, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
Additional suggestions (lines outside diff):
[Suggestion] toolFormatting.ts:218,222 — web_fetch URLs (truncateText(desc, 80)) and generic args.query (truncateText(String(args.query), 60)) still use hardcoded short caps inside getDescriptionFromArgs. This is inconsistent with the PR's design rationale of letting CSS handle truncation. With the current caps, expanding a web_fetch row still shows a truncated URL. Consider removing these inner truncations (the outer getToolDescription already applies MAX_DESCRIPTION_LENGTH = 2000 as a safety net).
[Suggestion] ToolGroup.tsx:672 — Agent rendering path omits the <ToggleChevron> spacer. Regular tool rows render <ToggleChevron ... /> as the first child of .lineMain (12px wide), but agent rows skip it and start with <StatusIcon>. In a mixed agent/regular tool group the icons are offset by 12px. Consider adding <ToggleChevron expandable={false} expanded={false} /> to agent rows.
…pand comment Review feedback on #5088: - shouldAutoExpand: rewrite the comment to match the code. Only the verbose kinds (shell/edit/write/ask) auto-expand and stay expanded on failure; other kinds are collapsed by default (their summary line shows the outcome and they stay click-to-expand). Force-expanding every failed tool was rejected because tools without an expanded-detail renderer would then hide the summary line and show an empty body — i.e. hide the error. - Auto-collapse-on-completion no longer overrides an explicit user toggle: a userToggledRef latch (set on header click, reset on tool-identity change) guards the collapse effect, so a row the user expanded/collapsed keeps its state when the tool finishes.
Review feedback (#5088): the expand test asserted container.textContent contains the command before and after the click, which passes regardless of whether the description is relocated from the header span to the wrapped block (textContent concatenates the whole subtree). Assert the DOM move instead — the command is in a leaf <span> while collapsed and in none while expanded — so a regression dropping the relocation now fails the test.
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
previously approved these changes
Jun 14, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
enabled auto-merge (squash)
June 14, 2026 01:16
wenshao
commented
Jun 14, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
Review feedback (#5088): a tool that is description-expandable but has no kind-specific detail renderer (grep/glob/web_fetch with a long description) used to lose its one-line result summary on expand and render an empty detail area instead. Gate both on a new hasDetailView(tool): the summary now stays visible (shown when collapsed OR when the expanded row has no detail view), and the empty detail div is no longer rendered. Description relocation is kept independent of detail content so long shell commands with short/no output remain click-to-reflow.
ytahdn
approved these changes
Jun 14, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
LGTM! ✅ — qwen3.7-max via Qwen Code /review
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jun 14, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
commented
Jun 14, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
| // ToolLine's lineDetail block below. Tools NOT in this set have nothing extra | ||
| // to show when expanded, so they keep their one-line result summary instead of | ||
| // hiding it behind an empty detail area. | ||
| function hasDetailView(tool: ACPToolCall): boolean { |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] hasDetailView duplicates the tool-name allowlist from hasExpandableContent (lines 69-85). Both functions hardcode the same set of names. Adding a new tool kind requires updating both functions in lockstep — if someone updates only one, hasDetailView silently stays out of sync, causing the result summary to hide behind an empty detail area on expand.
Consider deriving hasDetailView from a shared source of truth (e.g., a DETAIL_VIEW_TOOL_NAMES set that both functions reference, or a getDetailRenderer(tool) function that returns null when no renderer matches, making the two lists structurally impossible to desync).
— qwen3.7-max via Qwen Code /review
| /* Full, wrapped tool argument shown below the header when the row is expanded | ||
| (the header drops its single-line ellipsised copy). Aligns with the expanded | ||
| output block below it. */ | ||
| .lineFullArg { |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] .lineFullArg renders up to 2000 chars with pre-wrap and no max-height or overflow constraint. At maximum length this produces ~25-30 lines of wrapped monospace text that pushes all subsequent tool rows down with no scrolling. By contrast, .expandedOutput caps at max-height: 400px with overflow-y: auto.
Suggested change
| .lineFullArg { | |
| .lineFullArg { | |
| margin-left: 16px; | |
| margin-top: 2px; | |
| font-size: 12px; | |
| line-height: 1.4; | |
| color: var(--text-secondary); | |
| font-family: var(--font-mono); | |
| white-space: pre-wrap; | |
| word-break: break-word; | |
| overflow-wrap: anywhere; | |
| max-height: 200px; | |
| overflow-y: auto; | |
| } |
— qwen3.7-max via Qwen Code /review
| useEffect( | ||
| () => { | ||
| setExpanded(compactMode ? false : shouldAutoExpand(tool)); | ||
| // A new tool identity (or compact-mode toggle) resets the manual latch. |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] The userToggledRef.current = false latch reset on identity change has no test coverage. No test verifies that when tool.callId changes after the user manually toggled a row, the new tool's latch is properly reset and auto-collapse works correctly. If the reset line were accidentally removed, tools replacing a user-toggled predecessor would inherit the stale latch and never auto-collapse.
Consider adding a test that renders a running tool, clicks to expand (setting the latch), re-renders with a different callId and status: 'completed', and asserts the new tool is collapsed.
— qwen3.7-max via Qwen Code /review
| function isDescriptionExpandable(description: string): boolean { | ||
| return ( | ||
| description.length > DESCRIPTION_EXPAND_THRESHOLD || | ||
| description.includes('\n') |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] isDescriptionExpandable has two branches — description.length > DESCRIPTION_EXPAND_THRESHOLD and description.includes('\n') — but only the length branch is tested. A short (under 60 chars) description containing a newline should make the row expandable, but no test verifies this. A regression in the newline check (e.g., someone simplifies to length-only) would silently remove expandability for multi-line short descriptions.
Consider adding a test with a description under 60 chars containing \n (e.g., command: "echo a\necho b") and asserting the chevron appears.
— qwen3.7-max via Qwen Code /review
| args: { command: 'x'.repeat(5000) }, | ||
| }), | ||
| ); | ||
| expect(result.length).toBeLessThan(5000); |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] The assertion expect(result.length).toBeLessThan(5000) is too loose to verify the new MAX_DESCRIPTION_LENGTH = 2000 ceiling. This would also pass with the old limit of 120 or any other value below 5000, providing no regression safety for the actual boundary.
Consider tightening to expect(result.length).toBeLessThanOrEqual(2003) (2000 chars + "...") to pin the exact limit.
— qwen3.7-max via Qwen Code /review
This was referenced Jun 14, 2026
doudouOUC
pushed a commit
that referenced
this pull request
Jun 15, 2026
…ools (#5088) * feat(web-shell): reveal full tool detail and auto-collapse finished tools Long tool descriptions were hard-capped at 120 characters and finished tools (shell/edit/write) stayed expanded indefinitely, so commands were unreadable and the transcript filled with stale output. - Lift the 120-char description cap so the full command/path reaches the DOM; collapsed rows ellipsise via CSS (adapts to width) and a click reflows the full text into a wrapped block below the header. - Add a leading disclosure chevron; any row with detail output or a long description is now expandable. - Auto-collapse a tool to its one-line summary once it completes successfully. Running tools stay expanded (live output) and failures stay expanded (error visible); agents keep their own manual expand state. * fix(web-shell): preserve manual expand on completion; correct auto-expand comment Review feedback on #5088: - shouldAutoExpand: rewrite the comment to match the code. Only the verbose kinds (shell/edit/write/ask) auto-expand and stay expanded on failure; other kinds are collapsed by default (their summary line shows the outcome and they stay click-to-expand). Force-expanding every failed tool was rejected because tools without an expanded-detail renderer would then hide the summary line and show an empty body — i.e. hide the error. - Auto-collapse-on-completion no longer overrides an explicit user toggle: a userToggledRef latch (set on header click, reset on tool-identity change) guards the collapse effect, so a row the user expanded/collapsed keeps its state when the tool finishes. * test(web-shell): assert tool-detail relocation via DOM, not textContent Review feedback (#5088): the expand test asserted container.textContent contains the command before and after the click, which passes regardless of whether the description is relocated from the header span to the wrapped block (textContent concatenates the whole subtree). Assert the DOM move instead — the command is in a leaf <span> while collapsed and in none while expanded — so a regression dropping the relocation now fails the test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Improves how tool calls render in the web-shell transcript in two ways. First, the full tool description (the shell command, file path, or arguments) is now reachable: it was hard-capped at 120 characters in JS before reaching the DOM, so a long command was unreadable even on a wide screen — now the full text reaches the DOM, collapsed rows ellipsise via CSS (which adapts to window width), and clicking a row reflows the full description into a wrapped monospace block below the header, with a leading disclosure chevron (
▸/▾) marking expandable rows. Second, a tool now auto-collapses to its one-line summary once it completes successfully, while running tools stay expanded so you can watch live output and failed tools stay expanded so the error stays visible; sub-agent panels keep whatever expand state the user chose and are never force-collapsed on completion.Why it's needed
Two readability papercuts. A long shell command or path showed as
npm run build && npm run te…with no way to see the rest — the 120-char JS cap meant the remainder never even reached the DOM, so widening the window didn't help. And shell/edit/write rows stayed expanded forever, so a session with many tool calls filled the transcript with stale output and became hard to scan.Reviewer Test Plan
How to verify
npm --workspace packages/web-shell run devand open the web-shell.echowith a ~150-char argument). While it runs the row is expanded; once it completes it collapses to a one-line summary with a▸chevron.▾, and the full command reflows into a wrapped block above the output.Evidence (Before & After)
This is a DOM-behavior change verified via jsdom unit tests that pin the exact observable outcomes; a live cross-browser screenshot isn't producible in this environment, so the tests stand in as the evidence.
vitest run→ 261 pass (web-shell), including 9 new tests: full-command-reaches-DOM, chevron toggle, completed→collapsed (both on mount and on a livein_progress→completedtransition), running-stays-expanded, failed-stays-expanded, and a sub-agent regression guard (an expanded agent must survive completion).Tested on
Environment (optional)
Unit tests via
vitest(jsdom). Fullnpm run build(app + lib) passes;tscandeslintare clean on the changed files.Risk & Scope
TaskToolCallLine, shown only on history/session load) are intentionally unchanged — they are lightweight records with no expandable detail; no live cross-browser screenshot (verified via jsdom instead).Linked Issues
None.
中文说明
What this PR does（这个 PR 做了什么）
从两方面改进 web-shell 对话流里工具调用的渲染。其一，完整的工具描述（shell 命令、文件路径或参数）现在可见：它原本在进入 DOM 之前就被 JS 硬截断到 120 字符，所以一条长命令即使在宽屏上也看不全——现在完整文本进入 DOM，折叠行改由 CSS 省略（随窗口宽度自适应），点击工具行会把完整描述在表头下方换行成等宽块铺开，并以前置的展开三角（
▸/▾）标记可展开的行。其二，工具在成功完成后会自动收起成一行摘要，而运行中的工具保持展开以便看实时输出、失败的工具保持展开以便看到错误；子代理面板保留用户选择的展开状态，完成时绝不会被强制收起。Why it's needed（为什么需要）
两个可读性痛点。长 shell 命令或路径显示为
npm run build && npm run te…且无法看到剩余部分——120 字符的 JS 上限意味着剩余内容根本没进入 DOM，所以拉宽窗口也没用。而 shell/edit/write 行会一直保持展开，导致工具调用很多的会话历史区堆满陈旧输出、难以扫读。Reviewer Test Plan（审查者测试计划）
How to verify（如何验证）
npm --workspace packages/web-shell run dev并打开 web-shell。echo）。运行时该行展开；完成后收起成带▸三角的一行摘要。▾，完整命令在输出上方换行铺开。Evidence (Before & After)（前后证据）
这是一处 DOM 行为改动，通过 jsdom 单测精确锁定可观测结果来验证；此环境无法产出真实跨浏览器截图，故以单测作为证据。
vitest run→ web-shell 261 通过，含 9 个新增测试：完整命令进入 DOM、chevron 切换、完成即收起（mount 态 + 运行→完成的实时跃迁两种）、运行保持展开、失败保持展开、以及子代理回归守护（展开的代理在完成后必须保持展开）。Tested on（测试平台）
仅本地 macOS 验证（✅）；Windows / Linux 未测（⚠️ ，由 CI 覆盖）。
Environment (optional)（环境，可选）
通过
vitest（jsdom）跑单测。完整npm run build（app + lib）通过；改动文件tsc、eslint干净。Risk & Scope（风险与范围）
TaskToolCallLine，仅在历史/会话加载时出现）有意保持不变——它们是无可展开详情的轻量记录；未做真实跨浏览器截图（改以 jsdom 验证）。Linked Issues（关联 issue）
无。