Skip to content

feat(serve): Bound replay snapshot history - #6482

Merged
doudouOUC merged 11 commits into
QwenLM:mainfrom
doudouOUC:agent/bounded-replay-snapshot-window
Jul 8, 2026
Merged

feat(serve): Bound replay snapshot history#6482
doudouOUC merged 11 commits into
QwenLM:mainfrom
doudouOUC:agent/bounded-replay-snapshot-window

Conversation

@doudouOUC

Copy link
Copy Markdown
Collaborator

What this PR does

Adds a bounded replay snapshot window for live daemon sessions. Live-session POST /session/:id/load now returns only the retained in-memory replay window, while completed live turns and restored bulk replay are trimmed by a serialized-byte cap. When older replay has been dropped, the snapshot starts with an id-less history_truncated status marker. The cap is threaded through serve CLI options, fast-path parsing, server/bridge wiring, and daemon status diagnostics, and SDK/WebUI consumers render the truncation marker as transcript status instead of treating it as unknown or as a resync trigger.

Why it's needed

Long-running live sessions and response-mode restore replay could keep growing daemon resident memory through replay history. The daemon also had an implicit full-history expectation around /load, which made ring-eviction recovery and late attach semantics unclear. This change bounds the live in-memory replay contract and leaves full transcript reads to a future paginated or streaming endpoint rather than a single large array response.

Reviewer Test Plan

How to verify

Run the acp-bridge tests and confirm completed live turns, response-mode restore replay, marker placement, transient marker filtering, oversized newest retention, and safe sizing behavior all pass.

Run the serve CLI/status tests and confirm --compacted-replay-max-bytes parses through yargs and the fast path, invalid values fail before listen, server bridge wiring passes the cap, and daemon status exposes the resolved byte limit.

Run the SDK/WebUI tests and confirm history_truncated is a known event, malformed payloads route to debug, valid payloads render as status, and replay injection does not set awaitingResync.

Run the root verification command and confirm build, typecheck, and lint complete successfully.

Evidence (Before & After)

N/A. This is daemon replay-memory behavior, SDK event handling, and documentation, not a visual or TUI layout change.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

macOS local workspace. Verified with cd packages/acp-bridge && npx vitest run src/compactionEngine.test.ts src/eventBus.test.ts src/bridge.test.ts, cd packages/cli && npx vitest run src/commands/serve.test.ts src/serve/fast-path.test.ts src/serve/daemon-status.test.ts src/serve/server.test.ts src/serve/run-qwen-serve.test.ts, cd packages/sdk-typescript && npx vitest run test/unit/daemonEvents.test.ts test/unit/daemonUi.test.ts, cd packages/webui && npx vitest run src/daemon/session/DaemonSessionProvider.test.tsx, and npm run build && npm run typecheck && npm run lint.

Risk & Scope

  • Main risk or tradeoff: The live /load response no longer represents unbounded history for long sessions; clients that assumed complete replay need to treat history_truncated as status and wait for the future full-transcript API.
  • Not validated / out of scope: Single active in-flight turns are still uncapped in PR-1, and full transcript retrieval is intentionally left for a paginated or streaming PR-2 endpoint.
  • Breaking changes / migration notes: The event is additive and does not bump the daemon event schema version. Existing clients that ignore unknown events still apply retained replay, while updated clients can render the truncation status.

Linked Issues

N/A

中文说明

这个 PR 做了什么

为 live daemon session 增加有界 replay snapshot window。live session 的 POST /session/:id/load 现在只返回内存中保留的 replay 窗口,completed live turn 和 restore bulk replay 都会按序列化字节上限裁剪。旧 replay 被裁剪时,snapshot 开头会带一个无 id 的 history_truncated 状态 marker。该 cap 已贯通 serve CLI、fast-path parser、server/bridge wiring 和 daemon status 诊断,SDK/WebUI 会把截断 marker 渲染为 transcript status,而不是当作 unknown event 或 resync 触发器。

为什么需要

长时间运行的 live session 和 response-mode restore replay 可能让 daemon 常驻内存中的 replay history 持续增长。/load 之前也隐含完整历史预期,使 ring eviction recovery 和 late attach 语义不清晰。本 PR 明确 live 内存 replay 的有界契约,并把完整 transcript 读取留给未来分页或流式 endpoint,而不是一次性返回大数组。

Reviewer 测试计划

