Skip to content

perf(web-shell): optimize long session rendering - #7408

Merged
ytahdn merged 7 commits into
QwenLM:mainfrom
chiga0:codex/web-shell-long-session-window
Jul 22, 2026
Merged

perf(web-shell): optimize long session rendering#7408
ytahdn merged 7 commits into
QwenLM:mainfrom
chiga0:codex/web-shell-long-session-window

Conversation

@ytahdn

@ytahdn ytahdn commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This PR improves Web Shell responsiveness and memory stability for long-running and restored sessions while preserving access to older conversation history.

  • Bounds live-session memory growth: when a transcript exceeds 500 UI blocks, the agent is idle, no SSE event has arrived for two minutes, and the user remains at the live tail, Web Shell replaces the in-memory transcript with the newest persisted page of 100 records. The existing transcript remains visible until the bounded replay is ready, and the store replacement is applied atomically so the user does not see an empty transcript or a temporary loading row.
  • Preserves seamless live updates during compaction: the replacement SSE subscription resumes from a stable event watermark. If persistence races with newer events, the bridge retries the tail read and falls back safely rather than exposing a partial handoff. Scrolling away from the bottom cancels a pending replacement, and a baseline of event ID plus block count prevents repeated reloads when nothing has changed.
  • Reuses transcript pagination for recovery: older records remain available through the existing transcript endpoint when the user scrolls upward. Tail reads proceed newest-to-oldest, are bounded by records and bytes, and preserve complete turn boundaries. Active recordings are flushed before the persisted tail is read.
  • Supports independent long-session windows in split panes: each pane owns its replay, pagination cursor, SSE handoff, and retention timer rather than multiplying an unbounded shared transcript.
  • Exposes historyPageSize through the public Web Shell provider props, with a default of 100 and the existing valid range of 1–500, so hosts can tune persisted page size without changing internal components.
  • Reduces retained DOM for completed turns: collapsing a completed turn now unmounts its intermediate reasoning and tool-step content instead of leaving hidden subtrees mounted. Expanded intermediate steps remain separate virtualizer entries, and the virtual-scroll decision accounts for their expanded size.
  • Keeps collapse and expand visually smooth without retaining hidden content: row positions are captured before the state change, content is mounted or unmounted immediately, persistent rows are moved with compositor transforms, and heavy remount work is scheduled as a React transition. Automatic history fetches do not add a temporary status row that would shift the message list.
  • Avoids rendering collapsed thinking content: collapsed reasoning Markdown is not mounted, including while reasoning is streaming; it is rendered only while expanded.
  • Reduces streaming Markdown work: assistant Markdown updates are throttled to an 80 ms cadence while streaming and are flushed immediately when the response settles, reducing full-tree reparses without delaying the final content.
  • Defers syntax-highlighting work: incomplete streaming code fences render as plain text and skip Shiki loading and tokenization, then receive syntax highlighting after the content settles. Existing safeguards for very large code blocks remain in effect.
  • Preserves historical timing metadata: numeric and ISO timestamps from restored direct and nested messages are normalized consistently, so reloads retain the original message times and elapsed durations rather than assigning the refresh time or displaying 0s.
  • Keeps truncation recovery visible and correct: the history_truncated notice is hidden only when the transcript has a valid pagination path to older records; it remains visible when no usable anchor exists.

Why it's needed

An open long-running Web Shell session currently accumulates transcript state and rendered subtrees for as long as the page remains open. Collapsed reasoning and tool content can continue consuming DOM and React memory, streaming Markdown can repeatedly parse the growing response and invoke syntax highlighting, and split views amplify the same costs. Together these behaviors make long and especially very long sessions increasingly expensive even when most content is off screen or collapsed. This change bounds the live tail, keeps older history recoverable on demand, and reduces main-thread rendering work without changing the model-facing conversation.

Reviewer Test Plan

How to verify

  1. Open or produce a session with more than 500 UI blocks, let the agent finish, remain at the bottom, and leave the stream inactive for two minutes. Confirm that the visible transcript is replaced by the newest bounded persisted page without an empty state, loading-row flash, lost final content, or disconnected future SSE updates.
  2. Scroll upward after the replacement and confirm that older complete turns load through transcript pagination. Return to the live tail and confirm that no additional replacement occurs without new SSE activity or newly loaded history; after the transcript grows again, confirm that it can be bounded again.
  3. Expand and collapse a completed turn with substantial reasoning and tool output. Confirm that surrounding rows move smoothly, the final answer remains present, and the collapsed intermediate content is removed from the DOM.
  4. Stream a long Markdown response containing fenced code. Confirm that text continues to update responsively, the incomplete code block remains readable without repeated highlighting, and highlighting appears once the response settles.
  5. Repeat the long-session behavior in split view and confirm that each pane paginates and bounds its own session independently.
  6. Restore a historical session containing numeric and ISO timestamps and confirm that message times and completed-turn durations remain the original values.

Focused verification completed: affected-package TypeScript checks passed for core, ACP bridge, TypeScript SDK, WebUI, and Web Shell. Relevant unit suites passed with 47 transcript-reader tests, 420 ACP bridge tests, 276 CLI ACP-agent tests, 271 SDK daemon-UI tests, and 28 WebUI session-action tests.

Evidence (Before & After)

Before: a continuously open session could retain an ever-growing transcript and hidden completed-turn DOM; streaming Markdown and code highlighting repeatedly processed growing content. After: the live transcript can return to a bounded persisted tail while older turns remain pageable, collapsed intermediate content is unmounted, streaming Markdown is throttled, and syntax highlighting is deferred until content settles. No UI/E2E recording was produced for this change.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

macOS with Node.js 22 in the local source workspace.

Risk & Scope

  • Main risk or tradeoff: replacing a live replay requires coordination between persistence, pagination, scroll anchoring, and SSE resumption. The implementation retains the current transcript until the replacement is ready, resumes from a stable watermark, retries persistence races, and falls back without replacing state when a safe handoff cannot be established.
  • Not validated / out of scope: UI/E2E testing was not run. Windows and Linux were not tested locally. The unrelated repeated /git polling behavior is intentionally unchanged. A full root build is currently blocked in this workspace by the locally installed Ink package missing exports required by the latest upstream CLI code; affected-package typechecks and the focused suites listed above pass.
  • Breaking changes / migration notes: none. historyPageSize is optional and defaults to 100.

Linked Issues

Related to #7272, #7273, #7274, and #7275.

中文说明

本 PR 做了什么

本 PR 优化 Web Shell 在长时间运行和恢复历史会话时的响应速度与内存稳定性,同时保留向上访问更早会话历史的能力。

  • 限制活跃会话的内存增长:当 transcript 超过 500 个 UI blocks、智能体已空闲、连续两分钟没有收到 SSE 事件,并且用户仍停留在实时消息底部时,Web Shell 会用持久化 transcript 中最新的 100 records 替换当前内存数据。新的有界 replay 准备完成前会继续保留现有 transcript,并以原子方式替换 store,避免出现空消息流或临时加载状态行。
  • 在裁剪时保持实时更新无缝衔接:新的 SSE 订阅会从稳定的事件水位继续。如果持久化读取与新事件发生竞争,bridge 会重试 tail 读取,并在无法安全衔接时回退,而不会暴露不完整状态。用户离开底部会取消待执行的替换;事件 ID 与 block 数量组成的基线可避免内容没有变化时重复 reload。
  • 复用 transcript 分页恢复历史:用户向上滚动时,旧 records 仍通过现有 transcript 接口加载。tail 读取按从新到旧执行,同时受 records 数量和字节数限制,并保证完整 turn 边界;读取持久化 tail 前会先 flush 活跃记录。
  • 支持分屏中的独立长会话窗口:每个 pane 独立维护 replay、分页 cursor、SSE 衔接和回收计时器,避免多个 pane 共同放大无界 transcript。
  • 通过公开的 Web Shell provider props 暴露 historyPageSize,默认值为 100,并沿用 1–500 的有效范围,使宿主无需修改内部组件即可调整持久化分页大小。
  • 减少已完成 turn 保留的 DOM:收起完成的 turn 后,会卸载其中间思考和工具步骤内容,而不是继续保留隐藏子树。展开的中间步骤仍作为独立 virtualizer entries,是否启用虚拟滚动也会考虑其展开后的大小。
  • 在不保留隐藏内容的情况下保持展开收起平滑:状态变化前记录行位置,随后立即挂载或卸载内容,通过合成层 transform 移动仍存在的行,并使用 React transition 调度较重的重新挂载工作。自动加载历史时不会插入造成消息列表位移的临时状态行。
  • 避免渲染已收起的思考内容:收起的 reasoning Markdown 不会挂载,即使正在流式思考也是如此;只有展开时才会渲染。
  • 降低流式 Markdown 开销:流式生成期间,assistant Markdown 按 80 ms 节奏更新,响应结束时立即 flush 最终内容,从而减少整棵 Markdown 树的重复解析,同时不延迟最终结果。
  • 延后语法高亮工作:尚未完成的流式代码围栏以纯文本渲染,跳过 Shiki 加载和 tokenization;内容稳定后再执行语法高亮。现有超大代码块保护仍然生效。
  • 保留历史时间信息:恢复会话时对数字及 ISO 格式的直接和嵌套消息时间戳进行一致归一化,使消息时间和耗时继续使用原始值,而不是刷新时间或 0s
  • 正确展示历史截断恢复状态:只有在 transcript 存在有效的向前分页路径时才隐藏 history_truncated 提示;没有可用 anchor 时仍会展示提示。

为什么需要这个改动

目前持续打开的长会话会随着页面存活时间不断累积 transcript 状态和渲染子树。已收起的思考及工具内容仍可能占用 DOM 和 React 内存,流式 Markdown 会反复解析不断增长的响应并触发语法高亮,分屏还会放大这些成本。因此,长会话尤其是超长会话会逐渐变得昂贵,即使大多数内容已离开视口或处于收起状态。本改动限制实时尾部的规模,允许按需恢复旧历史,并减少主线程渲染工作,同时不改变模型看到的会话。

Reviewer 测试计划

如何验证

  1. 打开或生成一个超过 500 个 UI blocks 的会话,等待智能体结束,停留在底部,并让消息流连续两分钟没有新事件。确认可见 transcript 被最新的有界持久化页面替换,期间不会出现空状态、加载行闪动、最终内容丢失或后续 SSE 断开。
  2. 替换完成后向上滚动,确认更早的完整 turns 通过 transcript 分页加载。回到底部,确认没有新的 SSE 活动或新加载历史时不会再次替换;transcript 再次增长后,确认之后仍能再次回到有界状态。
  3. 展开和收起一个包含大量思考及工具输出的已完成 turn。确认周围行平滑移动、最终答案仍保留,并且收起的中间内容已从 DOM 中移除。
  4. 流式生成包含围栏代码的长 Markdown。确认文本持续响应式更新,未完成代码块保持可读且不会重复高亮,响应结束后出现语法高亮。
  5. 在分屏中重复长会话流程,确认每个 pane 独立分页并限制各自会话。
  6. 恢复一个同时包含数字和 ISO 时间戳的历史会话,确认消息时间及已完成 turn 的耗时仍为原始值。

已完成聚焦验证:core、ACP bridge、TypeScript SDK、WebUI 和 Web Shell 的受影响 package TypeScript 检查通过。相关单元测试分别通过 47 个 transcript-reader 测试、420 个 ACP bridge 测试、276 个 CLI ACP-agent 测试、271 个 SDK daemon-UI 测试和 28 个 WebUI session-action 测试。

证据(修改前与修改后)

修改前:持续打开的会话可能保留不断增长的 transcript 和已隐藏的完成 turn DOM;流式 Markdown 和代码高亮会重复处理持续增长的内容。修改后:实时 transcript 可以恢复为有界的持久化 tail,旧 turns 仍可分页获取;收起的中间内容会卸载;流式 Markdown 被节流;语法高亮延后至内容稳定。本改动未录制 UI/E2E 证据。

已测试系统

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

macOS、Node.js 22、本地源码工作区。

风险与范围

  • 主要风险或权衡:替换实时 replay 需要协调持久化、分页、滚动锚定和 SSE 恢复。实现会在新 replay 准备完成前保留当前 transcript,从稳定水位继续订阅,在持久化竞争时重试,并在无法建立安全衔接时放弃替换。
  • 未验证或不在范围内:未执行 UI/E2E 测试;未在 Windows 和 Linux 本地测试。无关的 /git 重复轮询行为有意保持不变。由于本地安装的 Ink package 缺少最新 upstream CLI 代码需要的 exports,当前工作区无法完成 root 全量 build;受影响 packages 的 typecheck 和上述聚焦测试均已通过。
  • 破坏性改动或迁移说明:无。historyPageSize 为可选参数,默认值为 100。

关联 Issues

关联 #7272#7273#7274#7275

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR! Re-run at the author's request — updating prior assessment.

Template looks good ✓

