Skip to content

fix(cli): unfreeze Ctrl+O compact-mode toggle on long conversations#3905

Merged
chiga0 merged 5 commits into
QwenLM:mainfrom
chiga0:fix/3899-ctrlo-freeze
May 9, 2026
Merged

fix(cli): unfreeze Ctrl+O compact-mode toggle on long conversations#3905
chiga0 merged 5 commits into
QwenLM:mainfrom
chiga0:fix/3899-ctrlo-freeze

Conversation

@chiga0

@chiga0 chiga0 commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Closes #3899.

Pressing Ctrl+O on a long conversation froze the terminal for seconds. The toggle handler called refreshStatic(), which clearTerminal'd and forced Ink's <Static> to remount every history item synchronously on the input thread — N items × per-item Ink layout/render blocked the keyboard until the redraw finished.

Two narrow changes preserve current UX:

  1. compactToggleHasVisualEffect(history) short-circuitmergeCompactToolGroups.ts exposes an O(N) scan; the Ctrl+O handler in AppContainer.tsx skips refreshStatic() entirely when no past item would render differently (history without tool_group / gemini_thought*). Future items still pick up the new mode via React state — <Static> is append-only. Plain-chat sessions no longer freeze on Ctrl+O regardless of length.

  2. Progressive <Static> replayMainContent.tsx feeds <Static> a growing slice of mergedHistory when the catch-up gap is large. Below 100 items the slice jumps to full length in one render (bit-identical to the previous behavior); above that, it grows in 50-item chunks via setImmediate so the event loop yields between batches and the input thread stays responsive during the post-Ctrl+O remount. This also covers the post-resume initial mount path — first paint of a large resumed history no longer blocks.

Why this approach

UX is unchanged: terminal is still cleared, all history items still render under the new mode after the toggle. We only changed how the work is scheduled (yielding between chunks vs. a single synchronous burst) and short-circuit the toggle when it would be a pixel-identical no-op.

Discussion of alternatives (sealed prefix + live tail, true viewport virtualization, ANSI-output caching) was considered but each changes UX or requires an architectural rewrite. This PR is the minimum-change fix consistent with the existing rendering model.

Test plan

  • compactToggleHasVisualEffect unit tests cover empty / plain / tool_group / gemini_thought / gemini_thought_content histories.
  • MainContent tests cover both the below-threshold (full slice in one render) and above-threshold (initial 50-item slice, grows after setImmediate ticks) paths.
  • Full packages/cli vitest suite green (5016 tests).
  • tsc --noEmit clean.
  • eslint clean (0 warnings).
  • Manual repro: open a session with > 200 turns containing tool calls, press Ctrl+O. Expect responsive input throughout the redraw rather than a multi-second freeze.

the demo video: a long conversation with 500 turns, and then press ctrl + o to switch from compact to verbose mode.

comparison.mp4

🤖 Generated with Qwen Code

@chiga0 chiga0 requested review from Copilot, wenshao and yiliang114 May 7, 2026 09:19
@chiga0 chiga0 assigned chiga0 and unassigned chiga0 May 7, 2026
@chiga0 chiga0 added the DDAR DataWorks Data Agent Ready label May 7, 2026

@chiga0 chiga0 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

自评 review

整体方向 OK,是当前架构下 minimum-change 路径。下面几条偏 follow-up,挂在内联评论里方便逐条回。

关键点(值得在合并前对一下)

  • 长会话分片回放时,新完成的 item 在 catch-up 窗口期会瞬时不可见 — pending 区已经移除它,但 Static 的 slice(0, replayCount) 还没覆盖到它的索引。500 条 history 约 100ms 级,5000 条会到秒级。是否在意取决于真实使用是否有"超长 resume 会话 + 切换瞬间提交新输入"的组合。
  • historyManager 进入 keypress deps 数组,需要确认 useHistory() 的返回引用稳定(仅 .history 在变),否则 handler 会随 history 增长频繁重建。

doc/test 上的 nit

  • compactToggleHasVisualEffect 的注释关于 force-expand 的解释不准确(force-expand 组的 tool_use_summary 实际上两种模式渲染相同)。检查本身保守正确,但说明可以改得更准。
  • 仅验证了 helper 的正确性,没有 integration-level 断言"plain 历史下 Ctrl+O 真的不会调用 refreshStatic"。useKeypress 已被 mock 成 no-op,加这条断言要补一点 scaffolding,但价值清晰。

详见行内。


review by Claude Opus 4.7 (claude-opus-4-7)

* Conservative: any `tool_group` returns true even if it's force-expanded
* (force-expand groups render via the same path in both modes, but their
* adjacent `tool_use_summary` may still render differently — checking the
* group alone is the cheap upper bound).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc 注释这里关于 force-expand 的解释不准确:force-expand tool_group 的 callId 不会进入 absorbedCallIds(见 MainContent.tsx:62-71),它相邻的 tool_use_summary 在两种 mode 下都会渲染(!compactMode || !summaryAbsorbed 在 absorbed=false 时永远为 true),即视觉是一致的。

所以严格说,仅含 force-expand tool_group 的 history 也是 toggle no-op。我们之所以保守地仍返回 true,原因是判断 force-expand 需要 embeddedShellFocused / activePtyId,在调用点拿不到那么便宜。建议把注释改成这个真实理由。


review by Claude Opus 4.7

// Below the threshold the slice jumps to full length in one render so normal
// runtime appends are bit-identical to the previous behavior.
const PROGRESSIVE_REPLAY_THRESHOLD = 100;
const PROGRESSIVE_REPLAY_CHUNK_SIZE = 50;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THRESHOLD = 100CHUNK_SIZE = 50 是拍脑袋定的。每条 item 的渲染开销差异巨大(几行的 user message vs 4k+ 行的 tool 输出),单一 chunk size 在两端都不理想:

  • 小 item 大量场景下 50/tick 仍可能浪费 yield 机会;
  • 大 tool 输出场景下一次 50 条仍可能阻塞数百 ms。

不必现在调,但建议加个 TODO 标注 "empirically tune",或考虑按"chunk 内总行数预算"而非"item 数"切片。


review by Claude Opus 4.7

() =>
replayCount >= mergedHistory.length
? mergedHistory
: mergedHistory.slice(0, replayCount),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

长会话下的可见性窗口问题。

Static 的 items 是 mergedHistory.slice(0, replayCount)。在 chunked replay 进行中(replayCount < length),pendingHistoryItems 已经把刚 finalize 的 item 移走,但 Static 的 slice 还没覆盖到它的索引 — 用户感知是"刚发的消息消失了,几百毫秒后才出现"。

  • 500 条历史:~10 个 setImmediate tick ≈ 100ms,可以接受。
  • 5000 条历史:~100 个 tick,> 1s,明显。

如果项目目标场景包含超长 resume 会话,建议加个 "tail buffer":catch-up 期间把超出 replayCount 的尾部继续在 dynamic 区域渲染,等 replayCount 追上再让 Static 接管。或者反向:让 Static 先吐尾巴,背景慢慢 backfill 头部 — 但 Static 的 append-only 决定了反向不可行,所以更现实的是 "tail buffer 兜底"。

现 PR 范围内可以先记 TODO,作为 follow-up。


review by Claude Opus 4.7

return () => clearImmediate(handle);
}, [replayCount, mergedHistory.length]);