如何验证

运行 acp-bridge 测试,确认 completed live turn、response-mode restore replay、marker 位置、transient marker 过滤、最新 oversized replay 保留和 safe sizing 行为都通过。

运行 serve CLI/status 测试,确认 --compacted-replay-max-bytes 通过 yargs 和 fast path 解析,非法值在 listen 前失败,server bridge wiring 传入 cap,daemon status 暴露解析后的字节上限。

运行 SDK/WebUI 测试,确认 history_truncated 是 known event,畸形 payload 进入 debug,合法 payload 渲染为 status,replay injection 不会设置 awaitingResync

运行根级验证命令,确认 build、typecheck 和 lint 成功完成。

证据(Before & After)

N/A。本 PR 修改 daemon replay 内存行为、SDK event handling 和文档,不是视觉或 TUI layout 改动。

测试平台

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

环境(可选)

macOS 本地工作区。已验证 cd packages/acp-bridge && npx vitest run src/compactionEngine.test.ts src/eventBus.test.ts src/bridge.test.tscd packages/cli && npx vitest run src/commands/serve.test.ts src/serve/fast-path.test.ts src/serve/daemon-status.test.ts src/serve/server.test.ts src/serve/run-qwen-serve.test.tscd packages/sdk-typescript && npx vitest run test/unit/daemonEvents.test.ts test/unit/daemonUi.test.tscd packages/webui && npx vitest run src/daemon/session/DaemonSessionProvider.test.tsx,以及 npm run build && npm run typecheck && npm run lint

风险与范围

  • 主要风险或取舍:长 session 的 live /load 响应不再代表无界完整历史;假设完整 replay 的客户端需要把 history_truncated 当作状态提示,并等待未来完整 transcript API。
  • 未验证 / 不在范围内:单个 active in-flight turn 在 PR-1 中仍不裁剪,完整 transcript retrieval 明确留给 PR-2 的分页或流式 endpoint。
  • Breaking changes / 迁移说明:该 event 是 additive,不提升 daemon event schema version。忽略 unknown event 的旧客户端仍会应用 retained replay;更新后的客户端可以渲染截断状态。

关联 Issue

N/A

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

Copy link
Copy Markdown
Collaborator Author

Ready for review. @wenshao @YiLiang could you take a look?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a bounded in-memory replay snapshot window for live daemon sessions, introducing a history_truncated marker so SDK/WebUI consumers can render truncation as transcript status (without triggering resync) while keeping daemon memory growth under control.