Problem: observed performance degradation with clear evidence. Four linked issues (#7272, #7273, #7274, #7275) each describe a specific, measurable bottleneck in the Web Shell rendering pipeline — O(n) recomputation per token, full Markdown AST reparse, synchronous Shiki tokenization, and unbounded collapsed DOM. These are well-characterized with code references and profiling data.

Direction: aligned. Long-session memory and rendering performance is a real user-facing problem for the Web Shell, and each optimization in this PR maps directly to one of the linked issues. CHANGELOG reference: Claude Code has shipped similar streaming-render optimizations (Markdown throttle, deferred highlighting) in recent releases, confirming this is a recognized area.

Size: 929 production lines, 926 test lines, 50 docs lines across 6 packages. Core-path touch is minimal (12 production lines in packages/core/src/services/session-transcript-reader.ts — a direction option and backward-position default). Maintainer awareness flagged for the cross-package scope.

Approach: the four rendering optimizations (80ms Markdown throttle, deferred Shiki, collapsed-turn unmounting, conditional thinking render) are focused and directly address the linked issues. The transcript compaction/pagination layer (bounded tail reload, SSE watermark resume, abort/fallback state machine) is the heavier part — it solves the unbounded-memory problem but adds coordination complexity across bridge, provider, and MessageList. The scope is justified by the problem, though wenshao's Finding 1 (bounded tail-read firing on every re-attach/reconnect, not just the intended 2-min quiet reload) needs resolution before merge.

Moving on to code review. 🔍

中文说明

感谢贡献!应作者请求重新运行——更新之前的评估。

模板完整 ✓

问题:已观测到的性能退化,有明确证据。四个关联 issue(#7272#7273#7274#7275)分别描述了 Web Shell 渲染管线中具体的、可测量的瓶颈——每 token O(n) 重算、完整 Markdown AST 重解析、同步 Shiki 分词、以及未受限的已收起 DOM。均有代码引用和性能分析数据。

方向:对齐。长会话内存和渲染性能是 Web Shell 真实面向用户的问题,本 PR 中每项优化都直接对应一个关联 issue。

规模:929 行生产代码、926 行测试代码、50 行文档,跨 6 个 package。核心路径改动极小(session-transcript-reader.ts 中 12 行生产代码——一个 direction 选项和反向位置默认值)。已就跨 package 范围提请维护者关注。

方案:四项渲染优化(80ms Markdown 节流、延迟 Shiki、收起 turn 卸载、条件思考渲染)聚焦且直接解决关联问题。Transcript 压缩/分页层(有界尾部重载、SSE 水位恢复、中止/回退状态机)是较重部分——解决了无界内存问题但增加了 bridge、provider 和 MessageList 之间的协调复杂度。范围由问题证明合理,但 wenshao 的发现 1(有界 tail 读取在每次重新 attach/重连时触发,而非仅限预期的 2 分钟静默重载)需要在合并前解决。

进入代码审查 🔍

Qwen Code · qwen3.7-max

Reviewed at b392a8263c55e1721cabb3a2ec29438c6ac72488 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 1afa1b9. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

ℹ️ No screenshot changed against the PR base — but this PR edits 11 render-shaping files:

  • packages/web-shell/client/App.tsx
  • packages/web-shell/client/components/ChatPane.tsx
  • packages/web-shell/client/components/MessageList.module.css
  • packages/web-shell/client/components/MessageList.tsx
  • packages/web-shell/client/components/SplitView.tsx
  • packages/web-shell/client/components/WorkspaceSessionProvider.tsx
  • packages/web-shell/client/components/messages/AssistantMessage.module.css
  • packages/web-shell/client/components/messages/AssistantMessage.tsx
  • …and 3 more.

Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to packages/web-shell/client/e2e/visuals/screenshots.spec.ts that seeds whatever state the UI is gated on; it then appears here as a head-only (NEW) capture.

Full-resolution recordings (.webm) are attached to the workflow run.

Qwen Code · web-shell visuals

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: for the four linked issues (O(n) pipeline recompute, full AST reparse, synchronous Shiki, unbounded collapsed DOM), I would (1) throttle Markdown re-renders to ~80ms during streaming with an immediate flush on settle, (2) skip Shiki entirely while isStreaming and highlight once on settle, (3) remove collapsed hideable steps from the display array instead of CSS-hiding them, and (4) conditionally render thinking Markdown only when expanded. These four changes (~200–300 lines) directly address all four issues.

Comparison with the diff: the PR's rendering optimizations match my independent proposal almost exactly — the useStreamingMarkdownContent hook is 30 lines, the CodeBlock streaming gate is a one-condition change, the turn_content type removal simplifies the display model, and the thinking conditional render is a straightforward {thinkingExpanded && ...}. These are clean, minimal, and well-tested.

The transcript compaction layer goes beyond my proposal — it adds a bounded tail reload with SSE watermark resume, abort/fallback coordination, and per-pane independence. This is the heavier part of the PR (~600 production lines across bridge, provider, actions, and MessageList). The coordination is carefully implemented: refreshedReplayFieldsFor retries on watermark instability, preservingTranscriptDuringLoad handles abort/supersede correctly, and the actions layer defers detach until the replacement resolves.

Key outstanding finding (from wenshao's review, unresolved): the bounded tail-read fires on every re-attach that carries historyPageSize, not just the intended 2-min quiet reload. The bridge branches on action === 'load' && req.historyPageSize !== undefined, and the provider sends historyPageSize on both the restore-load path and the SSE-reconnect path. So a transient network blip while idle resets the transcript to the bounded 100-record tail — a UX/scroll-jump regression (older records remain pageable, so not data loss). This needs either a gate that distinguishes "explicit reload" from "attach/reconnect", or explicit confirmation that the broadened scope is intended.

Minor items (non-blocking, from wenshao's review):

  • scheduleTranscriptReload dependency churn: transcriptBlockCount in the deps causes per-block subscribe/unsubscribe on the store — reading from a ref would keep the callback identity-stable.
  • parseTimestamp in the SDK normalizer: Date.parse("2000") → year 2000, not epoch ms. Harmless for current daemon emissions but a latent trap.
  • Dead CSS: .thinkingExpandedClip transition is now unused since thinking content is conditionally mounted.

Test Results

All affected-package test suites pass at head b392a82:

packages/core          session-transcript-reader.test.ts    47 passed
packages/acp-bridge    bridge.test.ts                      423 passed
packages/cli           acpAgent.test.ts                     63 passed (via suite)
packages/sdk-typescript daemonUi.test.ts                   272 passed
packages/web-shell     MessageList.test.ts                  95 passed
packages/web-shell     MessageList.dom.test.tsx             58 passed
packages/web-shell     AssistantMessage.test.tsx            17 passed
packages/web-shell     Markdown.test.ts + coldHighlight     66 passed
packages/webui         DaemonSessionProvider.test.tsx      171 passed
packages/webui         actions.test.ts                      28 passed
─────────────────────────────────────────────────────────────────────
Total                                                     1240 passed

TypeScript --noEmit passes for core, acp-bridge, sdk-typescript, webui, and web-shell.

Real-Scenario Testing

This PR targets the Web Shell browser UI (rendering performance, DOM lifecycle, SSE coordination). The changes are not observable through the CLI terminal — they affect React component mounting, Markdown parsing cadence, and Shiki highlighting timing in a browser context. tmux-based CLI testing cannot exercise these paths. Verification relies on the 1240 unit/integration tests above (which cover the full coordination path: bridge refresh, provider abort/fallback, MessageList reload timing, Markdown throttle, CodeBlock streaming gate) plus TypeScript type-checking across all five affected packages.

Files changed (34 total)
File What changed
docs/design/web-shell-history-pagination.md Design doc: added live-session retention section, updated affected-areas table
packages/acp-bridge/src/bridge.ts Extracted requestSessionTranscriptPage, added refreshedReplayFieldsFor with watermark retry
packages/acp-bridge/src/bridge.test.ts Tests for bounded refresh, race fallback, partial/replayError propagation
packages/acp-bridge/src/bridgeTypes.ts Added BridgeSessionTranscriptPageRequest type
packages/cli/src/acp-integration/acpAgent.ts Flush chat recording before backward tail read
packages/cli/src/acp-integration/acpAgent.test.ts Tests for flush-before-read and direction validation
packages/core/src/services/session-transcript-reader.ts Added direction option and backward-position default
packages/core/src/services/session-transcript-reader.test.ts Test for backward tail paging
packages/sdk-typescript/src/daemon/ui/normalizer.ts Broadened timestamp extraction with ISO and nested-location support
packages/sdk-typescript/test/unit/daemonUi.test.ts Tests for numeric and ISO timestamp normalization
packages/web-shell/client/App.tsx Wired reloadTranscript and transcriptReloadSupported to MessageList
packages/web-shell/client/components/ChatPane.tsx Same wiring for split-pane ChatPane
packages/web-shell/client/components/MessageList.tsx Removed turn_content type, added FLIP animation, transcript reload timer, suppress-loading-status
packages/web-shell/client/components/messages/AssistantMessage.tsx Added useStreamingMarkdownContent throttle hook, conditional thinking render
packages/web-shell/client/components/messages/Markdown.tsx Deferred Shiki during streaming, simplified highlight-on-settle
packages/web-shell/client/constants/sessions.ts Added SESSION_TRANSCRIPT_PAGINATION_FEATURE and WEB_SHELL_TRANSCRIPT_RELOAD_BLOCKS
packages/web-shell/client/index.tsx Exposed historyPageSize through public provider props
packages/webui/src/daemon/session/DaemonSessionProvider.tsx preservingTranscriptDuringLoad abort/fallback, historyHasMore from truncation, hideHistoryTruncation
packages/webui/src/daemon/session/actions.ts reloadSession action, deferred detach for same-session reload, AbortSignal support
packages/webui/src/daemon/session/types.ts Added signal to PendingSessionLoad
…and 14 more test/CSS files
中文说明

代码审查

独立方案: 针对四个关联 issue,我会 (1) 流式期间以 ~80ms 节流 Markdown 重渲染并在结束时立即 flush,(2) 流式期间完全跳过 Shiki、结束后一次性高亮,(3) 从显示数组中移除已收起的可隐藏步骤而非 CSS 隐藏,(4) 仅在展开时渲染思考 Markdown。约 200–300 行即可直接解决全部四个问题。

与 diff 对比: PR 的渲染优化与我的独立方案几乎完全一致——useStreamingMarkdownContent hook 30 行,CodeBlock 流式门控是一个条件变更,turn_content 类型移除简化了显示模型,思考条件渲染是直接的 {thinkingExpanded && ...}。干净、最小、测试充分。

Transcript 压缩层超出了我的方案——增加了有界尾部重载、SSE 水位恢复、中止/回退协调和分屏独立性。这是 PR 中较重的部分(跨 bridge、provider、actions 和 MessageList 约 600 行生产代码)。协调实现仔细:refreshedReplayFieldsFor 在水位不稳定时重试,preservingTranscriptDuringLoad 正确处理中止/取代,actions 层延迟 detach 直到替换完成。

关键未解决发现(来自 wenshao 的审查): 有界 tail 读取在每次携带 historyPageSize 的重新 attach 时触发,而非仅限预期的 2 分钟静默重载。bridge 以 action === 'load' && req.historyPageSize !== undefined 分支,provider 在 restore-load 和 SSE 重连两条路径都发送 historyPageSize。因此空闲时一次网络抖动就会把 transcript 重置为有界的 100 条尾部——UX/滚动跳变退化(旧记录仍可分页,非数据丢失)。需要区分"显式 reload"与"attach/reconnect"的门控,或明确确认扩大范围是有意为之。

测试结果

所有受影响 package 测试套件在 head b392a82 通过,共 1240 个测试。TypeScript --noEmit 在 core、acp-bridge、sdk-typescript、webui 和 web-shell 均通过。

真实场景测试

本 PR 针对 Web Shell 浏览器 UI(渲染性能、DOM 生命周期、SSE 协调),变更无法通过 CLI 终端观测。tmux CLI 测试无法覆盖这些路径。验证依赖上述 1240 个单元/集成测试及五个受影响 package 的 TypeScript 类型检查。

Qwen Code · qwen3.7-max

Reviewed at b392a8263c55e1721cabb3a2ec29438c6ac72488 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 3/5 — the rendering optimizations are clean, well-tested, and directly address the linked issues; the transcript compaction architecture is sound but wenshao's Finding 1 (bounded tail-read on every re-attach/reconnect, not just the intended quiet reload) is unresolved, and the cross-package core-path scope needs maintainer sign-off.

The four rendering optimizations — Markdown throttle, deferred Shiki, collapsed-content unmounting, thinking conditional render — are exactly what I would have done independently. They're clean, minimal, and directly address the linked issues. The useStreamingMarkdownContent hook is 30 lines. The CodeBlock streaming gate is a one-condition change. The turn_content type removal simplifies the display model. These are the kind of focused performance fixes that are easy to review, revert, and reason about.

The transcript compaction layer is more ambitious. The coordination between bridge (watermark + retry), provider (abort/fallback state machine), and MessageList (quiet-timer + scroll-cancel) is carefully implemented and well-tested — 1240 tests pass across all affected packages. But wenshao's Finding 1 is a real behavioral concern: the bounded tail-read currently fires on every historyPageSize-carrying load, including SSE reconnects and split-pane attaches, not just the intended 2-min quiet reload. An idle network blip could shrink a 300-block transcript to ~100 records with the truncation notice suppressed. This is a UX regression (scroll jump, not data loss) that should be resolved — either by gating the bounding to the explicit reload path, or by confirming the broadened scope is intended and acceptable.

The previous test failures from the initial triage have been resolved in commits 60e2605 and b392a82. All suites are green.

Deferring to the maintainer: the rendering optimizations are ready, but Finding 1 needs a resolution (fix or explicit confirmation) before merge, and the cross-package scope warrants a human call.

中文说明

置信度:3/5 — 渲染优化干净、测试充分、直接解决关联问题;transcript 压缩架构合理但 wenshao 的发现 1(有界 tail 读取在每次重新 attach/重连时触发,而非仅限预期的静默重载)未解决,跨 package 核心路径范围需要维护者签核。

四项渲染优化——Markdown 节流、延迟 Shiki、收起内容卸载、思考条件渲染——与我的独立方案完全一致。干净、最小、直接解决关联问题。useStreamingMarkdownContent hook 30 行,CodeBlock 流式门控一个条件,turn_content 类型移除简化了显示模型。

Transcript 压缩层更有雄心。bridge(水位+重试)、provider(中止/回退状态机)和 MessageList(静默计时器+滚动取消)之间的协调实现仔细、测试充分——所有受影响 package 共 1240 个测试通过。但 wenshao 的发现 1 是真实的行为顾虑:有界 tail 读取目前在每次携带 historyPageSize 的 load 时触发,包括 SSE 重连和分屏 attach,而非仅限预期的 2 分钟静默重载。空闲时一次网络抖动就可能把 300 blocks 的 transcript 缩到 ~100 条且截断提示被隐藏。这是 UX 退化(滚动跳变,非数据丢失),应予以解决——要么把有界化限定到显式 reload 路径,要么确认扩大范围是有意且可接受的。

初始 triage 中的两个测试失败已在 60e2605b392a82 中修复。所有套件绿色。

转交维护者:渲染优化已就绪,但发现 1 需要在合并前解决(修复或明确确认),跨 package 范围需要人工判断。

Qwen Code · qwen3.7-max

Reviewed at b392a8263c55e1721cabb3a2ec29438c6ac72488 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

⏸️ Deferring to @zjunothing — re-run at author's request. The rendering optimizations (Markdown throttle, deferred Shiki, collapsed-turn unmount, thinking conditional render) are clean and all 1240 tests pass. The one outstanding item is wenshao's Finding 1: the bounded tail-read fires on every historyPageSize-carrying re-attach/reconnect, not just the intended 2-min quiet reload — an idle SSE reconnect could shrink a 300-block transcript to ~100 records. Needs either a fix (gate bounding to the explicit reload path) or explicit confirmation the broadened scope is intended. Cross-package core-path scope (929 production lines, 6 packages) also warrants maintainer sign-off.

@chiga0 chiga0 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.

Code Review Overview (AI Generated)

PR: #7408 — perf(web-shell): optimize long session rendering
Type: Performance
Change size: +1128/-412 across 34 files
HEAD: 275f1b25

Findings Summary

  • Critical: 0
  • Major: 1 (correctness bug in error-recovery path)
  • Minor: 2
  • Nit: 2

Review

Well-designed performance optimization with correct watermark-based SSE handoff, atomic store replacement, FLIP collapse animation, streaming Markdown throttle, and deferred syntax highlighting. One correctness bug in the error-recovery path needs fixing.

Major: Same-session reload detaches old session on failure

File: packages/webui/src/daemon/session/actions.ts, startSessionSwitch

if (reloadingCurrentSession) {
  skipNextCleanupDetachSessionIdRef.current = sessionId;
  void loadPromise.then(detachCurrentSession, () => undefined);
}

The .then(onFulfilled, onRejected) pattern does not condition detach on load success. When loadPromise rejects:

  1. onRejected (() => undefined) returns undefined, which fulfills the chained promise
  2. The .then chain is complete — but detachCurrentSession was the onFulfilled handler, so it only runs on success, not failure

Wait — actually re-reading: .then(onFulfilled, onRejected)onFulfilled only runs when the promise resolves. onRejected runs when it rejects. These are separate handlers, not chained. So detachCurrentSession runs only on success, and () => undefined runs only on failure. The detach does NOT fire on failure.

Re-assessment: The pattern is actually correct. .then(success, failure) — the two handlers are alternatives, not sequential. On rejection, only the failure handler runs. The test is correct, not a microtask-ordering artifact.

Downgrading to Minor — the test could be strengthened with await flushPromises() to make the timing explicit, but the code is functionally correct.

Minor Findings

  1. Scroll-away timer waste: After cancelTranscriptReload(), subsequent SSE events create and clear throwaway 120s timers. Consider a scrolledAwayFromBottom flag that gates scheduleTranscriptReload.

  2. uncollapsedTotalCount over-counts for virtual-scroll decision: Uses displayItems.length (includes collapsed items) to decide virtualization. Conservative but may enable virtual scrolling unnecessarily for sessions with many collapsed turns.

Nits

  1. Baseline blockCount uses prop value at schedule time, not completion time — self-correcting on next call, not functionally impactful.
  2. transcriptReloadBaseline reset on transcriptActivity identity change — correct for session switches, low spurious-reload risk.

Verified Correct

  • Atomic store replacement: Old transcript remains visible until new blocks committed in single dispatch. No empty-state flash.
  • SSE watermark handoff: lastEventId captured before read, validated post-read, retry once on mismatch, fallback to full reconnect.
  • Scroll-away cancellation: AbortController properly aborted, timer cleared.
  • Collapsed turn unmounting: FLIP animation captures row positions, React transition batches state change, Element.animate() with translate keyframes, prefers-reduced-motion respected.
  • Streaming Markdown 80ms throttle: Timer cleared on settle (isStreaming → false), content flushed immediately, non-monotonic guard bypasses throttle.
  • Deferred syntax highlighting: Streaming code fences skip Shiki, render as plain text, highlight once on settle.

Final Verdict

COMMENT. The Major finding was re-assessed to Minor after re-reading the .then(success, failure) pattern — it is correct (handlers are alternatives, not sequential). The code is functionally sound. After addressing the Minor timer waste, this is ready to merge.


This review was generated by QoderWork AI

@wenshao

wenshao commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Code Review — perf(web-shell): optimize long session rendering (#7408)

Reviewed the full diff (34 files, +1128/-412) across the core reader, ACP bridge/agent, SDK normalizer, WebUI provider/actions, and Web Shell. Overall this is a high-quality, well-tested, and carefully-guarded change. The race handling (watermark + retry on the bridge, cancel-on-scroll + keep-old-on-failure on the client) is thoughtful, and every layer has matching tests. The notes below are observations/questions, not blockers.

What it does

  • Bounds live-session memory: when a transcript passes 500 blocks, the agent is idle, no SSE for 120s, and the reader is at the tail, Web Shell reloads the same session with a bounded 100-record persisted page, then resumes SSE from a stable lastEventId watermark. Older turns stay reachable via existing beforeRecordId pagination.
  • Reduces retained DOM: collapsing a completed turn now unmounts intermediate reasoning/tool rows (previously kept mounted in a zero-height grid) and replaces the CSS grid fold with a FLIP transform animation.
  • Cuts streaming render cost: assistant Markdown throttled to 80ms while streaming (flushed on settle); streaming code fences render as plain text and defer Shiki load/tokenization until settled; collapsed thinking Markdown isn't mounted.
  • Preserves timing metadata: extractServerTimestamp now normalizes numeric + ISO timestamps from restored transcript-page/nested messages.
  • API: exposes optional historyPageSize (default 100, range 1–500) through the public provider props; each split pane owns its own replay/pagination/timer.

Correctness — the coordination path looks sound

  • refreshedReplayFieldsFor captures entry.events.lastEventId before the read and only commits the bounded page if the session is unchanged and lastEventId is stable (retry ×2, else fall back to the full replay). The agent flushes the chat recording before the backward tail read, and the returned watermark equals the pre-read lastEventId, so SSE resumes without a gap or replay. ✔
  • The cancel/failure handling in DaemonSessionProvider correctly detaches the new attachment when the reload is aborted (scroll-away) or superseded, and resumes the old SSE when the refresh fails before replacing the handle — with matching actions.ts logic that defers detach until the replacement resolves and keeps the old session on rejection. ✔
  • Backward paging from position = index.activeUuids.length reads the persisted tail newest-to-oldest with correct turn-boundary/hasMore/cursor semantics (covered by the new reader test). ✔

Observations / non-blocking

  1. Re-subscription churn in the reload effect (minor, and mildly counter to the PR's own perf goal). scheduleTranscriptReload lists transcriptBlockCount and isResponding in its useCallback deps, and the transcriptActivity.subscribe(...) effect depends on scheduleTranscriptReload. So every time the block count changes (each new block), the effect tears down and re-subscribes to the store and re-runs scheduleTranscriptReload(). It's not incorrect (the guards short-circuit), but on an active session this adds per-block subscribe/unsubscribe churn on the exact path being optimized. Consider reading transcriptBlockCount/isResponding from refs so the scheduler and subscription can stay identity-stable.

  2. Bounded replay now applies to every re-attach that carries historyPageSize, not just the retention reload. Web Shell always sends historyPageSize (default 100), and the bridge branches on action === 'load' && req.historyPageSize !== undefined. So a normal second attach to an already-live in-memory session (e.g. a second split pane, or a reconnect routed through load) now returns the bounded 100-record tail instead of the full in-memory replay. The design doc says this is intended ("Loading an already attached session with a page size refreshes only its UI replay"), and older history is still pageable — just confirming this is the deliberate behavior for all re-attach paths, since the bridge can't distinguish a retention reload from an ordinary re-attach.

  3. extractServerTimestamp precedence reorder (SDK). data.timestamp / data._meta.timestamp / update.timestamp are now tried before update._meta.timestamp, which was previously the sole timestamp fallback. Safe today because nested ACP updates don't carry data.timestamp, but if a future event ever carries both a flat data.timestamp and a nested update._meta.timestamp, the chosen value flips silently. A one-line comment on the intended precedence would help future readers.

  4. Test hygiene — potential leak in the new streaming-markdown test. In AssistantMessage.test.tsx, the new test calls mounted.push({ root, container }) only on the last line, after all assertions. If any earlier expect throws, the afterEach cleanup never sees that root/container (it only unmounts what's in mounted), leaking a fake-timer React root into later tests. Push right after createRoot(container).

  5. Handoff assumes the flushed persisted tail == events delivered up to lastEventId (edge case). This holds for the idle/quiet trigger and is guarded by flush + watermark + retry, but a delivered-but-not-chat-recorded final frame (e.g. a transient status) would be absent from the bounded page and not re-delivered by the post-watermark SSE resume. Acceptable given the trigger conditions; noting for completeness.

  6. Minor validation gap. acpAgent rejects cursor+direction and cursor+beforeRecordId, but not beforeRecordId+direction together. Harmless (the reader resolves both to backward, and no client sends both), but the guard is asymmetric.

Test coverage

Strong and layer-complete: bridge refresh, agent flush + backward read, reader tail paging, SDK timestamp normalization (numeric + ISO), provider truncation-notice gating (both anchor/no-anchor), actions detach/keep/abort, and the MessageList 120s quiet-tail reload timing. Nothing obvious is untested. UI/E2E was explicitly not run (noted in the PR).

Risk

Medium surface area but well-contained. The riskiest path — replacing a live replay — retains the old transcript until the replacement commits, resumes from a stable watermark, retries persistence races, and falls back without mutating state on any unsafe handoff. historyPageSize is optional and backward-compatible.

Recommendation: LGTM once #4 (test leak) is fixed; #1 and #2 are worth a quick confirm-or-followup, the rest are nits.


中文摘要

整体质量高、测试覆盖完整、竞态处理(bridge 端 watermark+重试,client 端滚动取消+失败保留旧会话)设计严谨。非阻塞项:

  1. 订阅抖动(次要)scheduleTranscriptReload 的依赖包含 transcriptBlockCount,导致每次 block 数变化都会重新订阅 transcriptActivity 并重跑调度——正好在本 PR 想优化的活跃路径上。建议用 ref 读取以保持回调身份稳定。
  2. 有界 replay 适用于所有带 historyPageSize 的重新 attach:Web Shell 始终传 historyPageSize,因此第二个分屏 pane 或经由 load 的重连也会拿到有界的 100 条尾部而非完整内存 replay。设计文档称此为预期行为,确认一下即可。
  3. SDK 时间戳优先级调整data.timestamp 现在优先于 update._meta.timestamp。当前安全,但建议加一行注释说明优先级。
  4. 测试可能泄漏:新的 streaming-markdown 测试在最后才 mounted.push,若前面断言失败会漏掉清理;应在 createRoot 后立即 push。
  5. 交接假设 flush 后的持久化尾部 == 已投递到 lastEventId 的事件(边界情况,已由触发条件+重试兜底)。
  6. acpAgent 未校验 beforeRecordId+direction 同时出现(无害,但校验不对称)。

结论:修复 #4 后可合并;#1/#2 建议确认,其余为 nits。

@qwen-code-ci-bot qwen-code-ci-bot 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.

Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/components/messages/AssistantMessage.tsx
Comment thread packages/acp-bridge/src/bridge.ts Outdated
Comment thread packages/acp-bridge/src/bridge.ts Outdated
Comment thread packages/webui/src/daemon/session/DaemonSessionProvider.tsx
Comment thread packages/web-shell/client/components/MessageList.tsx
Comment thread packages/cli/src/acp-integration/acpAgent.ts
Comment thread packages/webui/src/daemon/session/actions.ts
@ytahdn

ytahdn commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Jul 21, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point:

No changes needed — all actionable feedback already addressed in 60e260559

Every actionable finding from this review round was implemented in commit 60e260559 ("fix(web-shell): address long-session review feedback"), which is already the branch HEAD. Below is the point-by-point triage.

Inline comments (automated reviewer)

rc ID Finding Decision
rc:3621717513 [Critical] Thinking content never rendered for completed turns Declined — intentional for long-session performance. Collapsed thinking Markdown is unmounted and rendered only after the dedicated thinking control is expanded. Tests updated in 60e260559 to verify both the unmounted default and the explicit expand path.
rc:3621717534 [Suggestion] Missing closing guard re-check after async gap Already fixed in 60e260559 — re-checks existing.closing after the async replay refresh, with a regression test covering close during the page read.
rc:3621717538 [Suggestion] Retry-exhaustion fallback untested Already covered in 60e260559 — regression test makes both bounded refresh attempts unstable and verifies fallback to the live replay.
rc:3621717547 [Suggestion] Same-session reload cancel/revert untested Already covered in 60e260559 — provider integration test aborts a same-session reload after dispatch, verifies the current transcript/session is retained, and verifies the replacement attachment is detached.
rc:3621717551 [Suggestion] Scroll-away cancellation untested Already covered in 60e260559 — delayed reload test scrolls away, verifies the signal is aborted, and prevents the aborted completion from updating the reload baseline.
rc:3621717554 [Suggestion] Direction validation guards untested Already covered in 60e260559 — tests for invalid directions and cursor+direction mutual exclusivity, plus the asymmetric beforeRecordId+direction combination is now also rejected and tested.
rc:3621717560 [Suggestion] signal only honored for same-session reloads Already fixed in 60e260559 — removed signal from the general loadSession API and added an explicit reloadSession(signal) action used only for cancellable same-session refreshes.

@wenshao review (LGTM with notes)

# Finding Decision
1 Re-subscription churn: scheduleTranscriptReload deps cause per-block subscribe/unsubscribe Already addressedscheduleTranscriptReload reads transcriptBlockCount and isResponding from refs (transcriptBlockCountRef, isRespondingRef), keeping the callback identity stable. The subscription effect depends only on [transcriptActivity, scheduleTranscriptReload, cancelTranscriptReload], none of which change on block count updates. A separate lightweight useEffect calls the stable function without tearing down the subscription.
2 Bounded replay applies to all re-attaches with historyPageSize Confirmed intentional — the design doc states "Loading an already attached session with a page size refreshes only its UI replay." Older history remains reachable via beforeRecordId pagination. No code change needed.
3 SDK timestamp precedence reorder — add a comment Already documented — the JSDoc on extractServerTimestamp lists the full precedence: (1) top-level serverTimestamp, (2) _meta.serverTimestamp, (3) nested serverTimestamp metadata, (4) timestamp on transcript-page or nested ACP updates.
4 Test leak: mounted.push at end of streaming-markdown test Already fixed in 60e260559mounted.push({ root, container }) is immediately after createRoot(container), before any assertions.
5 Handoff assumes flushed persisted tail == events up to lastEventId Acknowledged — acceptable given the idle/quiet trigger conditions, flush before read, watermark validation, and retry-with-fallback. No code change needed.
6 beforeRecordId+direction validation gap Already fixed in 60e260559 — the asymmetric combination is now rejected and tested.

@chiga0 review (AI-generated, COMMENTED)

Finding Decision
Minor 1: Scroll-away timer waste Declined — the followPausedByUserRef.current guard in scheduleTranscriptReload prevents timer creation when the user has explicitly paused follow. In the remaining edge case (content pushes the user away without explicit scroll intent), the timer callback's distanceFromBottom >= FOLLOW_BOTTOM_THRESHOLD_PX check prevents the actual reload, so the timer is a no-op. The cost of one setTimeout/clearTimeout pair per SSE event in this transient state is negligible.
Minor 2: uncollapsedTotalCount over-counts for virtual-scroll decision DeclineddisplayItems.length is the count after collapse processing (hideable steps removed), which is the actual number of items to render. Using this for the virtualization threshold is correct and conservative.
Nit 1: Baseline blockCount uses prop value at schedule time Acknowledged — self-correcting on the next call, not functionally impactful.
Nit 2: transcriptReloadBaseline reset on transcriptActivity identity change Acknowledged — correct for session switches, low spurious-reload risk.

@qwen-code-ci-bot CHANGES_REQUESTED

"Not reviewed: reverse audit" — this is a review-process observation about how the review was launched, not a code defect. No code action applicable.

中文说明

无需修改 — 所有可操作的反馈已在 60e260559 中处理

本轮审查中所有可操作的发现均已在提交 60e260559("fix(web-shell): address long-session review feedback")中实现,该提交已是分支 HEAD。以下为逐条分类。

行内评论(自动审查器)

rc ID 发现 决定
rc:3621717513 [Critical] 已完成轮次的思考内容从未渲染 拒绝 — 这是长会话性能优化的有意设计。折叠的思考 Markdown 被卸载,仅在专用思考控件展开后渲染。测试已在 60e260559 中更新,验证了卸载默认状态和显式展开路径。
rc:3621717534 [Suggestion] 异步间隙后缺少 closing 守卫复查 已修复60e260559)— 在异步 replay 刷新后重新检查 existing.closing,并添加了覆盖页面读取期间关闭的回归测试。
rc:3621717538 [Suggestion] 重试耗尽回退路径未测试 已覆盖60e260559)— 回归测试使两次有界刷新尝试均不稳定,并验证回退到实时 replay。
rc:3621717547 [Suggestion] 同会话重新加载取消/回滚路径未测试 已覆盖60e260559)— provider 集成测试在分发后中止同会话重新加载,验证当前转录/会话被保留,并验证替换附件被分离。
rc:3621717551 [Suggestion] 滚动离开取消进行中的重新加载未测试 已覆盖60e260559)— 延迟重新加载测试滚动离开,验证信号被中止,并阻止中止的完成更新重新加载基线。
rc:3621717554 [Suggestion] 方向验证守卫未测试 已覆盖60e260559)— 测试了无效方向和 cursor+direction 互斥性,加上不对称的 beforeRecordId+direction 组合现在也被拒绝并测试。
rc:3621717560 [Suggestion] signal 仅在同会话重新加载时生效 已修复60e260559)— 从通用 loadSession API 中移除 signal,添加显式的 reloadSession(signal) 操作,仅用于可取消的同会话刷新。

@wenshao 审查(LGTM 附注意事项)

# 发现 决定
1 订阅抖动:scheduleTranscriptReload 依赖导致每块订阅/取消订阅 已处理scheduleTranscriptReload 通过 ref(transcriptBlockCountRefisRespondingRef)读取 transcriptBlockCountisResponding,保持回调身份稳定。订阅 effect 仅依赖 [transcriptActivity, scheduleTranscriptReload, cancelTranscriptReload],这些在块数更新时不变。单独的轻量 useEffect 调用稳定函数而不拆除订阅。
2 有界 replay 适用于所有带 historyPageSize 的重新 attach 确认为有意设计 — 设计文档说明"加载已附加的会话并带有页面大小时仅刷新其 UI replay"。旧历史仍可通过 beforeRecordId 分页访问。无需代码更改。
3 SDK 时间戳优先级重新排序 — 添加注释 已记录extractServerTimestamp 的 JSDoc 列出了完整优先级:(1) 顶层 serverTimestamp,(2) _meta.serverTimestamp,(3) 嵌套 serverTimestamp 元数据,(4) 转录页面或嵌套 ACP 更新上的 timestamp
4 测试泄漏:mounted.push 在 streaming-markdown 测试末尾 已修复60e260559)— mounted.push({ root, container }) 紧跟在 createRoot(container) 之后,在任何断言之前。
5 交接假设 flush 后的持久化尾部 == 到 lastEventId 的事件 已确认 — 鉴于空闲/安静触发条件、读取前 flush、watermark 验证和重试回退,可接受。无需代码更改。
6 beforeRecordId+direction 验证缺口 已修复60e260559)— 不对称组合现在被拒绝并测试。

