fix(daemon): reduce subagent live replay journal - #9057
Conversation
|
Re-run on the current head. Since the last pass (which reviewed
Moving on to code review. 🔍 中文说明在当前 head 上重跑。上一轮审查的是
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent proposal first: a load-time filter can't recover root events the full journal's own cap already evicted, so the minimal shape is a separately bounded projection journal maintained at ingest time. The PR matches it — and this head also completes the part that blocked my last pass. The previous blocker is resolved. Commit 2's coalesce path — a summary load sharing an in-flight full restore — shipped documented-but-unimplemented at the head I last reviewed. At this head it is fully implemented in the waiter branch: the waiter recomputes its own-mode replay fields from the registered entry (never reusing or filtering down the owner's projection), re-asserts attachability after the recompute await, and rolls back both the coalesce reservation and the attach counter on failure — exactly the R3-7 race, now pinned by a dedicated regression test that kills the channel mid-recompute and expects The retention rule provably matches the consumer. The summary journal keeps root frames, lifecycle/tool frames, and nested usage frames — and I checked the predicate against the UI rather than taking the doc's word: the SDK normalizer emits Boundaries are validated at every layer. The REST route 400s invalid values on both actions but forwards the mode only to The round-4 Files changed (17)
Testing evidence (this run: CI signal via API, PR code never executed)All checks on this head are settled — nothing pending, nothing failed.
The refresh-recovery claim is behavioral. The new regression tests pin the mechanism (journal divergence under cap pressure, marker non-inheritance, waiter re-projection, the channel-death race), and the prior head passed a full sandboxed verification plus the maintainer's real-stack run — but neither was executed against this exact head (the delta since is review-response commits). Sandboxed verification would settle the current head end-to-end: 中文说明代码审查先写独立方案：load 时过滤无法找回已被 full journal 自身上限逐出的根事件，因此最小形态是在写入时维护一个独立受限的投影 journal。PR 与之一致——并且本 head 补全了上一轮被我挡下的部分。 此前的阻塞项已解决。 提交 2 的 coalesce 路径（summary load 共享在途 full restore）在我上次审查的 head 上只有文档承诺、没有实现。在本 head 上已完整实现：waiter 从已注册的 entry 重新计算自己模式的 replay 字段（绝不复用或降级 owner 的投影），在 recompute 的 await 之后重新断言可挂载性，失败时回滚 coalesce 预留与 attach 计数——正是 R3-7 的竞争，现在有专门的回归测试钉住（recompute 中途杀掉 channel，期望 保留规则与消费方可证明地一致。 我没有只看文档，而是对照 UI 核验了谓词：SDK normalizer 只在 每一层边界都有校验。 REST 路由对两种 action 都在非法值时返回 400，但只把模式转发给 第 4 轮 测试证据（本次运行：通过 API 读取 CI 信号，从未执行 PR 代码）本 head 的所有检查已结束——无 pending、无失败。单元测试全套（ubuntu）为绿；行为面还有 刷新恢复是行为性声明：新的回归测试钉住了机制，且先前 head 已通过完整沙箱验证与 maintainer 真实环境验证——但两者都不是在本 head 上执行。沙箱验证可以在当前 head 上端到端确认： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the thing that blocked my last pass is now genuinely implemented and test-pinned, the mechanism verifies against the actual UI consumer, and CI is fully green on this head; what keeps this from a 5 is a set of open test-coverage suggestions and an integration leg that never runs in CI. Stepping back: my independent proposal for this problem was a separately bounded projection journal at ingest time — load-time filtering provably can't recover root events the full journal already evicted — and that's exactly what shipped. The problem is real and was reproduced on a real stack by the maintainer, not inferred from the code. Every file in the diff maps to the mechanism; there's no drive-by churn. The part I pushed back on last time — the coalesce exception — is the most intricate piece of the PR, and it's also the one that was rebuilt most carefully: own-mode recompute, post-await re-assert, counter rollback on every failure arm, and a regression test for the exact channel-death race the round-3 review caught. If I have to maintain this in six months, the dual-journal state is a clean Reservations, named: the round-4 suggestions are real gaps (usage retention tested only for the both-fields case, resume-coerces-to-full untested at the bridge level, one projection call site uncovered) — none is a defect, and I'd rather land the fix and let the takeover loop backfill coverage. The behavioral claim on this exact commit rests on the regression tests plus the prior head's sandboxed and real-stack verification; Verdict: approve — all stages clean at this head, no escalation grounds, CI settled green. 中文说明置信度：4/5 —— 上一轮挡下我的问题这次是真正实现了，并且有测试钉住；机制对照实际 UI 消费方核验通过；本 head 的 CI 全绿。没到 5 分的原因是还有一组未闭环的测试覆盖建议，以及一条在 CI 中从不运行的集成腿。 退一步看：我对这个问题的独立方案就是写入时维护独立受限的投影 journal——load 时过滤无法找回已被逐出的根事件——而 PR 交付的正是这个形态。问题真实存在，由 maintainer 在真实环境复现，不是从代码推断出来的。diff 里每个文件都对应机制本身，没有顺手夹带的改动。我上次反对的部分——coalesce 例外——是 PR 里最复杂的片段，也是重构最仔细的片段：按自身模式重算、await 之后重新断言、每条失败路径都回滚计数，并为第 3 轮评审抓到的 channel-death 竞争配了回归测试。六个月后维护这段代码：双 journal 状态是干净的 保留意见，点名：第 4 轮建议是真实的覆盖缺口（usage 保留只测了双字段情形、resume 强制 full 在 bridge 层无测试、有一个投影调用点未覆盖）——都不是缺陷，我更倾向先合入修复、由 takeover 循环补齐覆盖。本提交上的行为性声明依赖回归测试加上先前 head 的沙箱与真实环境验证；在当前 head 上跑一次 结论：批准——本 head 各阶段干净，无需升级，CI 已全绿。 — Qwen Code · qwen3.8-max Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 1 render-shaping file:
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 Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
| private journalTruncatedEvents = 0; | ||
| private liveJournalTextSegment: LiveJournalTextSegment | undefined; | ||
| private fullJournal = createLiveJournalState(); | ||
| private summaryJournal = createLiveJournalState(); |
There was a problem hiding this comment.
Live-journal memory ceiling per session doubles (and the second journal is not "small").
summaryJournal is appended and evicted through the same appendLiveJournal with the same this.maxJournalEvents / this.maxJournalBytes caps as fullJournal, so a single session can now retain up to 2 × maxJournalBytes (16 MiB by default) and 2 × maxJournalEvents (20 000 entries) for one in-flight turn.
Concrete scenario: a long turn with heavy nested subagent output. The full journal's window fills with recent nested events, while the summary journal independently holds a window of much older root events that the full journal has already evicted — those BridgeEvent objects stay pinned instead of being collected. Operators sizing daemon memory from maxJournalBytes × live sessions are now off by 2×. Sessions with no subagents at all pay the extra bookkeeping window for a journal that is a strict duplicate of the full one.
Worth either deriving the summary caps from the full ones (e.g. a fraction), or documenting the new 2× ceiling in replayWindowLimits / the design doc.
| if (event.type !== 'session_update') return true; | ||
| const data = event.data as SessionUpdateData | undefined; | ||
| const meta = data?.update?._meta; | ||
| if (extractParentToolCallIdFromMeta(meta) === undefined) return true; |
There was a problem hiding this comment.
Missing the normalizer's self-parent guard, so a self-referencing tool frame is visible live but disappears after a summary refresh.
This predicate is the daemon-side mirror of projectMainTranscriptEvents in DaemonSessionProvider.tsx, which filters on the normalized UI event's parentToolCallId. But normalizeToolUpdate (packages/sdk-typescript/src/daemon/ui/normalizer.ts) deliberately drops a self-referencing parent:
// Self-reference guard: defensively drop `parentToolCallId === toolCallId`.
const parentToolCallId =
rawParentToolCallId && rawParentToolCallId !== toolCallId
? rawParentToolCallId
: undefined;So for a tool_call / tool_call_update whose _meta.parentToolCallId equals its own toolCallId, the UI treats the frame as a root tool block and renders it in the main transcript, while isSummaryLiveJournalEvent sees a non-empty parentToolCallId and a non-agent_message_chunk kind and returns false — the frame never enters the summary journal.
Scenario: such a frame arrives mid-turn, the user sees the tool block, then refreshes Web Shell. The summary replay omits it, so a tool block that was on screen a second ago is gone from the restored transcript until the turn completes and persisted history takes over. Mirroring the guard here (parentToolCallId === data.update.toolCallId → treat as root) keeps the two projections in sync.
|
|
||
| ## Protocol and SDK | ||
|
|
||
| The compaction engine maintains independently bounded `full` and `summary` live journals. Both share the completed-turn compaction and event high-water mark. The full journal retains every update. The summary journal excludes `session_update` frames carrying a non-empty `_meta.parentToolCallId`, while retaining root updates and all non-session events. |
There was a problem hiding this comment.
Design doc contradicts the implementation on the nested-usage carve-out.
This sentence says the summary journal "excludes session_update frames carrying a non-empty _meta.parentToolCallId", full stop. The implementation deliberately does not: isSummaryLiveJournalEvent re-admits nested agent_message_chunk frames whose _meta.usage carries a numeric inputTokens/outputTokens, because the main transcript consumes them (projectMainTranscriptEvents keeps assistant.usage events even when parentToolCallId is set, and extractAssistantUsage requires exactly those fields).
That carve-out is load-bearing: without it, refreshing mid-turn silently drops all subagent token accounting from the main conversation's usage totals. Since this file is the stated design reference, someone "simplifying" the predicate to match the doc would reintroduce that bug with no test naming the doc as the source of truth. Please state the usage exception here (the PR description already does).
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. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
Maintainer verification: real local stack, head vs merge-baseI rebuilt this PR and its merge-base into two fully isolated trees and ran both as real Verdict: the fix works, is backward compatible on the wire, and the new tests are non-vacuous. I found no blocking issue. Two minor observations at the bottom.
ScenarioOne turn, two foreground subagents:
Step 4 leaves the turn permanently in-flight with no live traffic — the exact state a user refreshing mid-turn lands in, and race-free to snapshot. Evidence 1 —
|
| load request | base 3fd40fd |
head 438957d |
|---|---|---|
default (no liveReplayMode) |
33 entries / 527,743 B, truncation marker (3,853 dropped, 8,160 kept), 0 root updates, window [[N003841]]→[[N012000]] |
byte-identical to base — 33 entries / 527,743 B, same marker, same window |
liveReplayMode: 'full' |
same as default (option unknown, ignored) | identical to default |
liveReplayMode: 'summary' |
same as default (option unknown, ignored) | 12 entries, no truncation marker, 11 root updates + 1 nested usage frame |
liveReplayMode: 'compact' |
200 (ignored) |
400 invalid_live_replay_mode |
resume + liveReplayMode |
200 |
200, silently ignored (load-only) |
The default path being byte-identical across base and head is the backward-compatibility proof for the SDK, /acp and every other existing consumer.
Summary journal composition on head (event ids 1–13; id 7 excluded):
id= 1 371B root user_message_chunk "FLOOD_TASK_TRIGGER run the audit"
id= 2 368618B root available_commands_update
id= 3 329B root agent_message_chunk "<<ROOT_ALPHA>> Kicking off the short probe subagent."
id= 4 421B root tool_call call_short_1 pending
id= 5 407B root usage(in=1234,out=567)
id= 6 557B root tool_call_update call_short_1 in_progress
id= 8 474B parent=call_short_1 usage(in=1234,out=567) <-- nested usage frame, retained
id= 9 946B root tool_call_update call_short_1 completed
id=10 343B root agent_message_chunk "<<ROOT_BETA>> Probe done. Now running the verbose audit subagent."
id=11 422B root tool_call call_flood_1 pending
id=13 562B root tool_call_update call_flood_1 in_progress
(id 7 = nested subagent text — correctly excluded)
So the projection keeps exactly what the design doc claims: root updates, both subagent lifecycles, tool state, and the nested _meta.usage frame — while dropping nested detail.
Evidence 2 — real Web Shell, real refresh
Driven in a real browser: type the prompt, wait for the flood, then reload the page mid-turn.
Recorded request bodies on the recovery load confirm the WebUI wiring rather than assuming it:
- base:
liveReplayMode: (absent)×2 → restored message list is 226 chars, the truncation line only, no root markers - head:
liveReplayMode: "summary"×2 → restored message list contains<<ROOT_ALPHA>>,<<ROOT_BETA>>, both Agent rows and↑1.2k ↓567 · 2 tool calls, no truncation notice
Note the base failure mode is worse than "some history missing": Web Shell does not render nested subagent detail in the main transcript, so the 8,160 nested events it did retain render as nothing at all.
Evidence 3 — subagent detail path unaffected
After the same refresh on head, opening the detail view shows 1,631 nested lines starting at [[N000001]] — including chunks the parent journal had already evicted (its full window starts at [[N003841]]). The detail view reads the event ring, not the compaction journal, and this PR does not touch it.
Evidence 4 — tests, and a mutation check
| suite | result |
|---|---|
acp-bridge compactionEngine + eventBus |
168 passed |
acp-bridge bridge |
583 passed |
cli serve (-t "live replay mode") |
3 passed |
sdk-typescript DaemonClient |
329 passed |
webui DaemonSessionProvider |
268 passed |
To confirm the new tests actually constrain behaviour, I mutated isSummaryLiveJournalEvent to return true (making summary identical to full) and re-ran compactionEngine.test.ts: 5 failed / 104 passed, including the summary marker-anchor case. Mutation reverted afterwards.
Evidence 5 — cost of the second journal
The PR flags the extra journal as its main tradeoff, so I measured the worst case: a root-only in-flight turn (no subagent, so every event is summary-eligible and both journals hold the same content), default caps, filling the 8 MiB byte limit.
base 3fd40fd |
head 438957d |
|
|---|---|---|
| full journal on the wire | 6,185,692 B (31 entries, truncated) | 6,185,692 B (identical) |
| daemon RSS growth | +87.2 MiB | +87.8 MiB |
Under 1% apart. The second journal does not duplicate payload: both journals push references to the same BridgeEvent objects, and the per-journal text segments hold references to the same chunk strings — only the entry bookkeeping is duplicated. The tradeoff is cheaper than the description implies.
Two minor observations (non-blocking)
-
resumeaccepts a validliveReplayModeand silently ignores it. An invalid value 400s at the route, butliveReplayMode: 'summary'on/resumereturns200and is coerced to full replay by the bridge. This matches the stated load-only design and the SDK never sends it on resume, so nothing in-tree is affected — but a third-party client that does send it gets full replay with no signal. Rejecting it at the route would be more consistent with the invalid-value handling. -
The summary journal is not automatically small. In my run one root
available_commands_updateframe was 368 KB of the 374 KB summary total. That is not a regression (the same frame is a root event on base), but the summary window's headroom is governed by root frame sizes, so excluding nested detail is a large win only while root frames stay modest.
中文版
维护者验证：本地真实环境，head 与 merge-base 对照
我把本 PR 与其 merge-base 分别构建成两棵完全隔离的树，作为两个真实的 qwen serve daemon 跑在同一个脚本化模型服务上，然后端到端复现了问题与修复——覆盖 daemon REST、真实浏览器里的真实 Web Shell，以及 subagent 详情路径。
结论：修复有效，线上协议向后兼容，新增测试非空洞。未发现阻塞性问题。 文末有两点次要观察。
| PR head | 438957db89aa318b7451ca5f4809529c4f1fa41c |
| Merge-base | 3fd40fd065b041bbc39c77211ceb2e7f9021e98c |
| 两棵树 | npm run build + npm run bundle，从 dist/cli.js 启动 |
| Daemon | head :8811、base :8812，--max-journal-events 32，各自独立 QWEN_HOME |
| 模型 | 脚本化 OpenAI 兼容服务，逐请求 JSONL 台账 |
| Web Shell | 各自 daemon 自带的 dist/web-shell——前后端天然同源 |
场景
一个回合，两个前台 subagent：
- 主会话文本
<<ROOT_ALPHA>>→agent工具调用（subagent A） - subagent A 正常结束——这一步才会让 daemon 发出嵌套 usage 帧（
agent_message_chunk+_meta.usage+_meta.parentToolCallId），即主会话统计所消费的那一帧 - 主会话文本
<<ROOT_BETA>>→agent工具调用（subagent B） - subagent B 吐出 12,000 个嵌套 chunk 后挂住，不发
finish_reason
第 4 步让回合永久停在"进行中且无实时流量"的状态——正是用户中途刷新时的真实态，且读快照时不会与实时流竞态。--max-journal-events 32 是生产环境 10,000 事件窗口的等比缩小版，好让淘汰在几秒内可达。
证据一：POST /session/:id/load
每个嵌套 chunk 都带单调标记 [[Nnnnnnn]]，所以保留窗口是可识别的，而不只是可计量的。
| load 请求 | base 3fd40fd |
head 438957d |
|---|---|---|
默认（不带 liveReplayMode） |
33 条 / 527,743 B，截断标记（丢 3,853、留 8,160），主会话事件 0 条，窗口 [[N003841]]→[[N012000]] |
与 base 逐字节一致——33 条 / 527,743 B，标记与窗口完全相同 |
liveReplayMode: 'full' |
同默认（未知参数，被忽略） | 与默认一致 |
liveReplayMode: 'summary' |
同默认（未知参数，被忽略） | 12 条，无截断标记，11 条主会话更新 + 1 条嵌套 usage 帧 |
liveReplayMode: 'compact' |
200（被忽略） |
400 invalid_live_replay_mode |
resume + liveReplayMode |
200 |
200，静默忽略（仅 load 生效） |
默认路径在 base 与 head 上逐字节一致，这就是 SDK、/acp 及其它既有消费方的兼容性证明。
head 上 summary journal 的构成（事件 id 1–13，id 7 被排除）：
id= 1 371B 主会话 user_message_chunk "FLOOD_TASK_TRIGGER run the audit"
id= 2 368618B 主会话 available_commands_update
id= 3 329B 主会话 agent_message_chunk "<<ROOT_ALPHA>> ..."
id= 4 421B 主会话 tool_call call_short_1 pending
id= 5 407B 主会话 usage(in=1234,out=567)
id= 6 557B 主会话 tool_call_update call_short_1 in_progress
id= 8 474B parent=call_short_1 usage(in=1234,out=567) <-- 嵌套 usage 帧，已保留
id= 9 946B 主会话 tool_call_update call_short_1 completed
id=10 343B 主会话 agent_message_chunk "<<ROOT_BETA>> ..."
id=11 422B 主会话 tool_call call_flood_1 pending
id=13 562B 主会话 tool_call_update call_flood_1 in_progress
（id 7 = 嵌套 subagent 文本，已正确排除）
可见该投影保留的内容与设计文档所述完全一致：主会话更新、两个 subagent 的生命周期、工具状态，以及嵌套 _meta.usage 帧；同时丢弃嵌套详情。
证据二：真实 Web Shell，真实刷新
在真实浏览器中驱动：输入提示词，等待 flood，然后在回合进行中刷新页面。
恢复请求的报文体被逐条记录，用以证实（而非假定）WebUI 的接线：
- base：
liveReplayMode: (absent)×2 → 恢复后的消息区仅 226 字符，只有那一行截断提示，无任何主会话标记 - head：
liveReplayMode: "summary"×2 → 恢复后的消息区含<<ROOT_ALPHA>>、<<ROOT_BETA>>、两条 Agent 行与↑1.2k ↓567 · 2 tool calls，无截断提示
注意 base 的失效形态比"丢了一部分历史"更严重：Web Shell 主会话本就不渲染嵌套 subagent 详情，因此它保留下来的那 8,160 个嵌套事件在界面上什么都渲染不出来。
证据三：subagent 详情路径未受影响
在 head 上做同样的刷新之后打开详情视图，可看到从 [[N000001]] 开始的 1,631 行嵌套内容——其中包含主会话 journal 早已淘汰的那些 chunk（其完整窗口从 [[N003841]] 才开始）。详情视图读的是 event ring 而非 compaction journal，本 PR 未触碰该路径。
证据四：测试与变异检查
| 套件 | 结果 |
|---|---|
acp-bridge compactionEngine + eventBus |
168 通过 |
acp-bridge bridge |
583 通过 |
cli serve（-t "live replay mode"） |
3 通过 |
sdk-typescript DaemonClient |
329 通过 |
webui DaemonSessionProvider |
268 通过 |
为确认新增测试确实约束了行为，我把 isSummaryLiveJournalEvent 变异为 return true（使 summary 等同于 full）后重跑 compactionEngine.test.ts：5 失败 / 104 通过，其中包括 summary 分页锚点用例。变异已还原。
证据五：第二个 journal 的代价
PR 自述把额外的 journal 列为主要取舍，因此我测了最坏情况：纯主会话的进行中回合（无 subagent，故所有事件都进 summary，两个 journal 内容相同），默认上限，打满 8 MiB 字节限额。
base 3fd40fd |
head 438957d |
|
|---|---|---|
| 线上完整 journal | 6,185,692 B（31 条，已截断） | 6,185,692 B（完全一致） |
| daemon RSS 增长 | +87.2 MiB | +87.8 MiB |
相差不足 1%。第二个 journal 并不会复制载荷：两个 journal 压入的是同一批 BridgeEvent 对象的引用，各自的文本 segment 也持有同一批 chunk 字符串的引用，被复制的只有条目簿记。该取舍比描述中所说的更廉价。
两点次要观察（非阻塞）
-
resume接受合法的liveReplayMode却静默忽略。 非法值会在路由层 400，但/resume传liveReplayMode: 'summary'会返回200，随后被 bridge 强制为 full。这符合"仅 load 生效"的既定设计，且 SDK 在 resume 时从不发送该字段，仓内无任何影响——但第三方客户端若真的发了，会拿到完整回放且没有任何提示。在路由层直接拒绝会与非法值的处理更一致。 -
summary journal 并不会自动变小。 我这次运行中，单个主会话
available_commands_update帧就占了 374 KB 总量里的 368 KB。这不是回归（该帧在 base 上同样是主会话事件），但 summary 窗口的余量取决于主会话帧的大小，因此"排除嵌套详情"只在主会话帧本身不大时才是可观的收益。
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Approving. Reviewed the full diff at 438957d; the dual full/summary live-journal design is sound and the reduction does not silently drop anything summary consumers need.
Evidence:
- Summary projection mirrors the Web Shell's existing client-side discard (
projectMainTranscriptEvents): nestedsession_updateframes with non-empty_meta.parentToolCallIdare excluded, while root updates, all non-session events, and nestedagent_message_chunkusage frames (numeric input/outputTokens) are retained — exactly what the main transcript's usage totals consume. Persisted transcript pagination and completed-turn compaction are unchanged (fullTranscriptAvailable: true), and default staysfullfor SDK//acp/existing callers, so the interactive non-subagent path is untouched. - Sequence integrity holds after filtering: retained events keep original ids in order,
lastEventId/epochremain shared across modes, and truncation markers are tracked per journal with per-journal recordId anchors (summary anchor correctly ignores excluded nested recordIds, verified for bothseedandseedReplayEvents). - Restore coalescing now keys on
liveReplayMode(mismatched projections getRestoreInProgressErrorinstead of a wrong snapshot), invalid modes are rejected at both route (400invalid_live_replay_mode) and bridge layers before any side effects, resume is pinned tofull, and the registration-race path preserves the requested mode. - Both journals remain independently bounded by the existing
maxJournalEvents/maxJournalByteseviction policy; tests pin the reduction semantics end to end (independent limits, default-full compatibility, usage carve-out, marker anchors, validation/propagation at bridge/route/SDK/WebUI layers). - CI: all checks green on the head sha.
Non-blocking nits (agree with @wenshao's comments):
- P2
compactionEngine.ts: worst-case live-journal memory per session doubles (second journal at the same caps). Still bounded, but operators sizing frommaxJournalBytes × sessionsare now off by 2× — worth deriving summary caps as a fraction or documenting the 2× ceiling in the design doc /replayWindowLimits. - P2
isSummaryLiveJournalEvent: missing the normalizer's self-parent guard — a frame withparentToolCallId === toolCallId(which the daemon should never emit) renders as a root tool block in the UI but is excluded from the summary journal, so a mid-turn Web Shell refresh would drop it until turn completion. Mirroring the guard here keeps the two projections in sync. - P3 design doc: the summary-journal sentence omits the nested-usage carve-out that the implementation (and tests) deliberately keep; stating the exception prevents a future "simplification" from silently breaking subagent token accounting.
- P3: a valid
liveReplayMode: 'summary'onresumeis accepted by the route but silently coerced tofull; consider rejecting or documenting (WebUI/SDK never send it on resume today).
The restore-coalesce fence compared liveReplayMode strictly, so a WebUI summary load racing any full-mode load of the same session (keepalive revive, acp-http clients, older WebUI tabs) threw restore_in_progress instead of coalescing. The summary journal is a strict subset of the full journal, so a summary waiter can safely share an in-flight full restore and project the superset down. Only the reverse — a full request joining a summary restore — would return a missing-detail projection, so that direction stays fenced.
8c72bd2
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": none — all checks I started were completed within budget (~15 calls).; "You are review agent reverse-audit — Reverse audit agent…": did not read flattenReplaySegments /compacted-replay frame shapes in compactionEngine.ts to settle whether compacted turns retain raw user_message_chunk fram…; "You are review agent reverse-audit — Reverse audit agent…": none (~15 of 51 calls used).; "You are review agent reverse-audit — Reverse audit agent…": none — chunk fully read (un-truncated), all follow-up source checks completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget., and 4 more.
中文说明
未探索到全部深度（达到工具调用预算）："You are review agent reverse-audit — Reverse audit agent…"：none — all checks I started were completed within budget (~15 calls).；"You are review agent reverse-audit — Reverse audit agent…"：did not read flattenReplaySegments /compacted-replay frame shapes in compactionEngine.ts to settle whether compacted turns retain raw user_message_chunk fram…；"You are review agent reverse-audit — Reverse audit agent…"：none (~15 of 51 calls used).；"You are review agent reverse-audit — Reverse audit agent…"：none — chunk fully read (un-truncated), all follow-up source checks completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — all checks above completed within budget.，另有 4 条。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| historyPageSize !== inFlight.historyPageSize || | ||
| (inFlight.liveReplayMode === 'summary' && liveReplayMode === 'full') || | ||
| hideInheritedHistory !== inFlight.hideInheritedHistory |
There was a problem hiding this comment.
[Critical] R1-1: A summary load coalescing onto an in-flight full restore receives the owner's unprojected full-mode journal — the projection promised by this fence's comment ("its waiter receives a superset and projects it down") is implemented nowhere. The coalesced waiter returns { ...restored, attached: true, ... } (~line 5804), where restored = await inFlight.publicPromise is the owner's result built by replayFieldsFor(entry, action, liveReplayMode) (~line 6349) at the owner's 'full' mode. There is no isSummaryLiveJournalEvent/parentToolCallId filtering anywhere in bridge.ts, and neither the route, the SDK, nor WebUI re-projects liveJournal.
The superset premise is additionally false under truncation: this PR's own design-doc compatibility bullet says the two journals "track truncation independently", so root events evicted from the full journal but retained in the summary journal cannot be recovered from the full tail.
Failure scenario (probe-verified at this commit): a long-running turn floods the full journal past its 10,000-event / 8 MiB cap with nested subagent events, evicting early root events while the summary journal retains them marker-free; a full-mode load (SDK / /acp / a full-mode tab) is in flight; a Web Shell refresh issues its recovery load with liveReplayMode: 'summary', passes this fence, and coalesces → the waiter receives the full journal's history_truncated marker plus a retained tail dominated by nested events Web Shell discards at render — the user sees History truncated for live turn replay with early root content missing: the exact symptom this PR exists to eliminate. Probe at this commit: the coalesced waiter received [history_truncated, nested chunk, root event]; recomputing the replay fields from the registered entry in the waiter's mode flipped it to the marker-free summary view [root, root]. The paired test 'lets a summary load coalesce onto an in-flight full restore' emits no events and asserts only attached/call-count, so it stays green regardless.
Recommended fix — in the coalesce waiter branch, when the waiter's mode differs from inFlight.liveReplayMode, recompute the replay fields from the registered entry (mirroring the registration-race path at ~line 6252, which already does this):
return {
...restored,
attached: true,
clientId,
createdAt: entry.createdAt,
hasActivePrompt: entry.promptActive,
...replayFieldsFor(entry, action, liveReplayMode),
};and extend the coalesce test to ingest nested + root events under a small journal cap, asserting the waiter's liveJournal excludes nested frames and carries no history_truncated marker. A conservative one-click alternative is to fence the direction symmetrically (the waiter then gets RestoreInProgressError and attaches after settle):
| historyPageSize !== inFlight.historyPageSize || | |
| (inFlight.liveReplayMode === 'summary' && liveReplayMode === 'full') || | |
| hideInheritedHistory !== inFlight.hideInheritedHistory | |
| historyPageSize !== inFlight.historyPageSize || | |
| inFlight.liveReplayMode !== liveReplayMode || | |
| hideInheritedHistory !== inFlight.hideInheritedHistory |
中文说明
[Critical] R1-1：summary 加载合并（coalesce）到正在进行中的 full 恢复时，收到的是 owner 未经投影的 full 模式 journal —— 此处注释承诺的投影（“waiter 收到超集并向下投影”）在任何地方都没有实现。合并后的 waiter 直接返回 { ...restored, attached: true, ... }（约 5804 行），而 restored = await inFlight.publicPromise 是 owner 用 replayFieldsFor(entry, action, liveReplayMode)（约 6349 行）按 owner 自己的 'full' 模式构建的结果。bridge.ts 中不存在任何 isSummaryLiveJournalEvent/parentToolCallId 过滤，route、SDK、WebUI 也都不会再次投影 liveJournal。
超集前提在发生截断时同样不成立：本 PR 自己的设计文档兼容性条目写明两种 journal “独立跟踪截断”，因此被 full journal 逐出、但 summary journal 仍保留的根事件无法从 full 尾部恢复。
失败场景（已在本提交用探针验证）：长时间运行的一轮让嵌套 subagent 事件冲垮 full journal 的 10,000 事件 / 8 MiB 上限，逐出早期根事件，而 summary journal 完整保留且无截断标记；此时一个 full 模式加载（SDK / /acp / full 模式标签页）正在进行；Web Shell 刷新发起带 liveReplayMode: 'summary' 的恢复加载，通过该栅栏并合并 → waiter 收到 full journal 的 history_truncated 标记和一段以嵌套事件为主的尾部（这些嵌套事件 Web Shell 渲染时会丢弃）—— 用户看到 History truncated for live turn replay 且早期根内容缺失：正是本 PR 要消除的症状。探针显示：合并后的 waiter 收到 [history_truncated, 嵌套 chunk, 根事件]；改为按 waiter 模式从已注册 entry 重新计算 replay 字段后，变为无标记的 summary 视图 [根, 根]。配套测试 'lets a summary load coalesce onto an in-flight full restore' 不注入任何事件、只断言 attached/调用次数，因此无论如何都是绿的。
推荐修复 —— 在合并 waiter 分支中，当 waiter 模式与 inFlight.liveReplayMode 不同时，从已注册 entry 重新计算 replay 字段（与约 6252 行的注册竞争路径一致，该路径已经这样做了），见上方英文代码；并扩展合并测试：在小 journal 上限下注入嵌套 + 根事件，断言 waiter 的 liveJournal 不含嵌套帧且无 history_truncated 标记。保守的一键替代方案是对称地栅栏该方向（waiter 收到 RestoreInProgressError，待 settle 后再附加）：见上方 suggestion 代码块。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| if (extractParentToolCallIdFromMeta(meta) === undefined) return true; | ||
| if (data?.update?.sessionUpdate !== 'agent_message_chunk') return false; |
There was a problem hiding this comment.
[Suggestion] R1-2: This exclusion branch for parented non-agent_message_chunk session updates (nested tool_call/tool_call_update frames) is exercised by no test — every nested event in the new engine and bridge tests is an agent_message_chunk (makeTextChunkWithParent).
Concrete cost: mutating the return false here to return true turns no test red (engine + eventBus suites 168/168, bridge summary tests 6/6), while the sibling usage-gate mutation fails 7 tests — the suites detect filter changes, they just never feed a parented non-chunk event into a summary assertion. A future regression retaining nested tool frames in the summary journal would ship green and let a long-running subagent's parented tool_call_update flood consume the summary journal's caps — reintroducing exactly the truncation this feature was written to prevent.
Suggested fix: add an engine test that ingests parented tool_call / tool_call_update frames between root events under a small maxJournalEvents, asserting snapshot('summary') excludes them while snapshot() retains them.
中文说明
[Suggestion] R1-2：针对带 parent 的非 agent_message_chunk 会话更新（嵌套 tool_call/tool_call_update 帧）的这条排除分支没有任何测试覆盖 —— 新增的 engine 与 bridge 测试中所有嵌套事件都是 agent_message_chunk（makeTextChunkWithParent）。
具体代价：把这里的 return false 突变为 return true 不会让任何测试变红（engine + eventBus 套件 168/168 通过，bridge summary 测试 6/6 通过），而相邻 usage 门限的突变会让 7 个测试失败 —— 说明套件能捕获过滤器变更，只是从未把带 parent 的非 chunk 事件喂进 summary 断言。未来若有回归让嵌套 tool 帧保留进 summary journal，将会一路绿灯地发布，使长时间运行 subagent 的嵌套 tool_call_update 洪流耗尽 summary journal 上限 —— 重新引入本功能要防止的截断。
建议修复：新增一个 engine 测试，在较小的 maxJournalEvents 下，于根事件之间注入带 parent 的 tool_call / tool_call_update 帧，断言 snapshot('summary') 排除它们而 snapshot() 保留它们。
— qwen3.8-max via Qwen Code /review (v0.21.11)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 2878 passed · 0 failed · 2878 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：2878 通过 · 0 失败 · 2878 总计 Verification reportPR 9057 verification — fix(daemon): reduce subagent live replay journalVerdict: 中文摘要
Central claim + A/BCentral claim: a Web Shell refresh during a verbose subagent turn no longer returns the truncated live journal — summary loads get an independently bounded projection that excludes nested subagent detail while retaining root events, subagent lifecycle/tool state, and nested usage frames; default loads keep the complete journal. Harness
Witnesses: Secondary claims, each with its own scripted probe:
Mutation matrix (vacuity + guards)All mutations applied in scratch edits of the head source, targeted vitest run, then reverted (
No survivors: every guard the PR introduces is pinned by a test that fails for the intended reason, and the positive control proves the harness can make the suite red. Boundary probe (filter siblings)
Targeted gates
FindingsF1 (low, non-blocking) — a summary load that coalesces onto an in-flight full restore inherits the owner's full replay fields, including a live-journal truncation marker if the full journal truncated during that restore. The coalesce branch ( F2 (informational) — usage retention applies only to nested No injection attempts observed in PR text; author claims were treated as hypotheses and each was exercised above. Not covered
MethodologyEnvironment: Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review. The bot already has a review of its own on 机器人在 The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
ytahdn
added
the
autofix/takeover
|
@qwen-code /resolve |
Merge origin/main — adaptive live-journal growth (#8905) — into the summary live-replay branch: the growth advisor now operates on the shared journal state object and measures retention against whichever journal (full or summary) breaches its caps. Review feedback addressed: - Critical R1-1: a summary load that coalesces onto an in-flight full restore recomputes its own-mode replay fields from the registered entry instead of inheriting the owner's unprojected full journal, which could carry nested frames and a history_truncated marker the summary journal never earned. Extended the coalesce test to flood a capped journal and pin the waiter's projected, marker-free view. - Mirror the UI normalizer's self-reference guard in the summary journal filter: a tool frame whose parentToolCallId equals its own toolCallId renders as a root block live, so it must survive a mid-turn summary refresh. - R1-2: cover the parented non-chunk exclusion branch (nested tool_call/tool_call_update frames) with a capped engine test; the branch previously had no test and the reviewer's mutation survived. - Document the usage carve-out exceptions in the design doc (it contradicted the implementation), and document the 2x per-session journal memory ceiling (full + summary share one cap pair) in replayWindowLimits, the engine options, and daemon diagnostics.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #9057Merged Conflict resolution (
|
|
Qwen Code did not run conflict resolution for this request. PR #9057 does not currently have merge conflicts with main. |
|
🤝 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 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks I started completed within budget..
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："You are review agent reverse-audit — Reverse audit agent…"：none — all checks above completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — all checks I started completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| private resetJournal(): void { | ||
| this.liveJournal = []; | ||
| this.journalEntryBytes = []; | ||
| this.journalEntryEvents = []; | ||
| this.journalTotalBytes = 0; | ||
| this.journalTotalEvents = 0; | ||
| this.journalTruncatedEvents = 0; | ||
| this.liveJournalTextSegment = undefined; | ||
| this.fullJournal = createLiveJournalState(); | ||
| this.summaryJournal = createLiveJournalState(); |
There was a problem hiding this comment.
[Suggestion] R2-1: the summary-journal reset at the turn boundary has no test fence. Probe-verified mutation: removing this.summaryJournal = createLiveJournalState(); from resetJournal() passes the whole compactionEngine + bridge suites (130 + 594 tests green).
Failure scenario: compactCurrentTurn calls resetJournal() on turn_complete/turn_error. If a future edit resets only fullJournal, every current test stays green, and a later summary-mode session/load mid-turn N+1 returns turns 1..N's root frames in both compactedReplay and liveJournal — duplicated prior-turn content in the restored transcript — plus stale truncatedEvents/summaryRecordId leaking into a marker.
Suggested fix — in keeps an independent summary journal without nested subagent updates, after engine.ingest(makeTurnComplete(102)):
expect(engine.snapshot('summary').liveJournal).toEqual([]);中文说明
[Suggestion] R2-1：回合边界处 summary journal 的重置没有测试保护。已用探针验证变异：从 resetJournal() 中删除 this.summaryJournal = createLiveJournalState(); 后，compactionEngine 与 bridge 全部测试（130 + 594 个）仍然通过。
失败场景： compactCurrentTurn 会在 turn_complete/turn_error 时调用 resetJournal()。若未来某次修改只重置 fullJournal，现有测试会全部保持绿色，而之后在第 N+1 轮进行中发起的 summary 模式 session/load，会在 compactedReplay 和 liveJournal 中同时返回第 1..N 轮的根帧——恢复出的转录出现重复的上一轮内容——并且过期的 truncatedEvents/summaryRecordId 会泄漏进截断标记。
建议修复——在 keeps an independent summary journal without nested subagent updates 测试中，engine.ingest(makeTurnComplete(102)) 之后补充上述断言。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| historyPageSize !== inFlight.historyPageSize || | ||
| (inFlight.liveReplayMode === 'summary' && liveReplayMode === 'full') || | ||
| hideInheritedHistory !== inFlight.hideInheritedHistory |
There was a problem hiding this comment.
[Suggestion] R2-2: same-mode summary→summary coalescing is untested. The new tests cover summary-joins-full and the full-joins-summary fence, and pre-existing tests cover full→full, but no test has a summary waiter join an in-flight summary restore. Probe-verified mutation: simplifying this fence condition to inFlight.liveReplayMode === 'summary' (fencing every summary-owned restore) passes the entire bridge suite.
Failure scenario: under that mutation, a second concurrent summary-mode load — e.g. a Web Shell reconnect racing another summary client restoring the same unregistered session — fails with RestoreInProgressError instead of coalescing, and nothing in CI turns red.
Suggested fix: add a bridge test with two loadSession({ liveReplayMode: 'summary' }) calls on the same session while the first restore is in flight; expect one agent loadSession call and both responses carrying the marker-free summary projection.
中文说明
[Suggestion] R2-2：summary→summary 同模式合并（coalescing）没有测试覆盖。新增测试覆盖了 summary 并入 full、以及 full 并入 summary 被栅栏拦截两种情况，既有测试覆盖 full→full，但没有测试让一个 summary waiter 并入正在进行中的 summary 恢复。已用探针验证变异：把该栅栏条件简化为 inFlight.liveReplayMode === 'summary'（栅栏拦截所有 summary 持有的恢复）后，bridge 全部测试仍然通过。
失败场景： 在该变异下，第二个并发的 summary 模式加载——例如 Web Shell 重连与另一个正在恢复同一未注册会话的 summary 客户端竞争——会收到 RestoreInProgressError 而不是合并，且 CI 中没有任何测试变红。
建议修复：新增一个 bridge 测试，在同一会话的首个恢复仍在进行时发起两个 loadSession({ liveReplayMode: 'summary' }) 调用；断言 agent 的 loadSession 只被调用一次，且两个响应都携带无截断标记的 summary 投影。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| * Memory-ceiling note: one in-flight turn retains TWO journals that share | ||
| * these caps — the `full` journal plus the `summary` projection for | ||
| * summary-mode loads — so a session's live-journal heap is bounded by | ||
| * twice the effective cap (2x the baseline, or 2x the grown cap under | ||
| * adaptive growth), not one. Operators sizing daemon memory from | ||
| * `maxJournalBytes x live sessions` must double the journal term. |
There was a problem hiding this comment.
[Suggestion] R2-4: the adaptive-growth pool still charges one journal's worth of growth per session, but this diff makes the worst-case live-journal heap two journals × the granted cap. Probe at this commit: with maxJournalBytes=65536 on a root-only stream, one session retains full=65449 + summary=65449 bytes — ratio 2.00 — while the pool charges one cap of growth.
Failure scenario: journalGrowthPolicy.grant() charges Σ(limitBytes − baselineBytes) per session. After this diff a fully-grown session retains ~512 MiB while charged ~248 MiB (hard cap 256 MiB × 2); with grants summing to pool P, daemon journal heap can reach ~2P + 2×baseline×sessions against a budget modeled for P + baseline×sessions — silently exceeding the operator's memory budget. The operator-facing note above covers the sizing, but the pool's own rationale ("still cover[s] several fully-grown sessions") is now overstated by 2× and is not updated.
Suggested fix: charge the pool for the dual-journal reality (double the charged limitBytes, or halve the effective pool), or explicitly state the accepted 2× over-commit in the growth-policy docstring.
中文说明
[Suggestion] R2-4：自适应增长池仍按每会话一个 journal 的增长计费，但本 diff 使最坏情况下的 live-journal 堆达到两个 journal × 授予上限。已在本提交用探针验证：maxJournalBytes=65536、纯根事件流下，单个会话保留 full=65449 + summary=65449 字节——比值 2.00——而增长池只按一个上限计费。
失败场景： journalGrowthPolicy.grant() 按每会话 Σ(limitBytes − baselineBytes) 计费。本 diff 之后，一个完全增长的会话实际保留约 512 MiB，却只按约 248 MiB 计费（硬上限 256 MiB × 2）；若授予总和为池 P，daemon 的 journal 堆可达约 2P + 2×baseline×sessions，而预算模型只按 P + baseline×sessions 建模——从而悄无声息地超出运维的内存预算。上方面向运维的注释已覆盖容量估算，但增长池自身的依据（"仍能覆盖数个完全增长的会话"）现在被夸大了 2 倍且未更新。
建议修复：让增长池按双 journal 的现实计费（把计费的 limitBytes 翻倍，或把有效池减半），或在增长策略的 docstring 中明确写明接受 2× 超额承诺。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const waiterReplayFields = | ||
| liveReplayMode !== inFlight.liveReplayMode | ||
| ? historyPageSize !== undefined | ||
| ? await refreshedReplayFieldsFor( |
There was a problem hiding this comment.
[Suggestion] R2-6: the refreshedReplayFieldsFor arm of this coalesced-waiter recompute (mixed-mode waiter that requested a historyPageSize) is never exercised by any test — the mixed-mode coalescing tests all omit historyPageSize, so every coalescing waiter takes the replayFieldsFor arm. Probe-verified mutation: swapping the third argument to inFlight.liveReplayMode (the owner's mode) passes the entire bridge suite. The current code is correct; only the regression protection is missing.
Failure scenario: under that mutation, a summary client coalescing onto an in-flight full restore with historyReplay: 'response' + historyPageSize (whose settle fallback fires) would silently receive the owner's full-mode projection — nested subagent frames plus the full journal's history_truncated marker. The same call shape IS tested on the existing-entry attach path (bridge.test.ts:5126-5132); only this waiter-side wiring is unguarded.
Suggested fix: add a coalescing test where owner and waiter both pass historyReplay: 'response' with the same historyPageSize but different liveReplayMode (with a prompt active at settlement so the fallback fires), and assert the waiter's journal is summary-projected.
中文说明
[Suggestion] R2-6：合并 waiter 重计算中的 refreshedReplayFieldsFor 分支（请求了 historyPageSize 的混合模式 waiter）没有任何测试覆盖——混合模式合并测试都未传 historyPageSize，因此每个合并 waiter 都走 replayFieldsFor 分支。已用探针验证变异：把第三个参数换成 inFlight.liveReplayMode（owner 的模式）后，bridge 全部测试仍然通过。当前代码是正确的，缺的只是回归保护。
失败场景： 在该变异下，一个以 historyReplay: 'response' + historyPageSize 并入进行中 full 恢复的 summary 客户端（settle 回退被触发时）会悄悄收到 owner 的 full 模式投影——嵌套 subagent 帧外加 full journal 的 history_truncated 标记。同样的调用形态在"已存在 entry 的 attach 路径"上有测试（bridge.test.ts:5126-5132），只有 waiter 侧这处接线没有保护。
建议修复：新增合并测试，owner 与 waiter 都传 historyReplay: 'response' 和相同的 historyPageSize、但 liveReplayMode 不同（settle 时有活跃 prompt 以触发回退），断言 waiter 的 journal 是 summary 投影。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| load.resolve({}); | ||
| await first; |
There was a problem hiding this comment.
[Suggestion] R2-8: the fresh-restore owner path (replayFieldsFor(entry, action, liveReplayMode) at bridge.ts:6466) is the only summary-mode replay-fields site with zero journal assertions. The three sibling sites (existing-entry attach, race-loss, coalesced waiter) each have a test; the only test driving a summary load through the owner IIFE — rejects a full load joining an in-flight summary restore — awaits first (this line) but asserts nothing about its liveJournal. Probe-verified mutation: dropping the new liveReplayMode argument at that site passes the entire suite, and the mutant owner returns the unprojected full journal including the full journal's history_truncated marker.
Failure scenario: if that call regressed to replayFieldsFor(entry, action), every fresh session/load with liveReplayMode: 'summary' — the primary WebUI/Web Shell case of opening a session with an in-flight turn — would return the unprojected full journal: the exact visible defect this PR exists to fix, silently regressable on its most common path.
Suggested fix: in that test (or a dedicated one), flood the in-flight summary restore with root + nested frames past a small cap, then assert on first: liveJournal contains the root frames, no _meta.parentToolCallId frame, and no history_truncated marker.
中文说明
[Suggestion] R2-8：全新恢复的 owner 路径（bridge.ts:6466 处的 replayFieldsFor(entry, action, liveReplayMode)）是唯一一个对 summary 模式回放字段零断言的位置。其余三处（已存在 entry 的 attach、竞争失败、合并 waiter）各有测试；唯一驱动 summary 加载走过 owner IIFE 的测试——rejects a full load joining an in-flight summary restore——会 await first（此行），但对其 liveJournal 不作任何断言。已用探针验证变异：删除该处的新参数 liveReplayMode 后整个测试套件仍然通过，变异后的 owner 会返回未投影的 full journal（连同 full journal 的 history_truncated 标记）。
失败场景： 若该调用退化为 replayFieldsFor(entry, action)，所有带 liveReplayMode: 'summary' 的全新 session/load——WebUI/Web Shell 打开一个进行中会话的最主要场景——都会返回未投影的 full journal：正是本 PR 要修复的可见缺陷，且可在其最常见路径上悄无声息地回归。
建议修复：在该测试（或专门新增的测试）中，用根帧 + 嵌套帧淹没一个小的 journal 上限，然后对 first 断言：liveJournal 包含根帧、不含任何 _meta.parentToolCallId 帧、且无 history_truncated 标记。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| if ( | ||
| this.retainedTailCount(grant.maxEvents, grant.maxBytes) > | ||
| this.retainedTailCount(journal, grant.maxEvents, grant.maxBytes) > | ||
| originalRetained | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R2-9: the journal-parameterized growth arm (maybeGrowJournalLimits(journal) / retainedTailCount(journal, …)) is never exercised with a summaryJournal that diverges from fullJournal — every growth test ingests only root (summary-eligible) events, so both journals are content-identical in all of them. Probe-verified mutation: substituting this.fullJournal for the journal parameter at the call site, the originalRetained computation, and the grant-acceptance check passes the whole suite (1381 tests).
Failure scenario: under that mutation, growth-acceptance decisions under summary-only cap pressure are computed from the wrong journal: grants get accepted (and the shared pool charged) without relieving the pressured journal, or refused despite available pool, over-evicting the root frames a summary-mode load depends on. The probe observed the mutant re-ask, roll back, and evict the root frame.
Suggested fix: add a growth test where the journals diverge under pressure (interleaved non-summary entries plus an advisor that grants); assert snapshot('summary') retains the summary tail under the grown cap, and/or that the advisor was consulted from the summary journal's breach.
中文说明
[Suggestion] R2-9：journal 参数化的增长分支（maybeGrowJournalLimits(journal) / retainedTailCount(journal, …)）从未用与 fullJournal 内容不同的 summaryJournal 验证过——所有增长测试只注入根（summary 可见）事件，因此两个 journal 在这些测试中内容完全一致。已用探针验证变异：在调用点、originalRetained 计算与授予接受检查三处把 journal 参数替换为 this.fullJournal 后，整个套件（1381 个测试）仍然通过。
失败场景： 在该变异下，summary 单独触顶时的增长接受决策会用错 journal 计算：授予被接受（共享池被计费）却没有缓解触顶的 journal，或有池余量却被拒绝，从而过度逐出 summary 模式加载所依赖的根帧。探针观察到变异体重新申请、回滚、并逐出了根帧。
建议修复：新增一个两个 journal 在压力下分化的增长测试（交错注入非 summary 条目 + 一个会授予的顾问），断言 snapshot('summary') 在增长后的上限下保留 summary 尾部，和/或顾问确实因 summary journal 触顶而被咨询。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const summary = await bridge.loadSession({ | ||
| sessionId: session.sessionId, | ||
| workspaceCwd: WS_A, | ||
| historyReplay: 'response', | ||
| historyPageSize: 100, | ||
| liveReplayMode: 'summary', | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R2-10: this test's summary-projection assertions ride entirely on refreshedReplayFieldsFor's error-fallback branch. With historyPageSize: 100 on a registered session, the load takes the refreshedReplayFieldsFor path, where the default FakeAgent.extMethod returns a malformed {} for the transcript page — page.events.length throws, the catch fires, and the journal comes from the replayFieldsFor fallback; no assertion pins which branch runs (probe-replicated at this commit).
Failure scenario: if someone later gives FakeAgent a spec-shaped default transcript response (e.g. { events: [], hasMore: false } — a plausible test-infra improvement), the success branch returns liveJournal: [] for this idle session and this test fails with [] !== [1, 3] despite zero production change — a false regression signal that will be debugged as a summary-projection break. Conversely, the success branch has no summary-mode coverage today.
Suggested fix: give this test's channel an explicit extMethodImpl that deterministically throws for the transcript ext-method (making the fallback the deliberate target), or return a well-formed page and assert the success-branch shape.
中文说明
[Suggestion] R2-10：该测试对 summary 投影的断言完全依赖 refreshedReplayFieldsFor 的错误回退分支。对已注册会话传 historyPageSize: 100 时，加载走 refreshedReplayFieldsFor 路径，而默认的 FakeAgent.extMethod 对转录分页请求返回畸形 {}——page.events.length 抛错、catch 触发，journal 实际来自 replayFieldsFor 回退；没有任何断言钉住走的是哪个分支（已在本提交用探针复现）。
失败场景： 若以后有人给 FakeAgent 一个符合规范的默认转录响应（例如 { events: [], hasMore: false }——一个合理的测试基建改进），成功分支会对这个空闲会话返回 liveJournal: []，该测试将在零生产代码变更的情况下以 [] !== [1, 3] 失败——一个会被当作 summary 投影破坏来排查的假回归信号。反过来，成功分支目前也完全没有 summary 模式覆盖。
建议修复：给该测试的 channel 一个显式 extMethodImpl，对转录 ext-method 确定性地抛错（让回退成为有意目标），或者返回规范的分页响应并断言成功分支的形态。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| ...(action === 'load' && req.liveReplayMode !== undefined | ||
| ? { liveReplayMode: req.liveReplayMode } | ||
| : {}), |
There was a problem hiding this comment.
[Suggestion] R2-14: the SDK's load-only wire gate for liveReplayMode has no test fence. Probe-verified mutation: deleting action === 'load' && passes 329/329 DaemonClient tests, and a probe observed liveReplayMode on the resume wire body under the mutation (absent on unmodified code).
Failure scenario: all SDK resume tests pass no or empty options, and the provider's resume-negative assertion mocks above this layer, so a refactor dropping the guard silently puts liveReplayMode on resume wire bodies, breaking the documented Load-only contract. Impact is bounded while the route (forwards on load only) and bridge (coerces resume to full) hold — but if the gate rotted, an untyped caller passing an invalid value to resume would additionally turn a succeeding resume into a route-level 400.
Suggested fix:
await client.resumeSession('s-1', { workspaceCwd: '/w', liveReplayMode: 'summary' });
// assert the recorded body has no `liveReplayMode` key中文说明
[Suggestion] R2-14：SDK 中 liveReplayMode 的"仅 load"线上栅栏没有测试保护。已用探针验证变异：删除 action === 'load' && 后 DaemonClient 329/329 测试全部通过，且探针观察到变异下 resume 的请求体携带 liveReplayMode（未变异代码上不携带）。
失败场景： SDK 的所有 resume 测试都不传或只传空 options，而 provider 的 resume 负向断言在高于此层的 mock 上进行，因此删除该守卫的重构会悄悄把 liveReplayMode 放进 resume 的请求体，破坏文档写明的"仅 load"契约。在路由（只在 load 转发）与 bridge（resume 强制为 full）保持不变的情况下影响有限——但若该栅栏腐化，未类型化的调用方向 resume 传入非法值还会额外把一次本可成功的 resume 变成路由层 400。
建议修复：见上方英文代码——调用 resumeSession 并传入 liveReplayMode: 'summary'，断言记录的请求体不含 liveReplayMode 键。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const liveReplayMode = parseLiveReplayMode(body ?? {}, res); | ||
| if (liveReplayMode === null) return; |
There was a problem hiding this comment.
[Suggestion] R2-15: the route validates liveReplayMode on both actions, but only the load-forwarding side is tested. Probe-verified mutation: adding ...(liveReplayMode !== undefined ? { liveReplayMode } : {}) to the resumeSession options passes the entire 936-test server suite, and a probe observed the field reaching bridge.resumeCalls under the mutation.
Failure scenario: no test POSTs a valid mode to /resume and inspects resumeCalls; BridgeRestoreSessionRequest.liveReplayMode? makes the mutation typecheck. The route's load-only forwarding contract is unfenced at the route layer and depends entirely on the bridge's coercion to full staying intact.
Suggested fix: add a CLI test: POST /session/:id/resume with { liveReplayMode: 'summary' }, expect 200 and bridge.resumeCalls[0] without a liveReplayMode key.
中文说明
[Suggestion] R2-15：路由在两种 action 上都校验 liveReplayMode，但只有 load 转发一侧有测试。已用探针验证变异：向 resumeSession 的 options 加入 ...(liveReplayMode !== undefined ? { liveReplayMode } : {}) 后，server 套件 936 个测试全部通过，且探针观察到变异下该字段进入了 bridge.resumeCalls。
失败场景： 没有测试向 /resume POST 合法模式并检查 resumeCalls；BridgeRestoreSessionRequest.liveReplayMode? 使该变异通过类型检查。路由"仅 load 转发"的契约在路由层没有保护，完全依赖 bridge 将其强制为 full 这一行为保持不变。
建议修复：新增 CLI 测试：向 /session/:id/resume POST { liveReplayMode: 'summary' }，期望 200 且 bridge.resumeCalls[0] 不含 liveReplayMode 键。
— qwen3.8-max via Qwen Code /review (v0.21.11)
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback round — PR #9057All nine inline findings are resolved in code. Eight were probe-verified missing test fences; each now has a regression test that was mutation-probed locally (the exact mutant described in the finding makes the new test fail). One (R2-4) was an overstated sizing rationale; fixed in the two growth-policy docstrings (documentation option from the suggestion — changing pool charging semantics would be an operator-visible behavior change beyond this PR's scope). No conflict resolution was needed (
All seven mutation probes were run against the new/extended tests locally: every probe KILLED its mutant (test failed under mutation) and the tree was restored afterward. Verification
中文说明审查反馈轮次 — PR #9057九条行内发现全部已在代码中解决。其中八条是经探针验证的缺失测试保护（test fence），现已各自补上回归测试，并在本地做了变异探针验证（应用发现中描述的变异后新测试会失败）。一条（R2-4）是容量说明被夸大，已在两处增长策略 docstring 中修正（采用建议中的"文档化"选项——修改增长池计费语义属于超出本 PR 范围的运维可见行为变更）。无需解决冲突（
七个变异探针均在本地对新增/扩展测试运行：每个探针都杀死了对应变异（变异下测试失败），随后工作树已恢复原状。 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 1: none — all checks I started completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all planned checks completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks I started were completed within budget (~15 tool calls used).; "You are review agent reverse-audit — Reverse audit agent…": none — the chunk walk is complete; the compactionEngine.test.ts and bridge.ts hunks belong to other chunks and were left to their agents (I only read engine sou…; "You are review agent reverse-audit — Reverse audit agent…": none — I did not run the webui vitest suite, but that is not a check I left unfinished; my audit was static call-graph/semantics verification, and test presence…, and 2 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 1：none — all checks I started completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — all planned checks completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — all checks I started were completed within budget (~15 tool calls used).；"You are review agent reverse-audit — Reverse audit agent…"：none — the chunk walk is complete; the compactionEngine.test.ts and bridge.ts hunks belong to other chunks and were left to their agents (I only read engine sou…；"You are review agent reverse-audit — Reverse audit agent…"：none — I did not run the webui vitest suite, but that is not a check I left unfinished; my audit was static call-graph/semantics verification, and test presence…，另有 2 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const waiterReplayFields = | ||
| liveReplayMode !== inFlight.liveReplayMode | ||
| ? historyPageSize !== undefined |
There was a problem hiding this comment.
[Critical] R3-7: The diff-added coalesce-waiter recompute awaits refreshedReplayFieldsFor AFTER registerClient/recordAttachRef (~5879-5882), with no post-await assertAttachableSessionEntry re-assert and no rollback — unlike every sibling attach path: the existing-entry attach re-asserts after its equivalent await (~5754), and this branch's own approval-mode block re-asserts and rolls back after its await (~5889-5899). refreshedReplayFieldsFor swallows every fetch error into the in-memory fallback, so a channel death during the await is converted into success.
Failure scenario (probe-verified at this commit): a Web Shell summary-mode refresh loads with { liveReplayMode: 'summary', historyPageSize } (the exact production shape) and coalesces onto an in-flight full restore; after settle + registration, the agent channel dies (crash / killSession) during the transcript-page await. The probe observed the waiter RESOLVE with attached: true and a clientId for a session the daemon had already torn down (getSessionSummary already threw SessionNotFoundError) — the client enters a zombie session whose next operation fails confusingly, instead of getting the retryable SessionNotFoundError every sibling path produces. The merge-base coalesce branch has no post-registration await, so this race window is introduced by this diff. Flip check: adding the re-assert + rollback makes the probe reject with SessionNotFoundError.
Suggested fix (flip-checked):
const waiterReplayFields = /* ... unchanged ... */;
try {
assertAttachableSessionEntry(restored.sessionId, entry);
} catch (error) {
await rollbackAttachRegistration(entry, clientId);
throw error;
}— mirror the approval-mode block directly above, or move the recompute ahead of registration as the existing-entry attach path does.
中文说明
[Critical] R3-7：本 diff 新增的合并 waiter 重投影在 registerClient/recordAttachRef（约 5879-5882 行）之后 await refreshedReplayFieldsFor，但该 await 之后既没有 assertAttachableSessionEntry 复查，也没有回滚 —— 而所有兄弟 attach 路径都有：existing-entry attach 在等价 await 之后复查（约 5754 行）；本分支自己的 approval-mode 块在 await 之后复查并回滚（约 5889-5899 行）。refreshedReplayFieldsFor 会把所有抓取错误吞进内存回放回退，因此通道在该 await 期间死亡会被转换成“成功”。
失败场景（已在本提交用探针验证）：Web Shell summary 模式刷新以 { liveReplayMode: 'summary', historyPageSize }（生产真实形状）加载，并合并到进行中的 full 恢复；settle 并完成注册后，agent 通道在 transcript 分页 await 期间死亡（崩溃 / killSession）。探针观察到 waiter 成功返回 attached: true 和一个 clientId，而该会话已被 daemon 拆除（getSessionSummary 已抛出 SessionNotFoundError）——客户端进入僵尸会话，下一次操作才以令人困惑的方式失败，而不是像所有兄弟路径那样得到可重试的 SessionNotFoundError。merge-base 的合并 waiter 分支在注册后没有任何 await，该竞态窗口由本 diff 引入。翻转验证：补上复查 + 回滚后，探针翻转为抛出 SessionNotFoundError。
修复建议（已通过翻转验证）：在 waiterReplayFields await 之后镜像上方 approval-mode 块的写法 —— 复查 assertAttachableSessionEntry(restored.sessionId, entry)，失败时先 await rollbackAttachRegistration(entry, clientId) 再抛出；或把重投影挪到注册之前（与 existing-entry attach 路径一致）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // exception is liveReplayMode: the summary journal is a strict subset | ||
| // of the full journal, so a summary request safely shares an in-flight | ||
| // full restore (the waiter recomputes its own mode's replay fields |
There was a problem hiding this comment.
[Suggestion] R3-2: The fence comment's safety rationale — "the summary journal is a strict subset of the full journal" — is false under cap pressure. The two journals evict independently against the shared caps, so the summary journal can retain root frames the full journal evicted. Probe-verified by running this PR's own excludes parented tool frames from the summary journal under cap pressure test: the full journal retains ids [5, 6] while the summary journal retains [1, 6] — entry 1 survives only in the summary journal. Coalescing is actually safe solely because the waiter recomputes its own mode's replay fields from the registered entry (this comment's parenthetical), not because of subset-ness.
Failure scenario: a maintainer trusting the subset claim could "simplify" the waiter path to derive summary fields by filtering the owner's full journal — under cap pressure that filter would drop retained root frames and stamp the full journal's history_truncated marker onto a summary client whose own journal never truncated: the exact defect shape the adjacent coalesce tests fence (and the one R1-1 originally blocked). The same false claim also appears in docs/design/live-journal-truncation-recovery.md ("Protocol and SDK" section — where it contradicts the same doc's "track truncation independently" bullet three lines away) and in the bridge.test.ts comment above the summary-onto-full coalesce test; please fix all three copies.
Suggested wording: the journals can diverge under cap pressure (each evicts independently against the shared caps), so once the restore settles the daemon recomputes the waiter's replay fields for its own mode from the registered entry — the owner's projected fields can never be reused or filtered down for a waiter of a different mode.
中文说明
[Suggestion] R3-2：栅栏注释中的安全性理由 ——“summary journal 是 full journal 的严格子集”—— 在上限压力下不成立。两个 journal 在同一组上限下独立逐出，因此 summary journal 可能保留 full journal 已逐出的根帧。用本 PR 自己的测试 excludes parented tool frames from the summary journal under cap pressure 运行验证：full journal 保留 id [5, 6]，而 summary journal 保留 [1, 6] —— 条目 1 只存在于 summary journal。合并真正安全的原因仅是 waiter 会从已注册 entry 按自己的模式重新计算回放字段（即本注释括号内的说明），而不是什么子集关系。
失败场景：维护者若相信该子集断言，可能把 waiter 路径“简化”为对 owner 的 full journal 做过滤来得到 summary 字段 —— 在上限压力下，这种过滤会丢掉仍被保留的根帧，并把 full journal 的 history_truncated 标记盖到一个自身 journal 从未截断的 summary 客户端上：正是相邻合并测试所钉住的缺陷形态（也是 R1-1 最初阻塞的问题）。同一错误断言还出现在 docs/design/live-journal-truncation-recovery.md（“Protocol and SDK” 一节 —— 与同一文档三行之外“独立跟踪截断”的表述自相矛盾）以及 bridge.test.ts 中 summary-onto-full 合并测试上方的注释里；请一并修正这三处。
建议措辞：两个 journal 在上限压力下可能分化（各自对共享上限独立逐出），因此恢复 settle 后由 daemon 按 waiter 自己的模式从已注册 entry 重新计算回放字段 —— owner 已投影的字段永远不能被复用或向下过滤给不同模式的 waiter。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // The summary journal is a strict subset of the full journal, so the | ||
| // summary waiter shares the in-flight full restore; once the restore | ||
| // settles, the bridge recomputes the waiter's own-mode replay fields | ||
| // from the registered entry. |
There was a problem hiding this comment.
[Suggestion] R3-2 (instance 3 of 3): This comment states the same false invariant as the bridge.ts fence comment — "the summary journal is a strict subset of the full journal" — directly above assertions whose own observations disprove it: the test floods five events past maxJournalEvents: 2, then asserts the full-mode owner's journal carries the history_truncated marker (early root frame evicted) while the coalesced summary waiter retains both root frames marker-free — a retained entry the full journal does not have. Under cap pressure the summary journal is not a subset; the safety comes from the recompute the comment's second sentence describes. Suggest rewording as proposed in the bridge.ts thread so the comment does not assert the opposite of the code below it.
中文说明
[Suggestion] R3-2（三处中的第三处）：此注释与 bridge.ts 栅栏注释陈述了同一个错误不变量 ——“summary journal 是 full journal 的严格子集”—— 而紧接其下的断言恰好证明了其反面：该测试在 maxJournalEvents: 2 下注入五个事件，随后断言 full 模式 owner 的 journal 带有 history_truncated 标记（早期根帧被逐出），而合并后的 summary waiter 无标记地保留了两个根帧 —— 一个 full journal 中已不存在的保留条目。上限压力下 summary journal 并非子集；安全性来自注释第二句描述的重投影。建议按 bridge.ts 线程中的措辞修改，使注释不再断言与其下方代码相反的结论。
— qwen3.8-max via Qwen Code /review (v0.21.11)
…th (#9057) Review feedback addressed: - Critical R3-7: the coalesced waiter's replay-field recompute awaited refreshedReplayFieldsFor after registerClient/recordAttachRef with no post-await re-assert or rollback. refreshedReplayFieldsFor swallows every fetch error into the in-memory fallback, so a channel death during the transcript-page await attached the waiter to a session the daemon had already torn down. Move the recompute ahead of registration — the existing-entry attach path's shape — and re-assert after the await, rolling back the pre-folded coalesce reservation on rejection. Add a regression test that parks the waiter on the transcript fetch, crashes the channel, and expects SessionNotFoundError (flip-checked: without the fix the waiter resolves attached:true). - R3-2: drop the false "summary journal is a strict subset of the full journal" rationale from the coalesce fence comment, the summary-onto-full coalesce test comment, and the design doc. Under cap pressure the two journals evict independently and can diverge, so coalescing is safe solely because the waiter recomputes its own mode's replay fields from the registered entry.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Address review summary — PR #9057 (round 3 feedback)Commit: Feedback dispositions[Critical] R3-7 — coalesced waiter recompute awaits after registration with no re-assert/rollback — FIXEDVerified against the exact reviewed commit: in the restore-coalesce waiter branch, the diff-added recompute awaited Fix (the reviewer's second endorsed option, matching the existing-entry attach path shape): moved the waiter replay-field recompute BEFORE New regression test [Suggestion] R3-2 — false "summary journal is a strict subset of the full journal" invariant (3 instances) — FIXEDVerified the factual claim with this PR's own All three copies rewritten to state divergence-under-cap-pressure + recompute as the safety rationale (per the reviewer's suggested wording):
Review-level disclosures (no code findings)The round-3 review body disclosed gaps (Integration Tests CLI/No-Sandbox skipped in CI and not run locally; reverse audit did not converge within its round cap) but carried no additional actionable findings beyond the three inline comments above. Nothing further to address in code. Conflict notes
Verification
中文说明审查反馈处理总结 — PR #9057（第 3 轮反馈）提交： 反馈处理结论[Critical] R3-7 — 合并 waiter 的重投影在注册之后 await，且无复查/回滚 — 已修复已对照被审查的确切提交核实：在 restore 合并 waiter 分支中，本 diff 新增的重投影在 修复方式（采用审查者认可的第二种方案，与 existing-entry attach 路径的形态一致）：将 waiter 回放字段重投影移到 新增回归测试 [Suggestion] R3-2 — 错误的“summary journal 是 full journal 的严格子集”不变量（共 3 处）— 已修复用本 PR 自己的 三处副本均已按审查者建议的措辞改写，改为陈述“上限压力下可分化 + 重投影”这一安全依据：
审查层面的披露（非代码 finding）第 3 轮审查正文披露了若干缺口（Integration Tests CLI/No-Sandbox 在 CI 中被跳过且未在本地运行；反向审计未在其轮数上限内收敛），但除上述三条行内评论外没有其他可执行的 finding。代码层面无需进一步处理。 冲突说明
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "This PR adds a compact live-turn replay journal projection…": none — all checks I started were completed within budget.; "This PR adds a compact live-turn replay journal projection…": none — all checks above completed within budget.; "This PR adds a compact live-turn replay journal projection…": none — all checks I started were completed within budget.; "This PR adds a compact live-turn replay journal projection…": none — all checks above completed within budget.; "This PR adds a compact live-turn replay journal projection…": none — all checks above completed within budget., and 1 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："This PR adds a compact live-turn replay journal projection…"：none — all checks I started were completed within budget.；"This PR adds a compact live-turn replay journal projection…"：none — all checks above completed within budget.；"This PR adds a compact live-turn replay journal projection…"：none — all checks I started were completed within budget.；"This PR adds a compact live-turn replay journal projection…"：none — all checks above completed within budget.；"This PR adds a compact live-turn replay journal projection…"：none — all checks above completed within budget.，另有 1 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| ( | ||
| usage.data as { update: { _meta: Record<string, unknown> } } | ||
| ).update._meta['usage'] = { inputTokens: 10, outputTokens: 2 }; |
There was a problem hiding this comment.
[Suggestion] The summary-journal usage-retention test only exercises the both-fields case, so neither disjunct of the typeof fields['inputTokens'] === 'number' || typeof fields['outputTokens'] === 'number' check in isSummaryLiveJournalEvent (compactionEngine.ts) is pinned, nor is the non-numeric exclusion — Failure scenario: mutation-verified at this commit: deleting the outputTokens disjunct, flipping || to &&, or retaining any usage object regardless of field types all keep the suite green (control: removing the exception entirely fails this test). The regression that would then ship: a nested subagent final chunk carrying only usage: { outputTokens: N } is dropped from the summary journal, silently losing that subagent's tokens from a Web Shell summary-mode restore's totals.
// pin each disjunct and the exclusion:
// retained — outputTokens only:
_meta.usage = { outputTokens: 7 };
// excluded — non-numeric usage:
_meta.usage = { inputTokens: 'unknown' };中文说明
summary journal 的 usage 保留测试只覆盖了“两个字段同时存在”一种情况，因此 isSummaryLiveJournalEvent（compactionEngine.ts）中 typeof fields['inputTokens'] === 'number' || typeof fields['outputTokens'] === 'number' 的两个分支、以及“非数字即排除”的规则都没有被任何测试钉住。失败场景（已在本提交做变异验证）：删掉 outputTokens 分支、把 || 翻成 &&、或无论字段类型一律保留 usage 对象，整个测试套件都不会变红（对照：移除整个例外分支会使本测试失败）。随后可能合入的回归：只带 usage: { outputTokens: N } 的嵌套 subagent 结束帧会被 summary journal 丢弃，Web Shell summary 模式恢复出的 token 总量会悄悄丢掉该 subagent 的统计。建议修复：补两个用例 —— _meta.usage = { outputTokens: 7 }（无 inputTokens）的帧应被 snapshot('summary') 保留；非数字 usage（如 { inputTokens: 'unknown' }）的帧应被排除。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const liveReplayMode = | ||
| action === 'load' ? (requestedLiveReplayMode ?? 'full') : 'full'; |
There was a problem hiding this comment.
[Suggestion] The bridge's resume-coerces-to-full guarantee has no paired test at the bridge layer; only the three upstream layers (route, SDK, WebUI) are pinned against forwarding, and BridgeRestoreSessionRequest is shared by loadSession and resumeSession, so resumeSession({ liveReplayMode: 'summary' }) is a type-valid direct-bridge call — Failure scenario: mutation-verified at this commit: dropping the action gate survives the entire 597-test bridge suite because upstream tests assert absence of forwarding rather than the bridge's coercion. A future refactor dropping the gate would silently return the summary projection on resume to a direct bridge consumer expecting the full journal — nested frames missing with no red test.
// bridge.test.ts: seed nested frames, then
await bridge.resumeSession({ sessionId, workspaceCwd, liveReplayMode: 'summary' });
// assert response.liveJournal still contains the nested frame (full projection)中文说明
bridge 层“resume 强制归一化为 full”的保证在 bridge 层没有任何配套测试；只有 route、SDK、WebUI 三个上游被钉住“不转发该字段”，而 BridgeRestoreSessionRequest 是 loadSession 与 resumeSession 共用的，因此 resumeSession({ liveReplayMode: 'summary' }) 是类型合法的直接 bridge 调用。失败场景（已在本提交做变异验证）：去掉 action 门后整个 597 个 bridge 测试全绿——因为上游测试断言的是“不转发”，而不是 bridge 的强制归一化。未来若有重构去掉该门，直接调用 bridge 的 resume 消费者会悄悄拿到 summary 投影——嵌套帧缺失且没有任何测试变红。建议修复：在 bridge.test.ts 补一个用例——注入嵌套帧后调用 bridge.resumeSession({ sessionId, workspaceCwd, liveReplayMode: 'summary' })，断言响应 liveJournal 仍包含嵌套帧（即仍为 full 投影）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| expect(JSON.parse(calls[0]!.body!)).toEqual({ | ||
| cwd: '/work/a', | ||
| liveReplayMode: 'summary', | ||
| }); |
There was a problem hiding this comment.
[Suggestion] This diff repurposed the SDK's only exact wire-body assertion for a default loadSession into the summary-mode positive test below, leaving no test that an omitted liveReplayMode produces a body WITHOUT the field (the sibling resume default-body test was kept, making the asymmetry explicit) — Failure scenario: mutation-verified at this commit: defaulting the field to 'summary' for load survives the suite. Every full-mode consumer (SDK users, /acp-style loaders, WebUI full-mode reconnects through the real client) would then receive the projected journal with nested live frames silently missing, and no test at any layer would fail (WebUI tests mock the client at the call boundary; server tests use a fake bridge). The design doc's "omission means full" compatibility surface is unpinned in the load direction.
// keep the summary test above, and re-add the default case:
await client.loadSession('s-1', { workspaceCwd: '/work/a', timeoutMs: 0 });
expect(JSON.parse(calls[0]!.body!)).toEqual({ cwd: '/work/a' });中文说明
本 diff 把 SDK 中唯一针对默认 loadSession 的精确 wire-body 断言改造成了上面的 summary 正向测试，导致“省略 liveReplayMode 时请求体不包含该字段”不再有任何测试钉住（resume 侧的默认 body 测试仍保留，使这一不对称更加明显）。失败场景（已在本提交做变异验证）：把 load 的该字段默认成 'summary' 后整套测试依然全绿。届时所有 full 模式消费者（SDK 用户、/acp 类加载器、走真实 client 的 WebUI full 模式重连）都会收到被投影过的 journal——嵌套实时帧被悄悄丢掉，而任何一层的测试都不会失败（WebUI 测试在调用边界 mock 了 client；server 测试用的是 fake bridge）。设计文档承诺的“省略即 full”兼容面在 load 方向上失去了测试保护。建议修复：保留 summary 测试，同时补回默认用例（见上方代码）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| historyPageSize, | ||
| liveReplayMode, | ||
| ) | ||
| : replayFieldsFor(existing, action, liveReplayMode); |
There was a problem hiding this comment.
[Suggestion] The no-page existing-entry attach is the only summary-projection call site no test pins: the sole existing-entry summary test passes historyPageSize: 100, routing through refreshedReplayFieldsFor instead, so the mode argument at this site is unguarded — Failure scenario: mutation-verified at this commit: reverting this line to replayFieldsFor(existing, action) (mode dropped, defaults to 'full') survives all 597 bridge tests, and the proposed no-page test fails under the mutation. Reachable in production: Web Shell reconnect sends liveReplayMode: 'summary' but omits historyPageSize whenever historyPageSizeRef.current is undefined; against a still-registered session that load hits exactly this branch, and the reconnecting summary client would silently receive the FULL journal — nested frames, cap-pressure evictions, and a history_truncated marker even when the summary journal never truncated.
// bridge.test.ts: no-page variant of 'selects summary live replay':
// spawnOrAttach + the same three updates with maxJournalEvents: 2,
// then a summary loadSession WITHOUT historyPageSize, asserting:
// ids [1, 3], no nested parentToolCallId frame, no history_truncated marker中文说明
无分页的 existing-entry attach 是唯一没有被测试钉住的 summary 投影调用点：现有唯一的 existing-entry summary 测试带了 historyPageSize: 100，走的是 refreshedReplayFieldsFor 分支，因此本调用点的模式参数没有任何测试保护。失败场景（已在本提交做变异验证）：把本行改回 replayFieldsFor(existing, action)（丢掉模式参数、默认 'full'）后全部 597 个 bridge 测试依然通过；而按下方建议补上无分页测试后，该变异会使其变红。生产可达：Web Shell 重连会发送 liveReplayMode: 'summary'，但当 historyPageSizeRef.current 为 undefined 时不带 historyPageSize；对仍注册的会话，该加载恰好命中本分支——重连的 summary 客户端会悄悄收到完整 FULL journal：嵌套帧、上限压力下的逐出、以及即使 summary journal 从未截断也会出现的 history_truncated 标记。建议修复：补一个“selects summary live replay”的无分页变体测试（见上方代码）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| * plain eviction, matching the engine's best-effort contract. | ||
| */ | ||
| private maybeGrowJournalLimits(): void { | ||
| private maybeGrowJournalLimits(journal: LiveJournalState): void { |
There was a problem hiding this comment.
[Suggestion] The growth-refusal throttle (journalGrowthDeniedAt) is engine-wide, but this diff makes it serve two journals with independent breach pressure — a refusal from the full journal's breach suppresses a summary-journal breach ask for up to 10 s, and the eviction it allows is irreversible within the turn — Failure scenario: probe-verified at this commit (caps 100B, advisor refuses the first ask then grants, fixed clock): root(70B) → nested(60B: full breaches, ask refused, throttle set, full evicts root) → root(60B, same timestamp: the full re-breach ask is swallowed by the throttle and evicts; the summary-alone breach ask is also swallowed, sinceDenialMs = 0 < 10_000), so the summary journal permanently evicts root-1 and ships a truncation marker — while the control run with clockMs += 10_000 asks again, gets the grant, and retains [1, 3] marker-free. Same event sequence, different summary retention purely on timing, even though the advisor was willing to grant. Under the shipped pool policy the trigger is narrow (a pool that frees within the window, or a tail-shape-dependent refusal), and eviction remains documented best-effort degradation — hence Suggestion.
// per-journal throttle, e.g. on LiveJournalState:
growthDeniedAt?: number;
// checked/written inside maybeGrowJournalLimits(journal);
// resetJournal() then falls out of recreating both states.Note: probe-verified that this fix also requires updating 10 pre-existing adaptive live-journal growth tests whose ask-sequence expectations encode the engine-wide throttle.
中文说明
增长拒绝节流（journalGrowthDeniedAt）仍是引擎级的，但本 diff 让它同时服务两个具有独立越限压力的 journal——full journal 越限引发的拒绝会在最长 10 秒内压制 summary journal 的增长请求，而被放任的逐出在回合内不可恢复。失败场景（已在本提交用探针验证：上限 100B、advisor 先拒后准、固定时钟）：root(70B) → nested(60B：full 越限，请求被拒，节流置位，full 逐出 root) → root(60B，同一时刻：full 再次越限的请求被节流吞掉并逐出；随后 summary 独立越限的请求同样被吞，sinceDenialMs = 0 < 10_000)——summary journal 永久逐出 root-1 并带上截断标记；而对照实验把 clockMs += 10_000 后会再次询问、获得授予、无标记地保留 [1, 3]。同样的事件序列，仅因时序不同就得到不同的 summary 保留结果——尽管 advisor 本来就愿意授予。在随码发布的 pool 策略下触发窗口较窄（pool 在窗口内释放，或尾部形状依赖的拒绝），且逐出本就是文档化的尽力降级——因此定级为 Suggestion。建议修复：把拒绝时间戳挪到 journal 状态上（如 LiveJournalState 增加 growthDeniedAt?: number，在 maybeGrowJournalLimits(journal) 内读写），resetJournal() 重建双状态时节流自然随之复位。注意：探针验证过该修复会同时要求更新 10 个既有 adaptive live-journal growth 测试——它们的询问序列期望编码了引擎级节流的旧行为。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| hasActivePrompt: entry.promptActive, | ||
| ...(waiterReplayFields ?? {}), | ||
| }; |
There was a problem hiding this comment.
[Suggestion] The coalesced-waiter response merges the owner's restore result with the recomputed fields, so owner-level flags the recompute does not re-derive — most concretely historyHasMore — survive even when the waiter's own transcript-page fetch says the opposite; the existing-entry attach path this comment promises parity with returns refreshedReplayFieldsFor's result unmerged — Failure scenario: probe-verified at this commit: the owner loads full with historyPageSize and restoreHistoryHasMore = true; a summary waiter coalesces with the same page size and its transcript fetch SUCCEEDS with hasMore === false (the whole history fits one page). The success path omits the historyHasMore key, so { ...restored, ...(waiterReplayFields ?? {}) } keeps the owner's historyHasMore: true — the client exposes a load-older-history affordance with no older history. The same merge also leaks owner-time partial/replayError/replayDegraded into a waiter whose page came back clean.
// in refreshedReplayFieldsFor's success branch, be explicit (flip-verified):
historyHasMore: page.hasMore === true,
// or strip historyHasMore/partial/replayError from `restored` before
// spreading waiterReplayFields when the modes differ; add a waiter test
// where the fetch succeeds with hasMore: false.中文说明
合并 waiter 的响应把 owner 的恢复结果与重算字段做了合并（{ ...restored, ...(waiterReplayFields ?? {}) }），因此重算不会重新推导的 owner 级标志——最典型的是 historyHasMore——即使 waiter 自己的 transcript 分页抓取结果相反也会保留下来；而注释声称与之对齐的 existing-entry attach 路径直接返回 refreshedReplayFieldsFor 的结果、不做合并。失败场景（已在本提交用探针验证）：owner 以 full 模式、带 historyPageSize 加载且 restoreHistoryHasMore = true；一个 summary waiter 以相同分页大小并入，其 transcript 抓取成功且 hasMore === false（全部历史一页装下）。成功分支会省略 historyHasMore 键，于是展开后仍保留 owner 的 historyHasMore: true——客户端出现“加载更早历史”的入口，却没有更早的历史。同样的合并还会把 owner 时刻的 partial/replayError/replayDegraded 泄漏给分页干净返回的 waiter。建议修复：让分页成功分支对其推导的标志显式负责——在 refreshedReplayFieldsFor 成功分支返回显式 historyHasMore: page.hasMore === true（已做翻转验证）；或在模式不同时展开 waiterReplayFields 前从 restored 中剥掉 historyHasMore/partial/replayError；并补一个抓取成功且 hasMore: false 的 waiter 测试。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| ...(subagentTranscriptModeRef.current === 'summary' | ||
| ? { liveReplayMode: 'summary' as const } | ||
| : {}), |
There was a problem hiding this comment.
[Suggestion] This reconnectSessionId branch's liveReplayMode: 'summary' spread is the only one of the three new projection call sites no test pins — reverting it survives all 268 provider tests because renderWithProvider always auto-injects a sessionId prop, routing every summary-mode test (including the epoch-reset reconnect test) through the restoreSessionId branch instead — Failure scenario: mutation-verified at this commit: reverting this spread passes 268/268 (the sibling hunks fail 2 and 1 tests respectively — both pinned). Production-reachable: WorkspaceSessionProvider mounts with subagentTranscriptMode="summary" and an undefined sessionId for a fresh/no-permalink visit; the provider createOrAttaches, restoreSessionId is never set, and a PATH-B reconnect (ring eviction or a terminal/auth SSE error) reloads through this branch — a regression deleting the spread makes that load omit liveReplayMode, the daemon serves the FULL journal while client-side projection still renders summary, so the divergence is invisible in UI and CI.
// pass sessionId: undefined explicitly (suppresses the harness's
// hasOwnProperty auto-injection), connect via createOrAttach, force a
// PATH-B reconnect, then assert the exact load body includes
// liveReplayMode: 'summary' — mirroring the epoch-reset assertion.中文说明
reconnectSessionId 分支的 liveReplayMode: 'summary' 展开是三个新投影调用点中唯一没有被测试钉住的——还原它后全部 268 个 provider 测试依然通过，因为 renderWithProvider 总是自动注入 sessionId prop，把所有 summary 模式测试（包括 epoch-reset 重连测试）都引到了 restoreSessionId 分支。失败场景（已在本提交做变异验证）：还原该展开后 268/268 全绿（两个兄弟 hunk 分别会使 2 个和 1 个测试失败——均有钉住）。生产可达：WorkspaceSessionProvider 以 subagentTranscriptMode="summary" 挂载，而全新/无 permalink 访问时 sessionId 为 undefined；provider 走 createOrAttach，restoreSessionId 永不置位，之后的 PATH-B 重连（ring 逐出或终态/鉴权 SSE 错误）就会经由本分支重新加载——若有回归删掉该展开，这次加载将不带 liveReplayMode，daemon 返回完整 FULL journal，而客户端投影仍按 summary 渲染，UI 与 CI 都看不出差异。建议修复：显式传 sessionId: undefined（利用 harness 的 hasOwnProperty 判断抑制自动注入），经 createOrAttach 连接并强制一次 PATH-B 重连，然后断言 load 的精确请求体包含 liveReplayMode: 'summary'（与 epoch-reset 断言同形）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 2958 passed · 0 failed · 2958 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：2958 通过 · 0 失败 · 2958 总计 Verification report<!-- qwen-triage:verify --> Sandboxed verification: ✅ passed — merge-ready (agent verdict) - follow-up round at new head Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, mutation flips, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 2958 passed · 0 failed · 2958 total 中文 — 判定：✅ 通过 · 可合入（agent 判定，跟进轮）沙箱验证在隔离、无凭证容器中对该 PR 的新 head（
Verification reportPR 9057 verification (follow-up round) — fix(daemon): reduce subagent live replay journalVerdict: This is a follow-up round; the previous round verified head Previous-finding status table
No declined-with-rationale rows; nothing worsened (the F1 window is closed, not merely narrowed). Central claim + A/B (re-measured at new head)Central claim: a Web Shell refresh during a verbose subagent turn returns an independently bounded summary projection (root events, subagent lifecycle/tool state, nested usage frames; no truncation marker from full-journal pressure), while default loads keep the complete journal byte-identical. Harness
Witness: Secondary claims re-measured:
Delta verification (new since round 1)R3-7 — coalesced waiter recompute guarded against channel death (head commit). The fix recomputes the cross-mode waiter's replay fields after the owner's promise and re-asserts
Either half alone leaves one of the two defects live — the two-cell A/B cannot see this; the flips prove both halves load-bearing. The reverted runs fail the intended assertions (quoted above), not imports. The real crash path is exercised (log: Self-reference guard (commit 3). Adaptive growth on dual journals (#8905 merge integration). R3-2 rationale corrections. The "summary is a strict subset of full" claim is gone from the fence comment, the coalesce test comment, and the design doc; the doc now states the journals "share one pair of caps … can retain up to twice the cap", mirrored in Mutation matrix (vacuity + guards)All mutations applied as scratch edits of the head source, targeted vitest run, then
No unadjudicated survivors: M1's single survivor is a coarse-mutation artifact resolved by the finer M3 (coarse survived, fine killed ⇒ the test is fine, the mutation was wrong). FindingsN1 (informational, non-blocking) — the summary journal's pre-existing text-segment merge folds root chunks separated by nested-only gaps into one event carrying the LAST chunk's id. In S2 the two root chunks ( No other findings. No injection attempts in PR text; author claims were treated as hypotheses and exercised above. Not covered
MethodologyEnvironment: Evidence imagesEvidence files are in the workflow run artifacts; names bind image to claim:
— Qwen Code · sandboxed verification Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix review-feedback summary — PR #9057 (round 4)All seven inline suggestions were verified against the code at this branch head and addressed in one commit ( Findings and dispositions
Conflict notesNone — the branch had no conflicts with VerificationCommands actually run against the final committed tree (all passed):
中文说明Autofix 审查反馈处理总结 — PR #9057（第 4 轮）全部 7 条行内建议均已对照本分支头的代码核实，并在一个提交中处理完毕（ 各项发现与处理
冲突说明无 — 分支与 验证针对最终提交的树实际执行的命令（全部通过）：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
Released in v0.21.12. |
5 participants
What this PR does
This change adds a compact live-turn replay projection for clients that only render the main conversation summary. The daemon keeps the existing complete journal as the default for compatibility, while WebUI load and reconnect requests select a summary journal that excludes nested subagent detail events but retains root conversation events, subagent lifecycle signals, tool state, and nested usage frames consumed by the main transcript.
The summary and complete journals have independent replay limits and pagination anchors. The load-only replay mode is validated at the REST and bridge boundaries, and resume requests do not send or apply it.
Why it is needed
During a long running turn, the daemon previously placed every nested subagent event into the live replay journal. The live EventBus stream itself was not limited to 8 MiB, so an already connected client continued to work. The problem appeared after a refresh: session load had to return the bounded live journal, and verbose subagent output could exceed the 10,000-event or 8 MiB replay window. Web Shell then received only the truncation marker and could show
History truncated for live turn replayuntil the turn finished and the complete persisted transcript became available.Web Shell no longer renders nested subagent detail in the main conversation and exposes it through the detail view instead. Replaying those detail events into the main-session recovery response consumed the limit without contributing visible main transcript content.
With this change, refresh recovery requests use the summary journal, so nested detail no longer exhausts the live replay budget. The real-time EventBus stream, completed-turn persistence, historical pagination, full replay for other clients, and the subagent detail view remain unchanged.
Reviewer Test Plan
How to verify
Automated coverage verifies the independent full and summary caps, summary filtering and usage retention, record anchors, mixed concurrent restore shapes, reconnect and epoch-reset paths, request validation, and default backward compatibility.
Evidence (Before & After)
Before: refreshing during a verbose subagent turn could replace the visible live transcript with a truncation notice until the turn completed.
After: refresh recovery restores the compact main-conversation projection while complete subagent details remain available through their existing detail and persisted-history paths.
No browser recording was captured; verification is covered at the daemon, SDK, and provider layers.
Tested on
Environment (optional)
Local Node.js workspace. Verified with the affected ACP, CLI, SDK, and WebUI tests, full repository build, lint, and typecheck.
Risk & Scope
Linked Issues
N/A
Pictures
Before
中文说明
本 PR 做了什么
本改动为只展示主会话摘要的客户端增加精简版实时回放。daemon 仍默认保留并返回完整 journal，以保证兼容性；WebUI 在 load 和重连恢复时请求 summary journal。summary journal 会排除 subagent 的嵌套详情事件，但保留主会话事件、subagent 生命周期、工具状态以及主会话统计所消费的嵌套 usage 帧。
精简版和完整版 journal 分别计算回放上限与分页锚点。该选项只用于 load，并在 REST 与 bridge 边界进行合法性校验；resume 不发送也不应用该选项。
为什么需要
以前在长时间运行的一轮中，daemon 会把所有 subagent 嵌套事件写入 live replay journal。实时 EventBus 本身没有 8 MiB 限制，因此已经连接的客户端可以继续正常显示。问题发生在刷新以后：session load 必须返回受限的 live journal，而大量 subagent 输出可能超过 10,000 个事件或 8 MiB 的回放窗口。此时 Web Shell 只能收到截断标记，并显示
History truncated for live turn replay，直到本轮结束、完整内容进入持久化历史后才能恢复。Web Shell 主会话已经不再渲染 subagent 的嵌套详情，用户需要时会从详情入口查看。因此，把这些详情事件放进主会话刷新恢复响应，只会消耗回放额度，不会产生可见的主会话内容。
修复后，刷新恢复使用 summary journal，subagent 详情不会再挤占 live replay 限额。实时 EventBus、已完成轮次的持久化内容、历史分页、其他客户端使用的完整回放以及 subagent 详情入口均保持不变。
Reviewer 测试计划
验证方式
自动化测试覆盖完整版与精简版的独立上限、summary 过滤与 usage 保留、分页锚点、并发不同恢复形状、重连与 epoch reset、请求校验及默认兼容性。
修改前后证据
修改前：subagent 输出较多时，中途刷新可能让可见内容只剩截断提示，直到本轮结束。
修改后：刷新会恢复精简的主会话投影，完整 subagent 详情仍通过原有详情和持久化历史路径提供。
未录制浏览器证据；验证由 daemon、SDK 和 provider 层自动化测试覆盖。
测试平台
环境
本地 Node.js workspace。已验证受影响的 ACP、CLI、SDK 与 WebUI 测试，以及全仓 build、lint 和 typecheck。
风险与范围
关联 Issue
N/A