Changes:

  • Implement byte-capped replay snapshot retention in the bridge compaction engine and seed path, emitting history_truncated when older replay is dropped.
  • Thread a new --compacted-replay-max-bytes limit through the serve CLI (yargs + fast-path), server/bridge wiring, and daemon status reporting.
  • Teach SDK/WebUI event normalization/reducers/tests and docs to treat history_truncated as a status marker (not a resync trigger).

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/webui/src/daemon/session/DaemonSessionProvider.tsx Updates provider comments/semantics to reflect bounded snapshot reload behavior.
packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx Adds coverage that loaded snapshot truncation renders as status without resync.
packages/sdk-typescript/test/unit/daemonUi.test.ts Tests UI normalizer/reducer projection of history_truncated to status without resync.
packages/sdk-typescript/test/unit/daemonEvents.test.ts Tests SDK known-event validation + session state bookkeeping for history_truncated.
packages/sdk-typescript/src/daemon/ui/normalizer.ts Normalizes history_truncated into a status UI event (with payload validation).
packages/sdk-typescript/src/daemon/events.ts Adds history_truncated to known events, types, passthrough set, and reducer state.
packages/sdk-typescript/scripts/build.js Bumps browser bundle size budget to accommodate new SDK/UI logic.
packages/cli/src/serve/types.ts Extends ServeOptions with compactedReplayMaxBytes.
packages/cli/src/serve/server.ts Wires compactedReplayMaxBytes into bridge creation.
packages/cli/src/serve/server.test.ts Updates server wiring expectations for the new replay-byte cap.
packages/cli/src/serve/run-qwen-serve.ts Adds defaults + pre-listen validation for compactedReplayMaxBytes.
packages/cli/src/serve/run-qwen-serve.test.ts Adds validation and snapshot limit coverage for compactedReplayMaxBytes.
packages/cli/src/serve/fast-path.ts Enables fast-path parsing for --compacted-replay-max-bytes.
packages/cli/src/serve/fast-path.test.ts Tests fast-path parsing for --compacted-replay-max-bytes.
packages/cli/src/serve/daemon-status.ts Exposes resolved compactedReplayMaxBytes in daemon status limits.
packages/cli/src/serve/daemon-status.test.ts Updates daemon status snapshot fixtures for the new limit.
packages/cli/src/commands/serve.ts Adds yargs flag --compacted-replay-max-bytes with default and help text.
packages/cli/src/commands/serve.test.ts Tests CLI parsing + wiring of --compacted-replay-max-bytes into runQwenServe.
packages/acp-bridge/src/eventBus.ts Adds bulk replay seeding API and exports safe event sizing helper.
packages/acp-bridge/src/compactionEngine.ts Implements byte-bounded replay window segmentation and history_truncated marker emission.
packages/acp-bridge/src/compactionEngine.test.ts Adds unit tests for truncation behavior, marker filtering, and sizing failure behavior.
packages/acp-bridge/src/bridgeTypes.ts Extends daemon status limits with compactedReplayMaxBytes.
packages/acp-bridge/src/bridgeOptions.ts Extends BridgeOptions with compactedReplayMaxBytes (and minor type formatting).
packages/acp-bridge/src/bridge.ts Normalizes the cap, instantiates compaction engine with it, and surfaces it in status limits.
packages/acp-bridge/src/bridge.test.ts Adds bridge-level tests for validation and bounded replay load behavior.
docs/users/qwen-serve.md Documents the new CLI flag and updated /load bounded-replay semantics.
docs/developers/qwen-serve-protocol.md Updates protocol docs for bounded /load replay and history_truncated semantics.
docs/developers/daemon/17-configuration.md Adds --compacted-replay-max-bytes to daemon configuration reference.
docs/developers/daemon/14-cli-tui-adapter.md Updates resync guidance to account for bounded snapshot reload and truncation marker.
docs/developers/daemon/13-sdk-daemon-client.md Documents SDK consumer handling for state_resync_required vs history_truncated.
docs/developers/daemon/10-event-bus.md Updates replay-gap semantics to include resync frame + bounded snapshot reload note.
docs/developers/daemon/09-event-schema.md Updates schema docs to include the new history_truncated event.
docs/developers/daemon/08-session-lifecycle.md Documents bounded replay window for /load response fields and truncation marker.
docs/developers/daemon/03-acp-bridge.md Documents bounded replay fields and history_truncated behavior in bridge response.
docs/developers/daemon/01-architecture.md Updates architecture narrative for resync + bounded snapshot reload semantics.
docs/design/2026-07-07-bounded-replay-snapshot-window.md Adds design doc describing goals, segmentation strategy, and validation/UX semantics.

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

Comment thread docs/users/qwen-serve.md Outdated
Comment thread packages/cli/src/commands/serve.ts
Comment thread packages/sdk-typescript/scripts/build.js
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

@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 review findings. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.

@wenshao

wenshao commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

No new Suggestion-level findings this round — all prior suggestions have been addressed or superseded.

— glm-5.2 via Qwen Code /review

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC
doudouOUC requested a review from wenshao July 7, 2026 23:44
@wenshao

wenshao commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

✅ Independent verification — Linux, real running daemon

Verified GOOD at head 60bc061c1. The PR body reports macOS-only + unit tests; I re-ran on Linux and, beyond the unit suites, drove a real qwen serve daemon over HTTP (built from the PR head, mock ACP child, no model) to observe the bounded window behaving end-to-end.

The HEAD advanced mid-verification (c0ac3567460bc061c1, "address PR review suggestions"). I re-verified everything at the new head. That commit only hardens the feature — the fast-path now validates the cap too, run-qwen-serve reuses the canonical normalizeCompactedReplayMaxBytes, and the SDK history_truncated payload check got stricter (non-negative integers). compactionEngine.ts / bridge.ts / server.ts / daemon-status.ts are byte-identical between the two heads.

How it was verified (3 independent layers)

Layer Method Result
Real daemon E2E qwen serve in tmux + curl over HTTP; a mock ACP child produces ~8 KB completed turns so replay overflows a tiny cap ✅ all arms pass
Compiled-dist engine probe Node script driving the compiled TurnBoundaryCompactionEngine / EventBus the daemon actually loads ✅ 37 / 37
Unit suites on Linux The exact files listed in the test plan (acp-bridge / cli-serve / sdk / webui) ✅ 1793 / 0