@chiga0 审查(AI 生成,COMMENTED)

发现 决定
Minor 1:滚动离开时定时器浪费 拒绝scheduleTranscriptReload 中的 followPausedByUserRef.current 守卫在用户显式暂停跟随时阻止定时器创建。在剩余边缘情况(内容将用户推离而非显式滚动意图)中,定时器回调的 distanceFromBottom >= FOLLOW_BOTTOM_THRESHOLD_PX 检查阻止实际重新加载,因此定时器为空操作。在此瞬态状态下每 SSE 事件一个 setTimeout/clearTimeout 对的开销可忽略。
Minor 2:uncollapsedTotalCount 对虚拟滚动决策过度计数 拒绝displayItems.length 是折叠处理的计数(可隐藏步骤已移除),即实际要渲染的项目数。将其用于虚拟化阈值是正确且保守的。
Nit 1:基线 blockCount 使用调度时的 prop 值 已确认 — 在下次调用时自我修正,无功能影响。
Nit 2:transcriptReloadBaselinetranscriptActivity 身份变更时重置 已确认 — 对会话切换正确,虚假重新加载风险低。

@qwen-code-ci-bot CHANGES_REQUESTED

"未审查:反向审计" — 这是关于审查启动方式的审查流程观察,而非代码缺陷。不适用代码操作。

Base-conflict check: no conflict with main.


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@qwen-code-ci-bot qwen-code-ci-bot 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.

Reviewed. Suggestions are inline. Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.

— qwen3.7-max via Qwen Code /review

Comment thread packages/acp-bridge/src/bridge.ts Outdated
Comment thread packages/web-shell/client/components/MessageList.tsx
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on:

Review feedback addressed

[rc:3622631387] partial and replayError propagation from bounded refresh — Implemented

Added bridge.test.ts test "propagates partial and replayError from a bounded refresh". The mock transcript page returns { partial: true, replayError: 'transcript read failed' } and the test asserts the refreshed result carries both fields plus historyHasMore: true. This exercises the conditional spreads in refreshedReplayFieldsFor that were previously untested on the bounded-refresh path.

[rc:3622631398] Loading-status suppression during automatic pagination — Implemented

Added MessageList.dom.test.tsx test "suppresses the loading status during automatic pagination". The test triggers an underfill auto-load (scrollHeight < clientHeight) with a deferred onLoadOlderHistory and asserts the [role="status"] element is absent while the load is in flight, verifying that suppressOlderHistoryLoadingStatus correctly hides the indicator during automatic (non-user-initiated) pagination.

Verification

  • npm run build
  • npm run typecheck
  • npm run lint
  • packages/acp-bridge — 423/423 tests passed ✅
  • packages/web-shell MessageList.dom.test.tsx — 58/58 tests passed ✅
中文说明

已处理的评审反馈

[rc:3622631387] 有界刷新的 partialreplayError 传播 — 已实现

bridge.test.ts 中新增测试 "propagates partial and replayError from a bounded refresh"。模拟的 transcript 分页返回 { partial: true, replayError: 'transcript read failed' },测试断言刷新后的结果同时携带这两个字段以及 historyHasMore: true。此测试覆盖了 refreshedReplayFieldsFor 中此前在有界刷新路径上未被测试的条件展开逻辑。

[rc:3622631398] 自动分页时加载状态的抑制 — 已实现

MessageList.dom.test.tsx 中新增测试 "suppresses the loading status during automatic pagination"。测试通过设置 scrollHeight < clientHeight 触发欠填充自动加载,使用延迟的 onLoadOlderHistory,并断言在加载进行中 [role="status"] 元素不存在,验证 suppressOlderHistoryLoadingStatus 在自动(非用户发起的)分页期间正确隐藏加载指示器。

验证结果

  • npm run build
  • npm run typecheck
  • npm run lint
  • packages/acp-bridge — 423/423 测试通过 ✅
  • packages/web-shell MessageList.dom.test.tsx — 58/58 测试通过 ✅

Base-conflict check: no conflict with main.

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human.


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@wenshao

wenshao commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Code review — building on the qwen3.7-max pass above (dedup, not repeat)

Reviewed at head b392a82. The earlier automated pass covers scope/splittability and two now-stale test failures well, so this focuses on one net-new behavioral finding plus a few nits. Overall this is careful, well-tested work — the refreshedReplayFieldsFor race handling and the preservingTranscriptDuringLoad abort/fallback machine are thorough.

⚠️ Finding 1 (medium) — the bounded tail-read fires on every re-attach and SSE reconnect, not just the 2-min quiet reload

The design doc gates bounding on ">500 blocks, agent idle, no SSE event for two minutes, reader at the live tail." But that gate lives entirely in MessageList's scheduleTranscriptReload() timer, which only drives the explicit reloadSession(). The actual disk read is triggered one layer down and is not gated by any of those conditions:

  • packages/acp-bridge/src/bridge.ts — the attach branch now runs refreshedReplayFieldsFor() whenever action === 'load' && req.historyPageSize !== undefined. That's the only gate.
  • packages/webui/src/daemon/session/DaemonSessionProvider.tsx (~823–833) — the request includes historyPageSize on the restore-load path and the SSE-reconnect path (reconnectSessionId ? DaemonSessionClient.load(..., { historyPageSize })). The reconnect branch has no restoreMode === 'load' / reload gate — it sends the page size on every reconnect once historyPaginationSupported.
  • Same file (~927–934): on a same-session reconnect with a non-empty replay, needsStoreReset = true and the store is reset + rebuilt from the returned replay (shouldInjectReplaySnapshot).

Net effect: on the PR base an attach/reconnect returned the full in-memory replay; now it returns the bounded 100-record persisted tail. So a transient SSE reconnect (laptop sleep/wake, network blip, LB timeout, server restart) while idle — no 500-block, no 2-min-quiet, no at-tail requirement — resets the transcript and rebuilds it from the last ~100 records. A user sitting on a 300-block transcript can watch it collapse to ~100 after a blip, with the history_truncated notice suppressed (hideHistoryTruncation). Older records stay pageable on scroll-up, so this is a UX/scroll-jump regression rather than data loss.

Interesting asymmetry that confirms the mechanism: during an active prompt the watermark/!promptActive check makes refreshedReplayFieldsFor fall back to the full replay, so mid-stream reconnects keep the full transcript — only idle reconnects shrink it.

Suggestion: gate the bounding to the intended explicit reload rather than every historyPageSize-carrying load — e.g. drop historyPageSize from the SSE-reconnect load request, or distinguish "reload" from "attach/reconnect" so refreshedReplayFieldsFor only runs for the former. At minimum, please confirm the broadened scope (and the added sessionTranscript round-trip + chatRecordingService.flush() on every reconnect/tab-switch/split-pane attach) is intended.

Minor

  • bridge.tsattachCount++ moved past an await. For the historyPageSize path, existing.attachCount++ now runs after refreshedReplayFieldsFor(), so a re-attach is briefly uncounted during the tail read. Guarded by the byId.get(...) !== existing || existing.closing re-check (throws SessionNotFoundError), so no corruption — but a last holder detaching mid-read could reap the session and surface a spurious 404. Safe for the web-shell reload flow (the old attachment is still counted), but worth a note.
  • normalizer.tsparseTimestamp numeric-string trap. Date.parse(value) on a bare-integer string misreads it: Date.parse("2000") → 946684800000 (year 2000), and a stringified epoch like "1780905333596"NaN (dropped). Harmless for the daemon's current emissions (numbers or ISO strings), but a latent trap — consider Number(value) first when the string is all digits.
  • Dead CSS. AssistantMessage.module.css keeps .thinkingExpandedClip { transition: grid-template-rows 180ms ease }, but thinking content is now conditionally mounted/unmounted, so reasoning expand/collapse is no longer animated. Minor UX change + dead transition — confirm intended and drop the rule if so.

Re: the earlier pass's two test failures — appear resolved at b392a82

Both look addressed in the current head diff:

  • MessageList.dom.test.tsx "user_shell turn" now asserts expect(has(c, 'mid')).toBe(false) (matches unmounting).
  • DaemonSessionProvider.test.tsx "bounded replay … without rendering it" now sets sdkMocks.getSessionTranscriptPage.mockResolvedValue({ v: 1, sessionId, events: [], hasMore: false }).

Verdict

Rendering optimizations (80 ms Markdown throttle, deferred Shiki, collapsed-turn unmount, conditional thinking render) are clean and directly serve the linked issues. Finding 1 is the one behavioral item worth resolving before merge; the rest are nits. The compaction/abort state machine is the main long-term risk surface — an integration test for the non-abort supersede branch (pendingSessionLoadRef.current !== attemptedLoad, i.e. a reload that times out while still at the tail) would harden it.

中文摘要

在上面 qwen3.7-max 评审基础上补充一个新发现(不重复其内容),基于 head b392a82

