fix(cli,runtime-host): recover from turn consumer lag without eviction loops - #3181
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review. 📝 WalkthroughProblem solvedThe PR makes TUI turn-stream overflow recoverable. Slow consumers no longer permanently fail after exceeding 1,024 pending events. The queue sheds The CLI also recovers when a live stream ends without Source of truth and solution scopeThe PR extends the existing subscription recovery and canonical resynchronization paths. It does not create a parallel state-management path or a new public API. The solution is the smallest coherent approach shown by the current diff. Queue shedding, hysteresis, recovery, termination handling, and Host-side coalescing address separate failure modes. No code or tests can be removed without weakening coverage for flooding, recovery, terminal delivery, hysteresis, termination, or coalescing limits. ValidationThe diff adds tests for:
The PR reports 269 CLI tests and 964 Runtime Host tests passing. Biome and formatting checks are reported clean. Final required-check status is unverified from direct evidence. Complexity delta
Maintenance complexity increases in local branches but remains justified. These branches address failures that previously froze the live projection. A stalled consumer can still lose intermediate deltas, while canonical state heals after reattachment. Fully non-sheddable backlogs remain a documented boundary. Review-relevant risksThe diff changes user-visible TUI behavior during slow-consumer conditions. Recovery can discard intermediate text, thinking, and tool-output deltas. Host coalescing changes frame grouping while preserving offsets and sequence continuity. Live-stream termination now triggers recoverable The diff changes protocol frame construction and queue admission. The current summary reports bounds for live-delta and subscription-frame sizes, with tests for these limits. The review also identified a remaining concern that merged assistant-delta frames may exceed receiver protocol-size limits unless merge output is bounded independently. Material changes to user-visible behavior or protocol contracts require independent human review under repository policy. No security, licensing, release, or governance effect was identified in the current diff. The person performing the merge reviews the final diff, and a maintainer makes the final determination. WalkthroughThe runtime session channel now recovers local consumer lag instead of failing the turn stream. It sheds recoverable deltas, preserves terminal events and tool results, resubscribes, and resynchronizes canonical state. The runtime host also coalesces compatible assistant deltas. ChangesSession stream reliability
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🔵 Low · up to The change makes lagged TUI subscriptions recoverable while preserving terminal records and shedding high-volume deltas. It is mergeable with owner awareness because the full-queue terminal/tool-result admission path lacks direct regression coverage. Sequence Diagram(s)sequenceDiagram
participant TUI
participant SessionEventQueue
participant RuntimeHostSessionChannel
participant RuntimeHostSubscription
participant Transcript
TUI->>SessionEventQueue: consume turn and tool events
SessionEventQueue->>RuntimeHostSessionChannel: report consumer lag
RuntimeHostSessionChannel->>RuntimeHostSubscription: retire active subscription
RuntimeHostSessionChannel->>RuntimeHostSubscription: resubscribe with bounded retry
RuntimeHostSubscription->>Transcript: resynchronize canonical state
RuntimeHostSubscription-->>TUI: deliver replacement events and terminal outcome
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoRecover lagging CLI turn consumers without subscription eviction loops
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Known limitation (and the two qodo findings)Documenting one boundary I was already aware of, which overlaps with the qodo-code-review findings above: Terminal drop on a fully non-delta backlog (qodo finding 2). When the queue is full of tool/control events with no sheddable delta to evict, an incoming terminal event ( Stale sheddable backlog after lag recovery (qodo finding 1). Correct as stated, with one nuance: Both are refinements on top of this PR's guarantee (the stream no longer dies); neither reintroduces the permanent failure this PR removes. I'm glad to address either here or in a follow-up — maintainer's call. |
…og on resync Address qodo-code-review findings on maka-agent#3181: - A full queue with no sheddable delta silently dropped an incoming complete/error/abort, letting the consumer reach end-of-stream without a terminal outcome. Terminal outcomes now evict the oldest event in that corner, so they always land. - After lag recovery, a still-full queue kept shedding the fresh post-resync stream behind stale deltas the canonical replacement had already superseded. Lagging queues now drop their unseen sheddable backlog when the canonical replacement lands. Generated-by: Maka
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c551d8b2-2317-4f0c-a216-8c71e8cfb86a
📒 Files selected for processing (2)
packages/cli/src/__tests__/runtime-host-session-driver.test.tspackages/cli/src/runtime-host-session-channel.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.
CodeRabbit review on maka-agent#3181: after a lag recovery over a non-delta backlog, the latch stayed on until the queue fully emptied, so fresh output shed while the consumer was still behind could never schedule another canonical recovery. Re-arm once the backlog drains to half the bound: a consumer making progress gets later episodes recovered, while a wedged consumer never drains and cannot loop resubscribes. Generated-by: Maka
|
CI note: the |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bbed7817-6417-41c8-8a99-0fab1ec05daf
📒 Files selected for processing (2)
packages/cli/src/__tests__/runtime-host-session-driver.test.tspackages/cli/src/runtime-host-session-channel.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/src/runtime-host-session-channel.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review.
CodeRabbit review on maka-agent#3181: the repeated-recovery test sent sequences 1, then 100..199, then a flood restarting at 1 with the first subscription's id — a stream the real ClientSessionSubscription would reject as a sequence gap, masked by the fake. Thread the subscription id and a starting sequence through floodToolStream so every fake stream stays valid. Generated-by: Maka
CodeRabbit review on maka-agent#3181: the repeated-recovery test sent sequences 1, then 100..199, then a flood restarting at 1 with the first subscription's id — a stream the real ClientSessionSubscription would reject as a sequence gap, masked by the fake. Thread the subscription id and a starting sequence through floodToolStream so every fake stream stays valid. Generated-by: Maka
5ec410b to
e9aed36
Compare
|
Thanks for the fix — the diagnosis is right (#3180/#2630's frozen-stream failure is a dead queue: Conclusion: PASS — one P2 (partially documented; see below), otherwise P3s. P2 — non-delta, non-terminal events are silently dropped when there's no sheddable delta to evict, and the canonical resync does not replay them. Your "Known limitation" covers the terminal case ( P3: the AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on 中文摘要（AI 辅助审查）结论：PASS（1 P2，部分已被作者文档化）。诊断正确（#3180/#2630 的流冻结是死队列：push 在积压≥1024 时 fail 会话，turn 流永久死亡），方案最小且正确（复用 host eviction 的 #scheduleRecovery→#recover→#acceptCanonicalReplacement 重订阅路径，CLI 连接本就是 reconnecting 连接）。本地实跑测试文件 27/27 通过，测试 1/2/4 在 main 上必然失败（旧代码第 1025 个事件死流）——核心主张成立。竞态分析健全（泵拆除 vs 订阅交换有守卫、#ready/#recoveryTask/#lagging 闩锁防通知风暴和死循环、shedLaggedDeltas 先于 seedActive 重放、next() 等待态的消费者不可能持满积压）。P2：非 delta 非终局事件在无 delta 可逐出时被静默丢弃且 canonical resync 不重放——你的 Known limitation 只覆盖 terminal 事件，没覆盖工具流事件（tool_output_delta/tool_progress/tool_result/text_complete），工具输出洪峰时实时工具面板缺失、工具卡停留在"运行中"；durable 转录能治愈消息列表但影响限于实时面板（正是本 PR 想修的症状在工具卡粒度的缩小版），无测试界定该边界。建议把 shed/replay 扩展到工具流，或显式文档化"lag 期间工具流事件可能丢失、终局由 durable 转录治愈"并补 flood tool_output_delta 的回归测试。P3：#retiringSubscriptions 守卫吞掉退役订阅泵上的真实并发错误（恢复路径错误仍经 #fail 上浮，非静默失聪，建议注释/测试钉住）、hysteresis 边界仅测试 4 间接覆盖、单队列失败升级为会话级恢复属良性超集行为。 |
|
先转回 draft：用本 PR 的构建 dogfood 时观察到一例疑似相关的卡住案例，需要定位清楚再交 review。 现象（resume 的会话 a24c5b58，新构建的 CLI + runtime-host）：
初步怀疑方向：error → 恢复 → 刷新之后，新订阅的推流链路（server 端 per-subscriber flush 或 client 端 recovery 后的事件接收）存在静默中断的可能，且中断后没有任何一侧重新发现异常。正在继续定位根因，确认与本 PR 改动的关系后再转 ready。 |
Astro-Han review on apache#3181 (P2): during a tool-output flood the queue filled with non-sheddable tool_output_delta events, so an incoming tool_result had nothing to evict and was silently dropped, leaving the live tool card stuck at "running" until the durable transcript healed. tool_output_delta is sheddable by design: the protocol documents its chunks as transient UI updates with a monotonic per-tool seq that renderers de-dupe and order by, and the terminal tool_result plus the durable transcript remain the authoritative output. Shedding them under lag matches the text_delta story; a shed range leaves a display gap, never corruption. The remaining boundary is documented at the drop branch: a non-delta, non-terminal event behind a backlog with nothing sheddable (e.g. an all-control backlog) is still dropped, and the durable transcript heals the terminal state.
Astro-Han review on apache#3181 (P3s): the re-arm boundary was only indirectly covered. The new test drains a full non-delta backlog to one event above the watermark (513 pending) and asserts a fresh overflow does not resubscribe, then drains to the watermark (512 pending) and asserts the next overflow is treated as a new lag episode. Also pin two reviewed behaviors in comments: the retiring-subscription guard swallowing a genuine error racing the deliberate close (the replacement pump re-surfaces real failures via #fail), and the per-queue lag escalating to a session-wide recovery (benign superset: the resync heals every turn, and the latch plus hysteresis prevent resubscribe loops).
me2seeks
changed the title
|
根因已定位并修复（a1ec77d2d），draft 状态等 CI 绿后转回 ready。 Dogfood 复现链（探针直连 live host 实测）：活跃 turn 的 thinking/text delta 洪峰（每秒几十条小帧）超过 coordinator 一帧一 awaited send 的排空速度，32 帧的订阅者队列秒级溢出 → Host 以 slow_consumer 驱逐 → 本分支的重订阅路径在洪峰下每次重订阅后几秒内再次被驱逐，形成驱逐-恢复循环；且循环中客户端有两条死路：① catch-up 期间订阅迭代器干净结束（closed 帧被缓冲未处理）→ 不可恢复的永久失败；② 恢复挂起 → 无错误静默冻结（用户实机卡的最终状态）。 修复：
验证：runtime-host 964/964、cli 269/269；两处修复均做变异验证（回退 channel 修复 → 新回归测试挂起；回退合并 → 两个新测试挂）。 遗留边界（文档化在 PR body）：真正卡死的消费端（渲染循环停滞）最终仍停在 latched shed 态——run 在服务端继续，重连后 transcript 治愈；让 driver 不被渲染背压卡住是后续工作。 |
|
Code review by qodo was updated up to the latest commit a1ec77d |
|
Thanks for the deep dogfooding — the delta-storm root cause and the fix direction are right (deltas shouldn't be able to trigger cancel semantics: the Conclusion: FAIL — P1: the server-side merge can produce frames beyond the receiver's protocol limits, causing connection-level failure + recovery loop (the exact eviction loop this PR promises to kill, just moved from frame-count eviction to frame-size rejection). P1 — merge budget vs receiver ceiling mismatch. Predicted failure path: under the exact dogfood storm (delta flood + lagging consumer with send backpressure), the merge tail accumulates past 16 KiB within seconds (tens of frames/sec × 3s, or 3 large chunks) → receiver rejects the frame → Fix (small): bound the merge by P2 residual (your documented deferral — acceptable, but please name P3 (optional): AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on 中文摘要复评（head a1ec77d）结论：FAIL（P1）——服务端合并帧可超出接收端协议上限，造成连接级失败+恢复循环（正是本 PR 承诺消灭的 eviction loop，只是从帧数驱逐变成帧大小拒绝）。P1：session-continuity-coordinator.ts:1361-1382 合并 mergeableAssistantDeltaText 只校验 256 KiB 队列预算，从不校验合并后的 text/帧大小；接收端硬上限 SESSION_SUBSCRIPTION_FRAME_MAX_BYTES（64KiB−1）/帧 + SESSION_LIVE_DELTA_MAX_BYTES（16 KiB）/delta text（session-continuity.ts:298,678-681）。已实测复现：3 个各自合法（16334 B text/16566 B 编码均在限内）的连续 16 KiB delta 合并成 49002 B text 帧后 decodeSubscriptionFrame 直接抛 invalid_frame。合并预算（256 KiB）远超接收端 64 KiB 上限，#pump 发送前不复查大小→超限帧必发必拒。预测失败路径：dogfood 洪峰（+消费落后+send 背压）数秒内合并 tail 超 16 KiB→接收端拒帧→decodeHostFrame→#fail→整个连接 terminal（全部订阅 connection_closed、全部 pending 请求 reject、transport abort，connection.ts:753,790,1021-1045）→恢复重订阅+全量 transcript 重载→洪峰继续→再超限→恢复循环以连接级形式复现，且每轮杀死该连接所有其他订阅与在途请求（爆炸半径大于修复前的 channel 局部失败）。你的 9.6s/109 帧 dogfood 样本完全在触发范围内。服务端已有先例约束每帧 ≤ 64 KiB（:456 PTY 路径）与 delta 拆分（:1510-1521），合并路径唯独缺。修复很小：合并预算改 min(256 KiB 队列预算, SESSION_SUBSCRIPTION_FRAME_MAX_BYTES−信封, SESSION_LIVE_DELTA_MAX_BYTES−已有 text)，超限回退普通入队（现有 :1383-1387 eviction 兜底）+ 补"大 delta 合并产物 ≤ 上限 / 过 decodeSubscriptionFrame"测试（现有两个合并测试只合并 64 个 ~500 B 小帧，CI 绿不构成反驳）。P2 残余（你已文档化延后，可接受，但建议 PR body 的 known limitation 点名 tool_progress/tool_result——代码注释已覆盖但 body 未点名）：全非-delta 1024 backlog 时 tool_progress/tool_result/text_complete/thinking_complete 仍在 push else 分支被静默丢弃，canonical resync seedActive(false) 不重放 assistant text/工具流（靠 durable transcript + terminal 事件治愈）；补一句 body 说明+一条边界测试即闭环。P3（可选）：#retiringSubscriptions 守卫吞并发真实错误（有安全论证无测试）；非可合并洪峰（工具事件/交替流）下服务端 slow_consumer 驱逐-恢复循环仍在（pre-existing），"真正卡死的消费端会稳定在 latched shed 态"仅对可合并 delta 洪峰成立，措辞应收敛；shed 的 text/thinking delta 在 text_complete 前不补（TUI 纯 append 无 startOffset 消费，已文档化）。 |
…og on resync Address qodo-code-review findings on apache#3181: - A full queue with no sheddable delta silently dropped an incoming complete/error/abort, letting the consumer reach end-of-stream without a terminal outcome. Terminal outcomes now evict the oldest event in that corner, so they always land. - After lag recovery, a still-full queue kept shedding the fresh post-resync stream behind stale deltas the canonical replacement had already superseded. Lagging queues now drop their unseen sheddable backlog when the canonical replacement lands. Generated-by: Maka
CodeRabbit review on maka-agent#3181: after a lag recovery over a non-delta backlog, the latch stayed on until the queue fully emptied, so fresh output shed while the consumer was still behind could never schedule another canonical recovery. Re-arm once the backlog drains to half the bound: a consumer making progress gets later episodes recovered, while a wedged consumer never drains and cannot loop resubscribes. Generated-by: Maka
CodeRabbit review on maka-agent#3181: the repeated-recovery test sent sequences 1, then 100..199, then a flood restarting at 1 with the first subscription's id — a stream the real ClientSessionSubscription would reject as a sequence gap, masked by the fake. Thread the subscription id and a starting sequence through floodToolStream so every fake stream stays valid. Generated-by: Maka
Astro-Han review on apache#3181 (P2): during a tool-output flood the queue filled with non-sheddable tool_output_delta events, so an incoming tool_result had nothing to evict and was silently dropped, leaving the live tool card stuck at "running" until the durable transcript healed. tool_output_delta is sheddable by design: the protocol documents its chunks as transient UI updates with a monotonic per-tool seq that renderers de-dupe and order by, and the terminal tool_result plus the durable transcript remain the authoritative output. Shedding them under lag matches the text_delta story; a shed range leaves a display gap, never corruption. The remaining boundary is documented at the drop branch: a non-delta, non-terminal event behind a backlog with nothing sheddable (e.g. an all-control backlog) is still dropped, and the durable transcript heals the terminal state.
Astro-Han review on apache#3181 (P3s): the re-arm boundary was only indirectly covered. The new test drains a full non-delta backlog to one event above the watermark (513 pending) and asserts a fresh overflow does not resubscribe, then drains to the watermark (512 pending) and asserts the next overflow is treated as a new lag episode. Also pin two reviewed behaviors in comments: the retiring-subscription guard swallowing a genuine error racing the deliberate close (the replacement pump re-surfaces real failures via #fail), and the per-queue lag escalating to a session-wide recovery (benign superset: the resync heals every turn, and the latch plus hysteresis prevent resubscribe loops).
a1ec77d to
97bde9b
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for addressing both sides of the lag problem. Host-side contiguous delta coalescing reduces avoidable pressure, and converting CLI lag from a fatal channel error into canonical resynchronization is the right recovery model. The frame-size/sequence checks and terminal-outcome preservation are thoughtful. Independent reviewer passes and a separate read-only DeepSeek V4 Flash high-effort pass reviewed this head.
The remaining invariant is that a canonical resync is a cut: no event from before that cut may be replayed afterward unless it is intentionally regenerated from the replacement snapshot. Today only text/thinking/tool-output deltas are removed; queued tool/control events survive, so a slow consumer can observe stale progress, starts, or interaction events after onTranscriptReplaced() and the new snapshot have already established later state. Clean EOF is also newly routed into a recovery loop with no retry budget or backoff.
From first principles, replace the lagged queue wholesale at the canonical cut, seed the necessary active/terminal/interaction state from the replacement snapshot, and then accept only post-open frames. Put retry/backoff ownership at the reconnecting subscription boundary. This is simpler than classifying an expanding list of old event kinds as individually safe to retain.
I did not run a local test suite; this review is based on the latest head, focused static tracing, current green CI, independent @reviewer passes, and a read-only ollama-cloud/deepseek-v4-flash high-effort review. AI-assisted review; I verified the queue and recovery paths.
中文评论
感谢同时处理 lag 的 Host 和 CLI 两侧。Host 合并连续 delta 可以减少不必要压力；CLI 从致命错误改为 canonical resync，也是正确的恢复模型。frame size、sequence 和 terminal outcome 的约束都考虑得很细。
剩余不变量是：canonical resync 必须形成一个 cut，cut 之前的事件不能在之后重新播放，除非由 replacement snapshot 明确重建。目前只删除 text/thinking/tool-output delta，排队中的 tool/control 事件仍保留，因此慢消费者可能在 onTranscriptReplaced() 和新 snapshot 已建立较新状态后，再收到旧 progress/start/interaction。clean EOF 也被新接入一个没有退避或次数上限的恢复循环。
更符合第一性原理和奥卡姆剃刀的方案，是在 canonical cut 直接替换整个 lagged queue，从 replacement snapshot 重建必要的 active/terminal/interaction 状态，之后只接收新 subscription 的 post-open frame；重试退避由 reconnecting subscription boundary 统一拥有。这样无需持续分类哪些旧事件可以保留。
本次未运行本地测试；结论来自最新 head 静态追踪、当前绿色 CI、独立 reviewer 和一次只读的 DeepSeek V4 Flash high-effort 审查。AI 辅助审查；我已人工复核 queue 与 recovery 路径。
…ehind A TUI turn event consumer that fell more than MAX_PENDING_EVENTS_PER_TURN behind had its stream failed permanently while the Host turn kept running; the channel kept draining frames into the dead queue and nothing recovered until the terminal transcript refresh. Shed offset-bearing deltas (healed by the next canonical resync or text completion) and evict the oldest sheddable delta to make room for other events so terminal records always land, and notify the channel once per lag episode. The channel retires the healthy-but-lagged subscription through the existing recovery path — the same resubscribe a Host slow-consumer eviction triggers, and the TUI equivalent of the Desktop subscription owner (apache#2630). Fixes apache#3180 Generated-by: Maka
Flood an unconsumed turn stream past its bound: the channel resubscribes, the stream never rejects, live deltas continue after recovery, and terminal events still land while deltas are shed. Generated-by: Maka
…og on resync Address qodo-code-review findings on apache#3181: - A full queue with no sheddable delta silently dropped an incoming complete/error/abort, letting the consumer reach end-of-stream without a terminal outcome. Terminal outcomes now evict the oldest event in that corner, so they always land. - After lag recovery, a still-full queue kept shedding the fresh post-resync stream behind stale deltas the canonical replacement had already superseded. Lagging queues now drop their unseen sheddable backlog when the canonical replacement lands. Generated-by: Maka
Both new tests fail against the pre-review channel: the resync marker delta is shed behind the uncompacted backlog, and the terminal outcome is dropped from an all-tool-event backlog. Deterministic ordering: wait for the canonical resync before producing the terminal frame, so the backlog is still full when it is emitted. Generated-by: Maka
CodeRabbit review on apache#3181: after a lag recovery over a non-delta backlog, the latch stayed on until the queue fully emptied, so fresh output shed while the consumer was still behind could never schedule another canonical recovery. Re-arm once the backlog drains to half the bound: a consumer making progress gets later episodes recovered, while a wedged consumer never drains and cannot loop resubscribes. Generated-by: Maka
Sends fresh output after a non-delta-backlog recovery, then lags the consumer a second time and expects a third subscription. Fails without the hysteresis re-arm: the latch never clears while events remain queued. Generated-by: Maka
CodeRabbit review on apache#3181: the repeated-recovery test sent sequences 1, then 100..199, then a flood restarting at 1 with the first subscription's id — a stream the real ClientSessionSubscription would reject as a sequence gap, masked by the fake. Thread the subscription id and a starting sequence through floodToolStream so every fake stream stays valid. Generated-by: Maka
Astro-Han review on apache#3181 (P2): during a tool-output flood the queue filled with non-sheddable tool_output_delta events, so an incoming tool_result had nothing to evict and was silently dropped, leaving the live tool card stuck at "running" until the durable transcript healed. tool_output_delta is sheddable by design: the protocol documents its chunks as transient UI updates with a monotonic per-tool seq that renderers de-dupe and order by, and the terminal tool_result plus the durable transcript remain the authoritative output. Shedding them under lag matches the text_delta story; a shed range leaves a display gap, never corruption. The remaining boundary is documented at the drop branch: a non-delta, non-terminal event behind a backlog with nothing sheddable (e.g. an all-control backlog) is still dropped, and the durable transcript heals the terminal state. Generated-by: Maka
Astro-Han review on apache#3181 (P3s): the re-arm boundary was only indirectly covered. The new test drains a full non-delta backlog to one event above the watermark (513 pending) and asserts a fresh overflow does not resubscribe, then drains to the watermark (512 pending) and asserts the next overflow is treated as a new lag episode. Also pin two reviewed behaviors in comments: the retiring-subscription guard swallowing a genuine error racing the deliberate close (the replacement pump re-surfaces real failures via #fail), and the per-queue lag escalating to a session-wide recovery (benign superset: the resync heals every turn, and the latch plus hysteresis prevent resubscribe loops). Generated-by: Maka
Dogfooding this branch surfaced the loop the resubscribe fix could spin: an active turn's thinking/text delta flood outpaces the one-awaited-send flush, the 32-frame subscriber budget overflowed, and the Host evicted the subscription as slow_consumer within seconds of every resubscribe — while the recovering channel had a permanently fatal path (clean iterator end mid-catch-up) and a silently freezing one. The coordinator now folds a queued assistant delta into its queued tail when it continues the same stream contiguously: projectors apply deltas by absolute startOffset, so a merged frame carries byte-identical content, and the absorbed frame never spends a sequence. Eviction stays the backstop for genuinely undrainable backlogs (covered by alternating-stream tests). The channel treats a live stream that ends without subscription.closed as connection_closed, routing it through resync recovery instead of failing the session. Co-Authored-By: Maka <noreply@maka.dev> Generated-by: Maka
Generated-by: Maka
Generated-by: Maka
Generated-by: Maka
97bde9b to
e3c4e19
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The canonical replacement approach is the right owner-level response to consumer lag, and Host-side frame bounding remains coherent. Two P1 lifecycle failures remain in the CLI channel: the retry budget is reset by any single frame, permitting endless one-frame/EOF reconnect churn, and the canonical backlog cut can discard the only queued terminal outcome while retaining the queue's finished state. Both violate the PR's central promise that recovery converges without turning a successful terminal turn into an error.
AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified exact head e3c4e1913a5fd0eb4e3bb41177ed029563d73c50, queue state transitions, replacement seeding, retry/backoff behavior, and current CI. No local tests were run.
中文审查
canonical replacement 是处理 consumer lag 的正确责任层，Host 侧 frame bound 也保持一致。CLI channel 仍有两个 P1：任何单帧都会重置 retry budget，导致“一帧后 EOF”无限重连；canonical cut 会删除唯一的 terminal outcome，却保留 queue 已结束状态，使成功 turn 变成无 completion 的错误。两项都违反本 PR“恢复最终收敛”的核心目标。
本次为 AI 辅助审查，已核验精确 head、queue 状态转换、replacement seeding、retry/backoff 和当前 CI；未运行本地测试。
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f582396a-96dd-4bee-a13a-f436573b9fe5
📒 Files selected for processing (4)
packages/cli/src/__tests__/runtime-host-session-driver.test.tspackages/cli/src/runtime-host-session-channel.tspackages/runtime-host/src/__tests__/session-continuity-coordinator.test.tspackages/runtime-host/src/server/session-continuity-coordinator.ts
Included review availability: Your plan provides up to 3 included reviews per hour; 0 remain after this review.
me2seeks
left a comment
me2seeks
left a comment
There was a problem hiding this comment.
Publishing verified review remediation.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The layered direction is sound: Runtime Host reduces avoidable pressure by coalescing compatible deltas, while the CLI uses the existing canonical resubscription path as the recovery backstop. The current head also closes the previously reported single-frame retry reset and terminal-cut issues.
Three concrete edge cases remain in the admission and recovery boundaries below. They are local fixes rather than reasons to redesign the approach. Current required checks are green and the head is cleanly mergeable, but I recommend addressing these before approval.
Reviewed with Codex using three independent reviewer agents and OpenCode Go DeepSeek V4 Flash (high); I verified the findings against the exact current head and live CI.
中文
整体分层方向是对的：Runtime Host 通过合并兼容 delta 降低可避免的压力，CLI 则复用现有 canonical resubscription 作为恢复后盾。当前 head 也已经修复此前提出的单帧重置重试预算和 terminal cut 问题。
目前还剩下下面三个具体的 admission / recovery 边界问题。它们都可以局部修复，不需要重新设计整体方案。当前必需检查全绿、分支可干净合并，但建议修复后再批准。
本次由 Codex 配合三个独立 reviewer agent，以及 OpenCode Go DeepSeek V4 Flash（high）审查；我按当前精确 head 和实时 CI 复核了这些结论。
Start recovery stability only after a post-hydration live frame, recover buffered slow-consumer closure during initial hydration, and guarantee assistant completion admission over saturated control backlogs. Generated-by: Maka
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The latest head closes all three previously reported boundaries: retry stability now requires a post-hydration live frame, initial buffered slow-consumer closure enters canonical recovery, and assistant completion events receive guaranteed admission under a full non-delta backlog. The new regressions exercise each exact interleaving, including final output preservation.
I found no remaining concrete P0–P3 in the changed queue/recovery paths. One conservative edge remains: a live frame buffered during replacement hydration does not count toward stability, which can exhaust the retry budget earlier than necessary; that fails closed and is suitable as a follow-up rather than a blocker.
The current test_runtime_host run failed in the unrelated owned-candidate prompt-exit test while affected CLI coverage passed. This approval is for the code at the exact head; the failed required check still needs a successful rerun before merge.
Reviewed with Codex using two independent reviewer agents and OpenCode Go DeepSeek V4 Flash (high); I verified the exact delta, all prior findings, recovery/admission ordering, focused tests, and live CI.
中文
最新 head 已修复之前三个边界：retry stability 只有在 hydration 后收到 live frame 才成立；initial buffered slow-consumer close 会进入 canonical recovery；full non-delta backlog 下 assistant completion 也获得 guaranteed admission。新增回归测试覆盖了每个精确交错，包括 final output 保留。
当前 queue/recovery 增量中未发现剩余具体 P0–P3。仍有一个保守边界：replacement hydration 期间已缓冲的 live frame 不计入 stability，可能较早耗尽 retry budget；该行为 fail closed，适合作为 follow-up，不阻塞本次。
当前 test_runtime_host 在无关的 owned-candidate prompt-exit 测试中失败，而受影响 CLI coverage 已通过。本次批准针对精确 head 的代码；合并前仍需把失败的 required check 重跑成功。
本次由 Codex 配合两个独立 reviewer agent，以及 OpenCode Go DeepSeek V4 Flash（high）审查；我核验了精确增量、全部既有 finding、recovery/admission 顺序、聚焦测试和实时 CI。
|
The exact-head CI failure is runner-side: test_workspaces spent about 4h43m in Install Linux runtime dependencies and was cancelled before npm install, build, or any workspace test started. All other lanes on this head passed, and the existing exact-head approval remains valid. I attempted a failed-lane rerun, but GitHub requires repository admin rights for this run. Maintainer action requested: rerun the failed jobs for https://github.com/apache/maka/actions/runs/32228596412. I have not changed the approved head and have not merged. |
2 participants
Summary
A TUI turn event consumer that fell more than
MAX_PENDING_EVENTS_PER_TURN(1024) events behind had its stream failed permanently (Runtime Host Session event consumer is too slow) while the Host turn kept running; the channel kept draining frames into the dead queue and nothing recovered until the terminal transcript refresh — the same "stream appears frozen" class #2630 fixed on Desktop.The queue now sheds offset-bearing deltas (
text_delta/thinking_delta, healed by the next canonical resync or text completion) and evicts the oldest sheddable delta to make room for non-delta events so terminal records always land, and notifies the channel once per lag episode. The channel retires the healthy-but-lagged subscription through the existing#scheduleRecoverypath — the same resubscribe a Host slow-consumer eviction triggers — instead of killing the stream.Fixes #3180
Verification
npm testinpackages/cli: 264 tests pass, including 2 new regression tests that flood an unconsumed turn stream past the bound — the channel resubscribes, the stream never rejects, live deltas continue after recovery, and terminal events still land while deltas are shedgit diff --checkcleanAI use
Tool(s) and scope: Maka (this project's agent) investigated the Desktop/TUI divergence, authored the fix and its tests, and ran the simplify-audit review.
Checklist
Does this PR entail a change in behavior?
Dogfood root-cause (a1ec77d)
Running this branch against a long resumed session reproduced a worse failure than #3180: the Host evicted the TUI's subscription as
slow_consumerseconds after every (re)subscribe, because an active turn's thinking/text delta flood outpaces the coordinator's one-awaited-send-at-a-time flush and overflows the 32-frame subscriber queue. The channel's resubscribe path then either died permanently (a clean iterator end mid-catch-up was a non-recoverable error) or froze silently. Direct probe against the live host: fresh subscription evicted at +9.6s with 109/110 frames being deltas.Two fixes:
runtime-host: the coordinator coalesces a queued assistant delta into its queued tail when it continues the same stream contiguously (absolutestartOffsetsemantics make a merged frame byte-identical in content; absorbed frames never spend a sequence). Eviction remains the backstop for genuinely undrainable backlogs.cli: a live stream that ends withoutsubscription.closedis nowconnection_closed-recoverable, routing through the same resync instead of failing the channel.Verification:
packages/runtime-host964/964 (incl. 2 new coalescing tests; eviction tests now flood with non-coalescable alternating streams);packages/cli269/269 (incl. clean-end resubscribe regression, mutation-verified — reverting the channel fix hangs it, reverting coalescing fails the two new tests); biome clean.Known remaining boundary: a truly wedged consumer (render loop stalled) still settles into the latched shed state by design — the run continues server-side and the transcript heals on the next attach; un-wedging the driver from render backpressure is follow-up work.