1) Real daemon E2E — bounded /load window over HTTP

daemon E2E

  • Cap threads to a live daemon: --compacted-replay-max-bytes 4096 and 4194304 are both reflected verbatim at GET /daemon/status?detail=fulllimits.compactedReplayMaxBytes. CLI flag → fast-path → ServeOptionsBridgeOptions → bridge status wiring all confirmed on a running process.
  • Marker surfaces in a real late-attach /load: with a 4096-byte cap, after 4 completed turns (12 events total) a fresh client's POST /session/:id/load returns compactedReplay[0] = history_truncated, id-less (synthetic), payload {"reason":"replay_window_exceeded","truncatedEvents":9,"retainedEvents":3,"maxBytes":4096,"truncatedTurns":3,"fullTranscriptAvailable":false}.
  • Control arm: the same 4 turns under the default 4 MiB cap return the full 12-event replay with no marker — proving the marker is driven by the byte cap, not the flow.
  • Boot validation (now dual-layer): 0, -5, 1048576.5, 999999999999 each exit 1 before listen, daemon never binds. The new fast-path guard rejects most (qwen serve: --compacted-replay-max-bytes must be a positive safe integer in [1, 268435456].) and the runtime validator backstops the rest (Invalid compactedReplayMaxBytes: -5. …). Ceiling = 268435456 (256 MiB).

2) Compiled-dist compaction-engine probe (37 assertions)

engine probe

Drives the compiled artifact directly (deterministic, no model): oldest-dropped / newest-kept, keep-newest-when-a-single-turn-exceeds-cap (window never empties), seedReplayEvents filters an injected history_truncated so markers never compound, EventBus batched seed → engine wiring, full normalizeCompactedReplayMaxBytes validation matrix, and safe-sizing never-throws on an unserializable event.

3) PR unit suites on Linux — 1793 passed, 0 failed

unit suites

acp-bridge 429 · cli/serve 860 · sdk 360 (history_truncated known event, renders as status, does not set awaitingResync) · webui 144.


Scope actually exercised vs. left to unit tests

  • The HTTP E2E drove the live late-attach path (replayFieldsFor(existing,'load')). The response-mode disk-restore seed path (EventBus.seedReplayEventsTurnBoundaryCompactionEngine.seedReplayEvents) is covered by the engine probe (arms D/F) and the acp-bridge unit suite, not through a full restore-from-disk over HTTP.
  • SDK/WebUI rendering verified via unit suites only (not a live browser).

Verdict

From a verification standpoint this is safe to merge: the CLI flag, boot validation, /daemon/status exposure, the bounded live /load window, the synthetic history_truncated marker (placement + payload + non-compounding + non-resync semantics), and keep-newest-oversized all behave as designed on Linux against a real daemon.

🇨🇳 中文版验证报告(点击展开)

✅ 独立验证 —— Linux,真实运行的 daemon

在 head 60bc061c1 验证 通过。PR 描述仅在 macOS + 单元测试上验证;我在 Linux 上重跑,并且在单元测试之外,用 curl 通过 HTTP 驱动了一个真实的 qwen serve daemon(从 PR head 构建,mock ACP child,无需真实模型),端到端观察有界窗口的行为。

验证过程中 HEAD 前进了(c0ac3567460bc061c1"address PR review suggestions")。我已在新 head 上重新验证全部内容。该 commit 只是加固了功能:fast-path 现在也会校验 cap、run-qwen-serve 复用统一的 normalizeCompactedReplayMaxBytes、SDK 的 history_truncated payload 校验更严格(要求非负整数)。compactionEngine.ts / bridge.ts / server.ts / daemon-status.ts 在两个 head 之间逐字节相同。

验证方式(3 个独立层次)

层次 方法 结果
真实 daemon E2E tmux 里跑 qwen serve + curl;mock ACP child 产生约 8 KB 的完成 turn,使 replay 超过很小的 cap ✅ 全部通过
编译产物引擎探针 Node 脚本直接驱动 daemon 实际加载的已编译 TurnBoundaryCompactionEngine / EventBus ✅ 37 / 37
Linux 单元测试 测试计划里列出的全部文件(acp-bridge / cli-serve / sdk / webui) ✅ 1793 / 0