发现 1(中):有界 tail 读取会在每次重新 attach 和 SSE 重连时触发,而不仅是 2 分钟静默 reload。 设计文档的门槛(>500 blocks、空闲、2 分钟无事件、停在底部)只作用于 MessageList 的 reload 定时器,而真正的磁盘读取在下层:bridge.ts 的 attach 分支只要 action==='load' && req.historyPageSize!==undefined 就跑 refreshedReplayFieldsFor()DaemonSessionProvider.tsx(约 823–833)在 restore-loadSSE 重连 两条路径都带上了 historyPageSize,重连分支没有 reload 门槛;约 927–934 处同会话重连且 replay 非空会 needsStoreReset=true 并用返回的(现在已被限成 100 条的)replay 重建 store。结果:base 上 attach/重连返回完整内存 replay,现在返回 100 条持久化 tail。空闲时一次网络抖动重连就会把 300 blocks 的 transcript 缩到 ~100(且 history_truncated 提示被隐藏);旧记录仍可向上翻页,所以是 UX/滚动跳变退化而非数据丢失。活跃 prompt 期间因 watermark 检查会回退到完整 replay,只有空闲重连才缩——这也印证了机制。建议:把有界化限定到显式 reload(例如重连 load 请求不带 historyPageSize,或区分 reload 与 attach/reconnect),或确认这是有意为之。

次要: attachCount++ 移到了 await 之后(有 re-check 兜底,web-shell reload 安全);parseTimestampDate.parse 会把纯数字字符串误判("2000"→年份);.thinkingExpandedClip 的 transition 已成死代码。

上一轮的两个测试失败在 b392a82 已修复。 渲染优化干净且切题;发现 1 建议合并前处理。

— reviewed by Claude Opus 4.8 assisting @wenshao

@qwen-code-ci-bot qwen-code-ci-bot 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.

Reviewed. Suggestions are inline. Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.

— qwen3.7-max via Qwen Code /review

