feat(acp): Batch session load replay - #6309
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in “response-mode” history replay path for ACP session/load, allowing the bridge/daemon to seed history in bulk (without per-frame fanout) and then replay the seeded snapshot when the session SSE stream first attaches. It also updates CLI serve routes to request response-mode replay for REST loads while keeping default ACP behavior streamed unless explicitly requested.
Changes:
- Add bulk replay metadata plumbing (
_meta) forsession/load, and seed replay events without populating the reconnect ring. - Update ACP HTTP dispatcher/connection handling to (a) mark initial replay pending on
session/load, (b) replay the bridge snapshot on first attach, and (c) defer prompt replies until replay completion. - Extend Vitest coverage across bridge, CLI ACP transport, and server route tests for response-mode load behavior.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/src/serve/server.test.ts | Adjusts serve tests to assert REST load uses response-mode replay. |
| packages/cli/src/serve/routes/session.ts | Forces REST session/load to request historyReplay: 'response'. |
| packages/cli/src/serve/acp-http/transport.test.ts | Adds over-the-wire tests for response-mode load + initial snapshot replay and replay deferral semantics. |
| packages/cli/src/serve/acp-http/dispatch.ts | Implements initial snapshot replay on first attach and tracks initial replay pending state. |
| packages/cli/src/serve/acp-http/connection-registry.ts | Adds initialReplayPending marker and treats initial replay like a replaying attach for deferral logic. |
| packages/cli/src/acp-integration/session/Session.ts | Extracts primeTurnFromHistory so turn can be seeded without emitting replay updates. |
| packages/cli/src/acp-integration/acpAgent.ts | Adds bulk replay request detection and builds replay updates into _meta without streaming history. |
| packages/cli/src/acp-integration/acpAgent.test.ts | Adds test ensuring load can return bulk replay updates without streaming history and preserves ordering invariants. |
| packages/acp-bridge/src/eventBus.ts | Adds seedReplayEvents and new resync signaling when seeded replay isn’t in the ring. |
| packages/acp-bridge/src/compactionEngine.test.ts | Tests that seeded replay advances compaction state but does not populate the ring. |
| packages/acp-bridge/src/bridgeTypes.ts | Introduces replay meta constants/envelope types and adds getSessionReplaySnapshot to bridge interface. |
| packages/acp-bridge/src/bridgeClient.ts | Refactors update publishing to allow bulk seeding of replay updates and artifact publication. |
| packages/acp-bridge/src/bridge.ts | Wires response-mode replay into loadSession, strips replay meta from the returned ACP state, and seeds replay updates. |
| packages/acp-bridge/src/bridge.test.ts | Adds tests for response-mode replay extraction, ordering relative to buffered early events, and coalescing mode compatibility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for the PR! Template looks good ✓ Problem: This is a tracked performance optimization — P1 item in #6312 (reduce per-session overhead on the daemon session-creation path). Large restored sessions replay history through per-frame child-to-daemon notifications, causing event-loop work and ring churn during load. The problem is real and well-documented via the tracking issue; prerequisite #6263 (NDJSON stream optimization + event-loop lag metric) is already merged. Direction: Clearly aligned — batch replay is the natural architecture for removing per-frame fanout during session load. The opt-in design ( Approach: Scope feels right. The bridge extraction, EventBus seeding, bridgeClient refactoring, and connection-registry initial replay flag are each a natural layer of the change. No drive-by refactors or scope creep — the Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题: 这是跟踪中的性能优化项——#6312(降低 daemon session 创建路径开销)的 P1 条目。大型 session 恢复时通过逐帧 child-to-daemon 通知 replay 历史,导致 load 期间的 event-loop 工作和 ring churn。问题真实存在,已通过跟踪 issue 充分记录;前置依赖 #6263(NDJSON stream 优化 + event-loop lag 指标)已合并。 方向: 明确对齐——批量 replay 是移除 session load 期间逐帧 fanout 的自然架构方案。可选设计( 方案: 范围合理。Bridge 抽取、EventBus seeding、bridgeClient 重构、connection-registry 初始 replay 标记是该变更的自然层次。没有顺手重构或范围膨胀—— 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal (before reading the diff): For "large sessions replay history through per-frame notifications causing event-loop work and ring churn," I'd add a batch replay mode where the agent collects all replay updates in Comparison with the diff: The PR's approach matches this proposal closely and exceeds it in edge-case handling — partial replay metadata with Findings: No critical blockers or AGENTS.md violations found.
TestingThis is a daemon-level protocol change with no TUI or visual output — tmux before/after testing doesn't apply (the PR author correctly notes "N/A; this is a protocol and daemon replay behavior change"). Validation comes from unit tests and CI. CI results (Linux): Unit test coverage from the diff:
Reviewer sign-off: 中文说明代码审查独立方案(读 diff 之前): 针对"大型 session 通过逐帧通知 replay 历史导致 event-loop 工作和 ring churn"的问题,我会添加一个批量 replay 模式,让 agent 在 与 diff 的比较: PR 的方案与此提案高度一致,并在边缘情况处理上超越——partial replay metadata 配合 发现: 未发现关键阻塞项或 AGENTS.md 违规。一个小的观察: 测试这是 daemon 层协议变更,没有 TUI 或视觉输出——tmux before/after 测试不适用。验证来自单元测试和 CI。 CI(Linux)全部通过。维护者 — Qwen Code · qwen3.7-max |
|
This is a clean, well-engineered optimization that does exactly what the tracking issue describes. The bulk replay path is the right architecture — the agent collects updates, the bridge seeds them into EventBus without ring/subscriber churn, and the dispatch layer replays from snapshot on first attach. Going back to my independent proposal: the PR matches it structurally and exceeds it on edge cases. The partial replay degradation, the 10K update safety cap, the coalesce conflict detection, and the seed-failure session cleanup are all things I wouldn't have thought of upfront but are clearly needed for production robustness. Every change in the diff is load-bearing — no drive-by refactors, no scope creep. The CI passes on Linux. Maintainer The one honest caveat: this is internal protocol plumbing with no TUI surface, so the only way to know it works in production is to ship it and watch the LGTM. ✅ 中文说明这是一个干净、工程良好的优化,完全按照跟踪 issue 的描述执行。批量 replay 路径是正确的架构——agent 收集 updates,bridge 将它们 seed 到 EventBus 而不产生 ring/subscriber churn,dispatch 层在首次 attach 时从 snapshot replay。 回顾我的独立方案:PR 在结构上与之匹配,并在边缘情况上超越。partial replay 降级、10K update 安全上限、coalesce 冲突检测、以及 seed 失败 session 清理都是我事先不会想到但显然是生产健壮性所需的东西。 diff 中的每个变更都是必要的——没有顺手重构,没有范围膨胀。测试套件(5 个文件 17 个新测试)覆盖了有意义的边缘情况。 CI(Linux)通过。维护者 LGTM. ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@qwen-code /review |
| _Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/28709530631)._ |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code review timed out. Qwen review timed out after 120 minutes. For large PRs, retry with a longer timeout by commenting: |
Initial response-mode replay was filtering the bridge snapshot down to session_update frames before subscribing from the snapshot high-water mark. That could skip other snapshot-backed events permanently. Extend the ACP transport regression test so initial replay includes a non-session_update bridge event. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Clean up response-mode restore entries if replay seeding fails so retries do not attach to a closed zombie bus. Also normalize bulk replay timestamps before returning the private envelope and preserve partial replay usage accounting. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM ✅ — Downgraded from Approve to Comment: CI still running.
No new issues found beyond the existing open review comments. Build and typecheck clean. All 377 acp-bridge tests and 1019 CLI tests pass.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Downgraded from Approve to Comment: CI still running.
Well-layered optimization. Bulk replay path cleanly separates bridge-side restore from live event fanout. Most prior review feedback (zombie entry cleanup, cumulativeUsage isolation, validation context, MAX_BULK_REPLAY_UPDATES cap, liftSessionUpdateTimestamps on bulk path) is addressed in the current code. Build passes, deterministic analysis (tsc + eslint) clean, all 927 targeted tests pass.
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qqqys
left a comment
There was a problem hiding this comment.
The critical load-replay corruption issue is resolved at the current head: response-mode replay now degrades to partial replay metadata instead of failing the session load, and failed setup removes the stored session so retries are not poisoned. I found no new critical blocker in this pass.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
What this PR does
This PR adds an opt-in bulk history replay path for daemon
session/loadso the bridge can request replay updates in the ACP response instead of receiving one child-to-daemon notification per historical frame. Default ACP behavior remains streamed replay unless the bridge explicitly requests response-mode replay.The bridge now parses and strips Qwen-private replay metadata, seeds replay snapshots without notifying subscribers or filling the reconnect ring, and replays initial ACP session streams from the current bridge snapshot while keeping the JSON-RPC load result ACP-standard. REST load now uses response-mode replay and continues returning the existing replay snapshot fields.
Why it's needed
Large restored sessions previously replayed history through the live event fanout path, which caused event-loop work, ring churn, and interference with subscribers during load. Moving the bridge-owned restore path to a bulk response removes that fanout while preserving compatibility for direct ACP clients and older streamed behavior.
Reviewer Test Plan
How to verify
Run the targeted ACP bridge and CLI serve tests and confirm response-mode load returns a replay snapshot without per-frame child-to-daemon replay fanout, default ACP load still streams history,
/acpload returns a standard result while the session stream receives replayedsession/updateframes, and reconnect withLast-Event-IDresumes initial replay deterministically.Evidence (Before & After)
N/A; this is a protocol and daemon replay behavior change with no TUI or visual output.
Tested on
Environment (optional)
Local macOS worktree using Node/npm from the repository environment. Verified with
cd packages/acp-bridge && npx vitest run src/bridge.test.ts src/bridgeClient.test.ts src/compactionEngine.test.ts,cd packages/acp-bridge && npx vitest run src/eventBus.test.ts src/compactionEngine.test.ts,cd packages/cli && npx vitest run src/acp-integration/acpAgent.test.ts src/serve/acp-http/transport.test.ts,git diff --check, andnpm run build && npm run typecheck.Risk & Scope
_metakeys and an internal bridge request option.Linked Issues
Related: #6263, part of #6312
中文说明
What this PR does
这个 PR 为 daemon
session/load增加了可选的批量历史 replay 路径,让 bridge 可以通过 ACP response 获取 replay updates,而不是让 child 对每个历史帧逐条通知 daemon。默认 ACP 行为仍然是 streamed replay,只有 bridge 显式请求 response-mode replay 时才启用新路径。bridge 现在会解析并剥离 Qwen 私有 replay metadata,以不通知 subscriber、也不填充 reconnect ring 的方式 seed replay snapshot,并在 ACP session stream 初次 attach 时从当前 bridge snapshot 流式补 replay,同时保持 JSON-RPC load result 符合 ACP 标准。REST load 现在使用 response-mode replay,并继续返回现有 replay snapshot 字段。
Why it's needed
大历史 session restore 之前会通过 live event fanout 路径 replay 历史,导致 load 期间有 event-loop 工作、ring churn,并可能干扰 subscriber。把 bridge 自己使用的 restore 路径改为 bulk response,可以移除这部分 fanout,同时保持 direct ACP client 和旧 streamed 行为兼容。
Reviewer Test Plan
How to verify
运行目标 ACP bridge 和 CLI serve 测试,确认 response-mode load 会返回 replay snapshot 且不会通过 child-to-daemon 逐帧 replay 通知扇出,默认 ACP load 仍然流式 replay 历史,
/acpload 返回标准结果且 session stream 会收到 replayedsession/updateframes,并且带Last-Event-ID重连时初始 replay 可以确定性续传。Evidence (Before & After)
N/A;这是协议和 daemon replay 行为改动,没有 TUI 或视觉输出。
Tested on
Environment (optional)
本地 macOS worktree,使用仓库环境中的 Node/npm。已通过
cd packages/acp-bridge && npx vitest run src/bridge.test.ts src/bridgeClient.test.ts src/compactionEngine.test.ts、cd packages/acp-bridge && npx vitest run src/eventBus.test.ts src/compactionEngine.test.ts、cd packages/cli && npx vitest run src/acp-integration/acpAgent.test.ts src/serve/acp-http/transport.test.ts、git diff --check和npm run build && npm run typecheck验证。Risk & Scope
_metakey 和内部 bridge request option。Linked Issues
Related: #6263, part of #6312