1)真实 daemon E2E —— HTTP 上的有界 /load 窗口

  • cap 贯通到运行中的 daemon: --compacted-replay-max-bytes 40964194304 都原样体现在 GET /daemon/status?detail=fulllimits.compactedReplayMaxBytes。CLI flag → fast-path → ServeOptionsBridgeOptions → bridge status 整条链路在真实进程上确认。
  • marker 在真实的 late-attach /load 中出现: cap=4096 时,4 个完成 turn(共 12 个 event)后,一个新客户端的 POST /session/:id/load 返回 compactedReplay[0] = history_truncated无 id(合成),payload 为 {"reason":"replay_window_exceeded","truncatedEvents":9,"retainedEvents":3,"maxBytes":4096,"truncatedTurns":3,"fullTranscriptAvailable":false}
  • 对照组: 同样的 4 个 turn 在默认 4 MiB cap 下返回完整的 12 个 event、无 marker —— 证明 marker 由字节上限触发,而非流程本身。
  • 启动校验(现在是双层): 0-51048576.5999999999999 都在 listen 之前 exit 1,daemon 从不 bind。新的 fast-path 守卫拦下大部分(qwen serve: --compacted-replay-max-bytes must be a positive safe integer in [1, 268435456].),运行时校验器兜底其余(Invalid compactedReplayMaxBytes: -5. …)。上限 = 268435456(256 MiB)。

2)编译产物压缩引擎探针(37 项断言)

直接驱动编译产物(确定性、无模型):丢最旧 / 保最新单个 turn 超过 cap 时仍保留最新(窗口永不清空)、seedReplayEvents 会过滤注入的 history_truncated,marker 不会叠加EventBus 批量 seed → engine 的接线、完整的 normalizeCompactedReplayMaxBytes 校验矩阵,以及对不可序列化 event 的 safe-sizing 永不抛错。

3)Linux 单元测试 —— 1793 通过,0 失败

acp-bridge 429 · cli/serve 860 · sdk 360(history_truncated 为已知 event,渲染为 status置位 awaitingResync)· webui 144。

实际覆盖范围 vs. 交给单元测试

  • HTTP E2E 驱动的是活跃 late-attach 路径(replayFieldsFor(existing,'load'))。response-mode 磁盘恢复的 seed 路径(EventBus.seedReplayEventsTurnBoundaryCompactionEngine.seedReplayEvents)由引擎探针(D/F 组)和 acp-bridge 单元测试覆盖,未通过完整的“从磁盘恢复”走一遍 HTTP。
  • SDK/WebUI 渲染仅由单元测试覆盖(非真实浏览器)。

结论

从验证角度看可以合入:CLI flag、启动校验、/daemon/status 暴露、有界的活跃 /load 窗口、合成 history_truncated marker(位置 + payload + 不叠加 + 非 resync 语义)、以及 keep-newest-oversized,在 Linux 上针对真实 daemon 均符合设计。

Verification harness: qwen serve built from PR head 60bc061c1 (acp-bridge + cli + sdk compiled from source), driven over HTTP with a mock ACP child; no real model. Engine probe runs against the compiled dist.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
wenshao previously approved these changes Jul 8, 2026
@wenshao

wenshao commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Thanks for the PR! (Re-run post conflict resolution)

Template looks good ✓ — all required headings present, bilingual body, specific test commands.

Problem: Real and well-scoped. Long-running daemon sessions accumulate replay history in memory with no bound — the design doc articulates clearly why turn-count caps are insufficient (large tool outputs) and why /load semantics need an explicit bounded contract. Not theoretical — any long-lived daemon will hit this.

Direction: Aligned. Bounding daemon memory is core infrastructure work. The history_truncated marker as a status-only event (not resync) is the right call — avoids reload loops. CHANGELOG has no direct reference but the daemon serve layer is clearly in-scope.

Size: Cross-package core change. Production logic: ~393 lines (additions + deletions across 4 packages). Tests: ~709 lines. Docs/design: ~149 lines. Below the 500-line core threshold, so no maintainer escalation needed on size. The threading across packages is necessary — the cap must flow from CLI flag through server to bridge internals.

Approach: Scope feels proportional. The cap threads through CLI → fast-path → server → bridge → compaction engine → SDK → WebUI — each layer does the minimum needed. The replayWindowLimits.ts module is a clean extraction for normalization + validation. Design doc's five audit rounds show the author considered simpler alternatives (turn-count cap, reusing state_resync_required) and rejected them for good reasons. No drive-by refactors or scope creep.

