fix(webui): allow retry after session load timeout - #8883
Merged
Conversation
Collaborator
Collaborator
|
Re-run on new head Template: complete — all required sections present, with the reviewer test plan and before/after evidence intact. ✓ Problem: observed, not theoretical — the bounded follow-up to #8864 (merged): the watchdog settlement transitioned the connection to Direction: aligned — a state-consistency bugfix in the webui daemon session layer; no auth/sandbox/model/telemetry/public-contract surface. Size: not a core-module path ( Approach: the scope grew, but every addition traces to a specific review finding — clearing the restore identity/workspace latch, marking the settlement as a manual clear so reconnect takes the session-less path, and reusing the existing Risk: no elevated risk signals — no high-risk paths matched. Moving on to code review. 🔍 中文说明在新 head 模板： 完整——所有必填部分齐全，reviewer 测试计划与 before/after 证据仍在。✓ 问题： 已观测到，非理论性——#8864（已合并）的有限 follow-up：watchdog 结算把连接切换为 方向： 对齐——webui daemon 会话层的状态一致性修复；不涉及 auth/sandbox/model/telemetry/public-contract。 规模： 非核心模块路径（ 方案： 范围有所扩大，但每处新增都对应一条具体的 review 意见——清除 restore identity/workspace 锁存、把该结算标记为手动清理以便重连走无会话路径、复用既有的 风险： 无升级风险信号——未命中高风险路径。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewRe-reviewed at
CI evidence (this PR's own checks, via API — no PR code executed)All checks that ran on
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The one claim CI cannot settle: the end-to-end retry itself — the sidebar re-selecting the timed-out target takes the normal load path. No automated test drives that cross-file interplay (round 1's R1-3, deferred by the author); the unit tests pin the settlement shape and the late-attach guard, and I traced the controlled-effect path statically. A green suite proves the tests pass, not that the untested behaviour holds — sandboxed verification would settle it: Live Web Shell interaction is never run in unattended triage; not verified here for that reason. 中文说明代码审查在
CI 证据（通过 API 读取本 PR 自身的 checks——未执行任何 PR 代码）
CI 无法定论的一项：端到端的重试本身——侧边栏重新选择超时目标会走正常加载路径。没有自动化测试驱动这一跨文件交互（首轮 R1-3，作者已搁置）；单元测试锁定了结算形状与迟到附着守卫，受控 effect 路径经静态走查。绿色套件证明测试通过，不能证明未经测试的行为成立——沙箱验证可以定论： 无人值守 triage 从不运行真实 Web Shell 交互，因此此处未验证。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — solid; the only open items are the round-3 test-pin suggestions the author explicitly deferred (named below), and I verified each is non-blocking. Stepping back: this started as a one-line settlement fix, and review rounds legitimately grew it — the timeout settlement now clears the full identity atomically (connection state, restore latches, manual-clear flag) instead of half-resetting, which is exactly what the earlier Critical findings demanded and what I would have proposed independently. I re-verified both round-1 Criticals against this head rather than taking the closeout's word: the controlled-deselection skip is harmless because the watchdog already cleared everything The 4 rather than 5: three suggestions remain open by the author's choice — no test exercises the new Approving, pinned to the reviewed commit. ✅ 中文说明置信度：4/5 —— 稳健；唯一遗留项是作者明确搁置的第三轮测试锁定建议（见下），并已逐条确认不构成阻塞。 退一步看：这最初是一行结算修复，review 过程中合理地扩大——超时结算现在原子清除完整 identity（连接状态、restore 锁存、手动清理标志），而不是半重置，这正是早先 Critical 意见所要求、也是我独立会提出的方案。我没有轻信 closeout 的说法，而是在本 head 上重新验证了两条首轮 Critical：受控取消选择的跳过无害，因为 watchdog 已清除 给 4 而非 5 的原因：三条建议按作者意愿保持开放——没有测试在非零 fresh-session nonce 下演练新的 批准，并锁定到被审查的提交。✅ — 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ℹ️ No screenshot changed against the PR base — but this PR edits 1 render-shaping file:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
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 10, 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. ✅
qwen-code-ci-bot
requested changes
Aug 10, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): PR #8883 (QwenLM/qwen-code) clears the never-attached tar...: running packages/webui/src/daemon/session/actions.test.ts — attempted, but the review worktree lacks built packages/acp-bridge dist artifacts ( @qwen-code/a…; PR #8883 (QwenLM/qwen-code) clears the never-attached tar...: exhaustive per-effect audit of all 278 connection.sessionId read sites in packages/web-shell/client/App.tsx — sampled the switch/reload/deselect-relevant ones…; You are review agent reverse-audit — Reverse audit agen...: did not trace whether any out-of-repo host (e.g. the desktop app) feeds onSessionIdChange back into the provider sessionId prop — the remount variant of the….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
未探索到全部深度（达到工具调用预算）：PR #8883 (QwenLM/qwen-code) clears the never-attached tar...：running packages/webui/src/daemon/session/actions.test.ts — attempted, but the review worktree lacks built packages/acp-bridge dist artifacts ( @qwen-code/a…；PR #8883 (QwenLM/qwen-code) clears the never-attached tar...：exhaustive per-effect audit of all 278 connection.sessionId read sites in packages/web-shell/client/App.tsx — sampled the switch/reload/deselect-relevant ones…；You are review agent reverse-audit — Reverse audit agen...：did not trace whether any out-of-repo host (e.g. the desktop app) feeds onSessionIdChange back into the provider sessionId prop — the remount variant of the…。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.21.8)
Collaborator
Author
|
Closeout: made the load watchdog clear restore identity atomically with connection identity and added action/provider regressions for controlled deselection and late load completion. The first exact-head CI failure was the async-assertion lint in the new test; fixed in 中文摘要已原子清理超时恢复身份并覆盖受控取消选择与迟到 load；新测试的 ESLint 红叉已在 `f36f8e7d6d4` 修复，聚焦 lint 和 255 个测试通过，等待新 CI。 |
Collaborator
Author
|
Revalidated the current timeout cleanup in the Desktop 0.1.1 integration candidate. The timeout now clears both the restore identity and visible connection session id before returning to the session-less workspace state, so a retry cannot reuse the timed-out session. The focused session-actions suite passed: 51 tests. |
qwen-code-ci-bot
requested changes
Aug 10, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): PR #8883 is a follow-up bugfix in the webui daemon sessio...: none — all checks I started were completed within budget.; PR #8883 is a follow-up bugfix in the webui daemon sessio...: none — all checks I started were completed within budget (~9 of ~33 calls used).; PR #8883 is a follow-up bugfix in the webui daemon sessio...: none — all checks above were carried to a conclusion.; PR #8883 is a follow-up bugfix in the webui daemon sessio...: did not trace getStableClientId / persistStableClientId storage semantics to settle whether the post-timeout createOrAttach re-attaches or spawns.; PR #8883 is a follow-up bugfix in the webui daemon sessio...: did not run the full DaemonSessionProvider.test.tsx / actions.test.ts suites (only the two diff-named tests), so collateral test breakage elsewhere in those…, and 2 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
未探索到全部深度（达到工具调用预算）：PR #8883 is a follow-up bugfix in the webui daemon sessio...：none — all checks I started were completed within budget.；PR #8883 is a follow-up bugfix in the webui daemon sessio...：none — all checks I started were completed within budget (~9 of ~33 calls used).；PR #8883 is a follow-up bugfix in the webui daemon sessio...：none — all checks above were carried to a conclusion.；PR #8883 is a follow-up bugfix in the webui daemon sessio...：did not trace getStableClientId / persistStableClientId storage semantics to settle whether the post-timeout createOrAttach re-attaches or spawns.；PR #8883 is a follow-up bugfix in the webui daemon sessio...：did not run the full DaemonSessionProvider.test.tsx / actions.test.ts suites (only the two diff-named tests), so collateral test breakage elsewhere in those…，另有 2 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| ? { | ||
| ...current, | ||
| status: 'disconnected', | ||
| sessionId: undefined, |
Collaborator
There was a problem hiding this comment.
[Suggestion] R2-2: The watchdog's identity clear resets status/sessionId/loadingTranscript/catchingUp but leaves the previous session's session-scoped snapshots (tokenUsage, tokenCount, context, supportedCommands) on the connection state — the exact fields getConnectionAfterSessionClear (actions.ts ~121-156) deliberately deletes, with a comment explaining they "carry the cleared sessionId". The watchdog now adopts manual-clear identity semantics (it sets manualSessionClearRef) without that deletion list. Probe-verified at this commit: after the watchdog fires, the connection still carries all four snapshots tagged with the previous session; deleting them in this branch flips the settled state (flip check run, all 51 actions tests still pass). — Failure scenario: session A attached → select B → B's load times out → the session-less settled state still renders session A's token usage/context in the composer pill and footer (tokenCount={connection.tokenCount ?? 0}, App.tsx ~10923; the occupancy pill has no sessionId gate) where a manual clear would show zero; additionally, if the retried load of B succeeds but B's context()/supportedCommands() fetch rejects, the fallback context ?? current.context (provider ~1826) stamps session A's snapshot onto session B. — Concrete cost: stale usage/context displayed in the empty post-timeout state; a stale snapshot misattributed to the retried session on a fetch failure.
Suggested fix: delete the session-scoped snapshots the same way getConnectionAfterSessionClear does, e.g.:
setConnection((current) => {
if (
current.status !== 'connecting' ||
current.sessionId !== sessionId
) {
return current;
}
const next = {
...current,
status: 'disconnected',
sessionId: undefined,
loadingTranscript: undefined,
catchingUp: undefined,
};
delete next.tokenUsage;
delete next.tokenCount;
delete next.supportedCommands;
delete next.context;
return next;
});中文说明
[建议] R2-2：watchdog 的身份清理重置了 status/sessionId/loadingTranscript/catchingUp，却把上一个会话的会话级快照（tokenUsage、tokenCount、context、supportedCommands）留在了连接状态上 —— 而这正是 getConnectionAfterSessionClear（actions.ts ~121-156）刻意删除的字段，其注释说明它们"携带被清理的 sessionId"。watchdog 现在采用了手动清理的身份语义（设置 manualSessionClearRef），却没有采用那份删除清单。已在本 commit 通过探针验证：watchdog 触发后，连接仍携带标记为上一个会话的全部四个快照；在该分支删除它们后结算状态翻转（已做翻转验证，51 个 actions 测试全部仍然通过）。 — 故障场景：会话 A 已附着 → 选择 B → B 的加载超时 → 无会话的结算状态仍渲染会话 A 的 token 用量/上下文（输入框徽章与页脚，tokenCount={connection.tokenCount ?? 0}，App.tsx ~10923；占用率徽章没有 sessionId 门控），而手动清理会显示为零；此外，如果重试加载 B 成功但 B 的 context()/supportedCommands() 拉取失败，回退逻辑 context ?? current.context（provider ~1826）会把会话 A 的快照盖到会话 B 上。 — 具体代价：超时后的空状态显示过期的用量/上下文；拉取失败时过期快照被错误归属到重试的会话。
修复建议：像 getConnectionAfterSessionClear 一样删除会话级快照（见上方代码示例）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
Collaborator
Author
Final installed-candidate regressionI rebuilt and installed an arm64 Desktop
Final session after the four sidebar switches:
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): PR #8883 (QwenLM/qwen-code) makes the webui daemon sessio...: none — all checks I started were completed within budget.; PR #8883 (QwenLM/qwen-code) makes the webui daemon sessio...: none — all checks I started were completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above were completed within budget..
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：PR #8883 (QwenLM/qwen-code) makes the webui daemon sessio...：none — all checks I started were completed within budget.；PR #8883 (QwenLM/qwen-code) makes the webui daemon sessio...：none — all checks I started were completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above were completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
doudouOUC
approved these changes
Aug 10, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: 无——所有检查均在 37 次调用的预算内完成。; You are review agent 1b — Agent 1b: Removed-behavior au...: 无。.
中文说明
未发现问题。LGTM！✅
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：无——所有检查均在 37 次调用的预算内完成。；You are review agent 1b — Agent 1b: Removed-behavior au...：无。。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
doudouOUC
reviewed
Aug 10, 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): PR #8883 is a follow-up bugfix to #8864: when the webui d...: did not verify whether web-shell's git/commit dialog ( resolveSessionForWorkspace → createSession ) is actually invocable while connection.loadingTranscript …; PR #8883 is a follow-up bugfix to #8864: when the webui d...: did not run the webui test suite locally (author reports 255 WebUI tests green at head 0a17bb7 ; CI green per triage bot).; PR #8883 is a follow-up bugfix to #8864: when the webui d...: verify in packages/web-shell whether any UI path can invoke actions.createSession() while a loadSession load is still pending (reachability of the watchdo…; PR #8883 is a follow-up bugfix to #8864: when the webui d...: did not trace preservingTranscriptDuringLoad to confirm the exact connection status when the attach watchdog's setConnection guard evaluates (does not affec…; PR #8883 is a follow-up bugfix to #8864: when the webui d...: did not run the new/updated tests ( DaemonSessionProvider.test.tsx , actions.test.ts ) to confirm they pass; assessed them by reading only..
中文说明
已审查。 建议见行内评论。 未探索到全部深度（达到工具调用预算）：PR #8883 is a follow-up bugfix to #8864: when the webui d...：did not verify whether web-shell's git/commit dialog ( resolveSessionForWorkspace → createSession ) is actually invocable while connection.loadingTranscript …；PR #8883 is a follow-up bugfix to #8864: when the webui d...：did not run the webui test suite locally (author reports 255 WebUI tests green at head 0a17bb7 ; CI green per triage bot).；PR #8883 is a follow-up bugfix to #8864: when the webui d...：verify in packages/web-shell whether any UI path can invoke actions.createSession() while a loadSession load is still pending (reachability of the watchdo…；PR #8883 is a follow-up bugfix to #8864: when the webui d...：did not trace preservingTranscriptDuringLoad to confirm the exact connection status when the attach watchdog's setConnection guard evaluates (does not affec…；PR #8883 is a follow-up bugfix to #8864: when the webui d...：did not run the new/updated tests ( DaemonSessionProvider.test.tsx , actions.test.ts ) to confirm they pass; assessed them by reading only.。
— qwen3.8-max via Qwen Code /review (v0.21.8)
Collaborator
Author
|
Changed: no code change was needed. The current head already clears the manual session identity/workspace and blocks the fresh-session reconnect path, so the two anchored findings are stale. Verified: exact-head code inspection against the watchdog and reconnect gates; the existing exact-head checks were green in this run snapshot. Intentionally not changed: additional late-round mutation coverage and logging requests. Pending: none from this comment pass. 中文摘要当前 head 已修复 watchdog 身份清理与误建新会话问题；本轮未改代码，额外 mutation/日志扩张未纳入。 |
yiliang114
enabled auto-merge
August 11, 2026 06:17
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
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: 2013 passed · 0 failed · 2013 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：2013 通过 · 0 失败 · 2013 总计 Verification reportPR #8883 — fix(webui): allow retry after session load timeoutVerdict: 中文摘要
Central claim + A/BClaim: when a session-switch load watchdog expires and the target never attached, the connection settles as Witness:
The base cell fails with exactly the behavioral mismatch ( Mutation matrixWitness:
FindingsS1 (Suggestion, non-blocking) — the provider
|
qwen-code-ci-bot
approved these changes
Aug 11, 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. The earlier round-1/round-2 findings are addressed at this head — verified against the code. ✅
Collaborator
|
Released in v0.21.10. |
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
3 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
Clears the never-attached target session identity when a session-switch watchdog expires. The disconnected state no longer treats that timed-out target as the current session, so selecting it again starts a new load normally.
Why it's needed
PR #8864 added watchdog settlement for a session switch that keeps receiving the transient closing response. The timeout changed the connection to
disconnectedbut retained the target session ID. The Web Shell sidebar derives its current selection from that ID, so clicking the same target again returned early instead of retrying the load.This is the bounded follow-up identified after #8864 merged. The broader transactional switching work in #8882 avoids publishing a pending target in its modern path, while this patch keeps the existing watchdog and legacy fallback path recoverable.
Reviewer Test Plan
How to verify
Start from session A and load session B without allowing B to attach. Advance the load watchdog to its deadline and confirm the load rejects, the connection settles as disconnected without a session ID, and selecting session B again follows the normal load path. Confirm that reloading the attached session and superseding a pending load still keep their existing guards.
Evidence (Before & After)
Before: the watchdog test settled as
{ status: 'disconnected', sessionId: 'session-b' }, causing the sidebar to treat session B as already selected.After: the same test settles as
{ status: 'disconnected', sessionId: undefined }; the sidebar derives no current session identity and can load session B again. The focused action suite passes 51/51 tests.Tested on
Environment (optional)
macOS, Node.js v22.22.0, npm 10.9.4. Verified with the focused WebUI action suite, WebUI typecheck/build, focused ESLint, Prettier, and
git diff --check.Risk & Scope
Linked Issues
Follow-up to #8864. Related to #8882.
中文说明
这个 PR 做了什么
当会话切换的 watchdog 到期时，清除从未完成附着的目标会话 identity。断开状态不再把超时目标视为当前会话，因此再次选择该会话会正常发起新的加载。
为什么需要
PR #8864 为持续收到临时 closing 响应的会话切换增加了 watchdog 结算。超时会把连接切换为
disconnected，但仍保留目标 session ID。Web Shell 侧边栏从该 ID 派生当前选择，因此再次点击同一个目标时会提前返回，而不会重试加载。这是 #8864 合并后确认的有限 follow-up。#8882 中更广泛的事务化切换在现代路径上不会发布 pending 目标；本补丁则确保现有 watchdog 与 legacy fallback 路径仍可恢复。
Reviewer 测试计划
如何验证
从 session A 开始加载 session B，但不允许 B 完成附着。将加载 watchdog 推进到截止时间，确认加载被拒绝、连接进入不带 session ID 的 disconnected 状态，并且再次选择 session B 会进入正常加载路径。同时确认重载已附着会话和替换 pending load 的既有守卫保持不变。
证据（修改前后）
修改前：watchdog 测试最终得到
{ status: 'disconnected', sessionId: 'session-b' }，导致侧边栏把 session B 当作已选择。修改后：同一测试最终得到
{ status: 'disconnected', sessionId: undefined }；侧边栏不再派生出当前 session identity，可以重新加载 session B。聚焦 action 测试共 51/51 通过。测试平台
环境（可选）
macOS、Node.js v22.22.0、npm 10.9.4。已验证 WebUI 聚焦 action 测试、WebUI typecheck/build、聚焦 ESLint、Prettier 和
git diff --check。风险与范围
关联 Issue
#8864 的 follow-up；与 #8882 相关。