const visibleMergedHistory = useMemo(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor:useMemoreplayCount >= length 分支返回原引用,slice 分支每次都新建数组 — 节省的是"length 变了但 replayCount 也跟到顶"那一帧的多余 slice 调用,意义不大。可以简化成裸表达式。无伤大雅。


review by Claude Opus 4.7

Comment thread packages/cli/src/ui/AppContainer.tsx Outdated
// append-only. Issue #3899: this unfreezes Ctrl+O for plain-chat
// long sessions; tool/thinking-bearing sessions still go through
// the (now chunked) full path in MainContent.
if (compactToggleHasVisualEffect(historyManager.history)) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

缺一个 integration-level 测试:在 plain history 下按 Ctrl+O 应调用 refreshStatic,在含 tool_group 的 history 下应该调用。helper 单元测试只证明了谓词正确,没证明 handler 真的尊重它。

现有 AppContainer.test.tsxuseKeypress mock 成 no-op,所以补这条断言需要稍微改下 mock 让它 capture 回调(或换成 KeypressContext 注入)。建议作为这个 PR 的兜底加进来。


review by Claude Opus 4.7

Comment thread packages/cli/src/ui/AppContainer.tsx Outdated
setCompactMode,
refreshStatic,
handleDoubleEscRewind,
historyManager,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

historyManager 加进 deps 后,确认下 useHistory() 的返回是否引用稳定(仅 .history 在内部变化)。如果它是每次 history 变更都新建对象(不少 hook 是这样),那这个 keypress handler 会随每条新消息全量重建,订阅/取消订阅成本会上去。

如果不稳定,建议改成在 handler 内通过一个 ref 读取最新 history。


review by Claude Opus 4.7

expect(staticItemsSpy.mock.calls.at(-1)?.[0]).toHaveLength(53);

// Drain setImmediate ticks to let progressive replay catch up.
for (let i = 0; i < 10; i++) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 次 setImmediate 是相对宽裕的上界(200/50 = 4 次足够),但靠"够多就行"略脆弱。更稳的写法是循环里逐 tick 断言 staticItemsSpy 的长度单调增长,并在前一次没增长时再 break — 这样如果将来 chunk size 变了或 effect 调度有变化,测试不会静默失效。

现 PR 范围内不必硬要改,留个 follow-up 思路。


review by Claude Opus 4.7

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a CLI UI freeze when toggling compact mode (Ctrl+O) on long conversations by avoiding unnecessary <Static> remounts and by yielding during large history replays to keep the input loop responsive.

Changes:

  • Added compactToggleHasVisualEffect(history) to skip refreshStatic() when the toggle would not change already-rendered output.
  • Implemented progressive <Static> “replay” in MainContent for large history remounts (chunked growth via setImmediate).
  • Added/updated unit tests covering the new heuristic and progressive replay behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/cli/src/ui/utils/mergeCompactToolGroups.ts Exposes a new O(N) heuristic to decide if toggling compact mode affects existing rendered history.
packages/cli/src/ui/utils/mergeCompactToolGroups.test.ts Adds unit tests for compactToggleHasVisualEffect.
packages/cli/src/ui/components/MainContent.tsx Adds progressive replay logic to feed <Static> a growing slice of history on large remounts.
packages/cli/src/ui/components/MainContent.test.tsx Adds tests for below-threshold full render and above-threshold progressive replay catch-up.
packages/cli/src/ui/AppContainer.tsx Uses the new heuristic to skip expensive refreshStatic() on Ctrl+O when it’s a visual no-op.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +215 to +220
useEffect(() => {
if (replayCount >= mergedHistory.length) return;
const remaining = mergedHistory.length - replayCount;
if (remaining <= PROGRESSIVE_REPLAY_CHUNK_SIZE) {
setReplayCount(mergedHistory.length);
return;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this still looks like a real edge case after the latest revision. The chunked replay path is meant for large catch-up/remount gaps, but normal small appends still render once with the previous replayCount before the effect catches up. When a pending item finalizes into history, that can briefly remove it from both the pending area and the <Static> slice.

Could we make the small-delta path synchronous, for example by rendering the full list whenever mergedHistory.length - replayCount <= PROGRESSIVE_REPLAY_CHUNK_SIZE, while keeping chunked replay only for the large catch-up/remount case? That would preserve the freeze fix without introducing a transient missing-message frame for ordinary appends.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep this as the only remaining blocker from my pass. The normal small-append path still renders one frame with the previous replayCount: once a pending item finalizes, it is removed from pendingHistoryItems, but the Static slice has not caught up yet, so the item can briefly disappear.

Could we make the small-delta path synchronous, for example by rendering the full history when mergedHistory.length - replayCount <= PROGRESSIVE_REPLAY_CHUNK_SIZE, while keeping chunked replay for large remount/catch-up gaps?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8edb15d. Replaced the TODO comment and the condition with a gap-based check: . When the tail gap is ≤ CHUNK_SIZE (50), the render shows the full list synchronously — covering the normal append path without blocking the input thread for large remounts.

The scenario this pins: 100 items (at the replay threshold, replayCount=100, fully stable), one new item appended (gap=1≤50). Without the fix the first render shows 103 items (slice to 100, new item absent); with the fix the first render shows 104. Regression test added.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yiliang114 Fixed in commit 8edb15d. The gap-based condition covers exactly the scenario you described.

Instead of replayCount >= length, the condition is now length - replayCount <= PROGRESSIVE_REPLAY_CHUNK_SIZE. When the tail gap is small (<=50), the full list renders synchronously — no disappear frame on the normal small-append path. Chunked replay still applies for large remount gaps (gap >> 50).

Regression test in MainContent.test.tsx: start with 100 items (fully shown, below threshold), rerender with 101 items, verify the FIRST render sees 104 items, not 103. Thanks for flagging this as a blocker.

Toggling compact mode on a long conversation called refreshStatic(),
which clearTerminal'd and forced <Static> to remount every history
item synchronously on the input thread — N items × per-item Ink
layout/render blocked the keyboard for seconds. Issue QwenLM#3899.

Two narrow changes that preserve current UX:

1. compactToggleHasVisualEffect(history) skips refreshStatic() when
   no past item would render differently (history without tool_group
   or gemini_thought*). Future items still pick up the new mode via
   the live React state — Static is append-only. Plain-chat sessions
   no longer freeze on Ctrl+O regardless of length.

2. MainContent now feeds <Static> a progressive slice of mergedHistory
   when the catch-up gap is large. Below 100 items the slice jumps to
   full length in one render (bit-identical to the previous behavior);
   above that, it grows in 50-item chunks via setImmediate so the
   event loop yields between batches and the input thread stays
   responsive during the post-Ctrl+O remount.

Also covers the post-resume initial mount path: if a session resumes
with a large history, the first paint is no longer a single blocking
render of every item.

Tests: 7 new (5 for compactToggleHasVisualEffect, 2 for the
progressive Static replay path).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@chiga0 chiga0 force-pushed the fix/3899-ctrlo-freeze branch from 6ef877b to c87b420 Compare May 7, 2026 09:26

@chiga0 chiga0 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

复审 (post-rebase)

PR rebase 到最新 main(含 #3680 markdown 渲染扩展)后冲突已解。整体结论与上次一致:方向正确、UX 保持、改动可控。这次新增观察:

rebase 合并的处理 OK

  • 上游加的 historyItemsWithSourceCopyOffsets 仍按 full mergedHistory 计算 running offsets,本 PR 把切片操作放在 offsets 算完之后(新增 visibleHistoryItemsWithSourceCopyOffsets)。这是正确选择 — code block copy index 必须跨 chunked replay 保持稳定,否则 catch-up 过程中 copy 命令会指向错误的 block。

新发现的小风险

  • catch-up 中后段,pendingHistoryItems 可见且其 pendingStartSourceCopyOffsets 已经累计了"未可见前缀"的 offset。短暂时间窗内用户在 pending 区看到 "code block #N",但只有部分前缀实际可见。窗口短(≤1s 量级),UX 上可接受,但值得记一笔。

之前的几条 follow-up 仍未处理(行内重新挂上):

  1. compactToggleHasVisualEffect doc 注释关于 force-expand 的解释仍不准确
  2. THRESHOLD/CHUNK_SIZE 仍是拍脑袋常量,建议至少加 TODO
  3. 长会话 catch-up 期间新 finalize 的 item 短暂消失
  4. 缺 AppContainer 层的 integration-level 断言
  5. useHistory() 返回引用稳定性需要验证(影响 keypress handler 重建频率)
  6. MainContent.test.tsx 的 setImmediate 计数靠"够多就行"略脆弱

详见行内。这些都是 follow-up 性质,不阻塞合并。


review by Claude Opus 4.7 (claude-opus-4-7)

* Conservative: any `tool_group` returns true even if it's force-expanded
* (force-expand groups render via the same path in both modes, but their
* adjacent `tool_use_summary` may still render differently — checking the
* group alone is the cheap upper bound).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc 注释这里关于 force-expand 的解释不准确:force-expand tool_group 的 callId 不会进入 absorbedCallIds(见 MainContent.tsx 中 absorbedCallIds 的计算),它相邻的 tool_use_summary 在两种 mode 下都会渲染(!compactMode || !summaryAbsorbed 在 absorbed=false 时永远为 true),即视觉是一致的。

所以严格说,仅含 force-expand tool_group 的 history 也是 toggle no-op。我们之所以保守地仍返回 true,原因是判断 force-expand 需要 embeddedShellFocused / activePtyId,在调用点(AppContainer keypress handler)拿不到那么便宜。建议把注释改成这个真实理由。


review by Claude Opus 4.7

// Below the threshold the slice jumps to full length in one render so normal
// runtime appends are bit-identical to the previous behavior.
const PROGRESSIVE_REPLAY_THRESHOLD = 100;
const PROGRESSIVE_REPLAY_CHUNK_SIZE = 50;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THRESHOLD = 100CHUNK_SIZE = 50 是拍脑袋定的。每条 item 的渲染开销差异巨大(几行的 user message vs 4k+ 行的 tool 输出),单一 chunk size 在两端都不理想:

  • 小 item 大量场景下 50/tick 仍可能浪费 yield 机会;
  • 大 tool 输出场景下一次 50 条仍可能阻塞数百 ms。

不必现在调,但建议加个 TODO 标注 "empirically tune",或考虑按"chunk 内总行数预算"而非"item 数"切片。


review by Claude Opus 4.7

() =>
replayCount >= historyItemsWithSourceCopyOffsets.length
? historyItemsWithSourceCopyOffsets
: historyItemsWithSourceCopyOffsets.slice(0, replayCount),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

长会话下的可见性窗口问题。

Static 的 items 是 historyItemsWithSourceCopyOffsets.slice(0, replayCount)。在 chunked replay 进行中(replayCount < length),pendingHistoryItems 已经把刚 finalize 的 item 移走,但 Static 的 slice 还没覆盖到它的索引 — 用户感知是"刚发的消息消失了,几百毫秒后才出现"。

  • 500 条历史:~10 个 setImmediate tick ≈ 100ms,可以接受。
  • 5000 条历史:~100 个 tick,> 1s,明显。

如果项目目标场景包含超长 resume 会话,建议加个 "tail buffer":catch-up 期间把超出 replayCount 的尾部继续在 dynamic 区域渲染,等 replayCount 追上再让 Static 接管。现 PR 范围内可以先记 TODO,作为 follow-up。


review by Claude Opus 4.7

return () => clearImmediate(handle);
}, [replayCount, mergedHistory.length]);

const visibleHistoryItemsWithSourceCopyOffsets = useMemo(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebase 合并这块处理是正确的:source-copy offsets 仍按 full mergedHistory 计算,slice 在拿到 offsets 之后再做。这保证了 code block copy index 跨 chunked replay 稳定 — 否则 catch-up 过程中 copy 命令会指向错乱的 block。

附带一个新小风险:catch-up 中后段 pendingHistoryItems 可见,其 pendingStartSourceCopyOffsets 已累计 "未可见前缀"的 offset。短暂时间窗内用户在 pending 区看到 "code block #N",但只有部分前缀实际可见。窗口短,UX 可接受,记一笔即可。


review by Claude Opus 4.7

Comment thread packages/cli/src/ui/AppContainer.tsx Outdated
// append-only. Issue #3899: this unfreezes Ctrl+O for plain-chat
// long sessions; tool/thinking-bearing sessions still go through
// the (now chunked) full path in MainContent.
if (compactToggleHasVisualEffect(historyManager.history)) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

缺一个 integration-level 测试:在 plain history 下按 Ctrl+O 应调用 refreshStatic,在含 tool_group 的 history 下应该调用。helper 单元测试只证明了谓词正确,没证明 handler 真的尊重它。

现有 AppContainer.test.tsxuseKeypress mock 成 no-op,所以补这条断言需要稍微改下 mock 让它 capture 回调(或换成 KeypressContext 注入)。建议作为这个 PR 的兜底加进来。


review by Claude Opus 4.7

Comment thread packages/cli/src/ui/AppContainer.tsx Outdated
setRenderMode,
refreshStatic,
handleDoubleEscRewind,
historyManager,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

historyManager 加进 deps 后,确认下 useHistory() 的返回是否引用稳定(仅 .history 在内部变化)。如果它是每次 history 变更都新建对象(不少 hook 是这样),那这个 keypress handler 会随每条新消息全量重建,订阅/取消订阅成本会上去。

如果不稳定,建议改成在 handler 内通过一个 ref 读取最新 history(例如 historyRef.current = historyManager.history; 然后在 handler 内 historyRef.current)。


review by Claude Opus 4.7

expect(staticItemsSpy.mock.calls.at(-1)?.[0]).toHaveLength(53);

// Drain setImmediate ticks to let progressive replay catch up.
for (let i = 0; i < 10; i++) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 次 setImmediate 是相对宽裕的上界(200/50 = 4 次足够),但靠"够多就行"略脆弱。更稳的写法是循环里逐 tick 断言 staticItemsSpy 的长度单调增长,并在前一次没增长时再 break — 这样如果将来 chunk size 变了或 effect 调度有变化,测试不会静默失效。

现 PR 范围内不必硬要改,留个 follow-up 思路。


review by Claude Opus 4.7

// Intentionally only depends on historyRemountKey — we react to remount
// events, not to incremental history growth (handled by the next effect).
// mergedLengthRef is a ref so it doesn't need to be in deps.
setReplayCount(initialReplayCount(mergedLengthRef.current));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] Resetting replayCount in an effect is too late for the Ctrl+O remount that this PR is trying to fix. When refreshStatic() increments historyRemountKey, this component first renders with the previous replayCount value; after a long session has caught up, that value is the full history length. Because the <Static> key also changes in that same render, Ink remounts and synchronously receives the full history before this effect can reduce replayCount, so tool/thinking histories still hit the blocking full replay path.

Move the remount reset into render state selection, for example by tracking the last seen historyRemountKey with replayCount and deriving the effective count for the key-changing render, or remount a child component keyed by historyRemountKey so useState(() => initialReplayCount(...)) is re-initialized before <Static> receives items. Add a test that first lets a >100-item history catch up to full length, then changes historyRemountKey and asserts the immediate remount render goes back to the first chunk.

— gpt-5.5 via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right — fixed in 1bd8f5e. Switched from useEffect-based reset to the canonical "store previous prop in state" pattern: lastRemountKey is tracked alongside replayCount, and a key change triggers a synchronous setState during render. React discards the in-flight render before commit, so <Static> never sees the stale full slice on the remount frame.

Added the regression test you asked for in MainContent.test.tsx (synchronously resets to the first chunk on historyRemountKey change after a full catch-up) — it drives a 200-item history to full catch-up, bumps historyRemountKey, and asserts the very next render returns 53 items (50 + 3 prefix), not 203.

Refs alone wouldn't have worked here — they don't trigger the re-render that lets the new slice take effect — and a child component keyed on historyRemountKey was the alternative I considered, but the in-place state-pair pattern was a smaller diff and avoided introducing a new component layer just for the reset semantics.

秦奇 and others added 2 commits May 7, 2026 19:54
Follow-up to PR review on QwenLM#3899:

- Rewrite compactToggleHasVisualEffect doc comment to give the actual
  reason for the conservative tool_group → true rule (force-expand
  detection needs embeddedShellFocused/activePtyId, unavailable cheaply
  at the keypress call site).
- Add a TODO above PROGRESSIVE_REPLAY_* constants noting the
  unbenchmarked thresholds and the line-budget alternative.
- Add a TODO at visibleHistoryItemsWithSourceCopyOffsets documenting
  the catch-up invisibility window and the tail-buffer follow-up.
- Drop redundant useMemo wrapper around the slice — the underlying
  inputs only change when the slice would change anyway.
- Mirror historyManager.history into a ref so the keypress handler
  reads the latest snapshot at call time without depending on the
  full historyManager object (which useMemo rebuilds on every history
  change). Removes historyManager from the keypress callback deps.
- Add AppContainer integration tests verifying the Ctrl+O handler
  skips refreshStatic for plain history and triggers it for tool_group
  history (uses the same handler-discovery pattern as the existing
  renderMode toggle test).
- Tighten the progressive-replay test: assert monotonic growth per
  drained setImmediate tick instead of relying on a 'drain enough
  ticks' upper bound.

All 5134 tests green; lint clean.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Wenshao's [Critical] review on QwenLM#3905: the previous useEffect-based
reset fired AFTER the render that already passed the (post-catch-up)
full replayCount to <Static>. Because <Static>'s key is bumped in the
same render, Ink remounts synchronously with the full history before
the effect ever runs — defeating the chunked-replay protection that
the PR is supposed to provide for tool/thinking-bearing histories.

Switch to the canonical "store previous prop in state" pattern: track
the last seen historyRemountKey alongside replayCount and resync both
during render. setState during render queues a discard-and-rerun, so
<Static> only ever commits with the post-reset (first-chunk) slice on
the remount.

Refs alone aren't enough — they don't trigger the re-render that lets
the slice take effect.

Add a regression test that drives a 200-item history to full catch-up,
then bumps historyRemountKey and asserts the very next render falls
back to the first chunk (53 items, not 203).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

callId: 'c1',
name: 'shell',
description: 'shell description',
status: 'Success',

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 1a4a030, swapped to ToolCallStatus.Success so the fixture is type-checked against the enum (ToolCallStatus was already imported at line 41).

Copilot review nit on QwenLM#3905: the tool_group fixture in the Ctrl+O
integration test used the raw string 'Success' for `status` even
though `ToolCallStatus` is already imported in this file. Switching
to the enum keeps the fixture type-safe and would catch silent
regressions if the enum values ever change.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main Ctrl+O freeze fix looks good to me after the latest revision, including the synchronous replay reset on remount.

The only remaining concern from my pass is the normal small-append path discussed in the existing thread: a finalized pending item can briefly disappear before replayCount catches up. I think we should address that before approval, likely by making the small-delta path render synchronously while keeping chunked replay for large catch-up/remount gaps.

// 500 items, ~1 s for 5000). For multi-thousand-item resumes a "tail
// buffer" — keep the trailing un-replayed items in the dynamic area until
// <Static> catches up — would close this gap.
const visibleHistoryItemsWithSourceCopyOffsets =

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不只是 TODO/follow-up:当前实现会让普通小增量路径出现可见回归。比如长会话正在 catch-up,replayCount 还停在 50;此时一个 pending item finalize 到 mergedHistory 尾部后,pending 区会立刻清空,但这里仍然只把 slice(0, replayCount) 交给 <Static>,新完成的 item 在下一轮 effect 把 replayCount 补到尾部之前既不在 dynamic 区,也不在 Static 区。用户会看到“刚生成完的消息短暂消失”,在几千条历史下可能持续明显时间。

建议把小 delta 路径同步处理掉:当 historyItemsWithSourceCopyOffsets.length - replayCount <= PROGRESSIVE_REPLAY_CHUNK_SIZE 时,本次 render 直接使用 full list;只在大的 catch-up/remount gap 上继续分片。这样保留 Ctrl+O 防卡顿,同时不改变普通 append/finalize 的可见性语义。

由GPT-5模型生成,CR地址:#3905

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8edb15d — same gap-based condition, same regression test. The new check renders the full list whenever the tail gap is small (≤ CHUNK_SIZE=50), which covers both the 'fully caught up + 1 new item' path and the last chunk of a large catch-up. TODO comment removed.

… replay

When a pending item finalizes into mergedHistory while replayCount lags
behind, the item was removed from pendingHistoryItems but not yet visible
in the Static slice — creating a brief "disappear frame" for one render.

Fix: render the full list whenever
`historyItemsWithSourceCopyOffsets.length - replayCount <= PROGRESSIVE_REPLAY_CHUNK_SIZE`
(small tail gap). This covers the normal append path without blocking
the input thread, while still yielding via setImmediate for large
remount gaps (the Ctrl+O freeze the PR is fixing).

Removes the TODO comment and adds a regression test that pins the
"no disappear frame" behaviour for small-delta appends.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅ — glm-5.1 via Qwen Code /review

* call site — we accept the false-positive in exchange for keeping this
* predicate self-contained and O(N).
*/
export function compactToggleHasVisualEffect(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] compactToggleHasVisualEffect (line 120) and isHiddenInCompactMode (line 95) each independently enumerate which item types are visually affected by compact mode — but the lists differ (tool_group | gemini_thought | gemini_thought_content vs gemini_thought | gemini_thought_content | tool_use_summary). If a future type is added that renders differently in compact vs detailed mode and a developer updates only one function, Ctrl+O freeze (#3899) returns silently.

Suggested change
export function compactToggleHasVisualEffect(
/**
* NOTE: If you add a type here that renders differently in compact mode,
* also update `isHiddenInCompactMode` and vice versa.
* Consider extracting a shared `COMPACT_VISUALLY_AFFECTED_TYPES` constant.
*/
export function compactToggleHasVisualEffect(

— DeepSeek/deepseek-v4-pro via Qwen Code /review

const historyManager = useHistory();
// `useHistory()` returns a fresh memoized object whenever `history` changes,
// so depending on `historyManager` directly inside event-handler callbacks
// would rebuild them on every message. Mirror history into a ref so

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The historyRef pattern here is load-bearing for the Ctrl+O fix — useHistory() returns a new object reference whenever history changes, so depending on historyManager in handler deps would rebuild the keypress handler on every message. This constraint is only documented in this code comment; it should also be noted on the UseHistoryManagerReturn interface or useHistory() JSDoc in useHistoryManager.ts so future developers adding new history-inspecting keybindings don't unintentionally reintroduce the per-message rebuild.

Suggested change
// would rebuild them on every message. Mirror history into a ref so
// `useHistory()` returns a fresh memoized object whenever `history` changes,
// so depending on `historyManager` directly inside event-handler callbacks
// would rebuild them on every message. Mirror history into a ref so
// handlers can read the latest snapshot at call time without reactive deps.
// NOTE: Also documented on `UseHistoryManagerReturn` in useHistoryManager.ts.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

// under-yields for big ones. Consider switching to a *line-budget* per
// chunk once we have telemetry on actual render times.
const PROGRESSIVE_REPLAY_THRESHOLD = 100;
const PROGRESSIVE_REPLAY_CHUNK_SIZE = 50;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] PROGRESSIVE_REPLAY_CHUNK_SIZE (50) is used for three conceptually distinct purposes: (1) initial display count in initialReplayCount, (2) per-tick increment step in the setImmediate callback, and (3) tail-gap threshold in visibleHistoryItemsWithSourceCopyOffsets. These three are logically coupled — adjusting one silently affects the others. Consider extracting the tail-gap threshold as a separate constant to make the coupling explicit.

Suggested change
const PROGRESSIVE_REPLAY_CHUNK_SIZE = 50;
const PROGRESSIVE_REPLAY_CHUNK_SIZE = 50;
// Tail gap within which we render the full list instead of a chunked slice
const PROGRESSIVE_REPLAY_TAIL_GAP_THRESHOLD = PROGRESSIVE_REPLAY_CHUNK_SIZE;

— DeepSeek/deepseek-v4-pro via Qwen Code /review

chiga0 pushed a commit that referenced this pull request May 18, 2026
PR #4146 review feedback (wenshao + Claude Opus 4.7 audit) addressed:

Code:
- MainContent.test: activePtyId typed as number (was 'pty-xyz' string,
  broke tsc with TS2322 — the test only relies on reference change so
  any number works).
- VirtualizedList: sanitize renderItem error path. Display becomes the
  generic `[render error]` marker; full err goes to debugLogger.debug
  so file paths / partial tool state don't leak to scrollback.
- MainContent: move pendingSourceCopyOffsetsByIndex into a ref so it
  no longer rebuilds renderVirtualItem identity every streaming tick.
  Without this, VirtualizedList.renderedItems useMemo invalidated
  per-tick → JSX rebuilt for every visible item → memo(HistoryItem
  Display) was still bailing but allocations were O(visible) per tick.
- AppContainer: drop the misleading "state-driven scroll reset" claim
  in the VP refreshStatic comment. VP is intentionally near-no-op:
  the React tree owns the visible region, mergedHistory mutation is
  what refreshes the screen, and the remount-key bump is preserved
  only to keep the legacy Static branch in sync if the user toggles
  the flag off mid-session.
- StaticRender: rewrite JSDoc to match reality. The custom React.memo
  is NOT output caching like @jrichman/ink's StaticRender export;
  the comparator rarely matches (parent allocates fresh JSX); the
  real skip happens at memo(HistoryItemDisplay) one level deeper.

Docs:
- docs/design/virtual-viewport: sync file map (drop non-existent
  ScrollProvider.tsx / useAnimatedScrollbar.ts), PR sequence (one PR
  #4146, V.3-V.5 deferred), open-question + checklist resolution for
  #3905 (superseded) and base branch rename.
- docs/users/reference/keyboard-shortcuts: document the 6 VP scroll
  keys (Shift+↑/↓, PgUp/PgDn, Ctrl+Home/End) under a "History
  scrollback (when ui.useTerminalBuffer is on)" section. Previously
  the only discovery path was the Settings dialog description.

Verified: tsc --noEmit -p packages/cli ✓, vitest 160/160 ✓ across
AppContainer / MainContent / VirtualizedList / useBatchedScroll /
keyMatchers / settingsSchema, eslint clean on touched files.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
chiga0 pushed a commit that referenced this pull request May 28, 2026
PR #4146 review feedback (wenshao + Claude Opus 4.7 audit) addressed:

Code:
- MainContent.test: activePtyId typed as number (was 'pty-xyz' string,
  broke tsc with TS2322 — the test only relies on reference change so
  any number works).
- VirtualizedList: sanitize renderItem error path. Display becomes the
  generic `[render error]` marker; full err goes to debugLogger.debug
  so file paths / partial tool state don't leak to scrollback.
- MainContent: move pendingSourceCopyOffsetsByIndex into a ref so it
  no longer rebuilds renderVirtualItem identity every streaming tick.
  Without this, VirtualizedList.renderedItems useMemo invalidated
  per-tick → JSX rebuilt for every visible item → memo(HistoryItem
  Display) was still bailing but allocations were O(visible) per tick.
- AppContainer: drop the misleading "state-driven scroll reset" claim
  in the VP refreshStatic comment. VP is intentionally near-no-op:
  the React tree owns the visible region, mergedHistory mutation is
  what refreshes the screen, and the remount-key bump is preserved
  only to keep the legacy Static branch in sync if the user toggles
  the flag off mid-session.
- StaticRender: rewrite JSDoc to match reality. The custom React.memo
  is NOT output caching like @jrichman/ink's StaticRender export;
  the comparator rarely matches (parent allocates fresh JSX); the
  real skip happens at memo(HistoryItemDisplay) one level deeper.

Docs:
- docs/design/virtual-viewport: sync file map (drop non-existent
  ScrollProvider.tsx / useAnimatedScrollbar.ts), PR sequence (one PR
  #4146, V.3-V.5 deferred), open-question + checklist resolution for
  #3905 (superseded) and base branch rename.
- docs/users/reference/keyboard-shortcuts: document the 6 VP scroll
  keys (Shift+↑/↓, PgUp/PgDn, Ctrl+Home/End) under a "History
  scrollback (when ui.useTerminalBuffer is on)" section. Previously
  the only discovery path was the Settings dialog description.

Verified: tsc --noEmit -p packages/cli ✓, vitest 160/160 ✓ across
AppContainer / MainContent / VirtualizedList / useBatchedScroll /
keyMatchers / settingsSchema, eslint clean on touched files.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao pushed a commit that referenced this pull request Jun 2, 2026
* chore(deps): re-upgrade ink 6 → 7.0.3 (upstream Static remount fix landed)

PR #3860 first upgraded ink 6 → 7.0.2. PR #4083 reverted because of a
TUI regression: `<Static>` did not re-emit items when its `key` prop
was bumped, so `/clear` / Ctrl+O / refreshStatic left the history area
blank under ink 7.0.2.

ink 7.0.3 (released after #4083) contains the exact fixes:

  - be9f44cda Fix: <Static> remount via key change drops new items (#948)
  - 669c4386c Fix: Drop stale <Static> output from fullStaticOutput on identity change (#950)
  - 7c2267c01 Fix `useBoxMetrics` not accepting ref objects with an initial null value (#945)

Changes:
  - `ink` ^6.2.3 → ^7.0.3 (root hoist + cli direct)
  - `react` ^19.1.0 → ^19.2.4 (cli direct; ink 7.0.3 peerDeps requires >=19.2.0)
  - `react`/`react-dom` overrides ^19.2.4 added so the transitive graph
    stays deduped to a single instance (avoids `Invalid hook call` from
    multiple React copies, the classic ink-upgrade hazard)
  - `wrap-ansi` already on ^10.0.0 from #4083's partial-revert (no change)

Verified:
  - `npm ls ink` → single `ink@7.0.3` across all peer deps
  - `npm ls react` → single `react@19.2.4`
  - `npm run typecheck --workspace=@qwen-code/qwen-code` clean
  - `npm run typecheck --workspace=@qwen-code/qwen-code-core` clean
  - Composer.test.tsx 20/20, MainContent.test.tsx 6/6, TableRenderer.test.tsx
    59/59 + 1 skipped — all key UI components green on the new ink

The Static-remount regression is upstream-fixed in 7.0.3, so the
runtime path is restored without needing #3941's overflowY-self-managed
viewport. #3941 (virtual viewport) remains an opt-in performance
feature on top.

* fix(deps,cli): add @types/react overrides + move refreshStatic out of setCurrentModel updater

Two follow-ups from the multi-round audit of the ink 7.0.3 re-upgrade:

1. @types/react / @types/react-dom now pinned to ^19.2.0 in root
   overrides. packages/web-templates still declares @types/react ^18.2.0
   in its devDeps. Today the CLI build is unaffected (web-templates's
   18.x types are nested in its own node_modules and the React-using
   src/insight and src/export-html files are excluded from its tsconfig
   build), but a future reincludes-or-hoist accident would land
   conflicting global JSX namespaces in the CLI compile graph. Match
   the dep dedup we already enforce for `react` and `react-dom` so the
   type graph stays as deduped as the runtime graph.

2. AppContainer's onModelChange handler was calling refreshStatic() as
   a side-effect inside the setCurrentModel updater. React.StrictMode
   double-invokes state updaters in dev, so model swaps fired two
   clearTerminal writes + two <Static> key bumps. The double work was
   masked under ink 6 (key changes were no-ops on <Static>), but ink
   7.0.3 honors key changes — the doubled work is now potentially
   visible as a faster flash-flash on every model switch.

   Refactor: setCurrentModel becomes a pure setter; refreshStatic
   moves into a useEffect keyed on currentModel with a ref-comparison
   guard so the first render doesn't fire. Single clearTerminal write
   per real model change, even under StrictMode.

Verified: npm ls ink → single 7.0.3, npm ls react → single 19.2.4,
npm ls @types/react → 19.2.10 hoisted (npm flags web-templates's 18.x
constraint as overridden, which is the intended behavior). Typecheck
clean across cli + core workspaces.

* docs(design): virtual viewport on ink 7 — analysis + PR sequence

Captures the architectural analysis of how to thoroughly close the
flicker / refresh-storm class of issues (#2950, #3118, #3007, #3838 UI
side, #3899 follow-on) using a virtualized history viewport.

- Surveys claude-code (forked ink) and gemini-cli (@jrichman/ink +
  ScrollableList + VirtualizedList) reference implementations.
- Confirms ink 7 already exposes the primitives needed
  (`useBoxMetrics`, `measureElement`, `useWindowSize`,
  `useAnimation`) — no fork swap required.
- Picks porting gemini-cli's virtualized list components to ink 7 with
  `ResizeObserver` -> `useBoxMetrics` and a custom `StaticRender`.
- Splits the work into V.0..V.4 PRs with scope, dependencies, risk.
- Lists open questions + 11-item approval checklist that must clear
  before V.0 implementation begins.

This is a docs-only PR per the project's design-first workflow. No
runtime code changes.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): virtual viewport for long conversations on ink 7

Port gemini-cli's VirtualizedList + ScrollableList to stock ink 7,
adapting for ink 7's available primitives:

- `overflowY="hidden"` + `marginTop={-scrollTop}` instead of ink-fork's
  `overflowY="scroll"` (ink 7 has proper clip/unclip in render-node-to-output)
- `useBoxMetrics` inside each VirtualizedListItem (Option A) instead of a
  single ResizeObserver WeakMap; reports height changes via onHeightChange
  callback so the parent can update its heights record
- Custom `StaticRender` as `React.memo` with a reference-equality comparator,
  keyed on `itemKey-static-{width}` to freeze completed conversation items
- Character scrollbar column (`│` track / `█` thumb) since ink 7 has no
  native scrollbar prop
- No ScrollProvider / mouse drag (deferred to a follow-up PR)

Wire into MainContent.tsx behind `ui.useTerminalBuffer` setting (Settings
dialog → UI → Virtualized History; default false — opt-in).

Key bindings: Shift+↑/↓ (line), PgUp/PgDn (page), Ctrl+Home/End (top/bottom).

Re-render optimisations:
- renderItem wrapped in useCallback so renderedItems useMemo only recomputes
  when actual deps change (not on every streaming tick)
- Completed history items passed by original object reference so
  VirtualHistoryItem = memo(HistoryItemDisplay) can bail out on stable props
- estimatedItemHeight / keyExtractor / isStaticItem defined as module-level
  constants with no closure deps

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(cli): add test coverage for virtual viewport scroll bindings and settings

- keyMatchers.test.ts: 6 new test cases for SCROLL_UP/DOWN, PAGE_UP/DOWN,
  SCROLL_HOME/END commands (41 tests total)
- settingsSchema.test.ts: assert ui.useTerminalBuffer is boolean, default false,
  showInDialog true, requiresRestart false

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): use ink 7 native overflow for VP pending items

In VP mode, pending items are rendered inside VirtualizedList's
overflowY="hidden" container, which uses ink 7's native clipping
as the viewport guard. Remove the availableTerminalHeight JS-
truncation bound from pending items in renderVirtualItem:

- JS truncation at terminal height would silently cut off content
  the user could scroll to read within the virtual viewport.
- ink 7 overflowY="hidden" on the VirtualizedList container is the
  correct clip guard — no JS line-counting workaround needed.
- Remove uiState.constrainHeight from renderVirtualItem deps (no
  longer referenced in the VP rendering path).

The legacy <Static> path is unchanged.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* perf(cli): binary-search offsets in virtualized list hot path

Replace linear findLastIndex / findIndex scans on the offsets array with
upperBound. Offsets are monotonic by construction, so the lookups inside
the render body and getAnchorForScrollTop drop from O(n) to O(log n).
Material for thousand-turn sessions where the lookup runs on every frame.

* fix(cli): wire ShowMoreLines + skip clearTerminal in VP mode

Two audit-found bugs in the VP path:

1. `<ShowMoreLines>` was outside the `<OverflowProvider>` that wraps
   `<ScrollableList>` in VP mode. `useOverflowState()` returns
   `undefined` outside the provider, so the component returned `null`
   and the "press ctrl-s to show more lines" affordance silently
   disappeared. Move `<ShowMoreLines>` inside the provider so the hook
   sees the live overflow state, matching the legacy path.

2. `refreshStatic()` and `repaintStaticViewport()` wrote
   `clearTerminal` / `cursorTo+eraseDown` to the host terminal
   unconditionally. In VP mode the React tree owns the visible region
   via ink 7's native `overflowY="hidden"` clipping — the physical
   write is a wasted flash on Ctrl+O / Alt+M / model change / resize.
   Guard both writes on `useTerminalBuffer === false`. The
   `historyRemountKey` bump still fires so the legacy `<Static>`
   fallback would still remount if someone toggled the setting mid-
   session.

Extends the targeted-repaint pattern introduced in #3967 to all
refreshStatic call sites, gated by the VP setting instead of by event
type.

* fix(cli): VP renderItem stability + source-copy offsets + heights GC

Three audit-found regressions tightened, in order of severity:

1. **Source-copy index offsets missing in VP** — legacy `<Static>` path
   threads per-item `sourceCopyIndexOffsets` so `/copy mermaid N` /
   `/copy latex N` hints stay stable across continuation messages. VP
   `renderVirtualItem` was not passing this prop, so the copy hints
   shown under each diagram drifted on every `gemini_content` chunk
   (the clipboard mechanism itself still worked from raw history; only
   the displayed number was wrong). Add two lookup tables —
   identity-keyed for static items, index-keyed for pending — without
   changing the VirtualizedList data signature, and thread offsets in
   both render branches.

2. **`renderVirtualItem` callback invalidated on every streaming tick**
   — its deps included `activePtyId` / `embeddedShellFocused` /
   `isEditorDialogOpen`, all of which flip mid-stream when a shell
   tool runs or a dialog opens. Each flip rebuilt the callback,
   invalidated `VirtualizedList.renderedItems`'s useMemo, and forced
   every static item to re-render through `<StaticRender>` — defeating
   the very memoization the design relies on. Move the three pending-
   only fields into a ref read inside the callback. Static-item closure
   now depends only on inputs that legitimately affect static output
   (terminalWidth, slashCommands, getCompactLabel, …). Pending items
   still re-render correctly because their item identity changes per
   tick, so the callback is called fresh each time and reads the
   latest ref.

3. **`pending` items now honour `constrainHeight`** in VP, matching the
   legacy path. Previously VP unconditionally passed `undefined` for
   `availableTerminalHeight` on pending, relying on the viewport
   `overflowY="hidden"` clip to limit visible size — but that hid the
   `<ShowMoreLines>` affordance from the user. Now that ShowMoreLines
   is correctly wired (previous commit), restore parity.

4. **Heights map memory leak** in `VirtualizedList` — `setHeights` only
   grew. Each `/clear` left orphan `h-N` keys; each pending → completed
   transition left orphan `p-N` keys. Add a `useLayoutEffect` that
   prunes entries whose keys are not in the current `data`. Runs in
   layout phase so the prune commits in the same paint as the data
   change — no stale-offsets frame.

* test+fix(cli): VP path coverage + stabilize absorbedCallIds empty Set

Completion-pass artifacts driven by the multi-agent audit:

- Settings description rewritten to enumerate the symptoms VP fixes so
  users with active flicker reports can find the toggle without reading
  the design doc.
- `absorbedCallIds` returns a module-level constant Set when compact mode
  is off, instead of a fresh `new Set()` per render. Fixes a hidden
  cascade: `activePtyId` flip mid-stream → useMemo runs → returns a new
  empty Set → `isSummaryAbsorbed` rebuilds → `renderVirtualItem`
  rebuilds → `VirtualizedList.renderedItems` recomputes → every static
  item re-renders. With the constant, the cascade dies at the source.
  Helps both VP and legacy paths.
- VP-path unit tests for MainContent (4 cases): ScrollableList mounts
  and Static does not when `useTerminalBuffer: true`; ShowMoreLines is
  reachable in VP mode (regression of the OverflowProvider mis-wrap);
  source-copy index offsets thread into renderItem for static items;
  renderItem callback identity is stable across `activePtyId` flips
  (proves the ref-based read keeps StaticRender memo effective).

* fix(cli): stabilize absorbedCallIds in compact mode + gate heights prune + tighten ShowMoreLines test

Round-2 audit follow-ups. Three real findings addressed; one flagged
false positive documented separately.

1. **absorbedCallIds Set identity now content-stable when compact mode is
   on.** The earlier EMPTY constant only short-circuited the compactMode=
   false path; when compact mode is enabled (some users default-on it),
   activePtyId / embeddedShellFocused flips during streaming still
   produced fresh Sets per render even when membership was unchanged,
   restarting the same cascade the pendingStateRef fix was meant to
   avoid. Compare-and-reuse via a ref: if the new Set has identical
   membership to the previous one, return the previous reference.

2. **`heights` map prune in `VirtualizedList` is gated.** Previously
   every streaming tick rebuilt an N-key Set and walked all heights,
   even on the steady-state path where nothing changes. Now only fires
   when the heights record has clearly outpaced live data
   (`size > max(8, 2 × data.length)`) — covers `/clear` and accumulated
   pending → completed transitions, skips the 30-Hz hot path entirely.

3. **VP ShowMoreLines test now actually verifies overflow connectivity.**
   Previous mock unconditionally rendered "SHOW_MORE", so the test only
   proved the JSX mounted — it would still pass if a future refactor
   moved `<OverflowProvider>` out of the VP tree again. The mock now
   reads `useOverflowState()` and emits "OVERFLOW_DISCONNECTED" when the
   context is missing. The VP test asserts both presence of "SHOW_MORE"
   and absence of the disconnected marker, so the regression is now
   caught.

Not addressed:
- Audit P0-1 claim that `renderMode` (Alt+M) / model-change updates
  don't reach VP static items: false positive. `renderMode` is a React
  Context (`RenderModeContext`), and Context propagation traverses the
  tree past `memo` boundaries — MarkdownDisplay's `useRenderMode()`
  consumer re-renders on context change regardless of whether
  `StaticRender` bails out. Verified by reading
  `packages/cli/src/ui/contexts/RenderModeContext.tsx` and
  `MarkdownDisplay.tsx:172`. No code change.
- Audit P1-2 pendingStateRef write-during-render race: speculative,
  relies on a multi-pass render path React 18+ does not currently use.
  Documented assumption in the existing inline comment.

* fix(cli): isolate renderItem errors + defensive height coerce + compact-mode mergedHistory stability

Round-3 audit follow-ups. Three real findings; the rest verified clean.

1. **`renderItem` errors no longer crash the CLI.** Previously a throw
   inside a per-item render propagated through `VirtualizedList`'s
   useMemo into React's commit phase, tearing down the whole Ink tree —
   one bad history record could nuke the session. Wrap each call in a
   try/catch and substitute a small red `[render error] …` text box on
   failure. The row stays in the viewport so the user can scroll past
   it.

2. **Defensive height coerce in offset accumulation.** A buggy
   `estimatedItemHeight` returning NaN / negative / Infinity would
   poison every downstream offset and break the `upperBound` /
   `findLastLE` binary search (which assumes monotonic offsets). Clamp
   to `Number.isFinite(raw) && raw > 0 ? raw : 0`. No-op for the
   in-tree estimators that return 3; insurance against future
   consumers.

3. **`mergedHistory` is content-stable when compact mode is on.** The
   Round-2 absorbedCallIds stability fix didn't reach this path:
   `mergeCompactToolGroups` always allocates a fresh array, and
   `mergedHistory`'s useMemo lists `activePtyId` / `embeddedShellFocused`
   as deps, so every streaming tick mid-shell-tool produced a new array
   even when items aligned. Cascade went `mergedHistory` → offsets map
   → `renderVirtualItem` → every static item re-rendered. Pair-wise
   compare new vs previous and return the previous reference when items
   align. Restores StaticRender memo effectiveness for compact-mode
   users.

Not addressed (audit findings deemed not worth fixing in this PR):
- `scrollToItem` silently no-ops when item is not in data — no current
  caller checks the return value, low impact.
- `allVirtualItems` array spread is O(n) per streaming tick — real but
  not a crash; revisit in a perf-focused follow-up.
- `itemRefs.current` is dead surface (never read) — cosmetic.
- StrictMode-only-in-DEBUG double-invoke paths verified safe.

* test+chore(cli): VP review round 4 — VirtualizedList/useBatchedScroll coverage + cleanups

Addresses wenshao's CHANGES_REQUESTED review on PR #3941.

- Add focused unit tests for `VirtualizedList` (9 cases) covering empty
  data, `renderStatic` full-render, `initialScrollIndex` with
  `SCROLL_TO_ITEM_END`, `targetScrollIndex` anchoring, imperative
  `scrollToEnd` / `scrollToIndex`, per-item `renderItem` error isolation,
  NaN/negative estimator coercion, and out-of-range `initialScrollIndex`
  clamping.
- Add `useBatchedScroll` unit tests (4 cases) covering initial reads,
  pending-value reads in the same tick, post-commit pending reset, and
  callback identity stability across rerenders.
- Remove dead `itemRefs` / `onSetRef` plumbing (declared, written, never
  read; `useCallback` with empty deps was also a stale-closure trap).
- Remove unused `isStatic?: boolean` from `VirtualizedListProps`
  (only `isStaticItem` is actually consumed).
- Tighten the render-phase setState block: each setter is now guarded
  by an equality check so React bails out of redundant updates, and a
  comment documents that this is the React-endorsed "adjusting state
  while rendering" pattern (the synchronous update avoids a one-frame
  flash at the previous position when `targetScrollIndex` changes).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* chore(cli): remove dead `dataRef` from VirtualizedList (round-4 followup)

Declared and written in a `useLayoutEffect` on every `data` change but
never read anywhere in the component. Flagged in wenshao's round-4 review
of PR #3941.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): collapse model-change effect back into one batched handler

wenshao's PR #4119 review correctly flagged that splitting the
onModelChange flow into two effects (b25831b) reintroduced the
issue #3899 freeze regression on every model switch:

  1. setCurrentModel(model) commits first, with the OLD
     historyRemountKey.
  2. <Static key={`${historyRemountKey}-${currentModel}`}> sees its
     key change (because currentModel did) and remounts immediately.
  3. MainContent's render-phase progressive-replay reset only fires
     when historyRemountKey changes, so replayCount is still the
     full mergedHistory.length from any prior catch-up.
  4. The remounted Static dumps the entire history in one synchronous
     layout pass — exactly the freeze progressive replay was added
     to avoid (#3899). The second effect's refreshStatic() bump
     arrives a render too late.

Fix: do not split. Both side effects (refreshStatic, which writes
clearTerminal + bumps historyRemountKey, and setCurrentModel) live
in the event handler again, with a ref guard for same-model
notifications. The React.StrictMode concern that motivated b25831b
is addressed by keeping the side effect OUT of the setState updater
(it now runs once per event-handler invocation, not once per
double-invoked updater call). Both setState calls land in the same
React batch, so historyRemountKey and currentModel update together —
MainContent's render-phase reset sees the new key, replayCount drops
to the first chunk, and Static remounts with chunked replay intact.

Tests:
- AppContainer.test.tsx: 4 new tests covering the synchronous
  refreshStatic side-effect contract, same-model no-op, ref-guarded
  StrictMode double-invoke, and unsubscribe-on-unmount.
- MainContent.test.tsx: new regression guard — when currentModel
  changes but historyRemountKey is held constant, progressive replay
  must NOT reset (pins the MainContent invariant the two-effect
  refactor accidentally relied on).

Verified: vitest packages/cli AppContainer + MainContent green (82/82).
Typecheck clean.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix+docs(cli): VP review round 5 — typecheck, doc drift, scroll keys

PR #4146 review feedback (wenshao + Claude Opus 4.7 audit) addressed:

Code:
- MainContent.test: activePtyId typed as number (was 'pty-xyz' string,
  broke tsc with TS2322 — the test only relies on reference change so
  any number works).
- VirtualizedList: sanitize renderItem error path. Display becomes the
  generic `[render error]` marker; full err goes to debugLogger.debug
  so file paths / partial tool state don't leak to scrollback.
- MainContent: move pendingSourceCopyOffsetsByIndex into a ref so it
  no longer rebuilds renderVirtualItem identity every streaming tick.
  Without this, VirtualizedList.renderedItems useMemo invalidated
  per-tick → JSX rebuilt for every visible item → memo(HistoryItem
  Display) was still bailing but allocations were O(visible) per tick.
- AppContainer: drop the misleading "state-driven scroll reset" claim
  in the VP refreshStatic comment. VP is intentionally near-no-op:
  the React tree owns the visible region, mergedHistory mutation is
  what refreshes the screen, and the remount-key bump is preserved
  only to keep the legacy Static branch in sync if the user toggles
  the flag off mid-session.
- StaticRender: rewrite JSDoc to match reality. The custom React.memo
  is NOT output caching like @jrichman/ink's StaticRender export;
  the comparator rarely matches (parent allocates fresh JSX); the
  real skip happens at memo(HistoryItemDisplay) one level deeper.

Docs:
- docs/design/virtual-viewport: sync file map (drop non-existent
  ScrollProvider.tsx / useAnimatedScrollbar.ts), PR sequence (one PR
  #4146, V.3-V.5 deferred), open-question + checklist resolution for
  #3905 (superseded) and base branch rename.
- docs/users/reference/keyboard-shortcuts: document the 6 VP scroll
  keys (Shift+↑/↓, PgUp/PgDn, Ctrl+Home/End) under a "History
  scrollback (when ui.useTerminalBuffer is on)" section. Previously
  the only discovery path was the Settings dialog description.

Verified: tsc --noEmit -p packages/cli ✓, vitest 160/160 ✓ across
AppContainer / MainContent / VirtualizedList / useBatchedScroll /
keyMatchers / settingsSchema, eslint clean on touched files.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): SGR mouse wheel scroll in VP mode

Recovers the most-felt UX regression vs legacy `<Static>` mode: when
`ui.useTerminalBuffer` is on, legacy users lose mouse wheel as a way
to scroll history (the host terminal stopped seeing the conversation
in its scrollback buffer). This PR enables button-event tracking
(`?1002h`) + SGR coordinates (`?1006h`) while the ScrollableList has
focus, parses wheel events off stdin, and routes them to scrollBy.

Scope kept tight on purpose:
- Wheel only. Hit-testing for scrollbar drag / click-to-position
  needs screen-absolute element coords; stock ink 7's useBoxMetrics
  returns yoga's parent-relative layout. Deferred to V.4 with two
  exit paths (upstream getBoundingBox to ink 7, or local yoga walker).
- Mouse mode is enabled only while ScrollableList is mounted; non-VP
  users never see their terminal flipped into button-event tracking.
- Side effect: native click-and-drag text selection is captured by
  the program. Docs + settings dialog description now spell out the
  Shift / Option (macOS) bypass.

Implementation:
- `ui/utils/mouse.ts` — SGR + X11 parser, ported and trimmed from
  gemini-cli (Google LLC, Apache-2.0). Single-consumer.
- `ui/hooks/useMouseEvents.ts` — enable/parse/disable lifecycle
  hook. Listens on stdin via `useStdin().stdin`, runs handler
  through a ref so callers don't have to memoize.
- `ui/components/shared/ScrollableList.tsx` — subscribe to mouse
  events, route wheel → `scrollBy(±3)`. Also drops a dead outer
  `<Box flexGrow={1}>` wrapper that held an unread containerRef
  and collapsed to zero height in ink-testing-library (the test
  renderer has no flex parent, so flexGrow=1 → 0 height → no items
  ever rendered, which is how this dead code was exposed).

Tests:
- `ui/utils/mouse.test.ts` — 14 cases: SGR parsing (wheel, presses,
  modifiers, move), X11 parsing, fallback chain, incomplete-sequence
  guard (including the >50-byte garbage cap).
- `ui/components/shared/ScrollableList.test.tsx` — 3 cases: wheel
  events shift the rendered window; hasFocus=false makes the mouse
  pipeline inactive (no throw); non-wheel events leave the window
  unchanged. Renders are wrapped in `<KeypressProvider>` (required
  by useKeypress in production but easy to forget in standalone
  tests).

Docs:
- `docs/users/reference/keyboard-shortcuts.md` — adds "Mouse wheel"
  row + the Shift/Option-to-select note.
- `packages/cli/src/config/settingsSchema.ts` — the in-app dialog
  description now mentions mouse wheel and the text-select bypass.
- `docs/design/virtual-viewport/README.md` — §1 status, §5 file map,
  §7 PR sequence all reflect mouse wheel landing in #4146 and the
  V.4–V.7 follow-up split (scrollbar drag / in-app search / alt-
  buffer / host-scrollback dual-write research).

Verified: tsc --noEmit -p packages/cli ✓, vitest 182/182 ✓ across
AppContainer / MainContent / VirtualizedList / ScrollableList /
useBatchedScroll / mouse / keyMatchers / settingsSchema.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): auto-hide animation for VP scrollbar thumb

Pairs with the SGR mouse-wheel work from the previous commit:
when the user actually scrolls, the thumb pops bright; after a
1.5s idle it fades into the dim track so the bar stops competing
with the conversation. The track column itself stays in layout
regardless, so the viewport never reflows mid-flash (which would
trigger per-item re-measure and a visible jitter).

Implementation kept minimal for stock ink 7:
- gemini-cli's `useAnimatedScrollbar` interpolates RGB colors via
  a theme + per-frame setInterval. The terminal can't render
  smooth fades anyway, so this hook collapses the state to a
  binary `isVisible` flag with a single setTimeout. ~75 LoC.
- `VirtualizedList` calls `flashScrollbar()` from a useLayoutEffect
  keyed on `clampedScrollTop`. The very first commit is skipped
  via a ref so initial mount doesn't paint a flash.
- The render switches the thumb glyph (`█` vs `│`) and `dimColor`
  based on `isVisible && inThumb`. Width stays 1 either way.

Tests (6 new):
- initial mount stays hidden (no spurious mount flash)
- flash → visible, hides after idle timeout, successive flashes
  reset the timer (no premature hide), idleHideMs<=0 disables
  auto-hide for tests that want to assert on the visible state,
  unmount cleans up the pending timer.

Doc updates:
- `docs/design/virtual-viewport/README.md` §1 status, §5 file map,
  §7 PR sequence — V.4 row now scopes only the drag/click-jump
  work (still coord-blocked); animated scrollbar moved out of
  deferred and into shipped.
- PR #4146 body — architecture table mentions the auto-hide, new
  files list adds `useAnimatedScrollbar.ts`, test count refreshed
  to 188/188.

Verified: tsc --noEmit -p packages/cli ✓, vitest 188/188 ✓.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): VP review round 6 — ESC bug, CI lint, scope-controlled cleanup

Triage of /review feedback from 2026-05-18 + 2026-05-19. Took the
ones that are real and small; declined the ones that are
false-positive / out-of-scope so this PR stops expanding.

Must-fix:
- CI Lint failure: vscode-ide-companion/schemas/settings.schema.json
  was stale after the keyboard-shortcuts description bump. Regenerated
  via `npm run generate:settings-schema`.
- useMouseEvents.ts had `const ESC = '';` (literal empty string after
  the raw 0x1B byte got stripped somewhere in the source pipeline).
  `buffer.indexOf('', 1) === 1` would have degraded garbage skipping
  to a one-byte scan, and the `else { buffer = ''; break }` branch
  could never run. Fixed by switching to the `'\x1b'` text escape and
  doing the same in `mouse.ts` (which had the raw byte, also fragile).
  Comment explains why.

Small wins (one-liners taken from the review batch):
- ScrollableList: rest-spread separates `hasFocus` from the props
  forwarded to VirtualizedList. Latent collision risk; no behaviour
  change today.
- VirtualizedList: `debugLogger.debug` when isReady=false so blank-
  viewport edge cases (tiny terminal / mid-resize race) become
  diagnosable from the debug log instead of looking like a hang.

Real perf (VP-only):
- MainContent: gated the progressive-Static-replay machinery behind
  `!useVirtualScroll`. The render-phase reset still consumes the
  remount-key bump so flag-off toggles mid-session catch up cleanly,
  but `setReplayCount` and the setImmediate chunking effect are now
  skipped for VP users. Saves ~M/CHUNK_SIZE wasted re-renders per
  Ctrl+O / model change on a 1000-turn session.

Belt-and-braces:
- useMouseEvents: added a `process.on('exit')` handler that writes
  the SGR mouse disable seq again. The React cleanup already covers
  normal unmount, but Ctrl+C / SIGTERM / parent kill bypass it and
  the terminal would otherwise stay in button-event-tracking mode
  after qwen exits.

Explicitly declined / deferred (with reasoning logged on the PR):
- requestAnimationFrame wheel throttle: rAF doesn't exist in Node;
  React 19 already batches state updates within a tick, and the
  renderedItems memo bounds the actual work to visible items. Will
  revisit if profiling shows it.
- Stable pending-item IDs (`p-N` keys shifting on completion): the
  observable jitter is at most one frame of estimated-vs-actual
  height delta. Moderate scope (creation-time ID allocation); fits
  better in a focused follow-up than in this PR.

Verified: tsc --noEmit -p packages/cli ✓, vitest 188/188 ✓ across
the full VP suite.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): scrollBy bottom uses live end anchor in virtualized list

When keyboard scroll reaches the bottom, scrollBy set isStickingToBottom
but anchored via getAnchorForScrollTop(maxScroll), a fixed {index,offset}
pixel anchor. scrollTo/scrollToEnd instead use {index: last, offset:
SCROLL_TO_ITEM_END}, which recomputes the bottom from live item heights
each render. The fixed anchor did not track the last item growing during
streaming, so scroll-to-bottom via keyboard lagged behind new tokens.
Align scrollBy's bottom branch with the sibling methods.

Reported by wenshao in PR review.

* fix(cli): parse mouse events via ink useInput, not a stdin data listener

useMouseEvents attached its own stdin.on('data', ...) listener. Adding a
'data' listener switches stdin into flowing mode, which drains the buffer
before ink's readable + stdin.read() reader (ink App) can consume it, so
all keyboard input routed through useInput was silently starved while
mouse mode was active.

Parse mouse sequences from ink's existing input pipeline via useInput
instead, so there is only one stdin reader. ink captures a full SGR
sequence (ESC [ < .. M/m) as a single CSI event and delivers it with the
leading ESC stripped, so we re-prepend it before parsing. Non-mouse input
does not match and is ignored; ink still routes input to the app's other
useInput handlers, so keyboard navigation keeps working.

Only SGR mode (1006h, which we enable) is parsed via this path; the legacy
X11 encoding is not recoverable through ink's CSI parser, which is the
encoding modern terminals stop emitting once 1006h is set.

Reported by wenshao in PR review.

* fix(cli): parse only SGR in mouse hook to avoid X11 paste misfire

The useInput-based mouse hook called parseMouseEvent, which also tries the
X11 fallback (parseX11MouseEvent). An X11 prefix (ESC [ M + 3 bytes) can
reach the handler via pasted text — ink emits paste content as input when
no paste listener is registered — and would misfire a spurious mouse event.
Call parseSGRMouseEvent directly so only the SGR encoding we enable (1006h)
is parsed, matching the hook's documented contract.

Reported by wenshao in PR review.

* test(cli): assert SGR mouse parser rejects X11 sequences

Locks in the security property behind the parseMouseEvent ->
parseSGRMouseEvent switch in useMouseEvents: an X11 sequence arriving as
pasted text must not misfire a mouse event. Asserts a well-formed X11
sequence is a valid X11 event yet returns null from parseSGRMouseEvent, so
a future revert to parseMouseEvent fails this test.

Reported by wenshao in PR review.

* test(cli): add VP scroll coverage + eslint-disable for useBatchedScroll

Cover keyboard scroll commands (Shift+Up/Down, PageUp/Down, Ctrl+Home/End),
scrollBy/scrollTo imperative API (positive/negative/overflow/clamp), and
auto-scroll-during-streaming state machine (stick-to-bottom, disengage on
user scroll, re-engage on scrollToEnd). Add missing eslint-disable-next-line
for intentionally dep-free useLayoutEffect in useBatchedScroll.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* chore(cli): remove trailing whitespace in useBatchedScroll

The eslint-disable-next-line comment was removed by eslint --fix as an
unused directive (exhaustive-deps does not flag a useLayoutEffect with
no dependency array). Clean up the residual blank line.

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>
TaimoorSiddiquiOfficial added a commit to TaimoorSiddiquiOfficial/HopCode that referenced this pull request Jun 2, 2026
* fix(cli): persist /memory toggle state across dialog reopen (#4650)

The Auto-memory / Auto-dream / Auto-skill rows initialized their state
from Config getters, which are frozen at startup and never reflect a
setValue() write. Each /memory reopen re-mounts the dialog and re-reads
that stale snapshot, so a just-flipped toggle appeared to revert. Read the
initial state from the live merged settings instead, matching the existing
write path (bareMode semantics preserved).

Also switch the test's `act` import to `react` — the previously used
@testing-library/react is declared in package.json but not installed, so
the suite could not run — and add a mount/unmount/remount regression test.

* Hide internal docs from docs site (#4357)

* fix(core): preserve uid in atomicWriteFile to avoid breaking shared-write files (#4431)

* fix(core): preserve uid/gid in atomicWriteFile to avoid breaking shared-write files

atomicWriteFile uses write-to-tmp + rename for crash atomicity. POSIX
rename creates a new inode owned by the calling process's euid/egid, so
the rename silently strips the original uid/gid. On shared-write setups
(e.g. a group-writable file owned by another user in a shared workspace
where the current user has group-write access), every Write/Edit/
NotebookEdit through qwen-code would reset ownership to the running
user and effectively revoke write access for the original collaborators.

The fix:

1. If the target exists and is owned by a different uid/gid than the
   process's effective uid/gid (and we are not root), fall back to
   in-place writeFile. This truncates the existing inode in place,
   preserving uid/gid. The trade-off is loss of crash atomicity for
   this specific case — an acceptable trade for not silently breaking
   shared-write file ownership.

2. If running as root, atomic rename is still used, and ownership is
   restored via chown(uid, gid) after the rename. Root can chown back;
   non-root cannot, hence the in-place fallback for non-root.

3. Windows is unaffected (no POSIX ownership semantics).

Tests:

- New: in-place fallback on uid mismatch — verify content updates, mode
  preserved, and inode unchanged (the inode is the signal that the
  fallback path ran rather than rename).
- New: same scenario triggered via gid mismatch.
- New: positive case — ownership matches → atomic rename → inode changes.

Regression: a v0.16.0 user reported "every write turns a world-writable
file into one other users can no longer write." Bisected to #4096 which
introduced atomicWriteFile + write-to-tmp + rename.

* fix(core): route root through in-place fallback + doc/test follow-ups

Review follow-ups on the atomic-write ownership fix:

1. Remove the root-special-case (rename + post-rename chown). chown
   silently fails inside user-namespaced or CAP_CHOWN-stripped Docker
   containers, which re-triggers the original bug for root-in-Docker
   users — exactly the scenario this fix was reported against. Routing
   root through the same in-place fallback as non-root eliminates this
   failure mode and drops an untestable branch (chown-back can't be
   exercised under non-root CI).

2. Document the three properties traded away by the in-place fallback:
   crash atomicity, concurrent-reader isolation, inotify watcher
   semantics (MODIFY vs MOVED_TO).

3. Document that the in-place fallback surfaces EACCES when the file's
   mode forbids the current user from writing — this is correct
   behavior (atomic rename used to silently replace files the user had
   no permission on, which was arguably a privilege issue).

4. Replace the brittle "see step 6 in the function doc" comment with a
   step-number-independent reference.

5. New test covering the EACCES path: chmod 0o444 + mocked geteuid
   triggers the fallback, fallback hits the read-only file, EACCES
   propagates cleanly, original content is preserved.

* fix(core): harden in-place fallback against symlink/unlink/inode races + doc/test follow-ups

Review follow-ups on #4431 ownership-preservation fix:

CRITICAL — in-place fallback security hardening (wenshao review):

The path-based `fs.writeFile(targetPath, ...)` fallback introduced
three races that the prior `rename(tmp, target)` form did not have:

1. Non-regular files (FIFO/socket/device): fs.writeFile calls
   open(O_WRONLY|O_CREAT|O_TRUNC). On a FIFO this blocks forever
   waiting for a reader. On a character/block device it writes to
   the actual device. The rename path replaced these with a
   regular file.

2. Symlink-swap TOCTOU: an attacker with parent-dir write can swap
   targetPath for a symlink between our stat and our writeFile.
   fs.writeFile follows symlinks at the destination; POSIX rename
   does not. In the very "shared-write workspace / Docker bind-mount"
   scenarios this PR targets, this lets a directory-writable
   attacker redirect agent writes elsewhere (e.g. /etc/passwd if
   the agent runs as root).

3. Unlink race: if targetPath is unlinked between stat and write,
   O_CREAT silently recreates it owned by the calling user — the
   exact ownership change the fallback was designed to prevent.
   Silent regression to the pre-fix bug under this race.

Fix: extract the fallback into writeInPlaceWithFdGuards():

  - open(target, O_WRONLY | O_TRUNC | O_NOFOLLOW) — no O_CREAT, so
    unlink-race surfaces ENOENT instead of silently recreating; and
    O_NOFOLLOW rejects symlink-swaps with ELOOP.
  - fstat(fd) verifies the bound inode's uid/gid still match
    existingStat — refuses the write if an inode-swap happened
    between stat and open.
  - Write through the fd (locked to the verified inode), chmod
    through the fd, close.

Caller now gates the fallback on existingStat.isFile() — non-regular
targets fall through to the atomic path which has well-defined
"replace special-file with regular-file" semantics.

DOC / TEST follow-ups:

- Add hardlink-propagation as a 4th trade-off in the in-place
  fallback JSDoc (review comment #4): rename creates a new inode so
  sibling hardlinks keep old content; in-place truncate+write keeps
  the inode so all hardlinks see new content.

- Update atomicWriteJSON JSDoc to note the write is now
  *conditionally* atomic (review comment #5): atomic when uid/gid
  matches the process, in-place when ownership differs. Previously
  the JSDoc still claimed unconditional atomicity.

- Update caller comments at runtimeStatus.ts and
  worktreeSessionService.ts that advertised crash-atomic writes via
  tmp+rename — those guarantees are now conditional (review
  comment #6).

- Add mode + tmp-leftover assertions to the gid-mismatch test to
  match the uid-mismatch test (review comment #2 — test
  consistency). Without these, a gid-fallback regression that
  silently dropped permissions or left a tmp file would not be
  caught.

- New test: FIFO + ownership mismatch must take the atomic path,
  not in-place (verifies the existingStat.isFile() guard works;
  hang on in-place would trip vitest timeout).

- New test: writing through a symlink with ownership mismatch
  exercises the resolve-then-stat-then-open flow and verifies the
  symlink itself is preserved.

Tests: 192/192 pass (atomicFileWrite + write-file + edit +
fileSystemService).

* fix(core): defer O_TRUNC and verify dev+ino in writeInPlaceWithFdGuards

PR #4431 review follow-up (wenshao critical):

The previous form opened with `O_WRONLY | O_TRUNC | O_NOFOLLOW`, which
truncated the bound file *before* the fd-bound fstat verification ran.
If an attacker swapped the path between the caller's stat and our
open, we would truncate the attacker's substituted inode (destroying
unrelated content) before detecting the swap.

Two fixes:

1. Open without O_TRUNC. Verify dev+ino+uid+gid+isFile match
   expectedStat through fh.stat(). Only then call fh.truncate(0)
   through the validated fd.

2. Expand the verification beyond uid+gid to include dev+ino+isFile.
   uid+gid alone misses a same-owner inode swap (attacker replaces
   the path with a different inode they own). dev+ino is the strong
   identity check; isFile catches a swap to FIFO/socket/device after
   the caller's existingStat.isFile() gate.

JSDoc updated to enumerate the four guards (NOFOLLOW, no CREAT, no
TRUNC at open, dev+ino+uid+gid+isFile via fstat) and explain why
truncation must wait until after verification.

192/192 tests pass.

* fix(core): close FIFO swap race with O_NONBLOCK + cover EOWNERSHIP_CHANGED path

PR #4431 review follow-up (deepseek-v4-pro via /review):

CRITICAL — FIFO swap TOCTOU:

The caller's `existingStat.isFile()` gate uses stat data captured
earlier. An attacker with parent-dir write can swap the regular file
for a FIFO between the caller's stat and our open inside
`writeInPlaceWithFdGuards`. The previous `O_WRONLY | O_NOFOLLOW` open
would then block indefinitely waiting for a FIFO reader; O_NOFOLLOW
only catches symlinks.

Fix: add O_NONBLOCK to the open flags. Defense in depth:

- On a reader-less FIFO, `open(O_WRONLY | O_NONBLOCK)` returns ENXIO
  immediately — no hang.
- If the FIFO has a reader (open succeeds), the subsequent fstat
  isFile() check still refuses the write via EOWNERSHIP_CHANGED.
- For regular files, O_NONBLOCK is a no-op.

CRITICAL test gap — EOWNERSHIP_CHANGED branch untested:

The primary TOCTOU defense (fdStat dev/ino/uid/gid/isFile vs
expectedStat) had no coverage. Exported `writeInPlaceWithFdGuards` so
it can be unit-tested directly:

- New test: simulate post-stat inode swap (unlink + recreate at same
  path), call helper with stale stat, assert EOWNERSHIP_CHANGED and
  that the attacker's content survives.
- New test: simulate post-stat regular→FIFO swap, assert open fails
  fast (ENXIO) or fstat catches it — either way no hang, no write.

DOC fix:

JSDoc said "we open read-write without truncating" but the code uses
O_WRONLY. Wording corrected to "write-only".

194/194 tests pass.

* fix(core): fix flaky inode-swap test + apply review follow-ups

PR #4431 review follow-up (glm-5.1 via /review) — 7 suggestions adopted,
1 partially adopted, 0 rejected:

CI FIX (Ubuntu test failure on tmpfs inode reuse):

The EOWNERSHIP_CHANGED inode-swap test used unlink+create to simulate
a post-stat swap. On Linux tmpfs the freshly-freed inode number is
often reused by the immediately-following create, so dev+ino remained
identical and the guard didn't trip (intermittent on Ubuntu CI; macOS
APFS happened to allocate different inodes). Switched to rename(decoy,
target) which moves an existing distinct inode into place, guaranteed
to differ from the original.

CODE:

- Wrap fh.writeFile failure after fh.truncate(0) with
  EINPLACE_WRITE_FAILED + cause, so callers see explicitly that the
  file was truncated and the write didn't complete (otherwise they
  see raw ENOSPC/EIO and may wrongly assume the original is intact
  given this lives in atomicFileWrite.ts).
- Skip fh.chmod when euid is neither root nor expectedStat.uid —
  chmod is guaranteed to fail with EPERM in that case (POSIX requires
  owner or root). Avoids a guaranteed-failing syscall on every call.
- Caller catches ENOENT from writeInPlaceWithFdGuards and falls
  through to atomic rename path. If the file was deleted between
  caller's stat and our open there is no ownership to preserve; the
  rename path correctly creates a new file at targetPath.

DOC:

- Replaced "defends against four races" with "hardened against
  post-stat races" (the bullet list has 5 items, the count was wrong).
- Reworded "non-regular targets must not reach this function" to
  describe defense-in-depth — O_NONBLOCK + !fdStat.isFile() reject
  post-stat regular→FIFO/socket/device swaps. The old wording made
  it look like O_NONBLOCK was redundant.
- Documented the dual chmod behavior (root vs non-root with foreign
  uid) inline.

TESTS:

- Added happy-path test for writeInPlaceWithFdGuards (write succeeds,
  inode preserved, mode preserved).
- Added ENOENT regression test (verifies the missing-O_CREAT
  property — if file unlinked between stat and open, no silent
  recreate with caller's uid).
- Renamed the misleading "O_NOFOLLOW guard" test (it actually tests
  resolve-through-symlink, not O_NOFOLLOW) to reflect what it does,
  and added a direct ELOOP test that drives writeInPlaceWithFdGuards
  with a path whose final component is a symlink — that's the real
  O_NOFOLLOW exercise.
- Fixed the FIFO test to pass a stat captured from the FIFO itself
  (not a stale regular-file stat) so only the FIFO-specific defense
  fires, not the inode/dev mismatch from a different file.

NOT ADOPTED:

- Skip-when-non-root chmod optimization adopted (small, useful), but
  the larger "structured chmod error model" deferred — best-effort
  matches the existing tryChmod pattern at file scope.

197/197 tests pass.

* fix(core): wrap truncate err + post-write nlink check + guard close + chmod sync

PR #4431 review follow-up (qwen-latest-series-invite-beta-v34 via /review)
— 7 of 10 suggestions adopted, 3 deferred:

CODE:

- **EINPLACE_TRUNCATE_FAILED wrap** (review #3291863048): symmetric to
  the existing EINPLACE_WRITE_FAILED — distinguishes "truncate failed,
  original intact" from "write failed post-truncate, original lost".

- **Post-write nlink === 0 check** (review #3291863059):
  EINODE_UNLINKED_DURING_WRITE detects the fstat-to-close window where
  a concurrent rename-over drops our bound inode's link count to zero
  and our write goes to an anonymous inode close will free. Silent
  data loss path now surfaces.

- **fh.close() guarded in finally** (review #3291863044): close failure
  on NFS/FUSE was masking the original try-body exception (including
  the meaningful EOWNERSHIP_CHANGED, EINPLACE_*, EINODE_*). flush:true
  already fsync'd, so close-after-flush is best-effort.

- **fdStat.uid in canChmod** (review #3291863055 part 1): use the
  fd-bound verified value instead of expectedStat.uid. Defense in depth
  — a future weakening of the fstat guard won't silently widen chmod
  privilege.

- **fh.sync() after chmod** (review #3291863053): chmod is metadata,
  not covered by writeFile({ flush: true }). A crash before lazy
  metadata flush would lose the mode restoration (matters for
  setuid/setgid). One extra syscall, best-effort.

- **@remarks freshness contract** (review #3291863051 partial): JSDoc
  now spells out that expectedStat MUST be a fresh stat captured
  immediately before the call. Stale stats nullify every guard.

- **Concurrent-writer limitation noted** (review #3291863061 partial):
  added a "Known limitation — no advisory locking" paragraph to JSDoc
  rather than adopting flock (Linux-specific, NFS issues, scope
  expansion). Callers needing multi-process coordination should layer
  their own lockfile.

- **@throws documentation** (review #3291863051 partial): four
  documented error codes (EOWNERSHIP_CHANGED, EINODE_UNLINKED_DURING_WRITE,
  EINPLACE_TRUNCATE_FAILED, EINPLACE_WRITE_FAILED).

TESTS:

- **EINPLACE_WRITE_FAILED via FileHandle.prototype.writeFile monkey-patch**
  (review #3291863040): triggers the data-loss path, asserts the wrapped
  code + message + cause, and verifies the file is empty (truncate ran).

- **canChmod=false actually skips chmod** (review #3291863055 part 2):
  prior uid-mismatch test had desiredMode === current mode, couldn't
  distinguish "skipped" from "no-op". New test uses desiredMode=0o755
  on a 0o644 file under canChmod=false → asserts mode stays 0o644.

NOT ADOPTED:

- ENOENT/ELOOP/ENXIO catch extension (review #3291863043): keeping the
  strict refusal for swap-to-special-file. Silent fallthrough-to-replace
  was pre-PR atomic-rename behavior, but in shared-write workspaces
  (this PR's target users) a special-file appearing at the target path
  is a signal worth surfacing, not papering over.

- Diagnostic logging (review #3291863049): the function has no logger
  dependency today; adding one is an architecture decision outside
  this PR's scope. The path taken is implied by the side effects
  (inode preserved vs new) but agreed: out-of-band telemetry would
  help ops. Defer to follow-up.

- flock advisory locking (review #3291863061 main): scope expansion;
  Linux-specific semantics, NFS edge cases. Documented as known
  limitation instead.

- Integration test for ENOENT fallthrough at atomicWriteFile level
  (review #3291863043 part 1): ESM module bindings prevent monkey-
  patching writeInPlaceWithFdGuards from outside. The unit test for
  the helper's ENOENT path covers the throwing behavior; the catch is
  3 lines and review-visible. Defer until a refactor opens an
  injection seam.

- Error code string constants export (review #3291863051 part 3): two
  codes don't merit a constant module. Magic strings are fine at this
  size.

199/199 tests pass.

* docs(core): sync writeRuntimeStatus JSDoc with conditional-atomic contract

PR #4431 review follow-up: function-level JSDoc still claimed
unconditional "Atomically write" and "never sees a partially written
file", inconsistent with the module-level docblock updated in earlier
commits. Updated to describe the conditional-atomic behavior (atomic
when uid/gid matches, in-place fallback when ownership differs) and
explicitly note the concurrent-reader visibility trade-off in the
fallback path. Links to atomicWriteJSON for the full contract.

Doc-only change. 199/199 tests pass.

* fix(core): add explicit fh.sync() — FileHandle.writeFile ignores flush option

PR #4431 review follow-up (qwen3.7-max via /review):

CRITICAL — FileHandle.writeFile silently ignores flush:

Node.js FileHandle.writeFile takes an early-return path that bypasses
the flush option entirely (the option is only honored on the
path-based fs.writeFile form). Our previous code passed
{ flush: true } to fh.writeFile and relied on the implicit fsync.
The only explicit fh.sync() was nested in the chmod block guarded by
canChmod — which is FALSE precisely when a non-root group member
writes to a group-writable file they don't own (the exact shared-write
scenario this PR targets). Net effect: in that branch, zero fsync.
Data sits in the kernel page cache; a crash before lazy flush leaves
the file empty (truncate succeeded) or partially written.

Fix:
- Drop flush from the fhWriteOptions object (silently ignored anyway).
- Add an explicit `fh.sync()` after writeFile succeeds, gated on
  options.flush. Runs BEFORE the chmod block so the canChmod=false
  branch also fsyncs.
- The chmod-block fh.sync() becomes metadata-only (covers the mode
  change), as the data is already on disk.

Updated comments to reflect the actual semantics rather than the
incorrect "writeFile({ flush: true }) fsyncs" assumption.

TESTS (partial adoption of review #3293252349):

- EINPLACE_TRUNCATE_FAILED: sibling test to EINPLACE_WRITE_FAILED.
  Monkey-patches FileHandle.prototype.truncate to throw EIO; asserts
  err.code + cause + "original content is intact" message, and
  verifies the file's original bytes are unchanged (truncate didn't
  run).
- Buffer in in-place fallback: locks in binary fidelity (byte-exact
  comparison) so a future encoding-passthrough regression for Buffer
  data would be caught.

NOT ADOPTED in this commit:

- EINODE_UNLINKED_DURING_WRITE test: requires post-write fh.stat()
  mocking with call-count discrimination (first call: real stat for
  verification; second call: nlink=0). The monkey-patch pattern works
  but is fragile; deferred to a follow-up that may also refactor the
  helper to accept an injectable stat fn for cleaner testability.

201/201 tests pass.

* fix: correct stale flush comment + add fh.sync() regression test

- Fix misleading close() comment that said "flush:true already
  fsync'd" — the explicit fh.sync() does the actual fsync, not the
  flush option (which is silently ignored on FileHandle.writeFile).
- Add regression test verifying fh.sync() is called when flush:true
  and skipped when flush is absent, preventing silent removal of the
  core durability fix.

Addresses wenshao review threads from 2026-05-23.

* test: add EINODE_UNLINKED_DURING_WRITE regression test

Monkey-patches FileHandle.stat to return nlink:0 on the post-write
check, verifying the nlink guard throws with the correct error code.
Addresses wenshao review from 2026-05-28.

* simplify: replace writeInPlaceWithFdGuards with plain fs.writeFile

Address yiliang114's review (CHANGES_REQUESTED):

1. [Critical] Remove ~120 lines of fd-level TOCTOU hardening
   (writeInPlaceWithFdGuards) — over-engineering for a local CLI.
   The in-place fallback now uses plain fs.writeFile + tryChmod,
   matching the EXDEV fallback pattern.

2. [Suggestion] Fix macOS GID false-positive: only compare uid in
   ownershipWouldChange(). macOS inherits parent dir GID for new
   files, so egid !== file.gid was a false positive that needlessly
   dropped crash atomicity.

3. [Suggestion] Trim 60+ lines of JSDoc to project style (AGENTS.md:
   "default to none, add only when WHY is non-obvious").

Net: -748 lines. 24 tests pass.

* fix: restore Stats type import (TS2304 build failure)

* docs: narrow scope from uid/gid to uid-only preservation

The gid check is intentionally skipped because macOS inherits the
parent directory's GID for new files, making egid !== file.gid a
false positive. Update comments and PR description to match the
actual implementation scope.

* test: add inode assertion to symlink ownership-mismatch test

Proves the in-place fallback actually ran instead of atomic rename.

* Improve hooks matcher display (#4545)

* feat(cli): improve hooks matcher display

* test(cli): cover hooks navigation levels

* fix(cli): use session channel when closing ACP sessions (#4522)

Detach closeSession/killSession from the session entry's owning channel instead of the current attach target, so the correct channel is decremented and killed during channel overlap (old channel dying while a fresh channel is current). Extracts findChannelInfoForEntry/detachSessionIdFromEntryChannel helpers with unit + integration coverage. Fixes #4325.

* fix(core,cli): replace full-history structuredClone with shallow/tail variants to prevent OOM on resume (#4644)

* fix(core,cli): replace full-history structuredClone with shallow/tail variants to prevent OOM on resume

Several UI and service call sites clone the entire chat history via
structuredClone(getHistory()) every turn. On a resumed session with
thousands of entries, each clone allocates 150-200 MB transiently.
When multiple async side-requests overlap (suggestion generation,
auto-title, checkpointing), multiple clones coexist on the heap,
pushing V8 past its limit within 10 turns (2 GB heap cap).

Changes:
- AppContainer.tsx: use getHistoryTail(40, true) instead of
  getHistory(true) + slice(-40)
- btwCommand.ts: same pattern, use getHistoryTail(40, true)
- sessionTitle.ts: use getHistoryShallow() (read-only filtering)
- sessionRecap.ts: use getHistoryShallow() (read-only filtering)
- useGeminiStream.ts: use getHistoryShallow() for checkpoint
  serialization (only needs to survive JSON.stringify)

Closes #4624

* fix(test): update mocks for getHistoryShallow/getHistoryTail in sessionTitle and btwCommand tests

* fix(cli): migrate remaining getHistory() clone sites to shallow/tail variants

- AppContainer.tsx rewind path: getHistory() → getHistoryShallow()
  (only used read-only by computeApiTruncationIndex)
- Session.ts ACP rewind: getHistory() → getHistoryShallow()
  (only walks entries to compute truncation index)
- Session.ts stop-hook: getHistory() + filter(.model).pop() →
  getLastModelMessageText() (O(1) backward scan, no clone)

* fix(core): use client-level getHistoryShallow with fallback

sessionTitle.ts and sessionRecap.ts were calling
chat.getHistoryShallow() directly, bypassing the client-level
wrapper that provides a getHistory() fallback when the chat
implementation doesn't support shallow reads. Use
geminiClient.getHistoryShallow() instead.

Update test mocks to match the new call site.

* fix(test): add getHistoryShallow and getLastModelMessageText to Session test mocks

Session.ts now calls chat.getHistoryShallow() in rewindToTurn and
chat.getLastModelMessageText() in the Stop hook. Update all mockChat
instances in Session.test.ts to provide these methods.

* feat(cli): add respectUserColors and hideContextIndicator options for statusline (#4670)

* feat(cli): add respectUserColors option to preserve ANSI colors in
     statusline command output

* test(cli): add respectUserColors tests for useStatusLine and Footer

* feat(cli): add hideContextIndicator option to hide built-in context usage in footer

* docs: update statusline configuration docs with respectUserColors and hideContextIndicator

* fix(core): tolerate unsupported Streamable HTTP GET SSE (#4521)

Fixes #4326

* fix(insight): Harden insight facet normalization and empty qualitative handling (#3557)

* Harden insight facet normalization and empty qualitative handling

* feat: enhance AtAGlance component to accept target sections for dynamic rendering

* feat(cli): notify when background shells finish (#4355)

* feat(core): add simplify bundled skill (#3570)

* feat(core): add simplify bundled skill

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(cli): stabilize SettingsDialog restart prompt test

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(skills): use agent tool instead of task in simplify skill

The simplify skill referenced the 'task' tool for launching review passes,
but Qwen Code exposes 'agent' as the callable subagent tool ('task' is only
a legacy permission alias). Using 'task' would cause /simplify to stall when
trying to launch parallel review passes.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs: document simplify bundled skill

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* Update packages/core/src/skills/skill-manager.test.ts

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* fix(core): repair simplify skill tests

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* Update packages/core/src/skills/bundled/simplify/SKILL.md

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* fix(skills): address simplify review feedback (read-only passes, gitignore scope, safer dead-code removal)

- drop inert `argument-hint` frontmatter (argumentHint is never parsed or
  rendered anywhere; no other bundled skill uses it)
- mark Step 2 review passes read-only so edits stay isolated to Step 4
- narrow the no-diff fallback to `git ls-files --modified --others
  --exclude-standard` so ignored build output is excluded
- require a repo-wide caller check before removing code
- make the commands.md row state it edits code directly
- assert non-conflicting bundled skills survive cross-level dedup

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: wenshao <wenshao@U-K7F6PQY3-2157.local>

* feat(skills): add agent reproduction workflows (#4118)

* chore(skills): add codex reproduce workflows

* feat(agent-reproduce): implement agent reproduction workflow and supporting scripts

* feat(skills): capture reference agent state diffs

* feat(cli): virtual viewport for long conversations on ink 7 (#4146)

* chore(deps): re-upgrade ink 6 → 7.0.3 (upstream Static remount fix landed)

PR #3860 first upgraded ink 6 → 7.0.2. PR #4083 reverted because of a
TUI regression: `<Static>` did not re-emit items when its `key` prop
was bumped, so `/clear` / Ctrl+O / refreshStatic left the history area
blank under ink 7.0.2.

ink 7.0.3 (released after #4083) contains the exact fixes:

  - be9f44cda Fix: <Static> remount via key change drops new items (#948)
  - 669c4386c Fix: Drop stale <Static> output from fullStaticOutput on identity change (#950)
  - 7c2267c01 Fix `useBoxMetrics` not accepting ref objects with an initial null value (#945)

Changes:
  - `ink` ^6.2.3 → ^7.0.3 (root hoist + cli direct)
  - `react` ^19.1.0 → ^19.2.4 (cli direct; ink 7.0.3 peerDeps requires >=19.2.0)
  - `react`/`react-dom` overrides ^19.2.4 added so the transitive graph
    stays deduped to a single instance (avoids `Invalid hook call` from
    multiple React copies, the classic ink-upgrade hazard)
  - `wrap-ansi` already on ^10.0.0 from #4083's partial-revert (no change)

Verified:
  - `npm ls ink` → single `ink@7.0.3` across all peer deps
  - `npm ls react` → single `react@19.2.4`
  - `npm run typecheck --workspace=@qwen-code/qwen-code` clean
  - `npm run typecheck --workspace=@qwen-code/qwen-code-core` clean
  - Composer.test.tsx 20/20, MainContent.test.tsx 6/6, TableRenderer.test.tsx
    59/59 + 1 skipped — all key UI components green on the new ink

The Static-remount regression is upstream-fixed in 7.0.3, so the
runtime path is restored without needing #3941's overflowY-self-managed
viewport. #3941 (virtual viewport) remains an opt-in performance
feature on top.

* fix(deps,cli): add @types/react overrides + move refreshStatic out of setCurrentModel updater

Two follow-ups from the multi-round audit of the ink 7.0.3 re-upgrade:

1. @types/react / @types/react-dom now pinned to ^19.2.0 in root
   overrides. packages/web-templates still declares @types/react ^18.2.0
   in its devDeps. Today the CLI build is unaffected (web-templates's
   18.x types are nested in its own node_modules and the React-using
   src/insight and src/export-html files are excluded from its tsconfig
   build), but a future reincludes-or-hoist accident would land
   conflicting global JSX namespaces in the CLI compile graph. Match
   the dep dedup we already enforce for `react` and `react-dom` so the
   type graph stays as deduped as the runtime graph.

2. AppContainer's onModelChange handler was calling refreshStatic() as
   a side-effect inside the setCurrentModel updater. React.StrictMode
   double-invokes state updaters in dev, so model swaps fired two
   clearTerminal writes + two <Static> key bumps. The double work was
   masked under ink 6 (key changes were no-ops on <Static>), but ink
   7.0.3 honors key changes — the doubled work is now potentially
   visible as a faster flash-flash on every model switch.

   Refactor: setCurrentModel becomes a pure setter; refreshStatic
   moves into a useEffect keyed on currentModel with a ref-comparison
   guard so the first render doesn't fire. Single clearTerminal write
   per real model change, even under StrictMode.

Verified: npm ls ink → single 7.0.3, npm ls react → single 19.2.4,
npm ls @types/react → 19.2.10 hoisted (npm flags web-templates's 18.x
constraint as overridden, which is the intended behavior). Typecheck
clean across cli + core workspaces.

* docs(design): virtual viewport on ink 7 — analysis + PR sequence

Captures the architectural analysis of how to thoroughly close the
flicker / refresh-storm class of issues (#2950, #3118, #3007, #3838 UI
side, #3899 follow-on) using a virtualized history viewport.

- Surveys claude-code (forked ink) and gemini-cli (@jrichman/ink +
  ScrollableList + VirtualizedList) reference implementations.
- Confirms ink 7 already exposes the primitives needed
  (`useBoxMetrics`, `measureElement`, `useWindowSize`,
  `useAnimation`) — no fork swap required.
- Picks porting gemini-cli's virtualized list components to ink 7 with
  `ResizeObserver` -> `useBoxMetrics` and a custom `StaticRender`.
- Splits the work into V.0..V.4 PRs with scope, dependencies, risk.
- Lists open questions + 11-item approval checklist that must clear
  before V.0 implementation begins.

This is a docs-only PR per the project's design-first workflow. No
runtime code changes.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): virtual viewport for long conversations on ink 7

Port gemini-cli's VirtualizedList + ScrollableList to stock ink 7,
adapting for ink 7's available primitives:

- `overflowY="hidden"` + `marginTop={-scrollTop}` instead of ink-fork's
  `overflowY="scroll"` (ink 7 has proper clip/unclip in render-node-to-output)
- `useBoxMetrics` inside each VirtualizedListItem (Option A) instead of a
  single ResizeObserver WeakMap; reports height changes via onHeightChange
  callback so the parent can update its heights record
- Custom `StaticRender` as `React.memo` with a reference-equality comparator,
  keyed on `itemKey-static-{width}` to freeze completed conversation items
- Character scrollbar column (`│` track / `█` thumb) since ink 7 has no
  native scrollbar prop
- No ScrollProvider / mouse drag (deferred to a follow-up PR)

Wire into MainContent.tsx behind `ui.useTerminalBuffer` setting (Settings
dialog → UI → Virtualized History; default false — opt-in).

Key bindings: Shift+↑/↓ (line), PgUp/PgDn (page), Ctrl+Home/End (top/bottom).

Re-render optimisations:
- renderItem wrapped in useCallback so renderedItems useMemo only recomputes
  when actual deps change (not on every streaming tick)
- Completed history items passed by original object reference so
  VirtualHistoryItem = memo(HistoryItemDisplay) can bail out on stable props
- estimatedItemHeight / keyExtractor / isStaticItem defined as module-level
  constants with no closure deps

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(cli): add test coverage for virtual viewport scroll bindings and settings

- keyMatchers.test.ts: 6 new test cases for SCROLL_UP/DOWN, PAGE_UP/DOWN,
  SCROLL_HOME/END commands (41 tests total)
- settingsSchema.test.ts: assert ui.useTerminalBuffer is boolean, default false,
  showInDialog true, requiresRestart false

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): use ink 7 native overflow for VP pending items

In VP mode, pending items are rendered inside VirtualizedList's
overflowY="hidden" container, which uses ink 7's native clipping
as the viewport guard. Remove the availableTerminalHeight JS-
truncation bound from pending items in renderVirtualItem:

- JS truncation at terminal height would silently cut off content
  the user could scroll to read within the virtual viewport.
- ink 7 overflowY="hidden" on the VirtualizedList container is the
  correct clip guard — no JS line-counting workaround needed.
- Remove uiState.constrainHeight from renderVirtualItem deps (no
  longer referenced in the VP rendering path).

The legacy <Static> path is unchanged.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* perf(cli): binary-search offsets in virtualized list hot path

Replace linear findLastIndex / findIndex scans on the offsets array with
upperBound. Offsets are monotonic by construction, so the lookups inside
the render body and getAnchorForScrollTop drop from O(n) to O(log n).
Material for thousand-turn sessions where the lookup runs on every frame.

* fix(cli): wire ShowMoreLines + skip clearTerminal in VP mode

Two audit-found bugs in the VP path:

1. `<ShowMoreLines>` was outside the `<OverflowProvider>` that wraps
   `<ScrollableList>` in VP mode. `useOverflowState()` returns
   `undefined` outside the provider, so the component returned `null`
   and the "press ctrl-s to show more lines" affordance silently
   disappeared. Move `<ShowMoreLines>` inside the provider so the hook
   sees the live overflow state, matching the legacy path.

2. `refreshStatic()` and `repaintStaticViewport()` wrote
   `clearTerminal` / `cursorTo+eraseDown` to the host terminal
   unconditionally. In VP mode the React tree owns the visible region
   via ink 7's native `overflowY="hidden"` clipping — the physical
   write is a wasted flash on Ctrl+O / Alt+M / model change / resize.
   Guard both writes on `useTerminalBuffer === false`. The
   `historyRemountKey` bump still fires so the legacy `<Static>`
   fallback would still remount if someone toggled the setting mid-
   session.

Extends the targeted-repaint pattern introduced in #3967 to all
refreshStatic call sites, gated by the VP setting instead of by event
type.

* fix(cli): VP renderItem stability + source-copy offsets + heights GC

Three audit-found regressions tightened, in order of severity:

1. **Source-copy index offsets missing in VP** — legacy `<Static>` path
   threads per-item `sourceCopyIndexOffsets` so `/copy mermaid N` /
   `/copy latex N` hints stay stable across continuation messages. VP
   `renderVirtualItem` was not passing this prop, so the copy hints
   shown under each diagram drifted on every `gemini_content` chunk
   (the clipboard mechanism itself still worked from raw history; only
   the displayed number was wrong). Add two lookup tables —
   identity-keyed for static items, index-keyed for pending — without
   changing the VirtualizedList data signature, and thread offsets in
   both render branches.

2. **`renderVirtualItem` callback invalidated on every streaming tick**
   — its deps included `activePtyId` / `embeddedShellFocused` /
   `isEditorDialogOpen`, all of which flip mid-stream when a shell
   tool runs or a dialog opens. Each flip rebuilt the callback,
   invalidated `VirtualizedList.renderedItems`'s useMemo, and forced
   every static item to re-render through `<StaticRender>` — defeating
   the very memoization the design relies on. Move the three pending-
   only fields into a ref read inside the callback. Static-item closure
   now depends only on inputs that legitimately affect static output
   (terminalWidth, slashCommands, getCompactLabel, …). Pending items
   still re-render correctly because their item identity changes per
   tick, so the callback is called fresh each time and reads the
   latest ref.

3. **`pending` items now honour `constrainHeight`** in VP, matching the
   legacy path. Previously VP unconditionally passed `undefined` for
   `availableTerminalHeight` on pending, relying on the viewport
   `overflowY="hidden"` clip to limit visible size — but that hid the
   `<ShowMoreLines>` affordance from the user. Now that ShowMoreLines
   is correctly wired (previous commit), restore parity.

4. **Heights map memory leak** in `VirtualizedList` — `setHeights` only
   grew. Each `/clear` left orphan `h-N` keys; each pending → completed
   transition left orphan `p-N` keys. Add a `useLayoutEffect` that
   prunes entries whose keys are not in the current `data`. Runs in
   layout phase so the prune commits in the same paint as the data
   change — no stale-offsets frame.

* test+fix(cli): VP path coverage + stabilize absorbedCallIds empty Set

Completion-pass artifacts driven by the multi-agent audit:

- Settings description rewritten to enumerate the symptoms VP fixes so
  users with active flicker reports can find the toggle without reading
  the design doc.
- `absorbedCallIds` returns a module-level constant Set when compact mode
  is off, instead of a fresh `new Set()` per render. Fixes a hidden
  cascade: `activePtyId` flip mid-stream → useMemo runs → returns a new
  empty Set → `isSummaryAbsorbed` rebuilds → `renderVirtualItem`
  rebuilds → `VirtualizedList.renderedItems` recomputes → every static
  item re-renders. With the constant, the cascade dies at the source.
  Helps both VP and legacy paths.
- VP-path unit tests for MainContent (4 cases): ScrollableList mounts
  and Static does not when `useTerminalBuffer: true`; ShowMoreLines is
  reachable in VP mode (regression of the OverflowProvider mis-wrap);
  source-copy index offsets thread into renderItem for static items;
  renderItem callback identity is stable across `activePtyId` flips
  (proves the ref-based read keeps StaticRender memo effective).

* fix(cli): stabilize absorbedCallIds in compact mode + gate heights prune + tighten ShowMoreLines test

Round-2 audit follow-ups. Three real findings addressed; one flagged
false positive documented separately.

1. **absorbedCallIds Set identity now content-stable when compact mode is
   on.** The earlier EMPTY constant only short-circuited the compactMode=
   false path; when compact mode is enabled (some users default-on it),
   activePtyId / embeddedShellFocused flips during streaming still
   produced fresh Sets per render even when membership was unchanged,
   restarting the same cascade the pendingStateRef fix was meant to
   avoid. Compare-and-reuse via a ref: if the new Set has identical
   membership to the previous one, return the previous reference.

2. **`heights` map prune in `VirtualizedList` is gated.** Previously
   every streaming tick rebuilt an N-key Set and walked all heights,
   even on the steady-state path where nothing changes. Now only fires
   when the heights record has clearly outpaced live data
   (`size > max(8, 2 × data.length)`) — covers `/clear` and accumulated
   pending → completed transitions, skips the 30-Hz hot path entirely.

3. **VP ShowMoreLines test now actually verifies overflow connectivity.**
   Previous mock unconditionally rendered "SHOW_MORE", so the test only
   proved the JSX mounted — it would still pass if a future refactor
   moved `<OverflowProvider>` out of the VP tree again. The mock now
   reads `useOverflowState()` and emits "OVERFLOW_DISCONNECTED" when the
   context is missing. The VP test asserts both presence of "SHOW_MORE"
   and absence of the disconnected marker, so the regression is now
   caught.

Not addressed:
- Audit P0-1 claim that `renderMode` (Alt+M) / model-change updates
  don't reach VP static items: false positive. `renderMode` is a React
  Context (`RenderModeContext`), and Context propagation traverses the
  tree past `memo` boundaries — MarkdownDisplay's `useRenderMode()`
  consumer re-renders on context change regardless of whether
  `StaticRender` bails out. Verified by reading
  `packages/cli/src/ui/contexts/RenderModeContext.tsx` and
  `MarkdownDisplay.tsx:172`. No code change.
- Audit P1-2 pendingStateRef write-during-render race: speculative,
  relies on a multi-pass render path React 18+ does not currently use.
  Documented assumption in the existing inline comment.

* fix(cli): isolate renderItem errors + defensive height coerce + compact-mode mergedHistory stability

Round-3 audit follow-ups. Three real findings; the rest verified clean.

1. **`renderItem` errors no longer crash the CLI.** Previously a throw
   inside a per-item render propagated through `VirtualizedList`'s
   useMemo into React's commit phase, tearing down the whole Ink tree —
   one bad history record could nuke the session. Wrap each call in a
   try/catch and substitute a small red `[render error] …` text box on
   failure. The row stays in the viewport so the user can scroll past
   it.

2. **Defensive height coerce in offset accumulation.** A buggy
   `estimatedItemHeight` returning NaN / negative / Infinity would
   poison every downstream offset and break the `upperBound` /
   `findLastLE` binary search (which assumes monotonic offsets). Clamp
   to `Number.isFinite(raw) && raw > 0 ? raw : 0`. No-op for the
   in-tree estimators that return 3; insurance against future
   consumers.

3. **`mergedHistory` is content-stable when compact mode is on.** The
   Round-2 absorbedCallIds stability fix didn't reach this path:
   `mergeCompactToolGroups` always allocates a fresh array, and
   `mergedHistory`'s useMemo lists `activePtyId` / `embeddedShellFocused`
   as deps, so every streaming tick mid-shell-tool produced a new array
   even when items aligned. Cascade went `mergedHistory` → offsets map
   → `renderVirtualItem` → every static item re-rendered. Pair-wise
   compare new vs previous and return the previous reference when items
   align. Restores StaticRender memo effectiveness for compact-mode
   users.

Not addressed (audit findings deemed not worth fixing in this PR):
- `scrollToItem` silently no-ops when item is not in data — no current
  caller checks the return value, low impact.
- `allVirtualItems` array spread is O(n) per streaming tick — real but
  not a crash; revisit in a perf-focused follow-up.
- `itemRefs.current` is dead surface (never read) — cosmetic.
- StrictMode-only-in-DEBUG double-invoke paths verified safe.

* test+chore(cli): VP review round 4 — VirtualizedList/useBatchedScroll coverage + cleanups

Addresses wenshao's CHANGES_REQUESTED review on PR #3941.

- Add focused unit tests for `VirtualizedList` (9 cases) covering empty
  data, `renderStatic` full-render, `initialScrollIndex` with
  `SCROLL_TO_ITEM_END`, `targetScrollIndex` anchoring, imperative
  `scrollToEnd` / `scrollToIndex`, per-item `renderItem` error isolation,
  NaN/negative estimator coercion, and out-of-range `initialScrollIndex`
  clamping.
- Add `useBatchedScroll` unit tests (4 cases) covering initial reads,
  pending-value reads in the same tick, post-commit pending reset, and
  callback identity stability across rerenders.
- Remove dead `itemRefs` / `onSetRef` plumbing (declared, written, never
  read; `useCallback` with empty deps was also a stale-closure trap).
- Remove unused `isStatic?: boolean` from `VirtualizedListProps`
  (only `isStaticItem` is actually consumed).
- Tighten the render-phase setState block: each setter is now guarded
  by an equality check so React bails out of redundant updates, and a
  comment documents that this is the React-endorsed "adjusting state
  while rendering" pattern (the synchronous update avoids a one-frame
  flash at the previous position when `targetScrollIndex` changes).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* chore(cli): remove dead `dataRef` from VirtualizedList (round-4 followup)

Declared and written in a `useLayoutEffect` on every `data` change but
never read anywhere in the component. Flagged in wenshao's round-4 review
of PR #3941.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): collapse model-change effect back into one batched handler

wenshao's PR #4119 review correctly flagged that splitting the
onModelChange flow into two effects (b25831b0e) reintroduced the
issue #3899 freeze regression on every model switch:

  1. setCurrentModel(model) commits first, with the OLD
     historyRemountKey.
  2. <Static key={`${historyRemountKey}-${currentModel}`}> sees its
     key change (because currentModel did) and remounts immediately.
  3. MainContent's render-phase progressive-replay reset only fires
     when historyRemountKey changes, so replayCount is still the
     full mergedHistory.length from any prior catch-up.
  4. The remounted Static dumps the entire history in one synchronous
     layout pass — exactly the freeze progressive replay was added
     to avoid (#3899). The second effect's refreshStatic() bump
     arrives a render too late.

Fix: do not split. Both side effects (refreshStatic, which writes
clearTerminal + bumps historyRemountKey, and setCurrentModel) live
in the event handler again, with a ref guard for same-model
notifications. The React.StrictMode concern that motivated b25831b0e
is addressed by keeping the side effect OUT of the setState updater
(it now runs once per event-handler invocation, not once per
double-invoked updater call). Both setState calls land in the same
React batch, so historyRemountKey and currentModel update together —
MainContent's render-phase reset sees the new key, replayCount drops
to the first chunk, and Static remounts with chunked replay intact.

Tests:
- AppContainer.test.tsx: 4 new tests covering the synchronous
  refreshStatic side-effect contract, same-model no-op, ref-guarded
  StrictMode double-invoke, and unsubscribe-on-unmount.
- MainContent.test.tsx: new regression guard — when currentModel
  changes but historyRemountKey is held constant, progressive replay
  must NOT reset (pins the MainContent invariant the two-effect
  refactor accidentally relied on).

Verified: vitest packages/cli AppContainer + MainContent green (82/82).
Typecheck clean.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix+docs(cli): VP review round 5 — typecheck, doc drift, scroll keys

PR #4146 review feedback (wenshao + Claude Opus 4.7 audit) addressed:

Code:
- MainContent.test: activePtyId typed as number (was 'pty-xyz' string,
  broke tsc with TS2322 — the test only relies on reference change so
  any number works).
- VirtualizedList: sanitize renderItem error path. Display becomes the
  generic `[render error]` marker; full err goes to debugLogger.debug
  so file paths / partial tool state don't leak to scrollback.
- MainContent: move pendingSourceCopyOffsetsByIndex into a ref so it
  no longer rebuilds renderVirtualItem identity every streaming tick.
  Without this, VirtualizedList.renderedItems useMemo invalidated
  per-tick → JSX rebuilt for every visible item → memo(HistoryItem
  Display) was still bailing but allocations were O(visible) per tick.
- AppContainer: drop the misleading "state-driven scroll reset" claim
  in the VP refreshStatic comment. VP is intentionally near-no-op:
  the React tree owns the visible region, mergedHistory mutation is
  what refreshes the screen, and the remount-key bump is preserved
  only to keep the legacy Static branch in sync if the user toggles
  the flag off mid-session.
- StaticRender: rewrite JSDoc to match reality. The custom React.memo
  is NOT output caching like @jrichman/ink's StaticRender export;
  the comparator rarely matches (parent allocates fresh JSX); the
  real skip happens at memo(HistoryItemDisplay) one level deeper.

Docs:
- docs/design/virtual-viewport: sync file map (drop non-existent
  ScrollProvider.tsx / useAnimatedScrollbar.ts), PR sequence (one PR
  #4146, V.3-V.5 deferred), open-question + checklist resolution for
  #3905 (superseded) and base branch rename.
- docs/users/reference/keyboard-shortcuts: document the 6 VP scroll
  keys (Shift+↑/↓, PgUp/PgDn, Ctrl+Home/End) under a "History
  scrollback (when ui.useTerminalBuffer is on)" section. Previously
  the only discovery path was the Settings dialog description.

Verified: tsc --noEmit -p packages/cli ✓, vitest 160/160 ✓ across
AppContainer / MainContent / VirtualizedList / useBatchedScroll /
keyMatchers / settingsSchema, eslint clean on touched files.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): SGR mouse wheel scroll in VP mode

Recovers the most-felt UX regression vs legacy `<Static>` mode: when
`ui.useTerminalBuffer` is on, legacy users lose mouse wheel as a way
to scroll history (the host terminal stopped seeing the conversation
in its scrollback buffer). This PR enables button-event tracking
(`?1002h`) + SGR coordinates (`?1006h`) while the ScrollableList has
focus, parses wheel events off stdin, and routes them to scrollBy.

Scope kept tight on purpose:
- Wheel only. Hit-testing for scrollbar drag / click-to-position
  needs screen-absolute element coords; stock ink 7's useBoxMetrics
  returns yoga's parent-relative layout. Deferred to V.4 with two
  exit paths (upstream getBoundingBox to ink 7, or local yoga walker).
- Mouse mode is enabled only while ScrollableList is mounted; non-VP
  users never see their terminal flipped into button-event tracking.
- Side effect: native click-and-drag text selection is captured by
  the program. Docs + settings dialog description now spell out the
  Shift / Option (macOS) bypass.

Implementation:
- `ui/utils/mouse.ts` — SGR + X11 parser, ported and trimmed from
  gemini-cli (Google LLC, Apache-2.0). Single-consumer.
- `ui/hooks/useMouseEvents.ts` — enable/parse/disable lifecycle
  hook. Listens on stdin via `useStdin().stdin`, runs handler
  through a ref so callers don't have to memoize.
- `ui/components/shared/ScrollableList.tsx` — subscribe to mouse
  events, route wheel → `scrollBy(±3)`. Also drops a dead outer
  `<Box flexGrow={1}>` wrapper that held an unread containerRef
  and collapsed to zero height in ink-testing-library (the test
  renderer has no flex parent, so flexGrow=1 → 0 height → no items
  ever rendered, which is how this dead code was exposed).

Tests:
- `ui/utils/mouse.test.ts` — 14 cases: SGR parsing (wheel, presses,
  modifiers, move), X11 parsing, fallback chain, incomplete-sequence
  guard (including the >50-byte garbage cap).
- `ui/components/shared/ScrollableList.test.tsx` — 3 cases: wheel
  events shift the rendered window; hasFocus=false makes the mouse
  pipeline inactive (no throw); non-wheel events leave the window
  unchanged. Renders are wrapped in `<KeypressProvider>` (required
  by useKeypress in production but easy to forget in standalone
  tests).

Docs:
- `docs/users/reference/keyboard-shortcuts.md` — adds "Mouse wheel"
  row + the Shift/Option-to-select note.
- `packages/cli/src/config/settingsSchema.ts` — the in-app dialog
  description now mentions mouse wheel and the text-select bypass.
- `docs/design/virtual-viewport/README.md` — §1 status, §5 file map,
  §7 PR sequence all reflect mouse wheel landing in #4146 and the
  V.4–V.7 follow-up split (scrollbar drag / in-app search / alt-
  buffer / host-scrollback dual-write research).

Verified: tsc --noEmit -p packages/cli ✓, vitest 182/182 ✓ across
AppContainer / MainContent / VirtualizedList / ScrollableList /
useBatchedScroll / mouse / keyMatchers / settingsSchema.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): auto-hide animation for VP scrollbar thumb

Pairs with the SGR mouse-wheel work from the previous commit:
when the user actually scrolls, the thumb pops bright; after a
1.5s idle it fades into the dim track so the bar stops competing
with the conversation. The track column itself stays in layout
regardless, so the viewport never reflows mid-flash (which would
trigger per-item re-measure and a visible jitter).

Implementation kept minimal for stock ink 7:
- gemini-cli's `useAnimatedScrollbar` interpolates RGB colors via
  a theme + per-frame setInterval. The terminal can't render
  smooth fades anyway, so this hook collapses the state to a
  binary `isVisible` flag with a single setTimeout. ~75 LoC.
- `VirtualizedList` calls `flashScrollbar()` from a useLayoutEffect
  keyed on `clampedScrollTop`. The very first commit is skipped
  via a ref so initial mount doesn't paint a flash.
- The render switches the thumb glyph (`█` vs `│`) and `dimColor`
  based on `isVisible && inThumb`. Width stays 1 either way.

Tests (6 new):
- initial mount stays hidden (no spurious mount flash)
- flash → visible, hides after idle timeout, successive flashes
  reset the timer (no premature hide), idleHideMs<=0 disables
  auto-hide for tests that want to assert on the visible state,
  unmount cleans up the pending timer.

Doc updates:
- `docs/design/virtual-viewport/README.md` §1 status, §5 file map,
  §7 PR sequence — V.4 row now scopes only the drag/click-jump
  work (still coord-blocked); animated scrollbar moved out of
  deferred and into shipped.
- PR #4146 body — architecture table mentions the auto-hide, new
  files list adds `useAnimatedScrollbar.ts`, test count refreshed
  to 188/188.

Verified: tsc --noEmit -p packages/cli ✓, vitest 188/188 ✓.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): VP review round 6 — ESC bug, CI lint, scope-controlled cleanup

Triage of /review feedback from 2026-05-18 + 2026-05-19. Took the
ones that are real and small; declined the ones that are
false-positive / out-of-scope so this PR stops expanding.

Must-fix:
- CI Lint failure: vscode-ide-companion/schemas/settings.schema.json
  was stale after the keyboard-shortcuts description bump. Regenerated
  via `npm run generate:settings-schema`.
- useMouseEvents.ts had `const ESC = '';` (literal empty string after
  the raw 0x1B byte got stripped somewhere in the source pipeline).
  `buffer.indexOf('', 1) === 1` would have degraded garbage skipping
  to a one-byte scan, and the `else { buffer = ''; break }` branch
  could never run. Fixed by switching to the `'\x1b'` text escape and
  doing the same in `mouse.ts` (which had the raw byte, also fragile).
  Comment explains why.

Small wins (one-liners taken from the review batch):
- ScrollableList: rest-spread separates `hasFocus` from the props
  forwarded to VirtualizedList. Latent collision risk; no behaviour
  change today.
- VirtualizedList: `debugLogger.debug` when isReady=false so blank-
  viewport edge cases (tiny terminal / mid-resize race) become
  diagnosable from the debug log instead of looking like a hang.

Real perf (VP-only):
- MainContent: gated the progressive-Static-replay machinery behind
  `!useVirtualScroll`. The render-phase reset still consumes the
  remount-key bump so flag-off toggles mid-session catch up cleanly,
  but `setReplayCount` and the setImmediate chunking effect are now
  skipped for VP users. Saves ~M/CHUNK_SIZE wasted re-renders per
  Ctrl+O / model change on a 1000-turn session.

Belt-and-braces:
- useMouseEvents: added a `process.on('exit')` handler that writes
  the SGR mouse disable seq again. The React cleanup already covers
  normal unmount, but Ctrl+C / SIGTERM / parent kill bypass it and
  the terminal would otherwise stay in button-event-tracking mode
  after qwen exits.

Explicitly declined / deferred (with reasoning logged on the PR):
- requestAnimationFrame wheel throttle: rAF doesn't exist in Node;
  React 19 already batches state updates within a tick, and the
  renderedItems memo bounds the actual work to visible items. Will
  revisit if profiling shows it.
- Stable pending-item IDs (`p-N` keys shifting on completion): the
  observable jitter is at most one frame of estimated-vs-actual
  height delta. Moderate scope (creation-time ID allocation); fits
  better in a focused follow-up than in this PR.

Verified: tsc --noEmit -p packages/cli ✓, vitest 188/188 ✓ across
the full VP suite.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): scrollBy bottom uses live end anchor in virtualized list

When keyboard scroll reaches the bottom, scrollBy set isStickingToBottom
but anchored via getAnchorForScrollTop(maxScroll), a fixed {index,offset}
pixel anchor. scrollTo/scrollToEnd instead use {index: last, offset:
SCROLL_TO_ITEM_END}, which recomputes the bottom from live item heights
each render. The fixed anchor did not track the last item growing during
streaming, so scroll-to-bottom via keyboard lagged behind new tokens.
Align scrollBy's bottom branch with the sibling methods.

Reported by wenshao in PR review.

* fix(cli): parse mouse events via ink useInput, not a stdin data listener

useMouseEvents attached its own stdin.on('data', ...) listener. Adding a
'data' listener switches stdin into flowing mode, which drains the buffer
before ink's readable + stdin.read() reader (ink App) can consume it, so
all keyboard input routed through useInput was silently starved while
mouse mode was active.

Parse mouse sequences from ink's existing input pipeline via useInput
instead, so there is only one stdin reader. ink captures a full SGR
sequence (ESC [ < .. M/m) as a single CSI event and delivers it with the
leading ESC stripped, so we re-prepend it before parsing. Non-mouse input
does not match and is ignored; ink still routes input to the app's other
useInput handlers, so keyboard navigation keeps working.

Only SGR mode (1006h, which we enable) is parsed via this path; the legacy
X11 encoding is not recoverable through ink's CSI parser, which is the
encoding modern terminals stop emitting once 1006h is set.

Reported by wenshao in PR review.

* fix(cli): parse only SGR in mouse hook to avoid X11 paste misfire

The useInput-based mouse hook called parseMouseEvent, which also tries the
X11 fallback (parseX11MouseEvent). An X11 prefix (ESC [ M + 3 bytes) can
reach the handler via pasted text — ink emits paste content as input when
no paste listener is registered — and would misfire a spurious mouse event.
Call parseSGRMouseEvent directly so only the SGR encoding we enable (1006h)
is parsed, matching the hook's documented contract.

Reported by wenshao in PR review.

* test(cli): assert SGR mouse parser rejects X11 sequences

Locks in the security property behind the parseMouseEvent ->
parseSGRMouseEvent switch in useMouseEvents: an X11 sequence arriving as
pasted text must not misfire a mouse event. Asserts a well-formed X11
sequence is a valid X11 event yet returns null from parseSGRMouseEvent, so
a future revert to parseMouseEvent fails this test.

Reported by wenshao in PR review.

* test(cli): add VP scroll coverage + eslint-disable for useBatchedScroll

Cover keyboard scroll commands (Shift+Up/Down, PageUp/Down, Ctrl+Home/End),
scrollBy/scrollTo imperative API (positive/negative/overflow/clamp), and
auto-scroll-during-streaming state machine (stick-to-bottom, disengage on
user scroll, re-engage on scrollToEnd). Add missing eslint-disable-next-line
for intentionally dep-free useLayoutEffect in useBatchedScroll.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* chore(cli): remove trailing whitespace in useBatchedScroll

The eslint-disable-next-line comment was removed by eslint --fix as an
unused directive (exhaustive-deps does not flag a useLayoutEffect with
no dependency array). Clean up the residual blank line.

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>

* feat(cli): background housekeeping for stale file-history dirs (#4414)

PR #4064 introduced ~/.qwen/file-history/{sessionId}/ for /rewind but had
no cross-session cleanup — directories accumulated indefinitely. This adds
a generic background housekeeping framework with file-history cleanup as
its first user.

- 30-day mtime sweep, configurable via general.cleanupPeriodDays
- 10-min startup delay (1-min catch-up if last run >7d ago)
- 24h recurring cadence, idle-gated (defers if user typed in last 1 min)
- O_EXCL lockfile + marker mtime throttle (multi-process safe)
- Current session whitelisted via lazy config.getSessionId() — defends
  against long-idle active sessions and /clear minting a new session
- Negative cleanupPeriodDays values clamp to 1h minimum (defends against
  schema-bypass: a future cutoff would otherwise sweep everything)
- Zero new prod dependencies; ~70 lines of self-written O_EXCL throttle
  primitive in lieu of proper-lockfile (which pulls graceful-fs and
  monkey-patches every fs method on first require)
- All setTimeout(...).unref() — never blocks process exit

Closes #4173.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* fix(core): loosen auto-mode classifier timeouts, disable stage-2 thinking (#4680)

* fix(core): loosen auto-mode classifier timeouts, disable stage-2 thinking

The AUTO-mode classifier fails closed on timeout — a timed-out judge call
blocks the action as "unavailable". The tight 3s/10s stage budgets turned
transient slowness (slow network, large transcript, model queueing) into
spurious blocks of otherwise-valid actions. Raise them to 10s/30s so a
slow-but-healthy call is not treated as a hard block.

Also disable thinking in stage 2 (previously the only stage with
includeThoughts: true). This is a latency-sensitive permission gate the
user is actively waiting on; allocating a reasoning budget made the review
path slower and more expensive, which directly worsened the fail-closed
timeout. The model still records its reasoning in the structured
`thinking` output field — it just no longer gets an allocated budget.

Closes #4676

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(core): trim verbose comments in auto-mode classifier

Condense the three comments touched by this change (module docstring
stage-2 note, timeout-budget rationale, stage-2 thinkingConfig) while
keeping the essential "why". No logic changes.

Co-authored-by: Qwen-Coder <noreply@qwenlm.ai>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <noreply@qwenlm.ai>

* fix(core): coerce hostile-provider usage token counts (#4350 part 1) (#4439)

* fix(core): coerce hostile-provider usage token counts (#4350 part 1)

Hostile providers (broken upstream, OpenAI-compat proxy returning
null/NaN, misconfigured override) can emit non-finite or negative
values for `usageMetadata.{prompt,candidates,cached,total}TokenCount`.
Captured unguarded in `processStreamResponse`, these poison the
compaction gate arithmetic:

- `lastPromptTokenCount + NaN >= hard` is always false → hard-rescue
  is silently disabled, eventually OOMing the V8 heap.
- `Infinity >= hard` is always true → hard-rescue fires every send.

Route the four API capture sites through a `coerceUsageCount` helper
that maps unknown / non-finite / negative to 0. `Number.isFinite(-1)`
is true, so an explicit `>= 0` is needed in addition to `isFinite`.

Part 1 of the hostile-provider hardening from #4350. The companion
`computeThresholds` guard depends on the un-merged three-tier ladder
in #4345 and is deferred until that lands.

Covered by parametrized tests in `geminiChat.test.ts` over NaN,
±Infinity, negative, null, undefined, and string inputs, plus a
fallback test asserting …
TaimoorSiddiquiOfficial added a commit to TaimoorSiddiquiOfficial/HopCode that referenced this pull request Jun 2, 2026
* fix(cli): persist /memory toggle state across dialog reopen (#4650)

The Auto-memory / Auto-dream / Auto-skill rows initialized their state
from Config getters, which are frozen at startup and never reflect a
setValue() write. Each /memory reopen re-mounts the dialog and re-reads
that stale snapshot, so a just-flipped toggle appeared to revert. Read the
initial state from the live merged settings instead, matching the existing
write path (bareMode semantics preserved).

Also switch the test's `act` import to `react` — the previously used
@testing-library/react is declared in package.json but not installed, so
the suite could not run — and add a mount/unmount/remount regression test.

* Hide internal docs from docs site (#4357)

* fix(core): preserve uid in atomicWriteFile to avoid breaking shared-write files (#4431)

* fix(core): preserve uid/gid in atomicWriteFile to avoid breaking shared-write files

atomicWriteFile uses write-to-tmp + rename for crash atomicity. POSIX
rename creates a new inode owned by the calling process's euid/egid, so
the rename silently strips the original uid/gid. On shared-write setups
(e.g. a group-writable file owned by another user in a shared workspace
where the current user has group-write access), every Write/Edit/
NotebookEdit through qwen-code would reset ownership to the running
user and effectively revoke write access for the original collaborators.

The fix:

1. If the target exists and is owned by a different uid/gid than the
   process's effective uid/gid (and we are not root), fall back to
   in-place writeFile. This truncates the existing inode in place,
   preserving uid/gid. The trade-off is loss of crash atomicity for
   this specific case — an acceptable trade for not silently breaking
   shared-write file ownership.

2. If running as root, atomic rename is still used, and ownership is
   restored via chown(uid, gid) after the rename. Root can chown back;
   non-root cannot, hence the in-place fallback for non-root.

3. Windows is unaffected (no POSIX ownership semantics).

Tests:

- New: in-place fallback on uid mismatch — verify content updates, mode
  preserved, and inode unchanged (the inode is the signal that the
  fallback path ran rather than rename).
- New: same scenario triggered via gid mismatch.
- New: positive case — ownership matches → atomic rename → inode changes.

Regression: a v0.16.0 user reported "every write turns a world-writable
file into one other users can no longer write." Bisected to #4096 which
introduced atomicWriteFile + write-to-tmp + rename.

* fix(core): route root through in-place fallback + doc/test follow-ups

Review follow-ups on the atomic-write ownership fix:

1. Remove the root-special-case (rename + post-rename chown). chown
   silently fails inside user-namespaced or CAP_CHOWN-stripped Docker
   containers, which re-triggers the original bug for root-in-Docker
   users — exactly the scenario this fix was reported against. Routing
   root through the same in-place fallback as non-root eliminates this
   failure mode and drops an untestable branch (chown-back can't be
   exercised under non-root CI).

2. Document the three properties traded away by the in-place fallback:
   crash atomicity, concurrent-reader isolation, inotify watcher
   semantics (MODIFY vs MOVED_TO).

3. Document that the in-place fallback surfaces EACCES when the file's
   mode forbids the current user from writing — this is correct
   behavior (atomic rename used to silently replace files the user had
   no permission on, which was arguably a privilege issue).

4. Replace the brittle "see step 6 in the function doc" comment with a
   step-number-independent reference.

5. New test covering the EACCES path: chmod 0o444 + mocked geteuid
   triggers the fallback, fallback hits the read-only file, EACCES
   propagates cleanly, original content is preserved.

* fix(core): harden in-place fallback against symlink/unlink/inode races + doc/test follow-ups

Review follow-ups on #4431 ownership-preservation fix:

CRITICAL — in-place fallback security hardening (wenshao review):

The path-based `fs.writeFile(targetPath, ...)` fallback introduced
three races that the prior `rename(tmp, target)` form did not have:

1. Non-regular files (FIFO/socket/device): fs.writeFile calls
   open(O_WRONLY|O_CREAT|O_TRUNC). On a FIFO this blocks forever
   waiting for a reader. On a character/block device it writes to
   the actual device. The rename path replaced these with a
   regular file.

2. Symlink-swap TOCTOU: an attacker with parent-dir write can swap
   targetPath for a symlink between our stat and our writeFile.
   fs.writeFile follows symlinks at the destination; POSIX rename
   does not. In the very "shared-write workspace / Docker bind-mount"
   scenarios this PR targets, this lets a directory-writable
   attacker redirect agent writes elsewhere (e.g. /etc/passwd if
   the agent runs as root).

3. Unlink race: if targetPath is unlinked between stat and write,
   O_CREAT silently recreates it owned by the calling user — the
   exact ownership change the fallback was designed to prevent.
   Silent regression to the pre-fix bug under this race.

Fix: extract the fallback into writeInPlaceWithFdGuards():

  - open(target, O_WRONLY | O_TRUNC | O_NOFOLLOW) — no O_CREAT, so
    unlink-race surfaces ENOENT instead of silently recreating; and
    O_NOFOLLOW rejects symlink-swaps with ELOOP.
  - fstat(fd) verifies the bound inode's uid/gid still match
    existingStat — refuses the write if an inode-swap happened
    between stat and open.
  - Write through the fd (locked to the verified inode), chmod
    through the fd, close.

Caller now gates the fallback on existingStat.isFile() — non-regular
targets fall through to the atomic path which has well-defined
"replace special-file with regular-file" semantics.

DOC / TEST follow-ups:

- Add hardlink-propagation as a 4th trade-off in the in-place
  fallback JSDoc (review comment #4): rename creates a new inode so
  sibling hardlinks keep old content; in-place truncate+write keeps
  the inode so all hardlinks see new content.

- Update atomicWriteJSON JSDoc to note the write is now
  *conditionally* atomic (review comment #5): atomic when uid/gid
  matches the process, in-place when ownership differs. Previously
  the JSDoc still claimed unconditional atomicity.

- Update caller comments at runtimeStatus.ts and
  worktreeSessionService.ts that advertised crash-atomic writes via
  tmp+rename — those guarantees are now conditional (review
  comment #6).

- Add mode + tmp-leftover assertions to the gid-mismatch test to
  match the uid-mismatch test (review comment #2 — test
  consistency). Without these, a gid-fallback regression that
  silently dropped permissions or left a tmp file would not be
  caught.

- New test: FIFO + ownership mismatch must take the atomic path,
  not in-place (verifies the existingStat.isFile() guard works;
  hang on in-place would trip vitest timeout).

- New test: writing through a symlink with ownership mismatch
  exercises the resolve-then-stat-then-open flow and verifies the
  symlink itself is preserved.

Tests: 192/192 pass (atomicFileWrite + write-file + edit +
fileSystemService).

* fix(core): defer O_TRUNC and verify dev+ino in writeInPlaceWithFdGuards

PR #4431 review follow-up (wenshao critical):

The previous form opened with `O_WRONLY | O_TRUNC | O_NOFOLLOW`, which
truncated the bound file *before* the fd-bound fstat verification ran.
If an attacker swapped the path between the caller's stat and our
open, we would truncate the attacker's substituted inode (destroying
unrelated content) before detecting the swap.

Two fixes:

1. Open without O_TRUNC. Verify dev+ino+uid+gid+isFile match
   expectedStat through fh.stat(). Only then call fh.truncate(0)
   through the validated fd.

2. Expand the verification beyond uid+gid to include dev+ino+isFile.
   uid+gid alone misses a same-owner inode swap (attacker replaces
   the path with a different inode they own). dev+ino is the strong
   identity check; isFile catches a swap to FIFO/socket/device after
   the caller's existingStat.isFile() gate.

JSDoc updated to enumerate the four guards (NOFOLLOW, no CREAT, no
TRUNC at open, dev+ino+uid+gid+isFile via fstat) and explain why
truncation must wait until after verification.

192/192 tests pass.

* fix(core): close FIFO swap race with O_NONBLOCK + cover EOWNERSHIP_CHANGED path

PR #4431 review follow-up (deepseek-v4-pro via /review):

CRITICAL — FIFO swap TOCTOU:

The caller's `existingStat.isFile()` gate uses stat data captured
earlier. An attacker with parent-dir write can swap the regular file
for a FIFO between the caller's stat and our open inside
`writeInPlaceWithFdGuards`. The previous `O_WRONLY | O_NOFOLLOW` open
would then block indefinitely waiting for a FIFO reader; O_NOFOLLOW
only catches symlinks.

Fix: add O_NONBLOCK to the open flags. Defense in depth:

- On a reader-less FIFO, `open(O_WRONLY | O_NONBLOCK)` returns ENXIO
  immediately — no hang.
- If the FIFO has a reader (open succeeds), the subsequent fstat
  isFile() check still refuses the write via EOWNERSHIP_CHANGED.
- For regular files, O_NONBLOCK is a no-op.

CRITICAL test gap — EOWNERSHIP_CHANGED branch untested:

The primary TOCTOU defense (fdStat dev/ino/uid/gid/isFile vs
expectedStat) had no coverage. Exported `writeInPlaceWithFdGuards` so
it can be unit-tested directly:

- New test: simulate post-stat inode swap (unlink + recreate at same
  path), call helper with stale stat, assert EOWNERSHIP_CHANGED and
  that the attacker's content survives.
- New test: simulate post-stat regular→FIFO swap, assert open fails
  fast (ENXIO) or fstat catches it — either way no hang, no write.

DOC fix:

JSDoc said "we open read-write without truncating" but the code uses
O_WRONLY. Wording corrected to "write-only".

194/194 tests pass.

* fix(core): fix flaky inode-swap test + apply review follow-ups

PR #4431 review follow-up (glm-5.1 via /review) — 7 suggestions adopted,
1 partially adopted, 0 rejected:

CI FIX (Ubuntu test failure on tmpfs inode reuse):

The EOWNERSHIP_CHANGED inode-swap test used unlink+create to simulate
a post-stat swap. On Linux tmpfs the freshly-freed inode number is
often reused by the immediately-following create, so dev+ino remained
identical and the guard didn't trip (intermittent on Ubuntu CI; macOS
APFS happened to allocate different inodes). Switched to rename(decoy,
target) which moves an existing distinct inode into place, guaranteed
to differ from the original.

CODE:

- Wrap fh.writeFile failure after fh.truncate(0) with
  EINPLACE_WRITE_FAILED + cause, so callers see explicitly that the
  file was truncated and the write didn't complete (otherwise they
  see raw ENOSPC/EIO and may wrongly assume the original is intact
  given this lives in atomicFileWrite.ts).
- Skip fh.chmod when euid is neither root nor expectedStat.uid —
  chmod is guaranteed to fail with EPERM in that case (POSIX requires
  owner or root). Avoids a guaranteed-failing syscall on every call.
- Caller catches ENOENT from writeInPlaceWithFdGuards and falls
  through to atomic rename path. If the file was deleted between
  caller's stat and our open there is no ownership to preserve; the
  rename path correctly creates a new file at targetPath.

DOC:

- Replaced "defends against four races" with "hardened against
  post-stat races" (the bullet list has 5 items, the count was wrong).
- Reworded "non-regular targets must not reach this function" to
  describe defense-in-depth — O_NONBLOCK + !fdStat.isFile() reject
  post-stat regular→FIFO/socket/device swaps. The old wording made
  it look like O_NONBLOCK was redundant.
- Documented the dual chmod behavior (root vs non-root with foreign
  uid) inline.

TESTS:

- Added happy-path test for writeInPlaceWithFdGuards (write succeeds,
  inode preserved, mode preserved).
- Added ENOENT regression test (verifies the missing-O_CREAT
  property — if file unlinked between stat and open, no silent
  recreate with caller's uid).
- Renamed the misleading "O_NOFOLLOW guard" test (it actually tests
  resolve-through-symlink, not O_NOFOLLOW) to reflect what it does,
  and added a direct ELOOP test that drives writeInPlaceWithFdGuards
  with a path whose final component is a symlink — that's the real
  O_NOFOLLOW exercise.
- Fixed the FIFO test to pass a stat captured from the FIFO itself
  (not a stale regular-file stat) so only the FIFO-specific defense
  fires, not the inode/dev mismatch from a different file.

NOT ADOPTED:

- Skip-when-non-root chmod optimization adopted (small, useful), but
  the larger "structured chmod error model" deferred — best-effort
  matches the existing tryChmod pattern at file scope.

197/197 tests pass.

* fix(core): wrap truncate err + post-write nlink check + guard close + chmod sync

PR #4431 review follow-up (qwen-latest-series-invite-beta-v34 via /review)
— 7 of 10 suggestions adopted, 3 deferred:

CODE:

- **EINPLACE_TRUNCATE_FAILED wrap** (review #3291863048): symmetric to
  the existing EINPLACE_WRITE_FAILED — distinguishes "truncate failed,
  original intact" from "write failed post-truncate, original lost".

- **Post-write nlink === 0 check** (review #3291863059):
  EINODE_UNLINKED_DURING_WRITE detects the fstat-to-close window where
  a concurrent rename-over drops our bound inode's link count to zero
  and our write goes to an anonymous inode close will free. Silent
  data loss path now surfaces.

- **fh.close() guarded in finally** (review #3291863044): close failure
  on NFS/FUSE was masking the original try-body exception (including
  the meaningful EOWNERSHIP_CHANGED, EINPLACE_*, EINODE_*). flush:true
  already fsync'd, so close-after-flush is best-effort.

- **fdStat.uid in canChmod** (review #3291863055 part 1): use the
  fd-bound verified value instead of expectedStat.uid. Defense in depth
  — a future weakening of the fstat guard won't silently widen chmod
  privilege.

- **fh.sync() after chmod** (review #3291863053): chmod is metadata,
  not covered by writeFile({ flush: true }). A crash before lazy
  metadata flush would lose the mode restoration (matters for
  setuid/setgid). One extra syscall, best-effort.

- **@remarks freshness contract** (review #3291863051 partial): JSDoc
  now spells out that expectedStat MUST be a fresh stat captured
  immediately before the call. Stale stats nullify every guard.

- **Concurrent-writer limitation noted** (review #3291863061 partial):
  added a "Known limitation — no advisory locking" paragraph to JSDoc
  rather than adopting flock (Linux-specific, NFS issues, scope
  expansion). Callers needing multi-process coordination should layer
  their own lockfile.

- **@throws documentation** (review #3291863051 partial): four
  documented error codes (EOWNERSHIP_CHANGED, EINODE_UNLINKED_DURING_WRITE,
  EINPLACE_TRUNCATE_FAILED, EINPLACE_WRITE_FAILED).

TESTS:

- **EINPLACE_WRITE_FAILED via FileHandle.prototype.writeFile monkey-patch**
  (review #3291863040): triggers the data-loss path, asserts the wrapped
  code + message + cause, and verifies the file is empty (truncate ran).

- **canChmod=false actually skips chmod** (review #3291863055 part 2):
  prior uid-mismatch test had desiredMode === current mode, couldn't
  distinguish "skipped" from "no-op". New test uses desiredMode=0o755
  on a 0o644 file under canChmod=false → asserts mode stays 0o644.

NOT ADOPTED:

- ENOENT/ELOOP/ENXIO catch extension (review #3291863043): keeping the
  strict refusal for swap-to-special-file. Silent fallthrough-to-replace
  was pre-PR atomic-rename behavior, but in shared-write workspaces
  (this PR's target users) a special-file appearing at the target path
  is a signal worth surfacing, not papering over.

- Diagnostic logging (review #3291863049): the function has no logger
  dependency today; adding one is an architecture decision outside
  this PR's scope. The path taken is implied by the side effects
  (inode preserved vs new) but agreed: out-of-band telemetry would
  help ops. Defer to follow-up.

- flock advisory locking (review #3291863061 main): scope expansion;
  Linux-specific semantics, NFS edge cases. Documented as known
  limitation instead.

- Integration test for ENOENT fallthrough at atomicWriteFile level
  (review #3291863043 part 1): ESM module bindings prevent monkey-
  patching writeInPlaceWithFdGuards from outside. The unit test for
  the helper's ENOENT path covers the throwing behavior; the catch is
  3 lines and review-visible. Defer until a refactor opens an
  injection seam.

- Error code string constants export (review #3291863051 part 3): two
  codes don't merit a constant module. Magic strings are fine at this
  size.

199/199 tests pass.

* docs(core): sync writeRuntimeStatus JSDoc with conditional-atomic contract

PR #4431 review follow-up: function-level JSDoc still claimed
unconditional "Atomically write" and "never sees a partially written
file", inconsistent with the module-level docblock updated in earlier
commits. Updated to describe the conditional-atomic behavior (atomic
when uid/gid matches, in-place fallback when ownership differs) and
explicitly note the concurrent-reader visibility trade-off in the
fallback path. Links to atomicWriteJSON for the full contract.

Doc-only change. 199/199 tests pass.

* fix(core): add explicit fh.sync() — FileHandle.writeFile ignores flush option

PR #4431 review follow-up (qwen3.7-max via /review):

CRITICAL — FileHandle.writeFile silently ignores flush:

Node.js FileHandle.writeFile takes an early-return path that bypasses
the flush option entirely (the option is only honored on the
path-based fs.writeFile form). Our previous code passed
{ flush: true } to fh.writeFile and relied on the implicit fsync.
The only explicit fh.sync() was nested in the chmod block guarded by
canChmod — which is FALSE precisely when a non-root group member
writes to a group-writable file they don't own (the exact shared-write
scenario this PR targets). Net effect: in that branch, zero fsync.
Data sits in the kernel page cache; a crash before lazy flush leaves
the file empty (truncate succeeded) or partially written.

Fix:
- Drop flush from the fhWriteOptions object (silently ignored anyway).
- Add an explicit `fh.sync()` after writeFile succeeds, gated on
  options.flush. Runs BEFORE the chmod block so the canChmod=false
  branch also fsyncs.
- The chmod-block fh.sync() becomes metadata-only (covers the mode
  change), as the data is already on disk.

Updated comments to reflect the actual semantics rather than the
incorrect "writeFile({ flush: true }) fsyncs" assumption.

TESTS (partial adoption of review #3293252349):

- EINPLACE_TRUNCATE_FAILED: sibling test to EINPLACE_WRITE_FAILED.
  Monkey-patches FileHandle.prototype.truncate to throw EIO; asserts
  err.code + cause + "original content is intact" message, and
  verifies the file's original bytes are unchanged (truncate didn't
  run).
- Buffer in in-place fallback: locks in binary fidelity (byte-exact
  comparison) so a future encoding-passthrough regression for Buffer
  data would be caught.

NOT ADOPTED in this commit:

- EINODE_UNLINKED_DURING_WRITE test: requires post-write fh.stat()
  mocking with call-count discrimination (first call: real stat for
  verification; second call: nlink=0). The monkey-patch pattern works
  but is fragile; deferred to a follow-up that may also refactor the
  helper to accept an injectable stat fn for cleaner testability.

201/201 tests pass.

* fix: correct stale flush comment + add fh.sync() regression test

- Fix misleading close() comment that said "flush:true already
  fsync'd" — the explicit fh.sync() does the actual fsync, not the
  flush option (which is silently ignored on FileHandle.writeFile).
- Add regression test verifying fh.sync() is called when flush:true
  and skipped when flush is absent, preventing silent removal of the
  core durability fix.

Addresses wenshao review threads from 2026-05-23.

* test: add EINODE_UNLINKED_DURING_WRITE regression test

Monkey-patches FileHandle.stat to return nlink:0 on the post-write
check, verifying the nlink guard throws with the correct error code.
Addresses wenshao review from 2026-05-28.

* simplify: replace writeInPlaceWithFdGuards with plain fs.writeFile

Address yiliang114's review (CHANGES_REQUESTED):

1. [Critical] Remove ~120 lines of fd-level TOCTOU hardening
   (writeInPlaceWithFdGuards) — over-engineering for a local CLI.
   The in-place fallback now uses plain fs.writeFile + tryChmod,
   matching the EXDEV fallback pattern.

2. [Suggestion] Fix macOS GID false-positive: only compare uid in
   ownershipWouldChange(). macOS inherits parent dir GID for new
   files, so egid !== file.gid was a false positive that needlessly
   dropped crash atomicity.

3. [Suggestion] Trim 60+ lines of JSDoc to project style (AGENTS.md:
   "default to none, add only when WHY is non-obvious").

Net: -748 lines. 24 tests pass.

* fix: restore Stats type import (TS2304 build failure)

* docs: narrow scope from uid/gid to uid-only preservation

The gid check is intentionally skipped because macOS inherits the
parent directory's GID for new files, making egid !== file.gid a
false positive. Update comments and PR description to match the
actual implementation scope.

* test: add inode assertion to symlink ownership-mismatch test

Proves the in-place fallback actually ran instead of atomic rename.

* Improve hooks matcher display (#4545)

* feat(cli): improve hooks matcher display

* test(cli): cover hooks navigation levels

* fix(cli): use session channel when closing ACP sessions (#4522)

Detach closeSession/killSession from the session entry's owning channel instead of the current attach target, so the correct channel is decremented and killed during channel overlap (old channel dying while a fresh channel is current). Extracts findChannelInfoForEntry/detachSessionIdFromEntryChannel helpers with unit + integration coverage. Fixes #4325.

* fix(core,cli): replace full-history structuredClone with shallow/tail variants to prevent OOM on resume (#4644)

* fix(core,cli): replace full-history structuredClone with shallow/tail variants to prevent OOM on resume

Several UI and service call sites clone the entire chat history via
structuredClone(getHistory()) every turn. On a resumed session with
thousands of entries, each clone allocates 150-200 MB transiently.
When multiple async side-requests overlap (suggestion generation,
auto-title, checkpointing), multiple clones coexist on the heap,
pushing V8 past its limit within 10 turns (2 GB heap cap).

Changes:
- AppContainer.tsx: use getHistoryTail(40, true) instead of
  getHistory(true) + slice(-40)
- btwCommand.ts: same pattern, use getHistoryTail(40, true)
- sessionTitle.ts: use getHistoryShallow() (read-only filtering)
- sessionRecap.ts: use getHistoryShallow() (read-only filtering)
- useGeminiStream.ts: use getHistoryShallow() for checkpoint
  serialization (only needs to survive JSON.stringify)

Closes #4624

* fix(test): update mocks for getHistoryShallow/getHistoryTail in sessionTitle and btwCommand tests

* fix(cli): migrate remaining getHistory() clone sites to shallow/tail variants

- AppContainer.tsx rewind path: getHistory() → getHistoryShallow()
  (only used read-only by computeApiTruncationIndex)
- Session.ts ACP rewind: getHistory() → getHistoryShallow()
  (only walks entries to compute truncation index)
- Session.ts stop-hook: getHistory() + filter(.model).pop() →
  getLastModelMessageText() (O(1) backward scan, no clone)

* fix(core): use client-level getHistoryShallow with fallback

sessionTitle.ts and sessionRecap.ts were calling
chat.getHistoryShallow() directly, bypassing the client-level
wrapper that provides a getHistory() fallback when the chat
implementation doesn't support shallow reads. Use
geminiClient.getHistoryShallow() instead.

Update test mocks to match the new call site.

* fix(test): add getHistoryShallow and getLastModelMessageText to Session test mocks

Session.ts now calls chat.getHistoryShallow() in rewindToTurn and
chat.getLastModelMessageText() in the Stop hook. Update all mockChat
instances in Session.test.ts to provide these methods.

* feat(cli): add respectUserColors and hideContextIndicator options for statusline (#4670)

* feat(cli): add respectUserColors option to preserve ANSI colors in
     statusline command output

* test(cli): add respectUserColors tests for useStatusLine and Footer

* feat(cli): add hideContextIndicator option to hide built-in context usage in footer

* docs: update statusline configuration docs with respectUserColors and hideContextIndicator

* fix(core): tolerate unsupported Streamable HTTP GET SSE (#4521)

Fixes #4326

* fix(insight): Harden insight facet normalization and empty qualitative handling (#3557)

* Harden insight facet normalization and empty qualitative handling

* feat: enhance AtAGlance component to accept target sections for dynamic rendering

* feat(cli): notify when background shells finish (#4355)

* feat(core): add simplify bundled skill (#3570)

* feat(core): add simplify bundled skill

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(cli): stabilize SettingsDialog restart prompt test

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(skills): use agent tool instead of task in simplify skill

The simplify skill referenced the 'task' tool for launching review passes,
but Qwen Code exposes 'agent' as the callable subagent tool ('task' is only
a legacy permission alias). Using 'task' would cause /simplify to stall when
trying to launch parallel review passes.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs: document simplify bundled skill

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* Update packages/core/src/skills/skill-manager.test.ts

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* fix(core): repair simplify skill tests

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* Update packages/core/src/skills/bundled/simplify/SKILL.md

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* fix(skills): address simplify review feedback (read-only passes, gitignore scope, safer dead-code removal)

- drop inert `argument-hint` frontmatter (argumentHint is never parsed or
  rendered anywhere; no other bundled skill uses it)
- mark Step 2 review passes read-only so edits stay isolated to Step 4
- narrow the no-diff fallback to `git ls-files --modified --others
  --exclude-standard` so ignored build output is excluded
- require a repo-wide caller check before removing code
- make the commands.md row state it edits code directly
- assert non-conflicting bundled skills survive cross-level dedup

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: wenshao <wenshao@U-K7F6PQY3-2157.local>

* feat(skills): add agent reproduction workflows (#4118)

* chore(skills): add codex reproduce workflows

* feat(agent-reproduce): implement agent reproduction workflow and supporting scripts

* feat(skills): capture reference agent state diffs

* feat(cli): virtual viewport for long conversations on ink 7 (#4146)

* chore(deps): re-upgrade ink 6 → 7.0.3 (upstream Static remount fix landed)

PR #3860 first upgraded ink 6 → 7.0.2. PR #4083 reverted because of a
TUI regression: `<Static>` did not re-emit items when its `key` prop
was bumped, so `/clear` / Ctrl+O / refreshStatic left the history area
blank under ink 7.0.2.

ink 7.0.3 (released after #4083) contains the exact fixes:

  - be9f44cda Fix: <Static> remount via key change drops new items (#948)
  - 669c4386c Fix: Drop stale <Static> output from fullStaticOutput on identity change (#950)
  - 7c2267c01 Fix `useBoxMetrics` not accepting ref objects with an initial null value (#945)

Changes:
  - `ink` ^6.2.3 → ^7.0.3 (root hoist + cli direct)
  - `react` ^19.1.0 → ^19.2.4 (cli direct; ink 7.0.3 peerDeps requires >=19.2.0)
  - `react`/`react-dom` overrides ^19.2.4 added so the transitive graph
    stays deduped to a single instance (avoids `Invalid hook call` from
    multiple React copies, the classic ink-upgrade hazard)
  - `wrap-ansi` already on ^10.0.0 from #4083's partial-revert (no change)

Verified:
  - `npm ls ink` → single `ink@7.0.3` across all peer deps
  - `npm ls react` → single `react@19.2.4`
  - `npm run typecheck --workspace=@qwen-code/qwen-code` clean
  - `npm run typecheck --workspace=@qwen-code/qwen-code-core` clean
  - Composer.test.tsx 20/20, MainContent.test.tsx 6/6, TableRenderer.test.tsx
    59/59 + 1 skipped — all key UI components green on the new ink

The Static-remount regression is upstream-fixed in 7.0.3, so the
runtime path is restored without needing #3941's overflowY-self-managed
viewport. #3941 (virtual viewport) remains an opt-in performance
feature on top.

* fix(deps,cli): add @types/react overrides + move refreshStatic out of setCurrentModel updater

Two follow-ups from the multi-round audit of the ink 7.0.3 re-upgrade:

1. @types/react / @types/react-dom now pinned to ^19.2.0 in root
   overrides. packages/web-templates still declares @types/react ^18.2.0
   in its devDeps. Today the CLI build is unaffected (web-templates's
   18.x types are nested in its own node_modules and the React-using
   src/insight and src/export-html files are excluded from its tsconfig
   build), but a future reincludes-or-hoist accident would land
   conflicting global JSX namespaces in the CLI compile graph. Match
   the dep dedup we already enforce for `react` and `react-dom` so the
   type graph stays as deduped as the runtime graph.

2. AppContainer's onModelChange handler was calling refreshStatic() as
   a side-effect inside the setCurrentModel updater. React.StrictMode
   double-invokes state updaters in dev, so model swaps fired two
   clearTerminal writes + two <Static> key bumps. The double work was
   masked under ink 6 (key changes were no-ops on <Static>), but ink
   7.0.3 honors key changes — the doubled work is now potentially
   visible as a faster flash-flash on every model switch.

   Refactor: setCurrentModel becomes a pure setter; refreshStatic
   moves into a useEffect keyed on currentModel with a ref-comparison
   guard so the first render doesn't fire. Single clearTerminal write
   per real model change, even under StrictMode.

Verified: npm ls ink → single 7.0.3, npm ls react → single 19.2.4,
npm ls @types/react → 19.2.10 hoisted (npm flags web-templates's 18.x
constraint as overridden, which is the intended behavior). Typecheck
clean across cli + core workspaces.

* docs(design): virtual viewport on ink 7 — analysis + PR sequence

Captures the architectural analysis of how to thoroughly close the
flicker / refresh-storm class of issues (#2950, #3118, #3007, #3838 UI
side, #3899 follow-on) using a virtualized history viewport.

- Surveys claude-code (forked ink) and gemini-cli (@jrichman/ink +
  ScrollableList + VirtualizedList) reference implementations.
- Confirms ink 7 already exposes the primitives needed
  (`useBoxMetrics`, `measureElement`, `useWindowSize`,
  `useAnimation`) — no fork swap required.
- Picks porting gemini-cli's virtualized list components to ink 7 with
  `ResizeObserver` -> `useBoxMetrics` and a custom `StaticRender`.
- Splits the work into V.0..V.4 PRs with scope, dependencies, risk.
- Lists open questions + 11-item approval checklist that must clear
  before V.0 implementation begins.

This is a docs-only PR per the project's design-first workflow. No
runtime code changes.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): virtual viewport for long conversations on ink 7

Port gemini-cli's VirtualizedList + ScrollableList to stock ink 7,
adapting for ink 7's available primitives:

- `overflowY="hidden"` + `marginTop={-scrollTop}` instead of ink-fork's
  `overflowY="scroll"` (ink 7 has proper clip/unclip in render-node-to-output)
- `useBoxMetrics` inside each VirtualizedListItem (Option A) instead of a
  single ResizeObserver WeakMap; reports height changes via onHeightChange
  callback so the parent can update its heights record
- Custom `StaticRender` as `React.memo` with a reference-equality comparator,
  keyed on `itemKey-static-{width}` to freeze completed conversation items
- Character scrollbar column (`│` track / `█` thumb) since ink 7 has no
  native scrollbar prop
- No ScrollProvider / mouse drag (deferred to a follow-up PR)

Wire into MainContent.tsx behind `ui.useTerminalBuffer` setting (Settings
dialog → UI → Virtualized History; default false — opt-in).

Key bindings: Shift+↑/↓ (line), PgUp/PgDn (page), Ctrl+Home/End (top/bottom).

Re-render optimisations:
- renderItem wrapped in useCallback so renderedItems useMemo only recomputes
  when actual deps change (not on every streaming tick)
- Completed history items passed by original object reference so
  VirtualHistoryItem = memo(HistoryItemDisplay) can bail out on stable props
- estimatedItemHeight / keyExtractor / isStaticItem defined as module-level
  constants with no closure deps

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(cli): add test coverage for virtual viewport scroll bindings and settings

- keyMatchers.test.ts: 6 new test cases for SCROLL_UP/DOWN, PAGE_UP/DOWN,
  SCROLL_HOME/END commands (41 tests total)
- settingsSchema.test.ts: assert ui.useTerminalBuffer is boolean, default false,
  showInDialog true, requiresRestart false

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): use ink 7 native overflow for VP pending items

In VP mode, pending items are rendered inside VirtualizedList's
overflowY="hidden" container, which uses ink 7's native clipping
as the viewport guard. Remove the availableTerminalHeight JS-
truncation bound from pending items in renderVirtualItem:

- JS truncation at terminal height would silently cut off content
  the user could scroll to read within the virtual viewport.
- ink 7 overflowY="hidden" on the VirtualizedList container is the
  correct clip guard — no JS line-counting workaround needed.
- Remove uiState.constrainHeight from renderVirtualItem deps (no
  longer referenced in the VP rendering path).

The legacy <Static> path is unchanged.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* perf(cli): binary-search offsets in virtualized list hot path

Replace linear findLastIndex / findIndex scans on the offsets array with
upperBound. Offsets are monotonic by construction, so the lookups inside
the render body and getAnchorForScrollTop drop from O(n) to O(log n).
Material for thousand-turn sessions where the lookup runs on every frame.

* fix(cli): wire ShowMoreLines + skip clearTerminal in VP mode

Two audit-found bugs in the VP path:

1. `<ShowMoreLines>` was outside the `<OverflowProvider>` that wraps
   `<ScrollableList>` in VP mode. `useOverflowState()` returns
   `undefined` outside the provider, so the component returned `null`
   and the "press ctrl-s to show more lines" affordance silently
   disappeared. Move `<ShowMoreLines>` inside the provider so the hook
   sees the live overflow state, matching the legacy path.

2. `refreshStatic()` and `repaintStaticViewport()` wrote
   `clearTerminal` / `cursorTo+eraseDown` to the host terminal
   unconditionally. In VP mode the React tree owns the visible region
   via ink 7's native `overflowY="hidden"` clipping — the physical
   write is a wasted flash on Ctrl+O / Alt+M / model change / resize.
   Guard both writes on `useTerminalBuffer === false`. The
   `historyRemountKey` bump still fires so the legacy `<Static>`
   fallback would still remount if someone toggled the setting mid-
   session.

Extends the targeted-repaint pattern introduced in #3967 to all
refreshStatic call sites, gated by the VP setting instead of by event
type.

* fix(cli): VP renderItem stability + source-copy offsets + heights GC

Three audit-found regressions tightened, in order of severity:

1. **Source-copy index offsets missing in VP** — legacy `<Static>` path
   threads per-item `sourceCopyIndexOffsets` so `/copy mermaid N` /
   `/copy latex N` hints stay stable across continuation messages. VP
   `renderVirtualItem` was not passing this prop, so the copy hints
   shown under each diagram drifted on every `gemini_content` chunk
   (the clipboard mechanism itself still worked from raw history; only
   the displayed number was wrong). Add two lookup tables —
   identity-keyed for static items, index-keyed for pending — without
   changing the VirtualizedList data signature, and thread offsets in
   both render branches.

2. **`renderVirtualItem` callback invalidated on every streaming tick**
   — its deps included `activePtyId` / `embeddedShellFocused` /
   `isEditorDialogOpen`, all of which flip mid-stream when a shell
   tool runs or a dialog opens. Each flip rebuilt the callback,
   invalidated `VirtualizedList.renderedItems`'s useMemo, and forced
   every static item to re-render through `<StaticRender>` — defeating
   the very memoization the design relies on. Move the three pending-
   only fields into a ref read inside the callback. Static-item closure
   now depends only on inputs that legitimately affect static output
   (terminalWidth, slashCommands, getCompactLabel, …). Pending items
   still re-render correctly because their item identity changes per
   tick, so the callback is called fresh each time and reads the
   latest ref.

3. **`pending` items now honour `constrainHeight`** in VP, matching the
   legacy path. Previously VP unconditionally passed `undefined` for
   `availableTerminalHeight` on pending, relying on the viewport
   `overflowY="hidden"` clip to limit visible size — but that hid the
   `<ShowMoreLines>` affordance from the user. Now that ShowMoreLines
   is correctly wired (previous commit), restore parity.

4. **Heights map memory leak** in `VirtualizedList` — `setHeights` only
   grew. Each `/clear` left orphan `h-N` keys; each pending → completed
   transition left orphan `p-N` keys. Add a `useLayoutEffect` that
   prunes entries whose keys are not in the current `data`. Runs in
   layout phase so the prune commits in the same paint as the data
   change — no stale-offsets frame.

* test+fix(cli): VP path coverage + stabilize absorbedCallIds empty Set

Completion-pass artifacts driven by the multi-agent audit:

- Settings description rewritten to enumerate the symptoms VP fixes so
  users with active flicker reports can find the toggle without reading
  the design doc.
- `absorbedCallIds` returns a module-level constant Set when compact mode
  is off, instead of a fresh `new Set()` per render. Fixes a hidden
  cascade: `activePtyId` flip mid-stream → useMemo runs → returns a new
  empty Set → `isSummaryAbsorbed` rebuilds → `renderVirtualItem`
  rebuilds → `VirtualizedList.renderedItems` recomputes → every static
  item re-renders. With the constant, the cascade dies at the source.
  Helps both VP and legacy paths.
- VP-path unit tests for MainContent (4 cases): ScrollableList mounts
  and Static does not when `useTerminalBuffer: true`; ShowMoreLines is
  reachable in VP mode (regression of the OverflowProvider mis-wrap);
  source-copy index offsets thread into renderItem for static items;
  renderItem callback identity is stable across `activePtyId` flips
  (proves the ref-based read keeps StaticRender memo effective).

* fix(cli): stabilize absorbedCallIds in compact mode + gate heights prune + tighten ShowMoreLines test

Round-2 audit follow-ups. Three real findings addressed; one flagged
false positive documented separately.

1. **absorbedCallIds Set identity now content-stable when compact mode is
   on.** The earlier EMPTY constant only short-circuited the compactMode=
   false path; when compact mode is enabled (some users default-on it),
   activePtyId / embeddedShellFocused flips during streaming still
   produced fresh Sets per render even when membership was unchanged,
   restarting the same cascade the pendingStateRef fix was meant to
   avoid. Compare-and-reuse via a ref: if the new Set has identical
   membership to the previous one, return the previous reference.

2. **`heights` map prune in `VirtualizedList` is gated.** Previously
   every streaming tick rebuilt an N-key Set and walked all heights,
   even on the steady-state path where nothing changes. Now only fires
   when the heights record has clearly outpaced live data
   (`size > max(8, 2 × data.length)`) — covers `/clear` and accumulated
   pending → completed transitions, skips the 30-Hz hot path entirely.

3. **VP ShowMoreLines test now actually verifies overflow connectivity.**
   Previous mock unconditionally rendered "SHOW_MORE", so the test only
   proved the JSX mounted — it would still pass if a future refactor
   moved `<OverflowProvider>` out of the VP tree again. The mock now
   reads `useOverflowState()` and emits "OVERFLOW_DISCONNECTED" when the
   context is missing. The VP test asserts both presence of "SHOW_MORE"
   and absence of the disconnected marker, so the regression is now
   caught.

Not addressed:
- Audit P0-1 claim that `renderMode` (Alt+M) / model-change updates
  don't reach VP static items: false positive. `renderMode` is a React
  Context (`RenderModeContext`), and Context propagation traverses the
  tree past `memo` boundaries — MarkdownDisplay's `useRenderMode()`
  consumer re-renders on context change regardless of whether
  `StaticRender` bails out. Verified by reading
  `packages/cli/src/ui/contexts/RenderModeContext.tsx` and
  `MarkdownDisplay.tsx:172`. No code change.
- Audit P1-2 pendingStateRef write-during-render race: speculative,
  relies on a multi-pass render path React 18+ does not currently use.
  Documented assumption in the existing inline comment.

* fix(cli): isolate renderItem errors + defensive height coerce + compact-mode mergedHistory stability

Round-3 audit follow-ups. Three real findings; the rest verified clean.

1. **`renderItem` errors no longer crash the CLI.** Previously a throw
   inside a per-item render propagated through `VirtualizedList`'s
   useMemo into React's commit phase, tearing down the whole Ink tree —
   one bad history record could nuke the session. Wrap each call in a
   try/catch and substitute a small red `[render error] …` text box on
   failure. The row stays in the viewport so the user can scroll past
   it.

2. **Defensive height coerce in offset accumulation.** A buggy
   `estimatedItemHeight` returning NaN / negative / Infinity would
   poison every downstream offset and break the `upperBound` /
   `findLastLE` binary search (which assumes monotonic offsets). Clamp
   to `Number.isFinite(raw) && raw > 0 ? raw : 0`. No-op for the
   in-tree estimators that return 3; insurance against future
   consumers.

3. **`mergedHistory` is content-stable when compact mode is on.** The
   Round-2 absorbedCallIds stability fix didn't reach this path:
   `mergeCompactToolGroups` always allocates a fresh array, and
   `mergedHistory`'s useMemo lists `activePtyId` / `embeddedShellFocused`
   as deps, so every streaming tick mid-shell-tool produced a new array
   even when items aligned. Cascade went `mergedHistory` → offsets map
   → `renderVirtualItem` → every static item re-rendered. Pair-wise
   compare new vs previous and return the previous reference when items
   align. Restores StaticRender memo effectiveness for compact-mode
   users.

Not addressed (audit findings deemed not worth fixing in this PR):
- `scrollToItem` silently no-ops when item is not in data — no current
  caller checks the return value, low impact.
- `allVirtualItems` array spread is O(n) per streaming tick — real but
  not a crash; revisit in a perf-focused follow-up.
- `itemRefs.current` is dead surface (never read) — cosmetic.
- StrictMode-only-in-DEBUG double-invoke paths verified safe.

* test+chore(cli): VP review round 4 — VirtualizedList/useBatchedScroll coverage + cleanups

Addresses wenshao's CHANGES_REQUESTED review on PR #3941.

- Add focused unit tests for `VirtualizedList` (9 cases) covering empty
  data, `renderStatic` full-render, `initialScrollIndex` with
  `SCROLL_TO_ITEM_END`, `targetScrollIndex` anchoring, imperative
  `scrollToEnd` / `scrollToIndex`, per-item `renderItem` error isolation,
  NaN/negative estimator coercion, and out-of-range `initialScrollIndex`
  clamping.
- Add `useBatchedScroll` unit tests (4 cases) covering initial reads,
  pending-value reads in the same tick, post-commit pending reset, and
  callback identity stability across rerenders.
- Remove dead `itemRefs` / `onSetRef` plumbing (declared, written, never
  read; `useCallback` with empty deps was also a stale-closure trap).
- Remove unused `isStatic?: boolean` from `VirtualizedListProps`
  (only `isStaticItem` is actually consumed).
- Tighten the render-phase setState block: each setter is now guarded
  by an equality check so React bails out of redundant updates, and a
  comment documents that this is the React-endorsed "adjusting state
  while rendering" pattern (the synchronous update avoids a one-frame
  flash at the previous position when `targetScrollIndex` changes).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* chore(cli): remove dead `dataRef` from VirtualizedList (round-4 followup)

Declared and written in a `useLayoutEffect` on every `data` change but
never read anywhere in the component. Flagged in wenshao's round-4 review
of PR #3941.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): collapse model-change effect back into one batched handler

wenshao's PR #4119 review correctly flagged that splitting the
onModelChange flow into two effects (b25831b0e) reintroduced the
issue #3899 freeze regression on every model switch:

  1. setCurrentModel(model) commits first, with the OLD
     historyRemountKey.
  2. <Static key={`${historyRemountKey}-${currentModel}`}> sees its
     key change (because currentModel did) and remounts immediately.
  3. MainContent's render-phase progressive-replay reset only fires
     when historyRemountKey changes, so replayCount is still the
     full mergedHistory.length from any prior catch-up.
  4. The remounted Static dumps the entire history in one synchronous
     layout pass — exactly the freeze progressive replay was added
     to avoid (#3899). The second effect's refreshStatic() bump
     arrives a render too late.

Fix: do not split. Both side effects (refreshStatic, which writes
clearTerminal + bumps historyRemountKey, and setCurrentModel) live
in the event handler again, with a ref guard for same-model
notifications. The React.StrictMode concern that motivated b25831b0e
is addressed by keeping the side effect OUT of the setState updater
(it now runs once per event-handler invocation, not once per
double-invoked updater call). Both setState calls land in the same
React batch, so historyRemountKey and currentModel update together —
MainContent's render-phase reset sees the new key, replayCount drops
to the first chunk, and Static remounts with chunked replay intact.

Tests:
- AppContainer.test.tsx: 4 new tests covering the synchronous
  refreshStatic side-effect contract, same-model no-op, ref-guarded
  StrictMode double-invoke, and unsubscribe-on-unmount.
- MainContent.test.tsx: new regression guard — when currentModel
  changes but historyRemountKey is held constant, progressive replay
  must NOT reset (pins the MainContent invariant the two-effect
  refactor accidentally relied on).

Verified: vitest packages/cli AppContainer + MainContent green (82/82).
Typecheck clean.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix+docs(cli): VP review round 5 — typecheck, doc drift, scroll keys

PR #4146 review feedback (wenshao + Claude Opus 4.7 audit) addressed:

Code:
- MainContent.test: activePtyId typed as number (was 'pty-xyz' string,
  broke tsc with TS2322 — the test only relies on reference change so
  any number works).
- VirtualizedList: sanitize renderItem error path. Display becomes the
  generic `[render error]` marker; full err goes to debugLogger.debug
  so file paths / partial tool state don't leak to scrollback.
- MainContent: move pendingSourceCopyOffsetsByIndex into a ref so it
  no longer rebuilds renderVirtualItem identity every streaming tick.
  Without this, VirtualizedList.renderedItems useMemo invalidated
  per-tick → JSX rebuilt for every visible item → memo(HistoryItem
  Display) was still bailing but allocations were O(visible) per tick.
- AppContainer: drop the misleading "state-driven scroll reset" claim
  in the VP refreshStatic comment. VP is intentionally near-no-op:
  the React tree owns the visible region, mergedHistory mutation is
  what refreshes the screen, and the remount-key bump is preserved
  only to keep the legacy Static branch in sync if the user toggles
  the flag off mid-session.
- StaticRender: rewrite JSDoc to match reality. The custom React.memo
  is NOT output caching like @jrichman/ink's StaticRender export;
  the comparator rarely matches (parent allocates fresh JSX); the
  real skip happens at memo(HistoryItemDisplay) one level deeper.

Docs:
- docs/design/virtual-viewport: sync file map (drop non-existent
  ScrollProvider.tsx / useAnimatedScrollbar.ts), PR sequence (one PR
  #4146, V.3-V.5 deferred), open-question + checklist resolution for
  #3905 (superseded) and base branch rename.
- docs/users/reference/keyboard-shortcuts: document the 6 VP scroll
  keys (Shift+↑/↓, PgUp/PgDn, Ctrl+Home/End) under a "History
  scrollback (when ui.useTerminalBuffer is on)" section. Previously
  the only discovery path was the Settings dialog description.

Verified: tsc --noEmit -p packages/cli ✓, vitest 160/160 ✓ across
AppContainer / MainContent / VirtualizedList / useBatchedScroll /
keyMatchers / settingsSchema, eslint clean on touched files.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): SGR mouse wheel scroll in VP mode

Recovers the most-felt UX regression vs legacy `<Static>` mode: when
`ui.useTerminalBuffer` is on, legacy users lose mouse wheel as a way
to scroll history (the host terminal stopped seeing the conversation
in its scrollback buffer). This PR enables button-event tracking
(`?1002h`) + SGR coordinates (`?1006h`) while the ScrollableList has
focus, parses wheel events off stdin, and routes them to scrollBy.

Scope kept tight on purpose:
- Wheel only. Hit-testing for scrollbar drag / click-to-position
  needs screen-absolute element coords; stock ink 7's useBoxMetrics
  returns yoga's parent-relative layout. Deferred to V.4 with two
  exit paths (upstream getBoundingBox to ink 7, or local yoga walker).
- Mouse mode is enabled only while ScrollableList is mounted; non-VP
  users never see their terminal flipped into button-event tracking.
- Side effect: native click-and-drag text selection is captured by
  the program. Docs + settings dialog description now spell out the
  Shift / Option (macOS) bypass.

Implementation:
- `ui/utils/mouse.ts` — SGR + X11 parser, ported and trimmed from
  gemini-cli (Google LLC, Apache-2.0). Single-consumer.
- `ui/hooks/useMouseEvents.ts` — enable/parse/disable lifecycle
  hook. Listens on stdin via `useStdin().stdin`, runs handler
  through a ref so callers don't have to memoize.
- `ui/components/shared/ScrollableList.tsx` — subscribe to mouse
  events, route wheel → `scrollBy(±3)`. Also drops a dead outer
  `<Box flexGrow={1}>` wrapper that held an unread containerRef
  and collapsed to zero height in ink-testing-library (the test
  renderer has no flex parent, so flexGrow=1 → 0 height → no items
  ever rendered, which is how this dead code was exposed).

Tests:
- `ui/utils/mouse.test.ts` — 14 cases: SGR parsing (wheel, presses,
  modifiers, move), X11 parsing, fallback chain, incomplete-sequence
  guard (including the >50-byte garbage cap).
- `ui/components/shared/ScrollableList.test.tsx` — 3 cases: wheel
  events shift the rendered window; hasFocus=false makes the mouse
  pipeline inactive (no throw); non-wheel events leave the window
  unchanged. Renders are wrapped in `<KeypressProvider>` (required
  by useKeypress in production but easy to forget in standalone
  tests).

Docs:
- `docs/users/reference/keyboard-shortcuts.md` — adds "Mouse wheel"
  row + the Shift/Option-to-select note.
- `packages/cli/src/config/settingsSchema.ts` — the in-app dialog
  description now mentions mouse wheel and the text-select bypass.
- `docs/design/virtual-viewport/README.md` — §1 status, §5 file map,
  §7 PR sequence all reflect mouse wheel landing in #4146 and the
  V.4–V.7 follow-up split (scrollbar drag / in-app search / alt-
  buffer / host-scrollback dual-write research).

Verified: tsc --noEmit -p packages/cli ✓, vitest 182/182 ✓ across
AppContainer / MainContent / VirtualizedList / ScrollableList /
useBatchedScroll / mouse / keyMatchers / settingsSchema.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): auto-hide animation for VP scrollbar thumb

Pairs with the SGR mouse-wheel work from the previous commit:
when the user actually scrolls, the thumb pops bright; after a
1.5s idle it fades into the dim track so the bar stops competing
with the conversation. The track column itself stays in layout
regardless, so the viewport never reflows mid-flash (which would
trigger per-item re-measure and a visible jitter).

Implementation kept minimal for stock ink 7:
- gemini-cli's `useAnimatedScrollbar` interpolates RGB colors via
  a theme + per-frame setInterval. The terminal can't render
  smooth fades anyway, so this hook collapses the state to a
  binary `isVisible` flag with a single setTimeout. ~75 LoC.
- `VirtualizedList` calls `flashScrollbar()` from a useLayoutEffect
  keyed on `clampedScrollTop`. The very first commit is skipped
  via a ref so initial mount doesn't paint a flash.
- The render switches the thumb glyph (`█` vs `│`) and `dimColor`
  based on `isVisible && inThumb`. Width stays 1 either way.

Tests (6 new):
- initial mount stays hidden (no spurious mount flash)
- flash → visible, hides after idle timeout, successive flashes
  reset the timer (no premature hide), idleHideMs<=0 disables
  auto-hide for tests that want to assert on the visible state,
  unmount cleans up the pending timer.

Doc updates:
- `docs/design/virtual-viewport/README.md` §1 status, §5 file map,
  §7 PR sequence — V.4 row now scopes only the drag/click-jump
  work (still coord-blocked); animated scrollbar moved out of
  deferred and into shipped.
- PR #4146 body — architecture table mentions the auto-hide, new
  files list adds `useAnimatedScrollbar.ts`, test count refreshed
  to 188/188.

Verified: tsc --noEmit -p packages/cli ✓, vitest 188/188 ✓.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): VP review round 6 — ESC bug, CI lint, scope-controlled cleanup

Triage of /review feedback from 2026-05-18 + 2026-05-19. Took the
ones that are real and small; declined the ones that are
false-positive / out-of-scope so this PR stops expanding.

Must-fix:
- CI Lint failure: vscode-ide-companion/schemas/settings.schema.json
  was stale after the keyboard-shortcuts description bump. Regenerated
  via `npm run generate:settings-schema`.
- useMouseEvents.ts had `const ESC = '';` (literal empty string after
  the raw 0x1B byte got stripped somewhere in the source pipeline).
  `buffer.indexOf('', 1) === 1` would have degraded garbage skipping
  to a one-byte scan, and the `else { buffer = ''; break }` branch
  could never run. Fixed by switching to the `'\x1b'` text escape and
  doing the same in `mouse.ts` (which had the raw byte, also fragile).
  Comment explains why.

Small wins (one-liners taken from the review batch):
- ScrollableList: rest-spread separates `hasFocus` from the props
  forwarded to VirtualizedList. Latent collision risk; no behaviour
  change today.
- VirtualizedList: `debugLogger.debug` when isReady=false so blank-
  viewport edge cases (tiny terminal / mid-resize race) become
  diagnosable from the debug log instead of looking like a hang.

Real perf (VP-only):
- MainContent: gated the progressive-Static-replay machinery behind
  `!useVirtualScroll`. The render-phase reset still consumes the
  remount-key bump so flag-off toggles mid-session catch up cleanly,
  but `setReplayCount` and the setImmediate chunking effect are now
  skipped for VP users. Saves ~M/CHUNK_SIZE wasted re-renders per
  Ctrl+O / model change on a 1000-turn session.

Belt-and-braces:
- useMouseEvents: added a `process.on('exit')` handler that writes
  the SGR mouse disable seq again. The React cleanup already covers
  normal unmount, but Ctrl+C / SIGTERM / parent kill bypass it and
  the terminal would otherwise stay in button-event-tracking mode
  after qwen exits.

Explicitly declined / deferred (with reasoning logged on the PR):
- requestAnimationFrame wheel throttle: rAF doesn't exist in Node;
  React 19 already batches state updates within a tick, and the
  renderedItems memo bounds the actual work to visible items. Will
  revisit if profiling shows it.
- Stable pending-item IDs (`p-N` keys shifting on completion): the
  observable jitter is at most one frame of estimated-vs-actual
  height delta. Moderate scope (creation-time ID allocation); fits
  better in a focused follow-up than in this PR.

Verified: tsc --noEmit -p packages/cli ✓, vitest 188/188 ✓ across
the full VP suite.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): scrollBy bottom uses live end anchor in virtualized list

When keyboard scroll reaches the bottom, scrollBy set isStickingToBottom
but anchored via getAnchorForScrollTop(maxScroll), a fixed {index,offset}
pixel anchor. scrollTo/scrollToEnd instead use {index: last, offset:
SCROLL_TO_ITEM_END}, which recomputes the bottom from live item heights
each render. The fixed anchor did not track the last item growing during
streaming, so scroll-to-bottom via keyboard lagged behind new tokens.
Align scrollBy's bottom branch with the sibling methods.

Reported by wenshao in PR review.

* fix(cli): parse mouse events via ink useInput, not a stdin data listener

useMouseEvents attached its own stdin.on('data', ...) listener. Adding a
'data' listener switches stdin into flowing mode, which drains the buffer
before ink's readable + stdin.read() reader (ink App) can consume it, so
all keyboard input routed through useInput was silently starved while
mouse mode was active.

Parse mouse sequences from ink's existing input pipeline via useInput
instead, so there is only one stdin reader. ink captures a full SGR
sequence (ESC [ < .. M/m) as a single CSI event and delivers it with the
leading ESC stripped, so we re-prepend it before parsing. Non-mouse input
does not match and is ignored; ink still routes input to the app's other
useInput handlers, so keyboard navigation keeps working.

Only SGR mode (1006h, which we enable) is parsed via this path; the legacy
X11 encoding is not recoverable through ink's CSI parser, which is the
encoding modern terminals stop emitting once 1006h is set.

Reported by wenshao in PR review.

* fix(cli): parse only SGR in mouse hook to avoid X11 paste misfire

The useInput-based mouse hook called parseMouseEvent, which also tries the
X11 fallback (parseX11MouseEvent). An X11 prefix (ESC [ M + 3 bytes) can
reach the handler via pasted text — ink emits paste content as input when
no paste listener is registered — and would misfire a spurious mouse event.
Call parseSGRMouseEvent directly so only the SGR encoding we enable (1006h)
is parsed, matching the hook's documented contract.

Reported by wenshao in PR review.

* test(cli): assert SGR mouse parser rejects X11 sequences

Locks in the security property behind the parseMouseEvent ->
parseSGRMouseEvent switch in useMouseEvents: an X11 sequence arriving as
pasted text must not misfire a mouse event. Asserts a well-formed X11
sequence is a valid X11 event yet returns null from parseSGRMouseEvent, so
a future revert to parseMouseEvent fails this test.

Reported by wenshao in PR review.

* test(cli): add VP scroll coverage + eslint-disable for useBatchedScroll

Cover keyboard scroll commands (Shift+Up/Down, PageUp/Down, Ctrl+Home/End),
scrollBy/scrollTo imperative API (positive/negative/overflow/clamp), and
auto-scroll-during-streaming state machine (stick-to-bottom, disengage on
user scroll, re-engage on scrollToEnd). Add missing eslint-disable-next-line
for intentionally dep-free useLayoutEffect in useBatchedScroll.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* chore(cli): remove trailing whitespace in useBatchedScroll

The eslint-disable-next-line comment was removed by eslint --fix as an
unused directive (exhaustive-deps does not flag a useLayoutEffect with
no dependency array). Clean up the residual blank line.

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>

* feat(cli): background housekeeping for stale file-history dirs (#4414)

PR #4064 introduced ~/.qwen/file-history/{sessionId}/ for /rewind but had
no cross-session cleanup — directories accumulated indefinitely. This adds
a generic background housekeeping framework with file-history cleanup as
its first user.

- 30-day mtime sweep, configurable via general.cleanupPeriodDays
- 10-min startup delay (1-min catch-up if last run >7d ago)
- 24h recurring cadence, idle-gated (defers if user typed in last 1 min)
- O_EXCL lockfile + marker mtime throttle (multi-process safe)
- Current session whitelisted via lazy config.getSessionId() — defends
  against long-idle active sessions and /clear minting a new session
- Negative cleanupPeriodDays values clamp to 1h minimum (defends against
  schema-bypass: a future cutoff would otherwise sweep everything)
- Zero new prod dependencies; ~70 lines of self-written O_EXCL throttle
  primitive in lieu of proper-lockfile (which pulls graceful-fs and
  monkey-patches every fs method on first require)
- All setTimeout(...).unref() — never blocks process exit

Closes #4173.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* fix(core): loosen auto-mode classifier timeouts, disable stage-2 thinking (#4680)

* fix(core): loosen auto-mode classifier timeouts, disable stage-2 thinking

The AUTO-mode classifier fails closed on timeout — a timed-out judge call
blocks the action as "unavailable". The tight 3s/10s stage budgets turned
transient slowness (slow network, large transcript, model queueing) into
spurious blocks of otherwise-valid actions. Raise them to 10s/30s so a
slow-but-healthy call is not treated as a hard block.

Also disable thinking in stage 2 (previously the only stage with
includeThoughts: true). This is a latency-sensitive permission gate the
user is actively waiting on; allocating a reasoning budget made the review
path slower and more expensive, which directly worsened the fail-closed
timeout. The model still records its reasoning in the structured
`thinking` output field — it just no longer gets an allocated budget.

Closes #4676

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(core): trim verbose comments in auto-mode classifier

Condense the three comments touched by this change (module docstring
stage-2 note, timeout-budget rationale, stage-2 thinkingConfig) while
keeping the essential "why". No logic changes.

Co-authored-by: Qwen-Coder <noreply@qwenlm.ai>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <noreply@qwenlm.ai>

* fix(core): coerce hostile-provider usage token counts (#4350 part 1) (#4439)

* fix(core): coerce hostile-provider usage token counts (#4350 part 1)

Hostile providers (broken upstream, OpenAI-compat proxy returning
null/NaN, misconfigured override) can emit non-finite or negative
values for `usageMetadata.{prompt,candidates,cached,total}TokenCount`.
Captured unguarded in `processStreamResponse`, these poison the
compaction gate arithmetic:

- `lastPromptTokenCount + NaN >= hard` is always false → hard-rescue
  is silently disabled, eventually OOMing the V8 heap.
- `Infinity >= hard` is always true → hard-rescue fires every send.

Route the four API capture sites through a `coerceUsageCount` helper
that maps unknown / non-finite / negative to 0. `Number.isFinite(-1)`
is true, so an explicit `>= 0` is needed in addition to `isFinite`.

Part 1 of the hostile-provider hardening from #4350. The companion
`computeThresholds` guard depends on the un-merged three-tier ladder
in #4345 and is deferred until that lands.

Covered by parametrized tests in `geminiChat.test.ts` over NaN,
±Infinity, negative, null, undefined, and string inputs, plus a
fallback test asserting a…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DDAR DataWorks Data Agent Ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Switching from compact to verbose mode via Ctrl+O freezes the CLI in long conversations

4 participants