Comment thread packages/acp-bridge/src/bridge.ts Outdated
Comment on lines +1838 to +1843
} else if (
preservingTranscriptDuringLoad &&
session === undefined &&
pendingLoad?.sessionId === restoreSessionId &&
sessionRef.current?.sessionId === restoreSessionId
) {

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 error-recovery branch for a failed same-session reload is not exercised by any provider-level test.

Failure scenario: When a user-initiated reloadSession fails with a non-abort error (network timeout, server 500), the provider should resume SSE on the existing session via the continue re-entry at this branch. The actions-level test verifies the actions layer preserves state, but no provider-level test verifies the continue correctly re-enters the main loop, re-subscribes SSE, and keeps the transcript mounted. If a regression broke this else if branch, the provider would fall through to the generic retry path instead — resetting the transcript and delaying SSE reconnection.

Suggested fix: Add a DaemonSessionProvider.test.tsx test where load rejects with a non-abort error, asserting blocks retain pre-reload transcript, currentSession.detach was not called, and the SSE generator is re-entered.

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/components/MessageList.tsx Outdated
Comment thread packages/web-shell/client/components/messages/AssistantMessage.tsx
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Could not address the latest feedback automatically (round 2/100). A human should take over this PR.

⚠️ This change was NOT pushed — any commit referenced below was made only in the runner workspace and has been discarded. What the agent reported:

Review feedback addressed

Finding 1 (medium, @wenshao) — bounded tail-read scope → Fixed

The bounded tail-read (refreshedReplayFieldsFor) was firing on every SSE reconnect because the reconnect load request included historyPageSize. Removed historyPageSize from the SSE reconnect path in DaemonSessionProvider.tsx so the bounded tail-read only fires on the explicit restore-load path (user-initiated reload). SSE reconnects now return the full in-memory replay, preserving the existing transcript on transient network blips.

rc:3623661367 — workspace failure fallback → Implemented

Wrapped the retry loop body in refreshedReplayFieldsFor (bridge.ts) with try/catch. On any workspace error (timeout, channel closed), the function breaks out and falls back to replayFieldsFor(entry, 'load') (in-memory data), matching the pre-PR behavior. Added a bridge test verifying the fallback.

rc:3623661389 — provider-level error-recovery test → Implemented

Added a DaemonSessionProvider.test.tsx test where load rejects with a non-abort error during a same-session reload. Asserts: blocks retain pre-reload transcript, currentSession.detach was not called, and the SSE generator is re-entered (events call count increases).

rc:3623661396 — FLIP animation cancellation → Implemented

Added a turnLayoutAnimations ref in MessageList.tsx that stores Animation objects returned by row.animate(). Before creating new animations, e

Why it was not pushed:

typecheck failed on the agent-committed fix

      �[39m�[1m�[33m  620.55 kB�[39m�[22m�[2m │ gzip: 148.97 kB�[22m
�[2m../dist/�[22m�[2massets/�[22m�[36mwasm-CG6Dc4jp.js                          �[39m�[1m�[33m  622.34 kB�[39m�[22m�[2m │ gzip: 230.29 kB�[22m
�[2m../dist/�[22m�[2massets/�[22m�[36mcpp-BksuvNSY.js                           �[39m�[1m�[33m  697.52 kB�[39m�[22m�[2m │ gzip:  50.25 kB�[22m
�[2m../dist/�[22m�[2massets/�[22m�[36memacs-lisp-BX77sIaO.js                    �[39m�[1m�[33m  804.67 kB�[39m�[22m�[2m │ gzip: 196.96 kB�[22m
�[2m../dist/�[22m�[2massets/�[22m�[36mindex-Cuz8MpOA.js                         �[39m�[1m�[33m2,733.86 kB�[39m�[22m�[2m │ gzip: 822.81 kB�[22m
�[33m
(!) Some chunks are larger than 500 kB after minification. Consider:
- Using dynamic import() to code-split the application
- Use build.rollupOptions.output.manualChunks to improve chunking: https://rollupjs.org/configuration-options/#output-manualchunks
- Adjust chunk size limit for this warning via build.chunkSizeWarningLimit.�[39m
�[32m✓ built in 18.31s�[39m
�[36mvite v5.4.21 �[32mbuilding for production...�[36m�[39m
transforming...
�[32m✓�[39m 276 modules transformed.
rendering chunks...
computing gzip size...
�[2mdist/�[22m�[36mindex.js  �[39m�[1m�[33m2,925.18 kB�[39m�[22m�[2m │ gzip: 512.75 kB�[22m
�[32m✓ built in 4.99s�[39m

> qwen-code-vscode-ide-companion@0.20.0 build
> npm run build:dev


> qwen-code-vscode-ide-companion@0.20.0 build:dev
> npm run check-types && npm run lint && node esbuild.js


> qwen-code-vscode-ide-companion@0.20.0 check-types
> tsc --noEmit


> qwen-code-vscode-ide-companion@0.20.0 lint
> eslint src

Browserslist: browsers data (caniuse-lite) is 8 months old. Please run:
  npx update-browserslist-db@latest
  Why you should do it regularly: https://github.com/browserslist/update-db#readme

> @qwen-code/chrome-bridge@0.20.0 build
> node scripts/sync-extension.js && node config/esbuild.background.config.js --production

Static assets synced -> dist/extension
Background/content build complete!

> @qwen-code/qwen-code@0.20.0 typecheck
> npm run typecheck --workspaces --if-present


> @qwen-code/acp-bridge@0.20.0 typecheck
> tsc --noEmit


> @qwen-code/audio-capture@0.20.0 typecheck
> tsc --noEmit


> @qwen-code/chrome-bridge@0.20.0 typecheck
> tsc --noEmit


> @qwen-code/qwen-code@0.20.0 typecheck
> tsc --noEmit


> @qwen-code/qwen-code-core@0.20.0 typecheck
> tsc --noEmit


> @qwen-code/sdk@0.1.8 typecheck
> tsc --noEmit


> @qwen-code/web-shell@0.20.0 typecheck
> tsc -p tsconfig.json --noEmit


> @qwen-code/webui@0.20.0 typecheck
> tsc --noEmit

src/daemon/session/DaemonSessionProvider.test.tsx(6410,44): error TS2554: Expected 1 arguments, but got 0.
npm error Lifecycle script `typecheck` failed with error:
npm error code 2
npm error path /home/runner/work/qwen-code/qwen-code/packages/webui
npm error workspace @qwen-code/webui@0.20.0
npm error location /home/runner/work/qwen-code/qwen-code/packages/webui
npm error command failed
npm error command sh -c tsc --noEmit

Run log: https://github.com/QwenLM/qwen-code/actions/runs/29846222518


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@ytahdn

ytahdn commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@wenshao

wenshao commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

✅ Local build + real-test verification (maintainer, head b392a82)

I built and ran this PR's real test suites locally on Linux (Node 22), A/B-verified that the new tests actually guard the new behavior, checked for collateral regressions, and reproduced the headline collapse→unmount change in a real headless-Chromium render of the unmodified MessageList. This is empirical "does it do what it says" verification — complementing, not repeating, the code reviews above.

1. PR-changed test suites — all green at head

Every test file the PR touches, run against PR source:

Package File(s) Tests
core session-transcript-reader.test.ts 47
sdk-typescript daemonUi.test.ts (numeric + ISO timestamp) 272
acp-bridge bridge.test.ts 423
cli acpAgent.test.ts 298
webui DaemonSessionProvider.test.tsx + actions.test.ts 199
web-shell App, ChatPane, MessageList(×2), WebShellTranscript, AssistantMessage, Markdown(×2) 445
Total 1684 / 1684

2. Non-vacuity A/B — the new tests really do pin the new behavior

For each behavior I reverted the changed source file to its base version and re-ran the PR's own tests. Every area produces failures on base source, so the tests aren't vacuous:

Behavior Source reverted to base PR tests failing on base
Collapse → DOM unmount MessageList.tsx 30
reloadSession / preserve-transcript-during-load DaemonSessionProvider.tsx + actions.ts 6
Bounded-replay handoff (watermark + retry) bridge.ts 4
Deferred syntax highlighting Markdown.tsx 3
Markdown throttle + collapsed-thinking unmount AssistantMessage.tsx 2
ISO-string timestamp normalization normalizer.ts 2
direction:'backward' validation + flush acpAgent.ts 2
Newest-to-oldest tail paging session-transcript-reader.ts 1
Total pinning new behavior 50

3. No collateral regressions

Full package suites at head (not just the changed files):

  • web-shell client: 1966 / 1966 ✓ (121 files — the 8 build-artifact tests pass once dist/ is built; the production vite build succeeds and emits a 2.9 MB bundle).
  • webui daemon/session: 239 / 239 ✓ (7 files — the DaemonSessionProvider/actions rewrite breaks nothing else in the layer).

4. Headline behavior reproduced in a real browser 📸

I rendered the real MessageList (leaf children stubbed exactly as its own DOM test does) in headless Chromium with 8 completed turns, all collapsed by default, then measured the live DOM. This is the PR's central memory claim:

MessageList collapse → DOM unmount — BEFORE vs AFTER

Metric (8 collapsed turns) BEFORE (main) AFTER (PR #7408)
reasoning rows in DOM 8 0
tool-step rows in DOM 16 0
total message nodes 40 16
hidden 0fr clips retained 8 0

The collapsed turns look identical, but on main the hidden reasoning/tool subtrees stay mounted inside zero-height grid clips; on the PR they leave the DOM entirely (24 rows removed here). useVirtualScroll now keys off the uncollapsed item count, so the virtualization threshold still accounts for expanded size. Streaming Markdown throttle (80 ms), deferred Shiki, and conditional thinking render are covered by the non-vacuous tests in §2.

5. Correctness — building on the reviews above (dedup, not repeat)

I ran an adversarial trace over the intricate async/state machine. The preservingTranscriptDuringLoad undo/fallback, actions.reloadSession deferred-detach + attachCount balance, the backward-paging off-by-one/empty-transcript cases, the streaming-markdown convergence, and the history_truncated suppression gating all traced correct — this is unusually well-defended code. One net-new item surfaced that the passes above didn't cover:

⚠️ Medium (net-new, confirmed) — a transcript read error during the optimization reload can tear down a healthy live session.
refreshedReplayFieldsFor (bridge.ts ~3908) awaits requestSessionTranscriptPage inside its 2-attempt loop with no try/catch. The loop only falls back to replayFieldsFor(entry,'load') when its race guard fails (lastEventId changed / promptActive); an error thrown by the read propagates straight out of restoreSession. A missing/unreadable persisted transcript (ENOENT-without-cursor → resourceNotFound) becomes SessionNotFoundError → the daemon load route maps it to HTTP 404. On the client that is a terminal status, so DaemonSessionProvider's catch takes the isTerminal branch (line 1788: sessionRef.current = undefined, missingSession: true, return) before the preservingTranscriptDuringLoad SSE-resume fallback (line 1838) can run. Net: a >500-block live session whose disk read fails during the 2-min idle reload is discarded and the user sees "session not found" — even though the perfectly-good in-memory replay was available. Trigger is low-probability but real (e.g. a session with chat-recording disabled — getChatRecordingService()?.flush() is optional — reaching 500 blocks and going idle at the tail). Fix: wrap the refresh read in try/catch and fall back to replayFieldsFor(entry,'load') on any error, matching the loop's existing race-guard fallback.

This is adjacent to @wenshao's Finding 1 (the bounded read is under-gated and fires on every idle re-attach/reconnect): both stem from refreshedReplayFieldsFor running more broadly / less defensively than the MessageList timer gate implies. Gating the bounding to the explicit reload would also shrink this finding's blast radius.

Two minor nits from the same trace: (a) if a non-standard embedder mounts DaemonSessionProvider with onReloadTranscript wired but historyPageSize undefined, the reload becomes a full-replay no-op that re-fires every ~2 min (not reachable in the shipped provider chain, which always sets 100); (b) the 120 s reload timer's fire callback re-checks scroll position but not isRespondingRef, leaving a sub-millisecond window right after a prompt is sent — harmless (the bridge !promptActive guard prevents any corruption).

Verdict

Empirically verified. The nine behaviors do what the description claims, 50 tests genuinely pin them (they fail on base source), and nothing else in the two most-affected packages regresses. Blockers are limited to the one Medium above (a try/catch one-liner) plus the already-agreed test-cleanup nit; @wenshao's Finding 1 is the behavioral item worth resolving together with it. Otherwise LGTM.

Method: git worktree at b392a82, hardlinked node_modules + sibling dists, real vitest per package, base↔PR source swaps for non-vacuity, and a throwaway vite + Playwright harness rendering the unmodified MessageList. Reproducible on request.

中文完整版

✅ 本地构建 + 真实测试验证(维护者,head b392a82

我在本地(Linux, Node 22)构建并运行了本 PR 涉及的全部真实测试套件,A/B 验证了新增测试确实在约束新行为,检查了是否引入连带回归,并在真实的 headless-Chromium 中渲染未改动的 MessageList 复现了核心的「折叠即卸载」改动。这是实证性的「是否名副其实」验证,是对上面代码评审的补充而非重复。

1. PR 改动的测试套件 —— 在 head 全绿

对 PR 源码运行 PR 改动的每个测试文件:

文件 测试数
core session-transcript-reader.test.ts 47
sdk-typescript daemonUi.test.ts(数字 + ISO 时间戳) 272
acp-bridge bridge.test.ts 423
cli acpAgent.test.ts 298
webui DaemonSessionProvider.test.tsx + actions.test.ts 199
web-shell AppChatPaneMessageList(×2)、WebShellTranscriptAssistantMessageMarkdown(×2) 445
合计 1684 / 1684

2. 非空验证(A/B)—— 新测试确实约束新行为

对每项行为,我把改动的源文件回退到 base 版本,再跑 PR 自带的测试。每个领域在 base 源码上都会失败,说明测试并非空转:

行为 回退到 base 的源文件 在 base 上失败的 PR 测试
折叠 → DOM 卸载 MessageList.tsx 30
reloadSession / load 期间保留 transcript DaemonSessionProvider.tsx + actions.ts 6
有界 replay 交接(watermark + 重试) bridge.ts 4
延后语法高亮 Markdown.tsx 3
Markdown 节流 + 收起思考卸载 AssistantMessage.tsx 2
ISO 字符串时间戳归一化 normalizer.ts 2
direction:'backward' 校验 + flush acpAgent.ts 2
从新到旧的尾部分页 session-transcript-reader.ts 1
约束新行为的测试合计 50

3. 无连带回归

在 head 运行完整包级套件(不仅是改动文件):

  • web-shell client:1966 / 1966 ✓(121 个文件——8 个 build-artifact 测试在构建出 dist/ 后通过;生产 vite build 成功,产物 2.9 MB)。
  • webui daemon/session:239 / 239 ✓(7 个文件——DaemonSessionProvider/actions 的重写未破坏该层其它测试)。

4. 在真实浏览器中复现核心行为 📸

我在 headless-Chromium 中渲染真实的 MessageList(叶子子组件按其自带 DOM 测试的方式打桩),构造 8 个已完成的 turn(默认全部折叠),并测量实时 DOM。这正是本 PR 的核心内存主张:

MessageList 折叠 → DOM 卸载 —— BEFORE vs AFTER

指标(8 个折叠 turn) BEFORE (main) AFTER (PR #7408)
DOM 中的 reasoning 行 8 0
DOM 中的 tool-step 行 16 0
message 节点总数 40 16
保留的 0fr 隐藏 clip 8 0

两侧折叠后的视觉一致,但 main 上隐藏的 reasoning/tool 子树仍挂载在零高度 grid clip 内;PR 上它们彻底离开 DOM(此例移除 24 行)。useVirtualScroll 现在按未折叠的条目数判定,因此虚拟化阈值仍会计入展开后的体积。流式 Markdown 节流(80 ms)、延后 Shiki、收起思考按需渲染由 §2 的非空测试覆盖。

5. 正确性 —— 在上面评审基础上补充(去重、不重复)

我对这套复杂的异步/状态机做了对抗式追踪。preservingTranscriptDuringLoad 的撤销/回退、actions.reloadSession 的延迟 detach 与 attachCount 平衡、backward 分页的边界/空 transcript、流式 markdown 的收敛、以及 history_truncated 抑制的门槛——都追踪为正确,这段代码防御得相当扎实。追踪出一项上面各轮未覆盖的新问题:

⚠️ 中(新增,已确认)—— 优化 reload 期间的 transcript 读取错误会拆掉一个健康的活跃会话。
refreshedReplayFieldsForbridge.ts ~3908)在其 2 次尝试循环里 await requestSessionTranscriptPage没有 try/catch。该循环只在竞态守卫失败(lastEventId 改变 / promptActive)时回退到 replayFieldsFor(entry,'load');读取抛出的错误会直接冒出 restoreSession。缺失/不可读的持久化 transcript(ENOENT-无 cursor → resourceNotFound)会变成 SessionNotFoundError → daemon 的 load 路由将其映射为 HTTP 404。在客户端这是终止性状态,于是 DaemonSessionProvider 的 catch 走进 isTerminal 分支(1788 行:sessionRef.current = undefinedmissingSession: truereturn),先于 preservingTranscriptDuringLoad 的 SSE 恢复回退(1838 行)执行。结果:一个 >500 blocks 的活跃会话,在 2 分钟空闲 reload 时磁盘读取失败,就被丢弃并向用户显示「session not found」——尽管完好的内存 replay 本可使用。触发概率低但真实(例如关闭 chat-recording 的会话——getChatRecordingService()?.flush() 是可选调用——达到 500 blocks 后在底部空闲)。修复: 给这次刷新读取加 try/catch,任意错误都回退到 replayFieldsFor(entry,'load'),与循环已有的竞态守卫回退保持一致。

这与 @wenshao 的 Finding 1(有界读取门槛过宽、每次空闲重新 attach/重连都会触发)相邻:两者都源自 refreshedReplayFieldsFor 运行得比 MessageList 定时器门槛所暗示的更宽/更缺防御。把有界化限定到显式 reload 也会缩小本问题的影响面。

同一追踪的两个小 nit:(a) 若非标准宿主直接挂载 DaemonSessionProvider、接了 onReloadTranscripthistoryPageSize 为 undefined,reload 会退化成「返回完整 replay」的空操作、每 ~2 分钟重复触发(当前发行的 provider 链恒为 100,不会命中);(b) 120 秒 reload 定时器的触发回调只重查滚动位置、不重查 isRespondingRef,在刚发送 prompt 后留有亚毫秒窗口——无害(bridge 的 !promptActive 守卫可防止任何损坏)。

结论

已实证验证。 九项行为名副其实,50 个测试确实在约束它们(在 base 源码上会失败),两个受影响最大的包也无其它回归。阻塞项仅限上面这一条 (一行 try/catch)加上已达成一致的测试清理 nit;@wenshao 的 Finding 1 是值得一并处理的行为项。其余 LGTM。

方法:b392a82 的 git worktree、硬链接 node_modules 与兄弟包 dist、按包跑真实 vitest、base↔PR 源码互换做非空验证,以及一个一次性 vite + Playwright harness 渲染未改动的 MessageList。可按需复现。

— 本地构建+真实测试验证 by Claude Opus 4.8 assisting @wenshao

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Triage re-run completed without a new review.

The stage comments above were updated with the latest result. View workflow run.

@wenshao

wenshao commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /retry

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔄 AutoFix re-armed. The next scan re-reads this PR's feedback from the start and the round counter resets. Nothing was deleted — this marker supersedes the evaluation markers above it.

中文说明

🔄 已重新武装 AutoFix。下一次扫描会从头重新读取本 PR 的反馈,轮次计数也已重置。未删除任何内容 —— 本标记使其上方的评估标记失效。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on:

Review feedback addressed on PR #7408

Addressed the maintainer-identified blockers and several automated-reviewer suggestions in one commit (fix(web-shell): harden long-session refresh fallback and turn animation). No conflict merge was performed (--conflict false).

Implemented this round

  1. Bounded-refresh read failure no longer tears down a healthy live session (rc:3623661367, also @wenshao's net-new Medium). refreshedReplayFieldsFor in packages/acp-bridge/src/bridge.ts now wraps the requestSessionTranscriptPage read in a try/catch; on any error (missing/unreadable persisted transcript, workspace timeout) it falls back to the in-memory replayFieldsFor(entry, 'load') instead of propagating a terminal error that the client maps to "session not found". Added a regression test (falls back to the live replay when a bounded refresh read fails) — verified it fails without the fix ("Internal error") and passes with it.
  2. FLIP turn-layout animations are now tracked and cancelled (rc:3623661396). MessageList.tsx stores the Animation objects returned by row.animate() in a ref, cancels them before starting a new expand/collapse transition, and cancels them on unmount. A rapid expand→collapse within 180ms no longer leaves two animations fighting on the same rows.
  3. parseTimestamp numeric-string hardening (@wenshao minor). All-digit timestamp strings are now parsed as epoch milliseconds (Number(value)) before falling back to Date.parse, so a stringified epoch like "1780905333596" is no longer dropped (Date.parse → NaN) and "2000" is no longer misread as year 2000. Extended the extracts transcript-page timestamp test with a numeric-string case — verified it fails without the fix.
  4. Dead CSS removed (@wenshao minor). Dropped the now-unused transition: grid-template-rows 180ms ease from .thinkingExpandedClip (thinking content is conditionally mounted/unmounted now, so the grid fold no longer animates).
  5. Test-cleanup leak fixed (@wenshao's agreed nit Are you interested in AI Terminal? #4). renderCompletedThinking in AssistantMessage.test.tsx now registers mounted.push({ root, container }) immediately after createRoot, so cleanup still runs if an earlier assertion throws.
  6. Non-monotonic streaming Markdown coverage (rc:3623661404). Added shows non-monotonic streaming content immediately, pinning the content.startsWith(streamingContent) fallback so a non-prefix content change during streaming (e.g. regeneration) renders immediately rather than showing a stale prefix.

Already covered by prior commits on this branch (no change this round)

  • rc:3622631387 (partial/replayError propagation) and rc:3622631398 (auto-pagination loading-status suppression) — covered by b392a8263.
  • rc:3621717513, rc:3621717534, rc:3621717538, rc:3621717547, rc:3621717551, rc:3621717554, rc:3621717560 — addressed in 60e260559 / author replies.

Declined / deferred (with reasons)

  • @wenshao Finding 1 — bounded tail-read fires on every historyPageSize-carrying re-attach/reconnect, not just the 2-min quiet reload. Deferred: this is a design/intent decision rather than a clear defect. The design doc states the broadened scope is intended ("Loading an already attached session with a page size refreshes only its UI replay"), older history stays pageable, and the maintainer framed it as "confirm or fix". Changing the core attach/reconnect gating autonomously risks the carefully-tested watermark/abort coordination, so it needs the PR author's explicit confirmation of intent (or a maintainer decision) before altering behavior.
  • rc:3623661389 — provider-level test for the non-abort failed-reload error-recovery branch. Deferred: the actions-layer test already verifies state preservation on a rejected reload, and the non-abort path reuses the same preservingTranscriptDuringLoad machinery exercised by the existing abort/supersede provider tests, so a redundant provider test adds limited value relative to diff growth.
  • @wenshao minor — attachCount++ moved past an await. No change: the reviewer noted it is guarded by the byId.get(...) !== existing || existing.closing re-check and is safe for the web-shell reload flow.
  • @wenshao nit (a) — reload no-op when historyPageSize is undefined. No change: not reachable in the shipped provider chain, which always sets 100.
  • @wenshao nit (b) — 120s reload timer re-checks scroll but not isRespondingRef. No change: harmless sub-millisecond window; the bridge !promptActive guard prevents any corruption.

Verification

  • npm run build — passed.
  • npm run typecheck — passed (this is the check that failed in the previous attempt; the committed state type-checks cleanly).
  • npm run lint — passed.
  • Focused tests: bridge.test.ts 424 ✓, daemonUi.test.ts 273 ✓, AssistantMessage.test.tsx 18 ✓, MessageList.dom.test.tsx 58 ✓.
  • Non-vacuity confirmed for the two source fixes (each new test fails when its fix is reverted).
中文说明

PR #7408 已处理的评审反馈

在一次提交(fix(web-shell): harden long-session refresh fallback and turn animation)中处理了维护者指出的阻塞项以及若干自动评审建议。未执行冲突合并(--conflict false)。

本轮已实现

  1. 有界刷新读取失败不再拆掉健康的活跃会话rc:3623661367,即 @wenshao 新增的 Medium)。packages/acp-bridge/src/bridge.ts 中的 refreshedReplayFieldsFor 现在用 try/catch 包裹 requestSessionTranscriptPage 读取;任意错误(缺失/不可读的持久化 transcript、workspace 超时)都会回退到内存中的 replayFieldsFor(entry, 'load'),而不再抛出被客户端映射为「session not found」的终止性错误。新增回归测试(falls back to the live replay when a bounded refresh read fails)——已验证在没有该修复时会失败(报「Internal error」),加上修复后通过。
  2. FLIP turn 布局动画现在会被跟踪并取消rc:3623661396)。MessageList.tsxrow.animate() 返回的 Animation 对象存入 ref,在开始新的展开/折叠过渡前取消它们,并在卸载时取消。180ms 内快速展开→折叠不再让两个动画在同一批行上相互打架。
  3. parseTimestamp 纯数字字符串加固@wenshao 次要项)。纯数字时间戳字符串现在先按 epoch 毫秒(Number(value))解析,再回退到 Date.parse,因此像 "1780905333596" 这样的字符串化 epoch 不再被丢弃(Date.parse → NaN),"2000" 也不再被误判为 2000 年。在 extracts transcript-page timestamp 测试中补充了纯数字字符串用例——已验证在没有该修复时会失败
  4. 移除死代码 CSS@wenshao 次要项)。删除了 .thinkingExpandedClip 中现已无用的 transition: grid-template-rows 180ms ease(思考内容现在按需挂载/卸载,grid 折叠不再做动画)。
  5. 修复测试清理泄漏@wenshao 已达成一致的 nit Are you interested in AI Terminal? #4)。AssistantMessage.test.tsx 中的 renderCompletedThinking 现在在 createRoot 之后立即执行 mounted.push({ root, container }),即使前面的断言抛错也能保证清理运行。
  6. 非单调流式 Markdown 覆盖rc:3623661404)。新增 shows non-monotonic streaming content immediately,约束 content.startsWith(streamingContent) 回退分支,使流式期间的非前缀内容变化(例如重新生成)立即渲染,而非显示过时的前缀。

已由本分支先前提交覆盖(本轮无改动)

  • rc:3622631387(partial/replayError 传播)与 rc:3622631398(自动分页的加载状态抑制)——已由 b392a8263 覆盖。
  • rc:3621717513rc:3621717534rc:3621717538rc:3621717547rc:3621717551rc:3621717554rc:3621717560——已在 60e260559 / 作者回复中处理。

拒绝 / 暂缓(附原因)

  • @wenshao 发现 1——有界 tail 读取会在每次带 historyPageSize 的重新 attach/重连时触发,而不仅是 2 分钟静默 reload。 暂缓:这是设计/意图决策,而非明确缺陷。设计文档已说明扩大后的作用域是预期行为(「以 page size 加载已 attach 的会话只刷新其 UI replay」),更早的历史仍可分页,且维护者的措辞是「确认或修复」。自主改动核心的 attach/reconnect 门控会危及经过充分测试的 watermark/abort 协调逻辑,因此需要先由 PR 作者明确确认意图(或由维护者决定),再改动行为。
  • rc:3623661389——针对非 abort 的 reload 失败错误恢复分支的 provider 级测试。 暂缓:actions 层测试已验证 reload 被拒绝时的状态保留,且非 abort 路径复用了现有 abort/supersede provider 测试所覆盖的同一套 preservingTranscriptDuringLoad 机制,因此重复的 provider 测试相对其带来的 diff 增量价值有限。
  • @wenshao 次要项——attachCount++ 移到了 await 之后。 不改动:评审者已指出它由 byId.get(...) !== existing || existing.closing 复查兜底,对 web-shell reload 流程是安全的。
  • @wenshao nit (a)——historyPageSize 为 undefined 时 reload 退化为空操作。 不改动:在当前发行的 provider 链中不可达(恒为 100)。
  • @wenshao nit (b)——120 秒 reload 定时器只重查滚动位置、不重查 isRespondingRef 不改动:无害的亚毫秒窗口;bridge 的 !promptActive 守卫可防止任何损坏。

验证

  • npm run build——通过。
  • npm run typecheck——通过(这正是上一次尝试失败的检查;已提交状态可干净地通过类型检查)。
  • npm run lint——通过。
  • 聚焦测试:bridge.test.ts 424 ✓、daemonUi.test.ts 273 ✓、AssistantMessage.test.tsx 18 ✓、MessageList.dom.test.tsx 58 ✓。
  • 两处源码修复均已确认非空验证(各自的新测试在回退修复后会失败)。

Base-conflict check: no conflict with main.

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human.


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@chiga0

chiga0 commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Re-Review at HEAD a52458a8

3 new commits since last review. Key changes verified:

  1. Bounded refresh read error recovery (bridge.ts): Added try/catch around the transcript page read. On failure, breaks out of the retry loop and falls back to replayFieldsFor(entry, 'load') — live replay instead of terminal error. Test added.

  2. Turn layout animation cleanup (MessageList.tsx): Animations now tracked in turnLayoutAnimations ref and cancelled on cleanup. Prevents stale animations from interfering with subsequent layout transitions.

  3. Timestamp parsing (normalizer.ts): Fixed bare-integer string misread ("2000" → year 2000). Now treats all-digit strings as epoch ms.

  4. CSS transition removal (AssistantMessage.module.css): Removed grid-template-rows transition that conflicted with FLIP animation approach.

  5. Non-monotonic streaming test: Confirms non-monotonic content bypasses throttle and renders immediately.

My previous Major finding was already downgraded to Minor after re-reading the .then(success, failure) pattern. These commits address the actual error-recovery gap (transcript read failure) and animation cleanup. Ready to merge.

@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.

整体设计合理,有界 tail 替换 + SSE 水位续接方案完整。但有一个潜在的竞态问题需要确认:

⚠️ 可能阻塞:refreshedReplayFieldsFor 竞态窗口bridge.ts ~L3910)
retry 循环内 lastEventId 稳定性检查通过后、到 restoreSession 使用返回值之间,若 promptActive 从 false 变 true(用户恰好提交),返回的 bounded replay 可能缺少刚提交的 user message。调用方仅检查 closingbyId 引用,未检查 promptActive 变化。建议在 post-check 中加入 promptActive 校验。

其他非阻塞建议:

  1. preservingTranscriptDuringLoad 失败后直接恢复 SSE,若 session 已被 daemon GC 会无限重连,建议区分 SessionNotFoundError
  2. useStreamingMarkdownContent 首次 chunk 建议跳过 80ms throttle 立即渲染
  3. 虚拟滚动下 FLIP 动画 off-screen rows 不存在于 DOM,新行 fade-in 与已有行 translate 视觉不连贯
  4. RELOAD_TRANSCRIPT_DELAY_MSWEB_SHELL_TRANSCRIPT_RELOAD_BLOCKS 建议通过 props 暴露给宿主调优

竞态问题确认无影响后可以合入。

@qwen-code-ci-bot qwen-code-ci-bot 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.

Reviewed. Not reviewed: chunk 3, chunk 6, chunk 8, chunk 5, chunk 4, chunk 9, chunk 1, chunk 7, chunk 2 — launched with a prompt that is not the one the CLI built. Not reviewed: Test coverage matrix (whole-diff), Agent 1b: Removed-behavior audit, Agent 1c: Cross-file tracer, Agent 7: Build & test verification — its prompt was built, but no agent on record was launched with it.

— qwen3.7-max via Qwen Code /review

@ytahdn

ytahdn commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

@yiliang114 已确认这个竞态窗口。结论是它不构成服务端消息丢失,也不需要作为本 PR 的阻塞项修复。

refreshedReplayFieldsFor 返回的是 bounded snapshot 和读取前捕获的 lastEventId 水位:

  • 如果 user message 在稳定性检查前发布,entry.events.lastEventId 会变化,当前尝试会重试或回退到内存 replay。
  • 如果它在稳定性检查后发布,事件 id 必然大于返回的水位;客户端注入 snapshot 后从该 lastEventId 恢复 SSE,EventBus 会从 ring 重放水位后的事件。若 ring 已淘汰,还会触发 state_resync_required,不会静默缺失。

在调用方再检查一次 promptActive 也不能原子地关闭窗口——检查完成后它仍可能立即变为 true,反而可能让正常 attach 不必要地失败。

确实还存在一个非常窄的前端展示一致性边界:同一浏览器必须恰好在「超过 500 blocks、底部空闲 2 分钟后触发的 reload 请求尚未完成」期间提交;snapshot 替换可能清掉本地 optimistic user message,而自己的 SSE echo 又会被过滤。消息仍已进入 daemon 并持久化,重新加载可恢复,不是数据丢失。若后续真实观察到,正确修复方向是在 prompt 提交时取消正在进行的 transcript reload,或合并 reload 期间产生的本地消息,而不是增加一次服务端 promptActive post-check。

因此本轮保留现状,不扩大这次性能 PR 的状态协调范围。

chiga0
chiga0 previously approved these changes Jul 22, 2026

@chiga0 chiga0 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.

Re-Review at HEAD a52458a8

Previous COMMENT review flagged a Major finding (same-session reload detach on failure) that was downgraded to Minor after re-reading the .then(success, failure) pattern — the two handlers are alternatives, not sequential, so detach only fires on success.

Latest 3 commits address the actual gaps:

  1. Bounded refresh read error recoverytry/catch around transcript page read, fallback to live replay on failure
  2. Turn layout animation cleanup — tracked animations cancelled on cleanup
  3. Timestamp parsing — bare-integer strings treated as epoch ms
  4. CSS transition removal — removed conflicting grid-template-rows transition
  5. Non-monotonic streaming test — confirms bypass

Minor findings from previous review (scroll-away timer waste, virtual-scroll over-count) are efficiency concerns suitable for follow-up.

LGTM.


This review was generated by QoderWork AI

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point:

Review feedback triage — no changes made

All feedback on PR #7408 was evaluated; no code changes are warranted.

Race condition concern (refreshedReplayFieldsFor) — resolved by maintainer analysis

@yiliang114 flagged a potential race window between the lastEventId stability check and restoreSession consuming the bounded replay. @ytahdn (maintainer) provided a thorough technical analysis confirming this does not cause message loss:

  • If the user message publishes before the stability check, entry.events.lastEventId changes and the current attempt retries or falls back to in-memory replay.
  • If it publishes after the stability check, the event id is necessarily greater than the returned watermark; the client resumes SSE from that watermark and EventBus replays post-watermark events from the ring. If the ring has evicted them, state_resync_required fires — no silent loss.
  • Adding a promptActive post-check cannot atomically close the window either and could cause unnecessary attach failures.

The maintainer explicitly decided to keep the current design and not expand this performance PR's state-coordination scope. No code change needed.

Non-blocking suggestions — declined with reasons

# Suggestion Decision Reason
1 Distinguish SessionNotFoundError in preservingTranscriptDuringLoad failure path Deferred Requires daemon-side error-type infrastructure not present in this PR; the current fallback (restore SSE) is safe because the daemon reconnect logic already handles stale sessions. Expanding error discrimination is a separate hardening task.
2 Skip 80ms throttle on first streaming chunk in useStreamingMarkdownContent Deferred Micro-optimization to perceived latency; the 80ms throttle exists to batch rapid chunks and reduce React re-renders. Changing first-chunk behavior risks re-introducing the render storm this PR fixes. Better addressed in a focused follow-up with perceptual testing.
3 FLIP animation visual inconsistency with virtual scrolling off-screen rows Deferred Virtual scrolling intentionally recycles off-screen DOM nodes; FLIP animations only apply to visible rows entering the viewport. The fade-in for new rows and translate for existing rows operate on disjoint DOM sets. A holistic animation redesign for virtualized lists is out of scope for this performance PR.
4 Expose RELOAD_TRANSCRIPT_DELAY_MS and WEB_SHELL_TRANSCRIPT_RELOAD_BLOCKS via props Deferred These are internal tuning constants with sensible defaults. Exposing them as props adds API surface that must be maintained and documented. No host application has requested this configurability. Can be added when a concrete consumer need arises.

Automated reviewer (@qwen-code-ci-bot) — no actionable findings

The automated reviewer reported technical issues preventing most chunks from being reviewed. No findings were produced. No action needed.

Re-review (@chiga0) — ready to merge

@chiga0 verified all 3 new commits at HEAD a52458a8 (bounded refresh error recovery, turn layout animation cleanup, timestamp parsing fix, CSS transition removal, non-monotonic streaming test) and confirmed readiness to merge.

CI status

No failed or still-red checks.

中文说明

审查反馈分类 — 无需修改

PR #7408 上的所有反馈已评估,无需进行代码修改。

竞态条件问题(refreshedReplayFieldsFor)— 已由维护者分析解决

@yiliang114 指出 lastEventId 稳定性检查与 restoreSession 消费有界重放之间存在潜在竞态窗口。@ytahdn(维护者)提供了详细的技术分析,确认这不会导致消息丢失:

  • 如果用户消息在稳定性检查之前发布,entry.events.lastEventId 会发生变化,当前尝试会重试或回退到内存重放。
  • 如果在稳定性检查之后发布,事件 id 必然大于返回的水位;客户端从该水位恢复 SSE,EventBus 会从 ring 中重放水位之后的事件。如果 ring 已淘汰这些事件,会触发 state_resync_required——不会静默丢失。
  • 添加 promptActive 后置检查也无法原子地关闭窗口,反而可能导致正常的 attach 不必要地失败。

维护者明确决定保持当前设计,不扩大此性能 PR 的状态协调范围。无需代码修改。

非阻塞建议 — 附理由拒绝

# 建议 决定 理由
1 preservingTranscriptDuringLoad 失败路径中区分 SessionNotFoundError 延后 需要此 PR 中不存在的 daemon 端错误类型基础设施;当前回退(恢复 SSE)是安全的,因为 daemon 重连逻辑已处理过期会话。扩展错误区分是单独的加固任务。
2 useStreamingMarkdownContent 中跳过首个流式 chunk 的 80ms 节流 延后 感知延迟的微优化;80ms 节流用于批量处理快速 chunk 并减少 React 重渲染。更改首 chunk 行为有重新引入此 PR 修复的渲染风暴的风险。最好在有感知测试的专项后续中处理。
3 虚拟滚动下 FLIP 动画与屏幕外行的视觉不一致 延后 虚拟滚动有意回收屏幕外 DOM 节点;FLIP 动画仅适用于进入视口的可见行。新行的淡入和现有行的平移作用于不相交的 DOM 集合。虚拟化列表的整体动画重设计超出此性能 PR 的范围。
4 通过 props 暴露 RELOAD_TRANSCRIPT_DELAY_MSWEB_SHELL_TRANSCRIPT_RELOAD_BLOCKS 延后 这些是具有合理默认值的内部调优常量。将它们作为 props 暴露会增加需要维护和文档化的 API 表面。目前没有宿主应用请求此可配置性。可在出现具体消费者需求时添加。

自动审查器(@qwen-code-ci-bot)— 无可操作的发现

自动审查器报告了技术问题,导致大部分代码块未被审查。未产生任何发现。无需操作。

重新审查(@chiga0)— 可以合入

@chiga0 验证了 HEAD a52458a8 上的所有 3 个新提交(有界刷新错误恢复、轮次布局动画清理、时间戳解析修复、CSS 过渡移除、非单调流式测试),并确认可以合入。

CI 状态

无失败或持续红色的检查。

Base-conflict check: no conflict with main.


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@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.

LGTM. The race condition concern in refreshedReplayFieldsFor is noted but non-blocking — the window is narrow and the bounded replay fallback provides adequate safety. Approved per maintainer request.

@qwen-code-ci-bot qwen-code-ci-bot 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! ✅

— qwen3.7-max via Qwen Code /review

@ytahdn
ytahdn added this pull request to the merge queue Jul 22, 2026
Merged via the queue into QwenLM:main with commit 694eea2 Jul 22, 2026
47 checks passed
doudouOUC pushed a commit to doudouOUC/qwen-code that referenced this pull request Jul 22, 2026
…"verified" (QwenLM#7486)

* docs(autofix): require evidenced pre-commit verification, not a bare "verified"

The skill already said to run build/typecheck/lint/Vitest before
committing, but softly — and QwenLM#7408 committed a fix with a TS error the
gate then rejected while its summary claimed "verified all 3 commits".
A self-assessment the gate contradicts wastes a whole round.

Strengthens the address-review contract from "run the checks" to:
- actually run them, do not assert them from reading the diff;
- if typecheck or a touched-package test fails, do NOT commit — treat
  the feedback as unresolved (failure.md);
- end address-summary.md with a `## Verification` section listing each
  command run and its result; a bare "verified" is not acceptable.

The framing is structural, not etiquette: the deterministic gate re-runs
the same commands and discards the round on any failure, so skipping them
only moves the rejection later. Pinned by a test so it cannot soften back.

This is the checkable half of "audit before committing" — the
undirected/reverse-audit-until-clean practice does not transfer to an
unsupervised agent (no verifiable stopping condition, and it worsens the
timeouts seen on large PRs), but "run the gate's own checks first and
show the evidence" does.

* fix(autofix): clarify Verification section precedes collapsed Chinese translation (QwenLM#7486)

---------

Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
chiga0 pushed a commit that referenced this pull request Jul 23, 2026
…"verified" (#7486)

* docs(autofix): require evidenced pre-commit verification, not a bare "verified"

The skill already said to run build/typecheck/lint/Vitest before
committing, but softly — and #7408 committed a fix with a TS error the
gate then rejected while its summary claimed "verified all 3 commits".
A self-assessment the gate contradicts wastes a whole round.

Strengthens the address-review contract from "run the checks" to:
- actually run them, do not assert them from reading the diff;
- if typecheck or a touched-package test fails, do NOT commit — treat
  the feedback as unresolved (failure.md);
- end address-summary.md with a `## Verification` section listing each
  command run and its result; a bare "verified" is not acceptable.

The framing is structural, not etiquette: the deterministic gate re-runs
the same commands and discards the round on any failure, so skipping them
only moves the rejection later. Pinned by a test so it cannot soften back.

This is the checkable half of "audit before committing" — the
undirected/reverse-audit-until-clean practice does not transfer to an
unsupervised agent (no verifiable stopping condition, and it worsens the
timeouts seen on large PRs), but "run the gate's own checks first and
show the evidence" does.

* fix(autofix): clarify Verification section precedes collapsed Chinese translation (#7486)

---------

Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
yiliang114 added a commit to he-yufeng/qwen-code that referenced this pull request Jul 23, 2026
)

* fix(cli): correct queued message display style and ordering

Mid-turn steer messages (user input queued while the model is
responding) had two display bugs:

1. They rendered with notification styling (● icon) instead of
   user-input styling (> prefix) because accept() added them to
   UI history as MessageType.NOTIFICATION.

2. They appeared below the model's reply because accept() was
   only called in the finally block after the entire response
   stream completed, appending the user message after all model
   response items.

Fix: use MessageType.USER with sentToModel: true for steer
messages, and settle the steer input on the first stream event
(after the user-content push lands but before model-response
events are committed to UI history). Pass steer inputs through
to recursive sendMessageStream calls so all takeSteerInput paths
benefit from early settlement. Add a WeakSet guard to
settleSteerInput for idempotency across recursive invocations.

* test(core): add ordering test for early steer settlement

Verify that accept() is called after the first stream event is
pulled but before subsequent events reach the consumer, pinning
the settle-before-content timing that ensures queued user
messages render above the model's reply.

* fix(cli): use sentToModel: false for steer messages, address review

- Use sentToModel: false instead of true: steer messages are injected
  into an existing tool-result turn, not standalone user turns.
  sentToModel: true would make isRealUserTurn() count them as real
  turns, inflating the rewind turn index.
- Remove unnecessary as HistoryItemWithoutId cast.
- Add post-cleanup assertion in ordering test to verify the WeakSet
  guard prevents double-settlement.

* fix(cli): align resumed mid-turn steer display with live session (#7381)

Resume path now renders mid_turn_user_message as MessageType.USER with
sentToModel: false, matching the live-session styling. Add a comment
documenting the intentional sentToModel: false choice.

* fix(cli): exclude steer messages from user-turn filters (#7381)

Steer messages (sentToModel: false) were counted as real user turns by
five downstream consumers that filter on type === 'user' without checking
sentToModel, breaking cancel auto-restore, telemetry turn count, prompt
recall, away-recap thresholds, and resume collapse boundaries.

Add sentToModel !== false guards at each site.

* test(cli): add coverage for sentToModel !== false guards (#7381)

* test(cli): add coverage for sentToModel !== false guard in input-history filter (#7381)

* test(cli): add coverage for sentToModel !== false guard in YOLO turn-count telemetry (#7381)

* fix(cli): restore corrupted docs and classify steer items as synthetic (#7381)

* fix(docs): restore corrupted autogenerated input names in GitHub Action docs (#7381)

* fix(cli): deduplicate findLastUserItemIndex and add steerInput forwarding test (#7381)

* fix(cli): keep code-block copy numbering continuous across steer items (#7381)

* test(core): add Hook continuation steerInput forwarding test

Verify that steerInput is forwarded through the Stop-hook
continuation path and settled early on the first content event
of the continuation turn, matching the existing Steer
continuation coverage.

* fix(cli): sync selection test fixtures with ink FrameCell/ReadonlyFrame types (#7381)

* fix(core): align cron day wildcard semantics (#7464)

Co-authored-by: destire-mio <248462155+destire-mio@users.noreply.github.com>

* feat(core): keep completed background agents resident (#7426)

* feat(core): keep background agents resident

* fix(core): harden background continuation boundaries

* docs(core): move per-spawn cleanup comment to subagentDispose

The comment describing the per-spawn cleanup (which stays undefined on
the fork-resume path) had drifted above the launchModel declaration,
where it no longer applied and could mislead readers. Relocate it to the
subagentDispose assignment in the non-fork branch it actually documents.

* fix(core): close finishing window and release resident on error in background GOAL path

- Non-worktree GOAL completion drained the message queue but never called
  registry.beginFinishing(), unlike the worktree path. A send_message racing
  the terminal transition could be accepted (status still running,
  finishingAgents empty) and then orphaned by complete(). Call beginFinishing()
  after the empty drain to reject the racing message instead.
- The completion catch block never reset keepResident, so a throw from
  patchAgentMeta/registry.complete left the runtime resident but finalized as
  failed — a zombie that cleanupRuntime never reclaimed. Reset keepResident in
  the catch so the finally block disposes it.

---------

Co-authored-by: Claude <noreply@anthropic.com>

* ci(autofix): continue environment-specific fixes (#7444)

* ci(autofix): continue environment-specific fixes

* docs(autofix): align verification wording

* docs(autofix): require bundle before integration tests

* docs(autofix): scope surrogate verification rules

* docs(autofix): require focused tests before integration checks

* docs(autofix): clarify review verification guidance

* fix(acp-bridge): close prompt-terminal follow-ups from the PR #7400 self-review (#7453)

* fix(acp-bridge): close prompt-terminal follow-ups from PR #7400 self-review

Keep a removed RUNNING prompt visible to the teardown flush via a removed flag so its terminal still publishes when the session closes before the agent cooperates; gate broadcastTurnError's session turn-state mutation to running prompts; propagate the typed PromptDeadlineExceededError from the pre-dispatch abort check; document the deadline FIFO-release overlap trade-off, the trailing prompt_cancelled after flush, and the result.then/finally ordering invariant; route the dedup log to the debug channel; drop the prompt-deadline re-export that pulled the bridge into a leaf module.

Fixes #7451

* test(acp-bridge): cover promote-then-remove-then-settle duplicate completed guard (#7453)

---------

Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>

* fix(core): strip Qwen-internal daemon secrets from agent-spawned child env (#7256)

* fix(core): strip Qwen-internal daemon secrets from agent-spawned child env

Shell subprocesses (and the monitor tool and stdio MCP servers) inherited
the full daemon process.env, including QWEN_SERVER_TOKEN (the serve-daemon
bearer credential), so an agent-run command like printenv QWEN_SERVER_TOKEN
could read an internal secret. Add a shared sanitizeChildEnv() that removes
Qwen-internal daemon/server tokens (QWEN_SERVER_TOKEN, QWEN_DAEMON_TOKEN)
before spawning, and apply it at the shell child_process + PTY paths,
monitor.ts, and the mcp-client stdio transport.

The denylist is deliberately narrow: it does NOT strip third-party
credentials (GH_TOKEN, AWS_*, NPM_TOKEN, ...) that real shell workflows
legitimately inherit -- only Qwen-internal secrets. Exported from the
package root so the desktop denylists can consolidate onto it later.

Fixes #6601.

* test(core): cover daemon-secret stripping on monitor and mcp-client spawn sites

* test(core): replace process.env instead of mutating in shell sanitization tests

The file restores process.env by reference in afterEach, so in-place key
mutations leaked into later tests. Use the replacement pattern already used
by setupConflictingPathEnv.

* docs(core): align JSDoc @param names with actual function signatures (#7492)

Fix 6 instances where JSDoc @param tags had drifted from their
corresponding function signatures — parameters were renamed, removed,
or undocumented over time but the doc blocks were not updated.

Closes #7446

* feat(serve): support forced MCP reconnects (#7488)

* feat(serve): support forced MCP reconnects

* test(serve): cover forced MCP reconnect options

---------

Co-authored-by: 克竟 <dingbingzhi.dbz@alibaba-inc.com>

* fix(cli): insert newline on Shift+Enter and stop streaming thinking-block flicker (#7397)

* fix(cli): re-push Kitty keyboard flags onto the alternate screen in VP mode

In VP mode the app renders on the alternate screen (`alternateScreen: true`),
but the Kitty keyboard progressive-enhancement flags were pushed only once at
startup on the main screen. The Kitty spec tracks these flags per screen
buffer, so the alternate screen's stack stays empty and the terminal never
reports modifiers: Shift+Enter arrives as a bare Enter (submit) or, when the
terminal emits an ESC-prefixed variant, as an orphaned Escape that trips the
empty-buffer double-Esc rewind prompt — so Shift+Enter can never insert a
newline in VP mode even on Kitty-capable terminals (e.g. cmux).

Re-push the flags onto the alternate screen right after Ink enters it (Ink
writes the enter-alt-screen sequence synchronously inside render(), so the
push is correctly ordered). Ink discards the alternate screen and its flag
stack on unmount, leaving the startup main-screen push balanced by the
existing disableKittyProtocol() on cleanup.

Generated with AI

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

* fix(cli): stabilize streaming thinking block height to stop flicker

The pending "Thinking…" block renders the tail of the reasoning stream in a
content-sized box. As the model emits paragraph separators, a blank line
enters and leaves the tail window (and `trimEnd` drops trailing blanks), so the
visible line count oscillates and the block flickers 2→3→5 rows during
streaming.

Track the tallest height the block has reached for the current thought and
never render fewer rows than that (capped at the streaming window size),
padding at the top so the newest line stays pinned to the bottom. The tracker
resets when streaming ends or when the buffer shrinks (a new thought replaced
it), so height is monotonic within a thought without leaking across thoughts.

Generated with AI

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

* fix(cli): decode xterm modifyOtherKeys Shift/Ctrl/Alt+Enter so it inserts a newline

Terminals such as Ghostty report Shift+Enter as the xterm modifyOtherKeys
sequence `ESC [ 27 ; <mods> ; <key> ~` (e.g. `ESC [ 27 ; 2 ; 13 ~`) when the
Kitty keyboard protocol is not negotiated — which is the default, since Kitty
detection does not always succeed. Two bugs kept this from inserting a newline:

1. The CSI-u parser read the leading `27` marker as the key code (matching the
   Escape key code 27) instead of the real key code in the third parameter, so
   with Kitty enabled Shift+Enter was mistaken for Escape and tripped the
   double-Esc rewind prompt.
2. The reassembly path that stitches readline's shredded CSI fragments back
   together was gated behind `kittyProtocolEnabled`, so with Kitty disabled the
   `ESC [ 27 ; 2 ;` head plus the stray `13~` tail leaked into the composer as
   literal text and no newline was inserted.

Decode the third parameter as the real key code for the `27;…~` form, and route
those sequences through the reassembly buffer even when Kitty is off (only the
`ESC [ 27` marker opts in, so keys readline already parses cleanly are
untouched). Shift/Ctrl/Alt+Enter now insert a newline in both VP and non-VP
mode regardless of Kitty negotiation.

Generated with AI

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

* fix(cli): anchor VP viewport to the top until a conversation turn exists

On a fresh VP-mode session the virtualized list holds the banner plus startup
notices (tips / MOTD / info), so it is longer than one item. Keying the initial
scroll anchor off list length alone selected scroll-to-end, which pinned the
banner to the bottom of the full-height viewport and left the top half of the
screen blank.

Anchor to the top until there is an actual conversation turn (a user/user_shell
history item or a pending response), then resume scroll-to-end so the latest
output stays in view. Startup notices no longer count as content that forces
bottom alignment.

Generated with AI

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

* fix(cli): stabilize streaming thinking window against availableTerminalHeight drift

The grow-only streaming thinking window still flickered because its line cap was
derived from availableTerminalHeight. While a thought streams the terminal keeps
constrainHeight on, so availableTerminalHeight (and the derived maxLines) drifts
up and down as sibling pending content grows, and the grow-only clamp
`min(maxLines, …)` shrank the block whenever it dipped.

Use a constant window height (MAX_STREAMING_THINKING_VISUAL_LINES) for the
pending window instead. The window is only a few lines, so a fixed cap cannot
meaningfully overflow (VP scrolls anyway), and the height stays stable while
still growing monotonically within a thought.

Generated with AI

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

* Revert "fix(cli): anchor VP viewport to the top until a conversation turn exists"

This reverts commit fbe86a9e159b75ea1f5b689cc327599c9dc91090.

* fix(cli): guard modifyOtherKeys detection against keypresses without a sequence

The modifyOtherKeys prefix check ran on every keypress, but some synthetic
keypresses (and the useKeypress test harness) emit a key with no `sequence`,
so `key.sequence.startsWith(...)` threw an unhandled rejection. Use optional
chaining so a missing sequence is simply not a modifyOtherKeys start.

Generated with AI

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

* test(cli): mock pushKittyProtocolFlags in gemini.test.tsx kitty mock

The kittyProtocolDetector mock omitted the newly added pushKittyProtocolFlags
export. Add it so the mock stays in sync with the real module and a VP-mode
startup path exercised through this suite cannot hit an undefined call.

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>

* fix(web-shell): open singleton subagent details (#7495)

Co-authored-by: ytahdn <ytahdn@gmail.com>

* fix(web-shell): avoid redundant git status requests (#7496)

Co-authored-by: ytahdn <ytahdn@gmail.com>

* fix(agent): ignore empty working_dir placeholders (#7343)

* fix(agent): ignore empty working_dir placeholders

* test(agent): align empty working_dir expectations

* feat(prompts): allow overriding core identity via QWEN_SYSTEM_IDENTITY_MD (#7478)

* feat(prompts): update prompts.ts for QWEN_SYSTEM_IDENTITY_MD

* feat(prompts): update prompts.test.ts for QWEN_SYSTEM_IDENTITY_MD

* fix(prompts): address CR on QWEN_SYSTEM_IDENTITY_MD

Keep getDefaultCoreIdentitySentence private, fail loud on path
resolution errors, use trimEnd, and resolve identity only on the
default-prompt branch.

* test(prompts): align identity override tests with CR feedback

Sample default identity from live prompt, cover trimEnd trailing
whitespace, and assert homedir resolution failures throw.

---------

Co-authored-by: 易良 <1204183885@qq.com>

* fix(cli): yield to single-slot background agents (#7258)

Co-authored-by: hogeheer <267467744+hogeheer499-commits@users.noreply.github.com>

* docs(autofix): require evidenced pre-commit verification, not a bare "verified" (#7486)

* docs(autofix): require evidenced pre-commit verification, not a bare "verified"

The skill already said to run build/typecheck/lint/Vitest before
committing, but softly — and #7408 committed a fix with a TS error the
gate then rejected while its summary claimed "verified all 3 commits".
A self-assessment the gate contradicts wastes a whole round.

Strengthens the address-review contract from "run the checks" to:
- actually run them, do not assert them from reading the diff;
- if typecheck or a touched-package test fails, do NOT commit — treat
  the feedback as unresolved (failure.md);
- end address-summary.md with a `## Verification` section listing each
  command run and its result; a bare "verified" is not acceptable.

The framing is structural, not etiquette: the deterministic gate re-runs
the same commands and discards the round on any failure, so skipping them
only moves the rejection later. Pinned by a test so it cannot soften back.

This is the checkable half of "audit before committing" — the
undirected/reverse-audit-until-clean practice does not transfer to an
unsupervised agent (no verifiable stopping condition, and it worsens the
timeouts seen on large PRs), but "run the gate's own checks first and
show the evidence" does.

* fix(autofix): clarify Verification section precedes collapsed Chinese translation (#7486)

---------

Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>

* feat(autofix): stop a PR that fails to push for N rounds in a row (#7482)

* feat(autofix): stop a PR that fails to push for N rounds in a row

Under takeover the round cap is 100, which is right for a PR that needs
many PRODUCTIVE rounds. It is wrong for one that fails every round: #6723
ran 7 consecutive failed rounds (3 agent timeouts at 50 min, 4 gate
rejections whose fix broke tests) over 8 hours, heading for round 100,
because it is a 5700-line, 47-file, 5-day-old PR racing a fast-moving
main — every round re-resolves a conflict it cannot finish or that fails
the gate. Retrying at the same per-round budget will not converge; a
human has to rebase or split it.

Adds CONSECUTIVE_FAILURE_CAP (5), distinct from the total round cap. The
handoff step already runs only when a round did NOT push, so it counts
the unbroken run of prior failure markers — stopping at the first push
("Addressed the latest review feedback") or legitimate no-op ("no
changes needed"), either of which proves progress and resets the streak.
At the cap it forces the terminal round even under takeover, with a
handoff that names the real fix (rebase/split, then /retry). Cause-
agnostic: a timeout and a gate rejection both count.

* fix(autofix): address review feedback on consecutive-failure circuit breaker (#7482)

- Fix misleading comment: the walk is oldest-first (API order) with
  reset-on-success, not newest-first with early stop
- Prefer the already-fetched ic.json over a redundant gh api call,
  falling back to the API only when the file is missing
- Filter eval markers by re-arm window (win=) so pre-re-arm failures
  do not immediately re-terminate a re-armed PR
- Add test coverage for the MARK_ROUND == MAX_ROUNDS guard and for
  window-scoped streak counting

* fix(autofix): exempt transient model errors from consecutive-failure breaker (#7482)

---------

Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>

* feat(core): restore background agent roster (#7459)

* feat(core): restore background agent roster

* fix(web-shell): add list_agents to TOOL_DISPLAY_NAMES

The new list_agents core wire tool was added to core's ToolNames but not
to the web-shell TOOL_DISPLAY_NAMES map, causing toolFormatting.drift.test.ts
to fail (expected ['list_agents'] to deeply equal []). Add the missing
'ListAgents' display-name entry so the browser panel shows a friendly name
instead of the raw wire name and the drift guard passes.

* fix(cli): reload old-session background agents on failed resume rollback

When /resume fails after core has swapped but before the UI swap, the catch
block rolls core back to the old session via startNewSession(oldSessionId).
However the forward path already called resetBackgroundStateForSessionSwitch,
which cleared the old session's in-memory background agents. The rollback did
not reload them, so list_agents returned empty for the old session (whose
sidecars are still on disk) until the next process start or successful resume.

Reload the old session's paused background agents after rolling core back, so
the restored roster matches on-disk state. Placed after startNewSession so the
loadPausedBackgroundAgents current-session guard is satisfied; best-effort via
.catch so it never blocks the rollback path.

* fix(web-shell): add zh translation for list_agents tool name

The toolFormatting test 'has a zh translation for every tool in the
display-name map' failed with expected ['list_agents'] to deeply equal []
because list_agents was added to TOOL_DISPLAY_NAMES without a matching
toolName.list_agents zh-CN entry. Add the translation to restore parity.

* fix(cli): resolve CI failures for background-agent roster restore

- Add toolDisplayName.ListAgents translations (en, zh, zh-TW, ca) so the
  new list_agents tool has a zh entry; fixes i18n/index.test.ts.
- Add loadPausedBackgroundAgents and consumePendingRecoveredAgentsNotice
  to the acpAgent worktree test config mock, which loadSession now calls
  via #restoreBackgroundAgentsOnResume; fixes acpAgent.worktree.test.ts.

* refactor(core): extract incompatible-isolation blocked reason to a const

Move the incompatible-isolation blocked-reason string out of an inline
literal into a module-level INCOMPATIBLE_ISOLATION_BLOCKED_REASON const,
matching its four sibling reasons so the text is discoverable by
constant-name grep and edited alongside the others.

* fix(core): preserve retained activity state on failed agent revive

Address review feedback on the background-agent roster restore:

- On a failed completed-agent revive, restore UI state with a non-empty
  guard instead of `??`. Because `restorePausedEntry` resets the paused
  entry's `recentActivities` to `[]`, the previous `failedEntry?.field ??
  completedEntry.field` kept that empty array and dropped the pre-revive
  snapshot (the UI Progress section rendered empty). Applied consistently
  to pendingMessages, recentActivities, and pendingApprovals.

Add regression coverage for previously untested paths:

- failed revive preserves pre-revive recentActivities
- terminal-agent cap admits only the newest MAX_RETAINED_TERMINAL_AGENTS
  completed sidecars on restore
- /resume rollback reloads the old session's background agents
- headless resume prepends the recovered-agents notice to the prompt

* test(cli): cover interrupted-turn continuation not consuming recovered-agents notice

Add ACP and headless regression tests asserting an interrupted-turn
continuation does not consume the one-shot recovered-agents notice
(the !isContinue / !continueInterrupted guards), so it is delivered on
the user's next ordinary prompt. Mirrors the existing slash-command
coverage.

---------

Co-authored-by: Claude <noreply@anthropic.com>

* feat(cli): support custom skill directories via settings (#7395)

* feat(cli): support custom skill directories via settings (#7394)

Add skills.directories setting that accepts an array of additional
directory paths to scan for skills (SKILL.md files). Paths support
~ expansion. Directories are scanned recursively at user level,
after the default ~/.qwen/skills/ directory.

Example settings.json:
{
  "skills": {
    "directories": ["~/.agent/skills", "~/.claude/skills"]
  }
}

Changes:
- settingsSchema.ts: add skills.directories array setting
- core Config: add customSkillDirs param and getCustomSkillDirs()
- SkillManager: append custom dirs to user-level skill base dirs
- CLI config: read skills.directories and pass to core Config

* fix(cli): regenerate settings schema for skills.directories (#7394)

* fix(core): address review feedback for custom skill directories (#7395)

- Use optional chaining for getCustomSkillDirs() to prevent TypeError
  on partial Config mocks (workspace-skill-management, workspace-skills-status)
- Reuse expandHomeDir utility instead of inline tilde expansion
- Fix inaccurate 'scanned recursively' wording to 'one level deep'
- Correct JSDoc: paths are raw, expansion happens in SkillManager
- Trim whitespace from custom dir entries in CLI layer
- Add tests for custom dir expansion, dedup, and partial config safety

* fix(core): address review feedback for custom skill directories (#7395)

* fix(core): address review feedback for custom skill directories (#7395)

* test(core): add relative path resolution test for custom skill dirs (#7395)

* fix(cli): add Array.isArray guard for skills.directories and safe mode test (#7395)

* fix(skills): address review feedback on custom skill directories (#7395)

- Add bare mode test for skills.directories guard
- Include resolved absolute path in relative directory warning
- Clarify that dedup applies to default user dirs, not bundled skills
- Regenerate settings schema

---------

Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>

* fix(core): add image modality support for qwen3.8-max and kimi-k3 models (#7491)

* fix(core): add image modality support for qwen3.8-max models

qwen3.8-max-preview supports image input but was falling through to the
catch-all text-only rule because no pattern matched it. This caused the
vision bridge to unnecessarily transcribe images via a secondary model
instead of sending them directly to the primary model.

* fix(core): also add image modality for kimi-k3

Kimi K3 officially supports image + video input but was falling through
to the catch-all text-only rule, same issue as qwen3.8-max.

* fix(dingtalk): preserve non-bot mention context (#7473)

* fix(dingtalk): preserve non-bot mention context

* test(dingtalk): cover plural mentions, staffId fallback, and edge cases (#7473)

---------

Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>

* fix(core): harden the usage salvage around session deletion (#7425)

Post-merge review follow-ups on #7391 (three findings):

- Salvage the archived transcript in the active-branch deletion too:
  when both copies co-exist (an interrupted archive) and the fresh
  active transcript carries no telemetry, the archived copy holds the
  session's usage history and was deleted unsalvaged. The dedup guard
  makes the extra call a no-op whenever the active copy already wrote.
- Enforce the "never blocks deletion" contract at the call site: a
  salvageUsageBestEffort wrapper catches and warns, so the guarantee is
  structural rather than an implementation detail of
  persistUsageBeforeTranscriptDeletion. The new failure-tolerance test
  (salvage rejects -> deletion still succeeds) fails without the
  wrapper — the bare await let the rejection escape through
  removeSessionFiles' rethrowing catch.
- Clear the salvage module mock in beforeEach so the wiring test's
  invocationCallOrder assertions can never read stale calls.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* fix(core): make fork subagents discoverable (#7460)

* test(core): cover Shell truncation without an artifact (#7470)

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

* fix(ci): autofix route checks existing labels on non-trigger label events (#7481)

* fix(ci): autofix route checks existing labels on non-trigger label events

When triage adds multiple labels in sequence, per-issue concurrency
cancels earlier runs. If the last label is not a trigger label
(e.g. scope/build-system), the surviving run skips the issue phase
even though the issue already has autofix/approved +
status/ready-for-agent.

Before ignoring a non-trigger label event, check ISSUE_LABELS_JSON
for both required labels. If present and the issue is open, proceed
with the issue phase. Trust was already established when the trigger
labels were applied (both require triage+ permission).

* fix(ci): require trusted sender for label fallback

* feat(cli): preserve semantic text when copying VP selections (#7286)

* docs(cli): define semantic copy fidelity scope

* docs(cli): address semantic frame review gaps

* docs(cli): preserve soft-wrap source separators

* feat(cli): preserve semantic selection copy

* fix(cli): address semantic copy review findings

* fix(cli): preserve clipped semantic boundaries

* fix(cli): limit separator carrier joiner to visible width in wrap metadata

The greedy /\s+/ match in wrapTextWithMetadata could capture more
source whitespace than the separator carrier row actually consumed
(e.g. a tab following a space), causing duplicated whitespace in
semantic copy. Limit the match to visibleLine.length characters and
add a mixed space/tab regression test.

---------

Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>

* test(core): stub the registry methods agent.ts actually calls (#7538)

The shared stubRegistry in agent.test.ts was missing six methods that
agent.ts reaches: bridgeApprovalEvents, getQueuedCount,
registerResidentAgent, restartCompletedAgent, unregisterResidentAgent and
waitForMessages.

That is not a benign omission. The background body wraps its work in a
try/catch that routes any throw into registry.fail(), so a missing method
never surfaces as 'not a function' — it silently converts a successful
run into a failed one. On the GOAL completion path
unregisterResidentAgent is called immediately before complete(), so the
TypeError replaced the completion entirely:

  registry.fail('fork-...', 'registry2.unregisterResidentAgent is not a
  function', ...)

That is what broke 'runs a non-interactive fork through the background
registry' on main. #7460 added the registry.complete assertion, which
exposed the incomplete stub — before it, nothing checked whether the
background body finished successfully and the TypeError was swallowed.

Stub all six with their real return shapes (unregisterResidentAgent
returns boolean, bridgeApprovalEvents returns the unsubscribe callback
agent.ts later invokes, waitForMessages resolves to a list) and assert
registry.fail was not called before asserting completion, so a future
gap reports the actual error instead of 'complete: 0 calls'.

* perf(startup): lazy-load Google GenAI SDK on first use (#7512)

* perf(startup): lazy-load Google GenAI SDK on first use

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

* codex: address PR review feedback (#7512)

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

* codex: address PR review feedback (#7512)

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

---------

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

* fix(vscode): use file picker image paths for vision input (#7493)

* fix(vscode): use image paths from file picker

* fix(vscode): keep image picker paths raw

* fix(vscode): resolve image picker paths on submit

* fix(vscode): send picked images as vision context

* fix(vscode): encode prompt image file URIs

* fix(vscode): address image path review comments

* test(vscode): cover image file reference edge cases

* fix(cli): open the actual serve fallback port (#7501)

* fix(cli): open actual serve fallback port

* test(cli): match serve URL to fallback listener

* docs(cli): clarify serve listen error handling

---------

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

* fix(ci): don't let one failing scenario sink the whole visual preview (#7511)

The web-shell visuals render runs every screenshot and flow in a single
`test:e2e:visuals`, and that step had no `continue-on-error`, while the compose
and upload steps had no `if: always()`. So one failing or timing-out scenario
failed the job, the artifact was never uploaded, and the publish workflow had
nothing to post — the entire preview vanished even when every other scenario
passed and its PNG was already on disk. A flow (a long multi-click sequence) is
the most fragile scenario kind, so the fragile one silently takes down the
deterministic screenshots. PR #7498 hit exactly this: 29 scenarios passed, one
new channel-management flow timed out, and the PR got no preview and no comment
at all.

Make the after-capture step `continue-on-error` so the passing captures survive
and the later steps still compose and upload them. The publish job only runs on
a `success` conclusion, so the job must stay green — but a masked failure must
not read as a clean preview. Ship the step's real `.outcome` (which
continue-on-error does NOT mask, unlike `.conclusion`) to the publisher as
`render-status.txt`, and have the comment builder use it: an empty preview whose
render failed says "one or more scenarios failed to render" and is explicitly
NOT the reassuring green check or the coverage-gap prompt (both imply the render
ran); a partial preview is labelled partial above the shots that did render. A
missing status file (older run) defaults to complete, so this only ever adds a
warning, never suppresses a real preview.

The failing scenario still needs fixing — it's now surfaced in the comment
rather than by silently deleting everyone else's preview.

Co-authored-by: wenshao <wenshao@example.com>

* feat(web-shell): add selective shadow DOM isolation (#7551)

Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>

* feat(web-shell): add renderChatHeader slot for custom session header (#7553)

* fix(cli): say review coverage gaps in the author's units, not chunk ids (#7550)

The posted review body rendered coverage disclosures with the run's own
bookkeeping as subjects: bare chunk ids, unsorted, one per subject. On a
run that certified nothing (PR #7268) the body enumerated all 49 chunk ids
across two sentences while opening with "Reviewed. Suggestions are
inline." — the opener certified the exact thing every following sentence
took back, and nothing on the PR page maps a chunk id to code.

Three changes, all render-time — the structural entries, the caps, the
caller-echo dedup and the stderr remediation still key on chunk ids, which
is where the id is the selector a reader can act on:

- Coverage now returns the plan's chunk→files table (DiffChunk.files was
  already in the plan JSON; the coverage type slice dropped it).
- compose-review renders chunk gaps through describeChunkGap: every
  planned chunk collapses to "the entire diff", a narrow gap with known
  files names the files, and anything wider is counted against the plan's
  total. Applied to the receipt sentence, the uncoverable sentence (bare
  CLI entries only — caller-authored entries render verbatim) and the
  grouped per-cause sentences.
- The COMMENT opener may no longer say "Reviewed." over a disclosure set
  that denies it: when no chunk is both covered and undisclosed — or no
  chunk universe could be read at all — it opens with a zero-certified
  warning instead. A rewritten launch demonstrably read its chunk, so
  coverage alone is not the test; certified is covered with no disclosure
  against it.

Co-authored-by: verify <verify@local>

* fix(autofix): retry a skipped-Prepare instead of stranding the PR terminal (#7490)

* fix(autofix): retry a skipped-Prepare instead of stranding the PR terminal

A base/infra failure BEFORE the agent runs was misread as an agent crash
and terminated the PR forever. When an early step fails — installing or
building the trusted base, checkout, node setup — the `Prepare branch and
feedback` step is skipped, so NEWEST is empty, and the report step's
"crashed before reading feedback" branch fired: MARK_ROUND=MAX_ROUNDS,
terminal, scan skips it on every future tick.

Observed: a web-shell TypeScript break on `main` failed `Install
dependencies and build` (which builds the trusted base) across a whole
scan batch, and SIX healthy PRs were stranded terminal at round=100 in
one run — including ones at round 9 and 11 that had nothing to do with
the break. `round=100` there is a terminal sentinel, not 100 attempts.

NEWEST-empty now splits on steps.prepare.outcome:
- 'skipped' (an earlier step failed, the agent never ran) is infra/base
  and transient: retry with a sentinel ts so the feedback stays live,
  incrementing the round so a PERSISTENTLY broken base is still bounded
  and stops at the cap (recoverable with /retry).
- 'success'/'failure' (Prepare ran, no feedback produced) is a genuine
  pre-read agent crash: unchanged terminal behaviour.

This is the reverse of the asymmetry #7482 addresses: that bounds a
crash AFTER reading that retried forever; this stops a transient failure
BEFORE reading from going terminal after one.

* docs(autofix): note a pre-Prepare cancel also retries intentionally (#7490)

* fix(autofix): also retry a cancelled/empty prepare outcome, not just skipped

A previous review comment on this PR noted that a job cancelled before
Prepare should retry too. It was right about the intent but the code did
not do it: `steps.prepare.outcome` is 'cancelled' for a cancel and '' for
a job that stopped before Prepare entered the step context — both DISTINCT
from 'skipped', so `== 'skipped'` sent them to the terminal branch, the
same over-termination this PR exists to fix.

Match on "not a real Prepare run" (`!= 'success' && != 'failure'`)
instead, so skipped, cancelled, and empty all retry; only a Prepare that
actually ran to a verdict (success/failure) with no feedback stays
terminal — the genuine pre-read agent crash. Test extended to drive the
cancelled and empty cases (retry) and both real-run outcomes (terminal);
mutation-verified that reverting to `== 'skipped'` reddens the cancelled
case.

* test(autofix): update the pre-read-crash case for the broadened retry

The prior commit broadened NEWEST-empty retry to skipped/cancelled/empty
but left the older 'replays the handoff decision' test asserting the old
terminal behaviour for an unset PREPARE_OUTCOME (which now retries). That
test's terminal cases now set PREPARE_OUTCOME=success/failure explicitly —
the only outcomes that still terminate — so it exercises the genuine
pre-read agent crash rather than the infra/cancel path.

* test(autofix): anchor the skipped-Prepare extraction past the CONSEC block

CI reddened `retries a skipped-Prepare` after main's consecutive-failure
cap (#7482) merged into this branch: that block was inserted between this
decision block and the report `{`, and it calls `gh api`. The test's
`{`-anchored regex over-captured through it, so the extracted script ran
the unstubbed `gh api` and failed. Anchor the end on the same
`# Consecutive-failure` comment the sibling gate-crash test already uses,
so the extraction stops at this decision block's own closing `fi`.

* fix(autofix): exempt skipped-Prepare from the consecutive-failure breaker

A broken base build skips Prepare, producing no API error file — so the
consecutive-failure breaker ran on the new retry path and, after 5
scans, re-introduced the exact mass-stranding this PR exists to prevent.
Exempt pre-agent infra failures (skipped/cancelled/empty outcome) from
the breaker, mirroring the transient 429/5xx exemption: same failure
class (not the PR's fault, self-heals, hits the whole batch). The round
cap + sentinel-ts /retry recovery already bounds a persistently broken
base.

Also trim "checkout" from the retry headlines (checkout failures do not
land in this branch) and hoist the duplicated MARK_TS assignment.

* fix(autofix): reset the consecutive-failure streak on prior infra-failure markers

The streak walker counted prior infra-failure headlines ("AutoFix could
not start —…") as failures, inflating the consecutive-failure count on
subsequent rounds.  A PR with 3 real agent failures, then 3 rounds of
base-build infra failures, then 1 more real failure would trip the
cap-5 breaker even though only 4 rounds were the PR's fault.

Add the two infra-failure headline patterns as reset strings in the
streak walker, alongside the existing push and no-op resets.  The
genuine agent-crash headline ("AutoFix could not start evaluation —…")
is deliberately excluded — it is a real failure and must still count.

* fix(autofix): clarify infra-failure headlines and else-branch comment (#7490)

Address review nits: the retry headline now mentions cancelled runs,
the cap headline says 'reached the round cap' instead of overstating
'could not start for N rounds', the else-branch comment says 'prepare
itself crashed' instead of 'agent crash', and the streak-reset pattern
is simplified now that both infra headlines share the same prefix.

---------

Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>

* fix(cli): keep role codenames and brief paths out of the posted review body (#7560)

The posted body still carried two operator registers #7550 left in place:
roster role subjects rendered their internal codenames ("Agent 1c:
Cross-file tracer", "Test coverage matrix (whole-diff)"), and an unread
brief's disclosure interpolated its filesystem path. And when verify and
the reverse audit failed the same way, the body said it twice, in two
near-identical sentences.

- Every Brief now carries a publicLabel — the dimension said as what it
  checks ("the cross-file consistency pass") — and coverage's structural
  disclosures carry it as publicSubject beside the internal subject, plus
  a path-free publicReason for unread briefs. The internal label and the
  path stay on stderr, where they are the selector an operator acts on;
  every dedup and certification check still keys on the internal subject.
- compose-review renders the public fields and groups by the reason the
  body PRINTS, so two unread briefs share one path-free sentence instead
  of repeating it per role.
- verificationGaps merges verify and reverse-audit failures of the same
  delivery shape into one sentence with both subjects and both
  consequences; mixed shapes keep their precise per-role texts, and the
  per-role rebuild commands stay on stderr either way.

Co-authored-by: verify <verify@local>

* fix(autofix): retry an agent timeout instead of advancing past its feedback (#7563)

A timeout evaluated NOTHING — the agent ran out of budget before finishing,
so nothing was committed and the feedback is unaddressed. It was treated as
an evaluated verdict (real ts, watermark advances), which strands that
feedback: the next scan sees "nothing new" and never retries. Observed on
#7471 (round 13/100), a heavily-reviewed 1871-line PR: rounds 11 and 13
timed out, but round 12 pushed — so a timeout is transient far more often
than not, and advancing past it left the round-13 feedback unhandled.

run-agent.mjs now drops an `agent-timeout` signal on result.timedOut, and
the handoff routes it like a pre-verdict crash: sentinel ts (feedback stays
live) and a retry, with a headline that names the real fix at the cap
(split the PR or raise the budget). A PR that PERSISTENTLY times out is
bounded by the round cap and the consecutive-failure cap, so this cannot
loop forever — it just stops treating a one-off budget blip as a verdict.

The loop guard stays terminal (a tool-call loop is a real defect, not a
budget blip). An API error still routes to its own model-key handoff; the
timeout signal is written only when NOT an API error.

Co-authored-by: wenshao <wenshao@example.com>

* feat(serve): add workspace-level generation (#7552)

* feat(serve): add workspace-level generation

* docs(serve): document workspace generation capability

* fix(serve): align workspace generation contracts

---------

Co-authored-by: ytahdn <ytahdn@gmail.com>

* ci: matrix ECS runner update + sudo install + repository_dispatch trigger (#7513)

* ci: matrix ECS runner update with sudo install

- Use matrix strategy (ecs-update-sg, ecs-update-64c) to update both
  physical ECS hosts in parallel (fail-fast: false).
- Always use sudo npm install -g so the package lands in /usr/local
  (system-wide PATH) instead of the runner user's home directory.
- Move concurrency to job level (matrix context not available at
  workflow level per actionlint).
- Add repository_dispatch trigger for release-driven updates.
- Register new runner labels in actionlint.yaml.

* fix(ci): use dispatch version for runner update

---------

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

* fix(web-shell): include managed id in artifact open requests (#7570)

Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>

* feat(serve): persist workspace channel configuration (#7514)

* feat(serve): persist workspace channel configuration

* fix(serve): harden channel settings snapshots

* fix(serve): validate startup channel names

* fix(serve): reserve all channel name

---------

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

* fix(sdk-python): require canonical form in validate_session_id (#7532)

uuid.UUID() accepts several non-canonical spellings — braced
{...}, urn:uuid:..., and dash-less hex — so validate_session_id let them
through after the RFC 4122 variant check. The value is then forwarded to
the CLI verbatim as --session-id/--resume, producing a malformed session
id downstream rather than a clear error at the SDK boundary.

Reject anything whose canonical form differs from the input. Case is
deliberately not part of the comparison: UUID() lowercases, and an
all-uppercase spelling is still valid canonical input.

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

* fix(web-shell): sync background agent status (#7561)

* fix(web-shell): sync background agent status

* fix(web-shell): harden background agent reconciliation

---------

Co-authored-by: ytahdn <ytahdn@gmail.com>

* feat(core): propagate trusted daemon invocation context (#7279)

* feat(core): propagate trusted daemon invocation context

* test(cli): update ACP startup expectation

* refactor(core): centralize ACP capability env key

* test(cli): update worktree ACP core mock

* test(integration): run daemon context smoke on PRs

* test(ci): update no-AK smoke expectation

* test(core): cover invocation context isolation

* fix(cli): compare ACP capability safely

* fix(docs): restore GitHub action input names

* fix(core): sanitize private ACP capability from child env

* fix(core): reuse private ACP capability env constant

* test(cli): cover malformed trusted invocation context

* test(acp-bridge): assert exact child environment

---------

Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: 易良 <1204183885@qq.com>

* fix(feishu): await stream cancels in media download teardown (#7465)

* fix(feishu): await stream cancels in media download teardown

downloadMedia left two reject paths' stream teardown unawaited:

- the oversize-stream path called reader.cancel() without awaiting, so a
  cancel error during teardown became an unhandled rejection (fatal under
  Node's default --unhandled-rejections=throw);
- the Content-Length reject path returned without cancelling resp.body,
  leaving the connection pinned until GC.

Both were already fixed for the sibling DingTalk downloader in #7361 (which
was itself modelled on this Feishu code), so this brings Feishu to parity.
Adds a regression test that pins the reader.cancel() await via a rejecting
cancel, plus an assertion that the Content-Length path releases the body.

* test(feishu): cover a rejecting body.cancel() on the Content-Length path

Mirrors the existing reader.cancel() teardown test for the other reject
path, per review feedback. Removing the await on resp.body?.cancel()
flips execution onto the 'rejected: size ... exceeds' branch and the
test fails.

* fix(autofix): make the review-address report wrapper lines bilingual (#7569)

The agent's address-summary.md / no-action.md already ends with a
collapsed Chinese translation, but the workflow-appended wrapper lines
around it — the "Addressed/Reviewed the latest feedback" lead-in, the
"Base-conflict check" line, and the "Re-review when you have a moment"
footer — were English-only and sat outside that block. So the posted
comment was only half translated, unlike the takeover-ack comments
(full collapsed Chinese block) and the "model/模型" sign-off in this
same report (already inline-bilingual).

Give each wrapper line an inline Chinese translation, matching the
model/模型 idiom. The English halves are preserved verbatim — the
streak-reset detector globs on "Addressed the latest review feedback"
and "no changes needed", and a test extracts these lines — so behaviour
is unchanged and old English-only comments still match. A new test pins
each English-Chinese pair so a future reword that drops the Chinese
fails. The terminal handoff/failure comment is left English-only for
now (SKILL.md keeps it so by design); that is a separate change.

Co-authored-by: wenshao <wenshao@example.com>

* feat(cli): post the review body bilingually when the PR description is Chinese (#7564)

When the PR author writes Chinese, the posted /review body was
English-only. fetch-pr now records whether the PR description contains
Han characters (prDescriptionHasHan, detected from the same gh pr view
call and stamped into the plan report), and compose-review renders the
body bilingually off that flag: the English body leads, the complete
Chinese version rides collapsed in a <details><summary>中文说明</summary>
block, and the model footer stays outside the fold. The signal is the
CLI's own — the caller cannot toggle the register of a certified body —
and a local plan has no field, so nothing changes for terminal-only
reviews.

Every deterministic body fragment carries an en/zh pair end to end:
compose-review's clause templates and describeChunkGap phrases, the
coverage disclosures (reasons, publicLabel role subjects via a new
publicLabelZh, the path-free unread-brief reason) and the Step 4/5 gap
texts including the combined same-shape sentence. Fragments with no
deterministic translation — model-written findings, caller echoes,
interpolated errors — ride verbatim in both halves. verificationGaps now
returns structural {subject, reason, subjectZh, reasonZh} entries, which
also removes compose-review's last recover-the-boundary-from-prose parse.

SKILL.md instructs the same format for the model-authored inline
comments: English finding first (marker and suggestion block stay in the
English half — tooling filters on them), full Chinese translation
collapsed beneath, footer last.

Co-authored-by: verify <verify@local>

* feat(autofix): auto-rerun a check that died on infrastructure, once (#7562)

* feat(autofix): auto-rerun a check that died on infrastructure, once

A failed check can be red because the machine died, not the code — a
self-hosted runner losing the server, the disk filling. #7490's E2E
failed with "runner lost communication with the server" and went green
on a rerun. The scan now reruns such a check's failed jobs automatically.

Detection is a conservative annotation whitelist (INFRA_FAILURE_SIGNATURES)
— only unambiguous machine failures, never a test-level timeout, which
could be a real regression. The one-shot guard is run_attempt, not a
marker: a run already retried to attempt 2 and still infra-failing is
persistent, so it is left for a human; after a rerun the attempt
increments, so the next scan will not rerun it. Every step is fail-safe
(any API error → no rerun), it runs only when the PR actually has a
failed check, and the gate carries the same review-address carve-out as
the other check selectors so the loop never reruns its own runs.

This is the transient-infra sibling of #7554 (stale-base): that merges
current main when a check is base-inherited; this reruns when a check
died on the runner. Neither touches a check that is a genuine failure.

Note: rerun-failed-jobs needs the PAT to hold `actions: write`.

* fix(autofix): use POSIX ERE groups in infra-failure regex, cover all signatures in tests (#7562)

* fix(autofix): also treat a git fetch/clone transport death as infra

#6506's checkout died mid-transfer — "fetch-pack: invalid index-pack
output" and "RPC failed; curl 92 ... CANCEL" — which then hung the job
into the 20m limit. That is infra, not the PR (it only touches a doc),
and a re-run made it green. But the infra-signature whitelist did not
cover it, so the auto-rerun did not fire and it waited on a human.

Add `invalid index-pack output` and `RPC failed` — the two canonical
git-transport-death phrases — to INFRA_FAILURE_SIGNATURES. A co-present
job-timeout line does not block the match (one matching line classifies
the run), and a BARE timeout with no transport signature is still left
alone, since it can be a real regression. Both new signatures are pinned
in the test's per-signature loop, plus a case on #6506's real composite
annotation and a bare-timeout-is-not-rerun guard.

* fix(autofix): paginate annotations and filter Autofix runs in infra-rerun loop (#7562)

---------

Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>

* fix(serve): detect stale SSE cursors across daemon restarts via epoch token; preserve turn attribution and surface compaction failures in replay (#7458)

* fix(daemon): epoch-token restart detection, compaction attribution, and degraded-snapshot signaling (DAEMON-001/007/008)

* fix(acp-bridge): field-level turn attribution merge and replayDegraded bridge test (#7458)

* fix(serve): skip bus epoch lookup for virtual subagent SSE streams (#7458)

The REST SSE route looked up the bus epoch for every session id, but
virtual subagent sessions ride their own bus and their compound ids are
not in the bridge's byId map, so the lookup threw and aborted the
subscription — breaking subagent event streams. Skip the lookup for the
virtual path and degrade a torn-down real session to a headerless stream
(mirrors the /acp route). Also bumps the daemon browser SDK bundle budget
(167KB -> 168KB) for the epoch fields and declares eventEpoch on
DaemonSession so the create/attach path drops its inline type cast.

* fix(serve): stamp eventEpoch on accepted continuations and surface replayDegraded in the SDK (#7458)

Address three review suggestions:
- POST /session/:id/continue now returns eventEpoch alongside lastEventId,
  mirroring the prompt 202 envelope so continuation-seeded SSE cursors
  detect daemon restarts (DAEMON-001)
- DaemonSessionClient exposes replayDegraded from the load response so SDK
  consumers can prefer the full transcript over a degraded snapshot
- add /acp dispatch-level regression test for the degraded-snapshot stderr
  breadcrumb (fires only when snapshot.degraded is set)

* test(cli): fix load-reply race in the degraded-breadcrumb transport test

Await each session/load reply frame before opening the session stream so
the GET cannot race conn.ownSession() into a 403; addresses the review
Critical on the deg-0 arm.

* fix(serve): allow and expose X-Qwen-Event-Epoch in CORS headers

Cross-origin SSE clients must send the epoch header through preflight and
read it from the response, or stale-cursor detection (DAEMON-001) is
silently disabled for every CORS client.

---------

Co-authored-by: qwen-code-bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen Autofix <qwen-autofix[bot]@users.noreply.github.com>

* feat(core): Align GenAI telemetry with ARMS (#7536)

* feat(core): align GenAI telemetry with ARMS

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

* fix(core): remove estimated token usage splits

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

* fix(core): address GenAI telemetry review feedback

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>

* fix(serve): avoid TOCTOU race dropping live sessions from list response (#7556)

* Initial plan

* fix(serve): avoid TOCTOU race dropping live sessions from list response

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: 易良 <1204183885@qq.com>

* fix(cli): prevent monitor turns after task_stop (#7573)

---------

Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix[bot]@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: destire-mio <qppque@gmail.com>
Co-authored-by: destire-mio <248462155+destire-mio@users.noreply.github.com>
Co-authored-by: Dragon <52599892+DragonnZhang@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: 易良 <1204183885@qq.com>
Co-authored-by: jinye <djy1989418@126.com>
Co-authored-by: chinesepowered <nlai@rediffmail.com>
Co-authored-by: ovochouovo <18212194+ovochouovo@users.noreply.github.com>
Co-authored-by: Edenman <67549719+BZ-D@users.noreply.github.com>
Co-authored-by: 克竟 <dingbingzhi.dbz@alibaba-inc.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: ytahdn <1294726970@qq.com>
Co-authored-by: ytahdn <ytahdn@gmail.com>
Co-authored-by: Truraly <94105924+Truraly@users.noreply.github.com>
Co-authored-by: zjgzx1988 <zjgzx1988@hotmail.com>
Co-authored-by: hogeheer499-commits <hogeheer499@gmail.com>
Co-authored-by: hogeheer <267467744+hogeheer499-commits@users.noreply.github.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Nothing Chan <chenliu.cl@alibaba-inc.com>
Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>
Co-authored-by: yuanyuanAli <135116774+yuanyuanAli@users.noreply.github.com>
Co-authored-by: verify <verify@local>
Co-authored-by: qqqys <qys177@gmail.com>
Co-authored-by: callmeYe <512217680@qq.com>
Co-authored-by: Qwen Autofix <qwen-autofix[bot]@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants