feat(serve): adaptively grow live-journal caps before truncating mid-turn replay - #8905
Conversation
…turn replay A single turn fanning out many concurrent subagents (e.g. a /review run) can emit hundreds of thousands of source events, far past the per-session live-journal baseline caps (10 000 entries / 8 MiB), so a mid-turn (re)load silently shows a truncated replay until the turn finishes. Before evicting, the engine now asks a growth advisor: caps double (entries scaled proportionally) while the growth granted across the bridge's live sessions fits in a pool derived from the daemon memory budget (5%, clamped to [32, 1024] MB), never past a per-session hard cap of 256 MiB. Growth is on demand, throttled after a refusal, and accounted statelessly from the current caps of all live sessions, so granted headroom dies with its session. An operator-pinned --max-journal-events/--max-journal-bytes disables growth; without a pool the fixed-cap eviction behavior is unchanged.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Re-run after a sixth autofix round — head advanced from Template looks good ✓ Problem: observed, not theoretical — unchanged from earlier passes. A single Direction: aligned, unchanged. Live-replay fidelity for fan-out sessions is squarely Size: cross-package ( Approach: the scope still feels right. The new commit is exactly the round-6 feedback and nothing else: valueless Risk: no high-risk path matches from the revert-history signal. Moving on to code review. 🔍 中文说明第六轮 autofix 后的重新运行——本次运行刚启动时 head 恰好从 模板完整 ✓ 问题：已观测到的真实问题，不是理论假设——与早前各轮结论一致。单个 方向：对齐，不变。扇出会话的实时回放保真度正是 规模：跨包改动（ 方案：范围仍然合理。新提交恰恰是第六轮反馈本身，别无其他：无值的 风险：回滚历史信号无高风险路径命中。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
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. |
|
Code review of What landed since the last pass, and my read of it:
No critical findings this pass; nothing blocking. The one standing nit is unchanged and non-blocking: the PR body is stale in three places (per-bridge pools, the sequenceDiagram
participant P1 as Compaction Engine
participant P2 as Bridge advisor
participant P3 as Growth Policy
participant P4 as Daemon-wide aggregator
P1->>P2: journal breaches a cap, asks before evicting
P2->>P4: current caps and baselines of every live session, all bridges
P2->>P3: grant request with that aggregate
P3-->>P2: doubled caps within the remaining pool, or refusal
alt grant improves retention
P2-->>P1: caps raised in place, no eviction needed
else grant does not yet improve retention
P1->>P2: applied tentatively, walk re-asks toward an improving grant
P2-->>P1: rolled back to original caps and evicted if the walk never improves
else refusal
P2-->>P1: oldest-first eviction as before, re-ask throttled 10s
end
Files changed (29)
Testing — the PR's own CI on Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification status: the central claim is behavioural, and it is already substantiated — the earlier 中文说明对 上轮之后落地的改动及我的判断：
本轮无 critical 发现，无阻塞项。唯一持续的非阻塞卫生问题：PR 正文三处过期（per-bridge 池、实为 测试—— **沙箱验证状态：**核心主张是行为性的，且已被证实——更早的 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean across every stage on the current head; both round-6 findings resolved with regression tests; the only open items are the two primary CI lanes still running on Stepping back: this PR has converged. The approach still matches my independent proposal point for point, and the review process demonstrably worked — six rounds didn't just patch findings, they improved the design: one daemon-wide pool instead of one per bridge, in-flight restores accounted, zero-benefit grants refused, per-session baselines so mixed-baseline bridges can't mischarge each other, the tentative-grant walk with rollback, and now The honest reservations, both non-blocking: CI on the newest head hasn't landed yet (unit suite and Serve A/B still in flight; everything settled so far is green, and the prior head ran fully green with only 31 lines of parser strictness and test pins added since), and the PR body remains stale in three places. Neither is a reason to hold. Housekeeping: the outstanding Approval deferred until CI lands green on 中文说明置信度：4/5 —— 当前 head 每个阶段都干净；第六轮两项发现均已带回归测试解决；唯一未落定的是 整体看：这个 PR 已经收敛。方案与我独立提出的逐点对应；评审过程也证明其有效——六轮不只是修补发现，而是改进了设计：单一 daemon 级池取代 per-bridge 池、在途 restore 计入记账、零收益授予被拒绝、按会话基线使混合基线的 bridge 互不误计费、带试探性授予游走与回滚、以及现在的 诚实的保留意见（均不阻塞）：最新 head 的 CI 尚未落定（单测套件与 Serve A/B 仍在运行；目前已落定的全绿，且上一 head 全绿、其后仅新增 31 行解析严格化与测试固定）；PR 正文仍有三处过期。两者都不构成搁置理由。 事务性说明：本 PR 上未决的 批准将延后至 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
🩺 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 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — stopped at the 5-round cap; round 5 returned no new confirmed findings, but two consecutive dry rounds were never established (rounds 1-4 each reported confirmed findings).
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all planned checks completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget..
中文说明
未审查：reverse audit — stopped at the 5-round cap; round 5 returned no new confirmed findings, but two consecutive dry rounds were never established (rounds 1-4 each reported confirmed findings)。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all planned checks completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
@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. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed — PR #8905 (round: address-review)Commit: All 18 inline findings (1 Critical, 17 Suggestions) were verified against the code and resolved in code. Nothing was declined, deferred, or escalated. The issue-level Critical
Suggestions — code behavior
Suggestions — tests
Suggestions — docs, help text, comments
Conflict notesNone — VerificationCommands actually run (all on the final committed tree unless noted):
Integration tests after 中文说明已处理的评审反馈 — PR #8905（轮次：address-review）提交： 全部 18 条 inline 发现（1 条 Critical、17 条 Suggestion）均已对照代码核实并在代码中解决。没有拒绝、推迟或升级任何条目。issue 级评论 Critical
Suggestion — 代码行为
Suggestion — 测试
Suggestion — 文档、帮助文本、注释
冲突说明无 — 验证实际执行的命令（除注明外在最终提交的代码树上运行）：
未运行 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.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped at the 5-round cap without two consecutive dry rounds (rounds 1-5 each reported findings); the cap-round finding was verified separately.
Not explored to full depth (tool budget reached): chunk 8: executing the four new tests via npx vitest run — blocked by missing node_modules in the review worktree (startup error ERR_MODULE_NOT_FOUND for vitest )…; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed (~12 of ~46 tool calls used).; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; chunk 5: running the new tests ( npx vitest run src/compactionEngine.test.ts -t 'adaptive live-journal growth' ) — the worktree has no node_modules and a full monorepo i….
Test Plan (not a blocker): 1221 tests pass — this review observed 1226, 18910, 1502, 481, 3069, 473 passed.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：reverse audit — stopped at the 5-round cap without two consecutive dry rounds (rounds 1-5 each reported findings); the cap-round finding was verified separately。
未探索到全部深度（达到工具调用预算）：chunk 8：executing the four new tests via npx vitest run — blocked by missing node_modules in the review worktree (startup error ERR_MODULE_NOT_FOUND for vitest )…；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed (~12 of ~46 tool calls used).；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；chunk 5：running the new tests ( npx vitest run src/compactionEngine.test.ts -t 'adaptive live-journal growth' ) — the worktree has no node_modules and a full monorepo i…。
Test Plan（非阻断）：1221 tests pass — this review observed 1226, 18910, 1502, 481, 3069, 473 passed。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
@qwen-code /resolve |
…rnal-growth # Conflicts: # docs/developers/daemon/17-configuration.md
) Concurrent restores hold their buses in pendingRestoreEvents rather than byId, so each advisor ask only saw its own caps and concurrent restores could each draw a full doubling from the same pool. Sum the current caps of every in-flight restore bus into allSessionLimitBytes. Also skip the growth ask when the breaching append is a turn boundary — compactCurrentTurn discards the journal immediately afterwards, so the grant would be charged to the pool while buying zero eviction. Pin the previously untested contracts with tests: restore-window accounting, concurrent-restore accounting, headroom release on session close, the hard-cap clamp term, partial-grant eviction, requester discrimination in the policy fixtures, the maxEvents safe-integer conjunct, and the dynamic-workspace bridge pool wiring. Fix the docs: add the missing journal-flag rows to the daemon configuration and operations pages, and correct the effective-budget definition.
|
Qwen Code did not run conflict resolution for this request. PR #8905 does not currently have merge conflicts with main. |
…8905) The merge of main pulled in #8933, which gates historyPageSize on historyReplay === 'response'. The 'transport failure marks the channel dying before process exit' test (from #8947) passes historyPageSize with the default stream replay, so the paged transcript fetch it waits on is never issued and the test times out — a cross-PR interaction between two main commits, failing deterministically on main. Pin the response replay mode the paged fetch requires.
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round: no code changes needed — every actionable finding in this round's feedback was already fixed at HEAD ( Round-2 review findings — all resolved in code at HEAD
Failed check:
|
|
@qwen-code /resolve |
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge resolution: PR #8905 ← origin/mainRoot causeMain merged Textual or semanticNeither logic collided — the two sides were functionally identical. The HEAD const replayAttach = bridge.loadSession({
sessionId: first.sessionId,
workspaceCwd: WS_A,
// historyPageSize only takes effect in response-mode replay (#8933);
// without historyReplay the request silently degrades to 'stream',
// no transcript page is ever requested, and the waitFor below times
// out. This test predates that gating (logical merge conflict — both
// PRs were green on their own branches).
historyReplay: 'response',
historyPageSize: 10,
clientId: 'rejected-load-client',
});Verified: the resolved test now matches origin/main's copy byte-for-byte, and the What is load-bearingThe comment documents the invariant: Not verified hereNo build/tests run per instructions. Note for CI: the merged tree now contains 中文说明根因：main 合入了 文本还是语义：两边功能完全一致，无逻辑碰撞。冲突中 HEAD 一侧为空， 关键约束：该测试中 未验证：按指令未运行构建/测试。提醒 CI：合并后的树同时包含 #8933 的 |
doudouOUC
left a comment
doudouOUC
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 agen...: none** — all planned checks completed within the tool budget..
Test Plan (not a blocker): 1221 tests pass — this review observed 1309, 1510, 499, 3277, 523 passed.
中文说明
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none** — all planned checks completed within the tool budget.。
Test Plan（非阻断）：1221 tests pass — this review observed 1309, 1510, 499, 3277, 523 passed。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — stopped at the 5-round cap without global two-consecutive-dry convergence.
Test Plan (not a blocker): 1221 tests pass — this review observed 1309, 1510, 499, 3277 passed.
中文说明
未审查：reverse audit — stopped at the 5-round cap without global two-consecutive-dry convergence。
Test Plan（非阻断）：1221 tests pass — this review observed 1309, 1510, 499, 3277 passed。
— qwen3.7-max via Qwen Code /review (v0.21.10)
| const journalGrowthPoolBytes = | ||
| opts.maxJournalEvents === undefined && | ||
| opts.maxJournalBytes === undefined && |
There was a problem hiding this comment.
[Critical] Make the growth pool daemon-scoped rather than allocating the full value to every bridge. Each startup or dynamic workspace receives this same 5% pool, and the 32 MiB floor remains active even when the resolved budget has no usable headroom. Concurrent large turns across workspaces can therefore retain more journal memory than the configured daemon budget permits and drive the process into memory pressure/OOM. Please share one allocator across bridges (or partition one aggregate pool), and disable/bound growth when the budget resolution reports insufficient memory or no post-reserve headroom.
中文翻译
[严重] 请将增长池改为 daemon 级共享，而不是给每个 bridge 分配完整额度。当前每个启动或动态 workspace 都得到同一个 5% 池，并且即使解析后的预算没有可用余量，32 MiB 下限仍会生效。多个 workspace 同时出现大 turn 时，保留的 journal 内存可能超过 daemon 配置预算并导致内存压力或 OOM。建议所有 bridge 共享一个分配器（或切分一个总池），且当预算解析报告内存不足或预留后无余量时禁用或限制增长。
— qwen3.7-max via Qwen Code /review (v0.21.10)
| if ( | ||
| this.journalGrowthDeniedAt !== undefined && | ||
| now - this.journalGrowthDeniedAt < JOURNAL_GROWTH_REASK_INTERVAL_MS |
There was a problem hiding this comment.
[Critical] Use a monotonic clock for the refusal throttle. now defaults to Date.now(), so after a denied grant an NTP/manual backward correction makes this difference negative and suppresses advisor calls until wall time catches up. During that interval the engine evicts replay history even if pool headroom has become available. Please use an injectable monotonic elapsed-time source such as performance.now() and cover a backward wall-clock correction.
中文翻译
[严重] 拒绝后的节流应使用单调时钟。now 默认来自 Date.now()，因此一次拒绝后若 NTP 或人工将系统时间向后校正，该差值会变成负数，直到墙上时间追上前都不会再次调用 advisor。在此期间，即使增长池已有余量，引擎仍会淘汰 replay 历史。请使用可注入的单调耗时源（如 performance.now()），并覆盖系统时间回拨的测试。
— qwen3.7-max via Qwen Code /review (v0.21.10)
| const journalGrowthPoolBytes = | ||
| opts.maxJournalEvents === undefined && | ||
| opts.maxJournalBytes === undefined && |
There was a problem hiding this comment.
[Critical] Update /daemon/status for the new ownership and semantics. Once this pool is active, limits.memory.enforced: false no longer accurately describes the memory budget as wholly observational, while bridge status still publishes the closed-over 10,000/8 MiB baselines as maxJournalEvents/maxJournalBytes after sessions can grow beyond them. Operators and SDK clients will underestimate retained memory and misdiagnose whether --memory-budget-mb has runtime effect. Please expose the baseline, adaptive-growth enablement, aggregate pool ownership/size, hard cap, and effective/current session limits, and scope enforced: false specifically to the child-heap model.
中文翻译
[严重] 请同步更新 /daemon/status 的所有权与语义。启用该池后，limits.memory.enforced: false 已不能准确表示内存预算完全只是观测值；同时 bridge 状态仍把闭包中的 10,000/8 MiB 基线作为 maxJournalEvents/maxJournalBytes 返回，即使 session 已能增长到更高。运维人员和 SDK 客户端会低估保留内存，并错误判断 --memory-budget-mb 是否产生运行时效果。请暴露基线、自适应增长开关、总池所有权/大小、硬上限及 session 当前有效上限，并将 enforced: false 明确限定为 child-heap 模型。
— qwen3.7-max via Qwen Code /review (v0.21.10)
| const hardCapEvents = Math.max( | ||
| opts.baselineEvents, | ||
| Math.ceil((opts.hardCapBytes / opts.baselineBytes) * opts.baselineEvents), |
There was a problem hiding this comment.
[Critical] Clamp the proportional event hard cap to Number.MAX_SAFE_INTEGER. baselineEvents and the other inputs may each be valid safe integers, but their product/ratio can exceed the safe range here. The policy then returns an unsafe maxEvents, which the engine rejects together with an otherwise useful byte grant, causing truncation despite available headroom. Add a regression case with a large valid baseline.
中文翻译
[严重] 请将按比例计算的事件硬上限限制在 Number.MAX_SAFE_INTEGER。baselineEvents 及其他输入单独都可能是合法安全整数，但这里的乘除结果仍可能超出安全范围。policy 随后会返回不安全的 maxEvents，引擎会连同本来可用的 byte grant 一起拒绝，从而在仍有余量时发生截断。请增加大合法基线的回归测试。
— qwen3.7-max via Qwen Code /review (v0.21.10)
| if ( | ||
| !TURN_BOUNDARY_TYPES.has(event.type) && | ||
| (this.liveJournal.length > this.maxJournalEvents || |
There was a problem hiding this comment.
[Critical] Do not commit/account growth unless it improves retention. For an event larger than the 256 MiB hard cap, this path repeatedly asks for and permanently accounts larger session caps up to 256 MiB, but eviction still retains exactly one oversized event after every append. A stream of such events can consume the shared pool without preserving any additional replay and deny useful headroom to other sessions. Evaluate the post-grant retained set before accepting/accounting the grant, or otherwise refuse grants that cannot change the eviction result.
中文翻译
[严重] 只有增长确实改善保留结果时才应提交并记账。对于大于 256 MiB 硬上限的事件，此路径会反复申请并永久计入更大的 session 上限直至 256 MiB，但每次追加后淘汰仍只保留一个超大事件。连续此类事件会耗尽共享池，却没有多保留任何 replay，并挤占其他 session 的有效余量。请在接受/记账 grant 前评估增长后的保留集合，或拒绝无法改变淘汰结果的 grant。
— qwen3.7-max via Qwen Code /review (v0.21.10)
| export function normalizeJournalGrowthPoolBytes( | ||
| value: number | undefined, | ||
| ): number | undefined { |
There was a problem hiding this comment.
[Suggestion] Add focused tests for this new validation boundary. Please cover undefined, valid safe integers, zero, negatives, fractions, NaN, both infinities, and unsafe integers so a future change cannot silently admit values that break pool accounting.
中文翻译
[建议] 请为这个新增的校验边界补充专门测试。建议覆盖 undefined、合法安全整数、零、负数、小数、NaN、正负无穷及不安全整数，避免未来修改静默接收会破坏增长池记账的值。
— qwen3.7-max via Qwen Code /review (v0.21.10)
| | `--enable-session-shell` | boolean | `false` | Enables direct `POST /session/:id/shell` execution. Requires bearer token, and every call must carry a session-bound `X-Qwen-Client-Id`. | | ||
| | `--event-ring-size <n>` | number | `8000` | Per-session SSE replay ring; soft cap is `1_000_000`. | | ||
| | `--compacted-replay-max-bytes <n>` | positive integer | `4194304` | Byte cap for the bounded in-memory replay snapshot returned by `POST /session/:id/load`; hard cap is `268435456`. | | ||
| | `--max-journal-events <n>` | positive integer | `10000` | Per-session baseline cap on in-flight `liveJournal` replay entries for the unfinished turn. Adaptive growth can raise it (see `--max-journal-bytes`); pinning this flag disables growth. | |
There was a problem hiding this comment.
[Suggestion] Document this as a positive safe integer, matching the CLI validation. positive integer still appears to permit values above Number.MAX_SAFE_INTEGER, which startup rejects; please apply the same wording to both journal rows in this table.
中文翻译
[建议] 请按 CLI 的实际校验将类型写为 positive safe integer。positive integer 看起来仍允许超过 Number.MAX_SAFE_INTEGER 的值，但启动时会拒绝；请对本表两个 journal 参数使用一致表述。
— qwen3.7-max via Qwen Code /review (v0.21.10)
| | `--max-total-sessions <n>` | number | derived for multiple startup/restored workspaces | - | Daemon-wide active session cap. When omitted, a finite default is derived once from the per-workspace cap and startup/restored workspace count; dynamic registration does not recompute it. `0` means unlimited. | | ||
| | `--memory-budget-mb <n>` | integer in `[1024, 1048576]` | 50% of cgroup/host memory | Observation only | Total memory budget for the daemon process tree, capped at resolved available memory. Reported under `limits.memory`; modeled into a partition that nothing applies. | | ||
| | `--memory-budget-mb <n>` | integer in `[1024, 1048576]` | 50% of cgroup/host memory | - | Total memory budget for the daemon process tree, capped at resolved available memory. No child is sized from it; the one consumer today is the adaptive live-journal growth pool (see `--max-journal-bytes`). Reported under `limits.memory`, including a modeled per-child partition. | | ||
| | `--max-journal-events <n>` | number | `10000` | - | Per-session baseline cap on in-flight `liveJournal` replay entries. Adaptive growth can raise it (see `--max-journal-bytes`); pinning this flag disables growth. | |
There was a problem hiding this comment.
[Suggestion] Use positive safe integer for both journal options here. The current number wording appears to allow zero, fractions, and unsafe integers, all of which the CLI rejects at startup.
中文翻译
[建议] 此处两个 journal 参数都应写为 positive safe integer。当前的 number 表述看起来允许零、小数和不安全整数，但 CLI 会在启动时拒绝这些值。
— qwen3.7-max via Qwen Code /review (v0.21.10)
| .option('max-journal-events', { | ||
| type: 'number', | ||
| default: DEFAULT_MAX_JOURNAL_EVENTS, | ||
| description: |
There was a problem hiding this comment.
[Suggestion] Keep the public ServeOptions contract aligned with these CLI descriptions. Its maxJournalEvents/maxJournalBytes documentation still describes fixed caps and does not explain that omitting both enables adaptive growth while specifying either pins both dimensions. Embedders can otherwise opt into or disable growth unintentionally.
中文翻译
[建议] 请同步更新公开的 ServeOptions 契约。其 maxJournalEvents/maxJournalBytes 文档仍描述为固定上限，也未说明同时省略两项会启用自适应增长，而指定任意一项会固定两个维度。否则嵌入方可能无意启用或关闭增长。
— qwen3.7-max via Qwen Code /review (v0.21.10)
| | `--event-ring-size <n>` | number | `8000` | Per-session SSE replay ring; soft cap is `1_000_000`. | | ||
| | `--compacted-replay-max-bytes <n>` | positive integer | `4194304` | Byte cap for the bounded in-memory replay snapshot returned by `POST /session/:id/load`; hard cap is `268435456`. | | ||
| | `--max-journal-events <n>` | positive integer | `10000` | Per-session baseline cap on in-flight `liveJournal` replay entries for the unfinished turn. Adaptive growth can raise it (see `--max-journal-bytes`); pinning this flag disables growth. | | ||
| | `--max-journal-bytes <n>` | positive integer | `8388608` (8 MiB) | Per-session baseline byte cap on the in-flight `liveJournal`. When a turn breaches it, adaptive growth doubles the session's caps on demand — within a per-bridge pool of 5% of the effective `--memory-budget-mb`, clamped to `[32, 1024]` MB, and never past a 256 MiB per-session hard cap; without headroom the oldest entries are dropped with a `history_truncated` marker. Pinning either journal flag disables growth. | |
There was a problem hiding this comment.
[Suggestion] Avoid promising that every growth step doubles the caps. The policy can grant only the remaining pool headroom, so another session may leave this turn with an increase smaller than 2× and earlier truncation than this wording implies. Say that caps grow toward double, limited by remaining pool headroom and the per-session hard cap.
中文翻译
[建议] 不要承诺每次增长都会将上限翻倍。policy 只能授予池中剩余余量，因此其他 session 占用大部分池后，本 turn 的增幅可能小于 2 倍，并比当前文档暗示的更早截断。建议表述为上限会“朝两倍增长”，同时受剩余池余量和每 session 硬上限限制。
— qwen3.7-max via Qwen Code /review (v0.21.10)
Address the automated review of adaptive live-journal growth: - The growth pool is now one daemon-wide aggregate shared by every workspace bridge instead of a full pool per bridge, and growth is disabled when the budget is insufficient or leaves no headroom after the root reserve. - Grants that cannot retain any additional journal entries (an oversized event survives as the sole entry either way) are refused so the pool is never charged for growth that preserves no replay. - The refusal throttle defaults to a monotonic clock and treats a backward clock jump as an elapsed window. - The proportional event hard cap is clamped to MAX_SAFE_INTEGER so a valid-but-extreme baseline cannot poison every grant. - /daemon/status reports the growth semantics: limits.memory.journalGrowth (pool size, hard cap, baselines), per-session effective caps in full diagnostics, and enforced:false scoped to the child-heap model. - Validation-boundary tests for the growth-pool normalizer and doc fixes (positive safe integer types; growth toward double, limited by pool headroom).
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Addressed review feedback for PR #8905 (round: inline findings R2-1 … R2-6)All six inline findings were verified against the code, addressed in code, and committed as Critical
Suggestions
Declined / escalatedNone — all findings were actionable and in scope. VerificationCommands actually run (all in the PR checkout, commit 72f07a5):
Integration tests after 中文说明已处理 PR #8905 的评审反馈（本轮：行内发现 R2-1 … R2-6）六条行内发现全部经代码核实、在代码中处理完毕，并已提交为 严重（Critical）
建议（Suggestions）
拒绝 / 升级无 —— 所有发现均可处理且在范围内。 验证实际执行的命令（均在 PR 检出上，提交 72f07a5）：
未运行 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.
Reviewed. Suggestions are inline.
Not reviewed: reverse audit — stopped at the 5-round cap; rounds 1-5 each reported confirmed findings, so two consecutive dry rounds were never established.
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 agen...: none — all checks I started were completed; I did not run the test suite/typecheck since every premise was verifiable by source inspection and no candidate surv…; You are review agent reverse-audit — Reverse audit agen...: none — all checks completed within budget.; chunk 8: run compactionEngine.test.ts under vitest — no node_modules installed in the worktree or parent checkout; verification is by manual trace against the implem…; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget., and 6 more.
Test Plan (not a blocker): 1221 tests pass — this review observed 1353, 19151, 1511, 494, 3289, 551 passed.
中文说明
已审查。 建议见行内评论。
未审查：reverse audit — stopped at the 5-round cap; rounds 1-5 each reported confirmed findings, so two consecutive dry rounds were never established。
未审查：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 agen...：none — all checks I started were completed; I did not run the test suite/typecheck since every premise was verifiable by source inspection and no candidate surv…；You are review agent reverse-audit — Reverse audit agen...：none — all checks completed within budget.；chunk 8：run compactionEngine.test.ts under vitest — no node_modules installed in the worktree or parent checkout; verification is by manual trace against the implem…；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.，另有 6 条。
Test Plan（非阻断）：1221 tests pass — this review observed 1353, 19151, 1511, 494, 3289, 551 passed。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const snapA = await bridge.loadSession({ | ||
| sessionId: 'restore-a', | ||
| workspaceCwd: WS_A, | ||
| historyReplay: 'response', | ||
| }); | ||
| expect( | ||
| snapA.liveJournal?.find((event) => event.type === 'history_truncated'), | ||
| ).toBeUndefined(); |
There was a problem hiding this comment.
[Suggestion] Three tests assert the grown session's outcome only via the absence of a history_truncated marker — refuses growth for a second session… (session 1), accounts concurrent in-flight restores… (this snapshot), and accounts growth across bridges… (session A). The negative check also passes on an empty journal — the exact vacuous pass this diff guards against elsewhere: returns granted headroom… documents the hazard ("the negative marker check also passes on an empty journal") and adds a positive content loop; these three omit it. — Failure scenario: probe-verified — a regression that empties or reseeds the grown session's journal without emitting a marker (e.g. the restore-completion path dropping the pending bus's buffered live events) leaves these tests green while the grown session — the feature's beneficiary — silently retains nothing.
| const snapA = await bridge.loadSession({ | |
| sessionId: 'restore-a', | |
| workspaceCwd: WS_A, | |
| historyReplay: 'response', | |
| }); | |
| expect( | |
| snapA.liveJournal?.find((event) => event.type === 'history_truncated'), | |
| ).toBeUndefined(); | |
| const snapA = await bridge.loadSession({ | |
| sessionId: 'restore-a', | |
| workspaceCwd: WS_A, | |
| historyReplay: 'response', | |
| }); | |
| expect( | |
| snapA.liveJournal?.find((event) => event.type === 'history_truncated'), | |
| ).toBeUndefined(); | |
| for (const text of ['a-1', 'a-2', 'a-3']) { | |
| expect(JSON.stringify(snapA.liveJournal)).toContain(text); | |
| } |
中文说明
[建议] 三个测试仅通过 history_truncated 标记的缺失来断言增长后会话的结果（refuses growth for a second session… 的 session 1、accounts concurrent in-flight restores… 的该快照、accounts growth across bridges… 的 session A）。负向检查在 journal 为空时同样通过——这正是本 diff 在其他地方已防范的空洞通过：returns granted headroom… 明确记录了该风险（“负向标记检查在空 journal 下也能通过”）并补充了正向内容断言，而这三处遗漏了。— 失败场景（已用探针验证）：若某个回归在不发出标记的情况下清空或重置增长后会话的 journal（例如 restore 完成路径丢弃了 pending bus 缓冲的 live 事件），这些测试仍为绿色，而增长后的会话——该功能的受益者——已悄悄不保留任何内容。
修复：按上方 suggestion 补充同级测试已使用的正向内容断言。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| expect( | ||
| secondSnap.liveJournal?.find( | ||
| (event) => event.type === 'history_truncated', | ||
| ), | ||
| ).toMatchObject({ data: { scope: 'live_journal' } }); |
There was a problem hiding this comment.
[Suggestion] Four truncation-outcome assertions here and at lines 5300, 5377, 5650 pin only the marker's presence (toMatchObject({ data: { scope: 'live_journal' } })), never the retained window — the mirror-image vacuity of the marker-absence finding: these pass on an over-eviction. — Failure scenario: probe-verified — an over-eviction mutation that drops one extra entry whenever eviction ran leaves all nine bridge growth tests green (the engine control test evicts down to the raised cap… fails under the identical mutation), while events the cap should retain (b-2/b-3 at cap 2 here; r-5; b-3 in the other three) are silently dropped.
| expect( | |
| secondSnap.liveJournal?.find( | |
| (event) => event.type === 'history_truncated', | |
| ), | |
| ).toMatchObject({ data: { scope: 'live_journal' } }); | |
| expect( | |
| secondSnap.liveJournal?.find( | |
| (event) => event.type === 'history_truncated', | |
| ), | |
| ).toMatchObject({ data: { scope: 'live_journal' } }); | |
| expect(JSON.stringify(secondSnap.liveJournal)).toContain('b-3'); |
中文说明
[建议] 此处及 5300、5377、5650 行的四个截断结果断言只钉住了标记的存在，从未钉住保留窗口——这是“标记缺失”类发现的镜像空洞：这类断言在过度驱逐下也能通过。— 失败场景（已用探针验证）：一个在驱逐发生时多丢一条 entry 的变异会让全部九个 bridge 增长测试保持绿色（引擎侧对照测试 evicts down to the raised cap… 在相同变异下会失败），而按 cap 本应保留的事件（此处 cap 2 下的 b-2/b-3，以及另外三处的 r-5、b-3）被悄悄丢弃。
修复：按上方 suggestion 在标记断言后补充保留窗口断言（模式与 grows a breaching session…、returns granted headroom… 一致）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| await bridge.shutdown(); | ||
| }); | ||
|
|
||
| it('charges a mid-restore session for growth granted before registration', async () => { |
There was a problem hiding this comment.
[Suggestion] Mid-restore growth accounting is only tested on the restore-success exit; no test covers a restore that FAILs or times out after its pending bus has already grown. Pool correctness for that exit rests solely on the two pendingRestoreEvents.delete() cleanup sites (bridge.ts:6133 abandon-timeout, bridge.ts:6392 .finally()), and every existing restore test resolves its deferred loadSession successfully. — Failure scenario: a refactor dropping/skipping the failure-path delete leaves the dead session's grown cap enumerated forever: grant() then computes extraGranted including the dead session, available stays permanently shrunk, and every sibling session sharing the daemon-wide pool is truncated instead of grown until daemon restart. All nine growth tests stay green; the abandon path (a restore timing out mid event-flood — exactly the condition growth exists for) has zero coverage.
Suggested fix — a sibling test:
// hold the restore, breach the pending bus so it grows, then FAIL the load
load.reject(new Error('restore failed'));
// spawn a second session, breach it, assert it grows from the returned
// headroom: no history_truncated marker + the positive content loop中文说明
[建议] restore 期间的增长记账只在 restore 成功出口有测试；没有测试覆盖 pending bus 已经增长后 restore 失败或超时的情形。该出口的池正确性完全依赖两处 pendingRestoreEvents.delete() 清理点（bridge.ts:6133 超时放弃、bridge.ts:6392 .finally()），而现有 restore 测试全部成功 resolve 了延迟的 loadSession。— 失败场景：若重构删除/跳过了失败路径的 delete，死会话已增长的 cap 会被永久计入枚举：grant() 计算的 extraGranted 包含该死会话，available 被永久压低，daemon 级共享池的所有兄弟会话都会被截断而非增长，直到 daemon 重启。全部九个增长测试仍为绿色；放弃路径（restore 在事件洪流中超时——正是增长功能存在的场景）零覆盖。
修复：新增一个同级测试——挂起 restore、让 pending bus 超限增长、然后 reject 加载；再创建第二个会话并使其超限，断言它用归还的余量完成增长（无截断标记 + 正向内容断言）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const bridge = makeBridge({ | ||
| channelFactory: async () => handle.channel, | ||
| maxJournalEvents: 2, | ||
| journalGrowthPoolBytes: 256 * 1024 * 1024, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] Pattern: growth tests calibrating against the unpinned DEFAULT_MAX_JOURNAL_* constants (location 1 of 8). This test pins the entry baseline but leaves the byte baseline implicit, while its terminal assertions are exact multiples of the default — five doublings land exactly on the 256 MiB hard cap only because the default is 8 MiB (ceil((256 MiB / 8 MiB) × 2) === 64). The sibling locations: bridge.test.ts:5179, 5399, compactionEngine.test.ts:1343, 1503, 1541, 1762, run-qwen-serve.test.ts:7767. walks through intermediate grants… already pins maxJournalBytes explicitly — that is the intended pattern. — Failure scenario: probe-verified — moving DEFAULT_MAX_JOURNAL_BYTES fails the test with cryptic doubling-arithmetic mismatches pointing at nothing in the test (at 4 MiB the pool also funds two doublings, so scripted outcomes silently change).
| const bridge = makeBridge({ | |
| channelFactory: async () => handle.channel, | |
| maxJournalEvents: 2, | |
| journalGrowthPoolBytes: 256 * 1024 * 1024, | |
| }); | |
| const bridge = makeBridge({ | |
| channelFactory: async () => handle.channel, | |
| maxJournalEvents: 2, | |
| maxJournalBytes: 8 * 1024 * 1024, | |
| journalGrowthPoolBytes: 256 * 1024 * 1024, | |
| }); |
中文说明
[建议] 模式：增长测试隐式依赖未钉住的 DEFAULT_MAX_JOURNAL_* 常量（共 8 处，此处为第 1 处）。该测试钉住了条目基线但未钉住字节基线，而结尾断言恰是默认值的整数倍——只有默认值为 8 MiB 时五次翻倍才恰好落在 256 MiB 硬顶（ceil((256 MiB / 8 MiB) × 2) === 64）。同类位置：bridge.test.ts:5179、5399，compactionEngine.test.ts:1343、1503、1541、1762，run-qwen-serve.test.ts:7767。walks through intermediate grants… 已显式钉住 maxJournalBytes——那才是预期模式。— 失败场景（已用探针验证）：改动 DEFAULT_MAX_JOURNAL_BYTES 会让测试以莫名其妙的翻倍算术不匹配失败，指向不到测试中任何内容（改为 4 MiB 时池还会资助两次翻倍，脚本化的结果会被悄悄改变）。
修复：按上方 suggestion 在 options 中显式钉住 maxJournalBytes。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| sessionScope: 'thread', | ||
| maxJournalEvents: 2, | ||
| journalGrowthPoolBytes: 8 * 1024 * 1024, |
There was a problem hiding this comment.
[Suggestion] Pattern: growth tests calibrating against the unpinned DEFAULT_MAX_JOURNAL_* constants (location 2 of 8). refuses growth for a second session… and its three pool-calibrated siblings (charges a mid-restore session… ~5267, accounts concurrent in-flight restores… ~5319, accounts growth across bridges… ~5558, and does not mischarge… ~5696 via its literals) size the pool at exactly one unpinned DEFAULT_MAX_JOURNAL_BYTES baseline. — Failure scenario: probe-verified — at a 4 MiB default the 8 MiB pool funds TWO doublings instead of one: the scripted refusal becomes a grant, and the marker assertion fails pointing at nothing about the calibration; at a grown default the mischarge test degrades to a loud but misleading failure. The tests are sound only while the default stays 8 MiB.
| sessionScope: 'thread', | |
| maxJournalEvents: 2, | |
| journalGrowthPoolBytes: 8 * 1024 * 1024, | |
| sessionScope: 'thread', | |
| maxJournalEvents: 2, | |
| maxJournalBytes: 8 * 1024 * 1024, | |
| journalGrowthPoolBytes: 8 * 1024 * 1024, |
中文说明
[建议] 模式：增长测试隐式依赖未钉住的 DEFAULT_MAX_JOURNAL_* 常量（共 8 处，此处为第 2 处）。refuses growth for a second session… 及其三个按池校准的同类测试（charges a mid-restore session… ~5267、accounts concurrent in-flight restores… ~5319、accounts growth across bridges… ~5558，以及 does not mischarge… ~5696 的字面量）把池大小恰好设为一个未钉住的 DEFAULT_MAX_JOURNAL_BYTES 基线。— 失败场景（已用探针验证）：默认值变为 4 MiB 时，8 MiB 的池会资助两次翻倍而非一次：脚本化的拒绝变成授予，标记断言失败却指向不到校准问题；默认值变大时 mischarge 测试退化为响亮但误导的失败。这些测试只有在默认值保持 8 MiB 时才有效。
修复：按上方 suggestion 钉住基线（does not mischarge… 测试须钉在 bridgeA 自己的 options 上，勿放入最后展开进 bridgeB 的 sharedGrowthOpts）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| 'Per-session baseline source-event byte cap on the in-flight live ' + | ||
| 'journal. When a turn outgrows it, adaptive growth raises the ' + | ||
| "session's caps (per-session hard cap 256 MiB) within a growth " + |
There was a problem hiding this comment.
[Suggestion] The new help text hard-codes growth figures that exist as exported constants — 256 MiB here vs JOURNAL_GROWTH_HARD_CAP_BYTES, and in the adjacent --memory-budget-mb text 5% / capped at 1024 MB vs JOURNAL_GROWTH_POOL_FRACTION / MAX_JOURNAL_GROWTH_POOL_MB — even though this file already imports both modules (lines 20-24, 27-30) and interpolates DEFAULT_MAX_JOURNAL_EVENTS / DEFAULT_MAX_JOURNAL_BYTES in the very same descriptions. — Failure scenario: when someone changes JOURNAL_GROWTH_HARD_CAP_BYTES (or the pool fraction/cap), qwen serve --help keeps advertising the old numbers: an operator sizing daemon memory relies on a hard cap that no longer matches the enforced one, and the help text contradicts GET /daemon/status, which reports hardCapBytes live from the same constant.
| 'Per-session baseline source-event byte cap on the in-flight live ' + | |
| 'journal. When a turn outgrows it, adaptive growth raises the ' + | |
| "session's caps (per-session hard cap 256 MiB) within a growth " + | |
| 'Per-session baseline source-event byte cap on the in-flight live ' + | |
| 'journal. When a turn outgrows it, adaptive growth raises the ' + | |
| `session's caps (per-session hard cap ${JOURNAL_GROWTH_HARD_CAP_BYTES / (1024 * 1024)} MiB) within a growth ` + |
(and interpolate JOURNAL_GROWTH_POOL_FRACTION * 100 / MAX_JOURNAL_GROWTH_POOL_MB in the --memory-budget-mb text)
中文说明
[建议] 新的帮助文本把已有导出常量的增长参数硬编码了——此处的 256 MiB 对应 JOURNAL_GROWTH_HARD_CAP_BYTES，相邻 --memory-budget-mb 文本中的 5% / capped at 1024 MB 对应 JOURNAL_GROWTH_POOL_FRACTION / MAX_JOURNAL_GROWTH_POOL_MB——而本文件已导入这两个模块（20-24、27-30 行），并且在同样的描述里对 DEFAULT_MAX_JOURNAL_EVENTS / DEFAULT_MAX_JOURNAL_BYTES 做了插值。— 失败场景：一旦有人修改 JOURNAL_GROWTH_HARD_CAP_BYTES（或池比例/上限），qwen serve --help 会继续宣传旧数字：按此规划 daemon 内存的运维人员依赖的硬顶将与实际执行值不符，帮助文本也会与 GET /daemon/status（从同一常量实时上报 hardCapBytes）矛盾。
修复：按上方 suggestion 插值（并在 --memory-budget-mb 文本中同样插值池比例与上限）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| await startServeHandlerWithArgs( | ||
| '--no-web --max-journal-events 5000 --max-journal-bytes 1048576', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The new conditional-forwarding logic in serve.ts (two independent argv['max-journal-X'] !== undefined spreads) has forwarding tests only for the two pinned-together extremes — neither pinned (above) and both pinned (here) — but none for pinning a single journal flag, which is the case where the independence of the two spreads is load-bearing. (run-qwen-serve.test.ts tests the events-only pin at the runQwenServe options level, which bypasses this yargs handler layer.) — Failure scenario: probe-verified — a refactor coupling the two spreads (&& joining both conditions) passes both existing forwarding tests unchanged. An operator who pins only --max-journal-events 5000 then has that cap silently dropped and growth left enabled with the 10 000 default baseline — the exact opposite of the documented "Pinning this flag (or --max-journal-bytes) disables adaptive growth" contract — and no test turns red.
Suggested fix — boot with one flag pinned and assert the other key is absent:
await startServeHandlerWithArgs('--no-web --max-journal-events 5000');
expect(mockRunQwenServe).toHaveBeenCalledWith(
expect.objectContaining({ maxJournalEvents: 5000 }),
);
expect(mockRunQwenServe.mock.calls[0]?.[0]).not.toHaveProperty('maxJournalBytes');
// mirror for a single --max-journal-bytes pin中文说明
[建议] serve.ts 新的条件转发逻辑（两个独立的 argv['max-journal-X'] !== undefined 展开）只有“都不钉住”（上方用例）与“都钉住”（此处）两个极端的转发测试，缺少只钉住一个 journal flag 的用例——而这恰是两个展开相互独立性起决定作用的场景。（run-qwen-serve.test.ts 中只钉条目 cap 的测试走的是 runQwenServe options 层，绕过了这个 yargs handler 层。）— 失败场景（已用探针验证）：把两个展开耦合的重构（用 && 连接两个条件）能让两个既有转发测试原样通过。此时只传 --max-journal-events 5000 的运维会发现该 cap 被悄悄丢弃、增长保持开启且基线仍是默认 10 000——与文档承诺的“钉住此 flag（或 --max-journal-bytes）即禁用自适应增长”完全相反——且没有任何测试变红。
修复：按上方代码新增单 flag 钉住用例（两个方向各一）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| // The growth-parity assertions below derive the budget from host | ||
| // memory; pin the figure so a small or cgroup-constrained runner | ||
| // cannot flip this test red. | ||
| mockTotalMemBytes.value = 8 * 1024 * 1024 * 1024; |
There was a problem hiding this comment.
[Suggestion] The host-memory pin (8 GiB + constrainedMemory → 0) is active for the whole telemetry test — boot AND dynamic attach — which makes the growth-pool parity assertion below (createBridge.mock.calls[1] pool toBe the boot bridge's) blind to the regression it was written to catch: budget/pool re-derivation at attach time. resolveDaemonMemoryBudget reads exactly os.totalmem() and process.constrainedMemory(), both pinned identically from before runQwenServe until the finally cleanup. — Failure scenario: a future change re-deriving the pool at POST /workspaces reads the same pinned inputs, produces the same figure, and the parity assertion still passes — while in production host/cgroup readings drift between boot and attach, so the attached bridge would be constructed with a different journalGrowthPoolBytes; since grant() computes available = opts.poolBytes − extraGranted against each bridge's own poolBytes while extraGranted enumerates daemon-wide sessions, the larger-pool bridge grants growth beyond the single modeled budget — the over-grant the daemon-wide pool exists to prevent.
Suggested fix: shift the pin between the two derivation points — keep 8 GiB before boot, then set mockTotalMemBytes.value = 16 * 1024 * 1024 * 1024 after runQwenServe resolves and before the POST /workspaces attach. The correct boot-closure implementation never re-reads host memory, so it stays green; a re-deriving regression computes a different pool and fails the existing parity assertion. (No other assertion after boot depends on host-memory-derived figures.)
中文说明
[建议] 宿主内存钉住（8 GiB + constrainedMemory → 0）在整个遥测测试期间有效——启动与动态挂载都是——这使得下方的增长池一致性断言（createBridge.mock.calls[1] 的池 toBe 启动 bridge 的池）对它本要捕获的回归（挂载时重新推导预算/池）视而不见。resolveDaemonMemoryBudget 恰好只读 os.totalmem() 与 process.constrainedMemory()，两者从 runQwenServe 之前到 finally 清理被钉成完全相同的值。— 失败场景：未来若在 POST /workspaces 时重新推导池，重推导读到相同的钉住输入、得出相同数值，一致性断言照样通过——而生产中宿主/cgroup 读数在启动与挂载之间会漂移，挂载的 bridge 会带着不同的 journalGrowthPoolBytes 构造；由于 grant() 按每个 bridge 自己的 poolBytes 计算 available = opts.poolBytes − extraGranted、而 extraGranted 枚举的是 daemon 级全部会话，池更大的 bridge 会授予超出单一建模预算的增长——正是 daemon 级池要防止的超额授予。
修复：在两个推导点之间换钉住值——启动前保持 8 GiB，runQwenServe resolve 之后、POST /workspaces 挂载之前改为 16 GiB。正确的“启动期闭包”实现不会重读宿主内存，仍为绿色；重推导的回归会算出不同的池并使现有一致性断言失败。（启动之后没有其他依赖宿主内存推导值的断言。）
— qwen3.8-max via Qwen Code /review (v0.21.10)
| it('derives an adaptive journal growth pool into every bridge', async () => { | ||
| mockTotalMemBytes.value = 8 * 1024 * 1024 * 1024; |
There was a problem hiding this comment.
[Suggestion] The 8 GiB host pin makes the derived budget arithmetically identical to the flag budget (floor(8192 MiB × 0.5) = 4096 MB == this test's memoryBudgetMb: 4096), so the pool-parity assertion cannot tell whether the daemon actually consumed --memory-budget-mb when deriving the growth pool. The secondary-workspace sibling (derives the adaptive journal growth pool into secondary-workspace bridges too, ~2854) has the same structure. — Failure scenario: probe-verified — replacing the wiring with resolveDaemonMemoryBudget({}) (dropping budgetMb) leaves both tests green. In production, on any host where derived ≠ flag (e.g. a 16 GiB host with --memory-budget-mb 4096: pool 409 MB instead of 204 MB), the daemon would fund journal growth from a budget the operator never granted.
| it('derives an adaptive journal growth pool into every bridge', async () => { | |
| mockTotalMemBytes.value = 8 * 1024 * 1024 * 1024; | |
| it('derives an adaptive journal growth pool into every bridge', async () => { | |
| mockTotalMemBytes.value = 16 * 1024 * 1024 * 1024; |
(derived 8192 MB ≠ flag 4096 MB; the existing recompute reads the same pin and stays correct — apply the same shift to the sibling test)
中文说明
[建议] 8 GiB 的宿主钉住使推导预算与 flag 预算在算术上完全相同（floor(8192 MiB × 0.5) = 4096 MB == 本测试的 memoryBudgetMb: 4096），因此池一致性断言无法分辨 daemon 推导增长池时是否真的消费了 --memory-budget-mb。二级 workspace 同类测试（~2854）结构相同。— 失败场景（已用探针验证）：把接线替换为 resolveDaemonMemoryBudget({})（丢弃 budgetMb）后两个测试仍绿。生产中任何“推导 ≠ flag”的主机（例如 16 GiB 主机传 --memory-budget-mb 4096：池应为 204 MB 而非 409 MB）上，daemon 会从一个运维从未授予的预算里为 journal 增长出资。
修复：按上方 suggestion 把宿主钉住改为 16 GiB（推导 8192 ≠ flag 4096；既有的重算读同一钉住值仍正确——同类测试做同样调整）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| for (const unregister of unregisters) { | ||
| unregister?.(); | ||
| } |
There was a problem hiding this comment.
[Suggestion] The shared-pool test unregisters both providers before reading the views again, so it verifies only the bulk end state — a mutation that wipes the entire shared Set on any unregister call is indistinguishable from the correct per-provider delete in the production hook (run-qwen-serve.ts:2615-2622). No other test exercises this implementation's unregister behavior (the bridge.test.ts multi-bridge tests use a test-local Set/registrar). — Failure scenario: probe-verified — changing the production handle body to journalGrowthSessionLimitProviders.clear() leaves this test green. In a multi-workspace daemon, closing one workspace's bridge would then delete every surviving bridge's provider, so their live grown sessions drop out of sibling bridges' grant() accounting (extraGranted undercounted → available overcounted): each remaining bridge grants headroom past the pool's true remainder, and aggregate journal growth exceeds the daemon-wide budget — the over-grant the single shared pool exists to prevent.
Suggested fix — unregister one handle at a time with a view assertion between:
unregisters[0]?.();
// assert every view still enumerates the OTHER bridge's provider
// (e.g. [{ limitBytes: 1001, baselineBytes: 8 * 1024 * 1024 }])
unregisters[1]?.();
// assert the views are now []中文说明
[建议] 共享池测试在重新读取视图前把两个 provider 都注销了，因此只验证了批量终态——“任何一次注销都清空整个共享 Set”的变异与生产钩子（run-qwen-serve.ts:2615-2622）中正确的按 provider delete 无法区分。没有其他测试验证该实现的注销行为（bridge.test.ts 的多 bridge 测试用的是测试本地的 Set/registrar）。— 失败场景（已用探针验证）：把生产钩子主体改成 journalGrowthSessionLimitProviders.clear() 后该测试仍绿。在多 workspace daemon 中，关闭某个 workspace 的 bridge 会删掉所有幸存 bridge 的 provider，它们已增长的 live 会话将从兄弟 bridge 的 grant() 记账中消失（extraGranted 少计 → available 多计）：每个剩余 bridge 都会授予超出池真实余量的额度，journal 增长总量超出 daemon 级预算——正是单一共享池要防止的超额授予。
修复：按上方代码逐个注销并在中间断言视图。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #8905All 19 inline findings this round were Suggestions from the automated reviewer, and all 19 were resolved in code in a single commit ( Vacuous-pass hardening (bridge growth tests)
Unpinned
|
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed current head a4d0dab. No blocking issues found.\n\nI re-verified the previously unresolved Critical concerns against the current code: one daemon-wide pool is shared across all runtime bridge construction paths; each live or restoring session is charged against its own baseline; intermediate grants are walked until retention actually improves and otherwise rolled back; refusal throttling uses a monotonic clock; proportional event limits stay within the safe-integer range; and daemon status plus SDK types distinguish baseline limits from per-session effective caps.\n\nIndependent verification:\n- acp-bridge targeted suites: bridge, compaction engine, memory-budget, growth-policy, and replay-window tests passed\n- CLI targeted suites: serve command, run-qwen-serve, and daemon-status — 366 tests passed\n- changed-file ESLint and Prettier checks passed\n- acp-bridge and TypeScript SDK typechecks passed\n\nThe head remained unchanged throughout review and the required CI jobs on this commit are green.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
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 agen...: none — all checks above completed within budget.; PR #8905 makes the daemon's live-journal caps adaptive: w...: none — all planned checks completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; chunk 14: none — all checks I intended completed within budget.; PR #8905 makes the daemon's live-journal caps adaptive: w...: none — all planned checks completed within the tool budget..
Test Plan (not a blocker): 1221 tests pass — this review observed 1356, 1511, 494, 3289, 551 passed.
中文说明
未审查：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 agen...：none — all checks above completed within budget.；PR #8905 makes the daemon's live-journal caps adaptive: w...：none — all planned checks completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；chunk 14：none — all checks I intended completed within budget.；PR #8905 makes the daemon's live-journal caps adaptive: w...：none — all planned checks completed within the tool budget.。
Test Plan（非阻断）：1221 tests pass — this review observed 1356, 1511, 494, 3289, 551 passed。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| ...(argv['max-journal-events'] !== undefined | ||
| ? { maxJournalEvents: argv['max-journal-events'] } | ||
| : {}), |
There was a problem hiding this comment.
[Critical] R6-1: A bare --max-journal-events / --max-journal-bytes (flag present, no value) is silently treated as "not pinned" — yargs yields undefined for a valueless type: 'number' option, so the pin never reaches runQwenServe and adaptive growth stays enabled, contradicting the new help text ("Pinning this flag (or --max-journal-bytes) disables adaptive growth") with no error or warning. — Failure scenario: probe-confirmed against this repo's yargs 17.7.2: parseSync('--max-journal-events') yields undefined with zero parse errors (also when followed by another flag, both flags bare, or realistic --no-web invocations); the fast path falls back to yargs for the valueless number flag, so this is the production path. With both options undefined, serveJournalGrowthPoolMb skips both pin gates and wires the derived growth pool into every bridge — the operator expected fixed caps with growth off, but growth stays enabled, with no signal at any layer (observable only after the fact via GET /daemon/status). Pre-diff, the yargs default: made a bare flag resolve to the default value, so flag presence had no behavioral meaning; this diff introduces the silent drop by keying growth-disable on argv presence with no nargs/demandOption guard. (--max-journal-events= with = parses to 0 and fails loudly in normalization — the silent path is specifically the bare flag without =.) Probe flip: adding nargs: 1 to both options turns every bare case into "Not enough arguments following: max-journal-events" while leaving valid invocations unchanged.
Suggested fix — add nargs: 1 to both option declarations so yargs fails loudly instead of silently unpining:
.option('max-journal-events', {
type: 'number',
nargs: 1,
// ...rest unchanged
})
// and the same on 'max-journal-bytes'中文说明
[严重] R6-1：裸传 --max-journal-events / --max-journal-bytes（有 flag、无值）会被静默当作"未钉住"——yargs 对无值的 type: 'number' 选项返回 undefined，因此 pin 永远不会传到 runQwenServe，自适应增长保持启用，这与新增的帮助文本（"Pinning this flag (or --max-journal-bytes) disables adaptive growth"）矛盾，且没有任何报错或警告。— 失败场景：已用本仓库的 yargs 17.7.2 probe 确认：parseSync('--max-journal-events') 返回 undefined 且零解析错误（后接其他 flag、两个 flag 都裸传、以及真实的 --no-web 调用均相同）；fast-path 对无值 number flag 会回落到 yargs，因此这就是生产路径。两个选项都为 undefined 时，serveJournalGrowthPoolMb 跳过两个 pin 判断，把派生的增长池接入每个 bridge——operator 期望固定上限并关闭增长，但增长仍然启用，任何一层都没有信号（只能事后通过 GET /daemon/status 观察到）。改动前 yargs 的 default: 使裸 flag 解析为默认值，flag 的出现没有行为含义；本次改动把"禁用增长"键控在 argv 是否出现上、又没有 nargs/demandOption 保护，从而引入了这个静默丢弃。（--max-journal-events=（带 =）会解析为 0 并在 normalization 中大声失败——静默路径专指不带 = 的裸 flag。）Probe 翻转：给两个选项加上 nargs: 1 后，所有裸传场景都会报 "Not enough arguments following: max-journal-events"，合法调用不受影响。
修复建议：在两个选项声明处加上 nargs: 1，让 yargs 大声报错而不是静默取消钉住。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| // Pin the retained window too: marker presence alone also passes on | ||
| // an over-eviction that drops one extra entry. | ||
| expect(JSON.stringify(snap.liveJournal)).toContain('r-4'); | ||
| expect(JSON.stringify(snap.liveJournal)).toContain('r-5'); |
There was a problem hiding this comment.
[Suggestion] R6-2: The retained-window pin in 'charges a mid-restore session for growth granted before registration' is weaker than the sibling tests' pins: it pins only r-4/r-5 of the expected 4-entry retained window (r-2, r-3, r-4, r-5), so it cannot catch an over-eviction past one extra entry. — Failure scenario: probe-confirmed: a regression that resets the effective cap toward the 2-entry baseline on a denied grant (retaining only r-4, r-5) passes the test as written, even though it violates the invariant the test's own comment states ("the second breach must observe the grown cap"). Adding the r-2/r-3 pins makes the same regression fail. No other test in this block covers "grown caps survive a denied grant": tests 3/6/9 deny sessions that never grew, and test 2 never hits a denial.
| // Pin the retained window too: marker presence alone also passes on | |
| // an over-eviction that drops one extra entry. | |
| expect(JSON.stringify(snap.liveJournal)).toContain('r-4'); | |
| expect(JSON.stringify(snap.liveJournal)).toContain('r-5'); | |
| // Pin the retained window too: marker presence alone also passes on | |
| // an over-eviction that drops one extra entry. | |
| expect(JSON.stringify(snap.liveJournal)).toContain('r-2'); | |
| expect(JSON.stringify(snap.liveJournal)).toContain('r-3'); | |
| expect(JSON.stringify(snap.liveJournal)).toContain('r-4'); | |
| expect(JSON.stringify(snap.liveJournal)).toContain('r-5'); |
中文说明
[建议] R6-2：'charges a mid-restore session for growth granted before registration' 中的保留窗口钉扎弱于兄弟测试：期望的保留窗口是 4 条（r-2、r-3、r-4、r-5），但只钉了 r-4/r-5，因此超过"多丢一条"的过度驱逐无法被捕获。— 失败场景：probe 确认：一个在拒绝授权时把有效上限重置回 2 条基线的回归（只保留 r-4、r-5）在当前写法下能通过测试，尽管它违反了该测试自己的注释所声明的不变量（"the second breach must observe the grown cap"）。补上 r-2/r-3 钉扎后，同一回归即会失败。本块中没有其他测试覆盖"增长后的上限在被拒绝后仍然有效"：测试 3/6/9 拒绝的是从未增长的会话，测试 2 不会触发拒绝。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
@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: 94 passed · 0 failed · 94 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：94 通过 · 0 失败 · 94 总计 Verification reportPR 8905 — feat(serve): adaptively grow live-journal caps before truncating mid-turn replayVerdict: 中文摘要
Previous-finding status (follow-up round)
Central claim + A/BCentral claim: when an in-flight turn outgrows the per-session live-journal caps, the daemon grows that session's caps (doubling toward a 256 MiB hard cap, within a daemon-wide pool derived from the memory budget) and retains more of the turn for mid-turn replay; with no pool configured, behavior is exactly the pre-change fixed-cap eviction. Round-2 delta: growth is accounted per session's OWN baseline across bridges sharing one pool, and the engine walks intermediate grants on a breach — rolling back and counting a refusal when no reachable grant retains more. Harness
Reading the cells: A proves the central claim (6× retention, no marker). B proves graceful degradation — the final grant lands exactly at baseline+pool (40 MiB) and eviction resumes with the marker stamped at the grown caps. C is the round-2 walk working as the new commit intends: the 16 MiB intermediate grant alone retains nothing extra, so the walk continues and the 32 MiB grant retains both events (round 1 refused here — deliberate behavior change, now covered by Round-2 delta probes (43/43, witness
|
| Probe | Guard under test | Result |
|---|---|---|
| P1 | Per-session baseline accounting (the 72f07a5 core): two bridges (baselines 10k/8 MiB and 5k/4 MiB) share one 10 MiB pool. b1 grows 8→16 (charged 8); b2 gets the partial grant 4→6 (charged 2, entries scaled against its OWN 4 MiB baseline → 7 500); b1's re-ask is refused at exactly pool exhaustion. Discriminator: a single-shared-baseline mutant charges max(0, 6−8)=0 for b2 and would grant b1 16→18 MiB here — this assertion kills mutant M3 independently |
pass |
| P2 | Step budget + rollback: advisor granting +1 byte hits exactly 64 calls, caps roll back to baseline, plain eviction runs; no re-ask before 10 000 ms, re-ask resumes at exactly 10 000 ms | pass |
| P3 | Throwing advisor → degrades to fixed-cap eviction, never propagates | pass |
| P4 | Malformed grants rejected whole (non-safe-integer bytes; events below current; bytes not strictly larger) → caps unchanged, plain eviction (3 shapes) | pass |
| P5 | Turn-boundary skip: a breaching turn_complete append never asks the advisor and the journal is discarded (no pool charged for a journal about to vanish); grown caps persist across the boundary; the refusal throttle resets at a turn boundary |
pass |
| P6 | Pool envelope re-measurement (carry-over Correction, below) | pass |
| P7 | serve --help derives its figures from the exported constants (256 MiB hard cap, 5% fraction, 1024 MB cap, 1024 MB minimum budget, 10 000 / 8 388 608 defaults) |
pass |
Corrections
- (carried over, stands) The PR body's pool envelope is wrong. Both the English and 中文 bodies and the Reviewer Test Plan state the pool is "clamped to
[32, 1024]MB". Re-measured at the new head (print-envelope.mjs, witness04-pool-envelope-and-help.png):journalGrowthPoolMb=min(floor(5% × effectiveBudgetMb), 1024, childPoolMb), 0 oninsufficientMemory. Host sweep: 512/1024 MB → 0 (disabled); 2048 → 51; 4096 → 102; 8192 → 204; 16384 → 409; 32768 → 819; flag max → 1024. A sweep of the entire valid flag range[1024, 1048576]found 32 unreachable (it would require an effective budget of ~640–659 MB, below the 1024 MB minimum where growth is disabled instead). Reachable set{0} ∪ [51, 1024]MiB.docs/and the new constant-derived help text are accurate; only the PR description needs the fix. Severity: nit, description-only — no behavioral consequence.
Findings
No blocking findings, and no new findings this round. Notes:
- The round-2 commits changed one user-relevant behavior on purpose and it is covered: a breach whose first doubling does not yet retain an extra entry no longer counts as an immediate refusal — the engine walks intermediate grants within the same breach (scenario C: 10 MiB + 10 MiB now retains both at a 32 MiB grant; round 1 refused and evicted). The fail-safe side (scenario D: nothing reachable helps → full rollback, refusal, no pool charge) is equally pinned, by both the A/B cell and mutant M2.
- The PR body's suite counts remain stale (see status table row 3) — not a code issue.
Targeted gates (pristine, pre-mutation)
packages/acp-bridge:npx vitest run→ 28 files, 1364 passed, 0 failed (logs/gate-acp-bridge-pristine.log). Includes the bridge-level growth tests:grows a breaching session from the pool instead of truncating,refuses growth for a second session once the first consumes the pool,charges a mid-restore session for growth granted before registration,releases granted growth when the mid-restore session fails,accounts concurrent in-flight restores against the shared pool.packages/cliserve surface (7 files:serve.test,run-qwen-serve.test,daemon-status.test,server.test,multi-workspace-sessions.test,workspace-qualified-rest.test,routes/workspace-qualified-extensions.test) → 1464 passed, 0 failed (logs/gate-cli-serve-pristine.log).tsc --noEmitclean for bothpackages/acp-bridgeandpackages/cli; liveness proven by planting a type error injournalGrowthPolicy.tsthat was reported (TS2322) and then removed with sha256 verification (logs/typecheck-liveness.log).
Mutation matrix (vacuity, witness 03-mutation-matrix.png)
Each mutant was applied as an exact-string scratch edit, the named suite run against it, and the file restored from git with sha256 verification. Positive control: unmutated suites green (134 passed across the two engine/policy files; 255 in run-qwen-serve.test.ts).
| Mutant | Guard | Suite | Result — killed by |
|---|---|---|---|
| M1 | growth call in appendLiveJournal |
compactionEngine.test.ts | 17 of the 19 growth tests — e.g. grows the caps instead of evicting… failed on journalLimits() expected {maxBytes: 33554432, maxEvents: 8} received baseline {8388608, 2} (behavioral mismatch, not import breakage); two survivors, adjudicated below |
| M2 (round-2 guard) | rollback + refusal stamp after a fruitless walk | compactionEngine.test.ts | 6 tests — exactly the rollback/throttle set: rolls back and records a refusal when the walk exhausts its step budget, never charges growth when no reachable cap can retain more, refuses a grant that cannot retain more than eviction already keeps, throttles re-asks after a refusal until the interval elapses, degrades to eviction when the advisor throws, passes the current (already grown) caps to the advisor |
| M3 (round-2 guard) | per-session baseline → single shared opts.baselineBytes |
journalGrowthPolicy.test.ts | 2 tests — exactly the per-session-baseline tests added by 72f07a5: charges a grown session by its own baseline, not the policy baseline, does not charge an untouched session that started at a larger baseline; independently discriminated by probe P1 |
| M4 | runQwenServe → bridge pool wiring (primary site; pattern occurs 2×, first mutated) |
run-qwen-serve.test.ts | 4 tests — derives an adaptive journal growth pool into every bridge, …into secondary-workspace bridges too, wires every bridge to one shared daemon-wide growth-pool view, adds, advertises, and hot-removes a dynamic workspace runtime |
| M5 | turn-boundary condition only (!TURN_BOUNDARY_TYPES.has(event.type) → true), growth otherwise intact — the finer escalation of M1's boundary survivor |
compactionEngine.test.ts | exactly 1 test, 1 failed | 122 passed — does not ask for growth when the breaching append is a turn boundary |
| ctrl | none | all three | green (counts above) |
Attribution is clean: every mutant was killed by exactly the tests the commits say pin that guard. Survivor adjudication (M1's two): both assert the no-growth side of the engine and are structurally insensitive to removing the growth call entirely — falls back to eviction when the advisor refuses (eviction happens with or without the call) and does not ask for growth when the breaching append is a turn boundary (no ask happens at all). Neither is a coverage gap: the boundary test kills the directional mutant M5 cleanly, and the refusal-fallback test runs a real refusal scenario against the live walk in the control while the refusal-side state changes (rollback, throttle stamp) are pinned by M2's kill set. No dead-code survivors.
Not covered
- Per-commit attribution: the checkout is depth 2 —
git rev-list HEAD^1..HEAD^2returns 1 (a plausible small number at the shallow boundary) while the metadata lists 11 commits; the round-1 head9613be1and72f07a5are not object-reachable, so the round-1→round-2 delta is known from commit messages and the aggregate diff, not from a direct9613be1..a4d0dabdiff. Verified the aggregateHEAD^1..HEADdiff. - Full-daemon E2E with a real ACP child under growth pressure: bridge-level accounting is covered by the repo's own bridge tests (green in the gate) and the engine+policy harnesses drove the real compiled modules; no
qwen --acpchild was spawned. - TUI/web rendering of grown caps (
maxJournalEvents/maxJournalByteson session diagnostics,journalGrowthin status): types, status assembly, and SDK type additions are covered by tests + typecheck; no client was exercised. The sdk normalizer suite was not re-run here (CI covers it). - Windows/macOS (Linux container);
npm run bundlefast-path/bundle-closure suite — CI covers these. Repo-wide lint/format not run; targeted typecheck + the two suite gates above were run. - The A/B harness models the bridge's advisor seam faithfully (requester accounted at its live cap, siblings at theirs, per-session baselines) but is single/multi-engine at the engine+policy level, not a live HTTP daemon; the wiring half is pinned by M4 and the wiring tests instead.
Methodology
Environment: CI verify container (node:22-bookworm), merge-ref checkout at depth 2; npm ci + npm run build pre-ran at HEAD (dist verified fresh: maybeGrowJournalLimits present in dist/compactionEngine.js). Base arm: git worktree add tmp/base-tree HEAD^1; package.json/package-lock.json/packages/core are byte-identical across the PR (git diff HEAD^1..HEAD empty for all three), so the root node_modules is a clean control; per-package node_modules (version-conflict copies) were symlinked from the head tree into the base worktree — without them the base rebuild resolves ignore/ajv against root copies and fails with typing errors, the same class as the pre-existing @lydell/node-pty TS7016 in core/src/services/shellExecutionService.ts that reproduces identically on base and is unrelated to this PR. With the links in place the base acp-bridge emitted cleanly; realpath check: @qwen-code/qwen-code-core resolves to /__w/qwen-code/qwen-code/packages/core (head tree) and the exercised engine modules import only relative siblings, so no confound. The base arm was removed (git worktree remove --force) after the A/B capture and the tree verified pristine. Harnesses (lib.mjs, ab-journal-growth.mjs, probe-delta.mjs, print-envelope.mjs, run-mutations.sh) import only compiled dist/ output and use the engine's injected now seam for deterministic clocks — no mocks of code under test; raw logs in logs/ (gate outputs, per-mutant vitest runs, typecheck liveness, sha256 records), captures in evidence/ (01-ab-base-vs-head.png, 02-delta-probes-guards.png, 03-mutation-matrix.png, 04-pool-envelope-and-help.png).
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 6/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed — round 6Feedback points and dispositionsR6-1 [Critical] — bare
|
Round-6 findings resolved on 68fe75d: R6-1 — valueless journal flags now rejected via nargs: 1 with 25 lines of parser tests; R6-2 — retained-window pin now asserts the full 4-entry window (r-2 through r-5). Dismissed by qwen-triage re-run.
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow 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. ✅
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary — no action takenThis round ran in Critical-only mode (the PR has completed five change-producing rounds). After triaging all feedback newer than the last evaluation (2026-08-13T00:54:34Z), there was nothing actionable:
The one item listed under Deferred non-Critical feedback (a PR comment by @qwen-code-ci-bot) is an audit record of the Critical-only filter, not work for this round. Per policy, no code was changed for it, its thread was not resolved, and no reply was posted — it remains open for human follow-up. Result: no commit was created. The PR head remains at 中文说明Autofix 轮次总结 — 未采取任何操作本轮运行于仅处理 Critical 的模式（该 PR 已完成五个产生改动的轮次）。对上次评估（2026-08-13T00:54:34Z）之后的所有反馈进行分类处理后，没有任何可执行事项：
**Deferred non-Critical feedback（已延后的非 Critical 反馈）**中列出的唯一条目（@qwen-code-ci-bot 的一条 PR 评论）只是 Critical-only 过滤器的审计记录，不属于本轮的工作内容。按策略，未针对它修改任何代码、未关闭其话题、也未发布回复——它保持开放状态，留待人工跟进。 结果：未创建任何提交。PR 头部仍为 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Local real-stack verification (macOS) — behavior confirmed, recommend mergeI verified this PR end to end on a real daemon rather than from the unit tests alone: two full bundles (base Bundle provenance was checked first: Host / derived figures observed live: effective budget WorkloadOne session, one prompt, one turn that never ends during measurement: the mock streams 20 000 text deltas (~40.9 MB, 20 002 source events), each carrying a monotonic marker A/B/C — same workload, three daemons
The head daemon's growth ladder is visible in The pinned-flag arm is the control I care about most: on the head build, pinning one journal flag reproduces base byte for byte — same 15 entries, same Pool is a real, shared, aggregate ceilingTwo more arms with
Both arms saturate the pool to the byte and stop. The Web Shell — what a user actually sees on a mid-turn reloadSame experiment driven entirely from the UI (12.3 MB turn, 6 004 source events), then the page reloaded while the turn is still in flight: base — truncation banner, content starts at
head — no banner, content starts at
Suites re-run locally on the head worktree
Notes for reviewers (non-blocking)
Not covered locally
Verdict: every user-visible claim in the description reproduces on a real daemon, the disable path is byte-identical to base, and the memory bound holds exactly at the pool on both single-bridge and cross-bridge contention. Merging looks safe to me. 中文版本地真实环境验证（macOS）——行为符合描述，建议合并我没有只依赖单测，而是在真实 daemon 上端到端验证了这个 PR：构建了两套完整 bundle（base 先做了 bundle 溯源核对：head 的 实测到的宿主机/派生数值： 有效预算 负载一个会话、一个提示词、一个在测量期间永不结束的回合：mock 流式发出 20 000 个文本增量（约 40.9 MB，20 002 个 source 事件），每个带单调递增标记 A/B/C 三个 daemon，同一负载
head 侧的增长阶梯在 我最看重的是 pin flag 那条对照腿：在 head 构建上钉住任一 journal flag，结果与 base 逐字节一致——同样 15 条、同样 池是真实、共享、聚合的天花板另外两条腿使用
两条腿都把池精确授满到字节然后停住。其中 Web Shell——用户中途刷新真正看到的东西同样的实验完全从 UI 驱动（12.3 MB 回合，6 004 个 source 事件），然后在回合仍进行中刷新页面： base——截断横幅，内容从
head——无横幅，内容从
在 head worktree 上重跑的套件
给评审者的观察（不阻塞合并）
本地未覆盖
结论： 描述里每一条用户可见的主张都在真实 daemon 上复现了，关闭路径与 base 逐字节一致，内存上界在单 bridge 与跨 bridge 争用下都精确停在池边界。我认为可以安全合并。 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary — no action takenNo changes were made this round; the branch remains at Feedback triage:
Critical-only mode: active after five change-producing rounds. The Result: nothing actionable this round — no commits, no thread resolutions, no reply comments. The PR stays at its current verified head for maintainer review. 中文说明Autofix 轮次总结 — 本轮未采取任何行动本轮未做任何改动；分支保持在 反馈分类处理：
仅处理 Critical 模式： 在完成 5 个产生改动的轮次后已激活。 结果： 本轮没有可执行事项——没有提交、没有 resolve 线程、没有回复评论。PR 保持在当前已验证的 head，等待维护者审阅。 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 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.
Reviewed.
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 agen...: none — all planned checks completed within budget.; chunk 10: none — vitest was unavailable in this worktree (no node_modules), but the pure-policy execution above covered the test file's assertions directly.; PR #8905 makes the daemon's live-journal caps adaptive: a...: none — all checks above completed within budget.; chunk 8: could not execute npx vitest run src/compactionEngine.test.ts — node_modules is absent in the review worktree and a full install exceeded the tool budget; v…; You are review agent reverse-audit — Reverse audit agen...: none — all checks I started reached a verdict., and 2 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): 1221 tests pass — this review observed 1356, 19157, 1511, 494, 3289, 551 passed.
中文说明
已审查。
未审查：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 agen...：none — all planned checks completed within budget.；chunk 10：none — vitest was unavailable in this worktree (no node_modules), but the pure-policy execution above covered the test file's assertions directly.；PR #8905 makes the daemon's live-journal caps adaptive: a...：none — all checks above completed within budget.；chunk 8：could not execute npx vitest run src/compactionEngine.test.ts — node_modules is absent in the review worktree and a full install exceeded the tool budget; v…；You are review agent reverse-audit — Reverse audit agen...：none — all checks I started reached a verdict.，另有 2 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan（非阻断）：1221 tests pass — this review observed 1356, 19157, 1511, 494, 3289, 551 passed。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action taken this roundThe review feedback for this round contains no actionable items:
The PR is in Critical-only mode after five change-producing rounds. The single No conflict resolution was needed ( 中文说明本轮未采取任何操作本轮审查反馈中没有可处理的事项：
该 PR 在经历五个产生改动的轮次后已进入仅处理 Critical 的模式。 无需解决冲突（ Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed current head 68fe75d3f63aca88e9290c7ed0c319252330c73a. No blocking issues found.
I rechecked the still-open Critical and Suggestion threads against the exact commit. The current code walks retention-neutral intermediate grants and rolls them back when no replay is gained, accounts every live or restoring session against its own baseline through one daemon-wide pool, preserves grown caps across later denials, rejects valueless journal flags with nargs: 1, forwards either pinned flag independently, and pins the effective-budget and retained-window cases in tests. The remaining open anchors no longer describe the current code.
Scope reviewed: compaction and growth-policy arithmetic; live, restore, close, reap, channel-exit, and shutdown accounting; startup, secondary, and dynamically attached workspace bridge wiring; CLI pin semantics and fast-path fallback; daemon-status and SDK compatibility; documentation; and all changed tests.
Independent verification:
- acp-bridge suites: 6 files / 850 tests passed
- CLI serve, run-qwen-serve, and daemon-status suites: 3 files / 367 tests passed
- changed-file ESLint and Prettier checks passed
git diff --checkpassed- TypeScript SDK typecheck passed
- all non-skipped checks reported by
gh pr checksare green
The head remained unchanged through the final audit. Approved.
![qwen-code-autofix[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
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.
|
Released in v0.21.12. |
4 participants
What this PR does
When an in-flight turn outgrows the per-session live-journal caps, the daemon now tries to grow that session's caps before dropping the oldest replay entries. Growth doubles the caps (entries scaled proportionally with bytes) while the growth granted across the bridge's live sessions fits in a pool derived from the daemon memory budget (5% of the effective budget, clamped to
[32, 1024]MB), and never past a per-session hard cap of 256 MiB. Growth is on demand and throttled after a refusal; the accounting is stateless — each request re-sums the current caps of all live sessions, so granted headroom disappears automatically when a session is reaped. When no headroom is granted, behavior is exactly as before: the oldest entries are dropped whole and ahistory_truncatedmarker is prepended. The complete turn content remains available from the persisted transcript after the turn finishes, as today.Why it's needed
The journal caps were introduced as a memory-safety device against runaway turns, but the fixed defaults silently degrade exactly the sessions that benefit most from live replay: a single turn fanning out many concurrent subagents (the canonical case is a
/reviewrun with ~14 agents) can emit hundreds of thousands of source events in one turn, far past the 10 000-entry / 8 MiB baseline. A mid-turn (re)load then shows only a small retained tail — in one observed session 230 123 of 247 413 events were dropped — and the full content stays hidden until the turn ends. The only escape so far was a hidden boot flag (--max-journal-bytes) that no real user knows about and that, raised naively, would give every session an unbounded-by-concurrency allowance. This makes the caps adaptive by default: they grow only under pressure, only within memory the daemon believes it has, and never past bounded ceilings.Reviewer Test Plan
How to verify
[32, 1024]MB).--max-journal-bytesdisables growth (no pool passed).run-qwen-serve.test.ts245 pass; fast-path 88 pass (including the bundle-closure check); acp-http transport 298 pass; sdk daemon UI normalizer 291 pass; typecheck, lint, build, and bundle all green.--max-journal-eventsor--max-journal-bytesdisables it (explicit config wins); bridges without a configured pool keep the fixed-cap behavior unchanged.Evidence (Before & After)
N/A (daemon internals; no TUI change — the user-visible effect is that a mid-turn reload of a fan-out session retains much more of the in-flight turn instead of showing the truncation marker early).
Tested on
Environment (optional)
Unit/integration tests only (
npm run devdaemon boot is exercised by the daemon-boot tests); no model E2E.Risk & Scope
/daemon/status(limits there still report the configured baselines); the compacted-replay window caps are untouched.Linked Issues
None — observed locally: a
/reviewsession's single turn (14 concurrent review agents) hit the live-journal caps and dropped 230 123 of 247 413 source events for mid-turn replay.中文说明
这个 PR 做了什么
当一个进行中的回合超出会话级 live journal 上限时，daemon 现在会先尝试提高该会话的上限，而不是直接丢弃最老的回放条目。增长按翻倍进行（条目数与字节数按比例放大），前提是该 bridge 上所有活跃会话已获授的增长总量仍在增长池之内——增长池由 daemon 内存预算派生（有效预算的 5%，clamp 到
[32, 1024]MB），且单会话硬顶不超过 256 MiB。增长完全按需触发，被拒绝后有节流；会计是无状态的——每次请求都重新求和所有活跃会话的当前上限，因此会话被回收时其已获授额度自动消失。若未获得额度，行为与之前完全一致：整段丢弃最老条目并前置history_truncated标记。回合结束后，完整内容依旧可从持久化 transcript 获取，与现状一致。为什么需要
journal 上限最初是作为防失控回合的内存安全装置引入的，但固定默认值恰恰悄悄拖垮了最需要实时回放的那类会话：单个回合扇出大量并发子代理（典型场景是约 14 个代理的
/review运行）可以在一个回合内产生数十万个 source 事件，远超 10 000 条 / 8 MiB 基线。此时中途（重新）加载只会看到很小的保留尾部——实际观察到一个会话 247 413 个事件中丢弃了 230 123 个——完整内容要等回合结束才可见。此前唯一的办法是一个没有真实用户知道的隐藏启动参数（--max-journal-bytes），而且简单调大它等于给每个会话一个不受并发约束的额度。本 PR 让上限默认自适应：只在压力下增长、只在 daemon 认为拥有的内存范围内增长、且永远不超过有界硬顶。评审者测试计划
如何验证
[32, 1024]MB）。--max-journal-bytes时禁用增长（不传池）。run-qwen-serve.test.ts245 通过；fast-path 88 通过（含 bundle 闭包检查）；acp-http transport 298 通过；sdk daemon UI normalizer 291 通过；typecheck、lint、build、bundle 全绿。--max-journal-events或--max-journal-bytes任一则禁用增长（显式配置优先）；未配置池的 bridge（如 channel bridge）保持固定上限行为不变。前后对比证据
N/A（daemon 内部改动，无 TUI 变化——用户可见的效果是：扇出会话中途刷新页面时能保留进行中的回合的更多内容，而不是过早看到截断标记）。
测试环境
运行环境（可选）
仅单元/集成测试（daemon 启动路径由启动测试覆盖）；无模型 E2E。
风险与范围
/daemon/status上暴露池或已增长上限（limits 仍报告配置的基线）；compacted-replay 窗口上限不动。关联 Issue
无——来自本地观察：一个
/review会话的单回合（14 个并发 review 子代理）触碰 live journal 上限，中途回放丢弃了 247 413 个 source 事件中的 230 123 个。