fix(web-shell): bound daemon transcript retention to stop renderer OOM crashes - #9303
Conversation
…M crashes Watching a busy daemon session (long turn, many subagents) could exhaust the browser renderer: the replay snapshot stayed pinned on the session client for the whole attachment, replay rebuilds ran uncapped and could ratchet the transcript block cap above its configured limit, and a few retention windows were implicitly far larger than intended. Observed as a Chrome renderer abort after multi-GB growth, reproducible on every reload of the affected session. - Release the replay snapshot once it is injected into the transcript store (SSE resumes from lastEventId; older history via pagination) - Rebuild replays under the configured maxBlocks cap and never raise the committed cap above it; trimming keeps the most recent blocks - Cap the debug text embedded for unrecognized session_update kinds, which appended one block per frame with the full payload - Pass the web-shell block cap to the subagent detail provider and lower the provider default window from 200k to 50k blocks
|
@qwen-code /takeover |
|
🤝 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. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Thanks for the PR! Re-running triage on the author's re-trigger. Since the last pass ( Template looks good ✓ Problem: observed, not theoretical. First-hand crash evidence — a renderer growing 2.0GB → 4.2GB → 5.1GB on a busy session, a breakpoint-style abort consistent with heap exhaustion, Crashpad dumps with the same signature recurring over several days, and every reload re-ingesting the uncapped replay — with three retention paths identified in code (snapshot pinned for the attachment lifetime, uncapped replay rebuild, replay ratcheting the committed block cap). Direction: aligned. The web shell is a first-class surface and client-side retention must be bounded — the daemon's replay window grows adaptively during in-flight turns, so the client can't retain or rebuild it wholesale. No CHANGELOG signal needed for a stability fix in this repo's own web shell. Size: cross-package (sdk-typescript, webui, web-shell): ~966 production lines vs ~2,069 test lines (0 generated/schema) at this head — the three new commits are overwhelmingly regression tests. The author holds admin access, so the core-change gate is exempt, and production stays under the 1,000-line large-PR advisory regardless. Approach: unchanged from the prior pass — the retention mechanism is the minimal set needed for the stated goal, with no unrelated changes. The three new commits are tightly scoped to the exact defects rounds 13–15 filed; no drive-by edits. Risk: no high-risk path matches from the revert-history analysis (no shell/mcp/lsp/sandbox/acp surfaces touched). No elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献!应作者的重新触发再次 triage。自上次审查( 模板完整 ✓ 问题:已观测到的真实问题,非理论性加固。第一手崩溃证据——繁忙会话中 renderer 从 2.0GB → 4.2GB → 5.1GB 增长、与堆耗尽一致的断点式 abort、连续多天反复出现同签名的 Crashpad 转储、每次重载都重新吸入无上限 replay——并已在代码中定位三条保留路径。 方向:对齐。Web Shell 是一等界面,客户端保留量必须有界。本仓库自身 Web Shell 的稳定性修复无需 CHANGELOG 信号。 规模:跨三个包:当前 head 约 966 行生产代码、约 2,069 行测试代码(0 行生成/schema)——三个新提交绝大多数是回归测试。作者持有 admin 权限,核心改动门槛豁免;生产行数也低于 1,000 行大 PR 建议线。 方案:与上次审查结论一致——保留机制是达成目标的最小集合,无顺手改动。三个新提交都精准对应第 13–15 轮提出的缺陷,无顺带修改。 风险:回滚历史分析未命中任何高风险路径。无升级风险信号。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewThis re-run covers the delta The three round-13 Criticals that gated the PR are all fixed and present at this head. I re-read each implicated region at
Round-14 and round-15 fixes are also present and coherent. No new blockers found in this pass. The fixes are minimal, each pinned by a regression test, and I found no correctness regression introduced by them. Two things I did not re-verify here, disclosed: the block-count/byte caps and the snapshot-release mechanism were validated in prior passes and are untouched by this delta; and the persistent advisory verify findings below are unchanged by these commits. Sandboxed verification (behavioural claim)The central claim — retention stays bounded in the renderer — is substantiated by the last completed That run also carries three advisory, Suggestion-severity findings — explicitly "not a review, an approval, or a CI check" — that this delta does not touch and that a maintainer already weighed before approving:
These are genuine follow-ups, not merge-blockers; I'd suggest a tracking issue so they aren't lost once this lands. Test evidenceUnattended CI run — the PR's own checks are the evidence; no PR code was executed here. All substantive checks are green on CI results for
Green CI settles that the suite passes; the memory claim is settled by the A/B verify above, not by CI. Not verified here: long-soak renderer memory on the current head (the fresh 中文说明代码审查本次 re-run 覆盖 门槛上那三个第 13 轮 Critical 均已修复且存在于本 head。 我在
第 14、15 轮修复也已落地且自洽。 本次未发现新阻断项。 修复均为最小改动、各有回归测试钉住,未发现其引入正确性回退。 两处未在此重复核实,披露:block 数/字节上限与快照释放机制已在前几轮验证、且本增量未触及;下述持续性的 advisory verify 发现也未被这些提交改变。 沙箱验证(行为性主张)核心主张——renderer 中保留量保持有界——已由上一次完成的 该运行还报告三个 advisory、Suggestion 级 发现——明确"不构成评审、批准或 CI 检查"——本增量未触及,且 maintainer 在批准前已知悉:
这些是真实的后续项,非合并阻断;建议建一个跟踪 issue,避免合入后遗失。 测试证据无人值守 CI 运行——以 PR 自身检查为证据,未执行任何 PR 代码。 绿色 CI 只证明套件通过;内存主张由上方 A/B verify 证实,而非 CI。此处未验证:当前 head 的长时间浸泡 renderer 内存(新 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — every blocking Critical from rounds 13–15 is fixed and verified at this head, CI is fully green, and the central memory-bounding claim is A/B-proven; the remainder is advisory (byte-accounting exactness, a few unpinned guards), which is follow-up work, not a merge gate. Stepping back: the last pass held this at If I had to maintain this in six months the retention core reads clearly and the regression tests pin the sharp edges; I'd want the F1 accounting invariant closed and those three follow-ups in a tracking issue, but I wouldn't curse the author. The original reproduction (a real renderer OOM with Crashpad evidence) is exactly the kind of observed bug this should fix, and the A/B verify shows the bound holds. Verdict: approve — approving pinned to 中文说明置信度:4/5 —— 第 13–15 轮的每一个阻断级 Critical 都已修复并在本 head 上核实,CI 全绿,核心"内存有界"主张已经 A/B 证实;剩下的是 advisory 项(字节记账精确性、少数未钉住的守卫),属后续工作,不构成合并门槛。 退一步看:上次审查以三个第 13 轮 Critical 把本 PR 留在 若六个月后由我维护:保留核心清晰、回归测试钉住了锋利边角;我希望关闭 F1 记账不变量并把三个后续项放进跟踪 issue,但不会怪作者。最初的复现(真实 renderer OOM + Crashpad 证据)正是应当修复的已观测 bug,A/B verify 也表明有界性成立。 结论:批准 —— 固定到 — Qwen Code · qwen3.8-max Reviewed at |
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. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.13)
…M#9303) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Address-review summary — PR #9303 (round 1)Feedback points and dispositions[rc:3793989466] Stale doc comment on
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.13)
…ipt-memory-growth # Conflicts: # packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts
|
@qwen-code /resolve |
jifeng
left a comment
There was a problem hiding this comment.
Local validation report — f7301e448
Recommendation: do not merge yet. The functional regression tests pass and a 50 MB replay survives in a real Chromium run, but the new retention guard still permits renderer OOM well below the configured 50,000-block limit. The PR is also currently non-mergeable against latest main because of a content conflict.
Environment and checks
- macOS arm64, Node.js 24.18.0, npm 11.16.0.
- Built the prerequisite production packages: core, ACP bridge, TypeScript SDK, and WebUI — all passed.
- SDK targeted suites: 353/353 passed.
- WebUI
DaemonSessionProvidersuite: 217/217 passed. - SDK, WebUI, and Web Shell typechecks: all passed.
- GitHub Actions for this SHA: Qwen Code CI, Web-shell Visuals, and Security Checks are green.
Real browser replay
I ran the actual Web Shell in headless Chromium against the repository's daemon transport harness, kept SSE open, forced Chromium GC through CDP, then sent another live event:
- 50 MB replay (500 distinct 100 KB events): passed in 5.3 s.
- Post-GC heap: 126.5 MiB used / 157.1 MiB total.
- SSE remained connected and the post-GC assistant event rendered.
- 200 MB replay: Chromium closed/crashed before the SSE connection could open.
Production-code memory stress
Using the production normalizer and transcript store:
| Scenario | Retained blocks | Heap used | RSS | Result |
|---|---|---|---|---|
| Unrecognized debug frames, 100 KB each | 5,000 | 486.7 MiB | 710 MiB | completed |
| Unrecognized debug frames, 100 KB each | 10,000 | 965.7 MiB | 1,238.4 MiB | completed |
| Unrecognized debug frames, 100 KB each | 50,000 target | ~1.18 GiB before failure | — | OOM, exit 134 |
Tool rawInput, 100 KB each |
1,000 | 199.6 MiB | 387.7 MiB | completed |
Tool rawInput, 100 KB each |
4,000 | 774.2 MiB | 1,058.2 MiB | completed |
| Unrecognized debug frames, 1 KB each | 50,000 | 77 MiB | 228.1 MiB | completed |
The diagnostic text reports only 4,111 characters, but V8 still retains close to the original 100 KB per frame, consistent with a sliced string retaining its backing store. Separately, tool blocks keep uncapped rawInput/rawOutput; capping only the display details field does not bound retained bytes.
Base integration
Latest origin/main is b259bee2947e9fe241a7bd5e0bd049482ba21b11. A local git merge-tree reports a content conflict in packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts, matching GitHub's mergeable: false.
The direction of the PR is sound, snapshot consumption is covered, and the functional suites are green. Before merging, I recommend: (1) make diagnostic truncation detach or avoid creating the large backing string, (2) add a byte-aware retention policy or cap the raw tool payloads retained by transcript blocks, (3) rerun the 200 MB browser case, and (4) resolve/retest against current main.
中文版本
本地验证报告 — f7301e448
建议暂不合并。 功能回归测试均通过,真实 Chromium 中 50 MB replay 也能正常完成;但新的保留上限在远未达到 50,000 个 block 时仍可能让 renderer OOM。此外,该 PR 当前与最新 main 存在内容冲突,GitHub 显示不可合并。
环境与检查
- macOS arm64,Node.js 24.18.0,npm 11.16.0。
- core、ACP bridge、TypeScript SDK、WebUI 的前置生产构建全部通过。
- SDK 定向测试:353/353 通过。
- WebUI
DaemonSessionProvider:217/217 通过。 - SDK、WebUI、Web Shell typecheck:全部通过。
- 当前 SHA 的 Qwen Code CI、Web-shell Visuals、Security Checks 均为绿色。
真实浏览器 replay
使用实际 Web Shell + headless Chromium,连接仓库内 daemon transport harness,保持 SSE 打开,通过 CDP 强制 GC 后继续发送实时事件:
- 50 MB replay(500 条互不相同的 100 KB 事件):5.3 秒内通过。
- GC 后 heap:已用 126.5 MiB / 总计 157.1 MiB。
- SSE 保持连接,GC 后发送的 assistant 实时事件正常渲染。
- 200 MB replay:在 SSE 建连前 Chromium 页面/浏览器即关闭或崩溃。
生产代码内存压测
直接使用生产 normalizer 与 transcript store:
| 场景 | 保留 block | Heap used | RSS | 结果 |
|---|---|---|---|---|
| 未识别 debug frame,每条 100 KB | 5,000 | 486.7 MiB | 710 MiB | 完成 |
| 未识别 debug frame,每条 100 KB | 10,000 | 965.7 MiB | 1,238.4 MiB | 完成 |
| 未识别 debug frame,每条 100 KB | 目标 50,000 | 失败前约 1.18 GiB | — | OOM,退出码 134 |
Tool rawInput,每条 100 KB |
1,000 | 199.6 MiB | 387.7 MiB | 完成 |
Tool rawInput,每条 100 KB |
4,000 | 774.2 MiB | 1,058.2 MiB | 完成 |
| 未识别 debug frame,每条 1 KB | 50,000 | 77 MiB | 228.1 MiB | 完成 |
诊断文本表面长度只有 4,111 字符,但 V8 仍保留了接近原始 100 KB/条的内存,表现符合切片字符串继续引用原始 backing store。另一方面,tool block 仍保留未截断的 rawInput/rawOutput;只截断用于展示的 details 并不能限制实际保留字节数。
与最新主干集成
最新 origin/main 为 b259bee2947e9fe241a7bd5e0bd049482ba21b11。本地 git merge-tree 在 packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts 报内容冲突,与 GitHub 的 mergeable: false 一致。
该 PR 的方向正确,snapshot consume 有测试覆盖,功能测试也全部通过。合并前建议:(1) 让诊断截断真正脱离大字符串 backing store,或从源头避免构造完整大字符串;(2) 引入按字节的保留预算,或截断 transcript block 中保留的 tool 原始载荷;(3) 重新通过 200 MB 浏览器场景;(4) 解决与当前 main 的冲突并重新验证。
…ipt-memory-growth # Conflicts: # packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts
…-growth' into fix/web-shell-transcript-memory-growth
…ub.com/wenshao/qwen-code into fix/web-shell-transcript-memory-growth
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #9303Addressed the round-2 automated review and resolved the requested Feedback triage[rv:4949612613] Review — COMMENTED, no blockersContainer review from the automated reviewer. Its only actionable content is the [rc:3794742783] [Suggestion] Cap the payload at the producer — ACTEDThe finding: the payload cap was re-applied by hand at every consumer of I verified the claim against the code: all six debug- Fix (closes the class at the producer):
The three pre-existing, differently-shaped Merge / conflict notes (
|
|
Qwen Code did not run conflict resolution for this request. PR #9303 does not currently have merge conflicts with main. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 6b": none — no checks were cut short..
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/webui/src/daemon/session/DaemonSessionProvider.tsx:1503 — [probe] Reconnect guard mutations survive the suite (anchored reconnect variant unpinned)packages/webui/src/daemon/session/DaemonSessionProvider.tsx:504 — [probe] DEFAULT_MAX_BLOCKS 200k→50k is pinned by no test
中文说明
未探索到全部深度(达到工具调用预算):"agent 6b":none — no checks were cut short.。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
doudouOUC
left a comment
There was a problem hiding this comment.
评审总结 — PR #9303
概述
本 PR 针对 Web Shell 在浏览器中因 daemon transcript 无限制保留导致 renderer OOM 崩溃的问题,做了以下改进:
- replay 快照释放:
consumeReplaySnapshot()在注入后释放 session client 上的原始 wire events(可达数十 MiB) - replay 重建上限:replay 重建使用与实时增长相同的
maxBlocks上限,不再使用Number.MAX_SAFE_INTEGER - 上限不再被抬高:不再按 replay 规模抬高 committed
maxBlocks,防止一次大型 replay 导致永久性无限保留 - 默认值降低:
DEFAULT_MAX_BLOCKS从 200k 降至 50k - 诊断文本截断:
debugBlockText()统一截断未识别 payload 的嵌入文本 - preview 值截断:tool preview 中的大值也被截断
- SubagentDetail 使用上限:subagent 详情面板也使用
WEB_SHELL_MAX_TRANSCRIPT_BLOCKS
总体评价
这是一次目标明确、实现良好的内存安全改进。核心变更(replay 快照释放、replay 重建上限、上限不再被抬高)直接针对已确认的 OOM 根因,且新增的回归测试覆盖充分。代码质量高,JSDoc 注释清晰。
现有 Blockers 重新判定
R3-1(utils.ts:54 — capDetails 保留 backing store):依然成立。
debugBlockText 先通过 stringifyRedactedJson(data) 构造完整载荷字符串(可达 100KB),再传递给 capDetails 切片。在 V8 中,大字符串的 String.slice() 创建 SlicedString 内部类型,保留父字符串的 backing store。5,000 个 frame 保留 479.4 MiB(~98 KiB/frame)。修复方向:使用有界序列化器,或在切片后强制创建独立拷贝(如 [...str].join(''))。
R3-2(DaemonSessionProvider.tsx:503 — block 上限不是内存上限):部分缓解。
注释已更新,默认值从 200k 降至 50k 将最坏情况内存降低 4 倍(50,000 个 100KB 的 block 理论上可达 ~9.6 GiB)。但 blocker 的核心主张仍然成立:tool block 保留完整的 rawInput/rawOutput,只有展示用的 details 被截断。注释中的 "memory ceiling" 表述仍然具有误导性,建议改为 "block count ceiling"。
新发现的建议
-
consumeReplaySnapshot()调用时机(DaemonSessionProvider.tsx:1538):在 replay 分发循环之前被调用,如果分发中抛出异常,快照已经被消费。建议移到分发成功完成后。 -
replayExceededCapacity改用>=(DaemonSessionProvider.tsx:1715):blocks.length >= maxBlocks是正确的,但建议确认capacityReached的消费者逻辑兼容此变化。 -
WEB_SHELL_MAX_TRANSCRIPT_BLOCKS硬编码(sessions.ts:33):当前硬编码 50_000,建议从 provider 导出DEFAULT_MAX_BLOCKS常量并引用,避免未来不同步。
结论
建议:COMMENT(因 2 个现有 Critical blocker 仍然未解决)。PR 的核心变更方向正确、实现可靠,但 R3-1(backing store 保留)和 R3-2(block 上限非内存上限)两个 blocker 需要在后续迭代中处理。本 PR 可以合入,但建议记录后续改进项。
…ipt-memory-growth
…dle budget The consume-and-drop API landed as a getter plus a private backing field, which pushed the minified browser daemon bundle 51 bytes past the assertBrowserSafeBundle budget. Collapse it to a single mutable replaySnapshot field swapped by consumeReplaySnapshot(), preserving the external behavior while fitting the budget without raising it.
|
Sandboxed verification: ❌ not passed — findings reported (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: 6110 passed · 19 failed · 6129 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:6110 通过 · 19 失败 · 6129 总计 抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification report<!-- qwen-triage:verify --> Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Verdict: 中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)
Verification reportPR #9303 Deep Verification (follow-up round) —
|
| # | Finding (previous report, head cec40b8) | Severity | Status at new head 8447f29 |
|---|---|---|---|
| F1 | retainedBytes accounting misses block-mutation paths |
Suggestion | Stands — re-measured. Same under-count direction; drift sites at this head: applyAssistantUsage −192B, resolvePermissionBlock −32B, upsertPermissionBlock existing-branch (small→20KB toolCall) −40,230B, applySubagentUsageToParentTool −444B. Accounted paths (append, text growth, tool upsert/replace, image/file merge, count trim, store seeding) stay exact. The previously validated scratch fix was not applied. |
| F2 | toolPreview named-candidate rows (Path/Cwd/Query/Note) bypass capDetails |
Suggestion | Stands — re-measured. All four named rows retain 100,000 chars at head (4/4); generic control capped at 4111; base identical (pre-existing gap, not a regression). |
| F3 | R12-1 / R12-2 / R12-10 fixes ship unpinned | Suggestion | Stands — re-measured, M7 refined. Same 3 survivors (M5/M6/M7) in the 8-mutant matrix; M8 (R12-21) killed again by its own test. Census at new head: M5 boundary-dedup branch active in 13 scenarios with 0 same-text matches (no scenario even constructs the boundary collision, let alone a deeper same-text prompt); M6 rebuild-trim callbacks 5, rewinds 0; M7 loadMore catch entered 6 times (was 0 at the previous head — merged main tests now reach it) but the generation guard was true 0/6, so the guard itself remains unpinned. |
| M1–M4, M8 | Matrix kills (byte budget, snapshot release, rebuild cap, provider consume call, floor back-off) | — | Superseded by this round's matrix — re-run at the new head, same 5 killed, attributed to the intended regression tests. |
| Gates | sdk 1627 / webui 554 / web-shell 3800 | — | Re-run: sdk 1631 / webui 565 / web-shell 3850 (deltas are tests that landed via the main merges; all green). |
Carry-forward shortcut not used: the input closure changed (new head, new base, main content in shared files), so everything was re-executed.
Central claim and A/B
Central claim: the web shell's retention of daemon session history is bounded — replay rebuilds run under the configured block cap and can never ratchet it upward, oversized replays are trimmed to the most recent blocks, the replay snapshot is released after injection, diagnostic payloads are capped at the producer, and retention is byte-budgeted (128 MiB default) with record-boundary-aware eviction and floor back-off.
Harness ab-retention.mjs imports the compiled dist/daemon/index.js of head (CI-built at the merge commit) and of base (tmp/base-tree at HEAD^1 = a8a855914b, sdk-typescript rebuilt there against the root node_modules; lockfile untouched by the PR; base daemon dist verified free of external/@qwen-code imports, so the control cannot resolve into head code — realpath/purity check quoted in Methodology). Witness: 01-ab-retention-head-vs-base.png.
| # | Cell (oracle) | base (a8a8559) | head (8447f29) |
|---|---|---|---|
| C1 | Diagnostic cap — 4 producers × 100KB payload | 4/4 UNcapped (100,016–100,076 embedded) | 4/4 capped at exactly 4111 |
| C2 | Cap ladder around 4096 | uncapped at every rung | ≤4096 byte-identical / 4097 → 4111, nothing in between |
| C3 | Astral payload at the 100k text-block cut | capped (pre-existing) but raw slice leaves a lone surrogate | capped, no lone surrogate (detachString → U+FFFD) |
| C4 | Replay rebuild 300 blocks, cap 100, +50 live (provider lines emulated verbatim per arm) | 300 retained; committed cap ratcheted to 300; window 300 | trimmed to 100; committed cap 100; live growth trims at 100; trim detail oldestRetainedRecordId='record-200', evictedOldest=true; window keeps record-250.. |
| C5 | Byte budget — 100 × ~120KB blocks, budget 2MB | no mechanism (maxRetainedBytes undefined); all 100 retained |
evicted to 8 blocks; retained ≤ budget + worst block (2,058,162 ≤ 2,097,152 + 257,272); accounting exact over all 100 steps |
| C6 | Record-boundary snap — 3-block record-A, cap 8 | naive cut straddles (keeps 1 of 3) | cut advanced past the record; window = 7 × record-b* |
| C7 | Floor back-off (R12-21) — two ~100KB blocks sharing a record, budget 150KB | no mechanism; both trivially kept | both re-retained (floor backed off); distinct-record control keeps only the last |
| C8 | Snapshot release (consumeReplaySnapshot) |
method absent; snapshot pinned for client lifetime | returns snapshot once, swaps empty, idempotent |
| C9 | toolPreview generic row (100KB scalar) | UNcapped (100,000) | capped at 4111 |
| S1 | Store seeding counts retained bytes | — | seeded retainedBytes = Σ estimates (632 = 632) |
Result: 39/39 — every head behavior holds and every base control fails exactly as predicted. C4 emulates each arm's provider lines verbatim (base: replayMaxBlocks = Number.MAX_SAFE_INTEGER + committedMaxBlocks = Math.max(maxBlocks, replayState.blocks.length), base provider L1681-1685/L1715-1717; head: replayMaxBlocks = maxBlocks + committedMaxBlocks = replayMaxBlocks, head provider L1948/L2022).
Bundle budget: dist/daemon/index.js = 206,296 B ≤ 210,944 B (206KB budget after the PR's documented 199→206 bump; 4,648 B headroom). The enforcing assertBrowserSafeBundle ran inside CI's pre-run build that produced the tested dist; I re-checked the size statically.
Findings
F1 (Suggestion, carried forward — stands) — retainedBytes accounting still misses block-mutation paths
Re-measured at the new head with byte-accounting.mjs (22-step workload across three scenarios; asserts retainedBytes === Σ estimate(blocks) after every step). The invariant breaks at step S04 and never recovers: 12/22 step invariants fail. Witness 02-f1-byte-accounting-drift.png. Drift sites (all under-counts, all on paths that mutate a retained block without the measure-before/delta-after pattern the PR applies to upsertToolBlock, image/file deltas, and appendBoundedText):
| Step | Path | Unaccounted delta |
|---|---|---|
| S04 | applyAssistantUsage (usage object on active assistant) |
−192B |
| S09 | resolvePermissionBlock (resolved/eventId on COW clone) |
−32B |
| S10 | upsertPermissionBlock existing-branch (small → 20KB toolCall + options replace) |
−40,230B |
| S15/S16 | applySubagentUsageToParentTool (rawOutput summary replace) |
−444B |
At this head my finishAssistant and branchRecordId fixtures produced no net estimate change (the previous round measured −30B each with a different fixture); the four sites above are the measured drift. Direction is under-count, so the effective ceiling is budget + worst-case block + accumulated unaccounted deltas — bounded, no correctness hazard, but contradicts the PR's exactness invariant, and the suite pins nothing along these axes (green with and without the accounting). Repro: node tmp/pr9303-verify-20260820-085843/byte-accounting.mjs.
F2 (Suggestion, carried forward — stands) — toolPreview named-candidate rows still bypass capDetails
Re-measured: createDaemonToolPreview({ path|cwd|query|description: 'x'.repeat(100_000) }) retains 100,000 chars in the Path/Cwd/Query/Note rows on head (4/4), while the generic path caps at 4111 (control). Base is identical (uncapped everywhere) — residual gap in the PR's own hardening, not a regression; retention impact bounded because the row holds the same string reference the retained rawInput already carries. Witness 03-f2-toolpreview-uncapped-rows.png. One-line fix unchanged: route the named-candidate push through capDetails too.
F3 (Suggestion, carried forward — stands) — R12-1 / R12-2 / R12-10 still unpinned by any test
Mutation matrix at the new head (witness 04-mutation-matrix-5-killed-3-survived.png; positive controls: unmutated targeted suites green — sdk 388/388, provider 232/232 — and the same harness kills M1–M4/M8; live witness 05-live-mutant-m8-killed.png):
| Mutant | Single-point change | Result | Evidence |
|---|---|---|---|
| M1 | overByteBudget = false |
KILLED | sdk evicts oldest blocks to stay under the retention byte budget + accounts streamed assistant text against the retention byte budget |
| M2 | consumeReplaySnapshot keeps the snapshot |
KILLED | sdk releases the replay snapshot once consumed |
| M3 | rebuild cap → Number.MAX_SAFE_INTEGER |
KILLED | 6 provider tests incl. trims an oversized initial replay to the block cap and re-anchors older pagination, uses a bounded full-snapshot fallback after the marker block is trimmed |
| M4 | provider consumeReplaySnapshot() call removed |
KILLED | provider releases the replay snapshot after injection and never raises the block cap |
| M5 (R12-1) | dedup break removed → window-wide text keying |
SURVIVED | census: branch active 13×, same-text matches 0 |
| M6 (R12-2) | rewind gate dropped from rebuild observeReplayTrim |
SURVIVED | census: 5 trim callbacks, 0 rewinds during capped rebuilds |
| M7 (R12-10) | catch generation guard → if (false) |
SURVIVED | census: catch entered 6×, guard true 0/6 |
| M8 (R12-21) | floor back-off disabled | KILLED | sdk backs the record-boundary snap off the floor instead of cutting mid-record (R12-21) (319/320 pass, the one fail is the intended test) |
All three survivors classify as coverage gaps, not defects — the fixes read correctly and the R12-21 sibling proves the pattern testable. M7's census sharpened at this head: the catch is now reachable (6 entries, via merged-main tests) but never with a concurrent generation change, so the guard remains the exact unpinned axis. Pinning fixtures are unchanged from the previous round (a boundary-echo + deeper same-text prompt page for M5; a rewind during a capped rebuild for M6; a loadMore failure racing a retention re-anchor for M7).
Not covered
- Real browser renderer memory / long soak — no browser in this container; the A/B proves retention mechanics at the store/client level, not Chrome RSS. The crash scenario is reproduced in shape (oversized replay + live growth through the real reducer), not in cause. Author also states the long-soak before/after was not rerun.
- Provider-level reconciliation corners verified only via the PR's suite, mutation, and census — no independent provider harness (the functions are module-private). F3 quantifies what that means for the three unpinned fixes.
detachStringparent-release — output equality and absence of lone surrogates verified at the cut (C3); no heap snapshot proving V8 releases the oversized parent string.- Per-commit attribution — depth-2 shallow checkout; the previous head
cec40b87is not in the object store, and branch merge43ec3f56lists conflicts intranscript.ts/build.js/DaemonSessionClient.test.ts, so per-commit deltas since the last round are unreachable. Only the aggregateHEAD^1..HEADdiff was verified; the merge into the base tip changed none of the PR's files (verified), which keeps the A/B attribution clean. - Transient parse peak while downloading a large replay response (PR declares unchanged, daemon-side) and daemon-side adaptive journal growth — untouched, not measured.
- Snapshot metadata
baseRefOid(02d303f8…) has drifted past the local base tip; per the CI merge-ref contract the A/B usesHEAD^1(a8a855914b). - The flakiness gate (changed test files × 5 rounds) is run by the workflow lane, not by this agent.
Methodology
Environment: CI verify container (node:22-bookworm, Node v22.23.2), merge-ref checkout at depth 2 (HEAD = merge 8dffd72cc1, HEAD^1 = base a8a855914b, HEAD^2 = head 8447f297e8); npm ci + npm run build pre-run at head. Base side: scratch worktree at HEAD^1, only packages/sdk-typescript rebuilt via its own scripts/build.js with the root node_modules/.bin on PATH (lockfile untouched by the PR; base daemon dist recursively scanned — zero non-relative imports, so the control cannot resolve into head code); worktree removed after capture. Harnesses (ab-retention.mjs, byte-accounting.mjs, f2-toolpreview.mjs) drive the compiled dist directly — real normalizer, real reducer/store, real DaemonSessionClient prototype — with per-arm expected outcomes encoded so predicted base failures count as passed control assertions. Mutants M1–M8 were single-point edits applied by mutate.mjs, run against the targeted vitest file, and restored via git checkout -- (tree verified clean after each). Census probes were temporary __cBump appends in the provider, counted over a green provider-suite run, then restored. Workspace suites run via npx vitest run in each package. Raw logs in logs/. Evidence images rendered by scripts/verify-capture.mjs.
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonSessionClient.test.ts
file packages/sdk-typescript/test/unit/daemonUi.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/daemonUi.test.ts
file packages/web-shell/client/constants/sessions.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/constants/sessions.test.ts
file packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/DaemonSessionProvider.test.tsx
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: PPPPP
packages/sdk-typescript/test/unit/daemonUi.test.ts: PPPPP
packages/web-shell/client/constants/sessions.test.ts: PPPPP
packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/daemonUi.test.ts: P (exit 0)
round 1 · packages/web-shell/client/constants/sessions.test.ts: P (exit 0)
round 1 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 2 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 2 · packages/sdk-typescript/test/unit/daemonUi.test.ts: P (exit 0)
round 2 · packages/web-shell/client/constants/sessions.test.ts: P (exit 0)
round 2 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 3 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 3 · packages/sdk-typescript/test/unit/daemonUi.test.ts: P (exit 0)
round 3 · packages/web-shell/client/constants/sessions.test.ts: P (exit 0)
round 3 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 4 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 4 · packages/sdk-typescript/test/unit/daemonUi.test.ts: P (exit 0)
round 4 · packages/web-shell/client/constants/sessions.test.ts: P (exit 0)
round 4 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 5 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 5 · packages/sdk-typescript/test/unit/daemonUi.test.ts: P (exit 0)
round 5 · packages/web-shell/client/constants/sessions.test.ts: P (exit 0)
round 5 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Verification of round-13 findings at HEAD
|
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
doudouOUC
left a comment
There was a problem hiding this comment.
Comprehensive review — head 8447f29
Requesting changes. I reviewed all 21 changed files, traced the new retention fields through their downstream consumers, audited replay/pagination/trim failure paths and async races, and re-checked every unresolved Critical against this exact head.
Four blockers remain:
- The record-boundary floor back-off still cuts records that fan out into 3+ sibling blocks. The reducer probe on this head retains only
tool-bandtool-cfrom a three-blockrecord-x, permanently orphaningtool-aunder exclusive-before pagination. The correction also needs to preserve a real memory bound; simply retaining an arbitrarily large whole record would trade the data-gap bug for the OOM this PR is fixing. - An oldest-first live trim does not reopen older history when the session started with
hasMore=false, capacityReached=false. The anchor is updated, but the onlyhasMore=truepath is still nested underhistory.capacityReached, so persisted evicted history is unreachable until reload. - Boundary dedup is disabled for empty-text media/file-only optimistic echoes. The persisted twin is prepended as a duplicate because
boundaryEchoText !== ''conflates “no echo” with “empty-text echo”. - Blob/File payload bytes are invisible to the new estimator. A four-file probe retained 32 MiB of Blob data while reporting only 1,920 retained bytes, so failed attachment prompts can bypass the byte ceiling.
Verification completed on this head:
- SDK targeted suites: 388 tests passed.
- WebUI provider suite: 232 tests passed.
- Web Shell constants contract: 1 test passed.
- Targeted ESLint and
git diff --check: passed. - Current remote Test, Web Shell E2E, Linux Desktop, and Windows Desktop checks pass. The zero-second Desktop Shell entry is from a superseded cancelled run, not a code failure.
- A root build reached and successfully built acp-bridge and SDK, then stopped on unrelated CLI/Ink type mismatches in the shared local
node_modules; the current-head remote CI is green for that surface.
The older unresolved Suggestions are non-blocking under the PR's post-five-round Critical-only convergence rule; the four items above are correctness/memory blockers.
中文说明
全面评审结论 — head 8447f29
请求修改。我检查了全部 21 个变更文件,追踪了新增 retention 字段的所有下游消费者,审查了 replay、分页、裁剪的失败路径与异步竞态,并在这个精确 head 上重新验证了所有未解决 Critical。
目前仍有四个阻断问题:
- record 边界 floor 回退只回退一个 block,3 个及以上同 record 兄弟仍被从中间切断,exclusive-before 分页无法找回被驱逐部分。修复时还必须维持真实内存上限,不能简单把任意大的整条 record 全部保留,否则会把数据缺口换回本 PR 要解决的 OOM。
- 初始
hasMore=false, capacityReached=false的会话发生 live trim 后,只更新锚点而不重新开放历史入口;持久化的已驱逐历史在重载前不可达。 - 纯图片/纯文件的本地 optimistic echo 文本为空,
boundaryEchoText !== ''直接禁用了边界去重,分页会插入重复的持久化孪生 user block。 - Blob/File 的真实字节不进入估算器;实测 4 个文件保留 32 MiB,而
retainedBytes只有 1,920,失败的附件 prompt 可以绕过字节上限。
验证结果:SDK 388 tests、WebUI 232 tests、Web Shell 1 test 全部通过;定向 ESLint 与 git diff --check 通过;当前远端主测试、Web Shell E2E、Linux/Windows Desktop 均通过。根 build 在成功构建 acp-bridge 与 SDK 后,被共享本地 node_modules 的 CLI/Ink 版本不匹配阻断,该失败不在本 PR 变更范围,当前 head 的远端 CI 已通过对应表面。
既有未解决 Suggestion 按超过五轮后的 Critical-only 收敛规则不作为本轮阻断;上述四项属于正确性/内存阻断。
…als (QwenLM#9303) Round-13 review fixes: - Count binary payloads in the retention estimate: Blob/File, ArrayBuffer, and typed-array/DataView values carry their content in non-enumerable slots, so the record walk only charged the fixed object overhead — an 8 MiB attachment counted as ~64 bytes and the byte budget never fired for media-heavy transcripts (the OOM class this PR targets). Charge Blob.size / byteLength for these shapes. Adds a budget-eviction regression test. - Loop the record-boundary floor back-off (R12-21 follow-up): the single back-off only re-retained one block, still cutting a 3+-sibling record mid-record. Loop while the boundary pair shares a record, keeping the record whole; the removeCount===0 guard retains the whole window when nothing is evictable. Grows the snap test to a 3-block record. - Offer the load-older affordance on a live trim for sessions that loaded unlatched: eviction re-anchored beforeRecordId but the re-open path only ran under capacityReached, so a session whose replay fit under the caps could never surface older persisted history after the window trimmed. Mirror the replay path's olderHistoryReachable gates in the re-anchor branch. - Key the boundary-echo dedup on echo presence and fold media into the comparison (R12-1 follow-up): an empty-text gate skipped image/file-only echoes entirely, and a naive empty-text match would collapse two distinct media-only prompts. Compare text + image/file counts for the boundary pair.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not explored to full depth (tool budget reached): chunk 9: none — I did not trace materializeTranscriptHistory 's full admission byte math end-to-end (it lives outside my diff lines), but I read enough of it to confirm…; chunk 1: none (actual SDK bundle size vs the 206KB budget is enforced by the CI budget gate, not verified here — noted, not a gap in a check I started)..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 14, not a blocker) — recorded, not requested in this round:
packages/sdk-typescript/src/daemon/ui/toolPreview.ts:331 — [probe] Named-candidate preview loop (Path/Cwd/Query/Note) bypasses the new capDetails cappackages/webui/src/daemon/session/DaemonSessionProvider.tsx:175 — [review] TranscriptHistoryAdmission.reason write-only dead switch (AGENTS.md read-site rule)packages/sdk-typescript/src/daemon/ui/transcript.ts:2036 — [probe] Depth-capped subtrees retained in full but estimated at 0 bytespackages/sdk-typescript/src/daemon/ui/transcript.ts:1280 — [review] upsertPermissionBlock update branch + siblings skip the retainedBytes delta disciplinepackages/sdk-typescript/src/daemon/ui/transcript.ts:901 — [review] "(max blocks reached)" diagnostic misattributes byte-budget evictionspackages/webui/src/daemon/session/DaemonSessionProvider.tsx:446 — [review] Resurrection leaves the one-shot "output trimmed" error status block in the windowpackages/sdk-typescript/src/daemon/ui/utils.ts:62 — [probe] Cap/truncate surrogate-split at the 4096 boundary yields U+FFFD (display-only)packages/sdk-typescript/test/unit/daemonUi.test.ts:1264 — [probe] R12-21 test comment's size model wrong (~200 KB/block); vacuous-retune hazardpackages/webui/src/daemon/session/DaemonSessionProvider.tsx:324 — [probe] Two raw NUL bytes make the 4,523-line provider file binary for grep/ripgrep; \u0000 escape is runtime-identicalpackages/webui/src/daemon/session/DaemonSessionProvider.tsx:3670 — [review] Post-admission hasCapacity count-only while admission is dual-dimension (round-13 I1 rediscovery)packages/sdk-typescript/test/unit/daemonUi.test.ts:1238 (+17 locations) — [probe] Unpinned new branches: 17 surviving mutations / zero-coverage branches across the retention & pagination code
中文说明
仅完成部分审查,审查缺口已披露。
未探索到全部深度(达到工具调用预算):chunk 9:none — I did not trace materializeTranscriptHistory 's full admission byte math end-to-end (it lives outside my diff lines), but I read enough of it to confirm…;chunk 1:none (actual SDK bundle size vs the 206KB budget is enforced by the CI budget gate, not verified here — noted, not a gap in a check I started).。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后(第 14 轮,非阻断)——已记录,本轮不要求修改:共 11 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 AutoFix deferred this item to a human under instruction (round 10/100) — the agent's handoff note below names the decision and the options. The loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. What I found before stopping: Handoff — PR #9303 is not converging; a maintainer decision is neededAutofix stopped this round without any code changes. The workflow growth The decision: how to finish PR #9303 without letting the reconciliation Recommendation (upfront): option 1 — split. Land the current head as the What is landed and verified (head ca93bb0)Rounds 1–13 are in, with build/typecheck/lint/targeted suites green before 中文说明🤖 AutoFix 已按指示将此项移交人工处理(第 10/100 轮)—— 下方 agent 的 handoff 说明列出了待决决策与各选项。循环保持在线,仍会拾取新反馈与 base 冲突,但不会自行重试此项。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32383459737 🧠 Handled by Qwen Code · model/模型 |
…als (QwenLM#9303) Round-14 review fixes: - Clamp the trim floor to keep at least one block (R14-1): a non-positive or non-finite maxBlocks made the count floor evict the whole window, and the record snap then read one past the end of the block array, throwing on every dispatch. Treat maxBlocks < 1 / non-finite as 1 so the window always keeps a block. Adds a regression test. - Drop the rebuild-trim fail-closed anchor unconditionally (R14-2): scanning only the fresh replayEvents missed recordIds trimmed from the repair checkpoint in a marker-visible live-journal repair, leaving a stale anchor with the affordance still on. Mirror the live store's fail-closed branch — when no retained block carries a recordId any pre-trim anchor is stale, and dropping an already-undefined anchor is a no-op. - Grow the latched rejectedPage footprint by a re-anchoring trim's evicted band (R14-3): the daemon re-serves the evicted band on the next exclusive-before fetch, so the page is larger than latched; a stale footprint would churn fetch/reject or misclassify a now-larger page as terminal. The re-open gate now measures the grown page. Reworks the re-open test to the faithful daemon behavior (the latch stays closed once the evicted band re-joins the page). - Refuse anchor-less forced retries (R14-4): after a fail-closed trim drops both cursor and beforeRecordId, a forced retry fetched with neither anchor, and the daemon defaults that to the journal's oldest page — prepended below the window and re-stamping a bogus anchor. Return early until a later trim re-establishes an anchor.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 15, not a blocker) — recorded, not requested in this round:
packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx:13141 (+3 locations) — [probe] Three new retention tests allocate MB-scale payloads (~90 MB / ~70 MB / 8 MiB Blobs) where sibling tests drive the identical byte-trim and admis…packages/sdk-typescript/src/daemon/ui/transcript.ts:1625 — [probe] maxBlocks: Infinity collapses the count floor to 1 and silently over-evicts the whole window except the last record group (latent; no in-tree producer)packages/webui/src/daemon/session/DaemonSessionProvider.tsx:861 — [review] Fail-closed trim branch skips the rejectedPage footprint growth (R14-3's growth sits in the anchored arm); one bounded fetch/reject churn cycle, self-heals on re-lat…packages/webui/src/daemon/session/DaemonSessionProvider.tsx:3538 — [review] Anchor-less force-retry guard (the R14-4 fix) silently no-ops while the paginationError Retry affordance stays visibly latched
中文说明
仅完成部分审查,审查缺口已披露。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后(第 15 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 AutoFix deferred this item to a human under instruction (round 11/100) — the agent's handoff note below names the decision and the options. The loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. What I found before stopping: Maintainer decision needed: PR #9303 is not converging — round stopped with no code changesThe decisionThis PR's diff has stayed over the counting window's growth budget for 7+ rounds What was tried
What remains open
中文说明🤖 AutoFix 已按指示将此项移交人工处理(第 11/100 轮)—— 下方 agent 的 handoff 说明列出了待决决策与各选项。循环保持在线,仍会拾取新反馈与 base 冲突,但不会自行重试此项。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32411144143 🧠 Handled by Qwen Code · model/模型 |
…LM#9303) Round-15 review fix (R15-1): the R14-1 degenerate-maxBlocks clamp floored non-positive and non-finite values to 1 but passed positive fractional values through unchanged, leaving removeCount fractional so the record-boundary snap indexed blocks[removeCount] one past the end and threw on later dispatches — the same crash class R14-1 was added to prevent, re-entering through a sibling input. Floor the clamped value so the window always trims by a whole block count. Math.floor is a no-op for the integer maxBlocks every in-repo caller passes. Extends the degenerate-maxBlocks regression test with a fractional case.
ytahdn
left a comment
There was a problem hiding this comment.
Re-review at head 1fc998038 — verdict: ✅ Approve
本轮复审范围:我上一次 review 之后的 4 个提交(round-12 cec40b8、round-13 ca93bb0、round-14 0cc8d81、fractional maxBlocks 修复 1fc9980)。以下每一项都在 head tree 上逐段核实,而非只看 diff。
此前 4 项 blocker — 全部真实修复
(1)floor back-off 单 if 会切断 3+ 兄弟 record — 已修复。trimTranscriptState 的回退改为 while 循环(packages/sdk-typescript/src/daemon/ui/transcript.ts:1672-1681):循环必然终止(removeCount 每轮减 1)、索引始终有效、每次 re-retain 的兄弟块字节都正确加回。当无可驱逐块时 removeCount === 0 保留整窗而不是 mid-record 切断(transcript.ts:1685)。与新增的 3-block record 回归测试一致。
(2)live trim 不为 unlatched 会话重开旧历史入口 — 已修复。re-anchor 时,若会话以 hasMore=false、capacityReached=false 状态加载,现在会重开 affordance,门控为 pagination feature 支持 + trim 后字节余量(packages/webui/src/daemon/session/DaemonSessionProvider.tsx:833-853),与 replay 路径的门控完全镜像(DaemonSessionProvider.tsx:2172-2178);分支内 anchor 必然已赋值(:801)。
(3)空文本、仅媒体 echo 绕过去重 — 已修复。userBlockBoundaryKey 按 echo 存在性生成键并把 image/file 数量折入键中(DaemonSessionProvider.tsx:317-330),门槛由"文本非空"改为 !== undefined(DaemonSessionProvider.tsx:355-366)。注:键分隔符是两个字面 NUL 字节,属刻意的防碰撞选择;其"文件对 grep 呈 binary"的副作用已在 round-14 延后清单中记录,此处不再重复提出。
(4)Blob/File 载荷对字节估算器不可见 — 已修复。estimateRetainedBytes 对 Blob(含 File)按 .size 计费、对 ArrayBuffer/typed view 按 .byteLength 计费(transcript.ts:2053-2055),并有 typeof Blob 守卫保护非浏览器宿主。计费与驱逐均有新增测试覆盖。
Round-14 criticals 与 fractional maxBlocks — 全部修复并核实
- 退化
maxBlocks夹紧:Math.max(1, Math.floor(Number.isFinite(...) ? ... : 1))(transcript.ts:1623-1626),覆盖 0/负数/小数/NaN;1fc998038 的 floor 使 2.5 按 2 处理。 - rebuild fail-closed 分支无条件丢弃 anchor(DaemonSessionProvider.tsx:2156),配合
loadMoreTranscript中"无 anchor 拒绝 force-retry"的守卫(DaemonSessionProvider.tsx:3533-3538),堵住了"默认请求最旧页、重新盖上伪 anchor"的路径。 - latched
rejectedPagefootprint 按驱逐带增长(DaemonSessionProvider.tsx:812-826):我验证了它依赖的前提——回调触发时store.getSnapshot()仍是 trim 前状态,因为createDaemonTranscriptStore.dispatch在 reduce 返回后才交换state(packages/sdk-typescript/src/daemon/ui/store.ts:64-69)。增长恰好抵消释放的容量(daemon 会在下次 fetch 重新serve被驱逐带),因此 latch 只能经 rewind/重载重开——这正是重写后测试所断言的防 churn 行为,按有意设计看待。
我上一轮的 🟡 I1(hasCapacity 忽略字节预算)— 实质解决
admission 现在是双维度拦截(count:DaemonSessionProvider.tsx:386;bytes::397),capacity latch 的重开门控同样是双维度(:913-916)。成功路径上仅看块数的 hasCapacity(:3697)残余影响至多为:字节饱和而块数有余的窗口下出现一次用户主动的乐观 fetch,随后落入字节感知的 latch;MessageList 侧无自动重试循环,不会 churn。不再阻断;bot ledger 已将此条记录为延后项。
两个记录性观察(非阻断)
💡 Number.isFinite 夹紧会把 Infinity 当作退化输入夹到 1:假想中有调用方传 maxBlocks: Infinity 且字节预算有限时,字节压力下会被 count floor 一次驱逐到 1 块,而非纯字节驱动驱逐。当前无任何调用方传 Infinity(web-shell 全部使用 WEB_SHELL_MAX_TRANSCRIPT_BLOCKS,默认 50k),仅作为 SDK 公开选项的语义提示,不作修改要求。
English
Re-review scope: the four commits since my last review (round-12 cec40b8, round-13 ca93bb0, round-14 0cc8d81, fractional-maxBlocks fix 1fc9980). Every item below was verified against the head tree, not just the diff.
The four previously raised blockers — all genuinely fixed
(1) Floor back-off single-if cut through records with 3+ sibling blocks — fixed. The back-off is now a while loop (packages/sdk-typescript/src/daemon/ui/transcript.ts:1672-1681): it terminates (removeCount decreases each iteration), indexes stay valid, and each re-retained sibling's bytes are added back. When nothing is left to evict, removeCount === 0 keeps the whole window rather than cutting mid-record (transcript.ts:1685). Matches the new 3-block-record regression test.
(2) Live trim didn't re-open older history for unlatched sessions — fixed. On re-anchoring, a session loaded with hasMore=false and capacityReached=false now re-opens the affordance, gated on pagination feature support plus post-trim byte headroom (packages/webui/src/daemon/session/DaemonSessionProvider.tsx:833-853), exactly mirroring the replay-path gate (DaemonSessionProvider.tsx:2172-2178); the anchor is guaranteed in scope (assigned at :801).
(3) Empty-text media-only echo bypassed dedup — fixed. userBlockBoundaryKey keys on echo presence and folds image/file counts into the key (DaemonSessionProvider.tsx:317-330); the gate is !== undefined instead of non-empty text (DaemonSessionProvider.tsx:355-366). Note: the separators are two literal NUL bytes — a deliberate collision-proof choice; the "file looks binary to grep" side effect is already recorded in the round-14 deferred list, so I'm not re-raising it.
(4) Blob/File payloads invisible to the byte estimator — fixed. estimateRetainedBytes charges Blob (hence File) by .size and ArrayBuffer/typed views by .byteLength (transcript.ts:2053-2055), guarded by typeof Blob for non-browser hosts. Both charging and eviction are covered by new tests.
Round-14 criticals and fractional maxBlocks — fixed and verified
- Degenerate
maxBlocksclamp:Math.max(1, Math.floor(Number.isFinite(...) ? ... : 1))(transcript.ts:1623-1626) covers 0/negative/fractional/NaN; the floor in 1fc9980 makes 2.5 behave as 2. - The rebuild fail-closed branch drops the anchor unconditionally (DaemonSessionProvider.tsx:2156), and together with the anchor-less force-retry refusal in
loadMoreTranscript(DaemonSessionProvider.tsx:3533-3538) this closes the "default oldest-page request re-stamps a bogus anchor" path. - Latched
rejectedPagefootprint growth by the evicted band (DaemonSessionProvider.tsx:812-826): I verified the premise it relies on —store.getSnapshot()at callback time is still the pre-trim state becausecreateDaemonTranscriptStore.dispatchswapsstateonly after the reduce returns (packages/sdk-typescript/src/daemon/ui/store.ts:64-69). The growth cancels the freed capacity by design (the daemon re-serves the evicted band on the next fetch), so the latch re-opens only via rewind/reload — exactly the anti-churn behavior the rewritten test asserts; I treat it as intentional.
My previous 🟡 I1 (hasCapacity ignored the byte budget) — effectively resolved
Admission is now dual-dimension (count at DaemonSessionProvider.tsx:386, bytes at :397), and the capacity-latch re-open gate is dual-dimension too (:913-916). The remaining count-only hasCapacity on the success path (:3697) can at worst surface one user-initiated optimistic fetch when the window is byte-saturated with count headroom; it then lands on the byte-aware latch, and the MessageList side has no auto-retry loop. No longer blocking; the bot ledger already tracks this as a deferred item.
Two recorded observations (non-blocking)
💡 The Number.isFinite clamp treats Infinity as degenerate and clamps it to 1: a hypothetical consumer passing maxBlocks: Infinity with a finite byte budget would, under byte pressure, be evicted down to a single block by the count floor instead of pure byte-driven eviction. No current caller passes Infinity (web-shell uses WEB_SHELL_MAX_TRANSCRIPT_BLOCKS throughout; default 50k), so this is a semantics note on the SDK's public option, not a requested change.
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (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: 128 passed · 21 failed · 149 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:128 通过 · 21 失败 · 149 总计 抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9303 Deep Verification (follow-up round) —
|
| # | Finding (previous report, head 8447f29) | Severity | Status at new head 1fc9980 |
|---|---|---|---|
| F1 | retainedBytes accounting misses block-mutation paths |
Suggestion | Stands — re-measured. 13/17 step invariants fail; drift sites this round: applyAssistantUsage −192B, upsertPermissionBlock existing-branch shrink not deducted (+52B un-booked), resolvePermissionBlock −62B, upsertPermissionBlock existing-branch 20KB toolCall replace −79,998B, applySubagentUsageToParentTool −300B; cumulative −80,500B. New reverse-mutant M11 (correct accounting ADDED) leaves the suite green — the axis is unpinned in both directions. Witness 02-f1-byte-accounting-drift.png. |
| F2 | toolPreview named-candidate rows (Path/Cwd/Query/Note) bypass capDetails |
Suggestion | Stands — re-measured. All four named rows retain 100,000 chars at head (4/4); generic control capped at 4111; base identical (pre-existing gap, not a regression). Witness 03-f2-toolpreview-uncapped-rows.png. |
| F3 | R12-1 / R12-2 / R12-10 fixes ship unpinned | Suggestion | Stands — re-measured. Same 3 survivors (M5/M6/M7) under full-file provider runs (237/237 green on each mutant); killed siblings M3/M4 in the same file prove the command collects the coverage. Rounds 13/14 added 5 provider tests (232 → 237) but none pin these guards. Witness 04-mutation-matrix-6-killed-4-survived.png. |
| M1–M4, M8 | Matrix kills (byte budget, snapshot release, rebuild cap, provider consume call, floor back-off) | — | Superseded by this round's matrix — re-run at the new head, same kills, same attributed tests. |
| Gates | sdk 1631 / webui 565 / web-shell 3850 | — | Re-run: sdk 1638 / webui 595 / web-shell 3998 (deltas include this PR's new round-13/14/15 tests; all green). Witness 06-suite-gates-live.png. |
Central claim and A/B
Central claim: the web shell's retention of daemon session history is bounded — replay rebuilds run under the configured block cap and can never ratchet it upward, oversized replays are trimmed to the most recent blocks, the replay snapshot is released after injection, diagnostic payloads are capped at the producer, retention is byte-budgeted with record-boundary-aware eviction and floor back-off, and the trim floor tolerates degenerate/fractional maxBlocks (this round's delta).
Harness ab-retention.mjs imports the compiled dist/daemon/index.js of head (rebuilt from the verified merge tree) and of base (tmp/base-tree at HEAD^1 = 5715782279, sdk-typescript rebuilt there against the root node_modules; lockfile untouched by the PR). Control purity: both arms' daemon bundles have zero non-relative imports (import-scan quoted in Methodology), so neither cell can resolve into the other tree. Witness: 01-ab-retention-head-vs-base.png.
| # | Cell (oracle) | base (5715782) | head (1fc9980) |
|---|---|---|---|
| C1 | Diagnostic cap — 4 producers × 100KB payload | 4/4 UNcapped (≥90KB embedded; sidechannel/block routing identical) | 4/4 capped at exactly 4111 |
| C2 | Cap ladder around 4096 (pre-cap 3000/4090/4096/4097/5000) | uncapped at every rung | ≤4096 byte-identical / >4096 → exactly 4111 |
| C3 | Astral payload straddling the 100k text-block cut | capped (pre-existing) but raw slice leaves a lone surrogate | capped, no lone surrogate (detachString UTF-8 round-trip → U+FFFD) |
| C4 | Replay rebuild 300 blocks, cap 100, +50 live (provider lines emulated verbatim per arm) | 300 retained; committed cap ratcheted to 300; live window trims at the ratcheted 300 | trimmed to 100; committed cap 100; live growth trims at 100 with evictedOldest truncation detail |
| C5 | Byte budget — 100 × ~120KB blocks, budget 2MB | no mechanism (maxRetainedBytes undefined); all 100 retained |
evicted below 100; retained ≤ budget + worst block; accounting exact on all 100 pure-append steps |
| C6 | Record-boundary snap — 5-block record-A + 5-block record-B + trigger, cap 8 | naive cut straddles record-A (keeps a4/a5 of 5) | cut advanced past the record; window = 5 × record-b + trigger, no partial record |
| C7 | Floor back-off — ~100KB blocks sharing a record, budget 150KB | no byte mechanism; all kept trivially | shared-record pair re-retained (floor backed off); distinct-record control evicts the unshared head |
| C8 | Snapshot release (consumeReplaySnapshot) |
method absent; snapshot pinned for client lifetime | returns snapshot once, swaps empty, idempotent |
| C9 | toolPreview generic row (100KB scalar) | UNcapped (≥99KB) | capped at 4111 |
| S1 | Store seeding counts retained bytes | retainedBytes undefined | seeded retainedBytes = Σ estimates (368 = 368) |
| D1 | delta — maxBlocks=0 (R14-1) | n/a (base slice(-0) keeps all, no crash — hazard is PR-introduced) |
no throw; ≥1 block kept |
| D2 | delta — maxBlocks=2.5, 5 distinct blocks (R15-1) | n/a (base slice truncates fractionals harmlessly) |
no throw; keeps exactly floor(2.5)=2 newest blocks |
| D2b | delta — the PR's own R15-1 fixture (5× user.text.delta) |
— | census: 1 merged block → trim never reached (see F4) |
| D3 | delta — degenerate ladder NaN / −5 / Infinity | — | no throw; NaN→1 kept, −5→1 kept, Infinity→all 5 kept (early-return guard) |
Result: 54/54 head, 39/39 base — every head behavior holds and every base control behaves exactly as predicted. C4 emulates each arm's provider lines verbatim (base: replayMaxBlocks = Number.MAX_SAFE_INTEGER + committedMaxBlocks = Math.max(maxBlocks, replayState.blocks.length), base provider L1683-1722; head: replayMaxBlocks = maxBlocks + committedMaxBlocks = replayMaxBlocks, head provider L2013-2090).
Delta assessment (R14-1 / R15-1): both guards are real and load-bearing — mutant M10 (clamp removed) throws TypeError: Cannot read properties of undefined (reading 'sourceRecordIds') in the intended test, and mutant M9 (floor removed) crashes the harness's D2 cell with the same error while 5-block windows trim to exactly 2. Attribution note: the degenerate/fractional hazard is specific to the PR's snap-index arithmetic; base's naive slice(-maxBlocks) coerces fractionals without crashing (D-base cells), so R14-1/R15-1 defend a mechanism this PR introduced. The one defect found in the delta is the R15-1 test fixture (F4), not the fix.
Bundle budget: dist/daemon/index.js = 206,355 B ≤ 210,944 B (206KB budget); every rebuild exercised assertBrowserSafeBundle inside scripts/build.js and passed.
Findings
F1 (Suggestion, carried forward — stands) — retainedBytes accounting still misses block-mutation paths
Re-measured at the new head with byte-accounting.mjs (17 step invariants across append/growth/tool/permission/subagent/attachment/trim paths; asserts retainedBytes === Σ estimate(blocks) after every step). 13/17 step invariants fail; the invariant breaks at S04 and never recovers (cumulative drift −80,500B). Witness 02-f1-byte-accounting-drift.png.
| Step | Path | Unaccounted delta |
|---|---|---|
| S04 | applyAssistantUsage (usage object on active assistant) |
−192B |
| S08 | upsertPermissionBlock existing-branch (small replace shrinks block) |
+52B never deducted |
| S09 | resolvePermissionBlock (resolved/eventId on existing block) |
−62B |
| S10 | upsertPermissionBlock existing-branch (20KB toolCall replace) |
−79,998B |
| S12 | applySubagentUsageToParentTool (executionSummary on parent tool) |
−300B |
Direction is under-count (one missing deduction), so the effective ceiling is budget + worst-case block + accumulated unaccounted deltas — bounded, no correctness hazard, but contradicts the PR's exactness invariant. New this round: reverse-mutant M11 added correct measure-before/delta-after accounting to applyAssistantUsage — the suite stayed green (324/324) while the harness's S04 flipped FAIL→PASS (drift 0), then restored (FAIL again). The suite pins nothing along these axes in either direction. Repro: node tmp/pr9303-verify-20260821-024938/byte-accounting.mjs. The previously validated scratch fix was again not applied by the PR.
F2 (Suggestion, carried forward — stands) — toolPreview named-candidate rows still bypass capDetails
Re-measured: createDaemonToolPreview({ path|cwd|query|description: 'x'.repeat(100_000) }) retains 100,000 chars in the Path/Cwd/Query/Note rows on head (4/4), while the generic path caps at 4111 (control). Base is identical (uncapped everywhere) — residual gap in the PR's own hardening, not a regression; retention impact bounded because the row holds the same string reference the retained rawInput already carries. Witness 03-f2-toolpreview-uncapped-rows.png. One-line fix unchanged: route collectPreviewRows' named-candidate push through capDetails too.
F3 (Suggestion, carried forward — stands) — R12-1 / R12-2 / R12-10 still unpinned by any test
Mutation matrix at the new head (witness 04-mutation-matrix-6-killed-4-survived.png; positive controls: unmutated targeted files green — daemonUi 324/324, provider 237/237 — and six siblings killed by the same commands):
| Mutant | Single-point change | Result | Evidence |
|---|---|---|---|
| M1 | overByteBudget = false |
KILLED | sdk: evicts oldest blocks to stay under the retention byte budget + 2 more (3 failed) |
| M2 | consumeReplaySnapshot keeps the snapshot |
KILLED | sdk: releases the replay snapshot once consumed (1/69 failed) |
| M3 | rebuild cap → Number.MAX_SAFE_INTEGER |
KILLED | provider: 6 tests incl. trims an oversized initial replay to the block cap and re-anchors older pagination, uses a bounded full-snapshot fallback after the marker block is trimmed |
| M4 | provider consumeReplaySnapshot() call removed |
KILLED | provider: releases the replay snapshot after injection and never raises the block cap |
| M5 (R12-1) | boundary-dedup break removed → window-wide text keying |
SURVIVED | provider 237/237 green |
| M6 (R12-2) | rewind gate dropped from rebuild observeReplayTrim |
SURVIVED | provider 237/237 green |
| M7 (R12-10) | loadMore catch generation guard → if (false) |
SURVIVED | provider 237/237 green |
| M8 (R12-21) | floor back-off disabled | KILLED | sdk: both R12-21 back-off tests (2 failed) |
| M9 (R15-1) | Math.floor removed from trim floor |
SURVIVED the suite — but see F4 | sdk 324/324 green; harness D2 crashes |
| M10 (R14-1) | Math.max(1, …) clamp removed |
KILLED | sdk: the new tolerates a degenerate maxBlocks… (R14-1) test, failing at its .not.toThrow() with the exact TypeError the clamp prevents |
| M11 | F1 reverse: correct accounting ADDED | green (expected) | proves F1 axis unpinned; harness S04 flips PASS↔FAIL with it |
M5/M6/M7 classify as coverage gaps, not defects — the fixes read correctly and the killed siblings prove the pattern testable. Pinning fixtures unchanged from the previous round.
F4 (Suggestion, NEW this round) — the R15-1 fractional subcase of the new test never reaches the trim floor
The new test tolerates a degenerate maxBlocks without crashing the trim (R14-1) adds an R15-1 subcase: a store with maxBlocks: 2.5 receiving five user.text.delta dispatches, asserted not to throw. Census (D2b): five consecutive user.text.delta events merge into one block via the active-user-block path, and 1 <= 2.5 takes the trim's early return — the fractional removeCount path is never executed. Proof chain (05-r15-vacuity-proof.png):
- Mutant M9 (remove
Math.floor, keep the clamp) →daemonUi.test.tsstays 324/324 green. - Same mutant, rebuilt dist, harness D2 (five distinct tool blocks at cap 2.5, which does reach trim) → crashes:
TypeError: Cannot read properties of undefined (reading 'sourceRecordIds')— the floor is load-bearing. - Restored head: suite green again and harness back to 54/54.
The fix itself is correct and verified (D2/D3 at head); only the test's fixture under-covers it. Suggested change (measured, not eyeballed): in the fractional subcase, create ≥3 blocks that cannot merge — e.g. five tool.update events with distinct toolCallIds, exactly the harness D2 fixture. With that fixture the mutant is caught (it crashes at dispatch 3), and head stays green (D2: keeps exactly 2 blocks). No production hazard: this is a test-strength gap on a guard that otherwise holds.
Not covered
- Real browser renderer memory / long soak — no browser in this container; the A/B proves retention mechanics at the store/client level, not Chrome RSS. The crash scenario is reproduced in shape (oversized replay + live growth through the real reducer/store), not in cause. Author also states the long-soak before/after was not rerun.
- Provider-level reconciliation corners verified only via the PR's suite, mutation, and the round's mutants — the reconciliation functions are module-private; F3 quantifies what that means for the unpinned guards.
detachStringparent-release — output equality and absence of lone surrogates verified at the cut (C3); no heap snapshot proving V8 releases the oversized parent string.- Per-commit attribution — depth-2 shallow checkout: 1 commit reachable vs 28 in the snapshot (verified mismatch, not a bare count). Only the aggregate
HEAD^1..HEADdiff was verified; the round-13/14/15 commits cannot be separated, so their individual claims are covered only in aggregate (R14-1/R15-1 behavior was probed directly). - Offline/export projection (
projectChatRecordsToDaemonTranscriptbyte-budget opt-out viamaxRetainedBytes: POSITIVE_INFINITY) — covered by the sdk suite only (not exported from the daemon barrel the harness imports); no independent harness. - Transient parse peak while downloading a large replay response and daemon-side adaptive journal growth — untouched by the PR, not measured.
- Snapshot metadata
baseRefOid(02d303f8…) has drifted past the local base tip; per the CI merge-ref contract the A/B usesHEAD^1(5715782279). - The flakiness gate (changed test files × 5 rounds) is run by the workflow lane, not by this agent.
Methodology
Environment: CI verify container (node:22-bookworm, Node v22.23.2), merge-ref checkout at depth 2 (HEAD = merge adb2bf5268, HEAD^1 = base 5715782279, HEAD^2 = head 1fc9980382); npm ci + npm run build pre-run at head. Base side: scratch worktree at HEAD^1, only packages/sdk-typescript rebuilt via its own scripts/build.js with the root node_modules/.bin on PATH (lockfile untouched by the PR — clean control). Control purity: recursive import scan of both arms' dist/ shows the daemon bundle has zero non-relative imports on both sides, so neither arm can resolve into the other tree (@qwen-code/* workspace symlinks resolve into the head tree, but nothing in either daemon bundle imports them). The head daemon dist was rebuilt twice during mutant cycles (M9, M11) and restored to 54/54 after each. Harnesses (ab-retention.mjs, byte-accounting.mjs, f2-toolpreview.mjs) drive the compiled dist directly — real normalizer, real reducer/store, real DaemonSessionClient prototype — with per-arm expected outcomes encoded so predicted base behavior counts as passed control assertions; two intermediate fixture errors found during bring-up (C3 surrogate placement, C7 trigger record) were corrected before the final tallies. Mutants M1–M11 were single-point edits applied by mutate.mjs/mutate-m11.mjs (anchor-uniqueness enforced), run against the targeted vitest file, and restored via git checkout -- (tree verified clean after each; git status --porcelain empty at the end). Workspace suites run via npx vitest run in each package, once in background and once live inside capture 06 (identical counts). Evidence images rendered by scripts/verify-capture.mjs from live runs. Raw logs in logs/.
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonSessionClient.test.ts
file packages/sdk-typescript/test/unit/daemonUi.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/daemonUi.test.ts
file packages/web-shell/client/constants/sessions.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/constants/sessions.test.ts
file packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/DaemonSessionProvider.test.tsx
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: PPPPP
packages/sdk-typescript/test/unit/daemonUi.test.ts: PPPPP
packages/web-shell/client/constants/sessions.test.ts: PPPPP
packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/daemonUi.test.ts: P (exit 0)
round 1 · packages/web-shell/client/constants/sessions.test.ts: P (exit 0)
round 1 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 2 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 2 · packages/sdk-typescript/test/unit/daemonUi.test.ts: P (exit 0)
round 2 · packages/web-shell/client/constants/sessions.test.ts: P (exit 0)
round 2 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 3 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 3 · packages/sdk-typescript/test/unit/daemonUi.test.ts: P (exit 0)
round 3 · packages/web-shell/client/constants/sessions.test.ts: P (exit 0)
round 3 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 4 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 4 · packages/sdk-typescript/test/unit/daemonUi.test.ts: P (exit 0)
round 4 · packages/web-shell/client/constants/sessions.test.ts: P (exit 0)
round 4 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 5 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 5 · packages/sdk-typescript/test/unit/daemonUi.test.ts: P (exit 0)
round 5 · packages/web-shell/client/constants/sessions.test.ts: P (exit 0)
round 5 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
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
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.22.0. |











What this PR does
Bounds how much daemon session history the web shell retains in the browser. The raw replay snapshot fetched when a session loads is released as soon as it has been injected into the transcript store. Replay rebuilds now run under the same block cap as live growth, and a replay can no longer ratchet that cap above its configured limit — oversized replays are trimmed to the most recent blocks instead. Diagnostic blocks for unrecognized session update kinds embed a capped excerpt rather than the full payload. The subagent detail pane receives the same block cap as the rest of the web shell, and the implicit provider default retention window is lowered.
Why it's needed
Watching a busy daemon session — for example a long single turn fanning out to many subagents — could exhaust the browser renderer's memory and crash the tab. Confirmed from a real crash: a session that produced well over 100,000 session-update events during a 51-minute turn drove its Chrome renderer past 5GB RSS before it aborted (breakpoint-style crash signature consistent with heap exhaustion), and every reload of the same session re-ingested the uncapped replay and grew back to multiple GB within minutes. Crashpad dumps show the same renderer crash signature recurring over several days. The daemon's per-session replay window grows adaptively for in-flight turns (up to hundreds of MB), so the client must not retain or rebuild it without a bound.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before (observed live): the renderer hosting the web shell grew 2.0GB → 4.2GB → 5.1GB while streaming busy sessions, crashed with a breakpoint-style abort at ~10:11 today (Crashpad dump timestamp matches the SSE disconnect to the second), and the replacement renderer regrew to 2GB within ~5 minutes of reload; Crashpad holds identical-signature renderer dumps from Aug 10/14/15/16. Root causes confirmed by code path: the replay snapshot pinned on the session client for the attachment lifetime, an uncapped replay rebuild, and a replay-size escalation of the committed block cap.
After: the retention paths are bounded by construction and covered by the regression tests listed above (cap never raised, snapshot released at injection, per-frame diagnostic text capped, all providers on the 50k cap). A long-soak browser before/after comparison was not rerun.
Tested on
Environment (optional)
Local daemon + web shell via
npm run devon macOS (the crash investigation ran against the live daemon on 127.0.0.1:4170); unit suites via vitest.Risk & Scope
Linked Issues
None — found while investigating a live renderer crash; no existing issue to close.
中文说明
本 PR 做了什么
限制 Web Shell 在浏览器中保留的守护进程会话历史量。会话加载时拉取的原始 replay 快照在被注入 transcript store 后立即释放。replay 重建现在与实时增长使用同一个 block 上限,且 replay 不再能把该上限抬高到配置值以上——超限的 replay 会被修剪为最近的若干 block。针对未识别 session update 类型的诊断 block 只嵌入截断后的摘要,而不是完整载荷。subagent 详情面板现在与 Web Shell 其余部分使用相同的 block 上限,provider 的隐式默认保留窗口也相应调低。
为什么需要
观看一个繁忙的守护进程会话(例如单个超长 turn 扇出大量 subagent)可能耗尽浏览器 renderer 的内存并导致标签页崩溃。来自真实崩溃的确认:一个在 51 分钟的 turn 中产生了远超 10 万条 session-update 事件的会话,使其 Chrome renderer 在 abort 前超过 5GB RSS(断点式崩溃签名,与堆耗尽一致),并且每次重新加载同一会话都会重新吸入无上限的 replay,在几分钟内重新涨到数 GB。Crashpad 转储显示同一 renderer 崩溃签名已连续多天反复出现。守护进程的每会话 replay 窗口会对进行中的 turn 自适应增长(可达数百 MB),因此客户端必须对其保留与重建设置上限。
评审测试计划
如何验证
证据(修复前后)
修复前(实际观测):承载 Web Shell 的 renderer 在流式接收繁忙会话时从 2.0GB → 4.2GB → 5.1GB 增长,今天 10:11 以断点式 abort 崩溃(Crashpad 转储时间戳与 SSE 断开精确到秒一致),重载后的新 renderer 约 5 分钟内重新涨到 2GB;Crashpad 中存有 8 月 10/14/15/16 日同签名的 renderer 转储。根因已通过代码路径确认:replay 快照在整个 attach 生命周期被固定在会话客户端上、replay 重建无上限、以及按 replay 规模抬高 committed block 上限。
修复后:上述保留路径在构造上即有界,并由前述回归测试覆盖(上限不被抬高、注入时释放快照、每帧诊断文本截断、所有 provider 统一 50k 上限)。未重新进行长时间浏览器前后对照浸泡测试。
测试环境
环境(可选)
macOS 上通过
npm run dev运行本地 daemon + Web Shell(崩溃排查针对 127.0.0.1:4170 上的真实 daemon 进行);单测通过 vitest 运行。风险与范围
关联 Issue
无——排查线上 renderer 崩溃时发现,没有可关闭的既有 issue。