-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(web-shell): persist the split view across refresh, per tab #7136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
137 changes: 137 additions & 0 deletions
137
packages/web-shell/client/e2e/web-shell.split-persist.spec.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| import { expect, test, type Page, type TestInfo } from '@playwright/test'; | ||
| import { | ||
| createWebShellDaemonScenario, | ||
| installMockDaemon, | ||
| type MockDaemonController, | ||
| type WebShellDaemonScenario, | ||
| } from './utils/mockDaemon'; | ||
|
|
||
| const WORKSPACE_CWD = '/tmp/qwen-web-shell-e2e'; | ||
| const MAIN_SESSION = 'split-main-session'; | ||
| const SESSION_A = 'split-session-a'; | ||
| const SESSION_B = 'split-session-b'; | ||
| const STORAGE_KEY = 'qwen-webshell-split-sessions'; | ||
|
|
||
| function createSplitScenario(): WebShellDaemonScenario { | ||
| const at = '2026-07-03T00:00:00.000Z'; | ||
| return createWebShellDaemonScenario({ | ||
| workspaceCwd: WORKSPACE_CWD, | ||
| sessionId: MAIN_SESSION, | ||
| sessions: [ | ||
| { | ||
| sessionId: MAIN_SESSION, | ||
| workspaceCwd: WORKSPACE_CWD, | ||
| createdAt: at, | ||
| updatedAt: at, | ||
| displayName: 'Main Session', | ||
| clientCount: 1, | ||
| hasActivePrompt: false, | ||
| }, | ||
| { | ||
| sessionId: SESSION_A, | ||
| workspaceCwd: WORKSPACE_CWD, | ||
| createdAt: at, | ||
| updatedAt: at, | ||
| displayName: 'Session A', | ||
| clientCount: 0, | ||
| hasActivePrompt: false, | ||
| }, | ||
| { | ||
| sessionId: SESSION_B, | ||
| workspaceCwd: WORKSPACE_CWD, | ||
| createdAt: at, | ||
| updatedAt: at, | ||
| displayName: 'Session B', | ||
| clientCount: 0, | ||
| hasActivePrompt: false, | ||
| }, | ||
| ], | ||
| }); | ||
| } | ||
|
|
||
| async function installScenario( | ||
| page: Page, | ||
| scenario: WebShellDaemonScenario, | ||
| testInfo: TestInfo, | ||
| ): Promise<MockDaemonController> { | ||
| return installMockDaemon(page, scenario, { | ||
| baseURL: String(testInfo.project.use.baseURL), | ||
| }); | ||
| } | ||
|
|
||
| test('restores the split across a reload and isolates it per tab @smoke', async ({ | ||
| page, | ||
| context, | ||
| }, testInfo) => { | ||
| // Wide viewport so the split stays unfolded (it folds below the large-screen | ||
| // breakpoint). | ||
| await page.setViewportSize({ width: 1440, height: 900 }); | ||
|
|
||
| const scenario = createSplitScenario(); | ||
| await installScenario(page, scenario, testInfo); | ||
|
|
||
| // Open the split via the deep link — the exact URL "open in new tab" produces | ||
| // (path reset to `/`, sessions in `?split=`). | ||
| await page.goto(`/?split=${SESSION_A},${SESSION_B}`); | ||
|
|
||
| const split = page.locator('[data-testid="split-view"]'); | ||
| await expect(split).toBeVisible(); | ||
| await expect(page.locator('[data-testid="chat-pane"]')).toHaveCount(2); | ||
|
|
||
| // The session set lands in per-tab storage… | ||
| await expect | ||
| .poll(async () => | ||
| page.evaluate((key) => window.sessionStorage.getItem(key), STORAGE_KEY), | ||
| ) | ||
| .toBe(JSON.stringify([SESSION_A, SESSION_B])); | ||
|
|
||
| // …and the one-shot deep-link param is consumed so a bookmark isn't sticky. | ||
| await expect.poll(async () => new URL(page.url()).search).toBe(''); | ||
|
|
||
| // Reload (URL is now bare `/`): the split comes back from storage. | ||
| await page.reload(); | ||
| await expect(page.locator('[data-testid="split-view"]')).toBeVisible(); | ||
| await expect(page.locator('[data-testid="chat-pane"]')).toHaveCount(2); | ||
|
|
||
| // A brand-new tab has its own sessionStorage, so it must NOT inherit tab 1's | ||
| // split. (If persistence used localStorage, this tab would wrongly reopen it.) | ||
| const page2 = await context.newPage(); | ||
| await page2.setViewportSize({ width: 1440, height: 900 }); | ||
| await installScenario(page2, scenario, testInfo); | ||
| await page2.goto(`/session/${MAIN_SESSION}`); | ||
| await expect(page2.locator('[data-web-shell-root]')).toBeVisible(); | ||
| await expect(page2.locator('[data-testid="split-view"]')).toHaveCount(0); | ||
| }); | ||
|
|
||
| test('leaving the split clears storage so a refresh does not restore it', async ({ | ||
| page, | ||
| }, testInfo) => { | ||
| await page.setViewportSize({ width: 1440, height: 900 }); | ||
| const scenario = createSplitScenario(); | ||
| await installScenario(page, scenario, testInfo); | ||
|
|
||
| await page.goto(`/?split=${SESSION_A},${SESSION_B}`); | ||
| await expect(page.locator('[data-testid="split-view"]')).toBeVisible(); | ||
| await expect | ||
| .poll(async () => | ||
| page.evaluate((key) => window.sessionStorage.getItem(key), STORAGE_KEY), | ||
| ) | ||
| .toBe(JSON.stringify([SESSION_A, SESSION_B])); | ||
|
|
||
| // Leave via the split's back button. | ||
| await page | ||
| .locator('[data-testid="split-view"] header button') | ||
| .first() | ||
| .click(); | ||
| await expect(page.locator('[data-testid="split-view"]')).toHaveCount(0); | ||
| await expect | ||
| .poll(async () => | ||
| page.evaluate((key) => window.sessionStorage.getItem(key), STORAGE_KEY), | ||
| ) | ||
| .toBeNull(); | ||
|
|
||
| // A refresh now lands on the normal view, not the split. | ||
| await page.reload(); | ||
| await expect(page.locator('[data-web-shell-root]')).toBeVisible(); | ||
| await expect(page.locator('[data-testid="split-view"]')).toHaveCount(0); | ||
| }); |
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Critical] Navigation away from the split view via sidebar session click, new-session creation, or approval-notice button calls
setMainView('chat')directly —clearSplitSessions()is never invoked. Storage retains["A","B"], so a page refresh restores the split the user intentionally navigated away from.Failure scenario: user in split with sessions A,B → clicks session C in sidebar →
setMainView('chat')at line ~6081 bypasseshandleSplitExit→ refresh →loadSplitSessions()returns["A","B"]→ user is thrown back into the abandoned split.Multiple
setMainView('chat')call sites (~10) share this gap. Consider extending the mirror effect to clear storage whenmainViewtransitions away from'split'outside a shrink-fold, or routing all intentional departures throughhandleSplitExit.Additionally, no App-level unit test asserts that this save effect actually writes to sessionStorage — the write path is only covered by the E2E Playwright spec.
— qwen3.7-max via Qwen Code /review