fix(wework): restore workspace panes across task switches - #2428
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe desktop workbench now persists selected file targets and review state across runtime-task switches. Tests verify file-preview and review restoration, task isolation, and reload behavior. Documentation describes the restored Files and Review tab state. ChangesWorkspace restoration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant DesktopWorkbenchMain
participant WorkspaceTabs
participant ReviewPanel
User->>DesktopWorkbenchMain: switch runtime task
DesktopWorkbenchMain->>WorkspaceTabs: restore file target and review state
WorkspaceTabs->>ReviewPanel: reload previously loading diff
ReviewPanel-->>WorkspaceTabs: return review content
WorkspaceTabs-->>User: display restored file and review tabs
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/e2e/desktop/task-flow.e2e.mjs`:
- Around line 13946-13948: After the task switch performed by
control.command('click', `[data-testid="${secondTaskRowTestId}"]`), resolve the
currently selected workspace tab again instead of reusing
activeTaskWorkbenchSelector. Build a selector for the second task workbench from
that newly selected tab, then pass it to the snapshot command used to create
secondTaskWorkspaceSnapshot.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ce72d29f-cd76-4abe-a3b6-d70cfb7da9e1
📒 Files selected for processing (5)
docs/en/wework/workbench.mddocs/zh/wework/workbench.mdwework/e2e/desktop/task-flow.e2e.mjswework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchMain.tsx
| await control.command('click', `[data-testid="${secondTaskRowTestId}"]`) | ||
| const secondTaskWorkspaceSnapshot = JSON.parse( | ||
| await control.command('snapshot', ACTIVE_WORKBENCH_SELECTOR) | ||
| await control.command('snapshot', activeTaskWorkbenchSelector) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Snapshot the second task workbench after the task switch.
activeTaskWorkbenchSelector still identifies the first task tab. Line 13948 therefore snapshots the first task after Line 13946 selects the second task. The following assertions cannot detect resources that leak into the second task.
Resolve the selected workspace tab again after the switch. Build a second-task selector from that tab before taking the snapshot.
Proposed fix
await control.command('click', `[data-testid="${secondTaskRowTestId}"]`)
+await control.command('waitFor', activeWorkspaceTabSelector, {
+ timeoutMs: DEFAULT_STEP_TIMEOUT_MS,
+})
+const secondTaskWorkspaceTabTestId = await control.command(
+ 'getAttribute',
+ activeWorkspaceTabSelector,
+ { value: 'data-testid' }
+)
+const secondTaskWorkspaceTabId = secondTaskWorkspaceTabTestId.replace(
+ 'workspace-tab-select-',
+ ''
+)
+const secondTaskWorkbenchSelector =
+ `[data-testid="workspace-tab-content-${secondTaskWorkspaceTabId}"] ` +
+ '[data-testid="desktop-workbench-main"]'
const secondTaskWorkspaceSnapshot = JSON.parse(
- await control.command('snapshot', activeTaskWorkbenchSelector)
+ await control.command('snapshot', secondTaskWorkbenchSelector)
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await control.command('click', `[data-testid="${secondTaskRowTestId}"]`) | |
| const secondTaskWorkspaceSnapshot = JSON.parse( | |
| await control.command('snapshot', ACTIVE_WORKBENCH_SELECTOR) | |
| await control.command('snapshot', activeTaskWorkbenchSelector) | |
| await control.command('click', `[data-testid="${secondTaskRowTestId}"]`) | |
| await control.command('waitFor', activeWorkspaceTabSelector, { | |
| timeoutMs: DEFAULT_STEP_TIMEOUT_MS, | |
| }) | |
| const secondTaskWorkspaceTabTestId = await control.command( | |
| 'getAttribute', | |
| activeWorkspaceTabSelector, | |
| { value: 'data-testid' } | |
| ) | |
| const secondTaskWorkspaceTabId = secondTaskWorkspaceTabTestId.replace( | |
| 'workspace-tab-select-', | |
| '' | |
| ) | |
| const secondTaskWorkbenchSelector = | |
| `[data-testid="workspace-tab-content-${secondTaskWorkspaceTabId}"] ` + | |
| '[data-testid="desktop-workbench-main"]' | |
| const secondTaskWorkspaceSnapshot = JSON.parse( | |
| await control.command('snapshot', secondTaskWorkbenchSelector) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/e2e/desktop/task-flow.e2e.mjs` around lines 13946 - 13948, After the
task switch performed by control.command('click',
`[data-testid="${secondTaskRowTestId}"]`), resolve the currently selected
workspace tab again instead of reusing activeTaskWorkbenchSelector. Build a
selector for the second task workbench from that newly selected tab, then pass
it to the snapshot command used to create secondTaskWorkspaceSnapshot.
What changed
conversation-statedesktop checkpoint to verify file preview, review, browser, and terminal isolation/restoration across task switches.Root cause
The per-pane restorable workspace state stored only a derived file target key and did not store review state. An absolute file opened from an assistant message could therefore fall back to the task workspace when its pane was restored, while the review panel returned without its loaded diff or mode.
The existing desktop E2E checkpoint covered browser and terminal resources across task switches, but it closed the file preview before switching tasks and did not open or assert the review panel. The regression therefore remained outside its assertions.
Impact
Returning to a Wework runtime task now restores the same file preview and review content that was open before switching away, without leaking those resources into another task.
Validation
pnpm --filter wework exec vitest run src/components/layout/DesktopWorkbenchLayout.test.tsx -t "preserves (the open file|an absolute file opened from a message|the review) when switching runtime tasks"pnpm --filter wework exec tsc --noEmit --pretty falsepnpm --filter wework exec eslint src/components/layout/DesktopWorkbenchMain.tsx src/components/layout/DesktopWorkbenchLayout.test.tsx e2e/desktop/task-flow.e2e.mjspnpm --filter wework exec prettier --check src/components/layout/DesktopWorkbenchMain.tsx src/components/layout/DesktopWorkbenchLayout.test.tsx e2e/desktop/task-flow.e2e.mjspnpm --filter wework e2e:desktop -- --segment conversation-stateSummary by CodeRabbit
New Features
Bug Fixes
Documentation