Conflict resolution by qwen-code-dev-bot was clean — three independent test additions de-interleaved in serve.test.ts. No semantic overlap.

Moving on to code review. 🔍

中文说明

感谢贡献!(冲突解决后重新运行)

模板完整 ✓ — 所有必需标题齐全,双语正文,具体测试命令。

问题:真实且范围明确。长时间运行的 daemon session 会在内存中无限累积 replay 历史。设计文档清楚解释了为什么 turn 计数上限不够(大 tool 输出),以及为什么 /load 语义需要显式有界契约。不是理论假设——任何长期运行的 daemon 都会遇到。

方向:对齐。限制 daemon 内存是核心基础设施工作。history_truncated 作为纯状态标记(不触发 resync)是正确选择——避免重载循环。CHANGELOG 无直接参考,但 daemon serve 层显然在范围内。

规模:跨包核心改动。生产逻辑约 393 行(跨 4 个包增删合计),测试约 709 行,文档/设计约 149 行。低于核心 500 行阈值,无需维护者升级。跨包贯穿是必要的——cap 必须从 CLI 参数流经 server 到 bridge 内部。

方案:范围与目标相称。cap 贯穿 CLI → fast-path → server → bridge → compaction engine → SDK → WebUI——每层只做必要的最小改动。replayWindowLimits.ts 是对规范化+校验的干净提取。设计文档的五轮审计表明作者考虑过更简单的替代方案并有充分理由排除。未检测到顺手重构或范围蔓延。

冲突解决干净——serve.test.ts 中三个独立测试添加被正确拆分,无语义重叠。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Code Review

Independent baseline: I'd solve unbounded replay memory with a byte-capped ring that drops oldest entries while keeping at least the newest segment, plus a synthetic marker event that clients render as status (not resync). The cap would thread through CLI options and daemon status. That's essentially what this PR does.

The implementation matches the baseline and exceeds it in a few ways: event-level segments for restore replay (more granular eviction than turn-level), the seedReplayEvents() path that prevents bulk restore from polluting liveJournal, lazy segment-array compaction at threshold 64, and the explicit "keep newest even when oversized" guarantee.

Key observations from reading the code:

  • Compaction engine (compactionEngine.ts): Clean refactor from flat compactedTurns[] to ReplaySegment[] with byte accounting. The replaySegmentStart pointer with lazy compaction is a good optimization — avoids array copies on every eviction. history_truncated correctly added to TRANSIENT_TYPES so seed() filters it out on re-seeding.
  • Byte sizing (replayWindowLimits.ts): Reuses serializedBridgeEventByteLength from EventBus. Validation rejects 0, negative, non-integer, non-safe-integer, and values above 256 MiB — fails closed at boot. Constants exported cleanly.
  • SDK validation (events.ts): isHistoryTruncatedData is thorough — checks reason string literal, non-negative integers for all counts, fullTranscriptAvailable === false. Malformed payloads route to debug, not crash. The RESYNC_PASSTHROUGH_TYPES set correctly includes history_truncated so replay injection doesn't trigger resync.
  • Normalizer (normalizer.ts): normalizeHistoryTruncated validates all fields and produces a clean status line. Falls back to debug on malformed payloads.
  • WebUI (DaemonSessionProvider.tsx): Comment updates correctly change "full transcript" → "bounded replay window". No behavioral change needed since replay injection already works.

One minor inefficiency: enforceReplayWindow() calls flattenReplaySegments() to compute retainedEvents for the eviction notification — this flattens all remaining segments just to count events. A running counter would avoid the allocation, but this is called at eviction time (not per-event) so the impact is negligible. Not a blocker.

No correctness bugs, security holes, or convention violations found.

Test Results

All specified test suites pass on Linux:

packages/acp-bridge:           3 files, 442 tests passed (8.66s)
packages/cli:                  5 files, 873 tests passed (22.57s)
packages/sdk-typescript:       2 files, 360 tests passed (0.53s)
packages/webui:                1 file,  144 tests passed (4.31s)
───────────────────────────────────────────────────────────
Total:                        11 files, 1819 tests passed

npm run build ✅ · npm run typecheck ✅ · npm run lint

Real-Scenario Testing

