Repository navigation
fix(webui): revert #8882's transactional session switching to the loading-skeleton model - #9129
Merged
Merged
Conversation
…ding-skeleton model The transactional cross-session switching from #8882 staged a handoff and kept the old attachment live until the target load committed. It added a large transition state machine (intent staging, same-session capture, watchdog deadlines, controlled rebind) across the daemon session layer and the web-shell provider, and left the UI pinned to the previous session while a switch prepared. Restore the loading-skeleton model: switching a session clears the transcript, shows the loading skeleton, and waits for the load result. - Remove sessionTransition state, onSessionTransitionCommit and the transactional target logic from WorkspaceSessionProvider. - Strip the transition state machine from DaemonSessionProvider and restore single-session restores: restore_in_progress retries stay bounded by the existing watchdog, and the skeleton UI keys on loadingTranscript. - Move useDaemonSessionOwnerGuard back under the daemon index export. - Delete the transactional design docs and both daemon integration tests; the restored behavior is covered by unit tests. - Drop the dead desiredSessionTargetPending prop (write gating now keys on loadingTranscript alone) and stop a failed switch's target workspace from leaking into the next workspace-less load. Co-authored-by: Qwen-Coder <[email protected]>
ytahdn
added
the
autofix/takeover
Collaborator
|
🤝 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 冲突，直到移除标签或达到轮次上限。移除 |
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. |
ytahdn
commented
Aug 14, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
Author
There was a problem hiding this comment.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Integration Tests (CLI, No Sandbox) were skipped in CI and their suites did not run locally (review ran without build/test per reviewer request).
Not explored to full depth (tool budget reached): "You are review agent 1c — Cross-file tracer for PR…": none material — the full diff was read, and every brief item was grepped against the post-change worktree..
Not reviewed: the entire diff, the linked-issue fidelity pass, the whole-diff test-coverage check, the build-and-test check, the invariant check (state, timers, collections) on packages/web-shell/client/components/WorkspaceSessionProvider.tsx, the invariant check (counters, return values, error taxonomies) on packages/web-shell/client/components/WorkspaceSessionProvider.tsx, the invariant check (config fields, early returns) on packages/web-shell/client/components/WorkspaceSessionProvider.tsx, the invariant check (state, timers, collections) on packages/webui/src/daemon/session/DaemonSessionProvider.tsx, the invariant check (counters, return values, error taxonomies) on packages/webui/src/daemon/session/DaemonSessionProvider.tsx, the invariant check (config fields, early returns) on packages/webui/src/daemon/session/DaemonSessionProvider.tsx — its prompt was built, but no agent on record was launched with it.
Not reviewed: reverse audit — no auditor was launched with a prompt this skill builds — the pass that hunts what the rest of the review missed ran, if at all, without the method its brief carries.
Not reviewed: verification — a verifier ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and the posted findings cannot be counted as verified against it.
中文说明
未审查：build-and-test — Test (macos-latest, Node 22.x) and Integration Tests (CLI, No Sandbox) were skipped in CI and their suites did not run locally (review ran without build/test per reviewer request)。
未探索到全部深度（达到工具调用预算）："You are review agent 1c — Cross-file tracer for PR…"：none material — the full diff was read, and every brief item was grepped against the post-change worktree.。
未审查：整个 diff、关联 issue 一致性检查、全 diff 测试覆盖检查、构建与测试验证、不变量检查（状态、定时器、集合）（packages/web-shell/client/components/WorkspaceSessionProvider.tsx）、不变量检查（计数器、返回值、错误分类）（packages/web-shell/client/components/WorkspaceSessionProvider.tsx）、不变量检查（配置字段、提前返回）（packages/web-shell/client/components/WorkspaceSessionProvider.tsx）、不变量检查（状态、定时器、集合）（packages/webui/src/daemon/session/DaemonSessionProvider.tsx）、不变量检查（计数器、返回值、错误分类）（packages/webui/src/daemon/session/DaemonSessionProvider.tsx）、不变量检查（配置字段、提前返回）（packages/webui/src/daemon/session/DaemonSessionProvider.tsx）——它的 prompt 已构建，但没有任何 agent 有记录用它启动过。
未审查：反向审计——没有审计 agent 是用本 skill 构建的 prompt 启动的——负责搜寻评审其余部分遗漏问题的这道工序，即便运行过，也缺失了 brief 承载的方法。
未审查：验证——有验证 agent 运行并打开了自己的 brief，但没有 agent 是用 CLI 构建的 prompt 启动的——启动 prompt 是手写的，发布的发现不能算作经它验证。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.11)
| @@ -3089,6 +2586,20 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { | |||
| reconnectAttempt = 0; | |||
| skipMetadataRefresh = true; | |||
| continue; | |||
| } else if (isRestoreInProgressLoadError(error)) { | |||
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] The terminal restore_in_progress branch is untested — Failure scenario: a non-retryable 409 (or reason: 'awaiting_abandoned_cleanup', or autoReconnect off) lands on a switch and the terminal error rendering regresses undetected; grep shows only the two retryable cases are covered, and the 5000 ms default fallback in getRestoreInProgressRetryDelayMs plus the awaiting_abandoned_cleanup skip have no test.
中文说明
`restore_in_progress` 的终局分支（不可重试的 409 / `awaiting_abandoned_cleanup` / 缺少 `retryAfterSeconds` 时的 5000ms 默认值）没有测试覆盖：套件中仅有的两条 `restore_in_progress` 用例都是可重试且有延迟的。建议补充一条不可重试 409 的用例，验证终局错误态（`status: 'error'` 且不再重试）。— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.11)
Comment on lines
+11770
to
11773
| mockConnection.loadingTranscript = true; | ||
| rerender({ onSubmitBefore }); | ||
| }); | ||
| const allowBPrompt = vi.fn().mockResolvedValue(undefined); |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] Converted tests no longer verify write-blocking during a session load — Concrete cost: after the desiredSessionTargetPending prop was deleted, several tests toggle mockConnection.loadingTranscript with no assertion inside the loading window, so the toggles are inert scaffolding. Probe-verified: removing the toggle pair from this test leaves it green, meaning a regression in the write gate (Boolean(connection.loadingTranscript) in App) would not be caught here. Either assert inside the loading window (e.g. a submit is rejected) or drop the inert toggles.
中文说明
删除 `desiredSessionTargetPending` 后，多个转换后的测试只在相邻的 `act` 中切换 `mockConnection.loadingTranscript`，加载窗口内没有任何断言，属于无效脚手架。Probe 验证：删除本用例的 toggle 对后测试仍然通过，写闸门（App 中 `Boolean(connection.loadingTranscript)`）的回归不会被这里捕获。建议在加载窗口内补一条断言（如提交被拒绝），或删除无效的 toggle。— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.11)
Collaborator
Author
|
@qwen-code /takeover |
Collaborator
|
🔄 Takeover re-armed: the round counter starts a fresh window (previous rounds no longer count toward the cap); management continues. 中文说明🔄 已重新武装：轮次计数开启新窗口（此前轮次不再计入上限），托管继续。 |
wenshao
approved these changes
Aug 14, 2026
This was referenced Aug 14, 2026
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
This PR reverts the transactional cross-session switching introduced in #8882 and restores the loading-skeleton model for session switches: switching a session clears the current transcript, shows the loading skeleton, and waits for the load result. The transactional machinery — held transitions, staged handoffs, same-session event capture, watchdog deadlines and the controlled rebind path — is removed across the daemon session layer and the web-shell provider, along with the design docs and daemon integration tests that described it. Two follow-up fixes ride along: the dead write-gating prop left by the removal is dropped, and a failed switch no longer leaks its target workspace into the next workspace-less load.
Why it's needed
The transactional model kept the old attachment live and the UI pinned to the previous session while a switch prepared, trading a large state machine for behavior that was hard to reason about and to verify end to end. The loading-skeleton model is the simpler contract: clear the transcript, show the skeleton, wait for the load. Session restores keep their existing timeout and retry guarantees (restore_in_progress retries remain bounded by the daemon-advertised watchdog), and the restored behavior is covered by unit tests in place of the deleted integration tests.
Reviewer Test Plan
How to verify
Evidence (Before & After)
N/A — behavior-level revert; verified through unit suites: webui session tests (271) and web-shell tests (443) pass, and typecheck plus ESLint are clean on the changed packages.
Tested on
Environment (optional)
N/A — unit tests only.
Risk & Scope
Linked Issues
Reverts the transactional session switching from #8882.
中文说明
本 PR 回退 #8882 引入的事务式跨会话切换，恢复「切换会话 = 清空内容 → 展示骨架屏 → 等待加载结果」的模型。移除的内容包括跨会话切换状态机（暂存交接、同会话事件捕获、看门狗超时、受控重绑定）及其设计文档与 daemon 集成测试；同时附带两个修复：删除移除后遗留的死属性（写闸门改为仅由加载状态驱动），并阻止失败切换的目标工作区泄漏到下一次未指定工作区的加载。会话恢复的既有超时与重试保证保持不变（restore_in_progress 重试仍受 daemon 广告的看门狗约束），恢复后的行为由单元测试覆盖。
验证方式：webui 会话测试 271 个、web-shell 测试 443 个全部通过，改动包 typecheck 与 ESLint 干净（macOS 本地验证）。