feat(web-shell): decouple composer from catch-up and rebuild SSE on disconnected submit - #9323
Merged
Merged
Conversation
…isconnected submit Catch-up (SSE history replay) no longer disables the composer input or swaps its placeholder to "加载中": replaying history does not conflict with typing, and a stuck catch-up must never lock the input. A prompt submitted while the SSE stream is down is no longer blocked with a toast: admission rebuilds the stream immediately by aborting the reconnect backoff and resuming via Last-Event-ID, keeping the session handle intact. Drops the now-unreachable 'loading' placeholder state from the public WebShellComposerPlaceholderState type and syncs docs (README, design doc, provider and entry type comments).
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical — the mechanism is concrete and matches the current code: the catch-up flag is armed on every SSE subscription and today's state machine disables the composer while it is set ( Direction: aligned. The composer staying usable during background history replay, and a prompt rebuilding a downed stream instead of bouncing the user with a toast, both move in the direction the web shell should go. No CHANGELOG reference needed — this is the web-shell/webui surface itself. Size: ~70 production lines, ~176 test lines, 8 doc lines across Approach: scope feels right. The two changes share one concern — composer usability vs. the SSE lifecycle — and dropping the now-unreachable Risk: no elevated risk signals — none of the changed files match the revert-correlated paths. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的实际问题，不是理论假设——机制具体且与现状代码吻合：catch-up 标志在每次 SSE 订阅时置位，当前状态机会在置位期间禁用输入框（ 方向：对齐。历史重放是后台行为，输入框保持可用；断流时提交应重建流而不是弹提示让用户重试——两者都符合 web shell 应有的方向。无需 CHANGELOG 参照——这正是 web-shell/webui 界面本身的改进。 规模：约 70 行生产代码、176 行测试、8 行文档，跨 方案：范围合理。两处改动共享同一关切——输入框可用性与 SSE 生命周期的关系——移除现已不可达的 风险：无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewThe approach matches what I'd have done independently, and it's minimal: the state machine drops What I verified against the surrounding code, not just the diff:
No correctness, security, or convention findings. The disconnected-submit rebuild is the one genuinely new runtime flow, so here it is: sequenceDiagram
participant P1 as Composer (web-shell)
participant P2 as sendPrompt action
participant P3 as Daemon HTTP API
participant P4 as DaemonSessionProvider
participant P5 as SSE stream
P1->>P2: submit while stream is down
P2->>P3: submitPrompt over HTTP
P3-->>P2: admitted (promptId, lastEventId)
P2->>P4: restartEventStream(sessionId)
P4->>P4: abort reconnect backoff
P4->>P5: resubscribe immediately (Last-Event-ID resume)
P5-->>P1: response events arrive
TestingCI signal for the reviewed commit, fetched via API (per triage policy I don't run PR code): Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Reading of that signal: nothing red so far. The two checks that matter most for this PR — the ubuntu unit suite (which carries the new state-machine and provider tests) and the web-shell visuals capture — are still running; the table above is updated automatically once CI settles. The macOS/Windows test jobs are skipped by design (merge-queue-only per Sandboxed verification would settle the remaining gap: 中文说明方案与我独立设想的做法一致，且改动最小：状态机直接移除 结合周边代码（不只是 diff）核实过的点：
未发现正确性、安全性或规范问题。 （时序图见英文正文，描述断线提交重建流的完整路径。） 测试证据：按 triage 政策不运行 PR 代码，以上 CI 表格引自该 commit 自己的 CI 检查结果（API 获取）。目前无红色检查；最关键的两项——ubuntu 单测套件（含新增状态机与 provider 测试）与 web-shell 视觉截图——仍在运行，表格会在 CI 落定后自动更新。macOS/Windows 测试按 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after
Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Collaborator
|
Confidence: 4/5 — clean, minimal, well-pinned change; the only reservation is environmental, not code-level. Stepping back: the approach matches my independent proposal almost exactly — I'd have decoupled the state machine and added a backoff-abort lever the same way. Where the PR is braver than I'd have been, it's right: deleting the now-unreachable On "does the problem actually exist" I went back to the base code rather than the PR's framing, and it checks out: the provider arms The implementation holds up under the unglamorous checks: teardown vs prompt-triggered aborts are distinguishable by the loop's own condition, the listener bookkeeping leaks nothing, multi-pane providers can't cross-trigger, and the new provider test (60s backoff, second subscription resolves inside Reservations, named so they don't get lost: the real daemon's resume fidelity — rebuilt stream picking up at Verdict: approve. The unit suite and web-shell visuals are still in flight, so approval is deferred until CI lands green on 中文说明回顾整体：方案与我独立设想的做法几乎一致——同样会解耦状态机、同样会给退避加一个中断控制杆。PR 比我更果断的地方在于直接删除了不可达的 关于"问题是否真实存在"，我回到基础代码核实而非照单接受 PR 的说法，结论成立：provider 在 实现经受住了不起眼的细节检查：拆除与 prompt 触发的中断可由循环自身条件区分；监听器记账无泄漏；多窗格 provider 不会互相触发；新增 provider 测试（60 秒退避、第二次订阅在 保留意见，明说以免遗漏：真实 daemon 的续传保真——重建流以 结论：approve。由于 ubuntu 单测套件与 web-shell 视觉仍在进行，批准推迟到 CI 在该 commit 上全绿之后——下方标记承载该前置条件，finalize 任务会在所有检查落定后提交绑定该 commit 的批准（或标注状态）。 — Qwen Code · qwen3.8-max Reviewed at |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
approved these changes
Aug 17, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
chiga0
approved these changes
Aug 17, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Review Summary
Two independent, tightly-scoped improvements to the web-shell composer. Both are well-motivated and cleanly implemented.
Change 1 — Catch-up decoupling
Removes catchingUp from every composer-input decision (shouldDisableComposerInput, getComposerPlaceholderState, getComposerPlaceholderKey). The old behavior was a latent defect: the catchingUp flag is only cleared by the daemon's replay_complete sentinel, which is emitted only for subscriptions that carry Last-Event-ID. A cursor-less reconnect (e.g. first load after a daemon restart) would arm the flag and never clear it, locking the composer on "加载中" until a page refresh. Since replaying history does not conflict with typing or submitting, the fix is correct: drop catchingUp from the disable/placeholder logic entirely. The catchingUp field remains on DaemonConnectionState for any consumer that wants to render a background indicator, it just no longer gates the input.
Change 2 — Prompt-triggered SSE rebuild
The implementation is mechanically correct. Key verifications:
-
delay()resolves (not rejects) on abort (timing.tslines 1-10): whenreconnectAbort.signalfires,finish()→resolve()— soawait delay(...)completes normally and the reconnectwhileloop re-evaluates its condition, making an immediate new SSE connection attempt. -
while (!disposed && !abort.signal.aborted)at line 903 correctly separates the two abort paths:- Prompt abort: only
reconnectAbort.signalis aborted →abort.signal.abortedis stillfalse→ loop continues (reconnects immediately) ✅ - Effect cleanup:
disposed = true+abort.abort()→ both conditions are false → loop exits ✅
- Prompt abort: only
-
Listener cleanup — the
{ once: true }+removeEventListenercombination is correct for all three cases (natural timeout / prompt abort / effect cleanup). No listener leak. -
restartEventStreamguard — thesessionRef.current?.sessionId === sessionIdcheck ensures we only abort the reconnect for the same session the prompt was sent to. The live-stream path (eventStream?.sessionId === sessionId) is unchanged and still gated byrestartEventStreamOnPrompt. -
API semantics change is properly documented in
index.tsx,types.ts, andREADME.md:restartSseOnPromptnow only controls live-stream restarts; a down stream is always rebuilt on prompt admission.
Tests
The new DaemonSessionProvider test is well-designed: reconnectDelayMs: 60_000 proves the rebuild was triggered by the prompt, not by the natural timer. Verifying MockDaemonSessionClient.load was called only once confirms the session handle is preserved (no full reload). App.test.tsx and composerInputState.test.ts accurately reflect the new behavior — notably, catchingUp: true no longer produces disabled: true, and the loading composerPlaceholders key is correctly removed.
One nit
After reconnectAbortRef.current?.abort() skips the current backoff wait, the reconnectAttempt counter is NOT reset. If the prompt-triggered immediate reconnect also fails, the next delay will be longer than the initial base delay. This is probably intentional (preventing prompt submissions from resetting the backoff counter and causing retry storms), but the behavior is not documented. A brief comment at the reconnect counter increment explaining this would help future maintainers.
Cross-Validation
| Finding | CI bot | My Assessment |
|---|---|---|
| CI green, LGTM | qwen-code-ci-bot (APPROVED) | Confirmed — all five test suites pass per PR description |
Unique-1: delay() resolves on abort → reconnect loop continues correctly |
— | Verified at timing.ts, loop guard at line 903 |
Unique-2: reconnectAttempt not reset on prompt abort |
— | Nit — acceptable design, comment would help |
Additional Audit Coverage
Areas independently checked beyond CI bot approval:
- Listener lifecycle:
{ once: true }+removeEventListenerpairing verified for 3 scenarios (natural timeout / prompt abort / effect cleanup) — no leak delay()semantics: confirmed resolve-on-abort (not reject) — critical for loop correctness- Multi-iteration correctness:
reconnectAbortRef.current?.abort()at start of each delay setup is harmless (aborting an already-spent controller) sessionRefguard inrestartEventStream: ensures cross-session orphan calls are no-ops- Breaking change scope:
loadingremoved fromWebShellComposerPlaceholderState; package unpublished, no released consumer
No Critical, Major, or Minor issues. One nit on documenting the reconnectAttempt non-reset.
| const reconnectAbort = new AbortController(); | ||
| reconnectAbortRef.current = reconnectAbort; | ||
| const onEffectAbort = () => reconnectAbort.abort(); | ||
| abort.signal.addEventListener('abort', onEffectAbort, { once: true }); |
Collaborator
There was a problem hiding this comment.
Nit: The reconnectAttempt counter is not reset here. After a prompt-triggered abort the next reconnect attempt happens immediately, but if that attempt also fails, the subsequent delay will be getReconnectDelayMs(reconnectAttempt + 1, ...) — longer than the base delay — rather than starting from scratch.
This is probably intentional (preventing prompt submissions from resetting the backoff counter and causing fast-retry storms), but a brief comment would make the decision explicit for future readers:
// Intentionally don't reset reconnectAttempt: prompt admission skips the
// *current* wait only; the backoff counter is preserved so a prompt cannot
// be used to bypass the exponential backoff on repeated failures.
reconnectAbortRef.current?.abort();
doudouOUC
reviewed
Aug 17, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 2": none — all security dimensions were fully assessed.; "agent 6b": (none — I did not cut any checks short); "agent 6c": NO; "agent 6a": NO Budget gap: line.; "agent 3a": NO.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 2"：none — all security dimensions were fully assessed.；"agent 6b"：(none — I did not cut any checks short)；"agent 6c"：NO；"agent 6a"：NO Budget gap: line.；"agent 3a"：NO。
— deepseek-v4-flash via Qwen Code /review (v0.21.13)
Comment on lines
50
to
+51
| if (connectionStatus === 'error') return true; | ||
| return ( | ||
| connectionStatus === 'disconnected' && (!restartSseOnPrompt || !hasSession) | ||
| ); | ||
| return connectionStatus === 'disconnected' && !hasSession; |
Collaborator
There was a problem hiding this comment.
[Suggestion] Cross-module invariant between shouldBlockComposerSubmit and restartEventStream is undocumented
shouldBlockComposerSubmit now unconditionally allows submit for disconnected + hasSession, relying on the DaemonSessionProvider's restartEventStream handler to rebuild the down SSE stream. This invariant lives in a different package (packages/webui) and is not cross-referenced at the implementation level.
Failure scenario: a future maintainer adds a restartEventStreamOnPrompt guard to the down-stream path in restartEventStream, or removes the down-stream path entirely. The composer would allow submit while disconnected, the prompt would be accepted by the daemon, but the SSE stream would never be rebuilt, and the user would see no response until the natural reconnect timer fires (potentially minutes later).
Suggested fix: add a one-line comment in composerInputState.ts near this function documenting the invariant: "Disconnected-with-session is allowed because DaemonSessionProvider.restartEventStream always rebuilds the down stream on prompt admission."
中文说明
shouldBlockComposerSubmit 现在无条件允许 disconnected + hasSession 时提交，依赖于 DaemonSessionProvider 的 restartEventStream 处理函数来重建断开的 SSE 流。这个不变式位于不同包（packages/webui）中，在实现层面没有交叉引用。
触发场景：未来的维护者给 restartEventStream 的断流路径增加了 restartEventStreamOnPrompt 守卫，或移除了整个断流路径。输入框会允许断线时提交，prompt 会被 daemon 受理，但 SSE 流永远不会重建，用户直到自然重连定时器触发（可能几分钟后）才会看到响应。
建议：在 composerInputState.ts 中此函数附近添加一行注释，记录该不变式。
— deepseek-v4-flash via Qwen Code /review (v0.21.13)
doudouOUC
reviewed
Aug 17, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 6b": NO; "agent 4": NO — no checks were cut short.; "agent 6a": 无（在预算内完成所有检查）。.
中文说明
未探索到全部深度（达到工具调用预算）："agent 6b"：NO；"agent 4"：NO — no checks were cut short.；"agent 6a"：无（在预算内完成所有检查）。。
— deepseek-v4-flash via Qwen Code /review (v0.21.13)
Collaborator
|
Released in v0.21.14. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Two web-shell composer changes. First, SSE catch-up (history replay after a reconnect) no longer disables the composer input or swaps its placeholder to "加载中": replaying history does not conflict with typing or submitting, and a stuck catch-up must never lock the input. The unreachable
loadingplaceholder state is dropped from the publicWebShellComposerPlaceholderStatetype, with docs synced (README, design doc, provider and entry type comments). Second, submitting a prompt while the SSE stream is down is no longer blocked with a "connection disconnected" toast: the prompt is admitted over HTTP and the provider rebuilds the stream immediately by aborting the reconnect backoff and resuming viaLast-Event-ID, preserving the session handle (no full reload).Why it's needed
The catch-up indicator is armed on every SSE subscription and cleared only by the daemon's
replay_completesentinel, which is only emitted for subscriptions that carry aLast-Event-ID. A cursor-less reconnect therefore arms it and never clears it — the composer stayed stuck on "加载中" with input disabled until a page refresh. Catch-up is a background concern that does not require the user to wait, and a down stream does not block prompt admission, so the composer should stay usable in both states; sending while disconnected should just rebuild the stream instead of asking the user to retry.Reviewer Test Plan
How to verify
Disconnect scenario: with a session attached, block the SSE endpoint (or drop the network) so the connection status becomes disconnected, then send a message. It should submit without any warning toast, and the response should arrive once the stream is rebuilt. Previously this path was blocked with a "连接已断开" toast unless the
restartSseOnPromptoption was enabled. Catch-up scenario: reconnect/restore a session with history and confirm the composer stays editable with its normal placeholder instead of showing "加载中" / becoming disabled. Unit and provider tests cover the state machine (composerInputState.test.ts), the disconnected submit paths (App/ChatPanetests), and the down-stream rebuild (DaemonSessionProvider.test.tsx"rebuilds the SSE stream immediately when a prompt is submitted while the stream is down", which proves the second subscription starts from the prompt-triggered backoff abort, not the 60s natural reconnect).Evidence (Before & After)
N/A — state-machine and provider behavior, verified by tests. All affected suites pass on this branch: composerInputState 7, ChatPane+SplitView 131, DaemonSessionProvider 216 (incl. the new down-stream rebuild test), actions 79, App 449; typecheck clean for web-shell and webui.
Tested on
Environment (optional)
npm run dev:daemonon the target branch (web-shell + daemon dev).Risk & Scope
WebShellComposerPlaceholderStatetype loses theloadingmember (compile-time break for any embedder that used it; the package is not yet published, so no released consumer exists). Disconnected submits now always rebuild the SSE stream regardless ofrestartSseOnPrompt(that flag now only gates restarts of a live stream) — the flag's docs were updated accordingly.loadingremoved fromWebShellComposerPlaceholderState/WebShellComposerPlaceholders;restartSseOnPromptsemantics clarified in README/types.Linked Issues
中文说明
本 PR 包含 web-shell 输入框的两处改动。第一处：SSE 历史重放（catch-up）不再禁用输入框、也不再将其占位文案换成"加载中"——重放历史与输入、提交并不冲突，且 catch-up 卡死时绝不能锁死输入框。不可达的
loading占位状态已从公开类型WebShellComposerPlaceholderState移除，文档（README、设计文档、provider 与入口类型注释）同步更新。第二处：SSE 断开时提交 prompt 不再被"连接已断开"提示拦截——prompt 通过 HTTP 受理，provider 通过打断重连退避并基于Last-Event-ID续传立即重建 SSE，保留会话句柄（不做全量重载）。背景：catch-up 指示在每次 SSE 订阅时置位、仅靠 daemon 的
replay_complete哨兵清除，而该哨兵只对携带Last-Event-ID的订阅发送；无游标的重连因此会置位后永不解除——输入框会一直停在"加载中"且禁用，直到刷新页面。catch-up 是后台行为、不需要用户等待；断流也不阻塞 prompt 受理，因此两种状态下输入框都应保持可用；断线发送应直接重建流而不是让用户重试。验证方式：断线场景——会话已附着时断开 SSE（或断网）使连接进入 disconnected，再发送消息，应无警告提示且响应在流重建后到达；此前该路径会被"连接已断开"提示拦截（除非开启
restartSseOnPrompt）。catch-up 场景——恢复/重连带历史的会话，确认输入框保持可编辑且占位文案正常。单元与 provider 测试覆盖状态机（composerInputState.test.ts）、断线提交路径（App/ChatPane 测试）与断流重建（DaemonSessionProvider.test.tsx新增用例，证明第二次订阅由 prompt 触发的退避中断启动而非 60s 自然重连）。风险：公开类型移除
loading成员（对使用它的嵌入方是编译期破坏；该包尚未发布，无已发布消费者）。断线提交现在无论restartSseOnPrompt如何都会重建 SSE（该开关现仅控制对存活流的按 prompt 重启），开关文档已同步。