N/A for tmux — this is daemon-internal replay memory behavior, not a TUI/visual change. Triggering truncation would require a long-running daemon session generating >4 MiB of replay data. The unit tests comprehensively cover the byte-cap mechanics, marker placement, transient filtering, oversized-newest retention, SDK validation, and WebUI rendering. Maintainer @wenshao's independent verification (comment above) already drove a real qwen serve daemon over HTTP with mock ACP child and confirmed the bounded window end-to-end.

中文说明

代码审查

独立基线:我会用字节上限的环形缓冲来解决无限 replay 内存问题——丢弃最旧的条目但保留最新段,加一个合成标记事件让客户端渲染为状态(而非 resync)。cap 贯穿 CLI 选项和 daemon 状态。这正是本 PR 的做法。

实现匹配基线并在几个方面超越:restore replay 的事件级段(比 turn 级更细粒度的淘汰)、防止批量 restore 污染 liveJournalseedReplayEvents() 路径、阈值 64 的惰性段数组压缩、以及明确的"即使超限也保留最新"保证。

关键观察:

  • 压缩引擎:从平面 compactedTurns[]ReplaySegment[] 加字节计量的干净重构。replaySegmentStart 指针配合惰性压缩是好的优化。history_truncated 正确加入 TRANSIENT_TYPESseed() 时会过滤。
  • 字节计量:复用 EventBus 的 serializedBridgeEventByteLength。校验拒绝 0、负数、非整数、非安全整数和超过 256 MiB 的值——启动时 fail closed。
  • SDK 校验isHistoryTruncatedData 很全面。畸形 payload 路由到 debug。RESYNC_PASSTHROUGH_TYPES 正确包含 history_truncated
  • Normalizer:校验所有字段并产生干净的状态行。
  • WebUI:注释更新正确。

一个小低效:enforceReplayWindow() 调用 flattenReplaySegments() 计算 retainedEvents——在淘汰时展平所有剩余段只为计数。可以用运行计数器避免,但淘汰时调用(非每事件)影响可忽略。不阻塞。

未发现正确性 bug、安全漏洞或约定违反。

测试结果

所有指定测试套件在 Linux 上通过:

packages/acp-bridge:           3 文件, 442 测试通过 (8.66s)
packages/cli:                  5 文件, 873 测试通过 (22.57s)
packages/sdk-typescript:       2 文件, 360 测试通过 (0.53s)
packages/webui:                1 文件,  144 测试通过 (4.31s)
───────────────────────────────────────────────────────────
总计:                         11 文件, 1819 测试通过

npm run build ✅ · npm run typecheck ✅ · npm run lint

真实场景测试

tmux 不适用——这是 daemon 内部的 replay 内存行为,不是 TUI/视觉变更。单元测试全面覆盖。维护者 @wenshao 的独立验证(上方评论)已通过 HTTP 驱动真实 daemon 端到端确认有界窗口行为。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

This PR is well-executed. The problem is real (unbounded daemon memory for long sessions), the design is thoughtful (five audit rounds), and the implementation is clean.

Going back to my independent proposal — byte-capped ring with a status marker, threaded through CLI and daemon status — the PR matches it and exceeds it: event-level segments for restore replay give more granular eviction than turn-level, the seedReplayEvents() path cleanly separates bulk restore from live journal, and the "keep newest even when oversized" guarantee ensures clients never see an empty window.

On the reflection checklist: every change in the diff serves the stated goal, the design doc shows the author considered and rejected simpler alternatives for documented reasons, and the unit tests plus @wenshao's real-daemon E2E verification confirm it works as designed. The one minor inefficiency I spotted (flattening segments to count retained events in the eviction notification) is a negligible-cost operation at eviction time.

Maintainer @wenshao has already approved after independent verification. Code is green — build, typecheck, lint, and all 1819 tests pass. Approving.

中文说明

这个 PR 执行得很好。问题是真实的(长 session 的 daemon 内存无限增长),设计经过深思熟虑(五轮审计),实现干净。

回顾独立方案——字节上限环形缓冲加状态标记,贯穿 CLI 和 daemon 状态——PR 匹配并超越:restore replay 的事件级段提供更细粒度淘汰,seedReplayEvents() 路径干净分离批量 restore 和 live journal,"即使超限也保留最新"保证确保客户端不会看到空窗口。

