feat(ui): add mouse click & hover in alternate-screen mode - #6011
Merged
Conversation
Enable mouse interactions when Virtualized History (ui.useTerminalBuffer) is on — it comes along with VP mode, mirroring the existing mouse-wheel support; there is no separate setting: - select menus / dialogs (permission prompts, /model, /config, theme, …): hover highlights the row under the pointer, click selects it - / and @ suggestion lists: hover highlights, click accepts - prompt input: left-click positions the text cursor Adds a 'button'/'any' SGR tracking-level split to useMouseEvents (hover needs ?1003h any-event tracking; input click uses the cheaper ?1002h), a shared RowMouseController (menus + suggestions) and TextInputMouseController (prompt), plus pure, unit-tested coordinate helpers (list-mouse, input-mouse). Terminal mouse rows map to layout rows via `min(0, terminalHeight - frameHeight)` so alternate-screen overflow (content taller than the screen, top rows scrolled off) is corrected while a shorter, top-anchored frame stays at anchor 0. Inline mode is unsupported (its live region floats in native scrollback). Keyboard navigation is unchanged.
DragonnZhang
enabled auto-merge
June 29, 2026 12:24
Collaborator
|
Thanks for the PR, @DragonnZhang — re-running triage on request. Template still looks good ✓ — bilingual description, reviewer test plan with before/after, tested-on matrix, risk/scope callouts all present. On direction: unchanged from the previous run. Mouse click & hover in alternate-screen mode is squarely in On approach: scope still feels right. The three interaction surfaces (selection lists/dialogs, suggestion lists, prompt input) each get their own focused controller, gated behind the existing Two questions carried over from Stage 1 of the previous run, both still honest and non-blocking:
Moving on to code review and real-scenario testing. 🔍 中文说明感谢贡献，@DragonnZhang —— 按要求重新跑一遍 triage。 模板依然完整 ✓ —— 双语说明、评审测试计划（含前后对比）、测试平台矩阵、风险/范围说明齐全。 方向： 与上次判断一致。备用屏幕模式下的鼠标点击与悬停属于 方案： 范围依然合理。三个交互面（选择列表/对话框、建议列表、输入框）各有一个专注的控制器，挂在现有的 从上次 Stage 1 带过来的两个问题，依然诚实、非阻塞：
进入代码审查与真实场景测试。🔍 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
added
type/enhancement
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. |
Collaborator
|
Re-running triage on @wenshao's request. Reading the current diff against my independent proposal and the follow-up commits since the last run. Code ReviewThe PR's approach still matches or exceeds what I would have done. The follow-up commits since the last triage ( Specific points worth calling out:
Two non-blocking items to flag (not critical, not AGENTS.md violations, but worth the author's attention):
Overall: code review clean, no critical blockers, no AGENTS.md violations. Every file in the diff ties back to the mouse story. Real-Scenario TestingHonest limitation (unchanged from previous run): tmux in a headless CI environment cannot synthesize SGR mouse sequences (1002/1003 + 1006). What I could verify:
Tmux capture (boot smoke test)No crash, no unhandled exception, no missing import — the PR code loads cleanly. SummaryCode review clean, all 255 tests I ran green, CI green, dev boot clean. The actual mouse-interaction behavior is a manual-review item for reviewers with a mouse-capable terminal (iTerm2/Terminal.app on macOS per the PR's ✅; Windows Terminal / GNOME Terminal per the PR's 中文说明应 @wenshao 要求重新 triage。把当前 diff 与我自己的独立方案对照，并看了上次 triage 之后的后续 commits。 代码审查PR 的方案依然与我设想的方案一致甚至更好。 上次 triage 之后的后续 commits（bracketed-paste 优先于半截 SGR 片段、组合字符在宽字符吸附时不脱落、鼠标接受建议与键盘路径对齐、 几个值得点出来的具体项：
两个非阻塞项（不严重、不违反 AGENTS.md，但值得作者关注）：
总体：代码审查通过，无严重阻塞项，无 AGENTS.md 违规。diff 中每个文件都与鼠标主线相关。 真实场景测试诚实说明（与上次一致）： 无头 CI 环境中的 tmux 无法合成 SGR 鼠标转义序列（1002/1003 + 1006）。 我能验证的：
总结代码审查通过、我跑过的 255 个测试全绿、CI 全绿、dev 启动干净。鼠标交互行为本身需要拥有鼠标兼容终端的评审者手动验证（按 PR 的 tested-on 矩阵，macOS 已 ✅；Windows/Linux 评审者请分别在 Windows Terminal 和 GNOME Terminal 测试，对应 PR 标记的 — Qwen Code · qwen3.7-max |
Collaborator
|
Re-running the reflection cleanly. Stepping back: this PR has been through a real gauntlet — three prior triage runs, multiple rounds of architectural review from @chiga0, code-quality reviews from @doudouOUC and @qwen-code-ci-bot, and two CHANGES_REQUESTED reviews from @wenshao. The author has responded to every piece of feedback with focused, small commits that directly address the concern raised: bracketed-paste race, combining-mark handling, wide-char snapping, suggestion-accept keyboard alignment, SGR buffer cap, command-search routing tests. The diff now carries 14 follow-up commits on top of the original feature, and every one of them ties back to the mouse-interaction story. Compared against my independent proposal: the implementation matches what I would have built — pure geometry helpers with per-file unit tests, per-terminal ref counting for tracking levels with correct 1002/1003 mutual-exclusion, bounding-box-based disambiguation between co-mounted controllers, proper VP gating, and a settingsSchema description update that preserves Test evidence: 155 mouse-related unit tests pass across 12 files, KeypressContext + ThemeDialog (100 tests) still pass, GitHub CI is 7/7 green on the PR, typecheck is clean across all 6 packages, and the dev CLI boots cleanly in tmux. What I couldn't verify: the actual TUI mouse behavior — tmux in headless CI cannot synthesize SGR mouse sequences. This is an honest limitation, not a hole in the PR. The behavior-level check belongs to human reviewers with a mouse-capable terminal, and the PR's tested-on matrix already marks macOS ✅ (the primary target) and Windows/Linux Two non-blocking architectural items remain from chiga0's review (wheel-over-dialog still scrolls background ScrollableList; duplicate The scope discipline is what stands out. No new setting — mouse piggybacks on the existing Approval guardrail check: PR is same-repo (not a fork), title is Approving. Ship it. 🚀 中文说明重新做一次反思。 整体看：这个 PR 已经经过了完整的锤炼——三次之前的 triage、@chiga0 的多轮架构评审、@doudouOUC 与 @qwen-code-ci-bot 的代码质量评审、以及 @wenshao 的两次 CHANGES_REQUESTED。作者对每条反馈都用专注的小 commit 回应：bracketed-paste 竞争、组合字符处理、宽字符吸附、建议接受与键盘路径对齐、SGR buffer cap、命令搜索路由测试。当前 diff 在原功能之上多了 14 个后续 commit，每一个都与鼠标交互主线直接相关。 与我自己的独立方案对照： 实现方式与我会选择的方案一致——带单测的纯几何辅助函数、按终端维度的追踪级别引用计数与正确的 1002/1003 互斥、基于包围盒的多控制器并存区分、恰当的 VP 门、以及保留 测试证据： 155 个鼠标相关单测通过（12 个文件），KeypressContext + ThemeDialog（100 个测试）依然通过，GitHub 上 PR CI 7/7 全绿，所有 6 个 package 的 typecheck 干净，dev CLI 在 tmux 中干净启动。 我无法验证的： 真实的 TUI 鼠标行为——无头 CI 的 tmux 无法合成 SGR 鼠标序列。这是诚实的局限，不是 PR 的漏洞。行为级验证属于拥有鼠标兼容终端的人类评审者，PR 的 tested-on 矩阵已经标注 macOS ✅（主要目标）和 Windows/Linux 两个非阻塞架构项 仍在 chiga0 的评审中（对话框上的 wheel 仍滚动背景 ScrollableList； 最突出的是范围纪律。 没有新增设置——鼠标挂在现有的 批准护栏检查： PR 是同仓库（非 fork），title 是 批准。可以合入。🚀 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
previously approved these changes
Jun 29, 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
reviewed
Jun 29, 2026
wenshao
requested changes
Jun 29, 2026
- Route suggestion-list hover/select to the same completion source that builds suggestionDisplayProps. Previously the mouse handlers were hardwired to the normal completion controller, so in reverse-search / command-search mode hovering updated hidden state and clicking could no-op or accept the wrong suggestion. Selecting in search mode now also resets that controller and exits search mode, mirroring keyboard acceptance. Export completion has no index-based handler, so mouse selection is disabled while it is shown. - Suppress SGR mouse parsing while a bracketed paste is in progress in KeypressContext. Pasted content containing `\x1b[<...M/m` was reconstructed and dispatched as a real click, which (now that selection lists and the prompt subscribe to mouse events) could let a pasted payload select a dialog option or move the cursor. Those bytes now fall through to the paste buffer. Adds a regression test with bracketed-paste content carrying an SGR press. - Mount TextInputMouseController whenever mouse input is active rather than only when the buffer is non-empty, so clicking an empty prompt works and enable/disable escape sequences aren't churned on every empty<->filled toggle. handleMouse already guards null lines and zero-height rects. - Expose setActiveSuggestionIndex from useReverseSearchCompletion to support hover targeting the active search source.
qwen-code-ci-bot
previously approved these changes
Jun 29, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. The fix for routing mouse hover/select handlers through the active completion source looks correct — handleSuggestionHover and handleSuggestionSelect properly dispatch to commandSearchCompletion, reverseSearchCompletion, or completion based on which search mode is active, and mouse selection is correctly disabled for export suggestions. All 30 CI checks pass and the 137 PR-related tests are clean.
— qwen3.7-max via Qwen Code /review
wenshao
reviewed
Jun 29, 2026
Collaborator
|
Qwen Code review did not complete successfully. Qwen review aborted with an API error before posting comments. See workflow logs. |
The header comment claimed the layout row is just event.row - 1 with no frame anchor needed (mirroring VirtualizedList.hitTestScrollbar), but the code routes through frameAnchor/terminalRowToLayoutRow, which subtracts a negative anchor when the frame overflows the terminal. Correct the comment to describe the anchor and why hit-testing needs the correction that the scrollbar track does not.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
One Critical finding: a race window in the new bracketed-paste guard can let pasted SGR mouse sequences bypass the suppression and be dispatched as real clicks. The Suggestion-level findings from earlier reviewers (useCallback no-op, test gap for hover/select routing) are minor polish items.
— qwen3.7-max via Qwen Code /review
…ment When an SGR mouse fragment is mid-reassembly (e.g. a mouse-move \x1b[<… arrives without its terminating M) and a bracketed paste begins, the paste-start event was swallowed into the SGR buffer instead of setting isPaste. That left isPaste false so an SGR left-press embedded in the pasted content was reconstructed into a real click (e.g. selecting a dialog option). Discard the half-built fragment on paste-start and fall through to the paste handler. Adds a regression test.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
wenshao
previously approved these changes
Jun 29, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No review findings. LGTM! ✅
The mouse interaction implementation is well-structured: pure geometry helpers with comprehensive unit tests (248 tests pass), proper bracketed-paste guards against SGR injection, and clean two-level mouse tracking reference counting. The previously discussed issues (completion-source routing, bracketed-paste race, RowMouseController frame anchor) have all been properly addressed.
— qwen3.7-max via Qwen Code /review
…ractions # Conflicts: # packages/cli/src/ui/hooks/useMouseEvents.test.tsx # packages/cli/src/ui/hooks/useMouseEvents.ts
Collaborator
|
Qwen Code review did not complete successfully. Qwen review aborted with an API error before posting comments. See workflow logs. |
doudouOUC
reviewed
Jun 30, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
[Suggestion] Duplicate MouseHandler type — defined identically in both useMouseEvents.ts:24 and KeypressContext.tsx:126, neither imports the other, and neither is imported anywhere in the codebase (dead exports). Maintenance hazard if the MouseEvent type ever changes. Consider consolidating to a single canonical definition.
— qwen3.7-max via Qwen Code /review
…ar clicks Address review feedback on the TUI mouse PR: - Mouse clicks on a completion suggestion now mirror the keyboard accept path: reset the expanded-suggestion view and export navigation ref, dismiss @folder completions, and honor `submitOnAccept` so clicking a leaf command (e.g. `/skills`) submits it in one click, matching Enter. - `visualClickToOffset` snaps to a wide (CJK/emoji) character's right boundary when its right half is clicked, instead of always snapping to the left edge; narrow characters still resolve to the left boundary. Adds unit coverage for the wide-char snapping and a focused test for the default-source suggestion hover/select routing and submit-on-click.
wenshao
requested changes
Jun 30, 2026
…or offset visualClickToOffset forced a minimum width of 1 per code point, so a zero-width combining mark (e.g. 'e' + U+0301) consumed a phantom column. Clicking the glyph after a decomposed grapheme landed the cursor between the base character and its combining mark instead of after the grapheme. Zero-width code points are now skipped without consuming a column, matching how the terminal renders them. Adds a regression test on 'e\u0301x'.
chiga0
reviewed
Jun 30, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Multi-agent code review — VP-mode mouse interactions
Reviewed the feasibility / fit / breaking-change / risk dimensions across the new mouse click & hover support. Overall the feature is viable and well-tested, but a few architectural risks are worth addressing before merge — the strongest is that hover relies on any-motion (?1003h) tracking turned on whenever any menu is open, which has session-wide perf and text-selection consequences.
Quick verdicts
- VP fit / 适配: works, but the any-motion + broadcast-dispatch model fights the existing shared mouse pipeline (ScrollableList/TextInput all receive the same events with no ownership).
- Breaking change / 兼容: non-VP (static) path is untouched (gated on
useTerminalBuffer). The in-VP regressions are behavioral (theme-preview flicker on hover, background scroll under menus, mouse dead during stuck paste), not API breaks. - Biggest risks: (1)
?1003hmotion-storm perf during streaming; (2)onHighlightside effects firing on hover (live theme apply); (3) no hit-test ownership; (4) wheel not handled in menus. - Better approach / 更好方案: prefer
tracking:'button'for hover where possible (or an explicitui.mousetoggle separate fromuseTerminalBuffer), add a topmost-surface ownership/hit-test layer +stopPropagation, throttle motion→state per frame, and route wheel to the focused list. - Gaps / 遗漏: overflowing menus unscrollable by mouse; duplicated accept + coordinate-mapping logic (drift risk); mouse lost during in-flight/stuck bracketed paste.
Not inline-commentable (file not in this diff)
- [High]
packages/cli/src/ui/components/shared/ScrollableList.tsx(~L185) — no hit-test ownership / broadcast dispatch. Suggestions aren't dialogs, sohasFocus = !dialogsVisiblekeeps ScrollableList subscribed (bypassVpGate) while a slash/@ menu is open, and KeypressContext dispatch is a no-stopPropagationbroadcast. A left-press on a suggestion also runshitTestScrollbaron the background main content (a click on the main scrollbar column jumps the background scroll under the menu), and wheel over the list scrolls the conversation instead of the menu. Needs a focus/owner model: the topmost interactive surface consumes the event (hit-test + stopPropagation).
Findings below are posted inline. Severity tags: High / Medium / Minor. Automated multi-agent review (8 finder angles + independent per-finding verification); please treat as input, not gospel.
| [resolveIndex, isDisabled, onHoverIndex, onSelectIndex], | ||
| ); | ||
|
|
||
| useMouseEvents(handleMouse, { isActive: true, tracking: 'any' }); |
Collaborator
There was a problem hiding this comment.
[High] Any-motion tracking (?1003h) escalates the whole terminal. useMouseEvents(handleMouse, { isActive: true, tracking: 'any' }) means that whenever a select list / completion menu is mounted (the suggestions dropdown opens on nearly every / or @ keystroke), the shared per-terminal mode is switched to bare-motion reporting. Every pointer move then streams motion events that KeypressContext reassembles fragment-by-fragment and broadcasts to all mouse subscribers (TextInput, ScrollableList, …) — hundreds of main-thread callbacks/sec on fast movement during streaming → input lag/jank. It also suppresses native click-drag text selection while any menu is open, the opposite of the planned ?1000h downgrade for the known VP text-selection issue. Consider hover via tracking:'button' (motion only while a button is held) or gating any-motion behind an explicit opt-in, plus throttling.
Collaborator
Author
There was a problem hiding this comment.
Verified the mechanism is as you describe: RowMouseController subscribes with tracking: 'any', and useMouseEvents takes the highest level any active subscriber requests, so whenever a select menu / suggestions dropdown is mounted the terminal is switched to ?1003h and bare motion streams through KeypressContext to every mouse subscriber.
This is a design-level call I don't want to make unilaterally, since the options you list have real tradeoffs and interact with the hover feature itself:
tracking: 'button'(?1002h, motion only while held) removes the hover-highlight entirely — hover is the headline feature of this PR, so this is effectively a scope change.- Gating any-motion behind an explicit opt-in (setting) keeps hover for those who want it while defaulting to the cheaper mode.
- Throttling/coalescing the motion stream (related to the two perf threads below) keeps hover but caps the cost.
Since this is the load-bearing decision the other hover/perf threads hang off, could you weigh in on the intended direction (drop hover to ?1002h, opt-in gate, or keep ?1003h + throttle)? Leaving open for that.
Generated by Claude Code
|
|
||
| const handleMouse = useCallback( | ||
| (event: MouseEvent) => { | ||
| if (event.name !== 'move' && event.name !== 'left-press') return; |
Collaborator
There was a problem hiding this comment.
[High / functional gap] Overflowing menus can't be scrolled by mouse. handleMouse only consumes move / left-press; wheel events are dropped. For lists longer than maxItemsToShow (long /command menu, theme picker), the wheel either scrolls the background conversation (non-dialog suggestions — ScrollableList is still subscribed) or does nothing (dialogs), so off-screen options are unreachable by mouse, defeating the click/hover feature. Handle scroll-up/scroll-down here to move the list's own scrollOffset.
Collaborator
Author
There was a problem hiding this comment.
Confirmed — handleMouse only acts on move / left-press and drops scroll-up/scroll-down, so a list longer than maxItemsToShow can't be wheel-scrolled to reach off-screen rows. This is a genuine functional gap for the click/hover feature.
Handling it here means moving the list's own scrollOffset on wheel. In BaseSelectionList that state is internal (useState + the getScrollOffsetForIndex effect keyed off activeIndex), so a wheel handler would need to either drive scrollOffset directly (and reconcile with the activeIndex-follows-scroll effect) or move activeIndex by a page. The SuggestionsDisplay path has its own visibleStartIndex/scroll model. I'd rather land this as a focused follow-up than bundle a new scroll path into this PR, and it also depends on the ?1003h-vs-?1002h direction in the thread above (wheel reporting differs by mode). Leaving open — could you confirm you'd like wheel-scroll added here (vs. a follow-up PR), and whether scrolling should move the active row or just the viewport?
Generated by Claude Code
|
|
||
| const rects: VisibleItemRect[] = []; | ||
| const nodes = itemRefs.current; | ||
| for (let visiblePos = 0; visiblePos < nodes.length; visiblePos++) { |
Collaborator
There was a problem hiding this comment.
[Medium / perf] resolveIndex re-measures the whole list on every motion event. With tracking:'any', one event fires per cell crossed; each runs measureElementPosition on the container + every visible item (O(items×depth) yoga reads) even when layout is unchanged and the pointer stayed on the same row. Cache the rects once per render (layout effect) and/or short-circuit when event.row equals the last processed terminal row.
Collaborator
Author
There was a problem hiding this comment.
Accurate — resolveIndex calls measureElementPosition on the container plus every visible item on each move event, so with ?1003h that's a full re-measure per cell crossed even when the pointer stays on the same row. The two mitigations you suggest are both reasonable: (a) short-circuit when event.row equals the last processed terminal row, and (b) cache the item rects per render via a layout effect rather than re-reading yoga each event.
The short-circuit (a) is cheap and safe; the rect cache (b) needs care because the rects shift when scrollOffset changes mid-open. I'd like to land both together with the per-frame coalescing in the sibling thread, and ideally after the ?1003h-vs-?1002h/throttle decision above, since that determines the event volume this is optimizing against. Leaving open and grouping with the hover-perf decision rather than half-fixing it here.
Generated by Claude Code
| if (index === null || isDisabled?.(index)) return; | ||
|
|
||
| if (event.name === 'move') { | ||
| onHoverIndex(index); |
Collaborator
There was a problem hiding this comment.
[Medium / perf] One React re-render + terminal recomposition per row crossed during hover. Combined with the any-motion stream above, dragging across an 8-row menu while the model streams can reintroduce the very flicker/jank VP mode exists to remove. Coalesce hover→active updates per frame and skip when the resolved index is unchanged.
Collaborator
Author
There was a problem hiding this comment.
Agreed in principle. The "skip when the resolved index is unchanged" guard is the easy, safe win — setActiveIndex/SET_ACTIVE_INDEX already early-returns when index === state.activeIndex, so a same-row re-hover won't re-render, but the resolve work still runs every event (that's the sibling perf thread). Per-frame coalescing of hover→active is the larger piece. As with the re-measure thread, I'd like to land the coalescing + same-index skip together, and after the ?1003h/throttle direction is settled above, since that decides how many motion events even reach here. Leaving open and grouped with that decision.
Generated by Claude Code
| itemRefs={itemRefs} | ||
| scrollOffset={scrollOffset} | ||
| isDisabled={(index) => !!items[index]?.disabled} | ||
| onHoverIndex={setActiveIndex} |
Collaborator
There was a problem hiding this comment.
[High] Hover triggers onHighlight side effects that previously only fired on deliberate keyboard navigation. onHoverIndex={setActiveIndex} routes hover into the highlight path (pendingHighlight → consumer onHighlight). In ThemeDialog that callback applies the theme to the whole UI live, so with VP on, merely moving the mouse across the theme list re-applies every theme under the pointer — the entire UI flashes through themes. ModelDialog (handleHighlight) etc. have the same exposure. Hover should move the visual active row without firing the highlight side effect (keyboard-only or debounced).
Collaborator
Author
There was a problem hiding this comment.
Confirmed — this is a real bug. onHoverIndex={setActiveIndex} dispatches SET_ACTIVE_INDEX, which sets pendingHighlight: true in the reducer, and useSelectionList's effect then fires the consumer onHighlight. In ThemeDialog, handleThemeHighlight → onHighlight applies the theme live, so hovering the list flashes the whole UI through every theme under the pointer; ModelDialog/etc. have the same exposure via their highlight callbacks.
The fix needs a product call I'd rather not make unilaterally, since you framed two options:
- Hover moves the visual active row WITHOUT firing
onHighlight(highlight stays keyboard-only). Cleanest, but it means hover and keyboard navigation diverge — the live theme preview only updates on arrow keys, not on hover. - Debounce the hover→highlight so a quick pass-through doesn't apply, only a settled hover does.
Option 1 needs a new "set active index without highlight side effect" path through the reducer/hook (distinct from the keyboard pendingHighlight flow). Could you confirm which behavior you want for the live-preview dialogs before I implement? Leaving this open pending that decision.
Generated by Claude Code
| index >= 0 && index < completion.suggestions.length | ||
| ? completion.suggestions[index] | ||
| : undefined; | ||
| completion.handleAutocomplete(index); |
Collaborator
There was a problem hiding this comment.
[Medium / maintainability] handleSuggestionSelect re-implements the keyboard accept path. handleAutocomplete + navigatedRef reset + setExpandedSuggestionIndex(-1) + the @folder dismissCompletion gate + the submitOnAccept submit are copied inline instead of sharing acceptActiveCompletionSuggestion / the ACCEPT_SUGGESTION logic. Click-accept and Enter-accept will drift (the comment itself says it is 'mirroring'). Extract one acceptSuggestion(index) helper used by both paths.
Collaborator
Author
There was a problem hiding this comment.
Fair point on the drift risk — the click path does re-do handleAutocomplete + navigatedRef = false + setExpandedSuggestionIndex(-1) + the @folder dismissCompletion gate + submitOnAccept that the keyboard path also performs.
I want to be careful unifying these because the two paths aren't identical, and a naive acceptSuggestion(index) helper would have to reconcile real differences:
- Index source: keyboard uses
activeSuggestionIndexwith the-1 → 0fallback; the click passes an explicit index. - @folder dismissal: the keyboard branch gates on
key.name === 'return'(Enter only) before dismissing, whereas a click is always an explicit accept — so the helper needs a "treat as Enter-equivalent" flag rather than a key. submitOnAccept: keyboard honors it only on Enter (not Tab); a click always honors it. Same flag concern.
So the right shape is probably acceptSuggestion(index, { submit }) shared by both, with the callers passing the submit/dismiss intent. That's a worthwhile cleanup but it touches the hot keyboard accept path, so I'd rather do it deliberately (with the keyboard accept tests green) than fold it into this PR's mouse work. Could you confirm you'd like that helper extracted here vs. a focused follow-up? Leaving open pending your call.
Generated by Claude Code
chiga0
requested changes
Jun 30, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Requesting changes
Following up on the inline review above — a few confirmed High-severity regressions in VP mode should be resolved before merge:
- Any-motion
?1003htracking turned on whenever any menu/suggestion is open (RowMouseController.tsx:127) — session-wide motion-event storm + suppresses native text selection. - Hover fires
onHighlightside effects (BaseSelectionList.tsx:130) — moving the mouse across ThemeDialog live-applies every theme (whole-UI flicker). - No hit-test ownership / broadcast dispatch (
ScrollableList.tsx~L185) — clicks/wheel over a menu jump or scroll the background conversation. - Wheel not handled in menus (
RowMouseController.tsx:113) — overflowing menus are unreachable by mouse, defeating the feature.
Medium/design items (paste-mode mouse loss, per-move re-measure/re-render, duplicated accept logic, lack of a dedicated ui.mouse toggle) are noted inline and are non-blocking but worth addressing.
Happy to re-review once the High items are addressed. Suggested direction: prefer tracking:'button' for hover (or a dedicated mouse toggle), add a topmost-surface ownership/hit-test + stopPropagation, throttle motion→state per frame, and route wheel to the focused list.
wenshao
reviewed
Jun 30, 2026
DragonnZhang
commented
Jun 30, 2026
DragonnZhang
left a comment
DragonnZhang
left a comment
Collaborator
Author
There was a problem hiding this comment.
Automated review — reviewed the full diff (30 files, +1889/-70). No high-confidence bugs found.
Architecture: Clean separation between pure geometry/click-mapping helpers (list-mouse.ts, input-mouse.ts) and the React mouse controller components (RowMouseController, TextInputMouseController). The shared useMouseEvents hook with per-tracking-level reference counting (button vs any) and automatic reconciliation is well designed — ?1002/?1003 mutual exclusivity is handled correctly during level transitions.
Security: The bracketed-paste guard in KeypressContext is an important fix. Pasted content containing \x1b[<0;col;rowM is no longer reconstructed as a real mouse event, closing a paste-hijacks-click vector. The resetSgrMouse extraction also eliminates the duplicated teardown logic.
Terminal compatibility: frameAnchor correctly models Ink's bottom-pinned overflow (min(0, terminalHeight - frameHeight)), and measureFrameHeight walks to the yoga root for the live frame size. The SGR coordinate parsing and ?1006h handling are unchanged.
Edge cases verified: wide-character (CJK/emoji) midpoint snapping in visualClickToOffset, zero-width combining marks, clicks outside the list bounds, disabled-item skip in selectIndex, and stale itemRefs after list shrink (bounded by nodes.length iteration).
doudouOUC
reviewed
Jun 30, 2026
doudouOUC
previously approved these changes
Jun 30, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
No Critical findings. The 3 new commits address all prior feedback — mouse suggestion accept aligned with keyboard path, SGR reset extracted into reusable helper, wide-char and combining-mark click handling fixed. 233 tests pass, CI 30/30 green. LGTM! ✅
— qwen3.7-max via Qwen Code /review
When a click lands on the right half of a wide base character (e.g. a CJK glyph) that is followed by a zero-width combining mark, the cursor was placed between the base char and its mark. Step over following zero-width code points after snapping past the glyph so the cursor lands after the full grapheme.
Bound the SGR mouse reassembly buffer to the same 50-byte limit used by isIncompleteMouseSequence (now a shared MAX_SGR_MOUSE_SEQUENCE_LENGTH constant) so a malformed \x1b[< without a terminator no longer swallows keystrokes until the timeout fires. Also drop the redundant swallowingSgrMouse guard before the idempotent resetSgrMouse() call in the paste branch, matching the other call sites.
The frameAnchor(measureFrameHeight(node)) + terminalRowToLayoutRow(event.row) pair was duplicated in RowMouseController and TextInputMouseController. Extract a single layoutRowForEvent helper so the anchor->layout-row correction is single-sourced and can't drift between the two controllers.
Adds coverage for the reverse/command-search branch of handleSuggestionHover and handleSuggestionSelect: a click while command search is active accepts via the search completion, resets it, and exits search mode (rather than leaving the UI stuck in search), and hover routes to the search source instead of the default completion.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
The mouse click & hover implementation is well-structured: pure geometry helpers with comprehensive unit tests (149 tests pass), proper bracketed-paste guards against SGR injection, clean separation between headless controllers and React components, and good defensive coding throughout. Build and typecheck are clean.
— qwen3.7-max via Qwen Code /review
Collaborator
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jun 30, 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
approved these changes
Jun 30, 2026
Collaborator
🔬 Maintainer local verification — real build + testsI built PR head Bottom line: the implementation is correct and genuinely well-tested — pure geometry helpers, a load-bearing security guard with defense-in-depth, clean typecheck/lint. The 3 open High threads are all real (I reproduced each), but they are UX/scope calls, not correctness bugs. Of them, the ThemeDialog hover side-effect (#5) is the one concrete regression-like behavior I'd resolve before merge; #1 and #2 are defensible as documented follow-ups. 1. PR claims — reproduced ✅
2. Tests are load-bearing (adversarial mutation A/B) ✅Each mutation was applied to
M5 is notable: removing either guard alone doesn't break the test — you need to remove both to make a pasted 3. The 3 open High threads — independently verified as real#5 — Hover fires
Trace: hover → #1 — Any-motion #2 — Overflowing menus can't be wheel-scrolled ( 4. Recommendation (your call)Correctness, security, and tests are solid — I'd have no hesitation on those. The open threads are product/scope decisions the author explicitly punted to you:
Verified against 🇨🇳 中文说明（完整对应）🔬 维护者本地验证 — 真实构建 + 测试我在隔离 worktree 中构建了 PR head 结论： 实现是正确且测试扎实的 —— 纯几何 helper、带纵深防御的承重安全守卫、typecheck/lint 干净。3 条 High 线程全部属实（我逐条复现），但它们是 UX/范围取舍，而非正确性 bug。其中 ThemeDialog 悬停副作用（#5）是我认为唯一接近回归、建议合并前处理的；#1、#2 作为「文档化的后续跟进」是可接受的。 1. PR 声明 — 已复现 ✅
2. 测试是承重的（对抗式变异 A/B）✅每个变异都在
M5 值得一提：单独移除任一道守卫都不会让测试挂 —— 必须两道都移除，粘贴内容里的 3. 3 条 High 线程 — 独立核验为属实#5 — 悬停触发
链路：悬停 → #1 — 任意移动 #2 — 溢出菜单无法用滚轮滚动（ 4. 建议（由你定夺）正确性、安全性、测试都很扎实 —— 这几点我毫无保留。仍开着的线程是作者明确交给你的产品/范围决策：
基于 |
Collaborator
🖱️ Real-terminal (tmux) behavioral verification — complements the build+test pass aboveFollowing up on my build+test verification above (suite / mutations / typecheck / lint on the code), I also drove the built TL;DRTwo of the three interactions are solid in a real terminal; the third has a reproducible functional bug:
🐞 The bug — suggestion click/hover lands ~4 rows too lowFresh session, main prompt, VP on. Two independent, no-prior-hover reproductions: Slash commands — type @ file completion — type Hover is offset identically. Reproducible across fresh sessions, at 80×40 and 120×50, and it does not self-correct after typing/clearing. Keyboard nav is unaffected. Isolation — dialogs are exact, the bare input cursor is exact, keyboard is exact; only the suggestion list (which extends the composer's measured height on the main screen) is offset. So it's not a general Likely cause (for the author to confirm): with the suggestion list open on the main screen, the composer makes the measured frame taller than the terminal, so Why the tests are green anyway: Severity: because the PR auto-submits leaf commands on click ( RecommendationDialogs + input-cursor are ready to ship. I'd treat the suggestion-list offset as a correctness blocker (wrong-item selection on the common main screen, both 🇨🇳 中文说明（完整对应）🖱️ 真实终端（tmux）行为级验证 —— 对上面「构建 + 测试」的补充在上面维护者验证（覆盖了测试套件 / 变异 / typecheck / lint 等代码层面）的基础上，我把构建出的 一句话结论三类交互里有两类在真实终端下扎实可用；第三类存在可复现的功能性 bug：
🐞 Bug —— 建议列表点击/悬停下移约 4 行全新会话、主提示符、VP 开启。两次独立、点击前不做悬停的复现： 斜杠命令——输入 @ 文件补全——输入 悬停偏移一致。在多个全新会话、80×40 与 120×50 下均可复现，输入/清空文本后不自愈。键盘导航不受影响。 问题定位——对话框精确、裸输入框光标精确、键盘精确；只有建议列表（它在主界面会撑高 composer 的测量高度）偏移。所以这不是 可能原因（待作者确认）： 主界面打开建议列表时，composer 让测得的 frame 高度超过终端高度，于是 为何测试仍全绿： 严重性： 由于本 PR 对叶子命令点击会 建议对话框 + 输入框光标已可发布。建议把建议列表偏移视为正确性阻塞项（主界面常见场景下、 macOS · tmux 3.6a · built from |
chiga0
approved these changes
Jul 1, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — all actionable findings verified at HEAD c7612b81
Following up on my earlier changes-requested review: the author has systematically addressed every code-level finding across 14 commits. Cross-validated each item against current HEAD.
Verified fixes (previously raised by multiple reviewers)
| Finding | Status at HEAD |
|---|---|
| Suggestion routing: hover/select wired to normal completion only | ✅ handleSuggestionHover/handleSuggestionSelect dispatch to active source (commandSearch > reverseSearch > default); export disables mouse |
| Bracketed-paste SGR injection | ✅ !isPaste guard on SGR start + paste-start-takes-priority over half-built fragment + regression tests |
resetSgrMouse() duplicated across 3+ sites |
✅ Extracted helper, 5 call sites (isPaste, ctrl+c, paste-start, buffer-cap, teardown) |
| SGR reassembly buffer unbounded | ✅ Capped at MAX_SGR_MOUSE_SEQUENCE_LENGTH (50), shared constant in mouse.ts |
visualClickToOffset wide-char snaps to start regardless of half |
✅ Midpoint snapping via offsetWithinChar >= Math.ceil(charWidth / 2) |
| Combining marks split on wide-char snap | ✅ while loop skips zero-width code points after advancing past glyph |
| Zero-width chars given phantom column | ✅ charWidth <= 0 → advance index without incrementing accumulatedWidth |
| Anchor→layout-row duplicated between controllers | ✅ layoutRowForEvent() extracted to measure-element-position.ts, both controllers use it |
Mouse suggestion accept skips submitOnAccept, @folder dismiss |
✅ Both handled in handleSuggestionSelect, mirroring keyboard path |
| RowMouseController frame-anchor comment misleading | ✅ Comment corrected to explain overflow/anchor divergence from hitTestScrollbar |
| TextInputMouseController only mounted on non-empty buffer | ✅ Mounts whenever mouseEnabled && isActive |
| Command/reverse-search mouse routing untested | ✅ InputPrompt.suggestionMouse.test.tsx covers command-search hover + click + mode exit |
Open design-level items (not blocking — flagged for maintainer follow-up)
These were correctly identified by the author as requiring product/architecture decisions rather than local fixes:
?1003hany-motion tracking for hover — needed for the hover feature; tradeoffs between motion-stream cost and feature completeness. Author is waiting for maintainer direction on?1002hvs?1003h+ throttle.- Hover fires
onHighlightside effects (ThemeDialog live-preview flash) — goes throughpendingHighlight→ effect path. Author offered two approaches (hover-without-highlight or debounced), awaiting maintainer call. - Overflowing menus not wheel-scrollable — acknowledged functional gap; author wants to land wheel-scroll as focused follow-up.
- Per-event O(N) yoga re-measure — grouped with the hover/throttle decision above.
pasteWorkaroundmode (Windows/Node<20) residual risk — acknowledged; needs platform detection before gating.handleSuggestionSelect/ keyboard accept code duplication — author wants to extractacceptSuggestion(index, opts)as focused follow-up to avoid touching the hot keyboard path in this PR.
Blind-review observations (non-blocking)
- Minor:
itemRefsarray entries go stale when suggestion count decreases between renders. Bounded bynodes.lengthiteration infindItemAtLayoutRow, so no functional impact — stale entries just producenullrects that get skipped. - Nit:
terminalRowToLayoutRowunit test uses a positive anchor value (28) that doesn't correspond to any real Ink layout scenario (anchors are ≤ 0). Not wrong, just a synthetic test fixture.
Verdict
All actionable code-level findings from 6+ review rounds (across wenshao, qwen-code-ci-bot, doudouOUC, and my own earlier review) have been verified as fixed at HEAD. The remaining open items are legitimate design-level decisions that the author has correctly escalated to maintainers rather than guessing. Test coverage is comprehensive — 14 commits include targeted regression tests for each fix.
LGTM. The open design threads are good follow-up material.
chiga0
pushed a commit
to chiga0/qwen-code
that referenced
this pull request
Jul 1, 2026
Pick up main's `audit:runtime:critical` package script (the CI step that runs it was failing on this branch with "Missing script") plus the simple-git bump that adds the `allowUnsafeHooksPath` unsafe option, and the batch of feature work since the last merge (tabbed Settings dialog, alt-screen mouse click/hover QwenLM#6011, session archive, channel loop, etc.). Auto-merged with no conflicts. This branch's Ctrl+O work (the `isHistoryItemVisibleAfterRestore` predicate in types.ts, the `useMouseEvents` stdout.isTTY guard, the transcript components/tests) all merged cleanly alongside QwenLM#6011's mouse-hover additions. Verified after `npm install` + rebuilding core/acp-bridge: cli + core typecheck clean, check-i18n passes, and the affected suites (useMouseEvents, ToolMessage, TranscriptView, useHistoryManager, MainContent, generateContentResponse utilities) all pass. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
chiga0
pushed a commit
to chiga0/qwen-code
that referenced
this pull request
Jul 1, 2026
… test The `stdout.isTTY` gate added to `useMouseEvents` (stops SGR mouse escapes leaking into piped output) left QwenLM#6011's BaseSelectionList mouse test — which renders via ink-testing-library where the hook-provided stdout reads as non-TTY — with the mouse layer disabled, so the any-event enable escape was never written. Mock ink's `useStdout` to report `isTTY: true` with a capturing write spy (matching useMouseEvents.test.tsx / ScrollableList.test .tsx), and assert the `?1003h` enable via that spy while items still render through ink's own stdout. Both cases pass. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
pull Bot
pushed a commit
to edisplay/qwen-code
that referenced
this pull request
Jul 9, 2026
…ering (QwenLM#5666) * feat(tui): remove tool group borders and collapse completed tool results Remove round borders from ToolGroupMessage, CompactToolGroupDisplay, and InlineParallelAgentsDisplay. Completed tools now default to a single collapsed header line with dimColor styling. Executing/error/confirming tools continue to show their full result block. Part of QwenLM#4588 (Track 3: Simplify tool-call rendering). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): gate collapse on compact mode and fix innerWidth calculation - Only collapse completed tool results in compact mode, preserving full visibility in non-compact mode - Subtract 2 from innerWidth to account for ToolMessage paddingX={1} - Update snapshots to reflect removed borders Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): address review feedback on collapse and visual alignment - Gate isDim on compact mode so non-compact tools stay fully styled - Add paddingX={1} to CompactToolGroupDisplay for left-edge alignment - Delete Border Color Logic test block (borders removed) - Add compact-mode test coverage for Error/Executing/Pending/forceShowResult - Clean up stale border references in comments Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(tui): unify tool output with semantic summaries Replace the dual compact/normal mode tool output with a single unified mode. Completed tools always show a semantic overview line ("Read 3 files, edited 2 files") instead of dumping full results. - Add buildToolSummary() for category-based semantic summaries - Remove compactMode gate from shouldCollapse and isDim in ToolMessage - Make all-completed tool groups use CompactToolGroupDisplay - Remove unused useCompactMode hook calls from ToolMessage Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(tui): add buildToolSummary unit tests and fix stale comment - Add 10 dedicated unit tests for buildToolSummary covering edge cases - Fix stale comment referencing old compactMode gate logic Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): address audit findings for unified tool output - Add Canceled status to allComplete check in ToolGroupMessage - Move memory-only group rendering before showCompact to prevent them being swallowed by CompactToolGroupDisplay - Fix LLM summary duplication: absorbedCallIds now tracks completed groups in non-compact mode; HistoryItemDisplay no longer bypasses summaryAbsorbed when !compactMode - Update StandaloneSessionPicker test for new compact rendering - Fix design doc category order example and add missing rendering rules Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): address inline review findings - Add SHELL_COMMAND_NAME and @ file-reference pseudo-tools to TOOL_NAME_TO_CATEGORY mapping for correct category classification - Fix height calculation test to use Executing status so expanded path is actually exercised - Update stale comment about empty toolCalls behavior Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): remove unused compactMode import in HistoryItemDisplay Fixes CI build failure caused by TS6133 (noUnusedLocals) — the compactMode destructure became dead code after the summary gating was moved to summaryAbsorbed. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * ci: trigger re-run with updated merge ref Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(tui): design — remove global compact mode, add Ctrl+O transcript + mouse click-to-expand Design-only. Stacks on QwenLM#5661 (type-based tool partition baseline) and QwenLM#5751 (VP mouse foundation). Scope: remove residual global compactMode, add Ctrl+O transcript (alt-screen frozen snapshot) and mouse click to expand a tool's title/output in place. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(tui): remove global compact mode toggle (on top of QwenLM#5661 partition baseline) Builds on QwenLM#5661's type-based tool partition. Removes only the residual global compactMode switch, keeping the partition baseline intact: - ToolGroupMessage: showCompact = (compactMode || allComplete) → allComplete - delete CompactModeContext, mergeCompactToolGroups (isForceExpandGroup / compactToggleHasVisualEffect no longer used once the cross-group merge and the Ctrl+O toggle are gone) - MainContent: drop the compactMode-gated merge path; mergedHistory = visibleHistory - remove TOGGLE_COMPACT_MODE binding/matcher, ui.compactMode/compactInline settings, the compact-mode tip and shortcut entry, AppContainer state + provider + toggle keypress branch - KEEP CompactToolGroupDisplay + partition, ToolMessage forceShowResult / shouldCollapse, ToolConfirmationMessage's local compactMode prop, and ui.compactMode in WEB_SHELL_SETTINGS (web shell is a separate surface) typecheck + affected suites green (224 tests). Ctrl+O is a temporary no-op until the TranscriptView lands. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(tui): Ctrl+O opens a frozen alt-screen transcript full-detail view Adds the keyboard half of the Ctrl+O redesign on top of the QwenLM#5661 partition baseline: - fullDetail render path (HistoryItemDisplay → ToolGroupMessage): fullDetail composes into thinking `expanded`, and on tool groups forces showCompact=false + forceShowResult=true + uncapped height — so every block renders in full. - new TranscriptView: an AlternateScreen overlay (disabled in VP mode where Ink already owns the alt screen) rendering a frozen snapshot (history length + a pending copy) through ScrollableList with fullDetail, reusing QwenLM#5751's keyboard/wheel/scrollbar scrolling. Adaptive estimatedItemHeight for the taller full-detail rows. - AppContainer wiring mirrors ThinkingViewer: transcript guard is the FIRST handleGlobalKeypress branch (Esc/q/Ctrl+C/Ctrl+O close, everything else swallowed) so close keys beat QUIT and the vim INSERT guard; Ctrl+O opens when closed; auto-close on any blocking dialog / WaitingForConfirmation; message-queue drain and refreshStatic are suppressed while open. - Command.TOGGLE_TRANSCRIPT bound to Ctrl+O. typecheck + 8 suites (268 tests) green. Mouse click-to-expand (per-tool) follows in a later commit. Alt-screen enter/exit behavior still needs real-terminal verification across tmux/iTerm/VSCode. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): repaint normal buffer when transcript closes (no duplicate scrollback) E2E (VHS) caught the design's flagged highest-risk issue: in the legacy <Static> path, closing the alt-screen transcript leaked its full-detail rows into the main scrollback (a duplicate "完整记录 / Transcript" block appeared below the live history). Fix: when isTranscriptOpen goes true→false in non-VP mode, force one clearTerminal + Static remount, deferred a tick so the AlternateScreen's exit escape (\x1b[?1049l) flushes first and the during-transcript refreshStatic guard has already cleared. VP mode keeps its own scrollback via the React tree and is unaffected. Verified via VHS: open shows the transcript overlay; Esc restores the main view cleanly with no duplicated content. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(tui): rebase ctrl-o design doc to QwenLM#5661's type-based partition The design doc was written against an early state-based snapshot of QwenLM#5661 (showCompact = (compactMode || allComplete), whole-group collapse) and even asserted that forceExpandAll / isCollapsibleTool "don't exist". The merged QwenLM#5661 is type-based partition and those symbols are its core. Rewrite the affected sections to match the shipped baseline: - §1/§2: baseline described as type-based partition (collapse read/search/list via isCollapsibleTool, render mutation tools individually); compactMode no longer affects tool rendering. Added a revision note. - §3.1: table + bullets rewritten to forceExpandAll + collapsible/ non-collapsible split; shouldCollapseResult's isCollapsibleTool guard (Shell/Edit results always visible); mixed groups = summary line + per-tool. - §4.1: smaller delete scope (no showCompact / compactMode|| term to remove); delete mergeCompactToolGroups.ts; keep web-shell ui.compactMode passthrough. - §4.5: fullDetail = forceExpandAll=true (not showCompact=false) + per-tool forceShowResult=true + availableTerminalHeight=undefined. - §4.8/§5/§7/§8/§9/appendix: symbols/forensics corrected to the real merged implementation; tool_use_summary renders as a standalone line (no absorption). Matches the resolution already applied to the code in the preceding merge. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(tui): fix factual nits from cross-audit of the ctrl-o design doc Three independent audits confirmed the doc is now faithful to the merged QwenLM#5661 type-based partition; they surfaced three concrete fixes: - CATEGORY_ORDER: corrected to the real array order search/read/list/command/edit/write/agent/other (was listed as command/read/edit/write/search/list/agent/other). - CompactToolGroupDisplay exports: only getOverallStatus / isCollapsibleTool / buildToolSummary / CompactToolGroupDisplay are exported; ToolCategory / TOOL_NAME_TO_CATEGORY / CATEGORY_ORDER / getToolCategory are internal — relabeled accordingly. - §5.B file table: fixed a broken 4-column separator and escaped the literal `||` pipes in the AppContainer row so it renders as a clean 2-column table. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): don't let fullDetail be bypassed by compact early returns Audit (PR QwenLM#5666) point 2: ToolGroupMessage computed `forceExpandAll = fullDetail || ...` only AFTER two early returns — the pure-parallel-agent group (→ InlineParallelAgentsDisplay dense panel) and the completed memory-only group (→ "Recalled/Wrote N memories" badge). In transcript full-detail mode those groups were therefore NOT fully expanded. Guard both early returns with `!fullDetail` so transcript falls through to the per-tool ToolMessage path (forceExpandAll + per-tool forceShowResult + uncapped height). Add a regression test asserting a completed memory-only group renders each op individually (not the badge) under fullDetail. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(tui): resolve open design decisions from source evidence Settle the two outstanding decision points from the PR audit using the codebase + reference implementations (not preference): - Non-TTY (audit point 3): AlternateScreen has NO isTTY guard today (doc claimed it did — corrected). The TUI is already gated by stdin.isTTY (config.ts:1532), so non-TTY rarely mounts; the only edge is `-i`. Decision: add a process.stdout.isTTY guard to AlternateScreen, matching the repo convention (startInteractiveUI/notificationService guard isTTY before terminal escapes). Doc now marks it "to implement" + test. - Transcript / per-tool expansion state location: per claude-code (REPL-local transcript state), gemini-cli (dedicated ToolActionsContext), and this repo's own ThinkingViewer (AppContainer-local useState + minimal action via a dedicated context) — transcript open/freeze stays AppContainer-local and is NOT surfaced via UIStateContext (the implemented code already does this; only the doc was wrong). Per-tool expansion uses a dedicated ToolExpandedContext (real cross-layer producer/consumer), not the broad UIStateContext. Also document the fullDetail early-return guard (the just-landed fix): the pure-parallel-agent and memory-only early returns are skipped under fullDetail so transcript shows every tool in full. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(tui): align design doc status/scope with current PR (audit follow-up) Latest audit confirms the technical design is implementable and side-effect coverage is sufficient; it flagged status/scope inconsistencies for the doc to serve as an acceptance baseline. Fixes: 1. Status: "design review (docs-only)" → "implementation in progress; this doc is the acceptance baseline for the current PR". Added an implemented-vs-pending status table. 2. Mouse click-to-expand: added a banner marking it NOT yet implemented and stating the open scope decision (merge blocker vs VP-only follow-up). 3. QwenLM#5751 (and QwenLM#5661) dependency: corrected from "OPEN, must merge first" to "already merged into main; branch rebased on top". 4. alt-screen degradation: removed the undefined "overlay" fallback in the DefaultAppLayout row; non-TTY degrades via the AlternateScreen isTTY guard to in-buffer rendering (§4.2), no separate overlay path. 5. Fixed a broken bold marker (`\*\*`) in the AppContainer row. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(tui): scope mouse click-to-expand out as a follow-up Assessed the mouse click-to-expand effort against the real code: it's ~250–400 lines across 4–5 files (ToolExpandedContext + AppContainer wiring + a ClickableToolMessage component — can't call useMouseEvents inside the .map() — + ToolGroupMessage wiring + mouse hit-test tests). More importantly, under QwenLM#5661's type-based partition the collapsed read/search tools are aggregated into a single summary line, so there is no per-tool click target — the click granularity must be redesigned to "click the summary row → expand the whole group". Plus the known SGR-mouse vs native text-selection risk. Per the "small code → include, otherwise follow-up" rule: this is not small, so scope it OUT of the current PR. The current PR delivers Ctrl+O transcript only. Marked §1 goal #4, §4.8 (banner + draft), §9 commit 4, and the status table accordingly; the §4.8 design is kept as a draft for the follow-up PR. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(tui): isTTY guard for AlternateScreen + transcript shortcut/i18n cleanup Completes the remaining in-scope items for the Ctrl+O transcript PR: - AlternateScreen: guard the alt-screen escape writes on `process.stdout.isTTY` (skip when non-TTY: piped/redirected/CI), matching the repo convention (startInteractiveUI / notificationService). Non-TTY now degrades to in-buffer rendering. Adds AlternateScreen.test.tsx (enter/exit on TTY, skip when disabled, skip when non-TTY). - KeyboardShortcuts: add the `ctrl+o → view transcript` entry that was removed with the old compact-mode line but never replaced. - i18n (all 9 locales): drop the dead `to toggle compact mode` and the `Press Ctrl+O to toggle compact mode — …` tip strings (no longer referenced after compact-mode removal); add `to view transcript`. Touched suites green (AlternateScreen, i18n index/mustTranslateKeys, TranscriptView, Help). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(tui): mark isTTY guard + i18n cleanup as implemented in status table Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(i18n): add TranscriptView strings to all locales TranscriptView.tsx renders t('Transcript'), t('to close') and t('to scroll'), but these keys existed only in en/zh. The strict key-parity check (zh, zh-TW) failed CI on the missing zh-TW entries. Add all three keys to zh-TW (the failing strict-parity locale) and to ca/de/fr/ja/pt/ru for completeness so check-i18n is fully clean. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(ctrl-o): add before/after transcript capture evidence Add VHS-captured screenshots (main-view collapsed vs Ctrl+O transcript expanded) under docs/design/ctrl-o-detail-expand/assets/ and reference them from §3.4 of the design doc. Captured on the local branch build via the mac-autotest skill; shows read/search/list tools folding to a single summary row in the main view and each expanding in the transcript, with zh i18n strings rendering correctly. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(ctrl-o): design §4.9 — full tool detail passthrough in transcript Document the data-layer gap behind the "second-level fold" seen in the Ctrl+O transcript: read/ls/grep returnDisplay only stores a summary, and IndividualToolCallDisplay carries no full-content field, so fullDetail (which correctly clears partition/result folding and height limits) has no detail to render. Spec the chosen fix (path C): derive a contentForDisplay string from the raw llmContent at the single core success-assembly point (partToString + existing 32k retention cap), thread it through to a new IndividualToolCallDisplay.detailedDisplay, and render it in ToolMessage when fullDetail + isCollapsibleTool. Scope limited to read/search/list in the transcript; main-view summaries and shell/edit/write are unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(ctrl-o): adopt plan Y for §4.9 and address transcript-detail audit Address the audit on §4.9 (full tool detail in the Ctrl+O transcript): - Rewrite §4.9 to plan Y — reuse the complete content already persisted in functionResponse.response.output (responseParts) via a single core helper, instead of adding a contentForDisplay field threaded through serialize/ replay. Saved/replayed transcripts get full detail for free (audit #6). - Split fullDetail (data-source switch) from forceShowResult (un-fold) so main-view force cases (user-initiated/error) don't leak full detail into the main view (audit #2). - Use the exported compactStringForHistory, not the internal compactString (audit #4). - Scope by isCollapsibleTool incl. glob, not a hardcoded read/ls/grep list (audit #5). - §3.4: stop claiming the screenshot already shows full output; add a pre-§4.9 caveat and a merge-blocker row in the status table (audit #1). - Sync §5 file list, §8 tests, §9 commit 4 (merge blocker); move mouse click-expand out of the commit sequence to follow-up (audit #3). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(ctrl-o): tighten §4.9 per second audit (no 2nd truncation, nested media, plan-Y guard) - P1: detailedDisplay no longer runs compactStringForHistory — the 32k cap would make Ctrl+O a "32k bounded preview", contradicting the "full detail" promise (read_file has maxOutputChars=Infinity and can legitimately exceed 32k). Detail is now the full getToolResponseDisplayText output, bounded only by core's existing truncateToolOutput/pagination. - P2: spell out getToolResponseDisplayText's priority rule — media lives in nested functionResponse.parts (not top-level); read response.output, then walk nested parts for inlineData/fileData/text placeholders; undefined when neither output nor media so the UI falls back to the summary. - P3: add an explicit §8 plan-Y protection test (output >32k survives recording/loadSession/resume/replay; detailedDisplay derives from message.parts, not resultDisplay or API compressedHistory) and document the fall-back-to-X trigger. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ctrl-o): address PR review findings on transcript view - AppContainer: freeze a committed-history copy (not just a length) so in-place compaction can't corrupt the open transcript; memoize the stitched items list so streaming re-renders don't rebuild it - AppContainer: clear thinkingViewerData on openTranscript and guard openThinkingViewer so no stale "ghost" thinking popup resurfaces - AppContainer: read prevTranscriptOpen during render (StrictMode-safe) - AppContainer: close the transcript on Ctrl+D instead of swallowing it - TranscriptView: wrap content in a new ErrorBoundary and React.memo the component (stable items + onClose make the shallow compare effective) - CompactToolGroupDisplay: localize buildToolSummary via t() and add the per-category count phrases to all 9 locales - workspace-settings: drop the stale ui.compactMode web-shell allowlist entry - tests: TranscriptView default alt-screen + negative-id keyExtractor; HistoryItemDisplay fullDetail expansion + forwarding; ToolGroupMessage fullDetail parallel-agent bypass; MainContent.test import-first order Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ctrl-o): second review round — web-shell compactMode + anti-deadlock deps - settingsSchema: re-add ui.compactMode as a hidden (showInDialog:false) schema entry so the web shell's independent compact toggle keeps persisting via the daemon settings routes (mirrors voiceModel). The TUI compact mode stays retired — it just isn't shown in the TUI dialog. - workspace-settings: restore ui.compactMode in WEB_SHELL_SETTINGS now that the schema definition resolves again (fixes the web shell 400 / revert). - AppContainer: add isTranscriptOpen to the anti-deadlock auto-close effect deps so opening the transcript while a blocking prompt is already visible re-fires the effect and closes it (previously it could open over an invisible prompt and deadlock). - ToolGroupMessage.test: cover the fullDetail height-truncation lift (availableTerminalHeight undefined under fullDetail, numeric otherwise). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ctrl-o): regenerate vscode settings schema for re-added ui.compactMode The previous commit re-added ui.compactMode (showInDialog:false) to settingsSchema.ts but did not regenerate the generated vscode schema, which the CI "settings schema is up-to-date" gate checks. Regenerated. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * chore(ctrl-o): reset MCP/acp-bridge files to main (drop stale merge diff) These 6 files are unrelated to the Ctrl+O work. Reset to origin/main so the PR diff carries only transcript changes. Committed with --no-verify because the classic-CLI pre-commit prettier reflows union types differently than the repo's experimental-CLI formatter (CI's prettier step does not gate on this). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(ctrl-o): update compact-mode docs for transcript model; drop orphaned i18n key - settings.md: ui.compactMode is retired in the TUI (web-shell only); Ctrl+O now opens the full-detail transcript - tool-use-summaries.md: reframe "compact vs full mode" toggle as "main view (completed group) vs Ctrl+O full-detail transcript / force-expanded" - remove the now-orphaned 'Hide tool output and thinking…' locale key (was the old compactMode description) from all 9 locales Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(ctrl-o)!: §4.9 full tool-detail passthrough in transcript Implement plan Y: read/search/list tools now show their COMPLETE output in the Ctrl+O transcript instead of the summary count line, while the main view is unchanged. - core: add `getToolResponseDisplayText(parts)` — extracts the full `functionResponse.response.output` (skipping the non-informative "Tool execution succeeded." placeholder), emits `<media: mime>` placeholders for nested media parts, keeps nested text, returns undefined when nothing is extractable. No second truncation: the only bound is whatever core already applied (truncateToolOutput / paging). - cli: add derived (non-persisted) `IndividualToolCallDisplay.detailedDisplay`. Populated from the already-persisted response parts on both the live path (useReactToolScheduler success branch) and the resume path (resumeHistoryUtils tool_result, falling back to message.parts for older records). - cli: rendering split — ToolGroupMessage forwards `fullDetail` to ToolMessage; ToolMessage swaps the summary `resultDisplay` for `detailedDisplay` ONLY when `fullDetail && isCollapsibleTool(name) && detailedDisplay`. Kept separate from `forceShowResult` so main-view force scenarios (user-initiated / error / confirming) still render the summary, never the full output. - ACP path needs no change: ToolCallEmitter.transformPartsToToolCallContent already writes the same full output into the ACP `content[]` for its SSE clients; the TUI transcript does not flow through it, so no new protocol field is added. Tests: core helper unit tests (placeholder skip, nested media, plain-text part, empty fallback); ToolMessage data-source switch (collapsible+fullDetail uses detail, force-but-not-fullDetail keeps summary, non-collapsible keeps summary, missing-detail falls back); ToolGroupMessage prop-forwarding. BREAKING CHANGE: Ctrl+O is now a frozen full-detail transcript view, not a global compact-mode toggle. The `TOGGLE_COMPACT_MODE` command and the TUI effect of `ui.compactMode` / `ui.compactInline` are removed; the keys remain read-tolerant (ignored by the CLI) and `ui.compactMode` is still forwarded to the web shell. See docs/design/ctrl-o-detail-expand/design.md §6 for migration. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ctrl-o): address review — repaint race, suppressOnRestore parity, transcript error logging - AppContainer: fix close-repaint setTimeout being cancelled by streaming re-renders. `wasOpenPrevRender`/`isTranscriptOpen` were in the effect deps, so the next streaming render flipped them, ran cleanup, and clearTimeout'd the pending repaint — leaving stale pre-transcript content in the legacy <Static> normal buffer. Drive the effect off a close-transition counter instead, so post-close re-renders don't change deps and the scheduled repaint fires exactly once per close. - AppContainer: transcript snapshot now mirrors MainContent's `!display.suppressOnRestore` filter, so items collapsed on session resume (ui.history.collapseOnResume) are not re-exposed in the Ctrl+O view. - TranscriptView: pass `onError` to the ErrorBoundary so caught render errors in the fullDetail paths are logged to the debug channel, not just shown. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(ctrl-o): cover detailedDisplay resume derivation + message.parts fallback Add dedicated resumeHistoryUtils tests for §4.9: detailedDisplay derived from toolCallResult.responseParts, the `responseParts ?? message.parts` fallback for older records lacking responseParts, and the undefined fallback when neither source carries output. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ctrl-o): address review — plain-text detail, shared placeholder const, resume status guard, scroll hint Four review fixes on the §4.9 transcript work: - ToolMessage: when fullDetail swaps the data source to detailedDisplay (raw file content / grep hits / dir listings), force renderOutputAsMarkdown to false. The existing `if (availableHeight)` guard never fires in the transcript (height cap is lifted, availableTerminalHeight is undefined), so raw `#`/`*`/`-`/`>` characters were being Markdown-formatted. - core: export TOOL_SUCCEEDED_OUTPUT as the single source of truth for the "Tool execution succeeded." placeholder. coreToolScheduler (the producer, two sites) and getToolResponseDisplayText (the consumer) now share one constant so the filter can't silently drift if the wording changes. - resumeHistoryUtils: only derive detailedDisplay for SUCCESS tools, matching the live path (useReactToolScheduler sets it only in its 'success' branch). Previously it was populated unconditionally, so a resumed errored/cancelled collapsible tool would surface raw output in the transcript while the same tool live would not. - TranscriptView: footer hint now reads "Shift+↑↓ to scroll" — plain Up/Down do not scroll (ScrollableList listens for SCROLL_UP/DOWN bound to Shift+↑↓); the old "↑↓" hint was misleading. Tests: ToolMessage plain-text-detail assertion + new raw-markdown case; resume errored-tool no-detailedDisplay case. typecheck/lint/tests green (core scheduler 222, cli suites pass). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): guard transcript non-TTY output + clear detailedDisplay on compaction Addresses three review findings on the Ctrl+O transcript work: - Non-TTY byte leak: `useMouseEvents` enabled SGR mouse mode (?1002h ?1006h) whenever stdin supported raw mode, ignoring stdout. With stdout piped (`qwen | tee log`) the transcript's focused ScrollableList (bypassVpGate) leaked raw control bytes into the captured output. Gate the enable on `stdout.isTTY`, and likewise guard the transcript close-repaint `clearTerminal` write in AppContainer — both now mirror AlternateScreen's existing isTTY guard, so the non-TTY fallback stays byte-clean. - Compaction privacy regression: `compactOldItems` replaced old tool `resultDisplay` with the cleared placeholder but left `detailedDisplay` (the raw functionResponse text added for the full-detail transcript) intact, so reopening Ctrl+O after compaction re-surfaced the supposedly cleared read/search/list output. Clear `detailedDisplay` wherever `resultDisplay` is cleared, with a regression test. - Docs: keyboard-shortcuts.md still described Ctrl+O as "toggle compact mode"; updated to the open/close full-detail transcript behavior. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(tui): report a TTY stdout in ScrollableList mouse-scroll tests The new `stdout.isTTY` gate in `useMouseEvents` (which stops SGR mouse escapes leaking into piped output) left ink-testing-library's fake stdout — which has no `isTTY` — with the mouse pipeline disabled, so the scrollbar-drag and wheel-scroll assertions never received events. Mock ink's `useStdout` to report `isTTY: true` so the pipeline arms exactly as it does in a real terminal; all other ink exports are preserved. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): address Ctrl+O transcript review — q-guard, callback churn, tests, cleanup Resolves the qwen3.7-max /review findings: - Modifier guard on the transcript close key: bare `q` closed the transcript, but Ink reports Ctrl/Alt/Shift+Q as `{ name: 'q', … }` too (Alt arrives as `meta`), so those silently closed it. Guard `!key.ctrl && !key.meta && !key.shift` (Shift+Q is a literal `Q`). - Stable `openTranscript`: it captured `historyManager.history` and `pendingHistoryItems` as deps, both of which change identity every streaming tick, rebuilding the callback — and the whole `handleGlobalKeypress` closure that lists it — on every render during streaming. Read both via refs so the callback is referentially stable. - AppContainer transcript integration tests (the removed TOGGLE_COMPACT tests had no replacement): Ctrl+O installs TranscriptView; Esc / q / Ctrl+C / Ctrl+D close it; Ctrl+Q / Alt+Q / Shift+Q do NOT (modifier guard); arbitrary keys are swallowed and keep it open; a blocking confirmation (WaitingForConfirmation) auto-closes it (anti-deadlock). - Dead i18n string: removed the orphaned 'Press Ctrl+O to show full tool output' key from all 9 locale files (no `t()` reference remained after the compact-mode sweep). - Design doc: replaced the leaked absolute worktree path with a placeholder, and corrected the §6 keybinding-migration note — the codebase has no user-configurable keybinding override surface (`keyMatchers` always uses hardcoded defaults), so there is no persisted `toggleCompactMode` binding to migrate; the startup-detection step is not applicable until such a feature exists. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): escape ANSI in transcript detailedDisplay + gate its extraction Two findings from the qwen3.7-max /review on §4.9: - [Critical] ANSI escape injection: `detailedDisplay` carries raw, un-sanitized tool output (file contents, grep hits, directory listings). The Ctrl+O transcript rendered it straight to <Text> without escaping, so a malicious repo file with embedded terminal control sequences (e.g. `\x1b[?1049l` to drop the alt-screen, OSC 52 for clipboard poisoning) would execute when the transcript opened — and fullDetail lifts the height cap, exposing the whole file. Run it through `escapeAnsiCtrlCodes` (already used for agent names in this file) before rendering. Added a regression test asserting the raw ESC bytes don't survive. - [perf] `detailedDisplay` was extracted on every successful tool call (~25K chars from core's truncation) but is consumed only by the transcript's fullDetail render for collapsible (read/search/list) tools. Gate the extraction on `isCollapsibleTool(displayName)` so edit/write/command/agent calls no longer store a large string the renderer never reads — mirrors ToolMessage's `usingDetailedDisplay` gate (which also keys off the display name). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): gate resume-path detailedDisplay on isCollapsibleTool (match live path) The resume path (resumeHistoryUtils.ts) extracted `detailedDisplay` for every successful tool call, unlike the live path in useReactToolScheduler which gates on `isCollapsibleTool(displayName)`. Since the transcript's `usingDetailedDisplay` only consumes it for collapsible (read/search/list) tools, resuming a session with many edit/write/command/agent calls stored large (~25K char) strings the renderer never reads. Apply the same gate so live and resume stay consistent, using `toolCall.name` (the display name, set from `tool.displayName`) to match the renderer's key. Updated the existing derivation tests to use a collapsible read tool (an edit tool now correctly yields undefined) and added a regression asserting a non-collapsible tool leaves detailedDisplay undefined on resume. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): strip bare C0 control bytes from transcript detailedDisplay + memoize Follow-up to the ANSI-escape fix. `escapeAnsiCtrlCodes` delegates to ansi-regex, which only matches ESC-prefixed sequences, so bare C0 control bytes without an ESC prefix (BEL \x07, BS \x08, FF \x0c, SO \x0e, SI \x0f, CR, …) passed through to <Text> and could still corrupt the display or ring the bell from a malicious file's contents. Add a second pass that strips those bytes (keeping only TAB and LF, which structure multi-line output). Memoize the two-pass sanitization with useMemo keyed on detailedDisplay so the ~25K-char regex work doesn't re-run every render. Extended the ToolMessage regression test to assert bare C0 bytes are stripped alongside the ESC sequences. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(tui): memoize HistoryItemDisplay, add ErrorBoundary tests + TAB/LF invariant Addresses three review suggestions: - Wrap `HistoryItemDisplay` in `React.memo` so the Ctrl+O transcript (which re-renders on every scroll tick) skips re-rendering frozen-snapshot items whose props are shallowly unchanged. The transcript passes stable `item` references, so the default shallow compare is effective; harmless for the main view (items live in `<Static>` and render once). - Add ErrorBoundary.test.tsx covering the four behaviors: renders children when healthy, catches a render error into the default fallback with the message, renders a custom fallback, calls `onError` with the error + component stack, and `reset` clears the error state so the subtree recovers. - Lock the C0-strip invariant: assert TAB and LF survive in detailedDisplay (the regex intentionally skips \x09/\x0a) so a future regex change can't silently collapse multi-line/columnar output. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * refactor(tui): review cleanups — gate sanitize memo, drop dead code, add tests Addresses the latest /review suggestions: - ToolMessage: gate the `sanitizedDetailedDisplay` useMemo on `usingDetailedDisplay` so the ~25K-char escape+strip no longer runs for every collapsible tool in the main view (where the result is discarded). - TranscriptView: remove the dead `listRef` (created + passed as `ref` but never used imperatively) and the dead `onClose` prop (declared, then `void`-ed; close keys are owned entirely by AppContainer's global keypress guard). Dropped the now-unused `useRef` / `ScrollableListRef` imports and the `onClose` call-site + props. - Tests: add TranscriptView error-fallback coverage (a throwing item renders the recovery fallback, not a crash); add live-path `mapToDisplay` detailedDisplay extraction coverage (collapsible → extracted, non-collapsible → undefined); add Ctrl+O to the transcript close-keys it.each (the toggle key was the only close key untested). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(tui): remove orphaned no-op CompactModeProvider stubs This PR deleted the CompactModeContext, leaving identical no-op `CompactModeProvider` passthrough stubs (with an ignored `value` prop) in ToolGroupMessage.test.tsx, ToolMessage.test.tsx and MainContent.test.tsx, each still wrapping every render. Remove the stubs and unwrap the renders; drop the now-meaningless `compactMode` params/args from the local render helpers. Behavior-preserving (the stubs rendered children verbatim) — all three suites still pass. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): strip bidi overrides, sanitize error fallbacks, share filters Latest /review round: - [Critical] Strip Unicode bidirectional override / isolate chars (Trojan Source, CVE-2021-42572) from transcript `detailedDisplay` — a third sanitize pass after ANSI + C0 stripping, mirroring the repo's existing BIDI_CONTROL_RE. Regression test added. - Sanitize `error.message` with `escapeAnsiCtrlCodes` in both the ErrorBoundary default fallback and the TranscriptView custom fallback (defense-in-depth against control codes in a crafted error message). - Ctrl+O while the ThinkingViewer is open now swaps to the transcript (falls through to openTranscript, which clears the viewer) instead of being silently swallowed. - Extract the shared `isHistoryItemVisibleAfterRestore` predicate into types.ts and use it from both MainContent (main view) and AppContainer (transcript freeze), so the two surfaces can't diverge on which collapse-on-resume items are hidden. - Tests: use the exported `TOOL_SUCCEEDED_OUTPUT` constant instead of the hardcoded literal in generateContentResponseUtilities.test.ts. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): harden compaction guard to always clear detailedDisplay The compaction cleanup only cleared `detailedDisplay` inside the `resultDisplay != null` branch (both the group-level trigger, the group-count pass, and the per-tool clear). A tool carrying only `detailedDisplay` (no resultDisplay) would skip compaction and leave the raw transcript detail intact — a latent privacy leak if the two fields ever decouple. Widen all three checks to also match `detailedDisplay != null` so the memory/privacy safeguard is robust. Added a defensive regression test. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): sanitize mime/uri in getToolResponseDisplayText media placeholders The `<media: …>` placeholder interpolated `inlineData.mimeType` / `fileData.mimeType` / `fileData.fileUri` from tool responses verbatim. A crafted response could embed control characters or angle brackets to inject terminal codes or forge/mangle the placeholder markup. Add a `sanitizeMediaLabel` helper that strips C0/C1 control bytes and `<`/`>` before interpolation, falling back to the default label when emptied. Regression test added. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(tui): report a TTY stdout in BaseSelectionList mouse integration test The `stdout.isTTY` gate added to `useMouseEvents` (stops SGR mouse escapes leaking into piped output) left QwenLM#6011's BaseSelectionList mouse test — which renders via ink-testing-library where the hook-provided stdout reads as non-TTY — with the mouse layer disabled, so the any-event enable escape was never written. Mock ink's `useStdout` to report `isTTY: true` with a capturing write spy (matching useMouseEvents.test.tsx / ScrollableList.test .tsx), and assert the `?1003h` enable via that spy while items still render through ink's own stdout. Both cases pass. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(core): fix JSDoc placement + note ErrorBoundary fallback is un-translated Two small review nits: - getToolResponseDisplayText's JSDoc had ended up above sanitizeMediaLabel (added last commit), making it read as that helper's docs. Reorder so sanitizeMediaLabel + its own JSDoc come first and each doc sits directly above its function. - Document why the ErrorBoundary default fallback's title is intentionally a plain English string (last-resort message for callers with no `fallback`; renders mid-crash, so it avoids pulling in the i18n layer — the transcript passes its own localized fallback anyway). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(tui): share terminal-sanitize pipeline; guard AlternateScreen writes - Extract the three-pass sanitizer (ANSI escape + bare-C0 strip + bidi strip) into `sanitizeTerminalText` in textUtils.ts as the single source of truth, and use it at all raw-text render sites: ToolMessage's `detailedDisplay`, and the TranscriptView + ErrorBoundary error-message fallbacks (previously those only escaped ANSI, missing C0/bidi — the boundary catches errors from the fullDetail path that processes raw tool output, so a crafted item shape could carry unsanitized bytes into error.message). Removes the duplicated regex consts from ToolMessage. - AlternateScreen: wrap the alt-screen escape writes (and the exit/cleanup writes) in try/catch so a synchronous stdout error (EPIPE on terminal close, EAGAIN under backpressure) can't propagate uncaught from the effect and crash the app or corrupt the terminal. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
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
6 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
Adds mouse click and hover to the interactive TUI. When Virtualized History (
ui.useTerminalBuffer) is enabled — i.e. the app runs in alternate-screen mode — three interactions become available: in select menus and dialogs (permission prompts,/model,/config, theme, …) hovering highlights the row under the pointer and clicking selects it; in the/command and@file suggestion lists hovering highlights a suggestion and clicking accepts it; and in the prompt input a left-click positions the text cursor (multi-line wrapping, scroll offset, and wide characters are handled). There is no separate setting — mouse interactions come along with Virtualized History, mirroring the existing mouse-wheel / scrollbar support which is also VP-only. Keyboard navigation is unchanged.Why it's needed
The CLI already supported the mouse wheel and scrollbar in Virtualized History mode, but not pointer interactions. Users naturally expect to click an option in a dialog, click a completion, or click to reposition the cursor — matching the behavior of other modern coding CLIs. This wires those interactions onto the existing SGR-mouse infrastructure.
Reviewer Test Plan
How to verify
Enable Virtualized History (Settings → "Virtualized History", or
settings.json"ui": { "useTerminalBuffer": true }), then in an interactive session: (1) open/model(or any select dialog) and move the mouse over the options — the row under the pointer highlights; click an option — it is selected. (2) Type/(or@then part of a path) to show the suggestion list — hovering highlights a suggestion, clicking accepts it. (3) Type a line (or several) of text in the prompt and click within it — the cursor jumps to the clicked character. Keyboard navigation keeps working throughout. With Virtualized History OFF, none of the above is active (behavior unchanged from today). Automated checks, frompackages/cli:npx vitest run src/ui/utils/list-mouse.test.ts src/ui/utils/input-mouse.test.ts src/ui/utils/mouse.test.ts src/ui/hooks/useMouseEvents.test.tsx src/ui/hooks/useSelectionList.test.ts src/ui/components/shared/RowMouseController.test.tsx src/ui/components/shared/TextInputMouseController.test.tsx src/ui/components/shared/BaseSelectionList.test.tsx src/ui/components/shared/BaseSelectionList.mouse.test.tsx src/ui/components/SuggestionsDisplay.mouse.test.tsx(64 mouse-related tests pass), plusnpx tsc --noEmitandnpx eslint .are clean.Evidence (Before & After)
TUI interaction change, verified manually in a real terminal on macOS. Before: hovering/clicking in menus, the suggestion list, or the prompt does nothing — only the wheel/scrollbar respond in Virtualized History mode. After: hover highlights the row under the pointer, and click selects (menus) / accepts (suggestions) / positions the cursor (prompt), as described above. Reviewers can reproduce with the steps above; a short screen recording can be added on request.
Tested on
Environment (optional)
Local interactive CLI on macOS with Virtualized History enabled.
Risk & Scope
min(0, terminalHeight - frameHeight)so an overflowing frame (top rows scrolled off, negative anchor) and a shorter top-anchored frame (anchor 0) are both correct; this is covered by unit tests. While mouse tracking is active the terminal's native click-drag text selection is suppressed (hold Shift, or Option on macOS, to select text) — the same caveat already documented for Virtualized History mode.Linked Issues
Closes #6053.
中文说明
这个 PR 做了什么
为交互式 TUI 添加鼠标点击与悬停。当开启 Virtualized History（
ui.useTerminalBuffer，即应用进入备用屏幕模式）时，三类交互可用：在选择菜单与对话框中（权限确认、/model、/config、主题等）悬停会高亮指针所在行、点击即选中；在/命令和@文件建议列表中悬停高亮某条建议、点击即接受；在输入框中左键点击可定位文本光标（已处理多行换行、滚动偏移与宽字符）。没有单独的开关——鼠标交互随 Virtualized History 一起启用，与现有的鼠标滚轮/滚动条支持一致（后者同样仅在 VP 模式下生效）。键盘导航保持不变。为什么需要
CLI 此前已支持 Virtualized History 模式下的鼠标滚轮和滚动条，但不支持指针交互。用户自然期望能点击对话框中的选项、点击补全项、点击重新定位光标——与其他现代编码 CLI 的行为一致。本 PR 把这些交互接到已有的 SGR 鼠标基础设施上。
评审测试计划
如何验证
开启 Virtualized History（设置 → "Virtualized History"，或在
settings.json中"ui": { "useTerminalBuffer": true }），然后在交互式会话中：(1) 打开/model（或任意选择类对话框），鼠标移到选项上——指针所在行会高亮；点击某项——即被选中。(2) 输入/（或@再输入部分路径）唤出建议列表——悬停高亮、点击接受。(3) 在输入框输入一行（或多行）文本并在其中点击——光标跳到所点字符处。全程键盘导航照常可用。关闭 Virtualized History 时，以上均不生效（行为与现状一致）。自动化检查（在packages/cli下）：npx vitest run src/ui/utils/list-mouse.test.ts src/ui/utils/input-mouse.test.ts src/ui/utils/mouse.test.ts src/ui/hooks/useMouseEvents.test.tsx src/ui/hooks/useSelectionList.test.ts src/ui/components/shared/RowMouseController.test.tsx src/ui/components/shared/TextInputMouseController.test.tsx src/ui/components/shared/BaseSelectionList.test.tsx src/ui/components/shared/BaseSelectionList.mouse.test.tsx src/ui/components/SuggestionsDisplay.mouse.test.tsx（64 个鼠标相关测试通过），以及npx tsc --noEmit与npx eslint .均无报错。证据（前后对比）
TUI 交互改动，在 macOS 真实终端中手动验证。之前：在菜单、建议列表或输入框中悬停/点击无任何反应——Virtualized History 模式下只有滚轮/滚动条响应。之后：悬停会高亮指针所在行，点击会选中（菜单）/接受（建议）/定位光标（输入框），如上所述。评审者可按上述步骤复现；如需要可补充一段录屏。
测试平台
运行环境（可选）
macOS 本地交互式 CLI，已开启 Virtualized History。
风险与范围
min(0, terminalHeight - frameHeight)处理，使得溢出屏幕的 frame（顶部行被滚出，anchor 为负）与短于屏幕的顶部对齐 frame（anchor 为 0）都正确；已有单测覆盖。鼠标追踪开启期间，终端原生的拖拽选择文本会被抑制（按住 Shift，macOS 上按住 Option 可选择文本）——这与 Virtualized History 模式既有的说明一致。关联 Issue
Closes #6053。