fix(web-shell): persist collapsed session group sections across reload - #6878
Conversation
Store collapsed section ids in localStorage using the existing qwen-code-web-shell-* key namespace, and skip the first catalog sync auto-collapse so restored expand/collapse state survives remount. Fixes QwenLM#6870 Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks for the PR! (re-run) Template: all required headings present and match the template exactly — Problem: real bug with clear reproduction. Issue #6870 describes collapsed session groups resetting to expanded on every page reload. Desktop already persists this state ( Direction: aligned. CHANGELOG shows session groups landed in #6350 and custom colors in #6752 — persisting collapsed state is the natural next step. Local-only persistence, no daemon changes, consistent with the existing Size: not applicable — only Approach: minimal and well-targeted. The Moving on to code review. 🔍 中文说明感谢贡献！（重新审查） 模板：所有必需标题与模板完全匹配—— 问题：真实 bug，有清晰复现。Issue #6870 描述折叠的 session 分组在页面刷新后重新展开。Desktop 端已有相同的持久化能力（ 方向：对齐。CHANGELOG 显示 session groups 在 #6350 上线，自定义颜色在 #6752 加入——持久化折叠状态是自然的后续。仅本地持久化，无 daemon 改动，和已有的 规模：不适用——仅涉及 方案：最小化且目标明确。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code Review (re-run)Independent proposal: persist collapsed section IDs in No correctness issues, no security concerns, no regressions found. Prior critical findings — all resolved at this commit:
Code quality observations (non-blocking): The Test ResultsUnit tests (vitest)E2E test (Playwright)The test collapses a "Backend" group, verifies Build & Static Checks中文说明代码审查（重新审查）独立方案：在 无正确性问题、无安全隐患、无回归。 之前的 Critical 问题——在此 commit 均已解决：
代码质量： 测试结果单元测试 19/19 通过。Playwright e2e 测试通过——折叠 Backend 分组、验证 localStorage 写入、刷新页面、确认分组保持折叠。类型检查和 lint 全部通过。 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 — Clean across every stage; would merge without hesitation. This is a well-executed fix for a real UX gap that users hit immediately — collapsed session groups resetting on every page reload. The implementation is exactly what you'd want: 117 lines of clean localStorage helpers with proper owner-based write isolation, a dual-latch mechanism that correctly handles the non-obvious catalog timing edge case, and workspace-scoped persistence that doesn't clobber the primary sidebar's state. All three critical findings from the earlier review round are resolved at this commit: secondary-workspace persistence works through namespaced IDs, the demo-capture script (and its symlink race) is gone, and the first-sync latch is hardened against failed requests and late-arriving capabilities. The 9 collapse-persist unit tests cover primary persistence, workspace-scoped persistence, cross-workspace isolation, and the initial-catalog-latch behavior. The Playwright e2e test drives the full collapse → verify → reload → confirm cycle. 19 sidebar tests, 1 e2e test, typecheck, and lint all green. Approving. ✅ 中文说明信心度: 5/5 — 各阶段全部通过，毫不犹豫合并。 这是对真实 UX 缺陷的出色修复——折叠的 session 分组在页面刷新后重置是用户会立即注意到的问题。实现方式正是期望的：117 行干净的 localStorage helper，带有正确的基于 owner 的写入隔离，双 latch 机制正确处理了不直观的 catalog 时序边界情况，工作区作用域的持久化不会干扰主侧栏的状态。 之前审查轮次的三个 Critical 问题在此 commit 均已解决：次级工作区通过命名空间 ID 实现持久化，演示脚本（及其符号链接竞争）已移除，首同步 latch 对失败请求和延迟到达的 capabilities 进行了加固。9 个折叠持久化单测覆盖主要持久化、工作区作用域持久化、跨工作区隔离和初始 catalog latch 行为。Playwright e2e 测试驱动完整的折叠→验证→刷新→确认循环。19 个 sidebar 测试、1 个 e2e 测试、类型检查、lint 全部通过。 批准合并。✅ — Qwen Code · qwen3.7-max Reviewed at |
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. ✅
Crop to the sidebar, caption the four beats (expand → collapse → reload → still collapsed), and keep Pinned out of the organized session list mock so the Backend collapse is obvious. Co-authored-by: Cursor <cursoragent@cursor.com>
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: chunk 1, chunk 2, chunk 3 — no agent reported covering these; nobody read them.
— qwen3.7-max via Qwen Code /review
| const COLLAPSED_SESSION_SECTIONS_STORAGE_KEY = | ||
| 'qwen-code-web-shell-collapsed-session-groups'; |
There was a problem hiding this comment.
[Suggestion] The storage key 'qwen-code-web-shell-collapsed-session-groups' is independently hardcoded in three files (this test, the E2E spec, and WebShellSidebar.tsx) instead of being exported/imported from the source module.
Concrete cost: If someone renames the key in WebShellSidebar.tsx (e.g., to add a version suffix for a schema migration), both test files continue passing against a stale key — tests pass while production silently loses all persisted collapse state for every user.
Consider exporting the constant from WebShellSidebar.tsx (or a shared constants module) and importing it in the unit test. The E2E test is harder to couple due to different build contexts, so duplication there is more acceptable.
— qwen3.7-max via Qwen Code /review
| // First catalog sync: register ids only. Restored localStorage (or the | ||
| // empty default) owns expand/collapse; auto-collapse would otherwise wipe | ||
| // expanded sections on every remount because the known set resets. | ||
| const isInitialCatalog = knownSessionSectionIdsRef.current.size === 0; |
There was a problem hiding this comment.
[Suggestion] The isInitialCatalog heuristic (knownSessionSectionIdsRef.current.size === 0) silently depends on the invariant that nothing else pre-populates knownSessionSectionIdsRef before this effect fires. The comment explains why the initial-catalog skip exists, but not what would break the heuristic.
Failure scenario: A future maintainer adds an initializer that seeds knownSessionSectionIdsRef (e.g., to fix a flicker bug). The size === 0 check becomes false on the first real sync, so all restored-from-localStorage expanded sections get auto-collapsed on every mount — silently, with no error.
Consider strengthening the comment to state the invariant explicitly, or using a dedicated useRef<boolean>(true) flag that is flipped after the first run, decoupling the heuristic from the ref's size.
— qwen3.7-max via Qwen Code /review
| const parsed: unknown = JSON.parse(raw); | ||
| if (!Array.isArray(parsed)) return new Set(); |
There was a problem hiding this comment.
[Suggestion] No unit test verifies the fallback behavior when localStorage contains corrupt or unexpected data (invalid JSON, non-array values, non-string items). The production code handles all three gracefully, but a regression that removes the guard would go undetected.
Failure scenario: Someone removes the Array.isArray check during a cleanup, and a user whose localStorage contains "not-an-array" from a prior bug gets a crash instead of a graceful fallback.
| const parsed: unknown = JSON.parse(raw); | |
| if (!Array.isArray(parsed)) return new Set(); | |
| // Consider adding a test: | |
| it('tolerates corrupt localStorage data', async () => { | |
| window.localStorage.setItem( | |
| COLLAPSED_SESSION_SECTIONS_STORAGE_KEY, | |
| 'not valid json', | |
| ); | |
| renderSidebar(); | |
| await flushSidebar(); | |
| expect(groupHeader('Backend').getAttribute('aria-expanded')).toBe('true'); | |
| }); |
— qwen3.7-max via Qwen Code /review
| } catch { | ||
| // localStorage can be unavailable in private or embedded contexts. | ||
| } |
There was a problem hiding this comment.
[Suggestion] No test verifies the component remains functional when localStorage.setItem throws (e.g., private browsing, embedded iframe with storage disabled, quota exceeded).
Failure scenario: A regression removes the try/catch, and the useEffect that calls writeCollapsedSessionSectionIds crashes on toggle, breaking the component.
| } catch { | |
| // localStorage can be unavailable in private or embedded contexts. | |
| } | |
| // Consider adding: | |
| it('does not crash when localStorage.setItem throws', async () => { | |
| vi.spyOn(Storage.prototype, 'setItem').mockImplementation(() => { | |
| throw new Error('quota exceeded'); | |
| }); | |
| renderSidebar(); | |
| await flushSidebar(); | |
| act(() => click(groupHeader('Backend'))); | |
| await flushSidebar(); | |
| expect(groupHeader('Backend').getAttribute('aria-expanded')).toBe('false'); | |
| }); |
— qwen3.7-max via Qwen Code /review
| if (isInitialCatalog) return; | ||
| // Brand-new sections that appear mid-session still start collapsed. |
There was a problem hiding this comment.
[Suggestion] The non-initial-catalog path (auto-collapse of brand-new sections appearing mid-session) is not tested. The isInitialCatalog skip is implicitly covered, but the opposite branch — isInitialCatalog === false — has no test.
Failure scenario: A new group is created server-side while the sidebar is mounted. The auto-collapse code path could regress without detection.
Consider adding a test that renders the sidebar, flushes, then updates the mock to include a new group and verifies the new section starts collapsed.
— qwen3.7-max via Qwen Code /review
Export the storage key for unit tests, use an explicit first-catalog latch instead of size===0, and cover corrupt/disabled storage plus mid-session auto-collapse of newly appeared sections. Co-authored-by: Cursor <cursoragent@cursor.com>
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: coverage — could not read the agents' transcripts (the CLI did not export QWEN_CODE_PROJECT_DIR / QWEN_CODE_SESSION_ID, so this run cannot find the harness's record of what its agents did), so this run cannot show that any of the diff was read.
[Critical] Secondary-workspace session groups still use the memory-only collapsedGroupIds Set in WorkspaceSection, which is the exact root cause identified by issue #6870. Failure scenario: collapse a named or Ungrouped section under a non-primary workspace and reload the page; WorkspaceSection remounts with an empty Set and reopens it. Share the persisted collapse-state mechanism with this render path, using collision-safe workspace/section IDs as needed.
— Codex $qreview via Qwen Code /review
| const isInitialCatalog = awaitingInitialSessionCatalogRef.current; | ||
| awaitingInitialSessionCatalogRef.current = false; | ||
| for (const id of unseenIds) knownSessionSectionIdsRef.current.add(id); | ||
| if (isInitialCatalog) return; |
There was a problem hiding this comment.
[Critical] This one-shot latch flips on the first non-empty derived catalog even though groups and sessions hydrate independently. Failure scenario: if groups arrive first, later initial recent/color:* sections are treated as brand-new and auto-collapsed; if the initial catalog is empty, this effect returns before clearing the latch and the first genuinely new section later remains expanded. Gate the initial sync on explicit settlement/readiness of every catalog source, including empty responses.
— Codex $qreview via Qwen Code /review
| const sessions = | ||
| group === 'pinned' | ||
| ? scenario.sessions.filter((session) => Boolean(session.isPinned)) | ||
| : scenario.sessions.filter((session) => !session.isPinned); |
There was a problem hiding this comment.
[Suggestion] The mock treats every query other than group=pinned as non-pinned, but production explicitly returns all sessions for group=all. Failure scenario: the new browser regression can stay green while the real sidebar loses or duplicates pinned sessions between its All and Pinned requests. Mirror the production query modes explicitly and leave all unfiltered.
— Codex $qreview via Qwen Code /review
| npx playwright test \ | ||
| --config playwright.config.ts \ | ||
| client/e2e/web-shell.collapsed-groups-persist.spec.ts \ | ||
| > /tmp/qwen-6870-demo.log 2>&1 |
There was a problem hiding this comment.
[Critical] This predictable shared /tmp path is opened with truncating redirection. Failure scenario: another local user pre-creates /tmp/qwen-6870-demo.log as a symlink, then running the capture helper follows it and truncates any target writable by the developer account. Create the log with mktemp in a private temporary directory and remove it with a trap.
— Codex $qreview via Qwen Code /review
| expect( | ||
| window.localStorage.getItem(COLLAPSED_SESSION_SECTIONS_STORAGE_KEY), | ||
| ).toBe(JSON.stringify(['group:group-1'])); |
There was a problem hiding this comment.
[Suggestion] Every persistence assertion covers only one named group:* ID. Concrete failure scenario: a refactor that retains only the last toggled ID would keep all current unit and browser tests green, yet collapsing two groups would make the first reopen after reload; recent and color:* restoration are likewise untested. Add a remount case that preserves multiple IDs and at least one non-group section kind, including removal of one ID without losing the other.
— Codex $qreview via Qwen Code /review
| FRAMES_DIR="$ROOT_DIR/docs/assets/web-shell-collapsed-session-groups-frames" | ||
| GIF_PATH="$ROOT_DIR/docs/assets/web-shell-collapsed-session-groups.gif" |
There was a problem hiding this comment.
[Suggestion] A successful capture leaves its generated PNG frame directory under docs/assets, and that path is not ignored. Concrete cost: every run dirties the checkout and a broad stage can accidentally commit all intermediate frames. Use a temporary frame directory with trap-based cleanup, or explicitly clean/ignore the generated directory.
— Codex $qreview via Qwen Code /review
| return page.locator('aside[aria-label="Workspace sidebar"]'); | ||
| } | ||
|
|
||
| test('persists collapsed session groups across reload @demo', async ({ |
There was a problem hiding this comment.
[Suggestion] This regression is tagged only @demo, while pull-request CI runs the Web Shell suite with --grep @smoke; the full browser job is schedule/manual-only. Failure scenario: collapse persistence breaks on a PR and this test provides no pre-merge signal. Include this stable path in the smoke subset (for example, add @smoke) or run the spec from another PR job.
— Codex $qreview via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Unresolved, please confirm: [Critical] WebShellSidebar.tsx:1986 — latch timing depends on whether daemon delivers groups and sessions atomically; cannot determine from static analysis
Not reviewed: chunk 1, chunk 2, chunk 3 — no agent reported covering these; nobody read them.
[Critical] Existing Critical still stands (capture-collapsed-groups-demo.sh:17): predictable shared /tmp path opened with truncating redirection — symlink race (CWE-377) allows a local user to truncate arbitrary files writable by the developer.
— qwen3.7-max via Qwen Code /review
| import sys | ||
| from pathlib import Path | ||
|
|
||
| from PIL import Image, ImageDraw, ImageFont |
There was a problem hiding this comment.
[Suggestion] PIL (Pillow) is imported but not declared in any requirements.txt or dependency manifest. Concrete cost: A developer running capture-collapsed-groups-demo.sh on a clean machine gets ModuleNotFoundError: No module named 'PIL' with no indication of what to install.
| from PIL import Image, ImageDraw, ImageFont | |
| # Requires: pip install Pillow | |
| from PIL import Image, ImageDraw, ImageFont |
— qwen3.7-max via Qwen Code /review
| npx playwright test \ | ||
| --config playwright.config.ts \ | ||
| client/e2e/web-shell.collapsed-groups-persist.spec.ts \ | ||
| > /tmp/qwen-6870-demo.log 2>&1 |
There was a problem hiding this comment.
[Suggestion] Log path references issue 6870 instead of this PR (6878). Concrete cost: A future maintainer grepping for related issues will be confused by the mismatch; the path is also not cleaned up after the script finishes.
| > /tmp/qwen-6870-demo.log 2>&1 | |
| > /tmp/collapsed-groups-demo.log 2>&1 |
— qwen3.7-max via Qwen Code /review
Wait for groups+sessions catalog settlement before the initial latch, persist secondary-workspace collapse via shared namespaced localStorage, and keep primary/workspace writers from clobbering each other. Co-authored-by: Cursor <cursoragent@cursor.com>
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.
— qwen3.7-max via Qwen Code /review
| useEffect(() => { | ||
| setCollapsedGroupIds(readWorkspaceCollapsedGroupIds(workspace.id)); | ||
| }, [workspace.id]); |
There was a problem hiding this comment.
[Suggestion] The useState initializer already calls readWorkspaceCollapsedGroupIds(workspace.id) to seed the correct data on first render. This useEffect on [workspace.id] then calls the same function again, creating a new Set reference (different object identity). React sees a state change and schedules an unnecessary re-render. The subsequent write effect then fires and writes the same data back to localStorage. On every mount, this produces one redundant read, one redundant render, and one redundant localStorage.setItem.
Concrete cost: when a user has 5-10 workspace folders visible, a page load triggers 5-10 unnecessary localStorage read-parse-filter-stringify-write cycles during mount.
| useEffect(() => { | |
| setCollapsedGroupIds(readWorkspaceCollapsedGroupIds(workspace.id)); | |
| }, [workspace.id]); | |
| // Skip reload on mount — the useState initializer already hydrated from | |
| // localStorage. Only reload when workspace.id changes after the first render. | |
| const isFirstMount = useRef(true); | |
| useEffect(() => { | |
| if (isFirstMount.current) { | |
| isFirstMount.current = false; | |
| return; | |
| } | |
| setCollapsedGroupIds(readWorkspaceCollapsedGroupIds(workspace.id)); | |
| }, [workspace.id]); |
— qwen3.7-max via Qwen Code /review
| } catch { | ||
| // localStorage can be unavailable in private or embedded contexts. | ||
| } |
There was a problem hiding this comment.
[Suggestion] When localStorage.setItem throws (quota exceeded, SecurityError), the error is silently swallowed. The React state has already been updated, so the sidebar shows the group as collapsed. On next reload, the group reverts to expanded with zero console warnings — an investigator tracing through the latch logic and initialization would have no signal pointing at "the write silently failed."
| } catch { | |
| // localStorage can be unavailable in private or embedded contexts. | |
| } | |
| } catch (err) { | |
| // localStorage can be unavailable in private or embedded contexts. | |
| console.warn('Failed to persist collapsed session state:', err); | |
| } |
— qwen3.7-max via Qwen Code /review
| import { | ||
| COLLAPSED_SESSION_SECTIONS_STORAGE_KEY, | ||
| readWorkspaceCollapsedGroupIds, | ||
| replaceOwnedCollapsedSessionSectionIds, | ||
| writeWorkspaceCollapsedGroupIds, | ||
| } from './collapsedSessionSections'; |
There was a problem hiding this comment.
[Suggestion] This test file directly covers only replaceOwnedCollapsedSessionSectionIds and readWorkspaceCollapsedGroupIds/writeWorkspaceCollapsedGroupIds. Six other exports lack direct unit tests, including readCollapsedSessionSectionIds which has multiple defensive branches (corrupt JSON, non-array payloads, non-string array items) that the sidebar integration tests exercise only indirectly.
Concrete cost: a regression in readCollapsedSessionSectionIds's filtering (e.g., accepting non-string items) would not be caught at the unit level. The sidebar integration tests would still pass because they only write well-formed data.
— qwen3.7-max via Qwen Code /review
| const [collapsedGroupIds, setCollapsedGroupIds] = useState<Set<string>>(() => | ||
| readWorkspaceCollapsedGroupIds(workspace.id), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] Unlike the primary sidebar (which has a dedicated 435-line test file WebShellSidebar.collapse-persist.test.tsx), WorkspaceSection has zero tests covering the new localStorage persistence effects (the reload-on-workspace.id-change effect and the write-on-collapsedGroupIds-change effect).
Concrete cost: a refactor could write collapse state under the wrong workspace key after a workspace switch, or the reload effect could race with the write effect — and no test would catch it.
— qwen3.7-max via Qwen Code /review
…test The committed GIF, capture script, and frame-assembly helper only served the PR description's embedded image and were referenced by nothing else in the repo; the CAPTURE_DEMO branches in the e2e spec were pure screenshot staging with no assertions. The remaining spec still covers every acceptance criterion of QwenLM#6870 and keeps its @smoke tag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014KuV2hx2ZqEqdRwsJz4KJH
…ssfully Two paths could consume the first-sync latch against a partial catalog and then auto-collapse (and persist over) the user's restored expansions: a failed initial sessions/groups request counted as settled, and a mid-session organization_enabled flip let the auto-collapse effect run one commit before the groups gate closed. Errors no longer settle either readiness gate, and the gate now closes during the flip render itself. Also drop the WorkspaceSection reload effect and exhaustive-deps suppression that defended a workspace.id change which cannot happen (the render site keys the component by workspace id), and import the storage key in tests from collapsedSessionSections directly instead of re-exporting it through WebShellSidebar. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014KuV2hx2ZqEqdRwsJz4KJH
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. Unresolved, please confirm: [Critical] /tmp symlink race in capture-collapsed-groups-demo.sh (comment 3578704643) — file is not in this PR's diff, cannot verify Not reviewed: chunk 1, chunk 2, chunk 3, chunk 4 — no agent reported covering these; nobody read them.
— qwen3.7-max via Qwen Code /review
| useEffect(() => { | ||
| writeWorkspaceCollapsedGroupIds(workspace.id, collapsedGroupIds); | ||
| }, [collapsedGroupIds, workspace.id]); |
There was a problem hiding this comment.
[Suggestion] WorkspaceSection's localStorage persistence useEffect has no dedicated test. The primary sidebar has a 437-line collapse-persist test, but this per-workspace write path — which uses workspace-namespaced IDs (ws:<id>|group:<gid>) — is only covered by the helper-level round-trip test in collapsedSessionSections.test.ts, not by a component test.
Failure scenario: A bug in the WorkspaceSection component's useEffect wiring (e.g., wrong workspace.id passed, or the effect not firing on toggle) would leave secondary-workspace collapse state unpersisted. The helper test passes because it tests the function in isolation, but the component integration is unverified.
| useEffect(() => { | |
| writeWorkspaceCollapsedGroupIds(workspace.id, collapsedGroupIds); | |
| }, [collapsedGroupIds, workspace.id]); | |
| // Add a test in WorkspaceSection.test.tsx (or extend collapse-persist.test.tsx): | |
| // - Render WorkspaceSection with pre-seeded localStorage entry `ws:<workspaceId>|group:<groupId>` | |
| // - Toggle collapse, verify the round-trip through readWorkspaceCollapsedGroupIds |
| const [prevOrganizationEnabled, setPrevOrganizationEnabled] = | ||
| useState(organizationEnabled); | ||
| if (prevOrganizationEnabled !== organizationEnabled) { | ||
| setPrevOrganizationEnabled(organizationEnabled); | ||
| setGroupsCatalogReady(!organizationEnabled); | ||
| } |
There was a problem hiding this comment.
[Suggestion] The organizationEnabled transition logic (false→true mid-session) is not tested. All 9 sidebar integration tests initialize with organizationEnabled = true and never change it.
Failure scenario: Capabilities arrive after the sidebar's first render (common when /workspace capabilities land after the flat sessions request). organizationEnabled flips from false to true. The render-time state sync resets groupsCatalogReady to false, then the groups catalog reload sets it back to true. If there's a race between the reset and the auto-collapse effect consuming the first-sync latch against a stale pre-organized catalog, restored collapse state could be overwritten. No test exercises this transition.
| const [prevOrganizationEnabled, setPrevOrganizationEnabled] = | |
| useState(organizationEnabled); | |
| if (prevOrganizationEnabled !== organizationEnabled) { | |
| setPrevOrganizationEnabled(organizationEnabled); | |
| setGroupsCatalogReady(!organizationEnabled); | |
| } | |
| // Add a test that: | |
| // 1. Starts with workspace.capabilities = undefined (organizationEnabled=false) | |
| // 2. Renders the sidebar | |
| // 3. Sets workspace.capabilities = organizationCapabilities (flips to true) | |
| // 4. Re-renders and verifies pre-existing localStorage collapse state is still respected |
✅ Local verification — collapsed session groups persist across reloadBuilt and ran the real tests locally in an isolated worktree at head What I ran
Expand → collapse “Backend” → hard reload → still collapsed ✅ → re‑expand + reload → stays expanded ✅. Ungrouped is untouched throughout. Earlier Critical findings — all resolved
Notes
🇨🇳 中文说明（点击展开）✅ 本地验证 — 收合的会话分组在刷新后保持收合在隔离 worktree（head 运行了什么
（截图见上方英文部分：storyboard 展示 展开 → 收合 “Backend” → 硬刷新 → 仍收合 ✅ → 再展开+刷新 → 仍展开 ✅；Ungrouped 全程不受影响。） 之前的 Critical 问题 —— 均已解决
备注
|
|
@qwen-code /triage |
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. ✅
4 participants
What this PR does
Persist collapsed session-organization section ids (named groups / Ungrouped / color buckets) in
localStorageso a full page reload no longer re-expands every section. Key:qwen-code-web-shell-collapsed-session-groups, sameqwen-code-web-shell-*namespace as sidebar width / sidebar collapsed / theme.Restore on mount, write back on toggle. Skip auto-collapse on the first catalog sync so restored expand/collapse is not wiped on remount; brand-new sections that appear later in the same session still start collapsed.
Why it's needed
Issue #6870: collapsing named groups (or Ungrouped) works for the current page, but refresh restores every group to expanded. Desktop already persists
(craft-)collapsed-session-groups; Web Shell was missing the same preference.Reviewer Test Plan
How to verify
session_organizationenabled and at least one named group.Automated:
cd packages/web-shell && npm test -- client/components/sidebar/WebShellSidebar.collapse-persist.test.tsxcd packages/web-shell && npx playwright test client/e2e/web-shell.collapsed-groups-persist.spec.tsEvidence (Before & After)
Before: collapse Backend → reload → Backend reopens.
After: collapse Backend → reload → Backend stays collapsed.
Storyboard captioned in the GIF: expand → collapse Backend → reload → still collapsed.
Tested on
Environment (optional)
Local:
packages/web-shellvitest + Playwright againstnpm run devwith mock daemon routes.Risk & Scope
Linked Issues
Fixes #6870
中文说明
这个 PR 做什么
把会话组织分区（命名分组 / Ungrouped / 颜色桶）的收合状态写进
localStorage，全页刷新后不再全部展开。key：qwen-code-web-shell-collapsed-session-groups，与侧栏宽度等现有qwen-code-web-shell-*命名空间一致。挂载时恢复，toggle 时写回。首次 catalog 同步跳过自动收合，避免 remount 冲掉已恢复状态；同一次会话里中途新出现的 section 仍默认收合。
为什么需要
Issue #6870：收合命名分组在当前页有效，刷新后全部重新展开。Desktop 已有同类持久化；Web Shell 补齐。
Reviewer 验证
session_organization，收合命名分组后硬刷新。证据见上方 GIF。风险：仅本地偏好；不涉及 daemon。