反思检查:diff 中每个变更都服务于目标,设计文档表明作者考虑并排除了更简单的替代方案,单元测试加上 @wenshao 的真实 daemon E2E 验证确认符合设计。发现的一个小低效(淘汰时展平段来计数)在淘汰时操作成本可忽略。

维护者 @wenshao 已在独立验证后批准。代码全绿——build、typecheck、lint 和全部 1819 个测试通过。批准。

Qwen Code · qwen3.7-max

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

LGTM, looks ready to ship. ✅

@wenshao
wenshao added this pull request to the merge queue Jul 8, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Jul 8, 2026
@wenshao

wenshao commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /resolve

Comment thread packages/acp-bridge/src/compactionEngine.ts
…-bytes and max-total-sessions

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

Copy link
Copy Markdown
Collaborator

Qwen Code resolved the merge conflicts and pushed the branch update.

Conflict Resolution Summary — PR #6482

Conflicted file

packages/cli/src/commands/serve.test.ts

What conflicted

Three conflict regions, all in test code. The PR branch added tests for --compacted-replay-max-bytes (bound replay snapshot history), while origin/main independently added tests for --max-total-sessions. Both features touched the same describe blocks, producing three overlap regions:

  1. Argument parser tests (line ~60): Two independent it() blocks — one testing --compacted-replay-max-bytes parsing, the other testing --max-total-sessions parsing — shared a closing });, causing a structural conflict.
  2. Handler integration test title (line ~251): Two independent test titles at the same insertion point.
  3. Handler integration test bodies (line ~261): The test bodies for the two features above were interleaved because they shared a common mockRunQwenServe.mockResolvedValueOnce(...) preamble.

How resolved

All three conflicts were independent feature additions with no semantic overlap. Resolution kept both sides as separate, complete test cases:

  • Region 1: Added the missing }); closing brace so each parser test stands alone.
  • Regions 2–3: De-interleaved the merged bodies into two distinct it() blocks, each with its own mockResolvedValueOnce, startServeHandlerWithArgs, and expect assertion.

No other files were modified. All auto-merged files were accepted as-is.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC doudouOUC self-assigned this Jul 8, 2026
wenshao
wenshao previously approved these changes Jul 8, 2026
@wenshao

wenshao commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@doudouOUC
doudouOUC enabled auto-merge July 8, 2026 03:09
Comment thread packages/acp-bridge/src/compactionEngine.ts
Comment thread packages/acp-bridge/src/compactionEngine.ts
Comment thread packages/acp-bridge/src/compactionEngine.ts
Comment thread packages/acp-bridge/src/compactionEngine.test.ts
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Comment thread packages/acp-bridge/src/compactionEngine.ts Outdated
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No review findings. Downgraded from Approve to Comment: CI failing: review-pr, Test (ubuntu-latest, Node 22.x).

— glm-5.2 via Qwen Code /review

@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 review findings. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

Comment thread packages/cli/src/serve/daemon-status.ts
Comment thread packages/sdk-typescript/src/daemon/events.ts
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC
doudouOUC requested a review from wenshao July 8, 2026 06:27
@wenshao

wenshao commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

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

LGTM, looks ready to ship. ✅

@doudouOUC
doudouOUC added this pull request to the merge queue Jul 8, 2026
Merged via the queue into QwenLM:main with commit 1420566 Jul 8, 2026
27 of 28 checks passed

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] DaemonHistoryTruncatedData and DaemonHistoryTruncatedEvent are now exported from the SDK barrels, but @qwen-code/webui/daemon-react-sdk still does not re-export them from its curated facade. Consumers using that facade cannot name the new first-class event type without importing the lower-level SDK directly. Please add both types to packages/webui/src/daemon/index.ts and packages/webui/src/daemon-react-sdk.ts alongside the neighboring daemon event/status types.

— gpt-5 via Qwen Code /review

channelIdleTimeoutMs: number;
sessionIdleTimeoutMs: number;
acpConnectionCap: number | null;
compactedReplayMaxBytes: number;

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] DaemonStatusReport says additive status fields remain optional so SDKs can talk to older v1 daemons, but this new limit is typed as always present. Older daemons that predate compactedReplayMaxBytes can omit it, so typed consumers may read undefined despite the SDK promising a number.

Suggested change
compactedReplayMaxBytes: number;
compactedReplayMaxBytes?: number;

— gpt-5 via Qwen Code /review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants